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

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Fri, 14 May 2021 11:35:57 +0930
"Andrew Jeffery" <andrew@aj.id.au> wrote:

> On Thu, 13 May 2021, at 22:30, 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  
> 
> Much better to use lore.kernel.org:
> 
> https://lore.kernel.org/qemu-devel/20170630030058.28943-1-andrew@aj.id.au/
> 
> because...
> 
> > 
> > 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 */  
> 
> It becomes easier to find Paolo's suggestion to fix that here:
> 
> https://lore.kernel.org/qemu-devel/cd1aba90-176f-9ec6-3e2b-d1135156a96d@redhat.com/
> 
> Would love to see this resolved! Unfortunately I haven't had the 
> bandwidth to fix it all up for ... a long time now.
> 

There is a bigger issue with writes.  You have to do a RMW cycle
because we only want to update part of a larger region.

It would worry me that this might have unexpected side effects
in some device implementations. It also looks a bit fiddly to do given
we'll need to pass the read callbacks to the write functions.

So the read path is straight forwards, but write less so.

Jonathan


> Andrew