Re: [PATCH v2] x86-64/Xen: eliminate W+X mappings

[Ingo Molnar <mingo@kernel.org>]

* Jan Beulich <JBeulich@suse.com> wrote:

> A few thousand such pages are usually left around due to the re-use of
> L1 tables having been provided by the hypervisor (Dom0) or tool stack
> (DomU). Set NX in the direct map variant, which needs to be done in L2
> due to the dual use of the re-used L1s.
> 
> For x86_configure_nx() to actually do what it is supposed to do, call
> get_cpu_cap() first. This was broken by commit 4763ed4d45 ("x86, mm:
> Clean up and simplify NX enablement") when switching away from the
> direct EFER read.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> v2: Adjust comment style and indentation.
> ---
> While I certainly dislike the added header inclusion to obtain the
> prototype for get_cpu_cap(), I couldn't find a better alternative. I'm
> open to suggestions.
> ---
>  arch/x86/xen/enlighten_pv.c |    3 +++
>  arch/x86/xen/mmu_pv.c       |   10 ++++++++++
>  2 files changed, 13 insertions(+)
> 
> --- 4.15-rc3/arch/x86/xen/enlighten_pv.c
> +++ 4.15-rc3-x86_64-Xen-avoid-W+X/arch/x86/xen/enlighten_pv.c
> @@ -88,6 +88,8 @@
>  #include "multicalls.h"
>  #include "pmu.h"
>  
> +#include "../kernel/cpu/cpu.h" /* get_cpu_cap() */
> +
>  void *xen_initial_gdt;
>  
>  static int xen_cpu_up_prepare_pv(unsigned int cpu);
> @@ -1258,6 +1260,7 @@ asmlinkage __visible void __init xen_sta
>  	__userpte_alloc_gfp &= ~__GFP_HIGHMEM;
>  
>  	/* Work out if we support NX */
> +	get_cpu_cap(&boot_cpu_data);
>  	x86_configure_nx();
>  
>  	/* Get mfn list */
> --- 4.15-rc4/arch/x86/xen/mmu_pv.c
> +++ 4.15-rc4-x86_64-Xen-avoid-W+X/arch/x86/xen/mmu_pv.c
> @@ -1902,6 +1902,18 @@ void __init xen_setup_kernel_pagetable(p
>  	/* Graft it onto L4[511][510] */
>  	copy_page(level2_kernel_pgt, l2);
>  
> +	/*
> +	 * Zap execute permission from the ident map. Due to the sharing of
> +	 * L1 entries we need to do this in the L2.
> +	 */
> +	if (__supported_pte_mask & _PAGE_NX)
> +		for (i = 0; i < PTRS_PER_PMD; ++i) {
> +			if (pmd_none(level2_ident_pgt[i]))
> +				continue;
> +			level2_ident_pgt[i] = pmd_set_flags(level2_ident_pgt[i],
> +							    _PAGE_NX);
> +		}
> +

This chunk has two stylistic problems:

 - Curly braces need to be added
 - Line broken in an ugly fashion: just make it long and ignore the checkpatch col80 warning

looks good otherwise.

Thanks,

	Ingo