[PATCH] x86/bugs/multihit: Fix mitigation reporting when KVM is not in use

[PATCH] x86/bugs/multihit: Fix mitigation reporting when KVM is not in use

[Pawan Gupta]

On systems that have virtualization disabled or KVM module is not
loaded, sysfs mitigation state of X86_BUG_ITLB_MULTIHIT is reported
incorrectly as:

  $ cat /sys/devices/system/cpu/vulnerabilities/itlb_multihit
  KVM: Vulnerable

System is not vulnerable to DoS attack from a rogue guest when:
 - KVM module is not loaded or
 - Virtualization is disabled in the hardware or
 - Kernel was configured without support for KVM

Change the reporting to "Currently not affected (KVM not in use)" for
such cases.

Reported-by: Nelson Dsouza <nelson.dsouza@linux.intel.com>
Fixes: b8e8c8303ff2 ("kvm: mmu: ITLB_MULTIHIT mitigation")
Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
---
 .../admin-guide/hw-vuln/multihit.rst          |  5 +++-
 arch/x86/include/asm/processor.h              |  6 +++++
 arch/x86/kernel/cpu/bugs.c                    | 24 +++++++++----------
 arch/x86/kvm/mmu/mmu.c                        |  9 +++++--
 4 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/Documentation/admin-guide/hw-vuln/multihit.rst b/Documentation/admin-guide/hw-vuln/multihit.rst
index ba9988d8bce5..842961419f3e 100644
--- a/Documentation/admin-guide/hw-vuln/multihit.rst
+++ b/Documentation/admin-guide/hw-vuln/multihit.rst
@@ -82,7 +82,10 @@ The possible values in this file are:
        - Software changes mitigate this issue.
      * - KVM: Vulnerable
        - The processor is vulnerable, but no mitigation enabled
-
+     * - Currently not affected (KVM not in use)
+       - The processor is vulnerable but no mitigation is required because
+         KVM module is not loaded or virtualization is disabled in the hardware or
+         kernel was configured without support for KVM.
 
 Enumeration of the erratum
 --------------------------------
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 03b7c4ca425a..830a3e7725af 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -989,4 +989,10 @@ enum mds_mitigations {
 	MDS_MITIGATION_VMWERV,
 };
 
+enum itlb_multihit_mitigations {
+	ITLB_MULTIHIT_MITIGATION_OFF,
+	ITLB_MULTIHIT_MITIGATION_FULL,
+	ITLB_MULTIHIT_MITIGATION_NO_KVM,
+};
+
 #endif /* _ASM_X86_PROCESSOR_H */
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 0b71970d2d3d..97f66a93f2be 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -1395,8 +1395,15 @@ void x86_spec_ctrl_setup_ap(void)
 		x86_amd_ssb_disable();
 }
 
-bool itlb_multihit_kvm_mitigation;
-EXPORT_SYMBOL_GPL(itlb_multihit_kvm_mitigation);
+/* Default to KVM not in use, KVM module changes this later */
+enum itlb_multihit_mitigations itlb_multihit_mitigation = ITLB_MULTIHIT_MITIGATION_NO_KVM;
+EXPORT_SYMBOL_GPL(itlb_multihit_mitigation);
+
+static const char * const itlb_multihit_strings[] = {
+	[ITLB_MULTIHIT_MITIGATION_OFF]		= "KVM: Vulnerable",
+	[ITLB_MULTIHIT_MITIGATION_FULL]		= "KVM: Mitigation: Split huge pages",
+	[ITLB_MULTIHIT_MITIGATION_NO_KVM]	= "Currently not affected (KVM not in use)",
+};
 
 #undef pr_fmt
 #define pr_fmt(fmt)	"L1TF: " fmt
@@ -1553,25 +1560,18 @@ static ssize_t l1tf_show_state(char *buf)
 		       l1tf_vmx_states[l1tf_vmx_mitigation],
 		       sched_smt_active() ? "vulnerable" : "disabled");
 }
