All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] arm64: mm: convert cpu_do_switch_mm() to C
@ 2020-02-13 12:14 Mark Rutland
  2020-02-27 14:29 ` Catalin Marinas
  0 siblings, 1 reply; 6+ messages in thread
From: Mark Rutland @ 2020-02-13 12:14 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: Mark Rutland, Catalin Marinas, Will Deacon

There's no reason that cpu_do_switch_mm() needs to be written as an
assembly function, and having it as a C function would make it easier to
maintain.

This patch converts cpu_do_switch_mm() to C, removing code that this
change makes redundant (e.g. the mmid macro). Since the header comment
was stale and the prototype now implies all the necessary information,
this comment is removed. The 'pgd_phys' argument is made a phys_addr_t
to match the return type of virt_to_phys().

At the same time, post_ttbr_update_workaround() is updated to use
IS_ENABLED(), which allows the compiler to figure out it can elide calls
for !CONFIG_CAVIUM_ERRATUM_27456 builds.

There should be no functional change as a result of this patch.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/include/asm/assembler.h   |  6 ------
 arch/arm64/include/asm/mmu_context.h |  2 ++
 arch/arm64/include/asm/proc-fns.h    |  2 --
 arch/arm64/mm/context.c              | 32 ++++++++++++++++++++++++++++++--
 arch/arm64/mm/proc.S                 | 28 ----------------------------
 5 files changed, 32 insertions(+), 38 deletions(-)

I've been sitting on a version of this for a while; it's orthogonal to the
entry rework I'm working on, so I'm sending it separately. I've given this some
light testing in a VM, spawning a large number of tasks in parallel to force MM
switches.

Mark.

diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index aca337d79d12..af03001293c6 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -257,12 +257,6 @@ alternative_endif
 	.endm
 
 /*
- * mmid - get context id from mm pointer (mm->context.id)
- */
-	.macro	mmid, rd, rn
-	ldr	\rd, [\rn, #MM_CONTEXT_ID]
-	.endm
-/*
  * read_ctr - read CTR_EL0. If the system has mismatched register fields,
  * provide the system wide safe value from arm64_ftr_reg_ctrel0.sys_val
  */
diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h
index 3827ff4040a3..ab46187c6300 100644
--- a/arch/arm64/include/asm/mmu_context.h
+++ b/arch/arm64/include/asm/mmu_context.h
@@ -46,6 +46,8 @@ static inline void cpu_set_reserved_ttbr0(void)
 	isb();
 }
 
+void cpu_do_switch_mm(phys_addr_t pgd_phys, struct mm_struct *mm);
+
 static inline void cpu_switch_mm(pgd_t *pgd, struct mm_struct *mm)
 {
 	BUG_ON(pgd == swapper_pg_dir);
diff --git a/arch/arm64/include/asm/proc-fns.h b/arch/arm64/include/asm/proc-fns.h
index a2ce65a0c1fa..0d5d1f0525eb 100644
--- a/arch/arm64/include/asm/proc-fns.h
+++ b/arch/arm64/include/asm/proc-fns.h
@@ -13,11 +13,9 @@
 
 #include <asm/page.h>
 
-struct mm_struct;
 struct cpu_suspend_ctx;
 
 extern void cpu_do_idle(void);
-extern void cpu_do_switch_mm(unsigned long pgd_phys, struct mm_struct *mm);
 extern void cpu_do_suspend(struct cpu_suspend_ctx *ptr);
 extern u64 cpu_do_resume(phys_addr_t ptr, u64 idmap_ttbr);
 
diff --git a/arch/arm64/mm/context.c b/arch/arm64/mm/context.c
index 8ef73e89d514..5edefc8656c5 100644
--- a/arch/arm64/mm/context.c
+++ b/arch/arm64/mm/context.c
@@ -6,6 +6,7 @@
  * Copyright (C) 2012 ARM Ltd.
  */
 
+#include <linux/bitfield.h>
 #include <linux/bitops.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
@@ -254,10 +255,37 @@ void check_and_switch_context(struct mm_struct *mm, unsigned int cpu)
 /* Errata workaround post TTBRx_EL1 update. */
 asmlinkage void post_ttbr_update_workaround(void)
 {
+	if (!IS_ENABLED(CONFIG_CAVIUM_ERRATUM_27456))
+		return;
+
 	asm(ALTERNATIVE("nop; nop; nop",
 			"ic iallu; dsb nsh; isb",
-			ARM64_WORKAROUND_CAVIUM_27456,
-			CONFIG_CAVIUM_ERRATUM_27456));
+			ARM64_WORKAROUND_CAVIUM_27456));
+}
+
+void cpu_do_switch_mm(phys_addr_t pgd_phys, struct mm_struct *mm)
+{
+	unsigned long ttbr1 = read_sysreg(ttbr1_el1);
+	unsigned long asid = ASID(mm);
+	unsigned long ttbr0 = phys_to_ttbr(pgd_phys);
+
+	// Skip CNP for the reserved ASID
+	if (system_supports_cnp() && asid)
+		ttbr0 |= TTBR_CNP_BIT;
+
+	// SW PAN needs a copy of the ASID in TTBR0 for entry
+	if (IS_ENABLED(CONFIG_ARM64_SW_TTBR0_PAN))
+		ttbr0 |= FIELD_PREP(TTBR_ASID_MASK, asid);
+
+	// Set ASID in TTBR1 since TCR.A1 is set
+	ttbr1 &= ~TTBR_ASID_MASK;
+	ttbr1 |= FIELD_PREP(TTBR_ASID_MASK, asid);
+
+	write_sysreg(ttbr1, ttbr1_el1);
+	isb();
+	write_sysreg(ttbr0, ttbr0_el1);
+	isb();
+	post_ttbr_update_workaround();
 }
 
 static int asids_init(void)
diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
index aafed6902411..76899c6eee2b 100644
--- a/arch/arm64/mm/proc.S
+++ b/arch/arm64/mm/proc.S
@@ -142,34 +142,6 @@ SYM_FUNC_END(cpu_do_resume)
 	.popsection
 #endif
 
-/*
- *	cpu_do_switch_mm(pgd_phys, tsk)
- *
- *	Set the translation table base pointer to be pgd_phys.
- *
- *	- pgd_phys - physical address of new TTB
- */
-SYM_FUNC_START(cpu_do_switch_mm)
-	mrs	x2, ttbr1_el1
-	mmid	x1, x1				// get mm->context.id
-	phys_to_ttbr x3, x0
-
-alternative_if ARM64_HAS_CNP
-	cbz     x1, 1f                          // skip CNP for reserved ASID
-	orr     x3, x3, #TTBR_CNP_BIT
-1:
-alternative_else_nop_endif
-#ifdef CONFIG_ARM64_SW_TTBR0_PAN
-	bfi	x3, x1, #48, #16		// set the ASID field in TTBR0
-#endif
-	bfi	x2, x1, #48, #16		// set the ASID
-	msr	ttbr1_el1, x2			// in TTBR1 (since TCR.A1 is set)
-	isb
-	msr	ttbr0_el1, x3			// now update TTBR0
-	isb
-	b	post_ttbr_update_workaround	// Back to C code...
-SYM_FUNC_END(cpu_do_switch_mm)
-
 	.pushsection ".idmap.text", "awx"
 
 .macro	__idmap_cpu_set_reserved_ttbr1, tmp1, tmp2
-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] arm64: mm: convert cpu_do_switch_mm() to C
  2020-02-13 12:14 [PATCH] arm64: mm: convert cpu_do_switch_mm() to C Mark Rutland
@ 2020-02-27 14:29 ` Catalin Marinas
  2020-02-28 11:13   ` Will Deacon
  0 siblings, 1 reply; 6+ messages in thread
From: Catalin Marinas @ 2020-02-27 14:29 UTC (permalink / raw)
  To: Mark Rutland; +Cc: Will Deacon, linux-arm-kernel

On Thu, Feb 13, 2020 at 12:14:52PM +0000, Mark Rutland wrote:
> There's no reason that cpu_do_switch_mm() needs to be written as an
> assembly function, and having it as a C function would make it easier to
> maintain.
> 
> This patch converts cpu_do_switch_mm() to C, removing code that this
> change makes redundant (e.g. the mmid macro). Since the header comment
> was stale and the prototype now implies all the necessary information,
> this comment is removed. The 'pgd_phys' argument is made a phys_addr_t
> to match the return type of virt_to_phys().
> 
> At the same time, post_ttbr_update_workaround() is updated to use
> IS_ENABLED(), which allows the compiler to figure out it can elide calls
> for !CONFIG_CAVIUM_ERRATUM_27456 builds.
> 
> There should be no functional change as a result of this patch.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>

I'll queue this for 5.7. Does not seem to have any functional change (I
changed the comments to C-style ones /* */).

Thanks.

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] arm64: mm: convert cpu_do_switch_mm() to C
  2020-02-27 14:29 ` Catalin Marinas
