[boot-wrapper 0/3] aarch64: Enable access for FEAT_D128 registers in EL1/EL2

[boot-wrapper 0/3] aarch64: Enable access for FEAT_D128 registers in EL1/EL2

[Anshuman Khandual]

This series enables access for FEAT_D128 relevant registers in EL1/EL2 via
setting respective bits in SCR_EL3, when their corresponding features are
detected.

--------------------------------------------------------------
| FEAT_D128   | ID_AA64MMFR3_EL1_D128   |  SCR_EL3_D128En    |
| FEAT_SCTLR2 | ID_AA64MMFR3_EL1_SCTLRX |  SCR_EL3_SCTLR2En  |
| FEAT_THE    | ID_AA64PFR1_EL1_THE     |  SCR_EL3_RCWMASKEn |
--------------------------------------------------------------

Anshuman Khandual (3):
  aarch64: Enable access into 128 bit system registers from EL2 and below
  aarch64: Enable access into SCTLR2_ELx registers from EL2 and below
  aarch64: Enable access into RCW[S]MASK_EL1 registers from EL2 and below

 arch/aarch64/include/asm/cpu.h | 8 +++++++-
 arch/aarch64/init.c            | 9 +++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

-- 
2.25.1

[boot-wrapper 1/3] aarch64: Enable access into 128 bit system registers from EL2 and below

[Anshuman Khandual]

FEAT_D128 adds 128 bit system registers like PAR_EL1, TTBR0_EL1, TTBR1_EL1,
TTBR0_EL2, TTBR1_EL2, TTBR0_EL12, TTBR1_EL12 , VTTBR_EL2, RCWMASK_EL1, and
RCWSMASK_EL1. But access into these register from EL2 and below trap to EL3
unless SCR_EL3.D128En is set.

Enable access to 128 bit registers when they are implemented.

Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 arch/aarch64/include/asm/cpu.h | 2 ++
 arch/aarch64/init.c            | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h
index 124ef91..0b8b463 100644
--- a/arch/aarch64/include/asm/cpu.h
+++ b/arch/aarch64/include/asm/cpu.h
@@ -57,6 +57,7 @@
 #define SCR_EL3_EnTP2			BIT(41)
 #define SCR_EL3_TCR2EN			BIT(43)
 #define SCR_EL3_PIEN			BIT(45)
+#define SCR_EL3_D128En			BIT(47)
 
 #define HCR_EL2_RES1			BIT(1)
 
@@ -85,6 +86,7 @@
 #define ID_AA64MMFR3_EL1_S2PIE		BITS(15, 12)
 #define ID_AA64MMFR3_EL1_S1POE		BITS(19, 16)
 #define ID_AA64MMFR3_EL1_S2POE		BITS(23, 20)
+#define ID_AA64MMFR3_EL1_D128		BITS(35, 32)
 
 #define ID_AA64PFR1_EL1_MTE		BITS(11, 8)
 #define ID_AA64PFR1_EL1_SME		BITS(27, 24)
diff --git a/arch/aarch64/init.c b/arch/aarch64/init.c
index 37cb45f..7d9d0d9 100644
--- a/arch/aarch64/init.c
+++ b/arch/aarch64/init.c
@@ -89,6 +89,9 @@ void cpu_init_el3(void)
 	if (!kernel_is_32bit())
 		scr |= SCR_EL3_RW;
 
+	if (mrs_field(ID_AA64MMFR3_EL1, D128))
+		scr |= SCR_EL3_D128En;
+
 	msr(SCR_EL3, scr);
 
 	msr(CPTR_EL3, cptr);
-- 
2.25.1

[boot-wrapper 2/3] aarch64: Enable access into SCTLR2_ELx registers from EL2 and below

[Anshuman Khandual]

FEAT_SCTLR2 adds SCTLR2_EL1 and SCTLR2_EL2 system registers But access into
these register from EL2 and below trap to EL3 unless SCR_EL3.D128En is set.

