All of lore.kernel.org
 help / color / mirror / Atom feed
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 12:59:28 +0100	[thread overview]
Message-ID: <20210806115927.GJ6719@arm.com> (raw)
In-Reply-To: <20210806113109.2475-2-will@kernel.org>

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.
Otherwise the patch looks fine to me:

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

WARNING: multiple messages have this Message-ID (diff)
From: Catalin Marinas <catalin.marinas@arm.com>
To: Will Deacon <will@kernel.org>
Cc: linux-arch@vger.kernel.org, Marc Zyngier <maz@kernel.org>,
	stable@vger.kernel.org, kernel-team@android.com,
	kvmarm@lists.cs.columbia.edu,
	linux-arm-kernel@lists.infradead.org,
	Jade Alglave <jade.alglave@arm.com>
Subject: Re: [PATCH 1/4] arm64: mm: Fix TLBI vs ASID rollover
Date: Fri, 6 Aug 2021 12:59:28 +0100	[thread overview]
Message-ID: <20210806115927.GJ6719@arm.com> (raw)
In-Reply-To: <20210806113109.2475-2-will@kernel.org>

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.
Otherwise the patch looks fine to me:

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

WARNING: multiple messages have this Message-ID (diff)
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 12:59:28 +0100	[thread overview]
Message-ID: <20210806115927.GJ6719@arm.com> (raw)
In-Reply-To: <20210806113109.2475-2-will@kernel.org>

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.
Otherwise the patch looks fine to me:

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2021-08-06 11:59 UTC|newest]

Thread overview: 51+ 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 ` Will Deacon
2021-08-06 11:31 ` Will Deacon
2021-08-06 11:31 ` [PATCH 1/4] arm64: mm: Fix TLBI vs ASID rollover Will Deacon
2021-08-06 11:31   ` Will Deacon
2021-08-06 11:31   ` Will Deacon
2021-08-06 11:59   ` Catalin Marinas [this message]
2021-08-06 11:59     ` Catalin Marinas
2021-08-06 11:59     ` Catalin Marinas
2021-08-06 12:42     ` Will Deacon
2021-08-06 12:42       ` Will Deacon
2021-08-06 12:42       ` Will Deacon
2021-08-06 12:49       ` Catalin Marinas
2021-08-06 12:49         ` Catalin Marinas
2021-08-06 12:49         ` Catalin Marinas
2021-08-06 11:31 ` [PATCH] of: restricted dma: Don't fail device probe on rmem init failure Will Deacon
2021-08-06 11:31   ` Will Deacon
2021-08-06 11:31   ` Will Deacon
2021-08-06 11:34   ` Will Deacon
2021-08-06 11:34     ` 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   ` Will Deacon
2021-08-06 11:31   ` 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 11:31   ` Will Deacon
2021-08-06 11:31   ` Will Deacon
2021-08-06 13:40   ` Quentin Perret
2021-08-06 13:40     ` Quentin Perret
2021-08-06 13:40     ` Quentin Perret
2021-08-20  8:01     ` Marc Zyngier
2021-08-20  8:01       ` Marc Zyngier
2021-08-20  8:01       ` Marc Zyngier
2021-08-20  9:08       ` Quentin Perret
2021-08-20  9:08         ` Quentin Perret
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 11:31   ` Will Deacon
2021-08-06 11:31   ` Will Deacon
2021-08-06 14:24   ` Quentin Perret
2021-08-06 14:24     ` Quentin Perret
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-08-06 16:04   ` Catalin Marinas
2021-08-06 16:04   ` Catalin Marinas
2021-09-10  9:06 ` Shameerali Kolothum Thodi
2021-09-10  9:06   ` Shameerali Kolothum Thodi
2021-09-10  9:06   ` Shameerali Kolothum Thodi
2021-09-10  9:45   ` Catalin Marinas
2021-09-10  9:45     ` Catalin Marinas
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=20210806115927.GJ6719@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 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.