| Message ID | 20230309145246.22787-16-joey.gouly@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Permission Indirection Extension | expand |
On Thu, Mar 09, 2023 at 02:52:43PM +0000, Joey Gouly wrote: > Disable trapping of TCR2_EL1 and PIRx_EL1 registers, so they can be > accessed from by EL1. There should be an update to booting.rst documenting what's needed here, and also what we need EL3 to do for use at EL2. Reviewed-by: Mark Brown <broonie@kernel.org>
On 09/03/2023 14:52, Joey Gouly wrote: > Disable trapping of TCR2_EL1 and PIRx_EL1 registers, so they can be > accessed from by EL1. > > Signed-off-by: Joey Gouly <joey.gouly@arm.com> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Will Deacon <will@kernel.org> > --- > arch/arm64/include/asm/el2_setup.h | 27 ++++++++++++++++++++++++++- > 1 file changed, 26 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h > index 037724b19c5c..6e6675fae194 100644 > --- a/arch/arm64/include/asm/el2_setup.h > +++ b/arch/arm64/include/asm/el2_setup.h > @@ -22,6 +22,21 @@ > isb > .endm > > +.macro __init_el2_hcrx > + mrs x1, id_aa64mmfr1_el1 > + ubfx x0, x1, #ID_AA64MMFR1_EL1_HCX_SHIFT, 4 > + cbz x0, .Lskip_hcrx_\@ If TCR2 is available, HCX must also be supported ? > + > + mrs_s x1, SYS_ID_AA64MMFR2_EL1 MMFR3 ? > + ubfx x0, x1, #ID_AA64MMFR3_EL1_TCRX_SHIFT, 4 > + cbz x0, .Lskip_hcrx_\@ Suzuki > + > + mrs_s x0, SYS_HCRX_EL2 > + orr x0, x0, #HCRX_EL2_TCR2En > + msr_s SYS_HCRX_EL2, x0 > +.Lskip_hcrx_\@: > +.endm > + > /* > * Allow Non-secure EL1 and EL0 to access physical timer and counter. > * This is not necessary for VHE, since the host kernel runs in EL2, > @@ -150,12 +165,21 @@ > mov x0, xzr > mrs x1, id_aa64pfr1_el1 > ubfx x1, x1, #ID_AA64PFR1_EL1_SME_SHIFT, #4 > - cbz x1, .Lset_fgt_\@ > + cbz x1, .Lset_pie_fgt_\@ > > /* Disable nVHE traps of TPIDR2 and SMPRI */ > orr x0, x0, #HFGxTR_EL2_nSMPRI_EL1_MASK > orr x0, x0, #HFGxTR_EL2_nTPIDR2_EL0_MASK > > +.Lset_pie_fgt_\@: > + mrs_s x1, SYS_ID_AA64MMFR3_EL1 > + ubfx x1, x1, #ID_AA64MMFR3_EL1_S1PIE_SHIFT, #4 > + cbz x1, .Lset_fgt_\@ > + > + /* Disable trapping of PIR_EL1 / PIRE0_EL1 */ > + orr x0, x0, #HFGxTR_EL2_nPIR_EL0_MASK > + orr x0, x0, #HFGxTR_EL2_nPIREO_EL0_MASK > + > .Lset_fgt_\@: > msr_s SYS_HFGRTR_EL2, x0 > msr_s SYS_HFGWTR_EL2, x0 > @@ -184,6 +208,7 @@ > */ > .macro init_el2_state > __init_el2_sctlr > + __init_el2_hcrx > __init_el2_timers > __init_el2_debug > __init_el2_lor
On Thu, Mar 09, 2023 at 04:27:39PM +0000, Suzuki K Poulose wrote: > On 09/03/2023 14:52, Joey Gouly wrote: > > Disable trapping of TCR2_EL1 and PIRx_EL1 registers, so they can be > > accessed from by EL1. > > +.macro __init_el2_hcrx > > + mrs x1, id_aa64mmfr1_el1 > > + ubfx x0, x1, #ID_AA64MMFR1_EL1_HCX_SHIFT, 4 > > + cbz x0, .Lskip_hcrx_\@ > If TCR2 is available, HCX must also be supported ? OTOH given that an error here means that we'll die with no output and we *have* had issues with virtual implementations not implementing all the features they were supposed to (plus I guess some will allow user error in configuration) a bit of defensiveness doesn't hurt.
On Thu, Mar 09, 2023 at 02:52:43PM +0000, Joey Gouly wrote: > Disable trapping of TCR2_EL1 and PIRx_EL1 registers, so they can be > accessed from by EL1. > > Signed-off-by: Joey Gouly <joey.gouly@arm.com> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Will Deacon <will@kernel.org> > --- > arch/arm64/include/asm/el2_setup.h | 27 ++++++++++++++++++++++++++- > 1 file changed, 26 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h > index 037724b19c5c..6e6675fae194 100644 > --- a/arch/arm64/include/asm/el2_setup.h > +++ b/arch/arm64/include/asm/el2_setup.h > @@ -22,6 +22,21 @@ > isb > .endm > > +.macro __init_el2_hcrx > + mrs x1, id_aa64mmfr1_el1 > + ubfx x0, x1, #ID_AA64MMFR1_EL1_HCX_SHIFT, 4 > + cbz x0, .Lskip_hcrx_\@ > + > + mrs_s x1, SYS_ID_AA64MMFR2_EL1 > + ubfx x0, x1, #ID_AA64MMFR3_EL1_TCRX_SHIFT, 4 > + cbz x0, .Lskip_hcrx_\@ > + > + mrs_s x0, SYS_HCRX_EL2 > + orr x0, x0, #HCRX_EL2_TCR2En > + msr_s SYS_HCRX_EL2, x0 > +.Lskip_hcrx_\@: > +.endm Can we not just set HCRX_EL2_TCR2En without probing for TCRX? BTW, Kristina's doing some initialisation of HCRX_EL2 as well here, not sure whether they conflict: https://lore.kernel.org/all/20230216160012.272345-2-kristina.martsenko@arm.com/
Hi Catalin, On Mon, Mar 27, 2023 at 05:59:23PM +0100, Catalin Marinas wrote: > On Thu, Mar 09, 2023 at 02:52:43PM +0000, Joey Gouly wrote: > > Disable trapping of TCR2_EL1 and PIRx_EL1 registers, so they can be > > accessed from by EL1. > > > > Signed-off-by: Joey Gouly <joey.gouly@arm.com> > > Cc: Catalin Marinas <catalin.marinas@arm.com> > > Cc: Will Deacon <will@kernel.org> > > --- > > arch/arm64/include/asm/el2_setup.h | 27 ++++++++++++++++++++++++++- > > 1 file changed, 26 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h > > index 037724b19c5c..6e6675fae194 100644 > > --- a/arch/arm64/include/asm/el2_setup.h > > +++ b/arch/arm64/include/asm/el2_setup.h > > @@ -22,6 +22,21 @@ > > isb > > .endm > > > > +.macro __init_el2_hcrx > > + mrs x1, id_aa64mmfr1_el1 > > + ubfx x0, x1, #ID_AA64MMFR1_EL1_HCX_SHIFT, 4 > > + cbz x0, .Lskip_hcrx_\@ > > + > > + mrs_s x1, SYS_ID_AA64MMFR2_EL1 > > + ubfx x0, x1, #ID_AA64MMFR3_EL1_TCRX_SHIFT, 4 > > + cbz x0, .Lskip_hcrx_\@ > > + > > + mrs_s x0, SYS_HCRX_EL2 > > + orr x0, x0, #HCRX_EL2_TCR2En > > + msr_s SYS_HCRX_EL2, x0 > > +.Lskip_hcrx_\@: > > +.endm > > Can we not just set HCRX_EL2_TCR2En without probing for TCRX? It's RES0, so I avoided it, but if it's fine I can drop the check. > > BTW, Kristina's doing some initialisation of HCRX_EL2 as well here, not > sure whether they conflict: > > https://lore.kernel.org/all/20230216160012.272345-2-kristina.martsenko@arm.com/ I don't think they will conflict in the git sense. I think it should be pretty easy to deal with, this code can be dropped, and HCRX_{HOST, GUEST}_FLAGS will need to be modified to include HCRX_EL2_TCR2En. Thanks, Joey
On Tue, Mar 28, 2023 at 11:34:27AM +0100, Joey Gouly wrote: > On Mon, Mar 27, 2023 at 05:59:23PM +0100, Catalin Marinas wrote: > > On Thu, Mar 09, 2023 at 02:52:43PM +0000, Joey Gouly wrote: > > > diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h > > > index 037724b19c5c..6e6675fae194 100644 > > > --- a/arch/arm64/include/asm/el2_setup.h > > > +++ b/arch/arm64/include/asm/el2_setup.h > > > @@ -22,6 +22,21 @@ > > > isb > > > .endm > > > > > > +.macro __init_el2_hcrx > > > + mrs x1, id_aa64mmfr1_el1 > > > + ubfx x0, x1, #ID_AA64MMFR1_EL1_HCX_SHIFT, 4 > > > + cbz x0, .Lskip_hcrx_\@ > > > + > > > + mrs_s x1, SYS_ID_AA64MMFR2_EL1 > > > + ubfx x0, x1, #ID_AA64MMFR3_EL1_TCRX_SHIFT, 4 > > > + cbz x0, .Lskip_hcrx_\@ > > > + > > > + mrs_s x0, SYS_HCRX_EL2 > > > + orr x0, x0, #HCRX_EL2_TCR2En > > > + msr_s SYS_HCRX_EL2, x0 > > > +.Lskip_hcrx_\@: > > > +.endm > > > > Can we not just set HCRX_EL2_TCR2En without probing for TCRX? > > It's RES0, so I avoided it, but if it's fine I can drop the check. We do this in a few other places (SCTLR_ELx etc). When it's RES0, we only write 0 as we can't tell what it will do in a future architecture version. But now that it has a defined meaning, just set it and it has no effect on earlier CPUs (there is a definition of RES0 on page 11943 in the Arm ARM).
diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h index 037724b19c5c..6e6675fae194 100644 --- a/arch/arm64/include/asm/el2_setup.h +++ b/arch/arm64/include/asm/el2_setup.h @@ -22,6 +22,21 @@ isb .endm +.macro __init_el2_hcrx + mrs x1, id_aa64mmfr1_el1 + ubfx x0, x1, #ID_AA64MMFR1_EL1_HCX_SHIFT, 4 + cbz x0, .Lskip_hcrx_\@ + + mrs_s x1, SYS_ID_AA64MMFR2_EL1 + ubfx x0, x1, #ID_AA64MMFR3_EL1_TCRX_SHIFT, 4 + cbz x0, .Lskip_hcrx_\@ + + mrs_s x0, SYS_HCRX_EL2 + orr x0, x0, #HCRX_EL2_TCR2En + msr_s SYS_HCRX_EL2, x0 +.Lskip_hcrx_\@: +.endm + /* * Allow Non-secure EL1 and EL0 to access physical timer and counter. * This is not necessary for VHE, since the host kernel runs in EL2, @@ -150,12 +165,21 @@ mov x0, xzr mrs x1, id_aa64pfr1_el1 ubfx x1, x1, #ID_AA64PFR1_EL1_SME_SHIFT, #4 - cbz x1, .Lset_fgt_\@ + cbz x1, .Lset_pie_fgt_\@ /* Disable nVHE traps of TPIDR2 and SMPRI */ orr x0, x0, #HFGxTR_EL2_nSMPRI_EL1_MASK orr x0, x0, #HFGxTR_EL2_nTPIDR2_EL0_MASK +.Lset_pie_fgt_\@: + mrs_s x1, SYS_ID_AA64MMFR3_EL1 + ubfx x1, x1, #ID_AA64MMFR3_EL1_S1PIE_SHIFT, #4 + cbz x1, .Lset_fgt_\@ + + /* Disable trapping of PIR_EL1 / PIRE0_EL1 */ + orr x0, x0, #HFGxTR_EL2_nPIR_EL0_MASK + orr x0, x0, #HFGxTR_EL2_nPIREO_EL0_MASK + .Lset_fgt_\@: msr_s SYS_HFGRTR_EL2, x0 msr_s SYS_HFGWTR_EL2, x0 @@ -184,6 +208,7 @@ */ .macro init_el2_state __init_el2_sctlr + __init_el2_hcrx __init_el2_timers __init_el2_debug __init_el2_lor
Disable trapping of TCR2_EL1 and PIRx_EL1 registers, so they can be accessed from by EL1. Signed-off-by: Joey Gouly <joey.gouly@arm.com> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Will Deacon <will@kernel.org> --- arch/arm64/include/asm/el2_setup.h | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-)