Before this change, the cache configuration of the physical CPU was
exposed to vcpus. This is problematic because the cache configuration a
vcpu sees varies when it migrates between vcpus with different cache
configurations.
Fabricate cache configuration from the sanitized value, which holds the
CTR_EL0 value the userspace sees regardless of which physical CPU it
resides on.
V2 -> V3:
- Corrected message for patch "Normalize cache configuration"
- Split patch "Normalize cache configuration"
- Added handling for CSSELR_EL1.TnD
- Added code to ignore RES0 in CSSELR_EL1
- Replaced arm64_ftr_reg_ctrel0.sys_val with
read_sanitised_ftr_reg(SYS_CTR_EL0)
- Fixed vcpu->arch.ccsidr initialziation
- Added CCSIDR_EL1 sanitization
- Added FWB check
- Added a comment for CACHE_TYPE_SEPARATE
- Added MTE tag cache creation code for CLIDR_EL1 fabrication
- Removed CLIDR_EL1 reset code for reset caused by guest
- Added a comment for CCSIDR2
V2: https://lore.kernel.org/lkml/[email protected]/
V1: https://lore.kernel.org/lkml/[email protected]/
Akihiko Odaki (7):
arm64/sysreg: Convert CCSIDR_EL1 to automatic generation
arm64/sysreg: Add CCSIDR2_EL1
arm64/cache: Move CLIDR macro definitions
KVM: arm64: Always set HCR_TID2
KVM: arm64: Allow user to set CCSIDR_EL1
KVM: arm64: Mask FEAT_CCIDX
KVM: arm64: Normalize cache configuration
arch/arm64/include/asm/cache.h | 9 +
arch/arm64/include/asm/kvm_arm.h | 3 +-
arch/arm64/include/asm/kvm_emulate.h | 4 -
arch/arm64/include/asm/kvm_host.h | 6 +-
arch/arm64/include/asm/sysreg.h | 1 -
arch/arm64/kernel/cacheinfo.c | 5 -
arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 2 -
arch/arm64/kvm/reset.c | 1 +
arch/arm64/kvm/sys_regs.c | 246 +++++++++++++--------
arch/arm64/tools/sysreg | 16 ++
10 files changed, 182 insertions(+), 111 deletions(-)
--
2.38.1
Always set HCR_TID2 to trap CTR_EL0, CCSIDR2_EL1, CLIDR_EL1, and
CSSELR_EL1. This saves a few lines of code and allows to employ their
access trap handlers for more purposes anticipated by the old
condition for setting HCR_TID2.
Suggested-by: Marc Zyngier <[email protected]>
Signed-off-by: Akihiko Odaki <[email protected]>
---
arch/arm64/include/asm/kvm_arm.h | 3 ++-
arch/arm64/include/asm/kvm_emulate.h | 4 ----
arch/arm64/include/asm/kvm_host.h | 2 --
arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 2 --
4 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index 8aa8492dafc0..44be46c280c1 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -81,11 +81,12 @@
* SWIO: Turn set/way invalidates into set/way clean+invalidate
* PTW: Take a stage2 fault if a stage1 walk steps in device memory
* TID3: Trap EL1 reads of group 3 ID registers
+ * TID2: Trap CTR_EL0, CCSIDR2_EL1, CLIDR_EL1, and CSSELR_EL1
*/
#define HCR_GUEST_FLAGS (HCR_TSC | HCR_TSW | HCR_TWE | HCR_TWI | HCR_VM | \
HCR_BSU_IS | HCR_FB | HCR_TACR | \
HCR_AMO | HCR_SWIO | HCR_TIDCP | HCR_RW | HCR_TLOR | \
- HCR_FMO | HCR_IMO | HCR_PTW | HCR_TID3 )
+ HCR_FMO | HCR_IMO | HCR_PTW | HCR_TID3 | HCR_TID2)
#define HCR_VIRT_EXCP_MASK (HCR_VSE | HCR_VI | HCR_VF)
#define HCR_HOST_NVHE_FLAGS (HCR_RW | HCR_API | HCR_APK | HCR_ATA)
#define HCR_HOST_NVHE_PROTECTED_FLAGS (HCR_HOST_NVHE_FLAGS | HCR_TSC)
diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index 9bdba47f7e14..30c4598d643b 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -88,10 +88,6 @@ static inline void vcpu_reset_hcr(struct kvm_vcpu *vcpu)
if (vcpu_el1_is_32bit(vcpu))
vcpu->arch.hcr_el2 &= ~HCR_RW;
- if (cpus_have_const_cap(ARM64_MISMATCHED_CACHE_TYPE) ||
- vcpu_el1_is_32bit(vcpu))
- vcpu->arch.hcr_el2 |= HCR_TID2;
-
if (kvm_has_mte(vcpu->kvm))
vcpu->arch.hcr_el2 |= HCR_ATA;
}
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 45e2136322ba..cc2ede0eaed4 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -621,7 +621,6 @@ static inline bool __vcpu_read_sys_reg_from_cpu(int reg, u64 *val)
return false;
switch (reg) {
- case CSSELR_EL1: *val = read_sysreg_s(SYS_CSSELR_EL1); break;
case SCTLR_EL1: *val = read_sysreg_s(SYS_SCTLR_EL12); break;
case CPACR_EL1: *val = read_sysreg_s(SYS_CPACR_EL12); break;
case TTBR0_EL1: *val = read_sysreg_s(SYS_TTBR0_EL12); break;
@@ -666,7 +665,6 @@ static inline bool __vcpu_write_sys_reg_to_cpu(u64 val, int reg)
return false;
switch (reg) {
- case CSSELR_EL1: write_sysreg_s(val, SYS_CSSELR_EL1); break;
case SCTLR_EL1: write_sysreg_s(val, SYS_SCTLR_EL12); break;
case CPACR_EL1: write_sysreg_s(val, SYS_CPACR_EL12); break;
case TTBR0_EL1: write_sysreg_s(val, SYS_TTBR0_EL12); break;
diff --git a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
index baa5b9b3dde5..147cb4c846c6 100644
--- a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
+++ b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
@@ -39,7 +39,6 @@ static inline bool ctxt_has_mte(struct kvm_cpu_context *ctxt)
static inline void __sysreg_save_el1_state(struct kvm_cpu_context *ctxt)
{
- ctxt_sys_reg(ctxt, CSSELR_EL1) = read_sysreg(csselr_el1);
ctxt_sys_reg(ctxt, SCTLR_EL1) = read_sysreg_el1(SYS_SCTLR);
ctxt_sys_reg(ctxt, CPACR_EL1) = read_sysreg_el1(SYS_CPACR);
ctxt_sys_reg(ctxt, TTBR0_EL1) = read_sysreg_el1(SYS_TTBR0);
@@ -95,7 +94,6 @@ static inline void __sysreg_restore_user_state(struct kvm_cpu_context *ctxt)
static inline void __sysreg_restore_el1_state(struct kvm_cpu_context *ctxt)
{
write_sysreg(ctxt_sys_reg(ctxt, MPIDR_EL1), vmpidr_el2);
- write_sysreg(ctxt_sys_reg(ctxt, CSSELR_EL1), csselr_el1);
if (has_vhe() ||
!cpus_have_final_cap(ARM64_WORKAROUND_SPECULATIVE_AT)) {
--
2.38.1
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 <[email protected]>
---
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
+Field 27:13 NumSets
+Field 12:3 Associavity
+Field 2:0 LineSize
+EndSysreg
+
Sysreg CLIDR_EL1 3 1 0 0 1
Res0 63:47
Field 46:33 Ttypen
--
2.38.1
On Sun, 18 Dec 2022 05:14:06 +0000,
Akihiko Odaki <[email protected]> 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 <[email protected]>
> ---
> 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.
> +Field 27:13 NumSets
> +Field 12:3 Associavity
> +Field 2:0 LineSize
> +EndSysreg
> +
I don't think we have a good solution for overlapping fields that
depend on other factors, either contextual (such as a mode that
changes the layout of a sysreg), or architecture warts such as
FEAT_CCIDX (which changes the layout of a well-known sysreg).
At least, put a comment here that indicates the context of the
description.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
On 2022/12/18 20:23, Marc Zyngier wrote:
> On Sun, 18 Dec 2022 05:14:06 +0000,
> Akihiko Odaki <[email protected]> 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 <[email protected]>
>> ---
>> 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.
>
>> +Field 27:13 NumSets
>> +Field 12:3 Associavity
>> +Field 2:0 LineSize
>> +EndSysreg
>> +
>
> I don't think we have a good solution for overlapping fields that
> depend on other factors, either contextual (such as a mode that
> changes the layout of a sysreg), or architecture warts such as
> FEAT_CCIDX (which changes the layout of a well-known sysreg).
>
> At least, put a comment here that indicates the context of the
> description.
Sounds good. I'll do so with the next version.
Regards,
Akihiko Odaki
>
> Thanks,
>
> M.
>
On Sun, 18 Dec 2022 11:35:12 +0000,
Akihiko Odaki <[email protected]> wrote:
>
> On 2022/12/18 20:23, Marc Zyngier wrote:
> > On Sun, 18 Dec 2022 05:14:06 +0000,
> > Akihiko Odaki <[email protected]> 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 <[email protected]>
> >> ---
> >> 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 <[email protected]>
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 <[email protected]>
---
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.
On Sun, Dec 18, 2022 at 01:11:01PM +0000, Marc Zyngier wrote:
> Akihiko Odaki <[email protected]> wrote:
> > 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.
I agree, where practical we should add new field types and other
features as needed rather than trying to shoehorn things into what the
tool currently supports. It is very much a work in progress which can't
fully represent everything in the spec yet. For things like the
registers with multiple possible views it's much more effort which
shouldn't get in the way of progress on features but with something like
this just updating the tool so we can match the architecture spec is the
right thing.
> Define an 'Unkn' field type modeled after the Res0/Res1 types
> to allow such description. This allows the generation of
I'd be tempted to spell out Unknown fully since Unkn is not such a
common abbreviation but I can see the desire to keep the name shorter
and it doesn't really matter so either way:
Reviewed-by: Mark Brown <[email protected]>
On Mon, 19 Dec 2022 15:00:15 +0000,
Mark Brown <[email protected]> wrote:
>
> [1 <text/plain; us-ascii (7bit)>]
> On Sun, Dec 18, 2022 at 01:11:01PM +0000, Marc Zyngier wrote:
> > Akihiko Odaki <[email protected]> wrote:
>
> > > 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.
>
> I agree, where practical we should add new field types and other
> features as needed rather than trying to shoehorn things into what the
> tool currently supports. It is very much a work in progress which can't
> fully represent everything in the spec yet. For things like the
> registers with multiple possible views it's much more effort which
> shouldn't get in the way of progress on features but with something like
> this just updating the tool so we can match the architecture spec is the
> right thing.
I was tempted to add a Namespace tag that wouldn't generate the sysreg
#defines, but only generate the fields with a feature-specific
namespace. For example:
Sysreg CCSIDR_EL1 3 1 0 0 0
Res0 63:32
Unkn 31:28
Field 27:13 NumSets
Field 12:3 Associativity
Field 2:0 LineSize
EndSysreg
Namespace CCIDX CCSIDR_EL1
Res0 63:56
Field 55:32 NumSets
Res0 31:25
Field 24:3 Associativity
Field 2:0 LineSize
EndSysreg
the later generating:
#define CCIDR_EL1_CCIDX_RES0 (GENMASK(63, 56) | GENMASK(31, 25))
#define CCIDR_EL1_CCIDX_NumSets GENMASK(55, 32)
#define CCIDR_EL1_CCIDX_Associativity GENMASK(24, 3)
#define CCIDR_EL1_CCIDX_LineSize GENMASK(2, 0)
Thoughts?
>
> > Define an 'Unkn' field type modeled after the Res0/Res1 types
> > to allow such description. This allows the generation of
>
> I'd be tempted to spell out Unknown fully since Unkn is not such a
> common abbreviation but I can see the desire to keep the name shorter
> and it doesn't really matter so either way:
>
> Reviewed-by: Mark Brown <[email protected]>
Yeah, this stuff is write-only most of the time, and I like my fields
aligned if at all possible.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
On Mon, Dec 19, 2022 at 03:27:17PM +0000, Marc Zyngier wrote:
> Mark Brown <[email protected]> wrote:
> > fully represent everything in the spec yet. For things like the
> > registers with multiple possible views it's much more effort which
> > shouldn't get in the way of progress on features but with something like
> > this just updating the tool so we can match the architecture spec is the
> > right thing.
> I was tempted to add a Namespace tag that wouldn't generate the sysreg
> #defines, but only generate the fields with a feature-specific
> namespace. For example:
I think this is roughly where we'd end up - I was using the term view
when thinking about it but that's just bikeshed.
> Sysreg CCSIDR_EL1 3 1 0 0 0
> Res0 63:32
> Unkn 31:28
> Field 27:13 NumSets
> Field 12:3 Associativity
> Field 2:0 LineSize
> EndSysreg
>
> Namespace CCIDX CCSIDR_EL1
> Res0 63:56
> Field 55:32 NumSets
> Res0 31:25
> Field 24:3 Associativity
> Field 2:0 LineSize
> EndSysreg
Yeah, something like that. I think we also want a way to label bits in
the root register as only existing in namespaces/views for things where
there's no default (eg, where a feature adds two views at once or things
have been there since the base architecture), and I wasn't sure if it
made sense to nest the declaration of the views inside the Sysreg (I'm
tempted to think it's more trouble than it's worth especially on the
tooling side).
I also wanted to go through and do an audit of all the current registers
to make sure there were no nasty cases that'd complicate things. I
don't think there'd be anything but...
> the later generating:
> #define CCIDR_EL1_CCIDX_RES0 (GENMASK(63, 56) | GENMASK(31, 25))
> #define CCIDR_EL1_CCIDX_NumSets GENMASK(55, 32)
> #define CCIDR_EL1_CCIDX_Associativity GENMASK(24, 3)
> #define CCIDR_EL1_CCIDX_LineSize GENMASK(2, 0)
> Thoughts?
Definitely that for the output.