Re: [V2 PATCH 3/8] KVM: selftests: Add arch specific post vm load setup

[Sean Christopherson <seanjc@google.com>]

On Mon, Oct 03, 2022, Vishal Annapurve wrote:
> On Mon, Oct 3, 2022 at 8:42 AM Sean Christopherson <seanjc@google.com> wrote:
> > Even better, call it from __vm_create() and name it something like
> > kvm_arch_vm_post_create().  Like David said, while the hook has a dependency on
> > being called after loading the ELF image, the action that arch code is expected
> > to take doesn't have anything to do with loading the ELF image.
> >
> > And then instead of introducing an arch hook with no implementation, the patch that
> > adds the hook can instead use it to replace the x86-64 #ifdef in __vm_create().
> >
> 
> Today upstream kernel selftests don't have scenarios where
> kvm_vm_elf_load can get called directly outside ___vm_create but there
> are selftests that are up for review [1], [2] that may call
> kvm_vm_elf_load directly. Above suggestion will not work in this
> scenario, is it suitable to assume that all the callers of
> kvm_vm_elf_load will eventually execute kvm_arch_vm_post_create?

No, but that's irrelevant.  And actually, in any reasonable hypothetical situation
I can think of, it's actually undesirable to always call kvm_arch_vm_post_create()
after kvm_vm_elf_load().

Hypothetically, if there were a use case where kvm_vm_elf_load() is called multiple
times, then stuffing the "Intel vs. "AMD" flag should only be done for the binary
that actually defines that flag.  The flag is defined by the library's processor.c,
and so the hook should be tied to the library's loading of its binary, i.e. to the
creation of the VM.

If a test were loading multiple binaries, and the test wanted to tweak things
specific to a binary after loading said binary, then the test can and should do
that without needed a library arch hook.

If the arch hook was to take action specific to loading _any_ binary, than yes, a
hook in kvm_vm_elf_load() would be in order, but this hook is about taking action
when creating a VM, not to loading a binary.

But this is all very, very hypothetical.  I can't think of a scenario where
loading multiple binaries would be less complex than solving whatever hypothetical
problem makes it difficult to link everything into a single binary.

And if a test manually loads a binary _and_ wants to actually run the guest after
doing vm_create_barebones() or ____vm_create(), then the test is doing it wrong,
as those low level APIs are provided _only_ for cases where a test doesn't need
to run vCPUs.