@ 2020-02-28 11:13   ` Will Deacon
  2020-02-28 12:47     ` Will Deacon
  0 siblings, 1 reply; 6+ messages in thread
From: Will Deacon @ 2020-02-28 11:13 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: Mark Rutland, linux-arm-kernel

On Thu, Feb 27, 2020 at 02:29:46PM +0000, Catalin Marinas wrote:
> On Thu, Feb 13, 2020 at 12:14:52PM +0000, Mark Rutland wrote:
> > There's no reason that cpu_do_switch_mm() needs to be written as an
> > assembly function, and having it as a C function would make it easier to
> > maintain.
> > 
> > This patch converts cpu_do_switch_mm() to C, removing code that this
> > change makes redundant (e.g. the mmid macro). Since the header comment
> > was stale and the prototype now implies all the necessary information,
> > this comment is removed. The 'pgd_phys' argument is made a phys_addr_t
> > to match the return type of virt_to_phys().
> > 
> > At the same time, post_ttbr_update_workaround() is updated to use
> > IS_ENABLED(), which allows the compiler to figure out it can elide calls
> > for !CONFIG_CAVIUM_ERRATUM_27456 builds.
> > 
> > There should be no functional change as a result of this patch.
> > 
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > Cc: Will Deacon <will@kernel.org>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> 
> I'll queue this for 5.7. Does not seem to have any functional change (I
> changed the comments to C-style ones /* */).

