linux-kernel - Re: [RFC PATCH 19/20] cxl/pmem_region: Prep patch to accommodate pmem

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] [day] [month] [year] [list]

Message-ID: <1296674576.21750944303379.JavaMail.epsvc@epcpadp1new>
Date: Thu, 26 Jun 2025 16:15:28 +0530
From: Neeraj Kumar <s.neeraj@...sung.com>
To: Jonathan Cameron <Jonathan.Cameron@...wei.com>
Cc: dan.j.williams@...el.com, dave@...olabs.net, dave.jiang@...el.com,
	alison.schofield@...el.com, vishal.l.verma@...el.com, ira.weiny@...el.com,
	a.manzanares@...sung.com, nifan.cxl@...il.com, anisa.su@...sung.com,
	vishak.g@...sung.com, krish.reddy@...sung.com, arun.george@...sung.com,
	alok.rathore@...sung.com, neeraj.kernel@...il.com,
	linux-kernel@...r.kernel.org, linux-cxl@...r.kernel.org,
	nvdimm@...ts.linux.dev, gost.dev@...sung.com, cpgs@...sung.com
Subject: Re: [RFC PATCH 19/20] cxl/pmem_region: Prep patch to accommodate
 pmem_region attributes

On 23/06/25 10:53AM, Jonathan Cameron wrote:
>On Tue, 17 Jun 2025 18:09:43 +0530
>Neeraj Kumar <s.neeraj@...sung.com> wrote:
>
>> Created a separate file core/pmem_region.c along with CONFIG_PMEM_REGION
>> Moved pmem_region related code from core/region.c to core/pmem_region.c
>> For region label update, need to create device attribute, which calls
>> nvdimm exported function thus making pmem_region dependent on libnvdimm.
>> Because of this dependency of pmem region on libnvdimm, segregated pmem
>
>segregate
>

Thanks, Will fix the commit message.

>> region related code from core/region.c
>>
>> Signed-off-by: Neeraj Kumar <s.neeraj@...sung.com>
>> ---
>>  drivers/cxl/Kconfig            |  12 ++
>>  drivers/cxl/core/Makefile      |   1 +
>>  drivers/cxl/core/core.h        |   8 +-
>>  drivers/cxl/core/pmem_region.c | 222 +++++++++++++++++++++++++++++++++
>>  drivers/cxl/core/port.c        |   2 +-
>>  drivers/cxl/core/region.c      | 217 ++------------------------------
>>  drivers/cxl/cxl.h              |  42 +++++--
>>  tools/testing/cxl/Kbuild       |   1 +
>>  8 files changed, 283 insertions(+), 222 deletions(-)
>>  create mode 100644 drivers/cxl/core/pmem_region.c
>>
>> diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
>> index 876469e23f7a..f0cbb096bfe7 100644
>> --- a/drivers/cxl/Kconfig
>> +++ b/drivers/cxl/Kconfig
>> @@ -128,6 +128,18 @@ config CXL_REGION
>>
>>  	  If unsure say 'y'
>>
>> +config CXL_PMEM_REGION
>> +	bool "CXL: Pmem Region Support"
>> +	default CXL_BUS
>> +	depends on CXL_REGION
>> +	select LIBNVDIMM if CXL_BUS = y
>
>This is in the block covered by if CXL_BUS so I think you can simplify this check.
>

yes, its part of CXL_BUS block. I will update it as "select LIBNVDIMM".
Actually for CONFIG_CXL_PMEM_REGION=y we have dependency on CONFIG_LIBNVDIMM.

>> +	help
>> +	  Enable the CXL core to enumerate and provision CXL pmem regions.
>> +	  A CXL pmem region need to update region label into LSA. For LSA
>> +	  updation/deletion libnvdimm is required.
>> +
>> +	  If unsure say 'y'
>> +
>>  config CXL_REGION_INVALIDATION_TEST
>>  	bool "CXL: Region Cache Management Bypass (TEST)"
>>  	depends on CXL_REGION
>
>> diff --git a/drivers/cxl/core/pmem_region.c b/drivers/cxl/core/pmem_region.c
>> new file mode 100644
>> index 000000000000..a29526c27d40
>> --- /dev/null
>> +++ b/drivers/cxl/core/pmem_region.c
>> @@ -0,0 +1,222 @@
>
>> @@ -3273,92 +3155,6 @@ static struct cxl_dax_region *cxl_dax_region_alloc(struct cxl_region *cxlr)
>>  	return cxlr_dax;
>>  }
>
>>  static void cxlr_dax_unregister(void *_cxlr_dax)
>>  {
>>  	struct cxl_dax_region *cxlr_dax = _cxlr_dax;
>> @@ -3646,7 +3442,10 @@ static int cxl_region_probe(struct device *dev)
>>
>>  	switch (cxlr->mode) {
>>  	case CXL_DECODER_PMEM:
>> -		return devm_cxl_add_pmem_region(cxlr);
>> +		if (IS_ENABLED(CONFIG_CXL_PMEM_REGION))
>> +			return devm_cxl_add_pmem_region(cxlr);
>> +		else
>> +			return -EINVAL;
>		if (!IS_ENABLED(CONFIG_CXL_PMEM_REGION))
>			return -EINVAL;
>
>		return devm_cxl_add_pmem_region()
>
>Where ever possible keep the error conditions as the out of line ones.
>That generally improves code readability and is common practice in the kernel.
>

Thanks for suggestion Jonathan. Sure will fix it in V1

>>  	case CXL_DECODER_RAM:
>>  		/*
>>  		 * The region can not be manged by CXL if any portion of
>
>