Enable access to SCTLR2_ELx registers when they are implemented.

Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 arch/aarch64/include/asm/cpu.h | 4 +++-
 arch/aarch64/init.c            | 3 +++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h
index 0b8b463..57d66e4 100644
--- a/arch/aarch64/include/asm/cpu.h
+++ b/arch/aarch64/include/asm/cpu.h
@@ -56,6 +56,7 @@
 #define SCR_EL3_HXEn			BIT(38)
 #define SCR_EL3_EnTP2			BIT(41)
 #define SCR_EL3_TCR2EN			BIT(43)
+#define SCR_EL3_SCTLR2En		BIT(44)
 #define SCR_EL3_PIEN			BIT(45)
 #define SCR_EL3_D128En			BIT(47)
 
@@ -81,7 +82,8 @@
 
 #define ID_AA64MMFR1_EL1_HCX		BITS(43, 40)
 
-#define ID_AA64MMFR3_EL1_TCRX		BITS(4, 0)
+#define ID_AA64MMFR3_EL1_TCRX		BITS(3, 0)
+#define ID_AA64MMFR3_EL1_SCTLRX		BITS(7, 4)
 #define ID_AA64MMFR3_EL1_S1PIE		BITS(11, 8)
 #define ID_AA64MMFR3_EL1_S2PIE		BITS(15, 12)
 #define ID_AA64MMFR3_EL1_S1POE		BITS(19, 16)
diff --git a/arch/aarch64/init.c b/arch/aarch64/init.c
index 7d9d0d9..5b21cb8 100644
--- a/arch/aarch64/init.c
+++ b/arch/aarch64/init.c
@@ -92,6 +92,9 @@ void cpu_init_el3(void)
 	if (mrs_field(ID_AA64MMFR3_EL1, D128))
 		scr |= SCR_EL3_D128En;
 
+	if (mrs_field(ID_AA64MMFR3_EL1, SCTLRX))
+		scr |= SCR_EL3_SCTLR2En;
+
 	msr(SCR_EL3, scr);
 
 	msr(CPTR_EL3, cptr);
-- 
2.25.1

[boot-wrapper 3/3] aarch64: Enable access into RCW[S]MASK_EL1 registers from EL2 and below

[Anshuman Khandual]

FEAT_THE adds RCW[S]MASK_EL1 system registers. But access into these system
registers from EL2 and below trap to EL3 unless SCR_EL3.RCWMASKEn is set.

Enable access to RCW[S]MASK_EL1 registers when they are implemented.

Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 arch/aarch64/include/asm/cpu.h | 2 ++
 arch/aarch64/init.c            | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h
index 57d66e4..8404152 100644
--- a/arch/aarch64/include/asm/cpu.h
+++ b/arch/aarch64/include/asm/cpu.h
@@ -55,6 +55,7 @@
 #define SCR_EL3_TME			BIT(34)
 #define SCR_EL3_HXEn			BIT(38)
 #define SCR_EL3_EnTP2			BIT(41)
+#define SCR_EL3_RCWMASKEn		BIT(42)
 #define SCR_EL3_TCR2EN			BIT(43)
 #define SCR_EL3_SCTLR2En		BIT(44)
 #define SCR_EL3_PIEN			BIT(45)
@@ -92,6 +93,7 @@
 
 #define ID_AA64PFR1_EL1_MTE		BITS(11, 8)
 #define ID_AA64PFR1_EL1_SME		BITS(27, 24)
+#define ID_AA64PFR1_EL1_THE		BITS(51, 48)
 #define ID_AA64PFR0_EL1_SVE		BITS(35, 32)
 
 #define ID_AA64SMFR0_EL1		s3_0_c0_c4_5
diff --git a/arch/aarch64/init.c b/arch/aarch64/init.c
index 5b21cb8..13a2339 100644
--- a/arch/aarch64/init.c
+++ b/arch/aarch64/init.c
@@ -95,6 +95,9 @@ void cpu_init_el3(void)
 	if (mrs_field(ID_AA64MMFR3_EL1, SCTLRX))
 		scr |= SCR_EL3_SCTLR2En;
 
