|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH v3 5/9] x86/vmx: Improvements to LBR MSR handling
The main purpose of this patch is to only ever insert the LBR MSRs into the
guest load/save list once, as a future patch wants to change the behaviour of
vmx_add_guest_msr().
The repeated processing of lbr_info and the guests MSR load/save list is
redundant, and a guest using LBR itself will have to re-enable
MSR_DEBUGCTL.LBR in its #DB handler, meaning that Xen will repeat this
redundant processing every time the guest gets a debug exception.
Rename lbr_fixup_enabled to lbr_flags to be a little more generic, and use one
bit to indicate that the MSRs have been inserted into the load/save list.
Shorten the existing FIXUP* identifiers to reduce code volume.
Furthermore, handing the guest #MC on an error isn't a legitimate action. Two
of the three failure cases are definitely hypervisor bugs, and the third is a
boundary case which shouldn't occur in practice. The guest also won't execute
correctly, so handle errors by cleanly crashing the guest.
Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>
CC: Jun Nakajima <jun.nakajima@xxxxxxxxx>
CC: Kevin Tian <kevin.tian@xxxxxxxxx>
v2:
* New
v3:
* domain_crash() error handling. Add a comment explaining why this is the
best (least bad?) option available.
---
xen/arch/x86/hvm/vmx/vmx.c | 81 +++++++++++++++++++++++++++-----------
xen/include/asm-x86/hvm/vmx/vmcs.h | 2 +-
2 files changed, 59 insertions(+), 24 deletions(-)
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 25dd204..d4696e8 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2750,8 +2750,10 @@ enum
#define LBR_FROM_SIGNEXT_2MSB ((1ULL << 59) | (1ULL << 60))
-#define FIXUP_LBR_TSX (1u << 0)
-#define FIXUP_BDW_ERRATUM_BDF14 (1u << 1)
+#define LBR_MSRS_INSERTED (1u << 0)
+#define LBR_FIXUP_TSX (1u << 1)
+#define LBR_FIXUP_BDF14 (1u << 2)
+#define LBR_FIXUP_MASK (LBR_FIXUP_TSX | LBR_FIXUP_BDF14)
static bool __read_mostly lbr_tsx_fixup_needed;
static bool __read_mostly bdw_erratum_bdf14_fixup_needed;
@@ -3086,7 +3088,6 @@ static int vmx_msr_write_intercept(unsigned int msr,
uint64_t msr_content)
break;
case MSR_IA32_DEBUGCTLMSR: {
- int i, rc = 0;
uint64_t supported = IA32_DEBUGCTLMSR_LBR | IA32_DEBUGCTLMSR_BTF;
if ( boot_cpu_has(X86_FEATURE_RTM) )
@@ -3097,30 +3098,64 @@ static int vmx_msr_write_intercept(unsigned int msr,
uint64_t msr_content)
if ( vpmu_do_wrmsr(msr, msr_content, supported) )
break;
}
- if ( msr_content & IA32_DEBUGCTLMSR_LBR )
+
+ /*
+ * When a guest first enables LBR, arrange to save and restore the LBR
+ * MSRs and allow the guest direct access.
+ *
+ * MSR_DEBUGCTL and LBR has existed almost long as MSRs have existed,
+ * and there is no architectural way to hide the feature, or fail the
+ * attempt to enable LBR.
+ *
+ * Unknown host LBR MSRs or hitting -ENOSPC with the guest load/save
+ * list are definitely hypervisor bugs, whereas -ENOMEM for allocating
+ * the load/save list is simply unlucky (and shouldn't occur with
+ * sensible management by the toolstack).
+ *
+ * Either way, there is nothing we can do right now to recover, and
+ * the guest won't execute correctly either. Simply crash the domain
+ * to make the failure obvious.
+ */
+ if ( !(v->arch.hvm_vmx.lbr_flags & LBR_MSRS_INSERTED) &&
+ (msr_content & IA32_DEBUGCTLMSR_LBR) )
{
const struct lbr_info *lbr = last_branch_msr_get();
- if ( lbr == NULL )
- break;
- for ( ; (rc == 0) && lbr->count; lbr++ )
- for ( i = 0; (rc == 0) && (i < lbr->count); i++ )
- if ( (rc = vmx_add_guest_msr(v, lbr->base + i)) == 0 )
+ if ( unlikely(!lbr) )
+ {
+ gprintk(XENLOG_ERR, "Unknown Host LBR MSRs\n");
+ domain_crash(v->domain);
+ return X86EMUL_OKAY;
+ }
+
+ for ( ; lbr->count; lbr++ )
+ {
+ unsigned int i;
+
+ for ( i = 0; i < lbr->count; i++ )
+ {
+ int rc = vmx_add_guest_msr(v, lbr->base + i);
+
+ if ( unlikely(!rc) )
{
- vmx_clear_msr_intercept(v, lbr->base + i, VMX_MSR_RW);
- if ( lbr_tsx_fixup_needed )
- v->arch.hvm_vmx.lbr_fixup_enabled |= FIXUP_LBR_TSX;
- if ( bdw_erratum_bdf14_fixup_needed )
- v->arch.hvm_vmx.lbr_fixup_enabled |=
- FIXUP_BDW_ERRATUM_BDF14;
+ gprintk(XENLOG_ERR,
+ "Guest load/save list error %d\n", rc);
+ domain_crash(v->domain);
+ return X86EMUL_OKAY;
}
- }
- if ( rc < 0 )
- hvm_inject_hw_exception(TRAP_machine_check, X86_EVENT_NO_EC);
- else
- __vmwrite(GUEST_IA32_DEBUGCTL, msr_content);
+ vmx_clear_msr_intercept(v, lbr->base + i, VMX_MSR_RW);
+ }
+ }
+
+ v->arch.hvm_vmx.lbr_flags |= LBR_MSRS_INSERTED;
+ if ( lbr_tsx_fixup_needed )
+ v->arch.hvm_vmx.lbr_flags |= LBR_FIXUP_TSX;
+ if ( bdw_erratum_bdf14_fixup_needed )
+ v->arch.hvm_vmx.lbr_flags |= LBR_FIXUP_BDF14;
+ }
+ __vmwrite(GUEST_IA32_DEBUGCTL, msr_content);
break;
}
case MSR_IA32_FEATURE_CONTROL:
@@ -4203,9 +4238,9 @@ static void lbr_fixup(void)
{
struct vcpu *curr = current;
- if ( curr->arch.hvm_vmx.lbr_fixup_enabled & FIXUP_LBR_TSX )
+ if ( curr->arch.hvm_vmx.lbr_flags & LBR_FIXUP_TSX )
lbr_tsx_fixup();
- if ( curr->arch.hvm_vmx.lbr_fixup_enabled & FIXUP_BDW_ERRATUM_BDF14 )
+ if ( curr->arch.hvm_vmx.lbr_flags & LBR_FIXUP_BDF14 )
bdw_erratum_bdf14_fixup();
}
@@ -4273,7 +4308,7 @@ bool vmx_vmenter_helper(const struct cpu_user_regs *regs)
}
out:
- if ( unlikely(curr->arch.hvm_vmx.lbr_fixup_enabled) )
+ if ( unlikely(curr->arch.hvm_vmx.lbr_flags & LBR_FIXUP_MASK) )
lbr_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 62afebe..2c9e291 100644
--- a/xen/include/asm-x86/hvm/vmx/vmcs.h
+++ b/xen/include/asm-x86/hvm/vmx/vmcs.h
@@ -156,7 +156,7 @@ struct arch_vmx_struct {
/* Are we emulating rather than VMENTERing? */
uint8_t vmx_emulate;
- uint8_t lbr_fixup_enabled;
+ uint8_t lbr_flags;
/* Bitmask of segments that we can't safely use in virtual 8086 mode */
uint16_t vm86_segment_mask;
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |