All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Lutomirski <luto@kernel.org>
To: Russell King - ARM Linux admin <linux@armlinux.org.uk>
Cc: Andy Lutomirski <luto@kernel.org>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	x86 <x86@kernel.org>, linux-kernel <linux-kernel@vger.kernel.org>,
	Nicholas Piggin <npiggin@gmail.com>,
	Arnd Bergmann <arnd@arndb.de>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Paul Mackerras <paulus@samba.org>,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	stable <stable@vger.kernel.org>
Subject: Re: [RFC please help] membarrier: Rewrite sync_core_before_usermode()
Date: Mon, 28 Dec 2020 09:14:23 -0800	[thread overview]
Message-ID: <CALCETrWQx0qwthBc5pJBxs2PWAQo-roAz-6g=7HOs+dsiokVsg@mail.gmail.com> (raw)
In-Reply-To: <20201228102537.GG1551@shell.armlinux.org.uk>

On Mon, Dec 28, 2020 at 2:25 AM Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> On Sun, Dec 27, 2020 at 01:36:13PM -0800, Andy Lutomirski wrote:
> > On Sun, Dec 27, 2020 at 12:18 PM Mathieu Desnoyers
> > <mathieu.desnoyers@efficios.com> wrote:
> > >
> > > ----- On Dec 27, 2020, at 1:28 PM, Andy Lutomirski luto@kernel.org wrote:
> > >
> >
> > > >
> > > > I admit that I'm rather surprised that the code worked at all on arm64,
> > > > and I'm suspicious that it has never been very well tested.  My apologies
> > > > for not reviewing this more carefully in the first place.
> > >
> > > Please refer to Documentation/features/sched/membarrier-sync-core/arch-support.txt
> > >
> > > It clearly states that only arm, arm64, powerpc and x86 support the membarrier
> > > sync core feature as of now:
> >
> > Sigh, I missed arm (32).  Russell or ARM folks, what's the right
> > incantation to make the CPU notice instruction changes initiated by
> > other cores on 32-bit ARM?
>
> You need to call flush_icache_range(), since the changes need to be
> flushed from the data cache to the point of unification (of the Harvard
> I and D), and the instruction cache needs to be invalidated so it can
> then see those updated instructions. This will also take care of the
> necessary barriers that the CPU requires for you.

With what parameters?   From looking at the header, this is for the
case in which the kernel writes some memory and then intends to
execute it.  That's not what membarrier() does at all.  membarrier()
works like this:

User thread 1:

write to RWX memory *or* write to an RW alias of an X region.
membarrier(...);
somehow tell thread 2 that we're ready (with a store release, perhaps,
or even just a relaxed store.)

User thread 2:

wait for the indication from thread 1.
barrier();
jump to the code.

membarrier() is, for better or for worse, not given a range of addresses.

On x86, the documentation is a bit weak, but a strict reading
indicates that thread 2 must execute a serializing instruction at some
point after thread 1 writes the code and before thread 2 runs it.
membarrier() does this by sending an IPI and ensuring that a
"serializing" instruction (thanks for great terminology, Intel) is
executed.  Note that flush_icache_range() is a no-op on x86, and I've
asked Intel's architects to please clarify their precise rules.  No
response yet.

On arm64, flush_icache_range() seems to send an IPI, and that's not
what I want.  membarrier() already does an IPI.

I suppose one option is to revert support for this membarrier()
operation on arm, but it would be nice to just implement it instead.

--Andy

WARNING: multiple messages have this Message-ID (diff)
From: Andy Lutomirski <luto@kernel.org>
To: Russell King - ARM Linux admin <linux@armlinux.org.uk>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Arnd Bergmann <arnd@arndb.de>, x86 <x86@kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Nicholas Piggin <npiggin@gmail.com>,
	Will Deacon <will@kernel.org>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	stable <stable@vger.kernel.org>,
	Andy Lutomirski <luto@kernel.org>,
	Paul Mackerras <paulus@samba.org>,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: [RFC please help] membarrier: Rewrite sync_core_before_usermode()
Date: Mon, 28 Dec 2020 09:14:23 -0800	[thread overview]
Message-ID: <CALCETrWQx0qwthBc5pJBxs2PWAQo-roAz-6g=7HOs+dsiokVsg@mail.gmail.com> (raw)
In-Reply-To: <20201228102537.GG1551@shell.armlinux.org.uk>

