[PATCH v8 04/30] KVM: selftests: Add vCPU descriptor table initialization utility

Sagi Shahar posted 30 patches 1 month, 4 weeks ago
There is a newer version of this series
[PATCH v8 04/30] KVM: selftests: Add vCPU descriptor table initialization utility
Posted by Sagi Shahar 1 month, 4 weeks ago
From: Ackerley Tng <ackerleytng@google.com>

Turn vCPU descriptor table initialization into a utility for use by tests
needing finer control, for example for TDX TD creation.

Signed-off-by: Ackerley Tng <ackerleytng@google.com>
Signed-off-by: Sagi Shahar <sagis@google.com>
---
 tools/testing/selftests/kvm/include/x86/processor.h | 1 +
 tools/testing/selftests/kvm/lib/x86/processor.c     | 7 ++++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/include/x86/processor.h b/tools/testing/selftests/kvm/include/x86/processor.h
index f2eb764cbd7c..37ad1e4d86ba 100644
--- a/tools/testing/selftests/kvm/include/x86/processor.h
+++ b/tools/testing/selftests/kvm/include/x86/processor.h
@@ -1178,6 +1178,7 @@ struct idt_entry {
 	uint32_t offset2; uint32_t reserved;
 };
 
+void sync_exception_handlers_to_guest(struct kvm_vm *vm);
 void vm_install_exception_handler(struct kvm_vm *vm, int vector,
 			void (*handler)(struct ex_regs *));
 
diff --git a/tools/testing/selftests/kvm/lib/x86/processor.c b/tools/testing/selftests/kvm/lib/x86/processor.c
index da6e9315ebe2..d082d429e127 100644
--- a/tools/testing/selftests/kvm/lib/x86/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86/processor.c
@@ -585,6 +585,11 @@ void route_exception(struct ex_regs *regs)
 		   regs->vector, regs->rip);
 }
 
+void sync_exception_handlers_to_guest(struct kvm_vm *vm)
+{
+	*(vm_vaddr_t *)addr_gva2hva(vm, (vm_vaddr_t)(&exception_handlers)) = vm->handlers;
+}
+
 static void vm_init_descriptor_tables(struct kvm_vm *vm)
 {
 	extern void *idt_handlers;
@@ -600,7 +605,7 @@ static void vm_init_descriptor_tables(struct kvm_vm *vm)
 	for (i = 0; i < NUM_INTERRUPTS; i++)
 		set_idt_entry(vm, i, (unsigned long)(&idt_handlers)[i], 0, KERNEL_CS);
 
-	*(vm_vaddr_t *)addr_gva2hva(vm, (vm_vaddr_t)(&exception_handlers)) = vm->handlers;
+	sync_exception_handlers_to_guest(vm);
 
 	kvm_seg_set_kernel_code_64bit(&seg);
 	kvm_seg_fill_gdt_64bit(vm, &seg);
-- 
2.51.0.rc0.155.g4a0f42376b-goog
Re: [PATCH v8 04/30] KVM: selftests: Add vCPU descriptor table initialization utility
Posted by Sean Christopherson 1 month, 3 weeks ago
On Thu, Aug 07, 2025, Sagi Shahar wrote:
> From: Ackerley Tng <ackerleytng@google.com>
> 
> Turn vCPU descriptor table initialization into a utility for use by tests
> needing finer control, for example for TDX TD creation.

NAK.  "needing finer control" is not a sufficient explanation for why _this_
patch is necessary.  There's also zero argument made throughout any of these
patches as to why this pattern:

	vm = td_create();
	td_initialize(vm, VM_MEM_SRC_ANONYMOUS, 0);
	vcpu = td_vcpu_add(vm, 0, guest_io_writes);
	td_finalize(vm);

is the best approach.  IMO it is NOT the best approach.  I would much rather we
structure things so that creating TDs can use APIs like this:

static inline struct kvm_vm *td_create_with_vcpus(uint32_t nr_vcpus,
						  void *guest_code,
						  struct kvm_vcpu *vcpus[])
{
	return __vm_create_with_vcpus(VM_SHAPE_TDX, nr_vcpus, 0, guest_code, vcpus);
}

instead of open coding an entirely different set of APIs for creating TDs, which
is not maintanable.
Re: [PATCH v8 04/30] KVM: selftests: Add vCPU descriptor table initialization utility
Posted by Sagi Shahar 1 month, 3 weeks ago
On Mon, Aug 11, 2025 at 1:25 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Thu, Aug 07, 2025, Sagi Shahar wrote:
> > From: Ackerley Tng <ackerleytng@google.com>
> >
> > Turn vCPU descriptor table initialization into a utility for use by tests
> > needing finer control, for example for TDX TD creation.
>
> NAK.  "needing finer control" is not a sufficient explanation for why _this_
> patch is necessary.  There's also zero argument made throughout any of these
> patches as to why this pattern:
>
>         vm = td_create();
>         td_initialize(vm, VM_MEM_SRC_ANONYMOUS, 0);
>         vcpu = td_vcpu_add(vm, 0, guest_io_writes);
>         td_finalize(vm);
>
> is the best approach.  IMO it is NOT the best approach.  I would much rather we
> structure things so that creating TDs can use APIs like this:
>
> static inline struct kvm_vm *td_create_with_vcpus(uint32_t nr_vcpus,
>                                                   void *guest_code,
>                                                   struct kvm_vcpu *vcpus[])
> {
>         return __vm_create_with_vcpus(VM_SHAPE_TDX, nr_vcpus, 0, guest_code, vcpus);
> }
>
> instead of open coding an entirely different set of APIs for creating TDs, which
> is not maintanable.

Dropping this patch in the next version.