[MODERATED] Re: [PATCH v8 3/5] NX 3

[Tyler Hicks <tyhicks@canonical.com>]

On 2019-10-31 17:24:21, speck for Pawan Gupta wrote:
> On Fri, Nov 01, 2019 at 12:33:45AM +0100, speck for Paolo Bonzini wrote:
> > From: Paolo Bonzini <pbonzini@redhat.com>
> > Subject: [PATCH v8 3/5] kvm: mmu: ITLB_MULTIHIT mitigation
> >  
> > +	kvm.nx_huge_pages=
> > +			[KVM] Controls the sw workaround for bug
> > +			X86_BUG_ITLB_MULTIHIT.
> > +			force	: Always deploy workaround.
> > +			off	: Default. Never deploy workaround.
> 
> off is not the default in the code, so the default should be "auto" here.
> 
> > +			auto	: Deploy workaround based on presence of
> > +				  X86_BUG_ITLB_MULTIHIT.
> 
> Also mitigations=off is not disabling this mitigation. Below patch does
> that when mitigations=off and kvm.nx_huge_pages=auto.
> 
> ---
> From: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> Date: Wed, 30 Oct 2019 21:28:24 -0700
> Subject: [PATCH] kvm: x86: mmu: Apply global mitigations knob to ITLB_MULTIHIT
> 
> Problem: The global mitigation knob mitigations=off does not turn off
> X86_BUG_ITLB_MULTIHIT mitigation.
> 
> Fix: Turn off the mitigation when ITLB_MULTIHIT mitigation mode is
> "auto" and mitigations are turned off globally via cmdline
> mitigations=off.
> 
> Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> ---
>  Documentation/admin-guide/kernel-parameters.txt |  6 ++++++
>  arch/x86/kvm/mmu.c                              | 10 ++++++++--
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index c667844c1c42..422da241a4cb 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2653,6 +2653,12 @@
>  					       ssbd=force-off [ARM64]
>  					       l1tf=off [X86]
>  					       mds=off [X86]

This patch is not written against speck/master (which has the TAA
patches) and there's a very minor merge conflict here.

> +					       kvm.nx_huge_pages=off [X86].
> +
> +				Exceptions:
> +					       This does not have any effect on
> +					       kvm.nx_huge_pages when
> +					       kvm.nx_huge_pages=force.
>  
>  			auto (default)
>  				Mitigate all CPU vulnerabilities, but leave SMT
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index e6a5748a12d5..529589a42afb 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -6250,6 +6250,12 @@ static void kvm_set_mmio_spte_mask(void)
>  	kvm_mmu_set_mmio_spte_mask(mask, mask, ACC_WRITE_MASK | ACC_USER_MASK);
>  }
>  
> +static bool get_nx_auto_mode(void)
> +{
> +	/* Return true when CPU has the bug, and mitigations are ON */
> +	return boot_cpu_has_bug(X86_BUG_ITLB_MULTIHIT) && !cpu_mitigations_off();

The call to cpu_mitigations_off() causes a build failure when kvm is
built as a module (CONFIG_KVM=m):

ERROR: "cpu_mitigations" [arch/x86/kvm/kvm.ko] undefined!
make[2]: *** [__modpost] Error 1
make[1]: *** [modules] Error 2
make[1]: *** Waiting for unfinished jobs....
make: *** [sub-make] Error 2

The problem is due to cpu_mitigations_off() and
cpu_mitigations_auto_nosmt() being inlined in include/linux/cpu.h. Those
functions look to only be used in initialization/setup code so I think
you could fix this easily enough by unlining and exporting them.

Tyler

> +}
> +
>  static void __set_nx_huge_pages(bool val)
>  {
>  	nx_huge_pages = itlb_multihit_kvm_mitigation = val;
> @@ -6266,7 +6272,7 @@ static int set_nx_huge_pages(const char *val, const struct kernel_param *kp)
>  	else if (sysfs_streq(val, "force"))
>  		new_val = 1;
>  	else if (sysfs_streq(val, "auto"))
> -		new_val = boot_cpu_has_bug(X86_BUG_ITLB_MULTIHIT);
> +		new_val = get_nx_auto_mode();
>  	else if (strtobool(val, &new_val) < 0)
>  		return -EINVAL;
>  
> @@ -6296,7 +6302,7 @@ int kvm_mmu_module_init(void)
>  	int ret = -ENOMEM;
>  
>  	if (nx_huge_pages == -1)
> -		__set_nx_huge_pages(boot_cpu_has_bug(X86_BUG_ITLB_MULTIHIT));
> +		__set_nx_huge_pages(get_nx_auto_mode());
>  
>  	/*
>  	 * MMU roles use union aliasing which is, generally speaking, an
> -- 
> 2.20.1