On Tue, Aug 15, 2023 at 01:36:41PM -0700, Sean Christopherson wrote:
> Recompute whether or not XSAVES is enabled for the guest only if the
> guest's CPUID model changes instead of redoing the computation every time
> KVM generates vmcs01's secondary execution controls. The boot_cpu_has()
> and cpu_has_vmx_xsaves() checks should never change after KVM is loaded,
> and if they do the kernel/KVM is hosed.
>
> Opportunistically add a comment explaining _why_ XSAVES is effectively
> exposed to the guest if and only if XSAVE is also exposed to the guest.
>
> Practically speaking, no functional change intended (KVM will do fewer
> computations, but should still see the same xsaves_enabled value whenever
> KVM looks at it).
>
> Signed-off-by: Sean Christopherson <[email protected]>
> ---
> arch/x86/kvm/vmx/vmx.c | 24 +++++++++++-------------
> 1 file changed, 11 insertions(+), 13 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 434bf524e712..1bf85bd53416 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -4612,19 +4612,10 @@ static u32 vmx_secondary_exec_control(struct vcpu_vmx *vmx)
> if (!enable_pml || !atomic_read(&vcpu->kvm->nr_memslots_dirty_logging))
> exec_control &= ~SECONDARY_EXEC_ENABLE_PML;
>
> - if (cpu_has_vmx_xsaves()) {
> - /* Exposing XSAVES only when XSAVE is exposed */
> - bool xsaves_enabled =
> - boot_cpu_has(X86_FEATURE_XSAVE) &&
> - guest_cpuid_has(vcpu, X86_FEATURE_XSAVE) &&
> - guest_cpuid_has(vcpu, X86_FEATURE_XSAVES);
> -
> - vcpu->arch.xsaves_enabled = xsaves_enabled;
> -
> + if (cpu_has_vmx_xsaves())
> vmx_adjust_secondary_exec_control(vmx, &exec_control,
> SECONDARY_EXEC_XSAVES,
> - xsaves_enabled, false);
> - }
> + vcpu->arch.xsaves_enabled, false);
xsaves_enabled is same as before in case of init_vmcs().
Reviewed-by: Yuan Yao <[email protected]>
>
> /*
> * RDPID is also gated by ENABLE_RDTSCP, turn on the control if either
> @@ -7749,8 +7740,15 @@ static void vmx_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu)
> {
> struct vcpu_vmx *vmx = to_vmx(vcpu);
>
> - /* xsaves_enabled is recomputed in vmx_compute_secondary_exec_control(). */
> - vcpu->arch.xsaves_enabled = false;
> + /*
> + * XSAVES is effectively enabled if and only if XSAVE is also exposed
> + * to the guest. XSAVES depends on CR4.OSXSAVE, and CR4.OSXSAVE can be
> + * set if and only if XSAVE is supported.
> + */
> + vcpu->arch.xsaves_enabled = cpu_has_vmx_xsaves() &&
> + boot_cpu_has(X86_FEATURE_XSAVE) &&
> + guest_cpuid_has(vcpu, X86_FEATURE_XSAVE) &&
> + guest_cpuid_has(vcpu, X86_FEATURE_XSAVES);
>
> vmx_setup_uret_msrs(vmx);
>
> --
> 2.41.0.694.ge786442a9b-goog
>