CXL RAM and the 'Soft Reserved' => 'System RAM' default

Prior to Linus deciding that the kernel that following v5.19 would be v6.0, the CXL ABI documentation already referenced v5.20. In preparation for updating these entries update the kernel version to v6.0.

Signed-off-by Dan Williams <[email protected]>

---
 Documentation/ABI/testing/sysfs-bus-cxl |   30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

Documentation/ABI/testing/sysfs-bus-cxl +15 -15

index 329a7e46c805..5be032313e29 100644
@@ -198,7 +198,7 @@ Description:
 
 What:		/sys/bus/cxl/devices/endpointX/CDAT
 Date:		July, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
 Contact:	[email protected]
 Description:
 		(RO) If this sysfs entry is not present no DOE mailbox was

@@ -209,7 +209,7 @@ Description:
 
 What:		/sys/bus/cxl/devices/decoderX.Y/mode
 Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
 Contact:	[email protected]
 Description:
 		(RW) When a CXL decoder is of devtype "cxl_decoder_endpoint" it

@@ -229,7 +229,7 @@ Description:
 
 What:		/sys/bus/cxl/devices/decoderX.Y/dpa_resource
 Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
 Contact:	[email protected]
 Description:
 		(RO) When a CXL decoder is of devtype "cxl_decoder_endpoint",

@@ -240,7 +240,7 @@ Description:
 
 What:		/sys/bus/cxl/devices/decoderX.Y/dpa_size
 Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
 Contact:	[email protected]
 Description:
 		(RW) When a CXL decoder is of devtype "cxl_decoder_endpoint" it

@@ -260,7 +260,7 @@ Description:
 
 What:		/sys/bus/cxl/devices/decoderX.Y/interleave_ways
 Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
 Contact:	[email protected]
 Description:
 		(RO) The number of targets across which this decoder's host

@@ -275,7 +275,7 @@ Description:
 
 What:		/sys/bus/cxl/devices/decoderX.Y/interleave_granularity
 Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
 Contact:	[email protected]
 Description:
 		(RO) The number of consecutive bytes of host physical address

@@ -287,7 +287,7 @@ Description:
 
 What:		/sys/bus/cxl/devices/decoderX.Y/create_pmem_region
 Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
 Contact:	[email protected]
 Description:
 		(RW) Write a string in the form 'regionZ' to start the process

@@ -303,7 +303,7 @@ Description:
 
 What:		/sys/bus/cxl/devices/decoderX.Y/delete_region
 Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
 Contact:	[email protected]
 Description:
 		(WO) Write a string in the form 'regionZ' to delete that region,

@@ -312,7 +312,7 @@ Description:
 
 What:		/sys/bus/cxl/devices/regionZ/uuid
 Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
 Contact:	[email protected]
 Description:
 		(RW) Write a unique identifier for the region. This field must

@@ -322,7 +322,7 @@ Description:
 
 What:		/sys/bus/cxl/devices/regionZ/interleave_granularity
 Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
 Contact:	[email protected]
 Description:
 		(RW) Set the number of consecutive bytes each device in the

@@ -333,7 +333,7 @@ Description:
 
 What:		/sys/bus/cxl/devices/regionZ/interleave_ways
 Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
 Contact:	[email protected]
 Description:
 		(RW) Configures the number of devices participating in the

@@ -343,7 +343,7 @@ Description:
 
 What:		/sys/bus/cxl/devices/regionZ/size
 Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
 Contact:	[email protected]
 Description:
 		(RW) System physical address space to be consumed by the region.

@@ -360,7 +360,7 @@ Description:
 
 What:		/sys/bus/cxl/devices/regionZ/resource
 Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
 Contact:	[email protected]
 Description:
 		(RO) A region is a contiguous partition of a CXL root decoder

@@ -372,7 +372,7 @@ Description:
 
 What:		/sys/bus/cxl/devices/regionZ/target[0..N]
 Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
 Contact:	[email protected]
 Description:
 		(RW) Write an endpoint decoder object name to 'targetX' where X

@@ -391,7 +391,7 @@ Description:
 
 What:		/sys/bus/cxl/devices/regionZ/commit
 Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
 Contact:	[email protected]
 Description:
 		(RW) Write a boolean 'true' string value to this attribute to

Prior to Linus deciding that the kernel that following v5.19 would be v6.0, the CXL ABI documentation already referenced v5.20. In preparation for updating these entries update the kernel version to v6.0.

Signed-off-by Dan Williams <[email protected]>

Reviewed-by Jonathan Cameron <[email protected]>

Prior to Linus deciding that the kernel that following v5.19 would be v6.0, the CXL ABI documentation already referenced v5.20. In preparation for updating these entries update the kernel version to v6.0.

Signed-off-by Dan Williams <[email protected]>

---
 Documentation/ABI/testing/sysfs-bus-cxl |   30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

Documentation/ABI/testing/sysfs-bus-cxl +15 -15

index 329a7e46c805..5be032313e29 100644
@@ -198,7 +198,7 @@ Description:
 
 What:		/sys/bus/cxl/devices/endpointX/CDAT
 Date:		July, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
 Contact:	[email protected]
 Description:
 		(RO) If this sysfs entry is not present no DOE mailbox was

@@ -209,7 +209,7 @@ Description:
 
 What:		/sys/bus/cxl/devices/decoderX.Y/mode
 Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
 Contact:	[email protected]
 Description:
 		(RW) When a CXL decoder is of devtype "cxl_decoder_endpoint" it

@@ -229,7 +229,7 @@ Description:
 
 What:		/sys/bus/cxl/devices/decoderX.Y/dpa_resource
 Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
 Contact:	[email protected]
 Description:
 		(RO) When a CXL decoder is of devtype "cxl_decoder_endpoint",

@@ -240,7 +240,7 @@ Description:
 
 What:		/sys/bus/cxl/devices/decoderX.Y/dpa_size
 Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
 Contact:	[email protected]
 Description:
 		(RW) When a CXL decoder is of devtype "cxl_decoder_endpoint" it

@@ -260,7 +260,7 @@ Description:
 
 What:		/sys/bus/cxl/devices/decoderX.Y/interleave_ways
 Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
 Contact:	[email protected]
 Description:
 		(RO) The number of targets across which this decoder's host

@@ -275,7 +275,7 @@ Description:
 
 What:		/sys/bus/cxl/devices/decoderX.Y/interleave_granularity
 Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
 Contact:	[email protected]
 Description:
 		(RO) The number of consecutive bytes of host physical address

@@ -287,7 +287,7 @@ Description:
 
 What:		/sys/bus/cxl/devices/decoderX.Y/create_pmem_region
 Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
 Contact:	[email protected]
 Description:
 		(RW) Write a string in the form 'regionZ' to start the process

@@ -303,7 +303,7 @@ Description:
 
 What:		/sys/bus/cxl/devices/decoderX.Y/delete_region
 Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
 Contact:	[email protected]
 Description:
 		(WO) Write a string in the form 'regionZ' to delete that region,

@@ -312,7 +312,7 @@ Description:
 
 What:		/sys/bus/cxl/devices/regionZ/uuid
 Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
 Contact:	[email protected]
 Description:
 		(RW) Write a unique identifier for the region. This field must

@@ -322,7 +322,7 @@ Description:
 
 What:		/sys/bus/cxl/devices/regionZ/interleave_granularity
 Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
 Contact:	[email protected]
 Description:
 		(RW) Set the number of consecutive bytes each device in the

@@ -333,7 +333,7 @@ Description:
 
 What:		/sys/bus/cxl/devices/regionZ/interleave_ways
 Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
 Contact:	[email protected]
 Description:
 		(RW) Configures the number of devices participating in the

@@ -343,7 +343,7 @@ Description:
 
 What:		/sys/bus/cxl/devices/regionZ/size
 Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
 Contact:	[email protected]
 Description:
 		(RW) System physical address space to be consumed by the region.

@@ -360,7 +360,7 @@ Description:
 
 What:		/sys/bus/cxl/devices/regionZ/resource
 Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
 Contact:	[email protected]
 Description:
 		(RO) A region is a contiguous partition of a CXL root decoder

@@ -372,7 +372,7 @@ Description:
 
 What:		/sys/bus/cxl/devices/regionZ/target[0..N]
 Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
 Contact:	[email protected]
 Description:
 		(RW) Write an endpoint decoder object name to 'targetX' where X

@@ -391,7 +391,7 @@ Description:
 
 What:		/sys/bus/cxl/devices/regionZ/commit
 Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
 Contact:	[email protected]
 Description:
 		(RW) Write a boolean 'true' string value to this attribute to

Prior to Linus deciding that the kernel that following v5.19 would be v6.0, the CXL ABI documentation already referenced v5.20. In preparation for updating these entries update the kernel version to v6.0.

Signed-off-by Dan Williams <[email protected]>

---
 Documentation/ABI/testing/sysfs-bus-cxl |   30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

Prior to Linus deciding that the kernel that following v5.19 would be v6.0, the CXL ABI documentation already referenced v5.20. In preparation for updating these entries update the kernel version to v6.0.

Signed-off-by Dan Williams <[email protected]>

---
  Documentation/ABI/testing/sysfs-bus-cxl |   30 +++++++++++++++---------------
  1 file changed, 15 insertions(+), 15 deletions(-)

Documentation/ABI/testing/sysfs-bus-cxl +15 -15

index 329a7e46c805..5be032313e29 100644
@@ -198,7 +198,7 @@ Description:
 
  What:		/sys/bus/cxl/devices/endpointX/CDAT
  Date:		July, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
  Contact:	[email protected]
  Description:
  		(RO) If this sysfs entry is not present no DOE mailbox was

@@ -209,7 +209,7 @@ Description:
 
  What:		/sys/bus/cxl/devices/decoderX.Y/mode
  Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
  Contact:	[email protected]
  Description:
  		(RW) When a CXL decoder is of devtype "cxl_decoder_endpoint" it

@@ -229,7 +229,7 @@ Description:
 
  What:		/sys/bus/cxl/devices/decoderX.Y/dpa_resource
  Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
  Contact:	[email protected]
  Description:
  		(RO) When a CXL decoder is of devtype "cxl_decoder_endpoint",

@@ -240,7 +240,7 @@ Description:
 
  What:		/sys/bus/cxl/devices/decoderX.Y/dpa_size
  Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
  Contact:	[email protected]
  Description:
  		(RW) When a CXL decoder is of devtype "cxl_decoder_endpoint" it

@@ -260,7 +260,7 @@ Description:
 
  What:		/sys/bus/cxl/devices/decoderX.Y/interleave_ways
  Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
  Contact:	[email protected]
  Description:
  		(RO) The number of targets across which this decoder's host

@@ -275,7 +275,7 @@ Description:
 
  What:		/sys/bus/cxl/devices/decoderX.Y/interleave_granularity
  Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
  Contact:	[email protected]
  Description:
  		(RO) The number of consecutive bytes of host physical address

@@ -287,7 +287,7 @@ Description:
 
  What:		/sys/bus/cxl/devices/decoderX.Y/create_pmem_region
  Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
  Contact:	[email protected]
  Description:
  		(RW) Write a string in the form 'regionZ' to start the process

@@ -303,7 +303,7 @@ Description:
 
  What:		/sys/bus/cxl/devices/decoderX.Y/delete_region
  Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
  Contact:	[email protected]
  Description:
  		(WO) Write a string in the form 'regionZ' to delete that region,

@@ -312,7 +312,7 @@ Description:
 
  What:		/sys/bus/cxl/devices/regionZ/uuid
  Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
  Contact:	[email protected]
  Description:
  		(RW) Write a unique identifier for the region. This field must

@@ -322,7 +322,7 @@ Description:
 
  What:		/sys/bus/cxl/devices/regionZ/interleave_granularity
  Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
  Contact:	[email protected]
  Description:
  		(RW) Set the number of consecutive bytes each device in the

@@ -333,7 +333,7 @@ Description:
 
  What:		/sys/bus/cxl/devices/regionZ/interleave_ways
  Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
  Contact:	[email protected]
  Description:
  		(RW) Configures the number of devices participating in the

@@ -343,7 +343,7 @@ Description:
 
  What:		/sys/bus/cxl/devices/regionZ/size
  Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
  Contact:	[email protected]
  Description:
  		(RW) System physical address space to be consumed by the region.

@@ -360,7 +360,7 @@ Description:
 
  What:		/sys/bus/cxl/devices/regionZ/resource
  Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
  Contact:	[email protected]
  Description:
  		(RO) A region is a contiguous partition of a CXL root decoder

@@ -372,7 +372,7 @@ Description:
 
  What:		/sys/bus/cxl/devices/regionZ/target[0..N]
  Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
  Contact:	[email protected]
  Description:
  		(RW) Write an endpoint decoder object name to 'targetX' where X

@@ -391,7 +391,7 @@ Description:
 
  What:		/sys/bus/cxl/devices/regionZ/commit
  Date:		May, 2022
-KernelVersion:	v5.20
+KernelVersion:	v6.0
  Contact:	[email protected]
  Description:
  		(RW) Write a boolean 'true' string value to this attribute to

Prior to Linus deciding that the kernel that following v5.19 would be v6.0, the CXL ABI documentation already referenced v5.20. In preparation for updating these entries update the kernel version to v6.0.

Signed-off-by Dan Williams <[email protected]>

---
 Documentation/ABI/testing/sysfs-bus-cxl |   30 +++++++++++++++---------------

 1 file changed, 15 insertions(+), 15 deletions(-)

