| Message ID | 20220217053028.96432-3-leobras@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/kvm/fpu: Fix guest migration bugs that can crash guest | expand |
On Thursday, 2022-02-17 at 02:30:30 -03, Leonardo Bras wrote: > kvm_vcpu_arch currently contains the guest supported features in both > guest_supported_xcr0 and guest_fpu.fpstate->user_xfeatures field. > > Currently both fields are set to the same value in > kvm_vcpu_after_set_cpuid() and are not changed anywhere else after that. > > Since it's not good to keep duplicated data, remove guest_supported_xcr0. > > To keep the code more readable, introduce kvm_guest_supported_xcr() > and kvm_guest_supported_xfd() to replace the previous usages of > guest_supported_xcr0. > > Signed-off-by: Leonardo Bras <leobras@redhat.com> > --- > arch/x86/include/asm/kvm_host.h | 1 - > arch/x86/kvm/cpuid.c | 5 +++-- > arch/x86/kvm/x86.c | 20 +++++++++++++++----- > 3 files changed, 18 insertions(+), 8 deletions(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index 6dcccb304775..ec9830d2aabf 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -703,7 +703,6 @@ struct kvm_vcpu_arch { > struct fpu_guest guest_fpu; > > u64 xcr0; > - u64 guest_supported_xcr0; > > struct kvm_pio_request pio; > void *pio_data; > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > index 71125291c578..b8f8d268d058 100644 > --- a/arch/x86/kvm/cpuid.c > +++ b/arch/x86/kvm/cpuid.c > @@ -282,6 +282,7 @@ static void kvm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu) > { > struct kvm_lapic *apic = vcpu->arch.apic; > struct kvm_cpuid_entry2 *best; > + u64 guest_supported_xcr0; The intermediate variable seems unnecessary. > > best = kvm_find_cpuid_entry(vcpu, 1, 0); > if (best && apic) { > @@ -293,10 +294,10 @@ static void kvm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu) > kvm_apic_set_version(vcpu); > } > > - vcpu->arch.guest_supported_xcr0 = > + guest_supported_xcr0 = > cpuid_get_supported_xcr0(vcpu->arch.cpuid_entries, vcpu->arch.cpuid_nent); > > - vcpu->arch.guest_fpu.fpstate->user_xfeatures = vcpu->arch.guest_supported_xcr0; > + vcpu->arch.guest_fpu.fpstate->user_xfeatures = guest_supported_xcr0; > > kvm_update_pv_runtime(vcpu); > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 641044db415d..92177e2ff664 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -984,6 +984,18 @@ void kvm_load_host_xsave_state(struct kvm_vcpu *vcpu) > } > EXPORT_SYMBOL_GPL(kvm_load_host_xsave_state); > > +static inline u64 kvm_guest_supported_xcr(struct kvm_vcpu *vcpu) > +{ > + u64 guest_supported_xcr0 = vcpu->arch.guest_fpu.fpstate->user_xfeatures; ...and here. > + > + return guest_supported_xcr0; > +} > + > +static inline u64 kvm_guest_supported_xfd(struct kvm_vcpu *vcpu) > +{ > + return kvm_guest_supported_xcr(vcpu) & XFEATURE_MASK_USER_DYNAMIC; > +} > + > static int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr) > { > u64 xcr0 = xcr; > @@ -1003,7 +1015,7 @@ static int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr) > * saving. However, xcr0 bit 0 is always set, even if the > * emulated CPU does not support XSAVE (see kvm_vcpu_reset()). > */ > - valid_bits = vcpu->arch.guest_supported_xcr0 | XFEATURE_MASK_FP; > + valid_bits = kvm_guest_supported_xcr(vcpu) | XFEATURE_MASK_FP; > if (xcr0 & ~valid_bits) > return 1; > > @@ -3706,8 +3718,7 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) > !guest_cpuid_has(vcpu, X86_FEATURE_XFD)) > return 1; > > - if (data & ~(XFEATURE_MASK_USER_DYNAMIC & > - vcpu->arch.guest_supported_xcr0)) > + if (data & ~(kvm_guest_supported_xfd(vcpu))) Brackets could be removed... > return 1; > > fpu_update_guest_xfd(&vcpu->arch.guest_fpu, data); > @@ -3717,8 +3728,7 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) > !guest_cpuid_has(vcpu, X86_FEATURE_XFD)) > return 1; > > - if (data & ~(XFEATURE_MASK_USER_DYNAMIC & > - vcpu->arch.guest_supported_xcr0)) > + if (data & ~(kvm_guest_supported_xfd(vcpu))) ...and here. > return 1; > > vcpu->arch.guest_fpu.xfd_err = data; dme.
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 6dcccb304775..ec9830d2aabf 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -703,7 +703,6 @@ struct kvm_vcpu_arch { struct fpu_guest guest_fpu; u64 xcr0; - u64 guest_supported_xcr0; struct kvm_pio_request pio; void *pio_data; diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c index 71125291c578..b8f8d268d058 100644 --- a/arch/x86/kvm/cpuid.c +++ b/arch/x86/kvm/cpuid.c @@ -282,6 +282,7 @@ static void kvm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu) { struct kvm_lapic *apic = vcpu->arch.apic; struct kvm_cpuid_entry2 *best; + u64 guest_supported_xcr0; best = kvm_find_cpuid_entry(vcpu, 1, 0); if (best && apic) { @@ -293,10 +294,10 @@ static void kvm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu) kvm_apic_set_version(vcpu); } - vcpu->arch.guest_supported_xcr0 = + guest_supported_xcr0 = cpuid_get_supported_xcr0(vcpu->arch.cpuid_entries, vcpu->arch.cpuid_nent); - vcpu->arch.guest_fpu.fpstate->user_xfeatures = vcpu->arch.guest_supported_xcr0; + vcpu->arch.guest_fpu.fpstate->user_xfeatures = guest_supported_xcr0; kvm_update_pv_runtime(vcpu); diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 641044db415d..92177e2ff664 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -984,6 +984,18 @@ void kvm_load_host_xsave_state(struct kvm_vcpu *vcpu) } EXPORT_SYMBOL_GPL(kvm_load_host_xsave_state); +static inline u64 kvm_guest_supported_xcr(struct kvm_vcpu *vcpu) +{ + u64 guest_supported_xcr0 = vcpu->arch.guest_fpu.fpstate->user_xfeatures; + + return guest_supported_xcr0; +} + +static inline u64 kvm_guest_supported_xfd(struct kvm_vcpu *vcpu) +{ + return kvm_guest_supported_xcr(vcpu) & XFEATURE_MASK_USER_DYNAMIC; +} + static int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr) { u64 xcr0 = xcr; @@ -1003,7 +1015,7 @@ static int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr) * saving. However, xcr0 bit 0 is always set, even if the * emulated CPU does not support XSAVE (see kvm_vcpu_reset()). */ - valid_bits = vcpu->arch.guest_supported_xcr0 | XFEATURE_MASK_FP; + valid_bits = kvm_guest_supported_xcr(vcpu) | XFEATURE_MASK_FP; if (xcr0 & ~valid_bits) return 1; @@ -3706,8 +3718,7 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) !guest_cpuid_has(vcpu, X86_FEATURE_XFD)) return 1; - if (data & ~(XFEATURE_MASK_USER_DYNAMIC & - vcpu->arch.guest_supported_xcr0)) + if (data & ~(kvm_guest_supported_xfd(vcpu))) return 1; fpu_update_guest_xfd(&vcpu->arch.guest_fpu, data); @@ -3717,8 +3728,7 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) !guest_cpuid_has(vcpu, X86_FEATURE_XFD)) return 1; - if (data & ~(XFEATURE_MASK_USER_DYNAMIC & - vcpu->arch.guest_supported_xcr0)) + if (data & ~(kvm_guest_supported_xfd(vcpu))) return 1; vcpu->arch.guest_fpu.xfd_err = data;
kvm_vcpu_arch currently contains the guest supported features in both guest_supported_xcr0 and guest_fpu.fpstate->user_xfeatures field. Currently both fields are set to the same value in kvm_vcpu_after_set_cpuid() and are not changed anywhere else after that. Since it's not good to keep duplicated data, remove guest_supported_xcr0. To keep the code more readable, introduce kvm_guest_supported_xcr() and kvm_guest_supported_xfd() to replace the previous usages of guest_supported_xcr0. Signed-off-by: Leonardo Bras <leobras@redhat.com> --- arch/x86/include/asm/kvm_host.h | 1 - arch/x86/kvm/cpuid.c | 5 +++-- arch/x86/kvm/x86.c | 20 +++++++++++++++----- 3 files changed, 18 insertions(+), 8 deletions(-)