| Message ID | 20230828013224.669433-9-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 28/08/2023 02:32, Henry Wang wrote: > CAUTION: This message has originated from an External Source. Please use proper judgment and caution when opening attachments, clicking links, or responding to this email. > > > 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> > --- > 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 > Can you move ^^^ to before "#ifdef CONFIG_EARLY_PRINTK" so that the MMU related functionality are bundled together? Also AFAIU, mov_w has not effect on pt_enforce_wxn(). So that I can create a function "enable_secondary_cpu_mm()" - similar to one you introduced for arm64 /* This will contain all the MMU related function for secondary cpu */ enable_secondary_cpu_mm: bl create_page_tables mov_w lr, secondary_switched .... flush_xen_tlb_local r0 pt_enforce_wxn r0 ENDPROC(enable_secondary_cpu_mm) - Ayan > 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 a5271e3880..25028bdf07 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 bf2fe26f9e..a66aa219b1 100644 > --- a/xen/arch/arm/include/asm/mm.h > +++ b/xen/arch/arm/include/asm/mm.h > @@ -216,8 +216,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 f3ef0da0e3..3ee74542ba 100644 > --- a/xen/arch/arm/mm.c > +++ b/xen/arch/arm/mm.c > @@ -322,12 +322,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 e107b86b7b..ade2c77cf9 100644 > --- a/xen/arch/arm/smpboot.c > +++ b/xen/arch/arm/smpboot.c > @@ -359,8 +359,6 @@ void start_secondary(void) > */ > update_system_features(¤t_cpu_data); > > - mmu_init_secondary_cpu(); > - > gic_init_secondary_cpu(); > > set_current(idle_vcpu[cpuid]); > -- > 2.25.1 > >
Hi Ayan, > On Aug 31, 2023, at 17:12, Ayan Kumar Halder <ayankuma@amd.com> wrote: > > Hi Henry, > > On 28/08/2023 02:32, Henry Wang wrote: >> >> 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 >> > > Can you move ^^^ to before "#ifdef CONFIG_EARLY_PRINTK" so that the MMU related functionality are bundled together? > > Also AFAIU, mov_w has not effect on pt_enforce_wxn(). > > So that I can create a function "enable_secondary_cpu_mm()" - similar to one you introduced for arm64 Sure, I am good with this if other maintainers do not have any objections. Kind regards, Henry > > /* This will contain all the MMU related function for secondary cpu */ > > enable_secondary_cpu_mm: > > bl create_page_tables > > mov_w lr, secondary_switched > > .... > > flush_xen_tlb_local r0 > > pt_enforce_wxn r0 > > ENDPROC(enable_secondary_cpu_mm) > > > - Ayan
On 31/08/2023 10:16, Henry Wang wrote: >> On Aug 31, 2023, at 17:12, Ayan Kumar Halder <ayankuma@amd.com> wrote: >> >> Hi Henry, >> >> On 28/08/2023 02:32, Henry Wang wrote: >>> >>> 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 >>> >> >> Can you move ^^^ to before "#ifdef CONFIG_EARLY_PRINTK" so that the MMU related functionality are bundled together? >> >> Also AFAIU, mov_w has not effect on pt_enforce_wxn(). >> >> So that I can create a function "enable_secondary_cpu_mm()" - similar to one you introduced for arm64 > > Sure, I am good with this if other maintainers do not have any objections. I am objecting. It would be quite handy to print a message on the console to indicate that we are enforce WXN. So we want to update UART address (stored in r11) before hand. Cheers,
Hi Julien, On 11/09/2023 15:51, Julien Grall wrote: > > > On 31/08/2023 10:16, Henry Wang wrote: >>> On Aug 31, 2023, at 17:12, Ayan Kumar Halder <ayankuma@amd.com> wrote: >>> >>> Hi Henry, >>> >>> On 28/08/2023 02:32, Henry Wang wrote: >>>> >>>> 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 >>> >>> Can you move ^^^ to before "#ifdef CONFIG_EARLY_PRINTK" so that the >>> MMU related functionality are bundled together? >>> >>> Also AFAIU, mov_w has not effect on pt_enforce_wxn(). >>> >>> So that I can create a function "enable_secondary_cpu_mm()" - >>> similar to one you introduced for arm64 >> >> Sure, I am good with this if other maintainers do not have any >> objections. > > I am objecting. It would be quite handy to print a message on the > console to indicate that we are enforce WXN. So we want to update UART > address (stored in r11) before hand. You mean you want to add this snippet in the current patch diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S index 39218cf15f..282b89a96e 100644 --- a/xen/arch/arm/arm32/head.S +++ b/xen/arch/arm/arm32/head.S @@ -273,6 +273,7 @@ secondary_switched: /* Use a virtual address to access the UART. */ mov_w r11, EARLY_UART_VIRTUAL_ADDRESS #endif + PRINT("- Enforce WXN -\r\n") pt_enforce_wxn r0 PRINT("- Ready -\r\n") /* Jump to C world */ - Ayan > > Cheers, >
Hi Julien, > On Sep 11, 2023, at 22:51, Julien Grall <julien@xen.org> wrote: > On 31/08/2023 10:16, Henry Wang wrote: >>> On Aug 31, 2023, at 17:12, Ayan Kumar Halder <ayankuma@amd.com> wrote: >>> >>> Hi Henry, >>> >>> On 28/08/2023 02:32, Henry Wang wrote: >>>> >>>> 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 >>>> >>> >>> Can you move ^^^ to before "#ifdef CONFIG_EARLY_PRINTK" so that the MMU related functionality are bundled together? >>> >>> Also AFAIU, mov_w has not effect on pt_enforce_wxn(). >>> >>> So that I can create a function "enable_secondary_cpu_mm()" - similar to one you introduced for arm64 >> Sure, I am good with this if other maintainers do not have any objections. > > I am objecting. It would be quite handy to print a message on the console to indicate that we are enforce WXN. So we want to update UART address (stored in r11) before hand. Good idea about the printing, I am happy to add a printed message on top of the macro saying that we are enforcing WXN from here if you agree. Kind regards, Henry > > Cheers, > > -- > Julien Grall
Hi Henry, On 28/08/2023 02:32, 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> > --- > 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 a5271e3880..25028bdf07 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 > + It would be preferable if we can set the flag right when the MMU is initialized enabled configured. This would avoid the extra TLB flush and SCTLR dance. How about the following (not compiled/cleaned) code: diff --git a/xen/arch/arm/arm64/mmu/head.S b/xen/arch/arm/arm64/mmu/head.S index a5271e388071..6b19d15ff89f 100644 --- a/xen/arch/arm/arm64/mmu/head.S +++ b/xen/arch/arm/arm64/mmu/head.S @@ -264,10 +264,11 @@ ENDPROC(create_page_tables) * Inputs: * x0 : Physical address of the page tables. * - * Clobbers x0 - x4 + * Clobbers x0 - x6 */ enable_mmu: mov x4, x0 + mov x5, x1 PRINT("- Turning on paging -\r\n") /* @@ -283,6 +284,7 @@ enable_mmu: mrs x0, SCTLR_EL2 orr x0, x0, #SCTLR_Axx_ELx_M /* Enable MMU */ orr x0, x0, #SCTLR_Axx_ELx_C /* Enable D-cache */ + orr x0, x0, x5 /* Enable extra flags */ dsb sy /* Flush PTE writes and finish reads */ msr SCTLR_EL2, x0 /* now paging is enabled */ isb /* Now, flush the icache */ @@ -297,16 +299,17 @@ ENDPROC(enable_mmu) * Inputs: * lr : Virtual address to return to. * - * Clobbers x0 - x5 + * Clobbers x0 - x6 */ ENTRY(enable_secondary_cpu_mm) - mov x5, lr + mov x6, lr load_paddr x0, init_ttbr ldr x0, [x0] + mov x1, #SCTLR_Axx_ELx_WXN /* Enable WxN from the start */ bl enable_mmu - mov lr, x5 + mov lr, x6 /* Return to the virtual address requested by the caller. */ ret @@ -320,16 +323,17 @@ ENDPROC(enable_secondary_cpu_mm) * Inputs: * lr : Virtual address to return to. * - * Clobbers x0 - x5 + * Clobbers x0 - x6 */ ENTRY(enable_boot_cpu_mm) - mov x5, lr + mov x6, lr bl create_page_tables load_paddr x0, boot_pgtable + mov x1, #0 /* No extra SCTLR flags */ bl enable_mmu - mov lr, x5 + mov lr, x6 /* * The MMU is turned on and we are in the 1:1 mapping. Switch The same logic could be used for arm32. Cheers,
Hi Julien, > On Sep 16, 2023, at 05:58, Julien Grall <julien@xen.org> wrote: > > Hi Henry, > > On 28/08/2023 02:32, 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> >> --- >> 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 a5271e3880..25028bdf07 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 >> + > > It would be preferable if we can set the flag right when the MMU is initialized enabled configured. This would avoid the extra TLB flush and SCTLR dance. How about the following (not compiled/cleaned) code: Thank you for the detailed information. Sure, I will try below code and keep you updated about if it works. Will update the patch accordingly. > > diff --git a/xen/arch/arm/arm64/mmu/head.S b/xen/arch/arm/arm64/mmu/head.S > index a5271e388071..6b19d15ff89f 100644 > --- a/xen/arch/arm/arm64/mmu/head.S > +++ b/xen/arch/arm/arm64/mmu/head.S > @@ -264,10 +264,11 @@ ENDPROC(create_page_tables) > * Inputs: > * x0 : Physical address of the page tables. > * > - * Clobbers x0 - x4 > + * Clobbers x0 - x6 > */ > enable_mmu: > mov x4, x0 > + mov x5, x1 > PRINT("- Turning on paging -\r\n") > > /* > @@ -283,6 +284,7 @@ enable_mmu: > mrs x0, SCTLR_EL2 > orr x0, x0, #SCTLR_Axx_ELx_M /* Enable MMU */ > orr x0, x0, #SCTLR_Axx_ELx_C /* Enable D-cache */ > + orr x0, x0, x5 /* Enable extra flags */ > dsb sy /* Flush PTE writes and finish reads */ > msr SCTLR_EL2, x0 /* now paging is enabled */ > isb /* Now, flush the icache */ > @@ -297,16 +299,17 @@ ENDPROC(enable_mmu) > * Inputs: > * lr : Virtual address to return to. > * > - * Clobbers x0 - x5 > + * Clobbers x0 - x6 > */ > ENTRY(enable_secondary_cpu_mm) > - mov x5, lr > + mov x6, lr > > load_paddr x0, init_ttbr > ldr x0, [x0] > > + mov x1, #SCTLR_Axx_ELx_WXN /* Enable WxN from the start */ > bl enable_mmu > - mov lr, x5 > + mov lr, x6 > > /* Return to the virtual address requested by the caller. */ > ret > @@ -320,16 +323,17 @@ ENDPROC(enable_secondary_cpu_mm) > * Inputs: > * lr : Virtual address to return to. > * > - * Clobbers x0 - x5 > + * Clobbers x0 - x6 > */ > ENTRY(enable_boot_cpu_mm) > - mov x5, lr > + mov x6, lr > > bl create_page_tables > load_paddr x0, boot_pgtable > > + mov x1, #0 /* No extra SCTLR flags */ > bl enable_mmu > - mov lr, x5 > + mov lr, x6 > > /* > * The MMU is turned on and we are in the 1:1 mapping. Switch > > The same logic could be used for arm32. Sure. Will do that together. Kind regards, Henry > > Cheers, > > -- > Julien Grall
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 a5271e3880..25028bdf07 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 bf2fe26f9e..a66aa219b1 100644 --- a/xen/arch/arm/include/asm/mm.h +++ b/xen/arch/arm/include/asm/mm.h @@ -216,8 +216,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 f3ef0da0e3..3ee74542ba 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -322,12 +322,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 e107b86b7b..ade2c77cf9 100644 --- a/xen/arch/arm/smpboot.c +++ b/xen/arch/arm/smpboot.c @@ -359,8 +359,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> --- 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(-)