Re: [PATCH] powerpc: Remove inaccessible CMDLINE default

2019-08-01 Thread Christophe Leroy




Le 02/08/2019 à 07:02, Chris Packham a écrit :

Since commit cbe46bd4f510 ("powerpc: remove CONFIG_CMDLINE #ifdef mess")
CONFIG_CMDLINE has always had a value regardless of CONNIG_CMDLINE_BOOL.


s/CONNIG/CONFIG/



For example:

  $ make ARCH=powerpc defconfig
  $ cat .config
  # CONFIG_CMDLINE_BOOL is not set
  CONFIG_CMDLINE=""

When enabling CONNIG_CMDLINE_BOOL this value is kept making the 'default
"..." if CONNIG_CMDLINE_BOOL' ineffective.


s/CONNIG/CONFIG/



  $ ./scripts/config --enable CONFIG_CMDLINE_BOOL
  $ cat .config
  CONFIG_CMDLINE_BOOL=y
  CONFIG_CMDLINE=""

Additionally all the in-tree powerpc defconfigs that set
CONFIG_CMDLINE_BOOL=y also set CONFIG_CMDLINE to something else. For
these reasons remove the inaccessible default.

Signed-off-by: Chris Packham 


Reviewed-by: Christophe Leroy 


---
This should be independent of http://patchwork.ozlabs.org/patch/1140811/ but
I've generated this patch on a stream that has it applied locally.

  arch/powerpc/Kconfig | 1 -
  1 file changed, 1 deletion(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index d413fe1b4058..6fca6eba6aee 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -844,7 +844,6 @@ config CMDLINE_BOOL
  
  config CMDLINE

string "Initial kernel command string" if CMDLINE_BOOL
-   default "console=ttyS0,9600 console=tty0 root=/dev/sda2" if CMDLINE_BOOL
default ""
help
  On some platforms, there is currently no way for the boot loader to



I think we could also get rid of CMDLINE_BOOL totally and use CMDLINE != 
"" instead.


Christophe


[PATCH] powerpc: Remove inaccessible CMDLINE default

2019-08-01 Thread Chris Packham
Since commit cbe46bd4f510 ("powerpc: remove CONFIG_CMDLINE #ifdef mess")
CONFIG_CMDLINE has always had a value regardless of CONNIG_CMDLINE_BOOL.

For example:

 $ make ARCH=powerpc defconfig
 $ cat .config
 # CONFIG_CMDLINE_BOOL is not set
 CONFIG_CMDLINE=""

When enabling CONNIG_CMDLINE_BOOL this value is kept making the 'default
"..." if CONNIG_CMDLINE_BOOL' ineffective.

 $ ./scripts/config --enable CONFIG_CMDLINE_BOOL
 $ cat .config
 CONFIG_CMDLINE_BOOL=y
 CONFIG_CMDLINE=""

Additionally all the in-tree powerpc defconfigs that set
CONFIG_CMDLINE_BOOL=y also set CONFIG_CMDLINE to something else. For
these reasons remove the inaccessible default.

Signed-off-by: Chris Packham 
---
This should be independent of http://patchwork.ozlabs.org/patch/1140811/ but
I've generated this patch on a stream that has it applied locally.

 arch/powerpc/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index d413fe1b4058..6fca6eba6aee 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -844,7 +844,6 @@ config CMDLINE_BOOL
 
 config CMDLINE
string "Initial kernel command string" if CMDLINE_BOOL
-   default "console=ttyS0,9600 console=tty0 root=/dev/sda2" if CMDLINE_BOOL
default ""
help
  On some platforms, there is currently no way for the boot loader to
-- 
2.22.0



Re: [PATCH] hwrng: Use device-managed registration API

2019-08-01 Thread Herbert Xu
On Thu, Jul 25, 2019 at 04:01:55PM +0800, Chuhong Yuan wrote:
> Use devm_hwrng_register to simplify the implementation.
> Manual unregistration and some remove functions can be
> removed now.
> 
> Signed-off-by: Chuhong Yuan 
> ---
>  drivers/char/hw_random/atmel-rng.c |  3 +--
>  drivers/char/hw_random/cavium-rng-vf.c | 11 +--
>  drivers/char/hw_random/exynos-trng.c   |  3 +--
>  drivers/char/hw_random/n2-drv.c|  4 +---
>  drivers/char/hw_random/nomadik-rng.c   |  3 +--
>  drivers/char/hw_random/omap-rng.c  |  3 +--
>  drivers/char/hw_random/powernv-rng.c   | 10 +-
>  drivers/char/hw_random/st-rng.c|  4 +---
>  drivers/char/hw_random/xgene-rng.c |  4 +---
>  9 files changed, 9 insertions(+), 36 deletions(-)

Patch applied.  Thanks.
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt


Re: [PATCH v2] crypto: nx: nx-842-powernv: Add of_node_put() before return

2019-08-01 Thread Herbert Xu
On Wed, Jul 24, 2019 at 01:24:33PM +0530, Nishka Dasgupta wrote:
> Each iteration of for_each_compatible_node puts the previous node, but
> in the case of a return from the middle of the loop, there is no put,
> thus causing a memory leak. Add an of_node_put before the return.
> Issue found with Coccinelle.
> 
> Acked-by: Stewart Smith 
> Signed-off-by: Nishka Dasgupta 
> 
> ---
> Changes in v2:
> - Fixed commit message to match the loop in question.
> 
>  drivers/crypto/nx/nx-842-powernv.c | 1 +
>  1 file changed, 1 insertion(+)

Patch applied.  Thanks.
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt


Re: [PATCH v3] powerpc: Support CMDLINE_EXTEND

2019-08-01 Thread Christophe Leroy




Le 02/08/2019 à 00:50, Chris Packham a écrit :

Bring powerpc in line with other architectures that support extending or
overriding the bootloader provided command line.

The current behaviour is most like CMDLINE_FROM_BOOTLOADER where the
bootloader command line is preferred but the kernel config can provide a
fallback so CMDLINE_FROM_BOOTLOADER is the default. CMDLINE_EXTEND can
be used to append the CMDLINE from the kernel config to the one provided
by the bootloader.

Signed-off-by: Chris Packham 


Reviewed-by: Christophe Leroy 


---
Changes in v3:
- don't use BUG_ON in prom_strlcat
- rearrange things to eliminate prom_strlcpy

Changes in v2:
- incorporate ideas from Christope's patch 
https://patchwork.ozlabs.org/patch/1074126/
- support CMDLINE_FORCE

  arch/powerpc/Kconfig| 20 +-
  arch/powerpc/kernel/prom_init.c | 36 ++---
  2 files changed, 43 insertions(+), 13 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 77f6ebf97113..d413fe1b4058 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -852,15 +852,33 @@ config CMDLINE
  some command-line options at build time by entering them here.  In
  most cases you will need to specify the root device here.
  
+choice

+   prompt "Kernel command line type" if CMDLINE != ""
+   default CMDLINE_FROM_BOOTLOADER
+
+config CMDLINE_FROM_BOOTLOADER
+   bool "Use bootloader kernel arguments if available"
+   help
+ Uses the command-line options passed by the boot loader. If
+ the boot loader doesn't provide any, the default kernel command
+ string provided in CMDLINE will be used.
+
+config CMDLINE_EXTEND
+   bool "Extend bootloader kernel arguments"
+   help
+ The command-line arguments provided by the boot loader will be
+ appended to the default kernel command string.
+
  config CMDLINE_FORCE
bool "Always use the default kernel command string"
-   depends on CMDLINE_BOOL
help
  Always use the default kernel command string, even if the boot
  loader passes other arguments to the kernel.
  This is useful if you cannot or don't want to change the
  command-line options your boot loader passes to the kernel.
  
+endchoice

+
  config EXTRA_TARGETS
string "Additional default image types"
help
diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index 514707ef6779..1c7010cc6ec9 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -298,16 +298,24 @@ static char __init *prom_strstr(const char *s1, const 
char *s2)
return NULL;
  }
  
-static size_t __init prom_strlcpy(char *dest, const char *src, size_t size)

-{
-   size_t ret = prom_strlen(src);
+static size_t __init prom_strlcat(char *dest, const char *src, size_t count)
+{
+   size_t dsize = prom_strlen(dest);
+   size_t len = prom_strlen(src);
+   size_t res = dsize + len;
+
+   /* This would be a bug */
+   if (dsize >= count)
+   return count;
+
+   dest += dsize;
+   count -= dsize;
+   if (len >= count)
+   len = count-1;
+   memcpy(dest, src, len);
+   dest[len] = 0;
+   return res;
  
-	if (size) {

-   size_t len = (ret >= size) ? size - 1 : ret;
-   memcpy(dest, src, len);
-   dest[len] = '\0';
-   }
-   return ret;
  }
  
  #ifdef CONFIG_PPC_PSERIES

@@ -759,10 +767,14 @@ static void __init early_cmdline_parse(void)
  
  	prom_cmd_line[0] = 0;

p = prom_cmd_line;
-   if ((long)prom.chosen > 0)
+
+   if (!IS_ENABLED(CONFIG_CMDLINE_FORCE) && (long)prom.chosen > 0)
l = prom_getprop(prom.chosen, "bootargs", p, 
COMMAND_LINE_SIZE-1);
-   if (IS_ENABLED(CONFIG_CMDLINE_BOOL) && (l <= 0 || p[0] == '\0')) /* dbl 
check */
-   prom_strlcpy(prom_cmd_line, CONFIG_CMDLINE, 
sizeof(prom_cmd_line));
+
+   if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) || l <= 0 || p[0] == '\0')
+   prom_strlcat(prom_cmd_line, " " CONFIG_CMDLINE,
+sizeof(prom_cmd_line));
+
prom_printf("command line: %s\n", prom_cmd_line);
  
  #ifdef CONFIG_PPC64




Re: [PATCH v2] powerpc: Support CMDLINE_EXTEND

2019-08-01 Thread Christophe Leroy




Le 02/08/2019 à 00:32, Chris Packham a écrit :

On Thu, 2019-08-01 at 08:14 +0200, Christophe Leroy wrote:


Le 01/08/2019 à 04:12, Chris Packham a écrit :


Bring powerpc in line with other architectures that support
extending or
overriding the bootloader provided command line.

The current behaviour is most like CMDLINE_FROM_BOOTLOADER where
the
bootloader command line is preferred but the kernel config can
provide a
fallback so CMDLINE_FROM_BOOTLOADER is the default. CMDLINE_EXTEND
can
be used to append the CMDLINE from the kernel config to the one
provided
by the bootloader.

Signed-off-by: Chris Packham 
---
While I'm at it does anyone think it's worth getting rid of the
default CMDLINE
value if CMDLINE_BOOL and maybe CMDLINE_BOOL? Every defconfig in
the kernel
that sets CMDLINE_BOOL=y also sets CMDLINE to something other than
"console=ttyS0,9600 console=tty0 root=/dev/sda2". Removing
CMDLINE_BOOL and
unconditionally setting the default value of CMDLINE to "" would
clean up the
Kconfig even more.

Note
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/co
mmit/?id=cbe46bd4f5104552b612505b73d366f66efc2341
which is already a step forward.

I guess that default is for users selecting this option manually to
get
a first sensitive CMDLINE. But is it really worth it ?



I'm not even sure if it is working as intended right now. Even without
my changes if I use menuconfig and select CMDLINE_BOOL I end up with
CONFIG_CMDLINE="" in the resulting .config.


I guess if the CONFIG_CMDLINE doesn't exist yet, it will get the default 
value. But if it is already there allthough empty, it will remain empty.
So yes I guess you could just drop it for this reason and the other 
reasons you said.


Christophe



[PATCH v2 3/3] powerpc/spinlocks: Fix oops in shared-processor spinlocks

2019-08-01 Thread Christopher M. Riedl
Booting w/ ppc64le_defconfig + CONFIG_PREEMPT results in the attached
kernel trace due to calling shared-processor spinlocks while not running
in an SPLPAR. Previously, the out-of-line spinlocks implementations were
selected based on CONFIG_PPC_SPLPAR at compile time without a runtime
shared-processor LPAR check.

To fix, call the actual spinlock implementations from a set of common
functions, spin_yield() and rw_yield(), which check for shared-processor
LPAR during runtime and select the appropriate lock implementation.

[0.430878] BUG: Kernel NULL pointer dereference at 0x0100
[0.431991] Faulting instruction address: 0xc0097f88
[0.432934] Oops: Kernel access of bad area, sig: 7 [#1]
[0.433448] LE PAGE_SIZE=64K MMU=Radix MMU=Hash PREEMPT SMP NR_CPUS=2048 
NUMA PowerNV
[0.434479] Modules linked in:
[0.435055] CPU: 0 PID: 2 Comm: kthreadd Not tainted 
5.2.0-rc6-00491-g249155c20f9b #28
[0.435730] NIP:  c0097f88 LR: c0c07a88 CTR: c015ca10
[0.436383] REGS: c000727079f0 TRAP: 0300   Not tainted  
(5.2.0-rc6-00491-g249155c20f9b)
[0.437004] MSR:  92009033   CR: 
84000424  XER: 2004
[0.437874] CFAR: c0c07a84 DAR: 0100 DSISR: 0008 
IRQMASK: 1
[0.437874] GPR00: c0c07a88 c00072707c80 c1546300 
c0007be38a80
[0.437874] GPR04: c000726f0c00 0002 c0007279c980 
0100
[0.437874] GPR08: c1581b78 8001 0008 
c0007279c9b0
[0.437874] GPR12:  c173 c0142558 

[0.437874] GPR16:    

[0.437874] GPR20:    

[0.437874] GPR24: c0007be38a80 c0c002f4  

[0.437874] GPR28: c00072221a00 c000726c2600 c0007be38a80 
c0007be38a80
[0.443992] NIP [c0097f88] __spin_yield+0x48/0xa0
[0.444523] LR [c0c07a88] __raw_spin_lock+0xb8/0xc0
[0.445080] Call Trace:
[0.445670] [c00072707c80] [c00072221a00] 0xc00072221a00 
(unreliable)
[0.446425] [c00072707cb0] [c0bffb0c] __schedule+0xbc/0x850
[0.447078] [c00072707d70] [c0c002f4] schedule+0x54/0x130
[0.447694] [c00072707da0] [c01427dc] kthreadd+0x28c/0x2b0
[0.448389] [c00072707e20] [c000c1cc] 
ret_from_kernel_thread+0x5c/0x70
[0.449143] Instruction dump:
[0.449821] 4d9e0020 552a043e 210a07ff 79080fe0 0b08 3d020004 3908b878 
794a1f24
[0.450587] e8e8 7ce7502a e8e7 38e70100 <7ca03c2c> 70a70001 78a50020 
4d820020
[0.452808] ---[ end trace 474d6b2b8fc5cb7e ]---

Signed-off-by: Christopher M. Riedl 
---
 arch/powerpc/include/asm/spinlock.h | 36 -
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/arch/powerpc/include/asm/spinlock.h 
b/arch/powerpc/include/asm/spinlock.h
index 0a8270183770..6aed8a83b180 100644
--- a/arch/powerpc/include/asm/spinlock.h
+++ b/arch/powerpc/include/asm/spinlock.h
@@ -103,11 +103,9 @@ static inline int arch_spin_trylock(arch_spinlock_t *lock)
 /* We only yield to the hypervisor if we are in shared processor mode */
 void splpar_spin_yield(arch_spinlock_t *lock);
 void splpar_rw_yield(arch_rwlock_t *lock);
-#define __spin_yield(x) splpar_spin_yield(x)
-#define __rw_yield(x) splpar_rw_yield(x)
 #else /* SPLPAR */
-#define __spin_yield(x)barrier()
-#define __rw_yield(x)  barrier()
+static inline void splpar_spin_yield(arch_spinlock_t *lock) {};
+static inline void splpar_rw_yield(arch_rwlock_t *lock) {};
 #endif
 
 static inline bool is_shared_processor(void)
@@ -124,6 +122,22 @@ static inline bool is_shared_processor(void)
 #endif
 }
 
