| Message ID | 20230310173657.57228-1-joey.gouly@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1,1/2] aarch64: enable access to TCR2_ELx | expand |
On Fri, Mar 10, 2023 at 05:36:56PM +0000, Joey Gouly wrote: > Allow access the TCR2_ELx register which provides extended translation > controls similar to TCR_ELx. AFAICT TCR2_ELx reset to UNKNOWN values, and existing EL{2,1} software won't know that TCR2_ELx exists. So either: (a) The bootwrapper needs to initialize TCR2_ELx to some sane value to avoid subsequent problems. (b) The TCR2_ELx bits are gated by some existing RESx bit that EL{2,1} will configure (e.g. only apply after setting some existing TCR_ELx or SCTLR_Elx bit). I suspect that we fall into case (a), but I don't see any initialization of TCR2_ELx. Do we need to add initiailization, or does case (b) apply? If it's the latter, could we please add something to the commit message describing that? Other than that, this looks fine to me. Thanks, Mark. > Signed-off-by: Joey Gouly <joey.gouly@arm.com> > --- > arch/aarch64/include/asm/cpu.h | 5 +++++ > arch/aarch64/init.c | 3 +++ > 2 files changed, 8 insertions(+) > > diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h > index 89a8f78..3686e08 100644 > --- a/arch/aarch64/include/asm/cpu.h > +++ b/arch/aarch64/include/asm/cpu.h > @@ -51,6 +51,7 @@ > #define SCR_EL3_TME BIT(34) > #define SCR_EL3_HXEn BIT(38) > #define SCR_EL3_EnTP2 BIT(41) > +#define SCR_EL3_TCR2EN BIT(43) > > #define HCR_EL2_RES1 BIT(1) > > @@ -73,6 +74,8 @@ > > #define ID_AA64MMFR1_EL1_HCX BITS(43, 40) > > +#define ID_AA64MMFR3_EL1_TCRX BITS(4, 0) > + > #define ID_AA64PFR1_EL1_MTE BITS(11, 8) > #define ID_AA64PFR1_EL1_SME BITS(27, 24) > #define ID_AA64PFR0_EL1_SVE BITS(35, 32) > @@ -122,6 +125,8 @@ > > #define ID_AA64ISAR2_EL1 s3_0_c0_c6_2 > > +#define ID_AA64MMFR3_EL1 s3_0_c0_c7_3 > + > #define SCTLR_EL1_CP15BEN (1 << 5) > > #ifdef KERNEL_32 > diff --git a/arch/aarch64/init.c b/arch/aarch64/init.c > index 471e234..f004e41 100644 > --- a/arch/aarch64/init.c > +++ b/arch/aarch64/init.c > @@ -64,6 +64,9 @@ void cpu_init_el3(void) > if (mrs_field(ID_AA64MMFR1_EL1, HCX)) > scr |= SCR_EL3_HXEn; > > + if (mrs_field(ID_AA64MMFR3_EL1, TCRX)) > + scr |= SCR_EL3_TCR2EN; > + > if (mrs_field(ID_AA64PFR1_EL1, MTE) >= 2) > scr |= SCR_EL3_ATA; > > -- > 2.17.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
Hi, On Thu, Jun 15, 2023 at 04:21:06PM +0100, Mark Rutland wrote: > On Fri, Mar 10, 2023 at 05:36:56PM +0000, Joey Gouly wrote: > > Allow access the TCR2_ELx register which provides extended translation > > controls similar to TCR_ELx. > > AFAICT TCR2_ELx reset to UNKNOWN values, and existing EL{2,1} software won't > know that TCR2_ELx exists. So either: > > (a) The bootwrapper needs to initialize TCR2_ELx to some sane value to avoid > subsequent problems. > > (b) The TCR2_ELx bits are gated by some existing RESx bit that EL{2,1} will > configure (e.g. only apply after setting some existing TCR_ELx or SCTLR_Elx > bit). > > I suspect that we fall into case (a), but I don't see any initialization of > TCR2_ELx. You're right, I will intialise it (to 0, seems like the right thing to do looking at the definition) and send out a new series (along with the other fix). > > Do we need to add initiailization, or does case (b) apply? If it's the latter, > could we please add something to the commit message describing that? Looks like HCRX_EL2 is also missing initialisation. > > Other than that, this looks fine to me. Thanks, Joey
diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h index 89a8f78..3686e08 100644 --- a/arch/aarch64/include/asm/cpu.h +++ b/arch/aarch64/include/asm/cpu.h @@ -51,6 +51,7 @@ #define SCR_EL3_TME BIT(34) #define SCR_EL3_HXEn BIT(38) #define SCR_EL3_EnTP2 BIT(41) +#define SCR_EL3_TCR2EN BIT(43) #define HCR_EL2_RES1 BIT(1) @@ -73,6 +74,8 @@ #define ID_AA64MMFR1_EL1_HCX BITS(43, 40) +#define ID_AA64MMFR3_EL1_TCRX BITS(4, 0) + #define ID_AA64PFR1_EL1_MTE BITS(11, 8) #define ID_AA64PFR1_EL1_SME BITS(27, 24) #define ID_AA64PFR0_EL1_SVE BITS(35, 32) @@ -122,6 +125,8 @@ #define ID_AA64ISAR2_EL1 s3_0_c0_c6_2 +#define ID_AA64MMFR3_EL1 s3_0_c0_c7_3 + #define SCTLR_EL1_CP15BEN (1 << 5) #ifdef KERNEL_32 diff --git a/arch/aarch64/init.c b/arch/aarch64/init.c index 471e234..f004e41 100644 --- a/arch/aarch64/init.c +++ b/arch/aarch64/init.c @@ -64,6 +64,9 @@ void cpu_init_el3(void) if (mrs_field(ID_AA64MMFR1_EL1, HCX)) scr |= SCR_EL3_HXEn; + if (mrs_field(ID_AA64MMFR3_EL1, TCRX)) + scr |= SCR_EL3_TCR2EN; + if (mrs_field(ID_AA64PFR1_EL1, MTE) >= 2) scr |= SCR_EL3_ATA;
Allow access the TCR2_ELx register which provides extended translation controls similar to TCR_ELx. Signed-off-by: Joey Gouly <joey.gouly@arm.com> --- arch/aarch64/include/asm/cpu.h | 5 +++++ arch/aarch64/init.c | 3 +++ 2 files changed, 8 insertions(+)