-
-static ssize_t itlb_multihit_show_state(char *buf)
-{
-	if (itlb_multihit_kvm_mitigation)
-		return sprintf(buf, "KVM: Mitigation: Split huge pages\n");
-	else
-		return sprintf(buf, "KVM: Vulnerable\n");
-}
 #else
 static ssize_t l1tf_show_state(char *buf)
 {
 	return sprintf(buf, "%s\n", L1TF_DEFAULT_MSG);
 }
+#endif
 
 static ssize_t itlb_multihit_show_state(char *buf)
 {
-	return sprintf(buf, "Processor vulnerable\n");
+	return sprintf(buf, "%s\n",
+		       itlb_multihit_strings[itlb_multihit_mitigation]);
 }
-#endif
 
 static ssize_t mds_show_state(char *buf)
 {
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 6d6a0ae7800c..e089b9e565a5 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -50,7 +50,7 @@
 #include <asm/kvm_page_track.h>
 #include "trace.h"
 
-extern bool itlb_multihit_kvm_mitigation;
+extern enum itlb_multihit_mitigations itlb_multihit_mitigation;
 
 static int __read_mostly nx_huge_pages = -1;
 #ifdef CONFIG_PREEMPT_RT
@@ -6158,7 +6158,12 @@ static bool get_nx_auto_mode(void)
 
 static void __set_nx_huge_pages(bool val)
 {
-	nx_huge_pages = itlb_multihit_kvm_mitigation = val;
+	nx_huge_pages = val;
+
+	if (val)
+		itlb_multihit_mitigation = ITLB_MULTIHIT_MITIGATION_FULL;
+	else
+		itlb_multihit_mitigation = ITLB_MULTIHIT_MITIGATION_OFF;
 }
 
 static int set_nx_huge_pages(const char *val, const struct kernel_param *kp)
-- 
2.21.3

Re: [PATCH] x86/bugs/multihit: Fix mitigation reporting when KVM is not in use

[Sean Christopherson]

On Mon, Jul 13, 2020 at 06:18:54PM -0700, Pawan Gupta wrote:
> On systems that have virtualization disabled or KVM module is not
> loaded, sysfs mitigation state of X86_BUG_ITLB_MULTIHIT is reported
> incorrectly as:
> 
>   $ cat /sys/devices/system/cpu/vulnerabilities/itlb_multihit
>   KVM: Vulnerable
> 
> System is not vulnerable to DoS attack from a rogue guest when:
>  - KVM module is not loaded or
>  - Virtualization is disabled in the hardware or
>  - Kernel was configured without support for KVM
> 
> Change the reporting to "Currently not affected (KVM not in use)" for
> such cases.

This is all kinds of backwards.  Virtualization being disabled in hardware
is very, very different than KVM not being loaded.  One requires at the
very least a kernel reboot to change, the other does not.

And just because the kernel isn't configured for KVM doesn't mean VMX can't
be used, there are plenty of out-of-tree hypervisors that utilize VMX.

Ignoring the above issues, choosing KVM module load as the line in the sand
where the kernel suddenly becomes vulnerable is arbitrary.  Arguably, KVM
isn't vulnerable until it actually starts running a guest.

IMO, the sane/safe route would be to print e.g. "VMX not supported" when
VMX isn't supported or is disabled via FEAT_CTL.  And then if you want to
reflect current state, add another condition that checks hardware CR4.VMXE
and prints e.g. "VMX currently disabled".  The latter case still seems
somewhat dubious, but it's a lot better than keying off KVM being loaded.

> Reported-by: Nelson Dsouza <nelson.dsouza@linux.intel.com>
> Fixes: b8e8c8303ff2 ("kvm: mmu: ITLB_MULTIHIT mitigation")
> Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> Reviewed-by: Tony Luck <tony.luck@intel.com>
> ---
>  .../admin-guide/hw-vuln/multihit.rst          |  5 +++-
>  arch/x86/include/asm/processor.h              |  6 +++++
>  arch/x86/kernel/cpu/bugs.c                    | 24 +++++++++----------
>  arch/x86/kvm/mmu/mmu.c                        |  9 +++++--
>  4 files changed, 29 insertions(+), 15 deletions(-)
> 
> diff --git a/Documentation/admin-guide/hw-vuln/multihit.rst b/Documentation/admin-guide/hw-vuln/multihit.rst
> index ba9988d8bce5..842961419f3e 100644
> --- a/Documentation/admin-guide/hw-vuln/multihit.rst
> +++ b/Documentation/admin-guide/hw-vuln/multihit.rst
> @@ -82,7 +82,10 @@ The possible values in this file are:
>         - Software changes mitigate this issue.
>       * - KVM: Vulnerable
>         - The processor is vulnerable, but no mitigation enabled
> -
> +     * - Currently not affected (KVM not in use)
> +       - The processor is vulnerable but no mitigation is required because
> +         KVM module is not loaded or virtualization is disabled in the hardware or
> +         kernel was configured without support for KVM.
>  
>  Enumeration of the erratum
>  --------------------------------
> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> index 03b7c4ca425a..830a3e7725af 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -989,4 +989,10 @@ enum mds_mitigations {
>  	MDS_MITIGATION_VMWERV,
>  };
>  
> +enum itlb_multihit_mitigations {
> +	ITLB_MULTIHIT_MITIGATION_OFF,
> +	ITLB_MULTIHIT_MITIGATION_FULL,
> +	ITLB_MULTIHIT_MITIGATION_NO_KVM,
> +};
> +
>  #endif /* _ASM_X86_PROCESSOR_H */
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index 0b71970d2d3d..97f66a93f2be 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -1395,8 +1395,15 @@ void x86_spec_ctrl_setup_ap(void)
>  		x86_amd_ssb_disable();
>  }
>  
> -bool itlb_multihit_kvm_mitigation;
> -EXPORT_SYMBOL_GPL(itlb_multihit_kvm_mitigation);
> +/* Default to KVM not in use, KVM module changes this later */
> +enum itlb_multihit_mitigations itlb_multihit_mitigation = ITLB_MULTIHIT_MITIGATION_NO_KVM;
> +EXPORT_SYMBOL_GPL(itlb_multihit_mitigation);
> +
> +static const char * const itlb_multihit_strings[] = {
> +	[ITLB_MULTIHIT_MITIGATION_OFF]		= "KVM: Vulnerable",
> +	[ITLB_MULTIHIT_MITIGATION_FULL]		= "KVM: Mitigation: Split huge pages",
> +	[ITLB_MULTIHIT_MITIGATION_NO_KVM]	= "Currently not affected (KVM not in use)",
> +};
>  
>  #undef pr_fmt
>  #define pr_fmt(fmt)	"L1TF: " fmt
> @@ -1553,25 +1560,18 @@ static ssize_t l1tf_show_state(char *buf)
>  		       l1tf_vmx_states[l1tf_vmx_mitigation],
>  		       sched_smt_active() ? "vulnerable" : "disabled");
>  }
> -
> -static ssize_t itlb_multihit_show_state(char *buf)
> -{
> -	if (itlb_multihit_kvm_mitigation)
> -		return sprintf(buf, "KVM: Mitigation: Split huge pages\n");
> -	else
> -		return sprintf(buf, "KVM: Vulnerable\n");
> -}
>  #else
>  static ssize_t l1tf_show_state(char *buf)
>  {
>  	return sprintf(buf, "%s\n", L1TF_DEFAULT_MSG);
>  }
> +#endif
>  
>  static ssize_t itlb_multihit_show_state(char *buf)
>  {
> -	return sprintf(buf, "Processor vulnerable\n");
> +	return sprintf(buf, "%s\n",
> +		       itlb_multihit_strings[itlb_multihit_mitigation]);
>  }
> -#endif
>  
>  static ssize_t mds_show_state(char *buf)
>  {
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 6d6a0ae7800c..e089b9e565a5 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -50,7 +50,7 @@
>  #include <asm/kvm_page_track.h>
>  #include "trace.h"
>  
> -extern bool itlb_multihit_kvm_mitigation;
> +extern enum itlb_multihit_mitigations itlb_multihit_mitigation;
>  
>  static int __read_mostly nx_huge_pages = -1;
>  #ifdef CONFIG_PREEMPT_RT
> @@ -6158,7 +6158,12 @@ static bool get_nx_auto_mode(void)
>  
>  static void __set_nx_huge_pages(bool val)
>  {
> -	nx_huge_pages = itlb_multihit_kvm_mitigation = val;
> +	nx_huge_pages = val;
> +
> +	if (val)
> +		itlb_multihit_mitigation = ITLB_MULTIHIT_MITIGATION_FULL;
> +	else
> +		itlb_multihit_mitigation = ITLB_MULTIHIT_MITIGATION_OFF;
>  }
>  
>  static int set_nx_huge_pages(const char *val, const struct kernel_param *kp)
> -- 
> 2.21.3
> 

Re: [PATCH] x86/bugs/multihit: Fix mitigation reporting when KVM is not in use

[Dave Hansen]

On 7/13/20 6:45 PM, Sean Christopherson wrote:
> This is all kinds of backwards.  Virtualization being disabled in hardware
> is very, very different than KVM not being loaded.  One requires at the
> very least a kernel reboot to change, the other does not.

That's a very good point.

It's a pretty slippery slope if we go trying to figure out at runtime
what our vulnerabilities are.  We could, for instance, claim that we're
not vulnerable to Meltdown until we run non-root code, or something else
equally silly.

Let's stick to things which are at least static per reboot.  Checking
for X86_FEATURE_VMX or even CONFIG_KVM_INTEL seems like a good stopping
point.  "Could this kernel run a naughty guest?"  If so, report
"Vulnerable".  It's the same as Meltdown: "Could this kernel run
untrusted code?"  If so, report "Vulnerable".

I don't think we should care about random kernel modules flipping CR4
bits.  They can do much more harm than expose a system to an issue like
this.  If we care about reporting mitigation status once that happens,
maybe out-of-tree modules loads should just flip *all* these cpu bugs
from Mitigated->Vulerable. :)

