1. Patchew
  2. linux
  3. [PATCH v3 00/10] KVM: TDX: TD vcpu enter/exit
  4. View patch

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

Paolo Bonzini posted 10 patches 9 months, 1 week ago
[PATCH v3 05/10] KVM: TDX: restore host xsave state when exit from the guest TD
Posted by Paolo Bonzini 9 months, 1 week ago 
From: Isaku Yamahata <isaku.yamahata@intel.com>

On exiting from the guest TD, xsave state is clobbered; restore it.

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;
-- 
2.43.5
Re: [PATCH v3 05/10] KVM: TDX: restore host xsave state when exit from the guest TD
Posted by Xiaoyao Li 9 months, 1 week ago 
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;
Re: [PATCH v3 05/10] KVM: TDX: restore host xsave state when exit from the guest TD
Posted by Paolo Bonzini 9 months ago 
On Mon, Mar 10, 2025 at 8:24 AM Xiaoyao Li <xiaoyao.li@intel.com> wrote:
>
> 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)

Ok:

Do not use kvm_load_host_xsave_state(), as it relies on vcpu->arch
to find out whether other KVM_RUN code has loaded guest state into
XCR0/PKRU/XSS or not.  In the case of TDX, the exit values are known
independent of the guest CR0 and CR4, and in fact the latter are not
available.

Thanks!

Paolo
Re: [PATCH v3 05/10] KVM: TDX: restore host xsave state when exit from the guest TD
Posted by Xiaoyao Li 9 months ago 
On 3/12/2025 7:36 PM, Paolo Bonzini wrote:
> On Mon, Mar 10, 2025 at 8:24 AM Xiaoyao Li <xiaoyao.li@intel.com> wrote:
>>
>> 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)
> 
> Ok:
> 
> Do not use kvm_load_host_xsave_state(), as it relies on vcpu->arch
> to find out whether other KVM_RUN code has loaded guest state into
> XCR0/PKRU/XSS or not.  In the case of TDX, the exit values are known
> independent of the guest CR0 and CR4, and in fact the latter are not
> available.

In fact, I expected some description of how xsave state is clobbered and 
what value of them after TD exit.

   After return from TDH.VP.ENTER, XCR0 is set to TD's user-mode feature
   bits of XFAM and MSR_IA32_XSS is set to TD's supervisor-mode feature
   bits of XFAM. PKRU keeps unchanged if the TD is not exposed with PKU
   in XFAM or PKRU is set to 0 when XFAM.PKE(bit 9) is 1.

If the changelog has the description of TDX module, it indeed can help 
people understand the code.

> Thanks!
> 
> Paolo
>
Re: [PATCH v3 05/10] KVM: TDX: restore host xsave state when exit from the guest TD
Posted by Paolo Bonzini 9 months ago 
On Thu, Mar 13, 2025 at 4:17 AM Xiaoyao Li <xiaoyao.li@intel.com> wrote:
>
> On 3/12/2025 7:36 PM, Paolo Bonzini wrote:
> > On Mon, Mar 10, 2025 at 8:24 AM Xiaoyao Li <xiaoyao.li@intel.com> wrote:
> >>
> >> 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)
> >
> > Ok:
> >
> > Do not use kvm_load_host_xsave_state(), as it relies on vcpu->arch
> > to find out whether other KVM_RUN code has loaded guest state into
> > XCR0/PKRU/XSS or not.  In the case of TDX, the exit values are known
> > independent of the guest CR0 and CR4, and in fact the latter are not
> > available.
>
> In fact, I expected some description of how xsave state is clobbered and
> what value of them after TD exit.
>
>    After return from TDH.VP.ENTER, XCR0 is set to TD's user-mode feature
>    bits of XFAM and MSR_IA32_XSS is set to TD's supervisor-mode feature
>    bits of XFAM. PKRU keeps unchanged if the TD is not exposed with PKU
>    in XFAM or PKRU is set to 0 when XFAM.PKE(bit 9) is 1.

Ah, I didn't include that because it's just information from the TDX
module documentation.

Paolo

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.