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

[Ben Widawsky]

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.


> >> >                     Also, Markus has been trying to get rid of so-called
> >> > "simple unions" in favor of "flat unions" - every time we modify an
> >> > existing simple union, it is worth asking if it is time to first flatten
> >> > this.
> >> 
> >> 0. Simple unions can be transformed into flat unions.  The
> >> transformation can either preserve the nested wire format, or flatten
> >> it.  See docs/devel/qapi-code-gen.txt "A simple union can always be
> >> re-written as a flat union ..."
> >> 
> >> 1. No new simple unions.
> >> 
> >> 2. Existing simple unions that can be flattened without breaking
> >> backward compatibility have long been flattened.
> >> 
> >> 3. The remaining simple unions are part of QMP, where we need to
> >> preserve the wire format.  We could turn them into flat union preserving
> >> the wire format.  Only worthwhile if we kill simple unions and simplify
> >> scripts/qapi/.  Opportunity to make the flat union syntax less
> >> cumbersome.  Not done due to lack of time.
> >> 
> >> 4. Kevin and I have been experimenting with ways to provide both flat
> >> and nested wire format.  This would pave the way for orderly deprecation
> >> of the nested wire format.  May not be practical for QMP output.
> >> 
> >
> > So is there anything for me to do here?
> 
> No.  Extending an existing simple union is okay.
> 
> We should not add news ones.  We should think twice before we add new
> uses of existing ones.
> 
>