| Message ID | 20230927140123.5283-6-joey.gouly@arm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Permission Overlay Extension | expand |
On Wed, Sep 27, 2023 at 03:01:08PM +0100, Joey Gouly wrote: > +static void permission_overlay_switch(struct task_struct *next) > +{ > + if (alternative_has_cap_unlikely(ARM64_HAS_S1POE)) { > + current->thread.por_el0 = read_sysreg_s(SYS_POR_EL0); > + write_sysreg_s(next->thread.por_el0, SYS_POR_EL0); > + } > +} Does this need an ISB or is the POR_EL0 register write self-synchronising?
On Wed, Sep 27, 2023 at 03:01:08PM +0100, Joey Gouly wrote: > +/* Initial value for Permission Overlay Extension for EL0 */ > +#define POR_EL0_INIT UL(0x7) Might be useful to explain why this is the default (and possibly also define in terms of the constants for POR values)? > +static void permission_overlay_switch(struct task_struct *next) > +{ > + if (alternative_has_cap_unlikely(ARM64_HAS_S1POE)) { Why the _unlikely here?
Hi Mark, On Thu, Oct 05, 2023 at 03:14:50PM +0100, Mark Brown wrote: > On Wed, Sep 27, 2023 at 03:01:08PM +0100, Joey Gouly wrote: > > > +/* Initial value for Permission Overlay Extension for EL0 */ > > +#define POR_EL0_INIT UL(0x7) > > Might be useful to explain why this is the default (and possibly also > define in terms of the constants for POR values)? > > > +static void permission_overlay_switch(struct task_struct *next) > > +{ > > + if (alternative_has_cap_unlikely(ARM64_HAS_S1POE)) { > > Why the _unlikely here? The only options are alternative_has_cap_{likely,unlikely}(). I went with unlikely as currently it is unlikely! Thanks, Joey > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Tue, Oct 10, 2023 at 10:54:23AM +0100, Joey Gouly wrote: > On Thu, Oct 05, 2023 at 03:14:50PM +0100, Mark Brown wrote: > > On Wed, Sep 27, 2023 at 03:01:08PM +0100, Joey Gouly wrote: > > > + if (alternative_has_cap_unlikely(ARM64_HAS_S1POE)) { > > Why the _unlikely here? > The only options are alternative_has_cap_{likely,unlikely}(). > I went with unlikely as currently it is unlikely! Oh, if you have to pick one it doesn't really matter and that seems sensible. Usually they're optional so outside of very hot paths they tend to be an antipattern.
diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h index e5bc54522e71..b3ad719c2d0c 100644 --- a/arch/arm64/include/asm/processor.h +++ b/arch/arm64/include/asm/processor.h @@ -179,6 +179,7 @@ struct thread_struct { u64 sctlr_user; u64 svcr; u64 tpidr2_el0; + u64 por_el0; }; static inline unsigned int thread_get_vl(struct thread_struct *thread, diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h index cc2d61fd45c3..0dc8ee423af4 100644 --- a/arch/arm64/include/asm/sysreg.h +++ b/arch/arm64/include/asm/sysreg.h @@ -1007,6 +1007,9 @@ #define POE_RXW UL(0x7) #define POE_MASK UL(0xf) +/* Initial value for Permission Overlay Extension for EL0 */ +#define POR_EL0_INIT UL(0x7) + #define ARM64_FEATURE_FIELD_BITS 4 /* Defined for compatibility only, do not add new users. */ diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c index 0fcc4eb1a7ab..d33f9717bfcd 100644 --- a/arch/arm64/kernel/process.c +++ b/arch/arm64/kernel/process.c @@ -271,12 +271,19 @@ static void flush_tagged_addr_state(void) clear_thread_flag(TIF_TAGGED_ADDR); } +static void flush_poe(void) +{ + if (cpus_have_final_cap(ARM64_HAS_S1POE)) + write_sysreg_s(POR_EL0_INIT, SYS_POR_EL0); +} + void flush_thread(void) { fpsimd_flush_thread(); tls_thread_flush(); flush_ptrace_hw_breakpoint(current); flush_tagged_addr_state(); + flush_poe(); } void arch_release_task_struct(struct task_struct *tsk) @@ -498,6 +505,14 @@ static void erratum_1418040_new_exec(void) preempt_enable(); } +static void permission_overlay_switch(struct task_struct *next) +{ + if (alternative_has_cap_unlikely(ARM64_HAS_S1POE)) { + current->thread.por_el0 = read_sysreg_s(SYS_POR_EL0); + write_sysreg_s(next->thread.por_el0, SYS_POR_EL0); + } +} + /* * __switch_to() checks current->thread.sctlr_user as an optimisation. Therefore * this function must be called with preemption disabled and the update to @@ -533,6 +548,7 @@ struct task_struct *__switch_to(struct task_struct *prev, ssbs_thread_switch(next); erratum_1418040_thread_switch(next); ptrauth_thread_switch_user(next); + permission_overlay_switch(next); /* * Complete any pending TLB or cache maintenance on this CPU in case
POR_EL0 is a register that can be modified by userspace directly, so it must be context switched. 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/processor.h | 1 + arch/arm64/include/asm/sysreg.h | 3 +++ arch/arm64/kernel/process.c | 16 ++++++++++++++++ 3 files changed, 20 insertions(+)