| Message ID | 20241017165225.21206-3-alejandro.lucero-palau@amd.com |
|---|---|
| State | New |
| Headers | show |
| Series | cxl: add Type2 device support | expand |
Hi Alejandro, Thanks for sending this out, comments inline (for this patch and more). On 10/17/24 11:52 AM, alejandro.lucero-palau@amd.com wrote: > From: Alejandro Lucero <alucerop@amd.com> > > Add CXL initialization based on new CXL API for accel drivers and make > it dependable on kernel CXL configuration. > > Signed-off-by: Alejandro Lucero <alucerop@amd.com> > --- > drivers/net/ethernet/sfc/Kconfig | 1 + > drivers/net/ethernet/sfc/Makefile | 2 +- > drivers/net/ethernet/sfc/efx.c | 16 +++++ > drivers/net/ethernet/sfc/efx_cxl.c | 92 +++++++++++++++++++++++++++ > drivers/net/ethernet/sfc/efx_cxl.h | 29 +++++++++ > drivers/net/ethernet/sfc/net_driver.h | 6 ++ > 6 files changed, 145 insertions(+), 1 deletion(-) > create mode 100644 drivers/net/ethernet/sfc/efx_cxl.c > create mode 100644 drivers/net/ethernet/sfc/efx_cxl.h > > diff --git a/drivers/net/ethernet/sfc/Kconfig b/drivers/net/ethernet/sfc/Kconfig > index 3eb55dcfa8a6..b308a6f674b2 100644 > --- a/drivers/net/ethernet/sfc/Kconfig > +++ b/drivers/net/ethernet/sfc/Kconfig > @@ -20,6 +20,7 @@ config SFC > tristate "Solarflare SFC9100/EF100-family support" > depends on PCI > depends on PTP_1588_CLOCK_OPTIONAL > + depends on CXL_BUS && CXL_BUS=m && m It seems weird to me that this would be marked as a tristate Kconfig option, but is required to be set to 'm'. Also, I'm assuming that SFC cards exist without CXL support, so this would add an unecessary dependency for those cards. So, I'm going to suggest using a secondary Kconfig symbol like this: config SFC_CXL tristate "Colarflare SFC9100/EF100-family CXL support" depends on SFC && m depends on CXL_BUS=m help CXL support for SFC driver... And then only compiling efx_cxl.c when that symbol is selected. This would also require having a stub for efx_cxl_init()/exit() in efx_cxl.h. That *should* have the same behavior as what you want above, but without requiring CXL to enable the base SFC driver. I'm no Kconfig wizard, so it would pay to double check the above, but I don't see a reason why something like it shouldn't be possible. > select MDIO > select CRC32 > select NET_DEVLINK > 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 With above suggestion this becomes: + sfc-$(CONFIG_SFC_CXL) += 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..cc7cdaccc5ed 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. > @@ -1380,3 +1393,6 @@ MODULE_AUTHOR("Solarflare Communications and " > MODULE_DESCRIPTION("Solarflare network driver"); > MODULE_LICENSE("GPL"); > MODULE_DEVICE_TABLE(pci, efx_pci_table); > +#if IS_ENABLED(CONFIG_CXL_BUS) > +MODULE_SOFTDEP("pre: cxl_core cxl_port cxl_acpi cxl-mem"); > +#endif > diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c > new file mode 100644 > index 000000000000..fb3eef339b34 > --- /dev/null > +++ b/drivers/net/ethernet/sfc/efx_cxl.c > @@ -0,0 +1,92 @@ > +// 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 SZ_256M > + > +int efx_cxl_init(struct efx_nic *efx) > +{ > +#if IS_ENABLED(CONFIG_CXL_BUS) With suggestion above you can drop this #if, since the file won't be compiled when this is false. > + 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"); > + > + cxl = kzalloc(sizeof(*cxl), GFP_KERNEL); > + if (!cxl) > + return -ENOMEM; > + > + cxl->cxlds = cxl_accel_state_create(&pci_dev->dev); > + if (IS_ERR(cxl->cxlds)) { > + pci_err(pci_dev, "CXL accel device state failed"); > + rc = -ENOMEM; > + goto err1; > + } > + > + 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_RES_DPA)) { > + pci_err(pci_dev, "cxl_set_resource DPA failed\n"); > + rc = -EINVAL; > + goto err2; > + } > + > + res = DEFINE_RES_MEM_NAMED(0, EFX_CTPIO_BUFFER_SIZE, "ram"); > + if (cxl_set_resource(cxl->cxlds, res, CXL_RES_RAM)) { > + pci_err(pci_dev, "cxl_set_resource RAM failed\n"); > + rc = -EINVAL; > + goto err2; > + } > + > + efx->cxl = cxl; > +#endif > + > + return 0; > + > +#if IS_ENABLED(CONFIG_CXL_BUS) Same here... > +err2: > + kfree(cxl->cxlds); > +err1: > + kfree(cxl); > + return rc; > + > +#endif > +} > + > +void efx_cxl_exit(struct efx_nic *efx) > +{ > +#if IS_ENABLED(CONFIG_CXL_BUS) and here. > + if (efx->cxl) { > + kfree(efx->cxl->cxlds); > + kfree(efx->cxl); > + } > +#endif > +} > + > +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); As mentioned above, you would need a #ifdef block here with stubs for when CONFIG_SFC_CXL isn't enabled. > +#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; >
On 10/17/24 22:48, Ben Cheatham wrote: > Hi Alejandro, > > Thanks for sending this out, comments inline (for this patch and more). > > On 10/17/24 11:52 AM, alejandro.lucero-palau@amd.com wrote: >> From: Alejandro Lucero <alucerop@amd.com> >> >> Add CXL initialization based on new CXL API for accel drivers and make >> it dependable on kernel CXL configuration. >> >> Signed-off-by: Alejandro Lucero <alucerop@amd.com> >> --- >> drivers/net/ethernet/sfc/Kconfig | 1 + >> drivers/net/ethernet/sfc/Makefile | 2 +- >> drivers/net/ethernet/sfc/efx.c | 16 +++++ >> drivers/net/ethernet/sfc/efx_cxl.c | 92 +++++++++++++++++++++++++++ >> drivers/net/ethernet/sfc/efx_cxl.h | 29 +++++++++ >> drivers/net/ethernet/sfc/net_driver.h | 6 ++ >> 6 files changed, 145 insertions(+), 1 deletion(-) >> create mode 100644 drivers/net/ethernet/sfc/efx_cxl.c >> create mode 100644 drivers/net/ethernet/sfc/efx_cxl.h >> >> diff --git a/drivers/net/ethernet/sfc/Kconfig b/drivers/net/ethernet/sfc/Kconfig >> index 3eb55dcfa8a6..b308a6f674b2 100644 >> --- a/drivers/net/ethernet/sfc/Kconfig >> +++ b/drivers/net/ethernet/sfc/Kconfig >> @@ -20,6 +20,7 @@ config SFC >> tristate "Solarflare SFC9100/EF100-family support" >> depends on PCI >> depends on PTP_1588_CLOCK_OPTIONAL >> + depends on CXL_BUS && CXL_BUS=m && m > It seems weird to me that this would be marked as a tristate Kconfig option, but is > required to be set to 'm'. Also, I'm assuming that SFC cards exist without CXL support, > so this would add an unecessary dependency for those cards. So, I'm going to suggest > using a secondary Kconfig symbol like this: Yes, you are right. My idea was to force sfc as a module if cxl_bus was a module. I tested that case, the cxl_bus within the kernel image and sfc as module, and both cxl_bus and sfc part of the kernel image. And I forgot the case of no cxl_bus! So, I've already followed your suggestion, not exactly, but I think the idea remains. Now there's a cxl option only appearing and therefore configurable if the cxl_bus is a module. Inside sfc we have already another option, MTD, which is a similar case and requires kernel mtd as a module, so I think it is good enough. I'll do a bit more of testing and do the changes for v5. Thanks! > config SFC_CXL > tristate "Colarflare SFC9100/EF100-family CXL support" > depends on SFC && m > depends on CXL_BUS=m > help > CXL support for SFC driver... > > And then only compiling efx_cxl.c when that symbol is selected. This would also > require having a stub for efx_cxl_init()/exit() in efx_cxl.h. That *should* have > the same behavior as what you want above, but without requiring CXL to enable the > base SFC driver. I'm no Kconfig wizard, so it would pay to double check the above, > but I don't see a reason why something like it shouldn't be possible. > >> select MDIO >> select CRC32 >> select NET_DEVLINK >> 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 > With above suggestion this becomes: > > + sfc-$(CONFIG_SFC_CXL) += 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..cc7cdaccc5ed 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. >> @@ -1380,3 +1393,6 @@ MODULE_AUTHOR("Solarflare Communications and " >> MODULE_DESCRIPTION("Solarflare network driver"); >> MODULE_LICENSE("GPL"); >> MODULE_DEVICE_TABLE(pci, efx_pci_table); >> +#if IS_ENABLED(CONFIG_CXL_BUS) >> +MODULE_SOFTDEP("pre: cxl_core cxl_port cxl_acpi cxl-mem"); >> +#endif >> diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c >> new file mode 100644 >> index 000000000000..fb3eef339b34 >> --- /dev/null >> +++ b/drivers/net/ethernet/sfc/efx_cxl.c >> @@ -0,0 +1,92 @@ >> +// 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 SZ_256M >> + >> +int efx_cxl_init(struct efx_nic *efx) >> +{ >> +#if IS_ENABLED(CONFIG_CXL_BUS) > With suggestion above you can drop this #if, since the file won't be > compiled when this is false. > >> + 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"); >> + >> + cxl = kzalloc(sizeof(*cxl), GFP_KERNEL); >> + if (!cxl) >> + return -ENOMEM; >> + >> + cxl->cxlds = cxl_accel_state_create(&pci_dev->dev); >> + if (IS_ERR(cxl->cxlds)) { >> + pci_err(pci_dev, "CXL accel device state failed"); >> + rc = -ENOMEM; >> + goto err1; >> + } >> + >> + 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_RES_DPA)) { >> + pci_err(pci_dev, "cxl_set_resource DPA failed\n"); >> + rc = -EINVAL; >> + goto err2; >> + } >> + >> + res = DEFINE_RES_MEM_NAMED(0, EFX_CTPIO_BUFFER_SIZE, "ram"); >> + if (cxl_set_resource(cxl->cxlds, res, CXL_RES_RAM)) { >> + pci_err(pci_dev, "cxl_set_resource RAM failed\n"); >> + rc = -EINVAL; >> + goto err2; >> + } >> + >> + efx->cxl = cxl; >> +#endif >> + >> + return 0; >> + >> +#if IS_ENABLED(CONFIG_CXL_BUS) > Same here... > >> +err2: >> + kfree(cxl->cxlds); >> +err1: >> + kfree(cxl); >> + return rc; >> + >> +#endif >> +} >> + >> +void efx_cxl_exit(struct efx_nic *efx) >> +{ >> +#if IS_ENABLED(CONFIG_CXL_BUS) > and here. > >> + if (efx->cxl) { >> + kfree(efx->cxl->cxlds); >> + kfree(efx->cxl); >> + } >> +#endif >> +} >> + >> +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); > As mentioned above, you would need a #ifdef block here with stubs for when CONFIG_SFC_CXL isn't enabled. > >> +#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; >>
On Thu, 17 Oct 2024 17:52:01 +0100 <alejandro.lucero-palau@amd.com> wrote: > From: Alejandro Lucero <alucerop@amd.com> > > Add CXL initialization based on new CXL API for accel drivers and make > it dependable on kernel CXL configuration. > > Signed-off-by: Alejandro Lucero <alucerop@amd.com> > --- > drivers/net/ethernet/sfc/Kconfig | 1 + > drivers/net/ethernet/sfc/Makefile | 2 +- > drivers/net/ethernet/sfc/efx.c | 16 +++++ > drivers/net/ethernet/sfc/efx_cxl.c | 92 +++++++++++++++++++++++++++ > drivers/net/ethernet/sfc/efx_cxl.h | 29 +++++++++ > drivers/net/ethernet/sfc/net_driver.h | 6 ++ > 6 files changed, 145 insertions(+), 1 deletion(-) > create mode 100644 drivers/net/ethernet/sfc/efx_cxl.c > create mode 100644 drivers/net/ethernet/sfc/efx_cxl.h > > diff --git a/drivers/net/ethernet/sfc/Kconfig b/drivers/net/ethernet/sfc/Kconfig > index 3eb55dcfa8a6..b308a6f674b2 100644 > --- a/drivers/net/ethernet/sfc/Kconfig > +++ b/drivers/net/ethernet/sfc/Kconfig > @@ -20,6 +20,7 @@ config SFC > tristate "Solarflare SFC9100/EF100-family support" > depends on PCI > depends on PTP_1588_CLOCK_OPTIONAL > + depends on CXL_BUS && CXL_BUS=m && m Do some makefile magic and stubs to only support efx_cxl.c being built at all if necessary conditions met. Doesn't necessarily need a visible control. config SFC9100_CXL boolean then here have select SFC9100_CXL if CXL_BUS > select MDIO > select CRC32 > select NET_DEVLINK > diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c > new file mode 100644 > index 000000000000..fb3eef339b34 > --- /dev/null > +++ b/drivers/net/ethernet/sfc/efx_cxl.c > @@ -0,0 +1,92 @@ > +// 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 SZ_256M > + > +int efx_cxl_init(struct efx_nic *efx) > +{ > +#if IS_ENABLED(CONFIG_CXL_BUS) If it can't do anything useful, make the build depend on this and provide stubs for when it isn't enabled. I'd not expect to see any ifdef stuff for basic CXL in this file as it should build unless they are all configured. > + 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"); > + > + cxl = kzalloc(sizeof(*cxl), GFP_KERNEL); __free magic here. Assuming later changes don't make that a bad idea - I've not read the whole set for a while. > + if (!cxl) > + return -ENOMEM; > + > + cxl->cxlds = cxl_accel_state_create(&pci_dev->dev); Stash this in a local cxl_dev_state for now and use __free() for that. > + if (IS_ERR(cxl->cxlds)) { > + pci_err(pci_dev, "CXL accel device state failed"); > + rc = -ENOMEM; > + goto err1; > + } > + > + 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_RES_DPA)) { > + pci_err(pci_dev, "cxl_set_resource DPA failed\n"); > + rc = -EINVAL; > + goto err2; > + } > + > + res = DEFINE_RES_MEM_NAMED(0, EFX_CTPIO_BUFFER_SIZE, "ram"); > + if (cxl_set_resource(cxl->cxlds, res, CXL_RES_RAM)) { > + pci_err(pci_dev, "cxl_set_resource RAM failed\n"); > + rc = -EINVAL; > + goto err2; > + } > + cxl->cxlds = no_free_ptr(cxlds); efx->cxl = no_free_ptr(cxl); > + efx->cxl = cxl; > +#endif > + > + return 0; > + > +#if IS_ENABLED(CONFIG_CXL_BUS) With __free changes suggest above, no need to do anything here and can return directly from the error checks above. > +err2: > + kfree(cxl->cxlds); > +err1: > + kfree(cxl); > + return rc; > + > +#endif > +} > + > +void efx_cxl_exit(struct efx_nic *efx) > +{ > +#if IS_ENABLED(CONFIG_CXL_BUS) IS_REACHABLE() maybe, so if this driver is built in but CXL_BUS is not then this will go away. > + if (efx->cxl) { > + kfree(efx->cxl->cxlds); > + kfree(efx->cxl); > + } > +#endif > +} > + > +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 cxl_memdev; struct cxl_root_decoder; struct cxl_port; ... > + > +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. cxl Also, it's in a struct called efx_nic, so is the efx_ prefix useful? > * @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; >
On 10/25/24 15:03, Jonathan Cameron wrote: > On Thu, 17 Oct 2024 17:52:01 +0100 > <alejandro.lucero-palau@amd.com> wrote: > >> From: Alejandro Lucero <alucerop@amd.com> >> >> Add CXL initialization based on new CXL API for accel drivers and make >> it dependable on kernel CXL configuration. >> >> Signed-off-by: Alejandro Lucero <alucerop@amd.com> >> --- >> drivers/net/ethernet/sfc/Kconfig | 1 + >> drivers/net/ethernet/sfc/Makefile | 2 +- >> drivers/net/ethernet/sfc/efx.c | 16 +++++ >> drivers/net/ethernet/sfc/efx_cxl.c | 92 +++++++++++++++++++++++++++ >> drivers/net/ethernet/sfc/efx_cxl.h | 29 +++++++++ >> drivers/net/ethernet/sfc/net_driver.h | 6 ++ >> 6 files changed, 145 insertions(+), 1 deletion(-) >> create mode 100644 drivers/net/ethernet/sfc/efx_cxl.c >> create mode 100644 drivers/net/ethernet/sfc/efx_cxl.h >> >> diff --git a/drivers/net/ethernet/sfc/Kconfig b/drivers/net/ethernet/sfc/Kconfig >> index 3eb55dcfa8a6..b308a6f674b2 100644 >> --- a/drivers/net/ethernet/sfc/Kconfig >> +++ b/drivers/net/ethernet/sfc/Kconfig >> @@ -20,6 +20,7 @@ config SFC >> tristate "Solarflare SFC9100/EF100-family support" >> depends on PCI >> depends on PTP_1588_CLOCK_OPTIONAL >> + depends on CXL_BUS && CXL_BUS=m && m > Do some makefile magic and stubs to only support efx_cxl.c > being built at all if necessary conditions met. > Doesn't necessarily need a visible control. Yes, I have already change this after Ben Cheatham's comments. > config SFC9100_CXL > boolean > > then here have > select SFC9100_CXL if CXL_BUS > > >> select MDIO >> select CRC32 >> select NET_DEVLINK >> diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c >> new file mode 100644 >> index 000000000000..fb3eef339b34 >> --- /dev/null >> +++ b/drivers/net/ethernet/sfc/efx_cxl.c >> @@ -0,0 +1,92 @@ >> +// 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 SZ_256M >> + >> +int efx_cxl_init(struct efx_nic *efx) >> +{ >> +#if IS_ENABLED(CONFIG_CXL_BUS) > If it can't do anything useful, make the build depend on this > and provide stubs for when it isn't enabled. > > I'd not expect to see any ifdef stuff for basic CXL in this file > as it should build unless they are all configured. Right. I got rid of them after the changes commented above. >> + 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"); >> + >> + cxl = kzalloc(sizeof(*cxl), GFP_KERNEL); > __free magic here. > Assuming later changes don't make that a bad idea - I've not > read the whole set for a while. Remember we are in netdev territory and those free magic things are not liked ... > >> + if (!cxl) >> + return -ENOMEM; >> + >> + cxl->cxlds = cxl_accel_state_create(&pci_dev->dev); > Stash this in a local cxl_dev_state for now and use __free() for that. > >> + if (IS_ERR(cxl->cxlds)) { >> + pci_err(pci_dev, "CXL accel device state failed"); >> + rc = -ENOMEM; >> + goto err1; >> + } >> + >> + 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_RES_DPA)) { >> + pci_err(pci_dev, "cxl_set_resource DPA failed\n"); >> + rc = -EINVAL; >> + goto err2; >> + } >> + >> + res = DEFINE_RES_MEM_NAMED(0, EFX_CTPIO_BUFFER_SIZE, "ram"); >> + if (cxl_set_resource(cxl->cxlds, res, CXL_RES_RAM)) { >> + pci_err(pci_dev, "cxl_set_resource RAM failed\n"); >> + rc = -EINVAL; >> + goto err2; >> + } >> + > cxl->cxlds = no_free_ptr(cxlds); > efx->cxl = no_free_ptr(cxl); > >> + efx->cxl = cxl; >> +#endif >> + >> + return 0; >> + >> +#if IS_ENABLED(CONFIG_CXL_BUS) > With __free changes suggest above, no need to do anything here > and can return directly from the error checks above. > >> +err2: >> + kfree(cxl->cxlds); >> +err1: >> + kfree(cxl); >> + return rc; >> + >> +#endif >> +} >> + >> +void efx_cxl_exit(struct efx_nic *efx) >> +{ >> +#if IS_ENABLED(CONFIG_CXL_BUS) > IS_REACHABLE() maybe, so if this driver is built in but CXL_BUS > is not then this will go away. > >> + if (efx->cxl) { >> + kfree(efx->cxl->cxlds); >> + kfree(efx->cxl); >> + } >> +#endif >> +} >> + >> +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 cxl_memdev; > struct cxl_root_decoder; > struct cxl_port; > ... > Yes. I'll do it. >> + >> +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. > cxl Well spotted. I'll fix it. > Also, it's in a struct called efx_nic, so is the efx_ prefix > useful? I do not like to have the name as the struct ... Anyways, thanks for the review. >> * @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; >>
> >> + > >> + cxl = kzalloc(sizeof(*cxl), GFP_KERNEL); > > __free magic here. > > Assuming later changes don't make that a bad idea - I've not > > read the whole set for a while. > > > Remember we are in netdev territory and those free magic things are not > liked ... I'll keep forgetting that. Feel free to ignore me when I do! > >> 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. > > cxl > > > Well spotted. I'll fix it. > > > > Also, it's in a struct called efx_nic, so is the efx_ prefix > > useful? > > > I do not like to have the name as the struct ... You've lost me. efx_nic->cxl_pio_initialised was that I was suggesting and not setting how this comment applies. J
On 10/29/24 15:14, Jonathan Cameron wrote: >>>> + >>>> + cxl = kzalloc(sizeof(*cxl), GFP_KERNEL); >>> __free magic here. >>> Assuming later changes don't make that a bad idea - I've not >>> read the whole set for a while. >> >> Remember we are in netdev territory and those free magic things are not >> liked ... > I'll keep forgetting that. Feel free to ignore me when I do! > > > >>>> 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. >>> cxl >> >> Well spotted. I'll fix it. >> >> >>> Also, it's in a struct called efx_nic, so is the efx_ prefix >>> useful? >> >> I do not like to have the name as the struct ... > You've lost me. efx_nic->cxl_pio_initialised was that I was suggesting > and not setting how this comment applies. I could try some excuses ... but it was my silly side or maybe not enough coffee. You are of course right, and the prefix is not needed. I'll fix it for v5. Thanks! > J
diff --git a/drivers/net/ethernet/sfc/Kconfig b/drivers/net/ethernet/sfc/Kconfig index 3eb55dcfa8a6..b308a6f674b2 100644 --- a/drivers/net/ethernet/sfc/Kconfig +++ b/drivers/net/ethernet/sfc/Kconfig @@ -20,6 +20,7 @@ config SFC tristate "Solarflare SFC9100/EF100-family support" depends on PCI depends on PTP_1588_CLOCK_OPTIONAL + depends on CXL_BUS && CXL_BUS=m && m select MDIO select CRC32 select NET_DEVLINK 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..cc7cdaccc5ed 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. @@ -1380,3 +1393,6 @@ MODULE_AUTHOR("Solarflare Communications and " MODULE_DESCRIPTION("Solarflare network driver"); MODULE_LICENSE("GPL"); MODULE_DEVICE_TABLE(pci, efx_pci_table); +#if IS_ENABLED(CONFIG_CXL_BUS) +MODULE_SOFTDEP("pre: cxl_core cxl_port cxl_acpi cxl-mem"); +#endif diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c new file mode 100644 index 000000000000..fb3eef339b34 --- /dev/null +++ b/drivers/net/ethernet/sfc/efx_cxl.c @@ -0,0 +1,92 @@ +// 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 SZ_256M + +int efx_cxl_init(struct efx_nic *efx) +{ +#if IS_ENABLED(CONFIG_CXL_BUS) + 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"); + + cxl = kzalloc(sizeof(*cxl), GFP_KERNEL); + if (!cxl) + return -ENOMEM; + + cxl->cxlds = cxl_accel_state_create(&pci_dev->dev); + if (IS_ERR(cxl->cxlds)) { + pci_err(pci_dev, "CXL accel device state failed"); + rc = -ENOMEM; + goto err1; + } + + 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_RES_DPA)) { + pci_err(pci_dev, "cxl_set_resource DPA failed\n"); + rc = -EINVAL; + goto err2; + } + + res = DEFINE_RES_MEM_NAMED(0, EFX_CTPIO_BUFFER_SIZE, "ram"); + if (cxl_set_resource(cxl->cxlds, res, CXL_RES_RAM)) { + pci_err(pci_dev, "cxl_set_resource RAM failed\n"); + rc = -EINVAL; + goto err2; + } + + efx->cxl = cxl; +#endif + + return 0; + +#if IS_ENABLED(CONFIG_CXL_BUS) +err2: + kfree(cxl->cxlds); +err1: + kfree(cxl); + return rc; + +#endif +} + +void efx_cxl_exit(struct efx_nic *efx) +{ +#if IS_ENABLED(CONFIG_CXL_BUS) + if (efx->cxl) { + kfree(efx->cxl->cxlds); + kfree(efx->cxl); + } +#endif +} + +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;