+	if (mrs_field(ID_AA64PFR1_EL1, THE))
+		scr |= SCR_EL3_RCWMASKEn;
+
 	msr(SCR_EL3, scr);
 
 	msr(CPTR_EL3, cptr);
-- 
2.25.1

Re: [boot-wrapper 2/3] aarch64: Enable access into SCTLR2_ELx registers from EL2 and below

[Mark Rutland]

On Tue, Jul 23, 2024 at 04:36:29PM +0530, Anshuman Khandual wrote:
> FEAT_SCTLR2 adds SCTLR2_EL1 and SCTLR2_EL2 system registers But access into
> these register from EL2 and below trap to EL3 unless SCR_EL3.D128En is set.
> 
> Enable access to SCTLR2_ELx registers when they are implemented.
> 
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>

I think this should be first in the series, since SCTLR2 can exist
without D128, but not vice-versa.

> ---
>  arch/aarch64/include/asm/cpu.h | 4 +++-
>  arch/aarch64/init.c            | 3 +++
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h
> index 0b8b463..57d66e4 100644
> --- a/arch/aarch64/include/asm/cpu.h
> +++ b/arch/aarch64/include/asm/cpu.h
> @@ -56,6 +56,7 @@
>  #define SCR_EL3_HXEn			BIT(38)
>  #define SCR_EL3_EnTP2			BIT(41)
>  #define SCR_EL3_TCR2EN			BIT(43)
> +#define SCR_EL3_SCTLR2En		BIT(44)
>  #define SCR_EL3_PIEN			BIT(45)
>  #define SCR_EL3_D128En			BIT(47)
>  
> @@ -81,7 +82,8 @@
>  
>  #define ID_AA64MMFR1_EL1_HCX		BITS(43, 40)
>  
> -#define ID_AA64MMFR3_EL1_TCRX		BITS(4, 0)
> +#define ID_AA64MMFR3_EL1_TCRX		BITS(3, 0)
> +#define ID_AA64MMFR3_EL1_SCTLRX		BITS(7, 4)
>  #define ID_AA64MMFR3_EL1_S1PIE		BITS(11, 8)
>  #define ID_AA64MMFR3_EL1_S2PIE		BITS(15, 12)
>  #define ID_AA64MMFR3_EL1_S1POE		BITS(19, 16)
> diff --git a/arch/aarch64/init.c b/arch/aarch64/init.c
> index 7d9d0d9..5b21cb8 100644
> --- a/arch/aarch64/init.c
> +++ b/arch/aarch64/init.c
> @@ -92,6 +92,9 @@ void cpu_init_el3(void)
>  	if (mrs_field(ID_AA64MMFR3_EL1, D128))
>  		scr |= SCR_EL3_D128En;
>  
> +	if (mrs_field(ID_AA64MMFR3_EL1, SCTLRX))
> +		scr |= SCR_EL3_SCTLR2En;
> +

The SCTLR2_ELx registers reset to UNKNOWN values when the highest
implemented exception level is not ELx, so we need to initialize those
to safe values. Otherwise a kernel which is not aware of SCTLR2_ELx will
be subject to arbitrary behaviour as a result of the SCTLR2_ELx bits
which it will not have configured.

I know that we've failed to do that for other things (FGT and HCRX), and
those are latent bugs / mistakes in our appraoch that I'll see about
fixing.

Mark.

Re: [boot-wrapper 1/3] aarch64: Enable access into 128 bit system registers from EL2 and below

[Mark Rutland]

On Tue, Jul 23, 2024 at 04:36:28PM +0530, Anshuman Khandual wrote:
> FEAT_D128 adds 128 bit system registers like PAR_EL1, TTBR0_EL1, TTBR1_EL1,
> TTBR0_EL2, TTBR1_EL2, TTBR0_EL12, TTBR1_EL12 , VTTBR_EL2, RCWMASK_EL1, and
> RCWSMASK_EL1. But access into these register from EL2 and below trap to EL3
> unless SCR_EL3.D128En is set.