Re: [PATCH] x86/bugs/multihit: Fix mitigation reporting when KVM is not in use

[Pawan Gupta]

On Tue, Jul 14, 2020 at 07:57:53AM -0700, Dave Hansen wrote:
> Let's stick to things which are at least static per reboot.  Checking
> for X86_FEATURE_VMX or even CONFIG_KVM_INTEL seems like a good stopping
> point.  "Could this kernel run a naughty guest?"  If so, report
> "Vulnerable".  It's the same as Meltdown: "Could this kernel run
> untrusted code?"  If so, report "Vulnerable".

Thanks, These are good inputs. So what I need to add is a boot time
check for VMX feature and report "Vulnerable" or "Not
affected(VMX disabled)".

Are you suggesting to not change the reporting when KVM deploys the
"Split huge pages" mitigation? Is this because VMX can still be used by
other VMMs?

The current mitigation reporting is very specific to KVM:

	- "KVM: Vulnerable"
	- "KVM: Mitigation: Split huge pages"

As the kernel doesn't know about the mitigation state of out-of-tree
VMMs can we add VMX reporting to always say vulnerable when VMX is
enabled:

	- "VMX: Vulnerable, KVM: Vulnerable"
	- "VMX: Vulnerable, KVM: Mitigation: Split huge pages"

