On Thu, Aug 07, 2025, Sagi Shahar wrote:
> @@ -189,3 +199,19 @@ uint64_t tdg_vp_info(uint64_t *rcx, uint64_t *rdx,
>
> return ret;
> }
> +
> +uint64_t tdg_vp_vmcall_map_gpa(uint64_t address, uint64_t size, uint64_t
> *data_out)
> +{
> + struct tdx_hypercall_args args = {
> + .r11 = TDG_VP_VMCALL_MAP_GPA,
> + .r12 = address,
> + .r13 = size
> + };
> + uint64_t ret;
> +
> + ret = __tdx_hypercall(&args, TDX_HCALL_HAS_OUTPUT);
> +
> + if (data_out)
> + *data_out = args.r11;
> + return ret;
Assert instead of returning the error. If there's a use for negative tests,
then
add a double-underscores variant. And drop @data_out, IIUC, it's only relevant
on failure.
> +}
> 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 5e4455be828a..c5bee67099c5 100644
> --- a/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
> +++ b/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
> @@ -608,4 +608,36 @@ void td_finalize(struct kvm_vm *vm)
> void td_vcpu_run(struct kvm_vcpu *vcpu)
> {
> vcpu_run(vcpu);
> +
> + /* Handle TD VMCALLs that require userspace handling. */
> + if (vcpu->run->exit_reason == KVM_EXIT_HYPERCALL &&
> + vcpu->run->hypercall.nr == KVM_HC_MAP_GPA_RANGE) {
Unnecessary curly braces.
> + handle_userspace_map_gpa(vcpu);
> + }
> +}
> +
> +/*
> + * Handle conversion of memory with @size beginning @gpa for @vm. Set
> + * @shared_to_private to true for shared to private conversions and false
> + * otherwise.
> + *
> + * Since this is just for selftests, just keep both pieces of backing
> + * memory allocated and not deallocate/allocate memory; just do the
> + * minimum of calling KVM_MEMORY_ENCRYPT_REG_REGION and
> + * KVM_MEMORY_ENCRYPT_UNREG_REGION.
> + */
> +void handle_memory_conversion(struct kvm_vm *vm, uint32_t vcpu_id, uint64_t
> gpa,
> + uint64_t size, bool shared_to_private)
So, vm_set_memory_attributes()?
> +{
> + struct kvm_memory_attributes range;
> +
> + range.address = gpa;
> + range.size = size;
> + range.attributes = shared_to_private ? KVM_MEMORY_ATTRIBUTE_PRIVATE : 0;
> + range.flags = 0;
> +
> + pr_debug("\t... call KVM_SET_MEMORY_ATTRIBUTES ioctl from vCPU %u with
> gpa=%#lx, size=%#lx, attributes=%#llx\n",
> + vcpu_id, gpa, size, range.attributes);
Drop these types of prints. strace can probably do the job 99% of the time, and
for the remaining 1%, I doubt this help all that much.
> +
> + vm_ioctl(vm, KVM_SET_MEMORY_ATTRIBUTES, &range);
> }
> +void guest_shared_mem(void)
> +{
> + uint32_t *test_mem_shared_gva =
> + (uint32_t *)TDX_SHARED_MEM_TEST_SHARED_GVA;
> +
> + uint64_t placeholder;
> + uint64_t ret;
> +
> + /* Map gpa as shared */
> + ret = tdg_vp_vmcall_map_gpa(test_mem_shared_gpa, PAGE_SIZE,
> + &placeholder);
> + if (ret)
> + tdx_test_fatal_with_data(ret, __LINE__);
> +
> + *test_mem_shared_gva = TDX_SHARED_MEM_TEST_GUEST_WRITE_VALUE;
> +
> + /* Exit so host can read shared value */
> + ret = tdg_vp_vmcall_instruction_io(TDX_SHARED_MEM_TEST_INFO_PORT, 4,
> + PORT_WRITE, &placeholder);
> + if (ret)
GUEST_ASSERT(). Don't use TDX's "fatal error" crud to report test failures.
> + tdx_test_fatal_with_data(ret, __LINE__);
> +
> + /* Read value written by host and send it back out for verification */
> + ret = tdg_vp_vmcall_instruction_io(TDX_SHARED_MEM_TEST_INFO_PORT, 4,
> + PORT_WRITE,
> + (uint64_t *)test_mem_shared_gva);
> + if (ret)
> + tdx_test_fatal_with_data(ret, __LINE__);
> +}
> +
> +int verify_shared_mem(void)
> +{
> + vm_vaddr_t test_mem_private_gva;
> + uint64_t test_mem_private_gpa;
> + uint32_t *test_mem_hva;
> + struct kvm_vcpu *vcpu;
> + struct kvm_vm *vm;
> +
> + vm = td_create();
> + td_initialize(vm, VM_MEM_SRC_ANONYMOUS, 0);
> + vcpu = td_vcpu_add(vm, 0, guest_shared_mem);
> +
> + /*
> + * Set up shared memory page for testing by first allocating as private
> + * and then mapping the same GPA again as shared. This way, the TD does
> + * not have to remap its page tables at runtime.
> + */
> + test_mem_private_gva = vm_vaddr_alloc(vm, vm->page_size,
> + TDX_SHARED_MEM_TEST_PRIVATE_GVA);
> + TEST_ASSERT_EQ(test_mem_private_gva, TDX_SHARED_MEM_TEST_PRIVATE_GVA);
> +
> + test_mem_hva = addr_gva2hva(vm, test_mem_private_gva);
> + TEST_ASSERT(test_mem_hva,
> + "Guest address not found in guest memory regions\n");
> +
> + test_mem_private_gpa = addr_gva2gpa(vm, test_mem_private_gva);
> + virt_map_shared(vm, TDX_SHARED_MEM_TEST_SHARED_GVA,
> test_mem_private_gpa, 1);
> +
> + test_mem_shared_gpa = test_mem_private_gpa | vm->arch.s_bit;
> + sync_global_to_guest(vm, test_mem_shared_gpa);
> +
> + td_finalize(vm);
> +
> + vm_enable_cap(vm, KVM_CAP_EXIT_HYPERCALL,
> BIT_ULL(KVM_HC_MAP_GPA_RANGE));
> +
> + printf("Verifying shared memory accesses for TDX\n");
> +
> + /* Begin guest execution; guest writes to shared memory. */
> + printf("\t ... Starting guest execution\n");
> +
> + /* Handle map gpa as shared */
> + tdx_run(vcpu);
> +
> + tdx_run(vcpu);
> + tdx_test_assert_io(vcpu, TDX_SHARED_MEM_TEST_INFO_PORT, 4, PORT_WRITE);
AFAICT, there's nothing TDX-specific about these assert helpers. And I would
prefer they be macros; while ugly, macros provide precise file+line information,
i.e. don't require a stack trace.
Maybe TEST_ASSERT_EXIT_IO() and TEST_ASSERT_EXIT_MMIO()?
> + TEST_ASSERT_EQ(*test_mem_hva, TDX_SHARED_MEM_TEST_GUEST_WRITE_VALUE);
> +
> + *test_mem_hva = TDX_SHARED_MEM_TEST_HOST_WRITE_VALUE;
> + tdx_run(vcpu);
> + tdx_test_assert_io(vcpu, TDX_SHARED_MEM_TEST_INFO_PORT, 4, PORT_WRITE);
> + TEST_ASSERT_EQ(*(uint32_t *)((void *)vcpu->run +
> vcpu->run->io.data_offset),
> + TDX_SHARED_MEM_TEST_HOST_WRITE_VALUE);
> +
> + printf("\t ... PASSED\n");
No. Printing PASSED is worse than useless. Exit codes exist for a reason; this
is pure spam. pr_debug() if necessary, but all of these printfs can probably be
dropped.
> +
> + kvm_vm_free(vm);
> +
> + return 0;
> +}
> +
> +int main(int argc, char **argv)
> +{
> + if (!is_tdx_enabled()) {
> + printf("TDX is not supported by the KVM\n"
> + "Skipping the TDX tests.\n");
> + return 0;
TEST_REQUIRE()
> + }
> +
> + return verify_shared_mem();
> +}
> --
> 2.51.0.rc0.155.g4a0f42376b-goog
>
