All of lore.kernel.org
 help / color / mirror / Atom feed
From: Catalin Marinas <catalin.marinas@arm.com>
To: "Russell King (Oracle)" <linux@armlinux.org.uk>
Cc: Leo Yan <leo.yan@linaro.org>, Will Deacon <will@kernel.org>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Adrian Hunter <adrian.hunter@intel.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Borislav Petkov <bp@alien8.de>,
	x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
	Mathieu Poirier <mathieu.poirier@linaro.org>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Mike Leach <mike.leach@linaro.org>,
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
	coresight@lists.linaro.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v4 11/11] perf auxtrace: Add compat_auxtrace_mmap__{read_head|write_tail}
Date: Tue, 13 Jul 2021 19:13:02 +0100	[thread overview]
Message-ID: <20210713181301.GE13181@arm.com> (raw)
In-Reply-To: <20210713161441.GK22278@shell.armlinux.org.uk>

On Tue, Jul 13, 2021 at 05:14:41PM +0100, Russell King wrote:
> On Tue, Jul 13, 2021 at 11:46:02PM +0800, Leo Yan wrote:
> > On Mon, Jul 12, 2021 at 03:44:11PM +0100, Russell King (Oracle) wrote:
> > > On Sun, Jul 11, 2021 at 06:41:05PM +0800, Leo Yan wrote:
> > > > When perf runs in compat mode (kernel in 64-bit mode and the perf is in
> > > > 32-bit mode), the 64-bit value atomicity in the user space cannot be
> > > > assured, E.g. on some architectures, the 64-bit value accessing is split
> > > > into two instructions, one is for the low 32-bit word accessing and
> > > > another is for the high 32-bit word.
> > > 
> > > Does this apply to 32-bit ARM code on aarch64? I would not have thought
> > > it would, as the structure member is a __u64 and
> > > compat_auxtrace_mmap__read_head() doesn't seem to be marking anything
> > > as packed, so the compiler _should_ be able to use a LDRD instruction
> > > to load the value.
> > 
> > I think essentially your question is relevant to the memory model.
> > For 32-bit Arm application on aarch64, in the Armv8 architecture
> > reference manual ARM DDI 0487F.c, chapter "E2.2.1
> > Requirements for single-copy atomicity" describes:
> > 
> > "LDM, LDC, LDRD, STM, STC, STRD, PUSH, POP, RFE, SRS, VLDM, VLDR, VSTM,
> > and VSTR instructions are executed as a sequence of word-aligned word
> > accesses. Each 32-bit word access is guaranteed to be single-copy
> > atomic. The architecture does not require subsequences of two or more
> > word accesses from the sequence to be single-copy atomic."
> 
> ... which is an interesting statement for ARMv7 code. DDI0406C says
> similar but goes on to say:
> 
>    In an implementation that includes the Large Physical Address
>    Extension, LDRD and STRD accesses to 64-bit aligned locations
>    are 64-bit single-copy atomic as seen by translation table
>    walks and accesses to translation tables.
> 
> then states that LPAE page tables must be stored in memory that such
> page tables must be in memory that is capable of supporting 64-bit
> single-copy atomic accesses.

A similar statement is in the ARMv8 ARM (E2.2.1 in version G.a).

> In Linux, we assume all RAM that the kernel has access to can contain
> page tables. So by implication, all RAM that the kernel has access to
> and exposes to userspace must be 64-bit single-copy atomic (if not,
> we have a rather serious bug.)

Indeed. We should assume that the SDRAM supports all the CPU features.

> The remaining question is whether it would be sane for LDRD and STRD
> to be single-copy atomic to translation table walkers but not to other
> CPUs. Since Linux expects to be able to modify the page tables from
> any CPU in the system, this requirement must hold, otherwise it's going
> to be a really strangely designed system.

The above statement does say "translation table walks and accesses to
translation tables". The accesses can be LDRD/STRD instructions from
other CPUs. Since the hardware can't tell whether the access is to a
page table, the designers just made LDRD/STRD single-copy atomic.

> I'd be interested to hear what Catalin and Will have to say on this,
> but I suspect in practice, Arm systems that are running Linux with
> LPAE (ARMv7+LPAE, ARMv8) will implement LDRD and STRD with 64-bit
> single-copy atomic semantics.

That's my understanding as well. In theory one could have a page table
access from EL0, so it should be atomic.

We could try to clarify E2.2.1 to simply state that naturally aligned
LDRD/STRD are single-copy atomic without any subsequent statement on the
translation table.

-- 
Catalin

