[RFC PATCH] KVM: SVM: Use do_machine_check to pass MCE to the host

[RFC PATCH] KVM: SVM: Use do_machine_check to pass MCE to the host

[Uros Bizjak]

Use do_machine_check instead of INT $12 to pass MCE to the host,
the same approach VMX uses.

On a related note, there is no reason to limit the use of do_machine_check
to 64 bit targets, as is currently done for VMX. MCE handling works
for both target families.

The patch is only compile tested, for both, 64 and 32 bit targets,
someone should test the passing of the exception by injecting
some MCEs into the guest.

For future non-RFC patch, kvm_machine_check should be moved to some
appropriate header file.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Joerg Roedel <joro@8bytes.org>
Cc: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
---
 arch/x86/kvm/svm/svm.c | 26 +++++++++++++++++++++-----
 arch/x86/kvm/vmx/vmx.c |  2 +-
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 061d19e69c73..cd773f6261e3 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -33,6 +33,7 @@
 #include <asm/debugreg.h>
 #include <asm/kvm_para.h>
 #include <asm/irq_remapping.h>
+#include <asm/mce.h>
 #include <asm/spec-ctrl.h>
 #include <asm/cpu_device_id.h>
 
@@ -1839,6 +1840,25 @@ static bool is_erratum_383(void)
 	return true;
 }
 
