Re: [PATCH 0/3] physmem: Fix MemoryRegion for second access to cached MMIO Address Space

[Jonathan Cameron via <qemu-devel@nongnu.org>]

On Thu, 15 Feb 2024 14:28:14 +0000
Jonathan Cameron via <qemu-devel@nongnu.org> wrote:

Any comments?  Almost all the other fixes I need for CXL memory to
work as normal ram are queued up so I'd love it if we can solve this one as
well.

This looks like a big series, but it's really just a refactor + trivial
addition - so shouldn't be too scary!

Jonathan

> Issue seen testing virtio-blk-pci with CXL emulated interleave memory.
> Tests were done on arm64, but the issue isn't architecture specific.
> Note that some additional fixes are needed to TCG to be able to run far
> enough to hit this on arm64 or x86. They are issues so I'll post separate
> series shortly.
> 
> The address_space_read_cached_slow() and address_space_write_cached_slow()
> functions query the MemoryRegion for the cached address space correctly
> using address_space_translate_cached() but then call into
> flatview_read_continue() / flatview_write_continue()
> If the access is to a MMIO MemoryRegion and is bigger than the MemoryRegion
> supports, the loop will query the MemoryRegion for the next access to use.
> That query uses flatview_translate() but the address passed is suitable
> for the cache, not the flatview. On my test setup that mean the second
> 8 bytes and onwards of the virtio descriptor was read from flash memory
> at the beginning of the system address map, not the CXL emulated memory
> where the descriptor was found.  Result happened to be all fs so easy to
> spot.
> 
> Changes these calls to use address_space_translate_cached() to get the
> correct MemoryRegion for the cache. To avoid duplicating most of the
> code, the first 2 patches factor out the common parts of
> flatview_read_continue() and flatview_write_continue() so they can
> be reused.
> 
> Write path has not been tested but it so similar to the read path I've
> included it here.
> 
> Jonathan Cameron (3):
>   physmem: Reduce local variable scope in flatview_read/write_continue()
>   physmem: Factor out body of flatview_read/write_continue() loop
>   physmem: Fix wrong MR in large address_space_read/write_cached_slow()
> 
>  system/physmem.c | 245 ++++++++++++++++++++++++++++++++---------------
>  1 file changed, 170 insertions(+), 75 deletions(-)
>