| Message ID | 20231009010313.3668423-4-Henry.Wang@arm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/arm: Split MMU code as the prepration of MPU work | expand |
Hi Henry, On 09/10/2023 02:03, Henry Wang wrote: > Currently mmu_init_secondary_cpu() only enforces the page table > should not contain mapping that are both Writable and eXecutables > after boot. To ease the arch/arm/mm.c split work, fold this function > to head.S. > > Introduce assembly macro pt_enforce_wxn for both arm32 and arm64. > For arm64, the macro is called at the end of enable_secondary_cpu_mm(). > For arm32, the macro is called before secondary CPUs jumping into > the C world. > > Signed-off-by: Henry Wang <Henry.Wang@arm.com> > --- > v7: > - No change. Given the discusion on v6, I was expecting some changes here at least on arm64 side. For arm32, my proposal would not yet work because sadly the temporary page-tables for secondary bring-up will contain writable and executable mappings. > v6: > - New patch. > --- > xen/arch/arm/arm32/head.S | 20 ++++++++++++++++++++ > xen/arch/arm/arm64/mmu/head.S | 21 +++++++++++++++++++++ > xen/arch/arm/include/asm/mm.h | 2 -- > xen/arch/arm/mm.c | 6 ------ > xen/arch/arm/smpboot.c | 2 -- > 5 files changed, 41 insertions(+), 10 deletions(-) > > diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S > index 33b038e7e0..39218cf15f 100644 > --- a/xen/arch/arm/arm32/head.S > +++ b/xen/arch/arm/arm32/head.S > @@ -83,6 +83,25 @@ > isb > .endm > > +/* > + * Enforce Xen page-tables do not contain mapping that are both > + * Writable and eXecutables. > + * > + * This should be called on each secondary CPU. > + */ > +.macro pt_enforce_wxn tmp > + mrc CP32(\tmp, HSCTLR) > + orr \tmp, \tmp, #SCTLR_Axx_ELx_WXN > + dsb > + mcr CP32(\tmp, HSCTLR) > + /* > + * The TLBs may cache SCTLR_EL2.WXN. So ensure it is synchronized > + * before flushing the TLBs. > + */ > + isb > + flush_xen_tlb_local \tmp > +.endm > + > /* > * Common register usage in this file: > * r0 - > @@ -254,6 +273,7 @@ secondary_switched: > /* Use a virtual address to access the UART. */ > mov_w r11, EARLY_UART_VIRTUAL_ADDRESS > #endif > + pt_enforce_wxn r0 > PRINT("- Ready -\r\n") > /* Jump to C world */ > mov_w r2, start_secondary > diff --git a/xen/arch/arm/arm64/mmu/head.S b/xen/arch/arm/arm64/mmu/head.S > index 88075ef083..e4b07594b5 100644 > --- a/xen/arch/arm/arm64/mmu/head.S > +++ b/xen/arch/arm/arm64/mmu/head.S > @@ -31,6 +31,25 @@ > isb > .endm > > +/* > + * Enforce Xen page-tables do not contain mapping that are both > + * Writable and eXecutables. > + * > + * This should be called on each secondary CPU. > + */ > +.macro pt_enforce_wxn tmp > + mrs \tmp, SCTLR_EL2 > + orr \tmp, \tmp, #SCTLR_Axx_ELx_WXN > + dsb sy > + msr SCTLR_EL2, \tmp > + /* > + * The TLBs may cache SCTLR_EL2.WXN. So ensure it is synchronized > + * before flushing the TLBs. > + */ > + isb > + flush_xen_tlb_local > +.endm > + > /* > * Macro to find the slot number at a given page-table level > * > @@ -308,6 +327,8 @@ ENTRY(enable_secondary_cpu_mm) > bl enable_mmu > mov lr, x5 > > + pt_enforce_wxn x0 > + > /* Return to the virtual address requested by the caller. */ > ret > ENDPROC(enable_secondary_cpu_mm) > diff --git a/xen/arch/arm/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h > index d25e59f828..163d22ecd3 100644 > --- a/xen/arch/arm/include/asm/mm.h > +++ b/xen/arch/arm/include/asm/mm.h > @@ -214,8 +214,6 @@ extern void remove_early_mappings(void); > /* Allocate and initialise pagetables for a secondary CPU. Sets init_ttbr to the > * new page table */ > extern int init_secondary_pagetables(int cpu); > -/* Switch secondary CPUS to its own pagetables and finalise MMU setup */ > -extern void mmu_init_secondary_cpu(void); > /* > * For Arm32, set up the direct-mapped xenheap: up to 1GB of contiguous, > * always-mapped memory. Base must be 32MB aligned and size a multiple of 32MB. > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c > index b7eb3a6e08..923a90925c 100644 > --- a/xen/arch/arm/mm.c > +++ b/xen/arch/arm/mm.c > @@ -326,12 +326,6 @@ void __init setup_pagetables(unsigned long boot_phys_offset) > #endif > } > > -/* MMU setup for secondary CPUS (which already have paging enabled) */ > -void mmu_init_secondary_cpu(void) > -{ > - xen_pt_enforce_wnx(); > -} > - > #ifdef CONFIG_ARM_32 > /* > * Set up the direct-mapped xenheap: > diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c > index ec76de3cac..beb137d06e 100644 > --- a/xen/arch/arm/smpboot.c > +++ b/xen/arch/arm/smpboot.c > @@ -361,8 +361,6 @@ void start_secondary(void) > */ > update_system_features(¤t_cpu_data); > > - mmu_init_secondary_cpu(); > - > gic_init_secondary_cpu(); > > set_current(idle_vcpu[cpuid]); Cheers,
Hi Julien, > On Oct 14, 2023, at 01:26, Julien Grall <julien@xen.org> wrote: > > Hi Henry, > > On 09/10/2023 02:03, Henry Wang wrote: >> Currently mmu_init_secondary_cpu() only enforces the page table >> should not contain mapping that are both Writable and eXecutables >> after boot. To ease the arch/arm/mm.c split work, fold this function >> to head.S. >> Introduce assembly macro pt_enforce_wxn for both arm32 and arm64. >> For arm64, the macro is called at the end of enable_secondary_cpu_mm(). >> For arm32, the macro is called before secondary CPUs jumping into >> the C world. >> Signed-off-by: Henry Wang <Henry.Wang@arm.com> >> --- >> v7: >> - No change. > > Given the discusion on v6, I was expecting some changes here at least on arm64 side. Oh I am so sorry, I completely misunderstood the “leave the code as is” discussion and now I revisit that discussion and noticed that you did say the “leave the code as is” is for arm32 only ^^' I will fix the arm64 side in v8 following [1]. > > For arm32, my proposal would not yet work because sadly the temporary page-tables for secondary bring-up will contain writable and executable mappings. [1] https://lore.kernel.org/xen-devel/4d7a9849-8990-8ddd-3531-93f4e2e262b1@xen.org/ Kind regards, Henry
diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S index 33b038e7e0..39218cf15f 100644 --- a/xen/arch/arm/arm32/head.S +++ b/xen/arch/arm/arm32/head.S @@ -83,6 +83,25 @@ isb .endm +/* + * Enforce Xen page-tables do not contain mapping that are both + * Writable and eXecutables. + * + * This should be called on each secondary CPU. + */ +.macro pt_enforce_wxn tmp + mrc CP32(\tmp, HSCTLR) + orr \tmp, \tmp, #SCTLR_Axx_ELx_WXN + dsb + mcr CP32(\tmp, HSCTLR) + /* + * The TLBs may cache SCTLR_EL2.WXN. So ensure it is synchronized + * before flushing the TLBs. + */ + isb + flush_xen_tlb_local \tmp +.endm + /* * Common register usage in this file: * r0 - @@ -254,6 +273,7 @@ secondary_switched: /* Use a virtual address to access the UART. */ mov_w r11, EARLY_UART_VIRTUAL_ADDRESS #endif + pt_enforce_wxn r0 PRINT("- Ready -\r\n") /* Jump to C world */ mov_w r2, start_secondary diff --git a/xen/arch/arm/arm64/mmu/head.S b/xen/arch/arm/arm64/mmu/head.S index 88075ef083..e4b07594b5 100644 --- a/xen/arch/arm/arm64/mmu/head.S +++ b/xen/arch/arm/arm64/mmu/head.S @@ -31,6 +31,25 @@ isb .endm +/* + * Enforce Xen page-tables do not contain mapping that are both + * Writable and eXecutables. + * + * This should be called on each secondary CPU. + */ +.macro pt_enforce_wxn tmp + mrs \tmp, SCTLR_EL2 + orr \tmp, \tmp, #SCTLR_Axx_ELx_WXN + dsb sy + msr SCTLR_EL2, \tmp + /* + * The TLBs may cache SCTLR_EL2.WXN. So ensure it is synchronized + * before flushing the TLBs. + */ + isb + flush_xen_tlb_local +.endm + /* * Macro to find the slot number at a given page-table level * @@ -308,6 +327,8 @@ ENTRY(enable_secondary_cpu_mm) bl enable_mmu mov lr, x5 + pt_enforce_wxn x0 + /* Return to the virtual address requested by the caller. */ ret ENDPROC(enable_secondary_cpu_mm) diff --git a/xen/arch/arm/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h index d25e59f828..163d22ecd3 100644 --- a/xen/arch/arm/include/asm/mm.h +++ b/xen/arch/arm/include/asm/mm.h @@ -214,8 +214,6 @@ extern void remove_early_mappings(void); /* Allocate and initialise pagetables for a secondary CPU. Sets init_ttbr to the * new page table */ extern int init_secondary_pagetables(int cpu); -/* Switch secondary CPUS to its own pagetables and finalise MMU setup */ -extern void mmu_init_secondary_cpu(void); /* * For Arm32, set up the direct-mapped xenheap: up to 1GB of contiguous, * always-mapped memory. Base must be 32MB aligned and size a multiple of 32MB. diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index b7eb3a6e08..923a90925c 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -326,12 +326,6 @@ void __init setup_pagetables(unsigned long boot_phys_offset) #endif } -/* MMU setup for secondary CPUS (which already have paging enabled) */ -void mmu_init_secondary_cpu(void) -{ - xen_pt_enforce_wnx(); -} - #ifdef CONFIG_ARM_32 /* * Set up the direct-mapped xenheap: diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c index ec76de3cac..beb137d06e 100644 --- a/xen/arch/arm/smpboot.c +++ b/xen/arch/arm/smpboot.c @@ -361,8 +361,6 @@ void start_secondary(void) */ update_system_features(¤t_cpu_data); - mmu_init_secondary_cpu(); - gic_init_secondary_cpu(); set_current(idle_vcpu[cpuid]);
Currently mmu_init_secondary_cpu() only enforces the page table should not contain mapping that are both Writable and eXecutables after boot. To ease the arch/arm/mm.c split work, fold this function to head.S. Introduce assembly macro pt_enforce_wxn for both arm32 and arm64. For arm64, the macro is called at the end of enable_secondary_cpu_mm(). For arm32, the macro is called before secondary CPUs jumping into the C world. Signed-off-by: Henry Wang <Henry.Wang@arm.com> --- v7: - No change. v6: - New patch. --- xen/arch/arm/arm32/head.S | 20 ++++++++++++++++++++ xen/arch/arm/arm64/mmu/head.S | 21 +++++++++++++++++++++ xen/arch/arm/include/asm/mm.h | 2 -- xen/arch/arm/mm.c | 6 ------ xen/arch/arm/smpboot.c | 2 -- 5 files changed, 41 insertions(+), 10 deletions(-)