On Mon, Dec 28, 2020 at 2:25 AM Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> On Sun, Dec 27, 2020 at 01:36:13PM -0800, Andy Lutomirski wrote:
> > On Sun, Dec 27, 2020 at 12:18 PM Mathieu Desnoyers
> > <mathieu.desnoyers@efficios.com> wrote:
> > >
> > > ----- On Dec 27, 2020, at 1:28 PM, Andy Lutomirski luto@kernel.org wrote:
> > >
> >
> > > >
> > > > I admit that I'm rather surprised that the code worked at all on arm64,
> > > > and I'm suspicious that it has never been very well tested.  My apologies
> > > > for not reviewing this more carefully in the first place.
> > >
> > > Please refer to Documentation/features/sched/membarrier-sync-core/arch-support.txt
> > >
> > > It clearly states that only arm, arm64, powerpc and x86 support the membarrier
> > > sync core feature as of now:
> >
> > Sigh, I missed arm (32).  Russell or ARM folks, what's the right
> > incantation to make the CPU notice instruction changes initiated by
> > other cores on 32-bit ARM?
>
> You need to call flush_icache_range(), since the changes need to be
> flushed from the data cache to the point of unification (of the Harvard
> I and D), and the instruction cache needs to be invalidated so it can
> then see those updated instructions. This will also take care of the
> necessary barriers that the CPU requires for you.

With what parameters?   From looking at the header, this is for the
case in which the kernel writes some memory and then intends to
execute it.  That's not what membarrier() does at all.  membarrier()
works like this:

User thread 1:

write to RWX memory *or* write to an RW alias of an X region.
membarrier(...);
somehow tell thread 2 that we're ready (with a store release, perhaps,
or even just a relaxed store.)

User thread 2:

wait for the indication from thread 1.
barrier();
jump to the code.

membarrier() is, for better or for worse, not given a range of addresses.

On x86, the documentation is a bit weak, but a strict reading
indicates that thread 2 must execute a serializing instruction at some
point after thread 1 writes the code and before thread 2 runs it.
membarrier() does this by sending an IPI and ensuring that a
"serializing" instruction (thanks for great terminology, Intel) is
executed.  Note that flush_icache_range() is a no-op on x86, and I've
asked Intel's architects to please clarify their precise rules.  No
response yet.

On arm64, flush_icache_range() seems to send an IPI, and that's not
what I want.  membarrier() already does an IPI.

I suppose one option is to revert support for this membarrier()
operation on arm, but it would be nice to just implement it instead.

--Andy

WARNING: multiple messages have this Message-ID (diff)
From: Andy Lutomirski <luto@kernel.org>
To: Russell King - ARM Linux admin <linux@armlinux.org.uk>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Arnd Bergmann <arnd@arndb.de>,
	Michael Ellerman <mpe@ellerman.id.au>, x86 <x86@kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Nicholas Piggin <npiggin@gmail.com>,
	Will Deacon <will@kernel.org>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	stable <stable@vger.kernel.org>,
	Andy Lutomirski <luto@kernel.org>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Paul Mackerras <paulus@samba.org>,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: [RFC please help] membarrier: Rewrite sync_core_before_usermode()
Date: Mon, 28 Dec 2020 09:14:23 -0800	[thread overview]
Message-ID: <CALCETrWQx0qwthBc5pJBxs2PWAQo-roAz-6g=7HOs+dsiokVsg@mail.gmail.com> (raw)
In-Reply-To: <20201228102537.GG1551@shell.armlinux.org.uk>

