Sagi Shahar wrote:
[snip]
diff --git a/tools/testing/selftests/kvm/include/x86/tdx/tdx_util.h b/tools/testing/selftests/kvm/include/x86/tdx/tdx_util.h index dafdc7e46abe..a2509959c7ce 100644 --- a/tools/testing/selftests/kvm/include/x86/tdx/tdx_util.h +++ b/tools/testing/selftests/kvm/include/x86/tdx/tdx_util.h @@ -11,6 +11,60 @@ static inline bool is_tdx_vm(struct kvm_vm *vm) return vm->type == KVM_X86_TDX_VM; } +/*
- TDX ioctls
- */
+#define __vm_tdx_vm_ioctl(vm, cmd, metadata, arg) \
NIT: Why not call 'metadata' -> 'flags'?
+({ \
- int r; \
\- union { \
struct kvm_tdx_cmd c; \unsigned long raw; \- } tdx_cmd = { .c = { \
.id = (cmd), \.flags = (uint32_t)(metadata), \.data = (uint64_t)(arg), \- } }; \
\- r = __vm_ioctl(vm, KVM_MEMORY_ENCRYPT_OP, &tdx_cmd.raw); \
- r ?: tdx_cmd.c.hw_error; \
+})
I see this is a common pattern for kvm selftests but I'm struggling to figure out why this is a macro and not a function call?
+#define vm_tdx_vm_ioctl(vm, cmd, flags, arg) \ +({ \
- int ret = __vm_tdx_vm_ioctl(vm, cmd, flags, arg); \
\- __TEST_ASSERT_VM_VCPU_IOCTL(!ret, #cmd, ret, vm); \
+})
+#define __vm_tdx_vcpu_ioctl(vcpu, cmd, metadata, arg) \
NIT: Why not just call 'metadata', 'flags'?
+({ \
- int r; \
\- union { \
struct kvm_tdx_cmd c; \unsigned long raw; \- } tdx_cmd = { .c = { \
.id = (cmd), \.flags = (uint32_t)(metadata), \.data = (uint64_t)(arg), \- } }; \
\- r = __vcpu_ioctl(vcpu, KVM_MEMORY_ENCRYPT_OP, &tdx_cmd.raw); \
- r ?: tdx_cmd.c.hw_error; \
+})
[snip]
+static struct kvm_tdx_capabilities *tdx_read_capabilities(struct kvm_vm *vm) +{
- struct kvm_tdx_capabilities *tdx_cap = NULL;
- int nr_cpuid_configs = 4;
- int rc = -1;
- int i;
- do {
nr_cpuid_configs *= 2;tdx_cap = realloc(tdx_cap, sizeof(*tdx_cap) +sizeof(tdx_cap->cpuid) +(sizeof(struct kvm_cpuid_entry2) * nr_cpuid_configs));TEST_ASSERT(tdx_cap,"Could not allocate memory for tdx capability nr_cpuid_configs %d\n",nr_cpuid_configs);tdx_cap->cpuid.nent = nr_cpuid_configs;rc = __vm_tdx_vm_ioctl(vm, KVM_TDX_CAPABILITIES, 0, tdx_cap);
Why not use vm_tdx_vm_ioctl()?
Generally though it is good.
Reviewed-by: Ira Weiny ira.weiny@intel.com
[snip]