Re: [PATCH v3] console: use first console if stdout-path device doesn't appear

[Paul Burton]

Hi Larry,

On Monday, 7 November 2016 09:26:37 GMT Larry Finger wrote:
> > A revert was already submitted by Hans de Goede & is being discussed over
> > here:
> > 
> > https://marc.info/?l=linux-kernel=147826151427455=2
> 
> I am a little surprised that I was not CCd on that thread.

Hans started that thread without copying you just as you started your thread 
without copying Andreas, who reported issues first.

> To reiterate, my
> PowerBook G4 with a PPC32 processor CRASHES on boot. That is a lot more
> serious than the console output disappearing.

Crashes as in init dies due to lack of a console, right?

> As it seems unlikely that this regression will be fixed in the current
> cycle, I recommend that the reversion of commit 05fd007e4629 until a proper
> fix is available.

I agree that reverting is probably the best option for now, and have replied 
with that in the other thread too.

Thanks,
Paul

signature.asc
Description: This is a digitally signed message part.

Re: [PATCH v3] console: use first console if stdout-path device doesn't appear

[Paul Burton]

On Monday, 7 November 2016 19:27:32 GMT Michael Ellerman wrote:
> Paul Burton <paul.bur...@imgtec.com> writes:
> > If a device tree specified a preferred device for kernel console output
> > via the stdout-path or linux,stdout-path chosen node properties there's
> > no guarantee that it will have specified a device for which we have a
> > driver. It may also be the case that we do have a driver but it doesn't
> > call of_console_check() to register as a preferred console (eg. offb
> > driver as used on powermac systems).
> > 
> > In these cases try to ensure that we provide some console output by
> > enabling the first usable registered console, which we keep track of
> > with the of_fallback_console variable. Affected systems will enable
> > their console later than they did prior to commit 05fd007e4629
> > ("console: don't prefer first registered if DT specifies stdout-path")
> > but should otherwise produce the same output.
> > 
> > Tested in QEMU with a PowerPC pseries_defconfig kernel.
> 
> Hi Paul,
> 
> This does "work", as in it boots and I get a console. But the delay in
> getting output on the VGA is not workable. I get pretty much no output
> until the machine is booted entirely to userspace, meaning any crash
> prior to that will be undebuggable.
> 
> I also note Andreas reports it doesn't work at all on PowerMac.
> 
> Please send a revert and we can try again next cycle.
> 
> cheers

Hi Michael,

A revert was already submitted by Hans de Goede & is being discussed over 
here:

https://marc.info/?l=linux-kernel=147826151427455=2

Thanks,
Paul

signature.asc
Description: This is a digitally signed message part.

[PATCH] console: use first console if stdout-path device doesn't appear

[Paul Burton]

If a device tree specified a preferred device for kernel console output
via the stdout-path or linux,stdout-path chosen node properties there's
no guarantee that it will have specified a device for which we have a
driver. It may also be the case that we do have a driver but it doesn't
call of_console_check() to register as a preferred console (eg. offb
driver as used on powermac systems). In these cases try to ensure that
we provide some console output by enabling the first console in the
console_drivers list.

As I don't have access to an affected system this has only been build
tested - testing would be most appreciated.

