Re: [RFC PATCH 18/25] hw/cxl/device: Add a memory device (8.2.8.5)

[Markus Armbruster]

Ben Widawsky <ben@bwidawsk.net> writes:

> On 20-11-26 07:36:23, Markus Armbruster wrote:
>> Ben Widawsky <ben.widawsky@intel.com> writes:
>> 
>> > On 20-11-13 08:47:59, Markus Armbruster wrote:
>> >> Eric Blake <eblake@redhat.com> writes:
>> >> 
>> >> > On 11/10/20 11:47 PM, Ben Widawsky wrote:
>> >> >> A CXL memory device (AKA Type 3) is a CXL component that contains some
>> >> >> combination of volatile and persistent memory. It also implements the
>> >> >> previously defined mailbox interface as well as the memory device
>> >> >> firmware interface.
>> >> >> 
>> >> >> The following example will create a 256M device in a 512M window:
>> >> >> 
>> >> >> -object 
>> >> >> "memory-backend-file,id=cxl-mem1,share,mem-path=cxl-type3,size=512M"
>> >> >> -device "cxl-type3,bus=rp0,memdev=cxl-mem1,id=cxl-pmem0,size=256M"
>> >> >> 
>> >> >> Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
>> >> >> ---
>> >> >
>> >> >> +++ b/qapi/machine.json
>> >> >> @@ -1394,6 +1394,7 @@
>> >> >>  { 'union': 'MemoryDeviceInfo',
>> >> >>    'data': { 'dimm': 'PCDIMMDeviceInfo',
>> >> >>              'nvdimm': 'PCDIMMDeviceInfo',
>> >> >> +            'cxl': 'PCDIMMDeviceInfo',
>> >> >>              'virtio-pmem': 'VirtioPMEMDeviceInfo',
>> >> >>              'virtio-mem': 'VirtioMEMDeviceInfo'
>> >> >>            }
>> >> >
>> >> > Missing documentation of the new data type, and the fact that it will be
>> >> > introduced in 6.0.
>> >> 
>> >> Old wish list item: improve the QAPI schema frontend to flag this.
>> >> 
>> >
>> > "Introduced in 6.0" - quite the optimist. Kidding aside, this is the area 
>> > where
>> > I could use some feedback. CXL Type 3 memory devices can contain both 
>> > volatile
>> > and persistent memory at the same time. As such, I think I'll need a new 
>> > type to
>> > represent that, but I'd love to know how best to accomplish that.
>> 
>> We can help.  Tell us what information you want to provide in variant
>> 'cxl'.  If it's a superset of an existing variant, give us just the
>> delta.
>> 
>
> I'm not exactly sure what the best way to do this is in QEMU, so I'm not 
> really
> sure what to specify as the delta. A CXL memory device can have both volatile
> and persistent memory. Currently when a CXL memory device implements the
> TYPE_MEMORY_DEVICE interface. So I believe the shortest path is a
> MemoryDeviceInfo that can have two memory devices associated with it, but I
> don't know if that's the best path.

Perhaps a CXL device should contain two sub-devices implementing
TYPE_MEMORY_DEVICE.  Paolo, what do you think?

If yes, one of the existing variants fits the bill, I guess.

If no, I have more ramblings to offer.

query-memory-devices returns a MemoryDeviceInfo for each realized device
that implements interface TYPE_MEMORY_DEVICE.  The interface provides
callback ->fill_device_info() to fill in the MemoryDeviceInfo.  This is
its only use.

This means TYPE_MEMORY_DEVICE places no obvious constraints on how the
callbacks use MemoryDeviceInfo.  Each callback can pick whatever variant
it wants.  This also means *your* callback can pick a new one, which you
define however you want.

What if there are unobvious (and unwritten) constraints?

The existing variants overlap:

* All provide the device's ID: optional member @id

* All provide a physical address (base address, I supposed) and size,
  but the member names differ (argh!): @addr, @size vs. @memaddr, @size

* All provide the memory backend: member @memdev

The members that overlap by necessity (i.e. any conceivable
TYPE_MEMORY_DEVICE will have them) should be common members, not variant
members.  Requires converting the simple union to the equivalent flat
union.

Do these members overlap by necessity?  Paolo, Igor?

[...]