| Message ID | 20250220200041.3891165-2-alejandro.lucero-palau@amd.com |
|---|---|
| State | New |
| Headers | show |
| Series | type2 cxl initialization | expand |
alejandro.lucero-palau@ wrote: > From: Alejandro Lucero <alucerop@amd.com> > > Differentiate CXL memory expanders (type 3) from CXL device accelerators > (type 2) with a new function for initializing cxl_dev_state and a macro > for helping accel drivers to embed cxl_dev_state inside a private > struct. > > Move structs to include/cxl as the size of the accel driver private > struct embedding cxl_dev_state needs to know the size of this struct. > > Signed-off-by: Alejandro Lucero <alucerop@amd.com> > --- > drivers/cxl/core/mbox.c | 3 +- > drivers/cxl/core/memdev.c | 25 +++++ > drivers/cxl/core/pci.c | 1 + > drivers/cxl/core/regs.c | 1 + > drivers/cxl/cxl.h | 98 +---------------- > drivers/cxl/cxlmem.h | 108 +----------------- > drivers/cxl/cxlpci.h | 21 ---- > drivers/cxl/pci.c | 17 +-- > include/cxl/cxl.h | 225 ++++++++++++++++++++++++++++++++++++++ > include/cxl/pci.h | 23 ++++ > 10 files changed, 293 insertions(+), 229 deletions(-) > create mode 100644 include/cxl/cxl.h > create mode 100644 include/cxl/pci.h > > diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c > index c5eedcae3b02..ae92db38a921 100644 > --- a/drivers/cxl/core/mbox.c > +++ b/drivers/cxl/core/mbox.c > @@ -1426,7 +1426,8 @@ int cxl_mailbox_init(struct cxl_mailbox *cxl_mbox, struct device *host) > } > EXPORT_SYMBOL_NS_GPL(cxl_mailbox_init, "CXL"); > > -struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev) > +struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev, u64 serial, > + u16 dvsec) > { > struct cxl_memdev_state *mds; I would ultimately expect that this starts to use the cxl_dev_state_create() internally as well to keep those internals common. > > diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c > index 2914e3ff104b..14bd74823467 100644 > --- a/drivers/cxl/core/memdev.c > +++ b/drivers/cxl/core/memdev.c > @@ -636,6 +636,31 @@ static void detach_memdev(struct work_struct *work) > > static struct lock_class_key cxl_memdev_key; > > +void cxl_dev_state_init(struct cxl_dev_state *cxlds, struct device *dev, > + enum cxl_devtype type, u64 serial, u16 dvsec) > +{ > + cxlds->dev = dev; > + cxlds->type = type; > + cxlds->serial = serial; > + cxlds->cxl_dvsec = dvsec; > + cxlds->reg_map.host = dev; > + cxlds->reg_map.resource = CXL_RESOURCE_NONE; Let's take the opportunity to now use compound literal syntax which immediately answers the question of whether any new or not included fields are initialized. I.e. *cxlds = (struct cxl_dev_state) { .dev = dev, ... }; > +} > + > +struct cxl_dev_state *_cxl_dev_state_create(struct device *dev, > + u64 serial, u16 dvsec, > + size_t size) > +{ > + struct cxl_dev_state *cxlds __free(kfree) = kzalloc(size, GFP_KERNEL); > + > + if (!cxlds) > + return NULL; > + > + cxl_dev_state_init(cxlds, dev, CXL_DEVTYPE_DEVMEM, serial, dvsec); I expect the devtype will need to be passed in as _cxl_dev_state_create can not assume memory expander vs an accelerator. > + return_ptr(cxlds); > +} > +EXPORT_SYMBOL_NS_GPL(_cxl_dev_state_create, "CXL"); > + > static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds, > const struct file_operations *fops) > { > diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c > index a5c65f79db18..36a1cf4e59fb 100644 > --- a/drivers/cxl/core/pci.c > +++ b/drivers/cxl/core/pci.c > @@ -1,5 +1,6 @@ > // SPDX-License-Identifier: GPL-2.0-only > /* Copyright(c) 2021 Intel Corporation. All rights reserved. */ > +#include <cxl/pci.h> > #include <linux/units.h> > #include <linux/io-64-nonatomic-lo-hi.h> > #include <linux/device.h> > diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c > index 117c2e94c761..58a942a4946c 100644 > --- a/drivers/cxl/core/regs.c > +++ b/drivers/cxl/core/regs.c > @@ -4,6 +4,7 @@ > #include <linux/device.h> > #include <linux/slab.h> > #include <linux/pci.h> > +#include <cxl/pci.h> > #include <cxlmem.h> > #include <cxlpci.h> > #include <pmu.h> > diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h > index 6baec4ba9141..556b1ed24f0e 100644 > --- a/drivers/cxl/cxl.h > +++ b/drivers/cxl/cxl.h > @@ -9,8 +9,8 @@ > #include <linux/notifier.h> > #include <linux/bitops.h> > #include <linux/log2.h> > -#include <linux/node.h> > #include <linux/io.h> > +#include <cxl/cxl.h> > > extern const struct nvdimm_security_ops *cxl_security_ops; > > @@ -200,97 +200,6 @@ static inline int ways_to_eiw(unsigned int ways, u8 *eiw) > #define CXLDEV_MBOX_BG_CMD_COMMAND_VENDOR_MASK GENMASK_ULL(63, 48) > #define CXLDEV_MBOX_PAYLOAD_OFFSET 0x20 > > -/* > - * Using struct_group() allows for per register-block-type helper routines, > - * without requiring block-type agnostic code to include the prefix. > - */ > -struct cxl_regs { > - /* > - * Common set of CXL Component register block base pointers > - * @hdm_decoder: CXL 2.0 8.2.5.12 CXL HDM Decoder Capability Structure > - * @ras: CXL 2.0 8.2.5.9 CXL RAS Capability Structure > - */ > - struct_group_tagged(cxl_component_regs, component, > - void __iomem *hdm_decoder; > - void __iomem *ras; > - ); > - /* > - * Common set of CXL Device register block base pointers > - * @status: CXL 2.0 8.2.8.3 Device Status Registers > - * @mbox: CXL 2.0 8.2.8.4 Mailbox Registers > - * @memdev: CXL 2.0 8.2.8.5 Memory Device Registers > - */ > - struct_group_tagged(cxl_device_regs, device_regs, > - void __iomem *status, *mbox, *memdev; > - ); > - > - struct_group_tagged(cxl_pmu_regs, pmu_regs, > - void __iomem *pmu; > - ); > - > - /* > - * RCH downstream port specific RAS register > - * @aer: CXL 3.0 8.2.1.1 RCH Downstream Port RCRB > - */ > - struct_group_tagged(cxl_rch_regs, rch_regs, > - void __iomem *dport_aer; > - ); > - > - /* > - * RCD upstream port specific PCIe cap register > - * @pcie_cap: CXL 3.0 8.2.1.2 RCD Upstream Port RCRB > - */ > - struct_group_tagged(cxl_rcd_regs, rcd_regs, > - void __iomem *rcd_pcie_cap; > - ); > -}; > - > -struct cxl_reg_map { > - bool valid; > - int id; > - unsigned long offset; > - unsigned long size; > -}; > - > -struct cxl_component_reg_map { > - struct cxl_reg_map hdm_decoder; > - struct cxl_reg_map ras; > -}; > - > -struct cxl_device_reg_map { > - struct cxl_reg_map status; > - struct cxl_reg_map mbox; > - struct cxl_reg_map memdev; > -}; > - > -struct cxl_pmu_reg_map { > - struct cxl_reg_map pmu; > -}; > - > -/** > - * struct cxl_register_map - DVSEC harvested register block mapping parameters > - * @host: device for devm operations and logging > - * @base: virtual base of the register-block-BAR + @block_offset > - * @resource: physical resource base of the register block > - * @max_size: maximum mapping size to perform register search > - * @reg_type: see enum cxl_regloc_type > - * @component_map: cxl_reg_map for component registers > - * @device_map: cxl_reg_maps for device registers > - * @pmu_map: cxl_reg_maps for CXL Performance Monitoring Units > - */ > -struct cxl_register_map { > - struct device *host; > - void __iomem *base; > - resource_size_t resource; > - resource_size_t max_size; > - u8 reg_type; > - union { > - struct cxl_component_reg_map component_map; > - struct cxl_device_reg_map device_map; > - struct cxl_pmu_reg_map pmu_map; > - }; > -}; Ok, all of the above make sense to move because they are needed to be able to export the size. It would be nice for the follow on patches to delineate which fields of 'struct cxl_dev_state' are expected to be private to the CXL core. > - > void cxl_probe_component_regs(struct device *dev, void __iomem *base, > struct cxl_component_reg_map *map); > void cxl_probe_device_regs(struct device *dev, void __iomem *base, > @@ -480,11 +389,6 @@ struct cxl_region_params { > int nr_targets; > }; > > -enum cxl_partition_mode { > - CXL_PARTMODE_RAM, > - CXL_PARTMODE_PMEM, > -}; Makes sense, this is exported as part of struct cxl_dpa_info. > - > /* > * Indicate whether this region has been assembled by autodetection or > * userspace assembly. Prevent endpoint decoders outside of automatic > diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h > index 8e1e46c348f5..0bdf581044da 100644 > --- a/drivers/cxl/cxlmem.h > +++ b/drivers/cxl/cxlmem.h > @@ -4,9 +4,9 @@ > #define __CXL_MEM_H__ > #include <uapi/linux/cxl_mem.h> > #include <linux/pci.h> > -#include <linux/cdev.h> > #include <linux/uuid.h> > #include <linux/node.h> > +#include <cxl/cxl.h> > #include <cxl/event.h> > #include <cxl/mailbox.h> > #include "cxl.h" > @@ -34,30 +34,6 @@ > (FIELD_GET(CXLMDEV_RESET_NEEDED_MASK, status) != \ > CXLMDEV_RESET_NEEDED_NOT) > > -/** > - * struct cxl_memdev - CXL bus object representing a Type-3 Memory Device > - * @dev: driver core device object > - * @cdev: char dev core object for ioctl operations > - * @cxlds: The device state backing this device > - * @detach_work: active memdev lost a port in its ancestry > - * @cxl_nvb: coordinate removal of @cxl_nvd if present > - * @cxl_nvd: optional bridge to an nvdimm if the device supports pmem > - * @endpoint: connection to the CXL port topology for this memory device > - * @id: id number of this memdev instance. > - * @depth: endpoint port depth > - */ > -struct cxl_memdev { > - struct device dev; > - struct cdev cdev; > - struct cxl_dev_state *cxlds; > - struct work_struct detach_work; > - struct cxl_nvdimm_bridge *cxl_nvb; > - struct cxl_nvdimm *cxl_nvd; > - struct cxl_port *endpoint; > - int id; > - int depth; > -}; Why does this need to be public? > - > static inline struct cxl_memdev *to_cxl_memdev(struct device *dev) > { > return container_of(dev, struct cxl_memdev, dev); > @@ -357,83 +333,6 @@ struct cxl_security_state { > struct kernfs_node *sanitize_node; > }; > > -/* > - * 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 requirement 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_dpa_perf - DPA performance property entry > - * @dpa_range: range for DPA address > - * @coord: QoS performance data (i.e. latency, bandwidth) > - * @cdat_coord: raw QoS performance data from CDAT > - * @qos_class: QoS Class cookies > - */ > -struct cxl_dpa_perf { > - struct range dpa_range; > - struct access_coordinate coord[ACCESS_COORDINATE_MAX]; > - struct access_coordinate cdat_coord[ACCESS_COORDINATE_MAX]; > - int qos_class; > -}; > - > -/** > - * struct cxl_dpa_partition - DPA partition descriptor > - * @res: shortcut to the partition in the DPA resource tree (cxlds->dpa_res) > - * @perf: performance attributes of the partition from CDAT > - * @mode: operation mode for the DPA capacity, e.g. ram, pmem, dynamic... > - */ > -struct cxl_dpa_partition { > - struct resource res; > - struct cxl_dpa_perf perf; > - enum cxl_partition_mode mode; > -}; Makes sense, this needs to go public with 'struct cxl_dev_state'. > - > -/** > - * struct cxl_dev_state - The driver device state > - * > - * cxl_dev_state represents the CXL driver/device state. It provides an > - * interface to mailbox commands as well as some cached data about the device. > - * Currently only memory devices are represented. > - * > - * @dev: The device associated with this CXL state > - * @cxlmd: The device representing the CXL.mem capabilities of @dev > - * @reg_map: component and ras register mapping parameters > - * @regs: Parsed register blocks > - * @cxl_dvsec: Offset to the PCIe device DVSEC > - * @rcd: operating in RCD mode (CXL 3.0 9.11.8 CXL Devices Attached to an RCH) > - * @media_ready: Indicate whether the device media is usable > - * @dpa_res: Overall DPA resource tree for the device > - * @part: DPA partition array > - * @nr_partitions: Number of DPA partitions > - * @serial: PCIe Device Serial Number > - * @type: Generic Memory Class device or Vendor Specific Memory device > - * @cxl_mbox: CXL mailbox context > - */ > -struct cxl_dev_state { > - struct device *dev; > - struct cxl_memdev *cxlmd; > - struct cxl_register_map reg_map; > - struct cxl_regs regs; > - int cxl_dvsec; > - bool rcd; > - bool media_ready; > - struct resource dpa_res; > - struct cxl_dpa_partition part[CXL_NR_PARTITIONS_MAX]; > - unsigned int nr_partitions; > - u64 serial; > - enum cxl_devtype type; > - struct cxl_mailbox cxl_mbox; > -}; Ack. > - > static inline resource_size_t cxl_pmem_size(struct cxl_dev_state *cxlds) > { > /* > @@ -812,7 +711,10 @@ int cxl_dev_state_identify(struct cxl_memdev_state *mds); > int cxl_await_media_ready(struct cxl_dev_state *cxlds); > int cxl_enumerate_cmds(struct cxl_memdev_state *mds); > int cxl_mem_dpa_fetch(struct cxl_memdev_state *mds, struct cxl_dpa_info *info); > -struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev); > +struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev, u64 serial, > + u16 dvsec); > +void cxl_dev_state_init(struct cxl_dev_state *cxlds, struct device *dev, > + enum cxl_devtype type, u64 serial, u16 dvsec); > void set_exclusive_cxl_commands(struct cxl_memdev_state *mds, > unsigned long *cmds); > void clear_exclusive_cxl_commands(struct cxl_memdev_state *mds, > diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h > index 54e219b0049e..570e53e26f11 100644 > --- a/drivers/cxl/cxlpci.h > +++ b/drivers/cxl/cxlpci.h > @@ -7,29 +7,8 @@ > > #define CXL_MEMORY_PROGIF 0x10 > > -/* > - * See section 8.1 Configuration Space Registers in the CXL 2.0 > - * Specification. Names are taken straight from the specification with "CXL" and > - * "DVSEC" redundancies removed. When obvious, abbreviations may be used. > - */ > #define PCI_DVSEC_HEADER1_LENGTH_MASK GENMASK(31, 20) > > -/* CXL 2.0 8.1.3: PCIe DVSEC for CXL Device */ > -#define CXL_DVSEC_PCIE_DEVICE 0 > -#define CXL_DVSEC_CAP_OFFSET 0xA > -#define CXL_DVSEC_MEM_CAPABLE BIT(2) > -#define CXL_DVSEC_HDM_COUNT_MASK GENMASK(5, 4) > -#define CXL_DVSEC_CTRL_OFFSET 0xC > -#define CXL_DVSEC_MEM_ENABLE BIT(2) > -#define CXL_DVSEC_RANGE_SIZE_HIGH(i) (0x18 + (i * 0x10)) > -#define CXL_DVSEC_RANGE_SIZE_LOW(i) (0x1C + (i * 0x10)) > -#define CXL_DVSEC_MEM_INFO_VALID BIT(0) > -#define CXL_DVSEC_MEM_ACTIVE BIT(1) > -#define CXL_DVSEC_MEM_SIZE_LOW_MASK GENMASK(31, 28) > -#define CXL_DVSEC_RANGE_BASE_HIGH(i) (0x20 + (i * 0x10)) > -#define CXL_DVSEC_RANGE_BASE_LOW(i) (0x24 + (i * 0x10)) > -#define CXL_DVSEC_MEM_BASE_LOW_MASK GENMASK(31, 28) Makes sense. > - > #define CXL_DVSEC_RANGE_MAX 2 > > /* CXL 2.0 8.1.4: Non-CXL Function Map DVSEC */ > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c > index b2c943a4de0a..006a0036e779 100644 > --- a/drivers/cxl/pci.c > +++ b/drivers/cxl/pci.c > @@ -1,5 +1,7 @@ > // SPDX-License-Identifier: GPL-2.0-only > /* Copyright(c) 2020 Intel Corporation. All rights reserved. */ > +#include <cxl/cxl.h> > +#include <cxl/pci.h> > #include <linux/unaligned.h> > #include <linux/io-64-nonatomic-lo-hi.h> > #include <linux/moduleparam.h> > @@ -911,6 +913,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) > int rc, pmu_count; > unsigned int i; > bool irq_avail; > + u16 dvsec; > > /* > * Double check the anonymous union trickery in struct cxl_regs > @@ -924,19 +927,19 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) > return rc; > pci_set_master(pdev); > > - mds = cxl_memdev_state_create(&pdev->dev); > + dvsec = pci_find_dvsec_capability(pdev, PCI_VENDOR_ID_CXL, > + CXL_DVSEC_PCIE_DEVICE); > + if (!dvsec) > + dev_warn(&pdev->dev, > + "Device DVSEC not present, skip CXL.mem init\n"); > + > + mds = cxl_memdev_state_create(&pdev->dev, pci_get_dsn(pdev), dvsec); > if (IS_ERR(mds)) > return PTR_ERR(mds); > cxlds = &mds->cxlds; > pci_set_drvdata(pdev, cxlds); > > cxlds->rcd = is_cxl_restricted(pdev); > - cxlds->serial = pci_get_dsn(pdev); > - cxlds->cxl_dvsec = pci_find_dvsec_capability( > - pdev, PCI_VENDOR_ID_CXL, CXL_DVSEC_PCIE_DEVICE); > - if (!cxlds->cxl_dvsec) > - dev_warn(&pdev->dev, > - "Device DVSEC not present, skip CXL.mem init\n"); > > rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map); > if (rc) Looks good.
On 2/20/25 2:00 PM, alejandro.lucero-palau@amd.com wrote: > From: Alejandro Lucero <alucerop@amd.com> > > Differentiate CXL memory expanders (type 3) from CXL device accelerators > (type 2) with a new function for initializing cxl_dev_state and a macro > for helping accel drivers to embed cxl_dev_state inside a private > struct. > > Move structs to include/cxl as the size of the accel driver private > struct embedding cxl_dev_state needs to know the size of this struct. > > Signed-off-by: Alejandro Lucero <alucerop@amd.com> > --- > drivers/cxl/core/mbox.c | 3 +- > drivers/cxl/core/memdev.c | 25 +++++ > drivers/cxl/core/pci.c | 1 + > drivers/cxl/core/regs.c | 1 + > drivers/cxl/cxl.h | 98 +---------------- > drivers/cxl/cxlmem.h | 108 +----------------- > drivers/cxl/cxlpci.h | 21 ---- > drivers/cxl/pci.c | 17 +-- > include/cxl/cxl.h | 225 ++++++++++++++++++++++++++++++++++++++ > include/cxl/pci.h | 23 ++++ > 10 files changed, 293 insertions(+), 229 deletions(-) [snip] > +/** > + * struct cxl_memdev - CXL bus object representing a Type-3 Memory Device > + * @dev: driver core device object > + * @cdev: char dev core object for ioctl operations > + * @cxlds: The device state backing this device > + * @detach_work: active memdev lost a port in its ancestry > + * @cxl_nvb: coordinate removal of @cxl_nvd if present > + * @cxl_nvd: optional bridge to an nvdimm if the device supports pmem > + * @endpoint: connection to the CXL port topology for this memory device > + * @id: id number of this memdev instance. > + * @depth: endpoint port depth > + */ > +struct cxl_memdev { > + struct device dev; > + struct cdev cdev; > + struct cxl_dev_state *cxlds; > + struct work_struct detach_work; > + struct cxl_nvdimm_bridge *cxl_nvb; > + struct cxl_nvdimm *cxl_nvd; > + struct cxl_port *endpoint; > + int id; > + int depth; > +}; This can stay in cxlmem.h, only a pointer is being used in cxl_dev_state. > + > +#define CXL_NR_PARTITIONS_MAX 2 > + > +/** > + * struct cxl_dev_state - The driver device state > + * > + * cxl_dev_state represents the CXL driver/device state. It provides an > + * interface to mailbox commands as well as some cached data about the device. > + * Currently only memory devices are represented. > + * > + * @dev: The device associated with this CXL state > + * @cxlmd: The device representing the CXL.mem capabilities of @dev > + * @reg_map: component and ras register mapping parameters > + * @regs: Parsed register blocks > + * @cxl_dvsec: Offset to the PCIe device DVSEC > + * @rcd: operating in RCD mode (CXL 3.0 9.11.8 CXL Devices Attached to an RCH) > + * @media_ready: Indicate whether the device media is usable > + * @dpa_res: Overall DPA resource tree for the device > + * @part: DPA partition array > + * @nr_partitions: Number of DPA partitions > + * @serial: PCIe Device Serial Number > + * @type: Generic Memory Class device or Vendor Specific Memory device > + * @cxl_mbox: CXL mailbox context > + */ > +struct cxl_dev_state { > + struct device *dev; > + struct cxl_memdev *cxlmd; > + struct cxl_register_map reg_map; > + struct cxl_regs regs; > + int cxl_dvsec; > + bool rcd; > + bool media_ready; > + struct resource dpa_res; > + struct cxl_dpa_partition part[CXL_NR_PARTITIONS_MAX]; > + unsigned int nr_partitions; > + u64 serial; > + enum cxl_devtype type; > + struct cxl_mailbox cxl_mbox; > +}; > + > +struct cxl_dev_state *_cxl_dev_state_create(struct device *dev, > + u64 serial, u16 dvsec, > + size_t size); > + > +#define cxl_dev_state_create(parent, serial, dvsec, drv_struct, member) \ > + ({ \ > + static_assert(__same_type(struct cxl_dev_state, \ > + ((drv_struct *)NULL)->member)); \ > + static_assert(offsetof(drv_struct, member) == 0); \ > + (drv_struct *)_cxl_dev_state_create(parent, serial, dvsec, \ > + sizeof(drv_struct));\ > + }) This should take a CXL device type parameter as well, then you can also use it in cxl_memdev_state_create() like this: diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c index 2c49e33851b2..14d7f93e4584 100644 --- a/drivers/cxl/core/mbox.c +++ b/drivers/cxl/core/mbox.c @@ -1484,24 +1484,31 @@ int cxl_mailbox_init(struct cxl_mailbox *cxl_mbox, struct device *host) } EXPORT_SYMBOL_NS_GPL(cxl_mailbox_init, "CXL"); +static void cxl_memdev_state_destroy(void *cxlmds) +{ + kfree(cxlmds); +} + struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev, u64 serial, u16 dvsec) { struct cxl_memdev_state *mds; int rc; - mds = devm_kzalloc(dev, sizeof(*mds), GFP_KERNEL); + mds = cxl_dev_state_create(dev, serial, dvsec, + CXL_DEVTYPE_CLASSMEM, + struct cxl_memdev_state, + cxlds); if (!mds) { dev_err(dev, "No memory available\n"); return ERR_PTR(-ENOMEM); } + rc = devm_add_action_or_reset(dev, cxl_memdev_state_destroy, mds); + if (rc) + return ERR_PTR(rc); + mutex_init(&mds->event.log_lock); - mds->cxlds.dev = dev; - mds->cxlds.reg_map.host = dev; - mds->cxlds.cxl_mbox.host = dev; - mds->cxlds.reg_map.resource = CXL_RESOURCE_NONE; - mds->cxlds.type = CXL_DEVTYPE_CLASSMEM; Looking at the above, it occurs to me that it may be good to either: a) Have a devm version of cxl_dev_state_create() or, b) Have an init macro (in addition) that takes a pointer so that you can use an allocator of your choice Neither are necessary, but would be more ergonomic IMO. Thanks, Ben
Ben Cheatham wrote: > On 2/20/25 2:00 PM, alejandro.lucero-palau@amd.com wrote: > > From: Alejandro Lucero <alucerop@amd.com> > > > > Differentiate CXL memory expanders (type 3) from CXL device accelerators > > (type 2) with a new function for initializing cxl_dev_state and a macro > > for helping accel drivers to embed cxl_dev_state inside a private > > struct. > > > > Move structs to include/cxl as the size of the accel driver private > > struct embedding cxl_dev_state needs to know the size of this struct. > > > > Signed-off-by: Alejandro Lucero <alucerop@amd.com> [..] > diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c > index 2c49e33851b2..14d7f93e4584 100644 > --- a/drivers/cxl/core/mbox.c > +++ b/drivers/cxl/core/mbox.c > @@ -1484,24 +1484,31 @@ int cxl_mailbox_init(struct cxl_mailbox *cxl_mbox, struct device *host) > } > EXPORT_SYMBOL_NS_GPL(cxl_mailbox_init, "CXL"); > > +static void cxl_memdev_state_destroy(void *cxlmds) > +{ > + kfree(cxlmds); > +} > + > struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev, u64 serial, > u16 dvsec) > { > struct cxl_memdev_state *mds; > int rc; > > - mds = devm_kzalloc(dev, sizeof(*mds), GFP_KERNEL); > + mds = cxl_dev_state_create(dev, serial, dvsec, > + CXL_DEVTYPE_CLASSMEM, > + struct cxl_memdev_state, > + cxlds); > if (!mds) { > dev_err(dev, "No memory available\n"); > return ERR_PTR(-ENOMEM); > } > > + rc = devm_add_action_or_reset(dev, cxl_memdev_state_destroy, mds); > + if (rc) > + return ERR_PTR(rc); > + > mutex_init(&mds->event.log_lock); > - mds->cxlds.dev = dev; > - mds->cxlds.reg_map.host = dev; > - mds->cxlds.cxl_mbox.host = dev; > - mds->cxlds.reg_map.resource = CXL_RESOURCE_NONE; > - mds->cxlds.type = CXL_DEVTYPE_CLASSMEM; > > > Looking at the above, it occurs to me that it may be good to either: > > a) Have a devm version of cxl_dev_state_create() or, > b) Have an init macro (in addition) that takes a pointer so that you can use an > allocator of your choice The caller can always wrap the result with devm_add_action_or_reset(). No need to force comsumer drivers into an allocation regime.
On 3/3/25 2:51 PM, Dan Williams wrote: > Ben Cheatham wrote: >> On 2/20/25 2:00 PM, alejandro.lucero-palau@amd.com wrote: >>> From: Alejandro Lucero <alucerop@amd.com> >>> >>> Differentiate CXL memory expanders (type 3) from CXL device accelerators >>> (type 2) with a new function for initializing cxl_dev_state and a macro >>> for helping accel drivers to embed cxl_dev_state inside a private >>> struct. >>> >>> Move structs to include/cxl as the size of the accel driver private >>> struct embedding cxl_dev_state needs to know the size of this struct. >>> >>> Signed-off-by: Alejandro Lucero <alucerop@amd.com> > [..] >> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c >> index 2c49e33851b2..14d7f93e4584 100644 >> --- a/drivers/cxl/core/mbox.c >> +++ b/drivers/cxl/core/mbox.c >> @@ -1484,24 +1484,31 @@ int cxl_mailbox_init(struct cxl_mailbox *cxl_mbox, struct device *host) >> } >> EXPORT_SYMBOL_NS_GPL(cxl_mailbox_init, "CXL"); >> >> +static void cxl_memdev_state_destroy(void *cxlmds) >> +{ >> + kfree(cxlmds); >> +} >> + >> struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev, u64 serial, >> u16 dvsec) >> { >> struct cxl_memdev_state *mds; >> int rc; >> >> - mds = devm_kzalloc(dev, sizeof(*mds), GFP_KERNEL); >> + mds = cxl_dev_state_create(dev, serial, dvsec, >> + CXL_DEVTYPE_CLASSMEM, >> + struct cxl_memdev_state, >> + cxlds); >> if (!mds) { >> dev_err(dev, "No memory available\n"); >> return ERR_PTR(-ENOMEM); >> } >> >> + rc = devm_add_action_or_reset(dev, cxl_memdev_state_destroy, mds); >> + if (rc) >> + return ERR_PTR(rc); >> + >> mutex_init(&mds->event.log_lock); >> - mds->cxlds.dev = dev; >> - mds->cxlds.reg_map.host = dev; >> - mds->cxlds.cxl_mbox.host = dev; >> - mds->cxlds.reg_map.resource = CXL_RESOURCE_NONE; >> - mds->cxlds.type = CXL_DEVTYPE_CLASSMEM; >> >> >> Looking at the above, it occurs to me that it may be good to either: >> >> a) Have a devm version of cxl_dev_state_create() or, >> b) Have an init macro (in addition) that takes a pointer so that you can use an >> allocator of your choice > > The caller can always wrap the result with devm_add_action_or_reset(). > No need to force comsumer drivers into an allocation regime. Sounds good to me, figured I would suggest it more as a convenience option than anything. Thanks, Ben
Hi Dan, Thanks for the review. My comments below. On 3/3/25 20:15, Dan Williams wrote: > alejandro.lucero-palau@ wrote: >> From: Alejandro Lucero <alucerop@amd.com> >> >> Differentiate CXL memory expanders (type 3) from CXL device accelerators >> (type 2) with a new function for initializing cxl_dev_state and a macro >> for helping accel drivers to embed cxl_dev_state inside a private >> struct. >> >> Move structs to include/cxl as the size of the accel driver private >> struct embedding cxl_dev_state needs to know the size of this struct. >> >> Signed-off-by: Alejandro Lucero <alucerop@amd.com> >> --- >> drivers/cxl/core/mbox.c | 3 +- >> drivers/cxl/core/memdev.c | 25 +++++ >> drivers/cxl/core/pci.c | 1 + >> drivers/cxl/core/regs.c | 1 + >> drivers/cxl/cxl.h | 98 +---------------- >> drivers/cxl/cxlmem.h | 108 +----------------- >> drivers/cxl/cxlpci.h | 21 ---- >> drivers/cxl/pci.c | 17 +-- >> include/cxl/cxl.h | 225 ++++++++++++++++++++++++++++++++++++++ >> include/cxl/pci.h | 23 ++++ >> 10 files changed, 293 insertions(+), 229 deletions(-) >> create mode 100644 include/cxl/cxl.h >> create mode 100644 include/cxl/pci.h >> >> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c >> index c5eedcae3b02..ae92db38a921 100644 >> --- a/drivers/cxl/core/mbox.c >> +++ b/drivers/cxl/core/mbox.c >> @@ -1426,7 +1426,8 @@ int cxl_mailbox_init(struct cxl_mailbox *cxl_mbox, struct device *host) >> } >> EXPORT_SYMBOL_NS_GPL(cxl_mailbox_init, "CXL"); >> >> -struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev) >> +struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev, u64 serial, >> + u16 dvsec) >> { >> struct cxl_memdev_state *mds; > I would ultimately expect that this starts to use the > cxl_dev_state_create() internally as well to keep those internals > common. > OK. I think I can do this for the impending v11. >> >> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c >> index 2914e3ff104b..14bd74823467 100644 >> --- a/drivers/cxl/core/memdev.c >> +++ b/drivers/cxl/core/memdev.c >> @@ -636,6 +636,31 @@ static void detach_memdev(struct work_struct *work) >> >> static struct lock_class_key cxl_memdev_key; >> >> +void cxl_dev_state_init(struct cxl_dev_state *cxlds, struct device *dev, >> + enum cxl_devtype type, u64 serial, u16 dvsec) >> +{ >> + cxlds->dev = dev; >> + cxlds->type = type; >> + cxlds->serial = serial; >> + cxlds->cxl_dvsec = dvsec; >> + cxlds->reg_map.host = dev; >> + cxlds->reg_map.resource = CXL_RESOURCE_NONE; > Let's take the opportunity to now use compound literal syntax which > immediately answers the question of whether any new or not included > fields are initialized. I.e. > > *cxlds = (struct cxl_dev_state) { > .dev = dev, > ... > }; > OK. >> +} >> + >> +struct cxl_dev_state *_cxl_dev_state_create(struct device *dev, >> + u64 serial, u16 dvsec, >> + size_t size) >> +{ >> + struct cxl_dev_state *cxlds __free(kfree) = kzalloc(size, GFP_KERNEL); >> + >> + if (!cxlds) >> + return NULL; >> + >> + cxl_dev_state_init(cxlds, dev, CXL_DEVTYPE_DEVMEM, serial, dvsec); > I expect the devtype will need to be passed in as _cxl_dev_state_create > can not assume memory expander vs an accelerator. Yes, this is needed for the work to do related to the first comment. I'll do that. <snip> > Ok, all of the above make sense to move because they are needed to be > able to export the size. It would be nice for the follow on patches to > delineate which fields of 'struct cxl_dev_state' are expected to be > private to the CXL core. > I'll do so. <snip> >> >> -/** >> - * struct cxl_memdev - CXL bus object representing a Type-3 Memory Device >> - * @dev: driver core device object >> - * @cdev: char dev core object for ioctl operations >> - * @cxlds: The device state backing this device >> - * @detach_work: active memdev lost a port in its ancestry >> - * @cxl_nvb: coordinate removal of @cxl_nvd if present >> - * @cxl_nvd: optional bridge to an nvdimm if the device supports pmem >> - * @endpoint: connection to the CXL port topology for this memory device >> - * @id: id number of this memdev instance. >> - * @depth: endpoint port depth >> - */ >> -struct cxl_memdev { >> - struct device dev; >> - struct cdev cdev; >> - struct cxl_dev_state *cxlds; >> - struct work_struct detach_work; >> - struct cxl_nvdimm_bridge *cxl_nvb; >> - struct cxl_nvdimm *cxl_nvd; >> - struct cxl_port *endpoint; >> - int id; >> - int depth; >> -}; > Why does this need to be public? Good catch. This comes from the previous try for using this in v10. It is not needed for what the RFC tries to do. <snip> > Looks good. I think I got what I need now for v11. Thanks!
diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c index c5eedcae3b02..ae92db38a921 100644 --- a/drivers/cxl/core/mbox.c +++ b/drivers/cxl/core/mbox.c @@ -1426,7 +1426,8 @@ int cxl_mailbox_init(struct cxl_mailbox *cxl_mbox, struct device *host) } EXPORT_SYMBOL_NS_GPL(cxl_mailbox_init, "CXL"); -struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev) +struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev, u64 serial, + u16 dvsec) { struct cxl_memdev_state *mds; diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c index 2914e3ff104b..14bd74823467 100644 --- a/drivers/cxl/core/memdev.c +++ b/drivers/cxl/core/memdev.c @@ -636,6 +636,31 @@ static void detach_memdev(struct work_struct *work) static struct lock_class_key cxl_memdev_key; +void cxl_dev_state_init(struct cxl_dev_state *cxlds, struct device *dev, + enum cxl_devtype type, u64 serial, u16 dvsec) +{ + cxlds->dev = dev; + cxlds->type = type; + cxlds->serial = serial; + cxlds->cxl_dvsec = dvsec; + cxlds->reg_map.host = dev; + cxlds->reg_map.resource = CXL_RESOURCE_NONE; +} + +struct cxl_dev_state *_cxl_dev_state_create(struct device *dev, + u64 serial, u16 dvsec, + size_t size) +{ + struct cxl_dev_state *cxlds __free(kfree) = kzalloc(size, GFP_KERNEL); + + if (!cxlds) + return NULL; + + cxl_dev_state_init(cxlds, dev, CXL_DEVTYPE_DEVMEM, serial, dvsec); + return_ptr(cxlds); +} +EXPORT_SYMBOL_NS_GPL(_cxl_dev_state_create, "CXL"); + static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds, const struct file_operations *fops) { diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c index a5c65f79db18..36a1cf4e59fb 100644 --- a/drivers/cxl/core/pci.c +++ b/drivers/cxl/core/pci.c @@ -1,5 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only /* Copyright(c) 2021 Intel Corporation. All rights reserved. */ +#include <cxl/pci.h> #include <linux/units.h> #include <linux/io-64-nonatomic-lo-hi.h> #include <linux/device.h> diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c index 117c2e94c761..58a942a4946c 100644 --- a/drivers/cxl/core/regs.c +++ b/drivers/cxl/core/regs.c @@ -4,6 +4,7 @@ #include <linux/device.h> #include <linux/slab.h> #include <linux/pci.h> +#include <cxl/pci.h> #include <cxlmem.h> #include <cxlpci.h> #include <pmu.h> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h index 6baec4ba9141..556b1ed24f0e 100644 --- a/drivers/cxl/cxl.h +++ b/drivers/cxl/cxl.h @@ -9,8 +9,8 @@ #include <linux/notifier.h> #include <linux/bitops.h> #include <linux/log2.h> -#include <linux/node.h> #include <linux/io.h> +#include <cxl/cxl.h> extern const struct nvdimm_security_ops *cxl_security_ops; @@ -200,97 +200,6 @@ static inline int ways_to_eiw(unsigned int ways, u8 *eiw) #define CXLDEV_MBOX_BG_CMD_COMMAND_VENDOR_MASK GENMASK_ULL(63, 48) #define CXLDEV_MBOX_PAYLOAD_OFFSET 0x20 -/* - * Using struct_group() allows for per register-block-type helper routines, - * without requiring block-type agnostic code to include the prefix. - */ -struct cxl_regs { - /* - * Common set of CXL Component register block base pointers - * @hdm_decoder: CXL 2.0 8.2.5.12 CXL HDM Decoder Capability Structure - * @ras: CXL 2.0 8.2.5.9 CXL RAS Capability Structure - */ - struct_group_tagged(cxl_component_regs, component, - void __iomem *hdm_decoder; - void __iomem *ras; - ); - /* - * Common set of CXL Device register block base pointers - * @status: CXL 2.0 8.2.8.3 Device Status Registers - * @mbox: CXL 2.0 8.2.8.4 Mailbox Registers - * @memdev: CXL 2.0 8.2.8.5 Memory Device Registers - */ - struct_group_tagged(cxl_device_regs, device_regs, - void __iomem *status, *mbox, *memdev; - ); - - struct_group_tagged(cxl_pmu_regs, pmu_regs, - void __iomem *pmu; - ); - - /* - * RCH downstream port specific RAS register - * @aer: CXL 3.0 8.2.1.1 RCH Downstream Port RCRB - */ - struct_group_tagged(cxl_rch_regs, rch_regs, - void __iomem *dport_aer; - ); - - /* - * RCD upstream port specific PCIe cap register - * @pcie_cap: CXL 3.0 8.2.1.2 RCD Upstream Port RCRB - */ - struct_group_tagged(cxl_rcd_regs, rcd_regs, - void __iomem *rcd_pcie_cap; - ); -}; - -struct cxl_reg_map { - bool valid; - int id; - unsigned long offset; - unsigned long size; -}; - -struct cxl_component_reg_map { - struct cxl_reg_map hdm_decoder; - struct cxl_reg_map ras; -}; - -struct cxl_device_reg_map { - struct cxl_reg_map status; - struct cxl_reg_map mbox; - struct cxl_reg_map memdev; -}; - -struct cxl_pmu_reg_map { - struct cxl_reg_map pmu; -}; - -/** - * struct cxl_register_map - DVSEC harvested register block mapping parameters - * @host: device for devm operations and logging - * @base: virtual base of the register-block-BAR + @block_offset - * @resource: physical resource base of the register block - * @max_size: maximum mapping size to perform register search - * @reg_type: see enum cxl_regloc_type - * @component_map: cxl_reg_map for component registers - * @device_map: cxl_reg_maps for device registers - * @pmu_map: cxl_reg_maps for CXL Performance Monitoring Units - */ -struct cxl_register_map { - struct device *host; - void __iomem *base; - resource_size_t resource; - resource_size_t max_size; - u8 reg_type; - union { - struct cxl_component_reg_map component_map; - struct cxl_device_reg_map device_map; - struct cxl_pmu_reg_map pmu_map; - }; -}; - void cxl_probe_component_regs(struct device *dev, void __iomem *base, struct cxl_component_reg_map *map); void cxl_probe_device_regs(struct device *dev, void __iomem *base, @@ -480,11 +389,6 @@ struct cxl_region_params { int nr_targets; }; -enum cxl_partition_mode { - CXL_PARTMODE_RAM, - CXL_PARTMODE_PMEM, -}; - /* * Indicate whether this region has been assembled by autodetection or * userspace assembly. Prevent endpoint decoders outside of automatic diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h index 8e1e46c348f5..0bdf581044da 100644 --- a/drivers/cxl/cxlmem.h +++ b/drivers/cxl/cxlmem.h @@ -4,9 +4,9 @@ #define __CXL_MEM_H__ #include <uapi/linux/cxl_mem.h> #include <linux/pci.h> -#include <linux/cdev.h> #include <linux/uuid.h> #include <linux/node.h> +#include <cxl/cxl.h> #include <cxl/event.h> #include <cxl/mailbox.h> #include "cxl.h" @@ -34,30 +34,6 @@ (FIELD_GET(CXLMDEV_RESET_NEEDED_MASK, status) != \ CXLMDEV_RESET_NEEDED_NOT) -/** - * struct cxl_memdev - CXL bus object representing a Type-3 Memory Device - * @dev: driver core device object - * @cdev: char dev core object for ioctl operations - * @cxlds: The device state backing this device - * @detach_work: active memdev lost a port in its ancestry - * @cxl_nvb: coordinate removal of @cxl_nvd if present - * @cxl_nvd: optional bridge to an nvdimm if the device supports pmem - * @endpoint: connection to the CXL port topology for this memory device - * @id: id number of this memdev instance. - * @depth: endpoint port depth - */ -struct cxl_memdev { - struct device dev; - struct cdev cdev; - struct cxl_dev_state *cxlds; - struct work_struct detach_work; - struct cxl_nvdimm_bridge *cxl_nvb; - struct cxl_nvdimm *cxl_nvd; - struct cxl_port *endpoint; - int id; - int depth; -}; - static inline struct cxl_memdev *to_cxl_memdev(struct device *dev) { return container_of(dev, struct cxl_memdev, dev); @@ -357,83 +333,6 @@ struct cxl_security_state { struct kernfs_node *sanitize_node; }; -/* - * 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 requirement 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_dpa_perf - DPA performance property entry - * @dpa_range: range for DPA address - * @coord: QoS performance data (i.e. latency, bandwidth) - * @cdat_coord: raw QoS performance data from CDAT - * @qos_class: QoS Class cookies - */ -struct cxl_dpa_perf { - struct range dpa_range; - struct access_coordinate coord[ACCESS_COORDINATE_MAX]; - struct access_coordinate cdat_coord[ACCESS_COORDINATE_MAX]; - int qos_class; -}; - -/** - * struct cxl_dpa_partition - DPA partition descriptor - * @res: shortcut to the partition in the DPA resource tree (cxlds->dpa_res) - * @perf: performance attributes of the partition from CDAT - * @mode: operation mode for the DPA capacity, e.g. ram, pmem, dynamic... - */ -struct cxl_dpa_partition { - struct resource res; - struct cxl_dpa_perf perf; - enum cxl_partition_mode mode; -}; - -/** - * struct cxl_dev_state - The driver device state - * - * cxl_dev_state represents the CXL driver/device state. It provides an - * interface to mailbox commands as well as some cached data about the device. - * Currently only memory devices are represented. - * - * @dev: The device associated with this CXL state - * @cxlmd: The device representing the CXL.mem capabilities of @dev - * @reg_map: component and ras register mapping parameters - * @regs: Parsed register blocks - * @cxl_dvsec: Offset to the PCIe device DVSEC - * @rcd: operating in RCD mode (CXL 3.0 9.11.8 CXL Devices Attached to an RCH) - * @media_ready: Indicate whether the device media is usable - * @dpa_res: Overall DPA resource tree for the device - * @part: DPA partition array - * @nr_partitions: Number of DPA partitions - * @serial: PCIe Device Serial Number - * @type: Generic Memory Class device or Vendor Specific Memory device - * @cxl_mbox: CXL mailbox context - */ -struct cxl_dev_state { - struct device *dev; - struct cxl_memdev *cxlmd; - struct cxl_register_map reg_map; - struct cxl_regs regs; - int cxl_dvsec; - bool rcd; - bool media_ready; - struct resource dpa_res; - struct cxl_dpa_partition part[CXL_NR_PARTITIONS_MAX]; - unsigned int nr_partitions; - u64 serial; - enum cxl_devtype type; - struct cxl_mailbox cxl_mbox; -}; - static inline resource_size_t cxl_pmem_size(struct cxl_dev_state *cxlds) { /* @@ -812,7 +711,10 @@ int cxl_dev_state_identify(struct cxl_memdev_state *mds); int cxl_await_media_ready(struct cxl_dev_state *cxlds); int cxl_enumerate_cmds(struct cxl_memdev_state *mds); int cxl_mem_dpa_fetch(struct cxl_memdev_state *mds, struct cxl_dpa_info *info); -struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev); +struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev, u64 serial, + u16 dvsec); +void cxl_dev_state_init(struct cxl_dev_state *cxlds, struct device *dev, + enum cxl_devtype type, u64 serial, u16 dvsec); void set_exclusive_cxl_commands(struct cxl_memdev_state *mds, unsigned long *cmds); void clear_exclusive_cxl_commands(struct cxl_memdev_state *mds, diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h index 54e219b0049e..570e53e26f11 100644 --- a/drivers/cxl/cxlpci.h +++ b/drivers/cxl/cxlpci.h @@ -7,29 +7,8 @@ #define CXL_MEMORY_PROGIF 0x10 -/* - * See section 8.1 Configuration Space Registers in the CXL 2.0 - * Specification. Names are taken straight from the specification with "CXL" and - * "DVSEC" redundancies removed. When obvious, abbreviations may be used. - */ #define PCI_DVSEC_HEADER1_LENGTH_MASK GENMASK(31, 20) -/* CXL 2.0 8.1.3: PCIe DVSEC for CXL Device */ -#define CXL_DVSEC_PCIE_DEVICE 0 -#define CXL_DVSEC_CAP_OFFSET 0xA -#define CXL_DVSEC_MEM_CAPABLE BIT(2) -#define CXL_DVSEC_HDM_COUNT_MASK GENMASK(5, 4) -#define CXL_DVSEC_CTRL_OFFSET 0xC -#define CXL_DVSEC_MEM_ENABLE BIT(2) -#define CXL_DVSEC_RANGE_SIZE_HIGH(i) (0x18 + (i * 0x10)) -#define CXL_DVSEC_RANGE_SIZE_LOW(i) (0x1C + (i * 0x10)) -#define CXL_DVSEC_MEM_INFO_VALID BIT(0) -#define CXL_DVSEC_MEM_ACTIVE BIT(1) -#define CXL_DVSEC_MEM_SIZE_LOW_MASK GENMASK(31, 28) -#define CXL_DVSEC_RANGE_BASE_HIGH(i) (0x20 + (i * 0x10)) -#define CXL_DVSEC_RANGE_BASE_LOW(i) (0x24 + (i * 0x10)) -#define CXL_DVSEC_MEM_BASE_LOW_MASK GENMASK(31, 28) - #define CXL_DVSEC_RANGE_MAX 2 /* CXL 2.0 8.1.4: Non-CXL Function Map DVSEC */ diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c index b2c943a4de0a..006a0036e779 100644 --- a/drivers/cxl/pci.c +++ b/drivers/cxl/pci.c @@ -1,5 +1,7 @@ // SPDX-License-Identifier: GPL-2.0-only /* Copyright(c) 2020 Intel Corporation. All rights reserved. */ +#include <cxl/cxl.h> +#include <cxl/pci.h> #include <linux/unaligned.h> #include <linux/io-64-nonatomic-lo-hi.h> #include <linux/moduleparam.h> @@ -911,6 +913,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) int rc, pmu_count; unsigned int i; bool irq_avail; + u16 dvsec; /* * Double check the anonymous union trickery in struct cxl_regs @@ -924,19 +927,19 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) return rc; pci_set_master(pdev); - mds = cxl_memdev_state_create(&pdev->dev); + dvsec = pci_find_dvsec_capability(pdev, PCI_VENDOR_ID_CXL, + CXL_DVSEC_PCIE_DEVICE); + if (!dvsec) + dev_warn(&pdev->dev, + "Device DVSEC not present, skip CXL.mem init\n"); + + mds = cxl_memdev_state_create(&pdev->dev, pci_get_dsn(pdev), dvsec); if (IS_ERR(mds)) return PTR_ERR(mds); cxlds = &mds->cxlds; pci_set_drvdata(pdev, cxlds); cxlds->rcd = is_cxl_restricted(pdev); - cxlds->serial = pci_get_dsn(pdev); - cxlds->cxl_dvsec = pci_find_dvsec_capability( - pdev, PCI_VENDOR_ID_CXL, CXL_DVSEC_PCIE_DEVICE); - if (!cxlds->cxl_dvsec) - dev_warn(&pdev->dev, - "Device DVSEC not present, skip CXL.mem init\n"); rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map); if (rc) diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h new file mode 100644 index 000000000000..dc8a68104bfa --- /dev/null +++ b/include/cxl/cxl.h @@ -0,0 +1,225 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* Copyright(c) 2025 Advanced Micro Devices, Inc. */ + +#ifndef __CXL_H +#define __CXL_H + +#include <linux/cdev.h> +#include <linux/node.h> +#include <linux/ioport.h> +#include <cxl/mailbox.h> + +/* + * 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 requirement 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 device; + +/* + * Using struct_group() allows for per register-block-type helper routines, + * without requiring block-type agnostic code to include the prefix. + */ +struct cxl_regs { + /* + * Common set of CXL Component register block base pointers + * @hdm_decoder: CXL 2.0 8.2.5.12 CXL HDM Decoder Capability Structure + * @ras: CXL 2.0 8.2.5.9 CXL RAS Capability Structure + */ + struct_group_tagged(cxl_component_regs, component, + void __iomem *hdm_decoder; + void __iomem *ras; + ); + /* + * Common set of CXL Device register block base pointers + * @status: CXL 2.0 8.2.8.3 Device Status Registers + * @mbox: CXL 2.0 8.2.8.4 Mailbox Registers + * @memdev: CXL 2.0 8.2.8.5 Memory Device Registers + */ + struct_group_tagged(cxl_device_regs, device_regs, + void __iomem *status, *mbox, *memdev; + ); + + struct_group_tagged(cxl_pmu_regs, pmu_regs, + void __iomem *pmu; + ); + + /* + * RCH downstream port specific RAS register + * @aer: CXL 3.0 8.2.1.1 RCH Downstream Port RCRB + */ + struct_group_tagged(cxl_rch_regs, rch_regs, + void __iomem *dport_aer; + ); + + /* + * RCD upstream port specific PCIe cap register + * @pcie_cap: CXL 3.0 8.2.1.2 RCD Upstream Port RCRB + */ + struct_group_tagged(cxl_rcd_regs, rcd_regs, + void __iomem *rcd_pcie_cap; + ); +}; + +struct cxl_reg_map { + bool valid; + int id; + unsigned long offset; + unsigned long size; +}; + +struct cxl_component_reg_map { + struct cxl_reg_map hdm_decoder; + struct cxl_reg_map ras; +}; + +struct cxl_device_reg_map { + struct cxl_reg_map status; + struct cxl_reg_map mbox; + struct cxl_reg_map memdev; +}; + +struct cxl_pmu_reg_map { + struct cxl_reg_map pmu; +}; + +/** + * struct cxl_register_map - DVSEC harvested register block mapping parameters + * @host: device for devm operations and logging + * @base: virtual base of the register-block-BAR + @block_offset + * @resource: physical resource base of the register block + * @max_size: maximum mapping size to perform register search + * @reg_type: see enum cxl_regloc_type + * @component_map: cxl_reg_map for component registers + * @device_map: cxl_reg_maps for device registers + * @pmu_map: cxl_reg_maps for CXL Performance Monitoring Units + */ +struct cxl_register_map { + struct device *host; + void __iomem *base; + resource_size_t resource; + resource_size_t max_size; + u8 reg_type; + union { + struct cxl_component_reg_map component_map; + struct cxl_device_reg_map device_map; + struct cxl_pmu_reg_map pmu_map; + }; +}; + +/** + * struct cxl_dpa_perf - DPA performance property entry + * @dpa_range: range for DPA address + * @coord: QoS performance data (i.e. latency, bandwidth) + * @cdat_coord: raw QoS performance data from CDAT + * @qos_class: QoS Class cookies + */ +struct cxl_dpa_perf { + struct range dpa_range; + struct access_coordinate coord[ACCESS_COORDINATE_MAX]; + struct access_coordinate cdat_coord[ACCESS_COORDINATE_MAX]; + int qos_class; +}; + +enum cxl_partition_mode { + CXL_PARTMODE_RAM, + CXL_PARTMODE_PMEM, +}; + +/** + * struct cxl_dpa_partition - DPA partition descriptor + * @res: shortcut to the partition in the DPA resource tree (cxlds->dpa_res) + * @perf: performance attributes of the partition from CDAT + * @mode: operation mode for the DPA capacity, e.g. ram, pmem, dynamic... + */ +struct cxl_dpa_partition { + struct resource res; + struct cxl_dpa_perf perf; + enum cxl_partition_mode mode; +}; + +/** + * struct cxl_memdev - CXL bus object representing a Type-3 Memory Device + * @dev: driver core device object + * @cdev: char dev core object for ioctl operations + * @cxlds: The device state backing this device + * @detach_work: active memdev lost a port in its ancestry + * @cxl_nvb: coordinate removal of @cxl_nvd if present + * @cxl_nvd: optional bridge to an nvdimm if the device supports pmem + * @endpoint: connection to the CXL port topology for this memory device + * @id: id number of this memdev instance. + * @depth: endpoint port depth + */ +struct cxl_memdev { + struct device dev; + struct cdev cdev; + struct cxl_dev_state *cxlds; + struct work_struct detach_work; + struct cxl_nvdimm_bridge *cxl_nvb; + struct cxl_nvdimm *cxl_nvd; + struct cxl_port *endpoint; + int id; + int depth; +}; + +#define CXL_NR_PARTITIONS_MAX 2 + +/** + * struct cxl_dev_state - The driver device state + * + * cxl_dev_state represents the CXL driver/device state. It provides an + * interface to mailbox commands as well as some cached data about the device. + * Currently only memory devices are represented. + * + * @dev: The device associated with this CXL state + * @cxlmd: The device representing the CXL.mem capabilities of @dev + * @reg_map: component and ras register mapping parameters + * @regs: Parsed register blocks + * @cxl_dvsec: Offset to the PCIe device DVSEC + * @rcd: operating in RCD mode (CXL 3.0 9.11.8 CXL Devices Attached to an RCH) + * @media_ready: Indicate whether the device media is usable + * @dpa_res: Overall DPA resource tree for the device + * @part: DPA partition array + * @nr_partitions: Number of DPA partitions + * @serial: PCIe Device Serial Number + * @type: Generic Memory Class device or Vendor Specific Memory device + * @cxl_mbox: CXL mailbox context + */ +struct cxl_dev_state { + struct device *dev; + struct cxl_memdev *cxlmd; + struct cxl_register_map reg_map; + struct cxl_regs regs; + int cxl_dvsec; + bool rcd; + bool media_ready; + struct resource dpa_res; + struct cxl_dpa_partition part[CXL_NR_PARTITIONS_MAX]; + unsigned int nr_partitions; + u64 serial; + enum cxl_devtype type; + struct cxl_mailbox cxl_mbox; +}; + +struct cxl_dev_state *_cxl_dev_state_create(struct device *dev, + u64 serial, u16 dvsec, + size_t size); + +#define cxl_dev_state_create(parent, serial, dvsec, drv_struct, member) \ + ({ \ + static_assert(__same_type(struct cxl_dev_state, \ + ((drv_struct *)NULL)->member)); \ + static_assert(offsetof(drv_struct, member) == 0); \ + (drv_struct *)_cxl_dev_state_create(parent, serial, dvsec, \ + sizeof(drv_struct));\ + }) +#endif diff --git a/include/cxl/pci.h b/include/cxl/pci.h new file mode 100644 index 000000000000..ad63560caa2c --- /dev/null +++ b/include/cxl/pci.h @@ -0,0 +1,23 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* Copyright(c) 2020 Intel Corporation. All rights reserved. */ + +#ifndef __CXL_ACCEL_PCI_H +#define __CXL_ACCEL_PCI_H + +/* CXL 2.0 8.1.3: PCIe DVSEC for CXL Device */ +#define CXL_DVSEC_PCIE_DEVICE 0 +#define CXL_DVSEC_CAP_OFFSET 0xA +#define CXL_DVSEC_MEM_CAPABLE BIT(2) +#define CXL_DVSEC_HDM_COUNT_MASK GENMASK(5, 4) +#define CXL_DVSEC_CTRL_OFFSET 0xC +#define CXL_DVSEC_MEM_ENABLE BIT(2) +#define CXL_DVSEC_RANGE_SIZE_HIGH(i) (0x18 + ((i) * 0x10)) +#define CXL_DVSEC_RANGE_SIZE_LOW(i) (0x1C + ((i) * 0x10)) +#define CXL_DVSEC_MEM_INFO_VALID BIT(0) +#define CXL_DVSEC_MEM_ACTIVE BIT(1) +#define CXL_DVSEC_MEM_SIZE_LOW_MASK GENMASK(31, 28) +#define CXL_DVSEC_RANGE_BASE_HIGH(i) (0x20 + ((i) * 0x10)) +#define CXL_DVSEC_RANGE_BASE_LOW(i) (0x24 + ((i) * 0x10)) +#define CXL_DVSEC_MEM_BASE_LOW_MASK GENMASK(31, 28) + +#endif