[PATCH] cxl: Rename ACPI_CEDT_CFMWS_RESTRICT_TYPE2/TYPE3

[PATCH] cxl: Rename ACPI_CEDT_CFMWS_RESTRICT_TYPE2/TYPE3

[Huang Ying]

According to the description of the "Window Restrictions" field of
"CFMWS Structure" in the CXL spec v3.1 section 9.18.1.3: CXL Fixed
Memory Window Structure (CFMWS), the bit 0 of "Window Restrictions" is
formerly known as "CXL Type 2 Memory" and renamed to "Device
Coherent", while the bit 1 is formerly known as "CXL Type 3 Memory"
and renamed to "Host-only Coherent".  Because type 3 memory can only
be host-only coherent before, while it can be host-only coherent or
device coherent with "Back-Invalidate" now.

To avoid confusion about type 2/3 memory and device/host-only coherent
in Linux kernel, the patch renames corresponding bit definition from
ACPI_CEDT_CFMWS_RESTRICT_TYPE2/TYPE3 to
ACPI_CEDT_CFMWS_RESTRICT_DEVMEM/HOSTONLYMEM.  This makes the kernel
code consistent with the spec too.

The patch also renames the corresponding cxl_decoder flags
CXL_DECODER_F_TYPE2/TYPE3 to CXL_DECODER_F_DEVMEM/HOSTONLYMEM.

No functionality change is expected.

Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
Suggested-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
Reviewed-by: Gregory Price <gourry@gourry.net>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Dave Jiang <dave.jiang@intel.com>
Cc: Alison Schofield <alison.schofield@intel.com>
Cc: Vishal Verma <vishal.l.verma@intel.com>
Cc: Ira Weiny <ira.weiny@intel.com>
Cc: Alejandro Lucero <alucerop@amd.com>
Cc: Ben Cheatham <benjamin.cheatham@amd.com>
---
 drivers/cxl/acpi.c           |  8 ++++----
 drivers/cxl/core/port.c      |  8 ++++----
 drivers/cxl/cxl.h            | 14 +++++++-------
 include/acpi/actbl1.h        | 10 +++++-----
 tools/testing/cxl/test/cxl.c | 18 +++++++++---------
 5 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
index 82b78e331d8e..aca8cbb7540d 100644
--- a/drivers/cxl/acpi.c
+++ b/drivers/cxl/acpi.c
@@ -115,10 +115,10 @@ static unsigned long cfmws_to_decoder_flags(int restrictions)
 {
 	unsigned long flags = CXL_DECODER_F_ENABLE;
 
-	if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_TYPE2)
-		flags |= CXL_DECODER_F_TYPE2;
-	if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_TYPE3)
-		flags |= CXL_DECODER_F_TYPE3;
+	if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_DEVMEM)
+		flags |= CXL_DECODER_F_DEVMEM;
+	if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM)
+		flags |= CXL_DECODER_F_HOSTONLYMEM;
 	if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_VOLATILE)
 		flags |= CXL_DECODER_F_RAM;
 	if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_PMEM)
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index a5e6f3d23cfb..8524714968fd 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -125,8 +125,8 @@ static DEVICE_ATTR_RO(name)
 
 CXL_DECODER_FLAG_ATTR(cap_pmem, CXL_DECODER_F_PMEM);
 CXL_DECODER_FLAG_ATTR(cap_ram, CXL_DECODER_F_RAM);
