[MODERATED] Re: [PATCH v17.1 2/2] [PATCH v17.1 2/2] SSB Fix #2

[Greg KH <gregkh@linuxfoundation.org>]

On Wed, May 16, 2018 at 11:18:10PM -0400, speck for konrad.wilk_at_oracle.com wrote:
> xen: Don't call non-Xen routines early in the boot
> 
> Commit 2cc42bac1c79 ("x86-64/Xen: eliminate W+X mappings") added a call
> to  get_cpu_cap() to properly set boot_cpu_has(X86_FEATURE_NX) which is
> needed to correctly initialize __supported_pte_mask. Doing this so early
> in the boot can be dangerous --- while this currently works, there is no
> telling what get_cpu_cap() may decide to add in the future. For example,
> if it decides to do a printk (which is not something impossible to
> imagine) a PV guest will not be able to cope with it.
> 
> To avoid these problems check CPUID explicitly for NX support and
> open-code setting __supported_pte_mask.
> 
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Reviewed-by: Juergen Gross <jgross@suse.com>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  arch/x86/xen/enlighten_pv.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
> index c36d23aa6c35..f9f892488670 100644
> --- a/arch/x86/xen/enlighten_pv.c
> +++ b/arch/x86/xen/enlighten_pv.c
> @@ -88,8 +88,6 @@
>  #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);
> @@ -1253,6 +1251,13 @@ asmlinkage __visible void __init xen_start_kernel(void)
>  	/* Prevent unwanted bits from being set in PTEs. */
>  	__supported_pte_mask &= ~_PAGE_GLOBAL;
>  
> +	/* Work out if we support NX. */
> +	if ((cpuid_eax(0x80000000) >= 0x80000001) &&
> +	    (cpuid_edx(0x80000001) & (1U << (X86_FEATURE_NX & 31))))
> +		__supported_pte_mask |= _PAGE_NX;
> +	else
> +		__supported_pte_mask &= ~_PAGE_NX;
> +
>  	/*
>  	 * Prevent page tables from being allocated in highmem, even
>  	 * if CONFIG_HIGHPTE is enabled.
> @@ -1268,10 +1273,6 @@ asmlinkage __visible void __init xen_start_kernel(void)
>  	 */
>  	xen_setup_gdt(0);
>  
> -	/* Work out if we support NX */
> -	get_cpu_cap(&boot_cpu_data);
> -	x86_configure_nx();
> -
>  	xen_init_irq_ops();
>  
>  	/* Let's presume PV guests always boot on vCPU with id 0. */
> -- 
> 2.14.3

Why is this a part of this series?  Shouldn't this just go into the tree
through the "normal" patch process if this really is an issue?  And if
so, please cc: stable as the patch you are trying to "fix" here has been
backported already to stable kernel releases.

I really hate the "open coding" here though, it feels odd that the xen
developers feel that it is needed and they can't trust the fact that
since the kernel works fine today with this, it will continue to work ok
in the future (and if it breaks then, great, we can fix it...)

thanks,

greg k-h