WARNING: multiple messages have this Message-ID (diff)
From: Catalin Marinas <catalin.marinas@arm.com>
To: "Russell King (Oracle)" <linux@armlinux.org.uk>
Cc: Leo Yan <leo.yan@linaro.org>, Will Deacon <will@kernel.org>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Adrian Hunter <adrian.hunter@intel.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Borislav Petkov <bp@alien8.de>,
	x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
	Mathieu Poirier <mathieu.poirier@linaro.org>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Mike Leach <mike.leach@linaro.org>,
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
	coresight@lists.linaro.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v4 11/11] perf auxtrace: Add compat_auxtrace_mmap__{read_head|write_tail}
Date: Tue, 13 Jul 2021 19:13:02 +0100	[thread overview]
Message-ID: <20210713181301.GE13181@arm.com> (raw)
In-Reply-To: <20210713161441.GK22278@shell.armlinux.org.uk>

On Tue, Jul 13, 2021 at 05:14:41PM +0100, Russell King wrote:
> On Tue, Jul 13, 2021 at 11:46:02PM +0800, Leo Yan wrote:
> > On Mon, Jul 12, 2021 at 03:44:11PM +0100, Russell King (Oracle) wrote:
> > > On Sun, Jul 11, 2021 at 06:41:05PM +0800, Leo Yan wrote:
> > > > When perf runs in compat mode (kernel in 64-bit mode and the perf is in
> > > > 32-bit mode), the 64-bit value atomicity in the user space cannot be
> > > > assured, E.g. on some architectures, the 64-bit value accessing is split
> > > > into two instructions, one is for the low 32-bit word accessing and
> > > > another is for the high 32-bit word.
> > > 
> > > Does this apply to 32-bit ARM code on aarch64? I would not have thought
> > > it would, as the structure member is a __u64 and
> > > compat_auxtrace_mmap__read_head() doesn't seem to be marking anything
> > > as packed, so the compiler _should_ be able to use a LDRD instruction
> > > to load the value.
> > 
> > I think essentially your question is relevant to the memory model.
> > For 32-bit Arm application on aarch64, in the Armv8 architecture
> > reference manual ARM DDI 0487F.c, chapter "E2.2.1
> > Requirements for single-copy atomicity" describes:
> > 
> > "LDM, LDC, LDRD, STM, STC, STRD, PUSH, POP, RFE, SRS, VLDM, VLDR, VSTM,
> > and VSTR instructions are executed as a sequence of word-aligned word
> > accesses. Each 32-bit word access is guaranteed to be single-copy
> > atomic. The architecture does not require subsequences of two or more
> > word accesses from the sequence to be single-copy atomic."
> 
> ... which is an interesting statement for ARMv7 code. DDI0406C says
> similar but goes on to say:
> 
>    In an implementation that includes the Large Physical Address
>    Extension, LDRD and STRD accesses to 64-bit aligned locations
>    are 64-bit single-copy atomic as seen by translation table
>    walks and accesses to translation tables.
> 
> then states that LPAE page tables must be stored in memory that such
> page tables must be in memory that is capable of supporting 64-bit
> single-copy atomic accesses.

A similar statement is in the ARMv8 ARM (E2.2.1 in version G.a).

> In Linux, we assume all RAM that the kernel has access to can contain
> page tables. So by implication, all RAM that the kernel has access to
> and exposes to userspace must be 64-bit single-copy atomic (if not,
> we have a rather serious bug.)

Indeed. We should assume that the SDRAM supports all the CPU features.

> The remaining question is whether it would be sane for LDRD and STRD
> to be single-copy atomic to translation table walkers but not to other
> CPUs. Since Linux expects to be able to modify the page tables from
> any CPU in the system, this requirement must hold, otherwise it's going
> to be a really strangely designed system.

The above statement does say "translation table walks and accesses to
translation tables". The accesses can be LDRD/STRD instructions from
other CPUs. Since the hardware can't tell whether the access is to a
page table, the designers just made LDRD/STRD single-copy atomic.

> I'd be interested to hear what Catalin and Will have to say on this,
> but I suspect in practice, Arm systems that are running Linux with
> LPAE (ARMv7+LPAE, ARMv8) will implement LDRD and STRD with 64-bit
> single-copy atomic semantics.

That's my understanding as well. In theory one could have a page table
access from EL0, so it should be atomic.

We could try to clarify E2.2.1 to simply state that naturally aligned
LDRD/STRD are single-copy atomic without any subsequent statement on the
translation table.

