| Message ID | 20210311231658.1243953-2-bgardon@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix RCU warnings in TDP MMU | expand |
On Thu, Mar 11, 2021, Ben Gardon wrote: > The pt passed into handle_removed_tdp_mmu_page does not need RCU > protection, as it is not at any risk of being freed by another thread at > that point. However, the implicit cast from tdp_sptep_t to u64 * dropped > the __rcu annotation without a proper rcu_derefrence. Fix this by > passing the pt as a tdp_ptep_t and then rcu_dereferencing it in > the function. > > Suggested-by: Sean Christopherson <seanjc@google.com> > Reported-by: kernel test robot <lkp@xxxxxxxxx> Should be <lkp@intel.com>. Looks like you've been taking pointers from Paolo :-) https://lkml.org/lkml/2019/6/17/1210 Other than that, Reviewed-by: Sean Christopherson <seanjc@google.com> > Signed-off-by: Ben Gardon <bgardon@google.com>
On 12/03/21 16:37, Sean Christopherson wrote: > On Thu, Mar 11, 2021, Ben Gardon wrote: >> The pt passed into handle_removed_tdp_mmu_page does not need RCU >> protection, as it is not at any risk of being freed by another thread at >> that point. However, the implicit cast from tdp_sptep_t to u64 * dropped >> the __rcu annotation without a proper rcu_derefrence. Fix this by >> passing the pt as a tdp_ptep_t and then rcu_dereferencing it in >> the function. >> >> Suggested-by: Sean Christopherson <seanjc@google.com> >> Reported-by: kernel test robot <lkp@xxxxxxxxx> > > Should be <lkp@intel.com>. Looks like you've been taking pointers from Paolo :-) The day someone starts confusing employers in CCs you should tell them "I see you have constructed a new email sending alias. Your skills are now complete". Paolo > https://lkml.org/lkml/2019/6/17/1210 > > Other than that, > > Reviewed-by: Sean Christopherson <seanjc@google.com> > >> Signed-off-by: Ben Gardon <bgardon@google.com> >
On Fri, Mar 12, 2021 at 7:43 AM Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 12/03/21 16:37, Sean Christopherson wrote: > > On Thu, Mar 11, 2021, Ben Gardon wrote: > >> The pt passed into handle_removed_tdp_mmu_page does not need RCU > >> protection, as it is not at any risk of being freed by another thread at > >> that point. However, the implicit cast from tdp_sptep_t to u64 * dropped > >> the __rcu annotation without a proper rcu_derefrence. Fix this by > >> passing the pt as a tdp_ptep_t and then rcu_dereferencing it in > >> the function. > >> > >> Suggested-by: Sean Christopherson <seanjc@google.com> > >> Reported-by: kernel test robot <lkp@xxxxxxxxx> > > > > Should be <lkp@intel.com>. Looks like you've been taking pointers from Paolo :-) I'll update that in v2. I was a little confused because I was looking at the report archived on Spinics, where all the domains are xxxxxxxx. Didn't notice that all the emails had been redacted like that. > > The day someone starts confusing employers in CCs you should tell them > "I see you have constructed a new email sending alias. Your skills are > now complete". > > Paolo > > > https://lkml.org/lkml/2019/6/17/1210 > > > > Other than that, > > > > Reviewed-by: Sean Christopherson <seanjc@google.com> > > > >> Signed-off-by: Ben Gardon <bgardon@google.com> > > >
diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c index c926c6b899a1..5387ac040f66 100644 --- a/arch/x86/kvm/mmu/tdp_mmu.c +++ b/arch/x86/kvm/mmu/tdp_mmu.c @@ -301,11 +301,16 @@ static void tdp_mmu_unlink_page(struct kvm *kvm, struct kvm_mmu_page *sp, * * Given a page table that has been removed from the TDP paging structure, * iterates through the page table to clear SPTEs and free child page tables. + * + * Note that pt is passed in as a tdp_ptep_t, but it does not need RCU + * protection. Since this thread removed it from the paging structure, + * this thread will be responsible for ensuring the page is freed. Hence the + * early rcu_dereferences in the function. */ -static void handle_removed_tdp_mmu_page(struct kvm *kvm, u64 *pt, +static void handle_removed_tdp_mmu_page(struct kvm *kvm, tdp_ptep_t pt, bool shared) { - struct kvm_mmu_page *sp = sptep_to_sp(pt); + struct kvm_mmu_page *sp = sptep_to_sp(rcu_dereference(pt)); int level = sp->role.level; gfn_t base_gfn = sp->gfn; u64 old_child_spte; @@ -318,7 +323,7 @@ static void handle_removed_tdp_mmu_page(struct kvm *kvm, u64 *pt, tdp_mmu_unlink_page(kvm, sp, shared); for (i = 0; i < PT64_ENT_PER_PAGE; i++) { - sptep = pt + i; + sptep = rcu_dereference(pt) + i; gfn = base_gfn + (i * KVM_PAGES_PER_HPAGE(level - 1)); if (shared) {
The pt passed into handle_removed_tdp_mmu_page does not need RCU protection, as it is not at any risk of being freed by another thread at that point. However, the implicit cast from tdp_sptep_t to u64 * dropped the __rcu annotation without a proper rcu_derefrence. Fix this by passing the pt as a tdp_ptep_t and then rcu_dereferencing it in the function. Suggested-by: Sean Christopherson <seanjc@google.com> Reported-by: kernel test robot <lkp@xxxxxxxxx> Signed-off-by: Ben Gardon <bgardon@google.com> --- arch/x86/kvm/mmu/tdp_mmu.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)