Re: [RFC PATCH 18/25] hw/cxl/device: Add a memory device (8.2.8.5)
Ben Widawsky writes:
> On 20-11-26 07:36:23, Markus Armbruster wrote:
>> Ben Widawsky writes:
>>
>> > On 20-11-13 08:47:59, Markus Armbruster wrote:
>> >> Eric Blake 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
>> >> >> ---
>> >> >
>> >> >> +++ 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?
[...]
Re: [RFC PATCH 18/25] hw/cxl/device: Add a memory device (8.2.8.5)
On 20-11-26 07:36:23, Markus Armbruster wrote:
> Ben Widawsky writes:
>
> > On 20-11-13 08:47:59, Markus Armbruster wrote:
> >> Eric Blake 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
> >> >> ---
> >> >
> >> >> +++ 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.
>
>
Re: [RFC PATCH 18/25] hw/cxl/device: Add a memory device (8.2.8.5)
Ben Widawsky writes:
> On 20-11-13 08:47:59, Markus Armbruster wrote:
>> Eric Blake 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
>> >> ---
>> >
>> >> +++ 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.
Re: [RFC PATCH 18/25] hw/cxl/device: Add a memory device (8.2.8.5)
On 20-11-13 08:47:59, Markus Armbruster wrote:
> Eric Blake 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
> >> ---
> >
> >> +++ 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.
> > 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?
Re: [RFC PATCH 18/25] hw/cxl/device: Add a memory device (8.2.8.5)
Eric Blake 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
>> ---
>
>> +++ 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.
> 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.
Re: [RFC PATCH 18/25] hw/cxl/device: Add a memory device (8.2.8.5)
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
> ---
> +++ 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. 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.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
[RFC PATCH 18/25] hw/cxl/device: Add a memory device (8.2.8.5)
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
---
hw/core/numa.c | 3 +
hw/i386/pc.c | 1 +
hw/mem/Kconfig | 5 +
hw/mem/cxl_type3.c | 262 +++
hw/mem/meson.build | 1 +
hw/pci/pcie.c| 30 +
include/hw/cxl/cxl.h | 2 +
include/hw/cxl/cxl_pci.h | 22
include/hw/pci/pci_ids.h | 1 +
monitor/hmp-cmds.c | 15 +++
qapi/machine.json| 1 +
11 files changed, 343 insertions(+)
create mode 100644 hw/mem/cxl_type3.c
diff --git a/hw/core/numa.c b/hw/core/numa.c
index 7c4dd4e68e..3ddeb23036 100644
--- a/hw/core/numa.c
+++ b/hw/core/numa.c
@@ -770,6 +770,9 @@ static void numa_stat_memory_devices(NumaNodeMem node_mem[])
node_mem[pcdimm_info->node].node_plugged_mem +=
pcdimm_info->size;
break;
+case MEMORY_DEVICE_INFO_KIND_CXL:
+/* FINISHME */
+break;
case MEMORY_DEVICE_INFO_KIND_VIRTIO_PMEM:
vpi = value->u.virtio_pmem.data;
/* TODO: once we support numa, assign to right node */
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 5e6c0023e0..ecfc497f71 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -79,6 +79,7 @@
#include "acpi-build.h"
#include "hw/mem/pc-dimm.h"
#include "hw/mem/nvdimm.h"
+#include "hw/cxl/cxl.h"
#include "qapi/error.h"
#include "qapi/qapi-visit-common.h"
#include "qapi/visitor.h"
diff --git a/hw/mem/Kconfig b/hw/mem/Kconfig
index a0ef2cf648..7d9d1ced3e 100644
--- a/hw/mem/Kconfig
+++ b/hw/mem/Kconfig
@@ -10,3 +10,8 @@ config NVDIMM
default y
depends on (PC || PSERIES || ARM_VIRT)
select MEM_DEVICE
+
+config CXL_MEM_DEVICE
+bool
+default y if CXL
+select MEM_DEVICE
diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
new file mode 100644
index 00..48c25922f3
--- /dev/null
+++ b/hw/mem/cxl_type3.c
@@ -0,0 +1,262 @@
+#include "qemu/osdep.h"
+#include "qemu/units.h"
+#include "qemu/error-report.h"
+#include "hw/mem/memory-device.h"
+#include "hw/mem/pc-dimm.h"
+#include "hw/pci/pci.h"
+#include "hw/qdev-properties.h"
+#include "qapi/error.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+#include "qemu/range.h"
+#include "qemu/rcu.h"
+#include "sysemu/hostmem.h"
+#include "hw/cxl/cxl.h"
+
+typedef struct cxl_type3_dev {
+/* Private */
+PCIDevice parent_obj;
+
+/* Properties */
+uint64_t size;
+HostMemoryBackend *hostmem;
+
+/* State */
+CXLComponentState cxl_cstate;
+CXLDeviceState cxl_dstate;
+} CXLType3Dev;
+
+#define CT3(obj) OBJECT_CHECK(CXLType3Dev, (obj), TYPE_CXL_TYPE3_DEV)
+
+static void build_dvsecs(CXLType3Dev *ct3d)
+{
+CXLComponentState *cxl_cstate = >cxl_cstate;
+uint8_t *dvsec;
+
+dvsec = (uint8_t *)&(struct dvsec_device){
+.cap = 0x1e,
+.ctrl = 0x6,
+.status2 = 0x2,
+.range1_size_hi = 0,
+.range1_size_lo = (2 << 5) | (2 << 2) | 0x3 | ct3d->size,
+.range1_base_hi = 0,
+.range1_base_lo = 0,
+};
+cxl_component_create_dvsec(cxl_cstate, PCIE_CXL_DEVICE_DVSEC_LENGTH,
+ PCIE_CXL_DEVICE_DVSEC,
+ PCIE_CXL_DEVICE_DVSEC_REVID, dvsec);
+
+dvsec = (uint8_t *)&(struct dvsec_register_locator){
+.rsvd = 0,
+.reg0_base_lo = RBI_COMPONENT_REG | COMPONENT_REG_BAR_IDX,
+.reg0_base_hi = 0,
+.reg1_base_lo = RBI_CXL_DEVICE_REG | DEVICE_REG_BAR_IDX,
+.reg1_base_hi = 0,
+};
+cxl_component_create_dvsec(cxl_cstate, REG_LOC_DVSEC_LENGTH, REG_LOC_DVSEC,
+ REG_LOC_DVSEC_REVID, dvsec);
+}
+
+static void ct3_instance_init(Object *obj)
+{
+/* MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(obj); */
+}
+
+static void ct3_finalize(Object *obj)
+{
+CXLType3Dev *ct3d = CT3(obj);
+
+g_free(ct3d->cxl_dstate.pmem);
+}
+
+static void cxl_setup_memory(CXLType3Dev *ct3d, Error **errp)
+{
+MemoryRegionSection mrs;
+MemoryRegion *mr;
+uint64_t offset = 0;
+size_t remaining_size;
+
+if (!ct3d->hostmem) {
+error_setg(errp, "memdev property must be set");
+return;
+}
+
+/* FIXME: need to check mr is the host bridge's MR */
+mr = host_memory_backend_get_memory(ct3d->hostmem);
+
+/* Create our new subregion */
+ct3d->cxl_dstate.pmem = g_new(MemoryRegion, 1);
+
+/* Find the first free space in the window */
+WITH_RCU_READ_LOCK_GUARD()
+{
+mrs = memory_region_find(mr,