Do we need to initialize any of these new bits, or do they have no
effect on a lower EL unless explicitly opted-in at that lower EL?

Otherwise this looks superficially fine, but it should come after
enabling SCTLR2.

Mark.

> 
> Enable access to 128 bit registers when they are implemented.
> 
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>  arch/aarch64/include/asm/cpu.h | 2 ++
>  arch/aarch64/init.c            | 3 +++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h
> index 124ef91..0b8b463 100644
> --- a/arch/aarch64/include/asm/cpu.h
> +++ b/arch/aarch64/include/asm/cpu.h
> @@ -57,6 +57,7 @@
>  #define SCR_EL3_EnTP2			BIT(41)
>  #define SCR_EL3_TCR2EN			BIT(43)
>  #define SCR_EL3_PIEN			BIT(45)
> +#define SCR_EL3_D128En			BIT(47)
>  
>  #define HCR_EL2_RES1			BIT(1)
>  
> @@ -85,6 +86,7 @@
>  #define ID_AA64MMFR3_EL1_S2PIE		BITS(15, 12)
>  #define ID_AA64MMFR3_EL1_S1POE		BITS(19, 16)
>  #define ID_AA64MMFR3_EL1_S2POE		BITS(23, 20)
> +#define ID_AA64MMFR3_EL1_D128		BITS(35, 32)
>  
>  #define ID_AA64PFR1_EL1_MTE		BITS(11, 8)
>  #define ID_AA64PFR1_EL1_SME		BITS(27, 24)
> diff --git a/arch/aarch64/init.c b/arch/aarch64/init.c
> index 37cb45f..7d9d0d9 100644
> --- a/arch/aarch64/init.c
> +++ b/arch/aarch64/init.c
> @@ -89,6 +89,9 @@ void cpu_init_el3(void)
>  	if (!kernel_is_32bit())
>  		scr |= SCR_EL3_RW;
>  
> +	if (mrs_field(ID_AA64MMFR3_EL1, D128))
> +		scr |= SCR_EL3_D128En;
> +
>  	msr(SCR_EL3, scr);
>  
>  	msr(CPTR_EL3, cptr);
> -- 
> 2.25.1
> 

Re: [boot-wrapper 3/3] aarch64: Enable access into RCW[S]MASK_EL1 registers from EL2 and below

[Mark Rutland]

On Tue, Jul 23, 2024 at 04:36:30PM +0530, Anshuman Khandual wrote:
> FEAT_THE adds RCW[S]MASK_EL1 system registers. But access into these system
> registers from EL2 and below trap to EL3 unless SCR_EL3.RCWMASKEn is set.
> 
> Enable access to RCW[S]MASK_EL1 registers when they are implemented.

This looks fine.

IIUC we don't need to initialize these new registers as they only affect
the behaviour of new instructions which we don't expect SW to use until
privileged SW has configured these registers (as they reset to UNKNOWN
values even at the highest implemented EL).

Mark.

> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>  arch/aarch64/include/asm/cpu.h | 2 ++
>  arch/aarch64/init.c            | 3 +++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h
> index 57d66e4..8404152 100644
> --- a/arch/aarch64/include/asm/cpu.h
> +++ b/arch/aarch64/include/asm/cpu.h
> @@ -55,6 +55,7 @@
>  #define SCR_EL3_TME			BIT(34)
>  #define SCR_EL3_HXEn			BIT(38)
>  #define SCR_EL3_EnTP2			BIT(41)
> +#define SCR_EL3_RCWMASKEn		BIT(42)
>  #define SCR_EL3_TCR2EN			BIT(43)
>  #define SCR_EL3_SCTLR2En		BIT(44)
>  #define SCR_EL3_PIEN			BIT(45)
> @@ -92,6 +93,7 @@
>  
>  #define ID_AA64PFR1_EL1_MTE		BITS(11, 8)
>  #define ID_AA64PFR1_EL1_SME		BITS(27, 24)
> +#define ID_AA64PFR1_EL1_THE		BITS(51, 48)
>  #define ID_AA64PFR0_EL1_SVE		BITS(35, 32)
>  
>  #define ID_AA64SMFR0_EL1		s3_0_c0_c4_5
> diff --git a/arch/aarch64/init.c b/arch/aarch64/init.c
> index 5b21cb8..13a2339 100644
> --- a/arch/aarch64/init.c
> +++ b/arch/aarch64/init.c
> @@ -95,6 +95,9 @@ void cpu_init_el3(void)
>  	if (mrs_field(ID_AA64MMFR3_EL1, SCTLRX))
>  		scr |= SCR_EL3_SCTLR2En;
>  
> +	if (mrs_field(ID_AA64PFR1_EL1, THE))
> +		scr |= SCR_EL3_RCWMASKEn;
> +
>  	msr(SCR_EL3, scr);
>  
>  	msr(CPTR_EL3, cptr);
> -- 
> 2.25.1
> 

