* Re: DT case sensitivity
From: Grant Likely @ 2018-08-23 12:48 UTC (permalink / raw)
To: Rob Herring, Benjamin Herrenschmidt
Cc: Stephen Rothwell, Michael Ellerman, Kumar Gala, David Gibson,
Frank Rowand, devicetree-spec, devicetree, linuxppc-dev
In-Reply-To: <CAL_JsqKnbFAydf7ets0=KySm3P3b_7np-EVWz3Gh6qO+imJ+eg@mail.gmail.com>
On 23/08/2018 13:08, Rob Herring wrote:
> On Thu, Aug 23, 2018 at 6:48 AM Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
>>
>> On Thu, 2018-08-23 at 06:43 -0500, Rob Herring wrote:
>>> On Thu, Aug 23, 2018 at 4:02 AM Grant Likely <grant.likely@arm.com> wro=
te:
>>>>
>>>>
>>>> What problem are you trying to solve?
>>>
>>> I'm looking at removing device_node.name and using full_name instead
>>> (which now is only the local node name plus unit-address). This means
>>> replacing of_node_cmp() (and still some strcmp) calls in a lot of
>>> places. I need to use either strncmp or strncasecmp instead.
>>>
>>>> I would think making everything
>>>> case insensitive would be the direction to go if you do anything. Leas=
t
>>>> possibility of breaking existing platforms in that scenario.
>>>
>>> Really? Even if all the "new" arches are effectively case sensitive?
>>> Anything using dtc and libfdt are (and json-schema certainly will be).
>>> But I frequently say the kernel's job is not DT validation, so you
>>> pass crap in, you get undefined results.
>>
>> I tend to agree with Grant. Let's put it this way:
>>
>> What is the drawback of being case insensitive ?
>
> It's a more expensive comparison. I don't think it's a hot path, but
> we do do a lot of string compares.
I'd hazard to argue that the cost of the string compare will not be a
source of performance problems when compared to doing a linear search
everytime a DT lookup is performed. The search algorithm is far more
problematic.
> Property names are case sensitive already. It would be nice if
> everything was treated the same way.
>
> If we're case sensitive then it is well defined which node we'll match
> and which one will be ignored. Otherwise, it is whichever one we
> happen to match first.
If case-insensitive-always is chosen, then the kernel should probably
complain at add node time if another matching node name already exists.
That's a relatively easy thing to test.
You are right however that in the FDT world we're case-sensitive now and
if it is relaxed then we're never going back. You could try switching
everyone over to case-sensitive and see if anything falls out in
linux-next for a few cycles. I would not touch the PPC or SPARC
behaviour. I don't think it is worth the risk to make the kernel more
strict when we cannot test the result. Only if everything was changed to
case-insensitive would it make sense to touch PPC and SPARC at the same
time because it reduces the number of platform variations.
If you do change compatible matches to be case sensitive, then you
should be prepared to fix bugs where the driver uses a different case
from the DTS. In which case drivers will need to be modified to accept
the deviant property names.
g.
IMPORTANT NOTICE: The contents of this email and any attachments are confid=
ential and may also be privileged. If you are not the intended recipient, p=
lease notify the sender immediately and do not disclose the contents to any=
other person, use it for any purpose, or store or copy the information in =
any medium. Thank you.
^ permalink raw reply
* Re: DT case sensitivity
From: Segher Boessenkool @ 2018-08-23 12:36 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Rob Herring, Stephen Rothwell, Kumar Gala, devicetree,
devicetree-spec, linuxppc-dev, Grant Likely, Frank Rowand,
David Gibson
In-Reply-To: <18175413a0148f7859ab36ed9715dce7a598f318.camel@kernel.crashing.org>
On Thu, Aug 23, 2018 at 11:29:01AM +1000, Benjamin Herrenschmidt wrote:
> On Wed, 2018-08-22 at 20:26 -0500, Rob Herring wrote:
> > On Wed, Aug 22, 2018 at 8:14 PM Benjamin Herrenschmidt
> > <benh@kernel.crashing.org> wrote:
> > >
> > > On Wed, 2018-08-22 at 19:47 -0500, Rob Herring wrote:
> > > > The default DT string handling in the kernel is node names and
> > > > compatibles are case insensitive and property names are case sensitive
> > > > (Sparc is the the only variation and is opposite). It seems only PPC
> > > > (and perhaps only Power Macs?) needs to support case insensitive
> > > > comparisons. It was probably a mistake to follow PPC for new arches
> > > > and we should have made everything case sensitive from the start. So I
> > > > have a few questions for the DT historians. :)
> > >
> > > Open Firmware itself is insensitive.
> >
> > Doesn't it depend on the implementation? Otherwise, how is Sparc different?
>
> Not sure ...
The standard requires case-sensitive.
> Forth itself is insensitive for words
Not even. http://forth.sourceforge.net/std/dpans/dpans3.htm#3.3.1.2
(Most non-ancient implementations are though).
> but maybe not for string comparisons.
Only COMPARE is standardised, and that is case-sensitive comparison. Many
systems have other words to do case-insensitive comparisons, or words where
some runtime flag determines the case-sensitivity.
Btw. A node name in Open Firmware is generically
driver-name@unit-address:device-arguments
where driver-name is the part that is in the "name" property; this whole
case-sensitivity business is even worse for FDT, where you also treat the
unit address as part of the name. In real Open Firmware the address is
compared *as a number* (or as a few numbers), so it is naturally case-
insensitive (it does not care if you write 01a0 or 01A0, or 1a0 or 000001a0
etc.)
Segher
^ permalink raw reply
* Re: [PATCH] powerpc/xive: Initialize symbol before usage
From: Cédric Le Goater @ 2018-08-23 5:26 UTC (permalink / raw)
To: Michael Ellerman, Breno Leitao, linuxppc-dev
In-Reply-To: <87lg8xlq9a.fsf@concordia.ellerman.id.au>
On 08/23/2018 05:24 AM, Michael Ellerman wrote:
> Hi Breno,
>
> Breno Leitao <leitao@debian.org> writes:
>> Function xive_native_get_ipi() might uses chip_id without it being
>> initialized. This gives the following error on 'smatch' tool:
>>
>> error: uninitialized symbol 'chip_id'
>
> Which is correct, it can be used uninitialised. I'm surprised GCC
> doesn't warn about it.
>
>> This patch simply sets chip_id initial value to 0.
>
> I'd prefer we fixed it differently, by explicitly initialising to zero
> at the appropriate place in the code.
>
>> diff --git a/arch/powerpc/sysdev/xive/native.c b/arch/powerpc/sysdev/xive/native.c
>> index 311185b9960a..fc56673a3c0f 100644
>> --- a/arch/powerpc/sysdev/xive/native.c
>> +++ b/arch/powerpc/sysdev/xive/native.c
>> @@ -239,7 +239,7 @@ static bool xive_native_match(struct device_node *node)
>> static int xive_native_get_ipi(unsigned int cpu, struct xive_cpu *xc)
>> {
>> struct device_node *np;
>> - unsigned int chip_id;
>> + unsigned int chip_id = 0;
>> s64 irq;
>>
>> /* Find the chip ID */
>
> The current code is:
>
> /* Find the chip ID */
> np = of_get_cpu_node(cpu, NULL);
> if (np) {
> if (of_property_read_u32(np, "ibm,chip-id", &chip_id) < 0)
> chip_id = 0;
> }
>
> Where if np is NULL then we don't initialise chip_id.
>
> Which could be:
>
> np = of_get_cpu_node(cpu, NULL);
> if (of_property_read_u32(np, "ibm,chip-id", &chip_id) < 0)
> chip_id = 0;
>
> Because of_property_read_u32() will just return an error if np is NULL.
>
> It's also missing an of_node_put() of np, you should do a separate patch
> to fix that. You can just do it unconditionally after the
> of_property_read_u32().
I think we can simply get rid of the OF code under xive_native_get_ipi()
and use xc->chip_id instead. It should be safe to use as xive_prepare_cpu()
should have initialized ->chip_id by the time xive_native_get_ipi() is
called.
Cheers,
C.
^ permalink raw reply
* Re: [PATCH] powerpc/xive: Initialize symbol before usage
From: Michael Ellerman @ 2018-08-23 12:25 UTC (permalink / raw)
To: Cédric Le Goater, Breno Leitao, linuxppc-dev
In-Reply-To: <3b8841ac-ebbb-523e-0d65-ebe27a702d3c@kaod.org>
C=C3=A9dric Le Goater <clg@kaod.org> writes:
> On 08/23/2018 05:24 AM, Michael Ellerman wrote:
>> Breno Leitao <leitao@debian.org> writes:
>>> diff --git a/arch/powerpc/sysdev/xive/native.c b/arch/powerpc/sysdev/xi=
ve/native.c
>>> index 311185b9960a..fc56673a3c0f 100644
>>> --- a/arch/powerpc/sysdev/xive/native.c
>>> +++ b/arch/powerpc/sysdev/xive/native.c
>>> @@ -239,7 +239,7 @@ static bool xive_native_match(struct device_node *n=
ode)
>>> static int xive_native_get_ipi(unsigned int cpu, struct xive_cpu *xc)
>>> {
>>> struct device_node *np;
>>> - unsigned int chip_id;
>>> + unsigned int chip_id =3D 0;
>>> s64 irq;
>>>=20=20
>>> /* Find the chip ID */
>>=20
>> The current code is:
>>=20
>> /* Find the chip ID */
>> np =3D of_get_cpu_node(cpu, NULL);
>> if (np) {
>> if (of_property_read_u32(np, "ibm,chip-id", &chip_id) < 0)
>> chip_id =3D 0;
>> }
>>=20
>> Where if np is NULL then we don't initialise chip_id.
>>=20
>> Which could be:
>>=20
>> np =3D of_get_cpu_node(cpu, NULL);
>> if (of_property_read_u32(np, "ibm,chip-id", &chip_id) < 0)
>> chip_id =3D 0;
>>=20
>> Because of_property_read_u32() will just return an error if np is NULL.
>>=20
>> It's also missing an of_node_put() of np, you should do a separate patch
>> to fix that. You can just do it unconditionally after the
>> of_property_read_u32().
>
> I think we can simply get rid of the OF code under xive_native_get_ipi()
> and use xc->chip_id instead. It should be safe to use as xive_prepare_cpu=
()=20
> should have initialized ->chip_id by the time xive_native_get_ipi() is=20
> called.=20
Even better!
cheers
^ permalink raw reply
* Re: DT case sensitivity
From: Segher Boessenkool @ 2018-08-23 12:19 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Rob Herring, Stephen Rothwell, Grant Likely, Michael Ellerman,
Kumar Gala, David Gibson, Frank Rowand, devicetree-spec,
devicetree, linuxppc-dev
In-Reply-To: <3335cff2cc999c1dda58d75949ab3d12185fca79.camel@kernel.crashing.org>
On Thu, Aug 23, 2018 at 11:03:28AM +1000, Benjamin Herrenschmidt wrote:
> On Wed, 2018-08-22 at 19:47 -0500, Rob Herring wrote:
> > The default DT string handling in the kernel is node names and
> > compatibles are case insensitive and property names are case sensitive
> > (Sparc is the the only variation and is opposite). It seems only PPC
> > (and perhaps only Power Macs?) needs to support case insensitive
> > comparisons. It was probably a mistake to follow PPC for new arches
> > and we should have made everything case sensitive from the start. So I
> > have a few questions for the DT historians. :)
>
> Open Firmware itself is insensitive.
Some implementations are, yes. But those are technically broken.
> > What PPC systems are case insensitive? Can we limit that to certain systems?
>
> All PowerMacs at least, the problem is that I don't have DT images or
> access to all the historical systems (and yes some people occasionally
> still use them) to properly test a change in that area.
If one implementation does case insensitive, it will most likely just work,
because people do not make insane names differing only in case on purpose.
Now people write other things that they only test against that implementation,
and those things now only work with case-insensitive. And you do not know
without testing if anything breaks. (But the laws of big numbers are against
you here). Since there isn't really a drawback to doing case-insensitive
always, that is a much safer way forward, much less work for everyone, even
if technically the wrong thing to do :-)
Segher
^ permalink raw reply
* Re: DT case sensitivity
From: Rob Herring @ 2018-08-23 12:08 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Grant Likely, Stephen Rothwell, Michael Ellerman, Kumar Gala,
David Gibson, Frank Rowand, devicetree-spec, devicetree,
linuxppc-dev
In-Reply-To: <e9009f6dd3aded2b9aa735973f4eed8386dd9afb.camel@kernel.crashing.org>
On Thu, Aug 23, 2018 at 6:48 AM Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
>
> On Thu, 2018-08-23 at 06:43 -0500, Rob Herring wrote:
> > On Thu, Aug 23, 2018 at 4:02 AM Grant Likely <grant.likely@arm.com> wrote:
> > >
> > >
> > > What problem are you trying to solve?
> >
> > I'm looking at removing device_node.name and using full_name instead
> > (which now is only the local node name plus unit-address). This means
> > replacing of_node_cmp() (and still some strcmp) calls in a lot of
> > places. I need to use either strncmp or strncasecmp instead.
> >
> > > I would think making everything
> > > case insensitive would be the direction to go if you do anything. Least
> > > possibility of breaking existing platforms in that scenario.
> >
> > Really? Even if all the "new" arches are effectively case sensitive?
> > Anything using dtc and libfdt are (and json-schema certainly will be).
> > But I frequently say the kernel's job is not DT validation, so you
> > pass crap in, you get undefined results.
>
> I tend to agree with Grant. Let's put it this way:
>
> What is the drawback of being case insensitive ?
It's a more expensive comparison. I don't think it's a hot path, but
we do do a lot of string compares.
Property names are case sensitive already. It would be nice if
everything was treated the same way.
If we're case sensitive then it is well defined which node we'll match
and which one will be ignored. Otherwise, it is whichever one we
happen to match first.
> Do we expect that there exist a case where we will want to distinguish
> between nodes that have the same name with a different case ?
If someone has a DT with a node in the wrong case (as defined in the
DT spec or a binding doc) and the kernel accepts it, then it's an ABI.
Yes, as Grant says, validation is the place that should catch it, but
we're not there yet and it's cheap (cheaper in fact) for the kernel to
do.
Rob
^ permalink raw reply
* Re: [RESEND PATCH v2] powerpc/mce: Fix SLB rebolting during MCE recovery path.
From: Michael Ellerman @ 2018-08-23 12:05 UTC (permalink / raw)
To: Mahesh Jagannath Salgaonkar, linuxppc-dev
Cc: Nicholas Piggin, Aneesh Kumar K.V
In-Reply-To: <10dfea17-e98b-7a46-6dba-b222a6cf0c96@linux.vnet.ibm.com>
Mahesh Jagannath Salgaonkar <mahesh@linux.vnet.ibm.com> writes:
> On 08/23/2018 12:14 PM, Michael Ellerman wrote:
>> Mahesh J Salgaonkar <mahesh@linux.vnet.ibm.com> writes:
>>
>>> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>>>
>>> With the powerpc next commit e7e81847478 (powerpc/mce: Fix SLB rebolting
>>> during MCE recovery path.),
>>
>> That commit description is wrong, I'll fix it up.
>
> Ouch.. My bad.. :-(
To make it easier to get right, if you don't already, add these to your
~/.gitconfig:
[pretty]
fixes = Fixes: %h (\"%s\")
quote = %h (\"%s\")
And then you can do:
$ git log -1 --pretty=quote e7e81847478
e7e81847478b ("powerpc/64s: move machine check SLB flushing to mm/slb.c")
$ git log -1 --pretty=fixes e7e81847478
Fixes: e7e81847478b ("powerpc/64s: move machine check SLB flushing to mm/slb.c")
I then have a shell alias that pipes that into xclip, so I can just
paste it directly into the change log.
cheers
^ permalink raw reply
* Re: [RESEND PATCH v2] powerpc/mce: Fix SLB rebolting during MCE recovery path.
From: Michael Ellerman @ 2018-08-23 11:59 UTC (permalink / raw)
To: Nicholas Piggin, Mahesh J Salgaonkar; +Cc: linuxppc-dev, Aneesh Kumar K.V
In-Reply-To: <20180823170202.775e8bb0@roar.ozlabs.ibm.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> On Thu, 23 Aug 2018 12:06:53 +0530
> Mahesh J Salgaonkar <mahesh@linux.vnet.ibm.com> wrote:
>> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>>
>> With the powerpc next commit e7e81847478 (powerpc/mce: Fix SLB rebolting
>> during MCE recovery path.), the SLB error recovery is broken. The new
>> change now does not add index value to RB[52-63] that selects the SLB
>> entry while rebolting, instead it assumes that the shadow save area
>> already have index embeded correctly in esid field. While all valid bolted
>> save areas do contain index value set correctly, there is a case where
>> 3rd (KSTACK_INDEX) entry for kernel stack does not embed index for NULL
>> esid entry. This patch fixes that.
>>
>> Without this patch the SLB rebolt code overwrites the 1st entry of kernel
>> linear mapping and causes SLB recovery to fail.
>>
>> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
>
> Changelog just needs a little more work, maybe this?
>
>
> The commit e7e81847478 ("powerpc/64s: move machine check SLB flushing
> to mm/slb.c") introduced a bug in reloading bolted SLB entries. Unused
> bolted entries are stored with .esid=0 in the slb_shadow area, and
> that value is now used directly as the RB input to slbmte, which means
> the RB[52:63] index field is set to 0, which causes SLB entry 0 to be
> cleared.
>
> Fix this by storing the index bits in the unused bolted entries, which
> directs the slbmte to the right place.
>
> The SLB shadow area is also used by the hypervisor, but PAPR is okay
> with that, from LoPAPR v1.1, 14.11.1.3 SLB Shadow Buffer:
>
> Note: SLB is filled sequentially starting at index 0
> from the shadow buffer ignoring the contents of
> RB field bits 52-63
>
> Fixes: e7e81847478 ("powerpc/64s: move machine check SLB flushing to
> mm/slb.c")
Yep I'll take that, thanks.
cheers
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/mm/hash: Only need the Nest MMU workaround for R -> RW transition
From: Michael Ellerman @ 2018-08-23 11:57 UTC (permalink / raw)
To: Nicholas Piggin, Aneesh Kumar K.V; +Cc: benh, paulus, linuxppc-dev
In-Reply-To: <20180823192319.5f06041f@roar.ozlabs.ibm.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> On Wed, 22 Aug 2018 22:46:05 +0530
> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> wrote:
>
>> The Nest MMU workaround is only needed for RW upgrades. Avoid doing that
>> for other pte updates.
>>
>> We also avoid clearing the pte while marking it invalid. This is because other
>> page table walk will find this pte none and can result in unexpected behaviour
>> due to that. Instead we clear _PAGE_PRESENT and set the software pte bit
>> _PAGE_INVALID. pte_present is already updated to check for bot the bits. This
>> make sure page table walkers will find the pte present and things like
>> pte_pfn(pte) returns the right value.
>>
>> Based on the original patch from Benjamin Herrenschmidt <benh@kernel.crashing.org>
>>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>> ---
>> arch/powerpc/mm/pgtable-radix.c | 8 +++++---
>
> This is powerpc/mm/radix, isn't it? Subject says hash.
I fixed it when applying.
> Could we make this fix POWER9 only and use a RSV bit for it
> rather than use up a SW bit? Other than that,
>
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Thanks.
cheers
^ permalink raw reply
* Re: DT case sensitivity
From: Grant Likely @ 2018-08-23 11:56 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Rob Herring
Cc: Stephen Rothwell, Michael Ellerman, Kumar Gala, David Gibson,
Frank Rowand, devicetree-spec, devicetree, linuxppc-dev
In-Reply-To: <e9009f6dd3aded2b9aa735973f4eed8386dd9afb.camel@kernel.crashing.org>
On 23/08/2018 12:47, Benjamin Herrenschmidt wrote:
> On Thu, 2018-08-23 at 06:43 -0500, Rob Herring wrote:
>> On Thu, Aug 23, 2018 at 4:02 AM Grant Likely <grant.likely@arm.com> wrot=
e:
>>>
>>>
>>> What problem are you trying to solve?
>>
>> I'm looking at removing device_node.name and using full_name instead
>> (which now is only the local node name plus unit-address). This means
>> replacing of_node_cmp() (and still some strcmp) calls in a lot of
>> places. I need to use either strncmp or strncasecmp instead.
Makes sense. Simplifies the code.
>>
>>> I would think making everything
>>> case insensitive would be the direction to go if you do anything. Least
>>> possibility of breaking existing platforms in that scenario.
>>
>> Really? Even if all the "new" arches are effectively case sensitive?
>> Anything using dtc and libfdt are (and json-schema certainly will be).
>> But I frequently say the kernel's job is not DT validation, so you
>> pass crap in, you get undefined results.
>
> I tend to agree with Grant. Let's put it this way:
>
> What is the drawback of being case insensitive ?
>
> Do we expect that there exist a case where we will want to distinguish
> between nodes that have the same name with a different case ? >
> If not, I don't see the point of being strict about it.
I'd also add that any place where there are two nodes or props with the
same name, but different case, then it is probably a bug (or a really
bad design). Going the direction of case-insensitive eliminates the
possibility.
I do understand that that you'd like the kernel to be strict about what
it accepts, but that strictness probably makes more sense back in DTC
where it can also be checked against schema.
g.
IMPORTANT NOTICE: The contents of this email and any attachments are confid=
ential and may also be privileged. If you are not the intended recipient, p=
lease notify the sender immediately and do not disclose the contents to any=
other person, use it for any purpose, or store or copy the information in =
any medium. Thank you.
^ permalink raw reply
* Re: [PATCH v2 3/4] powerpc/mm: fix a warning when a cache is common to PGD and hugepages
From: Michael Ellerman @ 2018-08-23 11:56 UTC (permalink / raw)
To: ruscur
Cc: Aneesh Kumar K.V, Benjamin Herrenschmidt, Paul Mackerras,
aneesh.kumar, linuxppc-dev, linux-kernel, Christophe LEROY,
Segher Boessenkool
In-Reply-To: <8ada20e9-3bc1-fdaf-d78f-2f5a317e8216@c-s.fr>
Christophe LEROY <christophe.leroy@c-s.fr> writes:
> Le 23/08/2018 =C3=A0 12:36, Segher Boessenkool a =C3=A9crit=C2=A0:
>> On Thu, Aug 23, 2018 at 11:40:22AM +0200, Christophe LEROY wrote:
>>> The only small probl=C3=A8me I have is that some version of GCC seems to
>>> complain about big memset() (132k and 256k ones). Is there a way to tell
>>> GCC we really want to do it ?
>>=20
>> I'm not sure what you mean. Complain, is that a warning, is that an err=
or?
>> What does it say? Do you have some example code to reproduce it? Etc.
>
> I saw the warnings in the checks at=20
> https://patchwork.ozlabs.org/patch/957566/
> Unfortunatly the link is now broken.
ruscur/ajd any idea what happened to the snowpatch links here?
cheers
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/mm/hash: Only need the Nest MMU workaround for R -> RW transition
From: Benjamin Herrenschmidt @ 2018-08-23 11:51 UTC (permalink / raw)
To: Nicholas Piggin, Aneesh Kumar K.V; +Cc: paulus, mpe, linuxppc-dev
In-Reply-To: <20180823192319.5f06041f@roar.ozlabs.ibm.com>
On Thu, 2018-08-23 at 19:23 +1000, Nicholas Piggin wrote:
> On Wed, 22 Aug 2018 22:46:05 +0530
> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> wrote:
>
> > The Nest MMU workaround is only needed for RW upgrades. Avoid doing that
> > for other pte updates.
> >
> > We also avoid clearing the pte while marking it invalid. This is because other
> > page table walk will find this pte none and can result in unexpected behaviour
> > due to that. Instead we clear _PAGE_PRESENT and set the software pte bit
> > _PAGE_INVALID. pte_present is already updated to check for bot the bits. This
> > make sure page table walkers will find the pte present and things like
> > pte_pfn(pte) returns the right value.
> >
> > Based on the original patch from Benjamin Herrenschmidt <benh@kernel.crashing.org>
> >
> > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> > ---
> > arch/powerpc/mm/pgtable-radix.c | 8 +++++---
>
> This is powerpc/mm/radix, isn't it? Subject says hash.
>
> Could we make this fix POWER9 only and use a RSV bit for it
> rather than use up a SW bit? Other than that,
Well, the SW bit is necessary for THP as well isn't it ?
>
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
>
> > 1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/powerpc/mm/pgtable-radix.c b/arch/powerpc/mm/pgtable-radix.c
> > index 7be99fd9af15..c879979faa73 100644
> > --- a/arch/powerpc/mm/pgtable-radix.c
> > +++ b/arch/powerpc/mm/pgtable-radix.c
> > @@ -1045,20 +1045,22 @@ void radix__ptep_set_access_flags(struct vm_area_struct *vma, pte_t *ptep,
> > struct mm_struct *mm = vma->vm_mm;
> > unsigned long set = pte_val(entry) & (_PAGE_DIRTY | _PAGE_ACCESSED |
> > _PAGE_RW | _PAGE_EXEC);
> > +
> > + unsigned long change = pte_val(entry) ^ pte_val(*ptep);
> > /*
> > * To avoid NMMU hang while relaxing access, we need mark
> > * the pte invalid in between.
> > */
> > - if (atomic_read(&mm->context.copros) > 0) {
> > + if ((change & _PAGE_RW) && atomic_read(&mm->context.copros) > 0) {
> > unsigned long old_pte, new_pte;
> >
> > - old_pte = __radix_pte_update(ptep, ~0, 0);
> > + old_pte = __radix_pte_update(ptep, _PAGE_PRESENT, _PAGE_INVALID);
> > /*
> > * new value of pte
> > */
> > new_pte = old_pte | set;
> > radix__flush_tlb_page_psize(mm, address, psize);
> > - __radix_pte_update(ptep, 0, new_pte);
> > + __radix_pte_update(ptep, _PAGE_INVALID, new_pte);
> > } else {
> > __radix_pte_update(ptep, 0, set);
> > /*
^ permalink raw reply
* Re: DT case sensitivity
From: Benjamin Herrenschmidt @ 2018-08-23 11:47 UTC (permalink / raw)
To: Rob Herring, Grant Likely
Cc: Stephen Rothwell, Michael Ellerman, Kumar Gala, David Gibson,
Frank Rowand, devicetree-spec, devicetree, linuxppc-dev
In-Reply-To: <CAL_Jsq+1T8k5mtc-DgDBtX3Qco9sQG9RWv6FTZ8NXppNbNg3Ww@mail.gmail.com>
On Thu, 2018-08-23 at 06:43 -0500, Rob Herring wrote:
> On Thu, Aug 23, 2018 at 4:02 AM Grant Likely <grant.likely@arm.com> wrote:
> >
> >
> > What problem are you trying to solve?
>
> I'm looking at removing device_node.name and using full_name instead
> (which now is only the local node name plus unit-address). This means
> replacing of_node_cmp() (and still some strcmp) calls in a lot of
> places. I need to use either strncmp or strncasecmp instead.
>
> > I would think making everything
> > case insensitive would be the direction to go if you do anything. Least
> > possibility of breaking existing platforms in that scenario.
>
> Really? Even if all the "new" arches are effectively case sensitive?
> Anything using dtc and libfdt are (and json-schema certainly will be).
> But I frequently say the kernel's job is not DT validation, so you
> pass crap in, you get undefined results.
I tend to agree with Grant. Let's put it this way:
What is the drawback of being case insensitive ?
Do we expect that there exist a case where we will want to distinguish
between nodes that have the same name with a different case ?
If not, I don't see the point of being strict about it.
Cheers,
Ben.
^ permalink raw reply
* Re: DT case sensitivity
From: Rob Herring @ 2018-08-23 11:43 UTC (permalink / raw)
To: Grant Likely
Cc: Benjamin Herrenschmidt, Stephen Rothwell, Michael Ellerman,
Kumar Gala, David Gibson, Frank Rowand, devicetree-spec,
devicetree, linuxppc-dev
In-Reply-To: <13bf5b39-0461-351e-57e3-60264eaa374d@arm.com>
On Thu, Aug 23, 2018 at 4:02 AM Grant Likely <grant.likely@arm.com> wrote:
>
> On 23/08/2018 02:29, Benjamin Herrenschmidt wrote:
> > On Wed, 2018-08-22 at 20:26 -0500, Rob Herring wrote:
> >> On Wed, Aug 22, 2018 at 8:14 PM Benjamin Herrenschmidt
> >> <benh@kernel.crashing.org> wrote:
> >>>
> >>> On Wed, 2018-08-22 at 19:47 -0500, Rob Herring wrote:
> >>>> The default DT string handling in the kernel is node names and
> >>>> compatibles are case insensitive and property names are case sensitive
> >>>> (Sparc is the the only variation and is opposite). It seems only PPC
> >>>> (and perhaps only Power Macs?) needs to support case insensitive
> >>>> comparisons. It was probably a mistake to follow PPC for new arches
> >>>> and we should have made everything case sensitive from the start. So I
> >>>> have a few questions for the DT historians. :)
> >>>
> >>> Open Firmware itself is insensitive.
> >>
> >> Doesn't it depend on the implementation? Otherwise, how is Sparc different?
> >
> > Not sure ... Forth itself is insensitive for words but maybe not for
> > string comparisons.
>
> What problem are you trying to solve?
I'm looking at removing device_node.name and using full_name instead
(which now is only the local node name plus unit-address). This means
replacing of_node_cmp() (and still some strcmp) calls in a lot of
places. I need to use either strncmp or strncasecmp instead.
> I would think making everything
> case insensitive would be the direction to go if you do anything. Least
> possibility of breaking existing platforms in that scenario.
Really? Even if all the "new" arches are effectively case sensitive?
Anything using dtc and libfdt are (and json-schema certainly will be).
But I frequently say the kernel's job is not DT validation, so you
pass crap in, you get undefined results.
Rob
^ permalink raw reply
* Re: [PATCH v2 3/4] powerpc/mm: fix a warning when a cache is common to PGD and hugepages
From: Christophe LEROY @ 2018-08-23 10:39 UTC (permalink / raw)
To: Segher Boessenkool
Cc: Aneesh Kumar K.V, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, aneesh.kumar, linuxppc-dev, linux-kernel
In-Reply-To: <20180823103625.GR24439@gate.crashing.org>
Le 23/08/2018 à 12:36, Segher Boessenkool a écrit :
> On Thu, Aug 23, 2018 at 11:40:22AM +0200, Christophe LEROY wrote:
>> The only small problème I have is that some version of GCC seems to
>> complain about big memset() (132k and 256k ones). Is there a way to tell
>> GCC we really want to do it ?
>
> I'm not sure what you mean. Complain, is that a warning, is that an error?
> What does it say? Do you have some example code to reproduce it? Etc.
I saw the warnings in the checks at
https://patchwork.ozlabs.org/patch/957566/
Unfortunatly the link is now broken.
Christophe
>
> Very many things use tiny memsets like that, so you must mean something
> more specialised.
>
>
> Segher
>
^ permalink raw reply
* Re: [PATCH v2 3/4] powerpc/mm: fix a warning when a cache is common to PGD and hugepages
From: Segher Boessenkool @ 2018-08-23 10:36 UTC (permalink / raw)
To: Christophe LEROY
Cc: Aneesh Kumar K.V, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, aneesh.kumar, linuxppc-dev, linux-kernel
In-Reply-To: <cb121f38-ac89-4971-fd84-5473e7e9ced5@c-s.fr>
On Thu, Aug 23, 2018 at 11:40:22AM +0200, Christophe LEROY wrote:
> The only small problème I have is that some version of GCC seems to
> complain about big memset() (132k and 256k ones). Is there a way to tell
> GCC we really want to do it ?
I'm not sure what you mean. Complain, is that a warning, is that an error?
What does it say? Do you have some example code to reproduce it? Etc.
Very many things use tiny memsets like that, so you must mean something
more specialised.
Segher
^ permalink raw reply
* Re: [PATCH v2 3/4] powerpc/mm: fix a warning when a cache is common to PGD and hugepages
From: Christophe LEROY @ 2018-08-23 9:40 UTC (permalink / raw)
To: Aneesh Kumar K.V, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, aneesh.kumar
Cc: linux-kernel, linuxppc-dev
In-Reply-To: <3e6412ac-c645-908f-a3bb-c9a2a72f4b68@linux.ibm.com>
Le 22/08/2018 à 16:20, Aneesh Kumar K.V a écrit :
> On 08/17/2018 04:14 PM, Christophe LEROY wrote:
>>
>>
>> Le 17/08/2018 à 05:32, Aneesh Kumar K.V a écrit :
>>> On 08/14/2018 08:24 PM, Christophe Leroy wrote:
>>>> While implementing TLB miss HW assistance on the 8xx, the following
>>>> warning was encountered:
>>>>
>>>> [ 423.732965] WARNING: CPU: 0 PID: 345 at mm/slub.c:2412
>>>> ___slab_alloc.constprop.30+0x26c/0x46c
>>>> [ 423.733033] CPU: 0 PID: 345 Comm: mmap Not tainted
>>>> 4.18.0-rc8-00664-g2dfff9121c55 #671
>>>> [ 423.733075] NIP: c0108f90 LR: c0109ad0 CTR: 00000004
>>>> [ 423.733121] REGS: c455bba0 TRAP: 0700 Not tainted
>>>> (4.18.0-rc8-00664-g2dfff9121c55)
>>>> [ 423.733147] MSR: 00021032 <ME,IR,DR,RI> CR: 24224848 XER:
>>>> 20000000
>>>> [ 423.733319]
>>>> [ 423.733319] GPR00: c0109ad0 c455bc50 c4521910 c60053c0 007080c0
>>>> c0011b34 c7fa41e0 c455be30
>>>> [ 423.733319] GPR08: 00000001 c00103a0 c7fa41e0 c49afcc4 24282842
>>>> 10018840 c079b37c 00000040
>>>> [ 423.733319] GPR16: 73f00000 00210d00 00000000 00000001 c455a000
>>>> 00000100 00000200 c455a000
>>>> [ 423.733319] GPR24: c60053c0 c0011b34 007080c0 c455a000 c455a000
>>>> c7fa41e0 00000000 00009032
>>>> [ 423.734190] NIP [c0108f90] ___slab_alloc.constprop.30+0x26c/0x46c
>>>> [ 423.734257] LR [c0109ad0] kmem_cache_alloc+0x210/0x23c
>>>> [ 423.734283] Call Trace:
>>>> [ 423.734326] [c455bc50] [00000100] 0x100 (unreliable)
>>>> [ 423.734430] [c455bcc0] [c0109ad0] kmem_cache_alloc+0x210/0x23c
>>>> [ 423.734543] [c455bcf0] [c0011b34] huge_pte_alloc+0xc0/0x1dc
>>>> [ 423.734633] [c455bd20] [c01044dc] hugetlb_fault+0x408/0x48c
>>>> [ 423.734720] [c455bdb0] [c0104b20] follow_hugetlb_page+0x14c/0x44c
>>>> [ 423.734826] [c455be10] [c00e8e54] __get_user_pages+0x1c4/0x3dc
>>>> [ 423.734919] [c455be80] [c00e9924] __mm_populate+0xac/0x140
>>>> [ 423.735020] [c455bec0] [c00db14c] vm_mmap_pgoff+0xb4/0xb8
>>>> [ 423.735127] [c455bf00] [c00f27c0] ksys_mmap_pgoff+0xcc/0x1fc
>>>> [ 423.735222] [c455bf40] [c000e0f8] ret_from_syscall+0x0/0x38
>>>> [ 423.735271] Instruction dump:
>>>> [ 423.735321] 7cbf482e 38fd0008 7fa6eb78 7fc4f378 4bfff5dd 7fe3fb78
>>>> 4bfffe24 81370010
>>>> [ 423.735536] 71280004 41a2ff88 4840c571 4bffff80 <0fe00000>
>>>> 4bfffeb8 81340010 712a0004
>>>> [ 423.735757] ---[ end trace e9b222919a470790 ]---
>>>>
>>>> This warning occurs when calling kmem_cache_zalloc() on a
>>>> cache having a constructor.
>>>>
>>>> In this case it happens because PGD cache and 512k hugepte cache are
>>>> the same size (4k). While a cache with constructor is created for
>>>> the PGD, hugepages create cache without constructor and uses
>>>> kmem_cache_zalloc(). As both expect a cache with the same size,
>>>> the hugepages reuse the cache created for PGD, hence the conflict.
>>>>
>>>> In order to avoid this conflict, this patch:
>>>> - modifies pgtable_cache_add() so that a zeroising constructor is
>>>> added for any cache size.
>>>> - replaces calls to kmem_cache_zalloc() by kmem_cache_alloc()
>>>>
>>>
>>> Can't we just do kmem_cache_alloc with gfp flags __GFP_ZERO? and
>>> remove the constructor completely?
>>
>> I don't understand what you mean. That's exactly what I did in v1 (by
>> using kmem_cache_zalloc()), and you commented that doing this we would
>> zeroise at allocation whereas the constructors are called when adding
>> memory to the slab and when freeing the allocated block. Or did I
>> misunderstood your comment ?
>>
>> static inline void *kmem_cache_zalloc(struct kmem_cache *k, gfp_t flags)
>> {
>> return kmem_cache_alloc(k, flags | __GFP_ZERO);
>> }
>>
>>
>
> I completely misunderstood kmem_cache_zalloc. I took it as we zero out
> after each alloc. I guess your earlier patch is then good. We may want
> to double check this, I haven't looked at the slab internals.
In fact no, you were right. When kmem_cache_alloc() is called with
__GFP_ZERO, the object gets zeroised at allocation. This is done (at
least in SLUB) at the end of function slab_alloc_node()
>
> What we want is to make sure when we add new memory to slab, we want it
> zeroed. If we are allocating objects from existing slab memory pool, we
> don't need to zero out, because when we release objects to slab we make
> sure we clear it.
It looks like when we use constructors, they are called when adding an
object to the slab and when releasing it back to the slab. So that's
exactly what we want then, and therefore I have the feeling that we
should go with this v2 approach.
Those constructors are tiny (most of them are 3 insns) and we have only
16 cache sizes hence 16 constructors so it shoudln't be an issue to have
unused ones.
The only small problème I have is that some version of GCC seems to
complain about big memset() (132k and 256k ones). Is there a way to tell
GCC we really want to do it ?
Christophe
>
> -aneesh
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/mm/hash: Only need the Nest MMU workaround for R -> RW transition
From: Nicholas Piggin @ 2018-08-23 9:23 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: benh, paulus, mpe, linuxppc-dev
In-Reply-To: <20180822171605.15054-2-aneesh.kumar@linux.ibm.com>
On Wed, 22 Aug 2018 22:46:05 +0530
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> wrote:
> The Nest MMU workaround is only needed for RW upgrades. Avoid doing that
> for other pte updates.
>
> We also avoid clearing the pte while marking it invalid. This is because other
> page table walk will find this pte none and can result in unexpected behaviour
> due to that. Instead we clear _PAGE_PRESENT and set the software pte bit
> _PAGE_INVALID. pte_present is already updated to check for bot the bits. This
> make sure page table walkers will find the pte present and things like
> pte_pfn(pte) returns the right value.
>
> Based on the original patch from Benjamin Herrenschmidt <benh@kernel.crashing.org>
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> arch/powerpc/mm/pgtable-radix.c | 8 +++++---
This is powerpc/mm/radix, isn't it? Subject says hash.
Could we make this fix POWER9 only and use a RSV bit for it
rather than use up a SW bit? Other than that,
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/mm/pgtable-radix.c b/arch/powerpc/mm/pgtable-radix.c
> index 7be99fd9af15..c879979faa73 100644
> --- a/arch/powerpc/mm/pgtable-radix.c
> +++ b/arch/powerpc/mm/pgtable-radix.c
> @@ -1045,20 +1045,22 @@ void radix__ptep_set_access_flags(struct vm_area_struct *vma, pte_t *ptep,
> struct mm_struct *mm = vma->vm_mm;
> unsigned long set = pte_val(entry) & (_PAGE_DIRTY | _PAGE_ACCESSED |
> _PAGE_RW | _PAGE_EXEC);
> +
> + unsigned long change = pte_val(entry) ^ pte_val(*ptep);
> /*
> * To avoid NMMU hang while relaxing access, we need mark
> * the pte invalid in between.
> */
> - if (atomic_read(&mm->context.copros) > 0) {
> + if ((change & _PAGE_RW) && atomic_read(&mm->context.copros) > 0) {
> unsigned long old_pte, new_pte;
>
> - old_pte = __radix_pte_update(ptep, ~0, 0);
> + old_pte = __radix_pte_update(ptep, _PAGE_PRESENT, _PAGE_INVALID);
> /*
> * new value of pte
> */
> new_pte = old_pte | set;
> radix__flush_tlb_page_psize(mm, address, psize);
> - __radix_pte_update(ptep, 0, new_pte);
> + __radix_pte_update(ptep, _PAGE_INVALID, new_pte);
> } else {
> __radix_pte_update(ptep, 0, set);
> /*
^ permalink raw reply
* Re: [PATCH v8 5/5] powernv/pseries: consolidate code for mce early handling.
From: Nicholas Piggin @ 2018-08-23 9:02 UTC (permalink / raw)
To: Mahesh Jagannath Salgaonkar
Cc: linuxppc-dev, Ananth Narayan, Laurent Dufour, Aneesh Kumar K.V,
Michal Suchanek, Michael Ellerman
In-Reply-To: <57fb2d6c-4713-0fa8-3cff-6e66d2c02a69@linux.vnet.ibm.com>
On Thu, 23 Aug 2018 14:13:13 +0530
Mahesh Jagannath Salgaonkar <mahesh@linux.vnet.ibm.com> wrote:
> On 08/20/2018 05:04 PM, Nicholas Piggin wrote:
> > On Sun, 19 Aug 2018 22:38:39 +0530
> > Mahesh J Salgaonkar <mahesh@linux.vnet.ibm.com> wrote:
> >
> >> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
> >>
> >> Now that other platforms also implements real mode mce handler,
> >> lets consolidate the code by sharing existing powernv machine check
> >> early code. Rename machine_check_powernv_early to
> >> machine_check_common_early and reuse the code.
> >>
> >> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
> >> ---
> >> arch/powerpc/kernel/exceptions-64s.S | 155 ++++++----------------------------
> >> 1 file changed, 28 insertions(+), 127 deletions(-)
> >>
> >> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
> >> index 12f056179112..2f85a7baf026 100644
> >> --- a/arch/powerpc/kernel/exceptions-64s.S
> >> +++ b/arch/powerpc/kernel/exceptions-64s.S
> >> @@ -243,14 +243,13 @@ EXC_REAL_BEGIN(machine_check, 0x200, 0x100)
> >> SET_SCRATCH0(r13) /* save r13 */
> >> EXCEPTION_PROLOG_0(PACA_EXMC)
> >> BEGIN_FTR_SECTION
> >> - b machine_check_powernv_early
> >> + b machine_check_common_early
> >> FTR_SECTION_ELSE
> >> b machine_check_pSeries_0
> >> ALT_FTR_SECTION_END_IFSET(CPU_FTR_HVMODE)
> >> EXC_REAL_END(machine_check, 0x200, 0x100)
> >> EXC_VIRT_NONE(0x4200, 0x100)
> >> -TRAMP_REAL_BEGIN(machine_check_powernv_early)
> >> -BEGIN_FTR_SECTION
> >> +TRAMP_REAL_BEGIN(machine_check_common_early)
> >> EXCEPTION_PROLOG_1(PACA_EXMC, NOTEST, 0x200)
> >> /*
> >> * Register contents:
> >> @@ -306,7 +305,9 @@ BEGIN_FTR_SECTION
> >> /* Save r9 through r13 from EXMC save area to stack frame. */
> >> EXCEPTION_PROLOG_COMMON_2(PACA_EXMC)
> >> mfmsr r11 /* get MSR value */
> >> +BEGIN_FTR_SECTION
> >> ori r11,r11,MSR_ME /* turn on ME bit */
> >> +END_FTR_SECTION_IFSET(CPU_FTR_HVMODE)
> >> ori r11,r11,MSR_RI /* turn on RI bit */
> >> LOAD_HANDLER(r12, machine_check_handle_early)
> >> 1: mtspr SPRN_SRR0,r12
> >> @@ -325,7 +326,6 @@ BEGIN_FTR_SECTION
> >> andc r11,r11,r10 /* Turn off MSR_ME */
> >> b 1b
> >> b . /* prevent speculative execution */
> >> -END_FTR_SECTION_IFSET(CPU_FTR_HVMODE)
> >>
> >> TRAMP_REAL_BEGIN(machine_check_pSeries)
> >> .globl machine_check_fwnmi
> >> @@ -333,7 +333,7 @@ machine_check_fwnmi:
> >> SET_SCRATCH0(r13) /* save r13 */
> >> EXCEPTION_PROLOG_0(PACA_EXMC)
> >> BEGIN_FTR_SECTION
> >> - b machine_check_pSeries_early
> >> + b machine_check_common_early
> >> END_FTR_SECTION_IFCLR(CPU_FTR_HVMODE)
> >> machine_check_pSeries_0:
> >> EXCEPTION_PROLOG_1(PACA_EXMC, KVMTEST_PR, 0x200)
> >> @@ -346,103 +346,6 @@ machine_check_pSeries_0:
> >>
> >> TRAMP_KVM_SKIP(PACA_EXMC, 0x200)
> >>
> >> -TRAMP_REAL_BEGIN(machine_check_pSeries_early)
> >> -BEGIN_FTR_SECTION
> >> - EXCEPTION_PROLOG_1(PACA_EXMC, NOTEST, 0x200)
> >> - mr r10,r1 /* Save r1 */
> >> - lhz r11,PACA_IN_MCE(r13)
> >> - cmpwi r11,0 /* Are we in nested machine check */
> >> - bne 0f /* Yes, we are. */
> >> - /* First machine check entry */
> >> - ld r1,PACAMCEMERGSP(r13) /* Use MC emergency stack */
> >> -0: subi r1,r1,INT_FRAME_SIZE /* alloc stack frame */
> >> - addi r11,r11,1 /* increment paca->in_mce */
> >> - sth r11,PACA_IN_MCE(r13)
> >> - /* Limit nested MCE to level 4 to avoid stack overflow */
> >> - cmpwi r11,MAX_MCE_DEPTH
> >> - bgt 1f /* Check if we hit limit of 4 */
> >> - mfspr r11,SPRN_SRR0 /* Save SRR0 */
> >> - mfspr r12,SPRN_SRR1 /* Save SRR1 */
> >> - EXCEPTION_PROLOG_COMMON_1()
> >> - EXCEPTION_PROLOG_COMMON_2(PACA_EXMC)
> >> - EXCEPTION_PROLOG_COMMON_3(0x200)
> >> - addi r3,r1,STACK_FRAME_OVERHEAD
> >> - BRANCH_LINK_TO_FAR(machine_check_early) /* Function call ABI */
> >> - ld r12,_MSR(r1)
> >> - andi. r11,r12,MSR_PR /* See if coming from user. */
> >> - bne 2f /* continue in V mode if we are. */
> >> -
> >> - /*
> >> - * At this point we are not sure about what context we come from.
> >> - * We may be in the middle of swithing stack. r1 may not be valid.
> >> - * Hence stay on emergency stack, call machine_check_exception and
> >> - * return from the interrupt.
> >> - * But before that, check if this is an un-recoverable exception.
> >> - * If yes, then stay on emergency stack and panic.
> >> - */
> >> - andi. r11,r12,MSR_RI
> >> - beq 1f
> >> -
> >> - /*
> >> - * Check if we have successfully handled/recovered from error, if not
> >> - * then stay on emergency stack and panic.
> >> - */
> >> - cmpdi r3,0 /* see if we handled MCE successfully */
> >> - beq 1f /* if !handled then panic */
> >> -
> >> - /* Stay on emergency stack and return from interrupt. */
> >> - LOAD_HANDLER(r10,mce_return)
> >> - mtspr SPRN_SRR0,r10
> >> - ld r10,PACAKMSR(r13)
> >> - mtspr SPRN_SRR1,r10
> >> - RFI_TO_KERNEL
> >> - b .
> >> -
> >> -1: LOAD_HANDLER(r10,unrecover_mce)
> >> - mtspr SPRN_SRR0,r10
> >> - ld r10,PACAKMSR(r13)
> >> - /*
> >> - * We are going down. But there are chances that we might get hit by
> >> - * another MCE during panic path and we may run into unstable state
> >> - * with no way out. Hence, turn ME bit off while going down, so that
> >> - * when another MCE is hit during panic path, hypervisor will
> >> - * power cycle the lpar, instead of getting into MCE loop.
> >> - */
> >> - li r3,MSR_ME
> >> - andc r10,r10,r3 /* Turn off MSR_ME */
> >> - mtspr SPRN_SRR1,r10
> >> - RFI_TO_KERNEL
> >> - b .
> >> -
> >> - /* Move original SRR0 and SRR1 into the respective regs */
> >> -2: ld r9,_MSR(r1)
> >> - mtspr SPRN_SRR1,r9
> >> - ld r3,_NIP(r1)
> >> - mtspr SPRN_SRR0,r3
> >> - ld r9,_CTR(r1)
> >> - mtctr r9
> >> - ld r9,_XER(r1)
> >> - mtxer r9
> >> - ld r9,_LINK(r1)
> >> - mtlr r9
> >> - REST_GPR(0, r1)
> >> - REST_8GPRS(2, r1)
> >> - REST_GPR(10, r1)
> >> - ld r11,_CCR(r1)
> >> - mtcr r11
> >> - /* Decrement paca->in_mce. */
> >> - lhz r12,PACA_IN_MCE(r13)
> >> - subi r12,r12,1
> >> - sth r12,PACA_IN_MCE(r13)
> >> - REST_GPR(11, r1)
> >> - REST_2GPRS(12, r1)
> >> - /* restore original r1. */
> >> - ld r1,GPR1(r1)
> >> - SET_SCRATCH0(r13) /* save r13 */
> >> - EXCEPTION_PROLOG_0(PACA_EXMC)
> >> - b machine_check_pSeries_0
> >> -END_FTR_SECTION_IFCLR(CPU_FTR_HVMODE)
> >> -
> >> EXC_COMMON_BEGIN(machine_check_common)
> >> /*
> >> * Machine check is different because we use a different
> >> @@ -541,6 +444,9 @@ EXC_COMMON_BEGIN(machine_check_handle_early)
> >> bl machine_check_early
> >> std r3,RESULT(r1) /* Save result */
> >> ld r12,_MSR(r1)
> >> +BEGIN_FTR_SECTION
> >> + b 4f
> >> +END_FTR_SECTION_IFCLR(CPU_FTR_HVMODE)
> >>
> >> #ifdef CONFIG_PPC_P7_NAP
> >> /*
> >> @@ -564,10 +470,11 @@ EXC_COMMON_BEGIN(machine_check_handle_early)
> >> */
> >> rldicl. r11,r12,4,63 /* See if MC hit while in HV mode. */
> >> beq 5f
> >> - andi. r11,r12,MSR_PR /* See if coming from user. */
> >> +4: andi. r11,r12,MSR_PR /* See if coming from user. */
> >> bne 9f /* continue in V mode if we are. */
> >>
> >> 5:
> >> +BEGIN_FTR_SECTION
> >> #ifdef CONFIG_KVM_BOOK3S_64_HANDLER
> >> /*
> >> * We are coming from kernel context. Check if we are coming from
> >> @@ -578,6 +485,7 @@ EXC_COMMON_BEGIN(machine_check_handle_early)
> >> cmpwi r11,0 /* Check if coming from guest */
> >> bne 9f /* continue if we are. */
> >> #endif
> >> +END_FTR_SECTION_IFSET(CPU_FTR_HVMODE)
> >
> > Put these inside the ifdef?
> >
> >
> >> /*
> >> * At this point we are not sure about what context we come from.
> >> * Queue up the MCE event and return from the interrupt.
> >> @@ -611,6 +519,7 @@ EXC_COMMON_BEGIN(machine_check_handle_early)
> >> cmpdi r3,0 /* see if we handled MCE successfully */
> >>
> >> beq 1b /* if !handled then panic */
> >> +BEGIN_FTR_SECTION
> >> /*
> >> * Return from MC interrupt.
> >> * Queue up the MCE event so that we can log it later, while
> >> @@ -619,10 +528,24 @@ EXC_COMMON_BEGIN(machine_check_handle_early)
> >> bl machine_check_queue_event
> >> MACHINE_CHECK_HANDLER_WINDUP
> >> RFI_TO_USER_OR_KERNEL
> >> +FTR_SECTION_ELSE
> >> + /*
> >> + * pSeries: Return from MC interrupt. Before that stay on emergency
> >> + * stack and call machine_check_exception to log the MCE event.
> >> + */
> >> + LOAD_HANDLER(r10,mce_return)
> >> + mtspr SPRN_SRR0,r10
> >> + ld r10,PACAKMSR(r13)
> >> + mtspr SPRN_SRR1,r10
> >> + RFI_TO_KERNEL
> >> + b .
> >> +ALT_FTR_SECTION_END_IFSET(CPU_FTR_HVMODE)
> >
> > Do you still need mce_return? Why can't you consolidate it as well? ...
> > Hmm, okay so now I look back at patch 2, I don't think you should call
> > machine_check_exception there. You're supposed to call
> > machine_check_queue_event here and it will be handled by irq work.
>
> machine_check_queue_event does not handle RTAS mce event.
Yes it would need a bit of work.
> Also, we need
> to call fwnmi_release_errinfo() as early as possible which is why I am
> calling machine_check_exception() in mce_return path for pSeries.
> Otherwise if we get another MCE before calling fwnmi_release_errinfo()
> then lpar will get rebooted without any logs getting printed.
I think you can call that in your early handler, but then defer
the printing to the irq work.
Although hmm, maybe that's less of a problem now we do nmi_enter
in machine check exception so I think printk will use an NMI safe
buffer.
We have to be careful actually of soft irq state if we take a
machine check in an un-reconciled state or in the middle of
the irq replay code I'm not actually sure we do the right thing,
but that would be a bug in existing code too. And we definitely
have MSR[RI] vs DAR/DSISR bugs in existing code, sigh.
I don't know... maybe just push what you have and we'll try to do
some more fixes and cleanups on top of that.
Thanks,
Nick
^ permalink raw reply
* [PATCH 4.17 271/324] net/ethernet/freescale/fman: fix cross-build error
From: Greg Kroah-Hartman @ 2018-08-23 7:55 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable, Randy Dunlap, Madalin Bucur, netdev,
linuxppc-dev, David S. Miller, Sasha Levin
In-Reply-To: <20180823074955.885811006@linuxfoundation.org>
4.17-stable review patch. If anyone has any objections, please let me know.
------------------
From: Randy Dunlap <rdunlap@infradead.org>
[ Upstream commit c133459765fae249ba482f62e12f987aec4376f0 ]
CC [M] drivers/net/ethernet/freescale/fman/fman.o
In file included from ../drivers/net/ethernet/freescale/fman/fman.c:35:
../include/linux/fsl/guts.h: In function 'guts_set_dmacr':
../include/linux/fsl/guts.h:165:2: error: implicit declaration of function 'clrsetbits_be32' [-Werror=implicit-function-declaration]
clrsetbits_be32(&guts->dmacr, 3 << shift, device << shift);
^~~~~~~~~~~~~~~
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Cc: Madalin Bucur <madalin.bucur@nxp.com>
Cc: netdev@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <alexander.levin@microsoft.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
include/linux/fsl/guts.h | 1 +
1 file changed, 1 insertion(+)
--- a/include/linux/fsl/guts.h
+++ b/include/linux/fsl/guts.h
@@ -16,6 +16,7 @@
#define __FSL_GUTS_H__
#include <linux/types.h>
+#include <linux/io.h>
/**
* Global Utility Registers.
^ permalink raw reply
* Re: DT case sensitivity
From: Grant Likely @ 2018-08-23 9:02 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Rob Herring
Cc: Stephen Rothwell, Michael Ellerman, Kumar Gala, David Gibson,
Frank Rowand, devicetree-spec, devicetree, linuxppc-dev
In-Reply-To: <18175413a0148f7859ab36ed9715dce7a598f318.camel@kernel.crashing.org>
On 23/08/2018 02:29, Benjamin Herrenschmidt wrote:
> On Wed, 2018-08-22 at 20:26 -0500, Rob Herring wrote:
>> On Wed, Aug 22, 2018 at 8:14 PM Benjamin Herrenschmidt
>> <benh@kernel.crashing.org> wrote:
>>>
>>> On Wed, 2018-08-22 at 19:47 -0500, Rob Herring wrote:
>>>> The default DT string handling in the kernel is node names and
>>>> compatibles are case insensitive and property names are case sensitive
>>>> (Sparc is the the only variation and is opposite). It seems only PPC
>>>> (and perhaps only Power Macs?) needs to support case insensitive
>>>> comparisons. It was probably a mistake to follow PPC for new arches
>>>> and we should have made everything case sensitive from the start. So I
>>>> have a few questions for the DT historians. :)
>>>
>>> Open Firmware itself is insensitive.
>>
>> Doesn't it depend on the implementation? Otherwise, how is Sparc differe=
nt?
>
> Not sure ... Forth itself is insensitive for words but maybe not for
> string comparisons.
What problem are you trying to solve? I would think making everything
case insensitive would be the direction to go if you do anything. Least
possibility of breaking existing platforms in that scenario.
g.
>
>>
>>>> What PPC systems are case insensitive? Can we limit that to certain sy=
stems?
>>>
>>> All PowerMacs at least, the problem is that I don't have DT images or
>>> access to all the historical systems (and yes some people occasionally
>>> still use them) to properly test a change in that area.
>>
>> I'm temped to break them so I can find folks to provide me with DT dumps=
. :)
>
> I have a collection of DT dumps but I'm not sure about the legality of
> publishing them...
>
> Cheers,
> Ben.
>
>
IMPORTANT NOTICE: The contents of this email and any attachments are confid=
ential and may also be privileged. If you are not the intended recipient, p=
lease notify the sender immediately and do not disclose the contents to any=
other person, use it for any purpose, or store or copy the information in =
any medium. Thank you.
^ permalink raw reply
* Re: [PATCH v8 5/5] powernv/pseries: consolidate code for mce early handling.
From: Mahesh Jagannath Salgaonkar @ 2018-08-23 8:43 UTC (permalink / raw)
To: Nicholas Piggin
Cc: linuxppc-dev, Ananth Narayan, Laurent Dufour, Aneesh Kumar K.V,
Michal Suchanek, Michael Ellerman
In-Reply-To: <20180820213445.3b7dc3ee@roar.ozlabs.ibm.com>
On 08/20/2018 05:04 PM, Nicholas Piggin wrote:
> On Sun, 19 Aug 2018 22:38:39 +0530
> Mahesh J Salgaonkar <mahesh@linux.vnet.ibm.com> wrote:
>
>> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>>
>> Now that other platforms also implements real mode mce handler,
>> lets consolidate the code by sharing existing powernv machine check
>> early code. Rename machine_check_powernv_early to
>> machine_check_common_early and reuse the code.
>>
>> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>> ---
>> arch/powerpc/kernel/exceptions-64s.S | 155 ++++++----------------------------
>> 1 file changed, 28 insertions(+), 127 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
>> index 12f056179112..2f85a7baf026 100644
>> --- a/arch/powerpc/kernel/exceptions-64s.S
>> +++ b/arch/powerpc/kernel/exceptions-64s.S
>> @@ -243,14 +243,13 @@ EXC_REAL_BEGIN(machine_check, 0x200, 0x100)
>> SET_SCRATCH0(r13) /* save r13 */
>> EXCEPTION_PROLOG_0(PACA_EXMC)
>> BEGIN_FTR_SECTION
>> - b machine_check_powernv_early
>> + b machine_check_common_early
>> FTR_SECTION_ELSE
>> b machine_check_pSeries_0
>> ALT_FTR_SECTION_END_IFSET(CPU_FTR_HVMODE)
>> EXC_REAL_END(machine_check, 0x200, 0x100)
>> EXC_VIRT_NONE(0x4200, 0x100)
>> -TRAMP_REAL_BEGIN(machine_check_powernv_early)
>> -BEGIN_FTR_SECTION
>> +TRAMP_REAL_BEGIN(machine_check_common_early)
>> EXCEPTION_PROLOG_1(PACA_EXMC, NOTEST, 0x200)
>> /*
>> * Register contents:
>> @@ -306,7 +305,9 @@ BEGIN_FTR_SECTION
>> /* Save r9 through r13 from EXMC save area to stack frame. */
>> EXCEPTION_PROLOG_COMMON_2(PACA_EXMC)
>> mfmsr r11 /* get MSR value */
>> +BEGIN_FTR_SECTION
>> ori r11,r11,MSR_ME /* turn on ME bit */
>> +END_FTR_SECTION_IFSET(CPU_FTR_HVMODE)
>> ori r11,r11,MSR_RI /* turn on RI bit */
>> LOAD_HANDLER(r12, machine_check_handle_early)
>> 1: mtspr SPRN_SRR0,r12
>> @@ -325,7 +326,6 @@ BEGIN_FTR_SECTION
>> andc r11,r11,r10 /* Turn off MSR_ME */
>> b 1b
>> b . /* prevent speculative execution */
>> -END_FTR_SECTION_IFSET(CPU_FTR_HVMODE)
>>
>> TRAMP_REAL_BEGIN(machine_check_pSeries)
>> .globl machine_check_fwnmi
>> @@ -333,7 +333,7 @@ machine_check_fwnmi:
>> SET_SCRATCH0(r13) /* save r13 */
>> EXCEPTION_PROLOG_0(PACA_EXMC)
>> BEGIN_FTR_SECTION
>> - b machine_check_pSeries_early
>> + b machine_check_common_early
>> END_FTR_SECTION_IFCLR(CPU_FTR_HVMODE)
>> machine_check_pSeries_0:
>> EXCEPTION_PROLOG_1(PACA_EXMC, KVMTEST_PR, 0x200)
>> @@ -346,103 +346,6 @@ machine_check_pSeries_0:
>>
>> TRAMP_KVM_SKIP(PACA_EXMC, 0x200)
>>
>> -TRAMP_REAL_BEGIN(machine_check_pSeries_early)
>> -BEGIN_FTR_SECTION
>> - EXCEPTION_PROLOG_1(PACA_EXMC, NOTEST, 0x200)
>> - mr r10,r1 /* Save r1 */
>> - lhz r11,PACA_IN_MCE(r13)
>> - cmpwi r11,0 /* Are we in nested machine check */
>> - bne 0f /* Yes, we are. */
>> - /* First machine check entry */
>> - ld r1,PACAMCEMERGSP(r13) /* Use MC emergency stack */
>> -0: subi r1,r1,INT_FRAME_SIZE /* alloc stack frame */
>> - addi r11,r11,1 /* increment paca->in_mce */
>> - sth r11,PACA_IN_MCE(r13)
>> - /* Limit nested MCE to level 4 to avoid stack overflow */
>> - cmpwi r11,MAX_MCE_DEPTH
>> - bgt 1f /* Check if we hit limit of 4 */
>> - mfspr r11,SPRN_SRR0 /* Save SRR0 */
>> - mfspr r12,SPRN_SRR1 /* Save SRR1 */
>> - EXCEPTION_PROLOG_COMMON_1()
>> - EXCEPTION_PROLOG_COMMON_2(PACA_EXMC)
>> - EXCEPTION_PROLOG_COMMON_3(0x200)
>> - addi r3,r1,STACK_FRAME_OVERHEAD
>> - BRANCH_LINK_TO_FAR(machine_check_early) /* Function call ABI */
>> - ld r12,_MSR(r1)
>> - andi. r11,r12,MSR_PR /* See if coming from user. */
>> - bne 2f /* continue in V mode if we are. */
>> -
>> - /*
>> - * At this point we are not sure about what context we come from.
>> - * We may be in the middle of swithing stack. r1 may not be valid.
>> - * Hence stay on emergency stack, call machine_check_exception and
>> - * return from the interrupt.
>> - * But before that, check if this is an un-recoverable exception.
>> - * If yes, then stay on emergency stack and panic.
>> - */
>> - andi. r11,r12,MSR_RI
>> - beq 1f
>> -
>> - /*
>> - * Check if we have successfully handled/recovered from error, if not
>> - * then stay on emergency stack and panic.
>> - */
>> - cmpdi r3,0 /* see if we handled MCE successfully */
>> - beq 1f /* if !handled then panic */
>> -
>> - /* Stay on emergency stack and return from interrupt. */
>> - LOAD_HANDLER(r10,mce_return)
>> - mtspr SPRN_SRR0,r10
>> - ld r10,PACAKMSR(r13)
>> - mtspr SPRN_SRR1,r10
>> - RFI_TO_KERNEL
>> - b .
>> -
>> -1: LOAD_HANDLER(r10,unrecover_mce)
>> - mtspr SPRN_SRR0,r10
>> - ld r10,PACAKMSR(r13)
>> - /*
>> - * We are going down. But there are chances that we might get hit by
>> - * another MCE during panic path and we may run into unstable state
>> - * with no way out. Hence, turn ME bit off while going down, so that
>> - * when another MCE is hit during panic path, hypervisor will
>> - * power cycle the lpar, instead of getting into MCE loop.
>> - */
>> - li r3,MSR_ME
>> - andc r10,r10,r3 /* Turn off MSR_ME */
>> - mtspr SPRN_SRR1,r10
>> - RFI_TO_KERNEL
>> - b .
>> -
>> - /* Move original SRR0 and SRR1 into the respective regs */
>> -2: ld r9,_MSR(r1)
>> - mtspr SPRN_SRR1,r9
>> - ld r3,_NIP(r1)
>> - mtspr SPRN_SRR0,r3
>> - ld r9,_CTR(r1)
>> - mtctr r9
>> - ld r9,_XER(r1)
>> - mtxer r9
>> - ld r9,_LINK(r1)
>> - mtlr r9
>> - REST_GPR(0, r1)
>> - REST_8GPRS(2, r1)
>> - REST_GPR(10, r1)
>> - ld r11,_CCR(r1)
>> - mtcr r11
>> - /* Decrement paca->in_mce. */
>> - lhz r12,PACA_IN_MCE(r13)
>> - subi r12,r12,1
>> - sth r12,PACA_IN_MCE(r13)
>> - REST_GPR(11, r1)
>> - REST_2GPRS(12, r1)
>> - /* restore original r1. */
>> - ld r1,GPR1(r1)
>> - SET_SCRATCH0(r13) /* save r13 */
>> - EXCEPTION_PROLOG_0(PACA_EXMC)
>> - b machine_check_pSeries_0
>> -END_FTR_SECTION_IFCLR(CPU_FTR_HVMODE)
>> -
>> EXC_COMMON_BEGIN(machine_check_common)
>> /*
>> * Machine check is different because we use a different
>> @@ -541,6 +444,9 @@ EXC_COMMON_BEGIN(machine_check_handle_early)
>> bl machine_check_early
>> std r3,RESULT(r1) /* Save result */
>> ld r12,_MSR(r1)
>> +BEGIN_FTR_SECTION
>> + b 4f
>> +END_FTR_SECTION_IFCLR(CPU_FTR_HVMODE)
>>
>> #ifdef CONFIG_PPC_P7_NAP
>> /*
>> @@ -564,10 +470,11 @@ EXC_COMMON_BEGIN(machine_check_handle_early)
>> */
>> rldicl. r11,r12,4,63 /* See if MC hit while in HV mode. */
>> beq 5f
>> - andi. r11,r12,MSR_PR /* See if coming from user. */
>> +4: andi. r11,r12,MSR_PR /* See if coming from user. */
>> bne 9f /* continue in V mode if we are. */
>>
>> 5:
>> +BEGIN_FTR_SECTION
>> #ifdef CONFIG_KVM_BOOK3S_64_HANDLER
>> /*
>> * We are coming from kernel context. Check if we are coming from
>> @@ -578,6 +485,7 @@ EXC_COMMON_BEGIN(machine_check_handle_early)
>> cmpwi r11,0 /* Check if coming from guest */
>> bne 9f /* continue if we are. */
>> #endif
>> +END_FTR_SECTION_IFSET(CPU_FTR_HVMODE)
>
> Put these inside the ifdef?
>
>
>> /*
>> * At this point we are not sure about what context we come from.
>> * Queue up the MCE event and return from the interrupt.
>> @@ -611,6 +519,7 @@ EXC_COMMON_BEGIN(machine_check_handle_early)
>> cmpdi r3,0 /* see if we handled MCE successfully */
>>
>> beq 1b /* if !handled then panic */
>> +BEGIN_FTR_SECTION
>> /*
>> * Return from MC interrupt.
>> * Queue up the MCE event so that we can log it later, while
>> @@ -619,10 +528,24 @@ EXC_COMMON_BEGIN(machine_check_handle_early)
>> bl machine_check_queue_event
>> MACHINE_CHECK_HANDLER_WINDUP
>> RFI_TO_USER_OR_KERNEL
>> +FTR_SECTION_ELSE
>> + /*
>> + * pSeries: Return from MC interrupt. Before that stay on emergency
>> + * stack and call machine_check_exception to log the MCE event.
>> + */
>> + LOAD_HANDLER(r10,mce_return)
>> + mtspr SPRN_SRR0,r10
>> + ld r10,PACAKMSR(r13)
>> + mtspr SPRN_SRR1,r10
>> + RFI_TO_KERNEL
>> + b .
>> +ALT_FTR_SECTION_END_IFSET(CPU_FTR_HVMODE)
>
> Do you still need mce_return? Why can't you consolidate it as well? ...
> Hmm, okay so now I look back at patch 2, I don't think you should call
> machine_check_exception there. You're supposed to call
> machine_check_queue_event here and it will be handled by irq work.
machine_check_queue_event does not handle RTAS mce event. Also, we need
to call fwnmi_release_errinfo() as early as possible which is why I am
calling machine_check_exception() in mce_return path for pSeries.
Otherwise if we get another MCE before calling fwnmi_release_errinfo()
then lpar will get rebooted without any logs getting printed.
Thanks,
-Mahesh.
^ permalink raw reply
* [PATCH 4.14 186/217] net/ethernet/freescale/fman: fix cross-build error
From: Greg Kroah-Hartman @ 2018-08-23 7:54 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable, Randy Dunlap, Madalin Bucur, netdev,
linuxppc-dev, David S. Miller, Sasha Levin
In-Reply-To: <20180823075505.090246914@linuxfoundation.org>
4.14-stable review patch. If anyone has any objections, please let me know.
------------------
From: Randy Dunlap <rdunlap@infradead.org>
[ Upstream commit c133459765fae249ba482f62e12f987aec4376f0 ]
CC [M] drivers/net/ethernet/freescale/fman/fman.o
In file included from ../drivers/net/ethernet/freescale/fman/fman.c:35:
../include/linux/fsl/guts.h: In function 'guts_set_dmacr':
../include/linux/fsl/guts.h:165:2: error: implicit declaration of function 'clrsetbits_be32' [-Werror=implicit-function-declaration]
clrsetbits_be32(&guts->dmacr, 3 << shift, device << shift);
^~~~~~~~~~~~~~~
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Cc: Madalin Bucur <madalin.bucur@nxp.com>
Cc: netdev@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <alexander.levin@microsoft.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
include/linux/fsl/guts.h | 1 +
1 file changed, 1 insertion(+)
--- a/include/linux/fsl/guts.h
+++ b/include/linux/fsl/guts.h
@@ -16,6 +16,7 @@
#define __FSL_GUTS_H__
#include <linux/types.h>
+#include <linux/io.h>
/**
* Global Utility Registers.
^ permalink raw reply
* [PATCH 4.9 110/130] net/ethernet/freescale/fman: fix cross-build error
From: Greg Kroah-Hartman @ 2018-08-23 7:53 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable, Randy Dunlap, Madalin Bucur, netdev,
linuxppc-dev, David S. Miller, Sasha Levin
In-Reply-To: <20180823074927.161454870@linuxfoundation.org>
4.9-stable review patch. If anyone has any objections, please let me know.
------------------
From: Randy Dunlap <rdunlap@infradead.org>
[ Upstream commit c133459765fae249ba482f62e12f987aec4376f0 ]
CC [M] drivers/net/ethernet/freescale/fman/fman.o
In file included from ../drivers/net/ethernet/freescale/fman/fman.c:35:
../include/linux/fsl/guts.h: In function 'guts_set_dmacr':
../include/linux/fsl/guts.h:165:2: error: implicit declaration of function 'clrsetbits_be32' [-Werror=implicit-function-declaration]
clrsetbits_be32(&guts->dmacr, 3 << shift, device << shift);
^~~~~~~~~~~~~~~
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Cc: Madalin Bucur <madalin.bucur@nxp.com>
Cc: netdev@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <alexander.levin@microsoft.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
include/linux/fsl/guts.h | 1 +
1 file changed, 1 insertion(+)
--- a/include/linux/fsl/guts.h
+++ b/include/linux/fsl/guts.h
@@ -16,6 +16,7 @@
#define __FSL_GUTS_H__
#include <linux/types.h>
+#include <linux/io.h>
/**
* Global Utility Registers.
^ permalink raw reply
* Re: [RESEND PATCH v2] powerpc/mce: Fix SLB rebolting during MCE recovery path.
From: Mahesh Jagannath Salgaonkar @ 2018-08-23 8:01 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: Nicholas Piggin, Aneesh Kumar K.V
In-Reply-To: <87d0u9lgyt.fsf@concordia.ellerman.id.au>
On 08/23/2018 12:14 PM, Michael Ellerman wrote:
> Mahesh J Salgaonkar <mahesh@linux.vnet.ibm.com> writes:
>
>> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>>
>> With the powerpc next commit e7e81847478 (powerpc/mce: Fix SLB rebolting
>> during MCE recovery path.),
>
> That commit description is wrong, I'll fix it up.
Ouch.. My bad.. :-(
>
> cheers
>
>> the SLB error recovery is broken. The new
>> change now does not add index value to RB[52-63] that selects the SLB
>> entry while rebolting, instead it assumes that the shadow save area
>> already have index embeded correctly in esid field. While all valid bolted
>> save areas do contain index value set correctly, there is a case where
>> 3rd (KSTACK_INDEX) entry for kernel stack does not embed index for NULL
>> esid entry. This patch fixes that.
>>
>> Without this patch the SLB rebolt code overwrites the 1st entry of kernel
>> linear mapping and causes SLB recovery to fail.
>>
>> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>> arch/powerpc/mm/slb.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/mm/slb.c b/arch/powerpc/mm/slb.c
>> index 0b095fa54049..9f574e59d178 100644
>> --- a/arch/powerpc/mm/slb.c
>> +++ b/arch/powerpc/mm/slb.c
>> @@ -70,7 +70,7 @@ static inline void slb_shadow_update(unsigned long ea, int ssize,
>>
>> static inline void slb_shadow_clear(enum slb_index index)
>> {
>> - WRITE_ONCE(get_slb_shadow()->save_area[index].esid, 0);
>> + WRITE_ONCE(get_slb_shadow()->save_area[index].esid, cpu_to_be64(index));
>> }
>>
>> static inline void create_shadowed_slbe(unsigned long ea, int ssize,
>
^ permalink raw reply
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