diff mbox

x86/vmx: Fix vmentry failure because of invalid LER on Broadwell

Message ID 20170525121533.32409-1-ross.lagerwall@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ross Lagerwall May 25, 2017, 12:15 p.m. UTC
Occasionally, the top three bits of MSR_IA32_LASTINTTOIP
(MSR_LER_TO_LIP) may be incorrect, as though the MSR is using the
LBR_FORMAT_EIP_FLAGS_TSX format. The MSR should contain an offset into
the current code segment according to the Intel documentation. It is not
clear why this happens. It may be due to erratum BDF14, or some other
errata.  The result is a vmentry failure.

Workaround the issue by sign-extending into bits 61:63 for this MSR on
Broadwell CPUs.
---
 xen/arch/x86/hvm/vmx/vmx.c         | 29 +++++++++++++++++++++++++++++
 xen/include/asm-x86/hvm/vmx/vmcs.h |  1 +
 2 files changed, 30 insertions(+)

Comments

Sergey Dyasli May 26, 2017, 7:24 a.m. UTC | #1
Hi Ross,

On Thu, 2017-05-25 at 13:15 +0100, Ross Lagerwall wrote:
> Occasionally, the top three bits of MSR_IA32_LASTINTTOIP

> (MSR_LER_TO_LIP) may be incorrect, as though the MSR is using the

> LBR_FORMAT_EIP_FLAGS_TSX format. The MSR should contain an offset into

> the current code segment according to the Intel documentation. It is not

> clear why this happens. It may be due to erratum BDF14, or some other

> errata.  The result is a vmentry failure.

> 

> Workaround the issue by sign-extending into bits 61:63 for this MSR on

> Broadwell CPUs.

> ---

>  xen/arch/x86/hvm/vmx/vmx.c         | 29 +++++++++++++++++++++++++++++

>  xen/include/asm-x86/hvm/vmx/vmcs.h |  1 +

>  2 files changed, 30 insertions(+)

> 

> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c

> index c8ef18a..7d729af 100644

> --- a/xen/arch/x86/hvm/vmx/vmx.c

> +++ b/xen/arch/x86/hvm/vmx/vmx.c

> @@ -2434,6 +2434,7 @@ static void pi_notification_interrupt(struct cpu_user_regs *regs)

>  }

>  

>  static void __init lbr_tsx_fixup_check(void);

> +static void __init ler_bdw_fixup_check(void);

>  

>  const struct hvm_function_table * __init start_vmx(void)

>  {

> @@ -2499,6 +2500,7 @@ const struct hvm_function_table * __init start_vmx(void)

>      setup_vmcs_dump();

>  

>      lbr_tsx_fixup_check();

> +    ler_bdw_fixup_check();

>  

>      return &vmx_function_table;

>  }

> @@ -2790,8 +2792,10 @@ enum

>  };

>  

>  #define LBR_FROM_SIGNEXT_2MSB  ((1ULL << 59) | (1ULL << 60))

> +#define LER_TO_SIGNEXT_3MSB  (LBR_FROM_SIGNEXT_2MSB | (1ULL << 58))

>  

>  static bool __read_mostly lbr_tsx_fixup_needed;

> +static bool __read_mostly ler_bdw_fixup_needed;

>  static uint32_t __read_mostly lbr_from_start;

>  static uint32_t __read_mostly lbr_from_end;

>  static uint32_t __read_mostly lbr_lastint_from;

> @@ -2828,6 +2832,13 @@ static void __init lbr_tsx_fixup_check(void)

>      }

>  }

>  

> +static void __init ler_bdw_fixup_check(void)

> +{

> +    /* Broadwell E5-2600 v4 processors need to work around erratum BDF14. */

> +    if ( boot_cpu_data.x86 == 6 && boot_cpu_data.x86_model == 79 )

> +        ler_bdw_fixup_needed = true;

> +}

> +

>  static int is_last_branch_msr(u32 ecx)

