| Message ID | 20241126153955.477569-4-mark.rutland@arm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for FEAT_FPMR and FEAT_GCS | expand |
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.
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.
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.
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;
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(+)