Re: [boot-wrapper 2/3] aarch64: Enable access into SCTLR2_ELx registers from EL2 and below

[Anshuman Khandual]

On 7/25/24 14:10, Mark Rutland wrote:
> On Tue, Jul 23, 2024 at 04:36:29PM +0530, Anshuman Khandual wrote:
>> FEAT_SCTLR2 adds SCTLR2_EL1 and SCTLR2_EL2 system registers But access into
>> these register from EL2 and below trap to EL3 unless SCR_EL3.D128En is set.

Will fix a small nit here, s/SCR_EL3.D128En/SCR_EL3.SCTLR2En/ ^^^

>>
>> Enable access to SCTLR2_ELx registers when they are implemented.
>>
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> 
> I think this should be first in the series, since SCTLR2 can exist
> without D128, but not vice-versa.

Sure, will move it as the first patch in the series.

> 
>> ---
>>  arch/aarch64/include/asm/cpu.h | 4 +++-
>>  arch/aarch64/init.c            | 3 +++
>>  2 files changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h
>> index 0b8b463..57d66e4 100644
>> --- a/arch/aarch64/include/asm/cpu.h
>> +++ b/arch/aarch64/include/asm/cpu.h
>> @@ -56,6 +56,7 @@
>>  #define SCR_EL3_HXEn			BIT(38)
>>  #define SCR_EL3_EnTP2			BIT(41)
>>  #define SCR_EL3_TCR2EN			BIT(43)
>> +#define SCR_EL3_SCTLR2En		BIT(44)
>>  #define SCR_EL3_PIEN			BIT(45)
>>  #define SCR_EL3_D128En			BIT(47)
>>  
>> @@ -81,7 +82,8 @@
>>  
>>  #define ID_AA64MMFR1_EL1_HCX		BITS(43, 40)
>>  
>> -#define ID_AA64MMFR3_EL1_TCRX		BITS(4, 0)
>> +#define ID_AA64MMFR3_EL1_TCRX		BITS(3, 0)
>> +#define ID_AA64MMFR3_EL1_SCTLRX		BITS(7, 4)
>>  #define ID_AA64MMFR3_EL1_S1PIE		BITS(11, 8)
>>  #define ID_AA64MMFR3_EL1_S2PIE		BITS(15, 12)
>>  #define ID_AA64MMFR3_EL1_S1POE		BITS(19, 16)
>> diff --git a/arch/aarch64/init.c b/arch/aarch64/init.c
>> index 7d9d0d9..5b21cb8 100644
>> --- a/arch/aarch64/init.c
>> +++ b/arch/aarch64/init.c
>> @@ -92,6 +92,9 @@ void cpu_init_el3(void)
>>  	if (mrs_field(ID_AA64MMFR3_EL1, D128))
>>  		scr |= SCR_EL3_D128En;
>>  
>> +	if (mrs_field(ID_AA64MMFR3_EL1, SCTLRX))
>> +		scr |= SCR_EL3_SCTLR2En;
>> +
> 
> The SCTLR2_ELx registers reset to UNKNOWN values when the highest
> implemented exception level is not ELx, so we need to initialize those
> to safe values. Otherwise a kernel which is not aware of SCTLR2_ELx will
> be subject to arbitrary behaviour as a result of the SCTLR2_ELx bits
> which it will not have configured.

