Re: [PATCH v5 00/43] CXl 2.0 emulation Support

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

On Wed, 2 Feb 2022 14:09:54 +0000
Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:

> Changes since v4:
> https://lore.kernel.org/linux-cxl/20220124171705.10432-1-Jonathan.Cameron@huawei.com/
> 
> Note documentation patch that Alex requested to follow.
> I don't want to delay getting this out as Alex mentioned possibly
> having time to continue reviewing in latter part of this week.
> 
> Issues identified by CI / Alex Bennée
> - Stubs added for hw/cxl/cxl-host and hw/acpi/cxl plus related meson
>   changes to use them as necessary.
> - Drop uid from cxl-test (result of last minute change in v4 that was not
>   carried through to the test)
> - Fix naming clash with field name ERROR which on some arches is defined
>   and results in the string being replaced with 0 in some of the
>   register field related defines.  Call it ERR instead.
> - Fix type issue around mr->size by using 64 bit acessor functions.
> - Add a new patch to exclude pxb-cxl from device-crash-test in similar
>   fashion to pxb.
> 
> CI tests now passing with exception of checkpatch which has what
> I think is a false positive and build-oss-fuzz which keeps timing out.
> https://gitlab.com/jic23/qemu/-/pipelines/460109208
> There were a few tweaks to patch descriptions after I pushed that
> out (I missed a few RB from Alex).
> 
> Other changes (mostly from Alex's review)
> - Change component register handling to now report UNIMP and return 0
>   for 8 byte registers as we currently don't implement any of them.
>   Note that this means we need a kernel fix:
>   https://lore.kernel.org/linux-cxl/20220201153437.2873-1-Jonathan.Cameron@huawei.com/
> - Drop majority of the macros used in defining mailbox handlers in
>   favour of written out code.
> - Use REG64 where appropriate. This was introduced whilst this set
>   has been underdevelopment so I missed it.
> - Clarify some register access options wrt to CXL 2.0 Errata F4.
> - Change timestamp to qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)
> - Use typed enums to enforce types of function arguements.
> - Default to cxl being off in machine_class_init() removing
>   need to set it to off in machines where there is no support as yet.
> - Add Alex's RB where given.
> 
> Looking in particular for:
> * Review of the PCI interactions
> * x86 and ARM machine interactions (particularly the memory maps)
> * Review of the interleaving approach - is the basic idea
>   acceptable?
> * Review of the command line interface.
> * CXL related review welcome but much of that got reviewed
>   in earlier versions and hasn't changed substantially.
> 
> Big TODOs:
> 
> * Interleave boundary issues. I haven't yet solved this but didn't
>   want to futher delay the review of the rest of the series.

So... After fixing my test, it became clear that Qemu won't issue
unaligned memory accesses to device unless mr->ram == true.
We can't set that for a CXL Fixed Memory Window (CFMW) as we have only an
indirect association with the CXL type3 memory devices and their
backing RAM. The interleave decoding has to sit in between.

So it 'kind of' works without any special handling as QEMU splits the
accesses into two anyway.

I don't yet understand fully the implications of this and whether
it in any real way restricts what can be done with the interleaved
memory under a CXL fixed memory region.  Would definitely appreciate
inputs on this aspect.

The really short background story is:

1) Host PA memory region (CFMW) to which expectation is any access that
would be fine to normal DDR/Ram or NVDIMMs should work as long
as appropriate CXL topology and decoder configuration has been done
to get the memory accesses to actual memory.
2) The actual accesses to PAs in that region are interleaved
via several decoders on path to memory - min granularity is 256
bytes so any given access can only end up hitting 1 or 2 devices.
3) Fun corner cases are unaligned access crossing the interleave
boundary. 

Jonathan