Documentation/ABI/testing/sysfs-bus-cxl +0 -0

index 329a7e46c805..5be032313e29 100644
@@ -198,7 +198,7 @@ Description:

 What:          /sys/bus/cxl/devices/endpointX/CDAT Date:          July, 2022

Documentation/ABI/testing/sysfs-bus-cxl +1 -1

-KernelVersion: v5.20
+KernelVersion: v6.0

 Contact:       [email protected]
 Description:
                (RO) If this sysfs entry is not present no DOE mailbox was

Documentation/ABI/testing/sysfs-bus-cxl +0 -0

@@ -209,7 +209,7 @@ Description:

 What:          /sys/bus/cxl/devices/decoderX.Y/mode Date:          May, 2022

Documentation/ABI/testing/sysfs-bus-cxl +1 -1

-KernelVersion: v5.20
+KernelVersion: v6.0

 Contact:       [email protected]
 Description:
                (RW) When a CXL decoder is of devtype "cxl_decoder_endpoint" it

Documentation/ABI/testing/sysfs-bus-cxl +0 -0

@@ -229,7 +229,7 @@ Description:

 What:          /sys/bus/cxl/devices/decoderX.Y/dpa_resource Date:          May, 2022

Documentation/ABI/testing/sysfs-bus-cxl +1 -1

-KernelVersion: v5.20
+KernelVersion: v6.0

 Contact:       [email protected]
 Description:
                (RO) When a CXL decoder is of devtype "cxl_decoder_endpoint",

Documentation/ABI/testing/sysfs-bus-cxl +0 -0

@@ -240,7 +240,7 @@ Description:

 What:          /sys/bus/cxl/devices/decoderX.Y/dpa_size Date:          May, 2022

Documentation/ABI/testing/sysfs-bus-cxl +1 -1

-KernelVersion: v5.20
+KernelVersion: v6.0

 Contact:       [email protected]
 Description:
                (RW) When a CXL decoder is of devtype "cxl_decoder_endpoint" it

Documentation/ABI/testing/sysfs-bus-cxl +0 -0

@@ -260,7 +260,7 @@ Description:

 What:          /sys/bus/cxl/devices/decoderX.Y/interleave_ways Date:          May, 2022

Documentation/ABI/testing/sysfs-bus-cxl +1 -1

-KernelVersion: v5.20
+KernelVersion: v6.0

 Contact:       [email protected]
 Description:
                (RO) The number of targets across which this decoder's host

Documentation/ABI/testing/sysfs-bus-cxl +0 -0

@@ -275,7 +275,7 @@ Description:

 What:          /sys/bus/cxl/devices/decoderX.Y/interleave_granularity Date:          May, 2022

Documentation/ABI/testing/sysfs-bus-cxl +1 -1

-KernelVersion: v5.20
+KernelVersion: v6.0

 Contact:       [email protected]
 Description:
                (RO) The number of consecutive bytes of host physical address

Documentation/ABI/testing/sysfs-bus-cxl +0 -0

@@ -287,7 +287,7 @@ Description:

 What:          /sys/bus/cxl/devices/decoderX.Y/create_pmem_region Date:          May, 2022

Documentation/ABI/testing/sysfs-bus-cxl +1 -1

-KernelVersion: v5.20
+KernelVersion: v6.0

 Contact:       [email protected]
 Description:
                (RW) Write a string in the form 'regionZ' to start the process

Documentation/ABI/testing/sysfs-bus-cxl +0 -0

@@ -303,7 +303,7 @@ Description:

 What:          /sys/bus/cxl/devices/decoderX.Y/delete_region Date:          May, 2022

Documentation/ABI/testing/sysfs-bus-cxl +1 -1

-KernelVersion: v5.20
+KernelVersion: v6.0

 Contact:       [email protected]
 Description:
                (WO) Write a string in the form 'regionZ' to delete that region,

Documentation/ABI/testing/sysfs-bus-cxl +0 -0

@@ -312,7 +312,7 @@ Description:

 What:          /sys/bus/cxl/devices/regionZ/uuid
 Date:          May, 2022

Documentation/ABI/testing/sysfs-bus-cxl +1 -1

-KernelVersion: v5.20
+KernelVersion: v6.0

 Contact:       [email protected]
 Description:
                (RW) Write a unique identifier for the region. This field must

Documentation/ABI/testing/sysfs-bus-cxl +0 -0

@@ -322,7 +322,7 @@ Description:

 What:          /sys/bus/cxl/devices/regionZ/interleave_granularity Date:          May, 2022

Documentation/ABI/testing/sysfs-bus-cxl +1 -1

-KernelVersion: v5.20
+KernelVersion: v6.0

 Contact:       [email protected]
 Description:
                (RW) Set the number of consecutive bytes each device in the

Documentation/ABI/testing/sysfs-bus-cxl +0 -0

@@ -333,7 +333,7 @@ Description:

 What:          /sys/bus/cxl/devices/regionZ/interleave_ways Date:          May, 2022

Documentation/ABI/testing/sysfs-bus-cxl +1 -1

-KernelVersion: v5.20
+KernelVersion: v6.0

 Contact:       [email protected]
 Description:
                (RW) Configures the number of devices participating in the

Documentation/ABI/testing/sysfs-bus-cxl +0 -0

@@ -343,7 +343,7 @@ Description:

 What:          /sys/bus/cxl/devices/regionZ/size
 Date:          May, 2022

Documentation/ABI/testing/sysfs-bus-cxl +1 -1

-KernelVersion: v5.20
+KernelVersion: v6.0

 Contact:       [email protected]
 Description:
                (RW) System physical address space to be consumed by the region.

Documentation/ABI/testing/sysfs-bus-cxl +0 -0

@@ -360,7 +360,7 @@ Description:

 What:          /sys/bus/cxl/devices/regionZ/resource Date:          May, 2022

Documentation/ABI/testing/sysfs-bus-cxl +1 -1

-KernelVersion: v5.20
+KernelVersion: v6.0

 Contact:       [email protected]
 Description:
                (RO) A region is a contiguous partition of a CXL root decoder

Documentation/ABI/testing/sysfs-bus-cxl +0 -0

@@ -372,7 +372,7 @@ Description:

 What:          /sys/bus/cxl/devices/regionZ/target[0..N] Date:          May, 2022

Documentation/ABI/testing/sysfs-bus-cxl +1 -1

-KernelVersion: v5.20
+KernelVersion: v6.0

 Contact:       [email protected]
 Description:
                (RW) Write an endpoint decoder object name to 'targetX' where X

Documentation/ABI/testing/sysfs-bus-cxl +0 -0

@@ -391,7 +391,7 @@ Description:

 What:          /sys/bus/cxl/devices/regionZ/commit Date:          May, 2022

Documentation/ABI/testing/sysfs-bus-cxl +1 -1

-KernelVersion: v5.20
+KernelVersion: v6.0

 Contact:       [email protected]
 Description:
                (RW) Write a boolean 'true' string value to this attribute to

In preparation for a new region type, "ram" regions, add a mode attribute to clarify the mode of the decoders that can be added to a region. Share the internals of mode_show() (for decoders) with the region case.

Signed-off-by Dan Williams <[email protected]>

---
 Documentation/ABI/testing/sysfs-bus-cxl |   11 +++++++++++
 drivers/cxl/core/port.c                 |   12 +-----------
 drivers/cxl/core/region.c               |   10 ++++++++++
 drivers/cxl/cxl.h                       |   14 ++++++++++++++
 4 files changed, 36 insertions(+), 11 deletions(-)

Documentation/ABI/testing/sysfs-bus-cxl +11 -0

index 5be032313e29..058b0c45001f 100644
@@ -358,6 +358,17 @@ Description:
 		results in the same address being allocated.
 
 
+What:		/sys/bus/cxl/devices/regionZ/mode
+Date:		January, 2023
+KernelVersion:	v6.3
+Contact:	[email protected]
+Description:
+		(RO) The mode of a region is established at region creation time
+		and dictates the mode of the endpoint decoder that comprise the
+		region. For more details on the possible modes see
+		/sys/bus/cxl/devices/decoderX.Y/mode
+
+
 What:		/sys/bus/cxl/devices/regionZ/resource
 Date:		May, 2022
 KernelVersion:	v6.0

drivers/cxl/core/port.c +1 -11