Both SCTLR2_EL1 and SCTLR2_EL2 has the same register fields layout
except the very last bit i.e SCTLR2_EL2.EMEC which is available in
SCTLR2_EL2 but not in SCTLR2_EL1.

AFAICT all the above register fields are applicable for newer arch
features which the current kernel is not even aware about. So even
if the kernel is not ware about SCTLR2_EL2 or SCTLR2_EL1 registers,
there will not be any difference in behaviour related to these new
arch features.

Search for the registers in the current mainline kernel.

$git grep SCTLR2_EL

arch/arm64/include/asm/sysreg.h:#define SYS_SCTLR2_EL2                  sys_reg(3, 4, 1, 0, 3)
arch/arm64/include/asm/sysreg.h:#define SYS_SCTLR2_EL12                 sys_reg(3, 5, 1, 0, 3)
arch/arm64/kvm/emulate-nested.c:        SR_TRAP(SYS_SCTLR2_EL2,         CGT_HCR_NV),

$git grep SCTLR2En
arch/arm64/kvm/nested.c:                res0 |= HCRX_EL2_SCTLR2En;
arch/arm64/tools/sysreg:Field   15      SCTLR2En

Although if we are looking for safer values, guess resetting these
two registers might be sufficient here ?

+       if (mrs_field(ID_AA64MMFR3_EL1, SCTLRX)) {
+               scr |= SCR_EL3_SCTLR2En;
+               msr(SCTLR2_EL2, 0);
+               msr(SCTLR2_EL1, 0);
+       }
+

> 
> I know that we've failed to do that for other things (FGT and HCRX), and
> those are latent bugs / mistakes in our appraoch that I'll see about
> fixing.

Sure.

Re: [boot-wrapper 1/3] aarch64: Enable access into 128 bit system registers from EL2 and below

[Anshuman Khandual]

On 7/25/24 14:14, Mark Rutland wrote:
> On Tue, Jul 23, 2024 at 04:36:28PM +0530, Anshuman Khandual wrote:
>> FEAT_D128 adds 128 bit system registers like PAR_EL1, TTBR0_EL1, TTBR1_EL1,
>> TTBR0_EL2, TTBR1_EL2, TTBR0_EL12, TTBR1_EL12 , VTTBR_EL2, RCWMASK_EL1, and
>> RCWSMASK_EL1. But access into these register from EL2 and below trap to EL3
>> unless SCR_EL3.D128En is set.
> 
> Do we need to initialize any of these new bits, or do they have no
> effect on a lower EL unless explicitly opted-in at that lower EL?

Unlike SCTRL2_EL1/EL2, these are actual end use registers for the lower
EL and only available in 128 bit width format when FEAT_D128 is both
implemented and opted in.

> 
> Otherwise this looks superficially fine, but it should come after
> enabling SCTLR2.

Sure, will moved it after SCTLR2 patch.

