| Message ID | 20190417175815.16905-2-julien.grall@arm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/arm: TLB flush helpers rework | expand |
On 17.04.19 20:58, Julien Grall wrote: > The logic to set SCTLR_EL2.WXN is the same for the boot CPU and > non-boot CPU. So introduce a function to set the bit and clear TBLs. s/TBL/TLB/ > > This new function will help us to document and update the logic in a > single place. > > Signed-off-by: Julien Grall <julien.grall@arm.com> > --- > xen/arch/arm/mm.c | 22 +++++++++++++++------- > 1 file changed, 15 insertions(+), 7 deletions(-) > > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c > index 01ae2cccc0..93ad118183 100644 > --- a/xen/arch/arm/mm.c > +++ b/xen/arch/arm/mm.c > @@ -601,6 +601,19 @@ void __init remove_early_mappings(void) > flush_xen_data_tlb_range_va(BOOT_FDT_VIRT_START, BOOT_FDT_SLOT_SIZE); > } > > +/* > + * After boot, Xen page-tables should not contain mapping that are both > + * Writable and eXecutables. > + * > + * This should be called on each CPU to enforce the policy. > + */ > +static void xen_pt_enforce_wnx(void) Could it be inline? > +{ > + WRITE_SYSREG32(READ_SYSREG32(SCTLR_EL2) | SCTLR_WXN, SCTLR_EL2); > + /* Flush everything after setting WXN bit. */ > + flush_xen_text_tlb_local(); > +} > + > extern void switch_ttbr(uint64_t ttbr); > > /* Clear a translation table and clean & invalidate the cache */ > @@ -702,10 +715,7 @@ void __init setup_pagetables(unsigned long boot_phys_offset) > clear_table(boot_second); > clear_table(boot_third); > > - /* From now on, no mapping may be both writable and executable. */ > - WRITE_SYSREG32(READ_SYSREG32(SCTLR_EL2) | SCTLR_WXN, SCTLR_EL2); > - /* Flush everything after setting WXN bit. */ > - flush_xen_text_tlb_local(); > + xen_pt_enforce_wnx(); > > #ifdef CONFIG_ARM_32 > per_cpu(xen_pgtable, 0) = cpu0_pgtable; > @@ -777,9 +787,7 @@ int init_secondary_pagetables(int cpu) > /* MMU setup for secondary CPUS (which already have paging enabled) */ > void mmu_init_secondary_cpu(void) > { > - /* From now on, no mapping may be both writable and executable. */ > - WRITE_SYSREG32(READ_SYSREG32(SCTLR_EL2) | SCTLR_WXN, SCTLR_EL2); > - flush_xen_text_tlb_local(); > + xen_pt_enforce_wnx(); > } > > #ifdef CONFIG_ARM_32 > With minor notes, Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
Hi, On 25/04/2019 19:00, Andrii Anisov wrote: > > > On 17.04.19 20:58, Julien Grall wrote: >> The logic to set SCTLR_EL2.WXN is the same for the boot CPU and >> non-boot CPU. So introduce a function to set the bit and clear TBLs. > s/TBL/TLB/ > >> >> This new function will help us to document and update the logic in a >> single place. >> >> Signed-off-by: Julien Grall <julien.grall@arm.com> > >> --- >> xen/arch/arm/mm.c | 22 +++++++++++++++------- >> 1 file changed, 15 insertions(+), 7 deletions(-) >> >> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c >> index 01ae2cccc0..93ad118183 100644 >> --- a/xen/arch/arm/mm.c >> +++ b/xen/arch/arm/mm.c >> @@ -601,6 +601,19 @@ void __init remove_early_mappings(void) >> flush_xen_data_tlb_range_va(BOOT_FDT_VIRT_START, >> BOOT_FDT_SLOT_SIZE); >> } >> +/* >> + * After boot, Xen page-tables should not contain mapping that are both >> + * Writable and eXecutables. >> + * >> + * This should be called on each CPU to enforce the policy. >> + */ >> +static void xen_pt_enforce_wnx(void) > Could it be inline? Why can't we let the compiler deciding for us? The more that inline is pretty broken. See: https://www.kernel.org/doc/local/inline.html > >> +{ >> + WRITE_SYSREG32(READ_SYSREG32(SCTLR_EL2) | SCTLR_WXN, SCTLR_EL2); >> + /* Flush everything after setting WXN bit. */ >> + flush_xen_text_tlb_local(); >> +} >> + >> extern void switch_ttbr(uint64_t ttbr); >> /* Clear a translation table and clean & invalidate the cache */ >> @@ -702,10 +715,7 @@ void __init setup_pagetables(unsigned long >> boot_phys_offset) >> clear_table(boot_second); >> clear_table(boot_third); >> - /* From now on, no mapping may be both writable and executable. */ >> - WRITE_SYSREG32(READ_SYSREG32(SCTLR_EL2) | SCTLR_WXN, SCTLR_EL2); >> - /* Flush everything after setting WXN bit. */ >> - flush_xen_text_tlb_local(); >> + xen_pt_enforce_wnx(); >> #ifdef CONFIG_ARM_32 >> per_cpu(xen_pgtable, 0) = cpu0_pgtable; >> @@ -777,9 +787,7 @@ int init_secondary_pagetables(int cpu) >> /* MMU setup for secondary CPUS (which already have paging enabled) */ >> void mmu_init_secondary_cpu(void) >> { >> - /* From now on, no mapping may be both writable and executable. */ >> - WRITE_SYSREG32(READ_SYSREG32(SCTLR_EL2) | SCTLR_WXN, SCTLR_EL2); >> - flush_xen_text_tlb_local(); >> + xen_pt_enforce_wnx(); >> } >> #ifdef CONFIG_ARM_32 >> > > With minor notes, > > Reviewed-by: Andrii Anisov <andrii_anisov@epam.com> Thank you! Cheers, -- Julien Grall
Hello Julien, On 25.04.19 23:26, Julien Grall wrote: > Why can't we let the compiler deciding for us? The more that inline is > pretty broken. See: > > https://www.kernel.org/doc/local/inline.html Ah, ok. So let it be.
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index 01ae2cccc0..93ad118183 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -601,6 +601,19 @@ void __init remove_early_mappings(void) flush_xen_data_tlb_range_va(BOOT_FDT_VIRT_START, BOOT_FDT_SLOT_SIZE); } +/* + * After boot, Xen page-tables should not contain mapping that are both + * Writable and eXecutables. + * + * This should be called on each CPU to enforce the policy. + */ +static void xen_pt_enforce_wnx(void) +{ + WRITE_SYSREG32(READ_SYSREG32(SCTLR_EL2) | SCTLR_WXN, SCTLR_EL2); + /* Flush everything after setting WXN bit. */ + flush_xen_text_tlb_local(); +} + extern void switch_ttbr(uint64_t ttbr); /* Clear a translation table and clean & invalidate the cache */ @@ -702,10 +715,7 @@ void __init setup_pagetables(unsigned long boot_phys_offset) clear_table(boot_second); clear_table(boot_third); - /* From now on, no mapping may be both writable and executable. */ - WRITE_SYSREG32(READ_SYSREG32(SCTLR_EL2) | SCTLR_WXN, SCTLR_EL2); - /* Flush everything after setting WXN bit. */ - flush_xen_text_tlb_local(); + xen_pt_enforce_wnx(); #ifdef CONFIG_ARM_32 per_cpu(xen_pgtable, 0) = cpu0_pgtable; @@ -777,9 +787,7 @@ int init_secondary_pagetables(int cpu) /* MMU setup for secondary CPUS (which already have paging enabled) */ void mmu_init_secondary_cpu(void) { - /* From now on, no mapping may be both writable and executable. */ - WRITE_SYSREG32(READ_SYSREG32(SCTLR_EL2) | SCTLR_WXN, SCTLR_EL2); - flush_xen_text_tlb_local(); + xen_pt_enforce_wnx(); } #ifdef CONFIG_ARM_32
The logic to set SCTLR_EL2.WXN is the same for the boot CPU and non-boot CPU. So introduce a function to set the bit and clear TBLs. This new function will help us to document and update the logic in a single place. Signed-off-by: Julien Grall <julien.grall@arm.com> --- xen/arch/arm/mm.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-)