diff mbox series

[v1,05/20] arm64: context switch POR_EL0 register

Message ID 20230927140123.5283-6-joey.gouly@arm.com
State New
Headers show
Series Permission Overlay Extension | expand

Commit Message

Joey Gouly Sept. 27, 2023, 2:01 p.m. UTC
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(+)

Comments

Catalin Marinas Oct. 4, 2023, 4:49 p.m. UTC | #1
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?
Mark Brown Oct. 5, 2023, 2:14 p.m. UTC | #2
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?
Joey Gouly Oct. 10, 2023, 9:54 a.m. UTC | #3
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
Mark Brown Oct. 10, 2023, 11:47 a.m. UTC | #4
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 mbox series

Patch

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