Can you also update the comment in asm/mmu.h for the ASID() macro please?

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] arm64: mm: convert cpu_do_switch_mm() to C
  2020-02-28 11:13   ` Will Deacon
@ 2020-02-28 12:47     ` Will Deacon
  2020-02-28 13:38       ` Catalin Marinas
  2020-02-28 14:16       ` Mark Rutland
  0 siblings, 2 replies; 6+ messages in thread
From: Will Deacon @ 2020-02-28 12:47 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: Mark Rutland, linux-arm-kernel

On Fri, Feb 28, 2020 at 11:13:50AM +0000, Will Deacon wrote:
> On Thu, Feb 27, 2020 at 02:29:46PM +0000, Catalin Marinas wrote:
> > On Thu, Feb 13, 2020 at 12:14:52PM +0000, Mark Rutland wrote:
> > > There's no reason that cpu_do_switch_mm() needs to be written as an
> > > assembly function, and having it as a C function would make it easier to
> > > maintain.
> > > 
> > > This patch converts cpu_do_switch_mm() to C, removing code that this
> > > change makes redundant (e.g. the mmid macro). Since the header comment
> > > was stale and the prototype now implies all the necessary information,
> > > this comment is removed. The 'pgd_phys' argument is made a phys_addr_t
> > > to match the return type of virt_to_phys().
> > > 
> > > At the same time, post_ttbr_update_workaround() is updated to use
> > > IS_ENABLED(), which allows the compiler to figure out it can elide calls
> > > for !CONFIG_CAVIUM_ERRATUM_27456 builds.
> > > 
> > > There should be no functional change as a result of this patch.
> > > 
> > > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > > Cc: Will Deacon <will@kernel.org>
> > > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > 
> > I'll queue this for 5.7. Does not seem to have any functional change (I
> > changed the comments to C-style ones /* */).
> 
> Can you also update the comment in asm/mmu.h for the ASID() macro please?

Ah, I see this is already queued (I'm catching up with email after a trip
to the US) so here's a patch.

Will

--->8

From 2528094854c3b56ad3fe49d2164c9a920a251f05 Mon Sep 17 00:00:00 2001
From: Will Deacon <will@kernel.org>
Date: Fri, 28 Feb 2020 12:43:55 +0000
Subject: [PATCH] arm64: Update comment for ASID() macro

Commit 25b92693a1b6 ("arm64: mm: convert cpu_do_switch_mm() to C") added
a new use of the ASID() macro, so update the comment in asm/mmu.h which
reasons about why an atomic reload of 'mm->context.id.counter' is not
required.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Will Deacon <will@kernel.org>
---
 arch/arm64/include/asm/mmu.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
index e4d862420bb4..21a4bcfdb378 100644
--- a/arch/arm64/include/asm/mmu.h
+++ b/arch/arm64/include/asm/mmu.h
@@ -23,9 +23,9 @@ typedef struct {
 } mm_context_t;
 
 /*
- * This macro is only used by the TLBI code, which cannot race with an
- * ASID change and therefore doesn't need to reload the counter using
- * atomic64_read.
+ * This macro is only used by the TLBI and low-level switch_mm() code,
+ * neither of which can race with an ASID change. We therefore don't
+ * need to reload the counter using atomic64_read().
  */
 #define ASID(mm)	((mm)->context.id.counter & 0xffff)
 