And if VMX is disabled report:

	- "VMX: Not affected(VMX disabled)"

or something like that.

Thanks,
Pawan

Re: [PATCH] x86/bugs/multihit: Fix mitigation reporting when KVM is not in use

[Dave Hansen]

On 7/14/20 12:17 PM, Pawan Gupta wrote:
> On Tue, Jul 14, 2020 at 07:57:53AM -0700, Dave Hansen wrote:
>> Let's stick to things which are at least static per reboot.  Checking
>> for X86_FEATURE_VMX or even CONFIG_KVM_INTEL seems like a good stopping
>> point.  "Could this kernel run a naughty guest?"  If so, report
>> "Vulnerable".  It's the same as Meltdown: "Could this kernel run
>> untrusted code?"  If so, report "Vulnerable".
> 
> Thanks, These are good inputs. So what I need to add is a boot time
> check for VMX feature and report "Vulnerable" or "Not
> affected(VMX disabled)".
> 
> Are you suggesting to not change the reporting when KVM deploys the
> "Split huge pages" mitigation? Is this because VMX can still be used by
> other VMMs?
> 
> The current mitigation reporting is very specific to KVM:
> 
> 	- "KVM: Vulnerable"
> 	- "KVM: Mitigation: Split huge pages"
> 
> As the kernel doesn't know about the mitigation state of out-of-tree
> VMMs can we add VMX reporting to always say vulnerable when VMX is
> enabled:
> 
> 	- "VMX: Vulnerable, KVM: Vulnerable"
> 	- "VMX: Vulnerable, KVM: Mitigation: Split huge pages"
> 
> And if VMX is disabled report:
> 
> 	- "VMX: Not affected(VMX disabled)"