-CXL_DECODER_FLAG_ATTR(cap_type2, CXL_DECODER_F_TYPE2);
-CXL_DECODER_FLAG_ATTR(cap_type3, CXL_DECODER_F_TYPE3);
+CXL_DECODER_FLAG_ATTR(cap_type2, CXL_DECODER_F_DEVMEM);
+CXL_DECODER_FLAG_ATTR(cap_type3, CXL_DECODER_F_HOSTONLYMEM);
 CXL_DECODER_FLAG_ATTR(locked, CXL_DECODER_F_LOCK);
 
 static ssize_t target_type_show(struct device *dev,
@@ -326,14 +326,14 @@ static struct attribute *cxl_decoder_root_attrs[] = {
 
 static bool can_create_pmem(struct cxl_root_decoder *cxlrd)
 {
-	unsigned long flags = CXL_DECODER_F_TYPE3 | CXL_DECODER_F_PMEM;
+	unsigned long flags = CXL_DECODER_F_HOSTONLYMEM | CXL_DECODER_F_PMEM;
 
 	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;
+	unsigned long flags = CXL_DECODER_F_HOSTONLYMEM | CXL_DECODER_F_RAM;
 
 	return (cxlrd->cxlsd.cxld.flags & flags) == flags;
 }
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 0fc96f8bf15c..b9083ce1cf74 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -315,13 +315,13 @@ resource_size_t cxl_rcd_component_reg_phys(struct device *dev,
  * Additionally indicate whether decoder settings were autodetected,
  * user customized.
  */
-#define CXL_DECODER_F_RAM   BIT(0)
-#define CXL_DECODER_F_PMEM  BIT(1)
-#define CXL_DECODER_F_TYPE2 BIT(2)
-#define CXL_DECODER_F_TYPE3 BIT(3)
-#define CXL_DECODER_F_LOCK  BIT(4)
-#define CXL_DECODER_F_ENABLE    BIT(5)
-#define CXL_DECODER_F_MASK  GENMASK(5, 0)
+#define CXL_DECODER_F_RAM         BIT(0)
+#define CXL_DECODER_F_PMEM        BIT(1)
+#define CXL_DECODER_F_DEVMEM      BIT(2)
+#define CXL_DECODER_F_HOSTONLYMEM BIT(3)
+#define CXL_DECODER_F_LOCK        BIT(4)
+#define CXL_DECODER_F_ENABLE      BIT(5)
+#define CXL_DECODER_F_MASK        GENMASK(5, 0)
 
 enum cxl_decoder_type {
 	CXL_DECODER_DEVMEM = 2,
diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h
index 199afc2cd122..e195909928df 100644
--- a/include/acpi/actbl1.h
+++ b/include/acpi/actbl1.h
@@ -551,11 +551,11 @@ struct acpi_cedt_cfmws_target_element {
 
 /* Values for Restrictions field above */
 
-#define ACPI_CEDT_CFMWS_RESTRICT_TYPE2      (1)
-#define ACPI_CEDT_CFMWS_RESTRICT_TYPE3      (1<<1)
-#define ACPI_CEDT_CFMWS_RESTRICT_VOLATILE   (1<<2)
-#define ACPI_CEDT_CFMWS_RESTRICT_PMEM       (1<<3)
-#define ACPI_CEDT_CFMWS_RESTRICT_FIXED      (1<<4)
+#define ACPI_CEDT_CFMWS_RESTRICT_DEVMEM        (1)
+#define ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM   (1<<1)
+#define ACPI_CEDT_CFMWS_RESTRICT_VOLATILE      (1<<2)
+#define ACPI_CEDT_CFMWS_RESTRICT_PMEM          (1<<3)
+#define ACPI_CEDT_CFMWS_RESTRICT_FIXED         (1<<4)
 
 /* 2: CXL XOR Interleave Math Structure */
 
diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
index 90d5afd52dd0..9d919fc99f6b 100644
--- a/tools/testing/cxl/test/cxl.c
+++ b/tools/testing/cxl/test/cxl.c
@@ -209,7 +209,7 @@ static struct {
 			},
 			.interleave_ways = 0,
 			.granularity = 4,
-			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
+			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
 					ACPI_CEDT_CFMWS_RESTRICT_VOLATILE,
 			.qtg_id = FAKE_QTG_ID,
 			.window_size = SZ_256M * 4UL,
@@ -224,7 +224,7 @@ static struct {
 			},
 			.interleave_ways = 1,
 			.granularity = 4,
-			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
+			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
 					ACPI_CEDT_CFMWS_RESTRICT_VOLATILE,
 			.qtg_id = FAKE_QTG_ID,
 			.window_size = SZ_256M * 8UL,
@@ -239,7 +239,7 @@ static struct {
 			},
 			.interleave_ways = 0,
 			.granularity = 4,
-			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
+			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
 					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
 			.qtg_id = FAKE_QTG_ID,
 			.window_size = SZ_256M * 4UL,
@@ -254,7 +254,7 @@ static struct {
 			},
 			.interleave_ways = 1,
 			.granularity = 4,
-			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
+			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
 					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
 			.qtg_id = FAKE_QTG_ID,
 			.window_size = SZ_256M * 8UL,
@@ -269,7 +269,7 @@ static struct {
 			},
 			.interleave_ways = 0,
 			.granularity = 4,
-			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
+			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
 					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
 			.qtg_id = FAKE_QTG_ID,
 			.window_size = SZ_256M * 4UL,
@@ -284,7 +284,7 @@ static struct {
 			},
 			.interleave_ways = 0,
 			.granularity = 4,
-			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
+			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
 					ACPI_CEDT_CFMWS_RESTRICT_VOLATILE,
 			.qtg_id = FAKE_QTG_ID,
 			.window_size = SZ_256M,
@@ -301,7 +301,7 @@ static struct {
 			.interleave_arithmetic = ACPI_CEDT_CFMWS_ARITHMETIC_XOR,
 			.interleave_ways = 0,
 			.granularity = 4,
-			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
+			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
 					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
 			.qtg_id = FAKE_QTG_ID,
 			.window_size = SZ_256M * 8UL,
@@ -317,7 +317,7 @@ static struct {
 			.interleave_arithmetic = ACPI_CEDT_CFMWS_ARITHMETIC_XOR,
 			.interleave_ways = 1,
 			.granularity = 0,
-			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
+			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
 					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
 			.qtg_id = FAKE_QTG_ID,
 			.window_size = SZ_256M * 8UL,
@@ -333,7 +333,7 @@ static struct {
 			.interleave_arithmetic = ACPI_CEDT_CFMWS_ARITHMETIC_XOR,
 			.interleave_ways = 2,
 			.granularity = 0,
-			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
+			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
 					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
 			.qtg_id = FAKE_QTG_ID,
 			.window_size = SZ_256M * 16UL,
-- 
2.39.2

Re: [PATCH] cxl: Rename ACPI_CEDT_CFMWS_RESTRICT_TYPE2/TYPE3

[Ira Weiny]

Huang Ying wrote:
> According to the description of the "Window Restrictions" field of
> "CFMWS Structure" in the CXL spec v3.1 section 9.18.1.3: CXL Fixed
> Memory Window Structure (CFMWS), the bit 0 of "Window Restrictions" is
> formerly known as "CXL Type 2 Memory" and renamed to "Device
> Coherent", while the bit 1 is formerly known as "CXL Type 3 Memory"
> and renamed to "Host-only Coherent".  Because type 3 memory can only
> be host-only coherent before, while it can be host-only coherent or
> device coherent with "Back-Invalidate" now.
> 
> To avoid confusion about type 2/3 memory and device/host-only coherent
> in Linux kernel, the patch renames corresponding bit definition from
> ACPI_CEDT_CFMWS_RESTRICT_TYPE2/TYPE3 to
> ACPI_CEDT_CFMWS_RESTRICT_DEVMEM/HOSTONLYMEM.  This makes the kernel
> code consistent with the spec too.
> 
> The patch also renames the corresponding cxl_decoder flags
> CXL_DECODER_F_TYPE2/TYPE3 to CXL_DECODER_F_DEVMEM/HOSTONLYMEM.
> 
> No functionality change is expected.
> 
> Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
> Suggested-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
> Reviewed-by: Gregory Price <gourry@gourry.net>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Dave Jiang <dave.jiang@intel.com>
> Cc: Alison Schofield <alison.schofield@intel.com>
> Cc: Vishal Verma <vishal.l.verma@intel.com>
> Cc: Ira Weiny <ira.weiny@intel.com>

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

Re: [PATCH] cxl: Rename ACPI_CEDT_CFMWS_RESTRICT_TYPE2/TYPE3

[Fan Ni]

On Mon, Nov 04, 2024 at 04:41:10PM +0800, Huang Ying wrote:
> According to the description of the "Window Restrictions" field of
> "CFMWS Structure" in the CXL spec v3.1 section 9.18.1.3: CXL Fixed
> Memory Window Structure (CFMWS), the bit 0 of "Window Restrictions" is
> formerly known as "CXL Type 2 Memory" and renamed to "Device
> Coherent", while the bit 1 is formerly known as "CXL Type 3 Memory"
> and renamed to "Host-only Coherent".  Because type 3 memory can only
> be host-only coherent before, while it can be host-only coherent or
> device coherent with "Back-Invalidate" now.
> 
> To avoid confusion about type 2/3 memory and device/host-only coherent
> in Linux kernel, the patch renames corresponding bit definition from
> ACPI_CEDT_CFMWS_RESTRICT_TYPE2/TYPE3 to
> ACPI_CEDT_CFMWS_RESTRICT_DEVMEM/HOSTONLYMEM.  This makes the kernel
> code consistent with the spec too.
> 
> The patch also renames the corresponding cxl_decoder flags
> CXL_DECODER_F_TYPE2/TYPE3 to CXL_DECODER_F_DEVMEM/HOSTONLYMEM.
> 
> No functionality change is expected.
> 
> Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
> Suggested-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
> Reviewed-by: Gregory Price <gourry@gourry.net>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Dave Jiang <dave.jiang@intel.com>
> Cc: Alison Schofield <alison.schofield@intel.com>
> Cc: Vishal Verma <vishal.l.verma@intel.com>
> Cc: Ira Weiny <ira.weiny@intel.com>
> Cc: Alejandro Lucero <alucerop@amd.com>
> Cc: Ben Cheatham <benjamin.cheatham@amd.com>
> ---

Reviewed-by: Fan Ni <fan.ni@samsung.com>

>  drivers/cxl/acpi.c           |  8 ++++----
>  drivers/cxl/core/port.c      |  8 ++++----
>  drivers/cxl/cxl.h            | 14 +++++++-------
>  include/acpi/actbl1.h        | 10 +++++-----
>  tools/testing/cxl/test/cxl.c | 18 +++++++++---------
>  5 files changed, 29 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index 82b78e331d8e..aca8cbb7540d 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -115,10 +115,10 @@ static unsigned long cfmws_to_decoder_flags(int restrictions)
>  {
>  	unsigned long flags = CXL_DECODER_F_ENABLE;
>  
> -	if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_TYPE2)
> -		flags |= CXL_DECODER_F_TYPE2;
> -	if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_TYPE3)
> -		flags |= CXL_DECODER_F_TYPE3;
> +	if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_DEVMEM)
> +		flags |= CXL_DECODER_F_DEVMEM;
> +	if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM)
> +		flags |= CXL_DECODER_F_HOSTONLYMEM;
>  	if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_VOLATILE)
>  		flags |= CXL_DECODER_F_RAM;
>  	if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_PMEM)
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index a5e6f3d23cfb..8524714968fd 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -125,8 +125,8 @@ static DEVICE_ATTR_RO(name)
>  
>  CXL_DECODER_FLAG_ATTR(cap_pmem, CXL_DECODER_F_PMEM);
>  CXL_DECODER_FLAG_ATTR(cap_ram, CXL_DECODER_F_RAM);
> -CXL_DECODER_FLAG_ATTR(cap_type2, CXL_DECODER_F_TYPE2);
> -CXL_DECODER_FLAG_ATTR(cap_type3, CXL_DECODER_F_TYPE3);
> +CXL_DECODER_FLAG_ATTR(cap_type2, CXL_DECODER_F_DEVMEM);
> +CXL_DECODER_FLAG_ATTR(cap_type3, CXL_DECODER_F_HOSTONLYMEM);
>  CXL_DECODER_FLAG_ATTR(locked, CXL_DECODER_F_LOCK);
>  
>  static ssize_t target_type_show(struct device *dev,
> @@ -326,14 +326,14 @@ static struct attribute *cxl_decoder_root_attrs[] = {
>  
>  static bool can_create_pmem(struct cxl_root_decoder *cxlrd)
>  {
> -	unsigned long flags = CXL_DECODER_F_TYPE3 | CXL_DECODER_F_PMEM;
> +	unsigned long flags = CXL_DECODER_F_HOSTONLYMEM | CXL_DECODER_F_PMEM;
>  
>  	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;
> +	unsigned long flags = CXL_DECODER_F_HOSTONLYMEM | CXL_DECODER_F_RAM;
>  
>  	return (cxlrd->cxlsd.cxld.flags & flags) == flags;
>  }
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 0fc96f8bf15c..b9083ce1cf74 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -315,13 +315,13 @@ resource_size_t cxl_rcd_component_reg_phys(struct device *dev,
>   * Additionally indicate whether decoder settings were autodetected,
>   * user customized.
>   */
> -#define CXL_DECODER_F_RAM   BIT(0)
> -#define CXL_DECODER_F_PMEM  BIT(1)
> -#define CXL_DECODER_F_TYPE2 BIT(2)
> -#define CXL_DECODER_F_TYPE3 BIT(3)
> -#define CXL_DECODER_F_LOCK  BIT(4)
> -#define CXL_DECODER_F_ENABLE    BIT(5)
> -#define CXL_DECODER_F_MASK  GENMASK(5, 0)
> +#define CXL_DECODER_F_RAM         BIT(0)
> +#define CXL_DECODER_F_PMEM        BIT(1)
> +#define CXL_DECODER_F_DEVMEM      BIT(2)
> +#define CXL_DECODER_F_HOSTONLYMEM BIT(3)
> +#define CXL_DECODER_F_LOCK        BIT(4)
> +#define CXL_DECODER_F_ENABLE      BIT(5)
> +#define CXL_DECODER_F_MASK        GENMASK(5, 0)
>  
>  enum cxl_decoder_type {
>  	CXL_DECODER_DEVMEM = 2,
> diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h
> index 199afc2cd122..e195909928df 100644
> --- a/include/acpi/actbl1.h
> +++ b/include/acpi/actbl1.h
> @@ -551,11 +551,11 @@ struct acpi_cedt_cfmws_target_element {
>  
>  /* Values for Restrictions field above */
>  
> -#define ACPI_CEDT_CFMWS_RESTRICT_TYPE2      (1)
> -#define ACPI_CEDT_CFMWS_RESTRICT_TYPE3      (1<<1)
> -#define ACPI_CEDT_CFMWS_RESTRICT_VOLATILE   (1<<2)
> -#define ACPI_CEDT_CFMWS_RESTRICT_PMEM       (1<<3)
> -#define ACPI_CEDT_CFMWS_RESTRICT_FIXED      (1<<4)
> +#define ACPI_CEDT_CFMWS_RESTRICT_DEVMEM        (1)
> +#define ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM   (1<<1)
> +#define ACPI_CEDT_CFMWS_RESTRICT_VOLATILE      (1<<2)
> +#define ACPI_CEDT_CFMWS_RESTRICT_PMEM          (1<<3)
> +#define ACPI_CEDT_CFMWS_RESTRICT_FIXED         (1<<4)
>  
>  /* 2: CXL XOR Interleave Math Structure */
>  
> diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
> index 90d5afd52dd0..9d919fc99f6b 100644
> --- a/tools/testing/cxl/test/cxl.c
> +++ b/tools/testing/cxl/test/cxl.c
> @@ -209,7 +209,7 @@ static struct {
>  			},
>  			.interleave_ways = 0,
>  			.granularity = 4,
> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
>  					ACPI_CEDT_CFMWS_RESTRICT_VOLATILE,
>  			.qtg_id = FAKE_QTG_ID,
>  			.window_size = SZ_256M * 4UL,
> @@ -224,7 +224,7 @@ static struct {
>  			},
>  			.interleave_ways = 1,
>  			.granularity = 4,
> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
>  					ACPI_CEDT_CFMWS_RESTRICT_VOLATILE,
>  			.qtg_id = FAKE_QTG_ID,
>  			.window_size = SZ_256M * 8UL,
> @@ -239,7 +239,7 @@ static struct {
>  			},
>  			.interleave_ways = 0,
>  			.granularity = 4,
> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
>  					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
>  			.qtg_id = FAKE_QTG_ID,
>  			.window_size = SZ_256M * 4UL,
> @@ -254,7 +254,7 @@ static struct {
>  			},
>  			.interleave_ways = 1,
>  			.granularity = 4,
> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
>  					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
>  			.qtg_id = FAKE_QTG_ID,
>  			.window_size = SZ_256M * 8UL,
> @@ -269,7 +269,7 @@ static struct {
>  			},
>  			.interleave_ways = 0,
>  			.granularity = 4,
> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
>  					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
>  			.qtg_id = FAKE_QTG_ID,
>  			.window_size = SZ_256M * 4UL,
> @@ -284,7 +284,7 @@ static struct {
>  			},
>  			.interleave_ways = 0,
>  			.granularity = 4,
> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
>  					ACPI_CEDT_CFMWS_RESTRICT_VOLATILE,
>  			.qtg_id = FAKE_QTG_ID,
>  			.window_size = SZ_256M,
> @@ -301,7 +301,7 @@ static struct {
>  			.interleave_arithmetic = ACPI_CEDT_CFMWS_ARITHMETIC_XOR,
>  			.interleave_ways = 0,
>  			.granularity = 4,
> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
>  					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
>  			.qtg_id = FAKE_QTG_ID,
>  			.window_size = SZ_256M * 8UL,
> @@ -317,7 +317,7 @@ static struct {
>  			.interleave_arithmetic = ACPI_CEDT_CFMWS_ARITHMETIC_XOR,
>  			.interleave_ways = 1,
>  			.granularity = 0,
> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
>  					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
>  			.qtg_id = FAKE_QTG_ID,
>  			.window_size = SZ_256M * 8UL,
> @@ -333,7 +333,7 @@ static struct {
>  			.interleave_arithmetic = ACPI_CEDT_CFMWS_ARITHMETIC_XOR,
>  			.interleave_ways = 2,
>  			.granularity = 0,
> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
>  					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
>  			.qtg_id = FAKE_QTG_ID,
>  			.window_size = SZ_256M * 16UL,
> -- 
> 2.39.2
> 

-- 
Fan Ni

Re: [PATCH] cxl: Rename ACPI_CEDT_CFMWS_RESTRICT_TYPE2/TYPE3

[Alison Schofield]

On Mon, Nov 04, 2024 at 04:41:10PM +0800, Ying Huang wrote:
> According to the description of the "Window Restrictions" field of
> "CFMWS Structure" in the CXL spec v3.1 section 9.18.1.3: CXL Fixed
> Memory Window Structure (CFMWS), the bit 0 of "Window Restrictions" is
> formerly known as "CXL Type 2 Memory" and renamed to "Device
> Coherent", while the bit 1 is formerly known as "CXL Type 3 Memory"
> and renamed to "Host-only Coherent".  Because type 3 memory can only
> be host-only coherent before, while it can be host-only coherent or
> device coherent with "Back-Invalidate" now.
> 
> To avoid confusion about type 2/3 memory and device/host-only coherent
> in Linux kernel, the patch renames corresponding bit definition from
> ACPI_CEDT_CFMWS_RESTRICT_TYPE2/TYPE3 to
> ACPI_CEDT_CFMWS_RESTRICT_DEVMEM/HOSTONLYMEM.  This makes the kernel
> code consistent with the spec too.
> 
> The patch also renames the corresponding cxl_decoder flags
> CXL_DECODER_F_TYPE2/TYPE3 to CXL_DECODER_F_DEVMEM/HOSTONLYMEM.
> 
> No functionality change is expected.
> 
> Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
> Suggested-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
> Reviewed-by: Gregory Price <gourry@gourry.net>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Dave Jiang <dave.jiang@intel.com>
> Cc: Alison Schofield <alison.schofield@intel.com>
> Cc: Vishal Verma <vishal.l.verma@intel.com>
> Cc: Ira Weiny <ira.weiny@intel.com>
> Cc: Alejandro Lucero <alucerop@amd.com>
> Cc: Ben Cheatham <benjamin.cheatham@amd.com>
> ---
>  drivers/cxl/acpi.c           |  8 ++++----
>  drivers/cxl/core/port.c      |  8 ++++----
>  drivers/cxl/cxl.h            | 14 +++++++-------
>  include/acpi/actbl1.h        | 10 +++++-----
>  tools/testing/cxl/test/cxl.c | 18 +++++++++---------
>  5 files changed, 29 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index 82b78e331d8e..aca8cbb7540d 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -115,10 +115,10 @@ static unsigned long cfmws_to_decoder_flags(int restrictions)
>  {
>  	unsigned long flags = CXL_DECODER_F_ENABLE;
>  
> -	if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_TYPE2)
> -		flags |= CXL_DECODER_F_TYPE2;
> -	if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_TYPE3)
> -		flags |= CXL_DECODER_F_TYPE3;
> +	if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_DEVMEM)
> +		flags |= CXL_DECODER_F_DEVMEM;
> +	if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM)
> +		flags |= CXL_DECODER_F_HOSTONLYMEM;
>  	if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_VOLATILE)
>  		flags |= CXL_DECODER_F_RAM;
>  	if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_PMEM)
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index a5e6f3d23cfb..8524714968fd 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -125,8 +125,8 @@ static DEVICE_ATTR_RO(name)
>  
>  CXL_DECODER_FLAG_ATTR(cap_pmem, CXL_DECODER_F_PMEM);
>  CXL_DECODER_FLAG_ATTR(cap_ram, CXL_DECODER_F_RAM);
> -CXL_DECODER_FLAG_ATTR(cap_type2, CXL_DECODER_F_TYPE2);
> -CXL_DECODER_FLAG_ATTR(cap_type3, CXL_DECODER_F_TYPE3);
> +CXL_DECODER_FLAG_ATTR(cap_type2, CXL_DECODER_F_DEVMEM);
> +CXL_DECODER_FLAG_ATTR(cap_type3, CXL_DECODER_F_HOSTONLYMEM);
>  CXL_DECODER_FLAG_ATTR(locked, CXL_DECODER_F_LOCK);
>  
Hi Ying,

The commit log explains that type3 can now be 'either/or', so does
cap_type3_show() need to emit true for either:
	(flags & CXL_DECODER_F_HOSTONLYMEM)
	or
	(flags & CXL_DECODER_F_DEVMEM) & 'back invalidate')

Does this explanation in the ABI need updating:
What:           /sys/bus/cxl/devices/decoderX.Y/cap_{pmem,ram,type2,type3}
Date:           June, 2021
KernelVersion:  v5.14
Contact:        linux-cxl@vger.kernel.org
Description:
                (RO) When a CXL decoder is of devtype "cxl_decoder_root", it
                represents a fixed memory window identified by platform
                firmware. A fixed window may only support a subset of memory
                types. The 'cap_*' attributes indicate whether persistent
                memory, volatile memory, accelerator memory, and / or expander
                memory may be mapped behind this decoder's memory window.

--Alison


>  static ssize_t target_type_show(struct device *dev,
> @@ -326,14 +326,14 @@ static struct attribute *cxl_decoder_root_attrs[] = {
>  
>  static bool can_create_pmem(struct cxl_root_decoder *cxlrd)
>  {
> -	unsigned long flags = CXL_DECODER_F_TYPE3 | CXL_DECODER_F_PMEM;
> +	unsigned long flags = CXL_DECODER_F_HOSTONLYMEM | CXL_DECODER_F_PMEM;
>  
>  	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;
> +	unsigned long flags = CXL_DECODER_F_HOSTONLYMEM | CXL_DECODER_F_RAM;
>  
>  	return (cxlrd->cxlsd.cxld.flags & flags) == flags;
>  }
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 0fc96f8bf15c..b9083ce1cf74 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -315,13 +315,13 @@ resource_size_t cxl_rcd_component_reg_phys(struct device *dev,
>   * Additionally indicate whether decoder settings were autodetected,
>   * user customized.
>   */
> -#define CXL_DECODER_F_RAM   BIT(0)
> -#define CXL_DECODER_F_PMEM  BIT(1)
> -#define CXL_DECODER_F_TYPE2 BIT(2)
> -#define CXL_DECODER_F_TYPE3 BIT(3)
> -#define CXL_DECODER_F_LOCK  BIT(4)
> -#define CXL_DECODER_F_ENABLE    BIT(5)
> -#define CXL_DECODER_F_MASK  GENMASK(5, 0)
> +#define CXL_DECODER_F_RAM         BIT(0)
> +#define CXL_DECODER_F_PMEM        BIT(1)
> +#define CXL_DECODER_F_DEVMEM      BIT(2)
> +#define CXL_DECODER_F_HOSTONLYMEM BIT(3)
> +#define CXL_DECODER_F_LOCK        BIT(4)
> +#define CXL_DECODER_F_ENABLE      BIT(5)
> +#define CXL_DECODER_F_MASK        GENMASK(5, 0)
>  
>  enum cxl_decoder_type {
>  	CXL_DECODER_DEVMEM = 2,
> diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h
> index 199afc2cd122..e195909928df 100644
> --- a/include/acpi/actbl1.h
> +++ b/include/acpi/actbl1.h
> @@ -551,11 +551,11 @@ struct acpi_cedt_cfmws_target_element {
>  
>  /* Values for Restrictions field above */
>  
> -#define ACPI_CEDT_CFMWS_RESTRICT_TYPE2      (1)
> -#define ACPI_CEDT_CFMWS_RESTRICT_TYPE3      (1<<1)
> -#define ACPI_CEDT_CFMWS_RESTRICT_VOLATILE   (1<<2)
> -#define ACPI_CEDT_CFMWS_RESTRICT_PMEM       (1<<3)
> -#define ACPI_CEDT_CFMWS_RESTRICT_FIXED      (1<<4)
> +#define ACPI_CEDT_CFMWS_RESTRICT_DEVMEM        (1)
> +#define ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM   (1<<1)
> +#define ACPI_CEDT_CFMWS_RESTRICT_VOLATILE      (1<<2)
> +#define ACPI_CEDT_CFMWS_RESTRICT_PMEM          (1<<3)
> +#define ACPI_CEDT_CFMWS_RESTRICT_FIXED         (1<<4)
>  
>  /* 2: CXL XOR Interleave Math Structure */
>  
> diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
> index 90d5afd52dd0..9d919fc99f6b 100644
> --- a/tools/testing/cxl/test/cxl.c
> +++ b/tools/testing/cxl/test/cxl.c
> @@ -209,7 +209,7 @@ static struct {
>  			},
>  			.interleave_ways = 0,
>  			.granularity = 4,
> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
>  					ACPI_CEDT_CFMWS_RESTRICT_VOLATILE,
>  			.qtg_id = FAKE_QTG_ID,
>  			.window_size = SZ_256M * 4UL,
> @@ -224,7 +224,7 @@ static struct {
>  			},
>  			.interleave_ways = 1,
>  			.granularity = 4,
> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
>  					ACPI_CEDT_CFMWS_RESTRICT_VOLATILE,
>  			.qtg_id = FAKE_QTG_ID,
>  			.window_size = SZ_256M * 8UL,
> @@ -239,7 +239,7 @@ static struct {
>  			},
>  			.interleave_ways = 0,
>  			.granularity = 4,
> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
>  					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
>  			.qtg_id = FAKE_QTG_ID,
>  			.window_size = SZ_256M * 4UL,
> @@ -254,7 +254,7 @@ static struct {
>  			},
>  			.interleave_ways = 1,
>  			.granularity = 4,
> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
>  					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
>  			.qtg_id = FAKE_QTG_ID,
>  			.window_size = SZ_256M * 8UL,
> @@ -269,7 +269,7 @@ static struct {
>  			},
>  			.interleave_ways = 0,
>  			.granularity = 4,
> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
>  					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
>  			.qtg_id = FAKE_QTG_ID,
>  			.window_size = SZ_256M * 4UL,
> @@ -284,7 +284,7 @@ static struct {
>  			},
>  			.interleave_ways = 0,
>  			.granularity = 4,
> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
>  					ACPI_CEDT_CFMWS_RESTRICT_VOLATILE,
>  			.qtg_id = FAKE_QTG_ID,
>  			.window_size = SZ_256M,
> @@ -301,7 +301,7 @@ static struct {
>  			.interleave_arithmetic = ACPI_CEDT_CFMWS_ARITHMETIC_XOR,
>  			.interleave_ways = 0,
>  			.granularity = 4,
> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
>  					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
>  			.qtg_id = FAKE_QTG_ID,
>  			.window_size = SZ_256M * 8UL,
> @@ -317,7 +317,7 @@ static struct {
>  			.interleave_arithmetic = ACPI_CEDT_CFMWS_ARITHMETIC_XOR,
>  			.interleave_ways = 1,
>  			.granularity = 0,
> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
>  					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
>  			.qtg_id = FAKE_QTG_ID,
>  			.window_size = SZ_256M * 8UL,
> @@ -333,7 +333,7 @@ static struct {
>  			.interleave_arithmetic = ACPI_CEDT_CFMWS_ARITHMETIC_XOR,
>  			.interleave_ways = 2,
>  			.granularity = 0,
> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
>  					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
>  			.qtg_id = FAKE_QTG_ID,
>  			.window_size = SZ_256M * 16UL,
> -- 
> 2.39.2
> 

Re: [PATCH] cxl: Rename ACPI_CEDT_CFMWS_RESTRICT_TYPE2/TYPE3

[Huang, Ying]

Alison Schofield <alison.schofield@intel.com> writes:

> On Mon, Nov 04, 2024 at 04:41:10PM +0800, Ying Huang wrote:
>> According to the description of the "Window Restrictions" field of
>> "CFMWS Structure" in the CXL spec v3.1 section 9.18.1.3: CXL Fixed
>> Memory Window Structure (CFMWS), the bit 0 of "Window Restrictions" is
>> formerly known as "CXL Type 2 Memory" and renamed to "Device
>> Coherent", while the bit 1 is formerly known as "CXL Type 3 Memory"
>> and renamed to "Host-only Coherent".  Because type 3 memory can only
>> be host-only coherent before, while it can be host-only coherent or
>> device coherent with "Back-Invalidate" now.
>> 
>> To avoid confusion about type 2/3 memory and device/host-only coherent
>> in Linux kernel, the patch renames corresponding bit definition from
>> ACPI_CEDT_CFMWS_RESTRICT_TYPE2/TYPE3 to
>> ACPI_CEDT_CFMWS_RESTRICT_DEVMEM/HOSTONLYMEM.  This makes the kernel
>> code consistent with the spec too.
>> 
>> The patch also renames the corresponding cxl_decoder flags
>> CXL_DECODER_F_TYPE2/TYPE3 to CXL_DECODER_F_DEVMEM/HOSTONLYMEM.
>> 
>> No functionality change is expected.
>> 
>> Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
>> Suggested-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>> Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
>> Reviewed-by: Gregory Price <gourry@gourry.net>
>> Cc: Dan Williams <dan.j.williams@intel.com>
>> Cc: Dave Jiang <dave.jiang@intel.com>
>> Cc: Alison Schofield <alison.schofield@intel.com>
>> Cc: Vishal Verma <vishal.l.verma@intel.com>
>> Cc: Ira Weiny <ira.weiny@intel.com>
>> Cc: Alejandro Lucero <alucerop@amd.com>
>> Cc: Ben Cheatham <benjamin.cheatham@amd.com>
>> ---
>>  drivers/cxl/acpi.c           |  8 ++++----
>>  drivers/cxl/core/port.c      |  8 ++++----
>>  drivers/cxl/cxl.h            | 14 +++++++-------
>>  include/acpi/actbl1.h        | 10 +++++-----
>>  tools/testing/cxl/test/cxl.c | 18 +++++++++---------
>>  5 files changed, 29 insertions(+), 29 deletions(-)
>> 
>> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
>> index 82b78e331d8e..aca8cbb7540d 100644
>> --- a/drivers/cxl/acpi.c
>> +++ b/drivers/cxl/acpi.c
>> @@ -115,10 +115,10 @@ static unsigned long cfmws_to_decoder_flags(int restrictions)
>>  {
>>  	unsigned long flags = CXL_DECODER_F_ENABLE;
>>  
>> -	if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_TYPE2)
>> -		flags |= CXL_DECODER_F_TYPE2;
>> -	if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_TYPE3)
>> -		flags |= CXL_DECODER_F_TYPE3;
>> +	if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_DEVMEM)
>> +		flags |= CXL_DECODER_F_DEVMEM;
>> +	if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM)
>> +		flags |= CXL_DECODER_F_HOSTONLYMEM;
>>  	if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_VOLATILE)
>>  		flags |= CXL_DECODER_F_RAM;
>>  	if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_PMEM)
>> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
>> index a5e6f3d23cfb..8524714968fd 100644
>> --- a/drivers/cxl/core/port.c
>> +++ b/drivers/cxl/core/port.c
>> @@ -125,8 +125,8 @@ static DEVICE_ATTR_RO(name)
>>  
>>  CXL_DECODER_FLAG_ATTR(cap_pmem, CXL_DECODER_F_PMEM);
>>  CXL_DECODER_FLAG_ATTR(cap_ram, CXL_DECODER_F_RAM);
>> -CXL_DECODER_FLAG_ATTR(cap_type2, CXL_DECODER_F_TYPE2);
>> -CXL_DECODER_FLAG_ATTR(cap_type3, CXL_DECODER_F_TYPE3);
>> +CXL_DECODER_FLAG_ATTR(cap_type2, CXL_DECODER_F_DEVMEM);
>> +CXL_DECODER_FLAG_ATTR(cap_type3, CXL_DECODER_F_HOSTONLYMEM);
>>  CXL_DECODER_FLAG_ATTR(locked, CXL_DECODER_F_LOCK);
>>  
> Hi Ying,

Hi, Alison,

>
> The commit log explains that type3 can now be 'either/or', so does
> cap_type3_show() need to emit true for either:
> 	(flags & CXL_DECODER_F_HOSTONLYMEM)
> 	or
> 	(flags & CXL_DECODER_F_DEVMEM) & 'back invalidate')
>
> Does this explanation in the ABI need updating:
> What:           /sys/bus/cxl/devices/decoderX.Y/cap_{pmem,ram,type2,type3}
> Date:           June, 2021
> KernelVersion:  v5.14
> Contact:        linux-cxl@vger.kernel.org
> Description:
>                 (RO) When a CXL decoder is of devtype "cxl_decoder_root", it
>                 represents a fixed memory window identified by platform
>                 firmware. A fixed window may only support a subset of memory
>                 types. The 'cap_*' attributes indicate whether persistent
>                 memory, volatile memory, accelerator memory, and / or expander
>                 memory may be mapped behind this decoder's memory window.

I think so too.  However, I prefer to keep this patch just mechanic
renaming and do these changes in another patch.  Do you agree?

--
Best Regards,
Huang, Ying

>
>>  static ssize_t target_type_show(struct device *dev,
>> @@ -326,14 +326,14 @@ static struct attribute *cxl_decoder_root_attrs[] = {
>>  
>>  static bool can_create_pmem(struct cxl_root_decoder *cxlrd)
>>  {
>> -	unsigned long flags = CXL_DECODER_F_TYPE3 | CXL_DECODER_F_PMEM;
>> +	unsigned long flags = CXL_DECODER_F_HOSTONLYMEM | CXL_DECODER_F_PMEM;
>>  
>>  	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;
>> +	unsigned long flags = CXL_DECODER_F_HOSTONLYMEM | CXL_DECODER_F_RAM;
>>  
>>  	return (cxlrd->cxlsd.cxld.flags & flags) == flags;
>>  }
>> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
>> index 0fc96f8bf15c..b9083ce1cf74 100644
>> --- a/drivers/cxl/cxl.h
>> +++ b/drivers/cxl/cxl.h
>> @@ -315,13 +315,13 @@ resource_size_t cxl_rcd_component_reg_phys(struct device *dev,
>>   * Additionally indicate whether decoder settings were autodetected,
>>   * user customized.
>>   */
>> -#define CXL_DECODER_F_RAM   BIT(0)
>> -#define CXL_DECODER_F_PMEM  BIT(1)
>> -#define CXL_DECODER_F_TYPE2 BIT(2)
>> -#define CXL_DECODER_F_TYPE3 BIT(3)
>> -#define CXL_DECODER_F_LOCK  BIT(4)
>> -#define CXL_DECODER_F_ENABLE    BIT(5)
>> -#define CXL_DECODER_F_MASK  GENMASK(5, 0)
>> +#define CXL_DECODER_F_RAM         BIT(0)
>> +#define CXL_DECODER_F_PMEM        BIT(1)
>> +#define CXL_DECODER_F_DEVMEM      BIT(2)
>> +#define CXL_DECODER_F_HOSTONLYMEM BIT(3)
>> +#define CXL_DECODER_F_LOCK        BIT(4)
>> +#define CXL_DECODER_F_ENABLE      BIT(5)
>> +#define CXL_DECODER_F_MASK        GENMASK(5, 0)
>>  
>>  enum cxl_decoder_type {
>>  	CXL_DECODER_DEVMEM = 2,
>> diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h
>> index 199afc2cd122..e195909928df 100644
>> --- a/include/acpi/actbl1.h
>> +++ b/include/acpi/actbl1.h
>> @@ -551,11 +551,11 @@ struct acpi_cedt_cfmws_target_element {
>>  
>>  /* Values for Restrictions field above */
>>  
>> -#define ACPI_CEDT_CFMWS_RESTRICT_TYPE2      (1)
>> -#define ACPI_CEDT_CFMWS_RESTRICT_TYPE3      (1<<1)
>> -#define ACPI_CEDT_CFMWS_RESTRICT_VOLATILE   (1<<2)
>> -#define ACPI_CEDT_CFMWS_RESTRICT_PMEM       (1<<3)
>> -#define ACPI_CEDT_CFMWS_RESTRICT_FIXED      (1<<4)
>> +#define ACPI_CEDT_CFMWS_RESTRICT_DEVMEM        (1)
>> +#define ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM   (1<<1)
>> +#define ACPI_CEDT_CFMWS_RESTRICT_VOLATILE      (1<<2)
>> +#define ACPI_CEDT_CFMWS_RESTRICT_PMEM          (1<<3)
>> +#define ACPI_CEDT_CFMWS_RESTRICT_FIXED         (1<<4)
>>  
>>  /* 2: CXL XOR Interleave Math Structure */
>>  
>> diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
>> index 90d5afd52dd0..9d919fc99f6b 100644
>> --- a/tools/testing/cxl/test/cxl.c
>> +++ b/tools/testing/cxl/test/cxl.c
>> @@ -209,7 +209,7 @@ static struct {
>>  			},
>>  			.interleave_ways = 0,
>>  			.granularity = 4,
>> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
>> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
>>  					ACPI_CEDT_CFMWS_RESTRICT_VOLATILE,
>>  			.qtg_id = FAKE_QTG_ID,
>>  			.window_size = SZ_256M * 4UL,
>> @@ -224,7 +224,7 @@ static struct {
>>  			},
>>  			.interleave_ways = 1,
>>  			.granularity = 4,
>> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
>> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
>>  					ACPI_CEDT_CFMWS_RESTRICT_VOLATILE,
>>  			.qtg_id = FAKE_QTG_ID,
>>  			.window_size = SZ_256M * 8UL,
>> @@ -239,7 +239,7 @@ static struct {
>>  			},
>>  			.interleave_ways = 0,
>>  			.granularity = 4,
>> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
>> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
>>  					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
>>  			.qtg_id = FAKE_QTG_ID,
>>  			.window_size = SZ_256M * 4UL,
>> @@ -254,7 +254,7 @@ static struct {
>>  			},
>>  			.interleave_ways = 1,
>>  			.granularity = 4,
>> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
>> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
>>  					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
>>  			.qtg_id = FAKE_QTG_ID,
>>  			.window_size = SZ_256M * 8UL,
>> @@ -269,7 +269,7 @@ static struct {
>>  			},
>>  			.interleave_ways = 0,
>>  			.granularity = 4,
>> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
>> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
>>  					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
>>  			.qtg_id = FAKE_QTG_ID,
>>  			.window_size = SZ_256M * 4UL,
>> @@ -284,7 +284,7 @@ static struct {
>>  			},
>>  			.interleave_ways = 0,
>>  			.granularity = 4,
>> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
>> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
>>  					ACPI_CEDT_CFMWS_RESTRICT_VOLATILE,
>>  			.qtg_id = FAKE_QTG_ID,
>>  			.window_size = SZ_256M,
>> @@ -301,7 +301,7 @@ static struct {
>>  			.interleave_arithmetic = ACPI_CEDT_CFMWS_ARITHMETIC_XOR,
>>  			.interleave_ways = 0,
>>  			.granularity = 4,
>> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
>> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
>>  					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
>>  			.qtg_id = FAKE_QTG_ID,
>>  			.window_size = SZ_256M * 8UL,
>> @@ -317,7 +317,7 @@ static struct {
>>  			.interleave_arithmetic = ACPI_CEDT_CFMWS_ARITHMETIC_XOR,
>>  			.interleave_ways = 1,
>>  			.granularity = 0,
>> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
>> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
>>  					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
>>  			.qtg_id = FAKE_QTG_ID,
>>  			.window_size = SZ_256M * 8UL,
>> @@ -333,7 +333,7 @@ static struct {
>>  			.interleave_arithmetic = ACPI_CEDT_CFMWS_ARITHMETIC_XOR,
>>  			.interleave_ways = 2,
>>  			.granularity = 0,
>> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
>> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
>>  					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
>>  			.qtg_id = FAKE_QTG_ID,
>>  			.window_size = SZ_256M * 16UL,
>> -- 
>> 2.39.2
>> 

Re: [PATCH] cxl: Rename ACPI_CEDT_CFMWS_RESTRICT_TYPE2/TYPE3

[Alison Schofield]

On Wed, Nov 06, 2024 at 10:43:30AM +0800, Ying Huang wrote:
> Alison Schofield <alison.schofield@intel.com> writes:
> 
> > On Mon, Nov 04, 2024 at 04:41:10PM +0800, Ying Huang wrote:
> >> According to the description of the "Window Restrictions" field of
> >> "CFMWS Structure" in the CXL spec v3.1 section 9.18.1.3: CXL Fixed
> >> Memory Window Structure (CFMWS), the bit 0 of "Window Restrictions" is
> >> formerly known as "CXL Type 2 Memory" and renamed to "Device
> >> Coherent", while the bit 1 is formerly known as "CXL Type 3 Memory"
> >> and renamed to "Host-only Coherent".  Because type 3 memory can only
> >> be host-only coherent before, while it can be host-only coherent or
> >> device coherent with "Back-Invalidate" now.
> >> 
> >> To avoid confusion about type 2/3 memory and device/host-only coherent
> >> in Linux kernel, the patch renames corresponding bit definition from
> >> ACPI_CEDT_CFMWS_RESTRICT_TYPE2/TYPE3 to
> >> ACPI_CEDT_CFMWS_RESTRICT_DEVMEM/HOSTONLYMEM.  This makes the kernel
> >> code consistent with the spec too.
> >> 
> >> The patch also renames the corresponding cxl_decoder flags
> >> CXL_DECODER_F_TYPE2/TYPE3 to CXL_DECODER_F_DEVMEM/HOSTONLYMEM.
> >> 
> >> No functionality change is expected.
> >> 
> >> Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
> >> Suggested-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >> Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
> >> Reviewed-by: Gregory Price <gourry@gourry.net>
> >> Cc: Dan Williams <dan.j.williams@intel.com>
> >> Cc: Dave Jiang <dave.jiang@intel.com>
> >> Cc: Alison Schofield <alison.schofield@intel.com>
> >> Cc: Vishal Verma <vishal.l.verma@intel.com>
> >> Cc: Ira Weiny <ira.weiny@intel.com>
> >> Cc: Alejandro Lucero <alucerop@amd.com>
> >> Cc: Ben Cheatham <benjamin.cheatham@amd.com>
> >> ---
> >>  drivers/cxl/acpi.c           |  8 ++++----
> >>  drivers/cxl/core/port.c      |  8 ++++----
> >>  drivers/cxl/cxl.h            | 14 +++++++-------
> >>  include/acpi/actbl1.h        | 10 +++++-----
> >>  tools/testing/cxl/test/cxl.c | 18 +++++++++---------
> >>  5 files changed, 29 insertions(+), 29 deletions(-)
> >> 
> >> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> >> index 82b78e331d8e..aca8cbb7540d 100644
> >> --- a/drivers/cxl/acpi.c
> >> +++ b/drivers/cxl/acpi.c
> >> @@ -115,10 +115,10 @@ static unsigned long cfmws_to_decoder_flags(int restrictions)
> >>  {
> >>  	unsigned long flags = CXL_DECODER_F_ENABLE;
> >>  
> >> -	if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_TYPE2)
> >> -		flags |= CXL_DECODER_F_TYPE2;
> >> -	if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_TYPE3)
> >> -		flags |= CXL_DECODER_F_TYPE3;
> >> +	if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_DEVMEM)
> >> +		flags |= CXL_DECODER_F_DEVMEM;
> >> +	if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM)
> >> +		flags |= CXL_DECODER_F_HOSTONLYMEM;
> >>  	if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_VOLATILE)
> >>  		flags |= CXL_DECODER_F_RAM;
> >>  	if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_PMEM)
> >> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> >> index a5e6f3d23cfb..8524714968fd 100644
> >> --- a/drivers/cxl/core/port.c
> >> +++ b/drivers/cxl/core/port.c
> >> @@ -125,8 +125,8 @@ static DEVICE_ATTR_RO(name)
> >>  
> >>  CXL_DECODER_FLAG_ATTR(cap_pmem, CXL_DECODER_F_PMEM);
> >>  CXL_DECODER_FLAG_ATTR(cap_ram, CXL_DECODER_F_RAM);
> >> -CXL_DECODER_FLAG_ATTR(cap_type2, CXL_DECODER_F_TYPE2);
> >> -CXL_DECODER_FLAG_ATTR(cap_type3, CXL_DECODER_F_TYPE3);
> >> +CXL_DECODER_FLAG_ATTR(cap_type2, CXL_DECODER_F_DEVMEM);
> >> +CXL_DECODER_FLAG_ATTR(cap_type3, CXL_DECODER_F_HOSTONLYMEM);
> >>  CXL_DECODER_FLAG_ATTR(locked, CXL_DECODER_F_LOCK);
> >>  
> > Hi Ying,
> 
> Hi, Alison,
> 
> >
> > The commit log explains that type3 can now be 'either/or', so does
> > cap_type3_show() need to emit true for either:
> > 	(flags & CXL_DECODER_F_HOSTONLYMEM)
> > 	or
> > 	(flags & CXL_DECODER_F_DEVMEM) & 'back invalidate')
> >
> > Does this explanation in the ABI need updating:
> > What:           /sys/bus/cxl/devices/decoderX.Y/cap_{pmem,ram,type2,type3}
> > Date:           June, 2021
> > KernelVersion:  v5.14
> > Contact:        linux-cxl@vger.kernel.org
> > Description:
> >                 (RO) When a CXL decoder is of devtype "cxl_decoder_root", it
> >                 represents a fixed memory window identified by platform
> >                 firmware. A fixed window may only support a subset of memory
> >                 types. The 'cap_*' attributes indicate whether persistent
> >                 memory, volatile memory, accelerator memory, and / or expander
> >                 memory may be mapped behind this decoder's memory window.
> 
> I think so too.  However, I prefer to keep this patch just mechanic
> renaming and do these changes in another patch.  Do you agree?
> 

I don't know. I was just questioning where and how far the naming scheme
needs to change.

Maybe Jonathan, as the Suggested-by, can chime in and move this ahead.


> --
> Best Regards,
> Huang, Ying
> 
> >
> >>  static ssize_t target_type_show(struct device *dev,
> >> @@ -326,14 +326,14 @@ static struct attribute *cxl_decoder_root_attrs[] = {
> >>  
> >>  static bool can_create_pmem(struct cxl_root_decoder *cxlrd)
> >>  {
> >> -	unsigned long flags = CXL_DECODER_F_TYPE3 | CXL_DECODER_F_PMEM;
> >> +	unsigned long flags = CXL_DECODER_F_HOSTONLYMEM | CXL_DECODER_F_PMEM;
> >>  
> >>  	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;
> >> +	unsigned long flags = CXL_DECODER_F_HOSTONLYMEM | CXL_DECODER_F_RAM;
> >>  
> >>  	return (cxlrd->cxlsd.cxld.flags & flags) == flags;
> >>  }
> >> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> >> index 0fc96f8bf15c..b9083ce1cf74 100644
> >> --- a/drivers/cxl/cxl.h
> >> +++ b/drivers/cxl/cxl.h
> >> @@ -315,13 +315,13 @@ resource_size_t cxl_rcd_component_reg_phys(struct device *dev,
> >>   * Additionally indicate whether decoder settings were autodetected,
> >>   * user customized.
> >>   */
> >> -#define CXL_DECODER_F_RAM   BIT(0)
> >> -#define CXL_DECODER_F_PMEM  BIT(1)
> >> -#define CXL_DECODER_F_TYPE2 BIT(2)
> >> -#define CXL_DECODER_F_TYPE3 BIT(3)
> >> -#define CXL_DECODER_F_LOCK  BIT(4)
> >> -#define CXL_DECODER_F_ENABLE    BIT(5)
> >> -#define CXL_DECODER_F_MASK  GENMASK(5, 0)
> >> +#define CXL_DECODER_F_RAM         BIT(0)
> >> +#define CXL_DECODER_F_PMEM        BIT(1)
> >> +#define CXL_DECODER_F_DEVMEM      BIT(2)
> >> +#define CXL_DECODER_F_HOSTONLYMEM BIT(3)
> >> +#define CXL_DECODER_F_LOCK        BIT(4)
> >> +#define CXL_DECODER_F_ENABLE      BIT(5)
> >> +#define CXL_DECODER_F_MASK        GENMASK(5, 0)
> >>  
> >>  enum cxl_decoder_type {
> >>  	CXL_DECODER_DEVMEM = 2,
> >> diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h
> >> index 199afc2cd122..e195909928df 100644
> >> --- a/include/acpi/actbl1.h
> >> +++ b/include/acpi/actbl1.h
> >> @@ -551,11 +551,11 @@ struct acpi_cedt_cfmws_target_element {
> >>  
> >>  /* Values for Restrictions field above */
> >>  
> >> -#define ACPI_CEDT_CFMWS_RESTRICT_TYPE2      (1)
> >> -#define ACPI_CEDT_CFMWS_RESTRICT_TYPE3      (1<<1)
> >> -#define ACPI_CEDT_CFMWS_RESTRICT_VOLATILE   (1<<2)
> >> -#define ACPI_CEDT_CFMWS_RESTRICT_PMEM       (1<<3)
> >> -#define ACPI_CEDT_CFMWS_RESTRICT_FIXED      (1<<4)
> >> +#define ACPI_CEDT_CFMWS_RESTRICT_DEVMEM        (1)
> >> +#define ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM   (1<<1)
> >> +#define ACPI_CEDT_CFMWS_RESTRICT_VOLATILE      (1<<2)
> >> +#define ACPI_CEDT_CFMWS_RESTRICT_PMEM          (1<<3)
> >> +#define ACPI_CEDT_CFMWS_RESTRICT_FIXED         (1<<4)
> >>  
> >>  /* 2: CXL XOR Interleave Math Structure */
> >>  
> >> diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
> >> index 90d5afd52dd0..9d919fc99f6b 100644
> >> --- a/tools/testing/cxl/test/cxl.c
> >> +++ b/tools/testing/cxl/test/cxl.c
> >> @@ -209,7 +209,7 @@ static struct {
> >>  			},
> >>  			.interleave_ways = 0,
> >>  			.granularity = 4,
> >> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
> >> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
> >>  					ACPI_CEDT_CFMWS_RESTRICT_VOLATILE,
> >>  			.qtg_id = FAKE_QTG_ID,
> >>  			.window_size = SZ_256M * 4UL,
> >> @@ -224,7 +224,7 @@ static struct {
> >>  			},
> >>  			.interleave_ways = 1,
> >>  			.granularity = 4,
> >> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
> >> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
> >>  					ACPI_CEDT_CFMWS_RESTRICT_VOLATILE,
> >>  			.qtg_id = FAKE_QTG_ID,
> >>  			.window_size = SZ_256M * 8UL,
> >> @@ -239,7 +239,7 @@ static struct {
> >>  			},
> >>  			.interleave_ways = 0,
> >>  			.granularity = 4,
> >> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
> >> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
> >>  					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
> >>  			.qtg_id = FAKE_QTG_ID,
> >>  			.window_size = SZ_256M * 4UL,
> >> @@ -254,7 +254,7 @@ static struct {
> >>  			},
> >>  			.interleave_ways = 1,
> >>  			.granularity = 4,
> >> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
> >> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
> >>  					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
> >>  			.qtg_id = FAKE_QTG_ID,
> >>  			.window_size = SZ_256M * 8UL,
> >> @@ -269,7 +269,7 @@ static struct {
> >>  			},
> >>  			.interleave_ways = 0,
> >>  			.granularity = 4,
> >> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
> >> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
> >>  					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
> >>  			.qtg_id = FAKE_QTG_ID,
> >>  			.window_size = SZ_256M * 4UL,
> >> @@ -284,7 +284,7 @@ static struct {
> >>  			},
> >>  			.interleave_ways = 0,
> >>  			.granularity = 4,
> >> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
> >> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
> >>  					ACPI_CEDT_CFMWS_RESTRICT_VOLATILE,
> >>  			.qtg_id = FAKE_QTG_ID,
> >>  			.window_size = SZ_256M,
> >> @@ -301,7 +301,7 @@ static struct {
> >>  			.interleave_arithmetic = ACPI_CEDT_CFMWS_ARITHMETIC_XOR,
> >>  			.interleave_ways = 0,
> >>  			.granularity = 4,
> >> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
> >> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
> >>  					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
> >>  			.qtg_id = FAKE_QTG_ID,
> >>  			.window_size = SZ_256M * 8UL,
> >> @@ -317,7 +317,7 @@ static struct {
> >>  			.interleave_arithmetic = ACPI_CEDT_CFMWS_ARITHMETIC_XOR,
> >>  			.interleave_ways = 1,
> >>  			.granularity = 0,
> >> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
> >> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
> >>  					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
> >>  			.qtg_id = FAKE_QTG_ID,
> >>  			.window_size = SZ_256M * 8UL,
> >> @@ -333,7 +333,7 @@ static struct {
> >>  			.interleave_arithmetic = ACPI_CEDT_CFMWS_ARITHMETIC_XOR,
> >>  			.interleave_ways = 2,
> >>  			.granularity = 0,
> >> -			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_TYPE3 |
> >> +			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
> >>  					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
> >>  			.qtg_id = FAKE_QTG_ID,
> >>  			.window_size = SZ_256M * 16UL,
> >> -- 
> >> 2.39.2
> >> 

Re: [PATCH] cxl: Rename ACPI_CEDT_CFMWS_RESTRICT_TYPE2/TYPE3

[Dan Williams]

Alison Schofield wrote:
[..]
> > I think so too.  However, I prefer to keep this patch just mechanic
> > renaming and do these changes in another patch.  Do you agree?
> > 
> 
> I don't know. I was just questioning where and how far the naming scheme
> needs to change.
> 
> Maybe Jonathan, as the Suggested-by, can chime in and move this ahead.

I feel like we are going to be living with the ghosts of the original
"Type2 / Type3" naming problem for the rest of the subsystem's lifespan
especially since they were encoded in the ABI and ABI is forever.

I am not opposed to this localized rename in drivers/cxl/acpi.c on
principal, but in terms of incremental value relative to the thrash, it's
questionable.

For example changes to include/acpi/actbl1.h need to be chased through
ACPICA, at which point is this rename really worth it?

Re: [PATCH] cxl: Rename ACPI_CEDT_CFMWS_RESTRICT_TYPE2/TYPE3

[Huang, Ying]

Dan Williams <dan.j.williams@intel.com> writes:

> Alison Schofield wrote:
> [..]
>> > I think so too.  However, I prefer to keep this patch just mechanic
>> > renaming and do these changes in another patch.  Do you agree?
>> > 
>> 
>> I don't know. I was just questioning where and how far the naming scheme
>> needs to change.
>> 
>> Maybe Jonathan, as the Suggested-by, can chime in and move this ahead.
>
> I feel like we are going to be living with the ghosts of the original
> "Type2 / Type3" naming problem for the rest of the subsystem's lifespan
> especially since they were encoded in the ABI and ABI is forever.
>
> I am not opposed to this localized rename in drivers/cxl/acpi.c on
> principal, but in terms of incremental value relative to the thrash, it's
> questionable.
>
> For example changes to include/acpi/actbl1.h need to be chased through
> ACPICA, at which point is this rename really worth it?

I think that it's not too hard to change ACPI tables definition.  Added
Bob and Rui for ACPICA related change.

--
Best Regards,
Huang, Ying

Re: [PATCH] cxl: Rename ACPI_CEDT_CFMWS_RESTRICT_TYPE2/TYPE3

[Zhang, Rui]

CC Rafael,

On Sun, 2024-11-10 at 14:13 +0800, Huang, Ying wrote:
> Dan Williams <dan.j.williams@intel.com> writes:
> 
> > Alison Schofield wrote:
> > [..]
> > > > I think so too.  However, I prefer to keep this patch just
> > > > mechanic
> > > > renaming and do these changes in another patch.  Do you agree?
> > > > 
> > > 
> > > I don't know. I was just questioning where and how far the naming
> > > scheme
> > > needs to change.
> > > 
> > > Maybe Jonathan, as the Suggested-by, can chime in and move this
> > > ahead.
> > 
> > I feel like we are going to be living with the ghosts of the
> > original
> > "Type2 / Type3" naming problem for the rest of the subsystem's
> > lifespan
> > especially since they were encoded in the ABI and ABI is forever.
> > 
> > I am not opposed to this localized rename in drivers/cxl/acpi.c on
> > principal, but in terms of incremental value relative to the
> > thrash, it's
> > questionable.
> > 
> > For example changes to include/acpi/actbl1.h need to be chased
> > through
> > ACPICA, at which point is this rename really worth it?
> 
> I think that it's not too hard to change ACPI tables definition. 
> Added
> Bob and Rui for ACPICA related change.

For the change below,

diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h
index 199afc2cd122..e195909928df 100644
--- a/include/acpi/actbl1.h
+++ b/include/acpi/actbl1.h
@@ -551,11 +551,11 @@ struct acpi_cedt_cfmws_target_element {
 
 /* Values for Restrictions field above */
 
-#define ACPI_CEDT_CFMWS_RESTRICT_TYPE2      (1)
-#define ACPI_CEDT_CFMWS_RESTRICT_TYPE3      (1<<1)
-#define ACPI_CEDT_CFMWS_RESTRICT_VOLATILE   (1<<2)
-#define ACPI_CEDT_CFMWS_RESTRICT_PMEM       (1<<3)
-#define ACPI_CEDT_CFMWS_RESTRICT_FIXED      (1<<4)
+#define ACPI_CEDT_CFMWS_RESTRICT_DEVMEM        (1)
+#define ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM   (1<<1)
+#define ACPI_CEDT_CFMWS_RESTRICT_VOLATILE      (1<<2)
+#define ACPI_CEDT_CFMWS_RESTRICT_PMEM          (1<<3)
+#define ACPI_CEDT_CFMWS_RESTRICT_FIXED         (1<<4)

This change is made based on the spec update, right?

Do we have any user(other than Linux) for the old version of spec?
If yes, we probably need to keep the old ones. And IMO, if spec changes
and the bit definition changes, we should introduce new Macros for the
new definitions, together with spec revision info, say something like

#define ACPI_CEDT_CFMWS_V2_RESTRICT_DEVMEM        (1)
#define ACPI_CEDT_CFMWS_V2_RESTRICT_HOSTONLYMEM   (1<<1)
#define ACPI_CEDT_CFMWS_V2_RESTRICT_VOLATILE      (1<<2)
#define ACPI_CEDT_CFMWS_V2_RESTRICT_PMEM          (1<<3)
#define ACPI_CEDT_CFMWS_V2_RESTRICT_FIXED         (1<<4)

and make Linux stick with the new Macros?

thanks,
rui

Re: [PATCH] cxl: Rename ACPI_CEDT_CFMWS_RESTRICT_TYPE2/TYPE3

[Dan Williams]

Zhang, Rui wrote:
> CC Rafael,
> 
> On Sun, 2024-11-10 at 14:13 +0800, Huang, Ying wrote:
> > Dan Williams <dan.j.williams@intel.com> writes:
> > 
> > > Alison Schofield wrote:
> > > [..]
> > > > > I think so too.  However, I prefer to keep this patch just
> > > > > mechanic
> > > > > renaming and do these changes in another patch.  Do you agree?
> > > > > 
> > > > 
> > > > I don't know. I was just questioning where and how far the naming
> > > > scheme
> > > > needs to change.
> > > > 
> > > > Maybe Jonathan, as the Suggested-by, can chime in and move this
> > > > ahead.
> > > 
> > > I feel like we are going to be living with the ghosts of the
> > > original
> > > "Type2 / Type3" naming problem for the rest of the subsystem's
> > > lifespan
> > > especially since they were encoded in the ABI and ABI is forever.
> > > 
> > > I am not opposed to this localized rename in drivers/cxl/acpi.c on
> > > principal, but in terms of incremental value relative to the
> > > thrash, it's
> > > questionable.
> > > 
> > > For example changes to include/acpi/actbl1.h need to be chased
> > > through
> > > ACPICA, at which point is this rename really worth it?
> > 
> > I think that it's not too hard to change ACPI tables definition. 
> > Added
> > Bob and Rui for ACPICA related change.
> 
> For the change below,
> 
> diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h
> index 199afc2cd122..e195909928df 100644
> --- a/include/acpi/actbl1.h
> +++ b/include/acpi/actbl1.h
> @@ -551,11 +551,11 @@ struct acpi_cedt_cfmws_target_element {
>  
>  /* Values for Restrictions field above */
>  
> -#define ACPI_CEDT_CFMWS_RESTRICT_TYPE2      (1)
> -#define ACPI_CEDT_CFMWS_RESTRICT_TYPE3      (1<<1)
> -#define ACPI_CEDT_CFMWS_RESTRICT_VOLATILE   (1<<2)
> -#define ACPI_CEDT_CFMWS_RESTRICT_PMEM       (1<<3)
> -#define ACPI_CEDT_CFMWS_RESTRICT_FIXED      (1<<4)
> +#define ACPI_CEDT_CFMWS_RESTRICT_DEVMEM        (1)
> +#define ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM   (1<<1)
> +#define ACPI_CEDT_CFMWS_RESTRICT_VOLATILE      (1<<2)
> +#define ACPI_CEDT_CFMWS_RESTRICT_PMEM          (1<<3)
> +#define ACPI_CEDT_CFMWS_RESTRICT_FIXED         (1<<4)
> 
> This change is made based on the spec update, right?
> 
> Do we have any user(other than Linux) for the old version of spec?
> If yes, we probably need to keep the old ones. And IMO, if spec changes
> and the bit definition changes, we should introduce new Macros for the
> new definitions, together with spec revision info, say something like

At least for me the thrash does not have a signficant upside. The more
important terminology concepts are within the CXL core.

So I would just merge this rename below and call it a day, i.e. leave
the cxl_acpi rename alone.

diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 0fc96f8bf15c..b9083ce1cf74 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -315,13 +315,13 @@ resource_size_t cxl_rcd_component_reg_phys(struct device *dev,
  * Additionally indicate whether decoder settings were autodetected,
  * user customized.
  */
-#define CXL_DECODER_F_RAM   BIT(0)
-#define CXL_DECODER_F_PMEM  BIT(1)
-#define CXL_DECODER_F_TYPE2 BIT(2)
-#define CXL_DECODER_F_TYPE3 BIT(3)
-#define CXL_DECODER_F_LOCK  BIT(4)
-#define CXL_DECODER_F_ENABLE    BIT(5)
-#define CXL_DECODER_F_MASK  GENMASK(5, 0)
+#define CXL_DECODER_F_RAM         BIT(0)
+#define CXL_DECODER_F_PMEM        BIT(1)
+#define CXL_DECODER_F_DEVMEM      BIT(2)
+#define CXL_DECODER_F_HOSTONLYMEM BIT(3)
+#define CXL_DECODER_F_LOCK        BIT(4)
+#define CXL_DECODER_F_ENABLE      BIT(5)
+#define CXL_DECODER_F_MASK        GENMASK(5, 0)

 enum cxl_decoder_type {
        CXL_DECODER_DEVMEM = 2,