[PATCH v8 07/30] KVM: selftests: TDX: Use KVM_TDX_CAPABILITIES to validate TDs' attribute configuration

Sagi Shahar posted 30 patches 1 month, 4 weeks ago
There is a newer version of this series
[PATCH v8 07/30] KVM: selftests: TDX: Use KVM_TDX_CAPABILITIES to validate TDs' attribute configuration
Posted by Sagi Shahar 1 month, 4 weeks ago
From: Ackerley Tng <ackerleytng@google.com>

This also exercises the KVM_TDX_CAPABILITIES ioctl.

Suggested-by: Isaku Yamahata <isaku.yamahata@intel.com>
Co-developed-by: Isaku Yamahata <isaku.yamahata@intel.com>
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
Signed-off-by: Ackerley Tng <ackerleytng@google.com>
Signed-off-by: Sagi Shahar <sagis@google.com>
---
 .../selftests/kvm/lib/x86/tdx/tdx_util.c        | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c b/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
index 392d6272d17e..bb074af4a476 100644
--- a/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
+++ b/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
@@ -140,6 +140,21 @@ static void tdx_apply_cpuid_restrictions(struct kvm_cpuid2 *cpuid_data)
 	}
 }
 
+static void tdx_check_attributes(struct kvm_vm *vm, uint64_t attributes)
+{
+	struct kvm_tdx_capabilities *tdx_cap;
+
+	tdx_cap = tdx_read_capabilities(vm);
+
+	/* TDX spec: any bits 0 in supported_attrs must be 0 in attributes */
+	TEST_ASSERT_EQ(attributes & ~tdx_cap->supported_attrs, 0);
+
+	/* TDX spec: any bits 1 in attributes must be 1 in supported_attrs */
+	TEST_ASSERT_EQ(attributes & tdx_cap->supported_attrs, attributes);
+
+	free(tdx_cap);
+}
+
 #define KVM_MAX_CPUID_ENTRIES 256
 
 #define CPUID_EXT_VMX			BIT(5)
@@ -256,6 +271,8 @@ static void tdx_td_init(struct kvm_vm *vm, uint64_t attributes)
 	memcpy(&init_vm->cpuid, cpuid, kvm_cpuid2_size(cpuid->nent));
 	free(cpuid);
 
+	tdx_check_attributes(vm, attributes);
+
 	init_vm->attributes = attributes;
 
 	tdx_apply_cpuid_restrictions(&init_vm->cpuid);
-- 
2.51.0.rc0.155.g4a0f42376b-goog
Re: [PATCH v8 07/30] KVM: selftests: TDX: Use KVM_TDX_CAPABILITIES to validate TDs' attribute configuration
Posted by Chenyi Qiang 1 month, 3 weeks ago

On 8/8/2025 4:16 AM, Sagi Shahar wrote:
> From: Ackerley Tng <ackerleytng@google.com>
> 
> This also exercises the KVM_TDX_CAPABILITIES ioctl.
> 
> Suggested-by: Isaku Yamahata <isaku.yamahata@intel.com>
> Co-developed-by: Isaku Yamahata <isaku.yamahata@intel.com>
> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> Signed-off-by: Sagi Shahar <sagis@google.com>
> ---
>  .../selftests/kvm/lib/x86/tdx/tdx_util.c        | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c b/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
> index 392d6272d17e..bb074af4a476 100644
> --- a/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
> +++ b/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
> @@ -140,6 +140,21 @@ static void tdx_apply_cpuid_restrictions(struct kvm_cpuid2 *cpuid_data)
>  	}
>  }
>  
> +static void tdx_check_attributes(struct kvm_vm *vm, uint64_t attributes)
> +{
> +	struct kvm_tdx_capabilities *tdx_cap;
> +
> +	tdx_cap = tdx_read_capabilities(vm);
> +
> +	/* TDX spec: any bits 0 in supported_attrs must be 0 in attributes */
> +	TEST_ASSERT_EQ(attributes & ~tdx_cap->supported_attrs, 0);
> +
> +	/* TDX spec: any bits 1 in attributes must be 1 in supported_attrs */
> +	TEST_ASSERT_EQ(attributes & tdx_cap->supported_attrs, attributes);
> +
> +	free(tdx_cap);
> +}
> +
>  #define KVM_MAX_CPUID_ENTRIES 256
>  
>  #define CPUID_EXT_VMX			BIT(5)
> @@ -256,6 +271,8 @@ static void tdx_td_init(struct kvm_vm *vm, uint64_t attributes)
>  	memcpy(&init_vm->cpuid, cpuid, kvm_cpuid2_size(cpuid->nent));
>  	free(cpuid);
>  
> +	tdx_check_attributes(vm, attributes);
> +
>  	init_vm->attributes = attributes;
>  
>  	tdx_apply_cpuid_restrictions(&init_vm->cpuid);