On Mon, Dec 28, 2020 at 2:25 AM Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> On Sun, Dec 27, 2020 at 01:36:13PM -0800, Andy Lutomirski wrote:
> > On Sun, Dec 27, 2020 at 12:18 PM Mathieu Desnoyers
> > <mathieu.desnoyers@efficios.com> wrote:
> > >
> > > ----- On Dec 27, 2020, at 1:28 PM, Andy Lutomirski luto@kernel.org wrote:
> > >
> >
> > > >
> > > > I admit that I'm rather surprised that the code worked at all on arm64,
> > > > and I'm suspicious that it has never been very well tested.  My apologies
> > > > for not reviewing this more carefully in the first place.
> > >
> > > Please refer to Documentation/features/sched/membarrier-sync-core/arch-support.txt
> > >
> > > It clearly states that only arm, arm64, powerpc and x86 support the membarrier
> > > sync core feature as of now:
> >
> > Sigh, I missed arm (32).  Russell or ARM folks, what's the right
> > incantation to make the CPU notice instruction changes initiated by
> > other cores on 32-bit ARM?
>
> You need to call flush_icache_range(), since the changes need to be
> flushed from the data cache to the point of unification (of the Harvard
> I and D), and the instruction cache needs to be invalidated so it can
> then see those updated instructions. This will also take care of the
> necessary barriers that the CPU requires for you.

With what parameters?   From looking at the header, this is for the
case in which the kernel writes some memory and then intends to
execute it.  That's not what membarrier() does at all.  membarrier()
works like this:

User thread 1:

write to RWX memory *or* write to an RW alias of an X region.
membarrier(...);
somehow tell thread 2 that we're ready (with a store release, perhaps,
or even just a relaxed store.)

User thread 2:

wait for the indication from thread 1.
barrier();
jump to the code.

membarrier() is, for better or for worse, not given a range of addresses.

On x86, the documentation is a bit weak, but a strict reading
indicates that thread 2 must execute a serializing instruction at some
point after thread 1 writes the code and before thread 2 runs it.
membarrier() does this by sending an IPI and ensuring that a
"serializing" instruction (thanks for great terminology, Intel) is
executed.  Note that flush_icache_range() is a no-op on x86, and I've
asked Intel's architects to please clarify their precise rules.  No
response yet.

On arm64, flush_icache_range() seems to send an IPI, and that's not
what I want.  membarrier() already does an IPI.

I suppose one option is to revert support for this membarrier()
operation on arm, but it would be nice to just implement it instead.

--Andy

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

  reply	other threads:[~2020-12-28 17:15 UTC|newest]