> 
> Mark.
> 
>>
>> Enable access to 128 bit registers when they are implemented.
>>
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>>  arch/aarch64/include/asm/cpu.h | 2 ++
>>  arch/aarch64/init.c            | 3 +++
>>  2 files changed, 5 insertions(+)
>>
>> diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h
>> index 124ef91..0b8b463 100644
>> --- a/arch/aarch64/include/asm/cpu.h
>> +++ b/arch/aarch64/include/asm/cpu.h
>> @@ -57,6 +57,7 @@
>>  #define SCR_EL3_EnTP2			BIT(41)
>>  #define SCR_EL3_TCR2EN			BIT(43)
>>  #define SCR_EL3_PIEN			BIT(45)
>> +#define SCR_EL3_D128En			BIT(47)
>>  
>>  #define HCR_EL2_RES1			BIT(1)
>>  
>> @@ -85,6 +86,7 @@
>>  #define ID_AA64MMFR3_EL1_S2PIE		BITS(15, 12)
>>  #define ID_AA64MMFR3_EL1_S1POE		BITS(19, 16)
>>  #define ID_AA64MMFR3_EL1_S2POE		BITS(23, 20)
>> +#define ID_AA64MMFR3_EL1_D128		BITS(35, 32)
>>  
>>  #define ID_AA64PFR1_EL1_MTE		BITS(11, 8)
>>  #define ID_AA64PFR1_EL1_SME		BITS(27, 24)
>> diff --git a/arch/aarch64/init.c b/arch/aarch64/init.c
>> index 37cb45f..7d9d0d9 100644
>> --- a/arch/aarch64/init.c
>> +++ b/arch/aarch64/init.c
>> @@ -89,6 +89,9 @@ void cpu_init_el3(void)
>>  	if (!kernel_is_32bit())
>>  		scr |= SCR_EL3_RW;
>>  
>> +	if (mrs_field(ID_AA64MMFR3_EL1, D128))
>> +		scr |= SCR_EL3_D128En;
>> +
>>  	msr(SCR_EL3, scr);
>>  
>>  	msr(CPTR_EL3, cptr);
>> -- 
>> 2.25.1
>>

Re: [boot-wrapper 3/3] aarch64: Enable access into RCW[S]MASK_EL1 registers from EL2 and below

[Anshuman Khandual]

On 7/25/24 14:21, Mark Rutland wrote:
> On Tue, Jul 23, 2024 at 04:36:30PM +0530, Anshuman Khandual wrote:
>> FEAT_THE adds RCW[S]MASK_EL1 system registers. But access into these system
>> registers from EL2 and below trap to EL3 unless SCR_EL3.RCWMASKEn is set.
>>
>> Enable access to RCW[S]MASK_EL1 registers when they are implemented.
> 
> This looks fine.
> 
> IIUC we don't need to initialize these new registers as they only affect
> the behaviour of new instructions which we don't expect SW to use until
> privileged SW has configured these registers (as they reset to UNKNOWN
> values even at the highest implemented EL).

Right, will keep patch's position in the series unchanged as well.

> 
> Mark.
> 
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>>  arch/aarch64/include/asm/cpu.h | 2 ++
>>  arch/aarch64/init.c            | 3 +++
>>  2 files changed, 5 insertions(+)
>>
>> diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h
>> index 57d66e4..8404152 100644
>> --- a/arch/aarch64/include/asm/cpu.h
>> +++ b/arch/aarch64/include/asm/cpu.h
>> @@ -55,6 +55,7 @@
>>  #define SCR_EL3_TME			BIT(34)
>>  #define SCR_EL3_HXEn			BIT(38)
>>  #define SCR_EL3_EnTP2			BIT(41)
>> +#define SCR_EL3_RCWMASKEn		BIT(42)
>>  #define SCR_EL3_TCR2EN			BIT(43)
>>  #define SCR_EL3_SCTLR2En		BIT(44)
>>  #define SCR_EL3_PIEN			BIT(45)
>> @@ -92,6 +93,7 @@
>>  
>>  #define ID_AA64PFR1_EL1_MTE		BITS(11, 8)
>>  #define ID_AA64PFR1_EL1_SME		BITS(27, 24)
>> +#define ID_AA64PFR1_EL1_THE		BITS(51, 48)
>>  #define ID_AA64PFR0_EL1_SVE		BITS(35, 32)
>>  
>>  #define ID_AA64SMFR0_EL1		s3_0_c0_c4_5
>> diff --git a/arch/aarch64/init.c b/arch/aarch64/init.c
>> index 5b21cb8..13a2339 100644
>> --- a/arch/aarch64/init.c
>> +++ b/arch/aarch64/init.c
>> @@ -95,6 +95,9 @@ void cpu_init_el3(void)
>>  	if (mrs_field(ID_AA64MMFR3_EL1, SCTLRX))
>>  		scr |= SCR_EL3_SCTLR2En;
>>  
>> +	if (mrs_field(ID_AA64PFR1_EL1, THE))
>> +		scr |= SCR_EL3_RCWMASKEn;
>> +
>>  	msr(SCR_EL3, scr);
>>  
>>  	msr(CPTR_EL3, cptr);
>> -- 
>> 2.25.1
>>

