On Wed, Oct 5, 2022 at 1:40 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > On Mon, Oct 03, 2022, Vishal Annapurve wrote: > > On Mon, Oct 3, 2022 at 8:42 AM Sean Christopherson <seanjc@xxxxxxxxxx> 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. Ack, will update this patch as per the feedback here.
