Re: [PATCH v3 1/7] arm64/sysreg: Convert CCSIDR_EL1 to automatic generation

[Marc Zyngier]

On Sun, 18 Dec 2022 11:35:12 +0000,
Akihiko Odaki <akihiko.odaki@xxxxxxxxxx> wrote:
> 
> On 2022/12/18 20:23, Marc Zyngier wrote:
> > On Sun, 18 Dec 2022 05:14:06 +0000,
> > Akihiko Odaki <akihiko.odaki@xxxxxxxxxx> wrote:
> >> 
> >> Convert CCSIDR_EL1 to automatic generation as per DDI0487I.a. The field
> >> definition is for case when FEAT_CCIDX is not implemented. Fields WT,
> >> WB, RA and WA are defined as per A.j since they are now reserved and
> >> may have UNKNOWN values in I.a, which the file format cannot represent.
> >> 
> >> Signed-off-by: Akihiko Odaki <akihiko.odaki@xxxxxxxxxx>
> >> ---
> >>   arch/arm64/include/asm/sysreg.h |  1 -
> >>   arch/arm64/tools/sysreg         | 11 +++++++++++
> >>   2 files changed, 11 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> >> index 7d301700d1a9..910e960661d3 100644
> >> --- a/arch/arm64/include/asm/sysreg.h
> >> +++ b/arch/arm64/include/asm/sysreg.h
> >> @@ -425,7 +425,6 @@
> >>     #define SYS_CNTKCTL_EL1			sys_reg(3, 0, 14, 1,
> >> 0)
> >>   -#define SYS_CCSIDR_EL1			sys_reg(3, 1, 0, 0, 0)
> >>   #define SYS_AIDR_EL1			sys_reg(3, 1, 0, 0, 7)
> >>     #define SYS_RNDR_EL0			sys_reg(3, 3, 2, 4, 0)
> >> diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
> >> index 384757a7eda9..acc79b5ccf92 100644
> >> --- a/arch/arm64/tools/sysreg
> >> +++ b/arch/arm64/tools/sysreg
> >> @@ -871,6 +871,17 @@ Sysreg	SCXTNUM_EL1	3	0	13	0	7
> >>   Field	63:0	SoftwareContextNumber
> >>   EndSysreg
> >>   +Sysreg	CCSIDR_EL1	3	1	0	0	0
> >> +Res0	63:32
> >> +Field	31:31	WT
> >> +Field	30:30	WB
> >> +Field	29:29	RA
> >> +Field	28:28	WA
> > 
> > For fields described as a single bit, the tool supports simply
> > indicating the bit number (28 rather than 28:28).
> > 
> > However, I strongly recommend against describing fields that have been
> > dropped from the architecture.  This only happens when these fields
> > are never used by any implementation, so describing them is at best
> > useless.
> 
> arch/arm64/tools/gen-sysreg.awk does not allow a hole and requires all
> bits are described hence these descriptions. If you have an
> alternative idea I'd like to hear.

I'd simply suggest creating an UNKNOWN field encompassing bits
[21:28]. Alternatively, feel free to try the patch below, which allows
you to describe these 4 bits as "Unkn	31:28", similar to Res0/Res1.

>
> > 
> >> +Field	27:13	NumSets
> >> +Field	12:3	Associavity

Also, you may want to fix the typo here (Associativity).

Thanks,

	M.

From 3112be25ec785de4c92d11d5964d54f216a2289c Mon Sep 17 00:00:00 2001
From: Marc Zyngier <maz@xxxxxxxxxx>
Date: Sun, 18 Dec 2022 12:55:23 +0000
Subject: [PATCH] arm64: Allow the definition of UNKNOWN system register fields

The CCSIDR_EL1 register contains an UNKNOWN field (which replaces
fields that were actually defined in previous revisions of the
architecture).

Define an 'Unkn' field type modeled after the Res0/Res1 types
to allow such description. This allows the generation of

  #define CCSIDR_EL1_UNKN     (UL(0) | GENMASK_ULL(31, 28))

which may have its use one day. Hopefully the architecture doesn't
add too many of those in the future.

Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx>
---
 arch/arm64/tools/gen-sysreg.awk | 20 +++++++++++++++++++-
 arch/arm64/tools/sysreg         |  2 ++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/tools/gen-sysreg.awk b/arch/arm64/tools/gen-sysreg.awk
index c350164a3955..e1df4b956596 100755
--- a/arch/arm64/tools/gen-sysreg.awk
+++ b/arch/arm64/tools/gen-sysreg.awk
@@ -98,6 +98,7 @@ END {
 
 	res0 = "UL(0)"
 	res1 = "UL(0)"
+	unkn = "UL(0)"
 
 	next_bit = 63
 
@@ -112,11 +113,13 @@ END {
 
 	define(reg "_RES0", "(" res0 ")")
 	define(reg "_RES1", "(" res1 ")")
+	define(reg "_UNKN", "(" unkn ")")
 	print ""
 
 	reg = null
 	res0 = null
 	res1 = null
+	unkn = null
 
 	next
 }
@@ -134,6 +137,7 @@ END {
 
 	res0 = "UL(0)"
 	res1 = "UL(0)"
+	unkn = "UL(0)"
 
 	define("REG_" reg, "S" op0 "_" op1 "_C" crn "_C" crm "_" op2)
 	define("SYS_" reg, "sys_reg(" op0 ", " op1 ", " crn ", " crm ", " op2 ")")
@@ -161,7 +165,9 @@ END {
 		define(reg "_RES0", "(" res0 ")")
 	if (res1 != null)
 		define(reg "_RES1", "(" res1 ")")
-	if (res0 != null || res1 != null)
+	if (unkn != null)
+		define(reg "_UNKN", "(" unkn ")")
+	if (res0 != null || res1 != null || unkn != null)
 		print ""
 
 	reg = null
@@ -172,6 +178,7 @@ END {
 	op2 = null
 	res0 = null
 	res1 = null
+	unkn = null
 
 	next
 }
@@ -190,6 +197,7 @@ END {
         next_bit = 0
 	res0 = null
 	res1 = null
+	unkn = null
 
 	next
 }
@@ -215,6 +223,16 @@ END {
 	next
 }
 
+/^Unkn/ && (block == "Sysreg" || block == "SysregFields") {
+	expect_fields(2)
+	parse_bitdef(reg, "UNKN", $2)
+	field = "UNKN_" msb "_" lsb
+
+	unkn = unkn " | GENMASK_ULL(" msb ", " lsb ")"
+
+	next
+}
+
 /^Field/ && (block == "Sysreg" || block == "SysregFields") {
 	expect_fields(3)
 	field = $3
diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
index bd5fceb26c54..472f68f020d9 100644
--- a/arch/arm64/tools/sysreg
+++ b/arch/arm64/tools/sysreg
@@ -15,6 +15,8 @@
 
 # Res1	<msb>[:<lsb>]
 
+# Unkn	<msb>[:<lsb>]
+
 # Field	<msb>[:<lsb>]	<name>
 
 # Enum	<msb>[:<lsb>]	<name>
-- 
2.34.1


-- 
Without deviation from the norm, progress is not possible.