netdev - Re: [PATCH v2 06/15] cxl: add function for setting media ready by an accelerator

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: <032d42a4-a02c-c928-8bc2-1f20145a52de@amd.com>
Date: Fri, 16 Aug 2024 15:54:43 +0100
From: Alejandro Lucero Palau <alucerop@....com>
To: Jonathan Cameron <Jonathan.Cameron@...wei.com>,
 alejandro.lucero-palau@....com
Cc: 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
Subject: Re: [PATCH v2 06/15] cxl: add function for setting media ready by an
 accelerator


On 8/4/24 18:26, Jonathan Cameron wrote:
> On Mon, 15 Jul 2024 18:28:26 +0100
> alejandro.lucero-palau@....com wrote:
>
>> From: Alejandro Lucero <alucerop@....com>
>>
>> A Type-2 driver can require to set the memory availability explicitly.
>>
>> Add a function to the exported CXL API for accelerator drivers.
>>
>> Signed-off-by: Alejandro Lucero <alucerop@....com>
>> ---
>>   drivers/cxl/core/memdev.c          | 7 ++++++-
>>   drivers/net/ethernet/sfc/efx_cxl.c | 5 +++++
>>   include/linux/cxl_accel_mem.h      | 2 ++
>>   3 files changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
>> index b4205ecca365..58a51e7fd37f 100644
>> --- a/drivers/cxl/core/memdev.c
>> +++ b/drivers/cxl/core/memdev.c
>> @@ -714,7 +714,6 @@ static int cxl_memdev_open(struct inode *inode, struct file *file)
>>   	return 0;
>>   }
>>   
>> -
> Grumpy maintainer time ;)
> Scrub for this stuff before posting.  Move the whitespace cleanup to the
> earlier patch so we have less noise here.
>

I will avoid this kind of things in v3.


>>   void cxl_accel_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec)
>>   {
>>   	cxlds->cxl_dvsec = dvsec;
>> @@ -759,6 +758,12 @@ int cxl_accel_request_resource(struct cxl_dev_state *cxlds, bool is_ram)
>>   }
>>   EXPORT_SYMBOL_NS_GPL(cxl_accel_request_resource, CXL);
>>   
>> +void cxl_accel_set_media_ready(struct cxl_dev_state *cxlds)
>> +{
>> +	cxlds->media_ready = true;
>> +}
>> +EXPORT_SYMBOL_NS_GPL(cxl_accel_set_media_ready, 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 37d8bfdef517..a84fe7992c53 100644
>> --- a/drivers/net/ethernet/sfc/efx_cxl.c
>> +++ b/drivers/net/ethernet/sfc/efx_cxl.c
>> @@ -56,6 +56,11 @@ void efx_cxl_init(struct efx_nic *efx)
>>   
>>   	if (cxl_accel_request_resource(cxl->cxlds, true))
>>   		pci_info(pci_dev, "CXL accel resource request failed");
>> +
>> +	if (!cxl_await_media_ready(cxl->cxlds))
>> +		cxl_accel_set_media_ready(cxl->cxlds);
>> +	else
>> +		pci_info(pci_dev, "CXL accel media not active");
> Feels fatal. pci_err() and return an error.


As I commented yesterday when this patch was pointed to in another patch 
review, this is unnecessary in our case and it will be fixed in next 
version:

cxl_await_media_ready will not be invoked only using the accessor for 
manually setting the media ready.

Thanks


>>   }
>>   
>>   
>> diff --git a/include/linux/cxl_accel_mem.h b/include/linux/cxl_accel_mem.h
>> index 0ba2195b919b..b883c438a132 100644
>> --- a/include/linux/cxl_accel_mem.h
>> +++ b/include/linux/cxl_accel_mem.h
>> @@ -24,4 +24,6 @@ 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);
>> +void cxl_accel_set_media_ready(struct cxl_dev_state *cxlds);
>> +int cxl_await_media_ready(struct cxl_dev_state *cxlds);
>>   #endif