| Message ID | 20240907081836.5801-2-alejandro.lucero-palau@amd.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | cxl: add Type2 device support | expand |
Hi, kernel test robot noticed the following build errors: [auto build test ERROR on cxl/next] [also build test ERROR on linus/master v6.11-rc6 next-20240906] [cannot apply to cxl/pending horms-ipvs/master] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/alejandro-lucero-palau-amd-com/cxl-add-type2-device-basic-support/20240907-162231 base: https://git.kernel.org/pub/scm/linux/kernel/git/cxl/cxl.git next patch link: https://lore.kernel.org/r/20240907081836.5801-2-alejandro.lucero-palau%40amd.com patch subject: [PATCH v3 01/20] cxl: add type2 device basic support config: mips-ip27_defconfig (https://download.01.org/0day-ci/archive/20240908/202409080455.S5Uvi35I-lkp@intel.com/config) compiler: mips64-linux-gcc (GCC) 14.1.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240908/202409080455.S5Uvi35I-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202409080455.S5Uvi35I-lkp@intel.com/ All errors (new ones prefixed by >>, old ones prefixed by <<): WARNING: modpost: missing MODULE_DESCRIPTION() in fs/btrfs/btrfs.o WARNING: modpost: missing MODULE_DESCRIPTION() in crypto/cast_common.o WARNING: modpost: missing MODULE_DESCRIPTION() in lib/crypto/libarc4.o WARNING: modpost: missing MODULE_DESCRIPTION() in lib/zlib_inflate/zlib_inflate.o WARNING: modpost: missing MODULE_DESCRIPTION() in lib/zlib_deflate/zlib_deflate.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/base/regmap/regmap-i2c.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/mfd/mfd-core.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/mfd/pcf50633-gpio.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/uio/uio_aec.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/cdrom/cdrom.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/i2c/busses/i2c-ali1563.o >> ERROR: modpost: "cxl_accel_state_create" [drivers/net/ethernet/sfc/sfc.ko] undefined! >> ERROR: modpost: "cxl_set_serial" [drivers/net/ethernet/sfc/sfc.ko] undefined! ERROR: modpost: "cxl_set_resource" [drivers/net/ethernet/sfc/sfc.ko] undefined! ERROR: modpost: "cxl_set_dvsec" [drivers/net/ethernet/sfc/sfc.ko] undefined!
On 9/7/2024 4:18 PM, alejandro.lucero-palau@amd.com wrote: > From: Alejandro Lucero <alucerop@amd.com> > > Differientiate 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. > > Add SFC ethernet network driver as the client. > > Based on 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 | 52 ++++++++++++++++ > drivers/cxl/core/pci.c | 1 + > drivers/cxl/cxlpci.h | 16 ----- > drivers/cxl/pci.c | 13 ++-- > drivers/net/ethernet/sfc/Makefile | 2 +- > drivers/net/ethernet/sfc/efx.c | 13 ++++ > drivers/net/ethernet/sfc/efx_cxl.c | 86 +++++++++++++++++++++++++++ > drivers/net/ethernet/sfc/efx_cxl.h | 29 +++++++++ > drivers/net/ethernet/sfc/net_driver.h | 6 ++ > include/linux/cxl/cxl.h | 21 +++++++ > include/linux/cxl/pci.h | 23 +++++++ > 11 files changed, 241 insertions(+), 21 deletions(-) > create mode 100644 drivers/net/ethernet/sfc/efx_cxl.c > create mode 100644 drivers/net/ethernet/sfc/efx_cxl.h > create mode 100644 include/linux/cxl/cxl.h > create mode 100644 include/linux/cxl/pci.h > [...] > > diff --git a/include/linux/cxl/cxl.h b/include/linux/cxl/cxl.h > new file mode 100644 > index 000000000000..e78eefa82123 > --- /dev/null > +++ b/include/linux/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/device.h> > + > +enum cxl_resource { > + CXL_ACCEL_RES_DPA, > + CXL_ACCEL_RES_RAM, > + CXL_ACCEL_RES_PMEM, > +}; Can remove 'ACCEL' from the resource name? they can be used for both type-2 and type-3 devices. > + > +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/linux/cxl/pci.h b/include/linux/cxl/pci.h > new file mode 100644 > index 000000000000..c337ae8797e6 > --- /dev/null > +++ b/include/linux/cxl/pci.h > @@ -0,0 +1,23 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* Copyright(c) 2024 Advanced Micro Devices, Inc. */ > + > +#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 9/10/24 07:12, Li, Ming4 wrote: > On 9/7/2024 4:18 PM, alejandro.lucero-palau@amd.com wrote: >> From: Alejandro Lucero <alucerop@amd.com> >> >> Differientiate 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. >> >> Add SFC ethernet network driver as the client. >> >> Based on 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 | 52 ++++++++++++++++ >> drivers/cxl/core/pci.c | 1 + >> drivers/cxl/cxlpci.h | 16 ----- >> drivers/cxl/pci.c | 13 ++-- >> drivers/net/ethernet/sfc/Makefile | 2 +- >> drivers/net/ethernet/sfc/efx.c | 13 ++++ >> drivers/net/ethernet/sfc/efx_cxl.c | 86 +++++++++++++++++++++++++++ >> drivers/net/ethernet/sfc/efx_cxl.h | 29 +++++++++ >> drivers/net/ethernet/sfc/net_driver.h | 6 ++ >> include/linux/cxl/cxl.h | 21 +++++++ >> include/linux/cxl/pci.h | 23 +++++++ >> 11 files changed, 241 insertions(+), 21 deletions(-) >> create mode 100644 drivers/net/ethernet/sfc/efx_cxl.c >> create mode 100644 drivers/net/ethernet/sfc/efx_cxl.h >> create mode 100644 include/linux/cxl/cxl.h >> create mode 100644 include/linux/cxl/pci.h >> > [...] > >> >> diff --git a/include/linux/cxl/cxl.h b/include/linux/cxl/cxl.h >> new file mode 100644 >> index 000000000000..e78eefa82123 >> --- /dev/null >> +++ b/include/linux/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/device.h> >> + >> +enum cxl_resource { >> + CXL_ACCEL_RES_DPA, >> + CXL_ACCEL_RES_RAM, >> + CXL_ACCEL_RES_PMEM, >> +}; > Can remove 'ACCEL' from the resource name? they can be used for both type-2 and type-3 devices. > Sure. I'll do it. Thanks >> + >> +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/linux/cxl/pci.h b/include/linux/cxl/pci.h >> new file mode 100644 >> index 000000000000..c337ae8797e6 >> --- /dev/null >> +++ b/include/linux/cxl/pci.h >> @@ -0,0 +1,23 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +/* Copyright(c) 2024 Advanced Micro Devices, Inc. */ >> + >> +#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 Sat, 7 Sep 2024 09:18:17 +0100 <alejandro.lucero-palau@amd.com> wrote: > From: Alejandro Lucero <alucerop@amd.com> > > Differientiate 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. > > Add SFC ethernet network driver as the client. > > Based on > 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 | 52 ++++++++++++++++ > drivers/cxl/core/pci.c | 1 + > drivers/cxl/cxlpci.h | 16 ----- > drivers/cxl/pci.c | 13 ++-- > drivers/net/ethernet/sfc/Makefile | 2 +- > drivers/net/ethernet/sfc/efx.c | 13 ++++ > drivers/net/ethernet/sfc/efx_cxl.c | 86 > +++++++++++++++++++++++++++ drivers/net/ethernet/sfc/efx_cxl.h | > 29 +++++++++ drivers/net/ethernet/sfc/net_driver.h | 6 ++ > include/linux/cxl/cxl.h | 21 +++++++ > include/linux/cxl/pci.h | 23 +++++++ > 11 files changed, 241 insertions(+), 21 deletions(-) > create mode 100644 drivers/net/ethernet/sfc/efx_cxl.c > create mode 100644 drivers/net/ethernet/sfc/efx_cxl.h > create mode 100644 include/linux/cxl/cxl.h > create mode 100644 include/linux/cxl/pci.h > > diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c > index 0277726afd04..10c0a6990f9a 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 <linux/cxl/cxl.h> > #include <linux/io-64-nonatomic-lo-hi.h> > #include <linux/firmware.h> > #include <linux/device.h> > @@ -615,6 +616,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) > { > @@ -692,6 +712,38 @@ 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_ACCEL_RES_DPA: > + cxlds->dpa_res = res; > + return 0; > + case CXL_ACCEL_RES_RAM: > + cxlds->ram_res = res; > + return 0; > + case CXL_ACCEL_RES_PMEM: > + cxlds->pmem_res = res; > + return 0; > + default: > + dev_err(cxlds->dev, "unknown resource type (%u)\n", > type); > + 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 51132a575b27..3d6564dbda57 100644 > --- a/drivers/cxl/core/pci.c > +++ b/drivers/cxl/core/pci.c > @@ -7,6 +7,7 @@ > #include <linux/pci.h> > #include <linux/pci-doe.h> > #include <linux/aer.h> > +#include <linux/cxl/pci.h> > #include <cxlpci.h> > #include <cxlmem.h> > #include <cxl.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 4be35dc22202..742a7b2a1be5 100644 > --- a/drivers/cxl/pci.c > +++ b/drivers/cxl/pci.c > @@ -11,6 +11,8 @@ > #include <linux/pci.h> > #include <linux/aer.h> > #include <linux/io.h> > +#include <linux/cxl/cxl.h> > +#include <linux/cxl/pci.h> > #include "cxlmem.h" > #include "cxlpci.h" > #include "cxl.h" > @@ -795,6 +797,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 @@ -815,12 +818,14 @@ 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"); > + else > + cxl_set_dvsec(cxlds, dvsec); > > rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map); > if (rc) > diff --git a/drivers/net/ethernet/sfc/Makefile > b/drivers/net/ethernet/sfc/Makefile index 8f446b9bd5ee..e80c713c3b0c > 100644 --- a/drivers/net/ethernet/sfc/Makefile > +++ b/drivers/net/ethernet/sfc/Makefile > @@ -7,7 +7,7 @@ sfc-y += efx.o efx_common.o > efx_channels.o nic.o \ mcdi_functions.o mcdi_filters.o mcdi_mon.o \ > ef100.o ef100_nic.o ef100_netdev.o \ > ef100_ethtool.o ef100_rx.o ef100_tx.o \ > - efx_devlink.o > + efx_devlink.o efx_cxl.o > sfc-$(CONFIG_SFC_MTD) += mtd.o > sfc-$(CONFIG_SFC_SRIOV) += sriov.o ef10_sriov.o ef100_sriov.o > ef100_rep.o \ mae.o tc.o tc_bindings.o tc_counters.o \ > diff --git a/drivers/net/ethernet/sfc/efx.c > b/drivers/net/ethernet/sfc/efx.c index 6f1a01ded7d4..3a7406aa950c > 100644 --- a/drivers/net/ethernet/sfc/efx.c > +++ b/drivers/net/ethernet/sfc/efx.c > @@ -33,6 +33,7 @@ > #include "selftest.h" > #include "sriov.h" > #include "efx_devlink.h" > +#include "efx_cxl.h" > > #include "mcdi_port_common.h" > #include "mcdi_pcol.h" > @@ -899,6 +900,9 @@ static void efx_pci_remove(struct pci_dev > *pci_dev) efx_pci_remove_main(efx); > > efx_fini_io(efx); > + > + efx_cxl_exit(efx); > + > pci_dbg(efx->pci_dev, "shutdown successful\n"); > > efx_fini_devlink_and_unlock(efx); > @@ -1109,6 +1113,15 @@ static int efx_pci_probe(struct pci_dev > *pci_dev, if (rc) > goto fail2; > > + /* A successful cxl initialization implies a CXL region > created to be > + * used for PIO buffers. If there is no CXL support, or > initialization > + * fails, efx_cxl_pio_initialised wll be false and legacy > PIO buffers > + * defined at specific PCI BAR regions will be used. > + */ > + rc = efx_cxl_init(efx); > + if (rc) > + pci_err(pci_dev, "CXL initialization failed with > error %d\n", rc); + > rc = efx_pci_probe_post_io(efx); > if (rc) { > /* On failure, retry once immediately. > diff --git a/drivers/net/ethernet/sfc/efx_cxl.c > b/drivers/net/ethernet/sfc/efx_cxl.c new file mode 100644 > index 000000000000..bba36cbbab22 > --- /dev/null > +++ b/drivers/net/ethernet/sfc/efx_cxl.c > @@ -0,0 +1,86 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/**************************************************************************** > + * > + * Driver for AMD network controllers and boards > + * Copyright (C) 2024, Advanced Micro Devices, Inc. > + * > + * This program is free software; you can redistribute it and/or > modify it > + * under the terms of the GNU General Public License version 2 as > published > + * by the Free Software Foundation, incorporated herein by reference. > + */ > + > +#include <linux/cxl/cxl.h> > +#include <linux/cxl/pci.h> > +#include <linux/pci.h> > + > +#include "net_driver.h" > +#include "efx_cxl.h" > + > +#define EFX_CTPIO_BUFFER_SIZE (1024 * 1024 * 256) > + Use SZ_256M in include/linux/sizes.h > +int efx_cxl_init(struct efx_nic *efx) > +{ > + struct pci_dev *pci_dev = efx->pci_dev; > + struct efx_cxl *cxl; > + struct resource res; > + u16 dvsec; > + int rc; > + > + efx->efx_cxl_pio_initialised = false; > + > + dvsec = pci_find_dvsec_capability(pci_dev, PCI_VENDOR_ID_CXL, > + CXL_DVSEC_PCIE_DEVICE); > + > + if (!dvsec) > + return 0; > + > + pci_dbg(pci_dev, "CXL_DVSEC_PCIE_DEVICE capability found\n"); > + > + efx->cxl = kzalloc(sizeof(*cxl), GFP_KERNEL); > + if (!efx->cxl) > + return -ENOMEM; > + > + cxl = efx->cxl; > + > + cxl->cxlds = cxl_accel_state_create(&pci_dev->dev); > + if (IS_ERR(cxl->cxlds)) { > + pci_err(pci_dev, "CXL accel device state failed"); > + kfree(efx->cxl); > + return -ENOMEM; > + } > + > + cxl_set_dvsec(cxl->cxlds, dvsec); > + cxl_set_serial(cxl->cxlds, pci_dev->dev.id); > + > + res = DEFINE_RES_MEM(0, EFX_CTPIO_BUFFER_SIZE); > + if (cxl_set_resource(cxl->cxlds, res, CXL_ACCEL_RES_DPA)) { > + pci_err(pci_dev, "cxl_set_resource DPA failed\n"); > + rc = -EINVAL; > + goto err; > + } > + > + res = DEFINE_RES_MEM_NAMED(0, EFX_CTPIO_BUFFER_SIZE, "ram"); > + if (cxl_set_resource(cxl->cxlds, res, CXL_ACCEL_RES_RAM)) { > + pci_err(pci_dev, "cxl_set_resource RAM failed\n"); > + rc = -EINVAL; > + goto err; > + } > + > + return 0; > +err: > + kfree(cxl->cxlds); > + kfree(cxl); > + efx->cxl = NULL; > + > + return rc; > +} > + > +void efx_cxl_exit(struct efx_nic *efx) > +{ > + if (efx->cxl) { > + kfree(efx->cxl->cxlds); > + kfree(efx->cxl); > + } > +} > + > +MODULE_IMPORT_NS(CXL); > diff --git a/drivers/net/ethernet/sfc/efx_cxl.h > b/drivers/net/ethernet/sfc/efx_cxl.h new file mode 100644 > index 000000000000..f57fb2afd124 > --- /dev/null > +++ b/drivers/net/ethernet/sfc/efx_cxl.h > @@ -0,0 +1,29 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/**************************************************************************** > + * Driver for AMD network controllers and boards > + * Copyright (C) 2024, Advanced Micro Devices, Inc. > + * > + * This program is free software; you can redistribute it and/or > modify it > + * under the terms of the GNU General Public License version 2 as > published > + * by the Free Software Foundation, incorporated herein by reference. > + */ > + > +#ifndef EFX_CXL_H > +#define EFX_CXL_H > + > +struct efx_nic; > +struct cxl_dev_state; > + > +struct efx_cxl { > + struct cxl_dev_state *cxlds; > + struct cxl_memdev *cxlmd; > + struct cxl_root_decoder *cxlrd; > + struct cxl_port *endpoint; > + struct cxl_endpoint_decoder *cxled; > + struct cxl_region *efx_region; > + void __iomem *ctpio_cxl; > +}; > + > +int efx_cxl_init(struct efx_nic *efx); > +void efx_cxl_exit(struct efx_nic *efx); > +#endif > diff --git a/drivers/net/ethernet/sfc/net_driver.h > b/drivers/net/ethernet/sfc/net_driver.h index > b85c51cbe7f9..77261de65e63 100644 --- > a/drivers/net/ethernet/sfc/net_driver.h +++ > b/drivers/net/ethernet/sfc/net_driver.h @@ -817,6 +817,8 @@ enum > efx_xdp_tx_queues_mode { > struct efx_mae; > > +struct efx_cxl; > + > /** > * struct efx_nic - an Efx NIC > * @name: Device name (net device name or bus id before net device > registered) @@ -963,6 +965,8 @@ struct efx_mae; > * @tc: state for TC offload (EF100). > * @devlink: reference to devlink structure owned by this device > * @dl_port: devlink port associated with the PF > + * @cxl: details of related cxl objects > + * @efx_cxl_pio_initialised: clx initialization outcome. > * @mem_bar: The BAR that is mapped into membase. > * @reg_base: Offset from the start of the bar to the function > control window. > * @monitor_work: Hardware monitor workitem > @@ -1148,6 +1152,8 @@ struct efx_nic { > > struct devlink *devlink; > struct devlink_port *dl_port; > + struct efx_cxl *cxl; > + bool efx_cxl_pio_initialised; > unsigned int mem_bar; > u32 reg_base; > > diff --git a/include/linux/cxl/cxl.h b/include/linux/cxl/cxl.h > new file mode 100644 > index 000000000000..e78eefa82123 > --- /dev/null > +++ b/include/linux/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/device.h> > + > +enum cxl_resource { > + CXL_ACCEL_RES_DPA, > + CXL_ACCEL_RES_RAM, > + CXL_ACCEL_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/linux/cxl/pci.h b/include/linux/cxl/pci.h > new file mode 100644 > index 000000000000..c337ae8797e6 > --- /dev/null > +++ b/include/linux/cxl/pci.h > @@ -0,0 +1,23 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* Copyright(c) 2024 Advanced Micro Devices, Inc. */ > + > +#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 Sat, 7 Sep 2024 09:18:17 +0100 <alejandro.lucero-palau@amd.com> wrote: > From: Alejandro Lucero <alucerop@amd.com> > > Differientiate 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. > > Add SFC ethernet network driver as the client. Some thought when working with V2 about the driver initialization sequence. It seem the initialization sequence for the driver is quite long (since we do have a lot of stuff to handle) and there has to be a long error handling coming with it. Thinking the usual driver initialization sequence is long enough with error handling sequence, would it be better to lift this efx_cxl_init() as a initialization wrapper for the driver. It can take an init params and handling the initialization and errors by itself according to the params. (The new efx_cxl_init() just call the wrapper with params) Thanks, Zhi. > > Based on > 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 | 52 ++++++++++++++++ > drivers/cxl/core/pci.c | 1 + > drivers/cxl/cxlpci.h | 16 ----- > drivers/cxl/pci.c | 13 ++-- > drivers/net/ethernet/sfc/Makefile | 2 +- > drivers/net/ethernet/sfc/efx.c | 13 ++++ > drivers/net/ethernet/sfc/efx_cxl.c | 86 > +++++++++++++++++++++++++++ drivers/net/ethernet/sfc/efx_cxl.h | > 29 +++++++++ drivers/net/ethernet/sfc/net_driver.h | 6 ++ > include/linux/cxl/cxl.h | 21 +++++++ > include/linux/cxl/pci.h | 23 +++++++ > 11 files changed, 241 insertions(+), 21 deletions(-) > create mode 100644 drivers/net/ethernet/sfc/efx_cxl.c > create mode 100644 drivers/net/ethernet/sfc/efx_cxl.h > create mode 100644 include/linux/cxl/cxl.h > create mode 100644 include/linux/cxl/pci.h > > diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c > index 0277726afd04..10c0a6990f9a 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 <linux/cxl/cxl.h> > #include <linux/io-64-nonatomic-lo-hi.h> > #include <linux/firmware.h> > #include <linux/device.h> > @@ -615,6 +616,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) > { > @@ -692,6 +712,38 @@ 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_ACCEL_RES_DPA: > + cxlds->dpa_res = res; > + return 0; > + case CXL_ACCEL_RES_RAM: > + cxlds->ram_res = res; > + return 0; > + case CXL_ACCEL_RES_PMEM: > + cxlds->pmem_res = res; > + return 0; > + default: > + dev_err(cxlds->dev, "unknown resource type (%u)\n", > type); > + 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 51132a575b27..3d6564dbda57 100644 > --- a/drivers/cxl/core/pci.c > +++ b/drivers/cxl/core/pci.c > @@ -7,6 +7,7 @@ > #include <linux/pci.h> > #include <linux/pci-doe.h> > #include <linux/aer.h> > +#include <linux/cxl/pci.h> > #include <cxlpci.h> > #include <cxlmem.h> > #include <cxl.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 4be35dc22202..742a7b2a1be5 100644 > --- a/drivers/cxl/pci.c > +++ b/drivers/cxl/pci.c > @@ -11,6 +11,8 @@ > #include <linux/pci.h> > #include <linux/aer.h> > #include <linux/io.h> > +#include <linux/cxl/cxl.h> > +#include <linux/cxl/pci.h> > #include "cxlmem.h" > #include "cxlpci.h" > #include "cxl.h" > @@ -795,6 +797,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 @@ -815,12 +818,14 @@ 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"); > + else > + cxl_set_dvsec(cxlds, dvsec); > > rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map); > if (rc) > diff --git a/drivers/net/ethernet/sfc/Makefile > b/drivers/net/ethernet/sfc/Makefile index 8f446b9bd5ee..e80c713c3b0c > 100644 --- a/drivers/net/ethernet/sfc/Makefile > +++ b/drivers/net/ethernet/sfc/Makefile > @@ -7,7 +7,7 @@ sfc-y += efx.o efx_common.o > efx_channels.o nic.o \ mcdi_functions.o mcdi_filters.o mcdi_mon.o \ > ef100.o ef100_nic.o ef100_netdev.o \ > ef100_ethtool.o ef100_rx.o ef100_tx.o \ > - efx_devlink.o > + efx_devlink.o efx_cxl.o > sfc-$(CONFIG_SFC_MTD) += mtd.o > sfc-$(CONFIG_SFC_SRIOV) += sriov.o ef10_sriov.o ef100_sriov.o > ef100_rep.o \ mae.o tc.o tc_bindings.o tc_counters.o \ > diff --git a/drivers/net/ethernet/sfc/efx.c > b/drivers/net/ethernet/sfc/efx.c index 6f1a01ded7d4..3a7406aa950c > 100644 --- a/drivers/net/ethernet/sfc/efx.c > +++ b/drivers/net/ethernet/sfc/efx.c > @@ -33,6 +33,7 @@ > #include "selftest.h" > #include "sriov.h" > #include "efx_devlink.h" > +#include "efx_cxl.h" > > #include "mcdi_port_common.h" > #include "mcdi_pcol.h" > @@ -899,6 +900,9 @@ static void efx_pci_remove(struct pci_dev > *pci_dev) efx_pci_remove_main(efx); > > efx_fini_io(efx); > + > + efx_cxl_exit(efx); > + > pci_dbg(efx->pci_dev, "shutdown successful\n"); > > efx_fini_devlink_and_unlock(efx); > @@ -1109,6 +1113,15 @@ static int efx_pci_probe(struct pci_dev > *pci_dev, if (rc) > goto fail2; > > + /* A successful cxl initialization implies a CXL region > created to be > + * used for PIO buffers. If there is no CXL support, or > initialization > + * fails, efx_cxl_pio_initialised wll be false and legacy > PIO buffers > + * defined at specific PCI BAR regions will be used. > + */ > + rc = efx_cxl_init(efx); > + if (rc) > + pci_err(pci_dev, "CXL initialization failed with > error %d\n", rc); + > rc = efx_pci_probe_post_io(efx); > if (rc) { > /* On failure, retry once immediately. > diff --git a/drivers/net/ethernet/sfc/efx_cxl.c > b/drivers/net/ethernet/sfc/efx_cxl.c new file mode 100644 > index 000000000000..bba36cbbab22 > --- /dev/null > +++ b/drivers/net/ethernet/sfc/efx_cxl.c > @@ -0,0 +1,86 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/**************************************************************************** > + * > + * Driver for AMD network controllers and boards > + * Copyright (C) 2024, Advanced Micro Devices, Inc. > + * > + * This program is free software; you can redistribute it and/or > modify it > + * under the terms of the GNU General Public License version 2 as > published > + * by the Free Software Foundation, incorporated herein by reference. > + */ > + > +#include <linux/cxl/cxl.h> > +#include <linux/cxl/pci.h> > +#include <linux/pci.h> > + > +#include "net_driver.h" > +#include "efx_cxl.h" > + > +#define EFX_CTPIO_BUFFER_SIZE (1024 * 1024 * 256) > + > +int efx_cxl_init(struct efx_nic *efx) > +{ > + struct pci_dev *pci_dev = efx->pci_dev; > + struct efx_cxl *cxl; > + struct resource res; > + u16 dvsec; > + int rc; > + > + efx->efx_cxl_pio_initialised = false; > + > + dvsec = pci_find_dvsec_capability(pci_dev, PCI_VENDOR_ID_CXL, > + CXL_DVSEC_PCIE_DEVICE); > + > + if (!dvsec) > + return 0; > + > + pci_dbg(pci_dev, "CXL_DVSEC_PCIE_DEVICE capability found\n"); > + > + efx->cxl = kzalloc(sizeof(*cxl), GFP_KERNEL); > + if (!efx->cxl) > + return -ENOMEM; > + > + cxl = efx->cxl; > + > + cxl->cxlds = cxl_accel_state_create(&pci_dev->dev); > + if (IS_ERR(cxl->cxlds)) { > + pci_err(pci_dev, "CXL accel device state failed"); > + kfree(efx->cxl); > + return -ENOMEM; > + } > + > + cxl_set_dvsec(cxl->cxlds, dvsec); > + cxl_set_serial(cxl->cxlds, pci_dev->dev.id); > + > + res = DEFINE_RES_MEM(0, EFX_CTPIO_BUFFER_SIZE); > + if (cxl_set_resource(cxl->cxlds, res, CXL_ACCEL_RES_DPA)) { > + pci_err(pci_dev, "cxl_set_resource DPA failed\n"); > + rc = -EINVAL; > + goto err; > + } > + > + res = DEFINE_RES_MEM_NAMED(0, EFX_CTPIO_BUFFER_SIZE, "ram"); > + if (cxl_set_resource(cxl->cxlds, res, CXL_ACCEL_RES_RAM)) { > + pci_err(pci_dev, "cxl_set_resource RAM failed\n"); > + rc = -EINVAL; > + goto err; > + } > + > + return 0; > +err: > + kfree(cxl->cxlds); > + kfree(cxl); > + efx->cxl = NULL; > + > + return rc; > +} > + > +void efx_cxl_exit(struct efx_nic *efx) > +{ > + if (efx->cxl) { > + kfree(efx->cxl->cxlds); > + kfree(efx->cxl); > + } > +} > + > +MODULE_IMPORT_NS(CXL); > diff --git a/drivers/net/ethernet/sfc/efx_cxl.h > b/drivers/net/ethernet/sfc/efx_cxl.h new file mode 100644 > index 000000000000..f57fb2afd124 > --- /dev/null > +++ b/drivers/net/ethernet/sfc/efx_cxl.h > @@ -0,0 +1,29 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/**************************************************************************** > + * Driver for AMD network controllers and boards > + * Copyright (C) 2024, Advanced Micro Devices, Inc. > + * > + * This program is free software; you can redistribute it and/or > modify it > + * under the terms of the GNU General Public License version 2 as > published > + * by the Free Software Foundation, incorporated herein by reference. > + */ > + > +#ifndef EFX_CXL_H > +#define EFX_CXL_H > + > +struct efx_nic; > +struct cxl_dev_state; > + > +struct efx_cxl { > + struct cxl_dev_state *cxlds; > + struct cxl_memdev *cxlmd; > + struct cxl_root_decoder *cxlrd; > + struct cxl_port *endpoint; > + struct cxl_endpoint_decoder *cxled; > + struct cxl_region *efx_region; > + void __iomem *ctpio_cxl; > +}; > + > +int efx_cxl_init(struct efx_nic *efx); > +void efx_cxl_exit(struct efx_nic *efx); > +#endif > diff --git a/drivers/net/ethernet/sfc/net_driver.h > b/drivers/net/ethernet/sfc/net_driver.h index > b85c51cbe7f9..77261de65e63 100644 --- > a/drivers/net/ethernet/sfc/net_driver.h +++ > b/drivers/net/ethernet/sfc/net_driver.h @@ -817,6 +817,8 @@ enum > efx_xdp_tx_queues_mode { > struct efx_mae; > > +struct efx_cxl; > + > /** > * struct efx_nic - an Efx NIC > * @name: Device name (net device name or bus id before net device > registered) @@ -963,6 +965,8 @@ struct efx_mae; > * @tc: state for TC offload (EF100). > * @devlink: reference to devlink structure owned by this device > * @dl_port: devlink port associated with the PF > + * @cxl: details of related cxl objects > + * @efx_cxl_pio_initialised: clx initialization outcome. > * @mem_bar: The BAR that is mapped into membase. > * @reg_base: Offset from the start of the bar to the function > control window. > * @monitor_work: Hardware monitor workitem > @@ -1148,6 +1152,8 @@ struct efx_nic { > > struct devlink *devlink; > struct devlink_port *dl_port; > + struct efx_cxl *cxl; > + bool efx_cxl_pio_initialised; > unsigned int mem_bar; > u32 reg_base; > > diff --git a/include/linux/cxl/cxl.h b/include/linux/cxl/cxl.h > new file mode 100644 > index 000000000000..e78eefa82123 > --- /dev/null > +++ b/include/linux/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/device.h> > + > +enum cxl_resource { > + CXL_ACCEL_RES_DPA, > + CXL_ACCEL_RES_RAM, > + CXL_ACCEL_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/linux/cxl/pci.h b/include/linux/cxl/pci.h > new file mode 100644 > index 000000000000..c337ae8797e6 > --- /dev/null > +++ b/include/linux/cxl/pci.h > @@ -0,0 +1,23 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* Copyright(c) 2024 Advanced Micro Devices, Inc. */ > + > +#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 Sat, 7 Sep 2024 09:18:17 +0100 <alejandro.lucero-palau@amd.com> wrote: > From: Alejandro Lucero <alucerop@amd.com> Hi Alejandro, I'm mainly looking at these to get my head back into this support for discussions next week but will probably leave lots of trivial review feedback as I go. And to advertise that: https://lpc.events/event/18/contributions/1828/ > > Differientiate Type3, aka memory expanders, from Type2, aka device Spell check. Differentiate. > accelerators, with a new function for initializing cxl_dev_state. > > Create accessors to cxl_dev_state to be used by accel drivers. > > Add SFC ethernet network driver as the client. Minor thing (And others may disagree) but I'd split this to be nice to others who might want to backport the type2 support but not the sfc changes (as they are supporting some other hardware). > > Based on https://lore.kernel.org/linux-cxl/168592160379.1948938.12863272903570476312.stgit@dwillia2-xfh.jf.intel.com/ Maybe make that a link tag Link: .... # [1] and have Based on [1] here. > > Signed-off-by: Alejandro Lucero <alucerop@amd.com> > Co-developed-by: Dan Williams <dan.j.williams@intel.com> > +int cxl_set_resource(struct cxl_dev_state *cxlds, struct resource res, > + enum cxl_resource type) > +{ > + switch (type) { > + case CXL_ACCEL_RES_DPA: > + cxlds->dpa_res = res; > + return 0; > + case CXL_ACCEL_RES_RAM: > + cxlds->ram_res = res; > + return 0; > + case CXL_ACCEL_RES_PMEM: > + cxlds->pmem_res = res; > + return 0; > + default: > + dev_err(cxlds->dev, "unknown resource type (%u)\n", type); It's an enum, do we need the default? Hence do we need the return value? > + 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/pci.c b/drivers/cxl/pci.c > index 4be35dc22202..742a7b2a1be5 100644 > --- a/drivers/cxl/pci.c > +++ b/drivers/cxl/pci.c > @@ -11,6 +11,8 @@ > #include <linux/pci.h> > #include <linux/aer.h> > #include <linux/io.h> > +#include <linux/cxl/cxl.h> > +#include <linux/cxl/pci.h> > #include "cxlmem.h" > #include "cxlpci.h" > #include "cxl.h" > @@ -795,6 +797,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 > @@ -815,12 +818,14 @@ 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"); > + else > + cxl_set_dvsec(cxlds, dvsec); Set it unconditionally perhaps. If it's NULL that's fine and then it corresponds directly to the previous > > rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map); > if (rc) > diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c > index 6f1a01ded7d4..3a7406aa950c 100644 > --- a/drivers/net/ethernet/sfc/efx.c > +++ b/drivers/net/ethernet/sfc/efx.c > @@ -33,6 +33,7 @@ > #include "selftest.h" > #include "sriov.h" > #include "efx_devlink.h" > +#include "efx_cxl.h" > > #include "mcdi_port_common.h" > #include "mcdi_pcol.h" > @@ -899,6 +900,9 @@ static void efx_pci_remove(struct pci_dev *pci_dev) > efx_pci_remove_main(efx); > > efx_fini_io(efx); > + > + efx_cxl_exit(efx); > + > pci_dbg(efx->pci_dev, "shutdown successful\n"); > > efx_fini_devlink_and_unlock(efx); > @@ -1109,6 +1113,15 @@ static int efx_pci_probe(struct pci_dev *pci_dev, > if (rc) > goto fail2; > > + /* A successful cxl initialization implies a CXL region created to be > + * used for PIO buffers. If there is no CXL support, or initialization > + * fails, efx_cxl_pio_initialised wll be false and legacy PIO buffers > + * defined at specific PCI BAR regions will be used. > + */ > + rc = efx_cxl_init(efx); > + if (rc) > + pci_err(pci_dev, "CXL initialization failed with error %d\n", rc); If you are carrying on anyway is pci_info() more appropriate? Personally I dislike muddling on in error cases, but understand it can be useful on occasion at the cost of more complex flows. > + > rc = efx_pci_probe_post_io(efx); > if (rc) { > /* On failure, retry once immediately. > diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c > new file mode 100644 > index 000000000000..bba36cbbab22 > --- /dev/null > +++ b/drivers/net/ethernet/sfc/efx_cxl.c > @@ -0,0 +1,86 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/**************************************************************************** > + * > + * Driver for AMD network controllers and boards > + * Copyright (C) 2024, Advanced Micro Devices, Inc. > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License version 2 as published > + * by the Free Software Foundation, incorporated herein by reference. > + */ > + > +#include <linux/cxl/cxl.h> > +#include <linux/cxl/pci.h> > +#include <linux/pci.h> > + > +#include "net_driver.h" > +#include "efx_cxl.h" > + > +#define EFX_CTPIO_BUFFER_SIZE (1024 * 1024 * 256) > + > +int efx_cxl_init(struct efx_nic *efx) > +{ > + struct pci_dev *pci_dev = efx->pci_dev; > + struct efx_cxl *cxl; > + struct resource res; > + u16 dvsec; > + int rc; > + > + efx->efx_cxl_pio_initialised = false; > + > + dvsec = pci_find_dvsec_capability(pci_dev, PCI_VENDOR_ID_CXL, > + CXL_DVSEC_PCIE_DEVICE); > + Trivial but probably no blank line here. Keeps the error condition tightly grouped with the call. > + if (!dvsec) > + return 0; > + > + pci_dbg(pci_dev, "CXL_DVSEC_PCIE_DEVICE capability found\n"); > + > + efx->cxl = kzalloc(sizeof(*cxl), GFP_KERNEL); > + if (!efx->cxl) > + return -ENOMEM; > + > + cxl = efx->cxl; Rather than setting it back to zero in some error paths I'd suggest keeping it as local only until you know everything succeeded. cxl = kzalloc(...) //maybe also cxlds as then you can use __free() to handle the //cleanup paths for both allowing early returns instead //of gotos. ... efx->cxl = cxl; return 0; > + > + cxl->cxlds = cxl_accel_state_create(&pci_dev->dev); > + if (IS_ERR(cxl->cxlds)) { > + pci_err(pci_dev, "CXL accel device state failed"); > + kfree(efx->cxl); Use the a separate label below. Error blocks in a given function should probably do one or the other between going to labels or handling locally. Mixture is harder to read. > + return -ENOMEM; > + } > + > + cxl_set_dvsec(cxl->cxlds, dvsec); > + cxl_set_serial(cxl->cxlds, pci_dev->dev.id); > + > + res = DEFINE_RES_MEM(0, EFX_CTPIO_BUFFER_SIZE); > + if (cxl_set_resource(cxl->cxlds, res, CXL_ACCEL_RES_DPA)) { > + pci_err(pci_dev, "cxl_set_resource DPA failed\n"); > + rc = -EINVAL; > + goto err; > + } > + > + res = DEFINE_RES_MEM_NAMED(0, EFX_CTPIO_BUFFER_SIZE, "ram"); > + if (cxl_set_resource(cxl->cxlds, res, CXL_ACCEL_RES_RAM)) { > + pci_err(pci_dev, "cxl_set_resource RAM failed\n"); > + rc = -EINVAL; > + goto err; > + } > + > + return 0; > +err: > + kfree(cxl->cxlds); > + kfree(cxl); > + efx->cxl = NULL; > + > + return rc; > +} > + > +void efx_cxl_exit(struct efx_nic *efx) > +{ > + if (efx->cxl) { > + kfree(efx->cxl->cxlds); > + kfree(efx->cxl); > + } > +} > + > +MODULE_IMPORT_NS(CXL); > diff --git a/drivers/net/ethernet/sfc/efx_cxl.h b/drivers/net/ethernet/sfc/efx_cxl.h > new file mode 100644 > index 000000000000..f57fb2afd124 > --- /dev/null > +++ b/drivers/net/ethernet/sfc/efx_cxl.h ... > +struct efx_cxl { > + struct cxl_dev_state *cxlds; > + struct cxl_memdev *cxlmd; > + struct cxl_root_decoder *cxlrd; > + struct cxl_port *endpoint; > + struct cxl_endpoint_decoder *cxled; > + struct cxl_region *efx_region; Why is the region efx_ prefixed but nothing else? Feels a little random. > + void __iomem *ctpio_cxl; > +}; > + > +int efx_cxl_init(struct efx_nic *efx); > +void efx_cxl_exit(struct efx_nic *efx); > +#endif > diff --git a/include/linux/cxl/cxl.h b/include/linux/cxl/cxl.h > new file mode 100644 > index 000000000000..e78eefa82123 > --- /dev/null > +++ b/include/linux/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/device.h> > + > +enum cxl_resource { > + CXL_ACCEL_RES_DPA, > + CXL_ACCEL_RES_RAM, > + CXL_ACCEL_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/linux/cxl/pci.h b/include/linux/cxl/pci.h > new file mode 100644 > index 000000000000..c337ae8797e6 > --- /dev/null > +++ b/include/linux/cxl/pci.h > @@ -0,0 +1,23 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* Copyright(c) 2024 Advanced Micro Devices, Inc. */ Bit bold to claim sole copyright of a cut and paste blob. Fine to add AMD one, but keep the original copyright as well. > + > +#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)) Brackets around (i) to protect against stupid use of the macro. This is general kernel convention rather than a real problem here. Sure original code didn't do it but if we are touching the code might as well fix it ;) > +#define CXL_DVSEC_MEM_BASE_LOW_MASK GENMASK(31, 28) > + > +#endif
>> + >> +#include "net_driver.h" >> +#include "efx_cxl.h" >> + >> +#define EFX_CTPIO_BUFFER_SIZE (1024 * 1024 * 256) >> + > Use SZ_256M in include/linux/sizes.h > > I'll do. Thanks!
On 9/12/24 10:35, Zhi Wang wrote: > On Sat, 7 Sep 2024 09:18:17 +0100 > <alejandro.lucero-palau@amd.com> wrote: > >> From: Alejandro Lucero <alucerop@amd.com> >> >> Differientiate 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. >> >> Add SFC ethernet network driver as the client. > Some thought when working with V2 about the driver initialization > sequence. It seem the initialization sequence for the driver is quite > long (since we do have a lot of stuff to handle) and there has to be a > long error handling coming with it. > > Thinking the usual driver initialization sequence is long enough with > error handling sequence, would it be better to lift this efx_cxl_init() > as a initialization wrapper for the driver. > > It can take an init params and handling the initialization and errors by > itself according to the params. (The new efx_cxl_init() just > call the wrapper with params) Sorry, I do not follow your comment. If you are suggesting to efx_cxl_init being the main init function for sfc, I'm afraid that is not easy to happen and I do not think it is justified. Thanks > Thanks, > Zhi. >> Based on >> 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 | 52 ++++++++++++++++ >> drivers/cxl/core/pci.c | 1 + >> drivers/cxl/cxlpci.h | 16 ----- >> drivers/cxl/pci.c | 13 ++-- >> drivers/net/ethernet/sfc/Makefile | 2 +- >> drivers/net/ethernet/sfc/efx.c | 13 ++++ >> drivers/net/ethernet/sfc/efx_cxl.c | 86 >> +++++++++++++++++++++++++++ drivers/net/ethernet/sfc/efx_cxl.h | >> 29 +++++++++ drivers/net/ethernet/sfc/net_driver.h | 6 ++ >> include/linux/cxl/cxl.h | 21 +++++++ >> include/linux/cxl/pci.h | 23 +++++++ >> 11 files changed, 241 insertions(+), 21 deletions(-) >> create mode 100644 drivers/net/ethernet/sfc/efx_cxl.c >> create mode 100644 drivers/net/ethernet/sfc/efx_cxl.h >> create mode 100644 include/linux/cxl/cxl.h >> create mode 100644 include/linux/cxl/pci.h >> >> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c >> index 0277726afd04..10c0a6990f9a 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 <linux/cxl/cxl.h> >> #include <linux/io-64-nonatomic-lo-hi.h> >> #include <linux/firmware.h> >> #include <linux/device.h> >> @@ -615,6 +616,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) >> { >> @@ -692,6 +712,38 @@ 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_ACCEL_RES_DPA: >> + cxlds->dpa_res = res; >> + return 0; >> + case CXL_ACCEL_RES_RAM: >> + cxlds->ram_res = res; >> + return 0; >> + case CXL_ACCEL_RES_PMEM: >> + cxlds->pmem_res = res; >> + return 0; >> + default: >> + dev_err(cxlds->dev, "unknown resource type (%u)\n", >> type); >> + 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 51132a575b27..3d6564dbda57 100644 >> --- a/drivers/cxl/core/pci.c >> +++ b/drivers/cxl/core/pci.c >> @@ -7,6 +7,7 @@ >> #include <linux/pci.h> >> #include <linux/pci-doe.h> >> #include <linux/aer.h> >> +#include <linux/cxl/pci.h> >> #include <cxlpci.h> >> #include <cxlmem.h> >> #include <cxl.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 4be35dc22202..742a7b2a1be5 100644 >> --- a/drivers/cxl/pci.c >> +++ b/drivers/cxl/pci.c >> @@ -11,6 +11,8 @@ >> #include <linux/pci.h> >> #include <linux/aer.h> >> #include <linux/io.h> >> +#include <linux/cxl/cxl.h> >> +#include <linux/cxl/pci.h> >> #include "cxlmem.h" >> #include "cxlpci.h" >> #include "cxl.h" >> @@ -795,6 +797,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 @@ -815,12 +818,14 @@ 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"); >> + else >> + cxl_set_dvsec(cxlds, dvsec); >> >> rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map); >> if (rc) >> diff --git a/drivers/net/ethernet/sfc/Makefile >> b/drivers/net/ethernet/sfc/Makefile index 8f446b9bd5ee..e80c713c3b0c >> 100644 --- a/drivers/net/ethernet/sfc/Makefile >> +++ b/drivers/net/ethernet/sfc/Makefile >> @@ -7,7 +7,7 @@ sfc-y += efx.o efx_common.o >> efx_channels.o nic.o \ mcdi_functions.o mcdi_filters.o mcdi_mon.o \ >> ef100.o ef100_nic.o ef100_netdev.o \ >> ef100_ethtool.o ef100_rx.o ef100_tx.o \ >> - efx_devlink.o >> + efx_devlink.o efx_cxl.o >> sfc-$(CONFIG_SFC_MTD) += mtd.o >> sfc-$(CONFIG_SFC_SRIOV) += sriov.o ef10_sriov.o ef100_sriov.o >> ef100_rep.o \ mae.o tc.o tc_bindings.o tc_counters.o \ >> diff --git a/drivers/net/ethernet/sfc/efx.c >> b/drivers/net/ethernet/sfc/efx.c index 6f1a01ded7d4..3a7406aa950c >> 100644 --- a/drivers/net/ethernet/sfc/efx.c >> +++ b/drivers/net/ethernet/sfc/efx.c >> @@ -33,6 +33,7 @@ >> #include "selftest.h" >> #include "sriov.h" >> #include "efx_devlink.h" >> +#include "efx_cxl.h" >> >> #include "mcdi_port_common.h" >> #include "mcdi_pcol.h" >> @@ -899,6 +900,9 @@ static void efx_pci_remove(struct pci_dev >> *pci_dev) efx_pci_remove_main(efx); >> >> efx_fini_io(efx); >> + >> + efx_cxl_exit(efx); >> + >> pci_dbg(efx->pci_dev, "shutdown successful\n"); >> >> efx_fini_devlink_and_unlock(efx); >> @@ -1109,6 +1113,15 @@ static int efx_pci_probe(struct pci_dev >> *pci_dev, if (rc) >> goto fail2; >> >> + /* A successful cxl initialization implies a CXL region >> created to be >> + * used for PIO buffers. If there is no CXL support, or >> initialization >> + * fails, efx_cxl_pio_initialised wll be false and legacy >> PIO buffers >> + * defined at specific PCI BAR regions will be used. >> + */ >> + rc = efx_cxl_init(efx); >> + if (rc) >> + pci_err(pci_dev, "CXL initialization failed with >> error %d\n", rc); + >> rc = efx_pci_probe_post_io(efx); >> if (rc) { >> /* On failure, retry once immediately. >> diff --git a/drivers/net/ethernet/sfc/efx_cxl.c >> b/drivers/net/ethernet/sfc/efx_cxl.c new file mode 100644 >> index 000000000000..bba36cbbab22 >> --- /dev/null >> +++ b/drivers/net/ethernet/sfc/efx_cxl.c >> @@ -0,0 +1,86 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> +/**************************************************************************** >> + * >> + * Driver for AMD network controllers and boards >> + * Copyright (C) 2024, Advanced Micro Devices, Inc. >> + * >> + * This program is free software; you can redistribute it and/or >> modify it >> + * under the terms of the GNU General Public License version 2 as >> published >> + * by the Free Software Foundation, incorporated herein by reference. >> + */ >> + >> +#include <linux/cxl/cxl.h> >> +#include <linux/cxl/pci.h> >> +#include <linux/pci.h> >> + >> +#include "net_driver.h" >> +#include "efx_cxl.h" >> + >> +#define EFX_CTPIO_BUFFER_SIZE (1024 * 1024 * 256) >> + >> +int efx_cxl_init(struct efx_nic *efx) >> +{ >> + struct pci_dev *pci_dev = efx->pci_dev; >> + struct efx_cxl *cxl; >> + struct resource res; >> + u16 dvsec; >> + int rc; >> + >> + efx->efx_cxl_pio_initialised = false; >> + >> + dvsec = pci_find_dvsec_capability(pci_dev, PCI_VENDOR_ID_CXL, >> + CXL_DVSEC_PCIE_DEVICE); >> + >> + if (!dvsec) >> + return 0; >> + >> + pci_dbg(pci_dev, "CXL_DVSEC_PCIE_DEVICE capability found\n"); >> + >> + efx->cxl = kzalloc(sizeof(*cxl), GFP_KERNEL); >> + if (!efx->cxl) >> + return -ENOMEM; >> + >> + cxl = efx->cxl; >> + >> + cxl->cxlds = cxl_accel_state_create(&pci_dev->dev); >> + if (IS_ERR(cxl->cxlds)) { >> + pci_err(pci_dev, "CXL accel device state failed"); >> + kfree(efx->cxl); >> + return -ENOMEM; >> + } >> + >> + cxl_set_dvsec(cxl->cxlds, dvsec); >> + cxl_set_serial(cxl->cxlds, pci_dev->dev.id); >> + >> + res = DEFINE_RES_MEM(0, EFX_CTPIO_BUFFER_SIZE); >> + if (cxl_set_resource(cxl->cxlds, res, CXL_ACCEL_RES_DPA)) { >> + pci_err(pci_dev, "cxl_set_resource DPA failed\n"); >> + rc = -EINVAL; >> + goto err; >> + } >> + >> + res = DEFINE_RES_MEM_NAMED(0, EFX_CTPIO_BUFFER_SIZE, "ram"); >> + if (cxl_set_resource(cxl->cxlds, res, CXL_ACCEL_RES_RAM)) { >> + pci_err(pci_dev, "cxl_set_resource RAM failed\n"); >> + rc = -EINVAL; >> + goto err; >> + } >> + >> + return 0; >> +err: >> + kfree(cxl->cxlds); >> + kfree(cxl); >> + efx->cxl = NULL; >> + >> + return rc; >> +} >> + >> +void efx_cxl_exit(struct efx_nic *efx) >> +{ >> + if (efx->cxl) { >> + kfree(efx->cxl->cxlds); >> + kfree(efx->cxl); >> + } >> +} >> + >> +MODULE_IMPORT_NS(CXL); >> diff --git a/drivers/net/ethernet/sfc/efx_cxl.h >> b/drivers/net/ethernet/sfc/efx_cxl.h new file mode 100644 >> index 000000000000..f57fb2afd124 >> --- /dev/null >> +++ b/drivers/net/ethernet/sfc/efx_cxl.h >> @@ -0,0 +1,29 @@ >> +/* SPDX-License-Identifier: GPL-2.0-only */ >> +/**************************************************************************** >> + * Driver for AMD network controllers and boards >> + * Copyright (C) 2024, Advanced Micro Devices, Inc. >> + * >> + * This program is free software; you can redistribute it and/or >> modify it >> + * under the terms of the GNU General Public License version 2 as >> published >> + * by the Free Software Foundation, incorporated herein by reference. >> + */ >> + >> +#ifndef EFX_CXL_H >> +#define EFX_CXL_H >> + >> +struct efx_nic; >> +struct cxl_dev_state; >> + >> +struct efx_cxl { >> + struct cxl_dev_state *cxlds; >> + struct cxl_memdev *cxlmd; >> + struct cxl_root_decoder *cxlrd; >> + struct cxl_port *endpoint; >> + struct cxl_endpoint_decoder *cxled; >> + struct cxl_region *efx_region; >> + void __iomem *ctpio_cxl; >> +}; >> + >> +int efx_cxl_init(struct efx_nic *efx); >> +void efx_cxl_exit(struct efx_nic *efx); >> +#endif >> diff --git a/drivers/net/ethernet/sfc/net_driver.h >> b/drivers/net/ethernet/sfc/net_driver.h index >> b85c51cbe7f9..77261de65e63 100644 --- >> a/drivers/net/ethernet/sfc/net_driver.h +++ >> b/drivers/net/ethernet/sfc/net_driver.h @@ -817,6 +817,8 @@ enum >> efx_xdp_tx_queues_mode { >> struct efx_mae; >> >> +struct efx_cxl; >> + >> /** >> * struct efx_nic - an Efx NIC >> * @name: Device name (net device name or bus id before net device >> registered) @@ -963,6 +965,8 @@ struct efx_mae; >> * @tc: state for TC offload (EF100). >> * @devlink: reference to devlink structure owned by this device >> * @dl_port: devlink port associated with the PF >> + * @cxl: details of related cxl objects >> + * @efx_cxl_pio_initialised: clx initialization outcome. >> * @mem_bar: The BAR that is mapped into membase. >> * @reg_base: Offset from the start of the bar to the function >> control window. >> * @monitor_work: Hardware monitor workitem >> @@ -1148,6 +1152,8 @@ struct efx_nic { >> >> struct devlink *devlink; >> struct devlink_port *dl_port; >> + struct efx_cxl *cxl; >> + bool efx_cxl_pio_initialised; >> unsigned int mem_bar; >> u32 reg_base; >> >> diff --git a/include/linux/cxl/cxl.h b/include/linux/cxl/cxl.h >> new file mode 100644 >> index 000000000000..e78eefa82123 >> --- /dev/null >> +++ b/include/linux/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/device.h> >> + >> +enum cxl_resource { >> + CXL_ACCEL_RES_DPA, >> + CXL_ACCEL_RES_RAM, >> + CXL_ACCEL_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/linux/cxl/pci.h b/include/linux/cxl/pci.h >> new file mode 100644 >> index 000000000000..c337ae8797e6 >> --- /dev/null >> +++ b/include/linux/cxl/pci.h >> @@ -0,0 +1,23 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +/* Copyright(c) 2024 Advanced Micro Devices, Inc. */ >> + >> +#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 9/13/24 17:41, Jonathan Cameron wrote: > On Sat, 7 Sep 2024 09:18:17 +0100 > <alejandro.lucero-palau@amd.com> wrote: > >> From: Alejandro Lucero <alucerop@amd.com> > Hi Alejandro, > > I'm mainly looking at these to get my head back into this support > for discussions next week but will probably leave > lots of trivial review feedback as I go. > > And to advertise that: > https://lpc.events/event/18/contributions/1828/ Looking forward to see you there along with other CXL kernel guys. >> Differientiate Type3, aka memory expanders, from Type2, aka device > Spell check. Differentiate. Embarrassing ...I did fix that or I though I did since this was also pointed out by Dan Williams as well. I'll definitely fix it for v4. >> accelerators, with a new function for initializing cxl_dev_state. >> >> Create accessors to cxl_dev_state to be used by accel drivers. >> >> Add SFC ethernet network driver as the client. > Minor thing (And others may disagree) but I'd split this to be nice > to others who might want to backport the type2 support but not > the sfc changes (as they are supporting some other hardware). Should I then send incremental sfc changes as well as the API is introduced or just a final patch with all of it? >> Based on https://lore.kernel.org/linux-cxl/168592160379.1948938.12863272903570476312.stgit@dwillia2-xfh.jf.intel.com/ > Maybe make that a link tag Link: .... # [1] > and have > Based on [1] here. OK. >> Signed-off-by: Alejandro Lucero <alucerop@amd.com> >> Co-developed-by: Dan Williams <dan.j.williams@intel.com> > >> +int cxl_set_resource(struct cxl_dev_state *cxlds, struct resource res, >> + enum cxl_resource type) >> +{ >> + switch (type) { >> + case CXL_ACCEL_RES_DPA: >> + cxlds->dpa_res = res; >> + return 0; >> + case CXL_ACCEL_RES_RAM: >> + cxlds->ram_res = res; >> + return 0; >> + case CXL_ACCEL_RES_PMEM: >> + cxlds->pmem_res = res; >> + return 0; >> + default: >> + dev_err(cxlds->dev, "unknown resource type (%u)\n", type); > It's an enum, do we need the default? Hence do we need the return value? > I think it does not harm and helps with extending the enum without silently failing if all the places where it is used are not properly updated. >> + 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/pci.c b/drivers/cxl/pci.c >> index 4be35dc22202..742a7b2a1be5 100644 >> --- a/drivers/cxl/pci.c >> +++ b/drivers/cxl/pci.c >> @@ -11,6 +11,8 @@ >> #include <linux/pci.h> >> #include <linux/aer.h> >> #include <linux/io.h> >> +#include <linux/cxl/cxl.h> >> +#include <linux/cxl/pci.h> >> #include "cxlmem.h" >> #include "cxlpci.h" >> #include "cxl.h" >> @@ -795,6 +797,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 >> @@ -815,12 +818,14 @@ 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"); >> + else >> + cxl_set_dvsec(cxlds, dvsec); > Set it unconditionally perhaps. If it's NULL that's fine and then it corresponds > directly to the previous OK. I guess keeping the dev_warn. Right? >> >> rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map); >> if (rc) >> diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c >> index 6f1a01ded7d4..3a7406aa950c 100644 >> --- a/drivers/net/ethernet/sfc/efx.c >> +++ b/drivers/net/ethernet/sfc/efx.c >> @@ -33,6 +33,7 @@ >> #include "selftest.h" >> #include "sriov.h" >> #include "efx_devlink.h" >> +#include "efx_cxl.h" >> >> #include "mcdi_port_common.h" >> #include "mcdi_pcol.h" >> @@ -899,6 +900,9 @@ static void efx_pci_remove(struct pci_dev *pci_dev) >> efx_pci_remove_main(efx); >> >> efx_fini_io(efx); >> + >> + efx_cxl_exit(efx); >> + >> pci_dbg(efx->pci_dev, "shutdown successful\n"); >> >> efx_fini_devlink_and_unlock(efx); >> @@ -1109,6 +1113,15 @@ static int efx_pci_probe(struct pci_dev *pci_dev, >> if (rc) >> goto fail2; >> >> + /* A successful cxl initialization implies a CXL region created to be >> + * used for PIO buffers. If there is no CXL support, or initialization >> + * fails, efx_cxl_pio_initialised wll be false and legacy PIO buffers >> + * defined at specific PCI BAR regions will be used. >> + */ >> + rc = efx_cxl_init(efx); >> + if (rc) >> + pci_err(pci_dev, "CXL initialization failed with error %d\n", rc); > If you are carrying on anyway is pci_info() more appropriate? > Personally I dislike muddling on in error cases, but understand > it can be useful on occasion at the cost of more complex flows. > > Not sure. Note this is for the case something went wrong when the device has CXL support. It is not fatal, but it is an error. >> + >> rc = efx_pci_probe_post_io(efx); >> if (rc) { >> /* On failure, retry once immediately. >> diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c >> new file mode 100644 >> index 000000000000..bba36cbbab22 >> --- /dev/null >> +++ b/drivers/net/ethernet/sfc/efx_cxl.c >> @@ -0,0 +1,86 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> +/**************************************************************************** >> + * >> + * Driver for AMD network controllers and boards >> + * Copyright (C) 2024, Advanced Micro Devices, Inc. >> + * >> + * This program is free software; you can redistribute it and/or modify it >> + * under the terms of the GNU General Public License version 2 as published >> + * by the Free Software Foundation, incorporated herein by reference. >> + */ >> + >> +#include <linux/cxl/cxl.h> >> +#include <linux/cxl/pci.h> >> +#include <linux/pci.h> >> + >> +#include "net_driver.h" >> +#include "efx_cxl.h" >> + >> +#define EFX_CTPIO_BUFFER_SIZE (1024 * 1024 * 256) >> + >> +int efx_cxl_init(struct efx_nic *efx) >> +{ >> + struct pci_dev *pci_dev = efx->pci_dev; >> + struct efx_cxl *cxl; >> + struct resource res; >> + u16 dvsec; >> + int rc; >> + >> + efx->efx_cxl_pio_initialised = false; >> + >> + dvsec = pci_find_dvsec_capability(pci_dev, PCI_VENDOR_ID_CXL, >> + CXL_DVSEC_PCIE_DEVICE); >> + > Trivial but probably no blank line here. Keeps the error condition tightly > grouped with the call. OK >> + if (!dvsec) >> + return 0; >> + >> + pci_dbg(pci_dev, "CXL_DVSEC_PCIE_DEVICE capability found\n"); >> + >> + efx->cxl = kzalloc(sizeof(*cxl), GFP_KERNEL); >> + if (!efx->cxl) >> + return -ENOMEM; >> + >> + cxl = efx->cxl; > Rather than setting it back to zero in some error paths I'd > suggest keeping it as local only until you know everything > succeeded. > > cxl = kzalloc(...) > It makes sense. > //maybe also cxlds as then you can use __free() to handle the > //cleanup paths for both allowing early returns instead > //of gotos. Maybe, but using __free is discouraged in network code: 1.6.5 at https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html > ... > > efx->cxl = cxl; > > return 0; > >> + >> + cxl->cxlds = cxl_accel_state_create(&pci_dev->dev); >> + if (IS_ERR(cxl->cxlds)) { >> + pci_err(pci_dev, "CXL accel device state failed"); >> + kfree(efx->cxl); > Use the a separate label below. Error blocks in a given function > should probably do one or the other between going to labels > or handling locally. Mixture is harder to read. OK > >> + return -ENOMEM; >> + } >> + >> + cxl_set_dvsec(cxl->cxlds, dvsec); >> + cxl_set_serial(cxl->cxlds, pci_dev->dev.id); >> + >> + res = DEFINE_RES_MEM(0, EFX_CTPIO_BUFFER_SIZE); >> + if (cxl_set_resource(cxl->cxlds, res, CXL_ACCEL_RES_DPA)) { >> + pci_err(pci_dev, "cxl_set_resource DPA failed\n"); >> + rc = -EINVAL; >> + goto err; >> + } >> + >> + res = DEFINE_RES_MEM_NAMED(0, EFX_CTPIO_BUFFER_SIZE, "ram"); >> + if (cxl_set_resource(cxl->cxlds, res, CXL_ACCEL_RES_RAM)) { >> + pci_err(pci_dev, "cxl_set_resource RAM failed\n"); >> + rc = -EINVAL; >> + goto err; >> + } >> + >> + return 0; >> +err: >> + kfree(cxl->cxlds); >> + kfree(cxl); >> + efx->cxl = NULL; >> + >> + return rc; >> +} >> + >> +void efx_cxl_exit(struct efx_nic *efx) >> +{ >> + if (efx->cxl) { >> + kfree(efx->cxl->cxlds); >> + kfree(efx->cxl); >> + } >> +} >> + >> +MODULE_IMPORT_NS(CXL); >> diff --git a/drivers/net/ethernet/sfc/efx_cxl.h b/drivers/net/ethernet/sfc/efx_cxl.h >> new file mode 100644 >> index 000000000000..f57fb2afd124 >> --- /dev/null >> +++ b/drivers/net/ethernet/sfc/efx_cxl.h > ... > > >> +struct efx_cxl { >> + struct cxl_dev_state *cxlds; >> + struct cxl_memdev *cxlmd; >> + struct cxl_root_decoder *cxlrd; >> + struct cxl_port *endpoint; >> + struct cxl_endpoint_decoder *cxled; >> + struct cxl_region *efx_region; > Why is the region efx_ prefixed but nothing else? > Feels a little random. > >> + void __iomem *ctpio_cxl; >> +}; >> + >> +int efx_cxl_init(struct efx_nic *efx); >> +void efx_cxl_exit(struct efx_nic *efx); >> +#endif >> diff --git a/include/linux/cxl/cxl.h b/include/linux/cxl/cxl.h >> new file mode 100644 >> index 000000000000..e78eefa82123 >> --- /dev/null >> +++ b/include/linux/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/device.h> >> + >> +enum cxl_resource { >> + CXL_ACCEL_RES_DPA, >> + CXL_ACCEL_RES_RAM, >> + CXL_ACCEL_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/linux/cxl/pci.h b/include/linux/cxl/pci.h >> new file mode 100644 >> index 000000000000..c337ae8797e6 >> --- /dev/null >> +++ b/include/linux/cxl/pci.h >> @@ -0,0 +1,23 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +/* Copyright(c) 2024 Advanced Micro Devices, Inc. */ > Bit bold to claim sole copyright of a cut and paste blob. > Fine to add AMD one, but keep the original copyright as well. > Sure. >> + >> +#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)) > Brackets around (i) to protect against stupid use of the macro. > This is general kernel convention rather than a real problem here. > Sure original code didn't do it but if we are touching the code > might as well fix it ;) I found this warning when checkpatch and I thought it should not be done then as it was there from a previous patch. But I agree, I should fix it now. Thanks! > >> +#define CXL_DVSEC_MEM_BASE_LOW_MASK GENMASK(31, 28) >> + >> +#endif
On Mon, 16 Sep 2024 13:03:10 +0100 Alejandro Lucero Palau <alucerop@amd.com> wrote: > On 9/13/24 17:41, Jonathan Cameron wrote: > >> Add SFC ethernet network driver as the client. > > Minor thing (And others may disagree) but I'd split this to be nice > > to others who might want to backport the type2 support but not > > the sfc changes (as they are supporting some other hardware). > > > Should I then send incremental sfc changes as well as the API is > introduced or just a final patch with all of it? Given aim is to justify each step for this first user I think incremental sfc changes do make sense. > > > >> Signed-off-by: Alejandro Lucero <alucerop@amd.com> > >> Co-developed-by: Dan Williams <dan.j.williams@intel.com> > > > >> +int cxl_set_resource(struct cxl_dev_state *cxlds, struct resource res, > >> + enum cxl_resource type) > >> +{ > >> + switch (type) { > >> + case CXL_ACCEL_RES_DPA: > >> + cxlds->dpa_res = res; > >> + return 0; > >> + case CXL_ACCEL_RES_RAM: > >> + cxlds->ram_res = res; > >> + return 0; > >> + case CXL_ACCEL_RES_PMEM: > >> + cxlds->pmem_res = res; > >> + return 0; > >> + default: > >> + dev_err(cxlds->dev, "unknown resource type (%u)\n", type); > > It's an enum, do we need the default? Hence do we need the return value? > > > > I think it does not harm and helps with extending the enum without > silently failing if all the places where it is used are not properly > updated. It won't silently fail. The various build bots love to point out unhandled cases :) Adding the default means that you'll only see the problem in runtime testing rather than at build time. > > > >> + 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 = > >> + if (!dvsec) > >> dev_warn(&pdev->dev, > >> "Device DVSEC not present, skip CXL.mem init\n"); > >> + else > >> + cxl_set_dvsec(cxlds, dvsec); > > Set it unconditionally perhaps. If it's NULL that's fine and then it corresponds > > directly to the previous > > > OK. I guess keeping the dev_warn. Right? Absolutely. > >> diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c > >> index 6f1a01ded7d4..3a7406aa950c 100644 > >> --- a/drivers/net/ethernet/sfc/efx.c > >> +++ b/drivers/net/ethernet/sfc/efx.c > >> @@ -1109,6 +1113,15 @@ static int efx_pci_probe(struct pci_dev *pci_dev, > >> if (rc) > >> goto fail2; > >> > >> + /* A successful cxl initialization implies a CXL region created to be > >> + * used for PIO buffers. If there is no CXL support, or initialization > >> + * fails, efx_cxl_pio_initialised wll be false and legacy PIO buffers > >> + * defined at specific PCI BAR regions will be used. > >> + */ > >> + rc = efx_cxl_init(efx); > >> + if (rc) > >> + pci_err(pci_dev, "CXL initialization failed with error %d\n", rc); > > If you are carrying on anyway is pci_info() more appropriate? > > Personally I dislike muddling on in error cases, but understand > > it can be useful on occasion at the cost of more complex flows. > > > > > > Not sure. Note this is for the case something went wrong when the device > has CXL support. > > It is not fatal, but it is an error. Fair enough. I don't care that much about this. > > > >> + > >> rc = efx_pci_probe_post_io(efx); > >> if (rc) { > >> /* On failure, retry once immediately. > >> diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c > >> new file mode 100644 > >> index 000000000000..bba36cbbab22 > >> --- /dev/null > >> +++ b/drivers/net/ethernet/sfc/efx_cxl.c > > //maybe also cxlds as then you can use __free() to handle the > > //cleanup paths for both allowing early returns instead > > //of gotos. > > > Maybe, but using __free is discouraged in network code: 1.6.5 at > > https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html Fair enough. I've not been keeping up with networking maintainer preferences recently. Jonathan
diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c index 0277726afd04..10c0a6990f9a 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 <linux/cxl/cxl.h> #include <linux/io-64-nonatomic-lo-hi.h> #include <linux/firmware.h> #include <linux/device.h> @@ -615,6 +616,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) { @@ -692,6 +712,38 @@ 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_ACCEL_RES_DPA: + cxlds->dpa_res = res; + return 0; + case CXL_ACCEL_RES_RAM: + cxlds->ram_res = res; + return 0; + case CXL_ACCEL_RES_PMEM: + cxlds->pmem_res = res; + return 0; + default: + dev_err(cxlds->dev, "unknown resource type (%u)\n", type); + 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 51132a575b27..3d6564dbda57 100644 --- a/drivers/cxl/core/pci.c +++ b/drivers/cxl/core/pci.c @@ -7,6 +7,7 @@ #include <linux/pci.h> #include <linux/pci-doe.h> #include <linux/aer.h> +#include <linux/cxl/pci.h> #include <cxlpci.h> #include <cxlmem.h> #include <cxl.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 4be35dc22202..742a7b2a1be5 100644 --- a/drivers/cxl/pci.c +++ b/drivers/cxl/pci.c @@ -11,6 +11,8 @@ #include <linux/pci.h> #include <linux/aer.h> #include <linux/io.h> +#include <linux/cxl/cxl.h> +#include <linux/cxl/pci.h> #include "cxlmem.h" #include "cxlpci.h" #include "cxl.h" @@ -795,6 +797,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 @@ -815,12 +818,14 @@ 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"); + else + cxl_set_dvsec(cxlds, dvsec); rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map); if (rc) diff --git a/drivers/net/ethernet/sfc/Makefile b/drivers/net/ethernet/sfc/Makefile index 8f446b9bd5ee..e80c713c3b0c 100644 --- a/drivers/net/ethernet/sfc/Makefile +++ b/drivers/net/ethernet/sfc/Makefile @@ -7,7 +7,7 @@ sfc-y += efx.o efx_common.o efx_channels.o nic.o \ mcdi_functions.o mcdi_filters.o mcdi_mon.o \ ef100.o ef100_nic.o ef100_netdev.o \ ef100_ethtool.o ef100_rx.o ef100_tx.o \ - efx_devlink.o + efx_devlink.o efx_cxl.o sfc-$(CONFIG_SFC_MTD) += mtd.o sfc-$(CONFIG_SFC_SRIOV) += sriov.o ef10_sriov.o ef100_sriov.o ef100_rep.o \ mae.o tc.o tc_bindings.o tc_counters.o \ diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c index 6f1a01ded7d4..3a7406aa950c 100644 --- a/drivers/net/ethernet/sfc/efx.c +++ b/drivers/net/ethernet/sfc/efx.c @@ -33,6 +33,7 @@ #include "selftest.h" #include "sriov.h" #include "efx_devlink.h" +#include "efx_cxl.h" #include "mcdi_port_common.h" #include "mcdi_pcol.h" @@ -899,6 +900,9 @@ static void efx_pci_remove(struct pci_dev *pci_dev) efx_pci_remove_main(efx); efx_fini_io(efx); + + efx_cxl_exit(efx); + pci_dbg(efx->pci_dev, "shutdown successful\n"); efx_fini_devlink_and_unlock(efx); @@ -1109,6 +1113,15 @@ static int efx_pci_probe(struct pci_dev *pci_dev, if (rc) goto fail2; + /* A successful cxl initialization implies a CXL region created to be + * used for PIO buffers. If there is no CXL support, or initialization + * fails, efx_cxl_pio_initialised wll be false and legacy PIO buffers + * defined at specific PCI BAR regions will be used. + */ + rc = efx_cxl_init(efx); + if (rc) + pci_err(pci_dev, "CXL initialization failed with error %d\n", rc); + rc = efx_pci_probe_post_io(efx); if (rc) { /* On failure, retry once immediately. diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c new file mode 100644 index 000000000000..bba36cbbab22 --- /dev/null +++ b/drivers/net/ethernet/sfc/efx_cxl.c @@ -0,0 +1,86 @@ +// SPDX-License-Identifier: GPL-2.0-only +/**************************************************************************** + * + * Driver for AMD network controllers and boards + * Copyright (C) 2024, Advanced Micro Devices, Inc. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 as published + * by the Free Software Foundation, incorporated herein by reference. + */ + +#include <linux/cxl/cxl.h> +#include <linux/cxl/pci.h> +#include <linux/pci.h> + +#include "net_driver.h" +#include "efx_cxl.h" + +#define EFX_CTPIO_BUFFER_SIZE (1024 * 1024 * 256) + +int efx_cxl_init(struct efx_nic *efx) +{ + struct pci_dev *pci_dev = efx->pci_dev; + struct efx_cxl *cxl; + struct resource res; + u16 dvsec; + int rc; + + efx->efx_cxl_pio_initialised = false; + + dvsec = pci_find_dvsec_capability(pci_dev, PCI_VENDOR_ID_CXL, + CXL_DVSEC_PCIE_DEVICE); + + if (!dvsec) + return 0; + + pci_dbg(pci_dev, "CXL_DVSEC_PCIE_DEVICE capability found\n"); + + efx->cxl = kzalloc(sizeof(*cxl), GFP_KERNEL); + if (!efx->cxl) + return -ENOMEM; + + cxl = efx->cxl; + + cxl->cxlds = cxl_accel_state_create(&pci_dev->dev); + if (IS_ERR(cxl->cxlds)) { + pci_err(pci_dev, "CXL accel device state failed"); + kfree(efx->cxl); + return -ENOMEM; + } + + cxl_set_dvsec(cxl->cxlds, dvsec); + cxl_set_serial(cxl->cxlds, pci_dev->dev.id); + + res = DEFINE_RES_MEM(0, EFX_CTPIO_BUFFER_SIZE); + if (cxl_set_resource(cxl->cxlds, res, CXL_ACCEL_RES_DPA)) { + pci_err(pci_dev, "cxl_set_resource DPA failed\n"); + rc = -EINVAL; + goto err; + } + + res = DEFINE_RES_MEM_NAMED(0, EFX_CTPIO_BUFFER_SIZE, "ram"); + if (cxl_set_resource(cxl->cxlds, res, CXL_ACCEL_RES_RAM)) { + pci_err(pci_dev, "cxl_set_resource RAM failed\n"); + rc = -EINVAL; + goto err; + } + + return 0; +err: + kfree(cxl->cxlds); + kfree(cxl); + efx->cxl = NULL; + + return rc; +} + +void efx_cxl_exit(struct efx_nic *efx) +{ + if (efx->cxl) { + kfree(efx->cxl->cxlds); + kfree(efx->cxl); + } +} + +MODULE_IMPORT_NS(CXL); diff --git a/drivers/net/ethernet/sfc/efx_cxl.h b/drivers/net/ethernet/sfc/efx_cxl.h new file mode 100644 index 000000000000..f57fb2afd124 --- /dev/null +++ b/drivers/net/ethernet/sfc/efx_cxl.h @@ -0,0 +1,29 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/**************************************************************************** + * Driver for AMD network controllers and boards + * Copyright (C) 2024, Advanced Micro Devices, Inc. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 as published + * by the Free Software Foundation, incorporated herein by reference. + */ + +#ifndef EFX_CXL_H +#define EFX_CXL_H + +struct efx_nic; +struct cxl_dev_state; + +struct efx_cxl { + struct cxl_dev_state *cxlds; + struct cxl_memdev *cxlmd; + struct cxl_root_decoder *cxlrd; + struct cxl_port *endpoint; + struct cxl_endpoint_decoder *cxled; + struct cxl_region *efx_region; + void __iomem *ctpio_cxl; +}; + +int efx_cxl_init(struct efx_nic *efx); +void efx_cxl_exit(struct efx_nic *efx); +#endif diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h index b85c51cbe7f9..77261de65e63 100644 --- a/drivers/net/ethernet/sfc/net_driver.h +++ b/drivers/net/ethernet/sfc/net_driver.h @@ -817,6 +817,8 @@ enum efx_xdp_tx_queues_mode { struct efx_mae; +struct efx_cxl; + /** * struct efx_nic - an Efx NIC * @name: Device name (net device name or bus id before net device registered) @@ -963,6 +965,8 @@ struct efx_mae; * @tc: state for TC offload (EF100). * @devlink: reference to devlink structure owned by this device * @dl_port: devlink port associated with the PF + * @cxl: details of related cxl objects + * @efx_cxl_pio_initialised: clx initialization outcome. * @mem_bar: The BAR that is mapped into membase. * @reg_base: Offset from the start of the bar to the function control window. * @monitor_work: Hardware monitor workitem @@ -1148,6 +1152,8 @@ struct efx_nic { struct devlink *devlink; struct devlink_port *dl_port; + struct efx_cxl *cxl; + bool efx_cxl_pio_initialised; unsigned int mem_bar; u32 reg_base; diff --git a/include/linux/cxl/cxl.h b/include/linux/cxl/cxl.h new file mode 100644 index 000000000000..e78eefa82123 --- /dev/null +++ b/include/linux/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/device.h> + +enum cxl_resource { + CXL_ACCEL_RES_DPA, + CXL_ACCEL_RES_RAM, + CXL_ACCEL_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/linux/cxl/pci.h b/include/linux/cxl/pci.h new file mode 100644 index 000000000000..c337ae8797e6 --- /dev/null +++ b/include/linux/cxl/pci.h @@ -0,0 +1,23 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* Copyright(c) 2024 Advanced Micro Devices, Inc. */ + +#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