* Re: bit fields && data tearing
From: Peter Hurley @ 2014-09-05 20:01 UTC (permalink / raw)
To: Peter Zijlstra, Paul E. McKenney
Cc: Jakub Jelinek, linux-arch@vger.kernel.org, Tony Luck,
linux-ia64@vger.kernel.org, Michael Cree, linux-alpha,
Oleg Nesterov, linux-kernel@vger.kernel.org, David Laight,
Paul Mackerras, H. Peter Anvin, linuxppc-dev@lists.ozlabs.org,
Miroslav Franc, Richard Henderson
In-Reply-To: <20140905195234.GT4783@worktop.ger.corp.intel.com>
On 09/05/2014 03:52 PM, Peter Zijlstra wrote:
> On Fri, Sep 05, 2014 at 11:31:09AM -0700, Paul E. McKenney wrote:
>> compiler: Allow 1- and 2-byte smp_load_acquire() and smp_store_release()
>>
>> CPUs without single-byte and double-byte loads and stores place some
>> "interesting" requirements on concurrent code. For example (adapted
>> from Peter Hurley's test code), suppose we have the following structure:
>>
>> struct foo {
>> spinlock_t lock1;
>> spinlock_t lock2;
>> char a; /* Protected by lock1. */
>> char b; /* Protected by lock2. */
>> };
>> struct foo *foop;
>>
>> Of course, it is common (and good) practice to place data protected
>> by different locks in separate cache lines. However, if the locks are
>> rarely acquired (for example, only in rare error cases), and there are
>> a great many instances of the data structure, then memory footprint can
>> trump false-sharing concerns, so that it can be better to place them in
>> the same cache cache line as above.
>>
>> But if the CPU does not support single-byte loads and stores, a store
>> to foop->a will do a non-atomic read-modify-write operation on foop->b,
>> which will come as a nasty surprise to someone holding foop->lock2. So we
>> now require CPUs to support single-byte and double-byte loads and stores.
>> Therefore, this commit adjusts the definition of __native_word() to allow
>> these sizes to be used by smp_load_acquire() and smp_store_release().
>
> So does this patch depends on a patch that removes pre EV56 alpha
> support? I'm all for removing that, but I need to see the patch merged
> before we can do this.
I'm working on that but Alpha's Kconfig is not quite straightforward.
... and I'm wondering if I should _remove_ pre-EV56 configurations or
move the default choice and produce a warning about unsupported Alpha
CPUs instead?
Regards,
Peter Hurley
[ How does one do a red popup in kbuild?
The 'comment' approach is too subtle.
]
^ permalink raw reply
* Re: bit fields && data tearing
From: Paul E. McKenney @ 2014-09-05 20:09 UTC (permalink / raw)
To: Peter Hurley
Cc: Jakub Jelinek, linux-arch@vger.kernel.org, Tony Luck,
linux-ia64@vger.kernel.org, Michael Cree, linux-alpha,
Oleg Nesterov, linux-kernel@vger.kernel.org, David Laight,
Paul Mackerras, H. Peter Anvin, linuxppc-dev@lists.ozlabs.org,
Miroslav Franc, Richard Henderson
In-Reply-To: <540A0DF3.8030802@hurleysoftware.com>
On Fri, Sep 05, 2014 at 03:24:35PM -0400, Peter Hurley wrote:
> On 09/05/2014 03:05 PM, Paul E. McKenney wrote:
> > On Fri, Sep 05, 2014 at 02:50:31PM -0400, Peter Hurley wrote:
> >> On 09/05/2014 02:09 PM, Paul E. McKenney wrote:
>
> [cut]
>
> >>> ------------------------------------------------------------------------
> >>>
> >>> documentation: Record limitations of bitfields and small variables
> >>>
> >>> This commit documents the fact that it is not safe to use bitfields as
> >>> shared variables in synchronization algorithms. It also documents that
> >>> CPUs must provide one-byte and two-byte load and store instructions
> >> ^
> >> atomic
> >
> > Here you meant non-atomic? My guess is that you are referring to the
> > fact that you could emulate a one-byte store on pre-EV56 Alpha CPUs
> > using the ll and sc atomic-read-modify-write instructions, correct?
>
> Yes, that's what I meant. I must be tired and am misreading the commit
> message, or misinterpreting it's meaning.
Very good, got it!
Thanx, Paul
^ permalink raw reply
* Re: bit fields && data tearing
From: Peter Zijlstra @ 2014-09-05 20:12 UTC (permalink / raw)
To: Peter Hurley
Cc: Jakub Jelinek, linux-arch@vger.kernel.org, Tony Luck,
linux-ia64@vger.kernel.org, Michael Cree, linux-alpha,
Oleg Nesterov, linux-kernel@vger.kernel.org, David Laight,
Paul Mackerras, H. Peter Anvin, Paul E. McKenney,
linuxppc-dev@lists.ozlabs.org, Miroslav Franc, Richard Henderson
In-Reply-To: <540A169F.40906@hurleysoftware.com>
On Fri, Sep 05, 2014 at 04:01:35PM -0400, Peter Hurley wrote:
> > So does this patch depends on a patch that removes pre EV56 alpha
> > support? I'm all for removing that, but I need to see the patch merged
> > before we can do this.
>
> I'm working on that but Alpha's Kconfig is not quite straightforward.
>
>
> ... and I'm wondering if I should _remove_ pre-EV56 configurations or
> move the default choice and produce a warning about unsupported Alpha
> CPUs instead?
>
depends BROKEN
or is that deprecated?
^ permalink raw reply
* Re: bit fields && data tearing
From: Peter Hurley @ 2014-09-05 20:14 UTC (permalink / raw)
To: Marc Gauthier, paulmck@linux.vnet.ibm.com
Cc: Jakub Jelinek, linux-arch@vger.kernel.org, Tony Luck,
linux-ia64@vger.kernel.org, Michael Cree,
linux-alpha@vger.kernel.org, Oleg Nesterov,
linux-kernel@vger.kernel.org, David Laight, Paul Mackerras,
H. Peter Anvin, linuxppc-dev@lists.ozlabs.org, Miroslav Franc,
Richard Henderson
In-Reply-To: <8CA974F497CA064FA9926E10ABCC061F05F97E7B77@MAILSJ4.global.cadence.com>
On 09/05/2014 03:38 PM, Marc Gauthier wrote:
> Paul E. McKenney wrote:
>> On Fri, Sep 05, 2014 at 02:50:31PM -0400, Peter Hurley wrote:
>>> On 09/05/2014 02:09 PM, Paul E. McKenney wrote:
>>>> This commit documents the fact that it is not safe to use bitfields as
>>>> shared variables in synchronization algorithms. It also documents that
>>>> CPUs must provide one-byte and two-byte load and store instructions
>>> ^
>>> atomic
>>
>> Here you meant non-atomic? My guess is that you are referring to the
>> fact that you could emulate a one-byte store on pre-EV56 Alpha CPUs
>> using the ll and sc atomic-read-modify-write instructions, correct?
>>
>>>> in order to be supported by the Linux kernel. (Michael Cree
>>>> has agreed to the resulting non-support of pre-EV56 Alpha CPUs:
>>>> https://lkml.org/lkml/2014/9/5/143.
> [...]
>
>>>> + and 64-bit systems, respectively. Note that this means that the
>>>> + Linux kernel does not support pre-EV56 Alpha CPUs, because these
>>>> + older CPUs do not provide one-byte and two-byte loads and stores.
>>> ^
>>> non-atomic
>>
>> I took this, thank you!
>
> Eum, am I totally lost, or aren't both of these supposed to say "atomic" ?
>
> Can't imagine requiring a CPU to provide non-atomic loads and stores
> (i.e. requiring old Alpha behavior?).
Here's how I read the two statements.
First, the commit message:
"It [this commit] documents that CPUs [supported by the Linux kernel]
_must provide_ atomic one-byte and two-byte naturally aligned loads and stores."
Second, in the body of the document:
"The Linux kernel no longer supports pre-EV56 Alpha CPUs, because these
older CPUs _do not provide_ atomic one-byte and two-byte loads and stores."
Regards,
Peter Hurley
^ permalink raw reply
* Re: bit fields && data tearing
From: H. Peter Anvin @ 2014-09-05 20:15 UTC (permalink / raw)
To: Peter Zijlstra, Peter Hurley
Cc: Jakub Jelinek, linux-arch@vger.kernel.org, Tony Luck,
linux-ia64@vger.kernel.org, Michael Cree, Oleg Nesterov,
linux-kernel@vger.kernel.org, David Laight, Paul Mackerras,
linux-alpha, Paul E. McKenney, linuxppc-dev@lists.ozlabs.org,
Miroslav Franc, Richard Henderson
In-Reply-To: <20140905201230.GV4783@worktop.ger.corp.intel.com>
On 09/05/2014 01:12 PM, Peter Zijlstra wrote:
>>
>> ... and I'm wondering if I should _remove_ pre-EV56 configurations or
>> move the default choice and produce a warning about unsupported Alpha
>> CPUs instead?
>>
>
> depends BROKEN
>
> or is that deprecated?
>
Just rip it out, like I did for the i386.
-hpa
^ permalink raw reply
* Re: bit fields && data tearing
From: Paul E. McKenney @ 2014-09-05 20:19 UTC (permalink / raw)
To: Peter Hurley
Cc: Jakub Jelinek, linux-arch@vger.kernel.org, Tony Luck,
linux-ia64@vger.kernel.org, Michael Cree, Peter Zijlstra,
linux-alpha, Oleg Nesterov, linux-kernel@vger.kernel.org,
David Laight, Paul Mackerras, H. Peter Anvin,
linuxppc-dev@lists.ozlabs.org, Miroslav Franc, Richard Henderson
In-Reply-To: <540A169F.40906@hurleysoftware.com>
On Fri, Sep 05, 2014 at 04:01:35PM -0400, Peter Hurley wrote:
> On 09/05/2014 03:52 PM, Peter Zijlstra wrote:
> > On Fri, Sep 05, 2014 at 11:31:09AM -0700, Paul E. McKenney wrote:
> >> compiler: Allow 1- and 2-byte smp_load_acquire() and smp_store_release()
> >>
> >> CPUs without single-byte and double-byte loads and stores place some
> >> "interesting" requirements on concurrent code. For example (adapted
> >> from Peter Hurley's test code), suppose we have the following structure:
> >>
> >> struct foo {
> >> spinlock_t lock1;
> >> spinlock_t lock2;
> >> char a; /* Protected by lock1. */
> >> char b; /* Protected by lock2. */
> >> };
> >> struct foo *foop;
> >>
> >> Of course, it is common (and good) practice to place data protected
> >> by different locks in separate cache lines. However, if the locks are
> >> rarely acquired (for example, only in rare error cases), and there are
> >> a great many instances of the data structure, then memory footprint can
> >> trump false-sharing concerns, so that it can be better to place them in
> >> the same cache cache line as above.
> >>
> >> But if the CPU does not support single-byte loads and stores, a store
> >> to foop->a will do a non-atomic read-modify-write operation on foop->b,
> >> which will come as a nasty surprise to someone holding foop->lock2. So we
> >> now require CPUs to support single-byte and double-byte loads and stores.
> >> Therefore, this commit adjusts the definition of __native_word() to allow
> >> these sizes to be used by smp_load_acquire() and smp_store_release().
> >
> > So does this patch depends on a patch that removes pre EV56 alpha
> > support? I'm all for removing that, but I need to see the patch merged
> > before we can do this.
>
> I'm working on that but Alpha's Kconfig is not quite straightforward.
>
>
> ... and I'm wondering if I should _remove_ pre-EV56 configurations or
> move the default choice and produce a warning about unsupported Alpha
> CPUs instead?
I suspect that either would work, given that the Alpha community is
pretty close-knit. Just setting the appropriate flag to make the
compiler generate one-byte and two-byte loads and stores would
probably suffice. ;-)
Thanx, Paul
> Regards,
> Peter Hurley
>
> [ How does one do a red popup in kbuild?
> The 'comment' approach is too subtle.
> ]
>
>
>
^ permalink raw reply
* RE: bit fields && data tearing
From: Marc Gauthier @ 2014-09-05 19:38 UTC (permalink / raw)
To: paulmck@linux.vnet.ibm.com, Peter Hurley
Cc: Jakub Jelinek, linux-arch@vger.kernel.org, Tony Luck,
linux-ia64@vger.kernel.org, Michael Cree,
linux-alpha@vger.kernel.org, Oleg Nesterov,
linux-kernel@vger.kernel.org, David Laight, Paul Mackerras,
H. Peter Anvin, linuxppc-dev@lists.ozlabs.org, Miroslav Franc,
Richard Henderson
In-Reply-To: <20140905190506.GV5001@linux.vnet.ibm.com>
Paul E. McKenney wrote:
>On Fri, Sep 05, 2014 at 02:50:31PM -0400, Peter Hurley wrote:
>>On 09/05/2014 02:09 PM, Paul E. McKenney wrote:
>>> This commit documents the fact that it is not safe to use bitfields as
>>> shared variables in synchronization algorithms. It also documents that
>>> CPUs must provide one-byte and two-byte load and store instructions
>> ^
>> atomic
>=20
> Here you meant non-atomic? My guess is that you are referring to the
> fact that you could emulate a one-byte store on pre-EV56 Alpha CPUs
> using the ll and sc atomic-read-modify-write instructions, correct?
>=20
>>> in order to be supported by the Linux kernel. (Michael Cree
>>> has agreed to the resulting non-support of pre-EV56 Alpha CPUs:
>>> https://lkml.org/lkml/2014/9/5/143.
[...]
>>> + and 64-bit systems, respectively. Note that this means that the
>>> + Linux kernel does not support pre-EV56 Alpha CPUs, because these
>>> + older CPUs do not provide one-byte and two-byte loads and stores.
>> ^
>> non-atomic
>=20
> I took this, thank you!
Eum, am I totally lost, or aren't both of these supposed to say "atomic" ?
Can't imagine requiring a CPU to provide non-atomic loads and stores
(i.e. requiring old Alpha behavior?).
-Marc
^ permalink raw reply
* Re: bit fields && data tearing
From: H. Peter Anvin @ 2014-09-05 20:34 UTC (permalink / raw)
To: Peter Hurley, Marc Gauthier, paulmck@linux.vnet.ibm.com
Cc: Jakub Jelinek, linux-arch@vger.kernel.org, Tony Luck,
linux-ia64@vger.kernel.org, Michael Cree, Oleg Nesterov,
linux-kernel@vger.kernel.org, David Laight, Paul Mackerras,
linux-alpha@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
Miroslav Franc, Richard Henderson
In-Reply-To: <540A19B8.4010907@hurleysoftware.com>
On 09/05/2014 01:14 PM, Peter Hurley wrote:
>
> Here's how I read the two statements.
>
> First, the commit message:
>
> "It [this commit] documents that CPUs [supported by the Linux kernel]
> _must provide_ atomic one-byte and two-byte naturally aligned loads and stores."
>
> Second, in the body of the document:
>
> "The Linux kernel no longer supports pre-EV56 Alpha CPUs, because these
> older CPUs _do not provide_ atomic one-byte and two-byte loads and stores."
>
Does this apply in general or only to SMP configurations? I guess
non-SMP configurations would still have problems if interrupted in the
wrong place...
-hpa
^ permalink raw reply
* Re: bit fields && data tearing
From: Michael Cree @ 2014-09-05 20:39 UTC (permalink / raw)
To: Peter Hurley
Cc: Jakub Jelinek, linux-arch@vger.kernel.org, Tony Luck,
David Laight, linux-alpha@vger.kernel.org, Oleg Nesterov,
linux-kernel@vger.kernel.org, Marc Gauthier, Paul Mackerras,
H. Peter Anvin, linux-ia64@vger.kernel.org,
paulmck@linux.vnet.ibm.com, linuxppc-dev@lists.ozlabs.org,
Miroslav Franc, Richard Henderson
In-Reply-To: <540A19B8.4010907@hurleysoftware.com>
On Fri, Sep 05, 2014 at 04:14:48PM -0400, Peter Hurley wrote:
> Second, in the body of the document:
>
> "The Linux kernel no longer supports pre-EV56 Alpha CPUs, because these
> older CPUs _do not provide_ atomic one-byte and two-byte loads and stores."
Let's be clear here, the pre-EV56 Alpha CPUs do provide an atomic
one-byte and two-byte load and store; it's just that one must use
locked load and store sequences to achieve atomicity. The point,
I think, is that the pre-EV56 Alpha CPUs provide non-atomic one-byte
and two-byte load and stores as the norm, and that is the problem.
Cheers
Michael.
^ permalink raw reply
* Re: bit fields && data tearing
From: Paul E. McKenney @ 2014-09-05 20:42 UTC (permalink / raw)
To: Peter Hurley
Cc: Jakub Jelinek, linux-arch@vger.kernel.org, Tony Luck,
linux-ia64@vger.kernel.org, Michael Cree,
linux-alpha@vger.kernel.org, Oleg Nesterov,
linux-kernel@vger.kernel.org, Marc Gauthier, Paul Mackerras,
H. Peter Anvin, David Laight, linuxppc-dev@lists.ozlabs.org,
Miroslav Franc, Richard Henderson
In-Reply-To: <540A19B8.4010907@hurleysoftware.com>
On Fri, Sep 05, 2014 at 04:14:48PM -0400, Peter Hurley wrote:
> On 09/05/2014 03:38 PM, Marc Gauthier wrote:
> > Paul E. McKenney wrote:
> >> On Fri, Sep 05, 2014 at 02:50:31PM -0400, Peter Hurley wrote:
> >>> On 09/05/2014 02:09 PM, Paul E. McKenney wrote:
> >>>> This commit documents the fact that it is not safe to use bitfields as
> >>>> shared variables in synchronization algorithms. It also documents that
> >>>> CPUs must provide one-byte and two-byte load and store instructions
> >>> ^
> >>> atomic
> >>
> >> Here you meant non-atomic? My guess is that you are referring to the
> >> fact that you could emulate a one-byte store on pre-EV56 Alpha CPUs
> >> using the ll and sc atomic-read-modify-write instructions, correct?
> >>
> >>>> in order to be supported by the Linux kernel. (Michael Cree
> >>>> has agreed to the resulting non-support of pre-EV56 Alpha CPUs:
> >>>> https://lkml.org/lkml/2014/9/5/143.
> > [...]
> >
> >>>> + and 64-bit systems, respectively. Note that this means that the
> >>>> + Linux kernel does not support pre-EV56 Alpha CPUs, because these
> >>>> + older CPUs do not provide one-byte and two-byte loads and stores.
> >>> ^
> >>> non-atomic
> >>
> >> I took this, thank you!
> >
> > Eum, am I totally lost, or aren't both of these supposed to say "atomic" ?
> >
> > Can't imagine requiring a CPU to provide non-atomic loads and stores
> > (i.e. requiring old Alpha behavior?).
>
> Here's how I read the two statements.
>
> First, the commit message:
>
> "It [this commit] documents that CPUs [supported by the Linux kernel]
> _must provide_ atomic one-byte and two-byte naturally aligned loads and stores."
>
> Second, in the body of the document:
>
> "The Linux kernel no longer supports pre-EV56 Alpha CPUs, because these
> older CPUs _do not provide_ atomic one-byte and two-byte loads and stores."
Hmmm... It is a bit ambiguous. How about the following?
Thanx, Paul
------------------------------------------------------------------------
documentation: Record limitations of bitfields and small variables
This commit documents the fact that it is not safe to use bitfields
as shared variables in synchronization algorithms. It also documents
that CPUs must provide one-byte and two-byte normal load and store
instructions in order to be supported by the Linux kernel. (Michael
Cree has agreed to the resulting non-support of pre-EV56 Alpha CPUs:
https://lkml.org/lkml/2014/9/5/143.)
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt
index 87be0a8a78de..fe4d51b704c5 100644
--- a/Documentation/memory-barriers.txt
+++ b/Documentation/memory-barriers.txt
@@ -269,6 +269,37 @@ And there are a number of things that _must_ or _must_not_ be assumed:
STORE *(A + 4) = Y; STORE *A = X;
STORE {*A, *(A + 4) } = {X, Y};
+And there are anti-guarantees:
+
+ (*) These guarantees do not apply to bitfields, because compilers often
+ generate code to modify these using non-atomic read-modify-write
+ sequences. Do not attempt to use bitfields to synchronize parallel
+ algorithms.
+
+ (*) Even in cases where bitfields are protected by locks, all fields
+ in a given bitfield must be protected by one lock. If two fields
+ in a given bitfield are protected by different locks, the compiler's
+ non-atomic read-modify-write sequences can cause an update to one
+ field to corrupt the value of an adjacent field.
+
+ (*) These guarantees apply only to properly aligned and sized scalar
+ variables. "Properly sized" currently means variables that are
+ the same size as "char", "short", "int" and "long". "Properly
+ aligned" means the natural alignment, thus no constraints for
+ "char", two-byte alignment for "short", four-byte alignment for
+ "int", and either four-byte or eight-byte alignment for "long",
+ on 32-bit and 64-bit systems, respectively. Note that this means
+ that the Linux kernel does not support pre-EV56 Alpha CPUs,
+ because these older CPUs do not provide one-byte and two-byte
+ load and store instructions. (In theory, the pre-EV56 Alpha CPUs
+ can emulate these instructions using load-linked/store-conditional
+ instructions, but in practice this approach has excessive overhead.
+ Keep in mind that this emulation would be required on -all- single-
+ and double-byte loads and stores in order to handle adjacent bytes
+ protected by different locks.)
+
+ Alpha EV56 and later Alpha CPUs are still supported.
+
=========================
WHAT ARE MEMORY BARRIERS?
^ permalink raw reply related
* Re: bit fields && data tearing
From: Paul E. McKenney @ 2014-09-05 20:43 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Jakub Jelinek, linux-arch@vger.kernel.org, Tony Luck,
linux-ia64@vger.kernel.org, Peter Hurley, Michael Cree,
Oleg Nesterov, linux-kernel@vger.kernel.org, Marc Gauthier,
Paul Mackerras, linux-alpha@vger.kernel.org, David Laight,
linuxppc-dev@lists.ozlabs.org, Miroslav Franc, Richard Henderson
In-Reply-To: <540A1E6C.2020005@zytor.com>
On Fri, Sep 05, 2014 at 01:34:52PM -0700, H. Peter Anvin wrote:
> On 09/05/2014 01:14 PM, Peter Hurley wrote:
> >
> > Here's how I read the two statements.
> >
> > First, the commit message:
> >
> > "It [this commit] documents that CPUs [supported by the Linux kernel]
> > _must provide_ atomic one-byte and two-byte naturally aligned loads and stores."
> >
> > Second, in the body of the document:
> >
> > "The Linux kernel no longer supports pre-EV56 Alpha CPUs, because these
> > older CPUs _do not provide_ atomic one-byte and two-byte loads and stores."
> >
>
> Does this apply in general or only to SMP configurations? I guess
> non-SMP configurations would still have problems if interrupted in the
> wrong place...
And preemption could cause problems, too. So I believe that it needs
to be universal.
Thanx, Paul
^ permalink raw reply
* Re: bit fields && data tearing
From: Michael Cree @ 2014-09-05 20:42 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Jakub Jelinek, linux-arch@vger.kernel.org, Tony Luck,
David Laight, Peter Hurley, Oleg Nesterov,
linux-kernel@vger.kernel.org, Marc Gauthier, Paul Mackerras,
linux-alpha@vger.kernel.org, linux-ia64@vger.kernel.org,
paulmck@linux.vnet.ibm.com, linuxppc-dev@lists.ozlabs.org,
Miroslav Franc, Richard Henderson
In-Reply-To: <540A1E6C.2020005@zytor.com>
On Fri, Sep 05, 2014 at 01:34:52PM -0700, H. Peter Anvin wrote:
> On 09/05/2014 01:14 PM, Peter Hurley wrote:
> >
> > Here's how I read the two statements.
> >
> > First, the commit message:
> >
> > "It [this commit] documents that CPUs [supported by the Linux kernel]
> > _must provide_ atomic one-byte and two-byte naturally aligned loads and stores."
> >
> > Second, in the body of the document:
> >
> > "The Linux kernel no longer supports pre-EV56 Alpha CPUs, because these
> > older CPUs _do not provide_ atomic one-byte and two-byte loads and stores."
> >
>
> Does this apply in general or only to SMP configurations? I guess
> non-SMP configurations would still have problems if interrupted in the
> wrong place...
Yes.
Cheers
Michael.
^ permalink raw reply
* Re: bit fields && data tearing
From: Thomas Gleixner @ 2014-09-05 20:48 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Jakub Jelinek, linux-arch@vger.kernel.org, Tony Luck,
linux-ia64@vger.kernel.org, Peter Hurley, Michael Cree,
linux-alpha@vger.kernel.org, Oleg Nesterov,
linux-kernel@vger.kernel.org, Marc Gauthier, Paul Mackerras,
H. Peter Anvin, David Laight, linuxppc-dev@lists.ozlabs.org,
Miroslav Franc, Richard Henderson
In-Reply-To: <20140905204312.GA5001@linux.vnet.ibm.com>
On Fri, 5 Sep 2014, Paul E. McKenney wrote:
> On Fri, Sep 05, 2014 at 01:34:52PM -0700, H. Peter Anvin wrote:
> > On 09/05/2014 01:14 PM, Peter Hurley wrote:
> > >
> > > Here's how I read the two statements.
> > >
> > > First, the commit message:
> > >
> > > "It [this commit] documents that CPUs [supported by the Linux kernel]
> > > _must provide_ atomic one-byte and two-byte naturally aligned loads and stores."
> > >
> > > Second, in the body of the document:
> > >
> > > "The Linux kernel no longer supports pre-EV56 Alpha CPUs, because these
> > > older CPUs _do not provide_ atomic one-byte and two-byte loads and stores."
> > >
> >
> > Does this apply in general or only to SMP configurations? I guess
> > non-SMP configurations would still have problems if interrupted in the
> > wrong place...
>
> And preemption could cause problems, too. So I believe that it needs
> to be universal.
Well preemption is usually caused by an interrupt, except you have a
combined load and preempt instruction :)
Thanks,
tglx
^ permalink raw reply
* Re: bit fields && data tearing
From: Paul E. McKenney @ 2014-09-05 21:05 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Jakub Jelinek, linux-arch@vger.kernel.org, Tony Luck,
linux-ia64@vger.kernel.org, Peter Hurley, Michael Cree,
linux-alpha@vger.kernel.org, Oleg Nesterov,
linux-kernel@vger.kernel.org, Marc Gauthier, Paul Mackerras,
H. Peter Anvin, David Laight, linuxppc-dev@lists.ozlabs.org,
Miroslav Franc, Richard Henderson
In-Reply-To: <alpine.DEB.2.10.1409052247460.5472@nanos>
On Fri, Sep 05, 2014 at 10:48:34PM +0200, Thomas Gleixner wrote:
> On Fri, 5 Sep 2014, Paul E. McKenney wrote:
> > On Fri, Sep 05, 2014 at 01:34:52PM -0700, H. Peter Anvin wrote:
> > > On 09/05/2014 01:14 PM, Peter Hurley wrote:
> > > >
> > > > Here's how I read the two statements.
> > > >
> > > > First, the commit message:
> > > >
> > > > "It [this commit] documents that CPUs [supported by the Linux kernel]
> > > > _must provide_ atomic one-byte and two-byte naturally aligned loads and stores."
> > > >
> > > > Second, in the body of the document:
> > > >
> > > > "The Linux kernel no longer supports pre-EV56 Alpha CPUs, because these
> > > > older CPUs _do not provide_ atomic one-byte and two-byte loads and stores."
> > > >
> > >
> > > Does this apply in general or only to SMP configurations? I guess
> > > non-SMP configurations would still have problems if interrupted in the
> > > wrong place...
> >
> > And preemption could cause problems, too. So I believe that it needs
> > to be universal.
>
> Well preemption is usually caused by an interrupt, except you have a
> combined load and preempt instruction :)
Fair point! ;-)
Thanx, Paul
^ permalink raw reply
* Re: bit fields && data tearing
From: Peter Hurley @ 2014-09-05 21:12 UTC (permalink / raw)
To: Michael Cree, Marc Gauthier, paulmck@linux.vnet.ibm.com,
H. Peter Anvin, Benjamin Herrenschmidt, David Laight,
Jakub Jelinek, linux-arch@vger.kernel.org, Tony Luck,
linux-ia64@vger.kernel.org, Oleg Nesterov,
linux-kernel@vger.kernel.org, Paul Mackerras,
linuxppc-dev@lists.ozlabs.org, Miroslav Franc, Richard Henderson,
linux-alpha@vger.kernel.org
In-Reply-To: <20140905203951.GA4053@omega>
On 09/05/2014 04:39 PM, Michael Cree wrote:
> On Fri, Sep 05, 2014 at 04:14:48PM -0400, Peter Hurley wrote:
>> Second, in the body of the document:
>>
>> "The Linux kernel no longer supports pre-EV56 Alpha CPUs, because these
>> older CPUs _do not provide_ atomic one-byte and two-byte loads and stores."
>
> Let's be clear here, the pre-EV56 Alpha CPUs do provide an atomic
> one-byte and two-byte load and store; it's just that one must use
> locked load and store sequences to achieve atomicity. The point,
> I think, is that the pre-EV56 Alpha CPUs provide non-atomic one-byte
> and two-byte load and stores as the norm, and that is the problem.
I'm all for an Alpha expert to jump in here and meet the criteria;
which is that byte stores cannot corrupt adjacent storage (nor can
aligned short stores).
To my mind, a quick look at Documentation/circular-buffers.txt will
pretty much convince anyone that trying to differentiate by execution
context is undoable.
If someone wants to make Alphas do cmpxchg loops for every byte store,
then ok. Or any other solution that doesn't require subsystem code
changes.
Regards,
Peter Hurley
^ permalink raw reply
* Re: [PATCH 0/2] PCI/MSI: Remove arch_msi_check_device()
From: Bjorn Helgaas @ 2014-09-05 21:25 UTC (permalink / raw)
To: Alexander Gordeev; +Cc: linux-pci, linuxppc-dev, linux-kernel
In-Reply-To: <cover.1405160163.git.agordeev@redhat.com>
On Sat, Jul 12, 2014 at 01:21:06PM +0200, Alexander Gordeev wrote:
> Hello,
>
> This is a cleanup effort to get rid of useless arch_msi_check_device().
> I am not sure what were the reasons for its existence in the first place,
> but at the moment it appears totally unnecessary.
>
> Thanks!
>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-pci@vger.kernel.org
>
> Alexander Gordeev (2):
> PCI/MSI/PPC: Remove arch_msi_check_device()
> PCI/MSI: Remove arch_msi_check_device()
I applied these (with Michael's ack on the first, and v2 of the second) to
pci/msi for v3.18, thanks!
>
> arch/powerpc/include/asm/machdep.h | 2 -
> arch/powerpc/kernel/msi.c | 12 +-------
> arch/powerpc/platforms/cell/axon_msi.c | 9 ------
> arch/powerpc/platforms/powernv/pci.c | 19 +++---------
> arch/powerpc/platforms/pseries/msi.c | 42
> ++++++++++----------------- arch/powerpc/sysdev/fsl_msi.c |
> 12 ++------ arch/powerpc/sysdev/mpic_pasemi_msi.c | 11 +------
> arch/powerpc/sysdev/mpic_u3msi.c | 28 +++++++-----------
> arch/powerpc/sysdev/ppc4xx_hsta_msi.c | 18 ++++--------
> arch/powerpc/sysdev/ppc4xx_msi.c | 19 ++++--------
> drivers/pci/msi.c | 49
> ++++++++----------------------- include/linux/msi.h |
> 3 -- 12 files changed, 63 insertions(+), 161 deletions(-)
>
> -- 1.7.7.6
>
> -- To unsubscribe from this list: send the line "unsubscribe linux-pci"
> in the body of a message to majordomo@vger.kernel.org More majordomo info
> at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 0/2] PCI/MSI: Remove arch_msi_check_device()
From: Bjorn Helgaas @ 2014-09-05 21:27 UTC (permalink / raw)
To: Alexander Gordeev
Cc: linux-pci@vger.kernel.org, linuxppc-dev,
linux-kernel@vger.kernel.org
In-Reply-To: <20140905212528.GG8080@google.com>
On Fri, Sep 5, 2014 at 3:25 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Sat, Jul 12, 2014 at 01:21:06PM +0200, Alexander Gordeev wrote:
>> Hello,
>>
>> This is a cleanup effort to get rid of useless arch_msi_check_device().
>> I am not sure what were the reasons for its existence in the first place,
>> but at the moment it appears totally unnecessary.
>>
>> Thanks!
>>
>> Cc: linuxppc-dev@lists.ozlabs.org
>> Cc: linux-pci@vger.kernel.org
>>
>> Alexander Gordeev (2):
>> PCI/MSI/PPC: Remove arch_msi_check_device()
>> PCI/MSI: Remove arch_msi_check_device()
>
> I applied these (with Michael's ack on the first, and v2 of the second) to
> pci/msi for v3.18, thanks!
>>
>> arch/powerpc/include/asm/machdep.h | 2 -
>> arch/powerpc/kernel/msi.c | 12 +-------
>> arch/powerpc/platforms/cell/axon_msi.c | 9 ------
>> arch/powerpc/platforms/powernv/pci.c | 19 +++---------
>> arch/powerpc/platforms/pseries/msi.c | 42
>> ++++++++++----------------- arch/powerpc/sysdev/fsl_msi.c |
>> 12 ++------ arch/powerpc/sysdev/mpic_pasemi_msi.c | 11 +------
>> arch/powerpc/sysdev/mpic_u3msi.c | 28 +++++++-----------
>> arch/powerpc/sysdev/ppc4xx_hsta_msi.c | 18 ++++--------
>> arch/powerpc/sysdev/ppc4xx_msi.c | 19 ++++--------
>> drivers/pci/msi.c | 49
>> ++++++++----------------------- include/linux/msi.h |
>> 3 -- 12 files changed, 63 insertions(+), 161 deletions(-)
Oh, I forgot -- if you'd rather take the first one through the PPC
tree, you can do that and I can merge the second one later. Let me
know if you want to do that.
Bjorn
^ permalink raw reply
* Re: bit fields && data tearing
From: Michael Cree @ 2014-09-05 21:27 UTC (permalink / raw)
To: Peter Hurley
Cc: Jakub Jelinek, linux-arch@vger.kernel.org, Tony Luck,
David Laight, linux-alpha@vger.kernel.org, Oleg Nesterov,
linux-kernel@vger.kernel.org, Marc Gauthier, Paul Mackerras,
H. Peter Anvin, linux-ia64@vger.kernel.org,
paulmck@linux.vnet.ibm.com, linuxppc-dev@lists.ozlabs.org,
Miroslav Franc, Richard Henderson
In-Reply-To: <540A273C.7020804@hurleysoftware.com>
On Fri, Sep 05, 2014 at 05:12:28PM -0400, Peter Hurley wrote:
> On 09/05/2014 04:39 PM, Michael Cree wrote:
> > On Fri, Sep 05, 2014 at 04:14:48PM -0400, Peter Hurley wrote:
> >> Second, in the body of the document:
> >>
> >> "The Linux kernel no longer supports pre-EV56 Alpha CPUs, because these
> >> older CPUs _do not provide_ atomic one-byte and two-byte loads and stores."
> >
> > Let's be clear here, the pre-EV56 Alpha CPUs do provide an atomic
> > one-byte and two-byte load and store; it's just that one must use
> > locked load and store sequences to achieve atomicity. The point,
> > I think, is that the pre-EV56 Alpha CPUs provide non-atomic one-byte
> > and two-byte load and stores as the norm, and that is the problem.
>
> I'm all for an Alpha expert to jump in here and meet the criteria;
> which is that byte stores cannot corrupt adjacent storage (nor can
> aligned short stores).
>
> To my mind, a quick look at Documentation/circular-buffers.txt will
> pretty much convince anyone that trying to differentiate by execution
> context is undoable.
>
> If someone wants to make Alphas do cmpxchg loops for every byte store,
> then ok. Or any other solution that doesn't require subsystem code
> changes.
I am not suggesting that anyone do that work. I'm certainly not going
to do it.
All I was pointing out is that the claim that "_do not provide_" made
above with emphasis is, strictly interpreted, not true, thus should not
be committed to the documentation without further clarification.
Cheers
Michael.
^ permalink raw reply
* Re: [PATCH] PCI: Increase BAR size quirk for IBM ipr SAS Crocodile adapters
From: Bjorn Helgaas @ 2014-09-06 0:18 UTC (permalink / raw)
To: Anton Blanchard; +Cc: linux-pci, miltonm, linuxppc-dev, linux-kernel, dllehr
In-Reply-To: <20140821092652.604d7484@kryten>
On Thu, Aug 21, 2014 at 09:26:52AM +1000, Anton Blanchard wrote:
> From: Douglas Lehr <dllehr@us.ibm.com>
>
> The Crocodile chip occasionally comes up with 4k and 8k BAR sizes.
> Due to an errata, setting the SR-IOV page size causes the physical
> function BARs to expand to the system page size. Since ppc64 uses
> 64k pages, when Linux tries to assign the smaller resource sizes
> to the now 64k BARs the address will be truncated and the BARs will
> overlap.
>
> This quirk will force Linux to allocate the resource as a full page,
> which will avoid the overlap.
>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Douglas Lehr <dllehr@us.ibm.com>
> Signed-off-by: Anton Blanchard <anton@samba.org>
> Acked-by: Milton Miller <miltonm@us.ibm.com>
Applied to pci/misc for v3.18, thanks!
I tweaked it to print the expanded resource, see below.
> ---
> drivers/pci/quirks.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 80c2d01..45b946d 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -24,6 +24,7 @@
> #include <linux/ioport.h>
> #include <linux/sched.h>
> #include <linux/ktime.h>
> +#include <linux/mm.h>
> #include <asm/dma.h> /* isa_dma_bridge_buggy */
> #include "pci.h"
>
> @@ -287,6 +288,24 @@ static void quirk_citrine(struct pci_dev *dev)
> }
> DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_CITRINE, quirk_citrine);
>
> +/* On IBM Crocodile ipr SAS adapters, expand bar size to system page size. */
> +static void quirk_extend_bar_to_page(struct pci_dev *dev)
> +{
> + int i;
> +
> + for (i = 0; i < PCI_STD_RESOURCE_END; i++) {
> + struct resource *r = &dev->resource[i];
> +
> + if (r->flags & IORESOURCE_MEM && resource_size(r) < PAGE_SIZE) {
> + dev_info(&dev->dev, "Setting Bar size to Page size");
> + r->end = PAGE_SIZE-1;
> + r->start = 0;
> + r->flags |= IORESOURCE_UNSET;
> + }
> + }
> +}
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_IBM, 0x034a, quirk_extend_bar_to_page);
> +
> /*
> * S3 868 and 968 chips report region size equal to 32M, but they decode 64M.
> * If it's needed, re-allocate the region.
> --
commit 86b6431a306ab5a5204c436a45a3337fb17efa21
Author: Douglas Lehr <dllehr@us.ibm.com>
Date: Thu Aug 21 09:26:52 2014 +1000
PCI: Increase IBM ipr SAS Crocodile BARs to at least system page size
The Crocodile chip occasionally comes up with 4k and 8k BAR sizes. Due to
an erratum, setting the SR-IOV page size causes the physical function BARs
to expand to the system page size. Since ppc64 uses 64k pages, when Linux
tries to assign the smaller resource sizes to the now 64k BARs the address
will be truncated and the BARs will overlap.
Force Linux to allocate the resource as a full page, which avoids the
overlap.
[bhelgaas: print expanded resource, too]
Signed-off-by: Douglas Lehr <dllehr@us.ibm.com>
Signed-off-by: Anton Blanchard <anton@samba.org>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Acked-by: Milton Miller <miltonm@us.ibm.com>
CC: stable@vger.kernel.org
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 80c2d014283d..e73960311fb4 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -24,6 +24,7 @@
#include <linux/ioport.h>
#include <linux/sched.h>
#include <linux/ktime.h>
+#include <linux/mm.h>
#include <asm/dma.h> /* isa_dma_bridge_buggy */
#include "pci.h"
@@ -287,6 +288,25 @@ static void quirk_citrine(struct pci_dev *dev)
}
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_IBM, PCI_DEVICE_ID_IBM_CITRINE, quirk_citrine);
+/* On IBM Crocodile ipr SAS adapters, expand BAR to system page size */
+static void quirk_extend_bar_to_page(struct pci_dev *dev)
+{
+ int i;
+
+ for (i = 0; i < PCI_STD_RESOURCE_END; i++) {
+ struct resource *r = &dev->resource[i];
+
+ if (r->flags & IORESOURCE_MEM && resource_size(r) < PAGE_SIZE) {
+ r->end = PAGE_SIZE - 1;
+ r->start = 0;
+ r->flags |= IORESOURCE_UNSET;
+ dev_info(&dev->dev, "expanded BAR %d to page size: %pR\n",
+ r);
+ }
+ }
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_IBM, 0x034a, quirk_extend_bar_to_page);
+
/*
* S3 868 and 968 chips report region size equal to 32M, but they decode 64M.
* If it's needed, re-allocate the region.
^ permalink raw reply related
* Re: [PATCH v2] deb-pkg: Add support for powerpc little endian
From: Ben Hutchings @ 2014-09-07 2:42 UTC (permalink / raw)
To: Thadeu Lima de Souza Cascardo
Cc: Michal Marek, Michael Neuling, linux-kbuild, linux-kernel,
Anton Blanchard, Leann Ogasawara, linuxppc-dev
In-Reply-To: <20140905120925.GB3846@oc0268524204.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 2242 bytes --]
On Fri, 2014-09-05 at 09:09 -0300, Thadeu Lima de Souza Cascardo wrote:
> On Fri, Sep 05, 2014 at 05:55:18PM +1000, Michael Neuling wrote:
> > On Fri, 2014-09-05 at 09:13 +0200, Gabriel Paubert wrote:
> > > On Fri, Sep 05, 2014 at 03:28:47PM +1000, Michael Neuling wrote:
> > > > The Debian powerpc little endian architecture is called ppc64le. This
> > >
> > > Huh? ppc64le or ppc64el?
> >
> > ppc64el. Commit message is wrong. Fixed below.
> >
> > Mikey
> >
> >
>
> What about ppc64?
>
> Also, I sent that already a month ago. Both linuxppc-dev and Michal
> Marek were on cc.
>
> http://marc.info/?l=linux-kernel&m=140744360328562&w=2
Anyone using powerpc (32-bit) will then need to add ppc64 as a foreign
architecture before they can install a 64-bit custom kernel. This is
fine in principle, except that ppc64 is not an official Debian port and
its packages are not mirrored on the same servers.
I just tried something similar, which is to add x32 (also unofficial) as
an alternate architecture to my biarch x86 system. APT now complains:
W: Failed to fetch http://http.debian.net/debian/dists/experimental/InRelease Unable to find expected entry 'main/binary-x32/Packages' in Release file (Wrong sources.list entry or malformed file)
W: Failed to fetch http://http.debian.net/debian/dists/sid/Release Unable to find expected entry 'main/binary-x32/Packages' in Release file (Wrong sources.list entry or malformed file)
W: Failed to fetch http://http.debian.net/debian/dists/testing/Release Unable to find expected entry 'main/binary-x32/Packages' in Release file (Wrong sources.list entry or malformed file)
W: Failed to fetch http://http.debian.net/debian/dists/wheezy/Release Unable to find expected entry 'main/binary-x32/Packages' in Release file (Wrong sources.list entry or malformed file)
E: Some index files failed to download. They have been ignored, or old ones used instead.
So I think Michael's version, leaving big-endian kernels as powerpc by
default, is preferable for now.
Ben.
--
Ben Hutchings
Experience is directly proportional to the value of equipment destroyed.
- Carolyn Scheppner
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply
* Re: bit fields && data tearing
From: James Bottomley @ 2014-09-07 5:07 UTC (permalink / raw)
To: paulmck
Cc: Jakub Jelinek, One Thousand Gnomes, linux-arch, linux-ia64,
Peter Hurley, Mikael Pettersson, Oleg Nesterov, linux-kernel,
Tony Luck, Paul Mackerras, H. Peter Anvin, linuxppc-dev,
Miroslav Franc, Richard Henderson
In-Reply-To: <20140905040645.GO5001@linux.vnet.ibm.com>
On Thu, 2014-09-04 at 21:06 -0700, Paul E. McKenney wrote:
> On Thu, Sep 04, 2014 at 10:47:24PM -0400, Peter Hurley wrote:
> > Hi James,
> >
> > On 09/04/2014 10:11 PM, James Bottomley wrote:
> > > On Thu, 2014-09-04 at 17:17 -0700, Paul E. McKenney wrote:
> > >> +And there are anti-guarantees:
> > >> +
> > >> + (*) These guarantees do not apply to bitfields, because compilers often
> > >> + generate code to modify these using non-atomic read-modify-write
> > >> + sequences. Do not attempt to use bitfields to synchronize parallel
> > >> + algorithms.
> > >> +
> > >> + (*) Even in cases where bitfields are protected by locks, all fields
> > >> + in a given bitfield must be protected by one lock. If two fields
> > >> + in a given bitfield are protected by different locks, the compiler's
> > >> + non-atomic read-modify-write sequences can cause an update to one
> > >> + field to corrupt the value of an adjacent field.
> > >> +
> > >> + (*) These guarantees apply only to properly aligned and sized scalar
> > >> + variables. "Properly sized" currently means "int" and "long",
> > >> + because some CPU families do not support loads and stores of
> > >> + other sizes. ("Some CPU families" is currently believed to
> > >> + be only Alpha 21064. If this is actually the case, a different
> > >> + non-guarantee is likely to be formulated.)
> > >
> > > This is a bit unclear. Presumably you're talking about definiteness of
> > > the outcome (as in what's seen after multiple stores to the same
> > > variable).
> >
> > No, the last conditions refers to adjacent byte stores from different
> > cpu contexts (either interrupt or SMP).
> >
> > > The guarantees are only for natural width on Parisc as well,
> > > so you would get a mess if you did byte stores to adjacent memory
> > > locations.
> >
> > For a simple test like:
> >
> > struct x {
> > long a;
> > char b;
> > char c;
> > char d;
> > char e;
> > };
> >
> > void store_bc(struct x *p) {
> > p->b = 1;
> > p->c = 2;
> > }
> >
> > on parisc, gcc generates separate byte stores
> >
> > void store_bc(struct x *p) {
> > 0: 34 1c 00 02 ldi 1,ret0
> > 4: 0f 5c 12 08 stb ret0,4(r26)
> > 8: 34 1c 00 04 ldi 2,ret0
> > c: e8 40 c0 00 bv r0(rp)
> > 10: 0f 5c 12 0a stb ret0,5(r26)
> >
> > which appears to confirm that on parisc adjacent byte data
> > is safe from corruption by concurrent cpu updates; that is,
> >
> > CPU 0 | CPU 1
> > |
> > p->b = 1 | p->c = 2
> > |
> >
> > will result in p->b == 1 && p->c == 2 (assume both values
> > were 0 before the call to store_bc()).
>
> What Peter said. I would ask for suggestions for better wording, but
> I would much rather be able to say that single-byte reads and writes
> are atomic and that aligned-short reads and writes are also atomic.
>
> Thus far, it looks like we lose only very old Alpha systems, so unless
> I hear otherwise, I update my patch to outlaw these very old systems.
This isn't universally true according to the architecture manual. The
PARISC CPU can make byte to long word stores atomic against the memory
bus but not against the I/O bus for instance. Atomicity is a property
of the underlying substrate, not of the CPU. Implying that atomicity is
a CPU property is incorrect.
James
^ permalink raw reply
* Re: [RFC PATCH 0/6] Change vendor prefix for Intersil Corporation
From: Jason Cooper @ 2014-09-07 13:32 UTC (permalink / raw)
To: Philipp Zabel
Cc: Mark Rutland, devicetree, Alessandro Zummo, Russell King,
Pawel Moll, Ian Campbell, Rob Herring, Paul Mackerras, Kumar Gala,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <20140820121643.GU12769@titan.lakedaemon.net>
On Wed, Aug 20, 2014 at 08:16:43AM -0400, Jason Cooper wrote:
> Philipp,
>
> On Wed, Aug 20, 2014 at 10:42:58AM +0200, Philipp Zabel wrote:
> > Hi,
> >
> > currently there is a wild mixture of isl, isil, and intersil
> > compatibles in the kernel. I figure at this point it is still
> > possible to change this to use "isil" everywhere without too
> > much pain, but it might be preferred to keep the already
> > documented "isl" prefix, even though it doesn't follow the
> > convention to use the NASDAQ symbol where available.
> >
> > The current users of the "isil" prefix are the following drivers
> > and device trees, so we could just as well change those instead:
> > arch/arm/boot/dts/tegra20-seaboard.dts
> > arch/arm/boot/dts/tegra20-ventana.dts
> > arch/arm/boot/dts/tegra30-cardhu.dtsi
> > arch/powerpc/boot/dts/p1022rdk.dts
> > drivers/staging/iio/light/isl29018.c
> > drivers/staging/iio/light/isl29028.c
> >
> > regards
> > Philipp
> >
> > Philipp Zabel (6):
> > of: Change vendor prefix for Intersil Corporation to isil
> > Documentation: Add isl1208 and isl12022 to trivial-devices list
> > ARM: mvebu: Change vendor prefix for Intersil Corporation to isil
> > powerpc/85xx: Change vendor prefix for Intersil Corporation to isil
> > rtc: rtc-isl12022: Change vendor prefix for Intersil Corporation to
> > isil
> > rtc: rtc-isl12057: Change vendor prefix for Intersil Corporation to
> > isil
> >
> > Documentation/devicetree/bindings/i2c/trivial-devices.txt | 4 +++-
> > Documentation/devicetree/bindings/vendor-prefixes.txt | 2 +-
> > arch/arm/boot/dts/armada-370-netgear-rn102.dts | 2 +-
> > arch/arm/boot/dts/armada-370-netgear-rn104.dts | 2 +-
> > arch/arm/boot/dts/armada-xp-netgear-rn2120.dts | 2 +-
> > arch/powerpc/boot/dts/ppa8548.dts | 2 +-
> > drivers/rtc/rtc-isl12022.c | 3 ++-
> > drivers/rtc/rtc-isl12057.c | 3 ++-
> > 8 files changed, 12 insertions(+), 8 deletions(-)
>
> This looks good overall. My only nit is that I'd like to see the legacy
> name(s) captured in the binding docs.
>
> I'll take the Armada dts changes through mvebu/arm-soc after a few days.
Just to dot our i's and cross our t's, would you mind sending out a
non-rfc version of this series?
thx,
Jason.
^ permalink raw reply
* Re: bit fields && data tearing
From: Paul E. McKenney @ 2014-09-07 16:21 UTC (permalink / raw)
To: James Bottomley
Cc: Jakub Jelinek, One Thousand Gnomes, linux-arch, linux-ia64,
Peter Hurley, Mikael Pettersson, Oleg Nesterov, linux-kernel,
Tony Luck, Paul Mackerras, H. Peter Anvin, linuxppc-dev,
Miroslav Franc, Richard Henderson
In-Reply-To: <1410066442.12512.13.camel@jarvis.lan>
On Sat, Sep 06, 2014 at 10:07:22PM -0700, James Bottomley wrote:
> On Thu, 2014-09-04 at 21:06 -0700, Paul E. McKenney wrote:
> > On Thu, Sep 04, 2014 at 10:47:24PM -0400, Peter Hurley wrote:
> > > Hi James,
> > >
> > > On 09/04/2014 10:11 PM, James Bottomley wrote:
> > > > On Thu, 2014-09-04 at 17:17 -0700, Paul E. McKenney wrote:
> > > >> +And there are anti-guarantees:
> > > >> +
> > > >> + (*) These guarantees do not apply to bitfields, because compilers often
> > > >> + generate code to modify these using non-atomic read-modify-write
> > > >> + sequences. Do not attempt to use bitfields to synchronize parallel
> > > >> + algorithms.
> > > >> +
> > > >> + (*) Even in cases where bitfields are protected by locks, all fields
> > > >> + in a given bitfield must be protected by one lock. If two fields
> > > >> + in a given bitfield are protected by different locks, the compiler's
> > > >> + non-atomic read-modify-write sequences can cause an update to one
> > > >> + field to corrupt the value of an adjacent field.
> > > >> +
> > > >> + (*) These guarantees apply only to properly aligned and sized scalar
> > > >> + variables. "Properly sized" currently means "int" and "long",
> > > >> + because some CPU families do not support loads and stores of
> > > >> + other sizes. ("Some CPU families" is currently believed to
> > > >> + be only Alpha 21064. If this is actually the case, a different
> > > >> + non-guarantee is likely to be formulated.)
> > > >
> > > > This is a bit unclear. Presumably you're talking about definiteness of
> > > > the outcome (as in what's seen after multiple stores to the same
> > > > variable).
> > >
> > > No, the last conditions refers to adjacent byte stores from different
> > > cpu contexts (either interrupt or SMP).
> > >
> > > > The guarantees are only for natural width on Parisc as well,
> > > > so you would get a mess if you did byte stores to adjacent memory
> > > > locations.
> > >
> > > For a simple test like:
> > >
> > > struct x {
> > > long a;
> > > char b;
> > > char c;
> > > char d;
> > > char e;
> > > };
> > >
> > > void store_bc(struct x *p) {
> > > p->b = 1;
> > > p->c = 2;
> > > }
> > >
> > > on parisc, gcc generates separate byte stores
> > >
> > > void store_bc(struct x *p) {
> > > 0: 34 1c 00 02 ldi 1,ret0
> > > 4: 0f 5c 12 08 stb ret0,4(r26)
> > > 8: 34 1c 00 04 ldi 2,ret0
> > > c: e8 40 c0 00 bv r0(rp)
> > > 10: 0f 5c 12 0a stb ret0,5(r26)
> > >
> > > which appears to confirm that on parisc adjacent byte data
> > > is safe from corruption by concurrent cpu updates; that is,
> > >
> > > CPU 0 | CPU 1
> > > |
> > > p->b = 1 | p->c = 2
> > > |
> > >
> > > will result in p->b == 1 && p->c == 2 (assume both values
> > > were 0 before the call to store_bc()).
> >
> > What Peter said. I would ask for suggestions for better wording, but
> > I would much rather be able to say that single-byte reads and writes
> > are atomic and that aligned-short reads and writes are also atomic.
> >
> > Thus far, it looks like we lose only very old Alpha systems, so unless
> > I hear otherwise, I update my patch to outlaw these very old systems.
>
> This isn't universally true according to the architecture manual. The
> PARISC CPU can make byte to long word stores atomic against the memory
> bus but not against the I/O bus for instance. Atomicity is a property
> of the underlying substrate, not of the CPU. Implying that atomicity is
> a CPU property is incorrect.
OK, fair point.
But are there in-use-for-Linux PARISC memory fabrics (for normal memory,
not I/O) that do not support single-byte and double-byte stores?
Thanx, Paul
^ permalink raw reply
* [PATCH 0/2 v5] powerpc/kvm: support to handle sw breakpoint
From: Madhavan Srinivasan @ 2014-09-07 16:31 UTC (permalink / raw)
To: agraf, benh, paulus, mpe; +Cc: Madhavan Srinivasan, linuxppc-dev, kvm-ppc, kvm
This patchset adds ppc64 server side support for software breakpoint
and extends the use of illegal instruction as software
breakpoint across ppc platform.
Patch 1, adds kernel side support for software breakpoint.
Design is that, by using an illegal instruction, we trap to
hypervisor via Emulation Assistance interrupt, where we check
for the illegal instruction and accordingly we return to Host
or Guest. Patch also adds support for software breakpoint
in PR KVM.
Patch 2,extends the use of illegal instruction as software
breakpoint instruction across the ppc platform. Patch extends
booke program interrupt code to support software breakpoint.
Patch 2 is only compile tested. Will really help if
someone can try it out and let me know comments.
Changes v4->v5:
Made changes to code comments and commit messages
Added debugging active checks for illegal instr comparison
Added debug instruction check in emulate code
Extended SW breakpoint to booke
Changes v3->v4:
Made changes to code comments and removed #define of zero opcode
Added a new function to handle the debug instruction emulation in book3s_hv
Rebased the code to latest upstream source.
Changes v2->v3:
Changed the debug instructions. Using the all zero opcode in the instruction word
as illegal instruction as mentioned in Power ISA instead of ABS
Removed reg updated in emulation assist and added a call to
kvmppc_emulate_instruction for reg update.
Changes v1->v2:
Moved the debug instruction #def to kvm_book3s.h. This way PR_KVM can also share it.
Added code to use KVM get one reg infrastructure to get debug opcode.
Updated emulate.c to include emulation of debug instruction incase of PR_KVM.
Made changes to commit message.
Madhavan Srinivasan (2):
powerpc/kvm: support to handle sw breakpoint
powerpc/kvm: common sw breakpoint instr across ppc
arch/powerpc/include/asm/kvm_ppc.h | 6 ++++++
arch/powerpc/kvm/book3s.c | 3 ++-
arch/powerpc/kvm/book3s_hv.c | 41 ++++++++++++++++++++++++++++++++++----
arch/powerpc/kvm/book3s_pr.c | 3 +++
arch/powerpc/kvm/booke.c | 18 +++++++++++++++--
arch/powerpc/kvm/emulate.c | 18 +++++++++++++++++
6 files changed, 82 insertions(+), 7 deletions(-)
--
1.7.11.4
^ permalink raw reply
* [PATCH 1/2 v5] powerpc/kvm: support to handle sw breakpoint
From: Madhavan Srinivasan @ 2014-09-07 16:31 UTC (permalink / raw)
To: agraf, benh, paulus, mpe; +Cc: Madhavan Srinivasan, linuxppc-dev, kvm-ppc, kvm
In-Reply-To: <1410107494-25556-1-git-send-email-maddy@linux.vnet.ibm.com>
This patch adds kernel side support for software breakpoint.
Design is that, by using an illegal instruction, we trap to hypervisor
via Emulation Assistance interrupt, where we check for the illegal instruction
and accordingly we return to Host or Guest. Patch also adds support for
software breakpoint in PR KVM.
Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/kvm_ppc.h | 6 ++++++
arch/powerpc/kvm/book3s.c | 3 ++-
arch/powerpc/kvm/book3s_hv.c | 41 ++++++++++++++++++++++++++++++++++----
arch/powerpc/kvm/book3s_pr.c | 3 +++
arch/powerpc/kvm/emulate.c | 18 +++++++++++++++++
5 files changed, 66 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
index fb86a22..dd83c9a 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -38,6 +38,12 @@
#include <asm/paca.h>
#endif
+/*
+ * KVMPPC_INST_SW_BREAKPOINT is debug Instruction
+ * for supporting software breakpoint.
+ */
+#define KVMPPC_INST_SW_BREAKPOINT 0x00dddd00
+
enum emulation_result {
EMULATE_DONE, /* no further processing */
EMULATE_DO_MMIO, /* kvm_run filled with MMIO request */
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index dd03f6b..00e9c9f 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -778,7 +778,8 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu,
int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
struct kvm_guest_debug *dbg)
{
- return -EINVAL;
+ vcpu->guest_debug = dbg->control;
+ return 0;
}
void kvmppc_decrementer_func(unsigned long data)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 27cced9..3a2414c 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -725,6 +725,30 @@ static int kvmppc_hcall_impl_hv(unsigned long cmd)
return kvmppc_hcall_impl_hv_realmode(cmd);
}
+static int kvmppc_emulate_debug_inst(struct kvm_run *run,
+ struct kvm_vcpu *vcpu)
+{
+ u32 last_inst;
+
+ if(kvmppc_get_last_inst(vcpu, INST_GENERIC, &last_inst) !=
+ EMULATE_DONE) {
+ /*
+ * Fetch failed, so return to guest and
+ * try executing it again.
+ */
+ return RESUME_GUEST;
+ } else {
+ if (last_inst == KVMPPC_INST_SW_BREAKPOINT) {
+ run->exit_reason = KVM_EXIT_DEBUG;
+ run->debug.arch.address = kvmppc_get_pc(vcpu);
+ return RESUME_HOST;
+ } else {
+ kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
+ return RESUME_GUEST;
+ }
+ }
+}
+
static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu,
struct task_struct *tsk)
{
@@ -807,12 +831,18 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu,
break;
/*
* This occurs if the guest executes an illegal instruction.
- * We just generate a program interrupt to the guest, since
- * we don't emulate any guest instructions at this stage.
+ * If the guest debug is disabled, generate a program interrupt
+ * to the guest. If guest debug is enabled, we need to check
+ * whether the instruction is a software breakpoint instruction.
+ * Accordingly return to Guest or Host.
*/
case BOOK3S_INTERRUPT_H_EMUL_ASSIST:
- kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
- r = RESUME_GUEST;
+ if(vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP) {
+ r = kvmppc_emulate_debug_inst(run, vcpu);
+ } else {
+ kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
+ r = RESUME_GUEST;
+ }
break;
/*
* This occurs if the guest (kernel or userspace), does something that
@@ -922,6 +952,9 @@ static int kvmppc_get_one_reg_hv(struct kvm_vcpu *vcpu, u64 id,
long int i;
switch (id) {
+ case KVM_REG_PPC_DEBUG_INST:
+ *val = get_reg_val(id, KVMPPC_INST_SW_BREAKPOINT);
+ break;
case KVM_REG_PPC_HIOR:
*val = get_reg_val(id, 0);
break;
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index faffb27..6d73708 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -1319,6 +1319,9 @@ static int kvmppc_get_one_reg_pr(struct kvm_vcpu *vcpu, u64 id,
int r = 0;
switch (id) {
+ case KVM_REG_PPC_DEBUG_INST:
+ *val = get_reg_val(id, KVMPPC_INST_SW_BREAKPOINT);
+ break;
case KVM_REG_PPC_HIOR:
*val = get_reg_val(id, to_book3s(vcpu)->hior);
break;
diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c
index e96b50d..30f326b 100644
--- a/arch/powerpc/kvm/emulate.c
+++ b/arch/powerpc/kvm/emulate.c
@@ -274,6 +274,24 @@ int kvmppc_emulate_instruction(struct kvm_run *run, struct kvm_vcpu *vcpu)
}
break;
+ case 0:
+ /*
+ * Instruction with primary opcode 0. Based on PowerISA
+ * these are illegal instructions.
+ */
+ if(inst == KVMPPC_INST_SW_BREAKPOINT) {
+ run->exit_reason = KVM_EXIT_DEBUG;
+ run->debug.arch.address = kvmppc_get_pc(vcpu);
+ emulated = EMULATE_EXIT_USER;
+ advance = 0;
+ printk(KERN_INFO "pr_emulate: debug instr \n");
+ } else {
+ kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
+ emulated = EMULATE_DONE;
+ advance = 0;
+ }
+ break;
+
default:
emulated = EMULATE_FAIL;
}
--
1.7.11.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox