linux-kernel - Re: [PATCH 000/128] KVM: selftests: Overhaul APIs, purge VCPU

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 for Android: free password hash cracker in your pocket

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

Date:   Thu, 05 May 2022 16:26:43 +0200
From:   Vitaly Kuznetsov <vkuznets@...hat.com>
To:     Sean Christopherson <seanjc@...gle.com>,
        Paolo Bonzini <pbonzini@...hat.com>
Cc:     linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        Andrew Jones <drjones@...hat.com>,
        David Matlack <dmatlack@...gle.com>,
        Ben Gardon <bgardon@...gle.com>,
        Oliver Upton <oupton@...gle.com>,
        Sean Christopherson <seanjc@...gle.com>
Subject: Re: [PATCH 000/128] KVM: selftests: Overhaul APIs, purge VCPU_ID

Sean Christopherson <seanjc@...gle.com> writes:

> Overhaul KVM's selftest APIs to get selftests to a state where adding new
> features and writing tests is less painful/disgusting (I feel dirty every
> time I copy+paste VCPU_ID).
>
> The primary theme is to stop treating tests like second class citizens.
> Stop hiding vcpu, kvm_vm, etc...  There's no sensitive data/constructs, and
> the encapsulation has led to really, really bad and difficult to maintain
> code.  E.g. having to pass around the VM just to call a vCPU ioctl(),
> arbitrary non-zero vCPU IDs, tests having to care about the vCPU ID in the
> first place, etc...
>
> The other theme in the rework is to deduplicate code and try to set us
> up for success in the future.  E.g. provide macros/helpers instead of
> spamming CTRL-C => CTRL-V (see the -700 LoC), structure the VM creation
> APIs to build on one another, etc...
>
> The ridiculous patch count (as opposed to just laughable) is due to
> converting each test away from using hardcoded vCPU IDs in a separate patch.
> The vast majority of those patches probably aren't worth reviewing in depth,
> the changes are mostly mechanical in nature.
>
> However, _running_ non-x86 tests (or tests that have unique non-x86
> behavior) would be extremely valuable.  All patches have been compile tested
> on x86, arm, risc-v, and s390, but I've only run the tests on x86.  Based on
> my track record for the x86+common tests, I will be very, very surprised if
> I didn't break any of the non-x86 tests, e.g. pthread_create()'s 'void *'
> param tripped me up multiple times.
>
> I believe the only x86 test I haven't run is amx_test, due to lack of
> hardware.
>
> Based on kvm/queue + kvm/master (wanted to avoid conflicts with fixes that
> are sitting in kvm/master):
>
>   2fd3ab9c169a Merge remote-tracking branch 'kvm/master' into x86/selftests_overhaul
>   2764011106d0 (kvm/queue) KVM: VMX: Include MKTME KeyID bits in shadow_zero_check
>
> Cc: kvm@...r.kernel.org
> Cc: Vitaly Kuznetsov <vkuznets@...hat.com>

This looks like a really nice API cleanup and a lot of work, thanks!
I've skimmed through the series and nothing besides '-Werror' caught my
eye (I've commented separately). I think that if post-series selftests
still compile and run with the same result on VMX/SVM/ARM we should get
this merged early as any patch to selftests is going to break the
mergeability and rebasing this is no fun.

Also, this series conflicts with my Hyper-V IPI/TLB flush selftests
added in "[PATCH v3 00/34] KVM: x86: hyper-v: Fine-grained TLB flush +
L2 TLB flush feature" but I can certainly rebase as soon as this hits
kvm/queue.

...

-- 
Vitaly