>  {

>      const struct lbr_info *lbr = last_branch_msr_get();

> @@ -3089,6 +3100,8 @@ static int vmx_msr_write_intercept(unsigned int msr, uint64_t msr_content)

>                          vmx_disable_intercept_for_msr(v, lbr->base + i, MSR_TYPE_R | MSR_TYPE_W);

>                          v->arch.hvm_vmx.lbr_tsx_fixup_enabled =

>                              lbr_tsx_fixup_needed;

> +                        v->arch.hvm_vmx.ler_bdw_fixup_enabled =

> +                            ler_bdw_fixup_needed;

>                      }

>          }

>  

> @@ -4174,6 +4187,20 @@ static void lbr_tsx_fixup(void)

>          msr->data |= ((LBR_FROM_SIGNEXT_2MSB & msr->data) << 2);

>  }

>  

> +static void ler_bdw_fixup(void)

> +{

> +    struct vmx_msr_entry *msr;

> +

> +    /*

> +     * Occasionally, the top three bits of MSR_IA32_LASTINTTOIP may be

> +     * incorrect (possibly due to BDF14), as though the MSR is using the

> +     * LBR_FORMAT_EIP_FLAGS_TSX format. This is incorrect and causes a vmentry

> +     * failure -- the MSR should contain an offset into the current code

> +     * segment. Fix it up by sign-extending into bits 61:63. */

> +    if ( (msr = vmx_find_msr(MSR_IA32_LASTINTTOIP, VMX_GUEST_MSR)) != NULL )

> +        msr->data |= ((LER_TO_SIGNEXT_3MSB & msr->data) << 3);

> +}

> +

>  void vmx_vmenter_helper(const struct cpu_user_regs *regs)

>  {

>      struct vcpu *curr = current;

> @@ -4232,6 +4259,8 @@ void vmx_vmenter_helper(const struct cpu_user_regs *regs)

>   out:

>      if ( unlikely(curr->arch.hvm_vmx.lbr_tsx_fixup_enabled) )

>          lbr_tsx_fixup();

> +    if ( unlikely(curr->arch.hvm_vmx.ler_bdw_fixup_enabled) )

> +        ler_bdw_fixup();


Since the new ler_bdw_fixup() is also LBR-related, IMHO it would be
better to have only one check in vmx_vmenter_helper() for both fixups.
A new function can be introduced that will check for a specific
LBR-fixup that needs to be applied.

>  

>      HVMTRACE_ND(VMENTRY, 0, 1/*cycles*/, 0, 0, 0, 0, 0, 0, 0);

>  

> diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h b/xen/include/asm-x86/hvm/vmx/vmcs.h

> index 9507bd2..aedef82 100644

> --- a/xen/include/asm-x86/hvm/vmx/vmcs.h

> +++ b/xen/include/asm-x86/hvm/vmx/vmcs.h

> @@ -137,6 +137,7 @@ struct arch_vmx_struct {

>      uint8_t              vmx_emulate;

>  

>      bool                 lbr_tsx_fixup_enabled;

> +    bool                 ler_bdw_fixup_enabled;

>  

>      /* Bitmask of segments that we can't safely use in virtual 8086 mode */

>      uint16_t             vm86_segment_mask;

-- 
Thanks,
Sergey
diff mbox

Patch

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index c8ef18a..7d729af 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2434,6 +2434,7 @@  static void pi_notification_interrupt(struct cpu_user_regs *regs)
 }
 
 static void __init lbr_tsx_fixup_check(void);
+static void __init ler_bdw_fixup_check(void);
 
 const struct hvm_function_table * __init start_vmx(void)
 {
@@ -2499,6 +2500,7 @@  const struct hvm_function_table * __init start_vmx(void)
     setup_vmcs_dump();
 
     lbr_tsx_fixup_check();
+    ler_bdw_fixup_check();
 
     return &vmx_function_table;
 }
@@ -2790,8 +2792,10 @@  enum
 };
 
 #define LBR_FROM_SIGNEXT_2MSB  ((1ULL << 59) | (1ULL << 60))