Signed-off-by: Paul Burton <paul.bur...@imgtec.com>
Fixes: 05fd007e4629 ("console: don't prefer first registered if DT specifies 
stdout-path")
Reported-by: Andreas Schwab <sch...@linux-m68k.org>
Cc: Andreas Schwab <sch...@linux-m68k.org>
Cc: Andrew Morton <a...@linux-foundation.org>
Cc: Petr Mladek <pmla...@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhat...@gmail.com>
Cc: Borislav Petkov <b...@suse.de>
Cc: Tejun Heo <t...@kernel.org>
Cc: linux-ker...@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
---
A potential alternative to this might be to have the affected offb
driver call of_check_console(), and perhaps that should happen anyway,
but doing so seems non-trivial since the offb driver doesn't know the
index of the framebuffer console device it may be about to register &
the fbdev core doesn't know the associated device tree node. This also
wouldn't catch the case of us not having a driver for the device
specified by stdout-path, so this fallback seems worthwhile anyway.
---
 kernel/printk/printk.c | 37 -
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index d5e3973..7091e2f 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2835,10 +2835,45 @@ EXPORT_SYMBOL(unregister_console);
  * intersects with the init section. Note that code exists elsewhere to get
  * rid of the boot console as soon as the proper console shows up, so there
  * won't be side-effects from postponing the removal.
+ *
+ * Additionally we may be using a device tree which specifies valid
+ * stdout-path referencing a device for which we don't have a driver, or for
+ * which we have a driver that doesn't register itself as preferred console
+ * using of_console_check(). In these cases we attempt here to enable the
+ * first registered console.
  */
 static int __init printk_late_init(void)
 {
-   struct console *con;
+   struct console *con, *enabled;
+
+   if (of_specified_console) {
+   console_lock();
+
+   /* Find the enabled console, if there is one */
+   enabled = NULL;
+   for_each_console(con) {
+   if (!(con->flags & CON_ENABLED))
+   continue;
+
+   enabled = con;
+   break;
+   }
+
+   /* Enable the first console if none were already enabled */
+   con = console_drivers;
+   if (!enabled && con) {
+   if (con->index < 0)
+   con->index = 0;
+   if (con->setup == NULL ||
+   con->setup(con, NULL) == 0) {
+   con->flags |= CON_ENABLED;
+   if (con->device)
+   con->flags |= CON_CONSDEV;
+   }
+   }
+
+   console_unlock();
+   }
 
for_each_console(con) {
if (!keep_bootcon && con->flags & CON_BOOT) {
-- 
2.10.0

Re: [PATCH v2] console: Don't prefer first registered if DT specifies stdout-path

[Paul Burton]

On Monday, 17 October 2016 19:39:57 BST Andreas Schwab wrote:
> On Okt 17 2016, Paul Burton <paul.bur...@imgtec.com> wrote:
> > Could you share the device tree from your system?
> 
> This is the contents of chosen/linux,stdout-path on the systems I have:
> 
> chosen/linux,stdout-path
>  "/pci@f000/ATY,SnowyParent@10/ATY,Snowy_A@0"
> 
> chosen/linux,stdout-path
>  "/pci@0,f000/NVDA,Parent@10/NVDA,Display-B@1"
> 
> Is that what you need?  There is also chosen/stdout, but no
> aliases/stdout.
> 
> Andreas.

Hi Andreas,

I think I see the problem & I'm hoping this patch will fix it:

https://lkml.org/lkml/2016/10/18/142

Could you give it a try & let me know?

Thanks,
Paul

signature.asc
Description: This is a digitally signed message part.

[PATCH v3] console: use first console if stdout-path device doesn't appear

[Paul Burton]

If a device tree specified a preferred device for kernel console output
via the stdout-path or linux,stdout-path chosen node properties there's
no guarantee that it will have specified a device for which we have a
driver. It may also be the case that we do have a driver but it doesn't
call of_console_check() to register as a preferred console (eg. offb
driver as used on powermac systems).

In these cases try to ensure that we provide some console output by
enabling the first usable registered console, which we keep track of
with the of_fallback_console variable. Affected systems will enable
their console later than they did prior to commit 05fd007e4629
("console: don't prefer first registered if DT specifies stdout-path")
but should otherwise produce the same output.

Tested in QEMU with a PowerPC pseries_defconfig kernel.

Signed-off-by: Paul Burton <paul.bur...@imgtec.com>
Fixes: 05fd007e4629 ("console: don't prefer first registered if DT specifies 
stdout-path")
Reported-by: Andreas Schwab <sch...@linux-m68k.org>
Reported-by: Larry Finger <larry.fin...@lwfinger.net>
Reported-by: Michael Ellerman <m...@ellerman.id.au>
Cc: Andreas Schwab <sch...@linux-m68k.org>
Cc: Andrew Morton <a...@linux-foundation.org>
Cc: Borislav Petkov <b...@suse.de>
Cc: Larry Finger <larry.fin...@lwfinger.net>
Cc: Michael Ellerman <m...@ellerman.id.au>
Cc: Petr Mladek <pmla...@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhat...@gmail.com>
Cc: Tejun Heo <t...@kernel.org>
Cc: linux-ker...@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org

---

Changes in v3:
- Be less invasive by simply calling register_console() again rather than 
splitting it.
- Check for CON_CONSDEV on the first console driver rather than for CON_ENABLED 
on any.
- Clean up the tracking of the fallback console.

Changes in v2:
- Split enable_console() out of register_console() & call in the fallback case.
- Track the console we would have enabled as of_fallback_console.

 kernel/printk/printk.c | 44 ++--
 1 file changed, 42 insertions(+), 2 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index de08fc9..c86e6d0 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -260,10 +260,18 @@ void console_set_by_of(void)
 {
of_specified_console = true;
 }
+
+static void clear_of_specified_console(void)
+{
+   of_specified_console = false;
+}
 #else
 # define of_specified_console false
+static void clear_of_specified_console(void) { }
 #endif
 
+struct console *of_fallback_console;
+
 /* Flag: console code may call schedule() */
 static int console_may_schedule;
 
@@ -2657,10 +2665,26 @@ void register_console(struct console *newcon)
 *  didn't select a console we take the first one
 *  that registers here.
 */
-   if (preferred_console < 0 && !of_specified_console) {
+   if (preferred_console < 0) {
if (newcon->index < 0)
newcon->index = 0;
-   if (newcon->setup == NULL ||
+   if (of_specified_console) {
+   /*
+* The device tree stdout-path chosen node property was
+* specified so we don't want to enable the first
+* registered console just now in order to give the
+* device indicated by stdout-path a chance to be
+* registered first. Do however keep track of the
+* first console we see so that we can fall back to
+* using it if we don't see the desired device, either
+* because stdout-path isn't valid, or because we have
+* no driver for the device or our driver doesn't call
+* of_console_check(). See printk_late_init() for this
+* fallback.
+*/
+   if (!of_fallback_console)
+   of_fallback_console = newcon;
+   } else if (newcon->setup == NULL ||
newcon->setup(newcon, NULL) == 0) {
newcon->flags |= CON_ENABLED;
if (newcon->device) {
@@ -2844,6 +2868,22 @@ static int __init printk_late_init(void)
 {
struct console *con;
 
+   if (of_specified_console && of_fallback_console &&
+   (!console_drivers || !(console_drivers->flags & CON_CONSDEV))) {
+   /*
+* The system has a device tree which specified stdout-path,
+* but we haven't seen a console associated with the device
+* specified by the stdout-path chosen node property.
+*
+* We do however know which console would have be

Re: [PATCH v2] console: use first console if stdout-path device doesn't appear

[Paul Burton]

On Monday, 31 October 2016 18:31:43 GMT Larry Finger wrote:
> On 10/31/2016 06:09 PM, Sergey Senozhatsky wrote:
> > Hi,
> > 
> > On (10/31/16 15:50), Paul Burton wrote:
> > [..]
> > 
> >> Actually whilst this fixes the output in QEMU it has other problems. I'm
> >> still digging...
> > 
> > I propose a revert of '05fd007e46296', so you guys can find the
> > problem and fix it, not being under 'rc3' pressure.
> 
> If we were at -rc4 or -rc5, then I would agree; however, I think there is
> still time to fix the problem.
> 
> Larry

Hi Larry et al,

If you could give the v3 I just submitted a try that would be great:

https://patchwork.kernel.org/patch/9410849/

If it still doesn't work for you then we're back to a place where I can't test 
an affected system, since QEMU pseries now works fine with my patch. Would you 
perhaps be able to get some console output by specifying console= on 
the kernel command line? Doing that ought to be unaffected by 05fd007e4629 
("console: don't prefer first registered if DT specifies stdout-path") and 
could be a way to get us some debug output.

Thanks,
Paul

signature.asc
Description: This is a digitally signed message part.

Re: [PATCH v3] console: use first console if stdout-path device doesn't appear

[Paul Burton]

Hi Sergey,

On Friday, 4 November 2016 02:40:40 GMT Sergey Senozhatsky wrote:
> On (11/03/16 12:57), Paul Burton wrote:
> > If a device tree specified a preferred device for kernel console output
> > via the stdout-path or linux,stdout-path chosen node properties there's
> > no guarantee that it will have specified a device for which we have a
> > driver. It may also be the case that we do have a driver but it doesn't
> > call of_console_check() to register as a preferred console (eg. offb
> > driver as used on powermac systems).
> 
> so why that driver doesn't call of_console_check() then? if there is a
> misconfiguration then why do we want to fix it/fallback in printk code?

Ideally I think the driver (or perhaps fbdev/fbcon) ought to call 
of_console_check() which would allow the console to be enabled earlier again. 
However there isn't any single place I can see that currently has all the 
information required to do so - the device tree node, the name & index of the 
console.

Even if we change that in the future & do call of_console_check(), I can't 
guarantee that offb is the only driver that would encounter this case, and it 
still wouldn't cover the case of us not having any driver for the specified 
stdout-path device. The fallback thus seems like a sensible thing to do.

> 
> [..]
> 
> > @@ -260,10 +260,18 @@ void console_set_by_of(void)
> > 
> >  {
> >  
> > of_specified_console = true;
> >  
> >  }
> > 
> > +
> > +static void clear_of_specified_console(void)
> > +{
> > +   of_specified_console = false;
> > +}
> > 
> >  #else
> >  # define of_specified_console false
> > 
> > +static void clear_of_specified_console(void) { }
> > 
> >  #endif
> > 
> > +struct console *of_fallback_console;
> > +
> > 
> >  /* Flag: console code may call schedule() */
> >  static int console_may_schedule;
> > 
> > @@ -2657,10 +2665,26 @@ void register_console(struct console *newcon)
> > 
> >  *  didn't select a console we take the first one
> >  *  that registers here.
> >  */
> > 
> > -   if (preferred_console < 0 && !of_specified_console) {
> > +   if (preferred_console < 0) {
> > 
> > if (newcon->index < 0)
> > 
> > newcon->index = 0;
> > 
> > -   if (newcon->setup == NULL ||
> > +   if (of_specified_console) {
> > +   /*
> > +* The device tree stdout-path chosen node property was
> > +* specified so we don't want to enable the first
> > +* registered console just now in order to give the
> > +* device indicated by stdout-path a chance to be
> > +* registered first. Do however keep track of the
> > +* first console we see so that we can fall back to
> > +* using it if we don't see the desired device, either
> > +* because stdout-path isn't valid, or because we have
> > +* no driver for the device or our driver doesn't call
> > +* of_console_check(). See printk_late_init() for this
> > +* fallback.
> 
> if the path is not valid then correct the path. no?

...but what if the path is valid and we simply don't have a driver for the 
device it references? As I said in that comment we may not have a driver at 
all.

> 
> > +*/
> > +   if (!of_fallback_console)
> > +   of_fallback_console = newcon;
> > +   } else if (newcon->setup == NULL ||
> > 
> > newcon->setup(newcon, NULL) == 0) {
> > 
> > newcon->flags |= CON_ENABLED;
> > if (newcon->device) {
> > 
> > @@ -2844,6 +2868,22 @@ static int __init printk_late_init(void)
> > 
> >  {
> >  
> > struct console *con;
> > 
> > +   if (of_specified_console && of_fallback_console &&
> > +   (!console_drivers || !(console_drivers->flags & CON_CONSDEV))) {
> > +   /*
> > +* The system has a device tree which specified stdout-path,
> > +* but we haven't seen a console associated with the device
> > +* specified by the stdout-path chosen node property.
> > +*
> > +* We do however know which console would have been used
> > +* i

[PATCH v2] console: use first console if stdout-path device doesn't appear

[Paul Burton]

If a device tree specified a preferred device for kernel console output
via the stdout-path or linux,stdout-path chosen node properties there's
no guarantee that it will have specified a device for which we have a
driver. It may also be the case that we do have a driver but it doesn't
call of_console_check() to register as a preferred console (eg. offb
driver as used on powermac systems). In these cases try to ensure that
we provide some console output by enabling the first usable registered
console, which we keep track of with the of_fallback_console variable.

Tested in QEMU with a PowerPC pseries_defconfig kernel.

Signed-off-by: Paul Burton <paul.bur...@imgtec.com>
Fixes: 05fd007e4629 ("console: don't prefer first registered if DT specifies 
stdout-path")
Reported-by: Andreas Schwab <sch...@linux-m68k.org>
Reported-by: Larry Finger <larry.fin...@lwfinger.net>
Reported-by: Michael Ellerman <m...@ellerman.id.au>
Cc: Andreas Schwab <sch...@linux-m68k.org>
Cc: Andrew Morton <a...@linux-foundation.org>
Cc: Borislav Petkov <b...@suse.de>
Cc: Larry Finger <larry.fin...@lwfinger.net>
Cc: Michael Ellerman <m...@ellerman.id.au>
Cc: Petr Mladek <pmla...@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhat...@gmail.com>
Cc: Tejun Heo <t...@kernel.org>
Cc: linux-ker...@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org

---

Changes in v2:
- Split enable_console() out of register_console() & call in the fallback case.
- Track the console we would have enabled as of_fallback_console.

 kernel/printk/printk.c | 60 +-
 1 file changed, 55 insertions(+), 5 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index de08fc9..b02c00a 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -264,6 +264,13 @@ void console_set_by_of(void)
 # define of_specified_console false
 #endif
 
+/*
+ * The first usable console, which we'll fall back to using if the system
+ * uses a device tree which indicates a stdout-path device for which we
+ * have no driver, or for which our driver doesn't call of_console_check().
+ */
+static struct console *of_fallback_console;
+
 /* Flag: console code may call schedule() */
 static int console_may_schedule;
 
@@ -2598,6 +2605,8 @@ static int __init keep_bootcon_setup(char *str)
 
 early_param("keep_bootcon", keep_bootcon_setup);
 
+static void enable_console(struct console *newcon);
+
 /*
  * The console driver calls this routine during kernel initialization
  * to register the console printing procedure with printk() and to
@@ -2620,7 +2629,6 @@ early_param("keep_bootcon", keep_bootcon_setup);
 void register_console(struct console *newcon)
 {
int i;
-   unsigned long flags;
struct console *bcon = NULL;
struct console_cmdline *c;
 
@@ -2657,7 +2665,9 @@ void register_console(struct console *newcon)
 *  didn't select a console we take the first one
 *  that registers here.
 */
-   if (preferred_console < 0 && !of_specified_console) {
+   if (preferred_console < 0 && of_specified_console) {
+   of_fallback_console = of_fallback_console ?: newcon;
+   } else if (preferred_console < 0) {
if (newcon->index < 0)
newcon->index = 0;
if (newcon->setup == NULL ||
@@ -2705,8 +2715,18 @@ void register_console(struct console *newcon)
break;
}
 
-   if (!(newcon->flags & CON_ENABLED))
-   return;
+   if (newcon->flags & CON_ENABLED)
+   enable_console(newcon);
+}
+EXPORT_SYMBOL(register_console);
+
+static void enable_console(struct console *newcon)
+{
+   struct console *bcon = NULL;
+   unsigned long flags;
+
+   if (console_drivers && console_drivers->flags & CON_BOOT)
+   bcon = console_drivers;
 
/*
 * If we have a bootconsole, and are switching to a real console,
@@ -2777,7 +2797,6 @@ void register_console(struct console *newcon)
unregister_console(bcon);
}
 }
-EXPORT_SYMBOL(register_console);
 
 int unregister_console(struct console *console)
 {
@@ -2839,10 +2858,41 @@ EXPORT_SYMBOL(unregister_console);
  * intersects with the init section. Note that code exists elsewhere to get
  * rid of the boot console as soon as the proper console shows up, so there
  * won't be side-effects from postponing the removal.
+ *
+ * Additionally we may be using a device tree which specifies valid
+ * stdout-path referencing a device for which we don't have a driver, or for
+ * which we have a driver that doesn't register itself as preferred console
+ * using of_console_check(). In these cases we attempt here to enable the
+ * first usable registered console, which we assigned to of_fallback_console.

Re: [PATCH] console: use first console if stdout-path device doesn't appear

[Paul Burton]

Hi Michael et al,

On Monday, 31 October 2016 16:28:59 GMT Michael Ellerman wrote:
> Andreas Schwab  writes:
> > Any news?
> 
> We discovered it also breaks VGA on qemu, which presumably is not the
> type of news you were hoping for.

On the contrary, that's wonderful news - I can test that! Huzzah for 
brokenness!

Thanks for your steps to reproduce the issue. Could you try my v2 patch? It 
works for me in QEMU, at least as far as seeing the kernel console output for 
a panic due to not having a rootfs. Hopefully it works for you (& Andreas & 
Larry) too:

https://patchwork.kernel.org/patch/9405415/

Thanks,
Paul

signature.asc
Description: This is a digitally signed message part.

Re: [PATCH v2] console: use first console if stdout-path device doesn't appear

[Paul Burton]

On Monday, 31 October 2016 12:14:55 GMT Paul Burton wrote:
> If a device tree specified a preferred device for kernel console output
> via the stdout-path or linux,stdout-path chosen node properties there's
> no guarantee that it will have specified a device for which we have a
> driver. It may also be the case that we do have a driver but it doesn't
> call of_console_check() to register as a preferred console (eg. offb
> driver as used on powermac systems). In these cases try to ensure that
> we provide some console output by enabling the first usable registered
> console, which we keep track of with the of_fallback_console variable.
> 
> Tested in QEMU with a PowerPC pseries_defconfig kernel.

Actually whilst this fixes the output in QEMU it has other problems. I'm still 
digging...

Thanks,
Paul

> 
> Signed-off-by: Paul Burton <paul.bur...@imgtec.com>
> Fixes: 05fd007e4629 ("console: don't prefer first registered if DT specifies
> stdout-path") Reported-by: Andreas Schwab <sch...@linux-m68k.org>
> Reported-by: Larry Finger <larry.fin...@lwfinger.net>
> Reported-by: Michael Ellerman <m...@ellerman.id.au>
> Cc: Andreas Schwab <sch...@linux-m68k.org>
> Cc: Andrew Morton <a...@linux-foundation.org>
> Cc: Borislav Petkov <b...@suse.de>
> Cc: Larry Finger <larry.fin...@lwfinger.net>
> Cc: Michael Ellerman <m...@ellerman.id.au>
> Cc: Petr Mladek <pmla...@suse.com>
> Cc: Sergey Senozhatsky <sergey.senozhat...@gmail.com>
> Cc: Tejun Heo <t...@kernel.org>
> Cc: linux-ker...@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> 
> ---
> 
> Changes in v2:
> - Split enable_console() out of register_console() & call in the fallback
> case. - Track the console we would have enabled as of_fallback_console.
> 
>  kernel/printk/printk.c | 60
> +- 1 file changed, 55
> insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index de08fc9..b02c00a 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -264,6 +264,13 @@ void console_set_by_of(void)
>  # define of_specified_console false
>  #endif
> 
> +/*
> + * The first usable console, which we'll fall back to using if the system
> + * uses a device tree which indicates a stdout-path device for which we
> + * have no driver, or for which our driver doesn't call of_console_check().
> + */
> +static struct console *of_fallback_console;
> +
>  /* Flag: console code may call schedule() */
>  static int console_may_schedule;
> 
> @@ -2598,6 +2605,8 @@ static int __init keep_bootcon_setup(char *str)
> 
>  early_param("keep_bootcon", keep_bootcon_setup);
> 
> +static void enable_console(struct console *newcon);
> +
>  /*
>   * The console driver calls this routine during kernel initialization
>   * to register the console printing procedure with printk() and to
> @@ -2620,7 +2629,6 @@ early_param("keep_bootcon", keep_bootcon_setup);
>  void register_console(struct console *newcon)
>  {
>   int i;
> - unsigned long flags;
>   struct console *bcon = NULL;
>   struct console_cmdline *c;
> 
> @@ -2657,7 +2665,9 @@ void register_console(struct console *newcon)
>*  didn't select a console we take the first one
>*  that registers here.
>*/
> - if (preferred_console < 0 && !of_specified_console) {
> + if (preferred_console < 0 && of_specified_console) {
> + of_fallback_console = of_fallback_console ?: newcon;
> + } else if (preferred_console < 0) {
>   if (newcon->index < 0)
>   newcon->index = 0;
>   if (newcon->setup == NULL ||
> @@ -2705,8 +2715,18 @@ void register_console(struct console *newcon)
>   break;
>   }
> 
> - if (!(newcon->flags & CON_ENABLED))
> - return;
> + if (newcon->flags & CON_ENABLED)
> + enable_console(newcon);
> +}
> +EXPORT_SYMBOL(register_console);
> +
> +static void enable_console(struct console *newcon)
> +{
> + struct console *bcon = NULL;
> + unsigned long flags;
> +
> + if (console_drivers && console_drivers->flags & CON_BOOT)
> + bcon = console_drivers;
> 
>   /*
>* If we have a bootconsole, and are switching to a real console,
> @@ -2777,7 +2797,6 @@ void register_console(struct console *newcon)
>   unregister_console(bcon);
>   }
>  }
> -EXPORT_SYMBOL(register_console);
> 
>  int unregister_console(struct cons

Re: [PATCH v2] console: Don't prefer first registered if DT specifies stdout-path

[Paul Burton]

On Sunday, 16 October 2016 20:07:18 BST Andreas Schwab wrote:
> On Aug 09 2016, Paul Burton <paul.bur...@imgtec.com> wrote:
> > Fix this by not automatically preferring the first registered console if
> > one is specified by the device tree. This allows consoles to be
> > registered but not enabled, and once the driver for the console selected
> > by stdout-path calls of_console_check() the driver will be added to the
> > list of preferred consoles before any other console has been enabled.
> > When that console is then registered via register_console() it will be
> > enabled as expected.
> 
> This breaks the console on PowerMac.  There is no output and it panics
> eventually.
> 
> Andreas.

Hi Andreas,

Could you share the device tree from your system?

Thanks,
Paul

signature.asc
Description: This is a digitally signed message part.

[PATCH 0/3] Resolve -Wattribute-alias warnings from SYSCALL_DEFINEx()

[Paul Burton]

This series introduces infrastructure allowing compiler diagnostics to
be disabled or their severity modified for specific pieces of code, with
suitable abstractions to prevent that code from becoming tied to a
specific compiler.

This infrastructure is then used to disable the -Wattribute-alias
warning around syscall definitions, which rely on type mismatches to
sanitize arguments.

Finally PowerPC-specific #pragma's are removed now that the generic code
is handling this.

The series takes Arnd's RFC patches & addresses the review comments they
received. The most notable effect of this series to to avoid warnings &
build failures caused by -Wattribute-alias when compiling the kernel
with GCC 8.

Applies cleanly atop master as of 9215310cf13b ("Merge
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net").

Thanks,
Paul

Arnd Bergmann (2):
  kbuild: add macro for controlling warnings to linux/compiler.h
  disable -Wattribute-alias warning for SYSCALL_DEFINEx()

Paul Burton (1):
  Revert "powerpc: fix build failure by disabling attribute-alias
warning in pci_32"

 arch/powerpc/kernel/pci_32.c   |  4 ---
 include/linux/compat.h |  8 -
 include/linux/compiler-gcc.h   | 66 ++
 include/linux/compiler_types.h | 18 ++
 include/linux/syscalls.h   |  4 +++
 5 files changed, 95 insertions(+), 5 deletions(-)

-- 
2.17.1

[PATCH 2/3] disable -Wattribute-alias warning for SYSCALL_DEFINEx()

[Paul Burton]

From: Arnd Bergmann 

gcc-8 warns for every single definition of a system call entry
point, e.g.:

include/linux/compat.h:56:18: error: 'compat_sys_rt_sigprocmask' alias between 
functions of incompatible types 'long int(int,  compat_sigset_t *, 
compat_sigset_t *, compat_size_t)' {aka 'long int(int,  struct  *, 
struct  *, unsigned int)'} and 'long int(long int,  long int,  long 
int,  long int)' [-Werror=attribute-alias]
  asmlinkage long compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))\
  ^~
include/linux/compat.h:45:2: note: in expansion of macro 
'COMPAT_SYSCALL_DEFINEx'
  COMPAT_SYSCALL_DEFINEx(4, _##name, __VA_ARGS__)
  ^~
kernel/signal.c:2601:1: note: in expansion of macro 'COMPAT_SYSCALL_DEFINE4'
 COMPAT_SYSCALL_DEFINE4(rt_sigprocmask, int, how, compat_sigset_t __user *, 
nset,
 ^~
include/linux/compat.h:60:18: note: aliased declaration here
  asmlinkage long compat_SyS##name(__MAP(x,__SC_LONG,__VA_ARGS__))\
  ^~

The new warning seems reasonable in principle, but it doesn't
help us here, since we rely on the type mismatch to sanitize the
system call arguments. After I reported this as GCC PR82435, a new
-Wno-attribute-alias option was added that could be used to turn the
warning off globally on the command line, but I'd prefer to do it a
little more fine-grained.

Interestingly, turning a warning off and on again inside of
a single macro doesn't always work, in this case I had to add
an extra statement inbetween and decided to copy the __SC_TEST
one from the native syscall to the compat syscall macro.  See
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83256 for more details
about this.

[paul.bur...@mips.com:
  - Rebase atop current master.
  - Split GCC & version arguments to __diag_ignore() in order to match
changes to the preceding patch.
  - Add the comment argument to match the preceding patch.]

Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82435
Signed-off-by: Arnd Bergmann 
Signed-off-by: Paul Burton 
Cc: Michal Marek 
Cc: Masahiro Yamada 
Cc: Douglas Anderson 
Cc: Al Viro 
Cc: Heiko Carstens 
Cc: Mauro Carvalho Chehab 
Cc: Matthew Wilcox 
Cc: Matthias Kaehlcke 
Cc: Arnd Bergmann 
Cc: Ingo Molnar 
Cc: Josh Poimboeuf 
Cc: Kees Cook 
Cc: Andrew Morton 
Cc: Thomas Gleixner 
Cc: Gideon Israel Dsouza 
Cc: Christophe Leroy 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: Stafford Horne 
Cc: Khem Raj 
Cc: He Zhe 
Cc: linux-kbu...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-m...@linux-mips.org
Cc: linuxppc-dev@lists.ozlabs.org
---

 include/linux/compat.h   | 8 +++-
 include/linux/syscalls.h | 4 
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/include/linux/compat.h b/include/linux/compat.h
index b1a5562b3215..c68acc47da57 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -72,6 +72,9 @@
  */
 #ifndef COMPAT_SYSCALL_DEFINEx
 #define COMPAT_SYSCALL_DEFINEx(x, name, ...)   
\
+   __diag_push();  
\
+   __diag_ignore(GCC, 8, "-Wattribute-alias",  
\
+ "Type aliasing is used to sanitize syscall arguments");\
asmlinkage long compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__));   
\
asmlinkage long compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))
\
__attribute__((alias(__stringify(__se_compat_sys##name; 
\
@@ -80,8 +83,11 @@
asmlinkage long __se_compat_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__));  
\
asmlinkage long __se_compat_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__))   
\
{   
\
-   return 
__do_compat_sys##name(__MAP(x,__SC_DELOUSE,__VA_ARGS__));\
+   long ret = 
__do_compat_sys##name(__MAP(x,__SC_DELOUSE,__VA_ARGS__));\
+   __MAP(x,__SC_TEST,__VA_ARGS__); 
\
+   return ret; 
\
}   
\
+   __diag_pop();   
\
static inline long __do_compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))
 #endif /* COMPAT_SYSCALL_DEFINEx */
 
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 73810808cdf2..a368a68cb667 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -231,6 +231,9 @@ static inline int is_syscall_trace_event(struct 
trace_event_call *tp_event)
  */
 #ifndef __SYSCALL_DEFINEx
 #define __SYSCALL_DEFINEx(x, name, ...)
\
+   __diag_push();  \
+   __diag_ignore(GCC, 8, "-Wattribute-alias",  \
+ 

[PATCH 3/3] Revert "powerpc: fix build failure by disabling attribute-alias warning in pci_32"

[Paul Burton]

With SYSCALL_DEFINEx() disabling -Wattribute-alias generically, there's
no need to duplicate that for PowerPC's pciconfig_iobase syscall.

This reverts commit 415520373975 ("powerpc: fix build failure by
disabling attribute-alias warning in pci_32").

Signed-off-by: Paul Burton 
Cc: Michal Marek 
Cc: Masahiro Yamada 
Cc: Douglas Anderson 
Cc: Al Viro 
Cc: Heiko Carstens 
Cc: Mauro Carvalho Chehab 
Cc: Matthew Wilcox 
Cc: Matthias Kaehlcke 
Cc: Arnd Bergmann 
Cc: Ingo Molnar 
Cc: Josh Poimboeuf 
Cc: Kees Cook 
Cc: Andrew Morton 
Cc: Thomas Gleixner 
Cc: Gideon Israel Dsouza 
Cc: Christophe Leroy 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: Stafford Horne 
Cc: Khem Raj 
Cc: He Zhe 
Cc: linux-kbu...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-m...@linux-mips.org
Cc: linuxppc-dev@lists.ozlabs.org

---

 arch/powerpc/kernel/pci_32.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/arch/powerpc/kernel/pci_32.c b/arch/powerpc/kernel/pci_32.c
index 4f861055a852..d63b488d34d7 100644
--- a/arch/powerpc/kernel/pci_32.c
+++ b/arch/powerpc/kernel/pci_32.c
@@ -285,9 +285,6 @@ pci_bus_to_hose(int bus)
  * Note that the returned IO or memory base is a physical address
  */
 
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wpragmas"
-#pragma GCC diagnostic ignored "-Wattribute-alias"
 SYSCALL_DEFINE3(pciconfig_iobase, long, which,
unsigned long, bus, unsigned long, devfn)
 {
@@ -313,4 +310,3 @@ SYSCALL_DEFINE3(pciconfig_iobase, long, which,
 
return result;
 }
-#pragma GCC diagnostic pop
-- 
2.17.1

[PATCH 1/3] kbuild: add macro for controlling warnings to linux/compiler.h

[Paul Burton]

From: Arnd Bergmann 

I have occasionally run into a situation where it would make sense to
control a compiler warning from a source file rather than doing so from
a Makefile using the $(cc-disable-warning, ...) or $(cc-option, ...)
helpers.

The approach here is similar to what glibc uses, using __diag() and
related macros to encapsulate a _Pragma("GCC diagnostic ...") statement
that gets turned into the respective "#pragma GCC diagnostic ..." by
the preprocessor when the macro gets expanded.

Like glibc, I also have an argument to pass the affected compiler
version, but decided to actually evaluate that one. For now, this
supports GCC_4_6, GCC_4_7, GCC_4_8, GCC_4_9, GCC_5, GCC_6, GCC_7,
GCC_8 and GCC_9. Adding support for CLANG_5 and other interesting
versions is straightforward here. GNU compilers starting with gcc-4.2
could support it in principle, but "#pragma GCC diagnostic push"
was only added in gcc-4.6, so it seems simpler to not deal with those
at all. The same versions show a large number of warnings already,
so it seems easier to just leave it at that and not do a more
fine-grained control for them.

The use cases I found so far include:

- turning off the gcc-8 -Wattribute-alias warning inside of the
  SYSCALL_DEFINEx() macro without having to do it globally.

- Reducing the build time for a simple re-make after a change,
  once we move the warnings from ./Makefile and
  ./scripts/Makefile.extrawarn into linux/compiler.h

- More control over the warnings based on other configurations,
  using preprocessor syntax instead of Makefile syntax. This should make
  it easier for the average developer to understand and change things.

- Adding an easy way to turn the W=1 option on unconditionally
  for a subdirectory or a specific file. This has been requested
  by several developers in the past that want to have their subsystems
  W=1 clean.

- Integrating clang better into the build systems. Clang supports
  more warnings than GCC, and we probably want to classify them
  as default, W=1, W=2 etc, but there are cases in which the
  warnings should be classified differently due to excessive false
  positives from one or the other compiler.

- Adding a way to turn the default warnings into errors (e.g. using
  a new "make E=0" tag) while not also turning the W=1 warnings into
  errors.

This patch for now just adds the minimal infrastructure in order to
do the first of the list above. As the #pragma GCC diagnostic
takes precedence over command line options, the next step would be
to convert a lot of the individual Makefiles that set nonstandard
options to use __diag() instead.

[paul.bur...@mips.com:
  - Rebase atop current master.
  - Add __diag_GCC, or more generally __diag_, abstraction to
avoid code outside of linux/compiler-gcc.h needing to duplicate
knowledge about different GCC versions.
  - Add a comment argument to __diag_{ignore,warn,error} which isn't
used in the expansion of the macros but serves to push people to
document the reason for using them - per feedback from Kees Cook.]

Signed-off-by: Arnd Bergmann 
Signed-off-by: Paul Burton 
Cc: Michal Marek 
Cc: Masahiro Yamada 
Cc: Douglas Anderson 
Cc: Al Viro 
Cc: Heiko Carstens 
Cc: Mauro Carvalho Chehab 
Cc: Matthew Wilcox 
Cc: Matthias Kaehlcke 
Cc: Arnd Bergmann 
Cc: Ingo Molnar 
Cc: Josh Poimboeuf 
Cc: Kees Cook 
Cc: Andrew Morton 
Cc: Thomas Gleixner 
Cc: Gideon Israel Dsouza 
Cc: Christophe Leroy 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: Stafford Horne 
Cc: Khem Raj 
Cc: He Zhe 
Cc: linux-kbu...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-m...@linux-mips.org
Cc: linuxppc-dev@lists.ozlabs.org
---

 include/linux/compiler-gcc.h   | 66 ++
 include/linux/compiler_types.h | 18 ++
 2 files changed, 84 insertions(+)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index f1a7492a5cc8..aba64a2912d8 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -347,3 +347,69 @@
 #if GCC_VERSION >= 50100
 #define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1
 #endif
+
+/*
+ * turn individual warnings and errors on and off locally, depending
+ * on version.
+ */
+#define __diag_GCC(version, s) __diag_GCC_ ## version(s)
+
+#if GCC_VERSION >= 40600
+#define __diag_str1(s) #s
+#define __diag_str(s) __diag_str1(s)
+#define __diag(s) _Pragma(__diag_str(GCC diagnostic s))
+
+/* compilers before gcc-4.6 do not understand "#pragma GCC diagnostic push" */
+#define __diag_GCC_4_6(s) __diag(s)
+#else
+#define __diag(s)
+#define __diag_GCC_4_6(s)
+#endif
+
+#if GCC_VERSION >= 40700
+#define __diag_GCC_4_7(s) __diag(s)
+#else
+#define __diag_GCC_4_7(s)
+#endif
+
+#if GCC_VERSION >= 40800
+#define __diag_GCC_4_8(s) __diag(s)
+#else
+#define __diag_GCC_4_8(s)
+#endif
+
+#if GCC_VERSION >= 40900
+#define __diag_GCC_4_9(s) __diag(s)
+#else
+#define __diag_GCC

Re: [PATCH 1/3] kbuild: add macro for controlling warnings to linux/compiler.h

[Paul Burton]

Hi Masahiro,

On Wed, Jun 20, 2018 at 02:34:35AM +0900, Masahiro Yamada wrote:
> 2018-06-16 9:53 GMT+09:00 Paul Burton :
> > diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> > index f1a7492a5cc8..aba64a2912d8 100644
> > --- a/include/linux/compiler-gcc.h
> > +++ b/include/linux/compiler-gcc.h
> > @@ -347,3 +347,69 @@
> >  #if GCC_VERSION >= 50100
> >  #define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1
> >  #endif
> > +
> > +/*
> > + * turn individual warnings and errors on and off locally, depending
> > + * on version.
> > + */
> > +#define __diag_GCC(version, s) __diag_GCC_ ## version(s)
> > +
> > +#if GCC_VERSION >= 40600
> > +#define __diag_str1(s) #s
> > +#define __diag_str(s) __diag_str1(s)
> > +#define __diag(s) _Pragma(__diag_str(GCC diagnostic s))
> > +
> > +/* compilers before gcc-4.6 do not understand "#pragma GCC diagnostic 
> > push" */
> > +#define __diag_GCC_4_6(s) __diag(s)
> > +#else
> > +#define __diag(s)
> > +#define __diag_GCC_4_6(s)
> > +#endif
> > +
> > +#if GCC_VERSION >= 40700
> > +#define __diag_GCC_4_7(s) __diag(s)
> > +#else
> > +#define __diag_GCC_4_7(s)
> > +#endif
> > +
> > +#if GCC_VERSION >= 40800
> > +#define __diag_GCC_4_8(s) __diag(s)
> > +#else
> > +#define __diag_GCC_4_8(s)
> > +#endif
> > +
> > +#if GCC_VERSION >= 40900
> > +#define __diag_GCC_4_9(s) __diag(s)
> > +#else
> > +#define __diag_GCC_4_9(s)
> > +#endif
> > +
> > +#if GCC_VERSION >= 5
> > +#define __diag_GCC_5(s) __diag(s)
> > +#else
> > +#define __diag_GCC_5(s)
> > +#endif
> > +
> > +#if GCC_VERSION >= 6
> > +#define __diag_GCC_6(s) __diag(s)
> > +#else
> > +#define __diag_GCC_6(s)
> > +#endif
> > +
> > +#if GCC_VERSION >= 7
> > +#define __diag_GCC_7(s) __diag(s)
> > +#else
> > +#define __diag_GCC_7(s)
> > +#endif
> > +
> > +#if GCC_VERSION >= 8
> > +#define __diag_GCC_8(s) __diag(s)
> > +#else
> > +#define __diag_GCC_8(s)
> > +#endif
> > +
> > +#if GCC_VERSION >= 9
> > +#define __diag_GCC_9(s) __diag(s)
> > +#else
> > +#define __diag_GCC_9(s)
> > +#endif
> 
> 
> Hmm, we would have to add this for every release.

Well, strictly speaking only ones that we need to modify diags for - ie.
in this series we could get away with only adding the GCC 8 macro if we
wanted.

> > diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> > index 6b79a9bba9a7..313a2ad884e0 100644
> > --- a/include/linux/compiler_types.h
> > +++ b/include/linux/compiler_types.h
> > @@ -271,4 +271,22 @@ struct ftrace_likely_data {
> >  # define __native_word(t) (sizeof(t) == sizeof(char) || sizeof(t) == 
> > sizeof(short) || sizeof(t) == sizeof(int) || sizeof(t) == sizeof(long))
> >  #endif
> >
> > +#ifndef __diag
> > +#define __diag(string)
> > +#endif
> > +
> > +#ifndef __diag_GCC
> > +#define __diag_GCC(string)
> > +#endif
> 
> __diag_GCC() takes two arguments,
> so it should be:
> 
> #ifndef __diag_GCC
> #define __diag_GCC(version, s)
> #endif
> 
> 
> Otherwise, this would cause warning like this:
> 
> 
> arch/arm64/kernel/sys.c:40:1: error: macro "__diag_GCC" passed 2
> arguments, but takes just 1
>  SYSCALL_DEFINE1(arm64_personality, unsigned int, personality)
>  ^~

Yes, good catch.

> > +#define __diag_push()  __diag(push)
> > +#define __diag_pop()   __diag(pop)
> > +
> > +#define __diag_ignore(compiler, version, option, comment) \
> > +   __diag_ ## compiler(version, ignored option)
> > +#define __diag_warn(compiler, version, option, comment) \
> > +   __diag_ ## compiler(version, warning option)
> > +#define __diag_error(compiler, version, option, comment) \
> > +   __diag_ ## compiler(version, error   option)
> > +
> 
> To me, it looks like this is putting GCC/Clang specific things
> in the common file,  .
> 
> All compilers must use "ignored", "warning", "error",
> not allowed to use "ignore".

We could move that to linux/compiler-gcc.h pretty easily.

> I also wonder if we could avoid proliferating __diag_GCC_*.

My thought is that it's unlikely we'll ever support enough different
compilers for it to become problematic to list the ones we modify
warnings for in linux/compiler_types.h.

> I attached a bit different implementation below.
> 
> I used -Wno-pragmas to avoid u

[PATCH v2 0/3] Resolve -Wattribute-alias warnings from SYSCALL_DEFINEx()

[Paul Burton]

This series introduces infrastructure allowing compiler diagnostics to
be disabled or their severity modified for specific pieces of code, with
suitable abstractions to prevent that code from becoming tied to a
specific compiler.

This infrastructure is then used to disable the -Wattribute-alias
warning around syscall definitions, which rely on type mismatches to
sanitize arguments.

Finally PowerPC-specific #pragma's are removed now that the generic code
is handling this.

The series takes Arnd's RFC patches & addresses the review comments they
received. The most notable effect of this series to to avoid warnings &
build failures caused by -Wattribute-alias when compiling the kernel
with GCC 8.

Applies cleanly atop v4.18-rc1.

Thanks,
Paul

Arnd Bergmann (2):
  kbuild: add macro for controlling warnings to linux/compiler.h
  disable -Wattribute-alias warning for SYSCALL_DEFINEx()

Paul Burton (1):
  powerpc: Remove -Wattribute-alias pragmas

 arch/powerpc/kernel/pci_32.c|  4 
 arch/powerpc/kernel/pci_64.c|  4 
 arch/powerpc/kernel/rtas.c  |  4 
 arch/powerpc/kernel/signal_32.c |  8 
 arch/powerpc/kernel/signal_64.c |  4 
 arch/powerpc/kernel/syscalls.c  |  4 
 arch/powerpc/mm/subpage-prot.c  |  4 
 include/linux/compat.h  |  8 +++-
 include/linux/compiler-gcc.h| 27 +++
 include/linux/compiler_types.h  | 18 ++
 include/linux/syscalls.h|  4 
 11 files changed, 56 insertions(+), 33 deletions(-)

-- 
2.17.1

[PATCH v2 1/3] kbuild: add macro for controlling warnings to linux/compiler.h

[Paul Burton]

From: Arnd Bergmann 

I have occasionally run into a situation where it would make sense to
control a compiler warning from a source file rather than doing so from
a Makefile using the $(cc-disable-warning, ...) or $(cc-option, ...)
helpers.

The approach here is similar to what glibc uses, using __diag() and
related macros to encapsulate a _Pragma("GCC diagnostic ...") statement
that gets turned into the respective "#pragma GCC diagnostic ..." by
the preprocessor when the macro gets expanded.

Like glibc, I also have an argument to pass the affected compiler
version, but decided to actually evaluate that one. For now, this
supports GCC_4_6, GCC_4_7, GCC_4_8, GCC_4_9, GCC_5, GCC_6, GCC_7,
GCC_8 and GCC_9. Adding support for CLANG_5 and other interesting
versions is straightforward here. GNU compilers starting with gcc-4.2
could support it in principle, but "#pragma GCC diagnostic push"
was only added in gcc-4.6, so it seems simpler to not deal with those
at all. The same versions show a large number of warnings already,
so it seems easier to just leave it at that and not do a more
fine-grained control for them.

The use cases I found so far include:

- turning off the gcc-8 -Wattribute-alias warning inside of the
  SYSCALL_DEFINEx() macro without having to do it globally.

- Reducing the build time for a simple re-make after a change,
  once we move the warnings from ./Makefile and
  ./scripts/Makefile.extrawarn into linux/compiler.h

- More control over the warnings based on other configurations,
  using preprocessor syntax instead of Makefile syntax. This should make
  it easier for the average developer to understand and change things.

- Adding an easy way to turn the W=1 option on unconditionally
  for a subdirectory or a specific file. This has been requested
  by several developers in the past that want to have their subsystems
  W=1 clean.

- Integrating clang better into the build systems. Clang supports
  more warnings than GCC, and we probably want to classify them
  as default, W=1, W=2 etc, but there are cases in which the
  warnings should be classified differently due to excessive false
  positives from one or the other compiler.

- Adding a way to turn the default warnings into errors (e.g. using
  a new "make E=0" tag) while not also turning the W=1 warnings into
  errors.

This patch for now just adds the minimal infrastructure in order to
do the first of the list above. As the #pragma GCC diagnostic
takes precedence over command line options, the next step would be
to convert a lot of the individual Makefiles that set nonstandard
options to use __diag() instead.

[paul.bur...@mips.com:
  - Rebase atop current master.
  - Add __diag_GCC, or more generally __diag_, abstraction to
avoid code outside of linux/compiler-gcc.h needing to duplicate
knowledge about different GCC versions.
  - Add a comment argument to __diag_{ignore,warn,error} which isn't
used in the expansion of the macros but serves to push people to
document the reason for using them - per feedback from Kees Cook.
  - Translate severity to GCC-specific pragmas in linux/compiler-gcc.h
rather than using GCC-specific in linux/compiler_types.h.
  - Drop all but GCC 8 macros, since we only need to define macros for
versions that we need to introduce pragmas for, and as of this
series that's just GCC 8.
  - Capitalize comments in linux/compiler-gcc.h to match the style of
the rest of the file.
  - Line up macro definitions with tabs in linux/compiler-gcc.h.]

Signed-off-by: Arnd Bergmann 
Signed-off-by: Paul Burton 
Tested-by: Christophe Leroy 
Tested-by: Stafford Horne 
Cc: Michal Marek 
Cc: Masahiro Yamada 
Cc: Douglas Anderson 
Cc: Al Viro 
Cc: Heiko Carstens 
Cc: Mauro Carvalho Chehab 
Cc: Matthew Wilcox 
Cc: Matthias Kaehlcke 
Cc: Arnd Bergmann 
Cc: Ingo Molnar 
Cc: Josh Poimboeuf 
Cc: Kees Cook 
Cc: Andrew Morton 
Cc: Thomas Gleixner 
Cc: Gideon Israel Dsouza 
Cc: Christophe Leroy 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: Stafford Horne 
Cc: Khem Raj 
Cc: He Zhe 
Cc: linux-kbu...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-m...@linux-mips.org
Cc: linuxppc-dev@lists.ozlabs.org

---

Changes in v2:
- Add version argument to fallback __diag_GCC definition.
- Translate severity from generic ignore,warn,error to GCC-specific
  pragma content ignored,warning,error in linux/compiler-gcc.h in order
  to keep linux/compiler_types.h generic per feedback from Masahiro
  Yamada.
- Drop all but GCC 8 macros, since we only need to define macros for
  versions that we need to introduce pragmas for, and as of this series
  that's just GCC 8.
- Capitalize comments in linux/compiler-gcc.h to match the style of the
  rest of the file.
- Line up macro definitions with tabs in linux/compiler-gcc.h.

 include/linux/compiler-gcc.h   | 27 +++
 include/linux/compiler_types.h | 18 ++
 2 fil

[PATCH v2 2/3] disable -Wattribute-alias warning for SYSCALL_DEFINEx()

[Paul Burton]

From: Arnd Bergmann 

gcc-8 warns for every single definition of a system call entry
point, e.g.:

include/linux/compat.h:56:18: error: 'compat_sys_rt_sigprocmask' alias between 
functions of incompatible types 'long int(int,  compat_sigset_t *, 
compat_sigset_t *, compat_size_t)' {aka 'long int(int,  struct  *, 
struct  *, unsigned int)'} and 'long int(long int,  long int,  long 
int,  long int)' [-Werror=attribute-alias]
  asmlinkage long compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))\
  ^~
include/linux/compat.h:45:2: note: in expansion of macro 
'COMPAT_SYSCALL_DEFINEx'
  COMPAT_SYSCALL_DEFINEx(4, _##name, __VA_ARGS__)
  ^~
kernel/signal.c:2601:1: note: in expansion of macro 'COMPAT_SYSCALL_DEFINE4'
 COMPAT_SYSCALL_DEFINE4(rt_sigprocmask, int, how, compat_sigset_t __user *, 
nset,
 ^~
include/linux/compat.h:60:18: note: aliased declaration here
  asmlinkage long compat_SyS##name(__MAP(x,__SC_LONG,__VA_ARGS__))\
  ^~

The new warning seems reasonable in principle, but it doesn't
help us here, since we rely on the type mismatch to sanitize the
system call arguments. After I reported this as GCC PR82435, a new
-Wno-attribute-alias option was added that could be used to turn the
warning off globally on the command line, but I'd prefer to do it a
little more fine-grained.

Interestingly, turning a warning off and on again inside of
a single macro doesn't always work, in this case I had to add
an extra statement inbetween and decided to copy the __SC_TEST
one from the native syscall to the compat syscall macro.  See
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83256 for more details
about this.

[paul.bur...@mips.com:
  - Rebase atop current master.
  - Split GCC & version arguments to __diag_ignore() in order to match
changes to the preceding patch.
  - Add the comment argument to match the preceding patch.]

Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82435
Signed-off-by: Arnd Bergmann 
Signed-off-by: Paul Burton 
Tested-by: Christophe Leroy 
Tested-by: Stafford Horne 
Cc: Michal Marek 
Cc: Masahiro Yamada 
Cc: Douglas Anderson 
Cc: Al Viro 
Cc: Heiko Carstens 
Cc: Mauro Carvalho Chehab 
Cc: Matthew Wilcox 
Cc: Matthias Kaehlcke 
Cc: Arnd Bergmann 
Cc: Ingo Molnar 
Cc: Josh Poimboeuf 
Cc: Kees Cook 
Cc: Andrew Morton 
Cc: Thomas Gleixner 
Cc: Gideon Israel Dsouza 
Cc: Christophe Leroy 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: Stafford Horne 
Cc: Khem Raj 
Cc: He Zhe 
Cc: linux-kbu...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-m...@linux-mips.org
Cc: linuxppc-dev@lists.ozlabs.org
---

Changes in v2: None

 include/linux/compat.h   | 8 +++-
 include/linux/syscalls.h | 4 
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/include/linux/compat.h b/include/linux/compat.h
index b1a5562b3215..c68acc47da57 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -72,6 +72,9 @@
  */
 #ifndef COMPAT_SYSCALL_DEFINEx
 #define COMPAT_SYSCALL_DEFINEx(x, name, ...)   
\
+   __diag_push();  
\
+   __diag_ignore(GCC, 8, "-Wattribute-alias",  
\
+ "Type aliasing is used to sanitize syscall arguments");\
asmlinkage long compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__));   
\
asmlinkage long compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))
\
__attribute__((alias(__stringify(__se_compat_sys##name; 
\
@@ -80,8 +83,11 @@
asmlinkage long __se_compat_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__));  
\
asmlinkage long __se_compat_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__))   
\
{   
\
-   return 
__do_compat_sys##name(__MAP(x,__SC_DELOUSE,__VA_ARGS__));\
+   long ret = 
__do_compat_sys##name(__MAP(x,__SC_DELOUSE,__VA_ARGS__));\
+   __MAP(x,__SC_TEST,__VA_ARGS__); 
\
+   return ret; 
\
}   
\
+   __diag_pop();   
\
static inline long __do_compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))
 #endif /* COMPAT_SYSCALL_DEFINEx */
 
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 73810808cdf2..a368a68cb667 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -231,6 +231,9 @@ static inline int is_syscall_trace_event(struct 
trace_event_call *tp_event)
  */
 #ifndef __SYSCALL_DEFINEx
 #define __SYSCALL_DEFINEx(x, name, ...)
\
+   __diag_push();  \
+   

[PATCH v2 3/3] powerpc: Remove -Wattribute-alias pragmas

[Paul Burton]

With SYSCALL_DEFINEx() disabling -Wattribute-alias generically, there's
no need to duplicate that for PowerPC syscalls.

This reverts commit 415520373975 ("powerpc: fix build failure by
disabling attribute-alias warning in pci_32") and commit 2479bfc9bc60
("powerpc: Fix build by disabling attribute-alias warning for
SYSCALL_DEFINEx").

Signed-off-by: Paul Burton 
Cc: Michal Marek 
Cc: Masahiro Yamada 
Cc: Douglas Anderson 
Cc: Al Viro 
Cc: Heiko Carstens 
Cc: Mauro Carvalho Chehab 
Cc: Matthew Wilcox 
Cc: Matthias Kaehlcke 
Cc: Arnd Bergmann 
Cc: Ingo Molnar 
Cc: Josh Poimboeuf 
Cc: Kees Cook 
Cc: Andrew Morton 
Cc: Thomas Gleixner 
Cc: Gideon Israel Dsouza 
Cc: Christophe Leroy 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: Stafford Horne 
Cc: Khem Raj 
Cc: He Zhe 
Cc: linux-kbu...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-m...@linux-mips.org
Cc: linuxppc-dev@lists.ozlabs.org

---
Michael & Christophe, I didn't add your acks here yet since it changed
to include the second revert that Christophe pointed out I'd missed & I
didn't want to presume your acks extended to that.

Changes in v2:
- Also revert 2479bfc9bc60 ("powerpc: Fix build by disabling
  attribute-alias warning for SYSCALL_DEFINEx").
- Change subject now that it's not just a simple one-commit revert.

 arch/powerpc/kernel/pci_32.c| 4 
 arch/powerpc/kernel/pci_64.c| 4 
 arch/powerpc/kernel/rtas.c  | 4 
 arch/powerpc/kernel/signal_32.c | 8 
 arch/powerpc/kernel/signal_64.c | 4 
 arch/powerpc/kernel/syscalls.c  | 4 
 arch/powerpc/mm/subpage-prot.c  | 4 
 7 files changed, 32 deletions(-)

diff --git a/arch/powerpc/kernel/pci_32.c b/arch/powerpc/kernel/pci_32.c
index 4f861055a852..d63b488d34d7 100644
--- a/arch/powerpc/kernel/pci_32.c
+++ b/arch/powerpc/kernel/pci_32.c
@@ -285,9 +285,6 @@ pci_bus_to_hose(int bus)
  * Note that the returned IO or memory base is a physical address
  */
 
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wpragmas"
-#pragma GCC diagnostic ignored "-Wattribute-alias"
 SYSCALL_DEFINE3(pciconfig_iobase, long, which,
unsigned long, bus, unsigned long, devfn)
 {
@@ -313,4 +310,3 @@ SYSCALL_DEFINE3(pciconfig_iobase, long, which,
 
return result;
 }
-#pragma GCC diagnostic pop
diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
index 812171c09f42..dff28f903512 100644
--- a/arch/powerpc/kernel/pci_64.c
+++ b/arch/powerpc/kernel/pci_64.c
@@ -203,9 +203,6 @@ void pcibios_setup_phb_io_space(struct pci_controller *hose)
 #define IOBASE_ISA_IO  3
 #define IOBASE_ISA_MEM 4
 
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wpragmas"
-#pragma GCC diagnostic ignored "-Wattribute-alias"
 SYSCALL_DEFINE3(pciconfig_iobase, long, which, unsigned long, in_bus,
  unsigned long, in_devfn)
 {
@@ -259,7 +256,6 @@ SYSCALL_DEFINE3(pciconfig_iobase, long, which, unsigned 
long, in_bus,
 
return -EOPNOTSUPP;
 }
-#pragma GCC diagnostic pop
 
 #ifdef CONFIG_NUMA
 int pcibus_to_node(struct pci_bus *bus)
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 7fb9f83dcde8..8afd146bc9c7 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -1051,9 +1051,6 @@ struct pseries_errorlog *get_pseries_errorlog(struct 
rtas_error_log *log,
 }
 
 /* We assume to be passed big endian arguments */
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wpragmas"
-#pragma GCC diagnostic ignored "-Wattribute-alias"
 SYSCALL_DEFINE1(rtas, struct rtas_args __user *, uargs)
 {
struct rtas_args args;
@@ -1140,7 +1137,6 @@ SYSCALL_DEFINE1(rtas, struct rtas_args __user *, uargs)
 
return 0;
 }
-#pragma GCC diagnostic pop
 
 /*
  * Call early during boot, before mem init, to retrieve the RTAS
diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 5eedbb282d42..e6474a45cef5 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -1038,9 +1038,6 @@ static int do_setcontext_tm(struct ucontext __user *ucp,
 }
 #endif
 
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wpragmas"
-#pragma GCC diagnostic ignored "-Wattribute-alias"
 #ifdef CONFIG_PPC64
 COMPAT_SYSCALL_DEFINE3(swapcontext, struct ucontext __user *, old_ctx,
   struct ucontext __user *, new_ctx, int, ctx_size)
@@ -1134,7 +1131,6 @@ SYSCALL_DEFINE3(swapcontext, struct ucontext __user *, 
old_ctx,
set_thread_flag(TIF_RESTOREALL);
return 0;
 }
-#pragma GCC diagnostic pop
 
 #ifdef CONFIG_PPC64
 COMPAT_SYSCALL_DEFINE0(rt_sigreturn)
@@ -1231,9 +1227,6 @@ SYSCALL_DEFINE0(rt_sigreturn)
return 0;
 }
 
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wpragmas"
-#pragma GCC diagnostic ignored "-

Re: [PATCH 1/3] kbuild: add macro for controlling warnings to linux/compiler.h

[Paul Burton]

Hi Masahiro,

On Thu, Jun 21, 2018 at 08:21:01AM +0900, Masahiro Yamada wrote:
> V2 is good except one nit.
> (I left a comment in it)

Thanks, and yes I agree with your comment that the GCC<4.6 __diag()
definition can be removed.

> I can fix it up locally if it is tedious to re-spin, though.

If you wouldn't mind that'd be great :)

Thanks,
Paul

Re: [PATCH v4 00/11] hugetlb: Factorize hugetlb architecture primitives

[Paul Burton]

Hi Alexandre,

On Thu, Jul 05, 2018 at 11:07:05AM +, Alexandre Ghiti wrote:
> In order to reduce copy/paste of functions across architectures and then
> make riscv hugetlb port (and future ports) simpler and smaller, this
> patchset intends to factorize the numerous hugetlb primitives that are
> defined across all the architectures.
> 
> Except for prepare_hugepage_range, this patchset moves the versions that
> are just pass-through to standard pte primitives into
> asm-generic/hugetlb.h by using the same #ifdef semantic that can be
> found in asm-generic/pgtable.h, i.e. __HAVE_ARCH_***.
> 
> s390 architecture has not been tackled in this serie since it does not
> use asm-generic/hugetlb.h at all.
> powerpc could be factorized a bit more (cf huge_ptep_set_wrprotect).
> 
> This patchset has been compiled on x86 only. 

For MIPS these look good - I don't see any issues & they pass a build
test (using cavium_octeon_defconfig which enables huge pages), so:

Acked-by: Paul Burton  # MIPS parts

Thanks,
Paul

Re: [PATCH v2 6/9] kbuild: consolidate Devicetree dtb build rules

[Paul Burton]

Hi Rob,

On Wed, Sep 05, 2018 at 06:53:24PM -0500, Rob Herring wrote:
> There is nothing arch specific about building dtb files other than their
> location under /arch/*/boot/dts/. Keeping each arch aligned is a pain.
> The dependencies and supported targets are all slightly different.
> Also, a cross-compiler for each arch is needed, but really the host
> compiler preprocessor is perfectly fine for building dtbs. Move the
> build rules to a common location and remove the arch specific ones. This
> is done in a single step to avoid warnings about overriding rules.
> 
> The build dependencies had been a mixture of 'scripts' and/or 'prepare'.
> These pull in several dependencies some of which need a target compiler
> (specifically devicetable-offsets.h) and aren't needed to build dtbs.
> All that is really needed is dtc, so adjust the dependencies to only be
> dtc.
> 
> This change enables support 'dtbs_install' on some arches which were
> missing the target.
> 
>%
> Signed-off-by: Rob Herring 
> ---
> Please ack so I can take the whole series via the DT tree.

For MIPS:

Acked-by: Paul Burton 

Thanks,
Paul

Re: [RFC PATCH] lib: Introduce generic __cmpxchg_u64() and use it where needed

[Paul Burton]

Hi Guenter,

On Wed, Oct 31, 2018 at 12:52:18PM -0700, Guenter Roeck wrote:
> +/*
> + * Generic version of __cmpxchg_u64, to be used for cmpxchg64().
> + * Takes u64 parameters.
> + */
> +u64 __cmpxchg_u64(u64 *ptr, u64 old, u64 new)
> +{
> + raw_spinlock_t *lock = lock_addr(ptr);
> + unsigned long flags;
> + u64 prev;
> +
> + raw_spin_lock_irqsave(lock, flags);
> + prev = READ_ONCE(*ptr);
> + if (prev == old)
> + *ptr = new;
> + raw_spin_unlock_irqrestore(lock, flags);
> +
> + return prev;
> +}
> +EXPORT_SYMBOL(__cmpxchg_u64);

This is only going to work if we know that memory modified using
__cmpxchg_u64() is *always* modified using __cmpxchg_u64(). Without that
guarantee there's nothing to stop some other CPU writing to *ptr after
the READ_ONCE() above but before we write new to it.

As far as I'm aware this is not a guarantee we currently provide, so it
would mean making that a requirement for cmpxchg64() users & auditing
them all. That would also leave cmpxchg64() with semantics that differ
from plain cmpxchg(), and semantics that may surprise people. In my view
that's probably not worth it, and it would be better to avoid using
cmpxchg64() on systems that can't properly support it.

For MIPS the problem will go away with the nanoMIPS ISA which includes &
requires LLWP/SCWP (W = word, P = paired) instructions that we can use
to implement cmpxchg64() properly, but of course that won't help older
systems.

Thanks,
Paul

Re: [RFC PATCH] lib: Introduce generic __cmpxchg_u64() and use it where needed

[Paul Burton]

(Copying SunRPC & net maintainers.)

Hi Guenter,

On Wed, Oct 31, 2018 at 03:02:53PM -0700, Guenter Roeck wrote:
> The alternatives I can see are
> - Do not use cmpxchg64() outside architecture code (ie drop its use from
>   the offending driver, and keep doing the same whenever the problem comes
>   up again).
> or
> - Introduce something like ARCH_HAS_CMPXCHG64 and use it to determine
>   if cmpxchg64 is supported or not.
> 
> Any preference ?

My preference would be option 1 - avoiding cmpxchg64() where possible in
generic code. I wouldn't be opposed to the Kconfig option if there are
cases where cmpxchg64() can really help performance though.

The last time I'm aware of this coming up the affected driver was
modified to avoid cmpxchg64() [1].

In this particular case I have no idea why
net/sunrpc/auth_gss/gss_krb5_seal.c is using cmpxchg64() at all. It's
essentially reinventing atomic64_fetch_inc() which is already provided
everywhere via CONFIG_GENERIC_ATOMIC64 & the spinlock approach. At least
for atomic64_* functions the assumption that all access will be
performed using those same functions seems somewhat reasonable.

So how does the below look? Trond?

Thanks,
Paul

[1] https://patchwork.ozlabs.org/cover/891284/

---
diff --git a/include/linux/sunrpc/gss_krb5.h b/include/linux/sunrpc/gss_krb5.h
index 131424cefc6a..02c0412e368c 100644
--- a/include/linux/sunrpc/gss_krb5.h
+++ b/include/linux/sunrpc/gss_krb5.h
@@ -107,8 +107,8 @@ struct krb5_ctx {
u8  Ksess[GSS_KRB5_MAX_KEYLEN]; /* session key */
u8  cksum[GSS_KRB5_MAX_KEYLEN];
s32 endtime;
-   u32 seq_send;
-   u64 seq_send64;
+   atomic_tseq_send;
+   atomic64_t  seq_send64;
struct xdr_netobj   mech_used;
u8  initiator_sign[GSS_KRB5_MAX_KEYLEN];
u8  acceptor_sign[GSS_KRB5_MAX_KEYLEN];
@@ -118,9 +118,6 @@ struct krb5_ctx {
u8  acceptor_integ[GSS_KRB5_MAX_KEYLEN];
 };
 
-extern u32 gss_seq_send_fetch_and_inc(struct krb5_ctx *ctx);
-extern u64 gss_seq_send64_fetch_and_inc(struct krb5_ctx *ctx);
-
 /* The length of the Kerberos GSS token header */
 #define GSS_KRB5_TOK_HDR_LEN   (16)
 
diff --git a/net/sunrpc/auth_gss/gss_krb5_mech.c 
b/net/sunrpc/auth_gss/gss_krb5_mech.c
index 7f0424dfa8f6..eab71fc7af3e 100644
--- a/net/sunrpc/auth_gss/gss_krb5_mech.c
+++ b/net/sunrpc/auth_gss/gss_krb5_mech.c
@@ -274,6 +274,7 @@ get_key(const void *p, const void *end,
 static int
 gss_import_v1_context(const void *p, const void *end, struct krb5_ctx *ctx)
 {
+   u32 seq_send;
int tmp;
 
p = simple_get_bytes(p, end, >initiate, sizeof(ctx->initiate));
@@ -315,9 +316,10 @@ gss_import_v1_context(const void *p, const void *end, 
struct krb5_ctx *ctx)
p = simple_get_bytes(p, end, >endtime, sizeof(ctx->endtime));
if (IS_ERR(p))
goto out_err;
-   p = simple_get_bytes(p, end, >seq_send, sizeof(ctx->seq_send));
+   p = simple_get_bytes(p, end, _send, sizeof(seq_send));
if (IS_ERR(p))
goto out_err;
+   atomic_set(>seq_send, seq_send);
p = simple_get_netobj(p, end, >mech_used);
if (IS_ERR(p))
goto out_err;
@@ -607,6 +609,7 @@ static int
 gss_import_v2_context(const void *p, const void *end, struct krb5_ctx *ctx,
gfp_t gfp_mask)
 {
+   u64 seq_send64;
int keylen;
 
p = simple_get_bytes(p, end, >flags, sizeof(ctx->flags));
@@ -617,14 +620,15 @@ gss_import_v2_context(const void *p, const void *end, 
struct krb5_ctx *ctx,
p = simple_get_bytes(p, end, >endtime, sizeof(ctx->endtime));
if (IS_ERR(p))
goto out_err;
-   p = simple_get_bytes(p, end, >seq_send64, sizeof(ctx->seq_send64));
+   p = simple_get_bytes(p, end, _send64, sizeof(seq_send64));
if (IS_ERR(p))
goto out_err;
+   atomic64_set(>seq_send64, seq_send64);
/* set seq_send for use by "older" enctypes */
-   ctx->seq_send = ctx->seq_send64;
-   if (ctx->seq_send64 != ctx->seq_send) {
-   dprintk("%s: seq_send64 %lx, seq_send %x overflow?\n", __func__,
-   (unsigned long)ctx->seq_send64, ctx->seq_send);
+   atomic_set(>seq_send, seq_send64);
+   if (seq_send64 != atomic_read(>seq_send)) {
+   dprintk("%s: seq_send64 %llx, seq_send %x overflow?\n", 
__func__,
+   seq_send64, atomic_read(>seq_send));
p = ERR_PTR(-EINVAL);
goto out_err;
}
diff --git a/net/sunrpc/auth_gss/gss_krb5_seal.c 
b/net/sunrpc/auth_gss/gss_krb5_seal.c
index b4adeb06660b..48fe4a591b54 100644
--- a/net/sunrpc/auth_gss/gss_krb5_seal.c
+++ b/net/sunrpc/auth_gss/gss_krb5_seal.c
@@ -123,30 +123,6 @@ setup_token_v2(struct krb5_ctx *ctx, struct xdr_netobj 

Re: [PATCH v2 1/2] kbuild: replace cc-name test with CONFIG_CC_IS_CLANG

[Paul Burton]

Hi Masahiro,

On Tue, Oct 30, 2018 at 10:26:33PM +0900, Masahiro Yamada wrote:
> Evaluating cc-name invokes the compiler every time even when you are
> not compiling anything, like 'make help'. This is not efficient.
> 
> The compiler type has been already detected in the Kconfig stage.
> Use CONFIG_CC_IS_CLANG, instead.
> 
> Signed-off-by: Masahiro Yamada 
> Acked-by: Michael Ellerman  (powerpc)

Looks good to me:

    Acked-by: Paul Burton  (MIPS)

Thanks,
Paul

Re: [RFC PATCH] lib: Introduce generic __cmpxchg_u64() and use it where needed

[Paul Burton]

Hi Trond,

On Thu, Nov 01, 2018 at 12:17:31AM +, Trond Myklebust wrote:
> On Wed, 2018-10-31 at 23:32 +0000, Paul Burton wrote:
> > In this particular case I have no idea why
> > net/sunrpc/auth_gss/gss_krb5_seal.c is using cmpxchg64() at all. It's
> > essentially reinventing atomic64_fetch_inc() which is already
> > provided
> > everywhere via CONFIG_GENERIC_ATOMIC64 & the spinlock approach. At
> > least
> > for atomic64_* functions the assumption that all access will be
> > performed using those same functions seems somewhat reasonable.
> > 
> > So how does the below look? Trond?
> 
> My one question (and the reason why I went with cmpxchg() in the first
> place) would be about the overflow behaviour for atomic_fetch_inc() and
> friends. I believe those functions should be OK on x86, so that when we
> overflow the counter, it behaves like an unsigned value and wraps back
> around.  Is that the case for all architectures?
> 
> i.e. are atomic_t/atomic64_t always guaranteed to behave like u32/u64
> on increment?
> 
> I could not find any documentation that explicitly stated that they
> should.

Based on other replies it seems like it's at least implicitly assumed by
other code, even if not explicitly stated.

>From a MIPS perspective where atomics are implemented using load-linked
& store-conditional instructions the actual addition will be performed
using the same addu instruction that a plain integer addition would
generate (regardless of signedness), so there'll be absolutely no
difference in arithmetic between your gss_seq_send64_fetch_and_inc()
function and atomic64_fetch_inc(). I'd expect the same to be true for
other architectures with load-linked & store-conditional style atomics.

In any case, for the benefit of anyone interested who I didn't copy on
the patch submission, here it is:

https://lore.kernel.org/lkml/20181101175109.8621-1-paul.bur...@mips.com/

Thanks,
Paul

Re: [PATCH 05/17] mips: Remove support for BZIP2 and LZMA compressed kernel

[Paul Burton]

Hi Adam,

On Fri, Nov 09, 2018 at 08:02:52PM +0100, Adam Borowski wrote:
> @@ -122,7 +104,6 @@ $(obj)/vmlinux.its.S: $(addprefix 
> $(srctree)/arch/mips/$(PLATFORM)/,$(ITS_INPUTS
>  
>  targets += vmlinux.its
>  targets += vmlinux.gz.its
> -targets += vmlinux.bz2.its
>  targets += vmlinux.lzmo.its
>  targets += vmlinux.lzo.its

It looks to me like this "vmlinux.lzmo.its" was a typo & ought to have
been vmlinux.lzma.its, and thus ought to be removed.

Apart from that I'm fine with this in general:

Acked-by: Paul Burton 

Thanks,
Paul

Re: [PATCH] memblock: stop using implicit alignement to SMP_CACHE_BYTES

[Paul Burton]

Hi Mike,

On Fri, Oct 05, 2018 at 12:07:04AM +0300, Mike Rapoport wrote:
> When a memblock allocation APIs are called with align = 0, the alignment is
> implicitly set to SMP_CACHE_BYTES.
> 
> Replace all such uses of memblock APIs with the 'align' parameter explicitly
> set to SMP_CACHE_BYTES and stop implicit alignment assignment in the
> memblock internal allocation functions.
> 
>%
> 
> Suggested-by: Michal Hocko 
> Signed-off-by: Mike Rapoport 

  Acked-by: Paul Burton  # MIPS part

Thanks,
Paul

Re: Checkpatch bad Warning (Re: [PATCH] powerpc/kgdb: add kgdb_arch_set/remove_breakpoint())

[Paul Burton]

Hi Christophe,

On Thu, Sep 20, 2018 at 01:23:55AM +, Christophe Leroy wrote:
> On 09/20/2018 01:19 PM, Christophe LEROY wrote:
> > Le 20/09/2018 à 15:13, Michael Ellerman a écrit :
> > > Joe Perches  writes:
> > > > On Tue, 2018-09-18 at 09:33 +, Christophe Leroy wrote:
> > > > > On the below patch, checkpatch reports
> > > > > 
> > > > > WARNING: struct kgdb_arch should normally be const
> > > > > #127: FILE: arch/powerpc/kernel/kgdb.c:480:
> > > > > +struct kgdb_arch arch_kgdb_ops;
> > > > > 
> > > > > But when I add 'const', I get compilation failure
> > > > 
> > > > So don't add const.
> > > > 
> > > > checkpatch is stupid.  You are not.
> > > > 
> > > > _Always_ take checkpatch bleats with very
> > > > large grains of salt.
> > > > 
> > > > Perhaps send a patch to remove kgbd_arch
> > > > from scripts/const_structs.checkpatch as
> > > > it seems not ever to be const.
> > > 
> > > I think it could/should be const though, it just requires updating all
> > > arches.
> > > 
> > 
> > Yes I was thinking about doing it, but first thing is to change the way
> > MIPS initialises it:
> > 
> > struct kgdb_arch arch_kgdb_ops;
> > 
> > int kgdb_arch_init(void)
> > {
> >  union mips_instruction insn = {
> >      .r_format = {
> >      .opcode = spec_op,
> >      .func    = break_op,
> >      }
> >  };
> >  memcpy(arch_kgdb_ops.gdb_bpt_instr, insn.byte, BREAK_INSTR_SIZE);
> > 
> > 
> > Can this be done staticaly ?

Something like this ought to do the trick:

diff --git a/arch/mips/kernel/kgdb.c b/arch/mips/kernel/kgdb.c
index eb6c0d582626..31eff1bec577 100644
--- a/arch/mips/kernel/kgdb.c
+++ b/arch/mips/kernel/kgdb.c
@@ -394,18 +394,16 @@ int kgdb_arch_handle_exception(int vector, int signo, int 
err_code,
return -1;
 }
 
-struct kgdb_arch arch_kgdb_ops;
+struct kgdb_arch arch_kgdb_ops = {
+#ifdef CONFIG_CPU_BIG_ENDIAN
+   .gdb_bpt_instr = { spec_op << 2, 0x00, 0x00, break_op },
+#else
+   .gdb_bpt_instr = { break_op, 0x00, 0x00, spec_op << 2 },
+#endif
+};
 
 int kgdb_arch_init(void)
 {
-   union mips_instruction insn = {
-   .r_format = {
-   .opcode = spec_op,
-   .func   = break_op,
-   }
-   };
-   memcpy(arch_kgdb_ops.gdb_bpt_instr, insn.byte, BREAK_INSTR_SIZE);
-
register_die_notifier(_notifier);
 
return 0;

Thanks,
Paul

Re: [PATCH 12/21] arch: use memblock_alloc() instead of memblock_alloc_from(size, align, 0)

[Paul Burton]

Hi Mike,

On Wed, Jan 16, 2019 at 03:44:12PM +0200, Mike Rapoport wrote:
> The last parameter of memblock_alloc_from() is the lower limit for the
> memory allocation. When it is 0, the call is equivalent to
> memblock_alloc().
> 
> Signed-off-by: Mike Rapoport 

Acked-by: Paul Burton  # MIPS part

Thanks,
Paul

Re: [PATCH 19/21] treewide: add checks for the return value of memblock_alloc*()

[Paul Burton]

Hi Mike,

On Wed, Jan 16, 2019 at 03:44:19PM +0200, Mike Rapoport wrote:
> Add check for the return value of memblock_alloc*() functions and call
> panic() in case of error.
> The panic message repeats the one used by panicing memblock allocators with
> adjustment of parameters to include only relevant ones.
> 
> The replacement was mostly automated with semantic patches like the one
> below with manual massaging of format strings.
> 
> @@
> expression ptr, size, align;
> @@
> ptr = memblock_alloc(size, align);
> + if (!ptr)
> + panic("%s: Failed to allocate %lu bytes align=0x%lx\n", __func__,
> size, align);
> 
> Signed-off-by: Mike Rapoport 
> ---
>%
> diff --git a/arch/mips/cavium-octeon/dma-octeon.c 
> b/arch/mips/cavium-octeon/dma-octeon.c
> index e8eb60e..db1deb2 100644
> --- a/arch/mips/cavium-octeon/dma-octeon.c
> +++ b/arch/mips/cavium-octeon/dma-octeon.c
> @@ -245,6 +245,9 @@ void __init plat_swiotlb_setup(void)
>   swiotlbsize = swiotlb_nslabs << IO_TLB_SHIFT;
>  
>   octeon_swiotlb = memblock_alloc_low(swiotlbsize, PAGE_SIZE);
> + if (!octeon_swiotlb)
> + panic("%s: Failed to allocate %lu bytes align=%lx\n",
> +   __func__, swiotlbsize, PAGE_SIZE);
>  
>   if (swiotlb_init_with_tbl(octeon_swiotlb, swiotlb_nslabs, 1) == -ENOMEM)
>   panic("Cannot allocate SWIOTLB buffer");

That one should be %zu rather than %lu. The rest looks good, so with
that one tweak:

Acked-by: Paul Burton  # MIPS parts

Thanks,
Paul

Re: [PATCH 1/2] mips/kgdb: prepare arch_kgdb_ops for constness

[Paul Burton]

Hi Christophe & Daniel,

On Thu, Dec 06, 2018 at 02:09:02PM +, Daniel Thompson wrote:
> On Wed, Dec 05, 2018 at 04:41:09AM +, Christophe Leroy wrote:
> > MIPS is the only architecture modifying arch_kgdb_ops during init.
> > This patch makes the init static, so that it can be changed to
> > const in following patch, as recommended by checkpatch.pl
> > 
> > Suggested-by: Paul Burton 
> > Signed-off-by: Christophe Leroy 
> 
> From my side this is
> Acked-by: Daniel Thompson 
> 
> Since this is a dependency for the next patch I'd be happy to take via
> my tree... but would need an ack from the MIPS guys to do that.

For both patches in this series:

Acked-by: Paul Burton 

Thanks,
Paul

Re: [PATCH v2 16/15] syscall_get_arch: add "struct task_struct *" argument

[Paul Burton]

Hi Dmitry,

On Wed, Nov 21, 2018 at 03:44:22AM +0300, Dmitry V. Levin wrote:
> This argument is required to extend the generic ptrace API
> with PTRACE_GET_SYSCALL_INFO request: syscall_get_arch() is going to be
> called from ptrace_request() along with other syscall_get_* functions
> with a tracee as their argument.
> 
> This change partially reverts commit 5e937a9ae913 ("syscall_get_arch:
> remove useless function arguments").
> 
>%
> 
> diff --git a/arch/mips/include/asm/syscall.h b/arch/mips/include/asm/syscall.h
> index 0170602a1e4e..52b633f20abd 100644
> --- a/arch/mips/include/asm/syscall.h
> +++ b/arch/mips/include/asm/syscall.h
> @@ -73,7 +73,7 @@ static inline unsigned long mips_get_syscall_arg(unsigned 
> long *arg,
>  #ifdef CONFIG_64BIT
>   case 4: case 5: case 6: case 7:
>  #ifdef CONFIG_MIPS32_O32
> - if (test_thread_flag(TIF_32BIT_REGS))
> + if (test_ti_thread_flag(task_thread_info(task), TIF_32BIT_REGS))
>   return get_user(*arg, (int *)usp + n);
>   else
>  #endif

This ought to be test_tsk_thread_flag(task, TIF_32BIT_REGS) instead of
open-coding test_tsk_thread_flag.

More fundamentally though, this change doesn't seem to be (directly)
related to the change you describe in the commit message - it's not
syscall_get_arch being modified here. I suspect this should be a
separate commit, or if not please explain in the commit message why this
change is included.

Compounding the lack of clarity is the fact that I only received this
patch, not the whole series, so I can't view the change in the context
of the rest of the series.

> @@ -140,14 +140,14 @@ extern const unsigned long sys_call_table[];
>  extern const unsigned long sys32_call_table[];
>  extern const unsigned long sysn32_call_table[];
>  
> -static inline int syscall_get_arch(void)
> +static inline int syscall_get_arch(struct task_struct *task)
>  {
>   int arch = AUDIT_ARCH_MIPS;
>  #ifdef CONFIG_64BIT
> - if (!test_thread_flag(TIF_32BIT_REGS)) {
> + if (!test_ti_thread_flag(task_thread_info(task), TIF_32BIT_REGS)) {
>   arch |= __AUDIT_ARCH_64BIT;
>   /* N32 sets only TIF_32BIT_ADDR */
> - if (test_thread_flag(TIF_32BIT_ADDR))
> + if (test_ti_thread_flag(task_thread_info(task), TIF_32BIT_ADDR))
>   arch |= __AUDIT_ARCH_CONVENTION_MIPS64_N32;
>   }
>  #endif

This does seem like the described change, but there are 2 more instances
of open-coding test_tsk_thread_flag which ought to be cleaned up.

Thanks,
Paul

Re: [PATCH v2 16/15 v2] syscall_get_arch: add "struct task_struct *" argument

[Paul Burton]

Hi Dmitry,

On Wed, Nov 21, 2018 at 10:35:12PM +0300, Dmitry V. Levin wrote:
> This argument is required to extend the generic ptrace API with
> PTRACE_GET_SYSCALL_INFO request: syscall_get_arch() is going to be
> called from ptrace_request() along with other syscall_get_* functions
> with a tracee as their argument.
> 
> This change partially reverts commit 5e937a9ae913 ("syscall_get_arch:
> remove useless function arguments").
> 
> Reviewed-by: Andy Lutomirski  # for x86
> Reviewed-by: Palmer Dabbelt 
> Cc: linux-au...@redhat.com
> Cc: linux-al...@vger.kernel.org
> Cc: linux-a...@vger.kernel.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-c6x-...@linux-c6x.org
> Cc: linux-hexa...@vger.kernel.org
> Cc: linux-i...@vger.kernel.org
> Cc: linux-m...@lists.linux-m68k.org
> Cc: linux-m...@linux-mips.org
> Cc: linux-par...@vger.kernel.org
> Cc: linux-ri...@lists.infradead.org
> Cc: linux-s...@vger.kernel.org
> Cc: linux...@vger.kernel.org
> Cc: linux-snps-...@lists.infradead.org
> Cc: linux...@lists.infradead.org
> Cc: linux-xte...@linux-xtensa.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: nios2-...@lists.rocketboards.org
> Cc: openr...@lists.librecores.org
> Cc: sparcli...@vger.kernel.org
> Cc: uclinux-h8-de...@lists.sourceforge.jp
> Cc: x...@kernel.org
> Signed-off-by: Dmitry V. Levin 
> ---
> 
> v2: cleaned up mips part, added Reviewed-by

I thought the last one was v2? :)

Anyway, this looks fine to me now:

Acked-by: Paul Burton  # MIPS parts

Thanks,
Paul

Re: [PATCH 3/9] MIPS: remove the HT_PCI config option

[Paul Burton]

Hi Christoph,

On Thu, Nov 15, 2018 at 08:05:31PM +0100, Christoph Hellwig wrote:
> This option is always selected from LOONGSON_MACH3X.  Switch to just
> seleting PCI from that option and definining LOONGSON_PCIIO_BASE based
> on CONFIG_LOONGSON_MACH3X.
> 
> Signed-off-by: Christoph Hellwig 
> ---
>  arch/mips/Kconfig| 11 ---
>  arch/mips/include/asm/mach-loongson64/loongson.h |  2 +-
>  arch/mips/loongson64/Kconfig |  2 +-
>  3 files changed, 2 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index 8272ea4c7264..7d28c9dd75d0 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -3040,17 +3040,6 @@ config PCI
> your box. Other bus systems are ISA, EISA, or VESA. If you have PCI,
> say Y, otherwise N.
>  
> -config HT_PCI
> - bool "Support for HT-linked PCI"
> - default y
> - depends on CPU_LOONGSON3
> - select PCI
> - select PCI_DOMAINS
> - help
> -   Loongson family machines use Hyper-Transport bus for inter-core
> -   connection and device connection. The PCI bus is a subordinate
> -   linked at HT. Choose Y for Loongson-3 based machines.
> -
>  config PCI_DOMAINS
>   bool
>  
> diff --git a/arch/mips/loongson64/Kconfig b/arch/mips/loongson64/Kconfig
> index c865b4b9b775..781a5156ab21 100644
> --- a/arch/mips/loongson64/Kconfig
> +++ b/arch/mips/loongson64/Kconfig
> @@ -76,7 +76,7 @@ config LOONGSON_MACH3X
>   select CPU_HAS_WB
>   select HW_HAS_PCI
>   select ISA
> - select HT_PCI
> + select PCI
>   select I8259
>   select IRQ_MIPS_CPU
>   select NR_CPUS_DEFAULT_4

Should this also select PCI_DOMAINS to preserve the existing behavior?

If not, could you explain why in the commit message?

Thanks,
Paul

Re: [PATCH 3/9] MIPS: remove the HT_PCI config option

[Paul Burton]

On Mon, Nov 19, 2018 at 01:01:41PM -0800, Paul Burton wrote:
> On Thu, Nov 15, 2018 at 08:05:31PM +0100, Christoph Hellwig wrote:
> > This option is always selected from LOONGSON_MACH3X.  Switch to just
> > seleting PCI from that option and definining LOONGSON_PCIIO_BASE based
> > on CONFIG_LOONGSON_MACH3X.
> > 
> > Signed-off-by: Christoph Hellwig 
> > ---
> >  arch/mips/Kconfig| 11 ---
> >  arch/mips/include/asm/mach-loongson64/loongson.h |  2 +-
> >  arch/mips/loongson64/Kconfig |  2 +-
> >  3 files changed, 2 insertions(+), 13 deletions(-)
> > 
> > diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> > index 8272ea4c7264..7d28c9dd75d0 100644
> > --- a/arch/mips/Kconfig
> > +++ b/arch/mips/Kconfig
> > @@ -3040,17 +3040,6 @@ config PCI
> >   your box. Other bus systems are ISA, EISA, or VESA. If you have PCI,
> >   say Y, otherwise N.
> >  
> > -config HT_PCI
> > -   bool "Support for HT-linked PCI"
> > -   default y
> > -   depends on CPU_LOONGSON3
> > -   select PCI
> > -   select PCI_DOMAINS
> > -   help
> > - Loongson family machines use Hyper-Transport bus for inter-core
> > - connection and device connection. The PCI bus is a subordinate
> > - linked at HT. Choose Y for Loongson-3 based machines.
> > -
> >  config PCI_DOMAINS
> > bool
> >  
> > diff --git a/arch/mips/loongson64/Kconfig b/arch/mips/loongson64/Kconfig
> > index c865b4b9b775..781a5156ab21 100644
> > --- a/arch/mips/loongson64/Kconfig
> > +++ b/arch/mips/loongson64/Kconfig
> > @@ -76,7 +76,7 @@ config LOONGSON_MACH3X
> > select CPU_HAS_WB
> > select HW_HAS_PCI
> > select ISA
> > -   select HT_PCI
> > +   select PCI
> > select I8259
> > select IRQ_MIPS_CPU
> > select NR_CPUS_DEFAULT_4
> 
> Should this also select PCI_DOMAINS to preserve the existing behavior?
> 
> If not, could you explain why in the commit message?

Ah, I see - PCI already selects PCI_DOMAINS. I think it would have been
worth mentioning but I don't mind if you don't think it a big enough
deal to respin the patch, so:

Acked-by: Paul Burton 

Thanks,
Paul

Re: [PATCH 4/9] PCI: consolidate PCI config entry in drivers/pci

[Paul Burton]

Hi Christoph,

On Thu, Nov 15, 2018 at 08:05:32PM +0100, Christoph Hellwig wrote:
> There is no good reason to duplicate the PCI menu in every architecture.
> Instead provide a selectable HAVE_PCI symbol that indicates availability
> of PCI support, and a FORCE_PCI symbol to for PCI on and the handle the
> rest in drivers/pci.
> 
> Signed-off-by: Christoph Hellwig 
> Reviewed-by: Palmer Dabbelt 
> Acked-by: Max Filippov 
> Acked-by: Thomas Gleixner 
> Acked-by: Bjorn Helgaas 
> Acked-by: Geert Uytterhoeven 

For patches 4, 5, 7, 8 & 9:

Acked-by: Paul Burton  # MIPS parts

Thanks,
Paul

Re: [PATCH 11/15] mips: fix n32 compat_ipc_parse_version

[Paul Burton]

Hi Arnd,

On Thu, Jan 10, 2019 at 05:24:31PM +0100, Arnd Bergmann wrote:
> While reading through the sysvipc implementation, I noticed that the n32
> semctl/shmctl/msgctl system calls behave differently based on whether
> o32 support is enabled or not: Without o32, the IPC_64 flag passed by
> user space is rejected but calls without that flag get IPC_64 behavior.
> 
> As far as I can tell, this was inadvertently changed by a cleanup patch
> but never noticed by anyone, possibly nobody has tried using sysvipc
> on n32 after linux-3.19.
> 
> Change it back to the old behavior now.
> 
> Fixes: 78aaf956ba3a ("MIPS: Compat: Fix build error if CONFIG_MIPS32_COMPAT 
> but no compat ABI.")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Arnd Bergmann 
> ---
> As stated above, this was only found by inspection, the patch is not
> tested. Please review accordingly.

Nice catch! Would you prefer to merge this yourself, or that I take it
through the mips tree?

If the former then:

  Acked-by: Paul Burton 

I suspect kernels configured with n32 support but no o32 support are
probably not very common - for internal testing we currently always
enable both.

Thanks,
Paul

> ---
>  arch/mips/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index 787290781b8c..0d14f51d0002 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -3155,6 +3155,7 @@ config MIPS32_O32
>  config MIPS32_N32
>   bool "Kernel support for n32 binaries"
>   depends on 64BIT
> + select ARCH_WANT_COMPAT_IPC_PARSE_VERSION
>   select COMPAT
>   select MIPS32_COMPAT
>   select SYSVIPC_COMPAT if SYSVIPC
> -- 
> 2.20.0
> 

Re: [PATCH 11/15] mips: fix n32 compat_ipc_parse_version

[Paul Burton]

Hello,

Arnd Bergmann wrote:
> While reading through the sysvipc implementation, I noticed that the n32
> semctl/shmctl/msgctl system calls behave differently based on whether
> o32 support is enabled or not: Without o32, the IPC_64 flag passed by
> user space is rejected but calls without that flag get IPC_64 behavior.
> 
> As far as I can tell, this was inadvertently changed by a cleanup patch
> but never noticed by anyone, possibly nobody has tried using sysvipc
> on n32 after linux-3.19.
> 
> Change it back to the old behavior now.
> 
> Fixes: 78aaf956ba3a ("MIPS: Compat: Fix build error if CONFIG_MIPS32_COMPAT 
> but no compat ABI.")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Arnd Bergmann 

Applied to mips-fixes.

Thanks,
Paul

[ This message was auto-generated; if you believe anything is incorrect
  then please email paul.bur...@mips.com to report it. ]

Re: [PATCH 5/6 v3] syscalls: Remove start and number from syscall_get_arguments() args

[Paul Burton]

Hi Steven,

On Mon, Apr 01, 2019 at 09:41:09AM -0400, Steven Rostedt wrote:
> From: "Steven Rostedt (Red Hat)" 
> 
> At Linux Plumbers, Andy Lutomirski approached me and pointed out that the
> function call syscall_get_arguments() implemented in x86 was horribly
> written and not optimized for the standard case of passing in 0 and 6 for
> the starting index and the number of system calls to get. When looking at
> all the users of this function, I discovered that all instances pass in only
> 0 and 6 for these arguments. Instead of having this function handle
> different cases that are never used, simply rewrite it to return the first 6
> arguments of a system call.
> 
> This should help out the performance of tracing system calls by ptrace,
> ftrace and perf.
> 
> Link: http://lkml.kernel.org/r/20161107213233.754809...@goodmis.org
> 
> Cc: Oleg Nesterov 
> Cc: Thomas Gleixner 
> Cc: Kees Cook 
> Cc: Andy Lutomirski 
> Cc: Dominik Brodowski 
> Cc: Dave Martin 
> Cc: "Dmitry V. Levin" 
> Cc: x...@kernel.org
> Cc: linux-snps-...@lists.infradead.org
> Cc: linux-ker...@vger.kernel.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-c6x-...@linux-c6x.org
> Cc: uclinux-h8-de...@lists.sourceforge.jp
> Cc: linux-hexa...@vger.kernel.org
> Cc: linux-i...@vger.kernel.org
> Cc: linux-m...@vger.kernel.org
> Cc: nios2-...@lists.rocketboards.org
> Cc: openr...@lists.librecores.org
> Cc: linux-par...@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-ri...@lists.infradead.org
> Cc: linux-s...@vger.kernel.org
> Cc: linux...@vger.kernel.org
> Cc: sparcli...@vger.kernel.org
> Cc: linux...@lists.infradead.org
> Cc: linux-xte...@linux-xtensa.org
> Cc: linux-a...@vger.kernel.org
> Reported-by: Andy Lutomirski 
> Signed-off-by: Steven Rostedt (VMware) 

Acked-by: Paul Burton  # MIPS parts

Thanks,
Paul

Re: [PATCH 2/2] arch: add pidfd and io_uring syscalls everywhere

[Paul Burton]

Hi Arnd,

On Mon, Mar 25, 2019 at 03:47:37PM +0100, Arnd Bergmann wrote:
> Add the io_uring and pidfd_send_signal system calls to all architectures.
> 
> These system calls are designed to handle both native and compat tasks,
> so all entries are the same across architectures, only arm-compat and
> the generic tale still use an old format.
> 
> Signed-off-by: Arnd Bergmann 
> ---
>%
> diff --git a/arch/mips/kernel/syscalls/syscall_n64.tbl 
> b/arch/mips/kernel/syscalls/syscall_n64.tbl
> index c85502e67b44..c4a49f7d57bb 100644
> --- a/arch/mips/kernel/syscalls/syscall_n64.tbl
> +++ b/arch/mips/kernel/syscalls/syscall_n64.tbl
> @@ -338,3 +338,7 @@
>  327  n64 rseqsys_rseq
>  328  n64 io_pgetevents   sys_io_pgetevents
>  # 329 through 423 are reserved to sync up with other architectures
> +424  common  pidfd_send_signal   sys_pidfd_send_signal
> +425  common  io_uring_setup  sys_io_uring_setup
> +426  common  io_uring_enter  sys_io_uring_enter
> +427  common  io_uring_register   sys_io_uring_register

Shouldn't these declare the ABI as "n64"?

I don't see anywhere that it would actually change the generated code,
but a comment at the top of the file says that every entry should use
"n64" and so far they all do. Did you have something else in mind here?

Thanks,
Paul

Re: [PATCH 06/12] dma-mapping: improve selection of dma_declare_coherent availability

[Paul Burton]

Hi Christoph,

On Mon, Feb 11, 2019 at 02:35:48PM +0100, Christoph Hellwig wrote:
> This API is primarily used through DT entries, but two architectures
> and two drivers call it directly.  So instead of selecting the config
> symbol for random architectures pull it in implicitly for the actual
> users.  Also rename the Kconfig option to describe the feature better.
> 
> Signed-off-by: Christoph Hellwig 

Acked-by: Paul Burton  # MIPS

Thanks,
Paul

Re: [PATCH] [v2] arch: add pidfd and io_uring syscalls everywhere

[Paul Burton]

Hi Arnd,

On Mon, Apr 15, 2019 at 04:22:57PM +0200, Arnd Bergmann wrote:
> Add the io_uring and pidfd_send_signal system calls to all architectures.
> 
> These system calls are designed to handle both native and compat tasks,
> so all entries are the same across architectures, only arm-compat and
> the generic tale still use an old format.
> 
> Acked-by: Michael Ellerman  (powerpc)
> Acked-by: Heiko Carstens  (s390)
> Acked-by: Geert Uytterhoeven 
> Signed-off-by: Arnd Bergmann 
> ---
> Changes since v1:
> - fix s390 table
> - use 'n64' tag in mips-n64 instead of common.

Thanks for taking care of this:

  Acked-by: Paul Burton  # MIPS

Thanks,
Paul

Re: [PATCH 5/5] asm-generic: remove ptrace.h

[Paul Burton]

On Mon, Jun 24, 2019 at 07:47:28AM +0200, Christoph Hellwig wrote:
> No one is using this header anymore.
> 
> Signed-off-by: Christoph Hellwig 
> Acked-by: Arnd Bergmann 
> Acked-by: Oleg Nesterov 
> ---
>  MAINTAINERS|  1 -
>  arch/mips/include/asm/ptrace.h |  5 ---
>  include/asm-generic/ptrace.h   | 73 --
>  3 files changed, 79 deletions(-)
>  delete mode 100644 include/asm-generic/ptrace.h

FWIW:

Acked-by: Paul Burton 

Thanks,
Paul

> diff --git a/arch/mips/include/asm/ptrace.h b/arch/mips/include/asm/ptrace.h
> index b6578611dddb..1e76774b36dd 100644
> --- a/arch/mips/include/asm/ptrace.h
> +++ b/arch/mips/include/asm/ptrace.h
> @@ -56,11 +56,6 @@ static inline unsigned long kernel_stack_pointer(struct 
> pt_regs *regs)
>   return regs->regs[31];
>  }
>  
> -/*
> - * Don't use asm-generic/ptrace.h it defines FP accessors that don't make
> - * sense on MIPS.  We rather want an error if they get invoked.
> - */
> -
>  static inline void instruction_pointer_set(struct pt_regs *regs,
> unsigned long val)
>  {

Re: [PATCH 01/15] asm-generic, x86: introduce generic pte_{alloc,free}_one[_kernel]

[Paul Burton]

Hi Mike,

On Thu, May 02, 2019 at 06:28:28PM +0300, Mike Rapoport wrote:
> +/**
> + * pte_free_kernel - free PTE-level user page table page
> + * @mm: the mm_struct of the current context
> + * @pte_page: the `struct page` representing the page table
> + */
> +static inline void pte_free(struct mm_struct *mm, struct page *pte_page)
> +{
> + pgtable_page_dtor(pte_page);
> + __free_page(pte_page);
> +}

Nit: the comment names the wrong function (s/pte_free_kernel/pte_free/).

Thanks,
Paul

Re: [PATCH 08/15] mips: switch to generic version of pte allocation

[Paul Burton]

Hi Mike,

On Thu, May 02, 2019 at 06:28:35PM +0300, Mike Rapoport wrote:
> MIPS allocates kernel PTE pages with
> 
>   __get_free_pages(GFP_KERNEL | __GFP_ZERO, PTE_ORDER)
> 
> and user PTE pages with
> 
>   alloc_pages(GFP_KERNEL | __GFP_ZERO, PTE_ORDER)

That bit isn't quite true - we don't use __GFP_ZERO in pte_alloc_one() &
instead call clear_highpage() on the allocated page. Not that I have a
problem with using __GFP_ZERO - it seems like the more optimal choice.
It just might be worth mentioning the change & expected equivalent
behavior.

Otherwise:

Acked-by: Paul Burton 

Thanks,
Paul

Re: [PATCH 6/6] MIPS: document mixing "slightly different CCAs"

[Paul Burton]

Hi Christoph,

On Mon, Aug 26, 2019 at 03:25:53PM +0200, Christoph Hellwig wrote:
> Based on an email from Paul Burton, quoting section 4.8 "Cacheability and
> Coherency Attributes and Access Types" of "MIPS Architecture Volume 1:
> Introduction to the MIPS32 Architecture" (MD00080, revision 6.01).
> 
> Signed-off-by: Christoph Hellwig 

Acked-by: Paul Burton 

Thanks,
Paul

> ---
>  arch/mips/Kconfig | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index fc88f68ea1ee..aff1cadeea43 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -1119,6 +1119,13 @@ config DMA_PERDEV_COHERENT
>  
>  config DMA_NONCOHERENT
>   bool
> + #
> + # MIPS allows mixing "slightly different" Cacheability and Coherency
> + # Attribute bits.  It is believed that the uncached access through
> + # KSEG1 and the implementation specific "uncached accelerated" used
> + # by pgprot_writcombine can be mixed, and the latter sometimes provides
> + # significant advantages.
> + #
>   select ARCH_HAS_DMA_WRITE_COMBINE
>   select ARCH_HAS_SYNC_DMA_FOR_DEVICE
>   select ARCH_HAS_UNCACHED_SEGMENT
> -- 
> 2.20.1
> 

Re: [PATCH 3/6] dma-mapping: remove arch_dma_mmap_pgprot

[Paul Burton]

Hi Christoph,

On Mon, Aug 26, 2019 at 03:25:50PM +0200, Christoph Hellwig wrote:
> arch_dma_mmap_pgprot is used for two things:
> 
>  1) to override the "normal" uncached page attributes for mapping
> memory coherent to devices that can't snoop the CPU caches
>  2) to provide the special DMA_ATTR_WRITE_COMBINE semantics on older
> arm systems and some mips platforms
> 
> Replace one with the pgprot_dmacoherent macro that is already provided
> by arm and much simpler to use, and lift the DMA_ATTR_WRITE_COMBINE
> handling to common code with an explicit arch opt-in.
> 
> Signed-off-by: Christoph Hellwig 
> Acked-by: Geert Uytterhoeven 

For the MIPS bits:

Acked-by: Paul Burton 

Thanks,
Paul

> ---
>  arch/arm/Kconfig   |  2 +-
>  arch/arm/mm/dma-mapping.c  |  6 --
>  arch/arm64/Kconfig |  1 -
>  arch/arm64/include/asm/pgtable.h   |  4 
>  arch/arm64/mm/dma-mapping.c|  6 --
>  arch/m68k/Kconfig  |  1 -
>  arch/m68k/include/asm/pgtable_mm.h |  3 +++
>  arch/m68k/kernel/dma.c |  3 +--
>  arch/mips/Kconfig  |  2 +-
>  arch/mips/mm/dma-noncoherent.c |  8 
>  include/linux/dma-noncoherent.h| 13 +++--
>  kernel/dma/Kconfig | 12 +---
>  kernel/dma/mapping.c   |  8 +---
>  13 files changed, 35 insertions(+), 34 deletions(-)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 24360211534a..217083caeabd 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -8,7 +8,7 @@ config ARM
>   select ARCH_HAS_DEBUG_VIRTUAL if MMU
>   select ARCH_HAS_DEVMEM_IS_ALLOWED
>   select ARCH_HAS_DMA_COHERENT_TO_PFN if SWIOTLB
> - select ARCH_HAS_DMA_MMAP_PGPROT if SWIOTLB
> + select ARCH_HAS_DMA_WRITE_COMBINE if !ARM_DMA_MEM_BUFFERABLE
>   select ARCH_HAS_ELF_RANDOMIZE
>   select ARCH_HAS_FORTIFY_SOURCE
>   select ARCH_HAS_KEEPINITRD
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index d42557ee69c2..d27b12f61737 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -2402,12 +2402,6 @@ long arch_dma_coherent_to_pfn(struct device *dev, void 
> *cpu_addr,
>   return dma_to_pfn(dev, dma_addr);
>  }
>  
> -pgprot_t arch_dma_mmap_pgprot(struct device *dev, pgprot_t prot,
> - unsigned long attrs)
> -{
> - return __get_dma_pgprot(attrs, prot);
> -}
> -
>  void *arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *dma_handle,
>   gfp_t gfp, unsigned long attrs)
>  {
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 3adcec05b1f6..dab9dda34206 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -13,7 +13,6 @@ config ARM64
>   select ARCH_HAS_DEBUG_VIRTUAL
>   select ARCH_HAS_DEVMEM_IS_ALLOWED
>   select ARCH_HAS_DMA_COHERENT_TO_PFN
> - select ARCH_HAS_DMA_MMAP_PGPROT
>   select ARCH_HAS_DMA_PREP_COHERENT
>   select ARCH_HAS_ACPI_TABLE_UPGRADE if ACPI
>   select ARCH_HAS_ELF_RANDOMIZE
> diff --git a/arch/arm64/include/asm/pgtable.h 
> b/arch/arm64/include/asm/pgtable.h
> index e09760ece844..6700371227d1 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -435,6 +435,10 @@ static inline pmd_t pmd_mkdevmap(pmd_t pmd)
>   __pgprot_modify(prot, PTE_ATTRINDX_MASK, PTE_ATTRINDX(MT_NORMAL_NC) | 
> PTE_PXN | PTE_UXN)
>  #define pgprot_device(prot) \
>   __pgprot_modify(prot, PTE_ATTRINDX_MASK, PTE_ATTRINDX(MT_DEVICE_nGnRE) 
> | PTE_PXN | PTE_UXN)
> +#define pgprot_dmacoherent(prot) \
> + __pgprot_modify(prot, PTE_ATTRINDX_MASK, \
> + PTE_ATTRINDX(MT_NORMAL_NC) | PTE_PXN | PTE_UXN)
> +
>  #define __HAVE_PHYS_MEM_ACCESS_PROT
>  struct file;
>  extern pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
> index bd2b039f43a6..676efcda51e6 100644
> --- a/arch/arm64/mm/dma-mapping.c
> +++ b/arch/arm64/mm/dma-mapping.c
> @@ -11,12 +11,6 @@
>  
>  #include 
>  
> -pgprot_t arch_dma_mmap_pgprot(struct device *dev, pgprot_t prot,
> - unsigned long attrs)
> -{
> - return pgprot_writecombine(prot);
> -}
> -
>  void arch_sync_dma_for_device(struct device *dev, phys_addr_t paddr,
>   size_t size, enum dma_data_direction dir)
>  {
> diff --git a/arch/m68k/Kconfig b/arch/m68k/Kconfig
> index c518d695c376..a9e564306d3e 100644
> --- a/arch/m68k/Kconfig
> +++ b/arch/m68k/Kconfig
> @@ -4,7 +4,6 @@ config M68K
>   default y
>   select ARCH_32BIT_OFF_T
>  

Re: [PATCH v2 8/9] mips: numa: check the node id consistently for mips ip27

[Paul Burton]

Hi Yunsheng,

On Sat, Aug 31, 2019 at 01:58:22PM +0800, Yunsheng Lin wrote:
> According to Section 6.2.14 from ACPI spec 6.3 [1], the setting
> of proximity domain is optional, as below:
> 
> This optional object is used to describe proximity domain
> associations within a machine. _PXM evaluates to an integer
> that identifies a device as belonging to a Proximity Domain
> defined in the System Resource Affinity Table (SRAT).
> 
> Since mips ip27 uses hub_data instead of node_to_cpumask_map,
> this patch checks node id with the below case before returning
> _data(node)->h_cpus:
> 1. if node_id >= MAX_COMPACT_NODES, return cpu_none_mask
> 2. if node_id < 0, return cpu_online_mask
> 3. if hub_data(node) is NULL, return cpu_online_mask
> 
> [1] https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf

Similar to David's comment on the sparc patch, these systems don't use
ACPI so I don't see from your commit message why this change would be
relevant.

This same comment applies to patch 9 too.

Thanks,
Paul

> 
> Signed-off-by: Yunsheng Lin 
> ---
>  arch/mips/include/asm/mach-ip27/topology.h | 15 ---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/mips/include/asm/mach-ip27/topology.h 
> b/arch/mips/include/asm/mach-ip27/topology.h
> index 965f079..914a55a 100644
> --- a/arch/mips/include/asm/mach-ip27/topology.h
> +++ b/arch/mips/include/asm/mach-ip27/topology.h
> @@ -15,9 +15,18 @@ struct cpuinfo_ip27 {
>  extern struct cpuinfo_ip27 sn_cpu_info[NR_CPUS];
>  
>  #define cpu_to_node(cpu) (sn_cpu_info[(cpu)].p_nodeid)
> -#define cpumask_of_node(node)((node) == -1 ? 
> \
> -  cpu_all_mask : \
> -  _data(node)->h_cpus)
> +
> +static inline const struct cpumask *cpumask_of_node(int node)
> +{
> + if (node >= MAX_COMPACT_NODES)
> + return cpu_none_mask;
> +
> + if (node < 0 || !hub_data(node))
> + return cpu_online_mask;
> +
> + return _data(node)->h_cpus;
> +}
> +
>  struct pci_bus;
>  extern int pcibus_to_node(struct pci_bus *);
>  
> -- 
> 2.8.1
> 

Re: cleanup the dma_pgprot handling

[Paul Burton]

Hi Christoph,

On Fri, Aug 16, 2019 at 09:07:48AM +0200, Christoph Hellwig wrote:
> I'd still like to hear a confirmation from the mips folks how
> the write combibe attribute can or can't work with the KSEG1
> uncached segment.

Quoting section 4.8 "Cacheability and Coherency Attributes and Access
Types" of "MIPS Architecture Volume 1: Introduction to the MIPS32
Architecture" (MD00080, revision 6.01):

https://www.mips.com/?do-download=introduction-to-the-mips32-architecture-v6-01

> Memory access types are specified by architecturally-defined and
> implementation-specific Cacheability and Coherency Attribute bits
> (CCAs) generated by the MMU for the access.
>
> Slightly different cacheability and coherency attributes such as
> “cached coherent, update on write” and “cached coherent, exclusive on
> write” can map to the same memory access type; in this case they both
> map to cached coherent. In order to map to the same access type, the
> fundamental mechanisms of both CCAs must be the same.
>
> When the operation of the instruction is affected, the instructions
> are described in terms of memory access types. The load and store
> operations in a processor proceed according to the specific CCA of the
> reference, however, and the pseudocode for load and store common
> functions uses the CCA value rather than the corresponding memory
> access type.

So I believe uncached & uncached accelerated are another case like that
described above - they're 2 different CCAs but the same "access type",
namely uncached.

Section 4.9 then goes on to forbid mixing access types, but not CCAs.

It would be nice if the precise mapping from CCA to access type was
provided, but I don't see that anywhere. I can check with the
architecture team to be sure, but to my knowledge we're fine to mix
access via kseg1 (ie. uncached) & mappings with CCA=7 (uncached
accelerated).

Thanks,
Paul

Re: [EXTERNAL][PATCH 1/5] PCI: Convert pci_resource_to_user to a weak function

[Paul Burton]

Hi Denis,

On Sun, Jul 28, 2019 at 11:22:09PM +0300, Denis Efremov wrote:
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 9e700d9f9f28..1a19d0151b0a 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1870,25 +1870,13 @@ static inline const char *pci_name(const struct 
> pci_dev *pdev)
>   return dev_name(>dev);
>  }
>  
> -
>  /*
>   * Some archs don't want to expose struct resource to userland as-is
>   * in sysfs and /proc
>   */
> -#ifdef HAVE_ARCH_PCI_RESOURCE_TO_USER
> -void pci_resource_to_user(const struct pci_dev *dev, int bar,
> -   const struct resource *rsrc,
> -   resource_size_t *start, resource_size_t *end);
> -#else
> -static inline void pci_resource_to_user(const struct pci_dev *dev, int bar,
> - const struct resource *rsrc, resource_size_t *start,
> - resource_size_t *end)
> -{
> - *start = rsrc->start;
> - *end = rsrc->end;
> -}
> -#endif /* HAVE_ARCH_PCI_RESOURCE_TO_USER */
> -
> +void __weak pci_resource_to_user(const struct pci_dev *dev, int bar,
> +  const struct resource *rsrc,
> +  resource_size_t *start, resource_size_t *end);
>  
>  /*
>   * The world is not perfect and supplies us with broken PCI devices.

This is wrong - using __weak on the declaration in a header will cause
the weak attribute to be applied to all implementations too (presuming
the C files containing the implementations include the header). You then
get whichever impleentation the linker chooses, which isn't necessarily
the one you wanted.

checkpatch.pl should produce an error about this - see the
WEAK_DECLARATION error introduced in commit 619a908aa334 ("checkpatch:
add error on use of attribute((weak)) or __weak declarations").

Thanks,
Paul

Re: [PATCH v6] numa: make node_to_cpumask_map() NUMA_NO_NODE aware

[Paul Burton]

Hi Yunsheng,

On Tue, Sep 17, 2019 at 08:48:54PM +0800, Yunsheng Lin wrote:
> When passing the return value of dev_to_node() to cpumask_of_node()
> without checking if the device's node id is NUMA_NO_NODE, there is
> global-out-of-bounds detected by KASAN.
> 
> From the discussion [1], NUMA_NO_NODE really means no node affinity,
> which also means all cpus should be usable. So the cpumask_of_node()
> should always return all cpus online when user passes the node id as
> NUMA_NO_NODE, just like similar semantic that page allocator handles
> NUMA_NO_NODE.
> 
> But we cannot really copy the page allocator logic. Simply because the
> page allocator doesn't enforce the near node affinity. It just picks it
> up as a preferred node but then it is free to fallback to any other numa
> node. This is not the case here and node_to_cpumask_map will only restrict
> to the particular node's cpus which would have really non deterministic
> behavior depending on where the code is executed. So in fact we really
> want to return cpu_online_mask for NUMA_NO_NODE.
> 
> Also there is a debugging version of node_to_cpumask_map() for x86 and
> arm64, which is only used when CONFIG_DEBUG_PER_CPU_MAPS is defined, this
> patch changes it to handle NUMA_NO_NODE as normal node_to_cpumask_map().
> 
> [1] https://lore.kernel.org/patchwork/patch/1125789/
> Signed-off-by: Yunsheng Lin 
> Suggested-by: Michal Hocko 
> Acked-by: Michal Hocko 

If you end up sending another revision then I think it would be worth
replacing -1 with NUMA_NO_NODE in
arch/mips/include/asm/mach-ip27/topology.h for consistency, but in any
case:

Acked-by: Paul Burton  # MIPS bits

Thanks,
Paul

> ---
> V6: Drop the cpu_all_mask -> cpu_online_mask change for it seems a
> little controversial, may need deeper investigation, and rebased
> on the latest linux-next.
> V5: Drop unsigned "fix" change for x86/arm64, and change comment log
> according to Michal's comment.
> V4: Have all these changes in a single patch.
> V3: Change to only handle NUMA_NO_NODE, and return cpu_online_mask
> for NUMA_NO_NODE case, and change the commit log to better justify
> the change.
> V2: make the node id checking change to other arches too.
> ---
>  arch/arm64/include/asm/numa.h| 3 +++
>  arch/arm64/mm/numa.c | 3 +++
>  arch/mips/include/asm/mach-loongson64/topology.h | 4 +++-
>  arch/s390/include/asm/topology.h | 3 +++
>  arch/x86/include/asm/topology.h  | 3 +++
>  arch/x86/mm/numa.c   | 3 +++
>  6 files changed, 18 insertions(+), 1 deletion(-)

Re: [PATCH 08/10] soc: lantiq: convert to devm_platform_ioremap_resource

[Paul Burton]

Hello,

Yangtao Li wrote:
> Use devm_platform_ioremap_resource() to simplify code.

Applied to mips-next.

> commit 23c25c732530
> https://git.kernel.org/mips/c/23c25c732530
> 
> Signed-off-by: Yangtao Li 
> Signed-off-by: Paul Burton 

Thanks,
Paul

[ This message was auto-generated; if you believe anything is incorrect
  then please email paulbur...@kernel.org to report it. ]