Re: [PATCH v13 03/22] KVM: selftests: Initialize the TDX VM

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

On 6/25/2026 7:50 AM, Lisa Wang wrote:
> 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 ?

Personally, I like function over MARCO, but it doesn't make a huge 
difference either way. Let's keep it as-is and move on.

>>> +}
>>> +
>>> +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?

yes, please.

> Lisa
>