| Message ID | 168592151963.1948938.7831940219025967692.stgit@dwillia2-xfh.jf.intel.com |
|---|---|
| State | New, archived |
| Headers | show |
| Series | cxl: Device memory setup | expand |
On Sun, 04 Jun 2023 16:31:59 -0700 Dan Williams <dan.j.williams@intel.com> wrote: > In support of the Linux CXL core scaling for a wider set of CXL devices, > allow for the creation of memdevs with some memory device capabilities > disabled. Specifically, allow for CXL devices outside of those claiming > to be compliant with the generic CXL memory device class code, like > vendor specific Type-2/3 devices that host CXL.mem. This implies, allow > for the creation of memdevs that only support component-registers, not > necessarily memory-device-registers (like mailbox registers). A memdev > derived from a CXL endpoint that does not support generic class code > expectations is tagged "CXL_DEVTYPE_DEVMEM", while a memdev derived from a > class-code compliant endpoint is tagged "CXL_DEVTYPE_CLASSMEM". > > The primary assumption of a CXL_DEVTYPE_DEVMEM memdev is that it > optionally may not host a mailbox. Disable the command passthrough ioctl > for memdevs that are not CXL_DEVTYPE_CLASSMEM, and return empty strings > from memdev attributes associated with data retrieved via the > class-device-standard IDENTIFY command. Note that empty strings were > chosen over attribute visibility to maintain compatibility with shipping > versions of cxl-cli that expect those attributes to always be present. Hmm. I'm not keen on this, but I guess we've ended up in this corner so don't have much choice. > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> Trivial stuff inline. > diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h > index d3fe73d5ba4d..b8bdf7490d2c 100644 > --- a/drivers/cxl/cxlmem.h > +++ b/drivers/cxl/cxlmem.h > @@ -254,6 +254,20 @@ struct cxl_poison_state { > struct mutex lock; /* Protect reads of poison list */ > }; > > +/* > + * enum cxl_devtype - delineate type-2 from a generic type-3 device > + * @CXL_DEVTYPE_DEVMEM - Vendor specific CXL Type-2 device implementing HDM-D or Bit of a naming collision with other uses of DEVMEM but I can't immediately think of a better name so fair enough... > + * HDM-DB, no expectation that this device implements a > + * mailbox, or other memory-device-standard manageability > + * flows. no requirement that this device Expectation is a bit strong the other way to my reading. These device might well implement some or all of that + other stuff that means they don't want to use the class code. > + * @CXL_DEVTYPE_CLASSMEM - Common class definition of a CXL Type-3 device with > + * HDM-H and class-mandatory memory device registers > + */ > +enum cxl_devtype { > + CXL_DEVTYPE_DEVMEM, > + CXL_DEVTYPE_CLASSMEM, > +}; > + > /** > * struct cxl_dev_state - The driver device state > * > @@ -273,6 +287,7 @@ struct cxl_poison_state { > * @component_reg_phys: register base of component registers > * @info: Cached DVSEC information about the device. > * @serial: PCIe Device Serial Number > + * @type: Generic Memory Class device or Vendor Specific Memory device > */ > struct cxl_dev_state { > struct device *dev; > @@ -286,6 +301,7 @@ struct cxl_dev_state { > struct resource ram_res; > resource_size_t component_reg_phys; > u64 serial; > + enum cxl_devtype type; > }; > > /** > @@ -344,6 +360,8 @@ struct cxl_memdev_state { > static inline struct cxl_memdev_state * > to_cxl_memdev_state(struct cxl_dev_state *cxlds) > { > + if (cxlds->type != CXL_DEVTYPE_CLASSMEM) > + return NULL; > return container_of(cxlds, struct cxl_memdev_state, cxlds); > } > >
Jonathan Cameron wrote: > On Sun, 04 Jun 2023 16:31:59 -0700 > Dan Williams <dan.j.williams@intel.com> wrote: > > > In support of the Linux CXL core scaling for a wider set of CXL devices, > > allow for the creation of memdevs with some memory device capabilities > > disabled. Specifically, allow for CXL devices outside of those claiming > > to be compliant with the generic CXL memory device class code, like > > vendor specific Type-2/3 devices that host CXL.mem. This implies, allow > > for the creation of memdevs that only support component-registers, not > > necessarily memory-device-registers (like mailbox registers). A memdev > > derived from a CXL endpoint that does not support generic class code > > expectations is tagged "CXL_DEVTYPE_DEVMEM", while a memdev derived from a > > class-code compliant endpoint is tagged "CXL_DEVTYPE_CLASSMEM". > > > > The primary assumption of a CXL_DEVTYPE_DEVMEM memdev is that it > > optionally may not host a mailbox. Disable the command passthrough ioctl > > for memdevs that are not CXL_DEVTYPE_CLASSMEM, and return empty strings > > from memdev attributes associated with data retrieved via the > > class-device-standard IDENTIFY command. Note that empty strings were > > chosen over attribute visibility to maintain compatibility with shipping > > versions of cxl-cli that expect those attributes to always be present. > Hmm. I'm not keen on this, but I guess we've ended up in this corner > so don't have much choice. We could start with fixing that expectation and then set a deprecation schedule for this workaround. I filed an internal task to track this. > > > > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > > Trivial stuff inline. > > > diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h > > index d3fe73d5ba4d..b8bdf7490d2c 100644 > > --- a/drivers/cxl/cxlmem.h > > +++ b/drivers/cxl/cxlmem.h > > @@ -254,6 +254,20 @@ struct cxl_poison_state { > > struct mutex lock; /* Protect reads of poison list */ > > }; > > > > +/* > > + * enum cxl_devtype - delineate type-2 from a generic type-3 device > > + * @CXL_DEVTYPE_DEVMEM - Vendor specific CXL Type-2 device implementing HDM-D or > > Bit of a naming collision with other uses of DEVMEM but I can't immediately think > of a better name so fair enough... > > > + * HDM-DB, no expectation that this device implements a > > + * mailbox, or other memory-device-standard manageability > > + * flows. > no requirement that this device > > Expectation is a bit strong the other way to my reading. These device might well > implement some or all of that + other stuff that means they don't want to use the > class code. Fair enough.
diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c index 14805dae5a74..3ca0bf12c55f 100644 --- a/drivers/cxl/core/mbox.c +++ b/drivers/cxl/core/mbox.c @@ -1273,6 +1273,7 @@ struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev) mutex_init(&mds->mbox_mutex); mutex_init(&mds->event.log_lock); mds->cxlds.dev = dev; + mds->cxlds.type = CXL_DEVTYPE_CLASSMEM; return mds; } diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c index 15434b1b4909..3f2d54f30548 100644 --- a/drivers/cxl/core/memdev.c +++ b/drivers/cxl/core/memdev.c @@ -41,6 +41,8 @@ static ssize_t firmware_version_show(struct device *dev, struct cxl_dev_state *cxlds = cxlmd->cxlds; struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlds); + if (!mds) + return sysfs_emit(buf, "\n"); return sysfs_emit(buf, "%.16s\n", mds->firmware_version); } static DEVICE_ATTR_RO(firmware_version); @@ -52,6 +54,8 @@ static ssize_t payload_max_show(struct device *dev, struct cxl_dev_state *cxlds = cxlmd->cxlds; struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlds); + if (!mds) + return sysfs_emit(buf, "\n"); return sysfs_emit(buf, "%zu\n", mds->payload_size); } static DEVICE_ATTR_RO(payload_max); @@ -63,6 +67,8 @@ static ssize_t label_storage_size_show(struct device *dev, struct cxl_dev_state *cxlds = cxlmd->cxlds; struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlds); + if (!mds) + return sysfs_emit(buf, "\n"); return sysfs_emit(buf, "%zu\n", mds->lsa_size); } static DEVICE_ATTR_RO(label_storage_size); @@ -517,10 +523,12 @@ static long cxl_memdev_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { struct cxl_memdev *cxlmd = file->private_data; + struct cxl_dev_state *cxlds; int rc = -ENXIO; down_read(&cxl_memdev_rwsem); - if (cxlmd->cxlds) + cxlds = cxlmd->cxlds; + if (cxlds && cxlds->type == CXL_DEVTYPE_CLASSMEM) rc = __cxl_memdev_ioctl(cxlmd, cmd, arg); up_read(&cxl_memdev_rwsem); diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h index d3fe73d5ba4d..b8bdf7490d2c 100644 --- a/drivers/cxl/cxlmem.h +++ b/drivers/cxl/cxlmem.h @@ -254,6 +254,20 @@ struct cxl_poison_state { struct mutex lock; /* Protect reads of poison list */ }; +/* + * enum cxl_devtype - delineate type-2 from a generic type-3 device + * @CXL_DEVTYPE_DEVMEM - Vendor specific CXL Type-2 device implementing HDM-D or + * HDM-DB, no expectation that this device implements a + * mailbox, or other memory-device-standard manageability + * flows. + * @CXL_DEVTYPE_CLASSMEM - Common class definition of a CXL Type-3 device with + * HDM-H and class-mandatory memory device registers + */ +enum cxl_devtype { + CXL_DEVTYPE_DEVMEM, + CXL_DEVTYPE_CLASSMEM, +}; + /** * struct cxl_dev_state - The driver device state * @@ -273,6 +287,7 @@ struct cxl_poison_state { * @component_reg_phys: register base of component registers * @info: Cached DVSEC information about the device. * @serial: PCIe Device Serial Number + * @type: Generic Memory Class device or Vendor Specific Memory device */ struct cxl_dev_state { struct device *dev; @@ -286,6 +301,7 @@ struct cxl_dev_state { struct resource ram_res; resource_size_t component_reg_phys; u64 serial; + enum cxl_devtype type; }; /** @@ -344,6 +360,8 @@ struct cxl_memdev_state { static inline struct cxl_memdev_state * to_cxl_memdev_state(struct cxl_dev_state *cxlds) { + if (cxlds->type != CXL_DEVTYPE_CLASSMEM) + return NULL; return container_of(cxlds, struct cxl_memdev_state, cxlds); }
In support of the Linux CXL core scaling for a wider set of CXL devices, allow for the creation of memdevs with some memory device capabilities disabled. Specifically, allow for CXL devices outside of those claiming to be compliant with the generic CXL memory device class code, like vendor specific Type-2/3 devices that host CXL.mem. This implies, allow for the creation of memdevs that only support component-registers, not necessarily memory-device-registers (like mailbox registers). A memdev derived from a CXL endpoint that does not support generic class code expectations is tagged "CXL_DEVTYPE_DEVMEM", while a memdev derived from a class-code compliant endpoint is tagged "CXL_DEVTYPE_CLASSMEM". The primary assumption of a CXL_DEVTYPE_DEVMEM memdev is that it optionally may not host a mailbox. Disable the command passthrough ioctl for memdevs that are not CXL_DEVTYPE_CLASSMEM, and return empty strings from memdev attributes associated with data retrieved via the class-device-standard IDENTIFY command. Note that empty strings were chosen over attribute visibility to maintain compatibility with shipping versions of cxl-cli that expect those attributes to always be present. Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- drivers/cxl/core/mbox.c | 1 + drivers/cxl/core/memdev.c | 10 +++++++++- drivers/cxl/cxlmem.h | 18 ++++++++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-)