+static inline void spin_yield(arch_spinlock_t *lock)
+{
+   if (is_shared_processor())
+   splpar_spin_yield(lock);
+   else
+   barrier();
+}
+
+static inline void rw_yield(arch_rwlock_t *lock)
+{
+   if (is_shared_processor())
+   splpar_rw_yield(lock);
+   else
+   barrier();
+}
+
 static inline void arch_spin_lock(arch_spinlock_t *lock)
 {
while (1) {
@@ -132,7 +146,7 @@ static inline void arch_spin_lock(arch_spinlock_t *lock)
do {
HMT_low();
if (is_shared_processor())
-   __spin_yield(lock);
+   spin_yield(lock);
} while (unlikely(lock->slock != 0));
HMT_medium();
}
@@ -151,7 +165,7 @@ void arch_spin_lock_flags(arch_spinlock_t *lock, unsigned 
long flags)
do {
HMT_low();
if (is_shared_processor())
-   __spin_yield(lock);
+   spin_yield(lock);
  

[PATCH v2 1/3] powerpc/spinlocks: Refactor SHARED_PROCESSOR

2019-08-01 Thread Christopher M. Riedl
Determining if a processor is in shared processor mode is not a constant
so don't hide it behind a #define.

Signed-off-by: Christopher M. Riedl 
Reviewed-by: Andrew Donnellan 
---
 arch/powerpc/include/asm/spinlock.h | 24 ++--
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/include/asm/spinlock.h 
b/arch/powerpc/include/asm/spinlock.h
index a47f827bc5f1..dc5fcea1f006 100644
--- a/arch/powerpc/include/asm/spinlock.h
+++ b/arch/powerpc/include/asm/spinlock.h
@@ -101,15 +101,27 @@ static inline int arch_spin_trylock(arch_spinlock_t *lock)
 
 #if defined(CONFIG_PPC_SPLPAR)
 /* We only yield to the hypervisor if we are in shared processor mode */
-#define SHARED_PROCESSOR (lppaca_shared_proc(local_paca->lppaca_ptr))
 extern void __spin_yield(arch_spinlock_t *lock);
 extern void __rw_yield(arch_rwlock_t *lock);
 #else /* SPLPAR */
 #define __spin_yield(x)barrier()
 #define __rw_yield(x)  barrier()
-#define SHARED_PROCESSOR   0
 #endif
 
+static inline bool is_shared_processor(void)
+{
+/*
+ * LPPACA is only available on BOOK3S so guard anything LPPACA related to
+ * allow other platforms (which include this common header) to compile.
+ */
+#ifdef CONFIG_PPC_BOOK3S
+   return (IS_ENABLED(CONFIG_PPC_SPLPAR) &&
+   lppaca_shared_proc(local_paca->lppaca_ptr));
+#else
+   return false;
+#endif
+}
+
 static inline void arch_spin_lock(arch_spinlock_t *lock)
 {
while (1) {
@@ -117,7 +129,7 @@ static inline void arch_spin_lock(arch_spinlock_t *lock)
break;
do {
HMT_low();
-   if (SHARED_PROCESSOR)
+   if (is_shared_processor())
__spin_yield(lock);
} while (unlikely(lock->slock != 0));
HMT_medium();
@@ -136,7 +148,7 @@ void arch_spin_lock_flags(arch_spinlock_t *lock, unsigned 
long flags)
local_irq_restore(flags);
do {
HMT_low();
-   if (SHARED_PROCESSOR)
+   if (is_shared_processor())
__spin_yield(lock);
} while (unlikely(lock->slock != 0));
HMT_medium();
@@ -226,7 +238,7 @@ static inline void arch_read_lock(arch_rwlock_t *rw)
break;
do {
HMT_low();
-   if (SHARED_PROCESSOR)
+   if (is_shared_processor())
__rw_yield(rw);
} while (unlikely(rw->lock < 0));
HMT_medium();
@@ -240,7 +252,7 @@ static inline void arch_write_lock(arch_rwlock_t *rw)
break;
do {
HMT_low();
-   if (SHARED_PROCESSOR)
+   if (is_shared_processor())
__rw_yield(rw);
} while (unlikely(rw->lock != 0));
HMT_medium();
-- 
2.22.0



[PATCH v2 2/3] powerpc/spinlocks: Rename SPLPAR-only spinlocks

2019-08-01 Thread Christopher M. Riedl
The __rw_yield and __spin_yield locks only pertain to SPLPAR mode.
Rename them to make this relationship obvious.

Signed-off-by: Christopher M. Riedl 
Reviewed-by: Andrew Donnellan 
---
 arch/powerpc/include/asm/spinlock.h | 6 --
 arch/powerpc/lib/locks.c| 6 +++---
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/spinlock.h 
b/arch/powerpc/include/asm/spinlock.h
index dc5fcea1f006..0a8270183770 100644
--- a/arch/powerpc/include/asm/spinlock.h
+++ b/arch/powerpc/include/asm/spinlock.h
@@ -101,8 +101,10 @@ static inline int arch_spin_trylock(arch_spinlock_t *lock)
 
 #if defined(CONFIG_PPC_SPLPAR)
 /* We only yield to the hypervisor if we are in shared processor mode */
-extern void __spin_yield(arch_spinlock_t *lock);
-extern void __rw_yield(arch_rwlock_t *lock);
+void splpar_spin_yield(arch_spinlock_t *lock);
+void splpar_rw_yield(arch_rwlock_t *lock);
+#define __spin_yield(x) splpar_spin_yield(x)
+#define __rw_yield(x) splpar_rw_yield(x)
 #else /* SPLPAR */
 #define __spin_yield(x)barrier()
 #define __rw_yield(x)  barrier()
diff --git a/arch/powerpc/lib/locks.c b/arch/powerpc/lib/locks.c
index 6550b9e5ce5f..6440d5943c00 100644
--- a/arch/powerpc/lib/locks.c
+++ b/arch/powerpc/lib/locks.c
@@ -18,7 +18,7 @@
 #include 
 #include 
 
-void __spin_yield(arch_spinlock_t *lock)
+void splpar_spin_yield(arch_spinlock_t *lock)
 {
unsigned int lock_value, holder_cpu, yield_count;
 
@@ -36,14 +36,14 @@ void __spin_yield(arch_spinlock_t *lock)
plpar_hcall_norets(H_CONFER,
get_hard_smp_processor_id(holder_cpu), yield_count);
 }
-EXPORT_SYMBOL_GPL(__spin_yield);
+EXPORT_SYMBOL_GPL(splpar_spin_yield);
 
 /*
  * Waiting for a read lock or a write lock on a rwlock...
  * This turns out to be the same for read and write locks, since
  * we only know the holder if it is write-locked.
  */
-void __rw_yield(arch_rwlock_t *rw)
+void splpar_rw_yield(arch_rwlock_t *rw)
 {
int lock_value;
unsigned int holder_cpu, yield_count;
-- 
2.22.0



[PATCH v2 0/3] Fix oops in shared-processor spinlocks

2019-08-01 Thread Christopher M. Riedl
Fixes an oops when calling the shared-processor spinlock implementation
from a non-SP LPAR. Also take this opportunity to refactor
SHARED_PROCESSOR a bit.

Reference:  https://github.com/linuxppc/issues/issues/229

Changes since v1:
 - Improve comment wording to make it clear why the BOOK3S #ifdef is
   required in is_shared_processor() in spinlock.h
 - Replace empty #define of splpar_*_yield() with actual functions with
   empty bodies.

Christopher M. Riedl (3):
  powerpc/spinlocks: Refactor SHARED_PROCESSOR
  powerpc/spinlocks: Rename SPLPAR-only spinlocks
  powerpc/spinlocks: Fix oops in shared-processor spinlocks

 arch/powerpc/include/asm/spinlock.h | 62 +
 arch/powerpc/lib/locks.c|  6 +--
 2 files changed, 48 insertions(+), 20 deletions(-)

-- 
2.22.0



Re: [PATCH] powerpc/kasan: fix early boot failure on PPC32

2019-08-01 Thread Michael Ellerman
On Wed, 2019-07-31 at 06:01:42 UTC, Christophe Leroy wrote:
> Due to commit 4a6d8cf90017 ("powerpc/mm: don't use pte_alloc_kernel()
> until slab is available on PPC32"), pte_alloc_kernel() cannot be used
> during early KASAN init.
> 
> Fix it by using memblock_alloc() instead.
> 
> Reported-by: Erhard F. 
> Fixes: 2edb16efc899 ("powerpc/32: Add KASAN support")
> Signed-off-by: Christophe Leroy 

Applied to powerpc fixes, thanks.

https://git.kernel.org/powerpc/c/d7e23b887f67178c4f840781be7a6aa6aeb52ab1

cheers


Re: [PATCH] powerpc/spe: Mark expected switch fall-throughs

2019-08-01 Thread Michael Ellerman
On Tue, 2019-07-30 at 14:19:17 UTC, Michael Ellerman wrote:
> Mark switch cases where we are expecting to fall through.
> 
> Fixes errors such as below, seen with mpc85xx_defconfig:
> 
>   arch/powerpc/kernel/align.c: In function 'emulate_spe':
>   arch/powerpc/kernel/align.c:178:8: error: this statement may fall through
> ret |= __get_user_inatomic(temp.v[3], p++);
> ^~
> 
> Signed-off-by: Michael Ellerman 

Applied to powerpc fixes.

https://git.kernel.org/powerpc/c/7db57e77586744af46c8bbf8f831bb2b941b7afc

cheers


Re: [PATCH] drivers/macintosh/smu.c: Mark expected switch fall-through

2019-08-01 Thread Michael Ellerman
On Tue, 2019-07-30 at 04:37:04 UTC, Stephen Rothwell wrote:
> Mark switch cases where we are expecting to fall through.
> 
> This patch fixes the following warning (Building: powerpc):
> 
> drivers/macintosh/smu.c: In function 'smu_queue_i2c':
> drivers/macintosh/smu.c:854:21: warning: this statement may fall through [-=
> Wimplicit-fallthrough=3D]
>cmd->info.devaddr &=3D 0xfe;
>~~^~~
> drivers/macintosh/smu.c:855:2: note: here
>   case SMU_I2C_TRANSFER_STDSUB:
>   ^~~~
> 
> Cc: Benjamin Herrenschmidt 
> Cc: Gustavo A. R. Silva 
> Cc: Kees Cook 
> Signed-off-by: Stephen Rothwell 

Applied to powerpc fixes, thanks.

https://git.kernel.org/powerpc/c/7440ea8b2a4430eef5120d0a7faac6c39304ae6d

cheers


Re: [PATCH v3 00/10] implement KASLR for powerpc/fsl_booke/32

2019-08-01 Thread Jason Yan




On 2019/8/1 22:36, Diana Madalina Craciun wrote:

Hi Jason,

I have tested these series on a P4080 platform.

Regards,
Diana


Diana, thank you so much.

So can you take a look at the code of this version and give a 
Reviewed-by or Tested-by?


Thanks,
Jason



Re: [PATCH] powerpc/xive: Update comment referencing magic loads from an ESB

2019-08-01 Thread Stewart Smith
Jordan Niethe  writes:
> The comment above xive_esb_read() references magic loads from an ESB as
> described xive.h. This has been inaccurate since commit 12c1f339cd49
> ("powerpc/xive: Move definition of ESB bits") which moved the
> description. Update the comment to reference the new location of the
> description in xive-regs.h
>
> Signed-off-by: Jordan Niethe 

Acked-by: Stewart Smith 

-- 
Stewart Smith
OPAL Architect, IBM.



Re: [PATCH v2] PCI: rpaphp: Avoid a sometimes-uninitialized warning

2019-08-01 Thread Bjorn Helgaas
On Mon, Jul 22, 2019 at 02:05:12PM +1000, Michael Ellerman wrote:
> Nathan Chancellor  writes:
> > On Mon, Jun 03, 2019 at 03:11:58PM -0700, Nathan Chancellor wrote:
> >> When building with -Wsometimes-uninitialized, clang warns:
> >> 
> >> drivers/pci/hotplug/rpaphp_core.c:243:14: warning: variable 'fndit' is
> >> used uninitialized whenever 'for' loop exits because its condition is
> >> false [-Wsometimes-uninitialized]
> >> for (j = 0; j < entries; j++) {
> >> ^~~
> >> drivers/pci/hotplug/rpaphp_core.c:256:6: note: uninitialized use occurs
> >> here
> >> if (fndit)
> >> ^
> >> drivers/pci/hotplug/rpaphp_core.c:243:14: note: remove the condition if
> >> it is always true
> >> for (j = 0; j < entries; j++) {
> >> ^~~
> >> drivers/pci/hotplug/rpaphp_core.c:233:14: note: initialize the variable
> >> 'fndit' to silence this warning
> >> int j, fndit;
> >> ^
> >>  = 0
> >> 
> >> fndit is only used to gate a sprintf call, which can be moved into the
> >> loop to simplify the code and eliminate the local variable, which will
> >> fix this warning.
> >> 
> >> Link: https://github.com/ClangBuiltLinux/linux/issues/504
> >> Fixes: 2fcf3ae508c2 ("hotplug/drc-info: Add code to search ibm,drc-info 
> >> property")
> >> Suggested-by: Nick Desaulniers 
> >> Signed-off-by: Nathan Chancellor 
> >> ---
> >> 
> >> v1 -> v2:
> >> 
> >> * Eliminate fndit altogether by shuffling the sprintf call into the for
> >>   loop and changing the if conditional, as suggested by Nick.
> >> 
> >>  drivers/pci/hotplug/rpaphp_core.c | 18 +++---
> >>  1 file changed, 7 insertions(+), 11 deletions(-)
> >> 
> >> diff --git a/drivers/pci/hotplug/rpaphp_core.c 
> >> b/drivers/pci/hotplug/rpaphp_core.c
> >> index bcd5d357ca23..c3899ee1db99 100644
> >> --- a/drivers/pci/hotplug/rpaphp_core.c
> >> +++ b/drivers/pci/hotplug/rpaphp_core.c
> >> @@ -230,7 +230,7 @@ static int rpaphp_check_drc_props_v2(struct 
> >> device_node *dn, char *drc_name,
> >>struct of_drc_info drc;
> >>const __be32 *value;
> >>char cell_drc_name[MAX_DRC_NAME_LEN];
> >> -  int j, fndit;
> >> +  int j;
> >>  
> >>info = of_find_property(dn->parent, "ibm,drc-info", NULL);
> >>if (info == NULL)
> >> @@ -245,17 +245,13 @@ static int rpaphp_check_drc_props_v2(struct 
> >> device_node *dn, char *drc_name,
> >>  
> >>/* Should now know end of current entry */
> >>  
> >> -  if (my_index > drc.last_drc_index)
> >> -  continue;
> >> -
> >> -  fndit = 1;
> >> -  break;
> >> +  /* Found it */
> >> +  if (my_index <= drc.last_drc_index) {
> >> +  sprintf(cell_drc_name, "%s%d", drc.drc_name_prefix,
> >> +  my_index);
> >> +  break;
> >> +  }
> >>}
> >> -  /* Found it */
> >> -
> >> -  if (fndit)
> >> -  sprintf(cell_drc_name, "%s%d", drc.drc_name_prefix, 
> >> -  my_index);
> >>  
> >>if (((drc_name == NULL) ||
> >> (drc_name && !strcmp(drc_name, cell_drc_name))) &&
> >> -- 
> >> 2.22.0.rc3
> >> 
> >
> > Hi all,
> >
> > Could someone please pick this up?
> 
> I'll take it.
> 
> I was expecting Bjorn to take it as a PCI patch, but I realise now that
> I merged that code in the first place so may as well take this too.
> 
> I'll put it in my next branch once that opens next week.

Sorry, I should have done something with this.  Did you take it,
Michael?  I don't see it in -next and haven't figured out where to
look in your git tree, so I can't tell.  Just let me know either way
so I know whether to drop this or apply it.

Bjorn


[PATCH] powerpc/xive: Update comment referencing magic loads from an ESB

2019-08-01 Thread Jordan Niethe
The comment above xive_esb_read() references magic loads from an ESB as
described xive.h. This has been inaccurate since commit 12c1f339cd49
("powerpc/xive: Move definition of ESB bits") which moved the
description. Update the comment to reference the new location of the
description in xive-regs.h

Signed-off-by: Jordan Niethe 
---
 arch/powerpc/sysdev/xive/common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/sysdev/xive/common.c 
b/arch/powerpc/sysdev/xive/common.c
index 1cdb39575eae..083f657091d7 100644
--- a/arch/powerpc/sysdev/xive/common.c
+++ b/arch/powerpc/sysdev/xive/common.c
@@ -185,7 +185,7 @@ static u32 xive_scan_interrupts(struct xive_cpu *xc, bool 
just_peek)
 
 /*
  * This is used to perform the magic loads from an ESB
- * described in xive.h
+ * described in xive-regs.h
  */
 static notrace u8 xive_esb_read(struct xive_irq_data *xd, u32 offset)
 {
-- 
2.20.1



[PATCH 1/1] pseries/hotplug-memory.c: Change rc variable to bool

2019-08-01 Thread Leonardo Bras
Changes the return variable to bool (as the return value) and
avoids doing a ternary operation before returning.

Also, since rc will always be true, there is no need to do
rc &= bool, as (true && X) will result in X.

Signed-off-by: Leonardo Bras 
---
 arch/powerpc/platforms/pseries/hotplug-memory.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c 
b/arch/powerpc/platforms/pseries/hotplug-memory.c
index 8e700390f3d6..392deb4855e5 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -338,7 +338,7 @@ static int pseries_remove_mem_node(struct device_node *np)
 static bool lmb_is_removable(struct drmem_lmb *lmb)
 {
int i, scns_per_block;
-   int rc = 1;
+   bool rc = true;
unsigned long pfn, block_sz;
u64 phys_addr;
 
@@ -363,11 +363,11 @@ static bool lmb_is_removable(struct drmem_lmb *lmb)
if (!pfn_present(pfn))
continue;
 
-   rc &= is_mem_section_removable(pfn, PAGES_PER_SECTION);
+   rc = is_mem_section_removable(pfn, PAGES_PER_SECTION);
phys_addr += MIN_MEMORY_BLOCK_SIZE;
}
 
-   return rc ? true : false;
+   return rc;
 }
 
 static int dlpar_add_lmb(struct drmem_lmb *);
-- 
2.20.1



[PATCH 1/1] pseries/hotplug-memory.c: Replace nested ifs by switch-case

2019-08-01 Thread Leonardo Bras
I noticed these nested ifs can be easily replaced by switch-cases,
which can improve readability.

Signed-off-by: Leonardo Bras 
---
 .../platforms/pseries/hotplug-memory.c| 26 +--
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c 
b/arch/powerpc/platforms/pseries/hotplug-memory.c
index 46d0d35b9ca4..8e700390f3d6 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -880,34 +880,44 @@ int dlpar_memory(struct pseries_hp_errorlog *hp_elog)
 
switch (hp_elog->action) {
case PSERIES_HP_ELOG_ACTION_ADD:
-   if (hp_elog->id_type == PSERIES_HP_ELOG_ID_DRC_COUNT) {
+   switch (hp_elog->id_type) {
+   case PSERIES_HP_ELOG_ID_DRC_COUNT:
count = hp_elog->_drc_u.drc_count;
rc = dlpar_memory_add_by_count(count);
-   } else if (hp_elog->id_type == PSERIES_HP_ELOG_ID_DRC_INDEX) {
+   break;
+   case PSERIES_HP_ELOG_ID_DRC_INDEX:
drc_index = hp_elog->_drc_u.drc_index;
rc = dlpar_memory_add_by_index(drc_index);
-   } else if (hp_elog->id_type == PSERIES_HP_ELOG_ID_DRC_IC) {
+   break;
+   case PSERIES_HP_ELOG_ID_DRC_IC:
count = hp_elog->_drc_u.ic.count;
drc_index = hp_elog->_drc_u.ic.index;
rc = dlpar_memory_add_by_ic(count, drc_index);
-   } else {
+   break;
+   default:
rc = -EINVAL;
+   break;
}
 
break;
case PSERIES_HP_ELOG_ACTION_REMOVE:
-   if (hp_elog->id_type == PSERIES_HP_ELOG_ID_DRC_COUNT) {
+   switch (hp_elog->id_type) {
+   case PSERIES_HP_ELOG_ID_DRC_COUNT:
count = hp_elog->_drc_u.drc_count;
rc = dlpar_memory_remove_by_count(count);
-   } else if (hp_elog->id_type == PSERIES_HP_ELOG_ID_DRC_INDEX) {
+   break;
+   case PSERIES_HP_ELOG_ID_DRC_INDEX:
drc_index = hp_elog->_drc_u.drc_index;
rc = dlpar_memory_remove_by_index(drc_index);
-   } else if (hp_elog->id_type == PSERIES_HP_ELOG_ID_DRC_IC) {
+   break;
+   case PSERIES_HP_ELOG_ID_DRC_IC:
count = hp_elog->_drc_u.ic.count;
drc_index = hp_elog->_drc_u.ic.index;
rc = dlpar_memory_remove_by_ic(count, drc_index);
-   } else {
+   break;
+   default:
rc = -EINVAL;
+   break;
}
 
break;
-- 
2.20.1



[PATCH v3] powerpc: Support CMDLINE_EXTEND

2019-08-01 Thread Chris Packham
Bring powerpc in line with other architectures that support extending or
overriding the bootloader provided command line.

The current behaviour is most like CMDLINE_FROM_BOOTLOADER where the
bootloader command line is preferred but the kernel config can provide a
fallback so CMDLINE_FROM_BOOTLOADER is the default. CMDLINE_EXTEND can
be used to append the CMDLINE from the kernel config to the one provided
by the bootloader.

Signed-off-by: Chris Packham 
---
Changes in v3:
- don't use BUG_ON in prom_strlcat
- rearrange things to eliminate prom_strlcpy

Changes in v2:
- incorporate ideas from Christope's patch 
https://patchwork.ozlabs.org/patch/1074126/
- support CMDLINE_FORCE

 arch/powerpc/Kconfig| 20 +-
 arch/powerpc/kernel/prom_init.c | 36 ++---
 2 files changed, 43 insertions(+), 13 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 77f6ebf97113..d413fe1b4058 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -852,15 +852,33 @@ config CMDLINE
  some command-line options at build time by entering them here.  In
  most cases you will need to specify the root device here.
 
+choice
+   prompt "Kernel command line type" if CMDLINE != ""
+   default CMDLINE_FROM_BOOTLOADER
+
+config CMDLINE_FROM_BOOTLOADER
+   bool "Use bootloader kernel arguments if available"
+   help
+ Uses the command-line options passed by the boot loader. If
+ the boot loader doesn't provide any, the default kernel command
+ string provided in CMDLINE will be used.
+
+config CMDLINE_EXTEND
+   bool "Extend bootloader kernel arguments"
+   help
+ The command-line arguments provided by the boot loader will be
+ appended to the default kernel command string.
+
 config CMDLINE_FORCE
bool "Always use the default kernel command string"
-   depends on CMDLINE_BOOL
help
  Always use the default kernel command string, even if the boot
  loader passes other arguments to the kernel.
  This is useful if you cannot or don't want to change the
  command-line options your boot loader passes to the kernel.
 
+endchoice
+
 config EXTRA_TARGETS
string "Additional default image types"
help
diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index 514707ef6779..1c7010cc6ec9 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -298,16 +298,24 @@ static char __init *prom_strstr(const char *s1, const 
char *s2)
return NULL;
 }
 
-static size_t __init prom_strlcpy(char *dest, const char *src, size_t size)
-{
-   size_t ret = prom_strlen(src);
+static size_t __init prom_strlcat(char *dest, const char *src, size_t count)
+{
+   size_t dsize = prom_strlen(dest);
+   size_t len = prom_strlen(src);
+   size_t res = dsize + len;
+
+   /* This would be a bug */
+   if (dsize >= count)
+   return count;
+
+   dest += dsize;
+   count -= dsize;
+   if (len >= count)
+   len = count-1;
+   memcpy(dest, src, len);
+   dest[len] = 0;
+   return res;
 
-   if (size) {
-   size_t len = (ret >= size) ? size - 1 : ret;
-   memcpy(dest, src, len);
-   dest[len] = '\0';
-   }
-   return ret;
 }
 
 #ifdef CONFIG_PPC_PSERIES
@@ -759,10 +767,14 @@ static void __init early_cmdline_parse(void)
 
prom_cmd_line[0] = 0;
p = prom_cmd_line;
-   if ((long)prom.chosen > 0)
+
+   if (!IS_ENABLED(CONFIG_CMDLINE_FORCE) && (long)prom.chosen > 0)
l = prom_getprop(prom.chosen, "bootargs", p, 
COMMAND_LINE_SIZE-1);
-   if (IS_ENABLED(CONFIG_CMDLINE_BOOL) && (l <= 0 || p[0] == '\0')) /* dbl 
check */
-   prom_strlcpy(prom_cmd_line, CONFIG_CMDLINE, 
sizeof(prom_cmd_line));
+
+   if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) || l <= 0 || p[0] == '\0')
+   prom_strlcat(prom_cmd_line, " " CONFIG_CMDLINE,
+sizeof(prom_cmd_line));
+
prom_printf("command line: %s\n", prom_cmd_line);
 
 #ifdef CONFIG_PPC64
-- 
2.22.0



Re: [PATCH v2] powerpc: Support CMDLINE_EXTEND

2019-08-01 Thread Chris Packham
On Thu, 2019-08-01 at 08:14 +0200, Christophe Leroy wrote:
> 
> Le 01/08/2019 à 04:12, Chris Packham a écrit :
> > 
> > Bring powerpc in line with other architectures that support
> > extending or
> > overriding the bootloader provided command line.
> > 
> > The current behaviour is most like CMDLINE_FROM_BOOTLOADER where
> > the
> > bootloader command line is preferred but the kernel config can
> > provide a
> > fallback so CMDLINE_FROM_BOOTLOADER is the default. CMDLINE_EXTEND
> > can
> > be used to append the CMDLINE from the kernel config to the one
> > provided
> > by the bootloader.
> > 
> > Signed-off-by: Chris Packham 
> > ---
> > While I'm at it does anyone think it's worth getting rid of the
> > default CMDLINE
> > value if CMDLINE_BOOL and maybe CMDLINE_BOOL? Every defconfig in
> > the kernel
> > that sets CMDLINE_BOOL=y also sets CMDLINE to something other than
> > "console=ttyS0,9600 console=tty0 root=/dev/sda2". Removing
> > CMDLINE_BOOL and
> > unconditionally setting the default value of CMDLINE to "" would
> > clean up the
> > Kconfig even more.
> Note 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/co
> mmit/?id=cbe46bd4f5104552b612505b73d366f66efc2341 
> which is already a step forward.
> 
> I guess that default is for users selecting this option manually to
> get 
> a first sensitive CMDLINE. But is it really worth it ?
> 

I'm not even sure if it is working as intended right now. Even without
my changes if I use menuconfig and select CMDLINE_BOOL I end up with 
CONFIG_CMDLINE="" in the resulting .config.

> > 
> > 
> > Changes in v2:
> > - incorporate ideas from Christope's patch https://patchwork.ozlabs
> > .org/patch/1074126/
> > 
> >   arch/powerpc/Kconfig| 20 +++-
> >   arch/powerpc/kernel/prom_init.c | 26 +-
> >   2 files changed, 44 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> > index 77f6ebf97113..d413fe1b4058 100644
> > --- a/arch/powerpc/Kconfig
> > +++ b/arch/powerpc/Kconfig
> > @@ -852,15 +852,33 @@ config CMDLINE
> >       some command-line options at build time by entering
> > them here.  In
> >       most cases you will need to specify the root device
> > here.
> >   
> > +choice
> > +   prompt "Kernel command line type" if CMDLINE != ""
> > +   default CMDLINE_FROM_BOOTLOADER
> > +
> > +config CMDLINE_FROM_BOOTLOADER
> > +   bool "Use bootloader kernel arguments if available"
> > +   help
> > +     Uses the command-line options passed by the boot loader.
> > If
> > +     the boot loader doesn't provide any, the default kernel
> > command
> > +     string provided in CMDLINE will be used.
> > +
> > +config CMDLINE_EXTEND
> > +   bool "Extend bootloader kernel arguments"
> > +   help
> > +     The command-line arguments provided by the boot loader
> > will be
> > +     appended to the default kernel command string.
> > +
> >   config CMDLINE_FORCE
> >     bool "Always use the default kernel command string"
> > -   depends on CMDLINE_BOOL
> >     help
> >       Always use the default kernel command string, even if
> > the boot
> >       loader passes other arguments to the kernel.
> >       This is useful if you cannot or don't want to change
> > the
> >       command-line options your boot loader passes to the
> > kernel.
> >   
> > +endchoice
> > +
> >   config EXTRA_TARGETS
> >     string "Additional default image types"
> >     help
> > diff --git a/arch/powerpc/kernel/prom_init.c
> > b/arch/powerpc/kernel/prom_init.c
> > index 514707ef6779..df29f141dbd2 100644
> > --- a/arch/powerpc/kernel/prom_init.c
> > +++ b/arch/powerpc/kernel/prom_init.c
> > @@ -310,6 +310,25 @@ static size_t __init prom_strlcpy(char *dest,
> > const char *src, size_t size)
> >     return ret;
> >   }
> >   
> > +static size_t __init prom_strlcat(char *dest, const char *src,
> > size_t count)
> > +{
> > +   size_t dsize = prom_strlen(dest);
> > +   size_t len = prom_strlen(src);
> > +   size_t res = dsize + len;
> > +
> > +   /* This would be a bug */
> > +   BUG_ON(dsize >= count);
> Has you pointed in another mail, BUG_ON() should be avoided here.
> I guess if the destination is already full, just return count:
> 
> if (dsize >= count)
>   return count;
> 

Will fix in v3.

> > 
> > +
> > +   dest += dsize;
> > +   count -= dsize;
> > +   if (len >= count)
> > +   len = count-1;
> > +   memcpy(dest, src, len);
> > +   dest[len] = 0;
> > +   return res;
> > +
> > +}
> > +
> >   #ifdef CONFIG_PPC_PSERIES
> >   static int __init prom_strtobool(const char *s, bool *res)
> >   {
> > @@ -761,8 +780,13 @@ static void __init early_cmdline_parse(void)
> >     p = prom_cmd_line;
> >     if ((long)prom.chosen > 0)
> >     l = prom_getprop(prom.chosen, "bootargs", p,
> > COMMAND_LINE_SIZE-1);
> The above is pointless in case CONFIG_CMDLINE_FORCE is selected.
> 

Will fix in v3.

> > 
> > -   if (IS_ENABLED(CONFIG_CMDLINE_BOOL) && (l <= 0 || p[0] ==
> > '\0')) /* 

[PATCH v3 7/7] soc/fsl/qbman: Update device tree with reserved memory

2019-08-01 Thread Roy Pledge
When using the reserved memory node in the device tree there are
two options - dynamic or static. If a dynamic allocation was
selected (where the kernel selects the address for the allocation)
convert it to a static allocation by inserting the reg property.
This will ensure the same memory is reused after a kexec()

Signed-off-by: Roy Pledge 
---
 drivers/soc/fsl/qbman/dpaa_sys.c | 60 
 1 file changed, 36 insertions(+), 24 deletions(-)

diff --git a/drivers/soc/fsl/qbman/dpaa_sys.c b/drivers/soc/fsl/qbman/dpaa_sys.c
index 3e0a7f3..9dd8bb5 100644
--- a/drivers/soc/fsl/qbman/dpaa_sys.c
+++ b/drivers/soc/fsl/qbman/dpaa_sys.c
@@ -37,41 +37,53 @@
 int qbman_init_private_mem(struct device *dev, int idx, dma_addr_t *addr,
size_t *size)
 {
-   int ret;
struct device_node *mem_node;
-   u64 size64;
struct reserved_mem *rmem;
+   struct property *prop;
+   int len, err;
+   __be32 *res_array;
 
-   ret = of_reserved_mem_device_init_by_idx(dev, dev->of_node, idx);
-   if (ret) {
-   dev_err(dev,
-   "of_reserved_mem_device_init_by_idx(%d) failed 0x%x\n",
-   idx, ret);
-   return -ENODEV;
-   }
-   mem_node = of_parse_phandle(dev->of_node, "memory-region", 0);
-   if (mem_node) {
-   ret = of_property_read_u64(mem_node, "size", );
-   if (ret) {
-   dev_err(dev, "of_address_to_resource fails 0x%x\n",
-   ret);
-   return -ENODEV;
-   }
-   *size = size64;
-   } else {
+   mem_node = of_parse_phandle(dev->of_node, "memory-region", idx);
+   if (!mem_node) {
dev_err(dev, "No memory-region found for index %d\n", idx);
return -ENODEV;
}
 
rmem = of_reserved_mem_lookup(mem_node);
+   if (!rmem) {
+   dev_err(dev, "of_reserved_mem_lookup() returned NULL\n");
+   return -ENODEV;
+   }
*addr = rmem->base;
+   *size = rmem->size;
 
/*
-* Disassociate the reserved memory area from the device
-* because a device can only have one DMA memory area. This
-* should be fine since the memory is allocated and initialized
-* and only ever accessed by the QBMan device from now on
+* Check if the reg property exists - if not insert the node
+* so upon kexec() the same memory region address will be preserved.
+* This is needed because QBMan HW does not allow the base address/
+* size to be modified once set.
 */
-   of_reserved_mem_device_release(dev);
+   prop = of_find_property(mem_node, "reg", );
+   if (!prop) {
+   prop = devm_kzalloc(dev, sizeof(*prop), GFP_KERNEL);
+   if (!prop)
+   return -ENOMEM;
+   prop->value = res_array = devm_kzalloc(dev, sizeof(__be32) * 4,
+  GFP_KERNEL);
+   if (!prop->value)
+   return -ENOMEM;
+   res_array[0] = cpu_to_be32(upper_32_bits(*addr));
+   res_array[1] = cpu_to_be32(lower_32_bits(*addr));
+   res_array[2] = cpu_to_be32(upper_32_bits(*size));
+   res_array[3] = cpu_to_be32(lower_32_bits(*size));
+   prop->length = sizeof(__be32) * 4;
+   prop->name = devm_kstrdup(dev, "reg", GFP_KERNEL);
+   if (!prop->name)
+   return -ENOMEM;
+   err = of_add_property(mem_node, prop);
+   if (err)
+   return err;
+   }
+
return 0;
 }
-- 
2.7.4



[PATCH v3 6/7] soc/fsl/qbman: Fixup qman_shutdown_fq()

2019-08-01 Thread Roy Pledge
When shutting down a FQ on a dedicated channel only the
SW portal associated with that channel can dequeue from it.
Make sure the correct portal is use.

Signed-off-by: Roy Pledge 
---
 drivers/soc/fsl/qbman/qman.c | 53 +++-
 1 file changed, 42 insertions(+), 11 deletions(-)

diff --git a/drivers/soc/fsl/qbman/qman.c b/drivers/soc/fsl/qbman/qman.c
index 4a99ce5..bf68d86 100644
--- a/drivers/soc/fsl/qbman/qman.c
+++ b/drivers/soc/fsl/qbman/qman.c
@@ -1018,6 +1018,20 @@ static inline void put_affine_portal(void)
put_cpu_var(qman_affine_portal);
 }
 
+
+static inline struct qman_portal *get_portal_for_channel(u16 channel)
+{
+   int i;
+
+   for (i = 0; i < num_possible_cpus(); i++) {
+   if (affine_portals[i] &&
+   affine_portals[i]->config->channel == channel)
+   return affine_portals[i];
+   }
+
+   return NULL;
+}
+
 static struct workqueue_struct *qm_portal_wq;
 
 int qman_dqrr_set_ithresh(struct qman_portal *portal, u8 ithresh)
@@ -2601,7 +2615,7 @@ static int _qm_dqrr_consume_and_match(struct qm_portal 
*p, u32 fqid, int s,
 
 int qman_shutdown_fq(u32 fqid)
 {
-   struct qman_portal *p;
+   struct qman_portal *p, *channel_portal;
struct device *dev;
union qm_mc_command *mcc;
union qm_mc_result *mcr;
@@ -2641,17 +2655,28 @@ int qman_shutdown_fq(u32 fqid)
channel = qm_fqd_get_chan(>queryfq.fqd);
wq = qm_fqd_get_wq(>queryfq.fqd);
 
+   if (channel < qm_channel_pool1) {
+   channel_portal = get_portal_for_channel(channel);
+   if (channel_portal == NULL) {
+   dev_err(dev, "Can't find portal for dedicated channel 
0x%x\n",
+   channel);
+   ret = -EIO;
+   goto out;
+   }
+   } else
+   channel_portal = p;
+
switch (state) {
case QM_MCR_NP_STATE_TEN_SCHED:
case QM_MCR_NP_STATE_TRU_SCHED:
case QM_MCR_NP_STATE_ACTIVE:
case QM_MCR_NP_STATE_PARKED:
orl_empty = 0;
-   mcc = qm_mc_start(>p);
+   mcc = qm_mc_start(_portal->p);
qm_fqid_set(>fq, fqid);
-   qm_mc_commit(>p, QM_MCC_VERB_ALTER_RETIRE);
-   if (!qm_mc_result_timeout(>p, )) {
-   dev_err(dev, "QUERYFQ_NP timeout\n");
+   qm_mc_commit(_portal->p, QM_MCC_VERB_ALTER_RETIRE);
+   if (!qm_mc_result_timeout(_portal->p, )) {
+   dev_err(dev, "ALTER_RETIRE timeout\n");
ret = -ETIMEDOUT;
goto out;
}
@@ -2659,6 +2684,9 @@ int qman_shutdown_fq(u32 fqid)
QM_MCR_VERB_ALTER_RETIRE);
res = mcr->result; /* Make a copy as we reuse MCR below */
 
+   if (res == QM_MCR_RESULT_OK)
+   drain_mr_fqrni(_portal->p);
+
if (res == QM_MCR_RESULT_PENDING) {
/*
 * Need to wait for the FQRN in the message ring, which
@@ -2688,21 +2716,25 @@ int qman_shutdown_fq(u32 fqid)
}
/* Set the sdqcr to drain this channel */
if (channel < qm_channel_pool1)
-   qm_dqrr_sdqcr_set(>p,
+   qm_dqrr_sdqcr_set(_portal->p,
  QM_SDQCR_TYPE_ACTIVE |
  QM_SDQCR_CHANNELS_DEDICATED);
else
-   qm_dqrr_sdqcr_set(>p,
+   qm_dqrr_sdqcr_set(_portal->p,
  QM_SDQCR_TYPE_ACTIVE |
  QM_SDQCR_CHANNELS_POOL_CONV
  (channel));
do {
/* Keep draining DQRR while checking the MR*/
-   qm_dqrr_drain_nomatch(>p);
+   qm_dqrr_drain_nomatch(_portal->p);
/* Process message ring too */
-   found_fqrn = qm_mr_drain(>p, FQRN);
+   found_fqrn = qm_mr_drain(_portal->p,
+FQRN);
cpu_relax();
} while (!found_fqrn);
+   /* Restore SDQCR */
+   qm_dqrr_sdqcr_set(_portal->p,
+ channel_portal->sdqcr);
 
}
if (res != QM_MCR_RESULT_OK &&
@@ -2733,9 +2765,8 @@ int qman_shutdown_fq(u32 fqid)
 * Wait for a dequeue and process the dequeues,
 * 

[PATCH v3 5/7] soc/fsl/qbman: Disable interrupts during portal recovery

2019-08-01 Thread Roy Pledge
Disable the QBMan interrupts during recovery.

Signed-off-by: Roy Pledge 
---
 drivers/soc/fsl/qbman/qman.c  | 22 +++---
 drivers/soc/fsl/qbman/qman_ccsr.c |  1 +
 drivers/soc/fsl/qbman/qman_priv.h |  1 +
 3 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/soc/fsl/qbman/qman.c b/drivers/soc/fsl/qbman/qman.c
index 2989504..4a99ce5 100644
--- a/drivers/soc/fsl/qbman/qman.c
+++ b/drivers/soc/fsl/qbman/qman.c
@@ -1070,6 +1070,20 @@ int qman_wq_alloc(void)
return 0;
 }
 
+
+void qman_enable_irqs(void)
+{
+   int i;
+
+   for (i = 0; i < num_possible_cpus(); i++) {
+   if (affine_portals[i]) {
+   qm_out(_portals[i]->p, QM_REG_ISR, 0x);
+   qm_out(_portals[i]->p, QM_REG_IIR, 0);
+   }
+
+   }
+}
+
 /*
  * This is what everything can wait on, even if it migrates to a different cpu
  * to the one whose affine portal it is waiting on.
@@ -1269,8 +1283,8 @@ static int qman_create_portal(struct qman_portal *portal,
qm_out(p, QM_REG_ISDR, isdr);
portal->irq_sources = 0;
qm_out(p, QM_REG_IER, 0);
-   qm_out(p, QM_REG_ISR, 0x);
snprintf(portal->irqname, MAX_IRQNAME, IRQNAME, c->cpu);
+   qm_out(p, QM_REG_IIR, 1);
if (request_irq(c->irq, portal_isr, 0, portal->irqname, portal)) {
dev_err(c->dev, "request_irq() failed\n");
goto fail_irq;
@@ -1290,7 +1304,7 @@ static int qman_create_portal(struct qman_portal *portal,
isdr &= ~(QM_PIRQ_DQRI | QM_PIRQ_MRI);
qm_out(p, QM_REG_ISDR, isdr);
if (qm_dqrr_current(p)) {
-   dev_err(c->dev, "DQRR unclean\n");
+   dev_dbg(c->dev, "DQRR unclean\n");
qm_dqrr_cdc_consume_n(p, 0x);
}
if (qm_mr_current(p) && drain_mr_fqrni(p)) {
@@ -1303,8 +1317,10 @@ static int qman_create_portal(struct qman_portal *portal,
}
/* Success */
portal->config = c;
+   qm_out(p, QM_REG_ISR, 0x);
qm_out(p, QM_REG_ISDR, 0);
-   qm_out(p, QM_REG_IIR, 0);
+   if (!qman_requires_cleanup())
+   qm_out(p, QM_REG_IIR, 0);
/* Write a sane SDQCR */
qm_dqrr_sdqcr_set(p, portal->sdqcr);
return 0;
diff --git a/drivers/soc/fsl/qbman/qman_ccsr.c 
b/drivers/soc/fsl/qbman/qman_ccsr.c
index 709661b7b..157659f 100644
--- a/drivers/soc/fsl/qbman/qman_ccsr.c
+++ b/drivers/soc/fsl/qbman/qman_ccsr.c
@@ -744,6 +744,7 @@ int qman_requires_cleanup(void)
 
 void qman_done_cleanup(void)
 {
+   qman_enable_irqs();
__qman_requires_cleanup = 0;
 }
 
diff --git a/drivers/soc/fsl/qbman/qman_priv.h 
b/drivers/soc/fsl/qbman/qman_priv.h
index a8a35fe..fd1cf54 100644
--- a/drivers/soc/fsl/qbman/qman_priv.h
+++ b/drivers/soc/fsl/qbman/qman_priv.h
@@ -279,3 +279,4 @@ int qman_shutdown_fq(u32 fqid);
 
 int qman_requires_cleanup(void);
 void qman_done_cleanup(void);
+void qman_enable_irqs(void);
-- 
2.7.4



[PATCH v3 4/7] soc/fsl/qbman: Fix drain_mr_fqni()

2019-08-01 Thread Roy Pledge
The drain_mr_fqni() function may be called fron uninterruptable
context so convert the msleep() to an mdelay().  Also ensure that
the valid bit is updated while polling.

Signed-off-by: Roy Pledge 
---
 drivers/soc/fsl/qbman/qman.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/fsl/qbman/qman.c b/drivers/soc/fsl/qbman/qman.c
index f10f77d..2989504 100644
--- a/drivers/soc/fsl/qbman/qman.c
+++ b/drivers/soc/fsl/qbman/qman.c
@@ -1164,6 +1164,7 @@ static int drain_mr_fqrni(struct qm_portal *p)
 {
const union qm_mr_entry *msg;
 loop:
+   qm_mr_pvb_update(p);
msg = qm_mr_current(p);
if (!msg) {
/*
@@ -1180,7 +1181,8 @@ static int drain_mr_fqrni(struct qm_portal *p)
 * entries well before the ring has been fully consumed, so
 * we're being *really* paranoid here.
 */
-   msleep(1);
+   mdelay(1);
+   qm_mr_pvb_update(p);
msg = qm_mr_current(p);
if (!msg)
return 0;
-- 
2.7.4



[PATCH v3 3/7] soc/fsl/qbman: Cleanup QMan queues if device was already initialized

2019-08-01 Thread Roy Pledge
If the QMan device was previously initialized make sure all the
frame queues are out of service once all the portals are probed.
This handles the case where the kernel is restarted without the
SoC being reset (kexec for example)

Signed-off-by: Roy Pledge 
---
 drivers/soc/fsl/qbman/qman.c|  4 ++--
 drivers/soc/fsl/qbman/qman_ccsr.c   | 13 -
 drivers/soc/fsl/qbman/qman_portal.c | 18 +-
 drivers/soc/fsl/qbman/qman_priv.h   |  7 +++
 4 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/drivers/soc/fsl/qbman/qman.c b/drivers/soc/fsl/qbman/qman.c
index 636f83f..f10f77d 100644
--- a/drivers/soc/fsl/qbman/qman.c
+++ b/drivers/soc/fsl/qbman/qman.c
@@ -2581,7 +2581,7 @@ static int _qm_dqrr_consume_and_match(struct qm_portal 
*p, u32 fqid, int s,
 #define qm_dqrr_drain_nomatch(p) \
_qm_dqrr_consume_and_match(p, 0, 0, false)
 
-static int qman_shutdown_fq(u32 fqid)
+int qman_shutdown_fq(u32 fqid)
 {
struct qman_portal *p;
struct device *dev;
@@ -2754,7 +2754,7 @@ static int qman_shutdown_fq(u32 fqid)
 
DPAA_ASSERT((mcr->verb & QM_MCR_VERB_MASK) ==
QM_MCR_VERB_ALTER_OOS);
-   if (mcr->result) {
+   if (mcr->result != QM_MCR_RESULT_OK) {
dev_err(dev, "OOS fail: FQ 0x%x (0x%x)\n",
fqid, mcr->result);
ret = -EIO;
diff --git a/drivers/soc/fsl/qbman/qman_ccsr.c 
b/drivers/soc/fsl/qbman/qman_ccsr.c
index a3edefa..709661b7b 100644
--- a/drivers/soc/fsl/qbman/qman_ccsr.c
+++ b/drivers/soc/fsl/qbman/qman_ccsr.c
@@ -492,7 +492,7 @@ RESERVEDMEM_OF_DECLARE(qman_pfdr, "fsl,qman-pfdr", 
qman_pfdr);
 
 #endif
 
-static unsigned int qm_get_fqid_maxcnt(void)
+unsigned int qm_get_fqid_maxcnt(void)
 {
return fqd_sz / 64;
 }
@@ -737,6 +737,17 @@ int qman_is_probed(void)
 }
 EXPORT_SYMBOL_GPL(qman_is_probed);
 
+int qman_requires_cleanup(void)
+{
+   return __qman_requires_cleanup;
+}
+
+void qman_done_cleanup(void)
+{
+   __qman_requires_cleanup = 0;
+}
+
+
 static int fsl_qman_probe(struct platform_device *pdev)
 {
struct device *dev = >dev;
diff --git a/drivers/soc/fsl/qbman/qman_portal.c 
b/drivers/soc/fsl/qbman/qman_portal.c
index 75717bc..153727c 100644
--- a/drivers/soc/fsl/qbman/qman_portal.c
+++ b/drivers/soc/fsl/qbman/qman_portal.c
@@ -233,7 +233,7 @@ static int qman_portal_probe(struct platform_device *pdev)
struct device_node *node = dev->of_node;
struct qm_portal_config *pcfg;
struct resource *addr_phys[2];
-   int irq, cpu, err;
+   int irq, cpu, err, i;
u32 val;
 
err = qman_is_probed();
@@ -328,6 +328,22 @@ static int qman_portal_probe(struct platform_device *pdev)
if (!cpu_online(cpu))
qman_offline_cpu(cpu);
 
+   if (__qman_portals_probed == 1 && qman_requires_cleanup()) {
+   /*
+* QMan wasn't reset prior to boot (Kexec for example)
+* Empty all the frame queues so they are in reset state
+*/
+   for (i = 0; i < qm_get_fqid_maxcnt(); i++) {
+   err =  qman_shutdown_fq(i);
+   if (err) {
+   dev_err(dev, "Failed to shutdown frame queue 
%d\n",
+   i);
+   goto err_portal_init;
+   }
+   }
+   qman_done_cleanup();
+   }
+
return 0;
 
 err_portal_init:
diff --git a/drivers/soc/fsl/qbman/qman_priv.h 
b/drivers/soc/fsl/qbman/qman_priv.h
index 0451571..a8a35fe 100644
--- a/drivers/soc/fsl/qbman/qman_priv.h
+++ b/drivers/soc/fsl/qbman/qman_priv.h
@@ -272,3 +272,10 @@ extern struct qman_portal *affine_portals[NR_CPUS];
 extern struct qman_portal *qman_dma_portal;
 const struct qm_portal_config *qman_get_qm_portal_config(
struct qman_portal *portal);
+
+unsigned int qm_get_fqid_maxcnt(void);
+
+int qman_shutdown_fq(u32 fqid);
+
+int qman_requires_cleanup(void);
+void qman_done_cleanup(void);
-- 
2.7.4



[PATCH v3 2/7] soc/fsl/qbman: Cleanup buffer pools if BMan was initialized prior to bootup

2019-08-01 Thread Roy Pledge
Clean the BMan buffer pools if the device had been initialized
previously.  This will ensure a consistent state if the kernel
was soft restarted (kexec for example)

Signed-off-by: Roy Pledge 
---
 drivers/soc/fsl/qbman/bman.c| 17 +
 drivers/soc/fsl/qbman/bman_ccsr.c   | 10 ++
 drivers/soc/fsl/qbman/bman_portal.c | 18 +-
 drivers/soc/fsl/qbman/bman_priv.h   |  5 +
 4 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/drivers/soc/fsl/qbman/bman.c b/drivers/soc/fsl/qbman/bman.c
index f84ab59..f4fb527 100644
--- a/drivers/soc/fsl/qbman/bman.c
+++ b/drivers/soc/fsl/qbman/bman.c
@@ -635,30 +635,31 @@ int bman_p_irqsource_add(struct bman_portal *p, u32 bits)
return 0;
 }
 
-static int bm_shutdown_pool(u32 bpid)
+int bm_shutdown_pool(u32 bpid)
 {
+   int err = 0;
struct bm_mc_command *bm_cmd;
union bm_mc_result *bm_res;
 
+
+   struct bman_portal *p = get_affine_portal();
while (1) {
-   struct bman_portal *p = get_affine_portal();
/* Acquire buffers until empty */
bm_cmd = bm_mc_start(>p);
bm_cmd->bpid = bpid;
bm_mc_commit(>p, BM_MCC_VERB_CMD_ACQUIRE | 1);
if (!bm_mc_result_timeout(>p, _res)) {
-   put_affine_portal();
pr_crit("BMan Acquire Command timedout\n");
-   return -ETIMEDOUT;
+   err = -ETIMEDOUT;
+   goto done;
}
if (!(bm_res->verb & BM_MCR_VERB_ACQUIRE_BUFCOUNT)) {
-   put_affine_portal();
/* Pool is empty */
-   return 0;
+   goto done;
}
-   put_affine_portal();
}
-
+done:
+   put_affine_portal();
return 0;
 }
 
diff --git a/drivers/soc/fsl/qbman/bman_ccsr.c 
b/drivers/soc/fsl/qbman/bman_ccsr.c
index dc6d7e5..cb24a08 100644
--- a/drivers/soc/fsl/qbman/bman_ccsr.c
+++ b/drivers/soc/fsl/qbman/bman_ccsr.c
@@ -195,6 +195,16 @@ int bman_is_probed(void)
 }
 EXPORT_SYMBOL_GPL(bman_is_probed);
 
+int bman_requires_cleanup(void)
+{
+   return __bman_requires_cleanup;
+}
+
+void bman_done_cleanup(void)
+{
+   __bman_requires_cleanup = 0;
+}
+
 static int fsl_bman_probe(struct platform_device *pdev)
 {
int ret, err_irq;
diff --git a/drivers/soc/fsl/qbman/bman_portal.c 
b/drivers/soc/fsl/qbman/bman_portal.c
index c78cc69..cc06d95 100644
--- a/drivers/soc/fsl/qbman/bman_portal.c
+++ b/drivers/soc/fsl/qbman/bman_portal.c
@@ -100,7 +100,7 @@ static int bman_portal_probe(struct platform_device *pdev)
struct device_node *node = dev->of_node;
struct bm_portal_config *pcfg;
struct resource *addr_phys[2];
-   int irq, cpu, err;
+   int irq, cpu, err, i;
 
err = bman_is_probed();
if (!err)
@@ -181,6 +181,22 @@ static int bman_portal_probe(struct platform_device *pdev)
if (!cpu_online(cpu))
bman_offline_cpu(cpu);
 
+   if (__bman_portals_probed == 1 && bman_requires_cleanup()) {
+   /*
+* BMan wasn't reset prior to boot (Kexec for example)
+* Empty all the buffer pools so they are in reset state
+*/
+   for (i = 0; i < BM_POOL_MAX; i++) {
+   err =  bm_shutdown_pool(i);
+   if (err) {
+   dev_err(dev, "Failed to shutdown bpool %d\n",
+   i);
+   goto err_portal_init;
+   }
+   }
+   bman_done_cleanup();
+   }
+
return 0;
 
 err_portal_init:
diff --git a/drivers/soc/fsl/qbman/bman_priv.h 
b/drivers/soc/fsl/qbman/bman_priv.h
index 751ce90..aa3981e 100644
--- a/drivers/soc/fsl/qbman/bman_priv.h
+++ b/drivers/soc/fsl/qbman/bman_priv.h
@@ -76,3 +76,8 @@ int bman_p_irqsource_add(struct bman_portal *p, u32 bits);
 
 const struct bm_portal_config *
 bman_get_bm_portal_config(const struct bman_portal *portal);
+
+int bman_requires_cleanup(void);
+void bman_done_cleanup(void);
+
+int bm_shutdown_pool(u32 bpid);
-- 
2.7.4



[PATCH v3 1/7] soc/fsl/qbman: Rework QBMan private memory setup

2019-08-01 Thread Roy Pledge
Rework QBMan private memory setup so that the areas are not
zeroed if the device was previously initialized

If the QMan private memory was already initialized skip the PFDR
initialization.

Signed-off-by: Roy Pledge 
---
 drivers/soc/fsl/qbman/bman_ccsr.c | 26 +--
 drivers/soc/fsl/qbman/dpaa_sys.c  |  7 +++--
 drivers/soc/fsl/qbman/qman_ccsr.c | 54 +++
 3 files changed, 76 insertions(+), 11 deletions(-)

diff --git a/drivers/soc/fsl/qbman/bman_ccsr.c 
b/drivers/soc/fsl/qbman/bman_ccsr.c
index 7c3cc96..dc6d7e5 100644
--- a/drivers/soc/fsl/qbman/bman_ccsr.c
+++ b/drivers/soc/fsl/qbman/bman_ccsr.c
@@ -97,17 +97,40 @@ static void bm_get_version(u16 *id, u8 *major, u8 *minor)
 /* signal transactions for FBPRs with higher priority */
 #define FBPR_AR_RPRIO_HI BIT(30)
 
-static void bm_set_memory(u64 ba, u32 size)
+/* Track if probe has occurred and if cleanup is required */
+static int __bman_probed;
+static int __bman_requires_cleanup;
+
+
+static int bm_set_memory(u64 ba, u32 size)
 {
+   u32 bar, bare;
u32 exp = ilog2(size);
/* choke if size isn't within range */
DPAA_ASSERT(size >= 4096 && size <= 1024*1024*1024 &&
   is_power_of_2(size));
/* choke if '[e]ba' has lower-alignment than 'size' */
DPAA_ASSERT(!(ba & (size - 1)));
+
+   /* Check to see if BMan has already been initialized */
+   bar = bm_ccsr_in(REG_FBPR_BAR);
+   if (bar) {
+   /* Maker sure ba == what was programmed) */
+   bare = bm_ccsr_in(REG_FBPR_BARE);
+   if (bare != upper_32_bits(ba) || bar != lower_32_bits(ba)) {
+   pr_err("Attempted to reinitialize BMan with different 
BAR, got 0x%llx read BARE=0x%x BAR=0x%x\n",
+  ba, bare, bar);
+   return -ENOMEM;
+   }
+   pr_info("BMan BAR already configured\n");
+   __bman_requires_cleanup = 1;
+   return 1;
+   }
+
bm_ccsr_out(REG_FBPR_BARE, upper_32_bits(ba));
bm_ccsr_out(REG_FBPR_BAR, lower_32_bits(ba));
bm_ccsr_out(REG_FBPR_AR, exp - 1);
+   return 0;
 }
 
 /*
@@ -120,7 +143,6 @@ static void bm_set_memory(u64 ba, u32 size)
  */
 static dma_addr_t fbpr_a;
 static size_t fbpr_sz;
-static int __bman_probed;
 
 static int bman_fbpr(struct reserved_mem *rmem)
 {
diff --git a/drivers/soc/fsl/qbman/dpaa_sys.c b/drivers/soc/fsl/qbman/dpaa_sys.c
index e6d48dc..3e0a7f3 100644
--- a/drivers/soc/fsl/qbman/dpaa_sys.c
+++ b/drivers/soc/fsl/qbman/dpaa_sys.c
@@ -40,6 +40,7 @@ int qbman_init_private_mem(struct device *dev, int idx, 
dma_addr_t *addr,
int ret;
struct device_node *mem_node;
u64 size64;
+   struct reserved_mem *rmem;
 
ret = of_reserved_mem_device_init_by_idx(dev, dev->of_node, idx);
if (ret) {
@@ -62,10 +63,8 @@ int qbman_init_private_mem(struct device *dev, int idx, 
dma_addr_t *addr,
return -ENODEV;
}
 
-   if (!dma_alloc_coherent(dev, *size, addr, 0)) {
-   dev_err(dev, "DMA Alloc memory failed\n");
-   return -ENODEV;
-   }
+   rmem = of_reserved_mem_lookup(mem_node);
+   *addr = rmem->base;
 
/*
 * Disassociate the reserved memory area from the device
diff --git a/drivers/soc/fsl/qbman/qman_ccsr.c 
b/drivers/soc/fsl/qbman/qman_ccsr.c
index a6bb430..a3edefa 100644
--- a/drivers/soc/fsl/qbman/qman_ccsr.c
+++ b/drivers/soc/fsl/qbman/qman_ccsr.c
@@ -274,6 +274,7 @@ static u32 __iomem *qm_ccsr_start;
 /* A SDQCR mask comprising all the available/visible pool channels */
 static u32 qm_pools_sdqcr;
 static int __qman_probed;
+static int  __qman_requires_cleanup;
 
 static inline u32 qm_ccsr_in(u32 offset)
 {
@@ -340,19 +341,55 @@ static void qm_get_version(u16 *id, u8 *major, u8 *minor)
 }
 
 #define PFDR_AR_EN BIT(31)
-static void qm_set_memory(enum qm_memory memory, u64 ba, u32 size)
+static int qm_set_memory(enum qm_memory memory, u64 ba, u32 size)
 {
+   void *ptr;
u32 offset = (memory == qm_memory_fqd) ? REG_FQD_BARE : REG_PFDR_BARE;
u32 exp = ilog2(size);
+   u32 bar, bare;
 
/* choke if size isn't within range */
DPAA_ASSERT((size >= 4096) && (size <= 1024*1024*1024) &&
is_power_of_2(size));
/* choke if 'ba' has lower-alignment than 'size' */
DPAA_ASSERT(!(ba & (size - 1)));
+
+   /* Check to see if QMan has already been initialized */
+   bar = qm_ccsr_in(offset + REG_offset_BAR);
+   if (bar) {
+   /* Maker sure ba == what was programmed) */
+   bare = qm_ccsr_in(offset);
+   if (bare != upper_32_bits(ba) || bar != lower_32_bits(ba)) {
+   pr_err("Attempted to reinitialize QMan with different 
BAR, got 0x%llx read BARE=0x%x BAR=0x%x\n",
+  ba, bare, bar);
+   

[PATCH v3 0/7] soc/fsl/qbman: Enable Kexec for DPAA1 devices

2019-08-01 Thread Roy Pledge
Most DPAA1 devices do not support a soft reset which is an issue if
Kexec starts a new kernel. This patch series allows Kexec to function
by detecting that the QBMan device was previously initialized.

The patches fix some issues with device cleanup as well as ensuring
that the location of the QBMan private memories has not changed
after the execution of the Kexec.

Changes since v1:
- Removed a bug fix and sent it separately to ease backporting
Changes since v2:
- Expliciitly flush FQD memory from cache on PPC before unmapping

Roy Pledge (7):
  soc/fsl/qbman: Rework QBMan private memory setup
  soc/fsl/qbman: Cleanup buffer pools if BMan was initialized prior to
bootup
  soc/fsl/qbman: Cleanup QMan queues if device was already initialized
  soc/fsl/qbman: Fix drain_mr_fqni()
  soc/fsl/qbman: Disable interrupts during portal recovery
  soc/fsl/qbman: Fixup qman_shutdown_fq()
  soc/fsl/qbman: Update device tree with reserved memory

 drivers/soc/fsl/qbman/bman.c| 17 
 drivers/soc/fsl/qbman/bman_ccsr.c   | 36 +++-
 drivers/soc/fsl/qbman/bman_portal.c | 18 +++-
 drivers/soc/fsl/qbman/bman_priv.h   |  5 +++
 drivers/soc/fsl/qbman/dpaa_sys.c| 63 
 drivers/soc/fsl/qbman/qman.c| 83 +
 drivers/soc/fsl/qbman/qman_ccsr.c   | 68 +++---
 drivers/soc/fsl/qbman/qman_portal.c | 18 +++-
 drivers/soc/fsl/qbman/qman_priv.h   |  8 
 9 files changed, 255 insertions(+), 61 deletions(-)

--
2.7.4



Re: [PATCH] selftests/powerpc: Fix build failures with GCC 9

2019-08-01 Thread Segher Boessenkool
On Thu, Aug 01, 2019 at 10:26:28PM +1000, Michael Ellerman wrote:
> GCC 9 fails to build some of the ptrace TM tests, with errors such as:
> 
>   ptrace-tm-spd-vsx.c: In function 'tm_spd_vsx':
>   ptrace-tm-spd-vsx.c:51:2: error: listing the stack pointer register 'r1' in 
> a clobber list is deprecated [-Werror=deprecated]
>  51 |  asm __volatile__(
> |  ^~~
>   ptrace-tm-spd-vsx.c:51:2: note: the value of the stack pointer after an 
> 'asm' statement must be the same as it was before the statement
> 
> Which is probably fair enough.

Maybe you shouldn't build the tests with -Werror though?  (And you could
include the much more useful -Wextra while you're at it ;-) ).

> Some of these inline asm blocks are doing quite a lot and are probably
> pushing the boundaries of what's sane to do with inline asm,

These are just testcases, you sometimes need to do evil things there.
But yeah :-)

> but they shouldn't actually be returning with r1 modified.

But they *do* modify lr, and that one isnt't listed; I guess the r1
clobber was there because of the call in the asm, but that needs an
lr clobber, instead.

(Or it was because of the "or 1,1,1")?


Segher


Re: [PATCH] dma-mapping: fix page attributes for dma_mmap_*

2019-08-01 Thread Will Deacon
On Thu, Aug 01, 2019 at 06:34:57PM +0200, Christoph Hellwig wrote:
> On Thu, Aug 01, 2019 at 05:23:06PM +0100, Will Deacon wrote:
> > > - if (!dev_is_dma_coherent(dev) || (attrs & DMA_ATTR_WRITE_COMBINE))
> > > - return pgprot_writecombine(prot);
> > > - return prot;
> > > + return pgprot_writecombine(prot);
> > >  }
> > 
> > Seems like a sensible cleanup to me:
> > 
> > Acked-by: Will Deacon 
> > 
> > Although arch_dma_mmap_pgprot() is a bit of a misnomer now that it only
> > gets involved in the non-coherent case.
> 
> A better name is welcome.

How about arch_dma_noncoherent_mmap_pgprot() ? Too long?

> My other idea would be to just remove it entirely and do something like:
> 
> #ifndef pgprot_dmacoherent
> #define pgprot_dmacoherent pgprot_noncached
> #endif
> 
> pgprot_t dma_mmap_pgprot(struct device *dev, pgprot_t prot, unsigned long 
> attrs)
> {
>   if (dev_is_dma_coherent(dev) || (attrs & DMA_ATTR_NON_CONSISTENT))
>   return prot;
> #ifdef pgprot_writecombine
>   if (attrs & DMA_ATTR_WRITE_COMBINE)
>   return pgprot_writecombine(prot);
> #endif
>   return pgprot_dmacoherent(prot);
> }

Oh, I prefer that!

> But my worry is how this interacts with architectures that have an
> uncached segment (mips, nios2, microblaze, extensa) where we'd have
> the kernel access DMA_ATTR_WRITE_COMBINE mappigns using the uncached
> segment, and userspace mmaps using pgprot_writecombine, which could
> lead to aliasing issues.  But then again mips already supports
> DMA_ATTR_WRITE_COMBINE, so this must be ok somehow.  I guess I'll
> need to field that question to the relevant parties.

Or it's always been busted and happens to work out in practice...

Will


Re: [PATCH] dma-mapping: fix page attributes for dma_mmap_*

2019-08-01 Thread Christoph Hellwig
On Thu, Aug 01, 2019 at 05:23:06PM +0100, Will Deacon wrote:
> > -   if (!dev_is_dma_coherent(dev) || (attrs & DMA_ATTR_WRITE_COMBINE))
> > -   return pgprot_writecombine(prot);
> > -   return prot;
> > +   return pgprot_writecombine(prot);
> >  }
> 
> Seems like a sensible cleanup to me:
> 
> Acked-by: Will Deacon 
> 
> Although arch_dma_mmap_pgprot() is a bit of a misnomer now that it only
> gets involved in the non-coherent case.

A better name is welcome.  My other idea would be to just remove it
entirely and do something like:

#ifndef pgprot_dmacoherent
#define pgprot_dmacoherent pgprot_noncached
#endif

pgprot_t dma_mmap_pgprot(struct device *dev, pgprot_t prot, unsigned long attrs)
{
if (dev_is_dma_coherent(dev) || (attrs & DMA_ATTR_NON_CONSISTENT))
return prot;
#ifdef pgprot_writecombine
if (attrs & DMA_ATTR_WRITE_COMBINE)
return pgprot_writecombine(prot);
#endif
return pgprot_dmacoherent(prot);
}

But my worry is how this interacts with architectures that have an
uncached segment (mips, nios2, microblaze, extensa) where we'd have
the kernel access DMA_ATTR_WRITE_COMBINE mappigns using the uncached
segment, and userspace mmaps using pgprot_writecombine, which could
lead to aliasing issues.  But then again mips already supports
DMA_ATTR_WRITE_COMBINE, so this must be ok somehow.  I guess I'll
need to field that question to the relevant parties.


Re: [PATCH] dma-mapping: fix page attributes for dma_mmap_*

2019-08-01 Thread Will Deacon
On Thu, Aug 01, 2019 at 05:21:18PM +0300, Christoph Hellwig wrote:
> All the way back to introducing dma_common_mmap we've defaulyed to mark
> the pages as uncached.  But this is wrong for DMA coherent devices or
> if using DMA_ATTR_NON_CONSISTENT.  Later on DMA_ATTR_WRITE_COMBINE
> also got incorrect treatment as that flag is only treated special on
> the alloc side for non-coherent devices.
> 
> Introduce a new dma_mmap_pgprot helper that deals with the check
> for coherent devices and DMA_ATTR_NON_CONSISTENT so that only the
> remapping cases even reach arch_dma_mmap_pgprot and we thus ensure
> no aliasing of page attributes happens.
> 
> Signed-off-by: Christoph Hellwig 
> ---
>  arch/arm/mm/dma-mapping.c|  4 +---
>  arch/arm64/mm/dma-mapping.c  |  4 +---
>  arch/powerpc/kernel/Makefile |  3 +--
>  arch/powerpc/kernel/dma-common.c | 17 -
>  drivers/iommu/dma-iommu.c|  6 +++---
>  include/linux/dma-mapping.h  |  1 +
>  include/linux/dma-noncoherent.h  |  5 -
>  kernel/dma/mapping.c | 11 ++-
>  kernel/dma/remap.c   |  2 +-
>  9 files changed, 18 insertions(+), 35 deletions(-)
>  delete mode 100644 arch/powerpc/kernel/dma-common.c
> 
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index 9c9a23e5600d..cfe44df169c5 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -2397,9 +2397,7 @@ long arch_dma_coherent_to_pfn(struct device *dev, void 
> *cpu_addr,
>  pgprot_t arch_dma_mmap_pgprot(struct device *dev, pgprot_t prot,
>   unsigned long attrs)
>  {
> - if (!dev_is_dma_coherent(dev))
> - return __get_dma_pgprot(attrs, prot);
> - return prot;
> + return __get_dma_pgprot(attrs, prot);
>  }
>  
>  void *arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *dma_handle,
> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
> index 1d3f0b5a9940..bd2b039f43a6 100644
> --- a/arch/arm64/mm/dma-mapping.c
> +++ b/arch/arm64/mm/dma-mapping.c
> @@ -14,9 +14,7 @@
>  pgprot_t arch_dma_mmap_pgprot(struct device *dev, pgprot_t prot,
>   unsigned long attrs)
>  {
> - if (!dev_is_dma_coherent(dev) || (attrs & DMA_ATTR_WRITE_COMBINE))
> - return pgprot_writecombine(prot);
> - return prot;
> + return pgprot_writecombine(prot);
>  }

Seems like a sensible cleanup to me:

Acked-by: Will Deacon 

Although arch_dma_mmap_pgprot() is a bit of a misnomer now that it only
gets involved in the non-coherent case.

Will


Re: [PATCH 6/8] dma-direct: turn ARCH_ZONE_DMA_BITS into a variable

2019-08-01 Thread Nicolas Saenz Julienne
Hi Christoph, thanks for the review.

On Thu, 2019-08-01 at 16:04 +0200, Christoph Hellwig wrote:
> A few nitpicks, otherwise this looks great:
> 
> > @@ -201,7 +202,7 @@ static int __init mark_nonram_nosave(void)
> >   * everything else. GFP_DMA32 page allocations automatically fall back to
> >   * ZONE_DMA.
> >   *
> > - * By using 31-bit unconditionally, we can exploit ARCH_ZONE_DMA_BITS to
> > + * By using 31-bit unconditionally, we can exploit arch_zone_dma_bits to
> >   * inform the generic DMA mapping code.  32-bit only devices (if not
> > handled
> >   * by an IOMMU anyway) will take a first dip into ZONE_NORMAL and get
> >   * otherwise served by ZONE_DMA.
> > @@ -237,9 +238,18 @@ void __init paging_init(void)
> > printk(KERN_DEBUG "Memory hole size: %ldMB\n",
> >(long int)((top_of_ram - total_ram) >> 20));
> >  
> > +   /*
> > +* Allow 30-bit DMA for very limited Broadcom wifi chips on many
> > +* powerbooks.
> > +*/
> > +   if (IS_ENABLED(CONFIG_PPC32))
> > +   arch_zone_dma_bits = 30;
> > +   else
> > +   arch_zone_dma_bits = 31;
> > +
> 
> So the above unconditionally comment obviously isn't true any more, and
> Ben also said for the recent ppc32 hack he'd prefer dynamic detection.
> 
> Maybe Ben and or other ppc folks can chime in an add a patch to the series
> to sort this out now that we have a dynamic ZONE_DMA threshold?

Noted, for now I'll remove the comment.

> > diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> > index 59bdceea3737..40dfc9b4ee4c 100644
> > --- a/kernel/dma/direct.c
> > +++ b/kernel/dma/direct.c
> > @@ -19,9 +19,7 @@
> >   * Most architectures use ZONE_DMA for the first 16 Megabytes, but
> >   * some use it for entirely different regions:
> >   */
> > -#ifndef ARCH_ZONE_DMA_BITS
> > -#define ARCH_ZONE_DMA_BITS 24
> > -#endif
> > +unsigned int arch_zone_dma_bits __ro_after_init = 24;
> 
> I'd prefer to drop the arch_ prefix and just calls this zone_dma_bits.
> In the long run we really need to find a way to just automatically set
> this from the meminit code, but that is out of scope for this series.
> For now can you please just update the comment above to say something
> like:
> 
> /*
>  * Most architectures use ZONE_DMA for the first 16 Megabytes, but some use it
>  * it for entirely different regions.  In that case the arch code needs to
>  * override the variable below for dma-direct to work properly.
>  */

Ok perfect.



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


Re: [PATCH v3 00/10] implement KASLR for powerpc/fsl_booke/32

2019-08-01 Thread Diana Madalina Craciun
Hi Jason,

I have tested these series on a P4080 platform.

Regards,
Diana


On 7/31/2019 12:26 PM, Jason Yan wrote:
> This series implements KASLR for powerpc/fsl_booke/32, as a security
> feature that deters exploit attempts relying on knowledge of the location
> of kernel internals.
>
> Since CONFIG_RELOCATABLE has already supported, what we need to do is
> map or copy kernel to a proper place and relocate. Freescale Book-E
> parts expect lowmem to be mapped by fixed TLB entries(TLB1). The TLB1
> entries are not suitable to map the kernel directly in a randomized
> region, so we chose to copy the kernel to a proper place and restart to
> relocate.
>
> Entropy is derived from the banner and timer base, which will change every
> build and boot. This not so much safe so additionally the bootloader may
> pass entropy via the /chosen/kaslr-seed node in device tree.
>
> We will use the first 512M of the low memory to randomize the kernel
> image. The memory will be split in 64M zones. We will use the lower 8
> bit of the entropy to decide the index of the 64M zone. Then we chose a
> 16K aligned offset inside the 64M zone to put the kernel in.
>
> KERNELBASE
>
> |-->   64M   <--|
> |   |
> +---+++---+
> |   |||kernel||   |
> +---+++---+
> | |
> |->   offset<-|
>
>   kimage_vaddr
>
> We also check if we will overlap with some areas like the dtb area, the
> initrd area or the crashkernel area. If we cannot find a proper area,
> kaslr will be disabled and boot from the original kernel.
>
> Changes since v2:
>  - Remove unnecessary #ifdef
>  - Use SZ_64M instead of0x400
>  - Call early_init_dt_scan_chosen() to init boot_command_line
>  - Rename kaslr_second_init() to kaslr_late_init()
>
> Changes since v1:
>  - Remove some useless 'extern' keyword.
>  - Replace EXPORT_SYMBOL with EXPORT_SYMBOL_GPL
>  - Improve some assembly code
>  - Use memzero_explicit instead of memset
>  - Use boot_command_line and remove early_command_line
>  - Do not print kaslr offset if kaslr is disabled
>
> Jason Yan (10):
>   powerpc: unify definition of M_IF_NEEDED
>   powerpc: move memstart_addr and kernstart_addr to init-common.c
>   powerpc: introduce kimage_vaddr to store the kernel base
>   powerpc/fsl_booke/32: introduce create_tlb_entry() helper
>   powerpc/fsl_booke/32: introduce reloc_kernel_entry() helper
>   powerpc/fsl_booke/32: implement KASLR infrastructure
>   powerpc/fsl_booke/32: randomize the kernel image offset
>   powerpc/fsl_booke/kaslr: clear the original kernel if randomized
>   powerpc/fsl_booke/kaslr: support nokaslr cmdline parameter
>   powerpc/fsl_booke/kaslr: dump out kernel offset information on panic
>
>  arch/powerpc/Kconfig  |  11 +
>  arch/powerpc/include/asm/nohash/mmu-book3e.h  |  10 +
>  arch/powerpc/include/asm/page.h   |   7 +
>  arch/powerpc/kernel/Makefile  |   1 +
>  arch/powerpc/kernel/early_32.c|   2 +-
>  arch/powerpc/kernel/exceptions-64e.S  |  10 -
>  arch/powerpc/kernel/fsl_booke_entry_mapping.S |  23 +-
>  arch/powerpc/kernel/head_fsl_booke.S  |  55 ++-
>  arch/powerpc/kernel/kaslr_booke.c | 427 ++
>  arch/powerpc/kernel/machine_kexec.c   |   1 +
>  arch/powerpc/kernel/misc_64.S |   5 -
>  arch/powerpc/kernel/setup-common.c|  19 +
>  arch/powerpc/mm/init-common.c |   7 +
>  arch/powerpc/mm/init_32.c |   5 -
>  arch/powerpc/mm/init_64.c |   5 -
>  arch/powerpc/mm/mmu_decl.h|  10 +
>  arch/powerpc/mm/nohash/fsl_booke.c|   8 +-
>  17 files changed, 558 insertions(+), 48 deletions(-)
>  create mode 100644 arch/powerpc/kernel/kaslr_booke.c
>



[PATCH] dma-mapping: fix page attributes for dma_mmap_*

2019-08-01 Thread Christoph Hellwig
All the way back to introducing dma_common_mmap we've defaulyed to mark
the pages as uncached.  But this is wrong for DMA coherent devices or
if using DMA_ATTR_NON_CONSISTENT.  Later on DMA_ATTR_WRITE_COMBINE
also got incorrect treatment as that flag is only treated special on
the alloc side for non-coherent devices.

Introduce a new dma_mmap_pgprot helper that deals with the check
for coherent devices and DMA_ATTR_NON_CONSISTENT so that only the
remapping cases even reach arch_dma_mmap_pgprot and we thus ensure
no aliasing of page attributes happens.

Signed-off-by: Christoph Hellwig 
---
 arch/arm/mm/dma-mapping.c|  4 +---
 arch/arm64/mm/dma-mapping.c  |  4 +---
 arch/powerpc/kernel/Makefile |  3 +--
 arch/powerpc/kernel/dma-common.c | 17 -
 drivers/iommu/dma-iommu.c|  6 +++---
 include/linux/dma-mapping.h  |  1 +
 include/linux/dma-noncoherent.h  |  5 -
 kernel/dma/mapping.c | 11 ++-
 kernel/dma/remap.c   |  2 +-
 9 files changed, 18 insertions(+), 35 deletions(-)
 delete mode 100644 arch/powerpc/kernel/dma-common.c

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 9c9a23e5600d..cfe44df169c5 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -2397,9 +2397,7 @@ long arch_dma_coherent_to_pfn(struct device *dev, void 
*cpu_addr,
 pgprot_t arch_dma_mmap_pgprot(struct device *dev, pgprot_t prot,
unsigned long attrs)
 {
-   if (!dev_is_dma_coherent(dev))
-   return __get_dma_pgprot(attrs, prot);
-   return prot;
+   return __get_dma_pgprot(attrs, prot);
 }
 
 void *arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *dma_handle,
diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index 1d3f0b5a9940..bd2b039f43a6 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -14,9 +14,7 @@
 pgprot_t arch_dma_mmap_pgprot(struct device *dev, pgprot_t prot,
unsigned long attrs)
 {
-   if (!dev_is_dma_coherent(dev) || (attrs & DMA_ATTR_WRITE_COMBINE))
-   return pgprot_writecombine(prot);
-   return prot;
+   return pgprot_writecombine(prot);
 }
 
 void arch_sync_dma_for_device(struct device *dev, phys_addr_t paddr,
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index ea0c69236789..56dfa7a2a6f2 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -49,8 +49,7 @@ obj-y := cputable.o ptrace.o 
syscalls.o \
   signal.o sysfs.o cacheinfo.o time.o \
   prom.o traps.o setup-common.o \
   udbg.o misc.o io.o misc_$(BITS).o \
-  of_platform.o prom_parse.o \
-  dma-common.o
+  of_platform.o prom_parse.o
 obj-$(CONFIG_PPC64)+= setup_64.o sys_ppc32.o \
   signal_64.o ptrace32.o \
   paca.o nvram_64.o firmware.o
diff --git a/arch/powerpc/kernel/dma-common.c b/arch/powerpc/kernel/dma-common.c
deleted file mode 100644
index dc7ef6b17b69..
--- a/arch/powerpc/kernel/dma-common.c
+++ /dev/null
@@ -1,17 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-or-later
-/*
- * Contains common dma routines for all powerpc platforms.
- *
- * Copyright (C) 2019 Shawn Anastasio.
- */
-
-#include 
-#include 
-
-pgprot_t arch_dma_mmap_pgprot(struct device *dev, pgprot_t prot,
-   unsigned long attrs)
-{
-   if (!dev_is_dma_coherent(dev))
-   return pgprot_noncached(prot);
-   return prot;
-}
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index a7f9c3edbcb2..703de9a7553f 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -574,7 +574,7 @@ static void *iommu_dma_alloc_remap(struct device *dev, 
size_t size,
struct iova_domain *iovad = >iovad;
bool coherent = dev_is_dma_coherent(dev);
int ioprot = dma_info_to_prot(DMA_BIDIRECTIONAL, coherent, attrs);
-   pgprot_t prot = arch_dma_mmap_pgprot(dev, PAGE_KERNEL, attrs);
+   pgprot_t prot = dma_mmap_pgprot(dev, PAGE_KERNEL, attrs);
unsigned int count, min_size, alloc_sizes = domain->pgsize_bitmap;
struct page **pages;
struct sg_table sgt;
@@ -975,7 +975,7 @@ static void *iommu_dma_alloc_pages(struct device *dev, 
size_t size,
return NULL;
 
if (IS_ENABLED(CONFIG_DMA_REMAP) && (!coherent || PageHighMem(page))) {
-   pgprot_t prot = arch_dma_mmap_pgprot(dev, PAGE_KERNEL, attrs);
+   pgprot_t prot = dma_mmap_pgprot(dev, PAGE_KERNEL, attrs);
 
cpu_addr = dma_common_contiguous_remap(page, alloc_size,
VM_USERMAP, prot, __builtin_return_address(0));
@@ -1035,7 +1035,7 @@ static int 

fix default dma_mmap_* pgprot

2019-08-01 Thread Christoph Hellwig
Hi all,

As Shawn pointed out we've had issues with the dma mmap pgprots ever
since the dma_common_mmap helper was added beyong the initial
architectures - we default to uncached mappings, but for devices that
are DMA coherent, or if the DMA_ATTR_NON_CONSISTENT is set (and
supported) this can lead to aliasing of cache attributes.  This patch
fixes that.  My explanation of why this hasn't been much of an issue
is that the dma_mmap_ helpers aren't used widely and mostly just in
architecture specific drivers.


Re: [PATCH 6/8] dma-direct: turn ARCH_ZONE_DMA_BITS into a variable

2019-08-01 Thread Christoph Hellwig
A few nitpicks, otherwise this looks great:

> @@ -201,7 +202,7 @@ static int __init mark_nonram_nosave(void)
>   * everything else. GFP_DMA32 page allocations automatically fall back to
>   * ZONE_DMA.
>   *
> - * By using 31-bit unconditionally, we can exploit ARCH_ZONE_DMA_BITS to
> + * By using 31-bit unconditionally, we can exploit arch_zone_dma_bits to
>   * inform the generic DMA mapping code.  32-bit only devices (if not handled
>   * by an IOMMU anyway) will take a first dip into ZONE_NORMAL and get
>   * otherwise served by ZONE_DMA.
> @@ -237,9 +238,18 @@ void __init paging_init(void)
>   printk(KERN_DEBUG "Memory hole size: %ldMB\n",
>  (long int)((top_of_ram - total_ram) >> 20));
>  
> + /*
> +  * Allow 30-bit DMA for very limited Broadcom wifi chips on many
> +  * powerbooks.
> +  */
> + if (IS_ENABLED(CONFIG_PPC32))
> + arch_zone_dma_bits = 30;
> + else
> + arch_zone_dma_bits = 31;
> +

So the above unconditionally comment obviously isn't true any more, and
Ben also said for the recent ppc32 hack he'd prefer dynamic detection.

Maybe Ben and or other ppc folks can chime in an add a patch to the series
to sort this out now that we have a dynamic ZONE_DMA threshold?

> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 59bdceea3737..40dfc9b4ee4c 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -19,9 +19,7 @@
>   * Most architectures use ZONE_DMA for the first 16 Megabytes, but
>   * some use it for entirely different regions:
>   */
> -#ifndef ARCH_ZONE_DMA_BITS
> -#define ARCH_ZONE_DMA_BITS 24
> -#endif
> +unsigned int arch_zone_dma_bits __ro_after_init = 24;

I'd prefer to drop the arch_ prefix and just calls this zone_dma_bits.
In the long run we really need to find a way to just automatically set
this from the meminit code, but that is out of scope for this series.
For now can you please just update the comment above to say something
like:

/*
 * Most architectures use ZONE_DMA for the first 16 Megabytes, but some use it
 * it for entirely different regions.  In that case the arch code needs to
 * override the variable below for dma-direct to work properly.
 */


[Bug 204371] BUG kmalloc-4k (Tainted: G W ): Object padding overwritten

2019-08-01 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=204371

--- Comment #5 from Erhard F. (erhar...@mailbox.org) ---
On Wed, 31 Jul 2019 12:09:54 +
bugzilla-dae...@bugzilla.kernel.org wrote:

> https://bugzilla.kernel.org/show_bug.cgi?id=204371
> 
> --- Comment #4 from m...@ellerman.id.au ---
>
> > I suspect proc_cgroup_show() is innocent and that perhaps
> > bpf_prepare_filter() had a memory scribble.  iirc there has been at
> > least one recent pretty serious bpf fix applied recently.  Can others
> > please take a look?  
> 
> I haven't been able to reproduce this on a 64-bit or 32-bit powerpc
> machine here. But I don't run gentoo userspace, so I suspect I'm not
> tripping the same path at boot. I did run the seccomp selftest and that
> didn't trip it either.
> 
> cheers

Doing some fiddling around on another bug (bug #204375), I noticed that I get
this "kmalloc-4k (Tainted: G W ): Object padding overwritten" during boot only
when I boot from my btrfs partition, but not from my other ext4 partition. The
ext4 partition is not a clone, but pretty much the same stuff in the same
versions. My btrfs root is mounted with 'lazytime,compress=zstd:1', systemd is
242.

I built a 5.2.5 kernel on the Talos II with CONFIG_SLUB_DEBUG=y but here I
don't hit the bug, even if I boot from a btrfs partition with the same
settings. Have to test it on the G5 yet (kernel .config more similar to the G4
one than the Talos II one).

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[PATCH] selftests/powerpc: Fix build failures with GCC 9

2019-08-01 Thread Michael Ellerman
GCC 9 fails to build some of the ptrace TM tests, with errors such as:

  ptrace-tm-spd-vsx.c: In function 'tm_spd_vsx':
  ptrace-tm-spd-vsx.c:51:2: error: listing the stack pointer register 'r1' in a 
clobber list is deprecated [-Werror=deprecated]
 51 |  asm __volatile__(
|  ^~~
  ptrace-tm-spd-vsx.c:51:2: note: the value of the stack pointer after an 'asm' 
statement must be the same as it was before the statement

Which is probably fair enough.

Some of these inline asm blocks are doing quite a lot and are probably
pushing the boundaries of what's sane to do with inline asm, but they
shouldn't actually be returning with r1 modified.

So drop r1 from the clobbers for now, we should probably rewrite them
to be real asm functions at some point.

Signed-off-by: Michael Ellerman 
---
 tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-tar.c | 2 +-
 tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-vsx.c | 3 +--
 tools/testing/selftests/powerpc/ptrace/ptrace-tm-tar.c | 2 +-
 tools/testing/selftests/powerpc/ptrace/ptrace-tm-vsx.c | 3 +--
 4 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-tar.c 
b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-tar.c
index 25e23e73c72e..7b835ef4f8a6 100644
--- a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-tar.c
+++ b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-tar.c
@@ -73,7 +73,7 @@ void tm_spd_tar(void)
[sprn_texasr]"i"(SPRN_TEXASR), [tar_1]"i"(TAR_1),
[dscr_1]"i"(DSCR_1), [tar_2]"i"(TAR_2), [dscr_2]"i"(DSCR_2),
[tar_3]"i"(TAR_3), [dscr_3]"i"(DSCR_3)
-   : "memory", "r0", "r1", "r3", "r4", "r5", "r6"
+   : "memory", "r0", "r3", "r4", "r5", "r6"
);
 
/* TM failed, analyse */
diff --git a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-vsx.c 
b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-vsx.c
index f603fe5a445b..724e5aa499cd 100644
--- a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-vsx.c
+++ b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-spd-vsx.c
@@ -74,8 +74,7 @@ void tm_spd_vsx(void)
"3: ;"
: [res] "=r" (result), [texasr] "=r" (texasr)
: [sprn_texasr] "i"  (SPRN_TEXASR)
-   : "memory", "r0", "r1", "r3", "r4",
-   "r7", "r8", "r9", "r10", "r11"
+   : "memory", "r0", "r3", "r4", "r7", "r8", "r9", "r10", "r11"
);
 
if (result) {
diff --git a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-tar.c 
b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-tar.c
index e0d37f07bdeb..46ef378a15ec 100644
--- a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-tar.c
+++ b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-tar.c
@@ -62,7 +62,7 @@ void tm_tar(void)
[sprn_ppr]"i"(SPRN_PPR), [sprn_texasr]"i"(SPRN_TEXASR),
[tar_1]"i"(TAR_1), [dscr_1]"i"(DSCR_1), [tar_2]"i"(TAR_2),
[dscr_2]"i"(DSCR_2), [cptr1] "b" ([1])
-   : "memory", "r0", "r1", "r3", "r4", "r5", "r6"
+   : "memory", "r0", "r3", "r4", "r5", "r6"
);
 
/* TM failed, analyse */
diff --git a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-vsx.c 
b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-vsx.c
index 8027457b97b7..9f16f3a74e28 100644
--- a/tools/testing/selftests/powerpc/ptrace/ptrace-tm-vsx.c
+++ b/tools/testing/selftests/powerpc/ptrace/ptrace-tm-vsx.c
@@ -62,8 +62,7 @@ void tm_vsx(void)
"3: ;"
: [res] "=r" (result), [texasr] "=r" (texasr)
: [sprn_texasr] "i"  (SPRN_TEXASR), [cptr1] "b" ([1])
-   : "memory", "r0", "r1", "r3", "r4",
-   "r7", "r8", "r9", "r10", "r11"
+   : "memory", "r0", "r3", "r4", "r7", "r8", "r9", "r10", "r11"
);
 
if (result) {
-- 
2.21.0



[Bug 204375] kernel 5.2.4 w. KASAN enabled fails to boot on a PowerMac G4 3,6 at very early stage

2019-08-01 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=204375

--- Comment #13 from Erhard F. (erhar...@mailbox.org) ---
Thanks for the hint! But adding KASAN_SANITIZE := n to lib/mpi/Makefile did not
make a change in this case.

However I had the idea to disable btrfs (at all) and boot from my other ext4
partition. This worked out and I got further. After other stacktraces flying by
the boot process now stalls at offb switching over to radeondrmfb. Again reboot
after timer kicks in after 10min.

Seems btrfs is not in the best condition on ppc32 with 4K pagesize..

I would test this on the G5 or Talos II too, but there is no kernel option to
enable 'KASAN: runtime memory debugging' on ppc64.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

Re: [PATCH] powerpc/xive: Add some error handling code to 'xive_spapr_init()'

2019-08-01 Thread Cédric Le Goater
On 01/08/2019 13:09, Christophe JAILLET wrote:
> 'xive_irq_bitmap_add()' can return -ENOMEM.
> In this case, we should free the memory already allocated and return
> 'false' to the caller.
> 
> Also add an error path which undoes the 'tima = ioremap(...)'
> 
> Signed-off-by: Christophe JAILLET 
> ---
> NOT compile tested (I don't have a cross compiler and won't install one).

All distros have a packaged powerpc cross compiler. 

Then, you need to compile a kernel for a pseries machine and run a pseries
machine with it under QEMU. You can use a simple ppc initrd, a net install 
one for example.

You could also hack the device tree in QEMU to torture the XIVE sPAPR driver.
Nothing too complex, all is here : 

https://git.qemu.org/?p=qemu.git;a=blob;f=hw/intc/spapr_xive.c;h=097f88d4608d8ba160526756a3a224e5176b6e0f;hb=HEAD#l1427


> So if some correction or improvement are needed, feel free to propose and
> commit it directly.

Yes there is I think. I would move at the end all the code that needs a 
rollback.

Thanks for taking a look, I might do that one day.

Cheers,
C.  


> ---
>  arch/powerpc/sysdev/xive/spapr.c | 36 +---
>  1 file changed, 28 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/powerpc/sysdev/xive/spapr.c 
> b/arch/powerpc/sysdev/xive/spapr.c
> index 52198131c75e..b3ae0b76c433 100644
> --- a/arch/powerpc/sysdev/xive/spapr.c
> +++ b/arch/powerpc/sysdev/xive/spapr.c
> @@ -64,6 +64,17 @@ static int xive_irq_bitmap_add(int base, int count)
>   return 0;
>  }
>  
> +static void xive_irq_bitmap_remove_all(void)
> +{
> + struct xive_irq_bitmap *xibm, *tmp;
> +
> + list_for_each_entry_safe(xibm, tmp, _irq_bitmaps, list) {
> + list_del(>list);
> + kfree(xibm->bitmap);
> + kfree(xibm);
> + }
> +}
> +
>  static int __xive_irq_bitmap_alloc(struct xive_irq_bitmap *xibm)
>  {
>   int irq;
> @@ -723,7 +734,7 @@ bool __init xive_spapr_init(void)
>   u32 val;
>   u32 len;
>   const __be32 *reg;
> - int i;
> + int i, err;
>  
>   if (xive_spapr_disabled())
>   return false;
> @@ -748,23 +759,26 @@ bool __init xive_spapr_init(void)
>   }
>  
>   if (!xive_get_max_prio(_prio))
> - return false;
> + goto err_unmap;
>  
>   /* Feed the IRQ number allocator with the ranges given in the DT */
>   reg = of_get_property(np, "ibm,xive-lisn-ranges", );
>   if (!reg) {
>   pr_err("Failed to read 'ibm,xive-lisn-ranges' property\n");
> - return false;
> + goto err_unmap;
>   }
>  
>   if (len % (2 * sizeof(u32)) != 0) {
>   pr_err("invalid 'ibm,xive-lisn-ranges' property\n");
> - return false;
> + goto err_unmap;
>   }
>  
> - for (i = 0; i < len / (2 * sizeof(u32)); i++, reg += 2)
> - xive_irq_bitmap_add(be32_to_cpu(reg[0]),
> - be32_to_cpu(reg[1]));
> + for (i = 0; i < len / (2 * sizeof(u32)); i++, reg += 2) {
> + err = xive_irq_bitmap_add(be32_to_cpu(reg[0]),
> +   be32_to_cpu(reg[1]));
> + if (err < 0)
> + goto err_mem_free;
> + }
>  
>   /* Iterate the EQ sizes and pick one */
>   of_property_for_each_u32(np, "ibm,xive-eq-sizes", prop, reg, val) {
> @@ -775,8 +789,14 @@ bool __init xive_spapr_init(void)
>  
>   /* Initialize XIVE core with our backend */
>   if (!xive_core_init(_spapr_ops, tima, TM_QW1_OS, max_prio))
> - return false;
> + goto err_mem_free;
>  
>   pr_info("Using %dkB queues\n", 1 << (xive_queue_shift - 10));
>   return true;
> +
> +err_mem_free:
> + xive_irq_bitmap_remove_all();
> +err_unmap:
> + iounmap(tima);
> + return false;
>  }
> 



[PATCH] powerpc/mce: Schedule work from irq_work

2019-08-01 Thread Santosh Sivaraj
schedule_work() cannot be called from MCE exception context as MCE can
interrupt even in interrupt disabled context.

fixes: 733e4a4c ("powerpc/mce: hookup memory_failure for UE errors")
Signed-off-by: Santosh Sivaraj 
---
 arch/powerpc/kernel/mce.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
index b18df633eae9..0ab6fa7c 100644
--- a/arch/powerpc/kernel/mce.c
+++ b/arch/powerpc/kernel/mce.c
@@ -144,7 +144,6 @@ void save_mce_event(struct pt_regs *regs, long handled,
if (phys_addr != ULONG_MAX) {
mce->u.ue_error.physical_address_provided = true;
mce->u.ue_error.physical_address = phys_addr;
-   machine_check_ue_event(mce);
}
}
return;
@@ -275,8 +274,7 @@ static void machine_process_ue_event(struct work_struct 
*work)
}
 }
 /*
- * process pending MCE event from the mce event queue. This function will be
- * called during syscall exit.
+ * process pending MCE event from the mce event queue.
  */
 static void machine_check_process_queued_event(struct irq_work *work)
 {
@@ -292,6 +290,10 @@ static void machine_check_process_queued_event(struct 
irq_work *work)
while (__this_cpu_read(mce_queue_count) > 0) {
index = __this_cpu_read(mce_queue_count) - 1;
evt = this_cpu_ptr(_event_queue[index]);
+
+   if (evt->error_type == MCE_ERROR_TYPE_UE)
+   machine_check_ue_event(evt);
+
machine_check_print_event_info(evt, false, false);
__this_cpu_dec(mce_queue_count);
}
-- 
2.20.1



[PATCH] powerpc/xive: Add some error handling code to 'xive_spapr_init()'

2019-08-01 Thread Christophe JAILLET
'xive_irq_bitmap_add()' can return -ENOMEM.
In this case, we should free the memory already allocated and return
'false' to the caller.

Also add an error path which undoes the 'tima = ioremap(...)'

Signed-off-by: Christophe JAILLET 
---
NOT compile tested (I don't have a cross compiler and won't install one).
So if some correction or improvement are needed, feel free to propose and
commit it directly.
---
 arch/powerpc/sysdev/xive/spapr.c | 36 +---
 1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/sysdev/xive/spapr.c b/arch/powerpc/sysdev/xive/spapr.c
index 52198131c75e..b3ae0b76c433 100644
--- a/arch/powerpc/sysdev/xive/spapr.c
+++ b/arch/powerpc/sysdev/xive/spapr.c
@@ -64,6 +64,17 @@ static int xive_irq_bitmap_add(int base, int count)
return 0;
 }
 
+static void xive_irq_bitmap_remove_all(void)
+{
+   struct xive_irq_bitmap *xibm, *tmp;
+
+   list_for_each_entry_safe(xibm, tmp, _irq_bitmaps, list) {
+   list_del(>list);
+   kfree(xibm->bitmap);
+   kfree(xibm);
+   }
+}
+
 static int __xive_irq_bitmap_alloc(struct xive_irq_bitmap *xibm)
 {
int irq;
@@ -723,7 +734,7 @@ bool __init xive_spapr_init(void)
u32 val;
u32 len;
const __be32 *reg;
-   int i;
+   int i, err;
 
if (xive_spapr_disabled())
return false;
@@ -748,23 +759,26 @@ bool __init xive_spapr_init(void)
}
 
if (!xive_get_max_prio(_prio))
-   return false;
+   goto err_unmap;
 
/* Feed the IRQ number allocator with the ranges given in the DT */
reg = of_get_property(np, "ibm,xive-lisn-ranges", );
if (!reg) {
pr_err("Failed to read 'ibm,xive-lisn-ranges' property\n");
-   return false;
+   goto err_unmap;
}
 
if (len % (2 * sizeof(u32)) != 0) {
pr_err("invalid 'ibm,xive-lisn-ranges' property\n");
-   return false;
+   goto err_unmap;
}
 
-   for (i = 0; i < len / (2 * sizeof(u32)); i++, reg += 2)
-   xive_irq_bitmap_add(be32_to_cpu(reg[0]),
-   be32_to_cpu(reg[1]));
+   for (i = 0; i < len / (2 * sizeof(u32)); i++, reg += 2) {
+   err = xive_irq_bitmap_add(be32_to_cpu(reg[0]),
+ be32_to_cpu(reg[1]));
+   if (err < 0)
+   goto err_mem_free;
+   }
 
/* Iterate the EQ sizes and pick one */
of_property_for_each_u32(np, "ibm,xive-eq-sizes", prop, reg, val) {
@@ -775,8 +789,14 @@ bool __init xive_spapr_init(void)
 
/* Initialize XIVE core with our backend */
if (!xive_core_init(_spapr_ops, tima, TM_QW1_OS, max_prio))
-   return false;
+   goto err_mem_free;
 
pr_info("Using %dkB queues\n", 1 << (xive_queue_shift - 10));
return true;
+
+err_mem_free:
+   xive_irq_bitmap_remove_all();
+err_unmap:
+   iounmap(tima);
+   return false;
 }
-- 
2.20.1



Re: [PATCH 2/2] powerpc/xive: Add a check for memory allocation failure

2019-08-01 Thread Greg Kurz
On Thu,  1 Aug 2019 10:32:42 +0200
Christophe JAILLET  wrote:

> The result of this kzalloc is not checked. Add a check and corresponding
> error handling code.
> 
> Signed-off-by: Christophe JAILLET 
> ---

Reviewed-by: Greg Kurz 

> Note that 'xive_irq_bitmap_add()' failures are not handled in
> 'xive_spapr_init()'
> I guess that it is not really an issue. This function is _init, so if a
> memory allocation occures here, it is likely that the system will
> already be in bad shape.

Hmm not sure... The allocation could also fail if the "ibm,xive-lisn-ranges"
property contains an insanely big range, eg. count == 1 << 31. The system isn't
necessarily in bad shape in this case, but XIVE is definitely unusable and
we should let a chance to the kernel to switch to XICS in this case.

I guess it is worth adding proper error handling in xive_spapr_init() as well.

> Anyway, the check added here would at least keep the data linked in
> 'xive_irq_bitmaps' usable.
> ---
>  arch/powerpc/sysdev/xive/spapr.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/powerpc/sysdev/xive/spapr.c 
> b/arch/powerpc/sysdev/xive/spapr.c
> index b4f5eb9e0f82..52198131c75e 100644
> --- a/arch/powerpc/sysdev/xive/spapr.c
> +++ b/arch/powerpc/sysdev/xive/spapr.c
> @@ -53,6 +53,10 @@ static int xive_irq_bitmap_add(int base, int count)
>   xibm->base = base;
>   xibm->count = count;
>   xibm->bitmap = kzalloc(xibm->count, GFP_KERNEL);
> + if (!xibm->bitmap) {
> + kfree(xibm);
> + return -ENOMEM;
> + }
>   list_add(>list, _irq_bitmaps);
>  
>   pr_info("Using IRQ range [%x-%x]", xibm->base,



Re: [PATCH 1/2] powerpc/xive: Use GFP_KERNEL instead of GFP_ATOMIC in 'xive_irq_bitmap_add()'

2019-08-01 Thread Greg Kurz
On Thu,  1 Aug 2019 10:32:31 +0200
Christophe JAILLET  wrote:

> There is no need to use GFP_ATOMIC here. GFP_KERNEL should be enough.
> GFP_KERNEL is also already used for another allocation just a few lines
> below.
> 
> Signed-off-by: Christophe JAILLET 
> ---

Good catch.

Reviewed-by: Greg Kurz 

>  arch/powerpc/sysdev/xive/spapr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/sysdev/xive/spapr.c 
> b/arch/powerpc/sysdev/xive/spapr.c
> index 8ef9cf4ebb1c..b4f5eb9e0f82 100644
> --- a/arch/powerpc/sysdev/xive/spapr.c
> +++ b/arch/powerpc/sysdev/xive/spapr.c
> @@ -45,7 +45,7 @@ static int xive_irq_bitmap_add(int base, int count)
>  {
>   struct xive_irq_bitmap *xibm;
>  
> - xibm = kzalloc(sizeof(*xibm), GFP_ATOMIC);
> + xibm = kzalloc(sizeof(*xibm), GFP_KERNEL);
>   if (!xibm)
>   return -ENOMEM;
>  



Re: [PATCH] powerpc/powernv: Print helpful message when cores guarded

2019-08-01 Thread Cédric Le Goater
On 01/08/2019 07:16, Joel Stanley wrote:
> Often the firmware will guard out cores after a crash. This often
> undesirable, and is not immediately noticeable.
> 
> This adds an informative message when a CPU device tree nodes are marked
> bad in the device tree.
> 
> Signed-off-by: Joel Stanley 
> ---
> Tested on qemu 4.1 with this patch applied:
> 
>  
> https://ozlabs.org/~joel/uta2019/0001-TESTING-mark-every-second-core-as-guarded.patch
> 
> This will show no cores guarded:
> 
>  qemu-system-ppc64 -M powernv8 -nographic -smp 1,cores=1,threads=1 -kernel 
> zImage.epapr
> 
> This will show three:
> 
>  qemu-system-ppc64 -M powernv8 -nographic -smp 7,cores=7,threads=1 -kernel 
> zImage.epapr
> 
>  arch/powerpc/platforms/powernv/setup.c | 24 
>  1 file changed, 24 insertions(+)
> 
> diff --git a/arch/powerpc/platforms/powernv/setup.c 
> b/arch/powerpc/platforms/powernv/setup.c
> index a5e52f9eed3c..7107583d0c6b 100644
> --- a/arch/powerpc/platforms/powernv/setup.c
> +++ b/arch/powerpc/platforms/powernv/setup.c
> @@ -129,6 +129,28 @@ static void pnv_setup_rfi_flush(void)
>   setup_count_cache_flush();
>  }
>  
> +static void __init pnv_check_guarded_cores(void)
> +{
> + struct device_node *dn;
> + int bad_count = 0;
> +
> + for_each_node_by_type(dn, "cpu") {
> + if (of_property_match_string(dn, "status", "bad") >= 0)
> + bad_count++;
> + };
> +
> + if (bad_count) {
> + pr_cont("  __   \n");
> + pr_cont(" /  \\___\n");
> + pr_cont(" |  |   /   \\  \n");
> + pr_cont(" @  @   |WARNING!   |  \n");
> + pr_cont(" || ||  | It looks like |  \n");
> + pr_cont(" || ||   <--|  you have %*d |  \n", 3, bad_count);
> + pr_cont(" |\\_/|  | guarded cores |  \n");
> + pr_cont(" \\___/  \\___/  \n\n");
> + }
> +}

Is that a new animal ? We could use a Cerberus.

Cheers,
C. 

>  static void __init pnv_setup_arch(void)
>  {
>   set_arch_panic_timeout(10, ARCH_PANIC_TIMEOUT);
> @@ -149,6 +171,8 @@ static void __init pnv_setup_arch(void)
>   /* Enable NAP mode */
>   powersave_nap = 1;
>  
> + pnv_check_guarded_cores();
> +
>   /* XXX PMCS */
>  }
>  
> 



Re: [PATCH 1/2] powerpc/xive: Use GFP_KERNEL instead of GFP_ATOMIC in 'xive_irq_bitmap_add()'

2019-08-01 Thread Cédric Le Goater
On 01/08/2019 10:32, Christophe JAILLET wrote:
> There is no need to use GFP_ATOMIC here. GFP_KERNEL should be enough.
> GFP_KERNEL is also already used for another allocation just a few lines
> below.

This is correct.
 
> Signed-off-by: Christophe JAILLET 


Reviewed-by: Cédric Le Goater 

Thanks,

C.

> ---
>  arch/powerpc/sysdev/xive/spapr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/sysdev/xive/spapr.c 
> b/arch/powerpc/sysdev/xive/spapr.c
> index 8ef9cf4ebb1c..b4f5eb9e0f82 100644
> --- a/arch/powerpc/sysdev/xive/spapr.c
> +++ b/arch/powerpc/sysdev/xive/spapr.c
> @@ -45,7 +45,7 @@ static int xive_irq_bitmap_add(int base, int count)
>  {
>   struct xive_irq_bitmap *xibm;
>  
> - xibm = kzalloc(sizeof(*xibm), GFP_ATOMIC);
> + xibm = kzalloc(sizeof(*xibm), GFP_KERNEL);
>   if (!xibm)
>   return -ENOMEM;
>  
> 



[PATCH 2/2] powerpc/xive: Add a check for memory allocation failure

2019-08-01 Thread Christophe JAILLET
The result of this kzalloc is not checked. Add a check and corresponding
error handling code.

Signed-off-by: Christophe JAILLET 
---
Note that 'xive_irq_bitmap_add()' failures are not handled in
'xive_spapr_init()'
I guess that it is not really an issue. This function is _init, so if a
memory allocation occures here, it is likely that the system will
already be in bad shape.
Anyway, the check added here would at least keep the data linked in
'xive_irq_bitmaps' usable.
---
 arch/powerpc/sysdev/xive/spapr.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/arch/powerpc/sysdev/xive/spapr.c b/arch/powerpc/sysdev/xive/spapr.c
index b4f5eb9e0f82..52198131c75e 100644
--- a/arch/powerpc/sysdev/xive/spapr.c
+++ b/arch/powerpc/sysdev/xive/spapr.c
@@ -53,6 +53,10 @@ static int xive_irq_bitmap_add(int base, int count)
xibm->base = base;
xibm->count = count;
xibm->bitmap = kzalloc(xibm->count, GFP_KERNEL);
+   if (!xibm->bitmap) {
+   kfree(xibm);
+   return -ENOMEM;
+   }
list_add(>list, _irq_bitmaps);
 
pr_info("Using IRQ range [%x-%x]", xibm->base,
-- 
2.20.1



[PATCH 1/2] powerpc/xive: Use GFP_KERNEL instead of GFP_ATOMIC in 'xive_irq_bitmap_add()'

2019-08-01 Thread Christophe JAILLET
There is no need to use GFP_ATOMIC here. GFP_KERNEL should be enough.
GFP_KERNEL is also already used for another allocation just a few lines
below.

Signed-off-by: Christophe JAILLET 
---
 arch/powerpc/sysdev/xive/spapr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/sysdev/xive/spapr.c b/arch/powerpc/sysdev/xive/spapr.c
index 8ef9cf4ebb1c..b4f5eb9e0f82 100644
--- a/arch/powerpc/sysdev/xive/spapr.c
+++ b/arch/powerpc/sysdev/xive/spapr.c
@@ -45,7 +45,7 @@ static int xive_irq_bitmap_add(int base, int count)
 {
struct xive_irq_bitmap *xibm;
 
-   xibm = kzalloc(sizeof(*xibm), GFP_ATOMIC);
+   xibm = kzalloc(sizeof(*xibm), GFP_KERNEL);
if (!xibm)
return -ENOMEM;
 
-- 
2.20.1



[PATCH 0/2] powerpc/xive: 2 small tweaks in 'xive_irq_bitmap_add()'

2019-08-01 Thread Christophe JAILLET
The first patch uses GFP_KERNEL instead of GFP_ATOMIC.
The 2nd adds a check for memory allocation failure.

Christophe JAILLET (2):
  powerpc/xive: Use GFP_KERNEL instead of GFP_ATOMIC in
'xive_irq_bitmap_add()'
  powerpc/xive: Add a check for memory allocation failure

 arch/powerpc/sysdev/xive/spapr.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

-- 
2.20.1



Re: [PATCH v2] powerpc: Support CMDLINE_EXTEND

2019-08-01 Thread Christophe Leroy




Le 01/08/2019 à 04:12, Chris Packham a écrit :

Bring powerpc in line with other architectures that support extending or
overriding the bootloader provided command line.

The current behaviour is most like CMDLINE_FROM_BOOTLOADER where the
bootloader command line is preferred but the kernel config can provide a
fallback so CMDLINE_FROM_BOOTLOADER is the default. CMDLINE_EXTEND can
be used to append the CMDLINE from the kernel config to the one provided
by the bootloader.

Signed-off-by: Chris Packham 
---
While I'm at it does anyone think it's worth getting rid of the default CMDLINE
value if CMDLINE_BOOL and maybe CMDLINE_BOOL? Every defconfig in the kernel
that sets CMDLINE_BOOL=y also sets CMDLINE to something other than
"console=ttyS0,9600 console=tty0 root=/dev/sda2". Removing CMDLINE_BOOL and
unconditionally setting the default value of CMDLINE to "" would clean up the
Kconfig even more.


Note 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=cbe46bd4f5104552b612505b73d366f66efc2341 
which is already a step forward.


I guess that default is for users selecting this option manually to get 
a first sensitive CMDLINE. But is it really worth it ?




Changes in v2:
- incorporate ideas from Christope's patch 
https://patchwork.ozlabs.org/patch/1074126/

  arch/powerpc/Kconfig| 20 +++-
  arch/powerpc/kernel/prom_init.c | 26 +-
  2 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 77f6ebf97113..d413fe1b4058 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -852,15 +852,33 @@ config CMDLINE
  some command-line options at build time by entering them here.  In
  most cases you will need to specify the root device here.
  
+choice

+   prompt "Kernel command line type" if CMDLINE != ""
+   default CMDLINE_FROM_BOOTLOADER
+
+config CMDLINE_FROM_BOOTLOADER
+   bool "Use bootloader kernel arguments if available"
+   help
+ Uses the command-line options passed by the boot loader. If
+ the boot loader doesn't provide any, the default kernel command
+ string provided in CMDLINE will be used.
+
+config CMDLINE_EXTEND
+   bool "Extend bootloader kernel arguments"
+   help
+ The command-line arguments provided by the boot loader will be
+ appended to the default kernel command string.
+
  config CMDLINE_FORCE
bool "Always use the default kernel command string"
-   depends on CMDLINE_BOOL
help
  Always use the default kernel command string, even if the boot
  loader passes other arguments to the kernel.
  This is useful if you cannot or don't want to change the
  command-line options your boot loader passes to the kernel.
  
+endchoice

+
  config EXTRA_TARGETS
string "Additional default image types"
help
diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index 514707ef6779..df29f141dbd2 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -310,6 +310,25 @@ static size_t __init prom_strlcpy(char *dest, const char 
*src, size_t size)
return ret;
  }
  
+static size_t __init prom_strlcat(char *dest, const char *src, size_t count)

+{
+   size_t dsize = prom_strlen(dest);
+   size_t len = prom_strlen(src);
+   size_t res = dsize + len;
+
+   /* This would be a bug */
+   BUG_ON(dsize >= count);


Has you pointed in another mail, BUG_ON() should be avoided here.
I guess if the destination is already full, just return count:

if (dsize >= count)
return count;


+
+   dest += dsize;
+   count -= dsize;
+   if (len >= count)
+   len = count-1;
+   memcpy(dest, src, len);
+   dest[len] = 0;
+   return res;
+
+}
+
  #ifdef CONFIG_PPC_PSERIES
  static int __init prom_strtobool(const char *s, bool *res)
  {
@@ -761,8 +780,13 @@ static void __init early_cmdline_parse(void)
p = prom_cmd_line;
if ((long)prom.chosen > 0)
l = prom_getprop(prom.chosen, "bootargs", p, 
COMMAND_LINE_SIZE-1);


The above is pointless in case CONFIG_CMDLINE_FORCE is selected.



-   if (IS_ENABLED(CONFIG_CMDLINE_BOOL) && (l <= 0 || p[0] == '\0')) /* dbl 
check */
+
+   if (IS_ENABLED(CONFIG_CMDLINE_FORCE) || l <= 0 || p[0] == '\0')
prom_strlcpy(prom_cmd_line, CONFIG_CMDLINE, 
sizeof(prom_cmd_line));


If we can ensure that prom_cmd_line remains empty when 
CONFIG_CMDLINE_FORCE is selected (see above comment), then 
prom_strlcat() can be used in lieu of prom_strlcpy()



+   else if (IS_ENABLED(CONFIG_CMDLINE_EXTEND))
+   prom_strlcat(prom_cmd_line, " " CONFIG_CMDLINE,
+sizeof(prom_cmd_line));
+
prom_printf("command line: %s\n", prom_cmd_line);
  
  #ifdef CONFIG_PPC64




Christophe