Re: [PATCH v3 05/10] KVM: TDX: restore host xsave state when exit from the guest TD

[Xiaoyao Li <xiaoyao.li@intel.com>]

On 3/8/2025 5:20 AM, Paolo Bonzini wrote:
> From: Isaku Yamahata <isaku.yamahata@intel.com>
> 
> On exiting from the guest TD, xsave state is clobbered; restore it.

I prefer the implementation as this patch, which is straightforward.
(I would be much better if the changelog can describe more)

Anyway,

Reviewed-by: Xiayao Li <xiaoyao.li@intel.com>

> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> Message-ID: <20250129095902.16391-8-adrian.hunter@intel.com>
> [Rewrite to not use kvm_load_host_xsave_state. - Paolo]
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   arch/x86/kvm/vmx/tdx.c | 32 ++++++++++++++++++++++++++++++++
>   1 file changed, 32 insertions(+)
> 
> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index 94e08fdcb775..b2948318cd8b 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
> @@ -2,6 +2,7 @@
>   #include <linux/cleanup.h>
>   #include <linux/cpu.h>
>   #include <asm/cpufeature.h>
> +#include <asm/fpu/xcr.h>
>   #include <linux/misc_cgroup.h>
>   #include <linux/mmu_context.h>
>   #include <asm/tdx.h>
> @@ -735,6 +736,30 @@ static noinstr void tdx_vcpu_enter_exit(struct kvm_vcpu *vcpu)
>   				 BIT_ULL(VCPU_REGS_R14) | \
>   				 BIT_ULL(VCPU_REGS_R15))
>   
> +static void tdx_load_host_xsave_state(struct kvm_vcpu *vcpu)
> +{
> +	struct kvm_tdx *kvm_tdx = to_kvm_tdx(vcpu->kvm);
> +
> +	/*
> +	 * All TDX hosts support PKRU; but even if they didn't,
> +	 * vcpu->arch.host_pkru would be 0 and the wrpkru would be
> +	 * skipped.
> +	 */
> +	if (vcpu->arch.host_pkru != 0)
> +		wrpkru(vcpu->arch.host_pkru);
> +
> +	if (kvm_host.xcr0 != (kvm_tdx->xfam & kvm_caps.supported_xcr0))
> +		xsetbv(XCR_XFEATURE_ENABLED_MASK, kvm_host.xcr0);
> +
> +	/*
> +	 * Likewise, even if a TDX hosts didn't support XSS both arms of
> +	 * the comparison would be 0 and the wrmsrl would be skipped.
> +	 */
> +	if (kvm_host.xss != (kvm_tdx->xfam & kvm_caps.supported_xss))
> +		wrmsrl(MSR_IA32_XSS, kvm_host.xss);
> +}
> +EXPORT_SYMBOL_GPL(kvm_load_host_xsave_state);
> +
>   fastpath_t tdx_vcpu_run(struct kvm_vcpu *vcpu, bool force_immediate_exit)
>   {
>   	/*
> @@ -751,6 +776,8 @@ fastpath_t tdx_vcpu_run(struct kvm_vcpu *vcpu, bool force_immediate_exit)
>   
>   	tdx_vcpu_enter_exit(vcpu);
>   
> +	tdx_load_host_xsave_state(vcpu);
> +
>   	vcpu->arch.regs_avail &= TDX_REGS_AVAIL_SET;
>   
>   	trace_kvm_exit(vcpu, KVM_ISA_VMX);
> @@ -2326,6 +2353,11 @@ int __init tdx_bringup(void)
>   		goto success_disable_tdx;
>   	}
>   
> +	if (!cpu_feature_enabled(X86_FEATURE_OSXSAVE)) {
> +		pr_err("tdx: OSXSAVE is required for TDX\n");
> +		goto success_disable_tdx;
> +	}
> +
>   	if (!cpu_feature_enabled(X86_FEATURE_MOVDIR64B)) {
>   		pr_err("tdx: MOVDIR64B is required for TDX\n");
>   		goto success_disable_tdx;