From: Catalin Marinas <catalin.marinas@arm.com>
To: Will Deacon <will@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org, kernel-team@android.com,
Marc Zyngier <maz@kernel.org>,
Jade Alglave <jade.alglave@arm.com>,
Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>,
kvmarm@lists.cs.columbia.edu, linux-arch@vger.kernel.org,
stable@vger.kernel.org
Subject: Re: [PATCH 1/4] arm64: mm: Fix TLBI vs ASID rollover
Date: Fri, 6 Aug 2021 13:49:44 +0100 [thread overview]
Message-ID: <20210806124944.GK6719@arm.com> (raw)
In-Reply-To: <20210806124241.GA2814@willie-the-truck>
On Fri, Aug 06, 2021 at 01:42:42PM +0100, Will Deacon wrote:
> On Fri, Aug 06, 2021 at 12:59:28PM +0100, Catalin Marinas wrote:
> > On Fri, Aug 06, 2021 at 12:31:04PM +0100, Will Deacon wrote:
> > > diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
> > > index 75beffe2ee8a..e9c30859f80c 100644
> > > --- a/arch/arm64/include/asm/mmu.h
> > > +++ b/arch/arm64/include/asm/mmu.h
> > > @@ -27,11 +27,32 @@ typedef struct {
> > > } mm_context_t;
> > >
> > > /*
> > > - * This macro is only used by the TLBI and low-level switch_mm() code,
> > > - * neither of which can race with an ASID change. We therefore don't
> > > - * need to reload the counter using atomic64_read().
> > > + * We use atomic64_read() here because the ASID for an 'mm_struct' can
> > > + * be reallocated when scheduling one of its threads following a
> > > + * rollover event (see new_context() and flush_context()). In this case,
> > > + * a concurrent TLBI (e.g. via try_to_unmap_one() and ptep_clear_flush())
> > > + * may use a stale ASID. This is fine in principle as the new ASID is
> > > + * guaranteed to be clean in the TLB, but the TLBI routines have to take
> > > + * care to handle the following race:
> > > + *
> > > + * CPU 0 CPU 1 CPU 2
> > > + *
> > > + * // ptep_clear_flush(mm)
> > > + * xchg_relaxed(pte, 0)
> > > + * DSB ISHST
> > > + * old = ASID(mm)
> >
> > We'd need specs clarified (ARM ARM, cat model) that the DSB ISHST is
> > sufficient to order the pte write with the subsequent ASID read.
>
> Although I agree that the cat model needs updating and also that the Arm
> ARM isn't helpful by trying to define DMB and DSB at the same time, it
> does clearly state the following:
>
> // B2-149
> | A DSB instruction executed by a PE, PEe, completes when all of the
> | following apply:
> |
> | * All explicit memory accesses of the required access types appearing
> | in program order before the DSB are complete for the set of observers
> | in the required shareability domain.
>
> [...]
>
> // B2-150
> | In addition, no instruction that appears in program order after the
> | DSB instruction can alter any state of the system or perform any part
> | of its functionality until the DSB completes other than:
> |
> | * Being fetched from memory and decoded.
> | * Reading the general-purpose, SIMD and floating-point, Special-purpose,
> | or System registers that are directly or indirectly read without
> | causing side-effects.
>
> Which means that the ASID read cannot return its data before the DSB ISHST
> has completed and the DSB ISHST cannot complete until the PTE write has
> completed.
Thanks for the explanation.
> > Otherwise the patch looks fine to me:
> >
> > Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
>
> Thanks! Do you want to queue it for 5.15? I don't think there's a need to
> rush it into 5.14 given that we don't have any evidence of it happening
> in practice.
Happy to queue it for 5.15.
--
Catalin
next prev parent reply other threads:[~2021-08-06 12:50 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-06 11:31 [PATCH 0/4] Fix racing TLBI with ASID/VMID reallocation Will Deacon
2021-08-06 11:31 ` [PATCH 1/4] arm64: mm: Fix TLBI vs ASID rollover Will Deacon
2021-08-06 11:59 ` Catalin Marinas
2021-08-06 12:42 ` Will Deacon
2021-08-06 12:49 ` Catalin Marinas [this message]
2021-08-06 11:31 ` [PATCH] of: restricted dma: Don't fail device probe on rmem init failure Will Deacon
2021-08-06 11:34 ` Will Deacon
2021-08-06 11:31 ` [PATCH 2/4] KVM: arm64: Move kern_hyp_va() usage in __load_guest_stage2() into the callers Will Deacon
2021-08-06 11:31 ` [PATCH 3/4] KVM: arm64: Convert the host S2 over to __load_guest_stage2() Will Deacon
2021-08-06 13:40 ` Quentin Perret
2021-08-20 8:01 ` Marc Zyngier
2021-08-20 9:08 ` Quentin Perret
2021-08-06 11:31 ` [PATCH 4/4] KVM: arm64: Upgrade VMID accesses to {READ,WRITE}_ONCE Will Deacon
2021-08-06 14:24 ` Quentin Perret
2021-08-06 16:04 ` (subset) [PATCH 0/4] Fix racing TLBI with ASID/VMID reallocation Catalin Marinas
2021-09-10 9:06 ` Shameerali Kolothum Thodi
2021-09-10 9:45 ` Catalin Marinas
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=20210806124944.GK6719@arm.com \
--to=catalin.marinas@arm.com \
--cc=jade.alglave@arm.com \
--cc=kernel-team@android.com \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=linux-arch@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=maz@kernel.org \
--cc=shameerali.kolothum.thodi@huawei.com \
--cc=stable@vger.kernel.org \
--cc=will@kernel.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).