-- 
2.25.1.481.gfbce0eb801-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] arm64: mm: convert cpu_do_switch_mm() to C
  2020-02-28 12:47     ` Will Deacon
@ 2020-02-28 13:38       ` Catalin Marinas
  2020-02-28 14:16       ` Mark Rutland
  1 sibling, 0 replies; 6+ messages in thread
From: Catalin Marinas @ 2020-02-28 13:38 UTC (permalink / raw)
  To: Will Deacon; +Cc: Mark Rutland, linux-arm-kernel

On Fri, Feb 28, 2020 at 12:47:32PM +0000, Will Deacon wrote:
> From 2528094854c3b56ad3fe49d2164c9a920a251f05 Mon Sep 17 00:00:00 2001
> From: Will Deacon <will@kernel.org>
> Date: Fri, 28 Feb 2020 12:43:55 +0000
> Subject: [PATCH] arm64: Update comment for ASID() macro
> 
> Commit 25b92693a1b6 ("arm64: mm: convert cpu_do_switch_mm() to C") added
> a new use of the ASID() macro, so update the comment in asm/mmu.h which
> reasons about why an atomic reload of 'mm->context.id.counter' is not
> required.
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Will Deacon <will@kernel.org>

Thanks. Applied.

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] arm64: mm: convert cpu_do_switch_mm() to C
  2020-02-28 12:47     ` Will Deacon
  2020-02-28 13:38       ` Catalin Marinas
@ 2020-02-28 14:16       ` Mark Rutland
  1 sibling, 0 replies; 6+ messages in thread
From: Mark Rutland @ 2020-02-28 14:16 UTC (permalink / raw)
  To: Will Deacon; +Cc: Catalin Marinas, linux-arm-kernel

On Fri, Feb 28, 2020 at 12:47:32PM +0000, Will Deacon wrote:
> On Fri, Feb 28, 2020 at 11:13:50AM +0000, Will Deacon wrote:
> > On Thu, Feb 27, 2020 at 02:29:46PM +0000, Catalin Marinas wrote:
> > > On Thu, Feb 13, 2020 at 12:14:52PM +0000, Mark Rutland wrote:
> > > > There's no reason that cpu_do_switch_mm() needs to be written as an
> > > > assembly function, and having it as a C function would make it easier to
> > > > maintain.
> > > > 
> > > > This patch converts cpu_do_switch_mm() to C, removing code that this
> > > > change makes redundant (e.g. the mmid macro). Since the header comment
> > > > was stale and the prototype now implies all the necessary information,
> > > > this comment is removed. The 'pgd_phys' argument is made a phys_addr_t
> > > > to match the return type of virt_to_phys().
> > > > 
> > > > At the same time, post_ttbr_update_workaround() is updated to use
> > > > IS_ENABLED(), which allows the compiler to figure out it can elide calls
> > > > for !CONFIG_CAVIUM_ERRATUM_27456 builds.
> > > > 
> > > > There should be no functional change as a result of this patch.
> > > > 
> > > > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > > > Cc: Will Deacon <will@kernel.org>
> > > > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > > 
> > > I'll queue this for 5.7. Does not seem to have any functional change (I
> > > changed the comments to C-style ones /* */).
> > 
> > Can you also update the comment in asm/mmu.h for the ASID() macro please?
> 
> Ah, I see this is already queued (I'm catching up with email after a trip
> to the US) so here's a patch.
> 
> Will

Thanks for the fixup; sorry for missing that in the first place!

Mark.

> 
> --->8
> 
> From 2528094854c3b56ad3fe49d2164c9a920a251f05 Mon Sep 17 00:00:00 2001
> From: Will Deacon <will@kernel.org>
> Date: Fri, 28 Feb 2020 12:43:55 +0000
> Subject: [PATCH] arm64: Update comment for ASID() macro
> 
> Commit 25b92693a1b6 ("arm64: mm: convert cpu_do_switch_mm() to C") added
> a new use of the ASID() macro, so update the comment in asm/mmu.h which
> reasons about why an atomic reload of 'mm->context.id.counter' is not
> required.
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Will Deacon <will@kernel.org>
> ---
>  arch/arm64/include/asm/mmu.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
> index e4d862420bb4..21a4bcfdb378 100644
> --- a/arch/arm64/include/asm/mmu.h
> +++ b/arch/arm64/include/asm/mmu.h
> @@ -23,9 +23,9 @@ typedef struct {
>  } mm_context_t;
>  
>  /*
> - * This macro is only used by the TLBI code, which cannot race with an
> - * ASID change and therefore doesn't need to reload the counter using
> - * atomic64_read.
> + * This macro is only used by the TLBI and low-level switch_mm() code,
> + * neither of which can race with an ASID change. We therefore don't
> + * need to reload the counter using atomic64_read().
>   */
>  #define ASID(mm)	((mm)->context.id.counter & 0xffff)
>  
> -- 
> 2.25.1.481.gfbce0eb801-goog
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-02-28 14:16 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-13 12:14 [PATCH] arm64: mm: convert cpu_do_switch_mm() to C Mark Rutland
2020-02-27 14:29 ` Catalin Marinas
2020-02-28 11:13   ` Will Deacon
2020-02-28 12:47     ` Will Deacon
2020-02-28 13:38       ` Catalin Marinas
2020-02-28 14:16       ` Mark Rutland

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.