[BOOT-WRAPPER PATCH 0/3] Add support for FEAT_FPMR and FEAT_GCS

[BOOT-WRAPPER PATCH 0/3] Add support for FEAT_FPMR and FEAT_GCS

[Mark Rutland]

While trying to test a tip-of-tree kernel on a model, I discovered that
we never added boot-wrapper support for FEAT_FPMR of FEAT_GCS, resulting
in the kernel (silently) hanging early during boot, which isn't ideal.

These patches add the missing support.

Mark.

Mark Rutland (3):
  aarch64: shuffle ID_AA64PFR{0,1}_EL1 definitions
  aarch64: Enable use of FPMR for EL2 and below
  aarch64: Enable use of GCS for EL2 and below

 arch/aarch64/include/asm/cpu.h | 16 +++++++++++++---
 arch/aarch64/init.c            | 10 ++++++++++
 2 files changed, 23 insertions(+), 3 deletions(-)

-- 
2.30.2

[BOOT-WRAPPER PATCH 1/3] aarch64: shuffle ID_AA64PFR{0,1}_EL1 definitions

[Mark Rutland]

Usually the ID register definitions are sorted alphanumerically, but for
historical reasons the ID_AA64PFR0_* definitions are placed before the
ID_AA64PFR1_* definitions. Reorder these for consistency.

There should be no functional change as a result of this patch.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 arch/aarch64/include/asm/cpu.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h
index 0a6baa8..6fa11da 100644
--- a/arch/aarch64/include/asm/cpu.h
+++ b/arch/aarch64/include/asm/cpu.h
@@ -107,15 +107,15 @@
 #define ID_AA64MMFR3_EL1_S2POE		BITS(23, 20)
 #define ID_AA64MMFR3_EL1_D128		BITS(35, 32)
 
+#define ID_AA64PFR0_EL1_RAS		BITS(31, 28)
+#define ID_AA64PFR0_EL1_SVE		BITS(35, 32)
+#define ID_AA64PFR0_EL1_CSV2		BITS(59, 56)
+
 #define ID_AA64PFR1_EL1_MTE		BITS(11, 8)
 #define ID_AA64PFR1_EL1_SME		BITS(27, 24)
 #define ID_AA64PFR1_EL1_CSV2_frac	BITS(35, 32)
 #define ID_AA64PFR1_EL1_THE		BITS(51, 48)
 
-#define ID_AA64PFR0_EL1_RAS		BITS(31, 28)
-#define ID_AA64PFR0_EL1_SVE		BITS(35, 32)
-#define ID_AA64PFR0_EL1_CSV2		BITS(59, 56)
-
 #define ID_AA64SMFR0_EL1		s3_0_c0_c4_5
 #define ID_AA64SMFR0_EL1_FA64		BIT(63)
 
-- 
2.30.2

[BOOT-WRAPPER PATCH 2/3] aarch64: Enable use of FPMR for EL2 and below

[Mark Rutland]

FEAT_FPMR adds the FPMR register. Acceses to FPMR (whether direct or
indirect) trap to EL3 unless SCR_EL3.EnFPM is set, and so boot-wrapper
support is necessary.

Support for FEAT_FPMR was added to Linux in v6.8 without any
boot-wrapper support. Consequently when FPMR is enabled in a model, the
kernel will hang when attempting to write to the FPMR (e.g. when
entering userspace for the first time).

Add boot-wrapper support for FEAT_FPMR. As FPMR is not described in the
latest ARM ARM (ARM DDI 0487K.a), the relevant definitions are taken
from the 2024-09 release of the "Arm Architecture Registers" document,
ARM DDI 0601 (ID092424), which can be found at:

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

The ID_AA64PFR2_EL1 ID register has existed as reserved RES0 space since
ARMv8.0 but only recently gained a name, and so older assemblers may not
be able to encode ID_AA64PFR2_EL1 directly. Thus we need an explicit
definition of the sysreg encoding to support these assemblers.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Mark Brown <broonie@kernel.org>
---
 arch/aarch64/include/asm/cpu.h | 4 ++++
 arch/aarch64/init.c            | 3 +++
 2 files changed, 7 insertions(+)

diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h
index 6fa11da..3ef58f3 100644
--- a/arch/aarch64/include/asm/cpu.h
+++ b/arch/aarch64/include/asm/cpu.h
@@ -68,6 +68,7 @@
 #define SCR_EL3_SCTLR2En		BIT(44)
 #define SCR_EL3_PIEN			BIT(45)
 #define SCR_EL3_D128En			BIT(47)
+#define SCR_EL3_EnFPM			BIT(50)
 
 #define VTCR_EL2_MSA			BIT(31)
 
@@ -116,6 +117,9 @@
 #define ID_AA64PFR1_EL1_CSV2_frac	BITS(35, 32)
 #define ID_AA64PFR1_EL1_THE		BITS(51, 48)
 
+#define ID_AA64PFR2_EL1			s3_0_c0_c4_2
+#define ID_AA64PFR2_EL1_FPMR		BITS(35, 32)
+
 #define ID_AA64SMFR0_EL1		s3_0_c0_c4_5
 #define ID_AA64SMFR0_EL1_FA64		BIT(63)
 
diff --git a/arch/aarch64/init.c b/arch/aarch64/init.c
index da036bb..1f38516 100644
--- a/arch/aarch64/init.c
+++ b/arch/aarch64/init.c
@@ -124,6 +124,9 @@ static void cpu_init_el3(void)
 	if (mrs_field(ID_AA64PFR1_EL1, THE))
 		scr |= SCR_EL3_RCWMASKEn;
 
+	if (mrs_field(ID_AA64PFR2_EL1, FPMR))
+		scr |= SCR_EL3_EnFPM;
+
 	msr(SCR_EL3, scr);
 
 	msr(CPTR_EL3, cptr);
-- 
2.30.2

Re: [BOOT-WRAPPER PATCH 2/3] aarch64: Enable use of FPMR for EL2 and below

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 276 bytes --]

On Tue, Nov 26, 2024 at 03:39:54PM +0000, Mark Rutland wrote:
> FEAT_FPMR adds the FPMR register. Acceses to FPMR (whether direct or
> indirect) trap to EL3 unless SCR_EL3.EnFPM is set, and so boot-wrapper
> support is necessary.

Reviewed-by: Mark Brown <broonie@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[BOOT-WRAPPER PATCH 3/3] aarch64: Enable use of GCS for EL2 and below

[Mark Rutland]

FEAT_GCS adds a number of new system registers and instructions. Usage
of some of these can trap to EL3 unless SCR_EL3.GCSEn is set, and so
boot-wrapper support is necessary.

Support for FEAT_GCS was added to Linux in the v6.13-rc1 merge window
without any boot-wrapper support. Consequently when GCS is enabled in a
model, the kernel will hang when attempting to write to GCS control
registers, which happens early in boot when the kernel configures EL2,
before any console output is produced.

FEAT_GCS is described in the latest ARM ARM (ARM DDI 0487K.a), which can
be found at:

  https://developer.arm.com/documentation/ddi0487/ka/?lang=en

Add boot-wrapper support for FEAT_GCS. In addition to setting
SCR_EL3.GCSEn, it's necessary to initialize GCSCR_EL2, GCSCR_EL1, and
GCSCRE0_EL1 such that older kernel which are not aware of GCS don't find
GCS enabled unexpectedly.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Mark Brown <broonie@kernel.org>
---
 arch/aarch64/include/asm/cpu.h | 6 ++++++
 arch/aarch64/init.c            | 7 +++++++
 2 files changed, 13 insertions(+)

diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h
index 3ef58f3..4cf0ff7 100644
--- a/arch/aarch64/include/asm/cpu.h
+++ b/arch/aarch64/include/asm/cpu.h
@@ -62,6 +62,7 @@
 #define SCR_EL3_ECVEN			BIT(28)
 #define SCR_EL3_TME			BIT(34)
 #define SCR_EL3_HXEn			BIT(38)
