On Mon, Aug 11, 2025 at 1:11 PM Sean Christopherson <sea...@google.com> wrote:
>
> On Thu, Aug 07, 2025, Sagi Shahar wrote:
> > From: Ackerley Tng <ackerley...@google.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 <ackerley...@google.com>
> > Signed-off-by: Sagi Shahar <sa...@google.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);
>
>
Thanks for the suggestion. I'll be dropping this patch in the next version.
> > +}
> > +
> > +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
> >