Do we need to set the init_vm->xfam based on cpuid.0xd and validate it with tdx_cap->supported_xfam?
Re: [PATCH v8 07/30] KVM: selftests: TDX: Use KVM_TDX_CAPABILITIES to validate TDs' attribute configuration
Posted by Sagi Shahar 1 month, 2 weeks ago
On Wed, Aug 13, 2025 at 8:34 AM Chenyi Qiang <chenyi.qiang@intel.com> wrote:
>
>
>
> On 8/8/2025 4:16 AM, Sagi Shahar wrote:
> > From: Ackerley Tng <ackerleytng@google.com>
> >
> > This also exercises the KVM_TDX_CAPABILITIES ioctl.
> >
> > Suggested-by: Isaku Yamahata <isaku.yamahata@intel.com>
> > Co-developed-by: Isaku Yamahata <isaku.yamahata@intel.com>
> > Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> > Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> > Signed-off-by: Sagi Shahar <sagis@google.com>
> > ---
> >  .../selftests/kvm/lib/x86/tdx/tdx_util.c        | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> >
> > diff --git a/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c b/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
> > index 392d6272d17e..bb074af4a476 100644
> > --- a/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
> > +++ b/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
> > @@ -140,6 +140,21 @@ static void tdx_apply_cpuid_restrictions(struct kvm_cpuid2 *cpuid_data)
> >       }
> >  }
> >
> > +static void tdx_check_attributes(struct kvm_vm *vm, uint64_t attributes)
> > +{
> > +     struct kvm_tdx_capabilities *tdx_cap;
> > +
> > +     tdx_cap = tdx_read_capabilities(vm);
> > +
> > +     /* TDX spec: any bits 0 in supported_attrs must be 0 in attributes */
> > +     TEST_ASSERT_EQ(attributes & ~tdx_cap->supported_attrs, 0);
> > +
> > +     /* TDX spec: any bits 1 in attributes must be 1 in supported_attrs */
> > +     TEST_ASSERT_EQ(attributes & tdx_cap->supported_attrs, attributes);
> > +
> > +     free(tdx_cap);
> > +}
> > +
> >  #define KVM_MAX_CPUID_ENTRIES 256
> >
> >  #define CPUID_EXT_VMX                        BIT(5)
> > @@ -256,6 +271,8 @@ static void tdx_td_init(struct kvm_vm *vm, uint64_t attributes)
> >       memcpy(&init_vm->cpuid, cpuid, kvm_cpuid2_size(cpuid->nent));
> >       free(cpuid);
> >
> > +     tdx_check_attributes(vm, attributes);
> > +
> >       init_vm->attributes = attributes;
> >
> >       tdx_apply_cpuid_restrictions(&init_vm->cpuid);
>
> Do we need to set the init_vm->xfam based on cpuid.0xd and validate it with tdx_cap->supported_xfam?
>
I don't think it's necessary. And according to the TDX spec (TDX
Module Base Spec - 11.8.3. Extended Features Execution Control) the
mapping from CPUID to XFAM is not trivial. Checking attributes makes
sense since some tests use non-default attributes but right now we
don't have any test which uses XFAM features. We can add XFAM support
in the future if it's needed and do the check then.
Re: [PATCH v8 07/30] KVM: selftests: TDX: Use KVM_TDX_CAPABILITIES to validate TDs' attribute configuration
Posted by Sagi Shahar 1 month, 2 weeks ago
On Wed, Aug 20, 2025 at 4:18 PM Sagi Shahar <sagis@google.com> wrote:
>
> On Wed, Aug 13, 2025 at 8:34 AM Chenyi Qiang <chenyi.qiang@intel.com> wrote:
> >
> >
> >
> > On 8/8/2025 4:16 AM, Sagi Shahar wrote:
> > > From: Ackerley Tng <ackerleytng@google.com>
> > >
> > > This also exercises the KVM_TDX_CAPABILITIES ioctl.
> > >
> > > Suggested-by: Isaku Yamahata <isaku.yamahata@intel.com>
> > > Co-developed-by: Isaku Yamahata <isaku.yamahata@intel.com>
> > > Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> > > Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> > > Signed-off-by: Sagi Shahar <sagis@google.com>
> > > ---
> > >  .../selftests/kvm/lib/x86/tdx/tdx_util.c        | 17 +++++++++++++++++
> > >  1 file changed, 17 insertions(+)
> > >
> > > diff --git a/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c b/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
> > > index 392d6272d17e..bb074af4a476 100644
> > > --- a/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
> > > +++ b/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
> > > @@ -140,6 +140,21 @@ static void tdx_apply_cpuid_restrictions(struct kvm_cpuid2 *cpuid_data)
> > >       }
> > >  }
> > >
> > > +static void tdx_check_attributes(struct kvm_vm *vm, uint64_t attributes)
> > > +{
> > > +     struct kvm_tdx_capabilities *tdx_cap;
> > > +
> > > +     tdx_cap = tdx_read_capabilities(vm);
> > > +
> > > +     /* TDX spec: any bits 0 in supported_attrs must be 0 in attributes */
> > > +     TEST_ASSERT_EQ(attributes & ~tdx_cap->supported_attrs, 0);
> > > +
> > > +     /* TDX spec: any bits 1 in attributes must be 1 in supported_attrs */
> > > +     TEST_ASSERT_EQ(attributes & tdx_cap->supported_attrs, attributes);
> > > +
> > > +     free(tdx_cap);
> > > +}
> > > +
> > >  #define KVM_MAX_CPUID_ENTRIES 256
> > >
> > >  #define CPUID_EXT_VMX                        BIT(5)
> > > @@ -256,6 +271,8 @@ static void tdx_td_init(struct kvm_vm *vm, uint64_t attributes)
> > >       memcpy(&init_vm->cpuid, cpuid, kvm_cpuid2_size(cpuid->nent));
> > >       free(cpuid);
> > >
> > > +     tdx_check_attributes(vm, attributes);
> > > +
> > >       init_vm->attributes = attributes;
> > >
> > >       tdx_apply_cpuid_restrictions(&init_vm->cpuid);
> >
> > Do we need to set the init_vm->xfam based on cpuid.0xd and validate it with tdx_cap->supported_xfam?
> >
> I don't think it's necessary. And according to the TDX spec (TDX
> Module Base Spec - 11.8.3. Extended Features Execution Control) the
> mapping from CPUID to XFAM is not trivial. Checking attributes makes
> sense since some tests use non-default attributes but right now we
> don't have any test which uses XFAM features. We can add XFAM support
> in the future if it's needed and do the check then.

I just saw the comment on "KVM: selftests: TDX: Add basic TDX CPUID
test" which suggests adding xfam support. I can add a check for xfam
when I rework that patch but I still think that the values for xfam
should come from the test and validated here instead of being
calculated based on cpuid.