+/*
+ * Trigger machine check on the host. We assume all the MSRs are already set up
+ * by the CPU and that we still run on the same CPU as the MCE occurred on.
+ * We pass a fake environment to the machine check handler because we want
+ * the guest to be always treated like user space, no matter what context
+ * it used internally.
+ */
+static void kvm_machine_check(void)
+{
+#if defined(CONFIG_X86_MCE)
+	struct pt_regs regs = {
+		.cs = 3, /* Fake ring 3 no matter what the guest ran on */
+		.flags = X86_EFLAGS_IF,
+	};
+
+	do_machine_check(&regs, 0);
+#endif
+}
+
 static void svm_handle_mce(struct vcpu_svm *svm)
 {
 	if (is_erratum_383()) {
@@ -1857,11 +1877,7 @@ static void svm_handle_mce(struct vcpu_svm *svm)
 	 * On an #MC intercept the MCE handler is not called automatically in
 	 * the host. So do it by hand here.
 	 */
-	asm volatile (
-		"int $0x12\n");
-	/* not sure if we ever come back to this point */
-
-	return;
+	kvm_machine_check();
 }
 
 static int mc_interception(struct vcpu_svm *svm)
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 8959514eaf0f..01330096ff3e 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -4572,7 +4572,7 @@ static int handle_rmode_exception(struct kvm_vcpu *vcpu,
  */
 static void kvm_machine_check(void)
 {
-#if defined(CONFIG_X86_MCE) && defined(CONFIG_X86_64)
+#if defined(CONFIG_X86_MCE)
 	struct pt_regs regs = {
 		.cs = 3, /* Fake ring 3 no matter what the guest ran on */
 		.flags = X86_EFLAGS_IF,
-- 
2.25.2

Re: [RFC PATCH] KVM: SVM: Use do_machine_check to pass MCE to the host

[Sean Christopherson]

On Sat, Apr 11, 2020 at 05:36:27PM +0200, Uros Bizjak wrote:
> Use do_machine_check instead of INT $12 to pass MCE to the host,
> the same approach VMX uses.
> 
> On a related note, there is no reason to limit the use of do_machine_check
> to 64 bit targets, as is currently done for VMX. MCE handling works
> for both target families.

...

> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 8959514eaf0f..01330096ff3e 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -4572,7 +4572,7 @@ static int handle_rmode_exception(struct kvm_vcpu *vcpu,
>   */
>  static void kvm_machine_check(void)
>  {
> -#if defined(CONFIG_X86_MCE) && defined(CONFIG_X86_64)
> +#if defined(CONFIG_X86_MCE)

This VMX change needs to be a separate patch, and it arguably warrants:

  Cc: stable@vger.kernel.org
  Fixes: a0861c02a981 ("KVM: Add VT-x machine check support")

>  	struct pt_regs regs = {
>  		.cs = 3, /* Fake ring 3 no matter what the guest ran on */
>  		.flags = X86_EFLAGS_IF,
> -- 
> 2.25.2
> 

Re: [RFC PATCH] KVM: SVM: Use do_machine_check to pass MCE to the host

[Paolo Bonzini]

On 11/04/20 17:36, Uros Bizjak wrote:
> Use do_machine_check instead of INT $12 to pass MCE to the host,
> the same approach VMX uses.
> 
> On a related note, there is no reason to limit the use of do_machine_check
> to 64 bit targets, as is currently done for VMX. MCE handling works
> for both target families.
> 
> The patch is only compile tested, for both, 64 and 32 bit targets,
> someone should test the passing of the exception by injecting
> some MCEs into the guest.
> 
> For future non-RFC patch, kvm_machine_check should be moved to some
> appropriate header file.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Joerg Roedel <joro@8bytes.org>
> Cc: Sean Christopherson <sean.j.christopherson@intel.com>
> Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
> ---
>  arch/x86/kvm/svm/svm.c | 26 +++++++++++++++++++++-----
>  arch/x86/kvm/vmx/vmx.c |  2 +-
>  2 files changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 061d19e69c73..cd773f6261e3 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -33,6 +33,7 @@
>  #include <asm/debugreg.h>
>  #include <asm/kvm_para.h>
>  #include <asm/irq_remapping.h>
> +#include <asm/mce.h>
>  #include <asm/spec-ctrl.h>
>  #include <asm/cpu_device_id.h>
>  
> @@ -1839,6 +1840,25 @@ static bool is_erratum_383(void)
>  	return true;
>  }
>  
> +/*
> + * Trigger machine check on the host. We assume all the MSRs are already set up
> + * by the CPU and that we still run on the same CPU as the MCE occurred on.
> + * We pass a fake environment to the machine check handler because we want
> + * the guest to be always treated like user space, no matter what context
> + * it used internally.
> + */
> +static void kvm_machine_check(void)
> +{
> +#if defined(CONFIG_X86_MCE)
> +	struct pt_regs regs = {
> +		.cs = 3, /* Fake ring 3 no matter what the guest ran on */
> +		.flags = X86_EFLAGS_IF,
> +	};
> +
> +	do_machine_check(&regs, 0);
> +#endif
> +}
> +
>  static void svm_handle_mce(struct vcpu_svm *svm)
>  {
>  	if (is_erratum_383()) {
> @@ -1857,11 +1877,7 @@ static void svm_handle_mce(struct vcpu_svm *svm)
>  	 * On an #MC intercept the MCE handler is not called automatically in
>  	 * the host. So do it by hand here.
>  	 */
> -	asm volatile (
> -		"int $0x12\n");
> -	/* not sure if we ever come back to this point */
> -
> -	return;
> +	kvm_machine_check();
>  }
>  
>  static int mc_interception(struct vcpu_svm *svm)

Looks good, but please move kvm_machine_check() to x86.c instead.

Paolo

Re: [RFC PATCH] KVM: SVM: Use do_machine_check to pass MCE to the host

[Uros Bizjak]

On Tue, Apr 14, 2020 at 10:28 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 11/04/20 17:36, Uros Bizjak wrote:
> > Use do_machine_check instead of INT $12 to pass MCE to the host,
> > the same approach VMX uses.
> >
> > On a related note, there is no reason to limit the use of do_machine_check
> > to 64 bit targets, as is currently done for VMX. MCE handling works
> > for both target families.
> >
> > The patch is only compile tested, for both, 64 and 32 bit targets,
> > someone should test the passing of the exception by injecting
> > some MCEs into the guest.
> >
> > For future non-RFC patch, kvm_machine_check should be moved to some
> > appropriate header file.
> >
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Joerg Roedel <joro@8bytes.org>
> > Cc: Sean Christopherson <sean.j.christopherson@intel.com>
> > Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
> > ---
> >  arch/x86/kvm/svm/svm.c | 26 +++++++++++++++++++++-----
> >  arch/x86/kvm/vmx/vmx.c |  2 +-
> >  2 files changed, 22 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> > index 061d19e69c73..cd773f6261e3 100644
> > --- a/arch/x86/kvm/svm/svm.c
> > +++ b/arch/x86/kvm/svm/svm.c
> > @@ -33,6 +33,7 @@
> >  #include <asm/debugreg.h>
> >  #include <asm/kvm_para.h>
> >  #include <asm/irq_remapping.h>
> > +#include <asm/mce.h>
> >  #include <asm/spec-ctrl.h>
> >  #include <asm/cpu_device_id.h>
> >
> > @@ -1839,6 +1840,25 @@ static bool is_erratum_383(void)
> >       return true;
> >  }
> >
> > +/*
> > + * Trigger machine check on the host. We assume all the MSRs are already set up
> > + * by the CPU and that we still run on the same CPU as the MCE occurred on.
> > + * We pass a fake environment to the machine check handler because we want
> > + * the guest to be always treated like user space, no matter what context
> > + * it used internally.
> > + */
> > +static void kvm_machine_check(void)
> > +{
> > +#if defined(CONFIG_X86_MCE)
> > +     struct pt_regs regs = {
> > +             .cs = 3, /* Fake ring 3 no matter what the guest ran on */
> > +             .flags = X86_EFLAGS_IF,
> > +     };
> > +
> > +     do_machine_check(&regs, 0);
> > +#endif
> > +}
> > +
> >  static void svm_handle_mce(struct vcpu_svm *svm)
> >  {
> >       if (is_erratum_383()) {
> > @@ -1857,11 +1877,7 @@ static void svm_handle_mce(struct vcpu_svm *svm)
> >        * On an #MC intercept the MCE handler is not called automatically in
> >        * the host. So do it by hand here.
> >        */
> > -     asm volatile (
> > -             "int $0x12\n");
> > -     /* not sure if we ever come back to this point */
> > -
> > -     return;
> > +     kvm_machine_check();
> >  }
> >
> >  static int mc_interception(struct vcpu_svm *svm)
>
> Looks good, but please move kvm_machine_check() to x86.c instead.

Will do, after the confirmation that the patch works for AMD hosts.

OTOH, the function is just a simple wrapper around do_machine_check,
so I was thinking to move it to a kvm_host.h header as a static
inline. This way, we could save a call to a wrapper function.

Uros.

Re: [RFC PATCH] KVM: SVM: Use do_machine_check to pass MCE to the host

[Paolo Bonzini]

On 14/04/20 10:38, Uros Bizjak wrote:
> Will do, after the confirmation that the patch works for AMD hosts.

Ok, I will test it (I found an AMD machine with EINJ support).  In the
meanwhile I queued the VMX part.

> OTOH, the function is just a simple wrapper around do_machine_check,
> so I was thinking to move it to a kvm_host.h header as a static
> inline. This way, we could save a call to a wrapper function.

Yes, that works too.

Paolo