| Message ID | 20241118164434.7551-2-alejandro.lucero-palau@amd.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cxl: add type2 device basic support | expand |
On 11/18/24 9:44 AM, alejandro.lucero-palau@amd.com wrote: > From: Alejandro Lucero <alucerop@amd.com> > > Differentiate Type3, aka memory expanders, from Type2, aka device > accelerators, with a new function for initializing cxl_dev_state. Please consider: Differentiate CXL memory expanders (type 3) from CXL device accelerators (type 2) with a new ... > > Create accessors to cxl_dev_state to be used by accel drivers. > > Based on previous work by Dan Williams [1] > > Link: [1] https://lore.kernel.org/linux-cxl/168592160379.1948938.12863272903570476312.stgit@dwillia2-xfh.jf.intel.com/ > Signed-off-by: Alejandro Lucero <alucerop@amd.com> > Co-developed-by: Dan Williams <dan.j.williams@intel.com> Reviewed-by: Dave Jiang <dave.jiang@intel.com> > --- > drivers/cxl/core/memdev.c | 51 +++++++++++++++++++++++++++++++++++++++ > drivers/cxl/core/pci.c | 1 + > drivers/cxl/cxlpci.h | 16 ------------ > drivers/cxl/pci.c | 13 +++++++--- > include/cxl/cxl.h | 21 ++++++++++++++++ > include/cxl/pci.h | 23 ++++++++++++++++++ > 6 files changed, 105 insertions(+), 20 deletions(-) > create mode 100644 include/cxl/cxl.h > create mode 100644 include/cxl/pci.h > > diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c > index 84fefb76dafa..d083fd13a6dd 100644 > --- a/drivers/cxl/core/memdev.c > +++ b/drivers/cxl/core/memdev.c > @@ -1,6 +1,7 @@ > // SPDX-License-Identifier: GPL-2.0-only > /* Copyright(c) 2020 Intel Corporation. */ > > +#include <cxl/cxl.h> > #include <linux/io-64-nonatomic-lo-hi.h> > #include <linux/firmware.h> > #include <linux/device.h> > @@ -616,6 +617,25 @@ 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 *cxlds; > + > + cxlds = kzalloc(sizeof(*cxlds), GFP_KERNEL); > + if (!cxlds) > + return ERR_PTR(-ENOMEM); > + > + cxlds->dev = dev; > + cxlds->type = CXL_DEVTYPE_DEVMEM; > + > + cxlds->dpa_res = DEFINE_RES_MEM_NAMED(0, 0, "dpa"); > + cxlds->ram_res = DEFINE_RES_MEM_NAMED(0, 0, "ram"); > + cxlds->pmem_res = DEFINE_RES_MEM_NAMED(0, 0, "pmem"); > + > + return cxlds; > +} > +EXPORT_SYMBOL_NS_GPL(cxl_accel_state_create, CXL); > + > static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds, > const struct file_operations *fops) > { > @@ -693,6 +713,37 @@ static int cxl_memdev_open(struct inode *inode, struct file *file) > return 0; > } > > +void cxl_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec) > +{ > + cxlds->cxl_dvsec = dvsec; > +} > +EXPORT_SYMBOL_NS_GPL(cxl_set_dvsec, CXL); > + > +void cxl_set_serial(struct cxl_dev_state *cxlds, u64 serial) > +{ > + cxlds->serial = serial; > +} > +EXPORT_SYMBOL_NS_GPL(cxl_set_serial, CXL); > + > +int cxl_set_resource(struct cxl_dev_state *cxlds, struct resource res, > + enum cxl_resource type) > +{ > + switch (type) { > + case CXL_RES_DPA: > + cxlds->dpa_res = res; > + return 0; > + case CXL_RES_RAM: > + cxlds->ram_res = res; > + return 0; > + case CXL_RES_PMEM: > + cxlds->pmem_res = res; > + return 0; > + } > + > + return -EINVAL; > +} > +EXPORT_SYMBOL_NS_GPL(cxl_set_resource, CXL); > + > static int cxl_memdev_release_file(struct inode *inode, struct file *file) > { > struct cxl_memdev *cxlmd = > diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c > index 420e4be85a1f..ff266e91ea71 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/cxlpci.h b/drivers/cxl/cxlpci.h > index 4da07727ab9c..eb59019fe5f3 100644 > --- a/drivers/cxl/cxlpci.h > +++ b/drivers/cxl/cxlpci.h > @@ -14,22 +14,6 @@ > */ > #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 188412d45e0d..0b910ef52db7 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> > @@ -816,6 +818,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) > struct cxl_memdev *cxlmd; > int i, rc, pmu_count; > bool irq_avail; > + u16 dvsec; > > /* > * Double check the anonymous union trickery in struct cxl_regs > @@ -836,13 +839,15 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) > 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) > + cxl_set_serial(cxlds, pci_get_dsn(pdev)); > + 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"); > > + cxl_set_dvsec(cxlds, dvsec); > + > rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map); > if (rc) > return rc; > diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h > new file mode 100644 > index 000000000000..19e5d883557a > --- /dev/null > +++ b/include/cxl/cxl.h > @@ -0,0 +1,21 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* Copyright(c) 2024 Advanced Micro Devices, Inc. */ > + > +#ifndef __CXL_H > +#define __CXL_H > + > +#include <linux/ioport.h> > + > +enum cxl_resource { > + CXL_RES_DPA, > + CXL_RES_RAM, > + CXL_RES_PMEM, > +}; > + > +struct cxl_dev_state *cxl_accel_state_create(struct device *dev); > + > +void cxl_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec); > +void cxl_set_serial(struct cxl_dev_state *cxlds, u64 serial); > +int cxl_set_resource(struct cxl_dev_state *cxlds, struct resource res, > + enum cxl_resource); > +#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
On 11/18/24 21:55, Dave Jiang wrote: > > On 11/18/24 9:44 AM, alejandro.lucero-palau@amd.com wrote: >> From: Alejandro Lucero <alucerop@amd.com> >> >> Differentiate Type3, aka memory expanders, from Type2, aka device >> accelerators, with a new function for initializing cxl_dev_state. > Please consider: > Differentiate CXL memory expanders (type 3) from CXL device > accelerators (type 2) with a new ... I'll do. >> Create accessors to cxl_dev_state to be used by accel drivers. >> >> Based on previous work by Dan Williams [1] >> >> Link: [1] https://lore.kernel.org/linux-cxl/168592160379.1948938.12863272903570476312.stgit@dwillia2-xfh.jf.intel.com/ >> Signed-off-by: Alejandro Lucero <alucerop@amd.com> >> Co-developed-by: Dan Williams <dan.j.williams@intel.com> > Reviewed-by: Dave Jiang <dave.jiang@intel.com> Thanks! >> --- >> drivers/cxl/core/memdev.c | 51 +++++++++++++++++++++++++++++++++++++++ >> drivers/cxl/core/pci.c | 1 + >> drivers/cxl/cxlpci.h | 16 ------------ >> drivers/cxl/pci.c | 13 +++++++--- >> include/cxl/cxl.h | 21 ++++++++++++++++ >> include/cxl/pci.h | 23 ++++++++++++++++++ >> 6 files changed, 105 insertions(+), 20 deletions(-) >> create mode 100644 include/cxl/cxl.h >> create mode 100644 include/cxl/pci.h >> >> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c >> index 84fefb76dafa..d083fd13a6dd 100644 >> --- a/drivers/cxl/core/memdev.c >> +++ b/drivers/cxl/core/memdev.c >> @@ -1,6 +1,7 @@ >> // SPDX-License-Identifier: GPL-2.0-only >> /* Copyright(c) 2020 Intel Corporation. */ >> >> +#include <cxl/cxl.h> >> #include <linux/io-64-nonatomic-lo-hi.h> >> #include <linux/firmware.h> >> #include <linux/device.h> >> @@ -616,6 +617,25 @@ 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 *cxlds; >> + >> + cxlds = kzalloc(sizeof(*cxlds), GFP_KERNEL); >> + if (!cxlds) >> + return ERR_PTR(-ENOMEM); >> + >> + cxlds->dev = dev; >> + cxlds->type = CXL_DEVTYPE_DEVMEM; >> + >> + cxlds->dpa_res = DEFINE_RES_MEM_NAMED(0, 0, "dpa"); >> + cxlds->ram_res = DEFINE_RES_MEM_NAMED(0, 0, "ram"); >> + cxlds->pmem_res = DEFINE_RES_MEM_NAMED(0, 0, "pmem"); >> + >> + return cxlds; >> +} >> +EXPORT_SYMBOL_NS_GPL(cxl_accel_state_create, CXL); >> + >> static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds, >> const struct file_operations *fops) >> { >> @@ -693,6 +713,37 @@ static int cxl_memdev_open(struct inode *inode, struct file *file) >> return 0; >> } >> >> +void cxl_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec) >> +{ >> + cxlds->cxl_dvsec = dvsec; >> +} >> +EXPORT_SYMBOL_NS_GPL(cxl_set_dvsec, CXL); >> + >> +void cxl_set_serial(struct cxl_dev_state *cxlds, u64 serial) >> +{ >> + cxlds->serial = serial; >> +} >> +EXPORT_SYMBOL_NS_GPL(cxl_set_serial, CXL); >> + >> +int cxl_set_resource(struct cxl_dev_state *cxlds, struct resource res, >> + enum cxl_resource type) >> +{ >> + switch (type) { >> + case CXL_RES_DPA: >> + cxlds->dpa_res = res; >> + return 0; >> + case CXL_RES_RAM: >> + cxlds->ram_res = res; >> + return 0; >> + case CXL_RES_PMEM: >> + cxlds->pmem_res = res; >> + return 0; >> + } >> + >> + return -EINVAL; >> +} >> +EXPORT_SYMBOL_NS_GPL(cxl_set_resource, CXL); >> + >> static int cxl_memdev_release_file(struct inode *inode, struct file *file) >> { >> struct cxl_memdev *cxlmd = >> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c >> index 420e4be85a1f..ff266e91ea71 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/cxlpci.h b/drivers/cxl/cxlpci.h >> index 4da07727ab9c..eb59019fe5f3 100644 >> --- a/drivers/cxl/cxlpci.h >> +++ b/drivers/cxl/cxlpci.h >> @@ -14,22 +14,6 @@ >> */ >> #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 188412d45e0d..0b910ef52db7 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> >> @@ -816,6 +818,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) >> struct cxl_memdev *cxlmd; >> int i, rc, pmu_count; >> bool irq_avail; >> + u16 dvsec; >> >> /* >> * Double check the anonymous union trickery in struct cxl_regs >> @@ -836,13 +839,15 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) >> 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) >> + cxl_set_serial(cxlds, pci_get_dsn(pdev)); >> + 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"); >> >> + cxl_set_dvsec(cxlds, dvsec); >> + >> rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map); >> if (rc) >> return rc; >> diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h >> new file mode 100644 >> index 000000000000..19e5d883557a >> --- /dev/null >> +++ b/include/cxl/cxl.h >> @@ -0,0 +1,21 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +/* Copyright(c) 2024 Advanced Micro Devices, Inc. */ >> + >> +#ifndef __CXL_H >> +#define __CXL_H >> + >> +#include <linux/ioport.h> >> + >> +enum cxl_resource { >> + CXL_RES_DPA, >> + CXL_RES_RAM, >> + CXL_RES_PMEM, >> +}; >> + >> +struct cxl_dev_state *cxl_accel_state_create(struct device *dev); >> + >> +void cxl_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec); >> +void cxl_set_serial(struct cxl_dev_state *cxlds, u64 serial); >> +int cxl_set_resource(struct cxl_dev_state *cxlds, struct resource res, >> + enum cxl_resource); >> +#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
On Mon, Nov 18, 2024 at 04:44:08PM +0000, alejandro.lucero-palau@amd.com wrote: > From: Alejandro Lucero <alucerop@amd.com> > > Differentiate Type3, aka memory expanders, from Type2, aka device > accelerators, with a new function for initializing cxl_dev_state. > > Create accessors to cxl_dev_state to be used by accel drivers. > > Based on previous work by Dan Williams [1] > > Link: [1] https://lore.kernel.org/linux-cxl/168592160379.1948938.12863272903570476312.stgit@dwillia2-xfh.jf.intel.com/ > Signed-off-by: Alejandro Lucero <alucerop@amd.com> > Co-developed-by: Dan Williams <dan.j.williams@intel.com> Reviewed-by: Fan Ni <fan.ni@samsung.com> > --- > drivers/cxl/core/memdev.c | 51 +++++++++++++++++++++++++++++++++++++++ > drivers/cxl/core/pci.c | 1 + > drivers/cxl/cxlpci.h | 16 ------------ > drivers/cxl/pci.c | 13 +++++++--- > include/cxl/cxl.h | 21 ++++++++++++++++ > include/cxl/pci.h | 23 ++++++++++++++++++ > 6 files changed, 105 insertions(+), 20 deletions(-) > create mode 100644 include/cxl/cxl.h > create mode 100644 include/cxl/pci.h > > diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c > index 84fefb76dafa..d083fd13a6dd 100644 > --- a/drivers/cxl/core/memdev.c > +++ b/drivers/cxl/core/memdev.c > @@ -1,6 +1,7 @@ > // SPDX-License-Identifier: GPL-2.0-only > /* Copyright(c) 2020 Intel Corporation. */ > > +#include <cxl/cxl.h> > #include <linux/io-64-nonatomic-lo-hi.h> > #include <linux/firmware.h> > #include <linux/device.h> > @@ -616,6 +617,25 @@ 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 *cxlds; > + > + cxlds = kzalloc(sizeof(*cxlds), GFP_KERNEL); > + if (!cxlds) > + return ERR_PTR(-ENOMEM); > + > + cxlds->dev = dev; > + cxlds->type = CXL_DEVTYPE_DEVMEM; > + > + cxlds->dpa_res = DEFINE_RES_MEM_NAMED(0, 0, "dpa"); > + cxlds->ram_res = DEFINE_RES_MEM_NAMED(0, 0, "ram"); > + cxlds->pmem_res = DEFINE_RES_MEM_NAMED(0, 0, "pmem"); > + > + return cxlds; > +} > +EXPORT_SYMBOL_NS_GPL(cxl_accel_state_create, CXL); > + > static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds, > const struct file_operations *fops) > { > @@ -693,6 +713,37 @@ static int cxl_memdev_open(struct inode *inode, struct file *file) > return 0; > } > > +void cxl_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec) > +{ > + cxlds->cxl_dvsec = dvsec; > +} > +EXPORT_SYMBOL_NS_GPL(cxl_set_dvsec, CXL); > + > +void cxl_set_serial(struct cxl_dev_state *cxlds, u64 serial) > +{ > + cxlds->serial = serial; > +} > +EXPORT_SYMBOL_NS_GPL(cxl_set_serial, CXL); > + > +int cxl_set_resource(struct cxl_dev_state *cxlds, struct resource res, > + enum cxl_resource type) > +{ > + switch (type) { > + case CXL_RES_DPA: > + cxlds->dpa_res = res; > + return 0; > + case CXL_RES_RAM: > + cxlds->ram_res = res; > + return 0; > + case CXL_RES_PMEM: > + cxlds->pmem_res = res; > + return 0; > + } > + > + return -EINVAL; > +} > +EXPORT_SYMBOL_NS_GPL(cxl_set_resource, CXL); > + > static int cxl_memdev_release_file(struct inode *inode, struct file *file) > { > struct cxl_memdev *cxlmd = > diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c > index 420e4be85a1f..ff266e91ea71 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/cxlpci.h b/drivers/cxl/cxlpci.h > index 4da07727ab9c..eb59019fe5f3 100644 > --- a/drivers/cxl/cxlpci.h > +++ b/drivers/cxl/cxlpci.h > @@ -14,22 +14,6 @@ > */ > #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 188412d45e0d..0b910ef52db7 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> > @@ -816,6 +818,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) > struct cxl_memdev *cxlmd; > int i, rc, pmu_count; > bool irq_avail; > + u16 dvsec; > > /* > * Double check the anonymous union trickery in struct cxl_regs > @@ -836,13 +839,15 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) > 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) > + cxl_set_serial(cxlds, pci_get_dsn(pdev)); > + 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"); > > + cxl_set_dvsec(cxlds, dvsec); > + > rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map); > if (rc) > return rc; > diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h > new file mode 100644 > index 000000000000..19e5d883557a > --- /dev/null > +++ b/include/cxl/cxl.h > @@ -0,0 +1,21 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* Copyright(c) 2024 Advanced Micro Devices, Inc. */ > + > +#ifndef __CXL_H > +#define __CXL_H > + > +#include <linux/ioport.h> > + > +enum cxl_resource { > + CXL_RES_DPA, > + CXL_RES_RAM, > + CXL_RES_PMEM, > +}; > + > +struct cxl_dev_state *cxl_accel_state_create(struct device *dev); > + > +void cxl_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec); > +void cxl_set_serial(struct cxl_dev_state *cxlds, u64 serial); > +int cxl_set_resource(struct cxl_dev_state *cxlds, struct resource res, > + enum cxl_resource); > +#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 > -- > 2.17.1 >
On Mon, Nov 18, 2024 at 04:44:08PM +0000, alejandro.lucero-palau@amd.com wrote: > From: Alejandro Lucero <alucerop@amd.com> > > Differentiate Type3, aka memory expanders, from Type2, aka device > accelerators, with a new function for initializing cxl_dev_state. > > Create accessors to cxl_dev_state to be used by accel drivers. > > Based on previous work by Dan Williams [1] > > Link: [1] https://lore.kernel.org/linux-cxl/168592160379.1948938.12863272903570476312.stgit@dwillia2-xfh.jf.intel.com/ > Signed-off-by: Alejandro Lucero <alucerop@amd.com> > Co-developed-by: Dan Williams <dan.j.williams@intel.com> > --- > drivers/cxl/core/memdev.c | 51 +++++++++++++++++++++++++++++++++++++++ > drivers/cxl/core/pci.c | 1 + > drivers/cxl/cxlpci.h | 16 ------------ > drivers/cxl/pci.c | 13 +++++++--- > include/cxl/cxl.h | 21 ++++++++++++++++ As I mentioned in the cover letter, beginning w the first patch I have depmod issues building with the cxl-test module. I didn't get very far figuring it out, other than a work-around of moving the contents of include/cxl/cxl.h down into drivers/cxl/cxlmem.h. That band-aid got me a bit further. In fact I wasn't so concerned with breaking sfx as I was with regression testing the changes to drivers/cxl/. Please see if you can get the cxl-test module working again. > include/cxl/pci.h | 23 ++++++++++++++++++ > 6 files changed, 105 insertions(+), 20 deletions(-) > create mode 100644 include/cxl/cxl.h > create mode 100644 include/cxl/pci.h > > diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c > index 84fefb76dafa..d083fd13a6dd 100644 > --- a/drivers/cxl/core/memdev.c > +++ b/drivers/cxl/core/memdev.c > @@ -1,6 +1,7 @@ > // SPDX-License-Identifier: GPL-2.0-only > /* Copyright(c) 2020 Intel Corporation. */ > > +#include <cxl/cxl.h> > #include <linux/io-64-nonatomic-lo-hi.h> > #include <linux/firmware.h> > #include <linux/device.h> > @@ -616,6 +617,25 @@ 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 *cxlds; > + > + cxlds = kzalloc(sizeof(*cxlds), GFP_KERNEL); > + if (!cxlds) > + return ERR_PTR(-ENOMEM); > + > + cxlds->dev = dev; > + cxlds->type = CXL_DEVTYPE_DEVMEM; > + > + cxlds->dpa_res = DEFINE_RES_MEM_NAMED(0, 0, "dpa"); > + cxlds->ram_res = DEFINE_RES_MEM_NAMED(0, 0, "ram"); > + cxlds->pmem_res = DEFINE_RES_MEM_NAMED(0, 0, "pmem"); > + > + return cxlds; > +} > +EXPORT_SYMBOL_NS_GPL(cxl_accel_state_create, CXL); > + > static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds, > const struct file_operations *fops) > { > @@ -693,6 +713,37 @@ static int cxl_memdev_open(struct inode *inode, struct file *file) > return 0; > } > > +void cxl_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec) > +{ > + cxlds->cxl_dvsec = dvsec; > +} > +EXPORT_SYMBOL_NS_GPL(cxl_set_dvsec, CXL); > + > +void cxl_set_serial(struct cxl_dev_state *cxlds, u64 serial) > +{ > + cxlds->serial = serial; > +} > +EXPORT_SYMBOL_NS_GPL(cxl_set_serial, CXL); > + > +int cxl_set_resource(struct cxl_dev_state *cxlds, struct resource res, > + enum cxl_resource type) > +{ > + switch (type) { > + case CXL_RES_DPA: > + cxlds->dpa_res = res; > + return 0; > + case CXL_RES_RAM: > + cxlds->ram_res = res; > + return 0; > + case CXL_RES_PMEM: > + cxlds->pmem_res = res; > + return 0; > + } > + > + return -EINVAL; > +} > +EXPORT_SYMBOL_NS_GPL(cxl_set_resource, CXL); > + > static int cxl_memdev_release_file(struct inode *inode, struct file *file) > { > struct cxl_memdev *cxlmd = > diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c > index 420e4be85a1f..ff266e91ea71 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/cxlpci.h b/drivers/cxl/cxlpci.h > index 4da07727ab9c..eb59019fe5f3 100644 > --- a/drivers/cxl/cxlpci.h > +++ b/drivers/cxl/cxlpci.h > @@ -14,22 +14,6 @@ > */ > #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 188412d45e0d..0b910ef52db7 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> > @@ -816,6 +818,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) > struct cxl_memdev *cxlmd; > int i, rc, pmu_count; > bool irq_avail; > + u16 dvsec; > > /* > * Double check the anonymous union trickery in struct cxl_regs > @@ -836,13 +839,15 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) > 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) > + cxl_set_serial(cxlds, pci_get_dsn(pdev)); > + 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"); > > + cxl_set_dvsec(cxlds, dvsec); > + > rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map); > if (rc) > return rc; > diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h > new file mode 100644 > index 000000000000..19e5d883557a > --- /dev/null > +++ b/include/cxl/cxl.h > @@ -0,0 +1,21 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* Copyright(c) 2024 Advanced Micro Devices, Inc. */ > + > +#ifndef __CXL_H > +#define __CXL_H > + > +#include <linux/ioport.h> > + > +enum cxl_resource { > + CXL_RES_DPA, > + CXL_RES_RAM, > + CXL_RES_PMEM, > +}; > + > +struct cxl_dev_state *cxl_accel_state_create(struct device *dev); > + > +void cxl_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec); > +void cxl_set_serial(struct cxl_dev_state *cxlds, u64 serial); > +int cxl_set_resource(struct cxl_dev_state *cxlds, struct resource res, > + enum cxl_resource); > +#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 > -- > 2.17.1 > >
On 11/22/24 04:35, Alison Schofield wrote: > On Mon, Nov 18, 2024 at 04:44:08PM +0000, alejandro.lucero-palau@amd.com wrote: >> From: Alejandro Lucero <alucerop@amd.com> >> >> Differentiate Type3, aka memory expanders, from Type2, aka device >> accelerators, with a new function for initializing cxl_dev_state. >> >> Create accessors to cxl_dev_state to be used by accel drivers. >> >> Based on previous work by Dan Williams [1] >> >> Link: [1] https://lore.kernel.org/linux-cxl/168592160379.1948938.12863272903570476312.stgit@dwillia2-xfh.jf.intel.com/ >> Signed-off-by: Alejandro Lucero <alucerop@amd.com> >> Co-developed-by: Dan Williams <dan.j.williams@intel.com> >> --- >> drivers/cxl/core/memdev.c | 51 +++++++++++++++++++++++++++++++++++++++ >> drivers/cxl/core/pci.c | 1 + >> drivers/cxl/cxlpci.h | 16 ------------ >> drivers/cxl/pci.c | 13 +++++++--- >> include/cxl/cxl.h | 21 ++++++++++++++++ > As I mentioned in the cover letter, beginning w the first patch > I have depmod issues building with the cxl-test module. I didn't > get very far figuring it out, other than a work-around of moving > the contents of include/cxl/cxl.h down into drivers/cxl/cxlmem.h. > That band-aid got me a bit further. In fact I wasn't so concerned > with breaking sfx as I was with regression testing the changes to > drivers/cxl/. > > Please see if you can get the cxl-test module working again. Hi Allison, I have no problems building tools/testing/cxl and I can see cxl_test.ko in tools/testing/cxl/test I did try with the full patchset applied over 6.12-rc7 tag, and also with only the first patch since I was not sure if you meant the build after each patch is tried, but both worked once I modified the config for the checks inside config_check.c not to fail. I guess you meant this cxl test and not the one related to "git clone https://github.com/moking/cxl-test-tool.git" what I have no experience with. Could someone else try this as well? > >> include/cxl/pci.h | 23 ++++++++++++++++++ >> 6 files changed, 105 insertions(+), 20 deletions(-) >> create mode 100644 include/cxl/cxl.h >> create mode 100644 include/cxl/pci.h >> >> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c >> index 84fefb76dafa..d083fd13a6dd 100644 >> --- a/drivers/cxl/core/memdev.c >> +++ b/drivers/cxl/core/memdev.c >> @@ -1,6 +1,7 @@ >> // SPDX-License-Identifier: GPL-2.0-only >> /* Copyright(c) 2020 Intel Corporation. */ >> >> +#include <cxl/cxl.h> >> #include <linux/io-64-nonatomic-lo-hi.h> >> #include <linux/firmware.h> >> #include <linux/device.h> >> @@ -616,6 +617,25 @@ 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 *cxlds; >> + >> + cxlds = kzalloc(sizeof(*cxlds), GFP_KERNEL); >> + if (!cxlds) >> + return ERR_PTR(-ENOMEM); >> + >> + cxlds->dev = dev; >> + cxlds->type = CXL_DEVTYPE_DEVMEM; >> + >> + cxlds->dpa_res = DEFINE_RES_MEM_NAMED(0, 0, "dpa"); >> + cxlds->ram_res = DEFINE_RES_MEM_NAMED(0, 0, "ram"); >> + cxlds->pmem_res = DEFINE_RES_MEM_NAMED(0, 0, "pmem"); >> + >> + return cxlds; >> +} >> +EXPORT_SYMBOL_NS_GPL(cxl_accel_state_create, CXL); >> + >> static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds, >> const struct file_operations *fops) >> { >> @@ -693,6 +713,37 @@ static int cxl_memdev_open(struct inode *inode, struct file *file) >> return 0; >> } >> >> +void cxl_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec) >> +{ >> + cxlds->cxl_dvsec = dvsec; >> +} >> +EXPORT_SYMBOL_NS_GPL(cxl_set_dvsec, CXL); >> + >> +void cxl_set_serial(struct cxl_dev_state *cxlds, u64 serial) >> +{ >> + cxlds->serial = serial; >> +} >> +EXPORT_SYMBOL_NS_GPL(cxl_set_serial, CXL); >> + >> +int cxl_set_resource(struct cxl_dev_state *cxlds, struct resource res, >> + enum cxl_resource type) >> +{ >> + switch (type) { >> + case CXL_RES_DPA: >> + cxlds->dpa_res = res; >> + return 0; >> + case CXL_RES_RAM: >> + cxlds->ram_res = res; >> + return 0; >> + case CXL_RES_PMEM: >> + cxlds->pmem_res = res; >> + return 0; >> + } >> + >> + return -EINVAL; >> +} >> +EXPORT_SYMBOL_NS_GPL(cxl_set_resource, CXL); >> + >> static int cxl_memdev_release_file(struct inode *inode, struct file *file) >> { >> struct cxl_memdev *cxlmd = >> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c >> index 420e4be85a1f..ff266e91ea71 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/cxlpci.h b/drivers/cxl/cxlpci.h >> index 4da07727ab9c..eb59019fe5f3 100644 >> --- a/drivers/cxl/cxlpci.h >> +++ b/drivers/cxl/cxlpci.h >> @@ -14,22 +14,6 @@ >> */ >> #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 188412d45e0d..0b910ef52db7 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> >> @@ -816,6 +818,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) >> struct cxl_memdev *cxlmd; >> int i, rc, pmu_count; >> bool irq_avail; >> + u16 dvsec; >> >> /* >> * Double check the anonymous union trickery in struct cxl_regs >> @@ -836,13 +839,15 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) >> 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) >> + cxl_set_serial(cxlds, pci_get_dsn(pdev)); >> + 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"); >> >> + cxl_set_dvsec(cxlds, dvsec); >> + >> rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map); >> if (rc) >> return rc; >> diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h >> new file mode 100644 >> index 000000000000..19e5d883557a >> --- /dev/null >> +++ b/include/cxl/cxl.h >> @@ -0,0 +1,21 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +/* Copyright(c) 2024 Advanced Micro Devices, Inc. */ >> + >> +#ifndef __CXL_H >> +#define __CXL_H >> + >> +#include <linux/ioport.h> >> + >> +enum cxl_resource { >> + CXL_RES_DPA, >> + CXL_RES_RAM, >> + CXL_RES_PMEM, >> +}; >> + >> +struct cxl_dev_state *cxl_accel_state_create(struct device *dev); >> + >> +void cxl_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec); >> +void cxl_set_serial(struct cxl_dev_state *cxlds, u64 serial); >> +int cxl_set_resource(struct cxl_dev_state *cxlds, struct resource res, >> + enum cxl_resource); >> +#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 >> -- >> 2.17.1 >> >>
On 11/18/24 10:44 AM, alejandro.lucero-palau@amd.com wrote: > From: Alejandro Lucero <alucerop@amd.com> > > Differentiate Type3, aka memory expanders, from Type2, aka device > accelerators, with a new function for initializing cxl_dev_state. > > Create accessors to cxl_dev_state to be used by accel drivers. > > Based on previous work by Dan Williams [1] > > Link: [1] https://lore.kernel.org/linux-cxl/168592160379.1948938.12863272903570476312.stgit@dwillia2-xfh.jf.intel.com/ > Signed-off-by: Alejandro Lucero <alucerop@amd.com> > Co-developed-by: Dan Williams <dan.j.williams@intel.com> > --- > drivers/cxl/core/memdev.c | 51 +++++++++++++++++++++++++++++++++++++++ > drivers/cxl/core/pci.c | 1 + > drivers/cxl/cxlpci.h | 16 ------------ > drivers/cxl/pci.c | 13 +++++++--- > include/cxl/cxl.h | 21 ++++++++++++++++ > include/cxl/pci.h | 23 ++++++++++++++++++ > 6 files changed, 105 insertions(+), 20 deletions(-) > create mode 100644 include/cxl/cxl.h > create mode 100644 include/cxl/pci.h > > diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c > index 84fefb76dafa..d083fd13a6dd 100644 > --- a/drivers/cxl/core/memdev.c > +++ b/drivers/cxl/core/memdev.c > @@ -1,6 +1,7 @@ > // SPDX-License-Identifier: GPL-2.0-only > /* Copyright(c) 2020 Intel Corporation. */ > > +#include <cxl/cxl.h> Pedantic one, you'll want this at the end CXL does reverse christmas tree for #includes. > #include <linux/io-64-nonatomic-lo-hi.h> > #include <linux/firmware.h> > #include <linux/device.h> > @@ -616,6 +617,25 @@ 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 *cxlds; > + > + cxlds = kzalloc(sizeof(*cxlds), GFP_KERNEL); Would it be better to use a devm_kzalloc() here? I'd imagine this function will be called as part of probe a majority of the time so I think the automatic cleanup would be nice here. If you did that, then I'd also rename the function to include devm_ as well. > + if (!cxlds) > + return ERR_PTR(-ENOMEM); > + > + cxlds->dev = dev; > + cxlds->type = CXL_DEVTYPE_DEVMEM; > + > + cxlds->dpa_res = DEFINE_RES_MEM_NAMED(0, 0, "dpa"); > + cxlds->ram_res = DEFINE_RES_MEM_NAMED(0, 0, "ram"); > + cxlds->pmem_res = DEFINE_RES_MEM_NAMED(0, 0, "pmem"); > + > + return cxlds; > +} > +EXPORT_SYMBOL_NS_GPL(cxl_accel_state_create, CXL); > + > static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds, > const struct file_operations *fops) > { > @@ -693,6 +713,37 @@ static int cxl_memdev_open(struct inode *inode, struct file *file) > return 0; > } > > +void cxl_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec) > +{ > + cxlds->cxl_dvsec = dvsec; > +} > +EXPORT_SYMBOL_NS_GPL(cxl_set_dvsec, CXL); > + > +void cxl_set_serial(struct cxl_dev_state *cxlds, u64 serial) > +{ > + cxlds->serial = serial; > +} > +EXPORT_SYMBOL_NS_GPL(cxl_set_serial, CXL); > + > +int cxl_set_resource(struct cxl_dev_state *cxlds, struct resource res, > + enum cxl_resource type) > +{ > + switch (type) { > + case CXL_RES_DPA: > + cxlds->dpa_res = res; > + return 0; > + case CXL_RES_RAM: > + cxlds->ram_res = res; > + return 0; > + case CXL_RES_PMEM: > + cxlds->pmem_res = res; > + return 0; > + } > + > + return -EINVAL; > +} > +EXPORT_SYMBOL_NS_GPL(cxl_set_resource, CXL); > + > static int cxl_memdev_release_file(struct inode *inode, struct file *file) > { > struct cxl_memdev *cxlmd = > diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c > index 420e4be85a1f..ff266e91ea71 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/cxlpci.h b/drivers/cxl/cxlpci.h > index 4da07727ab9c..eb59019fe5f3 100644 > --- a/drivers/cxl/cxlpci.h > +++ b/drivers/cxl/cxlpci.h > @@ -14,22 +14,6 @@ > */ > #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 188412d45e0d..0b910ef52db7 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> > @@ -816,6 +818,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) > struct cxl_memdev *cxlmd; > int i, rc, pmu_count; > bool irq_avail; > + u16 dvsec; > > /* > * Double check the anonymous union trickery in struct cxl_regs > @@ -836,13 +839,15 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) > 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) > + cxl_set_serial(cxlds, pci_get_dsn(pdev)); > + 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"); > > + cxl_set_dvsec(cxlds, dvsec); > + > rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map); > if (rc) > return rc; > diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h > new file mode 100644 > index 000000000000..19e5d883557a > --- /dev/null > +++ b/include/cxl/cxl.h Is cxl.h the right name for this file? I initially thought this was the cxl.h under drivers/cxl. It looks like it's just type 2 related functions, so maybe "type2.h", or "accel.h" would be better? If the plan is to expose more CXL functionality not necessarily related to type 2 devices later I'm fine with it, and if no one else cares then I'm fine with it. > @@ -0,0 +1,21 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* Copyright(c) 2024 Advanced Micro Devices, Inc. */ > + > +#ifndef __CXL_H > +#define __CXL_H > + > +#include <linux/ioport.h> > + > +enum cxl_resource { > + CXL_RES_DPA, > + CXL_RES_RAM, > + CXL_RES_PMEM, > +}; > + > +struct cxl_dev_state *cxl_accel_state_create(struct device *dev); > + > +void cxl_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec); > +void cxl_set_serial(struct cxl_dev_state *cxlds, u64 serial); > +int cxl_set_resource(struct cxl_dev_state *cxlds, struct resource res, > + enum cxl_resource); > +#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
On Fri, Nov 22, 2024 at 09:27:34AM +0000, Alejandro Lucero Palau wrote: > > On 11/22/24 04:35, Alison Schofield wrote: > > On Mon, Nov 18, 2024 at 04:44:08PM +0000, alejandro.lucero-palau@amd.com wrote: > > > From: Alejandro Lucero <alucerop@amd.com> > > > > > > Differentiate Type3, aka memory expanders, from Type2, aka device > > > accelerators, with a new function for initializing cxl_dev_state. > > > > > > Create accessors to cxl_dev_state to be used by accel drivers. > > > > > > Based on previous work by Dan Williams [1] > > > > > > Link: [1] https://lore.kernel.org/linux-cxl/168592160379.1948938.12863272903570476312.stgit@dwillia2-xfh.jf.intel.com/ > > > Signed-off-by: Alejandro Lucero <alucerop@amd.com> > > > Co-developed-by: Dan Williams <dan.j.williams@intel.com> > > > --- > > > drivers/cxl/core/memdev.c | 51 +++++++++++++++++++++++++++++++++++++++ > > > drivers/cxl/core/pci.c | 1 + > > > drivers/cxl/cxlpci.h | 16 ------------ > > > drivers/cxl/pci.c | 13 +++++++--- > > > include/cxl/cxl.h | 21 ++++++++++++++++ > > As I mentioned in the cover letter, beginning w the first patch > > I have depmod issues building with the cxl-test module. I didn't > > get very far figuring it out, other than a work-around of moving > > the contents of include/cxl/cxl.h down into drivers/cxl/cxlmem.h. > > That band-aid got me a bit further. In fact I wasn't so concerned > > with breaking sfx as I was with regression testing the changes to > > drivers/cxl/. > > > > Please see if you can get the cxl-test module working again. > > > Hi Allison, > > > I have no problems building tools/testing/cxl and I can see cxl_test.ko in > tools/testing/cxl/test > Yes that's the one. It builds it just won't load because of the circular dependency. Sorry I haven't been able to dig into it further. I use run_qemu.sh [1] which uses mkosi to build the image. It fails at the depmod step and I haven't been able to dig further. depmod: ERROR: Cycle detected: cxl_mock -> cxl_core -> cxl_mock depmod: ERROR: Found 2 modules in dependency cycles! So, I'd expect it would fail at make modules_intall for you. BTW - this happens occasionally, but usually on a smaller scale, ie we know exactly what just changed. I suspect it happens with only Patch 1 applied - but even limiting it to that I could not nail it down. --Alison [1] https://github.com/pmem/run_qemu > > I did try with the full patchset applied over 6.12-rc7 tag, and also with > only the first patch since I was not sure if you meant the build after each > patch is tried, but both worked once I modified the config for the checks > inside config_check.c not to fail. > > > I guess you meant this cxl test and not the one related to "git clone > https://github.com/moking/cxl-test-tool.git" what I have no experience with. no > > > Could someone else try this as well? > > > > > > > include/cxl/pci.h | 23 ++++++++++++++++++ > > > 6 files changed, 105 insertions(+), 20 deletions(-) > > > create mode 100644 include/cxl/cxl.h > > > create mode 100644 include/cxl/pci.h > > > > > > diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c > > > index 84fefb76dafa..d083fd13a6dd 100644 > > > --- a/drivers/cxl/core/memdev.c > > > +++ b/drivers/cxl/core/memdev.c > > > @@ -1,6 +1,7 @@ > > > // SPDX-License-Identifier: GPL-2.0-only > > > /* Copyright(c) 2020 Intel Corporation. */ > > > +#include <cxl/cxl.h> > > > #include <linux/io-64-nonatomic-lo-hi.h> > > > #include <linux/firmware.h> > > > #include <linux/device.h> > > > @@ -616,6 +617,25 @@ 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 *cxlds; > > > + > > > + cxlds = kzalloc(sizeof(*cxlds), GFP_KERNEL); > > > + if (!cxlds) > > > + return ERR_PTR(-ENOMEM); > > > + > > > + cxlds->dev = dev; > > > + cxlds->type = CXL_DEVTYPE_DEVMEM; > > > + > > > + cxlds->dpa_res = DEFINE_RES_MEM_NAMED(0, 0, "dpa"); > > > + cxlds->ram_res = DEFINE_RES_MEM_NAMED(0, 0, "ram"); > > > + cxlds->pmem_res = DEFINE_RES_MEM_NAMED(0, 0, "pmem"); > > > + > > > + return cxlds; > > > +} > > > +EXPORT_SYMBOL_NS_GPL(cxl_accel_state_create, CXL); > > > + > > > static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds, > > > const struct file_operations *fops) > > > { > > > @@ -693,6 +713,37 @@ static int cxl_memdev_open(struct inode *inode, struct file *file) > > > return 0; > > > } > > > +void cxl_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec) > > > +{ > > > + cxlds->cxl_dvsec = dvsec; > > > +} > > > +EXPORT_SYMBOL_NS_GPL(cxl_set_dvsec, CXL); > > > + > > > +void cxl_set_serial(struct cxl_dev_state *cxlds, u64 serial) > > > +{ > > > + cxlds->serial = serial; > > > +} > > > +EXPORT_SYMBOL_NS_GPL(cxl_set_serial, CXL); > > > + > > > +int cxl_set_resource(struct cxl_dev_state *cxlds, struct resource res, > > > + enum cxl_resource type) > > > +{ > > > + switch (type) { > > > + case CXL_RES_DPA: > > > + cxlds->dpa_res = res; > > > + return 0; > > > + case CXL_RES_RAM: > > > + cxlds->ram_res = res; > > > + return 0; > > > + case CXL_RES_PMEM: > > > + cxlds->pmem_res = res; > > > + return 0; > > > + } > > > + > > > + return -EINVAL; > > > +} > > > +EXPORT_SYMBOL_NS_GPL(cxl_set_resource, CXL); > > > + > > > static int cxl_memdev_release_file(struct inode *inode, struct file *file) > > > { > > > struct cxl_memdev *cxlmd = > > > diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c > > > index 420e4be85a1f..ff266e91ea71 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/cxlpci.h b/drivers/cxl/cxlpci.h > > > index 4da07727ab9c..eb59019fe5f3 100644 > > > --- a/drivers/cxl/cxlpci.h > > > +++ b/drivers/cxl/cxlpci.h > > > @@ -14,22 +14,6 @@ > > > */ > > > #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 188412d45e0d..0b910ef52db7 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> > > > @@ -816,6 +818,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) > > > struct cxl_memdev *cxlmd; > > > int i, rc, pmu_count; > > > bool irq_avail; > > > + u16 dvsec; > > > /* > > > * Double check the anonymous union trickery in struct cxl_regs > > > @@ -836,13 +839,15 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) > > > 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) > > > + cxl_set_serial(cxlds, pci_get_dsn(pdev)); > > > + 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"); > > > + cxl_set_dvsec(cxlds, dvsec); > > > + > > > rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map); > > > if (rc) > > > return rc; > > > diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h > > > new file mode 100644 > > > index 000000000000..19e5d883557a > > > --- /dev/null > > > +++ b/include/cxl/cxl.h > > > @@ -0,0 +1,21 @@ > > > +/* SPDX-License-Identifier: GPL-2.0 */ > > > +/* Copyright(c) 2024 Advanced Micro Devices, Inc. */ > > > + > > > +#ifndef __CXL_H > > > +#define __CXL_H > > > + > > > +#include <linux/ioport.h> > > > + > > > +enum cxl_resource { > > > + CXL_RES_DPA, > > > + CXL_RES_RAM, > > > + CXL_RES_PMEM, > > > +}; > > > + > > > +struct cxl_dev_state *cxl_accel_state_create(struct device *dev); > > > + > > > +void cxl_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec); > > > +void cxl_set_serial(struct cxl_dev_state *cxlds, u64 serial); > > > +int cxl_set_resource(struct cxl_dev_state *cxlds, struct resource res, > > > + enum cxl_resource); > > > +#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 > > > -- > > > 2.17.1 > > > > > >
On 11/26/24 05:59, Alison Schofield wrote: > On Fri, Nov 22, 2024 at 09:27:34AM +0000, Alejandro Lucero Palau wrote: >> On 11/22/24 04:35, Alison Schofield wrote: >>> On Mon, Nov 18, 2024 at 04:44:08PM +0000, alejandro.lucero-palau@amd.com wrote: >>>> From: Alejandro Lucero <alucerop@amd.com> >>>> >>>> Differentiate Type3, aka memory expanders, from Type2, aka device >>>> accelerators, with a new function for initializing cxl_dev_state. >>>> >>>> Create accessors to cxl_dev_state to be used by accel drivers. >>>> >>>> Based on previous work by Dan Williams [1] >>>> >>>> Link: [1] https://lore.kernel.org/linux-cxl/168592160379.1948938.12863272903570476312.stgit@dwillia2-xfh.jf.intel.com/ >>>> Signed-off-by: Alejandro Lucero <alucerop@amd.com> >>>> Co-developed-by: Dan Williams <dan.j.williams@intel.com> >>>> --- >>>> drivers/cxl/core/memdev.c | 51 +++++++++++++++++++++++++++++++++++++++ >>>> drivers/cxl/core/pci.c | 1 + >>>> drivers/cxl/cxlpci.h | 16 ------------ >>>> drivers/cxl/pci.c | 13 +++++++--- >>>> include/cxl/cxl.h | 21 ++++++++++++++++ >>> As I mentioned in the cover letter, beginning w the first patch >>> I have depmod issues building with the cxl-test module. I didn't >>> get very far figuring it out, other than a work-around of moving >>> the contents of include/cxl/cxl.h down into drivers/cxl/cxlmem.h. >>> That band-aid got me a bit further. In fact I wasn't so concerned >>> with breaking sfx as I was with regression testing the changes to >>> drivers/cxl/. >>> >>> Please see if you can get the cxl-test module working again. >> >> Hi Allison, >> >> >> I have no problems building tools/testing/cxl and I can see cxl_test.ko in >> tools/testing/cxl/test >> > Yes that's the one. It builds it just won't load because of the > circular dependency. Sorry I haven't been able to dig into it > further. I use run_qemu.sh [1] which uses mkosi to build the image. > It fails at the depmod step and I haven't been able to dig further. > > depmod: ERROR: Cycle detected: cxl_mock -> cxl_core -> cxl_mock > depmod: ERROR: Found 2 modules in dependency cycles! > > So, I'd expect it would fail at make modules_intall for you. Linus's tree with 6.12 tag works fine for me. I'm doing it manually, creating the modules inside tools/testing/cxl and copying them to /lib/modules/6.12.0/kernel/drivers/cxl replacing the default cxl kernel modules by these with the testing stuff, and copying there the modules inside tools/testing/cxl/test where cxl_mock.ko and cxl_test.ko are. Depmod works fine after that and I can use that kernel+modules loading cxl_test.ko and cxl_mock.ko without any issue. I have problems with the cxl next tree and the current Linux's tree uptodate, with some error when installing modules but not related to this problem you report. IMO, it seems it is a problem with the cxl testing code itself, or the build, because I can not see a reason for the cxl_core depending on cxl_mock except for those changes introduced for testing. I have been all day moving from one tree to another and doing tests, so I leave this to someone familiar with the mock testing sources. BTW, I think all this should be documented along with the CXL tree to work with. I know there are instructions in that cxl-test-tool repo but it should be a reference in cxl.docs.kernel.org about it. > > BTW - this happens occasionally, but usually on a smaller scale, > ie we know exactly what just changed. I suspect it happens with > only Patch 1 applied - but even limiting it to that I could not > nail it down. > > --Alison > > > [1] https://github.com/pmem/run_qemu > >> I did try with the full patchset applied over 6.12-rc7 tag, and also with >> only the first patch since I was not sure if you meant the build after each >> patch is tried, but both worked once I modified the config for the checks >> inside config_check.c not to fail. >> >> >> I guess you meant this cxl test and not the one related to "git clone >> https://github.com/moking/cxl-test-tool.git" what I have no experience with. > no > > > >> >> Could someone else try this as well? >> >> >>>> include/cxl/pci.h | 23 ++++++++++++++++++ >>>> 6 files changed, 105 insertions(+), 20 deletions(-) >>>> create mode 100644 include/cxl/cxl.h >>>> create mode 100644 include/cxl/pci.h >>>> >>>> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c >>>> index 84fefb76dafa..d083fd13a6dd 100644 >>>> --- a/drivers/cxl/core/memdev.c >>>> +++ b/drivers/cxl/core/memdev.c >>>> @@ -1,6 +1,7 @@ >>>> // SPDX-License-Identifier: GPL-2.0-only >>>> /* Copyright(c) 2020 Intel Corporation. */ >>>> +#include <cxl/cxl.h> >>>> #include <linux/io-64-nonatomic-lo-hi.h> >>>> #include <linux/firmware.h> >>>> #include <linux/device.h> >>>> @@ -616,6 +617,25 @@ 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 *cxlds; >>>> + >>>> + cxlds = kzalloc(sizeof(*cxlds), GFP_KERNEL); >>>> + if (!cxlds) >>>> + return ERR_PTR(-ENOMEM); >>>> + >>>> + cxlds->dev = dev; >>>> + cxlds->type = CXL_DEVTYPE_DEVMEM; >>>> + >>>> + cxlds->dpa_res = DEFINE_RES_MEM_NAMED(0, 0, "dpa"); >>>> + cxlds->ram_res = DEFINE_RES_MEM_NAMED(0, 0, "ram"); >>>> + cxlds->pmem_res = DEFINE_RES_MEM_NAMED(0, 0, "pmem"); >>>> + >>>> + return cxlds; >>>> +} >>>> +EXPORT_SYMBOL_NS_GPL(cxl_accel_state_create, CXL); >>>> + >>>> static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds, >>>> const struct file_operations *fops) >>>> { >>>> @@ -693,6 +713,37 @@ static int cxl_memdev_open(struct inode *inode, struct file *file) >>>> return 0; >>>> } >>>> +void cxl_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec) >>>> +{ >>>> + cxlds->cxl_dvsec = dvsec; >>>> +} >>>> +EXPORT_SYMBOL_NS_GPL(cxl_set_dvsec, CXL); >>>> + >>>> +void cxl_set_serial(struct cxl_dev_state *cxlds, u64 serial) >>>> +{ >>>> + cxlds->serial = serial; >>>> +} >>>> +EXPORT_SYMBOL_NS_GPL(cxl_set_serial, CXL); >>>> + >>>> +int cxl_set_resource(struct cxl_dev_state *cxlds, struct resource res, >>>> + enum cxl_resource type) >>>> +{ >>>> + switch (type) { >>>> + case CXL_RES_DPA: >>>> + cxlds->dpa_res = res; >>>> + return 0; >>>> + case CXL_RES_RAM: >>>> + cxlds->ram_res = res; >>>> + return 0; >>>> + case CXL_RES_PMEM: >>>> + cxlds->pmem_res = res; >>>> + return 0; >>>> + } >>>> + >>>> + return -EINVAL; >>>> +} >>>> +EXPORT_SYMBOL_NS_GPL(cxl_set_resource, CXL); >>>> + >>>> static int cxl_memdev_release_file(struct inode *inode, struct file *file) >>>> { >>>> struct cxl_memdev *cxlmd = >>>> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c >>>> index 420e4be85a1f..ff266e91ea71 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/cxlpci.h b/drivers/cxl/cxlpci.h >>>> index 4da07727ab9c..eb59019fe5f3 100644 >>>> --- a/drivers/cxl/cxlpci.h >>>> +++ b/drivers/cxl/cxlpci.h >>>> @@ -14,22 +14,6 @@ >>>> */ >>>> #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 188412d45e0d..0b910ef52db7 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> >>>> @@ -816,6 +818,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) >>>> struct cxl_memdev *cxlmd; >>>> int i, rc, pmu_count; >>>> bool irq_avail; >>>> + u16 dvsec; >>>> /* >>>> * Double check the anonymous union trickery in struct cxl_regs >>>> @@ -836,13 +839,15 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) >>>> 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) >>>> + cxl_set_serial(cxlds, pci_get_dsn(pdev)); >>>> + 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"); >>>> + cxl_set_dvsec(cxlds, dvsec); >>>> + >>>> rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map); >>>> if (rc) >>>> return rc; >>>> diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h >>>> new file mode 100644 >>>> index 000000000000..19e5d883557a >>>> --- /dev/null >>>> +++ b/include/cxl/cxl.h >>>> @@ -0,0 +1,21 @@ >>>> +/* SPDX-License-Identifier: GPL-2.0 */ >>>> +/* Copyright(c) 2024 Advanced Micro Devices, Inc. */ >>>> + >>>> +#ifndef __CXL_H >>>> +#define __CXL_H >>>> + >>>> +#include <linux/ioport.h> >>>> + >>>> +enum cxl_resource { >>>> + CXL_RES_DPA, >>>> + CXL_RES_RAM, >>>> + CXL_RES_PMEM, >>>> +}; >>>> + >>>> +struct cxl_dev_state *cxl_accel_state_create(struct device *dev); >>>> + >>>> +void cxl_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec); >>>> +void cxl_set_serial(struct cxl_dev_state *cxlds, u64 serial); >>>> +int cxl_set_resource(struct cxl_dev_state *cxlds, struct resource res, >>>> + enum cxl_resource); >>>> +#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 >>>> -- >>>> 2.17.1 >>>> >>>>
On 11/22/24 20:43, Ben Cheatham wrote: > On 11/18/24 10:44 AM, alejandro.lucero-palau@amd.com wrote: >> From: Alejandro Lucero <alucerop@amd.com> >> >> Differentiate Type3, aka memory expanders, from Type2, aka device >> accelerators, with a new function for initializing cxl_dev_state. >> >> Create accessors to cxl_dev_state to be used by accel drivers. >> >> Based on previous work by Dan Williams [1] >> >> Link: [1] https://lore.kernel.org/linux-cxl/168592160379.1948938.12863272903570476312.stgit@dwillia2-xfh.jf.intel.com/ >> Signed-off-by: Alejandro Lucero <alucerop@amd.com> >> Co-developed-by: Dan Williams <dan.j.williams@intel.com> >> --- >> drivers/cxl/core/memdev.c | 51 +++++++++++++++++++++++++++++++++++++++ >> drivers/cxl/core/pci.c | 1 + >> drivers/cxl/cxlpci.h | 16 ------------ >> drivers/cxl/pci.c | 13 +++++++--- >> include/cxl/cxl.h | 21 ++++++++++++++++ >> include/cxl/pci.h | 23 ++++++++++++++++++ >> 6 files changed, 105 insertions(+), 20 deletions(-) >> create mode 100644 include/cxl/cxl.h >> create mode 100644 include/cxl/pci.h >> >> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c >> index 84fefb76dafa..d083fd13a6dd 100644 >> --- a/drivers/cxl/core/memdev.c >> +++ b/drivers/cxl/core/memdev.c >> @@ -1,6 +1,7 @@ >> // SPDX-License-Identifier: GPL-2.0-only >> /* Copyright(c) 2020 Intel Corporation. */ >> >> +#include <cxl/cxl.h> > Pedantic one, you'll want this at the end CXL does reverse christmas tree > for #includes. That seems to be true for this file, but the reverse christmas tree is not applied through all the files in the cxl directory. I was told to put it in alphabetical order (not remember which specific file), what implies there is no agreement about how to put the header references. Anyway, I think for this one your suggestion makes sense. >> #include <linux/io-64-nonatomic-lo-hi.h> >> #include <linux/firmware.h> >> #include <linux/device.h> >> @@ -616,6 +617,25 @@ 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 *cxlds; >> + >> + cxlds = kzalloc(sizeof(*cxlds), GFP_KERNEL); > Would it be better to use a devm_kzalloc() here? I'd imagine this function > will be called as part of probe a majority of the time so I think the automatic > cleanup would be nice here. If you did that, then I'd also rename the function to > include devm_ as well. This is complicated. As I have said in other previous reviews regarding use of devm_* by the sfc changes in this patchset, it is not advice to use them inside the netdev subsystem. This is not the case here since it is cxl code, but in this case used by a netdev client (although other clients from other subsystems will likely come soon). So, I'm not sure about this one. I could add the specific function to use when released like when cxl_memdev_alloc is used by devm_cxl_add_memdev, but frankly, mixing devm with no devm allocations is a mess, at least in my view. >> + if (!cxlds) >> + return ERR_PTR(-ENOMEM); >> + >> + cxlds->dev = dev; >> + cxlds->type = CXL_DEVTYPE_DEVMEM; >> + >> + cxlds->dpa_res = DEFINE_RES_MEM_NAMED(0, 0, "dpa"); >> + cxlds->ram_res = DEFINE_RES_MEM_NAMED(0, 0, "ram"); >> + cxlds->pmem_res = DEFINE_RES_MEM_NAMED(0, 0, "pmem"); >> + >> + return cxlds; >> +} >> +EXPORT_SYMBOL_NS_GPL(cxl_accel_state_create, CXL); >> + >> static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds, >> const struct file_operations *fops) >> { >> @@ -693,6 +713,37 @@ static int cxl_memdev_open(struct inode *inode, struct file *file) >> return 0; >> } >> >> +void cxl_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec) >> +{ >> + cxlds->cxl_dvsec = dvsec; >> +} >> +EXPORT_SYMBOL_NS_GPL(cxl_set_dvsec, CXL); >> + >> +void cxl_set_serial(struct cxl_dev_state *cxlds, u64 serial) >> +{ >> + cxlds->serial = serial; >> +} >> +EXPORT_SYMBOL_NS_GPL(cxl_set_serial, CXL); >> + >> +int cxl_set_resource(struct cxl_dev_state *cxlds, struct resource res, >> + enum cxl_resource type) >> +{ >> + switch (type) { >> + case CXL_RES_DPA: >> + cxlds->dpa_res = res; >> + return 0; >> + case CXL_RES_RAM: >> + cxlds->ram_res = res; >> + return 0; >> + case CXL_RES_PMEM: >> + cxlds->pmem_res = res; >> + return 0; >> + } >> + >> + return -EINVAL; >> +} >> +EXPORT_SYMBOL_NS_GPL(cxl_set_resource, CXL); >> + >> static int cxl_memdev_release_file(struct inode *inode, struct file *file) >> { >> struct cxl_memdev *cxlmd = >> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c >> index 420e4be85a1f..ff266e91ea71 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/cxlpci.h b/drivers/cxl/cxlpci.h >> index 4da07727ab9c..eb59019fe5f3 100644 >> --- a/drivers/cxl/cxlpci.h >> +++ b/drivers/cxl/cxlpci.h >> @@ -14,22 +14,6 @@ >> */ >> #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 188412d45e0d..0b910ef52db7 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> >> @@ -816,6 +818,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) >> struct cxl_memdev *cxlmd; >> int i, rc, pmu_count; >> bool irq_avail; >> + u16 dvsec; >> >> /* >> * Double check the anonymous union trickery in struct cxl_regs >> @@ -836,13 +839,15 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) >> 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) >> + cxl_set_serial(cxlds, pci_get_dsn(pdev)); >> + 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"); >> >> + cxl_set_dvsec(cxlds, dvsec); >> + >> rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map); >> if (rc) >> return rc; >> diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h >> new file mode 100644 >> index 000000000000..19e5d883557a >> --- /dev/null >> +++ b/include/cxl/cxl.h > Is cxl.h the right name for this file? I initially thought this was the cxl.h > under drivers/cxl. It looks like it's just type 2 related functions, so maybe > "type2.h", or "accel.h" would be better? If the plan is to expose more CXL > functionality not necessarily related to type 2 devices later I'm fine with it, > and if no one else cares then I'm fine with it. I agree, but I did use cxl_accel_* in version 2 and it was suggested then to remove the accel part, so leaving it as it is now if none else cares about it. Thanks! > >> @@ -0,0 +1,21 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +/* Copyright(c) 2024 Advanced Micro Devices, Inc. */ >> + >> +#ifndef __CXL_H >> +#define __CXL_H >> + >> +#include <linux/ioport.h> >> + >> +enum cxl_resource { >> + CXL_RES_DPA, >> + CXL_RES_RAM, >> + CXL_RES_PMEM, >> +}; >> + >> +struct cxl_dev_state *cxl_accel_state_create(struct device *dev); >> + >> +void cxl_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec); >> +void cxl_set_serial(struct cxl_dev_state *cxlds, u64 serial); >> +int cxl_set_resource(struct cxl_dev_state *cxlds, struct resource res, >> + enum cxl_resource); >> +#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
On 11/27/24 09:00, Alejandro Lucero Palau wrote: > > On 11/22/24 20:43, Ben Cheatham wrote: >> On 11/18/24 10:44 AM, alejandro.lucero-palau@amd.com wrote: >>> From: Alejandro Lucero <alucerop@amd.com> >>> >>> Differentiate Type3, aka memory expanders, from Type2, aka device >>> accelerators, with a new function for initializing cxl_dev_state. >>> >>> Create accessors to cxl_dev_state to be used by accel drivers. >>> >>> Based on previous work by Dan Williams [1] >>> >>> Link: [1] >>> https://lore.kernel.org/linux-cxl/168592160379.1948938.12863272903570476312.stgit@dwillia2-xfh.jf.intel.com/ >>> Signed-off-by: Alejandro Lucero <alucerop@amd.com> >>> Co-developed-by: Dan Williams <dan.j.williams@intel.com> >>> --- >>> drivers/cxl/core/memdev.c | 51 >>> +++++++++++++++++++++++++++++++++++++++ >>> drivers/cxl/core/pci.c | 1 + >>> drivers/cxl/cxlpci.h | 16 ------------ >>> drivers/cxl/pci.c | 13 +++++++--- >>> include/cxl/cxl.h | 21 ++++++++++++++++ >>> include/cxl/pci.h | 23 ++++++++++++++++++ >>> 6 files changed, 105 insertions(+), 20 deletions(-) >>> create mode 100644 include/cxl/cxl.h >>> create mode 100644 include/cxl/pci.h >>> >>> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c >>> index 84fefb76dafa..d083fd13a6dd 100644 >>> --- a/drivers/cxl/core/memdev.c >>> +++ b/drivers/cxl/core/memdev.c >>> @@ -1,6 +1,7 @@ >>> // SPDX-License-Identifier: GPL-2.0-only >>> /* Copyright(c) 2020 Intel Corporation. */ >>> +#include <cxl/cxl.h> >> Pedantic one, you'll want this at the end CXL does reverse christmas >> tree >> for #includes. > > > That seems to be true for this file, but the reverse christmas tree is > not applied through all the files in the cxl directory. > > I was told to put it in alphabetical order (not remember which > specific file), what implies there is no agreement about how to put > the header references. > > Anyway, I think for this one your suggestion makes sense. > > >>> #include <linux/io-64-nonatomic-lo-hi.h> >>> #include <linux/firmware.h> >>> #include <linux/device.h> >>> @@ -616,6 +617,25 @@ 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 *cxlds; >>> + >>> + cxlds = kzalloc(sizeof(*cxlds), GFP_KERNEL); >> Would it be better to use a devm_kzalloc() here? I'd imagine this >> function >> will be called as part of probe a majority of the time so I think the >> automatic >> cleanup would be nice here. If you did that, then I'd also rename the >> function to >> include devm_ as well. > > > This is complicated. As I have said in other previous reviews > regarding use of devm_* by the sfc changes in this patchset, it is > not advice to use them inside the netdev subsystem. This is not the > case here since it is cxl code, but in this case used by a netdev > client (although other clients from other subsystems will likely come > soon). > > > So, I'm not sure about this one. I could add the specific function to > use when released like when cxl_memdev_alloc is used by > devm_cxl_add_memdev, but frankly, mixing devm with no devm allocations > is a mess, at least in my view. > I forgot to mention another reason for not using devm and it is the fact that the memory is not released until the driver is detached. If the cxl initialization can fail and not being fatal, that means the memory not released while the driver is being used. A specific CXL accelerator driver, meaning the design relying on CXL, is not a problem, but a driver using CXL as an option for better performance could keep the memory unreleased as it is the case for sfc. > >>> + if (!cxlds) >>> + return ERR_PTR(-ENOMEM); >>> + >>> + cxlds->dev = dev; >>> + cxlds->type = CXL_DEVTYPE_DEVMEM; >>> + >>> + cxlds->dpa_res = DEFINE_RES_MEM_NAMED(0, 0, "dpa"); >>> + cxlds->ram_res = DEFINE_RES_MEM_NAMED(0, 0, "ram"); >>> + cxlds->pmem_res = DEFINE_RES_MEM_NAMED(0, 0, "pmem"); >>> + >>> + return cxlds; >>> +} >>> +EXPORT_SYMBOL_NS_GPL(cxl_accel_state_create, CXL); >>> + >>> static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state >>> *cxlds, >>> const struct file_operations *fops) >>> { >>> @@ -693,6 +713,37 @@ static int cxl_memdev_open(struct inode *inode, >>> struct file *file) >>> return 0; >>> } >>> +void cxl_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec) >>> +{ >>> + cxlds->cxl_dvsec = dvsec; >>> +} >>> +EXPORT_SYMBOL_NS_GPL(cxl_set_dvsec, CXL); >>> + >>> +void cxl_set_serial(struct cxl_dev_state *cxlds, u64 serial) >>> +{ >>> + cxlds->serial = serial; >>> +} >>> +EXPORT_SYMBOL_NS_GPL(cxl_set_serial, CXL); >>> + >>> +int cxl_set_resource(struct cxl_dev_state *cxlds, struct resource res, >>> + enum cxl_resource type) >>> +{ >>> + switch (type) { >>> + case CXL_RES_DPA: >>> + cxlds->dpa_res = res; >>> + return 0; >>> + case CXL_RES_RAM: >>> + cxlds->ram_res = res; >>> + return 0; >>> + case CXL_RES_PMEM: >>> + cxlds->pmem_res = res; >>> + return 0; >>> + } >>> + >>> + return -EINVAL; >>> +} >>> +EXPORT_SYMBOL_NS_GPL(cxl_set_resource, CXL); >>> + >>> static int cxl_memdev_release_file(struct inode *inode, struct >>> file *file) >>> { >>> struct cxl_memdev *cxlmd = >>> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c >>> index 420e4be85a1f..ff266e91ea71 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/cxlpci.h b/drivers/cxl/cxlpci.h >>> index 4da07727ab9c..eb59019fe5f3 100644 >>> --- a/drivers/cxl/cxlpci.h >>> +++ b/drivers/cxl/cxlpci.h >>> @@ -14,22 +14,6 @@ >>> */ >>> #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 188412d45e0d..0b910ef52db7 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> >>> @@ -816,6 +818,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, >>> const struct pci_device_id *id) >>> struct cxl_memdev *cxlmd; >>> int i, rc, pmu_count; >>> bool irq_avail; >>> + u16 dvsec; >>> /* >>> * Double check the anonymous union trickery in struct cxl_regs >>> @@ -836,13 +839,15 @@ static int cxl_pci_probe(struct pci_dev *pdev, >>> const struct pci_device_id *id) >>> 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) >>> + cxl_set_serial(cxlds, pci_get_dsn(pdev)); >>> + 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"); >>> + cxl_set_dvsec(cxlds, dvsec); >>> + >>> rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map); >>> if (rc) >>> return rc; >>> diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h >>> new file mode 100644 >>> index 000000000000..19e5d883557a >>> --- /dev/null >>> +++ b/include/cxl/cxl.h >> Is cxl.h the right name for this file? I initially thought this was >> the cxl.h >> under drivers/cxl. It looks like it's just type 2 related functions, >> so maybe >> "type2.h", or "accel.h" would be better? If the plan is to expose >> more CXL >> functionality not necessarily related to type 2 devices later I'm >> fine with it, >> and if no one else cares then I'm fine with it. > > > I agree, but I did use cxl_accel_* in version 2 and it was suggested > then to remove the accel part, so leaving it as it is now if none else > cares about it. > > Thanks! > > >>> @@ -0,0 +1,21 @@ >>> +/* SPDX-License-Identifier: GPL-2.0 */ >>> +/* Copyright(c) 2024 Advanced Micro Devices, Inc. */ >>> + >>> +#ifndef __CXL_H >>> +#define __CXL_H >>> + >>> +#include <linux/ioport.h> >>> + >>> +enum cxl_resource { >>> + CXL_RES_DPA, >>> + CXL_RES_RAM, >>> + CXL_RES_PMEM, >>> +}; >>> + >>> +struct cxl_dev_state *cxl_accel_state_create(struct device *dev); >>> + >>> +void cxl_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec); >>> +void cxl_set_serial(struct cxl_dev_state *cxlds, u64 serial); >>> +int cxl_set_resource(struct cxl_dev_state *cxlds, struct resource res, >>> + enum cxl_resource); >>> +#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
diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c index 84fefb76dafa..d083fd13a6dd 100644 --- a/drivers/cxl/core/memdev.c +++ b/drivers/cxl/core/memdev.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0-only /* Copyright(c) 2020 Intel Corporation. */ +#include <cxl/cxl.h> #include <linux/io-64-nonatomic-lo-hi.h> #include <linux/firmware.h> #include <linux/device.h> @@ -616,6 +617,25 @@ 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 *cxlds; + + cxlds = kzalloc(sizeof(*cxlds), GFP_KERNEL); + if (!cxlds) + return ERR_PTR(-ENOMEM); + + cxlds->dev = dev; + cxlds->type = CXL_DEVTYPE_DEVMEM; + + cxlds->dpa_res = DEFINE_RES_MEM_NAMED(0, 0, "dpa"); + cxlds->ram_res = DEFINE_RES_MEM_NAMED(0, 0, "ram"); + cxlds->pmem_res = DEFINE_RES_MEM_NAMED(0, 0, "pmem"); + + return cxlds; +} +EXPORT_SYMBOL_NS_GPL(cxl_accel_state_create, CXL); + static struct cxl_memdev *cxl_memdev_alloc(struct cxl_dev_state *cxlds, const struct file_operations *fops) { @@ -693,6 +713,37 @@ static int cxl_memdev_open(struct inode *inode, struct file *file) return 0; } +void cxl_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec) +{ + cxlds->cxl_dvsec = dvsec; +} +EXPORT_SYMBOL_NS_GPL(cxl_set_dvsec, CXL); + +void cxl_set_serial(struct cxl_dev_state *cxlds, u64 serial) +{ + cxlds->serial = serial; +} +EXPORT_SYMBOL_NS_GPL(cxl_set_serial, CXL); + +int cxl_set_resource(struct cxl_dev_state *cxlds, struct resource res, + enum cxl_resource type) +{ + switch (type) { + case CXL_RES_DPA: + cxlds->dpa_res = res; + return 0; + case CXL_RES_RAM: + cxlds->ram_res = res; + return 0; + case CXL_RES_PMEM: + cxlds->pmem_res = res; + return 0; + } + + return -EINVAL; +} +EXPORT_SYMBOL_NS_GPL(cxl_set_resource, CXL); + static int cxl_memdev_release_file(struct inode *inode, struct file *file) { struct cxl_memdev *cxlmd = diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c index 420e4be85a1f..ff266e91ea71 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/cxlpci.h b/drivers/cxl/cxlpci.h index 4da07727ab9c..eb59019fe5f3 100644 --- a/drivers/cxl/cxlpci.h +++ b/drivers/cxl/cxlpci.h @@ -14,22 +14,6 @@ */ #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 188412d45e0d..0b910ef52db7 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> @@ -816,6 +818,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) struct cxl_memdev *cxlmd; int i, rc, pmu_count; bool irq_avail; + u16 dvsec; /* * Double check the anonymous union trickery in struct cxl_regs @@ -836,13 +839,15 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) 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) + cxl_set_serial(cxlds, pci_get_dsn(pdev)); + 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"); + cxl_set_dvsec(cxlds, dvsec); + rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map); if (rc) return rc; diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h new file mode 100644 index 000000000000..19e5d883557a --- /dev/null +++ b/include/cxl/cxl.h @@ -0,0 +1,21 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* Copyright(c) 2024 Advanced Micro Devices, Inc. */ + +#ifndef __CXL_H +#define __CXL_H + +#include <linux/ioport.h> + +enum cxl_resource { + CXL_RES_DPA, + CXL_RES_RAM, + CXL_RES_PMEM, +}; + +struct cxl_dev_state *cxl_accel_state_create(struct device *dev); + +void cxl_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec); +void cxl_set_serial(struct cxl_dev_state *cxlds, u64 serial); +int cxl_set_resource(struct cxl_dev_state *cxlds, struct resource res, + enum cxl_resource); +#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