On Tue, 17 Jun 2025 18:07:23 +0100 Jonathan Cameron <[email protected]> wrote:
> On Tue, 17 Jun 2025 18:09:24 +0530 > Neeraj Kumar <[email protected]> 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 > > > > 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.st...@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 > >
