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
| ||
|
Message-ID: <Yh/KzDqsQSGm1CvK@google.com> Date: Wed, 2 Mar 2022 19:51:40 +0000 From: Sean Christopherson <seanjc@...gle.com> To: Mingwei Zhang <mizhang@...gle.com> Cc: Paolo Bonzini <pbonzini@...hat.com>, Christian Borntraeger <borntraeger@...ux.ibm.com>, Janosch Frank <frankja@...ux.ibm.com>, Claudio Imbrenda <imbrenda@...ux.ibm.com>, Vitaly Kuznetsov <vkuznets@...hat.com>, Wanpeng Li <wanpengli@...cent.com>, Jim Mattson <jmattson@...gle.com>, Joerg Roedel <joro@...tes.org>, David Hildenbrand <david@...hat.com>, kvm@...r.kernel.org, linux-kernel@...r.kernel.org, David Matlack <dmatlack@...gle.com>, Ben Gardon <bgardon@...gle.com> Subject: Re: [PATCH v3 01/28] KVM: x86/mmu: Use common iterator for walking invalid TDP MMU roots On Wed, Mar 02, 2022, Mingwei Zhang wrote: > On Sat, Feb 26, 2022, Sean Christopherson wrote: > > Now that tdp_mmu_next_root() can process both valid and invalid roots, > > extend it to be able to process _only_ invalid roots, add yet another > > iterator macro for walking invalid roots, and use the new macro in > > kvm_tdp_mmu_zap_invalidated_roots(). > > > > No functional change intended. > > > > Reviewed-by: David Matlack <dmatlack@...gle.com> > > Signed-off-by: Sean Christopherson <seanjc@...gle.com> > > --- > > arch/x86/kvm/mmu/tdp_mmu.c | 74 ++++++++++++++------------------------ > > 1 file changed, 26 insertions(+), 48 deletions(-) > > > > diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c > > index debf08212f12..25148e8b711d 100644 > > --- a/arch/x86/kvm/mmu/tdp_mmu.c > > +++ b/arch/x86/kvm/mmu/tdp_mmu.c > > @@ -98,6 +98,12 @@ void kvm_tdp_mmu_put_root(struct kvm *kvm, struct kvm_mmu_page *root, > > call_rcu(&root->rcu_head, tdp_mmu_free_sp_rcu_callback); > > } > > > > +enum tdp_mmu_roots_iter_type { > > + ALL_ROOTS = -1, > > + VALID_ROOTS = 0, > > + INVALID_ROOTS = 1, > > +}; > > I am wondering what the trick is to start from -1? -1 is arbitrary, any non-zero value would work. More below. > > /* > > * Returns the next root after @prev_root (or the first root if @prev_root is > > * NULL). A reference to the returned root is acquired, and the reference to > > @@ -110,10 +116,16 @@ void kvm_tdp_mmu_put_root(struct kvm *kvm, struct kvm_mmu_page *root, > > */ > > static struct kvm_mmu_page *tdp_mmu_next_root(struct kvm *kvm, > > struct kvm_mmu_page *prev_root, > > - bool shared, bool only_valid) > > + bool shared, > > + enum tdp_mmu_roots_iter_type type) > > { > > struct kvm_mmu_page *next_root; > > > > + kvm_lockdep_assert_mmu_lock_held(kvm, shared); > > + > > + /* Ensure correctness for the below comparison against role.invalid. */ > > + BUILD_BUG_ON(!!VALID_ROOTS || !INVALID_ROOTS); > > + > > rcu_read_lock(); > > > > if (prev_root) > > @@ -125,7 +137,7 @@ static struct kvm_mmu_page *tdp_mmu_next_root(struct kvm *kvm, > > typeof(*next_root), link); > > > > while (next_root) { > > - if ((!only_valid || !next_root->role.invalid) && > > + if ((type == ALL_ROOTS || (type == !!next_root->role.invalid)) && This is the code that deals with the enums. It's making the type a tri-state, where the values of VALID_ROOTS and INVALID_ROOTS align with converting role.invalid to a boolean (always '0' or '1') so that they can be directly compared as above. Any value for ALL_ROOTS (other than '0' or '1' obviously) would work since the above logic requires ALL_ROOTS to be explicitly checked first. > > kvm_tdp_mmu_get_root(next_root)) > > break; > >
Powered by blists - more mailing lists