netdev - Re: [PATCH v2 03/15] cxl: add function for type2 resource request

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20240804181654.00007279@Huawei.com>
Date: Sun, 4 Aug 2024 18:16:54 +0100
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: Dave Jiang <dave.jiang@...el.com>
CC: <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>,
	<richard.hughes@....com>, "Alejandro Lucero" <alucerop@....com>
Subject: Re: [PATCH v2 03/15] cxl: add function for type2 resource request

On Thu, 18 Jul 2024 16:36:00 -0700
Dave Jiang <dave.jiang@...el.com> wrote:

> On 7/15/24 10:28 AM, alejandro.lucero-palau@....com wrote:
> > From: Alejandro Lucero <alucerop@....com>
> > 
> > Create a new function for a type2 device requesting a resource
> > passing the opaque struct to work with.
> > 
> > Signed-off-by: Alejandro Lucero <alucerop@....com>
> > ---
> >  drivers/cxl/core/memdev.c          | 13 +++++++++++++
> >  drivers/net/ethernet/sfc/efx_cxl.c |  7 ++++++-
> >  include/linux/cxl_accel_mem.h      |  1 +
> >  3 files changed, 20 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> > index 61b5d35b49e7..04c3a0f8bc2e 100644
> > --- a/drivers/cxl/core/memdev.c
> > +++ b/drivers/cxl/core/memdev.c
> > @@ -744,6 +744,19 @@ void cxl_accel_set_resource(struct cxl_dev_state *cxlds, struct resource res,
> >  }
> >  EXPORT_SYMBOL_NS_GPL(cxl_accel_set_resource, CXL);
> >  
> > +int cxl_accel_request_resource(struct cxl_dev_state *cxlds, bool is_ram)  
> Maybe declare a common enum like cxl_resource_type instead of 'enum accel_resource' and use here instead of bool?
> 
> > +{
> > +	int rc;
> > +
> > +	if (is_ram)
> > +		rc = request_resource(&cxlds->dpa_res, &cxlds->ram_res);
> > +	else
> > +		rc = request_resource(&cxlds->dpa_res, &cxlds->pmem_res);
> > +
> > +	return rc;
> > +}
> > +EXPORT_SYMBOL_NS_GPL(cxl_accel_request_resource, CXL);
> > +
> >  static int cxl_memdev_release_file(struct inode *inode, struct file *file)
> >  {
> >  	struct cxl_memdev *cxlmd =
> > diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c
> > index 10c4fb915278..9cefcaf3caca 100644
> > --- a/drivers/net/ethernet/sfc/efx_cxl.c
> > +++ b/drivers/net/ethernet/sfc/efx_cxl.c
> > @@ -48,8 +48,13 @@ void efx_cxl_init(struct efx_nic *efx)
> >  	res = DEFINE_RES_MEM_NAMED(0, EFX_CTPIO_BUFFER_SIZE, "ram");
> >  	cxl_accel_set_resource(cxl->cxlds, res, CXL_ACCEL_RES_RAM);
> >  
> > -	if (cxl_pci_accel_setup_regs(pci_dev, cxl->cxlds))
> > +	if (cxl_pci_accel_setup_regs(pci_dev, cxl->cxlds)) {
> >  		pci_info(pci_dev, "CXL accel setup regs failed");
> > +		return;
> > +	}
> > +
> > +	if (cxl_accel_request_resource(cxl->cxlds, true))
> > +		pci_info(pci_dev, "CXL accel resource request failed");  
> 
> pci_warn()? also emitting the errno would be nice. 
Don't hide it at all.  Fail if this doesn't succeed and let the caller
know. Not to mention, tear down any other state already set up.
 
> >  }
> >  
> >  
> > diff --git a/include/linux/cxl_accel_mem.h b/include/linux/cxl_accel_mem.h
> > index ca7af4a9cefc..c7b254edc096 100644
> > --- a/include/linux/cxl_accel_mem.h
> > +++ b/include/linux/cxl_accel_mem.h
> > @@ -20,4 +20,5 @@ void cxl_accel_set_serial(cxl_accel_state *cxlds, u64 serial);
> >  void cxl_accel_set_resource(struct cxl_dev_state *cxlds, struct resource res,
> >  			    enum accel_resource);
> >  int cxl_pci_accel_setup_regs(struct pci_dev *pdev, struct cxl_dev_state *cxlds);
> > +int cxl_accel_request_resource(struct cxl_dev_state *cxlds, bool is_ram);
> >  #endif  
>