[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [PATCH 1/8] x86/msr: Fix migration compatibility issue with MSR_SPEC_CTRL
This bug existed in early in 2018 between MSR_SPEC_CTRL arriving in microcode, and SSBD arriving a few months later. It went unnoticed presumably because everyone was busy rebooting everything. The same bug will reappear when adding PSFD support. Clamp the guest MSR_SPEC_CTRL value to that permitted by CPUID on migrate. The guest is already playing with reserved bits at this point, and clamping the value will prevent a migration to a less capable host from failing. Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> --- CC: Jan Beulich <JBeulich@xxxxxxxx> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx> CC: Wei Liu <wl@xxxxxxx> --- xen/arch/x86/hvm/hvm.c | 25 +++++++++++++++++++++++-- xen/arch/x86/include/asm/msr.h | 2 ++ xen/arch/x86/msr.c | 33 +++++++++++++++++++++------------ 3 files changed, 46 insertions(+), 14 deletions(-) diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index d7d3299b431e..c4ddb8607d9c 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -1340,6 +1340,7 @@ static const uint32_t msrs_to_send[] = { static int hvm_save_cpu_msrs(struct vcpu *v, hvm_domain_context_t *h) { + const struct domain *d = v->domain; struct hvm_save_descriptor *desc = _p(&h->data[h->cur]); struct hvm_msr *ctxt; unsigned int i; @@ -1355,7 +1356,8 @@ static int hvm_save_cpu_msrs(struct vcpu *v, hvm_domain_context_t *h) for ( i = 0; i < ARRAY_SIZE(msrs_to_send); ++i ) { uint64_t val; - int rc = guest_rdmsr(v, msrs_to_send[i], &val); + unsigned int msr = msrs_to_send[i]; + int rc = guest_rdmsr(v, msr, &val); /* * It is the programmers responsibility to ensure that @@ -1375,7 +1377,26 @@ static int hvm_save_cpu_msrs(struct vcpu *v, hvm_domain_context_t *h) if ( !val ) continue; /* Skip empty MSRs. */ - ctxt->msr[ctxt->count].index = msrs_to_send[i]; + /* + * Guests are given full access to certain MSRs for performance + * reasons. A consequence is that Xen is unable to enforce that all + * bits disallowed by the CPUID policy yield #GP, and an enterprising + * guest may be able to set and use a bit it ought to leave alone. + * + * When migrating from a more capable host to a less capable one, such + * bits may be rejected by the destination, and the migration failed. + * + * Discard such bits here on the source side. Such bits have reserved + * behaviour, and the guest has only itself to blame. + */ + switch ( msr ) + { + case MSR_SPEC_CTRL: + val &= msr_spec_ctrl_valid_bits(d->arch.cpuid); + break; + } + + ctxt->msr[ctxt->count].index = msr; ctxt->msr[ctxt->count++].val = val; } diff --git a/xen/arch/x86/include/asm/msr.h b/xen/arch/x86/include/asm/msr.h index 10039c2d227b..657a3295613d 100644 --- a/xen/arch/x86/include/asm/msr.h +++ b/xen/arch/x86/include/asm/msr.h @@ -277,6 +277,8 @@ static inline void wrmsr_tsc_aux(uint32_t val) } } +uint64_t msr_spec_ctrl_valid_bits(const struct cpuid_policy *cp); + extern struct msr_policy raw_msr_policy, host_msr_policy, pv_max_msr_policy, diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c index 2cc355575d45..5e80c8b47c21 100644 --- a/xen/arch/x86/msr.c +++ b/xen/arch/x86/msr.c @@ -435,6 +435,24 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t *val) return X86EMUL_EXCEPTION; } +/* + * Caller to confirm that MSR_SPEC_CTRL is available. Intel and AMD have + * separate CPUID features for this functionality, but only set will be + * active. + */ +uint64_t msr_spec_ctrl_valid_bits(const struct cpuid_policy *cp) +{ + bool ssbd = cp->feat.ssbd; + + /* + * Note: SPEC_CTRL_STIBP is specified as safe to use (i.e. ignored) + * when STIBP isn't enumerated in hardware. + */ + return (SPEC_CTRL_IBRS | SPEC_CTRL_STIBP | + (ssbd ? SPEC_CTRL_SSBD : 0) | + 0); +} + int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val) { const struct vcpu *curr = current; @@ -508,18 +526,9 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val) break; case MSR_SPEC_CTRL: - if ( !cp->feat.ibrsb ) - goto gp_fault; /* MSR available? */ - - /* - * Note: SPEC_CTRL_STIBP is specified as safe to use (i.e. ignored) - * when STIBP isn't enumerated in hardware. - */ - rsvd = ~(SPEC_CTRL_IBRS | SPEC_CTRL_STIBP | - (cp->feat.ssbd ? SPEC_CTRL_SSBD : 0)); - - if ( val & rsvd ) - goto gp_fault; /* Rsvd bit set? */ + if ( !cp->feat.ibrsb || + (val & ~msr_spec_ctrl_valid_bits(cp)) ) + goto gp_fault; goto set_reg; case MSR_PRED_CMD: -- 2.11.0
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |