[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 1/3] edu: mmio: set 'max_access_size' to 8
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [Qemu-devel] [PATCH v2 1/3] edu: mmio: set 'max_access_size' to 8 |
Date: |
Mon, 22 Apr 2019 18:27:58 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 |
On 4/22/19 3:17 AM, Li Qiang wrote:
>
>
> Philippe Mathieu-Daudé <address@hidden <mailto:address@hidden>> 于
> 2019年4月21日周日 下午6:28写道:
>
> Hi Li,
>
> The patch title is not very descriptive, maybe "allow 64-bit access"
>
>
> On 4/20/19 6:14 PM, Li Qiang wrote:
> > The edu spec said, the MMIO area can be accessed by 8 bytes.
>
> or 64-bit...
>
> > However currently the 'max_access_size' is not so the MMIO
> > access dispatch can only access 4 bytes one time. This patch
>
> 32-bit
>
> > fixes this to respect the spec.
> >
> > Notice: here the 'min_access_size' is not a must, I set this
> > for completement.
>
> Which one? valid/impl? I think you can drop this comment from the commit
> description.
>
>
> Both needed. from memory_access_size, if we has no valid.max_access_size,
> this function will set it to 4.
>
> static int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr)
> {
> unsigned access_size_max = mr->ops->valid.max_access_size;
>
> /* Regions are assumed to support 1-4 byte accesses unless
> otherwise specified. */
> if (access_size_max == 0) {
> access_size_max = 4;
> }
> ...
> }
>
> From access_with_adjusted_size, if we has no impl.max_access_size,
> this function will set it to 4.
>
> ps: I will appreciate if anyone can explain what's the meaning of valid
> and impl's min/max_access_size
> and how it affects the behavior.
"valid" describes the valid access from the bus to the device.
Indeed in the EDU case those are 4 and 8.
"impl" describes the accesses implemented by the QEMU device model.
The developper who writes the device is free to choose the accesses he
will model.
If valid/impl accesses don't match, the function
access_with_adjusted_size() from memory.c will adjust the bus access to
the device implementation.
For example, if the device only implements 1 and 2 bytes accesses, with
a 1-4 valid access, if the CPU executes a 32-bit access, this function
will do 2x 16-bit access to the device (incrementing the address by 2)
and returns a 32-bit result.
Similarly, if the CPU does a 8-bit access on a 32-bit impl device,
access_with_adjusted_size() will execute a single 32-bit access to the
device, then mask/shift the returned value and returns a 8-bit result to
the caller.
> Thanks,
> Li Qiang
>
> >
> > Signed-off-by: Li Qiang <address@hidden <mailto:address@hidden>>
> > ---
> > hw/misc/edu.c | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/hw/misc/edu.c b/hw/misc/edu.c
> > index 91af452c9e..65fc32b928 100644
> > --- a/hw/misc/edu.c
> > +++ b/hw/misc/edu.c
> > @@ -289,6 +289,15 @@ static const MemoryRegionOps edu_mmio_ops = {
> > .read = edu_mmio_read,
> > .write = edu_mmio_write,
> > .endianness = DEVICE_NATIVE_ENDIAN,
> > + .valid = {
> > + .min_access_size = 4,
>
> Per the spec, this is correct.
>
> > + .max_access_size = 8,
>
> Correct.
>
> > + },
> > + .impl = {
> > + .min_access_size = 4,
>
> OK.
>
> > + .max_access_size = 8,
>
> Correct.
>
> > + },
> > +
> > };
> >
> > /*
> >
>
> With title/description updated:
> Reviewed-by: Philippe Mathieu-Daudé <address@hidden
> <mailto:address@hidden>>
>
[Qemu-devel] [PATCH v2 2/3] edu: mmio: allow mmio read dispatch accept 8 bytes, Li Qiang, 2019/04/20
[Qemu-devel] [PATCH v2 3/3] edu: uses uint64_t in dma operation, Li Qiang, 2019/04/20