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
| ||
|
Date: Tue, 6 Sep 2022 15:48:27 -0700 From: Vishal Annapurve <vannapurve@...gle.com> To: Sean Christopherson <seanjc@...gle.com> Cc: x86 <x86@...nel.org>, kvm list <kvm@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>, linux-kselftest@...r.kernel.org, Paolo Bonzini <pbonzini@...hat.com>, Vitaly Kuznetsov <vkuznets@...hat.com>, Wanpeng Li <wanpengli@...cent.com>, Jim Mattson <jmattson@...gle.com>, Joerg Roedel <joro@...tes.org>, Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>, dave.hansen@...ux.intel.com, "H . Peter Anvin" <hpa@...or.com>, shuah <shuah@...nel.org>, yang.zhong@...el.com, drjones@...hat.com, Ricardo Koller <ricarkol@...gle.com>, Aaron Lewis <aaronlewis@...gle.com>, wei.w.wang@...el.com, "Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>, Jonathan Corbet <corbet@....net>, Hugh Dickins <hughd@...gle.com>, Jeff Layton <jlayton@...nel.org>, "J . Bruce Fields" <bfields@...ldses.org>, Andrew Morton <akpm@...ux-foundation.org>, Chao Peng <chao.p.peng@...ux.intel.com>, Yu Zhang <yu.c.zhang@...ux.intel.com>, Jun Nakajima <jun.nakajima@...el.com>, Dave Hansen <dave.hansen@...el.com>, Michael Roth <michael.roth@....com>, Quentin Perret <qperret@...gle.com>, Steven Price <steven.price@....com>, Andi Kleen <ak@...ux.intel.com>, David Hildenbrand <david@...hat.com>, Andy Lutomirski <luto@...nel.org>, Vlastimil Babka <vbabka@...e.cz>, Marc Orr <marcorr@...gle.com>, Erdem Aktas <erdemaktas@...gle.com>, Peter Gonda <pgonda@...gle.com>, "Nikunj A. Dadhania" <nikunj@....com>, Austin Diviness <diviness@...gle.com>, maz@...nel.org, David Matlack <dmatlack@...gle.com>, Axel Rasmussen <axelrasmussen@...gle.com>, maciej.szmigiero@...cle.com, Mingwei Zhang <mizhang@...gle.com>, Ben Gardon <bgardon@...gle.com> Subject: Re: [RFC V3 PATCH 4/6] selftests: kvm: x86: Execute hypercall as per the cpu On Wed, Aug 24, 2022 at 5:07 PM Sean Christopherson <seanjc@...gle.com> wrote: > > On Fri, Aug 19, 2022, Vishal Annapurve wrote: > > Add support for executing vmmcall/vmcall instruction on amd/intel cpus. > > In general kvm patches the instruction according to the cpu > > implementation at runtime. While executing selftest vms from private > > memory KVM will not be able to update the private memory of the guest. > > > > Hypercall parameters are fixed to explicitly populate hypercall number > > in eax. Otherwise inlined function calls to kvm_hypercall would call > > vmmcall/vmcall instruction without updating eax with hypercall number. > > Can you send a seperate non-RFC series to clean up the selftests mess? kvm_hypercall() > isn't the only culprit. > > git grep \"vmcall | wc -l > 16 > > I'm pretty sure things work only because of KVM's dubious behavior of patching > VMCALL/VMMCALL by default. > > Note, svm_vmcall_test.c intentionally uses the wrong instructions and shouldn't > be converted. Actually, we can and should just delete that test, it's superseded > by the wonderfully named fix_hypercall_test. > > > Signed-off-by: Vishal Annapurve <vannapurve@...gle.com> > > --- > > .../testing/selftests/kvm/lib/x86_64/processor.c | 15 +++++++++++++-- > > 1 file changed, 13 insertions(+), 2 deletions(-) > > > > diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c > > index 53b115876417..09d757a0b148 100644 > > --- a/tools/testing/selftests/kvm/lib/x86_64/processor.c > > +++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c > > @@ -1254,10 +1254,21 @@ uint64_t kvm_hypercall(uint64_t nr, uint64_t a0, uint64_t a1, uint64_t a2, > > uint64_t a3) > > { > > uint64_t r; > > + static bool is_cpu_checked; > > + static bool is_cpu_amd; > > > > - asm volatile("vmcall" > > + if (!is_cpu_checked) > > + is_cpu_amd = is_amd_cpu(); > > This can be done using a single int, e.g. > > static bool is_cpu_amd = -1; > > if (is_cpu_amd < 0) > is_cpu_amd = is_amd_cpu(); > > Although... what if we declare main() in lib/kvm_util.c (or maybe a dedicated > file?) and rename all tests to use __main()? Then add an arch hook to do global > initialization and avoid the "did we check CPUID?!?!?" altogether. > > That would allow us to dedup all of the hilarious copy paste: > > /* Tell stdout not to buffer its content */ > setbuf(stdout, NULL); > > and we could turn is_amd_cpu() and is_intel_cpu() into bools. > > E.g. > > int main(int argc, char *argv[]) > { > /* Tell stdout not to buffer its content */ > setbuf(stdout, NULL); > > kvm_arch_main(); > > return __main(argc, argv); > } > > void kvm_arch_main(void) > { > is_cpu_amd = cpu_vendor_string_is("AuthenticAMD"); > is_cpu_intel = cpu_vendor_string_is("AuthenticAMD"); > } > > > And then we just need macro magic to emit the right VMCALL/VMMCALL instruction. Thanks Sean for the feedback here. I have posted a separate series addressing your comments: https://lore.kernel.org/all/20220903012849.938069-4-vannapurve@google.com/T/ Regards, Vishal
Powered by blists - more mailing lists