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

[Markus Armbruster]

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.

>> >                     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.