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] [thread-next>] [day] [month] [year] [list]

Message-ID: <1595d6b740602682f12a4e502e459b0d82e48711.camel@intel.com>
Date: Tue, 12 Nov 2024 02:37:40 +0000
From: "Zhang, Rui" <rui.zhang@...el.com>
To: "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

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

#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