-- 
Catalin

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

  reply	other threads:[~2021-07-13 18:13 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-11 10:40 [PATCH v4 00/11] perf: Refine barriers for AUX ring buffer Leo Yan
2021-07-11 10:40 ` Leo Yan
2021-07-11 10:40 ` [PATCH v4 01/11] perf/ring_buffer: Add comment for barriers on " Leo Yan
2021-07-11 10:40   ` Leo Yan
2021-07-11 10:40 ` [PATCH v4 02/11] coresight: tmc-etr: Add barrier after updating " Leo Yan
2021-07-11 10:40   ` Leo Yan
2021-07-12 10:40   ` Suzuki K Poulose
2021-07-12 10:40     ` Suzuki K Poulose
2021-07-12 10:54     ` Leo Yan
2021-07-12 10:54       ` Leo Yan
2021-07-11 10:40 ` [PATCH v4 03/11] coresight: tmc-etf: Add comment for store ordering Leo Yan
2021-07-11 10:40   ` Leo Yan
2021-07-13 12:56   ` Peter Zijlstra
2021-07-13 12:56     ` Peter Zijlstra
2021-07-13 15:49     ` Leo Yan
2021-07-13 15:49       ` Leo Yan
2021-07-11 10:40 ` [PATCH v4 04/11] perf/x86: Add barrier after updating bts Leo Yan
2021-07-11 10:40   ` Leo Yan
2021-07-13 13:01   ` Peter Zijlstra
2021-07-13 13:01     ` Peter Zijlstra
2021-07-11 10:40 ` [PATCH v4 05/11] perf auxtrace: Use WRITE_ONCE() for updating aux_tail Leo Yan
2021-07-11 10:40   ` Leo Yan
2021-07-11 10:41 ` [PATCH v4 06/11] perf auxtrace: Drop legacy __sync functions Leo Yan
2021-07-11 10:41   ` Leo Yan
2021-07-11 10:41 ` [PATCH v4 07/11] perf auxtrace: Remove auxtrace_mmap__read_snapshot_head() Leo Yan
2021-07-11 10:41   ` Leo Yan
2021-07-12 14:32   ` Adrian Hunter
2021-07-12 14:32     ` Adrian Hunter
2021-07-13 13:10     ` Leo Yan
2021-07-13 13:10       ` Leo Yan
2021-07-11 10:41 ` [PATCH v4 08/11] perf: Cleanup for HAVE_SYNC_COMPARE_AND_SWAP_SUPPORT Leo Yan
2021-07-11 10:41   ` Leo Yan
2021-07-11 10:41 ` [PATCH v4 09/11] tools: Remove feature-sync-compare-and-swap feature detection Leo Yan
2021-07-11 10:41   ` Leo Yan
2021-07-11 10:41 ` [PATCH v4 10/11] perf env: Set flag for kernel is 64-bit mode Leo Yan
2021-07-11 10:41   ` Leo Yan
2021-07-12 14:37   ` Adrian Hunter
2021-07-12 14:37     ` Adrian Hunter
2021-07-12 18:14   ` Arnaldo Carvalho de Melo
2021-07-12 18:14     ` Arnaldo Carvalho de Melo
2021-07-13 15:09     ` Leo Yan
2021-07-13 15:09       ` Leo Yan
2021-07-13 17:31       ` Hunter, Adrian
2021-07-13 17:31         ` Hunter, Adrian
2021-07-14 13:59         ` Arnaldo Carvalho de Melo
2021-07-14 13:59           ` Arnaldo Carvalho de Melo
2021-07-14 14:00           ` Arnaldo Carvalho de Melo
2021-07-14 14:00             ` Arnaldo Carvalho de Melo
2021-07-23  7:11             ` Leo Yan
2021-07-23  7:11               ` Leo Yan
2021-07-11 10:41 ` [PATCH v4 11/11] perf auxtrace: Add compat_auxtrace_mmap__{read_head|write_tail} Leo Yan
2021-07-11 10:41   ` Leo Yan
2021-07-12 14:44   ` Russell King (Oracle)
2021-07-12 14:44     ` Russell King (Oracle)
2021-07-13 15:46     ` Leo Yan
2021-07-13 15:46       ` Leo Yan
2021-07-13 16:14       ` Russell King (Oracle)
2021-07-13 16:14         ` Russell King (Oracle)
2021-07-13 18:13         ` Catalin Marinas [this message]
2021-07-13 18:13           ` Catalin Marinas
2021-07-14  8:40           ` Russell King (Oracle)
2021-07-14  8:40             ` Russell King (Oracle)
2021-07-23  7:23             ` Leo Yan
2021-07-23  7:23               ` Leo Yan
2021-07-13  7:07   ` Adrian Hunter
2021-07-13  7:07     ` Adrian Hunter
2021-07-13 15:48     ` Leo Yan
2021-07-13 15:48       ` Leo Yan

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=20210713181301.GE13181@arm.com \
    --to=catalin.marinas@arm.com \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=bp@alien8.de \
    --cc=coresight@lists.linaro.org \
    --cc=hpa@zytor.com \
    --cc=jolsa@redhat.com \
    --cc=leo.yan@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=mark.rutland@arm.com \
    --cc=mathieu.poirier@linaro.org \
    --cc=mike.leach@linaro.org \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=suzuki.poulose@arm.com \
    --cc=tglx@linutronix.de \
    --cc=will@kernel.org \
    --cc=x86@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.