+#define SCR_EL3_GCSEn			BIT(39)
 #define SCR_EL3_EnTP2			BIT(41)
 #define SCR_EL3_RCWMASKEn		BIT(42)
 #define SCR_EL3_TCR2EN			BIT(43)
@@ -115,6 +116,7 @@
 #define ID_AA64PFR1_EL1_MTE		BITS(11, 8)
 #define ID_AA64PFR1_EL1_SME		BITS(27, 24)
 #define ID_AA64PFR1_EL1_CSV2_frac	BITS(35, 32)
+#define ID_AA64PFR1_EL1_GCS		BITS(47, 44)
 #define ID_AA64PFR1_EL1_THE		BITS(51, 48)
 
 #define ID_AA64PFR2_EL1			s3_0_c0_c4_2
@@ -169,6 +171,10 @@
 #define SMCR_EL3_FA64		BIT(31)
 #define SMCR_EL3_LEN_MAX	0xf
 
+#define GCSCRE0_EL1		s3_0_c2_c5_2
+#define GCSCR_EL1		s3_0_c2_c5_0
+#define GCSCR_EL2		s3_4_c2_c5_0
+
 #define ID_AA64ISAR2_EL1	s3_0_c0_c6_2
 
 #define ID_AA64MMFR3_EL1	s3_0_c0_c7_3
diff --git a/arch/aarch64/init.c b/arch/aarch64/init.c
index 1f38516..61b55f9 100644
--- a/arch/aarch64/init.c
+++ b/arch/aarch64/init.c
@@ -124,6 +124,13 @@ static void cpu_init_el3(void)
 	if (mrs_field(ID_AA64PFR1_EL1, THE))
 		scr |= SCR_EL3_RCWMASKEn;
 
+	if (mrs_field(ID_AA64PFR1_EL1, GCS)) {
+		scr |= SCR_EL3_GCSEn;
+		msr(GCSCR_EL2, 0);
+		msr(GCSCR_EL1, 0);
+		msr(GCSCRE0_EL1, 0);
+	}
+
 	if (mrs_field(ID_AA64PFR2_EL1, FPMR))
 		scr |= SCR_EL3_EnFPM;
 
-- 
2.30.2

Re: [BOOT-WRAPPER PATCH 3/3] aarch64: Enable use of GCS for EL2 and below

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 673 bytes --]

On Tue, Nov 26, 2024 at 03:39:55PM +0000, Mark Rutland wrote:

> +	if (mrs_field(ID_AA64PFR1_EL1, GCS)) {
> +		scr |= SCR_EL3_GCSEn;
> +		msr(GCSCR_EL2, 0);
> +		msr(GCSCR_EL1, 0);
> +		msr(GCSCRE0_EL1, 0);
> +	}
> +

We're not doing anything for GCSCR_EL3 where (as for the other ELs)
RVCHKEN resets to an architecturally UNKNOWN value:  

   https://developer.arm.com/documentation/ddi0601/2024-09/AArch64-Registers/GCSCR-EL3--Guarded-Control-Stack-Control-Register--EL3-?lang=en

This is also an oversight in the TF-A GCS support.  One might question
the decisions of an implementation which doesn't reset it to 0 but it
should be safer to initialise.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [BOOT-WRAPPER PATCH 3/3] aarch64: Enable use of GCS for EL2 and below

[Mark Rutland]

On Tue, Nov 26, 2024 at 05:20:47PM +0000, Mark Brown wrote:
> On Tue, Nov 26, 2024 at 03:39:55PM +0000, Mark Rutland wrote:
> 
> > +	if (mrs_field(ID_AA64PFR1_EL1, GCS)) {
> > +		scr |= SCR_EL3_GCSEn;
> > +		msr(GCSCR_EL2, 0);
> > +		msr(GCSCR_EL1, 0);
> > +		msr(GCSCRE0_EL1, 0);
> > +	}
> > +
> 
> We're not doing anything for GCSCR_EL3 where (as for the other ELs)
> RVCHKEN resets to an architecturally UNKNOWN value:  
> 
>    https://developer.arm.com/documentation/ddi0601/2024-09/AArch64-Registers/GCSCR-EL3--Guarded-Control-Stack-Control-Register--EL3-?lang=en
> 
> This is also an oversight in the TF-A GCS support.  One might question
> the decisions of an implementation which doesn't reset it to 0 but it
> should be safer to initialise.