Re: [boot-wrapper 2/3] aarch64: Enable access into SCTLR2_ELx registers from EL2 and below

[Mark Rutland]

On Fri, Jul 26, 2024 at 12:25:14PM +0530, Anshuman Khandual wrote:
> On 7/25/24 14:10, Mark Rutland wrote:
> > On Tue, Jul 23, 2024 at 04:36:29PM +0530, Anshuman Khandual wrote:
> >> diff --git a/arch/aarch64/init.c b/arch/aarch64/init.c
> >> index 7d9d0d9..5b21cb8 100644
> >> --- a/arch/aarch64/init.c
> >> +++ b/arch/aarch64/init.c
> >> @@ -92,6 +92,9 @@ void cpu_init_el3(void)
> >>  	if (mrs_field(ID_AA64MMFR3_EL1, D128))
> >>  		scr |= SCR_EL3_D128En;
> >>  
> >> +	if (mrs_field(ID_AA64MMFR3_EL1, SCTLRX))
> >> +		scr |= SCR_EL3_SCTLR2En;
> >> +
> > 
> > The SCTLR2_ELx registers reset to UNKNOWN values when the highest
> > implemented exception level is not ELx, so we need to initialize those
> > to safe values. Otherwise a kernel which is not aware of SCTLR2_ELx will
> > be subject to arbitrary behaviour as a result of the SCTLR2_ELx bits
> > which it will not have configured.
> 
> Both SCTLR2_EL1 and SCTLR2_EL2 has the same register fields layout
> except the very last bit i.e SCTLR2_EL2.EMEC which is available in
> SCTLR2_EL2 but not in SCTLR2_EL1.
> 
> AFAICT all the above register fields are applicable for newer arch
> features which the current kernel is not even aware about. So even
> if the kernel is not ware about SCTLR2_EL2 or SCTLR2_EL1 registers,
> there will not be any difference in behaviour related to these new
> arch features.

There several are changes to existing behaviours. Looking at ARM DDI
0487K.a:

* EASE changes the way external aborts are routed, which could surprise
  the exception handling code.

* NMEA causes SError to be taken regardless of PSTATE.A. This *will*
  break exception handling.

... and regardless we have no idea how any of the RES0 bits will be used
in future.

Looking at DDI 0601 ID070124 from:

  https://developer.arm.com/documentation/ddi0601/2024-06/?lang=en

... there are other bits that would be problematic too. Consider how
EnPACM0 works with a kernel that is not PACM-aware but a userspace that
is, especially if CPUs have mismatched reset values.

> Search for the registers in the current mainline kernel.
> 
> $git grep SCTLR2_EL
> 
> arch/arm64/include/asm/sysreg.h:#define SYS_SCTLR2_EL2                  sys_reg(3, 4, 1, 0, 3)
> arch/arm64/include/asm/sysreg.h:#define SYS_SCTLR2_EL12                 sys_reg(3, 5, 1, 0, 3)
> arch/arm64/kvm/emulate-nested.c:        SR_TRAP(SYS_SCTLR2_EL2,         CGT_HCR_NV),
> 
> $git grep SCTLR2En
> arch/arm64/kvm/nested.c:                res0 |= HCRX_EL2_SCTLR2En;
> arch/arm64/tools/sysreg:Field   15      SCTLR2En
> 
> Although if we are looking for safer values, guess resetting these
> two registers might be sufficient here ?
> 
> +       if (mrs_field(ID_AA64MMFR3_EL1, SCTLRX)) {
> +               scr |= SCR_EL3_SCTLR2En;
> +               msr(SCTLR2_EL2, 0);
> +               msr(SCTLR2_EL1, 0);
> +       }

Using zero for both looks fine to me.

Mark.