All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Leo Yan <leo.yan@linaro.org>, Will Deacon <will@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>
Cc: 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 17:14:41 +0100	[thread overview]
Message-ID: <20210713161441.GK22278@shell.armlinux.org.uk> (raw)
In-Reply-To: <20210713154602.GD748506@leoy-ThinkPad-X240s>

On Tue, Jul 13, 2021 at 11:46:02PM +0800, Leo Yan wrote:
> Hi Russell,
> 
> 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.

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.)

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.

Therefore, I put it that for Linux to operate correctly on 32-bit Arm
CPUs with LPAE, LDRD and STRD must be 64-bit single-copy atomic
inspite of what the architecture reference documentation may allow.

Now, since we allow 32-bit ARM kernels to run under KVM on ARMv8, it
would be pretty silly if this was broken on aarch64 - it would mean
such a guest would have no way to atomically update the LPAE page
tables. We know that's not true, since we can run 32-bit kernels and
userspace just fine under aarch64.

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.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

WARNING: multiple messages have this Message-ID (diff)
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Leo Yan <leo.yan@linaro.org>, Will Deacon <will@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>
Cc: 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 17:14:41 +0100	[thread overview]
Message-ID: <20210713161441.GK22278@shell.armlinux.org.uk> (raw)
In-Reply-To: <20210713154602.GD748506@leoy-ThinkPad-X240s>

On Tue, Jul 13, 2021 at 11:46:02PM +0800, Leo Yan wrote:
> Hi Russell,
> 
> 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.

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.)

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.

Therefore, I put it that for Linux to operate correctly on 32-bit Arm
CPUs with LPAE, LDRD and STRD must be 64-bit single-copy atomic
inspite of what the architecture reference documentation may allow.

Now, since we allow 32-bit ARM kernels to run under KVM on ARMv8, it
would be pretty silly if this was broken on aarch64 - it would mean
such a guest would have no way to atomically update the LPAE page
tables. We know that's not true, since we can run 32-bit kernels and
userspace just fine under aarch64.

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.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
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 16:15 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) [this message]
2021-07-13 16:14         ` Russell King (Oracle)
2021-07-13 18:13         ` Catalin Marinas
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=20210713161441.GK22278@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=bp@alien8.de \
    --cc=catalin.marinas@arm.com \
    --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=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.