A need to initialize GCSCR_EL3.RVCHKEN would be an *architecture bug*,
not an oversight in TF-A. That bit should only reset to an UNKNOWN value
if it doesn't have a functional effect on EL3 code that doesn't touch
GCS functionality. If it has no functional effect it's fine to leave it
in an UNKNOWN state.

GCSCR_EL3.PCRSEL resets to 0, so the HW won't push to the GCS for BL/BLR
and won't pop from the GCS for a RET. That menas there's no GCS value
for a return value check to compare with...

That does raise the question of what specifically happens for a return
at EL3 when GCSCR_EL3.{PCRSEL,RVCHKEN} == {1,0}. Can you enlighten me?

Mark.

Re: [BOOT-WRAPPER PATCH 3/3] aarch64: Enable use of GCS for EL2 and below

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1179 bytes --]

On Tue, Nov 26, 2024 at 06:01:56PM +0000, Mark Rutland wrote:

> GCSCR_EL3.PCRSEL resets to 0, so the HW won't push to the GCS for BL/BLR
> and won't pop from the GCS for a RET. That menas there's no GCS value
> for a return value check to compare with...

Oh, good point - I'd trusted that the initialisations were required and
misremembered which of PCRSEL and RVCHKEN was which.  Sorry about that.
The values on reset follow the same pattern in all the GCS control
registers down to GCSCRE0_EL1 so the initialisations of the other
control registers are equally redundant.  We should be consistent here,
either initialising all the GCS control registers or relying on the
architecture defaults for all of them, and the note in the changelog
about them needs an update if the initialisation is there.

> That does raise the question of what specifically happens for a return
> at EL3 when GCSCR_EL3.{PCRSEL,RVCHKEN} == {1,0}. Can you enlighten me?

RET will attempt to load and use a GCS record, the pseudocode is in
LoadCheckGCSRecord() which isn't EL dependent other than the selection
of which GCSPR and GCSCR to use and setting the access as privileged if
we're not at EL0.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [BOOT-WRAPPER PATCH 3/3] aarch64: Enable use of GCS for EL2 and below

[Mark Rutland]

On Tue, Nov 26, 2024 at 06:53:03PM +0000, Mark Brown wrote:
> On Tue, Nov 26, 2024 at 06:01:56PM +0000, Mark Rutland wrote:
> 
> > GCSCR_EL3.PCRSEL resets to 0, so the HW won't push to the GCS for BL/BLR
> > and won't pop from the GCS for a RET. That menas there's no GCS value
> > for a return value check to compare with...
> 
> Oh, good point - I'd trusted that the initialisations were required and
> misremembered which of PCRSEL and RVCHKEN was which.  Sorry about that.
> The values on reset follow the same pattern in all the GCS control
> registers down to GCSCRE0_EL1 so the initialisations of the other
> control registers are equally redundant.  We should be consistent here,
> either initialising all the GCS control registers or relying on the
> architecture defaults for all of them, and the note in the changelog
> about them needs an update if the initialisation is there.

Hmm... that's somewhat unusual, usually the architecture has a "minimal
reset policy", where _ELx register fields are only reset when ELx is the
highest EL after the reset, leaving it to SW to initialise lower ELs.
It's not clear to me if not following that here was deliberate or an
oversight.

I can drop those for now. In future the boot-wrapper will probably need
to poke those (and many other register fields) to properly handle PSCI
CPU_OFF -> CPU_ON sequences, as we currently don't reset anything, and
that should really involve a true reset.

> > That does raise the question of what specifically happens for a return
> > at EL3 when GCSCR_EL3.{PCRSEL,RVCHKEN} == {1,0}. Can you enlighten me?
> 
> RET will attempt to load and use a GCS record, the pseudocode is in
> LoadCheckGCSRecord() which isn't EL dependent other than the selection
> of which GCSPR and GCSCR to use and setting the access as privileged if
> we're not at EL0.

