| Message ID | 20241104084110.583710-1-ying.huang@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | cxl: Rename ACPI_CEDT_CFMWS_RESTRICT_TYPE2/TYPE3 | expand |
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>
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 >
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 >
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 >>
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 > >>
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?
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
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
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,