+#define LER_TO_SIGNEXT_3MSB  (LBR_FROM_SIGNEXT_2MSB | (1ULL << 58))
 
 static bool __read_mostly lbr_tsx_fixup_needed;
+static bool __read_mostly ler_bdw_fixup_needed;
 static uint32_t __read_mostly lbr_from_start;
 static uint32_t __read_mostly lbr_from_end;
 static uint32_t __read_mostly lbr_lastint_from;
@@ -2828,6 +2832,13 @@  static void __init lbr_tsx_fixup_check(void)
     }
 }
 
+static void __init ler_bdw_fixup_check(void)
+{
+    /* Broadwell E5-2600 v4 processors need to work around erratum BDF14. */
+    if ( boot_cpu_data.x86 == 6 && boot_cpu_data.x86_model == 79 )
+        ler_bdw_fixup_needed = true;
+}
+
 static int is_last_branch_msr(u32 ecx)
 {
     const struct lbr_info *lbr = last_branch_msr_get();
@@ -3089,6 +3100,8 @@  static int vmx_msr_write_intercept(unsigned int msr, uint64_t msr_content)
                         vmx_disable_intercept_for_msr(v, lbr->base + i, MSR_TYPE_R | MSR_TYPE_W);
                         v->arch.hvm_vmx.lbr_tsx_fixup_enabled =
                             lbr_tsx_fixup_needed;
+                        v->arch.hvm_vmx.ler_bdw_fixup_enabled =
+                            ler_bdw_fixup_needed;
                     }
         }
 
@@ -4174,6 +4187,20 @@  static void lbr_tsx_fixup(void)
         msr->data |= ((LBR_FROM_SIGNEXT_2MSB & msr->data) << 2);
 }
 
+static void ler_bdw_fixup(void)
+{
+    struct vmx_msr_entry *msr;
+
+    /*
+     * Occasionally, the top three bits of MSR_IA32_LASTINTTOIP may be
+     * incorrect (possibly due to BDF14), as though the MSR is using the
+     * LBR_FORMAT_EIP_FLAGS_TSX format. This is incorrect and causes a vmentry
+     * failure -- the MSR should contain an offset into the current code
+     * segment. Fix it up by sign-extending into bits 61:63. */
+    if ( (msr = vmx_find_msr(MSR_IA32_LASTINTTOIP, VMX_GUEST_MSR)) != NULL )
+        msr->data |= ((LER_TO_SIGNEXT_3MSB & msr->data) << 3);
+}
+
 void vmx_vmenter_helper(const struct cpu_user_regs *regs)
 {
     struct vcpu *curr = current;
@@ -4232,6 +4259,8 @@  void vmx_vmenter_helper(const struct cpu_user_regs *regs)
  out:
     if ( unlikely(curr->arch.hvm_vmx.lbr_tsx_fixup_enabled) )
         lbr_tsx_fixup();
+    if ( unlikely(curr->arch.hvm_vmx.ler_bdw_fixup_enabled) )
+        ler_bdw_fixup();
 
     HVMTRACE_ND(VMENTRY, 0, 1/*cycles*/, 0, 0, 0, 0, 0, 0, 0);
 
diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h b/xen/include/asm-x86/hvm/vmx/vmcs.h
index 9507bd2..aedef82 100644
--- a/xen/include/asm-x86/hvm/vmx/vmcs.h
+++ b/xen/include/asm-x86/hvm/vmx/vmcs.h
@@ -137,6 +137,7 @@  struct arch_vmx_struct {
     uint8_t              vmx_emulate;
 
     bool                 lbr_tsx_fixup_enabled;
+    bool                 ler_bdw_fixup_enabled;
 
     /* Bitmask of segments that we can't safely use in virtual 8086 mode */
     uint16_t             vm86_segment_mask;