| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <2c217426-f1fa-9ee1-7c27-d1c1d4fec0f2@amd.com>
Date: Tue, 3 Dec 2024 15:30:34 +0000
From: Alejandro Lucero Palau <alucerop@....com>
To: alejandro.lucero-palau@....com, linux-cxl@...r.kernel.org,
netdev@...r.kernel.org, dan.j.williams@...el.com, martin.habets@...inx.com,
edward.cree@....com, davem@...emloft.net, kuba@...nel.org,
pabeni@...hat.com, edumazet@...gle.com, dave.jiang@...el.com
Subject: Re: [PATCH v6 28/28] sfc: support pio mapping based on cxl
On 12/3/24 14:52, Martin Habets wrote:
> Hi Alejandro,
>
> On Mon, Dec 02, 2024 at 05:12:22PM +0000, alejandro.lucero-palau@....com wrote:
>> From: Alejandro Lucero <alucerop@....com>
>>
>> With a device supporting CXL and successfully initialised, use the cxl
>> region to map the memory range and use this mapping for PIO buffers.
>>
>> Signed-off-by: Alejandro Lucero <alucerop@....com>
>> ---
>> drivers/net/ethernet/sfc/ef10.c | 49 +++++++++++++++++++++++----
>> drivers/net/ethernet/sfc/efx_cxl.c | 19 ++++++++++-
>> drivers/net/ethernet/sfc/net_driver.h | 2 ++
>> drivers/net/ethernet/sfc/nic.h | 3 ++
>> 4 files changed, 66 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/sfc/ef10.c b/drivers/net/ethernet/sfc/ef10.c
>> index 452009ed7a43..f2aeffc323c6 100644
>> --- a/drivers/net/ethernet/sfc/ef10.c
>> +++ b/drivers/net/ethernet/sfc/ef10.c
>> @@ -24,6 +24,7 @@
>> #include <linux/wait.h>
>> #include <linux/workqueue.h>
>> #include <net/udp_tunnel.h>
>> +#include "efx_cxl.h"
>>
>> /* Hardware control for EF10 architecture including 'Huntington'. */
>>
>> @@ -177,6 +178,12 @@ static int efx_ef10_init_datapath_caps(struct efx_nic *efx)
>> efx->num_mac_stats);
>> }
>>
>> + if (outlen < MC_CMD_GET_CAPABILITIES_V7_OUT_LEN)
>> + nic_data->datapath_caps3 = 0;
>> + else
>> + nic_data->datapath_caps3 = MCDI_DWORD(outbuf,
>> + GET_CAPABILITIES_V7_OUT_FLAGS3);
>> +
>> return 0;
>> }
>>
>> @@ -919,6 +926,9 @@ static void efx_ef10_forget_old_piobufs(struct efx_nic *efx)
>> static void efx_ef10_remove(struct efx_nic *efx)
>> {
>> struct efx_ef10_nic_data *nic_data = efx->nic_data;
>> +#ifdef CONFIG_SFC_CXL
>> + struct efx_probe_data *probe_data;
>> +#endif
>> int rc;
>>
>> #ifdef CONFIG_SFC_SRIOV
>> @@ -949,7 +959,12 @@ static void efx_ef10_remove(struct efx_nic *efx)
>>
>> efx_mcdi_rx_free_indir_table(efx);
>>
>> +#ifdef CONFIG_SFC_CXL
>> + probe_data = container_of(efx, struct efx_probe_data, efx);
>> + if (nic_data->wc_membase && !probe_data->cxl_pio_in_use)
>> +#else
>> if (nic_data->wc_membase)
>> +#endif
>> iounmap(nic_data->wc_membase);
>>
>> rc = efx_mcdi_free_vis(efx);
>> @@ -1140,6 +1155,9 @@ static int efx_ef10_dimension_resources(struct efx_nic *efx)
>> unsigned int channel_vis, pio_write_vi_base, max_vis;
>> struct efx_ef10_nic_data *nic_data = efx->nic_data;
>> unsigned int uc_mem_map_size, wc_mem_map_size;
>> +#ifdef CONFIG_SFC_CXL
>> + struct efx_probe_data *probe_data;
>> +#endif
>> void __iomem *membase;
>> int rc;
>>
>> @@ -1263,8 +1281,27 @@ static int efx_ef10_dimension_resources(struct efx_nic *efx)
>> iounmap(efx->membase);
>> efx->membase = membase;
>>
>> - /* Set up the WC mapping if needed */
>> - if (wc_mem_map_size) {
>> + if (!wc_mem_map_size) {
>> + netif_dbg(efx, probe, efx->net_dev, "wc_mem_map_size is 0\n");
> Please still print the details of the memory BAR that the netif_dbg has below.
> It is useful for debugging.
>
I guess just a goto there setting rc and removing this debug comment here.
It makes sense. I'll change it.
>> + return 0;
>> + }
>> +
>> + /* Set up the WC mapping */
>> +
>> +#ifdef CONFIG_SFC_CXL
>> + probe_data = container_of(efx, struct efx_probe_data, efx);
>> + if ((nic_data->datapath_caps3 &
>> + (1 << MC_CMD_GET_CAPABILITIES_V7_OUT_CXL_CONFIG_ENABLE_LBN)) &&
>> + probe_data->cxl_pio_initialised) {
>> + /* Using PIO through CXL mapping? */
>> + nic_data->pio_write_base = probe_data->cxl->ctpio_cxl +
>> + (pio_write_vi_base * efx->vi_stride +
>> + ER_DZ_TX_PIOBUF - uc_mem_map_size);
>> + probe_data->cxl_pio_in_use = true;
>> + } else
>> +#endif
>> + {
>> + /* Using legacy PIO BAR mapping */
>> nic_data->wc_membase = ioremap_wc(efx->membase_phys +
>> uc_mem_map_size,
>> wc_mem_map_size);
>> @@ -1279,12 +1316,12 @@ static int efx_ef10_dimension_resources(struct efx_nic *efx)
>> nic_data->wc_membase +
>> (pio_write_vi_base * efx->vi_stride + ER_DZ_TX_PIOBUF -
>> uc_mem_map_size);
>> -
>> - rc = efx_ef10_link_piobufs(efx);
>> - if (rc)
>> - efx_ef10_free_piobufs(efx);
>> }
>>
>> + rc = efx_ef10_link_piobufs(efx);
>> + if (rc)
>> + efx_ef10_free_piobufs(efx);
>> +
>> netif_dbg(efx, probe, efx->net_dev,
>> "memory BAR at %pa (virtual %p+%x UC, %p+%x WC)\n",
>> &efx->membase_phys, efx->membase, uc_mem_map_size,
>> diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c
>> index 71b32fc48ca7..78eb8aa9702a 100644
>> --- a/drivers/net/ethernet/sfc/efx_cxl.c
>> +++ b/drivers/net/ethernet/sfc/efx_cxl.c
>> @@ -24,9 +24,10 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
>> DECLARE_BITMAP(expected, CXL_MAX_CAPS);
>> DECLARE_BITMAP(found, CXL_MAX_CAPS);
>> struct pci_dev *pci_dev;
>> + resource_size_t max;
>> struct efx_cxl *cxl;
>> struct resource res;
>> - resource_size_t max;
>> + struct range range;
>> u16 dvsec;
>> int rc;
>>
>> @@ -135,10 +136,25 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
>> goto err_region;
>> }
>>
>> + rc = cxl_get_region_range(cxl->efx_region, &range);
>> + if (rc) {
>> + pci_err(pci_dev, "CXL getting regions params failed");
>> + goto err_region_params;
>> + }
>> +
>> + cxl->ctpio_cxl = ioremap(range.start, range.end - range.start);
>> + if (!cxl->ctpio_cxl) {
>> + pci_err(pci_dev, "CXL ioremap region failed");
> This error will be more useful if you print out the start & size. Users can
> the check that against /proc/iomem.
I'll do.
>> + goto err_region_params;
>> + }
>> +
>> probe_data->cxl = cxl;
>> + probe_data->cxl_pio_initialised = true;
>>
>> return 0;
>>
>> +err_region_params:
>> + cxl_accel_region_detach(cxl->cxled);
>> err_region:
>> cxl_dpa_free(cxl->cxled);
>> err3:
>> @@ -153,6 +169,7 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
>> void efx_cxl_exit(struct efx_probe_data *probe_data)
>> {
>> if (probe_data->cxl) {
>> + iounmap(probe_data->cxl->ctpio_cxl);
>> cxl_accel_region_detach(probe_data->cxl->cxled);
>> cxl_dpa_free(probe_data->cxl->cxled);
>> cxl_release_resource(probe_data->cxl->cxlds, CXL_RES_RAM);
>> diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
>> index 7f11ff200c25..79b0e6663f23 100644
>> --- a/drivers/net/ethernet/sfc/net_driver.h
>> +++ b/drivers/net/ethernet/sfc/net_driver.h
>> @@ -1209,6 +1209,7 @@ struct efx_cxl;
>> * @efx: Efx NIC details
>> * @cxl: details of related cxl objects
>> * @cxl_pio_initialised: cxl initialization outcome.
>> ++ * @cxl_pio_in_use: PIO using CXL mapping
> Extra + sign here isn't right. Please build kdoc, I expect it would have caught this.
OK. I'll fix it.
Thanks!
> Martin
>
>> */
>> struct efx_probe_data {
>> struct pci_dev *pci_dev;
>> @@ -1216,6 +1217,7 @@ struct efx_probe_data {
>> #ifdef CONFIG_SFC_CXL
>> struct efx_cxl *cxl;
>> bool cxl_pio_initialised;
>> + bool cxl_pio_in_use;
>> #endif
>> };
>>
>> diff --git a/drivers/net/ethernet/sfc/nic.h b/drivers/net/ethernet/sfc/nic.h
>> index 9fa5c4c713ab..c87cc9214690 100644
>> --- a/drivers/net/ethernet/sfc/nic.h
>> +++ b/drivers/net/ethernet/sfc/nic.h
>> @@ -152,6 +152,8 @@ enum {
>> * %MC_CMD_GET_CAPABILITIES response)
>> * @datapath_caps2: Further Capabilities of datapath firmware (FLAGS2 field of
>> * %MC_CMD_GET_CAPABILITIES response)
>> + * @datapath_caps3: Further Capabilities of datapath firmware (FLAGS3 field of
>> + * %MC_CMD_GET_CAPABILITIES response)
>> * @rx_dpcpu_fw_id: Firmware ID of the RxDPCPU
>> * @tx_dpcpu_fw_id: Firmware ID of the TxDPCPU
>> * @must_probe_vswitching: Flag: vswitching has yet to be setup after MC reboot
>> @@ -186,6 +188,7 @@ struct efx_ef10_nic_data {
>> bool must_check_datapath_caps;
>> u32 datapath_caps;
>> u32 datapath_caps2;
>> + u32 datapath_caps3;
>> unsigned int rx_dpcpu_fw_id;
>> unsigned int tx_dpcpu_fw_id;
>> bool must_probe_vswitching;
>> --
>> 2.17.1
>>
>>
Powered by blists - more mailing lists