index 410c036c09fa..8566451cb22f 100644
@@ -180,17 +180,7 @@ static ssize_t mode_show(struct device *dev, struct device_attribute *attr,
 {
 	struct cxl_endpoint_decoder *cxled = to_cxl_endpoint_decoder(dev);
 
-	switch (cxled->mode) {
-	case CXL_DECODER_RAM:
-		return sysfs_emit(buf, "ram\n");
-	case CXL_DECODER_PMEM:
-		return sysfs_emit(buf, "pmem\n");
-	case CXL_DECODER_NONE:
-		return sysfs_emit(buf, "none\n");
-	case CXL_DECODER_MIXED:
-	default:
-		return sysfs_emit(buf, "mixed\n");
-	}
+	return sysfs_emit(buf, "%s\n", cxl_decoder_mode_name(cxled->mode));
 }
 
 static ssize_t mode_store(struct device *dev, struct device_attribute *attr,

drivers/cxl/core/region.c +10 -0

index 60828d01972a..17d2d0c12725 100644
@@ -458,6 +458,15 @@ static ssize_t resource_show(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR_RO(resource);
 
+static ssize_t mode_show(struct device *dev, struct device_attribute *attr,
+			 char *buf)
+{
+	struct cxl_region *cxlr = to_cxl_region(dev);
+
+	return sysfs_emit(buf, "%s\n", cxl_decoder_mode_name(cxlr->mode));
+}
+static DEVICE_ATTR_RO(mode);
+
 static int alloc_hpa(struct cxl_region *cxlr, resource_size_t size)
 {
 	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent);

@@ -585,6 +594,7 @@ static struct attribute *cxl_region_attrs[] = {
 	&dev_attr_interleave_granularity.attr,
 	&dev_attr_resource.attr,
 	&dev_attr_size.attr,
+	&dev_attr_mode.attr,
 	NULL,
 };
 

drivers/cxl/cxl.h +14 -0

index aa3af3bb73b2..ca76879af1de 100644
@@ -320,6 +320,20 @@ enum cxl_decoder_mode {
 	CXL_DECODER_DEAD,
 };
 
+static inline const char *cxl_decoder_mode_name(enum cxl_decoder_mode mode)
+{
+	static const char * const names[] = {
+		[CXL_DECODER_NONE] = "none",
+		[CXL_DECODER_RAM] = "ram",
+		[CXL_DECODER_PMEM] = "pmem",
+		[CXL_DECODER_MIXED] = "mixed",
+	};
+
+	if (mode >= CXL_DECODER_NONE && mode <= CXL_DECODER_MIXED)
+		return names[mode];
+	return "mixed";
+}
+
 /**
  * struct cxl_endpoint_decoder - Endpoint  / SPA to DPA decoder
  * @cxld: base cxl_decoder_object

Quoted 5 lines · On Sun, 05 Feb 2023 17:02:40 -0800 Dan Williams <[email protected]>…

In preparation for a new region type, "ram" regions, add a mode attribute to clarify the mode of the decoders that can be added to a region. Share the internals of mode_show() (for decoders) with the region case.

Quoted 1 line

Signed-off-by Dan Williams <[email protected]>

I guess the DEAD decoder cases is an odd enough path that it is fine to not worry that it is presented as mixed (none might be more appropriate as you can't put any memory behind it).

Other than that this is straight forwards exposure of internal data that already existed...

Reviewed-by Jonathan Cameron <[email protected]>

In preparation for a new region type, "ram" regions, add a mode attribute to clarify the mode of the decoders that can be added to a region. Share the internals of mode_show() (for decoders) with the region case.

Signed-off-by Dan Williams <[email protected]>

In preparation for a new region type, "ram" regions, add a mode attribute to clarify the mode of the decoders that can be added to a region. Share the internals of mode_show() (for decoders) with the region case.

Signed-off-by Dan Williams <[email protected]>

---

In preparation for a new region type, "ram" regions, add a mode attribute to clarify the mode of the decoders that can be added to a region. Share the internals of mode_show() (for decoders) with the region case.

Signed-off-by Dan Williams <[email protected]>

---
  Documentation/ABI/testing/sysfs-bus-cxl |   11 +++++++++++
  drivers/cxl/core/port.c                 |   12 +-----------
  drivers/cxl/core/region.c               |   10 ++++++++++
  drivers/cxl/cxl.h                       |   14 ++++++++++++++
  4 files changed, 36 insertions(+), 11 deletions(-)

Documentation/ABI/testing/sysfs-bus-cxl +11 -0

index 5be032313e29..058b0c45001f 100644
@@ -358,6 +358,17 @@ Description:
  		results in the same address being allocated.
 
 
+What:		/sys/bus/cxl/devices/regionZ/mode
+Date:		January, 2023
+KernelVersion:	v6.3
+Contact:	[email protected]
+Description:
+		(RO) The mode of a region is established at region creation time
+		and dictates the mode of the endpoint decoder that comprise the
+		region. For more details on the possible modes see
+		/sys/bus/cxl/devices/decoderX.Y/mode
+
+
  What:		/sys/bus/cxl/devices/regionZ/resource
  Date:		May, 2022
  KernelVersion:	v6.0

drivers/cxl/core/port.c +1 -11

index 410c036c09fa..8566451cb22f 100644
@@ -180,17 +180,7 @@ static ssize_t mode_show(struct device *dev, struct device_attribute *attr,
  {
  	struct cxl_endpoint_decoder *cxled = to_cxl_endpoint_decoder(dev);
 
-	switch (cxled->mode) {
-	case CXL_DECODER_RAM:
-		return sysfs_emit(buf, "ram\n");
-	case CXL_DECODER_PMEM:
-		return sysfs_emit(buf, "pmem\n");
-	case CXL_DECODER_NONE:
-		return sysfs_emit(buf, "none\n");
-	case CXL_DECODER_MIXED:
-	default:
-		return sysfs_emit(buf, "mixed\n");
-	}
+	return sysfs_emit(buf, "%s\n", cxl_decoder_mode_name(cxled->mode));
  }
 
  static ssize_t mode_store(struct device *dev, struct device_attribute *attr,

drivers/cxl/core/region.c +10 -0

index 60828d01972a..17d2d0c12725 100644
@@ -458,6 +458,15 @@ static ssize_t resource_show(struct device *dev, struct device_attribute *attr,
  }
  static DEVICE_ATTR_RO(resource);
 
+static ssize_t mode_show(struct device *dev, struct device_attribute *attr,
+			 char *buf)
+{
+	struct cxl_region *cxlr = to_cxl_region(dev);
+
+	return sysfs_emit(buf, "%s\n", cxl_decoder_mode_name(cxlr->mode));
+}
+static DEVICE_ATTR_RO(mode);
+
  static int alloc_hpa(struct cxl_region *cxlr, resource_size_t size)
  {
  	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent);

@@ -585,6 +594,7 @@ static struct attribute *cxl_region_attrs[] = {
  	&dev_attr_interleave_granularity.attr,
  	&dev_attr_resource.attr,
  	&dev_attr_size.attr,
+	&dev_attr_mode.attr,
  	NULL,
  };
 

drivers/cxl/cxl.h +14 -0

index aa3af3bb73b2..ca76879af1de 100644
@@ -320,6 +320,20 @@ enum cxl_decoder_mode {
  	CXL_DECODER_DEAD,
  };
 
+static inline const char *cxl_decoder_mode_name(enum cxl_decoder_mode mode)
+{
+	static const char * const names[] = {
+		[CXL_DECODER_NONE] = "none",
+		[CXL_DECODER_RAM] = "ram",
+		[CXL_DECODER_PMEM] = "pmem",
+		[CXL_DECODER_MIXED] = "mixed",
+	};
+
+	if (mode >= CXL_DECODER_NONE && mode <= CXL_DECODER_MIXED)
+		return names[mode];
+	return "mixed";
+}
+
  /**
   * struct cxl_endpoint_decoder - Endpoint  / SPA to DPA decoder
   * @cxld: base cxl_decoder_object

In preparation for a new region type, "ram" regions, add a mode attribute to clarify the mode of the decoders that can be added to a region. Share the internals of mode_show() (for decoders) with the region case.

Signed-off-by Dan Williams <[email protected]>

---
 Documentation/ABI/testing/sysfs-bus-cxl |   11 +++++++++++ drivers/cxl/core/port.c                 |   12 +----------- drivers/cxl/core/region.c               |   10 ++++++++++ drivers/cxl/cxl.h                       |   14 ++++++++++++++

 4 files changed, 36 insertions(+), 11 deletions(-)

Documentation/ABI/testing/sysfs-bus-cxl +0 -0

index 5be032313e29..058b0c45001f 100644
@@ -358,6 +358,17 @@ Description:

                results in the same address being allocated.

Documentation/ABI/testing/sysfs-bus-cxl +6 -0

+What:          /sys/bus/cxl/devices/regionZ/mode
+Date:          January, 2023
+KernelVersion: v6.3
+Contact:       [email protected]
+Description:
+               (RO) The mode of a region is established at region

creation time
+               and dictates the mode of the endpoint decoder that comprise the
+               region. For more details on the possible modes see +               /sys/bus/cxl/devices/decoderX.Y/mode

Documentation/ABI/testing/sysfs-bus-cxl +2 -0

+
+

 What:          /sys/bus/cxl/devices/regionZ/resource Date:          May, 2022
 KernelVersion: v6.0

drivers/cxl/core/port.c +0 -0

index 410c036c09fa..8566451cb22f 100644
@@ -180,17 +180,7 @@ static ssize_t mode_show(struct device *dev,
 struct device_attribute *attr,
  {

        struct cxl_endpoint_decoder *cxled = to_cxl_endpoint_decoder(dev);

-       switch (cxled->mode) {
-       case CXL_DECODER_RAM:
-               return sysfs_emit(buf, "ram\n");
-       case CXL_DECODER_PMEM:
-               return sysfs_emit(buf, "pmem\n");
-       case CXL_DECODER_NONE:
-               return sysfs_emit(buf, "none\n");
-       case CXL_DECODER_MIXED:
-       default:
-               return sysfs_emit(buf, "mixed\n");
-       }
+       return sysfs_emit(buf, "%s\n", cxl_decoder_mode_name(cxled-

Quoted 1 line

mode));

 }

 static ssize_t mode_store(struct device *dev, struct device_attribute *attr,

drivers/cxl/core/region.c +0 -0

index 60828d01972a..17d2d0c12725 100644
@@ -458,6 +458,15 @@ static ssize_t resource_show(struct device *dev,
 struct device_attribute *attr,
  }
  static DEVICE_ATTR_RO(resource);

+static ssize_t mode_show(struct device *dev, struct device_attribute *attr,
+                        char *buf)

drivers/cxl/core/region.c +4 -0

+{
+       struct cxl_region *cxlr = to_cxl_region(dev);
+
+       return sysfs_emit(buf, "%s\n", cxl_decoder_mode_name(cxlr-

Quoted 1 line

mode));

drivers/cxl/core/region.c +3 -0

+}
+static DEVICE_ATTR_RO(mode);
+

 static int alloc_hpa(struct cxl_region *cxlr, resource_size_t size) {
        struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr-

Quoted 1 line

dev.parent);

drivers/cxl/core/region.c +1 -0

@@ -585,6 +594,7 @@ static struct attribute *cxl_region_attrs[] = {
         &dev_attr_interleave_granularity.attr,
         &dev_attr_resource.attr,
         &dev_attr_size.attr,
+       &dev_attr_mode.attr,
         NULL,
  };

drivers/cxl/cxl.h +0 -0

index aa3af3bb73b2..ca76879af1de 100644
@@ -320,6 +320,20 @@ enum cxl_decoder_mode {
         CXL_DECODER_DEAD,
  };

+static inline const char *cxl_decoder_mode_name(enum cxl_decoder_mode mode)

drivers/cxl/cxl.h +13 -0

+{
+       static const char * const names[] = {
+               [CXL_DECODER_NONE] = "none",
+               [CXL_DECODER_RAM] = "ram",
+               [CXL_DECODER_PMEM] = "pmem",
+               [CXL_DECODER_MIXED] = "mixed",
+       };
+
+       if (mode >= CXL_DECODER_NONE && mode <= CXL_DECODER_MIXED)
+               return names[mode];
+       return "mixed";
+}
+

 /**
  * struct cxl_endpoint_decoder - Endpoint  / SPA to DPA decoder
  * @cxld: base cxl_decoder_object

Shipping versions of the cxl-cli utility expect all regions to have a 'uuid' attribute. In preparation for 'ram' regions, update the 'uuid' attribute to return an empty string which satisfies the current expectations of 'cxl list -R'. Otherwise, 'cxl list -R' fails in the presence of regions with the 'uuid' attribute missing. Force the attribute to be read-only as there is no facility or expectation for a 'ram' region to recall its uuid from one boot to the next.

Signed-off-by Dan Williams <[email protected]>

---
 Documentation/ABI/testing/sysfs-bus-cxl |    3 ++-
 drivers/cxl/core/region.c               |    7 +++++--
 2 files changed, 7 insertions(+), 3 deletions(-)

Documentation/ABI/testing/sysfs-bus-cxl +2 -1

index 058b0c45001f..4c4e1cbb1169 100644
@@ -317,7 +317,8 @@ Contact:	[email protected]
 Description:
 		(RW) Write a unique identifier for the region. This field must
 		be set for persistent regions and it must not conflict with the
-		UUID of another region.
+		UUID of another region. For volatile ram regions this
+		attribute is a read-only empty string.
 
 
 What:		/sys/bus/cxl/devices/regionZ/interleave_granularity

drivers/cxl/core/region.c +4 -1

index 17d2d0c12725..c9e7f05caa0f 100644
@@ -45,7 +45,10 @@ static ssize_t uuid_show(struct device *dev, struct device_attribute *attr,
 	rc = down_read_interruptible(&cxl_region_rwsem);
 	if (rc)
 		return rc;
-	rc = sysfs_emit(buf, "%pUb\n", &p->uuid);
+	if (cxlr->mode != CXL_DECODER_PMEM)
+		rc = sysfs_emit(buf, "\n");
+	else
+		rc = sysfs_emit(buf, "%pUb\n", &p->uuid);
 	up_read(&cxl_region_rwsem);
 
 	return rc;

drivers/cxl/core/region.c +0 -0

 	return a->mode;
 }
 

Quoted 7 lines · On Sun, 05 Feb 2023 17:02:45 -0800 Dan Williams <[email protected]>…

Shipping versions of the cxl-cli utility expect all regions to have a 'uuid' attribute. In preparation for 'ram' regions, update the 'uuid' attribute to return an empty string which satisfies the current expectations of 'cxl list -R'. Otherwise, 'cxl list -R' fails in the presence of regions with the 'uuid' attribute missing. Force the attribute to be read-only as there is no facility or expectation for a 'ram' region to recall its uuid from one boot to the next.

This is new functionality, so do we need the backwards compatibility? Or does that cxl list -R not just skip these, but fail hard?

It's inelegant to have this visible at all (and it confused me when I was running tests as I assumed I needed to poke something in it).

If nothing else, good to have a comment to remind us in the code why there is the oddity of setting it to read only.

Shipping versions of the cxl-cli utility expect all regions to have a 'uuid' attribute. In preparation for 'ram' regions, update the 'uuid' attribute to return an empty string which satisfies the current expectations of 'cxl list -R'. Otherwise, 'cxl list -R' fails in the presence of regions with the 'uuid' attribute missing.

Force the
attribute to be read-only as there is no facility or expectation for a 'ram' region to recall its uuid from one boot to the next.

Signed-off-by Dan Williams <[email protected]>

---
 Documentation/ABI/testing/sysfs-bus-cxl |    3 ++-
 drivers/cxl/core/region.c               |    7 +++++--
 2 files changed, 7 insertions(+), 3 deletions(-)

Documentation/ABI/testing/sysfs-bus-cxl +2 -1

index 058b0c45001f..4c4e1cbb1169 100644
@@ -317,7 +317,8 @@ Contact:	[email protected]
 Description:
 		(RW) Write a unique identifier for the region. This field must
 		be set for persistent regions and it must not conflict with the
-		UUID of another region.
+		UUID of another region. For volatile ram regions this
+		attribute is a read-only empty string.
 
 
 What:		/sys/bus/cxl/devices/regionZ/interleave_granularity

drivers/cxl/core/region.c +0 -0

index 17d2d0c12725..c9e7f05caa0f 100644

Quoted 5 lines · Dan Williams

Shipping versions of the cxl-cli utility expect all regions to have a 'uuid' attribute. In preparation for 'ram' regions, update the 'uuid' attribute to return an empty string which satisfies the current expectations of 'cxl list -R'. Otherwise, 'cxl list -R' fails in the presence of regions with the 'uuid' attribute missing.

Would this be more appropriate as a change to cxl-cli?

Quoted 4 lines

Force the
attribute to be read-only as there is no facility or expectation for a 'ram' region to recall its uuid from one boot to the next.

This seems reasonable.

Quoted 20 lines

Signed-off-by Dan Williams <[email protected]>

---
 Documentation/ABI/testing/sysfs-bus-cxl |    3 ++-
 drivers/cxl/core/region.c               |    7 +++++--
 2 files changed, 7 insertions(+), 3 deletions(-)

Documentation/ABI/testing/sysfs-bus-cxl +2 -1

index 058b0c45001f..4c4e1cbb1169 100644
@@ -317,7 +317,8 @@ Contact:	[email protected]
 Description:
 		(RW) Write a unique identifier for the region. This field must
 		be set for persistent regions and it must not conflict with the
-		UUID of another region.
+		UUID of another region. For volatile ram regions this
+		attribute is a read-only empty string.
 
 

drivers/cxl/core/region.c static ssize_t uuid_show(struct device *dev, struct device_attribute *attr, · line 48

Quoted 10 lines

Documentation/ABI/testing/sysfs-bus-cxl +0 -0

 What:		/sys/bus/cxl/devices/regionZ/interleave_granularity

drivers/cxl/core/region.c +0 -1

index 17d2d0c12725..c9e7f05caa0f 100644
@@ -45,7 +45,10 @@ static ssize_t uuid_show(struct device *dev, struct device_attribute *attr,
 	rc = down_read_interruptible(&cxl_region_rwsem);
 	if (rc)
 		return rc;
-	rc = sysfs_emit(buf, "%pUb\n", &p->uuid);

I guess it all depends on what p->uuid is... Shouldn't this be all 0's for a ram region? Does sysfs_emit() choke on that?

Shipping versions of the cxl-cli utility expect all regions to have a 'uuid' attribute. In preparation for 'ram' regions, update the 'uuid' attribute to return an empty string which satisfies the current expectations of 'cxl list -R'. Otherwise, 'cxl list -R' fails in the presence of regions with the 'uuid' attribute missing. Force the attribute to be read-only as there is no facility or expectation for a 'ram' region to recall its uuid from one boot to the next.

Signed-off-by Dan Williams <[email protected]>

---
 Documentation/ABI/testing/sysfs-bus-cxl |    3 ++- drivers/cxl/core/region.c               |    7 +++++--

 2 files changed, 7 insertions(+), 3 deletions(-)

Documentation/ABI/testing/sysfs-bus-cxl +0 -0

index 058b0c45001f..4c4e1cbb1169 100644
@@ -317,7 +317,8 @@ Contact:    [email protected]

 Description:
                (RW) Write a unique identifier for the region. This field must be set for persistent regions and it must not conflict with the -               UUID of another region.
+               UUID of another region. For volatile ram regions this +               attribute is a read-only empty string.


 What:          /sys/bus/cxl/devices/regionZ/interleave_granularity

drivers/cxl/core/region.c +4 -1

index 17d2d0c12725..c9e7f05caa0f 100644
@@ -45,7 +45,10 @@ static ssize_t uuid_show(struct device *dev, struct device_attribute *attr,
         rc = down_read_interruptible(&cxl_region_rwsem);
         if (rc)
                 return rc;
-       rc = sysfs_emit(buf, "%pUb\n", &p->uuid);
+       if (cxlr->mode != CXL_DECODER_PMEM)
+               rc = sysfs_emit(buf, "\n");
+       else
+               rc = sysfs_emit(buf, "%pUb\n", &p->uuid);
         up_read(&cxl_region_rwsem);

        return rc;

drivers/cxl/core/region.c +0 -0

@@ -301,7 +304,7 @@ static umode_t cxl_region_visible(struct kobject *kobj, struct attribute *a,
         struct cxl_region *cxlr = to_cxl_region(dev);

        if (a == &dev_attr_uuid.attr && cxlr->mode != CXL_DECODER_PMEM) -               return 0;
+               return 0444;
        return a->mode;
 }

In preparation for a new region mode, do not, for example, allow 'ram' decoders to be assigned to 'pmem' regions and vice versa.

Signed-off-by Dan Williams <[email protected]>

---
 drivers/cxl/core/region.c |    6 ++++++
 1 file changed, 6 insertions(+)

drivers/cxl/core/region.c +6 -0

index c9e7f05caa0f..53d6dbe4de6d 100644
@@ -1217,6 +1217,12 @@ static int cxl_region_attach(struct cxl_region *cxlr,
 	struct cxl_dport *dport;
 	int i, rc = -ENXIO;
 
+	if (cxled->mode != cxlr->mode) {
+		dev_dbg(&cxlr->dev, "%s region mode: %d mismatch: %d\n",
+			dev_name(&cxled->cxld.dev), cxlr->mode, cxled->mode);
+		return -EINVAL;
+	}
+
 	if (cxled->mode == CXL_DECODER_DEAD) {
 		dev_dbg(&cxlr->dev, "%s dead\n", dev_name(&cxled->cxld.dev));
 		return -ENODEV;

Quoted 4 lines · On Sun, 05 Feb 2023 17:02:51 -0800 Dan Williams <[email protected]>…

In preparation for a new region mode, do not, for example, allow 'ram' decoders to be assigned to 'pmem' regions and vice versa.

Signed-off-by Dan Williams <[email protected]>

I guess we aren't support mixed endpoint decoders for now (and maybe never..) If we did I'd expect this to have to allow mixed with either RAM or PMEM.

Reviewed-by Jonathan Cameron <[email protected]>

Quoted 22 lines

---
 drivers/cxl/core/region.c |    6 ++++++
 1 file changed, 6 insertions(+)

drivers/cxl/core/region.c +6 -0

index c9e7f05caa0f..53d6dbe4de6d 100644
@@ -1217,6 +1217,12 @@ static int cxl_region_attach(struct cxl_region *cxlr,
 	struct cxl_dport *dport;
 	int i, rc = -ENXIO;
 
+	if (cxled->mode != cxlr->mode) {
+		dev_dbg(&cxlr->dev, "%s region mode: %d mismatch: %d\n",
+			dev_name(&cxled->cxld.dev), cxlr->mode, cxled->mode);
+		return -EINVAL;
+	}
+
 	if (cxled->mode == CXL_DECODER_DEAD) {
 		dev_dbg(&cxlr->dev, "%s dead\n", dev_name(&cxled->cxld.dev));
 		return -ENODEV;

In preparation for a new region mode, do not, for example, allow 'ram' decoders to be assigned to 'pmem' regions and vice versa.

Signed-off-by Dan Williams <[email protected]>

In preparation for a new region mode, do not, for example, allow 'ram' decoders to be assigned to 'pmem' regions and vice versa.

Signed-off-by Dan Williams <[email protected]>

---
 drivers/cxl/core/region.c |    6 ++++++
 1 file changed, 6 insertions(+)

drivers/cxl/core/region.c +0 -0

index c9e7f05caa0f..53d6dbe4de6d 100644
@@ -1217,6 +1217,12 @@ static int cxl_region_attach(struct cxl_region *cxlr,
 	struct cxl_dport *dport;
 	int i, rc = -ENXIO;

Quoted 16 lines · Dan Williams

In preparation for a new region mode, do not, for example, allow 'ram' decoders to be assigned to 'pmem' regions and vice versa.

Signed-off-by Dan Williams <[email protected]>

---
 drivers/cxl/core/region.c |    6 ++++++
 1 file changed, 6 insertions(+)

drivers/cxl/core/region.c +0 -0

index c9e7f05caa0f..53d6dbe4de6d 100644
@@ -1217,6 +1217,12 @@ static int cxl_region_attach(struct cxl_region *cxlr,
 	struct cxl_dport *dport;
 	int i, rc = -ENXIO;
 

drivers/cxl/core/region.c static int cxl_region_attach(struct cxl_region *cxlr, · line 1228

Quoted 10 lines

drivers/cxl/core/region.c +6 -0

+	if (cxled->mode != cxlr->mode) {
+		dev_dbg(&cxlr->dev, "%s region mode: %d mismatch: %d\n",
+			dev_name(&cxled->cxld.dev), cxlr->mode, cxled->mode);
+		return -EINVAL;
+	}
+
 	if (cxled->mode == CXL_DECODER_DEAD) {
 		dev_dbg(&cxlr->dev, "%s dead\n", dev_name(&cxled->cxld.dev));
 		return -ENODEV;

Maybe a stupid question. It will be entirely possible to "convert" pmem to "ram" (i.e. just online it as system memory).

Are type-3 devices with pmem expected to carry 2 decoders (one ram, 1 pmem), such that you can create a ram region from pmem, or is it expected that the pmem decoder will be used for pmem, even if it gets onlined as volatile?

Just asking because depending on how that flushes out, you might expected a "ram region" to get assigned to a "pmem decoder", unless you're expecting all pmem-carrying-devices to create 2 decoders per pmem region depending on how the end user intends to use it.

Quoted 9 lines · Gregory Price

Quoted 8 lines · Dan Williams

In preparation for a new region mode, do not, for example, allow 'ram' decoders to be assigned to 'pmem' regions and vice versa.

Signed-off-by Dan Williams <[email protected]>

---
 drivers/cxl/core/region.c |    6 ++++++
 1 file changed, 6 insertions(+)

In preparation for a new region mode, do not, for example, allow 'ram' decoders to be assigned to 'pmem' regions and vice versa.

Signed-off-by Dan Williams <[email protected]>

---
  drivers/cxl/core/region.c |    6 ++++++
  1 file changed, 6 insertions(+)

drivers/cxl/core/region.c +6 -0

index c9e7f05caa0f..53d6dbe4de6d 100644
@@ -1217,6 +1217,12 @@ static int cxl_region_attach(struct cxl_region *cxlr,
  	struct cxl_dport *dport;
  	int i, rc = -ENXIO;
 
+	if (cxled->mode != cxlr->mode) {
+		dev_dbg(&cxlr->dev, "%s region mode: %d mismatch: %d\n",
+			dev_name(&cxled->cxld.dev), cxlr->mode, cxled->mode);
+		return -EINVAL;
+	}
+
  	if (cxled->mode == CXL_DECODER_DEAD) {
  		dev_dbg(&cxlr->dev, "%s dead\n", dev_name(&cxled->cxld.dev));
  		return -ENODEV;

In preparation for a new region mode, do not, for example, allow 'ram' decoders to be assigned to 'pmem' regions and vice versa.

Signed-off-by Dan Williams <[email protected]>

---
 drivers/cxl/core/region.c |    6 ++++++

 1 file changed, 6 insertions(+)

drivers/cxl/core/region.c +0 -0

index c9e7f05caa0f..53d6dbe4de6d 100644
@@ -1217,6 +1217,12 @@ static int cxl_region_attach(struct cxl_region *cxlr,
         struct cxl_dport *dport;
         int i, rc = -ENXIO;

+       if (cxled->mode != cxlr->mode) {
+               dev_dbg(&cxlr->dev, "%s region mode: %d mismatch: %d\n", +                       dev_name(&cxled->cxld.dev), cxlr->mode, cxled->mode); +               return -EINVAL;
+       }
+
        if (cxled->mode == CXL_DECODER_DEAD) { dev_dbg(&cxlr->dev, "%s dead\n", dev_name(&cxled->cxld.dev)); return -ENODEV;

Expand the region creation infrastructure to enable 'ram' (volatile-memory) regions. The internals of create_pmem_region_store() and create_pmem_region_show() are factored out into helpers __create_region() and __create_region_show() for the 'ram' case to reuse.

Signed-off-by Dan Williams <[email protected]>

---
 Documentation/ABI/testing/sysfs-bus-cxl |   22 +++++-----
 drivers/cxl/core/core.h                 |    1 
 drivers/cxl/core/port.c                 |   14 ++++++
 drivers/cxl/core/region.c               |   71 +++++++++++++++++++++++++------
 4 files changed, 82 insertions(+), 26 deletions(-)

Documentation/ABI/testing/sysfs-bus-cxl +11 -11

index 4c4e1cbb1169..3acf2f17a73f 100644
@@ -285,20 +285,20 @@ Description:
 		interleave_granularity).
 
 
-What:		/sys/bus/cxl/devices/decoderX.Y/create_pmem_region
-Date:		May, 2022
-KernelVersion:	v6.0
+What:		/sys/bus/cxl/devices/decoderX.Y/create_{pmem,ram}_region
+Date:		May, 2022, January, 2023
+KernelVersion:	v6.0 (pmem), v6.3 (ram)
 Contact:	[email protected]
 Description:
 		(RW) Write a string in the form 'regionZ' to start the process
-		of defining a new persistent memory region (interleave-set)
-		within the decode range bounded by root decoder 'decoderX.Y'.
-		The value written must match the current value returned from
-		reading this attribute. An atomic compare exchange operation is
-		done on write to assign the requested id to a region and
-		allocate the region-id for the next creation attempt. EBUSY is
-		returned if the region name written does not match the current
-		cached value.
+		of defining a new persistent, or volatile memory region
+		(interleave-set) within the decode range bounded by root decoder
+		'decoderX.Y'. The value written must match the current value
+		returned from reading this attribute. An atomic compare exchange
+		operation is done on write to assign the requested id to a
+		region and allocate the region-id for the next creation attempt.
+		EBUSY is returned if the region name written does not match the
+		current cached value.
 
 
 What:		/sys/bus/cxl/devices/decoderX.Y/delete_region

drivers/cxl/core/core.h +1 -0

index 8c04672dca56..5eb873da5a30 100644
@@ -11,6 +11,7 @@ extern struct attribute_group cxl_base_attribute_group;
 
 #ifdef CONFIG_CXL_REGION
 extern struct device_attribute dev_attr_create_pmem_region;
+extern struct device_attribute dev_attr_create_ram_region;
 extern struct device_attribute dev_attr_delete_region;
 extern struct device_attribute dev_attr_region;
 extern const struct device_type cxl_pmem_region_type;

drivers/cxl/core/port.c +13 -1

index 8566451cb22f..47e450c3a5a9 100644
@@ -294,6 +294,7 @@ static struct attribute *cxl_decoder_root_attrs[] = {
 	&dev_attr_cap_type3.attr,
 	&dev_attr_target_list.attr,
 	SET_CXL_REGION_ATTR(create_pmem_region)
+	SET_CXL_REGION_ATTR(create_ram_region)
 	SET_CXL_REGION_ATTR(delete_region)
 	NULL,
 };

@@ -305,6 +306,13 @@ static bool can_create_pmem(struct cxl_root_decoder *cxlrd)
 	return (cxlrd->cxlsd.cxld.flags & flags) == flags;
 }
 
+static bool can_create_ram(struct cxl_root_decoder *cxlrd)
+{
+	unsigned long flags = CXL_DECODER_F_TYPE3 | CXL_DECODER_F_RAM;
+
+	return (cxlrd->cxlsd.cxld.flags & flags) == flags;
+}
+
 static umode_t cxl_root_decoder_visible(struct kobject *kobj, struct attribute *a, int n)
 {
 	struct device *dev = kobj_to_dev(kobj);

@@ -313,7 +321,11 @@ static umode_t cxl_root_decoder_visible(struct kobject *kobj, struct attribute *
 	if (a == CXL_REGION_ATTR(create_pmem_region) && !can_create_pmem(cxlrd))
 		return 0;
 
-	if (a == CXL_REGION_ATTR(delete_region) && !can_create_pmem(cxlrd))
+	if (a == CXL_REGION_ATTR(create_ram_region) && !can_create_ram(cxlrd))
+		return 0;
+
+	if (a == CXL_REGION_ATTR(delete_region) &&
+	    !(can_create_pmem(cxlrd) || can_create_ram(cxlrd)))
 		return 0;
 
 	return a->mode;

drivers/cxl/core/region.c +38 -3

index 53d6dbe4de6d..8dea49c021b8 100644
@@ -1685,6 +1685,15 @@ static struct cxl_region *devm_cxl_add_region(struct cxl_root_decoder *cxlrd,
 	struct device *dev;
 	int rc;
 
+	switch (mode) {
+	case CXL_DECODER_RAM:
+	case CXL_DECODER_PMEM:
+		break;
+	default:
+		dev_err(&cxlrd->cxlsd.cxld.dev, "unsupported mode %d\n", mode);
+		return ERR_PTR(-EINVAL);
+	}
+
 	cxlr = cxl_region_alloc(cxlrd, id);
 	if (IS_ERR(cxlr))
 		return cxlr;

@@ -1713,12 +1722,38 @@ static struct cxl_region *devm_cxl_add_region(struct cxl_root_decoder *cxlrd,
 	return ERR_PTR(rc);
 }
 
+static ssize_t __create_region_show(struct cxl_root_decoder *cxlrd, char *buf)
+{
+	return sysfs_emit(buf, "region%u\n", atomic_read(&cxlrd->region_id));
+}
+
 static ssize_t create_pmem_region_show(struct device *dev,
 				       struct device_attribute *attr, char *buf)
 {
-	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev);
+	return __create_region_show(to_cxl_root_decoder(dev), buf);
+}
 
-	return sysfs_emit(buf, "region%u\n", atomic_read(&cxlrd->region_id));
+static ssize_t create_ram_region_show(struct device *dev,
+				      struct device_attribute *attr, char *buf)
+{
+	return __create_region_show(to_cxl_root_decoder(dev), buf);
+}
+
+static struct cxl_region *__create_region(struct cxl_root_decoder *cxlrd,
+					  enum cxl_decoder_mode mode, int id)
+{
+	int rc;
+
+	rc = memregion_alloc(GFP_KERNEL);
+	if (rc < 0)
+		return ERR_PTR(rc);
+
+	if (atomic_cmpxchg(&cxlrd->region_id, id, rc) != id) {
+		memregion_free(rc);
+		return ERR_PTR(-EBUSY);
+	}
+
+	return devm_cxl_add_region(cxlrd, id, mode, CXL_DECODER_EXPANDER);
 }
 
 static ssize_t create_pmem_region_store(struct device *dev,

@@ -1727,29 +1762,37 @@ static ssize_t create_pmem_region_store(struct device *dev,
 {
 	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev);
 	struct cxl_region *cxlr;
-	int id, rc;
+	int rc, id;
 

drivers/cxl/core/region.c +10 -4

+	return len;
+}
+DEVICE_ATTR_RW(create_pmem_region);
 
-	if (atomic_cmpxchg(&cxlrd->region_id, id, rc) != id) {
-		memregion_free(rc);
-		return -EBUSY;
-	}
+static ssize_t create_ram_region_store(struct device *dev,
+				       struct device_attribute *attr,
+				       const char *buf, size_t len)
+{
+	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev);
+	struct cxl_region *cxlr;
+	int rc, id;
 

drivers/cxl/core/region.c +1 -1

 	return len;
 }
-DEVICE_ATTR_RW(create_pmem_region);
+DEVICE_ATTR_RW(create_ram_region);
 
 static ssize_t region_show(struct device *dev, struct device_attribute *attr,
 			   char *buf)

Quoted 7 lines · On Sun, 05 Feb 2023 17:02:56 -0800 Dan Williams <[email protected]>…

Expand the region creation infrastructure to enable 'ram' (volatile-memory) regions. The internals of create_pmem_region_store() and create_pmem_region_show() are factored out into helpers __create_region() and __create_region_show() for the 'ram' case to reuse.

Signed-off-by Dan Williams <[email protected]>

Entirely trivial comments inline.

Reviewed-by Jonathan Cameron <[email protected]>

Quoted 7 lines

drivers/cxl/core/region.c +1 -1

@@ -1727,29 +1762,37 @@ static ssize_t create_pmem_region_store(struct device *dev,
 {
 	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev);
 	struct cxl_region *cxlr;
-	int id, rc;
+	int rc, id;
 

drivers/cxl/core/region.c static ssize_t create_pmem_region_store(struct device *dev, · line 1773

Quoted 10 lines

drivers/cxl/core/region.c +3 -3

 	rc = sscanf(buf, "region%d\n", &id);
 	if (rc != 1)
 		return -EINVAL;
 
-	rc = memregion_alloc(GFP_KERNEL);
-	if (rc < 0)
-		return rc;
+	cxlr = __create_region(cxlrd, CXL_DECODER_PMEM, id);
+	if (IS_ERR(cxlr))
+		return PTR_ERR(cxlr);

I'd have a blank line here - see below.

drivers/cxl/core/region.c static ssize_t create_pmem_region_store(struct device *dev, · line 1749

Quoted 3 lines

drivers/cxl/core/region.c +0 -1

 	if (IS_ERR(cxlr))
 		return PTR_ERR(cxlr);
-

Just so you know I read to the end!

Spurious unrelated white space change :)

Expand the region creation infrastructure to enable 'ram' (volatile-memory) regions. The internals of create_pmem_region_store() and create_pmem_region_show() are factored out into helpers __create_region() and __create_region_show() for the 'ram' case to reuse.

Signed-off-by Dan Williams <[email protected]>

Expand the region creation infrastructure to enable 'ram' (volatile-memory) regions. The internals of create_pmem_region_store() and create_pmem_region_show() are factored out into helpers __create_region() and __create_region_show() for the 'ram' case to reuse.

Signed-off-by Dan Williams <[email protected]>

---

drivers/cxl/core/port.c +1 -0

index 8566451cb22f..47e450c3a5a9 100644
@@ -294,6 +294,7 @@ static struct attribute *cxl_decoder_root_attrs[] = {
 	&dev_attr_cap_type3.attr,
 	&dev_attr_target_list.attr,
 	SET_CXL_REGION_ATTR(create_pmem_region)
+	SET_CXL_REGION_ATTR(create_ram_region)
 	SET_CXL_REGION_ATTR(delete_region)
 	NULL,
 };

Quoted 9 lines · Dan Williams

Expand the region creation infrastructure to enable 'ram' (volatile-memory) regions. The internals of create_pmem_region_store() and create_pmem_region_show() are factored out into helpers __create_region() and __create_region_show() for the 'ram' case to reuse.

Signed-off-by Dan Williams <[email protected]>

---

drivers/cxl/core/port.c static bool can_create_pmem(struct cxl_root_decoder *cxlrd) · line 315

Quoted 11 lines

drivers/cxl/core/port.c +7 -0

@@ -305,6 +306,13 @@ static bool can_create_pmem(struct cxl_root_decoder *cxlrd)
 	return (cxlrd->cxlsd.cxld.flags & flags) == flags;
 }
 
+static bool can_create_ram(struct cxl_root_decoder *cxlrd)
+{
+	unsigned long flags = CXL_DECODER_F_TYPE3 | CXL_DECODER_F_RAM;
+
+	return (cxlrd->cxlsd.cxld.flags & flags) == flags;
+}
+

does this collide with either CXL_DECODE_F_ENABLE or CXL_DECODER_F_AUTO?

I think obviously if it's already enabled creating new regions in a decoder doesn't make sense, but if F_AUTO is set, does that imply the region settings cannot be changed?

Quoted 10 lines · Gregory Price

Quoted 9 lines · Dan Williams

Expand the region creation infrastructure to enable 'ram' (volatile-memory) regions. The internals of create_pmem_region_store() and create_pmem_region_show() are factored out into helpers __create_region() and __create_region_show() for the 'ram' case to reuse.

Signed-off-by Dan Williams <[email protected]>

---

[..]

Quoted 14 lines

drivers/cxl/core/port.c static bool can_create_pmem(struct cxl_root_decoder *cxlrd) · line 315

Quoted 11 lines

drivers/cxl/core/port.c +7 -0

@@ -305,6 +306,13 @@ static bool can_create_pmem(struct cxl_root_decoder *cxlrd)
 	return (cxlrd->cxlsd.cxld.flags & flags) == flags;
 }
 
+static bool can_create_ram(struct cxl_root_decoder *cxlrd)
+{
+	unsigned long flags = CXL_DECODER_F_TYPE3 | CXL_DECODER_F_RAM;
+
+	return (cxlrd->cxlsd.cxld.flags & flags) == flags;
+}
+

does this collide with either CXL_DECODE_F_ENABLE or CXL_DECODER_F_AUTO?

Quoted 3 lines

I think obviously if it's already enabled creating new regions in a decoder doesn't make sense, but if F_AUTO is set, does that imply the region settings cannot be changed?

That just cares if the root decoder supports TYPE3 and RAM independent of ENABLE or AUTO. Root decoders are always enabled. The AUTO flag, which is not applicable to root decoders, is just there to hold off userspace racing the attachment of endpoint decoders to a region until the autodiscovery process has completed. Once that completes and the region has been enabled then it can be destroyed to clear AUTO.

Expand the region creation infrastructure to enable 'ram' (volatile-memory) regions. The internals of create_pmem_region_store() and create_pmem_region_show() are factored out into helpers __create_region() and __create_region_show() for the 'ram' case to reuse.

Signed-off-by Dan Williams <[email protected]>

---
  Documentation/ABI/testing/sysfs-bus-cxl |   22 +++++-----
  drivers/cxl/core/core.h                 |    1
  drivers/cxl/core/port.c                 |   14 ++++++
  drivers/cxl/core/region.c               |   71 +++++++++++++++++++++++++------
  4 files changed, 82 insertions(+), 26 deletions(-)

Documentation/ABI/testing/sysfs-bus-cxl +3 -8

index 4c4e1cbb1169..3acf2f17a73f 100644
@@ -285,20 +285,20 @@ Description:
  		interleave_granularity).
 
 
-What:		/sys/bus/cxl/devices/decoderX.Y/create_pmem_region
-Date:		May, 2022
-KernelVersion:	v6.0
+What:		/sys/bus/cxl/devices/decoderX.Y/create_{pmem,ram}_region
+Date:		May, 2022, January, 2023
+KernelVersion:	v6.0 (pmem), v6.3 (ram)
  Contact:	[email protected]
  Description:
  		(RW) Write a string in the form 'regionZ' to start the process
-		of defining a new persistent memory region (interleave-set)
-		within the decode range bounded by root decoder 'decoderX.Y'.
-		The value written must match the current value returned from
-		reading this attribute. An atomic compare exchange operation is
-		done on write to assign the requested id to a region and

Documentation/ABI/testing/sysfs-bus-cxl +1 -0

+		current cached value.
 
 
  What:		/sys/bus/cxl/devices/decoderX.Y/delete_region

drivers/cxl/core/core.h +1 -0

index 8c04672dca56..5eb873da5a30 100644
@@ -11,6 +11,7 @@ extern struct attribute_group cxl_base_attribute_group;
 
  #ifdef CONFIG_CXL_REGION
  extern struct device_attribute dev_attr_create_pmem_region;
+extern struct device_attribute dev_attr_create_ram_region;
  extern struct device_attribute dev_attr_delete_region;
  extern struct device_attribute dev_attr_region;
  extern const struct device_type cxl_pmem_region_type;

drivers/cxl/core/port.c +13 -1

index 8566451cb22f..47e450c3a5a9 100644
@@ -294,6 +294,7 @@ static struct attribute *cxl_decoder_root_attrs[] = {
  	&dev_attr_cap_type3.attr,
  	&dev_attr_target_list.attr,
  	SET_CXL_REGION_ATTR(create_pmem_region)
+	SET_CXL_REGION_ATTR(create_ram_region)
  	SET_CXL_REGION_ATTR(delete_region)
  	NULL,
  };

@@ -305,6 +306,13 @@ static bool can_create_pmem(struct cxl_root_decoder *cxlrd)
  	return (cxlrd->cxlsd.cxld.flags & flags) == flags;
  }
 
+static bool can_create_ram(struct cxl_root_decoder *cxlrd)
+{
+	unsigned long flags = CXL_DECODER_F_TYPE3 | CXL_DECODER_F_RAM;
+
+	return (cxlrd->cxlsd.cxld.flags & flags) == flags;
+}
+
  static umode_t cxl_root_decoder_visible(struct kobject *kobj, struct attribute *a, int n)
  {
  	struct device *dev = kobj_to_dev(kobj);

@@ -313,7 +321,11 @@ static umode_t cxl_root_decoder_visible(struct kobject *kobj, struct attribute *
  	if (a == CXL_REGION_ATTR(create_pmem_region) && !can_create_pmem(cxlrd))
  		return 0;
 
-	if (a == CXL_REGION_ATTR(delete_region) && !can_create_pmem(cxlrd))
+	if (a == CXL_REGION_ATTR(create_ram_region) && !can_create_ram(cxlrd))
+		return 0;
+
+	if (a == CXL_REGION_ATTR(delete_region) &&
+	    !(can_create_pmem(cxlrd) || can_create_ram(cxlrd)))
  		return 0;
 
  	return a->mode;

drivers/cxl/core/region.c +57 -14

index 53d6dbe4de6d..8dea49c021b8 100644
@@ -1685,6 +1685,15 @@ static struct cxl_region *devm_cxl_add_region(struct cxl_root_decoder *cxlrd,
  	struct device *dev;
  	int rc;
 
+	switch (mode) {
+	case CXL_DECODER_RAM:
+	case CXL_DECODER_PMEM:
+		break;
+	default:
+		dev_err(&cxlrd->cxlsd.cxld.dev, "unsupported mode %d\n", mode);
+		return ERR_PTR(-EINVAL);
+	}
+
  	cxlr = cxl_region_alloc(cxlrd, id);
  	if (IS_ERR(cxlr))
  		return cxlr;

@@ -1713,12 +1722,38 @@ static struct cxl_region *devm_cxl_add_region(struct cxl_root_decoder *cxlrd,
  	return ERR_PTR(rc);
  }
 
+static ssize_t __create_region_show(struct cxl_root_decoder *cxlrd, char *buf)
+{
+	return sysfs_emit(buf, "region%u\n", atomic_read(&cxlrd->region_id));
+}
+
  static ssize_t create_pmem_region_show(struct device *dev,
  				       struct device_attribute *attr, char *buf)
  {
-	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev);
+	return __create_region_show(to_cxl_root_decoder(dev), buf);
+}
 
-	return sysfs_emit(buf, "region%u\n", atomic_read(&cxlrd->region_id));
+static ssize_t create_ram_region_show(struct device *dev,
+				      struct device_attribute *attr, char *buf)
+{
+	return __create_region_show(to_cxl_root_decoder(dev), buf);
+}
+
+static struct cxl_region *__create_region(struct cxl_root_decoder *cxlrd,
+					  enum cxl_decoder_mode mode, int id)
+{
+	int rc;
+
+	rc = memregion_alloc(GFP_KERNEL);
+	if (rc < 0)
+		return ERR_PTR(rc);
+
+	if (atomic_cmpxchg(&cxlrd->region_id, id, rc) != id) {
+		memregion_free(rc);
+		return ERR_PTR(-EBUSY);
+	}
+
+	return devm_cxl_add_region(cxlrd, id, mode, CXL_DECODER_EXPANDER);
  }
 
  static ssize_t create_pmem_region_store(struct device *dev,

@@ -1727,29 +1762,37 @@ static ssize_t create_pmem_region_store(struct device *dev,
  {
  	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev);
  	struct cxl_region *cxlr;
-	int id, rc;
+	int rc, id;
 
  	rc = sscanf(buf, "region%d\n", &id);
  	if (rc != 1)
  		return -EINVAL;
 
-	rc = memregion_alloc(GFP_KERNEL);
-	if (rc < 0)
-		return rc;
+	cxlr = __create_region(cxlrd, CXL_DECODER_PMEM, id);
+	if (IS_ERR(cxlr))
+		return PTR_ERR(cxlr);
+	return len;
+}
+DEVICE_ATTR_RW(create_pmem_region);
 
-	if (atomic_cmpxchg(&cxlrd->region_id, id, rc) != id) {
-		memregion_free(rc);
-		return -EBUSY;
-	}
+static ssize_t create_ram_region_store(struct device *dev,
+				       struct device_attribute *attr,
+				       const char *buf, size_t len)
+{
+	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev);
+	struct cxl_region *cxlr;
+	int rc, id;
 
-	cxlr = devm_cxl_add_region(cxlrd, id, CXL_DECODER_PMEM,
-				   CXL_DECODER_EXPANDER);
+	rc = sscanf(buf, "region%d\n", &id);
+	if (rc != 1)
+		return -EINVAL;
+
+	cxlr = __create_region(cxlrd, CXL_DECODER_RAM, id);
  	if (IS_ERR(cxlr))
  		return PTR_ERR(cxlr);
-
  	return len;
  }
-DEVICE_ATTR_RW(create_pmem_region);
+DEVICE_ATTR_RW(create_ram_region);
 
  static ssize_t region_show(struct device *dev, struct device_attribute *attr,
  			   char *buf)

Quoted 7 lines · Dan Williams

Expand the region creation infrastructure to enable 'ram' (volatile-memory) regions. The internals of create_pmem_region_store() and create_pmem_region_show() are factored out into helpers __create_region() and __create_region_show() for the 'ram' case to reuse.

Signed-off-by Dan Williams <[email protected]>

Minor comment below, otherwise:

Reviewed-by Dave Jiang <[email protected]>

Quoted 26 lines

Documentation/ABI/testing/sysfs-bus-cxl +4 -11

index 4c4e1cbb1169..3acf2f17a73f 100644
@@ -285,20 +285,20 @@ Description:
  		interleave_granularity).
 
 
-What:		/sys/bus/cxl/devices/decoderX.Y/create_pmem_region
-Date:		May, 2022
-KernelVersion:	v6.0
+What:		/sys/bus/cxl/devices/decoderX.Y/create_{pmem,ram}_region
+Date:		May, 2022, January, 2023
+KernelVersion:	v6.0 (pmem), v6.3 (ram)
  Contact:	[email protected]
  Description:
  		(RW) Write a string in the form 'regionZ' to start the process
-		of defining a new persistent memory region (interleave-set)
-		within the decode range bounded by root decoder 'decoderX.Y'.
-		The value written must match the current value returned from
-		reading this attribute. An atomic compare exchange operation is
-		done on write to assign the requested id to a region and
-		allocate the region-id for the next creation attempt. EBUSY is
-		returned if the region name written does not match the current
-		cached value.
+		of defining a new persistent, or volatile memory region

Documentation/ABI/testing/sysfs-bus-cxl Description: · line 300

Quoted 6 lines

Documentation/ABI/testing/sysfs-bus-cxl +6 -0

+		(interleave-set) within the decode range bounded by root decoder
+		'decoderX.Y'. The value written must match the current value
+		returned from reading this attribute. An atomic compare exchange
+		operation is done on write to assign the requested id to a
+		region and allocate the region-id for the next creation attempt.
+		EBUSY is returned if the region name written does not match the

-EBUSY?

Expand the region creation infrastructure to enable 'ram' (volatile-memory) regions. The internals of create_pmem_region_store() and create_pmem_region_show() are factored out into helpers __create_region() and __create_region_show() for the 'ram' case to reuse.

Signed-off-by Dan Williams <[email protected]>

---
 Documentation/ABI/testing/sysfs-bus-cxl |   22 +++++----- drivers/cxl/core/core.h                 |    1 drivers/cxl/core/port.c                 |   14 ++++++ drivers/cxl/core/region.c               |   71 +++++++++++++++++++++++++------

 4 files changed, 82 insertions(+), 26 deletions(-)

Documentation/ABI/testing/sysfs-bus-cxl +0 -0

index 4c4e1cbb1169..3acf2f17a73f 100644
@@ -285,20 +285,20 @@ Description:

                interleave_granularity).

Documentation/ABI/testing/sysfs-bus-cxl +3 -3

-What:          /sys/bus/cxl/devices/decoderX.Y/create_pmem_region
-Date:          May, 2022
-KernelVersion: v6.0
+What:          /sys/bus/cxl/devices/decoderX.Y/create_{pmem,ram}_region
+Date:          May, 2022, January, 2023
+KernelVersion: v6.0 (pmem), v6.3 (ram)

 Contact:       [email protected]
 Description:
                (RW) Write a string in the form 'regionZ' to start the process -               of defining a new persistent memory region (interleave-set) -               within the decode range bounded by root decoder 'decoderX.Y'. -               The value written must match the current value returned from -               reading this attribute. An atomic compare exchange operation is -               done on write to assign the requested id to a region and -               allocate the region-id for the next creation attempt. EBUSY is -               returned if the region name written does not match the current -               cached value.
+               of defining a new persistent, or volatile memory region +               (interleave-set) within the decode range bounded by root decoder +               'decoderX.Y'. The value written must match the current value +               returned from reading this attribute. An atomic compare exchange +               operation is done on write to assign the requested id to a +               region and allocate the region-id for the next creation attempt. +               EBUSY is returned if the region name written does not match the +               current cached value.


 What:          /sys/bus/cxl/devices/decoderX.Y/delete_region

drivers/cxl/core/core.h +0 -0

index 8c04672dca56..5eb873da5a30 100644
@@ -11,6 +11,7 @@ extern struct attribute_group cxl_base_attribute_group;

 #ifdef CONFIG_CXL_REGION
 extern struct device_attribute dev_attr_create_pmem_region; +extern struct device_attribute dev_attr_create_ram_region; extern struct device_attribute dev_attr_delete_region; extern struct device_attribute dev_attr_region; extern const struct device_type cxl_pmem_region_type;

drivers/cxl/core/port.c +1 -0

index 8566451cb22f..47e450c3a5a9 100644
@@ -294,6 +294,7 @@ static struct attribute *cxl_decoder_root_attrs[] = {
         &dev_attr_cap_type3.attr,
         &dev_attr_target_list.attr,
         SET_CXL_REGION_ATTR(create_pmem_region)
+       SET_CXL_REGION_ATTR(create_ram_region)
         SET_CXL_REGION_ATTR(delete_region)
         NULL,
  };

@@ -305,6 +306,13 @@ static bool can_create_pmem(struct cxl_root_decoder *cxlrd)
         return (cxlrd->cxlsd.cxld.flags & flags) == flags;
  }

drivers/cxl/core/port.c +7 -0

+static bool can_create_ram(struct cxl_root_decoder *cxlrd)
+{
+       unsigned long flags = CXL_DECODER_F_TYPE3 | CXL_DECODER_F_RAM;
+
+       return (cxlrd->cxlsd.cxld.flags & flags) == flags;
+}
+

 static umode_t cxl_root_decoder_visible(struct kobject *kobj, struct attribute *a, int n) {
        struct device *dev = kobj_to_dev(kobj);

drivers/cxl/core/port.c +0 -0

@@ -313,7 +321,11 @@ static umode_t cxl_root_decoder_visible(struct kobject *kobj, struct attribute *
         if (a == CXL_REGION_ATTR(create_pmem_region) && !can_create_pmem(cxlrd))
                 return 0;

-       if (a == CXL_REGION_ATTR(delete_region) && !can_create_pmem(cxlrd)) +       if (a == CXL_REGION_ATTR(create_ram_region) && !can_create_ram(cxlrd)) +               return 0;
+
+       if (a == CXL_REGION_ATTR(delete_region) &&
+           !(can_create_pmem(cxlrd) || can_create_ram(cxlrd))) return 0;

        return a->mode;

drivers/cxl/core/region.c +0 -0

index 53d6dbe4de6d..8dea49c021b8 100644
@@ -1685,6 +1685,15 @@ static struct cxl_region *devm_cxl_add_region(struct cxl_root_decoder *cxlrd,
         struct device *dev;
         int rc;

+       switch (mode) {
+       case CXL_DECODER_RAM:
+       case CXL_DECODER_PMEM:
+               break;
+       default:
+               dev_err(&cxlrd->cxlsd.cxld.dev, "unsupported mode %d\n", mode); +               return ERR_PTR(-EINVAL);
+       }
+
        cxlr = cxl_region_alloc(cxlrd, id);
        if (IS_ERR(cxlr))
                return cxlr;

drivers/cxl/core/region.c +0 -0

@@ -1713,12 +1722,38 @@ static struct cxl_region *devm_cxl_add_region(struct cxl_root_decoder *cxlrd,
         return ERR_PTR(rc);
  }

drivers/cxl/core/region.c +5 -0

+static ssize_t __create_region_show(struct cxl_root_decoder *cxlrd, char *buf)
+{
+       return sysfs_emit(buf, "region%u\n", atomic_read(&cxlrd->region_id));
+}
+

 static ssize_t create_pmem_region_show(struct device *dev,
                                       struct device_attribute *attr, char *buf) {
-       struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev); +       return __create_region_show(to_cxl_root_decoder(dev), buf); +}

-       return sysfs_emit(buf, "region%u\n", atomic_read(&cxlrd->region_id));

drivers/cxl/core/region.c +21 -0

+static ssize_t create_ram_region_show(struct device *dev,
+                                     struct device_attribute *attr, char *buf)
+{
+       return __create_region_show(to_cxl_root_decoder(dev), buf);
+}
+
+static struct cxl_region *__create_region(struct cxl_root_decoder *cxlrd,
+                                         enum cxl_decoder_mode mode, int id)
+{
+       int rc;
+
+       rc = memregion_alloc(GFP_KERNEL);
+       if (rc < 0)
+               return ERR_PTR(rc);
+
+       if (atomic_cmpxchg(&cxlrd->region_id, id, rc) != id) {
+               memregion_free(rc);
+               return ERR_PTR(-EBUSY);
+       }
+
+       return devm_cxl_add_region(cxlrd, id, mode, CXL_DECODER_EXPANDER);

 }

 static ssize_t create_pmem_region_store(struct device *dev,

drivers/cxl/core/region.c +1 -1

@@ -1727,29 +1762,37 @@ static ssize_t create_pmem_region_store(struct device *dev,
  {
         struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev);
         struct cxl_region *cxlr;
-       int id, rc;
+       int rc, id;

        rc = sscanf(buf, "region%d\n", &id);
        if (rc != 1)
                return -EINVAL;

-       rc = memregion_alloc(GFP_KERNEL);
-       if (rc < 0)
-               return rc;
+       cxlr = __create_region(cxlrd, CXL_DECODER_PMEM, id); +       if (IS_ERR(cxlr))
+               return PTR_ERR(cxlr);
+       return len;

drivers/cxl/core/region.c +2 -0

+}
+DEVICE_ATTR_RW(create_pmem_region);

-       if (atomic_cmpxchg(&cxlrd->region_id, id, rc) != id) { -               memregion_free(rc);
-               return -EBUSY;
-       }

drivers/cxl/core/region.c +7 -0

+static ssize_t create_ram_region_store(struct device *dev,
+                                      struct device_attribute *attr,
+                                      const char *buf, size_t len)
+{
+       struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev);
+       struct cxl_region *cxlr;
+       int rc, id;

-       cxlr = devm_cxl_add_region(cxlrd, id, CXL_DECODER_PMEM, -                                  CXL_DECODER_EXPANDER); +       rc = sscanf(buf, "region%d\n", &id);
+       if (rc != 1)
+               return -EINVAL;
+
+       cxlr = __create_region(cxlrd, CXL_DECODER_RAM, id); if (IS_ERR(cxlr))
                return PTR_ERR(cxlr);
-
        return len;
 }

drivers/cxl/core/region.c +1 -1

-DEVICE_ATTR_RW(create_pmem_region);
+DEVICE_ATTR_RW(create_ram_region);

 static ssize_t region_show(struct device *dev, struct device_attribute *attr, char *buf)

Region autodiscovery is the process of kernel creating 'struct cxl_region' object to represent active CXL memory ranges it finds already active in hardware when the driver loads. Typically this happens when platform firmware establishes CXL memory regions and then publishes them in the memory map. However, this can also happen in the case of kexec-reboot after the kernel has created regions.

In the autodiscovery case the region creation process starts with a known endpoint decoder. Refactor attach_target() into a helper that is suitable to be called from either sysfs, for runtime region creation, or from cxl_port_probe() after it has enumerated all endpoint decoders.

The cxl_port_probe() context is an async device-core probing context, so it is not appropriate to allow SIGTERM to interrupt the assembly process. Refactor attach_target() to take @cxled and @state as arguments where @state indicates whether waiting from the region rwsem is interruptible or not.

No behavior change is intended.

Signed-off-by Dan Williams <[email protected]>

---
 drivers/cxl/core/region.c |   47 +++++++++++++++++++++++++++------------------
 1 file changed, 28 insertions(+), 19 deletions(-)

drivers/cxl/core/region.c +3 -9

index 8dea49c021b8..97eafdd75675 100644
@@ -1418,31 +1418,25 @@ void cxl_decoder_kill_region(struct cxl_endpoint_decoder *cxled)
 	up_write(&cxl_region_rwsem);
 }
 
-static int attach_target(struct cxl_region *cxlr, const char *decoder, int pos)
+static int attach_target(struct cxl_region *cxlr,
+			 struct cxl_endpoint_decoder *cxled, int pos,
+			 unsigned int state)
 {
-	struct device *dev;
-	int rc;
-
-	dev = bus_find_device_by_name(&cxl_bus_type, NULL, decoder);
-	if (!dev)
-		return -ENODEV;
-
-	if (!is_endpoint_decoder(dev)) {

drivers/cxl/core/region.c +20 -6

 	if (rc)
-		goto out;
+		return rc;
+
 	down_read(&cxl_dpa_rwsem);
-	rc = cxl_region_attach(cxlr, to_cxl_endpoint_decoder(dev), pos);
+	rc = cxl_region_attach(cxlr, cxled, pos);
 	if (rc == 0)
 		set_bit(CXL_REGION_F_INCOHERENT, &cxlr->flags);
 	up_read(&cxl_dpa_rwsem);
 	up_write(&cxl_region_rwsem);
-out:
-	put_device(dev);
 	return rc;
 }
 

@@ -1480,8 +1474,23 @@ static size_t store_targetN(struct cxl_region *cxlr, const char *buf, int pos,
 
 	if (sysfs_streq(buf, "\n"))
 		rc = detach_target(cxlr, pos);
-	else
-		rc = attach_target(cxlr, buf, pos);
+	else {
+		struct device *dev;
+
+		dev = bus_find_device_by_name(&cxl_bus_type, NULL, buf);
+		if (!dev)
+			return -ENODEV;
+
+		if (!is_endpoint_decoder(dev)) {
+			rc = -EINVAL;
+			goto out;
+		}
+
+		rc = attach_target(cxlr, to_cxl_endpoint_decoder(dev), pos,
+				   TASK_INTERRUPTIBLE);
+out:
+		put_device(dev);
+	}
 
 	if (rc < 0)
 		return rc;

Quoted 17 lines · On Sun, 05 Feb 2023 17:03:02 -0800 Dan Williams <[email protected]>…

Region autodiscovery is the process of kernel creating 'struct cxl_region' object to represent active CXL memory ranges it finds already active in hardware when the driver loads. Typically this happens when platform firmware establishes CXL memory regions and then publishes them in the memory map. However, this can also happen in the case of kexec-reboot after the kernel has created regions.

In the autodiscovery case the region creation process starts with a known endpoint decoder. Refactor attach_target() into a helper that is suitable to be called from either sysfs, for runtime region creation, or from cxl_port_probe() after it has enumerated all endpoint decoders.

The cxl_port_probe() context is an async device-core probing context, so it is not appropriate to allow SIGTERM to interrupt the assembly process. Refactor attach_target() to take @cxled and @state as arguments where @state indicates whether waiting from the region rwsem is interruptible or not.

Quoted 22 lines

drivers/cxl/core/region.c +4 -12

@@ -1418,31 +1418,25 @@ void cxl_decoder_kill_region(struct cxl_endpoint_decoder *cxled)
 	up_write(&cxl_region_rwsem);
 }
 
-static int attach_target(struct cxl_region *cxlr, const char *decoder, int pos)
+static int attach_target(struct cxl_region *cxlr,
+			 struct cxl_endpoint_decoder *cxled, int pos,
+			 unsigned int state)
 {
-	struct device *dev;
-	int rc;
-
-	dev = bus_find_device_by_name(&cxl_bus_type, NULL, decoder);
-	if (!dev)
-		return -ENODEV;
-
-	if (!is_endpoint_decoder(dev)) {
-		put_device(dev);
-		return -EINVAL;
-	}
+	int rc = 0;
 

drivers/cxl/core/region.c void cxl_decoder_kill_region(struct cxl_endpoint_decoder *cxled) · line 1430

Quoted 5 lines

drivers/cxl/core/region.c +4 -1

-	rc = down_write_killable(&cxl_region_rwsem);
+	if (state == TASK_INTERRUPTIBLE)
+		rc = down_write_killable(&cxl_region_rwsem);
+	else
+		down_write(&cxl_region_rwsem);

I'd be tempted to do this in two hops for patch readability. First make the code reorg then follow up with this bit before the use of it in the next patch.

Region autodiscovery is the process of kernel creating 'struct cxl_region' object to represent active CXL memory ranges it finds already active in hardware when the driver loads. Typically this happens when platform firmware establishes CXL memory regions and then publishes them in the memory map. However, this can also happen in the case of kexec-reboot after the kernel has created regions.

In the autodiscovery case the region creation process starts with a known endpoint decoder. Refactor attach_target() into a helper that is suitable to be called from either sysfs, for runtime region creation, or from cxl_port_probe() after it has enumerated all endpoint decoders.

The cxl_port_probe() context is an async device-core probing context, so it is not appropriate to allow SIGTERM to interrupt the assembly process. Refactor attach_target() to take @cxled and @state as arguments where @state indicates whether waiting from the region rwsem is interruptible or not.

No behavior change is intended.

Signed-off-by Dan Williams <[email protected]>

Region autodiscovery is the process of kernel creating 'struct cxl_region' object to represent active CXL memory ranges it finds already active in hardware when the driver loads. Typically this happens when platform firmware establishes CXL memory regions and then publishes them in the memory map. However, this can also happen in the case of kexec-reboot after the kernel has created regions.

In the autodiscovery case the region creation process starts with a known endpoint decoder. Refactor attach_target() into a helper that is suitable to be called from either sysfs, for runtime region creation, or from cxl_port_probe() after it has enumerated all endpoint decoders.

The cxl_port_probe() context is an async device-core probing context, so it is not appropriate to allow SIGTERM to interrupt the assembly process. Refactor attach_target() to take @cxled and @state as arguments where @state indicates whether waiting from the region rwsem is interruptible or not.

No behavior change is intended.

Signed-off-by Dan Williams <[email protected]>

---
  drivers/cxl/core/region.c |   47 +++++++++++++++++++++++++++------------------
  1 file changed, 28 insertions(+), 19 deletions(-)

drivers/cxl/core/region.c +28 -19

index 8dea49c021b8..97eafdd75675 100644
@@ -1418,31 +1418,25 @@ void cxl_decoder_kill_region(struct cxl_endpoint_decoder *cxled)
  	up_write(&cxl_region_rwsem);
  }
 
-static int attach_target(struct cxl_region *cxlr, const char *decoder, int pos)
+static int attach_target(struct cxl_region *cxlr,
+			 struct cxl_endpoint_decoder *cxled, int pos,
+			 unsigned int state)
  {
-	struct device *dev;
-	int rc;
-
-	dev = bus_find_device_by_name(&cxl_bus_type, NULL, decoder);
-	if (!dev)
-		return -ENODEV;
-
-	if (!is_endpoint_decoder(dev)) {
-		put_device(dev);
-		return -EINVAL;
-	}
+	int rc = 0;
 
-	rc = down_write_killable(&cxl_region_rwsem);
+	if (state == TASK_INTERRUPTIBLE)
+		rc = down_write_killable(&cxl_region_rwsem);
+	else
+		down_write(&cxl_region_rwsem);
  	if (rc)
-		goto out;
+		return rc;
+
  	down_read(&cxl_dpa_rwsem);
-	rc = cxl_region_attach(cxlr, to_cxl_endpoint_decoder(dev), pos);
+	rc = cxl_region_attach(cxlr, cxled, pos);
  	if (rc == 0)
  		set_bit(CXL_REGION_F_INCOHERENT, &cxlr->flags);
  	up_read(&cxl_dpa_rwsem);
  	up_write(&cxl_region_rwsem);
-out:
-	put_device(dev);
  	return rc;
  }
 

@@ -1480,8 +1474,23 @@ static size_t store_targetN(struct cxl_region *cxlr, const char *buf, int pos,
 
  	if (sysfs_streq(buf, "\n"))
  		rc = detach_target(cxlr, pos);
-	else
-		rc = attach_target(cxlr, buf, pos);
+	else {
+		struct device *dev;
+
+		dev = bus_find_device_by_name(&cxl_bus_type, NULL, buf);
+		if (!dev)
+			return -ENODEV;
+
+		if (!is_endpoint_decoder(dev)) {
+			rc = -EINVAL;
+			goto out;
+		}
+
+		rc = attach_target(cxlr, to_cxl_endpoint_decoder(dev), pos,
+				   TASK_INTERRUPTIBLE);
+out:
+		put_device(dev);
+	}
 
  	if (rc < 0)
  		return rc;

Region autodiscovery is the process of kernel creating 'struct cxl_region' object to represent active CXL memory ranges it finds already active in hardware when the driver loads. Typically this happens when platform firmware establishes CXL memory regions and then publishes them in the memory map. However, this can also happen in the case of kexec-reboot after the kernel has created regions.

In the autodiscovery case the region creation process starts with a known endpoint decoder. Refactor attach_target() into a helper that is suitable to be called from either sysfs, for runtime region creation, or from cxl_port_probe() after it has enumerated all endpoint decoders.

The cxl_port_probe() context is an async device-core probing context, so it is not appropriate to allow SIGTERM to interrupt the assembly process. Refactor attach_target() to take @cxled and @state as arguments where @state indicates whether waiting from the region rwsem is interruptible or not.

No behavior change is intended.

Signed-off-by Dan Williams <[email protected]>

---
 drivers/cxl/core/region.c |   47 +++++++++++++++++++++++++++------------------

 1 file changed, 28 insertions(+), 19 deletions(-)

drivers/cxl/core/region.c +0 -0

index 8dea49c021b8..97eafdd75675 100644
@@ -1418,31 +1418,25 @@ void cxl_decoder_kill_region(struct cxl_endpoint_decoder *cxled)
         up_write(&cxl_region_rwsem);
  }

drivers/cxl/core/region.c +3 -1

-static int attach_target(struct cxl_region *cxlr, const char *decoder, int pos)
+static int attach_target(struct cxl_region *cxlr,
+                        struct cxl_endpoint_decoder *cxled, int pos,
+                        unsigned int state)

 {
-       struct device *dev;
-       int rc;

drivers/cxl/core/region.c +1 -9

-
-       dev = bus_find_device_by_name(&cxl_bus_type, NULL, decoder);
-       if (!dev)
-               return -ENODEV;
-
-       if (!is_endpoint_decoder(dev)) {
-               put_device(dev);
-               return -EINVAL;
-       }
+       int rc = 0;

-       rc = down_write_killable(&cxl_region_rwsem);
+       if (state == TASK_INTERRUPTIBLE)
+               rc = down_write_killable(&cxl_region_rwsem); +       else
+               down_write(&cxl_region_rwsem);
        if (rc)
-               goto out;
+               return rc;
+
        down_read(&cxl_dpa_rwsem);
-       rc = cxl_region_attach(cxlr, to_cxl_endpoint_decoder(dev), pos); +       rc = cxl_region_attach(cxlr, cxled, pos);
        if (rc == 0)
                set_bit(CXL_REGION_F_INCOHERENT, &cxlr->flags); up_read(&cxl_dpa_rwsem); up_write(&cxl_region_rwsem);
-out:
-       put_device(dev);
        return rc;
 }

drivers/cxl/core/region.c +0 -0

@@ -1480,8 +1474,23 @@ static size_t store_targetN(struct cxl_region *cxlr, const char *buf, int pos,

        if (sysfs_streq(buf, "\n"))
                rc = detach_target(cxlr, pos); -       else
-               rc = attach_target(cxlr, buf, pos); +       else {
+               struct device *dev;

drivers/cxl/core/region.c +15 -0

+
+               dev = bus_find_device_by_name(&cxl_bus_type, NULL, buf);
+               if (!dev)
+                       return -ENODEV;
+
+               if (!is_endpoint_decoder(dev)) {
+                       rc = -EINVAL;
+                       goto out;
+               }
+
+               rc = attach_target(cxlr, to_cxl_endpoint_decoder(dev), pos,
+                                  TASK_INTERRUPTIBLE);
+out:
+               put_device(dev);
+       }

        if (rc < 0)
                return rc;

In preparation for region autodiscovery, that needs all devices discovered before their relative position in the region can be determined, consolidate all position dependent validation in a helper.

Recall that in the on-demand region creation flow the end-user picks the position of a given endpoint decoder in a region. In the autodiscovery case the position of an endpoint decoder can only be determined after all other endpoint decoders that claim to decode the region's address range have been enumerated and attached. So, in the autodiscovery case endpoint decoders may be attached before their relative position is known. Once all decoders arrive, then positions can be determined and validated with cxl_region_validate_position() the same as user initiated on-demand creation.

Signed-off-by Dan Williams <[email protected]>

---
 drivers/cxl/core/region.c |  119 +++++++++++++++++++++++++++++----------------
 1 file changed, 76 insertions(+), 43 deletions(-)

drivers/cxl/core/region.c +0 -0

index 97eafdd75675..c82d3b6f3d1f 100644
@@ -1207,35 +1207,13 @@ static int cxl_region_setup_targets(struct cxl_region *cxlr)
 	return 0;
 }
 

drivers/cxl/core/region.c +8 -0

@@ -1274,6 +1252,71 @@ static int cxl_region_attach(struct cxl_region *cxlr,
 		}
 	}
 
+	return 0;
+}
+
+static int cxl_region_attach_position(struct cxl_region *cxlr,
+				      struct cxl_root_decoder *cxlrd,
+				      struct cxl_endpoint_decoder *cxled,
+				      const struct cxl_dport *dport, int pos)
+{

Quoted 27 lines · Dan Williams

In preparation for region autodiscovery, that needs all devices discovered before their relative position in the region can be determined, consolidate all position dependent validation in a helper.

Recall that in the on-demand region creation flow the end-user picks the position of a given endpoint decoder in a region. In the autodiscovery case the position of an endpoint decoder can only be determined after all other endpoint decoders that claim to decode the region's address range have been enumerated and attached. So, in the autodiscovery case endpoint decoders may be attached before their relative position is known. Once all decoders arrive, then positions can be determined and validated with cxl_region_validate_position() the same as user initiated on-demand creation.

Signed-off-by Dan Williams <[email protected]>

---
 drivers/cxl/core/region.c |  119 +++++++++++++++++++++++++++++----------------
 1 file changed, 76 insertions(+), 43 deletions(-)

drivers/cxl/core/region.c +0 -0

index 97eafdd75675..c82d3b6f3d1f 100644
@@ -1207,35 +1207,13 @@ static int cxl_region_setup_targets(struct cxl_region *cxlr)
 	return 0;
 }
 

[snip]

Quoted 19 lines

drivers/cxl/core/region.c +15 -0

@@ -1274,6 +1252,71 @@ static int cxl_region_attach(struct cxl_region *cxlr,
 		}
 	}
 
+	return 0;
+}
+
+static int cxl_region_attach_position(struct cxl_region *cxlr,
+				      struct cxl_root_decoder *cxlrd,
+				      struct cxl_endpoint_decoder *cxled,
+				      const struct cxl_dport *dport, int pos)
+{
+	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
+	struct cxl_port *iter;
+	int rc;
+
+	if (cxlrd->calc_hb(cxlrd, pos) != dport) {
+		dev_dbg(&cxlr->dev, "%s:%s invalid target position for %s\n",
+			dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev),

drivers/cxl/core/region.c static int cxl_region_attach(struct cxl_region *cxlr, · line 1272

Quoted 3 lines

drivers/cxl/core/region.c +3 -0

+			dev_name(&cxlrd->cxlsd.cxld.dev));
+		return -ENXIO;
+	}

I think I know the answer but I'm curious why this check is not part of validating the position? Is it because this is validating the position relative to the hostbridge which is not strictly part of the region validation?

In preparation for region autodiscovery, that needs all devices discovered before their relative position in the region can be determined, consolidate all position dependent validation in a helper.

Recall that in the on-demand region creation flow the end-user picks the position of a given endpoint decoder in a region. In the autodiscovery case the position of an endpoint decoder can only be determined after all other endpoint decoders that claim to decode the region's address range have been enumerated and attached. So, in the autodiscovery case endpoint decoders may be attached before their relative position is known. Once all decoders arrive, then positions can be determined and validated with cxl_region_validate_position() the same as user initiated on-demand creation.

Signed-off-by Dan Williams <[email protected]>

drivers/cxl/core/region.c +30 -0

+static int cxl_region_attach(struct cxl_region *cxlr,
+			     struct cxl_endpoint_decoder *cxled, int pos)
+{
+	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent);
+	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
+	struct cxl_region_params *p = &cxlr->params;
+	struct cxl_port *ep_port, *root_port;
+	struct cxl_dport *dport;
+	int rc = -ENXIO;
+
+	if (cxled->mode != cxlr->mode) {
+		dev_dbg(&cxlr->dev, "%s region mode: %d mismatch: %d\n",
+			dev_name(&cxled->cxld.dev), cxlr->mode, cxled->mode);
+		return -EINVAL;
+	}
+
+	if (cxled->mode == CXL_DECODER_DEAD) {
+		dev_dbg(&cxlr->dev, "%s dead\n", dev_name(&cxled->cxld.dev));
+		return -ENODEV;
+	}
+
+	/* all full of members, or interleave config not established? */
+	if (p->state > CXL_CONFIG_INTERLEAVE_ACTIVE) {
+		dev_dbg(&cxlr->dev, "region already active\n");
+		return -EBUSY;
+	} else if (p->state < CXL_CONFIG_INTERLEAVE_ACTIVE) {
+		dev_dbg(&cxlr->dev, "interleave config missing\n");
+		return -ENXIO;
+	}
+
 	ep_port = cxled_to_port(cxled);
 	root_port = cxlrd_to_port(cxlrd);
 	dport = cxl_find_dport_by_dev(root_port, ep_port->host_bridge);

drivers/cxl/core/region.c +0 -6

 	if (cxled->cxld.target_type != cxlr->type) {
 		dev_dbg(&cxlr->dev, "%s:%s type mismatch: %d vs %d\n",
 			dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev),

@@ -1314,12 +1350,13 @@ static int cxl_region_attach(struct cxl_region *cxlr,
 		return -EINVAL;
 	}
 
-	for (iter = ep_port; !is_cxl_root(iter);
-	     iter = to_cxl_port(iter->dev.parent)) {
-		rc = cxl_port_attach_region(iter, cxlr, cxled, pos);
-		if (rc)
-			goto err;
-	}

drivers/cxl/core/region.c +0 -4

@@ -1343,10 +1380,6 @@ static int cxl_region_attach(struct cxl_region *cxlr,
 
 err_decrement:
 	p->nr_targets--;
-err:
-	for (iter = ep_port; !is_cxl_root(iter);
-	     iter = to_cxl_port(iter->dev.parent))
-		cxl_port_detach_region(iter, cxlr, cxled);
 	return rc;
 }
 

Quoted 14 lines · On Sun, 05 Feb 2023 17:03:07 -0800 Dan Williams <[email protected]>…

In preparation for region autodiscovery, that needs all devices discovered before their relative position in the region can be determined, consolidate all position dependent validation in a helper.

Recall that in the on-demand region creation flow the end-user picks the position of a given endpoint decoder in a region. In the autodiscovery case the position of an endpoint decoder can only be determined after all other endpoint decoders that claim to decode the region's address range have been enumerated and attached. So, in the autodiscovery case endpoint decoders may be attached before their relative position is known. Once all decoders arrive, then positions can be determined and validated with cxl_region_validate_position() the same as user initiated on-demand creation.

Quoted 1 line

Signed-off-by Dan Williams <[email protected]>

Hi Dan,

A few comments inline, but mostly reflect the original code rather than the refactoring you have done in this patch.

Jonathan

Quoted 34 lines

drivers/cxl/core/region.c +30 -0

+static int cxl_region_attach(struct cxl_region *cxlr,
+			     struct cxl_endpoint_decoder *cxled, int pos)
+{
+	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent);
+	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
+	struct cxl_region_params *p = &cxlr->params;
+	struct cxl_port *ep_port, *root_port;
+	struct cxl_dport *dport;
+	int rc = -ENXIO;
+
+	if (cxled->mode != cxlr->mode) {
+		dev_dbg(&cxlr->dev, "%s region mode: %d mismatch: %d\n",
+			dev_name(&cxled->cxld.dev), cxlr->mode, cxled->mode);
+		return -EINVAL;
+	}
+
+	if (cxled->mode == CXL_DECODER_DEAD) {
+		dev_dbg(&cxlr->dev, "%s dead\n", dev_name(&cxled->cxld.dev));
+		return -ENODEV;
+	}
+
+	/* all full of members, or interleave config not established? */
+	if (p->state > CXL_CONFIG_INTERLEAVE_ACTIVE) {
+		dev_dbg(&cxlr->dev, "region already active\n");
+		return -EBUSY;
+	} else if (p->state < CXL_CONFIG_INTERLEAVE_ACTIVE) {
+		dev_dbg(&cxlr->dev, "interleave config missing\n");
+		return -ENXIO;
+	}
+
 	ep_port = cxled_to_port(cxled);
 	root_port = cxlrd_to_port(cxlrd);
 	dport = cxl_find_dport_by_dev(root_port, ep_port->host_bridge);

drivers/cxl/core/region.c static int cxl_region_attach(struct cxl_region *cxlr, · line 1293

Quoted 10 lines

drivers/cxl/core/region.c +0 -7

@@ -1284,13 +1327,6 @@ static int cxl_region_attach(struct cxl_region *cxlr,
 		return -ENXIO;
 	}
 
-	if (cxlrd->calc_hb(cxlrd, pos) != dport) {
-		dev_dbg(&cxlr->dev, "%s:%s invalid target position for %s\n",
-			dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev),
-			dev_name(&cxlrd->cxlsd.cxld.dev));
-		return -ENXIO;
-	}
-

In an ideal world, this would have been nice as two patches. One that reorders the various checks so that they are in the order after you have factored things out (easy to review for correctness) then one that factored it out.

Quoted 13 lines

drivers/cxl/core/region.c +0 -6

 	if (cxled->cxld.target_type != cxlr->type) {
 		dev_dbg(&cxlr->dev, "%s:%s type mismatch: %d vs %d\n",
 			dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev),

@@ -1314,12 +1350,13 @@ static int cxl_region_attach(struct cxl_region *cxlr,
 		return -EINVAL;
 	}
 
-	for (iter = ep_port; !is_cxl_root(iter);
-	     iter = to_cxl_port(iter->dev.parent)) {
-		rc = cxl_port_attach_region(iter, cxlr, cxled, pos);
-		if (rc)
-			goto err;
-	}

drivers/cxl/core/region.c static int cxl_region_attach(struct cxl_region *cxlr, · line 1362

Quoted 10 lines

drivers/cxl/core/region.c +7 -0

+	rc = cxl_region_validate_position(cxlr, cxled, pos);
+	if (rc)
+		return rc;
+
+	rc = cxl_region_attach_position(cxlr, cxlrd, cxled, dport, pos);
+	if (rc)
+		return rc;
 
 	p->targets[pos] = cxled;
 	cxled->pos = pos;

More something about original code than the refactoring...