Re: [RFC PATCH 00/20] Add CXL LSA 2.1 format support in nvdimm and cxl pmem

[Neeraj Kumar <s.neeraj@samsung.com>]

[-- Attachment #1: Type: text/plain, Size: 15805 bytes --]

On 23/06/25 10:57AM, Jonathan Cameron wrote:
>On Tue, 17 Jun 2025 18:07:23 +0100
>Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:
>
>> On Tue, 17 Jun 2025 18:09:24 +0530
>> Neeraj Kumar <s.neeraj@samsung.com> wrote:
>>
>> Hi Neeraj,
>>
>> First a process thing. Looks like threading is broken in your patch set.
>> https://lore.kernel.org/linux-cxl/1931444790.41750165203442.JavaMail.epsvc@epcpadp1new/T/#u
>> Lore had a go and seems to have figured out there is a thread for the rest.
>> If you can't fix it locally for your email, look at using the b4 gateway
>> (see the docs for the b4 tool)
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/palmer/b4.git/
>>
>> Also, in an RFC the first thing I expect to see if a 'Why this is an RFC statement'
>> vs ready for upstream merge.  What are the questions that need comments or
>> the blockers you know need to be resolved?
>>
>> Anyhow, good to see this. Not an area I'm that familiar with but
>> I'll try to take a detailed look a bit later this week.
>>
>Whilst I've looked through this series, most of my comments are superficial 'local'
>stuff.  This needs some attention from those more familiar with the persistent memory
>flows than I am.
>
>Jonathan
>

Thanks Jonathan for your valuable feedback

>> Thanks,
>>
>> Jonathan
>>
>>
>> > Introduction:
>> > =============
>> > CXL Persistent Memory (Pmem) devices region, namespace and content must be
>> > persistent across system reboot. In order to achieve this persistency, it
>> > uses Label Storage Area (LSA) to store respective metadata. During system
>> > reboot, stored metadata in LSA is used to bring back the region, namespace
>> > and content of CXL device in its previous state.
>> > CXL specification provides Get_LSA (4102h) and Set_LSA (4103h) mailbox
>> > commands to access the LSA area. nvdimm driver is using same commands to
>> > get/set LSA data.
>> >
>> > There are three types of LSA format and these are part of following
>> > specifications:
>> >  - v1.1: https://pmem.io/documents/NVDIMM_Namespace_Spec.pdf
>> >  - v1.2: https://uefi.org/sites/default/files/resources/UEFI_Spec_2_7.pdf
>> >  - v2.1: https://computeexpresslink.org/wp-content/uploads/2024/02/CXL-2.0-Specification.pdf
>> >
>> > Basic differences between these LSA formats:
>> >  - v1.1: Support Namespace persistency. Size of Namespace Label is 128 bytes
>> >  - v1.2: Support Namespace persistency. Size of Namespace Label is 256 bytes
>> >  - v2.1: Support Namespace and Region persistency. Size of Namespace and
>> >    Region Label is 256 bytes.
>> >
>> > Linux nvdimm driver supports only v1.1 and v1.2 LSA format. CXL pmem device
>> > require support of LSA v2.1 format for region and namespace persistency.
>> > Initial support of LSA 2.1 was add in [1].
>> >
>> > This patchset adds support of LSA 2.1 in nvdimm and cxl pmem driver.
>> >
>> > Patch 1:     Introduce NDD_CXL_LABEL flag and Update cxl label index as per v2.1
>> > Patch 2-4:   Update namespace label as per v2.1
>> > Patch 5-12:  Introduce cxl region labels and modify existing change accordingly
>> > Patch 13:    Refactor cxl pmem region auto assembly
>> > Patch 14-18: Save cxl region info in LSA and region recreation during reboot
>> > Patch 19:    Segregate out cxl pmem region code from region.c to pmem_region.c
>> > Patch 20:    Introduce cxl region addition/deletion attributes
>> >
>> >
>> > Testing:
>> > ========
>> > In order to test this patchset, I also added the support of LSA v2.1 format
>> > in ndctl. ndctl changes are available at [2]. After review, I’ll push in
>> > ndctl repo for community review.
>> >
>> > 1. Used Qemu using following CXL topology
>> >    M2="-object memory-backend-file,id=cxl-mem1,share=on,mem-path=$TMP_DIR/cxltest.raw,size=512M \
>> >        -object memory-backend-file,id=cxl-lsa1,share=on,mem-path=$TMP_DIR/lsa.raw,size=1M \
>> >        -object memory-backend-file,id=cxl-mem2,share=on,mem-path=$TMP_DIR/cxltest2.raw,size=512M \
>> >        -object memory-backend-file,id=cxl-lsa2,share=on,mem-path=$TMP_DIR/lsa2.raw,size=1M \
>> >        -device pxb-cxl,bus_nr=10,bus=pcie.0,id=cxl.1 \
>> >        -device cxl-rp,port=1,bus=cxl.1,id=root_port11,chassis=0,slot=1 \
>> >        -device cxl-type3,bus=root_port11,memdev=cxl-mem1,lsa=cxl-lsa1,id=cxl-pmem1,sn=1 \
>> >        -device cxl-rp,port=2,bus=cxl.1,id=root_port12,chassis=0,slot=2 \
>> >        -device cxl-type3,bus=root_port12,memdev=cxl-mem2,lsa=cxl-lsa2,id=cxl-pmem2,sn=2 \
>> >        -M cxl-fmw.0.targets.0=cxl.1,cxl-fmw.0.size=4G,cxl-fmw.0.interleave-granularity=8k"
>> >
>> > 2. Create cxl region on both devices
>> > 	cxl create-region -d decoder0.0 -m mem0
>> > 	cxl create-region -d decoder0.0 -m mem1
>> >
>> > 	root@QEMUCXL6060pmem:~# cxl list
>> > 	[
>> > 	  {
>> > 	    "memdevs":[
>> > 	      {
>> > 	        "memdev":"mem0",
>> > 	        "pmem_size":536870912,
>> > 	        "serial":2,
>> > 	        "host":"0000:0c:00.0",
>> > 	        "firmware_version":"BWFW VERSION 00"
>> > 	      },
>> > 	      {
>> > 	        "memdev":"mem1",
>> > 	        "pmem_size":536870912,
>> > 	        "serial":1,
>> > 	        "host":"0000:0b:00.0",
>> > 	        "firmware_version":"BWFW VERSION 00"
>> > 	      }
>> > 	    ]
>> > 	  },
>> > 	  {
>> > 	    "regions":[
>> > 	      {
>> > 	        "region":"region0",
>> > 	        "resource":45365592064,
>> > 	        "size":536870912,
>> > 	        "type":"pmem",
>> > 	        "interleave_ways":1,
>> > 	        "interleave_granularity":256,
>> > 	        "decode_state":"commit",
>> > 	        "qos_class_mismatch":true
>> > 	      },
>> > 	      {
>> > 	        "region":"region1",
>> > 	        "resource":45902462976,
>> > 	        "size":536870912,
>> > 	        "type":"pmem",
>> > 	        "interleave_ways":1,
>> > 	        "interleave_granularity":256,
>> > 	        "decode_state":"commit",
>> > 	        "qos_class_mismatch":true
>> > 	      }
>> > 	    ]
>> > 	  }
>> > 	]
>> >
>> > 3. Re-Start Qemu and we could see cxl region persistency using "cxl list"
>> >
>> > 4. Create namespace for both regions
>> > 	root@QEMUCXL6060pmem:~# ndctl create-namespace --mode=fsdax --region=region0 --size=128M
>> > 	{
>> > 	  "dev":"namespace0.0",
>> > 	  "mode":"fsdax",
>> > 	  "map":"dev",
>> > 	  "size":"124.00 MiB (130.02 MB)",
>> > 	  "uuid":"3f6dcdc5-d289-4b0c-ad16-82636c82bec1",
>> > 	  "sector_size":512,
>> > 	  "align":2097152,
>> > 	  "blockdev":"pmem0"
>> > 	}
>> > 	
>> > 	root@QEMUCXL6060pmem:~# ndctl create-namespace --mode=fsdax --region=region1 --size=256M
>> > 	{
>> > 	  "dev":"namespace1.0",
>> > 	  "mode":"fsdax",
>> > 	  "map":"dev",
>> > 	  "size":"250.00 MiB (262.14 MB)",
>> > 	  "uuid":"6b9083c9-cb1a-447b-894d-fdfd2f3dbed2",
>> > 	  "sector_size":512,
>> > 	  "align":2097152,
>> > 	  "blockdev":"pmem1"
>> > 	}
>> > 	
>> > 	root@QEMUCXL6060pmem:~# time ndctl create-namespace --mode=fsdax --region=region0 --size=256M
>> > 	{
>> > 	  "dev":"namespace0.1",
>> > 	  "mode":"fsdax",
>> > 	  "map":"dev",
>> > 	  "size":"250.00 MiB (262.14 MB)",
>> > 	  "uuid":"c2071802-8c24-4f9c-a6c1-f6bb6589e561",
>> > 	  "sector_size":512,
>> > 	  "align":2097152,
>> > 	  "blockdev":"pmem0.1"
>> > 	}
>> > 	
>> > 	real    0m47.517s
>> > 	user    0m0.209s
>> > 	sys     0m26.879s
>> > 	root@QEMUCXL6060pmem:~# time ndctl create-namespace --mode=fsdax --region=region1 --size=128M
>> > 	{
>> > 	  "dev":"namespace1.1",
>> > 	  "mode":"fsdax",
>> > 	  "map":"dev",
>> > 	  "size":"124.00 MiB (130.02 MB)",
>> > 	  "uuid":"13bc8de3-53d3-4c3d-b252-be7efefe80ee",
>> > 	  "sector_size":512,
>> > 	  "align":2097152,
>> > 	  "blockdev":"pmem1.1"
>> > 	}
>> > 	
>> > 	real    0m33.459s
>> > 	user    0m0.243s
>> > 	sys     0m13.590s
>> >
>> > 	root@QEMUCXL6060pmem:~# ndctl list
>> > 	[
>> > 	  {
>> > 	    "dev":"namespace1.0",
>> > 	    "mode":"fsdax",
>> > 	    "map":"dev",
>> > 	    "size":262144000,
>> > 	    "uuid":"6b9083c9-cb1a-447b-894d-fdfd2f3dbed2",
>> > 	    "sector_size":512,
>> > 	    "align":2097152,
>> > 	    "blockdev":"pmem1"
>> > 	  },
>> > 	  {
>> > 	    "dev":"namespace1.1",
>> > 	    "mode":"fsdax",
>> > 	    "map":"dev",
>> > 	    "size":130023424,
>> > 	    "uuid":"13bc8de3-53d3-4c3d-b252-be7efefe80ee",
>> > 	    "sector_size":512,
>> > 	    "align":2097152,
>> > 	    "blockdev":"pmem1.1"
>> > 	  },
>> > 	  {
>> > 	    "dev":"namespace0.1",
>> > 	    "mode":"fsdax",
>> > 	    "map":"dev",
>> > 	    "size":262144000,
>> > 	    "uuid":"c2071802-8c24-4f9c-a6c1-f6bb6589e561",
>> > 	    "sector_size":512,
>> > 	    "align":2097152,
>> > 	    "blockdev":"pmem0.1"
>> > 	  },
>> > 	  {
>> > 	    "dev":"namespace0.0",
>> > 	    "mode":"fsdax",
>> > 	    "map":"dev",
>> > 	    "size":130023424,
>> > 	    "uuid":"3f6dcdc5-d289-4b0c-ad16-82636c82bec1",
>> > 	    "sector_size":512,
>> > 	    "align":2097152,
>> > 	    "blockdev":"pmem0"
>> > 	  }
>> > 	]
>> > 	
>> > 5. Re-Start Qemu and we could see
>> > 	- Region persistency using "cxl list"
>> > 	- Namespace persistency using "ndctl list" and cat /proc/iomem
>> >
>> > 	root@QEMUCXL6060pmem:~# cat /proc/iomem
>> > 	
>> > 	a90000000-b8fffffff : CXL Window 0
>> > 	  a90000000-aafffffff : Persistent Memory
>> > 	    a90000000-aafffffff : region0
>> > 	      a90000000-a97ffffff : namespace0.0
>> > 	      a98000000-aa7ffffff : namespace0.1
>> > 	  ab0000000-acfffffff : Persistent Memory
>> > 	    ab0000000-acfffffff : region1
>> > 	      ab0000000-abfffffff : namespace1.0
>> > 	      ac0000000-ac7ffffff : namespace1.1
>> > 	
>> >
>> > 	- NOTE: We can see some lag in restart, Its WIP
>> >
>> > 6. Also verify LSA version using "ndctl read-labels -j nmem0 -u"
>> > 	root@QEMUCXL6060pmem:~# ndctl read-labels -j nmem0
>> > 	{
>> > 	  "dev":"nmem0",
>> > 	  "index":[
>> > 	    {
>> > 	      "signature":"NAMESPACE_INDEX",
>> > 	      "major":2,
>> > 	      "minor":1,
>> > 	      "labelsize":256,
>> > 	      "seq":2,
>> > 	      "nslot":4090
>> > 	    },
>> > 	    {
>> > 	      "signature":"NAMESPACE_INDEX",
>> > 	      "major":2,
>> > 	      "minor":1,
>> > 	      "labelsize":256,
>> > 	      "seq":1,
>> > 	      "nslot":4090
>> > 	    }
>> > 	  ],
>> > 	  "label":[
>> > 	    {
>> > 	      "type":"68bb2c0a-5a77-4937-9f85-3caf41a0f93c",
>> > 	      "uuid":"9a26b9ce-a859-4a63-b1b7-fd176105770d",
>> > 	      "name":"",
>> > 	      "flags":8,
>> > 	      "nrange":1,
>> > 	      "position":0,
>> > 	      "dpa":134217728,
>> > 	      "rawsize":268435456,
>> > 	      "slot":0,
>> > 	      "align":0,
>> > 	      "region_uuid":"de512bdc-9731-4e5f-838c-be2e9b94ea72",
>> > 	      "abstraction_uuid":"266400ba-fb9f-4677-bcb0-968f11d0d225",
>> > 	      "lbasize":512
>> > 	    },
>> > 	    {
>> > 	      "type":"529d7c61-da07-47c4-a93f-ecdf2c06f444",
>> > 	      "uuid":"de512bdc-9731-4e5f-838c-be2e9b94ea72",
>> > 	      "flags":0,
>> > 	      "nlabel":1,
>> > 	      "position":0,
>> > 	      "dpa":0,
>> > 	      "rawsize":536870912,
>> > 	      "hpa":45365592064,
>> > 	      "slot":1,
>> > 	      "interleave granularity":256,
>> > 	      "align":0
>> > 	    },
>> > 	    {
>> > 	      "type":"68bb2c0a-5a77-4937-9f85-3caf41a0f93c",
>> > 	      "uuid":"a90d211a-b28d-48c7-bfe7-99560d3825ef",
>> > 	      "name":"",
>> > 	      "flags":0,
>> > 	      "nrange":1,
>> > 	      "position":0,
>> > 	      "dpa":0,
>> > 	      "rawsize":134217728,
>> > 	      "slot":2,
>> > 	      "align":0,
>> > 	      "region_uuid":"de512bdc-9731-4e5f-838c-be2e9b94ea72",
>> > 	      "abstraction_uuid":"266400ba-fb9f-4677-bcb0-968f11d0d225",
>> > 	      "lbasize":512
>> > 	    },
>> > 	    {
>> > 	      "type":"68bb2c0a-5a77-4937-9f85-3caf41a0f93c",
>> > 	      "uuid":"9a26b9ce-a859-4a63-b1b7-fd176105770d",
>> > 	      "name":"",
>> > 	      "flags":0,
>> > 	      "nrange":1,
>> > 	      "position":0,
>> > 	      "dpa":134217728,
>> > 	      "rawsize":268435456,
>> > 	      "slot":3,
>> > 	      "align":0,
>> > 	      "region_uuid":"de512bdc-9731-4e5f-838c-be2e9b94ea72",
>> > 	      "abstraction_uuid":"266400ba-fb9f-4677-bcb0-968f11d0d225",
>> > 	      "lbasize":512
>> > 	    }
>> > 	  ]
>> > 	}
>> > 	read 1 nmem
>> >
>> > 	- NOTE: We have following UUID types as per CXL Spec
>> > 		"type":"529d7c61-da07-47c4-a93f-ecdf2c06f444" is region label
>> > 		"type":"68bb2c0a-5a77-4937-9f85-3caf41a0f93c" is namespace label
>> >
>> >
>> > Limitation (WIP):
>> > ================
>> > Current changes only support interleave way == 1
>> >
>> > Observation:
>> > ============
>> > First time namespace creation using ndctl takes around 10 to 20 second time
>> > while executing "devm_memremap_pages" at [3]
>> >
>> > As using this patchset, after auto region creation, namespace creation is
>> > happening in boot sequence (if nvdimm and cxl drivers are static), It is
>> > therefore boot sequence is increased by around 10 to 20 sec.
>> >
>> >
>> > [1]: https://lore.kernel.org/all/163116432405.2460985.5547867384570123403.stgit@dwillia2-desk3.amr.corp.intel.com/
>> > [2]: https://github.com/neerajoss/ndctl/tree/linux-cxl/CXL_LSA_2.1_Support
>> > [3]: https://elixir.bootlin.com/linux/v6.13.7/source/drivers/nvdimm/pmem.c#L520
>> >
>> >
>> >
>> >
>> > Neeraj Kumar (20):
>> >   nvdimm/label: Introduce NDD_CXL_LABEL flag to set cxl label format
>> >   nvdimm/label: Prep patch to accommodate cxl lsa 2.1 support
>> >   nvdimm/namespace_label: Add namespace label changes as per CXL LSAv2.1
>> >   nvdimm/label: CXL labels skip the need for 'interleave-set cookie'
>> >   nvdimm/region_label: Add region label updation routine
>> >   nvdimm/region_label: Add region label deletion routine
>> >   nvdimm/namespace_label: Update namespace init_labels and its region_uuid
>> >   nvdimm/label: Include region label in slot validation
>> >   nvdimm/namespace_label: Skip region label during ns label DPA reservation
>> >   nvdimm/region_label: Preserve cxl region information from region label
>> >   nvdimm/region_label: Export routine to fetch region information
>> >   nvdimm/namespace_label: Skip region label during namespace creation
>> >   cxl/mem: Refactor cxl pmem region auto-assembling
>> >   cxl/region: Add cxl pmem region creation routine for region persistency
>> >   cxl: Add a routine to find cxl root decoder on cxl bus
>> >   cxl/mem: Preserve cxl root decoder during mem probe
>> >   cxl/pmem: Preserve region information into nd_set
>> >   cxl/pmem: Add support of cxl lsa 2.1 support in cxl pmem
>> >   cxl/pmem_region: Prep patch to accommodate pmem_region attributes
>> >   cxl/pmem_region: Add cxl region label updation and deletion device attributes
>> >
>> >  drivers/cxl/Kconfig             |  12 +
>> >  drivers/cxl/core/Makefile       |   1 +
>> >  drivers/cxl/core/core.h         |   8 +-
>> >  drivers/cxl/core/pmem_region.c  | 325 ++++++++++++++++++++++++++
>> >  drivers/cxl/core/port.c         |  72 +++++-
>> >  drivers/cxl/core/region.c       | 383 +++++++++++++++---------------
>> >  drivers/cxl/cxl.h               |  46 +++-
>> >  drivers/cxl/cxlmem.h            |   1 +
>> >  drivers/cxl/mem.c               |  27 ++-
>> >  drivers/cxl/pmem.c              |  72 +++++-
>> >  drivers/cxl/port.c              |  38 ---
>> >  drivers/nvdimm/dimm.c           |   5 +
>> >  drivers/nvdimm/dimm_devs.c      |  28 +++
>> >  drivers/nvdimm/label.c          | 403 ++++++++++++++++++++++++++++----
>> >  drivers/nvdimm/label.h          |  20 +-
>> >  drivers/nvdimm/namespace_devs.c | 149 ++++++++----
>> >  drivers/nvdimm/nd-core.h        |   2 +
>> >  drivers/nvdimm/nd.h             |  81 ++++++-
>> >  drivers/nvdimm/region_devs.c    |   5 +
>> >  include/linux/libnvdimm.h       |  28 +++
>> >  tools/testing/cxl/Kbuild        |   1 +
>> >  21 files changed, 1364 insertions(+), 343 deletions(-)
>> >  create mode 100644 drivers/cxl/core/pmem_region.c
>> >
>> >
>> > base-commit: 448a60e85ae2afe2cb760f5d2ed2c8a49d2bd1b4
>>
>>
>




[-- Attachment #2: Type: text/plain, Size: 0 bytes --]