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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 13 Oct 2022 17:06:48 +0530
From:   Vishal Annapurve <vannapurve@...gle.com>
To:     Sean Christopherson <seanjc@...gle.com>
Cc:     David Matlack <dmatlack@...gle.com>, x86 <x86@...nel.org>,
        kvm list <kvm@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Linuxkselftest <linux-kselftest@...r.kernel.org>,
        Paolo Bonzini <pbonzini@...hat.com>, shuah <shuah@...nel.org>,
        Ben Gardon <bgardon@...gle.com>,
        Oliver Upton <oupton@...gle.com>, Peter Xu <peterx@...hat.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>
Subject: Re: [V2 PATCH 3/8] KVM: selftests: Add arch specific post vm load setup

On Wed, Oct 5, 2022 at 1:40 AM Sean Christopherson <seanjc@...gle.com> wrote:
>
> On Mon, Oct 03, 2022, Vishal Annapurve wrote:
> > On Mon, Oct 3, 2022 at 8:42 AM Sean Christopherson <seanjc@...gle.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.

Ack, will update this patch as per the feedback here.