[PATCH] KVM: VMX: Preserve host CR4.MCE value while in guest mode.

[PATCH] KVM: VMX: Preserve host CR4.MCE value while in guest mode.

[Ben Serebrin]

The host's decision to enable machine check exceptions should remain
in force during non-root mode.  KVM was writing 0 to cr4 on VCPU reset
and passed a slightly-modified 0 to the vmcs.guest_cr4 value.

Tested: Built.
On earlier version, tested by injecting machine check
while a guest is spinning.

Before the change, if guest CR4.MCE==0, then the machine check is
escalated to Catastrophic Error (CATERR) and the machine dies.
If guest CR4.MCE==1, then the machine check causes VMEXIT and is
handled normally by host Linux. After the change, injecting a machine
check causes normal Linux machine check handling.

Signed-off-by: Ben Serebrin <serebrin@google.com>
---
 arch/x86/kvm/vmx.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index f5e8dce..f7b6168 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -3622,8 +3622,16 @@ static void vmx_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3)
 
 static int vmx_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
 {
-	unsigned long hw_cr4 = cr4 | (to_vmx(vcpu)->rmode.vm86_active ?
-		    KVM_RMODE_VM_CR4_ALWAYS_ON : KVM_PMODE_VM_CR4_ALWAYS_ON);
+	/*
+	 * Pass through host's Machine Check Enable value to hw_cr4, which
+	 * is in force while we are in guest mode.  Do not let guests control
+	 * this bit, even if host CR4.MCE == 0.
+	 */
+	unsigned long hw_cr4 =
+		(cr4_read_shadow() & X86_CR4_MCE) |
+		(cr4 & ~X86_CR4_MCE) |
+		(to_vmx(vcpu)->rmode.vm86_active ?
+		 KVM_RMODE_VM_CR4_ALWAYS_ON : KVM_PMODE_VM_CR4_ALWAYS_ON);
 
 	if (cr4 & X86_CR4_VMXE) {
 		/*
-- 
2.2.0.rc0.207.ga3a616c

Re: [PATCH] KVM: VMX: Preserve host CR4.MCE value while in guest mode.

[Venkatesh Srinivas]

On Thu, Apr 16, 2015 at 11:58 AM, Ben Serebrin <serebrin@google.com> wrote:
> The host's decision to enable machine check exceptions should remain
> in force during non-root mode.  KVM was writing 0 to cr4 on VCPU reset
> and passed a slightly-modified 0 to the vmcs.guest_cr4 value.
>
> Tested: Built.
> On earlier version, tested by injecting machine check
> while a guest is spinning.
>
> Before the change, if guest CR4.MCE==0, then the machine check is
> escalated to Catastrophic Error (CATERR) and the machine dies.
> If guest CR4.MCE==1, then the machine check causes VMEXIT and is
> handled normally by host Linux. After the change, injecting a machine
> check causes normal Linux machine check handling.
>
> Signed-off-by: Ben Serebrin <serebrin@google.com>
> ---
>  arch/x86/kvm/vmx.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index f5e8dce..f7b6168 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -3622,8 +3622,16 @@ static void vmx_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3)
>
>  static int vmx_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
>  {
> -       unsigned long hw_cr4 = cr4 | (to_vmx(vcpu)->rmode.vm86_active ?
> -                   KVM_RMODE_VM_CR4_ALWAYS_ON : KVM_PMODE_VM_CR4_ALWAYS_ON);
> +       /*
> +        * Pass through host's Machine Check Enable value to hw_cr4, which
> +        * is in force while we are in guest mode.  Do not let guests control
> +        * this bit, even if host CR4.MCE == 0.
> +        */
> +       unsigned long hw_cr4 =
> +               (cr4_read_shadow() & X86_CR4_MCE) |
> +               (cr4 & ~X86_CR4_MCE) |
> +               (to_vmx(vcpu)->rmode.vm86_active ?
> +                KVM_RMODE_VM_CR4_ALWAYS_ON : KVM_PMODE_VM_CR4_ALWAYS_ON);
>
>         if (cr4 & X86_CR4_VMXE) {
>                 /*
> --
> 2.2.0.rc0.207.ga3a616c

Signed-off-by: Venkatesh Srinivas <venkateshs@google.com>

-- vs;

Re: [PATCH] KVM: VMX: Preserve host CR4.MCE value while in guest mode.

[Wanpeng Li]

Hi Ben,
On Thu, Apr 16, 2015 at 11:58:05AM -0700, Ben Serebrin wrote:
>The host's decision to enable machine check exceptions should remain
>in force during non-root mode.  KVM was writing 0 to cr4 on VCPU reset
>and passed a slightly-modified 0 to the vmcs.guest_cr4 value.
>
>Tested: Built.
>On earlier version, tested by injecting machine check
>while a guest is spinning.
>
>Before the change, if guest CR4.MCE==0, then the machine check is
>escalated to Catastrophic Error (CATERR) and the machine dies.

Could you point out which section of SDM describes that the machine check 
is escalated to a CATERR if CR4.MCE==0?

Regards,
Wanpeng Li 

>If guest CR4.MCE==1, then the machine check causes VMEXIT and is
>handled normally by host Linux. After the change, injecting a machine
>check causes normal Linux machine check handling.
>
>Signed-off-by: Ben Serebrin <serebrin@google.com>
>---
> arch/x86/kvm/vmx.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
>diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>index f5e8dce..f7b6168 100644
>--- a/arch/x86/kvm/vmx.c
>+++ b/arch/x86/kvm/vmx.c
>@@ -3622,8 +3622,16 @@ static void vmx_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3)
> 
> static int vmx_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
> {
>-	unsigned long hw_cr4 = cr4 | (to_vmx(vcpu)->rmode.vm86_active ?
>-		    KVM_RMODE_VM_CR4_ALWAYS_ON : KVM_PMODE_VM_CR4_ALWAYS_ON);
>+	/*
>+	 * Pass through host's Machine Check Enable value to hw_cr4, which
>+	 * is in force while we are in guest mode.  Do not let guests control
>+	 * this bit, even if host CR4.MCE == 0.
>+	 */
>+	unsigned long hw_cr4 =
>+		(cr4_read_shadow() & X86_CR4_MCE) |
>+		(cr4 & ~X86_CR4_MCE) |
>+		(to_vmx(vcpu)->rmode.vm86_active ?
>+		 KVM_RMODE_VM_CR4_ALWAYS_ON : KVM_PMODE_VM_CR4_ALWAYS_ON);
> 
> 	if (cr4 & X86_CR4_VMXE) {
> 		/*
>-- 
>2.2.0.rc0.207.ga3a616c
>
>--
>To unsubscribe from this list: send the line "unsubscribe kvm" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] KVM: VMX: Preserve host CR4.MCE value while in guest mode.

[Paolo Bonzini]

On 17/04/2015 07:10, Wanpeng Li wrote:
>> >
>> >Before the change, if guest CR4.MCE==0, then the machine check is
>> >escalated to Catastrophic Error (CATERR) and the machine dies.
> Could you point out which section of SDM describes that the machine check 
> is escalated to a CATERR if CR4.MCE==0?

It's under the description of "Interrupt 18--Machine-Check Exception (#MC)":

   The machine-check mechanism is enabled by setting the MCE flag in
   control register CR4.  [...] If the machine-check mechanism is not
   enabled (the MCE flag in control register CR4 is clear), a
   machine-check exception causes the processor to enter the shutdown
   state.

This of course also applies whenever the non-root mode CR4 (*not* the
shadow CR4 in the VMCS!) has the MCE flag cleared.

Paolo

Re: [PATCH] KVM: VMX: Preserve host CR4.MCE value while in guest mode.

[Benjamin Serebrin]

Thanks, Paolo, for the document reference and the fixup.

Greg: Sorry for the standard newbie gaffs.

On Fri, Apr 17, 2015 at 3:16 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 17/04/2015 07:10, Wanpeng Li wrote:
>>> >
>>> >Before the change, if guest CR4.MCE==0, then the machine check is
>>> >escalated to Catastrophic Error (CATERR) and the machine dies.
>> Could you point out which section of SDM describes that the machine check
>> is escalated to a CATERR if CR4.MCE==0?
>
> It's under the description of "Interrupt 18--Machine-Check Exception (#MC)":
>
>    The machine-check mechanism is enabled by setting the MCE flag in
>    control register CR4.  [...] If the machine-check mechanism is not
>    enabled (the MCE flag in control register CR4 is clear), a
>    machine-check exception causes the processor to enter the shutdown
>    state.
>
> This of course also applies whenever the non-root mode CR4 (*not* the
> shadow CR4 in the VMCS!) has the MCE flag cleared.
>
> Paolo

Re: [PATCH] KVM: VMX: Preserve host CR4.MCE value while in guest mode.

[Wanpeng Li]

On Fri, Apr 17, 2015 at 12:16:18PM +0200, Paolo Bonzini wrote:
>
>
>On 17/04/2015 07:10, Wanpeng Li wrote:
>>> >
>>> >Before the change, if guest CR4.MCE==0, then the machine check is
>>> >escalated to Catastrophic Error (CATERR) and the machine dies.
>> Could you point out which section of SDM describes that the machine check 
>> is escalated to a CATERR if CR4.MCE==0?
>
>It's under the description of "Interrupt 18--Machine-Check Exception (#MC)":
>
>   The machine-check mechanism is enabled by setting the MCE flag in
>   control register CR4.  [...] If the machine-check mechanism is not
>   enabled (the MCE flag in control register CR4 is clear), a
>   machine-check exception causes the processor to enter the shutdown
>   state.
>
>This of course also applies whenever the non-root mode CR4 (*not* the
>shadow CR4 in the VMCS!) has the MCE flag cleared.

Got it, thanks, :)

Regards,
Wanpeng Li 

>
>Paolo

Re: [PATCH] KVM: VMX: Preserve host CR4.MCE value while in guest mode.

[Greg KH]

On Thu, Apr 16, 2015 at 11:58:05AM -0700, Ben Serebrin wrote:
> The host's decision to enable machine check exceptions should remain
> in force during non-root mode.  KVM was writing 0 to cr4 on VCPU reset
> and passed a slightly-modified 0 to the vmcs.guest_cr4 value.
> 
> Tested: Built.
> On earlier version, tested by injecting machine check
> while a guest is spinning.
> 
> Before the change, if guest CR4.MCE==0, then the machine check is
> escalated to Catastrophic Error (CATERR) and the machine dies.
> If guest CR4.MCE==1, then the machine check causes VMEXIT and is
> handled normally by host Linux. After the change, injecting a machine
> check causes normal Linux machine check handling.
> 
> Signed-off-by: Ben Serebrin <serebrin@google.com>
> ---
>  arch/x86/kvm/vmx.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read Documentation/stable_kernel_rules.txt
for how to do this properly.

</formletter>

Re: [PATCH] KVM: VMX: Preserve host CR4.MCE value while in guest mode.

[Paolo Bonzini]

On 17/04/2015 10:55, Greg KH wrote:
> On Thu, Apr 16, 2015 at 11:58:05AM -0700, Ben Serebrin wrote:
>> > The host's decision to enable machine check exceptions should remain
>> > in force during non-root mode.  KVM was writing 0 to cr4 on VCPU reset
>> > and passed a slightly-modified 0 to the vmcs.guest_cr4 value.
>> > 
>> > Tested: Built.
>> > On earlier version, tested by injecting machine check
>> > while a guest is spinning.
>> > 
>> > Before the change, if guest CR4.MCE==0, then the machine check is
>> > escalated to Catastrophic Error (CATERR) and the machine dies.
>> > If guest CR4.MCE==1, then the machine check causes VMEXIT and is
>> > handled normally by host Linux. After the change, injecting a machine
>> > check causes normal Linux machine check handling.
>> > 
>> > Signed-off-by: Ben Serebrin <serebrin@google.com>
>> > ---
>> >  arch/x86/kvm/vmx.c | 12 ++++++++++--
>> >  1 file changed, 10 insertions(+), 2 deletions(-)
> <formletter>
> 
> This is not the correct way to submit patches for inclusion in the
> stable kernel tree.  Please read Documentation/stable_kernel_rules.txt
> for how to do this properly.

I'll fix it up when applying to the KVM tree.

Paolo