Thread overview: 97+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-27 18:28 [RFC please help] membarrier: Rewrite sync_core_before_usermode() Andy Lutomirski
2020-12-27 18:28 ` Andy Lutomirski
2020-12-27 18:28 ` Andy Lutomirski
2020-12-27 20:18 ` Mathieu Desnoyers
2020-12-27 20:18   ` Mathieu Desnoyers
2020-12-27 21:36   ` Andy Lutomirski
2020-12-27 21:36     ` Andy Lutomirski
2020-12-27 21:36     ` Andy Lutomirski
2020-12-28 10:25     ` Russell King - ARM Linux admin
2020-12-28 10:25       ` Russell King - ARM Linux admin
2020-12-28 10:25       ` Russell King - ARM Linux admin
2020-12-28 17:14       ` Andy Lutomirski [this message]
2020-12-28 17:14         ` Andy Lutomirski
2020-12-28 17:14         ` Andy Lutomirski
2020-12-28 17:23         ` Russell King - ARM Linux admin
2020-12-28 17:23           ` Russell King - ARM Linux admin
2020-12-28 17:23           ` Russell King - ARM Linux admin
2020-12-28 18:10           ` Andy Lutomirski
2020-12-28 18:10             ` Andy Lutomirski
2020-12-28 18:10             ` Andy Lutomirski
2020-12-28 18:29         ` Jann Horn
2020-12-28 18:29           ` Jann Horn
2020-12-28 18:29           ` Jann Horn
2020-12-28 18:50           ` Andy Lutomirski
2020-12-28 18:50             ` Andy Lutomirski
2020-12-28 18:50             ` Andy Lutomirski
2020-12-28 19:08           ` Russell King - ARM Linux admin
2020-12-28 19:08             ` Russell King - ARM Linux admin
2020-12-28 19:08             ` Russell King - ARM Linux admin
2020-12-28 19:44             ` Andy Lutomirski
2020-12-28 19:44               ` Andy Lutomirski
2020-12-28 19:44               ` Andy Lutomirski
2020-12-28 20:24               ` Russell King - ARM Linux admin
2020-12-28 20:24                 ` Russell King - ARM Linux admin
2020-12-28 20:24                 ` Russell King - ARM Linux admin
2020-12-28 20:40                 ` Mathieu Desnoyers
2020-12-28 20:40                   ` Mathieu Desnoyers
2020-12-28 20:32               ` Mathieu Desnoyers
2020-12-28 20:32                 ` Mathieu Desnoyers
2020-12-28 21:06                 ` Andy Lutomirski
2020-12-28 21:06                   ` Andy Lutomirski
2020-12-28 21:06                   ` Andy Lutomirski
2020-12-28 21:26                   ` Mathieu Desnoyers
2020-12-28 21:26                     ` Mathieu Desnoyers
2020-12-29  0:36                   ` Nicholas Piggin
2020-12-29  0:36                     ` Nicholas Piggin
2020-12-29  0:36                     ` Nicholas Piggin
2020-12-29  0:56                     ` Andy Lutomirski
2020-12-29  0:56                       ` Andy Lutomirski
2020-12-29  0:56                       ` Andy Lutomirski
2020-12-29  3:09                       ` Nicholas Piggin
2020-12-29  3:09                         ` Nicholas Piggin
2020-12-29  3:09                         ` Nicholas Piggin
2020-12-29 10:44                         ` Russell King - ARM Linux admin
2020-12-29 10:44                           ` Russell King - ARM Linux admin
2020-12-29 10:44                           ` Russell King - ARM Linux admin
2020-12-30  2:33                           ` Nicholas Piggin
2020-12-30  2:33                             ` Nicholas Piggin
2020-12-30  2:33                             ` Nicholas Piggin
2020-12-30 10:00                             ` Russell King - ARM Linux admin
2020-12-30 10:00                               ` Russell King - ARM Linux admin
2020-12-30 10:00                               ` Russell King - ARM Linux admin
2020-12-30 10:58                               ` Russell King - ARM Linux admin
2020-12-30 10:58                                 ` Russell King - ARM Linux admin
2020-12-30 10:58                                 ` Russell King - ARM Linux admin
2020-12-30 11:57                                 ` Nicholas Piggin
2020-12-30 11:57                                   ` Nicholas Piggin
2020-12-30 11:57                                   ` Nicholas Piggin
2020-12-28 21:09     ` Mathieu Desnoyers
2020-12-28 21:09       ` Mathieu Desnoyers
2020-12-29  0:30       ` Andy Lutomirski
2020-12-29  0:30         ` Andy Lutomirski
2020-12-29  0:30         ` Andy Lutomirski
2020-12-29  0:11 ` Nicholas Piggin
2020-12-29  0:11   ` Nicholas Piggin
2020-12-29  0:11   ` Nicholas Piggin
2020-12-29  0:36   ` Andy Lutomirski
2020-12-29  0:36     ` Andy Lutomirski
2020-12-29  0:36     ` Andy Lutomirski
2020-12-29  3:31     ` Nicholas Piggin
2020-12-29  3:31       ` Nicholas Piggin
2020-12-29  3:31       ` Nicholas Piggin
2021-01-01 18:33     ` David Laight
2021-01-01 18:33       ` David Laight
2021-01-01 18:33       ` David Laight
2021-01-05 13:26     ` Will Deacon
2021-01-05 13:26       ` Will Deacon
2021-01-05 13:26       ` Will Deacon
2021-01-05 16:20       ` Andy Lutomirski
2021-01-05 16:20         ` Andy Lutomirski
2021-01-05 16:20         ` Andy Lutomirski
2021-01-05 16:37         ` Peter Zijlstra
2021-01-05 16:37           ` Peter Zijlstra
2021-01-05 16:37           ` Peter Zijlstra
2021-01-05 22:41         ` Will Deacon
2021-01-05 22:41           ` Will Deacon
2021-01-05 22:41           ` Will Deacon

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='CALCETrWQx0qwthBc5pJBxs2PWAQo-roAz-6g=7HOs+dsiokVsg@mail.gmail.com' \
    --to=luto@kernel.org \
    --cc=arnd@arndb.de \
    --cc=benh@kernel.crashing.org \
    --cc=catalin.marinas@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mpe@ellerman.id.au \
    --cc=npiggin@gmail.com \
    --cc=paulus@samba.org \
    --cc=stable@vger.kernel.org \
    --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.