On Wed, Jun 17, 2026 at 11:21:49AM +0800, Xiaoyao Li wrote:
> > +++ b/tools/testing/selftests/kvm/include/x86/tdx/tdx_util.h
> > @@ -11,4 +11,34 @@ static inline bool is_tdx_vm(struct kvm_vm *vm)
> >     return vm->type == KVM_X86_TDX_VM;
> >   }

Thanks for the review. I will change all comments for this patch I did
not reply here in the next version.

> > +/*
> > + * TDX ioctls
> > + * Use underscores to avoid collisions with struct member names.
> > + */
> > +#define __tdx_vm_ioctl(vm, cmd, _flags, arg)                               
> > \
> > +({                                                                 \
> > +   int r;                                                          \
> > +                                                                   \
> > +   union {                                                         \
> > +           struct kvm_tdx_cmd c;                                   \
> > +           unsigned long raw;                                      \
> > +   } tdx_cmd = { .c = {                                            \
> > +           .id = (cmd),                                            \
> > +           .flags = (u32)(_flags),                         \
> > +           .data = (u64)(arg),                             \
> > +   } };                                                            \
> > +                                                                   \
> > +   r = __vm_ioctl(vm, KVM_MEMORY_ENCRYPT_OP, &tdx_cmd.raw);        \
> > +   r ?: tdx_cmd.c.hw_error;                                        \
> > +})
> 
> It looks __tdx_vm_ioctl() can be implemented as the static inline function.
> 
> Given all the existing xxx_ioctl() are implmeneted as MACRO, I'm OK with it.

Most vm_ioctl() and vcpu_ioctl() used MACRO.
I prefer to use MACRO for `tdx_vm_ioctl()` for the `#cmd`, but I am ok
to change `__tdx_vm_ioctl()` to static inline function.
If we can only change part of it, do you think it is better to change it
as the static inline fucntion ?

> > +}
> > +
> > +static struct kvm_cpuid_entry2 *tdx_find_cpuid_config(struct 
> > kvm_tdx_capabilities *cap,
> > +                                                 u32 leaf, u32 sub_leaf)
> > +{
> > +   struct kvm_cpuid_entry2 *config;
> > +   u32 i;
> > +
> > +   for (i = 0; i < cap->cpuid.nent; i++) {
> > +           config = &cap->cpuid.entries[i];
> > +
> > +           if (config->function == leaf && config->index == sub_leaf)
> > +                   return config;
> > +   }
> > +
> > +   return NULL;
> > +}
> 
> No need to introduce a new fucntin. We can use get_cpuid_entry().

I think we cannot use `get_cpuid_entry()` directly here.
This function is called by `tdx_filter_cpuid()` to check if KVM's
supported CPUID entries are present in the TDX capabilities list. Since
the TDX list only contains a subset of CPUID leaves, some queries will
naturally return NULL (which we want to gracefully filter out) However,
`get_cpuid_entry()` has an embedded TEST_FAIL(), which would abort for
any missing leaf.
How about I add a `__get_cpuid_entry()` to share the same part of them?

Lisa

Reply via email to