[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aJoyT-w9U9lJhR-z@google.com>
Date: Mon, 11 Aug 2025 11:11:27 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Sagi Shahar <sagis@...gle.com>
Cc: linux-kselftest@...r.kernel.org, Paolo Bonzini <pbonzini@...hat.com>,
Shuah Khan <shuah@...nel.org>, Ackerley Tng <ackerleytng@...gle.com>,
Ryan Afranji <afranji@...gle.com>, Andrew Jones <ajones@...tanamicro.com>,
Isaku Yamahata <isaku.yamahata@...el.com>, Erdem Aktas <erdemaktas@...gle.com>,
Rick Edgecombe <rick.p.edgecombe@...el.com>, Roger Wang <runanwang@...gle.com>,
Binbin Wu <binbin.wu@...ux.intel.com>, Oliver Upton <oliver.upton@...ux.dev>,
"Pratik R. Sampat" <pratikrajesh.sampat@....com>, Reinette Chatre <reinette.chatre@...el.com>,
Ira Weiny <ira.weiny@...el.com>, linux-kernel@...r.kernel.org, kvm@...r.kernel.org
Subject: Re: [PATCH v8 02/30] KVM: selftests: Expose function that sets up
sregs based on VM's mode
On Thu, Aug 07, 2025, Sagi Shahar wrote:
> From: Ackerley Tng <ackerleytng@...gle.com>
>
Make changelogs standalone, i.e. don't rely on the subject/shortlog for context.
> This allows initializing sregs without setting vCPU registers in
> KVM.
>
> No functional change intended.
>
> Signed-off-by: Ackerley Tng <ackerleytng@...gle.com>
> Signed-off-by: Sagi Shahar <sagis@...gle.com>
> ---
> .../selftests/kvm/include/x86/processor.h | 1 +
> .../testing/selftests/kvm/lib/x86/processor.c | 45 ++++++++++---------
> 2 files changed, 25 insertions(+), 21 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/include/x86/processor.h b/tools/testing/selftests/kvm/include/x86/processor.h
> index b11b5a53ebd5..f2eb764cbd7c 100644
> --- a/tools/testing/selftests/kvm/include/x86/processor.h
> +++ b/tools/testing/selftests/kvm/include/x86/processor.h
> @@ -1025,6 +1025,7 @@ static inline struct kvm_cpuid2 *allocate_kvm_cpuid2(int nr_entries)
> }
>
> void vcpu_init_cpuid(struct kvm_vcpu *vcpu, const struct kvm_cpuid2 *cpuid);
> +void vcpu_setup_mode_sregs(struct kvm_vm *vm, struct kvm_sregs *sregs);
>
> static inline void vcpu_get_cpuid(struct kvm_vcpu *vcpu)
> {
> diff --git a/tools/testing/selftests/kvm/lib/x86/processor.c b/tools/testing/selftests/kvm/lib/x86/processor.c
> index a92dc1dad085..002303e2a572 100644
> --- a/tools/testing/selftests/kvm/lib/x86/processor.c
> +++ b/tools/testing/selftests/kvm/lib/x86/processor.c
> @@ -488,34 +488,37 @@ static void kvm_seg_set_tss_64bit(vm_vaddr_t base, struct kvm_segment *segp)
> segp->present = 1;
> }
>
> -static void vcpu_init_sregs(struct kvm_vm *vm, struct kvm_vcpu *vcpu)
> +void vcpu_setup_mode_sregs(struct kvm_vm *vm, struct kvm_sregs *sregs)
> {
> - struct kvm_sregs sregs;
> -
> TEST_ASSERT_EQ(vm->mode, VM_MODE_PXXV48_4K);
>
> - /* Set mode specific system register values. */
> - vcpu_sregs_get(vcpu, &sregs);
> -
> - sregs.idt.base = vm->arch.idt;
> - sregs.idt.limit = NUM_INTERRUPTS * sizeof(struct idt_entry) - 1;
> - sregs.gdt.base = vm->arch.gdt;
> - sregs.gdt.limit = getpagesize() - 1;
> + sregs->idt.base = vm->arch.idt;
> + sregs->idt.limit = NUM_INTERRUPTS * sizeof(struct idt_entry) - 1;
> + sregs->gdt.base = vm->arch.gdt;
> + sregs->gdt.limit = getpagesize() - 1;
>
> - sregs.cr0 = X86_CR0_PE | X86_CR0_NE | X86_CR0_PG;
> - sregs.cr4 |= X86_CR4_PAE | X86_CR4_OSFXSR;
> + sregs->cr0 = X86_CR0_PE | X86_CR0_NE | X86_CR0_PG;
> + sregs->cr4 |= X86_CR4_PAE | X86_CR4_OSFXSR;
> if (kvm_cpu_has(X86_FEATURE_XSAVE))
> - sregs.cr4 |= X86_CR4_OSXSAVE;
> - sregs.efer |= (EFER_LME | EFER_LMA | EFER_NX);
> + sregs->cr4 |= X86_CR4_OSXSAVE;
> + sregs->efer |= (EFER_LME | EFER_LMA | EFER_NX);
> +
> + kvm_seg_set_unusable(&sregs->ldt);
> + kvm_seg_set_kernel_code_64bit(&sregs->cs);
> + kvm_seg_set_kernel_data_64bit(&sregs->ds);
> + kvm_seg_set_kernel_data_64bit(&sregs->es);
> + kvm_seg_set_kernel_data_64bit(&sregs->gs);
> + kvm_seg_set_tss_64bit(vm->arch.tss, &sregs->tr);
>
> - kvm_seg_set_unusable(&sregs.ldt);
> - kvm_seg_set_kernel_code_64bit(&sregs.cs);
> - kvm_seg_set_kernel_data_64bit(&sregs.ds);
> - kvm_seg_set_kernel_data_64bit(&sregs.es);
> - kvm_seg_set_kernel_data_64bit(&sregs.gs);
> - kvm_seg_set_tss_64bit(vm->arch.tss, &sregs.tr);
> + sregs->cr3 = vm->pgd;
Add helpers/macros for the few things that are open coded here so that the TDX
code can get the "default" values. Bouncing data through kvm_sregs is unnecessary
(unless you're trying to win the Obfuscated C contest) and makes it much harder to
understand what TDX actually needs, and why.
IDT.base, GDT.base, and CR3 (vm->pgd) are already available, so something like:
diff --git a/tools/testing/selftests/kvm/lib/x86/processor.c b/tools/testing/selftests/kvm/lib/x86/processor.c
index d4c19ac885a9..83efcf48faad 100644
--- a/tools/testing/selftests/kvm/lib/x86/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86/processor.c
@@ -498,15 +498,13 @@ static void vcpu_init_sregs(struct kvm_vm *vm, struct kvm_vcpu *vcpu)
vcpu_sregs_get(vcpu, &sregs);
sregs.idt.base = vm->arch.idt;
- sregs.idt.limit = NUM_INTERRUPTS * sizeof(struct idt_entry) - 1;
+ sregs.idt.limit = kvm_get_default_idt_limit();
sregs.gdt.base = vm->arch.gdt;
- sregs.gdt.limit = getpagesize() - 1;
+ sregs.gdt.limit = kvm_get_default_gdt_limit();
- sregs.cr0 = X86_CR0_PE | X86_CR0_NE | X86_CR0_PG;
- sregs.cr4 |= X86_CR4_PAE | X86_CR4_OSFXSR;
- if (kvm_cpu_has(X86_FEATURE_XSAVE))
- sregs.cr4 |= X86_CR4_OSXSAVE;
- sregs.efer |= (EFER_LME | EFER_LMA | EFER_NX);
+ sregs.cr0 = kvm_get_default_cr0();
+ sregs.cr4 |= kvm_get_default_cr4();
+ sregs.efer |= kvm_get_default_efer();
kvm_seg_set_unusable(&sregs.ldt);
kvm_seg_set_kernel_code_64bit(&sregs.cs);
> +}
> +
> +static void vcpu_init_sregs(struct kvm_vm *vm, struct kvm_vcpu *vcpu)
> +{
> + struct kvm_sregs sregs;
>
> - sregs.cr3 = vm->pgd;
> + vcpu_sregs_get(vcpu, &sregs);
> + vcpu_setup_mode_sregs(vm, &sregs);
> vcpu_sregs_set(vcpu, &sregs);
> }
>
> --
> 2.51.0.rc0.155.g4a0f42376b-goog
>
Powered by blists - more mailing lists