Re: [PATCH 2/2] KVM: selftests: Reuse kvm_setup_gdt in vcpu_init_descriptor_tables

[Sean Christopherson <seanjc@google.com>]

On Sat, Jan 14, 2023, Ackerley Tng wrote:
> Refactor vcpu_init_descriptor_tables to use kvm_setup_gdt
> 
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> ---
>  tools/testing/selftests/kvm/lib/x86_64/processor.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c
> index 33ca7f5232a4..8d544e9237aa 100644
> --- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
> +++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
> @@ -1119,8 +1119,7 @@ void vcpu_init_descriptor_tables(struct kvm_vcpu *vcpu)
>  	vcpu_sregs_get(vcpu, &sregs);
>  	sregs.idt.base = vm->idt;
>  	sregs.idt.limit = NUM_INTERRUPTS * sizeof(struct idt_entry) - 1;
> -	sregs.gdt.base = vm->gdt;
> -	sregs.gdt.limit = getpagesize() - 1;
> +	kvm_setup_gdt(vcpu->vm, &sregs.gdt);

*sigh*

The selftests infrastructure is so misguided.  Forcing tests to opt-in to
installing an IDT just to avoid allocating two pages is such an awful tradeoff.

Now that we have kvm_arch_vm_post_create(), I think we should always allocate
the GDT, IDT, and handlers, and then vCPU setup/creation can simply grab the
already-allocated values and stuff them into KVM.  That would then eliminate
kvm_setup_gdt() entirely.

And much of the setup code is also backwards and unnecessarily thread-unsafe, e.g.
vCPU initialization shouldn't need to fill GDT entries.

So, while I agree that using kvm_setup_gdt() is a good change on its own, I'd
rather go the more aggressive route and clean up the underlying mess.

I'll send patches sometime this week, unfortunately typing up what I have in mind
is harder than just reworking the code :-/