All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bjoern Doebel <doebel@amazon.de>
To: <sashiko-reviews@lists.linux.dev>
Cc: Bjoern Doebel <doebel@amazon.de>,
	Oliver Upton <oupton@kernel.org>, <kvmarm@lists.linux.dev>,
	Marc Zyngier <maz@kernel.org>
Subject: Re: [PATCH 5.10.y] KVM: arm64: Retry fault if vma_lookup() results become invalid
Date: Fri, 21 Aug 2026 09:46:29 +0000	[thread overview]
Message-ID: <aogeHAy/VCZuTX4i@amazon.de> (raw)
In-Reply-To: <20260821091439.8A6FC1F000E9@smtp.kernel.org>

On Fri, Aug 21, 2026 at 09:14:39AM +0000, sashiko-bot@kernel.org wrote:
> > -	}
> > -
> >  	mmu_seq = vcpu->kvm->mmu_notifier_seq;
> > -	/*
> > -	 * Ensure the read of mmu_notifier_seq happens before we call
> > -	 * gfn_to_pfn_prot (which calls get_user_pages), so that we don't risk
> > -	 * the page we just got a reference to gets unmapped before we have a
> > -	 * chance to grab the mmu_lock, which ensure that if the page gets
> > -	 * unmapped afterwards, the call to kvm_unmap_hva will take it away
> > -	 * from us again properly. This smp_rmb() interacts with the smp_wmb()
> > -	 * in kvm_mmu_notifier_invalidate_<page|range_end>.
> > -	 */
> > -	smp_rmb();
> > +	/*
> > +	 * Read mmu_notifier_seq so that KVM can detect if the results of
> > +	 * find_vma_intersection() or gfn_to_pfn_prot() become stale prior to
> > +	 * acquiring kvm->mmu_lock.
> > +	 *
> > +	 * Rely on mmap_read_unlock() for an implicit smp_rmb(), which pairs
> > +	 * with the smp_wmb() in kvm_mmu_notifier_invalidate_range_end().
> > +	 */
> > +	mmap_read_unlock(current->mm);
> >  
> >  	pfn = gfn_to_pfn_prot(kvm, gfn, write_fault, &writable);
> 
> [Severity: Critical]
> Does removing the explicit smp_rmb() here in user_mem_abort() break the MMU
> notifier retry protocol?
> 
> The comment states that we can rely on mmap_read_unlock() for an implicit
> smp_rmb(). However, mmap_read_unlock() via up_read() typically provides only
> RELEASE semantics.

Note that this removal of smp_rmb() is not new in this backport but
verbatim taken from the mainline patch. It is also contained in all
previous backports of the patch.

I'm not an expert on the memory semantics here and open to suggestions.
But _if_ we consider this a problem, we would have to go back and review
mainline and the other backports as well.

Bjoern


      reply	other threads:[~2026-08-21  9:46 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-21  9:05 [PATCH 5.10.y] KVM: arm64: Retry fault if vma_lookup() results become invalid Bjoern Doebel
2026-08-21  9:14 ` sashiko-bot
2026-08-21  9:46   ` Bjoern Doebel [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aogeHAy/VCZuTX4i@amazon.de \
    --to=doebel@amazon.de \
    --cc=kvmarm@lists.linux.dev \
    --cc=maz@kernel.org \
    --cc=oupton@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.