linux-kernel - Re: [PATCH] cxl: Rename ACPI_CEDT_CFMWS_RESTRICT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <67589fa6c84fe_10a083294fe@dwillia2-xfh.jf.intel.com.notmuch>
Date: Tue, 10 Dec 2024 12:08:06 -0800
From: Dan Williams <dan.j.williams@...el.com>
To: "Zhang, Rui" <rui.zhang@...el.com>, "Huang, Ying" <ying.huang@...el.com>,
	"Moore, Robert" <robert.moore@...el.com>, "Jonathan.Cameron@...wei.com"
	<Jonathan.Cameron@...wei.com>, "Williams, Dan J" <dan.j.williams@...el.com>,
	"Wysocki, Rafael J" <rafael.j.wysocki@...el.com>
CC: "benjamin.cheatham@....com" <benjamin.cheatham@....com>, "Jiang, Dave"
	<dave.jiang@...el.com>, "Schofield, Alison" <alison.schofield@...el.com>,
	"gourry@...rry.net" <gourry@...rry.net>, "linux-cxl@...r.kernel.org"
	<linux-cxl@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "Verma, Vishal L" <vishal.l.verma@...el.com>,
	"Weiny, Ira" <ira.weiny@...el.com>, "dave@...olabs.net" <dave@...olabs.net>,
	"alucerop@....com" <alucerop@....com>
Subject: Re: [PATCH] cxl: Rename ACPI_CEDT_CFMWS_RESTRICT_TYPE2/TYPE3

Zhang, Rui wrote:
> CC Rafael,
> 
> On Sun, 2024-11-10 at 14:13 +0800, Huang, Ying wrote:
> > Dan Williams <dan.j.williams@...el.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,