I see three inputs and four possible states (sorry for the ugly table,
it was this or a spreadsheet :):

X86_FEATURE_VMX	CONFIG_KVM_*	hpage split  Result	   Reason
	N		x	    x	     Not Affected  No VMX
	Y		N	    x	     Not affected  No KVM
	Y		Y	    Y	     Mitigated	   hpage split
	Y		Y	    N	     Vulnerable

I don't think we should worry about out-of-tree VMX.

Re: [PATCH] x86/bugs/multihit: Fix mitigation reporting when KVM is not in use

[Pawan Gupta]

On Tue, Jul 14, 2020 at 12:54:26PM -0700, Dave Hansen wrote:
> On 7/14/20 12:17 PM, Pawan Gupta wrote:
> > On Tue, Jul 14, 2020 at 07:57:53AM -0700, Dave Hansen wrote:
> >> Let's stick to things which are at least static per reboot.  Checking
> >> for X86_FEATURE_VMX or even CONFIG_KVM_INTEL seems like a good stopping
> >> point.  "Could this kernel run a naughty guest?"  If so, report
> >> "Vulnerable".  It's the same as Meltdown: "Could this kernel run
> >> untrusted code?"  If so, report "Vulnerable".
> > 
> > Thanks, These are good inputs. So what I need to add is a boot time
> > check for VMX feature and report "Vulnerable" or "Not
> > affected(VMX disabled)".
> > 
> > Are you suggesting to not change the reporting when KVM deploys the
> > "Split huge pages" mitigation? Is this because VMX can still be used by
> > other VMMs?
> > 
> > The current mitigation reporting is very specific to KVM:
> > 
> > 	- "KVM: Vulnerable"
> > 	- "KVM: Mitigation: Split huge pages"
> > 
> > As the kernel doesn't know about the mitigation state of out-of-tree
> > VMMs can we add VMX reporting to always say vulnerable when VMX is
> > enabled:
> > 
> > 	- "VMX: Vulnerable, KVM: Vulnerable"
> > 	- "VMX: Vulnerable, KVM: Mitigation: Split huge pages"
> > 
> > And if VMX is disabled report:
> > 
> > 	- "VMX: Not affected(VMX disabled)"
> 
> I see three inputs and four possible states (sorry for the ugly table,
> it was this or a spreadsheet :):
> 
> X86_FEATURE_VMX	CONFIG_KVM_*	hpage split  Result	   Reason
> 	N			x	    x	     Not Affected  No VMX
> 	Y			N	    x	     Not affected  No KVM
> 	Y			Y	    Y	     Mitigated	   hpage split
> 	Y			Y	    N	     Vulnerable

Thank you.

Just a note... for the last 2 cases kernel wont know about "hpage split"
mitigation until KVM is loaded. So for these cases reporting at boot
will be "Vulnerable" and would change to "Mitigated" once KVM is loaded
and deploys the mitigation. This is the current behavior.

Thanks,
Pawan

Re: [PATCH] x86/bugs/multihit: Fix mitigation reporting when KVM is not in use

[Dave Hansen]

