| Message ID | 20240715172835.24757-5-alejandro.lucero-palau@amd.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | cxl: add Type2 device support | expand |
On 7/15/24 10:28 AM, alejandro.lucero-palau@amd.com wrote: > From: Alejandro Lucero <alucerop@amd.com> > > Type2 devices have some Type3 functionalities as optional like an mbox > or an hdm decoder, and CXL core needs a way to know what a CXL accelerator > implements. > > Add a new field for keeping device capabilities to be initialised by > Type2 drivers. Advertise all those capabilities for Type3. > > Signed-off-by: Alejandro Lucero <alucerop@amd.com> > --- > drivers/cxl/core/mbox.c | 1 + > drivers/cxl/core/memdev.c | 4 +++- > drivers/cxl/core/port.c | 2 +- > drivers/cxl/core/regs.c | 11 ++++++----- > drivers/cxl/cxl.h | 2 +- > drivers/cxl/cxlmem.h | 4 ++++ > drivers/cxl/pci.c | 15 +++++++++------ > drivers/net/ethernet/sfc/efx_cxl.c | 3 ++- > include/linux/cxl_accel_mem.h | 5 ++++- > 9 files changed, 31 insertions(+), 16 deletions(-) > > diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c > index 2626f3fff201..2ba7d36e3f38 100644 > --- a/drivers/cxl/core/mbox.c > +++ b/drivers/cxl/core/mbox.c > @@ -1424,6 +1424,7 @@ struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev) > mds->cxlds.reg_map.host = dev; > mds->cxlds.reg_map.resource = CXL_RESOURCE_NONE; > mds->cxlds.type = CXL_DEVTYPE_CLASSMEM; > + mds->cxlds.capabilities = CXL_DRIVER_CAP_HDM | CXL_DRIVER_CAP_MBOX; > mds->ram_perf.qos_class = CXL_QOS_CLASS_INVALID; > mds->pmem_perf.qos_class = CXL_QOS_CLASS_INVALID; > > diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c > index 04c3a0f8bc2e..b4205ecca365 100644 > --- a/drivers/cxl/core/memdev.c > +++ b/drivers/cxl/core/memdev.c > @@ -616,7 +616,7 @@ static void detach_memdev(struct work_struct *work) > > static struct lock_class_key cxl_memdev_key; > > -struct cxl_dev_state *cxl_accel_state_create(struct device *dev) > +struct cxl_dev_state *cxl_accel_state_create(struct device *dev, uint8_t caps) > { > struct cxl_dev_state *cxlds; > > @@ -631,6 +631,8 @@ struct cxl_dev_state *cxl_accel_state_create(struct device *dev) > cxlds->ram_res = DEFINE_RES_MEM_NAMED(0, 0, "ram"); > cxlds->pmem_res = DEFINE_RES_MEM_NAMED(0, 0, "pmem"); > > + cxlds->capabilities = caps; > + > return cxlds; > } > EXPORT_SYMBOL_NS_GPL(cxl_accel_state_create, CXL); > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c > index 887ed6e358fb..d66c6349ed2d 100644 > --- a/drivers/cxl/core/port.c > +++ b/drivers/cxl/core/port.c > @@ -763,7 +763,7 @@ static int cxl_setup_comp_regs(struct device *host, struct cxl_register_map *map > map->reg_type = CXL_REGLOC_RBI_COMPONENT; > map->max_size = CXL_COMPONENT_REG_BLOCK_SIZE; > > - return cxl_setup_regs(map); > + return cxl_setup_regs(map, 0); > } > > static int cxl_port_setup_regs(struct cxl_port *port, > diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c > index e1082e749c69..9d218ebe180d 100644 > --- a/drivers/cxl/core/regs.c > +++ b/drivers/cxl/core/regs.c > @@ -421,7 +421,7 @@ static void cxl_unmap_regblock(struct cxl_register_map *map) > map->base = NULL; > } > > -static int cxl_probe_regs(struct cxl_register_map *map) > +static int cxl_probe_regs(struct cxl_register_map *map, uint8_t caps) > { > struct cxl_component_reg_map *comp_map; > struct cxl_device_reg_map *dev_map; > @@ -437,11 +437,12 @@ static int cxl_probe_regs(struct cxl_register_map *map) > case CXL_REGLOC_RBI_MEMDEV: > dev_map = &map->device_map; > cxl_probe_device_regs(host, base, dev_map); > - if (!dev_map->status.valid || !dev_map->mbox.valid || > + if (!dev_map->status.valid || > + ((caps & CXL_DRIVER_CAP_MBOX) && !dev_map->mbox.valid) || > !dev_map->memdev.valid) { > dev_err(host, "registers not found: %s%s%s\n", > !dev_map->status.valid ? "status " : "", > - !dev_map->mbox.valid ? "mbox " : "", > + ((caps & CXL_DRIVER_CAP_MBOX) && !dev_map->mbox.valid) ? "mbox " : "", According to the r3.1 8.2.8.2.1, the device status registers and the primary mailbox registers are both mandatory if regloc id=3 block is found. So if the type2 device does not implement a mailbox then it shouldn't be calling cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map) to begin with from the driver init right? If the type2 device defines a regblock with id=3 but without a mailbox, then isn't that a spec violation? DJ > !dev_map->memdev.valid ? "memdev " : ""); > return -ENXIO; > } > @@ -455,7 +456,7 @@ static int cxl_probe_regs(struct cxl_register_map *map) > return 0; > } > > -int cxl_setup_regs(struct cxl_register_map *map) > +int cxl_setup_regs(struct cxl_register_map *map, uint8_t caps) > { > int rc; > > @@ -463,7 +464,7 @@ int cxl_setup_regs(struct cxl_register_map *map) > if (rc) > return rc; > > - rc = cxl_probe_regs(map); > + rc = cxl_probe_regs(map, caps); > cxl_unmap_regblock(map); > > return rc; > diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h > index a6613a6f8923..9973430d975f 100644 > --- a/drivers/cxl/cxl.h > +++ b/drivers/cxl/cxl.h > @@ -300,7 +300,7 @@ int cxl_find_regblock_instance(struct pci_dev *pdev, enum cxl_regloc_type type, > struct cxl_register_map *map, int index); > int cxl_find_regblock(struct pci_dev *pdev, enum cxl_regloc_type type, > struct cxl_register_map *map); > -int cxl_setup_regs(struct cxl_register_map *map); > +int cxl_setup_regs(struct cxl_register_map *map, uint8_t caps); > struct cxl_dport; > resource_size_t cxl_rcd_component_reg_phys(struct device *dev, > struct cxl_dport *dport); > diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h > index af8169ccdbc0..8f2a820bd92d 100644 > --- a/drivers/cxl/cxlmem.h > +++ b/drivers/cxl/cxlmem.h > @@ -405,6 +405,9 @@ struct cxl_dpa_perf { > int qos_class; > }; > > +#define CXL_DRIVER_CAP_HDM 0x1 > +#define CXL_DRIVER_CAP_MBOX 0x2 > + > /** > * struct cxl_dev_state - The driver device state > * > @@ -438,6 +441,7 @@ struct cxl_dev_state { > struct resource ram_res; > u64 serial; > enum cxl_devtype type; > + uint8_t capabilities; > }; > > /** > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c > index b34d6259faf4..e2a978312281 100644 > --- a/drivers/cxl/pci.c > +++ b/drivers/cxl/pci.c > @@ -502,7 +502,8 @@ static int cxl_rcrb_get_comp_regs(struct pci_dev *pdev, > } > > static int cxl_pci_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type type, > - struct cxl_register_map *map) > + struct cxl_register_map *map, > + uint8_t cxl_dev_caps) > { > int rc; > > @@ -519,7 +520,7 @@ static int cxl_pci_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type type, > if (rc) > return rc; > > - return cxl_setup_regs(map); > + return cxl_setup_regs(map, cxl_dev_caps); > } > > int cxl_pci_accel_setup_regs(struct pci_dev *pdev, struct cxl_dev_state *cxlds) > @@ -527,7 +528,8 @@ int cxl_pci_accel_setup_regs(struct pci_dev *pdev, struct cxl_dev_state *cxlds) > struct cxl_register_map map; > int rc; > > - rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map); > + rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map, > + cxlds->capabilities); > if (rc) > return rc; > > @@ -536,7 +538,7 @@ int cxl_pci_accel_setup_regs(struct pci_dev *pdev, struct cxl_dev_state *cxlds) > return rc; > > rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_COMPONENT, > - &cxlds->reg_map); > + &cxlds->reg_map, cxlds->capabilities); > if (rc) > dev_warn(&pdev->dev, "No component registers (%d)\n", rc); > > @@ -850,7 +852,8 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) > dev_warn(&pdev->dev, > "Device DVSEC not present, skip CXL.mem init\n"); > > - rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map); > + rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map, > + cxlds->capabilities); > if (rc) > return rc; > > @@ -863,7 +866,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) > * still be useful for management functions so don't return an error. > */ > rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_COMPONENT, > - &cxlds->reg_map); > + &cxlds->reg_map, cxlds->capabilities); > if (rc) > dev_warn(&pdev->dev, "No component registers (%d)\n", rc); > else if (!cxlds->reg_map.component_map.ras.valid) > diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c > index 9cefcaf3caca..37d8bfdef517 100644 > --- a/drivers/net/ethernet/sfc/efx_cxl.c > +++ b/drivers/net/ethernet/sfc/efx_cxl.c > @@ -33,7 +33,8 @@ void efx_cxl_init(struct efx_nic *efx) > > pci_info(pci_dev, "CXL CXL_DVSEC_PCIE_DEVICE capability found"); > > - cxl->cxlds = cxl_accel_state_create(&pci_dev->dev); > + cxl->cxlds = cxl_accel_state_create(&pci_dev->dev, > + CXL_ACCEL_DRIVER_CAP_HDM); > if (IS_ERR(cxl->cxlds)) { > pci_info(pci_dev, "CXL accel device state failed"); > return; > diff --git a/include/linux/cxl_accel_mem.h b/include/linux/cxl_accel_mem.h > index c7b254edc096..0ba2195b919b 100644 > --- a/include/linux/cxl_accel_mem.h > +++ b/include/linux/cxl_accel_mem.h > @@ -12,8 +12,11 @@ enum accel_resource{ > CXL_ACCEL_RES_PMEM, > }; > > +#define CXL_ACCEL_DRIVER_CAP_HDM 0x1 > +#define CXL_ACCEL_DRIVER_CAP_MBOX 0x2 > + > typedef struct cxl_dev_state cxl_accel_state; > -cxl_accel_state *cxl_accel_state_create(struct device *dev); > +cxl_accel_state *cxl_accel_state_create(struct device *dev, uint8_t caps); > > void cxl_accel_set_dvsec(cxl_accel_state *cxlds, u16 dvsec); > void cxl_accel_set_serial(cxl_accel_state *cxlds, u64 serial);
On 7/19/24 20:01, Dave Jiang wrote: > >> >> -static int cxl_probe_regs(struct cxl_register_map *map) >> +static int cxl_probe_regs(struct cxl_register_map *map, uint8_t caps) >> { >> struct cxl_component_reg_map *comp_map; >> struct cxl_device_reg_map *dev_map; >> @@ -437,11 +437,12 @@ static int cxl_probe_regs(struct cxl_register_map *map) >> case CXL_REGLOC_RBI_MEMDEV: >> dev_map = &map->device_map; >> cxl_probe_device_regs(host, base, dev_map); >> - if (!dev_map->status.valid || !dev_map->mbox.valid || >> + if (!dev_map->status.valid || >> + ((caps & CXL_DRIVER_CAP_MBOX) && !dev_map->mbox.valid) || >> !dev_map->memdev.valid) { >> dev_err(host, "registers not found: %s%s%s\n", >> !dev_map->status.valid ? "status " : "", >> - !dev_map->mbox.valid ? "mbox " : "", >> + ((caps & CXL_DRIVER_CAP_MBOX) && !dev_map->mbox.valid) ? "mbox " : "", > According to the r3.1 8.2.8.2.1, the device status registers and the primary mailbox registers are both mandatory if regloc id=3 block is found. So if the type2 device does not implement a mailbox then it shouldn't be calling cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map) to begin with from the driver init right? If the type2 device defines a regblock with id=3 but without a mailbox, then isn't that a spec violation? > > DJ Right. The code needs to support the possibility of a Type2 having a mailbox, and if it is not supported, the rest of the dvsec regs initialization needs to be performed. This is not what the code does now, so I'll fix this. A wider explanation is, for the RFC I used a test driver based on QEMU emulating a Type2 which had a CXL Device Register Interface defined (03h) but not a CXL Device Capability with id 2 for the primary mailbox register, breaking the spec as you spotted. Thanks.
On Mon, 15 Jul 2024 18:28:24 +0100 alejandro.lucero-palau@amd.com wrote: > From: Alejandro Lucero <alucerop@amd.com> > > Type2 devices have some Type3 functionalities as optional like an mbox > or an hdm decoder, and CXL core needs a way to know what a CXL accelerator > implements. > > Add a new field for keeping device capabilities to be initialised by > Type2 drivers. Advertise all those capabilities for Type3. > > Signed-off-by: Alejandro Lucero <alucerop@amd.com> In general seems a reasonable approach, so just minor comments. > --- > drivers/cxl/core/mbox.c | 1 + > drivers/cxl/core/memdev.c | 4 +++- > drivers/cxl/core/port.c | 2 +- > drivers/cxl/core/regs.c | 11 ++++++----- > drivers/cxl/cxl.h | 2 +- > drivers/cxl/cxlmem.h | 4 ++++ > drivers/cxl/pci.c | 15 +++++++++------ > drivers/net/ethernet/sfc/efx_cxl.c | 3 ++- > include/linux/cxl_accel_mem.h | 5 ++++- > 9 files changed, 31 insertions(+), 16 deletions(-) > > diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c > index 2626f3fff201..2ba7d36e3f38 100644 > --- a/drivers/cxl/core/mbox.c > +++ b/drivers/cxl/core/mbox.c > @@ -1424,6 +1424,7 @@ struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev) > mds->cxlds.reg_map.host = dev; > mds->cxlds.reg_map.resource = CXL_RESOURCE_NONE; > mds->cxlds.type = CXL_DEVTYPE_CLASSMEM; > + mds->cxlds.capabilities = CXL_DRIVER_CAP_HDM | CXL_DRIVER_CAP_MBOX; Add a reference for this perhaps. Make it clear that a type3 device must support mailbox and hdm by pointing at requirement for the various structures in a spec reference. > diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h > index af8169ccdbc0..8f2a820bd92d 100644 > --- a/drivers/cxl/cxlmem.h > +++ b/drivers/cxl/cxlmem.h > @@ -405,6 +405,9 @@ struct cxl_dpa_perf { > int qos_class; > }; > > +#define CXL_DRIVER_CAP_HDM 0x1 > +#define CXL_DRIVER_CAP_MBOX 0x2 > + Enum and BIT() for the defines. Avoids someone in future thinking they can define 0x3 to be something. Definitely only one definition as well. Seems reasonable for this to be CXL wide. > /** > * struct cxl_dev_state - The driver device state > * > @@ -438,6 +441,7 @@ struct cxl_dev_state { > struct resource ram_res; > u64 serial; > enum cxl_devtype type; > + uint8_t capabilities; > };
On Mon, 15 Jul 2024 18:28:24 +0100 <alejandro.lucero-palau@amd.com> wrote: > From: Alejandro Lucero <alucerop@amd.com> > > Type2 devices have some Type3 functionalities as optional like an mbox > or an hdm decoder, and CXL core needs a way to know what a CXL > accelerator implements. > > Add a new field for keeping device capabilities to be initialised by > Type2 drivers. Advertise all those capabilities for Type3. > > Signed-off-by: Alejandro Lucero <alucerop@amd.com> > --- > drivers/cxl/core/mbox.c | 1 + > drivers/cxl/core/memdev.c | 4 +++- > drivers/cxl/core/port.c | 2 +- > drivers/cxl/core/regs.c | 11 ++++++----- > drivers/cxl/cxl.h | 2 +- > drivers/cxl/cxlmem.h | 4 ++++ > drivers/cxl/pci.c | 15 +++++++++------ > drivers/net/ethernet/sfc/efx_cxl.c | 3 ++- > include/linux/cxl_accel_mem.h | 5 ++++- > 9 files changed, 31 insertions(+), 16 deletions(-) > > diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c > index 2626f3fff201..2ba7d36e3f38 100644 > --- a/drivers/cxl/core/mbox.c > +++ b/drivers/cxl/core/mbox.c > @@ -1424,6 +1424,7 @@ struct cxl_memdev_state > *cxl_memdev_state_create(struct device *dev) mds->cxlds.reg_map.host > = dev; mds->cxlds.reg_map.resource = CXL_RESOURCE_NONE; > mds->cxlds.type = CXL_DEVTYPE_CLASSMEM; > + mds->cxlds.capabilities = CXL_DRIVER_CAP_HDM | > CXL_DRIVER_CAP_MBOX; mds->ram_perf.qos_class = CXL_QOS_CLASS_INVALID; > mds->pmem_perf.qos_class = CXL_QOS_CLASS_INVALID; > > diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c > index 04c3a0f8bc2e..b4205ecca365 100644 > --- a/drivers/cxl/core/memdev.c > +++ b/drivers/cxl/core/memdev.c > @@ -616,7 +616,7 @@ static void detach_memdev(struct work_struct > *work) > static struct lock_class_key cxl_memdev_key; > > -struct cxl_dev_state *cxl_accel_state_create(struct device *dev) > +struct cxl_dev_state *cxl_accel_state_create(struct device *dev, > uint8_t caps) { > struct cxl_dev_state *cxlds; > > @@ -631,6 +631,8 @@ struct cxl_dev_state > *cxl_accel_state_create(struct device *dev) cxlds->ram_res = > DEFINE_RES_MEM_NAMED(0, 0, "ram"); cxlds->pmem_res = > DEFINE_RES_MEM_NAMED(0, 0, "pmem"); > + cxlds->capabilities = caps; > + > return cxlds; > } > EXPORT_SYMBOL_NS_GPL(cxl_accel_state_create, CXL); > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c > index 887ed6e358fb..d66c6349ed2d 100644 > --- a/drivers/cxl/core/port.c > +++ b/drivers/cxl/core/port.c > @@ -763,7 +763,7 @@ static int cxl_setup_comp_regs(struct device > *host, struct cxl_register_map *map map->reg_type = > CXL_REGLOC_RBI_COMPONENT; map->max_size = > CXL_COMPONENT_REG_BLOCK_SIZE; > - return cxl_setup_regs(map); > + return cxl_setup_regs(map, 0); > } > > static int cxl_port_setup_regs(struct cxl_port *port, > diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c > index e1082e749c69..9d218ebe180d 100644 > --- a/drivers/cxl/core/regs.c > +++ b/drivers/cxl/core/regs.c > @@ -421,7 +421,7 @@ static void cxl_unmap_regblock(struct > cxl_register_map *map) map->base = NULL; > } > > -static int cxl_probe_regs(struct cxl_register_map *map) > +static int cxl_probe_regs(struct cxl_register_map *map, uint8_t caps) > { Can we not use uintxx_t? Just like any other one in the cxl-core. Generally, u{8,16...} are mostly used for kernel programming, and your previous patches use them nicely. Let's use u8 for caps. > struct cxl_component_reg_map *comp_map; > struct cxl_device_reg_map *dev_map; > @@ -437,11 +437,12 @@ static int cxl_probe_regs(struct > cxl_register_map *map) case CXL_REGLOC_RBI_MEMDEV: > dev_map = &map->device_map; > cxl_probe_device_regs(host, base, dev_map); > - if (!dev_map->status.valid || !dev_map->mbox.valid || > + if (!dev_map->status.valid || > + ((caps & CXL_DRIVER_CAP_MBOX) && > !dev_map->mbox.valid) || !dev_map->memdev.valid) { > dev_err(host, "registers not found: > %s%s%s\n", !dev_map->status.valid ? "status " : "", > - !dev_map->mbox.valid ? "mbox " : "", > + ((caps & CXL_DRIVER_CAP_MBOX) && > !dev_map->mbox.valid) ? "mbox " : "", !dev_map->memdev.valid ? > "memdev " : ""); return -ENXIO; > } > @@ -455,7 +456,7 @@ static int cxl_probe_regs(struct cxl_register_map > *map) return 0; > } > > -int cxl_setup_regs(struct cxl_register_map *map) > +int cxl_setup_regs(struct cxl_register_map *map, uint8_t caps) > { > int rc; > > @@ -463,7 +464,7 @@ int cxl_setup_regs(struct cxl_register_map *map) > if (rc) > return rc; > > - rc = cxl_probe_regs(map); > + rc = cxl_probe_regs(map, caps); > cxl_unmap_regblock(map); > > return rc; > diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h > index a6613a6f8923..9973430d975f 100644 > --- a/drivers/cxl/cxl.h > +++ b/drivers/cxl/cxl.h > @@ -300,7 +300,7 @@ int cxl_find_regblock_instance(struct pci_dev > *pdev, enum cxl_regloc_type type, struct cxl_register_map *map, int > index); int cxl_find_regblock(struct pci_dev *pdev, enum > cxl_regloc_type type, struct cxl_register_map *map); > -int cxl_setup_regs(struct cxl_register_map *map); > +int cxl_setup_regs(struct cxl_register_map *map, uint8_t caps); > struct cxl_dport; > resource_size_t cxl_rcd_component_reg_phys(struct device *dev, > struct cxl_dport *dport); > diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h > index af8169ccdbc0..8f2a820bd92d 100644 > --- a/drivers/cxl/cxlmem.h > +++ b/drivers/cxl/cxlmem.h > @@ -405,6 +405,9 @@ struct cxl_dpa_perf { > int qos_class; > }; > > +#define CXL_DRIVER_CAP_HDM 0x1 > +#define CXL_DRIVER_CAP_MBOX 0x2 > + > /** > * struct cxl_dev_state - The driver device state > * > @@ -438,6 +441,7 @@ struct cxl_dev_state { > struct resource ram_res; > u64 serial; > enum cxl_devtype type; > + uint8_t capabilities; > }; > > /** > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c > index b34d6259faf4..e2a978312281 100644 > --- a/drivers/cxl/pci.c > +++ b/drivers/cxl/pci.c > @@ -502,7 +502,8 @@ static int cxl_rcrb_get_comp_regs(struct pci_dev > *pdev, } > > static int cxl_pci_setup_regs(struct pci_dev *pdev, enum > cxl_regloc_type type, > - struct cxl_register_map *map) > + struct cxl_register_map *map, > + uint8_t cxl_dev_caps) > { > int rc; > > @@ -519,7 +520,7 @@ static int cxl_pci_setup_regs(struct pci_dev > *pdev, enum cxl_regloc_type type, if (rc) > return rc; > > - return cxl_setup_regs(map); > + return cxl_setup_regs(map, cxl_dev_caps); > } > > int cxl_pci_accel_setup_regs(struct pci_dev *pdev, struct > cxl_dev_state *cxlds) @@ -527,7 +528,8 @@ int > cxl_pci_accel_setup_regs(struct pci_dev *pdev, struct cxl_dev_state > *cxlds) struct cxl_register_map map; int rc; > > - rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map); > + rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map, > + cxlds->capabilities); > if (rc) > return rc; > > @@ -536,7 +538,7 @@ int cxl_pci_accel_setup_regs(struct pci_dev > *pdev, struct cxl_dev_state *cxlds) return rc; > > rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_COMPONENT, > - &cxlds->reg_map); > + &cxlds->reg_map, > cxlds->capabilities); if (rc) > dev_warn(&pdev->dev, "No component registers > (%d)\n", rc); > @@ -850,7 +852,8 @@ static int cxl_pci_probe(struct pci_dev *pdev, > const struct pci_device_id *id) dev_warn(&pdev->dev, > "Device DVSEC not present, skip CXL.mem > init\n"); > - rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map); > + rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map, > + cxlds->capabilities); > if (rc) > return rc; > > @@ -863,7 +866,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, > const struct pci_device_id *id) > * still be useful for management functions so don't return > an error. */ > rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_COMPONENT, > - &cxlds->reg_map); > + &cxlds->reg_map, > cxlds->capabilities); if (rc) > dev_warn(&pdev->dev, "No component registers > (%d)\n", rc); else if (!cxlds->reg_map.component_map.ras.valid) > diff --git a/drivers/net/ethernet/sfc/efx_cxl.c > b/drivers/net/ethernet/sfc/efx_cxl.c index 9cefcaf3caca..37d8bfdef517 > 100644 --- a/drivers/net/ethernet/sfc/efx_cxl.c > +++ b/drivers/net/ethernet/sfc/efx_cxl.c > @@ -33,7 +33,8 @@ void efx_cxl_init(struct efx_nic *efx) > > pci_info(pci_dev, "CXL CXL_DVSEC_PCIE_DEVICE capability > found"); > - cxl->cxlds = cxl_accel_state_create(&pci_dev->dev); > + cxl->cxlds = cxl_accel_state_create(&pci_dev->dev, > + > CXL_ACCEL_DRIVER_CAP_HDM); if (IS_ERR(cxl->cxlds)) { > pci_info(pci_dev, "CXL accel device state failed"); > return; > diff --git a/include/linux/cxl_accel_mem.h > b/include/linux/cxl_accel_mem.h index c7b254edc096..0ba2195b919b > 100644 --- a/include/linux/cxl_accel_mem.h > +++ b/include/linux/cxl_accel_mem.h > @@ -12,8 +12,11 @@ enum accel_resource{ > CXL_ACCEL_RES_PMEM, > }; > > +#define CXL_ACCEL_DRIVER_CAP_HDM 0x1 > +#define CXL_ACCEL_DRIVER_CAP_MBOX 0x2 > + > typedef struct cxl_dev_state cxl_accel_state; > -cxl_accel_state *cxl_accel_state_create(struct device *dev); > +cxl_accel_state *cxl_accel_state_create(struct device *dev, uint8_t > caps); > void cxl_accel_set_dvsec(cxl_accel_state *cxlds, u16 dvsec); > void cxl_accel_set_serial(cxl_accel_state *cxlds, u64 serial);
On Tue, 23 Jul 2024 14:43:24 +0100 Alejandro Lucero Palau <alucerop@amd.com> wrote: > > On 7/19/24 20:01, Dave Jiang wrote: > > > >> > >> -static int cxl_probe_regs(struct cxl_register_map *map) > >> +static int cxl_probe_regs(struct cxl_register_map *map, uint8_t > >> caps) { > >> struct cxl_component_reg_map *comp_map; > >> struct cxl_device_reg_map *dev_map; > >> @@ -437,11 +437,12 @@ static int cxl_probe_regs(struct > >> cxl_register_map *map) case CXL_REGLOC_RBI_MEMDEV: > >> dev_map = &map->device_map; > >> cxl_probe_device_regs(host, base, dev_map); > >> - if (!dev_map->status.valid || > >> !dev_map->mbox.valid || > >> + if (!dev_map->status.valid || > >> + ((caps & CXL_DRIVER_CAP_MBOX) && > >> !dev_map->mbox.valid) || !dev_map->memdev.valid) { > >> dev_err(host, "registers not found: > >> %s%s%s\n", !dev_map->status.valid ? "status " : "", > >> - !dev_map->mbox.valid ? "mbox " : > >> "", > >> + ((caps & CXL_DRIVER_CAP_MBOX) && > >> !dev_map->mbox.valid) ? "mbox " : "", > > According to the r3.1 8.2.8.2.1, the device status registers and > > the primary mailbox registers are both mandatory if regloc id=3 > > block is found. So if the type2 device does not implement a mailbox > > then it shouldn't be calling cxl_pci_setup_regs(pdev, > > CXL_REGLOC_RBI_MEMDEV, &map) to begin with from the driver init > > right? If the type2 device defines a regblock with id=3 but without > > a mailbox, then isn't that a spec violation? > > > > DJ > > > Right. The code needs to support the possibility of a Type2 having a > mailbox, and if it is not supported, the rest of the dvsec regs > initialization needs to be performed. This is not what the code does > now, so I'll fix this. > > > A wider explanation is, for the RFC I used a test driver based on > QEMU emulating a Type2 which had a CXL Device Register Interface > defined (03h) but not a CXL Device Capability with id 2 for the > primary mailbox register, breaking the spec as you spotted. > > Because SFC driver uses (the 8.2.8.5.1.1 Memory Device Status Register) to determine if the memory media is ready or not (in PATCH 6). That register should be in a regloc id=3 block. According to the spec paste above, the device that has regloc block id=3 needs to have device status and mailbox. Curious, does the SFC device have to implement the mailbox in this case for spec compliance? Previously, I always think that "CXL Memory Device" == "CXL Type-3 device" in the CXL spec. Now I am little bit confused if a type-2 device that supports cxl.mem == "CXL Memory Device" mentioned in the spec. If the answer == Y, then having regloc id ==3 and mailbox turn mandatory for a type-2 device that support cxl.mem for the spec compliance. If the answer == N, then a type-2 device can use approaches other than Memory Device Status Register to determine the readiness of the memory? ZW > Thanks. > >
On 8/9/24 10:10, Zhi Wang wrote: > On Mon, 15 Jul 2024 18:28:24 +0100 > <alejandro.lucero-palau@amd.com> wrote: > >> From: Alejandro Lucero <alucerop@amd.com> >> >> Type2 devices have some Type3 functionalities as optional like an mbox >> or an hdm decoder, and CXL core needs a way to know what a CXL >> accelerator implements. >> >> Add a new field for keeping device capabilities to be initialised by >> Type2 drivers. Advertise all those capabilities for Type3. >> >> Signed-off-by: Alejandro Lucero <alucerop@amd.com> >> --- >> drivers/cxl/core/mbox.c | 1 + >> drivers/cxl/core/memdev.c | 4 +++- >> drivers/cxl/core/port.c | 2 +- >> drivers/cxl/core/regs.c | 11 ++++++----- >> drivers/cxl/cxl.h | 2 +- >> drivers/cxl/cxlmem.h | 4 ++++ >> drivers/cxl/pci.c | 15 +++++++++------ >> drivers/net/ethernet/sfc/efx_cxl.c | 3 ++- >> include/linux/cxl_accel_mem.h | 5 ++++- >> 9 files changed, 31 insertions(+), 16 deletions(-) >> >> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c >> index 2626f3fff201..2ba7d36e3f38 100644 >> --- a/drivers/cxl/core/mbox.c >> +++ b/drivers/cxl/core/mbox.c >> @@ -1424,6 +1424,7 @@ struct cxl_memdev_state >> *cxl_memdev_state_create(struct device *dev) mds->cxlds.reg_map.host >> = dev; mds->cxlds.reg_map.resource = CXL_RESOURCE_NONE; >> mds->cxlds.type = CXL_DEVTYPE_CLASSMEM; >> + mds->cxlds.capabilities = CXL_DRIVER_CAP_HDM | >> CXL_DRIVER_CAP_MBOX; mds->ram_perf.qos_class = CXL_QOS_CLASS_INVALID; >> mds->pmem_perf.qos_class = CXL_QOS_CLASS_INVALID; >> >> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c >> index 04c3a0f8bc2e..b4205ecca365 100644 >> --- a/drivers/cxl/core/memdev.c >> +++ b/drivers/cxl/core/memdev.c >> @@ -616,7 +616,7 @@ static void detach_memdev(struct work_struct >> *work) >> static struct lock_class_key cxl_memdev_key; >> >> -struct cxl_dev_state *cxl_accel_state_create(struct device *dev) >> +struct cxl_dev_state *cxl_accel_state_create(struct device *dev, >> uint8_t caps) { >> struct cxl_dev_state *cxlds; >> >> @@ -631,6 +631,8 @@ struct cxl_dev_state >> *cxl_accel_state_create(struct device *dev) cxlds->ram_res = >> DEFINE_RES_MEM_NAMED(0, 0, "ram"); cxlds->pmem_res = >> DEFINE_RES_MEM_NAMED(0, 0, "pmem"); >> + cxlds->capabilities = caps; >> + >> return cxlds; >> } >> EXPORT_SYMBOL_NS_GPL(cxl_accel_state_create, CXL); >> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c >> index 887ed6e358fb..d66c6349ed2d 100644 >> --- a/drivers/cxl/core/port.c >> +++ b/drivers/cxl/core/port.c >> @@ -763,7 +763,7 @@ static int cxl_setup_comp_regs(struct device >> *host, struct cxl_register_map *map map->reg_type = >> CXL_REGLOC_RBI_COMPONENT; map->max_size = >> CXL_COMPONENT_REG_BLOCK_SIZE; >> - return cxl_setup_regs(map); >> + return cxl_setup_regs(map, 0); >> } >> >> static int cxl_port_setup_regs(struct cxl_port *port, >> diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c >> index e1082e749c69..9d218ebe180d 100644 >> --- a/drivers/cxl/core/regs.c >> +++ b/drivers/cxl/core/regs.c >> @@ -421,7 +421,7 @@ static void cxl_unmap_regblock(struct >> cxl_register_map *map) map->base = NULL; >> } >> >> -static int cxl_probe_regs(struct cxl_register_map *map) >> +static int cxl_probe_regs(struct cxl_register_map *map, uint8_t caps) >> { > Can we not use uintxx_t? Just like any other one in the > cxl-core. Generally, u{8,16...} are mostly used for kernel > programming, and your previous patches use them nicely. > > Let's use u8 for caps. > Sure. Thanks >> struct cxl_component_reg_map *comp_map; >> struct cxl_device_reg_map *dev_map; >> @@ -437,11 +437,12 @@ static int cxl_probe_regs(struct >> cxl_register_map *map) case CXL_REGLOC_RBI_MEMDEV: >> dev_map = &map->device_map; >> cxl_probe_device_regs(host, base, dev_map); >> - if (!dev_map->status.valid || !dev_map->mbox.valid || >> + if (!dev_map->status.valid || >> + ((caps & CXL_DRIVER_CAP_MBOX) && >> !dev_map->mbox.valid) || !dev_map->memdev.valid) { >> dev_err(host, "registers not found: >> %s%s%s\n", !dev_map->status.valid ? "status " : "", >> - !dev_map->mbox.valid ? "mbox " : "", >> + ((caps & CXL_DRIVER_CAP_MBOX) && >> !dev_map->mbox.valid) ? "mbox " : "", !dev_map->memdev.valid ? >> "memdev " : ""); return -ENXIO; >> } >> @@ -455,7 +456,7 @@ static int cxl_probe_regs(struct cxl_register_map >> *map) return 0; >> } >> >> -int cxl_setup_regs(struct cxl_register_map *map) >> +int cxl_setup_regs(struct cxl_register_map *map, uint8_t caps) >> { >> int rc; >> >> @@ -463,7 +464,7 @@ int cxl_setup_regs(struct cxl_register_map *map) >> if (rc) >> return rc; >> >> - rc = cxl_probe_regs(map); >> + rc = cxl_probe_regs(map, caps); >> cxl_unmap_regblock(map); >> >> return rc; >> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h >> index a6613a6f8923..9973430d975f 100644 >> --- a/drivers/cxl/cxl.h >> +++ b/drivers/cxl/cxl.h >> @@ -300,7 +300,7 @@ int cxl_find_regblock_instance(struct pci_dev >> *pdev, enum cxl_regloc_type type, struct cxl_register_map *map, int >> index); int cxl_find_regblock(struct pci_dev *pdev, enum >> cxl_regloc_type type, struct cxl_register_map *map); >> -int cxl_setup_regs(struct cxl_register_map *map); >> +int cxl_setup_regs(struct cxl_register_map *map, uint8_t caps); >> struct cxl_dport; >> resource_size_t cxl_rcd_component_reg_phys(struct device *dev, >> struct cxl_dport *dport); >> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h >> index af8169ccdbc0..8f2a820bd92d 100644 >> --- a/drivers/cxl/cxlmem.h >> +++ b/drivers/cxl/cxlmem.h >> @@ -405,6 +405,9 @@ struct cxl_dpa_perf { >> int qos_class; >> }; >> >> +#define CXL_DRIVER_CAP_HDM 0x1 >> +#define CXL_DRIVER_CAP_MBOX 0x2 >> + >> /** >> * struct cxl_dev_state - The driver device state >> * >> @@ -438,6 +441,7 @@ struct cxl_dev_state { >> struct resource ram_res; >> u64 serial; >> enum cxl_devtype type; >> + uint8_t capabilities; >> }; >> >> /** >> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c >> index b34d6259faf4..e2a978312281 100644 >> --- a/drivers/cxl/pci.c >> +++ b/drivers/cxl/pci.c >> @@ -502,7 +502,8 @@ static int cxl_rcrb_get_comp_regs(struct pci_dev >> *pdev, } >> >> static int cxl_pci_setup_regs(struct pci_dev *pdev, enum >> cxl_regloc_type type, >> - struct cxl_register_map *map) >> + struct cxl_register_map *map, >> + uint8_t cxl_dev_caps) >> { >> int rc; >> >> @@ -519,7 +520,7 @@ static int cxl_pci_setup_regs(struct pci_dev >> *pdev, enum cxl_regloc_type type, if (rc) >> return rc; >> >> - return cxl_setup_regs(map); >> + return cxl_setup_regs(map, cxl_dev_caps); >> } >> >> int cxl_pci_accel_setup_regs(struct pci_dev *pdev, struct >> cxl_dev_state *cxlds) @@ -527,7 +528,8 @@ int >> cxl_pci_accel_setup_regs(struct pci_dev *pdev, struct cxl_dev_state >> *cxlds) struct cxl_register_map map; int rc; >> >> - rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map); >> + rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map, >> + cxlds->capabilities); >> if (rc) >> return rc; >> >> @@ -536,7 +538,7 @@ int cxl_pci_accel_setup_regs(struct pci_dev >> *pdev, struct cxl_dev_state *cxlds) return rc; >> >> rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_COMPONENT, >> - &cxlds->reg_map); >> + &cxlds->reg_map, >> cxlds->capabilities); if (rc) >> dev_warn(&pdev->dev, "No component registers >> (%d)\n", rc); >> @@ -850,7 +852,8 @@ static int cxl_pci_probe(struct pci_dev *pdev, >> const struct pci_device_id *id) dev_warn(&pdev->dev, >> "Device DVSEC not present, skip CXL.mem >> init\n"); >> - rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map); >> + rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map, >> + cxlds->capabilities); >> if (rc) >> return rc; >> >> @@ -863,7 +866,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, >> const struct pci_device_id *id) >> * still be useful for management functions so don't return >> an error. */ >> rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_COMPONENT, >> - &cxlds->reg_map); >> + &cxlds->reg_map, >> cxlds->capabilities); if (rc) >> dev_warn(&pdev->dev, "No component registers >> (%d)\n", rc); else if (!cxlds->reg_map.component_map.ras.valid) >> diff --git a/drivers/net/ethernet/sfc/efx_cxl.c >> b/drivers/net/ethernet/sfc/efx_cxl.c index 9cefcaf3caca..37d8bfdef517 >> 100644 --- a/drivers/net/ethernet/sfc/efx_cxl.c >> +++ b/drivers/net/ethernet/sfc/efx_cxl.c >> @@ -33,7 +33,8 @@ void efx_cxl_init(struct efx_nic *efx) >> >> pci_info(pci_dev, "CXL CXL_DVSEC_PCIE_DEVICE capability >> found"); >> - cxl->cxlds = cxl_accel_state_create(&pci_dev->dev); >> + cxl->cxlds = cxl_accel_state_create(&pci_dev->dev, >> + >> CXL_ACCEL_DRIVER_CAP_HDM); if (IS_ERR(cxl->cxlds)) { >> pci_info(pci_dev, "CXL accel device state failed"); >> return; >> diff --git a/include/linux/cxl_accel_mem.h >> b/include/linux/cxl_accel_mem.h index c7b254edc096..0ba2195b919b >> 100644 --- a/include/linux/cxl_accel_mem.h >> +++ b/include/linux/cxl_accel_mem.h >> @@ -12,8 +12,11 @@ enum accel_resource{ >> CXL_ACCEL_RES_PMEM, >> }; >> >> +#define CXL_ACCEL_DRIVER_CAP_HDM 0x1 >> +#define CXL_ACCEL_DRIVER_CAP_MBOX 0x2 >> + >> typedef struct cxl_dev_state cxl_accel_state; >> -cxl_accel_state *cxl_accel_state_create(struct device *dev); >> +cxl_accel_state *cxl_accel_state_create(struct device *dev, uint8_t >> caps); >> void cxl_accel_set_dvsec(cxl_accel_state *cxlds, u16 dvsec); >> void cxl_accel_set_serial(cxl_accel_state *cxlds, u64 serial);
On 8/9/24 11:25, Zhi Wang wrote: > On Tue, 23 Jul 2024 14:43:24 +0100 > Alejandro Lucero Palau <alucerop@amd.com> wrote: > >> On 7/19/24 20:01, Dave Jiang wrote: >>>> >>>> -static int cxl_probe_regs(struct cxl_register_map *map) >>>> +static int cxl_probe_regs(struct cxl_register_map *map, uint8_t >>>> caps) { >>>> struct cxl_component_reg_map *comp_map; >>>> struct cxl_device_reg_map *dev_map; >>>> @@ -437,11 +437,12 @@ static int cxl_probe_regs(struct >>>> cxl_register_map *map) case CXL_REGLOC_RBI_MEMDEV: >>>> dev_map = &map->device_map; >>>> cxl_probe_device_regs(host, base, dev_map); >>>> - if (!dev_map->status.valid || >>>> !dev_map->mbox.valid || >>>> + if (!dev_map->status.valid || >>>> + ((caps & CXL_DRIVER_CAP_MBOX) && >>>> !dev_map->mbox.valid) || !dev_map->memdev.valid) { >>>> dev_err(host, "registers not found: >>>> %s%s%s\n", !dev_map->status.valid ? "status " : "", >>>> - !dev_map->mbox.valid ? "mbox " : >>>> "", >>>> + ((caps & CXL_DRIVER_CAP_MBOX) && >>>> !dev_map->mbox.valid) ? "mbox " : "", >>> According to the r3.1 8.2.8.2.1, the device status registers and >>> the primary mailbox registers are both mandatory if regloc id=3 >>> block is found. So if the type2 device does not implement a mailbox >>> then it shouldn't be calling cxl_pci_setup_regs(pdev, >>> CXL_REGLOC_RBI_MEMDEV, &map) to begin with from the driver init >>> right? If the type2 device defines a regblock with id=3 but without >>> a mailbox, then isn't that a spec violation? >>> >>> DJ >> >> Right. The code needs to support the possibility of a Type2 having a >> mailbox, and if it is not supported, the rest of the dvsec regs >> initialization needs to be performed. This is not what the code does >> now, so I'll fix this. >> >> >> A wider explanation is, for the RFC I used a test driver based on >> QEMU emulating a Type2 which had a CXL Device Register Interface >> defined (03h) but not a CXL Device Capability with id 2 for the >> primary mailbox register, breaking the spec as you spotted. >> >> > Because SFC driver uses (the 8.2.8.5.1.1 Memory Device Status > Register) to determine if the memory media is ready or not (in PATCH 6). > That register should be in a regloc id=3 block. Right. Note patch 6 calls first cxl_await_media_ready and if it returns error, what happens if the register is not found, it sets the media ready field since it is required later on. Damn it! I realize the code is wrong because the manual setting is based on no error. The testing has been a pain until recently with a partial emulation, so I had to follow undesired development steps. This is better now so v3 will fix some minor bugs like this one. I also realize in our case this first call is useless, so I plan to remove it in next version. Thanks! > According to the spec paste above, the device that has regloc block > id=3 needs to have device status and mailbox. > > Curious, does the SFC device have to implement the mailbox in this case > for spec compliance? I think It should, but no status register either in our case. > Previously, I always think that "CXL Memory Device" == "CXL Type-3 > device" in the CXL spec. > > Now I am little bit confused if a type-2 device that supports cxl.mem > == "CXL Memory Device" mentioned in the spec. > > If the answer == Y, then having regloc id ==3 and mailbox turn > mandatory for a type-2 device that support cxl.mem for the spec > compliance. > > If the answer == N, then a type-2 device can use approaches other than > Memory Device Status Register to determine the readiness of the memory? Right again. Our device is not advertised as a Memory Device but as a ethernet one, so we are not implementing those mandatory ones for a memory device. Regarding the readiness of the CXL memory, I have been told this is so once some initial negotiation is performed (I do not know the details). That is the reason for setting this manually by our driver and the accessor added. > ZW > >> Thanks. >> >>
On 8/4/24 18:22, Jonathan Cameron wrote: > On Mon, 15 Jul 2024 18:28:24 +0100 > alejandro.lucero-palau@amd.com wrote: > >> From: Alejandro Lucero <alucerop@amd.com> >> >> Type2 devices have some Type3 functionalities as optional like an mbox >> or an hdm decoder, and CXL core needs a way to know what a CXL accelerator >> implements. >> >> Add a new field for keeping device capabilities to be initialised by >> Type2 drivers. Advertise all those capabilities for Type3. >> >> Signed-off-by: Alejandro Lucero <alucerop@amd.com> > In general seems a reasonable approach, so just minor comments. > >> --- >> drivers/cxl/core/mbox.c | 1 + >> drivers/cxl/core/memdev.c | 4 +++- >> drivers/cxl/core/port.c | 2 +- >> drivers/cxl/core/regs.c | 11 ++++++----- >> drivers/cxl/cxl.h | 2 +- >> drivers/cxl/cxlmem.h | 4 ++++ >> drivers/cxl/pci.c | 15 +++++++++------ >> drivers/net/ethernet/sfc/efx_cxl.c | 3 ++- >> include/linux/cxl_accel_mem.h | 5 ++++- >> 9 files changed, 31 insertions(+), 16 deletions(-) >> >> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c >> index 2626f3fff201..2ba7d36e3f38 100644 >> --- a/drivers/cxl/core/mbox.c >> +++ b/drivers/cxl/core/mbox.c >> @@ -1424,6 +1424,7 @@ struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev) >> mds->cxlds.reg_map.host = dev; >> mds->cxlds.reg_map.resource = CXL_RESOURCE_NONE; >> mds->cxlds.type = CXL_DEVTYPE_CLASSMEM; >> + mds->cxlds.capabilities = CXL_DRIVER_CAP_HDM | CXL_DRIVER_CAP_MBOX; > Add a reference for this perhaps. Make it clear that a type3 device must > support mailbox and hdm by pointing at requirement for the various structures > in a spec reference. > I think it would be worth to have documentation, distilling out dis-ambiguities from the specs about mandatory/optional registers. >> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h >> index af8169ccdbc0..8f2a820bd92d 100644 >> --- a/drivers/cxl/cxlmem.h >> +++ b/drivers/cxl/cxlmem.h >> @@ -405,6 +405,9 @@ struct cxl_dpa_perf { >> int qos_class; >> }; >> >> +#define CXL_DRIVER_CAP_HDM 0x1 >> +#define CXL_DRIVER_CAP_MBOX 0x2 >> + > Enum and BIT() for the defines. Avoids someone in future > thinking they can define 0x3 to be something. > > Definitely only one definition as well. Seems reasonable for > this to be CXL wide. > OK. Thanks! >> /** >> * struct cxl_dev_state - The driver device state >> * >> @@ -438,6 +441,7 @@ struct cxl_dev_state { >> struct resource ram_res; >> u64 serial; >> enum cxl_devtype type; >> + uint8_t capabilities; >> };
On Thu, 15 Aug 2024 16:37:21 +0100 Alejandro Lucero Palau <alucerop@amd.com> wrote: > > On 8/9/24 11:25, Zhi Wang wrote: > > On Tue, 23 Jul 2024 14:43:24 +0100 > > Alejandro Lucero Palau <alucerop@amd.com> wrote: > > > >> On 7/19/24 20:01, Dave Jiang wrote: > >>>> > >>>> -static int cxl_probe_regs(struct cxl_register_map *map) > >>>> +static int cxl_probe_regs(struct cxl_register_map *map, uint8_t > >>>> caps) { > >>>> struct cxl_component_reg_map *comp_map; > >>>> struct cxl_device_reg_map *dev_map; > >>>> @@ -437,11 +437,12 @@ static int cxl_probe_regs(struct > >>>> cxl_register_map *map) case CXL_REGLOC_RBI_MEMDEV: > >>>> dev_map = &map->device_map; > >>>> cxl_probe_device_regs(host, base, dev_map); > >>>> - if (!dev_map->status.valid || > >>>> !dev_map->mbox.valid || > >>>> + if (!dev_map->status.valid || > >>>> + ((caps & CXL_DRIVER_CAP_MBOX) && > >>>> !dev_map->mbox.valid) || !dev_map->memdev.valid) { > >>>> dev_err(host, "registers not found: > >>>> %s%s%s\n", !dev_map->status.valid ? "status " : "", > >>>> - !dev_map->mbox.valid ? "mbox " : > >>>> "", > >>>> + ((caps & CXL_DRIVER_CAP_MBOX) && > >>>> !dev_map->mbox.valid) ? "mbox " : "", > >>> According to the r3.1 8.2.8.2.1, the device status registers and > >>> the primary mailbox registers are both mandatory if regloc id=3 > >>> block is found. So if the type2 device does not implement a > >>> mailbox then it shouldn't be calling cxl_pci_setup_regs(pdev, > >>> CXL_REGLOC_RBI_MEMDEV, &map) to begin with from the driver init > >>> right? If the type2 device defines a regblock with id=3 but > >>> without a mailbox, then isn't that a spec violation? > >>> > >>> DJ > >> > >> Right. The code needs to support the possibility of a Type2 having > >> a mailbox, and if it is not supported, the rest of the dvsec regs > >> initialization needs to be performed. This is not what the code > >> does now, so I'll fix this. > >> > >> > >> A wider explanation is, for the RFC I used a test driver based on > >> QEMU emulating a Type2 which had a CXL Device Register Interface > >> defined (03h) but not a CXL Device Capability with id 2 for the > >> primary mailbox register, breaking the spec as you spotted. > >> > >> > > Because SFC driver uses (the 8.2.8.5.1.1 Memory Device Status > > Register) to determine if the memory media is ready or not (in > > PATCH 6). That register should be in a regloc id=3 block. > > > Right. Note patch 6 calls first cxl_await_media_ready and if it > returns error, what happens if the register is not found, it sets the > media ready field since it is required later on. > > Damn it! I realize the code is wrong because the manual setting is > based on no error. The testing has been a pain until recently with a > partial emulation, so I had to follow undesired development steps. > This is better now so v3 will fix some minor bugs like this one. > > I also realize in our case this first call is useless, so I plan to > remove it in next version. > > Thanks! > Hi Alejandro: No worries. Let's push forward. :) For a type-2, I think cxl_await_media_ready() still gives value on provide a type-2 vendor driver a generic core call to make sure the HDM region is ready to use. Because judging CXL_RANGE active & valid in CXL_RANGE_{1,2}_SIZE_LO can be useful to type-2. I think the problem of cxl_await_media_ready() is: it assumes the Memory Device Status Register is always present, which is true for type-3 but not always true for type-2. I think we need: diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c index a663e7566c48..0ba1cedfc0ba 100644 --- a/drivers/cxl/core/pci.c +++ b/drivers/cxl/core/pci.c @@ -203,6 +203,9 @@ int cxl_await_media_ready(struct cxl_dev_state *cxlds) return rc; } + if (!cxlds->regs.memdev) + return 0; + md_status = readq(cxlds->regs.memdev + CXLMDEV_STATUS_OFFSET); if (!CXLMDEV_READY(md_status)) return -EIO; Then for the type-2 device, if it doesn't implement regloc=3, it can still call cxl_await_media_ready() to make sure the media is ready. For type-2 and type-3 which implements regloc=3, the check can continue. I think SFC can use this as well, because according to the spec 8.1.3.8 DVSEC CXL Range Registers: "The DVSEC CXL Range 1 register set must be implemented if Mem_Capable=1 in the DVSEC CXL Capability register. The DVSEC CXL Range 2 register set must be implemented if (Mem_Capable=1 and HDM_Count=10b in the DVSEC CXL Capability register)." So SFC should have this. With the change above maybe you don't need set_media_ready stuff in the later patch. Just simply call cxl_await_media_ready(), everything should be fine then. Thanks, Zhi. > > > According to the spec paste above, the device that has regloc block > > id=3 needs to have device status and mailbox. > > > > Curious, does the SFC device have to implement the mailbox in this > > case for spec compliance? > > > I think It should, but no status register either in our case. > > > > Previously, I always think that "CXL Memory Device" == "CXL Type-3 > > device" in the CXL spec. > > > > Now I am little bit confused if a type-2 device that supports > > cxl.mem == "CXL Memory Device" mentioned in the spec. > > > > If the answer == Y, then having regloc id ==3 and mailbox turn > > mandatory for a type-2 device that support cxl.mem for the spec > > compliance. > > > > If the answer == N, then a type-2 device can use approaches other > > than Memory Device Status Register to determine the readiness of > > the memory? > > > Right again. Our device is not advertised as a Memory Device but as a > ethernet one, so we are not implementing those mandatory ones for a > memory device. > > Regarding the readiness of the CXL memory, I have been told this is > so once some initial negotiation is performed (I do not know the > details). That is the reason for setting this manually by our driver > and the accessor added. > > > > ZW > > > >> Thanks. > >> > >> >
On 8/18/24 07:55, Zhi Wang wrote: > On Thu, 15 Aug 2024 16:37:21 +0100 > Alejandro Lucero Palau <alucerop@amd.com> wrote: > >> On 8/9/24 11:25, Zhi Wang wrote: >>> On Tue, 23 Jul 2024 14:43:24 +0100 >>> Alejandro Lucero Palau <alucerop@amd.com> wrote: >>> >>>> On 7/19/24 20:01, Dave Jiang wrote: >>>>>> >>>>>> -static int cxl_probe_regs(struct cxl_register_map *map) >>>>>> +static int cxl_probe_regs(struct cxl_register_map *map, uint8_t >>>>>> caps) { >>>>>> struct cxl_component_reg_map *comp_map; >>>>>> struct cxl_device_reg_map *dev_map; >>>>>> @@ -437,11 +437,12 @@ static int cxl_probe_regs(struct >>>>>> cxl_register_map *map) case CXL_REGLOC_RBI_MEMDEV: >>>>>> dev_map = &map->device_map; >>>>>> cxl_probe_device_regs(host, base, dev_map); >>>>>> - if (!dev_map->status.valid || >>>>>> !dev_map->mbox.valid || >>>>>> + if (!dev_map->status.valid || >>>>>> + ((caps & CXL_DRIVER_CAP_MBOX) && >>>>>> !dev_map->mbox.valid) || !dev_map->memdev.valid) { >>>>>> dev_err(host, "registers not found: >>>>>> %s%s%s\n", !dev_map->status.valid ? "status " : "", >>>>>> - !dev_map->mbox.valid ? "mbox " : >>>>>> "", >>>>>> + ((caps & CXL_DRIVER_CAP_MBOX) && >>>>>> !dev_map->mbox.valid) ? "mbox " : "", >>>>> According to the r3.1 8.2.8.2.1, the device status registers and >>>>> the primary mailbox registers are both mandatory if regloc id=3 >>>>> block is found. So if the type2 device does not implement a >>>>> mailbox then it shouldn't be calling cxl_pci_setup_regs(pdev, >>>>> CXL_REGLOC_RBI_MEMDEV, &map) to begin with from the driver init >>>>> right? If the type2 device defines a regblock with id=3 but >>>>> without a mailbox, then isn't that a spec violation? >>>>> >>>>> DJ >>>> Right. The code needs to support the possibility of a Type2 having >>>> a mailbox, and if it is not supported, the rest of the dvsec regs >>>> initialization needs to be performed. This is not what the code >>>> does now, so I'll fix this. >>>> >>>> >>>> A wider explanation is, for the RFC I used a test driver based on >>>> QEMU emulating a Type2 which had a CXL Device Register Interface >>>> defined (03h) but not a CXL Device Capability with id 2 for the >>>> primary mailbox register, breaking the spec as you spotted. >>>> >>>> >>> Because SFC driver uses (the 8.2.8.5.1.1 Memory Device Status >>> Register) to determine if the memory media is ready or not (in >>> PATCH 6). That register should be in a regloc id=3 block. >> >> Right. Note patch 6 calls first cxl_await_media_ready and if it >> returns error, what happens if the register is not found, it sets the >> media ready field since it is required later on. >> >> Damn it! I realize the code is wrong because the manual setting is >> based on no error. The testing has been a pain until recently with a >> partial emulation, so I had to follow undesired development steps. >> This is better now so v3 will fix some minor bugs like this one. >> >> I also realize in our case this first call is useless, so I plan to >> remove it in next version. >> >> Thanks! >> > Hi Alejandro: > > No worries. Let's push forward. :) > > For a type-2, I think cxl_await_media_ready() still gives value on > provide a type-2 vendor driver a generic core call to make sure the HDM > region is ready to use. Because judging CXL_RANGE active & valid in > CXL_RANGE_{1,2}_SIZE_LO can be useful to type-2. > > I think the problem of cxl_await_media_ready() is: it assumes the > Memory Device Status Register is always present, which is true for > type-3 but not always true for type-2. I think we need: > > diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c > index a663e7566c48..0ba1cedfc0ba 100644 > --- a/drivers/cxl/core/pci.c > +++ b/drivers/cxl/core/pci.c > @@ -203,6 +203,9 @@ int cxl_await_media_ready(struct cxl_dev_state > *cxlds) > return rc; > } > > + if (!cxlds->regs.memdev) > + return 0; > + > md_status = readq(cxlds->regs.memdev + CXLMDEV_STATUS_OFFSET); > if (!CXLMDEV_READY(md_status)) > return -EIO; > > Then for the type-2 device, if it doesn't implement regloc=3, it can > still call cxl_await_media_ready() to make sure the media is ready. For > type-2 and type-3 which implements regloc=3, the check can continue. In this case I think the driver should know if calling this function makes sense, apart from the code checking if the proper register does exist. > > I think SFC can use this as well, because according to the spec 8.1.3.8 > DVSEC CXL Range Registers: > > "The DVSEC CXL Range 1 register set must be implemented if > Mem_Capable=1 in the DVSEC CXL Capability register. The DVSEC CXL Range > 2 register set must be implemented if (Mem_Capable=1 and HDM_Count=10b > in the DVSEC CXL Capability register)." I have discussed this internally, and what you point to implies it is, as we understand it, only mandatory for memory devices what we are not. I guess this is an ambiguity in the specs but the fact is the current hardware design which will be part of the silicon coming has not such register implemented. > So SFC should have this. With the change above maybe you don't need > set_media_ready stuff in the later patch. Just simply call > cxl_await_media_ready(), everything should be fine then. The media_ready field inside cxl_dev_state needs to be set to true for avoiding later checks to preclude further initialization. I could avoid this accessor as we have decided to not make cxl_dev_state opaque but in prevision of core cxl struct refactoring in the future, I think it is worth to keep the accessor. Thanks > > Thanks, > Zhi. > >>> According to the spec paste above, the device that has regloc block >>> id=3 needs to have device status and mailbox. >>> >>> Curious, does the SFC device have to implement the mailbox in this >>> case for spec compliance? >> >> I think It should, but no status register either in our case. >> >> >>> Previously, I always think that "CXL Memory Device" == "CXL Type-3 >>> device" in the CXL spec. >>> >>> Now I am little bit confused if a type-2 device that supports >>> cxl.mem == "CXL Memory Device" mentioned in the spec. >>> >>> If the answer == Y, then having regloc id ==3 and mailbox turn >>> mandatory for a type-2 device that support cxl.mem for the spec >>> compliance. >>> >>> If the answer == N, then a type-2 device can use approaches other >>> than Memory Device Status Register to determine the readiness of >>> the memory? >> >> Right again. Our device is not advertised as a Memory Device but as a >> ethernet one, so we are not implementing those mandatory ones for a >> memory device. >> >> Regarding the readiness of the CXL memory, I have been told this is >> so once some initial negotiation is performed (I do not know the >> details). That is the reason for setting this manually by our driver >> and the accessor added. >> >> >>> ZW >>> >>>> Thanks. >>>> >>>>
diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c index 2626f3fff201..2ba7d36e3f38 100644 --- a/drivers/cxl/core/mbox.c +++ b/drivers/cxl/core/mbox.c @@ -1424,6 +1424,7 @@ struct cxl_memdev_state *cxl_memdev_state_create(struct device *dev) mds->cxlds.reg_map.host = dev; mds->cxlds.reg_map.resource = CXL_RESOURCE_NONE; mds->cxlds.type = CXL_DEVTYPE_CLASSMEM; + mds->cxlds.capabilities = CXL_DRIVER_CAP_HDM | CXL_DRIVER_CAP_MBOX; mds->ram_perf.qos_class = CXL_QOS_CLASS_INVALID; mds->pmem_perf.qos_class = CXL_QOS_CLASS_INVALID; diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c index 04c3a0f8bc2e..b4205ecca365 100644 --- a/drivers/cxl/core/memdev.c +++ b/drivers/cxl/core/memdev.c @@ -616,7 +616,7 @@ static void detach_memdev(struct work_struct *work) static struct lock_class_key cxl_memdev_key; -struct cxl_dev_state *cxl_accel_state_create(struct device *dev) +struct cxl_dev_state *cxl_accel_state_create(struct device *dev, uint8_t caps) { struct cxl_dev_state *cxlds; @@ -631,6 +631,8 @@ struct cxl_dev_state *cxl_accel_state_create(struct device *dev) cxlds->ram_res = DEFINE_RES_MEM_NAMED(0, 0, "ram"); cxlds->pmem_res = DEFINE_RES_MEM_NAMED(0, 0, "pmem"); + cxlds->capabilities = caps; + return cxlds; } EXPORT_SYMBOL_NS_GPL(cxl_accel_state_create, CXL); diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c index 887ed6e358fb..d66c6349ed2d 100644 --- a/drivers/cxl/core/port.c +++ b/drivers/cxl/core/port.c @@ -763,7 +763,7 @@ static int cxl_setup_comp_regs(struct device *host, struct cxl_register_map *map map->reg_type = CXL_REGLOC_RBI_COMPONENT; map->max_size = CXL_COMPONENT_REG_BLOCK_SIZE; - return cxl_setup_regs(map); + return cxl_setup_regs(map, 0); } static int cxl_port_setup_regs(struct cxl_port *port, diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c index e1082e749c69..9d218ebe180d 100644 --- a/drivers/cxl/core/regs.c +++ b/drivers/cxl/core/regs.c @@ -421,7 +421,7 @@ static void cxl_unmap_regblock(struct cxl_register_map *map) map->base = NULL; } -static int cxl_probe_regs(struct cxl_register_map *map) +static int cxl_probe_regs(struct cxl_register_map *map, uint8_t caps) { struct cxl_component_reg_map *comp_map; struct cxl_device_reg_map *dev_map; @@ -437,11 +437,12 @@ static int cxl_probe_regs(struct cxl_register_map *map) case CXL_REGLOC_RBI_MEMDEV: dev_map = &map->device_map; cxl_probe_device_regs(host, base, dev_map); - if (!dev_map->status.valid || !dev_map->mbox.valid || + if (!dev_map->status.valid || + ((caps & CXL_DRIVER_CAP_MBOX) && !dev_map->mbox.valid) || !dev_map->memdev.valid) { dev_err(host, "registers not found: %s%s%s\n", !dev_map->status.valid ? "status " : "", - !dev_map->mbox.valid ? "mbox " : "", + ((caps & CXL_DRIVER_CAP_MBOX) && !dev_map->mbox.valid) ? "mbox " : "", !dev_map->memdev.valid ? "memdev " : ""); return -ENXIO; } @@ -455,7 +456,7 @@ static int cxl_probe_regs(struct cxl_register_map *map) return 0; } -int cxl_setup_regs(struct cxl_register_map *map) +int cxl_setup_regs(struct cxl_register_map *map, uint8_t caps) { int rc; @@ -463,7 +464,7 @@ int cxl_setup_regs(struct cxl_register_map *map) if (rc) return rc; - rc = cxl_probe_regs(map); + rc = cxl_probe_regs(map, caps); cxl_unmap_regblock(map); return rc; diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h index a6613a6f8923..9973430d975f 100644 --- a/drivers/cxl/cxl.h +++ b/drivers/cxl/cxl.h @@ -300,7 +300,7 @@ int cxl_find_regblock_instance(struct pci_dev *pdev, enum cxl_regloc_type type, struct cxl_register_map *map, int index); int cxl_find_regblock(struct pci_dev *pdev, enum cxl_regloc_type type, struct cxl_register_map *map); -int cxl_setup_regs(struct cxl_register_map *map); +int cxl_setup_regs(struct cxl_register_map *map, uint8_t caps); struct cxl_dport; resource_size_t cxl_rcd_component_reg_phys(struct device *dev, struct cxl_dport *dport); diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h index af8169ccdbc0..8f2a820bd92d 100644 --- a/drivers/cxl/cxlmem.h +++ b/drivers/cxl/cxlmem.h @@ -405,6 +405,9 @@ struct cxl_dpa_perf { int qos_class; }; +#define CXL_DRIVER_CAP_HDM 0x1 +#define CXL_DRIVER_CAP_MBOX 0x2 + /** * struct cxl_dev_state - The driver device state * @@ -438,6 +441,7 @@ struct cxl_dev_state { struct resource ram_res; u64 serial; enum cxl_devtype type; + uint8_t capabilities; }; /** diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c index b34d6259faf4..e2a978312281 100644 --- a/drivers/cxl/pci.c +++ b/drivers/cxl/pci.c @@ -502,7 +502,8 @@ static int cxl_rcrb_get_comp_regs(struct pci_dev *pdev, } static int cxl_pci_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type type, - struct cxl_register_map *map) + struct cxl_register_map *map, + uint8_t cxl_dev_caps) { int rc; @@ -519,7 +520,7 @@ static int cxl_pci_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type type, if (rc) return rc; - return cxl_setup_regs(map); + return cxl_setup_regs(map, cxl_dev_caps); } int cxl_pci_accel_setup_regs(struct pci_dev *pdev, struct cxl_dev_state *cxlds) @@ -527,7 +528,8 @@ int cxl_pci_accel_setup_regs(struct pci_dev *pdev, struct cxl_dev_state *cxlds) struct cxl_register_map map; int rc; - rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map); + rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map, + cxlds->capabilities); if (rc) return rc; @@ -536,7 +538,7 @@ int cxl_pci_accel_setup_regs(struct pci_dev *pdev, struct cxl_dev_state *cxlds) return rc; rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_COMPONENT, - &cxlds->reg_map); + &cxlds->reg_map, cxlds->capabilities); if (rc) dev_warn(&pdev->dev, "No component registers (%d)\n", rc); @@ -850,7 +852,8 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) dev_warn(&pdev->dev, "Device DVSEC not present, skip CXL.mem init\n"); - rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map); + rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map, + cxlds->capabilities); if (rc) return rc; @@ -863,7 +866,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) * still be useful for management functions so don't return an error. */ rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_COMPONENT, - &cxlds->reg_map); + &cxlds->reg_map, cxlds->capabilities); if (rc) dev_warn(&pdev->dev, "No component registers (%d)\n", rc); else if (!cxlds->reg_map.component_map.ras.valid) diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c index 9cefcaf3caca..37d8bfdef517 100644 --- a/drivers/net/ethernet/sfc/efx_cxl.c +++ b/drivers/net/ethernet/sfc/efx_cxl.c @@ -33,7 +33,8 @@ void efx_cxl_init(struct efx_nic *efx) pci_info(pci_dev, "CXL CXL_DVSEC_PCIE_DEVICE capability found"); - cxl->cxlds = cxl_accel_state_create(&pci_dev->dev); + cxl->cxlds = cxl_accel_state_create(&pci_dev->dev, + CXL_ACCEL_DRIVER_CAP_HDM); if (IS_ERR(cxl->cxlds)) { pci_info(pci_dev, "CXL accel device state failed"); return; diff --git a/include/linux/cxl_accel_mem.h b/include/linux/cxl_accel_mem.h index c7b254edc096..0ba2195b919b 100644 --- a/include/linux/cxl_accel_mem.h +++ b/include/linux/cxl_accel_mem.h @@ -12,8 +12,11 @@ enum accel_resource{ CXL_ACCEL_RES_PMEM, }; +#define CXL_ACCEL_DRIVER_CAP_HDM 0x1 +#define CXL_ACCEL_DRIVER_CAP_MBOX 0x2 + typedef struct cxl_dev_state cxl_accel_state; -cxl_accel_state *cxl_accel_state_create(struct device *dev); +cxl_accel_state *cxl_accel_state_create(struct device *dev, uint8_t caps); void cxl_accel_set_dvsec(cxl_accel_state *cxlds, u16 dvsec); void cxl_accel_set_serial(cxl_accel_state *cxlds, u64 serial);