Re: RFC: Memory region accesses where .valid.min_access_size < .impl.min_access_size

[Philippe Mathieu-Daudé]

On 5/13/21 3:00 PM, Jonathan Cameron wrote:
> On Thu, 13 May 2021 14:36:27 +0200
> Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> 
>> On 5/13/21 2:23 PM, Peter Maydell wrote:
>>> On Thu, 13 May 2021 at 12:49, Jonathan Cameron
>>> <Jonathan.Cameron@huawei.com> wrote:  
>>>> My initial suggestion was to fix this by adding the relatively
>>>> simple code needed in the driver to implement byte read / write,
>>>> but Ben pointed at the QEMU docs - docs/devel/memory.rst which
>>>> says
>>>> "
>>>> .impl.min_access_size, .impl.max_access_size define the access sizes
>>>>    (in bytes) supported by the *implementation*; other access sizes will be
>>>>    emulated using the ones available. For example a 4-byte write will be
>>>>    emulated using four 1-byte writes, if .impl.max_access_size = 1.
>>>> "
>>>>
>>>> This isn't true when we have the situation where
>>>> .valid.min_access_size < .imp.min_access_size
>>>>
>>>> So change the docs or try to make this work?  
>>
>> See also this patch from Francisco:
>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg636935.html
>>
>> And full unaligned access support from Andrew:
>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg461247.html
> 
> Thanks - that's very similar to what I was carrying, but I think it
> only covers the read case.  That's backed up by the comment:
> /* XXX: Can't do this hack for writes */

You might use the "MMIO test device" to write your tests, see:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg730716.html
https://www.mail-archive.com/qemu-devel@nongnu.org/msg730720.html

I have a branch tagged v2, need to rebase it and post...

>>> I don't (yet) have a view on what the in-principle right thing
>>> should be, but in practice: how many devices do we have which
>>> set .valid.min_access_size < .imp.min_access_size ? If we want
>>> to change the semantics we'd need to look at those to see if they
>>> need to be adjusted (or if they're just currently buggy and would
>>> be fixed by the change).
> 
> I'm only aware of this one CXL emulated device (+ the proposed code in
> the ADC in the above patch set).  For the CXL device, working around
> this limitation is straight forward if that's the right option
> + updating the docs to slightly reduced chances of this being hit in
> the future.
> 
> Thanks,
> 
> Jonathan