If max leaf is greater than 0xd but xsave not available to the guest, then the current XSAVE size should not be filled in. This is a latent bug for now as the guest max leaf is 0xd, but will become problematic in the future.
The comment concerning XSS state is wrong. VT-x doesn't manage host/guest state automatically, but there is provision for "host only" bits to be set, so the implications are still accurate. Introduce {xstate,hw}_compressed_size() helpers to mirror the uncompressed ones. Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com> --- CC: Jan Beulich <jbeul...@suse.com> CC: Roger Pau Monné <roger....@citrix.com> CC: Wei Liu <w...@xen.org> --- xen/arch/x86/cpuid.c | 23 +++++++-------------- xen/arch/x86/xstate.c | 49 ++++++++++++++++++++++++++++++++++++++++++++ xen/include/asm-x86/xstate.h | 1 + 3 files changed, 57 insertions(+), 16 deletions(-) diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c index c7f8388e5d..92745aa63f 100644 --- a/xen/arch/x86/cpuid.c +++ b/xen/arch/x86/cpuid.c @@ -1041,24 +1041,15 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf, case XSTATE_CPUID: switch ( subleaf ) { + case 0: + if ( p->basic.xsave ) + res->b = xstate_uncompressed_size(v->arch.xcr0); + break; + case 1: if ( p->xstate.xsaves ) - { - /* - * TODO: Figure out what to do for XSS state. VT-x manages - * host vs guest MSR_XSS automatically, so as soon as we start - * supporting any XSS states, the wrong XSS will be in - * context. - */ - BUILD_BUG_ON(XSTATE_XSAVES_ONLY != 0); - - /* - * Read CPUID[0xD,0/1].EBX from hardware. They vary with - * enabled XSTATE, and appropraite XCR0|XSS are in context. - */ - case 0: - res->b = cpuid_count_ebx(leaf, subleaf); - } + res->b = xstate_compressed_size(v->arch.xcr0 | + v->arch.msrs->xss.raw); break; } break; diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c index d4c01da574..03489f0cf4 100644 --- a/xen/arch/x86/xstate.c +++ b/xen/arch/x86/xstate.c @@ -602,6 +602,55 @@ unsigned int xstate_uncompressed_size(uint64_t xcr0) return size; } +static unsigned int hw_compressed_size(uint64_t xstates) +{ + uint64_t curr_xcr0 = get_xcr0(), curr_xss = get_msr_xss(); + unsigned int size; + bool ok; + + ok = set_xcr0(xstates & ~XSTATE_XSAVES_ONLY); + ASSERT(ok); + set_msr_xss(xstates & XSTATE_XSAVES_ONLY); + + size = cpuid_count_ebx(XSTATE_CPUID, 1); + + ok = set_xcr0(curr_xcr0); + ASSERT(ok); + set_msr_xss(curr_xss); + + return size; +} + +unsigned int xstate_compressed_size(uint64_t xstates) +{ + unsigned int i, size = XSTATE_AREA_MIN_SIZE; + + xstates &= ~XSTATE_FP_SSE; + for_each_set_bit ( i, &xstates, 63 ) + { + if ( test_bit(i, &xstate_align) ) + size = ROUNDUP(size, 64); + + size += xstate_sizes[i]; + } + + /* In debug builds, cross-check our calculation with hardware. */ + if ( IS_ENABLED(CONFIG_DEBUG) ) + { + unsigned int hwsize; + + xstates |= XSTATE_FP_SSE; + hwsize = hw_compressed_size(xstates); + + if ( size != hwsize ) + printk_once(XENLOG_ERR "%s(%#"PRIx64") size %#x != hwsize %#x\n", + __func__, xstates, size, hwsize); + size = hwsize; + } + + return size; +} + /* Collect the information of processor's extended state */ void xstate_init(struct cpuinfo_x86 *c) { diff --git a/xen/include/asm-x86/xstate.h b/xen/include/asm-x86/xstate.h index 02d6f171b8..ecf7bbc5cd 100644 --- a/xen/include/asm-x86/xstate.h +++ b/xen/include/asm-x86/xstate.h @@ -108,6 +108,7 @@ void xstate_free_save_area(struct vcpu *v); int xstate_alloc_save_area(struct vcpu *v); void xstate_init(struct cpuinfo_x86 *c); unsigned int xstate_uncompressed_size(uint64_t xcr0); +unsigned int xstate_compressed_size(uint64_t states); static inline uint64_t xgetbv(unsigned int index) { -- 2.11.0