On 7/14/20 2:04 PM, Pawan Gupta wrote:
>> I see three inputs and four possible states (sorry for the ugly table,
>> it was this or a spreadsheet :):
>>
>> X86_FEATURE_VMX	CONFIG_KVM_*	hpage split  Result	   Reason
>> 	N			x	    x	     Not Affected  No VMX
>> 	Y			N	    x	     Not affected  No KVM
>> 	Y			Y	    Y	     Mitigated	   hpage split
>> 	Y			Y	    N	     Vulnerable
> Thank you.
> 
> Just a note... for the last 2 cases kernel wont know about "hpage split"
> mitigation until KVM is loaded. So for these cases reporting at boot
> will be "Vulnerable" and would change to "Mitigated" once KVM is loaded
> and deploys the mitigation. This is the current behavior.

That's OK with me, because it's actually pretty closely tied to reality.
 You are literally "vulnerable" until you've committed to a mitigation
and that doesn't happen until KVM is loaded.

When are we going to force kvm to be built in, again? ;)

Re: [PATCH] x86/bugs/multihit: Fix mitigation reporting when KVM is not in use

[Sean Christopherson]

On Tue, Jul 14, 2020 at 02:20:59PM -0700, Dave Hansen wrote:
> On 7/14/20 2:04 PM, Pawan Gupta wrote:
> >> I see three inputs and four possible states (sorry for the ugly table,
> >> it was this or a spreadsheet :):
> >>
> >> X86_FEATURE_VMX	CONFIG_KVM_*	hpage split  Result	   Reason
> >> 	N			x	    x	     Not Affected  No VMX
> >> 	Y			N	    x	     Not affected  No KVM

This line item is pointless, the relevant itlb_multihit_show_state()
implementation depends on CONFIG_KVM_INTEL.  The !KVM_INTEL version simply
prints ""Processor vulnerable".

> >> 	Y			Y	    Y	     Mitigated	   hpage split
> >> 	Y			Y	    N	     Vulnerable
> > Thank you.
> > 
> > Just a note... for the last 2 cases kernel wont know about "hpage split"
> > mitigation until KVM is loaded. So for these cases reporting at boot
> > will be "Vulnerable" and would change to "Mitigated" once KVM is loaded
> > and deploys the mitigation. This is the current behavior.
> 
> That's OK with me, because it's actually pretty closely tied to reality.
>  You are literally "vulnerable" until you've committed to a mitigation
> and that doesn't happen until KVM is loaded.

Not that it really matters since itlb_multihit_show_state() reflects current
state, but loading KVM doesn't commit to a mitigation.  KVM's nx_huge_pages
module param is writable by root, e.g. the mitigation can be turned off
while KVM is loaded and even while guests are running.

To do the above table, KVM will also need to update itlb_multihit_kvm_mitigation
when it is unloaded, which seems rather silly.  That's partly why I suggested
keying off CR4.VMXE as it doesn't require poking directly into KVM.  E.g. the
entire fix becomes:

diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index ed54b3b21c39..4452df7f332d 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -1447,7 +1447,12 @@ static ssize_t l1tf_show_state(char *buf)

 static ssize_t itlb_multihit_show_state(char *buf)
 {
-       if (itlb_multihit_kvm_mitigation)
+       if (!boot_cpu_has(X86_FEATURE_MSR_IA32_FEAT_CTL) ||
+           !boot_cpu_has(X86_FEATURE_VMX))
+               return sprintf(buf, "KVM: Mitigation: VMX unsupported\n");
+       else if (!(cr4_read_shadow() & X86_CR4_VMXE))
+               return sprintf(buf, "KVM: Mitigation: VMX disabled\n");
+       else if (itlb_multihit_kvm_mitigation)
                return sprintf(buf, "KVM: Mitigation: Split huge pages\n");
        else
                return sprintf(buf, "KVM: Vulnerable\n");

Re: [PATCH] x86/bugs/multihit: Fix mitigation reporting when KVM is not in use

[Dave Hansen]

On 7/14/20 5:51 PM, Sean Christopherson wrote:
> To do the above table, KVM will also need to update itlb_multihit_kvm_mitigation
> when it is unloaded, which seems rather silly.  That's partly why I suggested
> keying off CR4.VMXE as it doesn't require poking directly into KVM.  E.g. the
> entire fix becomes:

Failing to update itlb_multihit_kvm_mitigation leaves us with something
that's asymmetric.  A system with a never-loaded kvm module will say
something different than one that was loaded and then unloaded.

That's funky, but not the end of the world I guess.

> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index ed54b3b21c39..4452df7f332d 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -1447,7 +1447,12 @@ static ssize_t l1tf_show_state(char *buf)
> 
>  static ssize_t itlb_multihit_show_state(char *buf)
>  {
> -       if (itlb_multihit_kvm_mitigation)
> +       if (!boot_cpu_has(X86_FEATURE_MSR_IA32_FEAT_CTL) ||
> +           !boot_cpu_has(X86_FEATURE_VMX))
> +               return sprintf(buf, "KVM: Mitigation: VMX unsupported\n");
> +       else if (!(cr4_read_shadow() & X86_CR4_VMXE))
> +               return sprintf(buf, "KVM: Mitigation: VMX disabled\n");
> +       else if (itlb_multihit_kvm_mitigation)
>                 return sprintf(buf, "KVM: Mitigation: Split huge pages\n");
>         else
>                 return sprintf(buf, "KVM: Vulnerable\n");

That's at least short and sweet.  I wouldn't object to that at all.

Re: [PATCH] x86/bugs/multihit: Fix mitigation reporting when KVM is not in use

[Pawan Gupta]

On Tue, Jul 14, 2020 at 05:51:30PM -0700, Sean Christopherson wrote:
> On Tue, Jul 14, 2020 at 02:20:59PM -0700, Dave Hansen wrote:
> > On 7/14/20 2:04 PM, Pawan Gupta wrote:
> > >> I see three inputs and four possible states (sorry for the ugly table,
> > >> it was this or a spreadsheet :):
> > >>
> > >> X86_FEATURE_VMX	CONFIG_KVM_*	hpage split  Result	   Reason
> > >> 	N			x	    x	     Not Affected  No VMX
> > >> 	Y			N	    x	     Not affected  No KVM
> 
> This line item is pointless, the relevant itlb_multihit_show_state()
> implementation depends on CONFIG_KVM_INTEL.  The !KVM_INTEL version simply
> prints ""Processor vulnerable".

While we are on it, for CONFIG_KVM_INTEL=n would it make sense to report "Not
affected(No KVM)"? "Processor vulnerable" is not telling much about the
mitigation.

Thanks,
Pawan

Re: [PATCH] x86/bugs/multihit: Fix mitigation reporting when KVM is not in use

[Sean Christopherson]

On Wed, Jul 15, 2020 at 10:18:20AM -0700, Pawan Gupta wrote:
> On Tue, Jul 14, 2020 at 05:51:30PM -0700, Sean Christopherson wrote:
> > On Tue, Jul 14, 2020 at 02:20:59PM -0700, Dave Hansen wrote:
> > > On 7/14/20 2:04 PM, Pawan Gupta wrote:
> > > >> I see three inputs and four possible states (sorry for the ugly table,
> > > >> it was this or a spreadsheet :):
> > > >>
> > > >> X86_FEATURE_VMX	CONFIG_KVM_*	hpage split  Result	   Reason
> > > >> 	N			x	    x	     Not Affected  No VMX
> > > >> 	Y			N	    x	     Not affected  No KVM
> > 
> > This line item is pointless, the relevant itlb_multihit_show_state()
> > implementation depends on CONFIG_KVM_INTEL.  The !KVM_INTEL version simply
> > prints ""Processor vulnerable".
> 
> While we are on it, for CONFIG_KVM_INTEL=n would it make sense to report "Not
> affected(No KVM)"? "Processor vulnerable" is not telling much about the
> mitigation.

I know we don't care too much about out-of-tree hypervisors, but IMO stating
"Not affected" is unnecessarily hostile and "Processor vulnerable" is an
accurate statement.