netdev - Re: [PATCH v18 20/20] sfc: support pio mapping based on cxl

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250930155105.00001463@huawei.com>
Date: Tue, 30 Sep 2025 15:51:05 +0100
From: Jonathan Cameron <jonathan.cameron@...wei.com>
To: Alejandro Lucero Palau <alucerop@....com>
CC: <alejandro.lucero-palau@....com>, <linux-cxl@...r.kernel.org>,
	<netdev@...r.kernel.org>, <dan.j.williams@...el.com>, <edward.cree@....com>,
	<davem@...emloft.net>, <kuba@...nel.org>, <pabeni@...hat.com>,
	<edumazet@...gle.com>, <dave.jiang@...el.com>
Subject: Re: [PATCH v18 20/20] sfc: support pio mapping based on cxl

On Fri, 26 Sep 2025 10:47:27 +0100
Alejandro Lucero Palau <alucerop@....com> wrote:

> On 9/18/25 16:08, Jonathan Cameron wrote:
> > A few trivial things inline.
> >  
> >> diff --git a/drivers/net/ethernet/sfc/ef10.c b/drivers/net/ethernet/sfc/ef10.c
> >> index 47349c148c0c..7bc854e2d22a 100644
> >> --- a/drivers/net/ethernet/sfc/ef10.c
> >> +++ b/drivers/net/ethernet/sfc/ef10.c
> >> diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c
> >> index 85490afc7930..3dde59003cd9 100644
> >> --- a/drivers/net/ethernet/sfc/efx_cxl.c
> >> +++ b/drivers/net/ethernet/sfc/efx_cxl.c
> >> @@ -11,16 +11,23 @@
> >>   
> >>   #include "net_driver.h"
> >>   #include "efx_cxl.h"
> >> +#include "efx.h"
> >>   
> >>   #define EFX_CTPIO_BUFFER_SIZE	SZ_256M
> >>   
> >>   static void efx_release_cxl_region(void *priv_cxl)
> >>   {
> >>   	struct efx_probe_data *probe_data = priv_cxl;
> >> +	struct efx_nic *efx = &probe_data->efx;
> >>   	struct efx_cxl *cxl = probe_data->cxl;
> >>   
> >> +	/* Next avoid contention with efx_cxl_exit() */
> >>   	probe_data->cxl_pio_initialised = false;
> >> +
> >> +	/* Next makes cxl-based piobus to no be used */
> >> +	efx_ef10_disable_piobufs(efx);
> >>   	iounmap(cxl->ctpio_cxl);
> >> +  
> > Avoid extra white space changes. Perhaps push to earlier patch.  
> 
> 
> I'll fix the spaces. Not sure what you mean with the second part of your 
> comment, but if I understand it right, I think those changes should be 
> added in this patch, just when the final functionality is added.

Just the white space.  If you want that move it to where that iounmap() is
added.  This is just a patch cleanliness thing.

> 
> 
> FWIW, I have decided to drop this driver callback as Dan did not like 
> it, and after realizing those Dan's patches this patchset relies on fix 
> most of the problem this callback tried to address.
> 
> 
> >>   	cxl_put_root_decoder(cxl->cxlrd);
> >>   }
> >>   
> >> @@ -30,6 +37,7 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
> >>   	struct pci_dev *pci_dev = efx->pci_dev;
> >>   	resource_size_t max_size;
> >>   	struct efx_cxl *cxl;
> >> +	struct range range;
> >>   	u16 dvsec;
> >>   	int rc;
> >>   
> >> @@ -133,17 +141,34 @@ int efx_cxl_init(struct efx_probe_data *probe_data)
> >>   					    &probe_data);
> >>   	if (IS_ERR(cxl->efx_region)) {
> >>   		pci_err(pci_dev, "CXL accel create region failed");
> >> -		cxl_dpa_free(cxl->cxled);
> >>   		rc = PTR_ERR(cxl->efx_region);
> >> -		goto err_decoder;
> >> +		goto err_dpa;  
> > Why do we now need to call cxl_dpa_free() and didn't previously here? That
> > seems like a probably bug in earlier patch.  
> 
> 
> I think you misread it. We were calling cxl_dpa_free already, just 
> moving it to a goto label here.
> 
Indeed.  Needed more coffee that day (or less, can't remember which ;)