Sorry, I got the bits backwards, and had meant the case where:

	GCSCR_EL3.{PCRSEL,RVCHKEN} == {0,1}

... where I assume the RVCHKEN bit has no effect given we don't have a
GCS return value to compare against, but wanted to confirm that there
wasn't an architectural edge-case that we might need to feed back to
architects.

It *seems* like RVCHKEN has no effect for RET when PCRSEL is clear.
In ARM DDI 0487K.a, C6.2.291, the pseudocode for RET starts with:

| if (IsFeatureImplemented(FEAT_GCS) && GCSPCREnabled(PSTATE.EL)) then
|     target = LoadCheckGCSRecord(target, GCSInstType_PRET);
|     SetCurrentGCSPointer(GetCurrentGCSPointer() + 8);

... and RVCHKEN is consumed under LoadCheckGCSRecord(), which has:

| if GCSReturnValueCheckEnabled(PSTATE.EL) && (recorded_va != vaddress) then
|     GCSDataCheckException(gcsinst_type);

... where GCSReturnValueCheckEnabled() is:

| boolean GCSReturnValueCheckEnabled(bits(2) el)
|     if UsingAArch32() then
|         return FALSE;
|     case el of
|         when EL0 return GCSCRE0_EL1.RVCHKEN == '1';
|         when EL1 return GCSCR_EL1.RVCHKEN == '1';
|         when EL2 return GCSCR_EL2.RVCHKEN == '1';
|         when EL3 return GCSCR_EL3.RVCHKEN == '1';

AFAICT GCSReturnValueCheckEnabled() is only used by
LoadCheckGCSRecord(), and that's only used by the pseudocode for
RET/RETAA/RETAB, so it looks like we're good.

Mark.

Re: [BOOT-WRAPPER PATCH 3/3] aarch64: Enable use of GCS for EL2 and below

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 2244 bytes --]

On Wed, Nov 27, 2024 at 10:25:54AM +0000, Mark Rutland wrote:
> On Tue, Nov 26, 2024 at 06:53:03PM +0000, Mark Brown wrote:

> > The values on reset follow the same pattern in all the GCS control
> > registers down to GCSCRE0_EL1 so the initialisations of the other
> > control registers are equally redundant.  We should be consistent here,
> > either initialising all the GCS control registers or relying on the
> > architecture defaults for all of them, and the note in the changelog
> > about them needs an update if the initialisation is there.

> Hmm... that's somewhat unusual, usually the architecture has a "minimal
> reset policy", where _ELx register fields are only reset when ELx is the
> highest EL after the reset, leaving it to SW to initialise lower ELs.
> It's not clear to me if not following that here was deliberate or an
> oversight.

Yeah, I remember being a bit surprised by the choice.

> > > That does raise the question of what specifically happens for a return
> > > at EL3 when GCSCR_EL3.{PCRSEL,RVCHKEN} == {1,0}. Can you enlighten me?

> > RET will attempt to load and use a GCS record, the pseudocode is in
> > LoadCheckGCSRecord() which isn't EL dependent other than the selection
> > of which GCSPR and GCSCR to use and setting the access as privileged if
> > we're not at EL0.

> Sorry, I got the bits backwards, and had meant the case where:

> 	GCSCR_EL3.{PCRSEL,RVCHKEN} == {0,1}

> ... where I assume the RVCHKEN bit has no effect given we don't have a
> GCS return value to compare against, but wanted to confirm that there
> wasn't an architectural edge-case that we might need to feed back to
> architects.

Oh, good - I was wondering why you weren't asking that rather than what
you asked.

> It *seems* like RVCHKEN has no effect for RET when PCRSEL is clear.
> In ARM DDI 0487K.a, C6.2.291, the pseudocode for RET starts with:

...

> AFAICT GCSReturnValueCheckEnabled() is only used by
> LoadCheckGCSRecord(), and that's only used by the pseudocode for
> RET/RETAA/RETAB, so it looks like we're good.

Yes, we only do a return value check if PCRSEL is active - it is only
meaningful as part of a GCS load since it's a comparison of the value we
got from the GCS with the value passed into the ret.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]