LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [RFC PATCH 2/6] fs: provide a generic compat_sys_truncate64() implementation
From: Kevin Easton @ 2018-03-19  6:29 UTC (permalink / raw)
  To: Dominik Brodowski
  Cc: linux-kernel, torvalds, arnd, viro, linux-arch, Ralf Baechle,
	James Hogan, linux-mips, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, linuxppc-dev, Martin Schwidefsky,
	Heiko Carstens, linux-s390, David S . Miller, sparclinux,
	Ingo Molnar, Jiri Slaby, x86
In-Reply-To: <20180318161056.5377-3-linux@dominikbrodowski.net>

On Sun, Mar 18, 2018 at 05:10:52PM +0100, Dominik Brodowski wrote:
> The compat_sys_truncate64() implementations in mips, powerpc, s390, sparc
> and x86 only differed based on whether the u64 parameter needed padding
> and on its endianness.
> 

...
  
> +#ifdef __ARCH_WANT_COMPAT_SYS_TRUNCATE64
> +#if defined(__ARCH_WANT_COMPAT_SYS_WITH_PADDING) && \
> +	defined(__ARCH_WANT_LE_COMPAT_SYS)
> +COMPAT_SYSCALL_DEFINE4(truncate64, const char __user *, filename, u32 padding,
> +		       unsigned int, offset_low, unsigned int, offset_high)
> +#elif defined(__ARCH_WANT_COMPAT_SYS_WITH_PADDING) && \
> +	!defined(__ARCH_WANT_LE_COMPAT_SYS)
> +COMPAT_SYSCALL_DEFINE4(truncate64, const char __user *, filename, u32 padding,
> +		       unsigned int, offset_high, unsigned int, offset_low)

Notwithstanding the other comments, shouldn't there be a comma between
'u32' and 'padding' in those?

    - Kevin

^ permalink raw reply

* Re: [PATCH 3/5] powerpc/mm/32: Use page_is_ram to check for RAM
From: Michael Ellerman @ 2018-03-19  6:05 UTC (permalink / raw)
  To: Jonathan Neuschäfer, linuxppc-dev
  Cc: linux-kernel, linux-mm, Joel Stanley, Christophe LEROY,
	Jonathan Neuschäfer, Benjamin Herrenschmidt, Paul Mackerras,
	Balbir Singh, Guenter Roeck
In-Reply-To: <20180222121516.23415-4-j.neuschaefer@gmx.net>

Jonathan Neusch=C3=A4fer <j.neuschaefer@gmx.net> writes:

> Signed-off-by: Jonathan Neusch=C3=A4fer <j.neuschaefer@gmx.net>
> ---
>  arch/powerpc/mm/pgtable_32.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
> index d35d9ad3c1cd..d54e1a9c1c99 100644
> --- a/arch/powerpc/mm/pgtable_32.c
> +++ b/arch/powerpc/mm/pgtable_32.c
> @@ -145,9 +145,8 @@ __ioremap_caller(phys_addr_t addr, unsigned long size=
, unsigned long flags,
>  #ifndef CONFIG_CRASH_DUMP
>  	/*
>  	 * Don't allow anybody to remap normal RAM that we're using.
> -	 * mem_init() sets high_memory so only do the check after that.
>  	 */
> -	if (slab_is_available() && (p < virt_to_phys(high_memory)) &&
> +	if (page_is_ram(__phys_to_pfn(p)) &&
>  	    !(__allow_ioremap_reserved && memblock_is_region_reserved(p, size))=
) {
>  		printk("__ioremap(): phys addr 0x%llx is RAM lr %ps\n",
>  		       (unsigned long long)p, __builtin_return_address(0));


This is killing my p5020ds (Freescale e5500) unfortunately:

  smp: Bringing up secondary CPUs ...
  __ioremap(): phys addr 0x7fef5000 is RAM lr smp_85xx_kick_cpu
  Unable to handle kernel paging request for data at address 0x00000000
  Faulting instruction address: 0xc0029080
  Oops: Kernel access of bad area, sig: 11 [#1]
  BE SMP NR_CPUS=3D24 CoreNet Generic
  Modules linked in:
  CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.16.0-rc4-gcc-4.6.3-00076-g853=
19478bdb4 #86
  NIP:  c0029080 LR: c0029020 CTR: 00000001
  REGS: e804bd40 TRAP: 0300   Not tainted  (4.16.0-rc4-gcc-4.6.3-00076-g853=
19478bdb4)
  MSR:  00021002 <CE,ME>  CR: 24ad4e22  XER: 00000000
  DEAR: 00000000 ESR: 00000000=20
  GPR00: c0029020 e804bdf0 e8050000 00000000 00021002 0000004d 00000000 c0a=
aaeed=20
  GPR08: 00000000 00000000 00000000 2d57d000 22adbe84 00000000 c0002630 000=
00000=20
  GPR16: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 c0a=
8b4a4=20
  GPR24: 00000001 00000000 00000000 00029002 00000000 00000001 00000001 000=
00001=20
  NIP [c0029080] smp_85xx_kick_cpu+0x100/0x2c0
  LR [c0029020] smp_85xx_kick_cpu+0xa0/0x2c0
  Call Trace:
  [e804bdf0] [c0029020] smp_85xx_kick_cpu+0xa0/0x2c0 (unreliable)
  [e804be30] [c0011194] __cpu_up+0xb4/0x1c0
  [e804be60] [c002f16c] bringup_cpu+0x2c/0xf0
  [e804be80] [c002ec9c] cpuhp_invoke_callback+0x12c/0x310
  [e804beb0] [c002fdd8] cpu_up+0x108/0x230
  [e804bee0] [c09f7438] smp_init+0x84/0x104
  [e804bf00] [c09e9acc] kernel_init_freeable+0xc4/0x228
  [e804bf30] [c0002644] kernel_init+0x14/0x110
  [e804bf40] [c000f3b0] ret_from_kernel_thread+0x5c/0x64
  Instruction dump:
  57990032 3b1c0057 7f19c050 7f3acb78 5718d1be 2e180000 41920024 7f29cb78=20
  7f0903a6 60000000 60000000 60000000 <7c0048ac> 39290040 4200fff8 7c0004ac=
=20
  random: get_random_bytes called from init_oops_id+0x5c/0x70 with crng_ini=
t=3D0
  ---[ end trace c3807aa91cf16cd8 ]---


The obvious fix of changing the test in smp_85xx_start_cpu() didn't
work, I get a different oops:

  Unable to handle kernel paging request for data at address 0x3fef5140
  Faulting instruction address: 0xc00290a0
  Oops: Kernel access of bad area, sig: 11 [#1]
  BE SMP NR_CPUS=3D24 CoreNet Generic
  Modules linked in:
  CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.16.0-rc4-gcc-4.6.3-00076-g853=
19478bdb4-dirty #90
  NIP:  c00290a0 LR: c0029040 CTR: 00000001
  REGS: e804bd50 TRAP: 0300   Not tainted  (4.16.0-rc4-gcc-4.6.3-00076-g853=
19478bdb4-dirty)
  MSR:  00021002 <CE,ME>  CR: 24a24e22  XER: 00000000
  DEAR: 3fef5140 ESR: 00000000=20
  GPR00: c0029040 e804be00 e8050000 00000023 00021002 0000004e 00000000 c0a=
aaed3=20
  GPR08: 00000000 3fef5140 00000000 2d57d000 22a2be84 00000000 c0002630 000=
00000=20
  GPR16: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 c0a=
8b4a4=20
  GPR24: 00000004 00000001 3fef5140 3fef5140 00029002 3fef5140 00000001 000=
00001=20
  NIP [c00290a0] smp_85xx_kick_cpu+0x120/0x2e0
  LR [c0029040] smp_85xx_kick_cpu+0xc0/0x2e0
  Call Trace:
  [e804be00] [c0029040] smp_85xx_kick_cpu+0xc0/0x2e0 (unreliable)
  [e804be30] [c0011194] __cpu_up+0xb4/0x1c0
  [e804be60] [c002f18c] bringup_cpu+0x2c/0xf0
  [e804be80] [c002ecbc] cpuhp_invoke_callback+0x12c/0x310
  [e804beb0] [c002fdf8] cpu_up+0x108/0x230
  [e804bee0] [c09f7438] smp_init+0x84/0x104
  [e804bf00] [c09e9acc] kernel_init_freeable+0xc4/0x228
  [e804bf30] [c0002644] kernel_init+0x14/0x110
  [e804bf40] [c000f3b0] ret_from_kernel_thread+0x5c/0x64
  Instruction dump:
  7c0903a6 4e800421 57ba0032 3b3d0057 7f3ac850 7f5bd378 5739d1be 2e190000=20
  4192001c 7f49d378 7f2903a6 60000000 <7c0048ac> 39290040 4200fff8 7c0004ac=
=20
  random: get_random_bytes called from init_oops_id+0x5c/0x70 with crng_ini=
t=3D0
  ---[ end trace 950df40ee04f2d5e ]---


So that will require a bit more debugging.

cheers

^ permalink raw reply

* [PATCH 5/5] powerpc/zImage: Also check for stdout-path
From: Oliver O'Halloran @ 2018-03-19  5:14 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Oliver O'Halloran
In-Reply-To: <20180319051403.23275-1-oohall@gmail.com>

The /chosen/linux,stdout-path is "deprecated" in favour of
/chosen/stdout-path so we should be checking for both.

Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
 arch/powerpc/boot/serial.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/boot/serial.c b/arch/powerpc/boot/serial.c
index a7f9ac9a27b5..9ef1ed35ae62 100644
--- a/arch/powerpc/boot/serial.c
+++ b/arch/powerpc/boot/serial.c
@@ -92,7 +92,8 @@ static void *serial_get_stdout_devp(void)
 	if (devp == NULL)
 		goto err_out;
 
-	if (getprop(devp, "linux,stdout-path", path, MAX_PATH_LEN) > 0) {
+	if (getprop(devp, "linux,stdout-path", path, MAX_PATH_LEN) > 0 ||
+		getprop(devp, "stdout-path", path, MAX_PATH_LEN) > 0) {
 		devp = finddevice(path);
 		if (devp == NULL)
 			goto err_out;
-- 
2.9.5

^ permalink raw reply related

* [PATCH 2/5] powerpc/zImage: Don't build zlib object files
From: Oliver O'Halloran @ 2018-03-19  5:14 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Oliver O'Halloran
In-Reply-To: <20180319051403.23275-1-oohall@gmail.com>

The required source files are directly #include`ed into
decompress.c when using gzip compression. Building the
separate .o files for the zlib sources isn't required
and can cause linker errors due to the symbols being
defined in decompress.o and the zlib .o files.

Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
You can stop throwing up now
---
 arch/powerpc/boot/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
index 26d5d2a5b8e9..f66b9dba99fd 100644
--- a/arch/powerpc/boot/Makefile
+++ b/arch/powerpc/boot/Makefile
@@ -108,7 +108,7 @@ $(addprefix $(obj)/,$(libfdt) libfdt-wrapper.o simpleboot.o epapr.o opal.o \
 src-wlib-y := string.S crt0.S stdio.c decompress.c main.c \
 		$(libfdt) libfdt-wrapper.c \
 		ns16550.c serial.c simple_alloc.c div64.S util.S \
-		elf_util.c $(zlib-y) devtree.c stdlib.c \
+		elf_util.c devtree.c stdlib.c \
 		oflib.c ofconsole.c cuboot.c
 
 src-wlib-$(CONFIG_PPC_MPC52XX) += mpc52xx-psc.c
-- 
2.9.5

^ permalink raw reply related

* [PATCH 4/5] powerpc/zImage: Rework serial driver probing
From: Oliver O'Halloran @ 2018-03-19  5:14 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Oliver O'Halloran
In-Reply-To: <20180319051403.23275-1-oohall@gmail.com>

Rather than checking the compatible string in serial_driver_init()
we call into the driver's init function and wait for a driver to
inidicate it bound to the device by returning zero.

The pointers to each driver probe functions are stored in the
".serialdrv" section of the zImage, similar to how initcalls and
whatnot are handled inside Linux. This structure allows us to
conditionally compile specific driver files based on the KConfig
settings. This is needed because we don't have access to the
KConfig #defines in the zImage source files (it's a giant #include
headache) so conditional compilation is the only way to eliminate
unnecessary code.

Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
 arch/powerpc/boot/cpm-serial.c      |  1 +
 arch/powerpc/boot/mpc52xx-psc.c     |  1 +
 arch/powerpc/boot/mpsc.c            |  1 +
 arch/powerpc/boot/ns16550.c         |  2 ++
 arch/powerpc/boot/opal.c            |  1 +
 arch/powerpc/boot/ops.h             | 16 +++++++++------
 arch/powerpc/boot/serial.c          | 41 +++++++++++--------------------------
 arch/powerpc/boot/uartlite.c        |  1 +
 arch/powerpc/boot/wrapper           |  2 +-
 arch/powerpc/boot/zImage.coff.lds.S |  4 ++++
 arch/powerpc/boot/zImage.lds.S      |  8 ++++++++
 11 files changed, 42 insertions(+), 36 deletions(-)

diff --git a/arch/powerpc/boot/cpm-serial.c b/arch/powerpc/boot/cpm-serial.c
index f6b1cf23946e..5ebf3dfd810a 100644
--- a/arch/powerpc/boot/cpm-serial.c
+++ b/arch/powerpc/boot/cpm-serial.c
@@ -295,3 +295,4 @@ int cpm_console_init(void *devp, struct serial_console_data *scdp)
 
 	return 0;
 }
+SERIAL_DRIVER(cpm_console_init);
diff --git a/arch/powerpc/boot/mpc52xx-psc.c b/arch/powerpc/boot/mpc52xx-psc.c
index 75470936e661..c09e82eaf006 100644
--- a/arch/powerpc/boot/mpc52xx-psc.c
+++ b/arch/powerpc/boot/mpc52xx-psc.c
@@ -66,3 +66,4 @@ int mpc5200_psc_console_init(void *devp, struct serial_console_data *scdp)
 
 	return 0;
 }
+SERIAL_DRIVER(mpc5200_psc_console_init);
diff --git a/arch/powerpc/boot/mpsc.c b/arch/powerpc/boot/mpsc.c
index 59e23e886440..2d0a72522b3c 100644
--- a/arch/powerpc/boot/mpsc.c
+++ b/arch/powerpc/boot/mpsc.c
@@ -170,3 +170,4 @@ int mpsc_console_init(void *devp, struct serial_console_data *scdp)
 err_out:
 	return -1;
 }
+SERIAL_DRIVER(mpsc_console_init);
diff --git a/arch/powerpc/boot/ns16550.c b/arch/powerpc/boot/ns16550.c
index 0e7bd5284255..49e8c299b829 100644
--- a/arch/powerpc/boot/ns16550.c
+++ b/arch/powerpc/boot/ns16550.c
@@ -81,3 +81,5 @@ int ns16550_console_init(void *devp, struct serial_console_data *scdp)
 
 	return 0;
 }
+
+SERIAL_DRIVER(ns16550_console_init);
diff --git a/arch/powerpc/boot/opal.c b/arch/powerpc/boot/opal.c
index 0e92537536b9..dc345ff63422 100644
--- a/arch/powerpc/boot/opal.c
+++ b/arch/powerpc/boot/opal.c
@@ -102,3 +102,4 @@ int opal_console_init(void *devp, struct serial_console_data *scdp)
 
 	return 0;
 }
+SERIAL_DRIVER(opal_console_init);
diff --git a/arch/powerpc/boot/ops.h b/arch/powerpc/boot/ops.h
index fad1862f4b2d..06151ecb2f45 100644
--- a/arch/powerpc/boot/ops.h
+++ b/arch/powerpc/boot/ops.h
@@ -84,13 +84,17 @@ extern struct loader_info loader_info;
 
 void start(void);
 void fdt_init(void *blob);
+
+struct driver_ptr {
+	int (*ptr)(void *node, struct serial_console_data *scdp);
+};
+
+#define SERIAL_DRIVER(init_function) static const struct driver_ptr \
+	__attribute__((__section__(".serialdrv"))) __attribute__((used)) \
+	init_function##_ptr = { .ptr = &init_function, }
+
 int serial_console_init(void);
-int ns16550_console_init(void *devp, struct serial_console_data *scdp);
-int mpsc_console_init(void *devp, struct serial_console_data *scdp);
-int cpm_console_init(void *devp, struct serial_console_data *scdp);
-int mpc5200_psc_console_init(void *devp, struct serial_console_data *scdp);
-int uartlite_console_init(void *devp, struct serial_console_data *scdp);
-int opal_console_init(void *devp, struct serial_console_data *scdp);
+
 void *simple_alloc_init(char *base, unsigned long heap_size,
 			unsigned long granularity, unsigned long max_allocs);
 extern void flush_cache(void *, unsigned long);
diff --git a/arch/powerpc/boot/serial.c b/arch/powerpc/boot/serial.c
index 88955095ec07..a7f9ac9a27b5 100644
--- a/arch/powerpc/boot/serial.c
+++ b/arch/powerpc/boot/serial.c
@@ -105,11 +105,15 @@ static void *serial_get_stdout_devp(void)
 	return NULL;
 }
 
+extern unsigned long _serial_drv_start;
+extern unsigned long _serial_drv_end;
+
 static struct serial_console_data serial_cd;
 
 /* Node's "compatible" property determines which serial driver to use */
 int serial_console_init(void)
 {
+	struct driver_ptr *start, *end, *probe;
 	void *devp;
 	int rc = -1;
 
@@ -117,35 +121,14 @@ int serial_console_init(void)
 	if (devp == NULL)
 		goto err_out;
 
-	if (dt_is_compatible(devp, "ns16550") ||
-	    dt_is_compatible(devp, "pnpPNP,501"))
-		rc = ns16550_console_init(devp, &serial_cd);
-#ifdef CONFIG_EMBEDDED6xx
-	else if (dt_is_compatible(devp, "marvell,mv64360-mpsc"))
-		rc = mpsc_console_init(devp, &serial_cd);
-#endif
-#ifdef CONFIG_CPM
-	else if (dt_is_compatible(devp, "fsl,cpm1-scc-uart") ||
-	         dt_is_compatible(devp, "fsl,cpm1-smc-uart") ||
-	         dt_is_compatible(devp, "fsl,cpm2-scc-uart") ||
-	         dt_is_compatible(devp, "fsl,cpm2-smc-uart"))
-		rc = cpm_console_init(devp, &serial_cd);
-#endif
-#ifdef CONFIG_PPC_MPC52XX
-	else if (dt_is_compatible(devp, "fsl,mpc5200-psc-uart"))
-		rc = mpc5200_psc_console_init(devp, &serial_cd);
-#endif
-#ifdef CONFIG_XILINX_VIRTEX
-	else if (dt_is_compatible(devp, "xlnx,opb-uartlite-1.00.b") ||
-		 dt_is_compatible(devp, "xlnx,xps-uartlite-1.00.a"))
-		rc = uartlite_console_init(devp, &serial_cd);
-#endif
-#ifdef CONFIG_PPC64_BOOT_WRAPPER
-	else if (dt_is_compatible(devp, "ibm,opal-console-raw"))
-		rc = opal_console_init(devp, &serial_cd);
-#endif
-
-	/* Add other serial console driver calls here */
+	start = (void *) &_serial_drv_start;
+	end = (void *) &_serial_drv_end;
+
+	for (probe = start; probe < end; probe++) {
+		rc = probe->ptr(devp, &serial_cd);
+		if (rc <= 0)
+			break;
+	}
 
 	if (!rc) {
 		console_ops.open = serial_open;
diff --git a/arch/powerpc/boot/uartlite.c b/arch/powerpc/boot/uartlite.c
index fc66a5fcea66..92d4f00bf626 100644
--- a/arch/powerpc/boot/uartlite.c
+++ b/arch/powerpc/boot/uartlite.c
@@ -82,3 +82,4 @@ int uartlite_console_init(void *devp, struct serial_console_data *scdp)
 	scdp->close = NULL;
 	return 0;
 }
+SERIAL_DRIVER(uartlite_console_init);
diff --git a/arch/powerpc/boot/wrapper b/arch/powerpc/boot/wrapper
index 76fe3ccfd381..b5c887ae729d 100755
--- a/arch/powerpc/boot/wrapper
+++ b/arch/powerpc/boot/wrapper
@@ -469,7 +469,7 @@ if [ "$platform" != "miboot" ]; then
     fi
 #link everything
     ${CROSS}ld -m $format -T $lds $text_start $pie $nodl -o "$ofile" \
-	$platformo $tmp $object/wrapper.a
+	$platformo $tmp --whole-archive $object/wrapper.a
     rm $tmp
 fi
 
diff --git a/arch/powerpc/boot/zImage.coff.lds.S b/arch/powerpc/boot/zImage.coff.lds.S
index 117951295117..d667fe3f5999 100644
--- a/arch/powerpc/boot/zImage.coff.lds.S
+++ b/arch/powerpc/boot/zImage.coff.lds.S
@@ -20,6 +20,10 @@ SECTIONS
     *(.sdata*)
     *(.got2)
 
+    _serial_drv_start = .;
+    *(.serialdrv)
+    _serial_drv_end = .;
+
     _dtb_start = .;
     *(.kernel:dtb)
     _dtb_end = .;
diff --git a/arch/powerpc/boot/zImage.lds.S b/arch/powerpc/boot/zImage.lds.S
index 4ac1e36edfe7..9dda24bcee00 100644
--- a/arch/powerpc/boot/zImage.lds.S
+++ b/arch/powerpc/boot/zImage.lds.S
@@ -45,6 +45,14 @@ SECTIONS
   }
 
   . = ALIGN(8);
+  .serialdrv :
+  {
+    _serial_drv_start = .;
+    KEEP(*(.serialdrv));
+    _serial_drv_end = .;
+  }
+
+  . = ALIGN(8);
   .kernel:dtb :
   {
     _dtb_start = .;
-- 
2.9.5

^ permalink raw reply related

* [PATCH 3/5] powerpc/zImage: Check compatible at driver init
From: Oliver O'Halloran @ 2018-03-19  5:14 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Oliver O'Halloran
In-Reply-To: <20180319051403.23275-1-oohall@gmail.com>

Have each driver's init function check the compatible string
of the node given by the stdout path. This gives the drivers
a more traditional probe-also-inits structure.

Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
 arch/powerpc/boot/cpm-serial.c  | 2 ++
 arch/powerpc/boot/mpc52xx-psc.c | 3 +++
 arch/powerpc/boot/mpsc.c        | 3 +++
 arch/powerpc/boot/ns16550.c     | 4 ++++
 arch/powerpc/boot/opal.c        | 3 +++
 arch/powerpc/boot/uartlite.c    | 5 +++++
 6 files changed, 20 insertions(+)

diff --git a/arch/powerpc/boot/cpm-serial.c b/arch/powerpc/boot/cpm-serial.c
index dfb56829cace..f6b1cf23946e 100644
--- a/arch/powerpc/boot/cpm-serial.c
+++ b/arch/powerpc/boot/cpm-serial.c
@@ -212,6 +212,8 @@ int cpm_console_init(void *devp, struct serial_console_data *scdp)
 	} else if (dt_is_compatible(devp, "fsl,cpm2-smc-uart")) {
 		is_cpm2 = 1;
 		is_smc = 1;
+	} else if (!dt_is_compatible(devp, "fsl,cpm1-scc-uart")) {
+		return 1; /* not compatible */
 	}
 
 	if (is_smc) {
diff --git a/arch/powerpc/boot/mpc52xx-psc.c b/arch/powerpc/boot/mpc52xx-psc.c
index c2c08633ee35..75470936e661 100644
--- a/arch/powerpc/boot/mpc52xx-psc.c
+++ b/arch/powerpc/boot/mpc52xx-psc.c
@@ -52,6 +52,9 @@ static unsigned char psc_getc(void)
 
 int mpc5200_psc_console_init(void *devp, struct serial_console_data *scdp)
 {
+	if (!dt_is_compatible(devp, "fsl,mpc5200-psc-uart"))
+		return 1;
+
 	/* Get the base address of the psc registers */
 	if (dt_get_virtual_reg(devp, &psc, 1) < 1)
 		return -1;
diff --git a/arch/powerpc/boot/mpsc.c b/arch/powerpc/boot/mpsc.c
index 425ad88cce8d..59e23e886440 100644
--- a/arch/powerpc/boot/mpsc.c
+++ b/arch/powerpc/boot/mpsc.c
@@ -128,6 +128,9 @@ int mpsc_console_init(void *devp, struct serial_console_data *scdp)
 	int n, reg_set;
 	volatile char *sdma_base;
 
+	if (!dt_is_compatible(devp, "marvell,mv64360-mpsc"))
+		return 1;
+
 	n = getprop(devp, "virtual-reg", &v, sizeof(v));
 	if (n != sizeof(v))
 		goto err_out;
diff --git a/arch/powerpc/boot/ns16550.c b/arch/powerpc/boot/ns16550.c
index b0da4466d419..0e7bd5284255 100644
--- a/arch/powerpc/boot/ns16550.c
+++ b/arch/powerpc/boot/ns16550.c
@@ -58,6 +58,10 @@ int ns16550_console_init(void *devp, struct serial_console_data *scdp)
 	int n;
 	u32 reg_offset;
 
+	if (!dt_is_compatible(devp, "ns16550") &&
+	    !dt_is_compatible(devp, "pnpPNP,501"))
+		return 1;
+
 	if (dt_get_virtual_reg(devp, (void **)&reg_base, 1) < 1)
 		return -1;
 
diff --git a/arch/powerpc/boot/opal.c b/arch/powerpc/boot/opal.c
index dfb199ef5b94..0e92537536b9 100644
--- a/arch/powerpc/boot/opal.c
+++ b/arch/powerpc/boot/opal.c
@@ -83,6 +83,9 @@ static void opal_init(void)
 
 int opal_console_init(void *devp, struct serial_console_data *scdp)
 {
+	if (!dt_is_compatible(devp, "ibm,opal-console-raw"))
+		return 1;
+
 	opal_init();
 
 	if (devp) {
diff --git a/arch/powerpc/boot/uartlite.c b/arch/powerpc/boot/uartlite.c
index 46bed69b4169..fc66a5fcea66 100644
--- a/arch/powerpc/boot/uartlite.c
+++ b/arch/powerpc/boot/uartlite.c
@@ -62,6 +62,11 @@ int uartlite_console_init(void *devp, struct serial_console_data *scdp)
 	int n;
 	unsigned long reg_phys;
 
+
+	if (!dt_is_compatible(devp, "xlnx,opb-uartlite-1.00.b") &&
+	    !dt_is_compatible(devp, "xlnx,xps-uartlite-1.00.a"))
+		return 1;
+
 	n = getprop(devp, "virtual-reg", &reg_base, sizeof(reg_base));
 	if (n != sizeof(reg_base)) {
 		if (!dt_xlate_reg(devp, 0, &reg_phys, NULL))
-- 
2.9.5

^ permalink raw reply related

* [PATCH 1/5] powerpc/zImage: Remove #ifdef in opal.c
From: Oliver O'Halloran @ 2018-03-19  5:13 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Oliver O'Halloran

This file is only ever compiled if CONFIG_PPC64_BOOT_WRAPPER is set
so this check is unnecessary.

Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
 arch/powerpc/boot/opal.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/arch/powerpc/boot/opal.c b/arch/powerpc/boot/opal.c
index 0272570d02de..dfb199ef5b94 100644
--- a/arch/powerpc/boot/opal.c
+++ b/arch/powerpc/boot/opal.c
@@ -13,8 +13,6 @@
 #include <libfdt.h>
 #include "../include/asm/opal-api.h"
 
-#ifdef CONFIG_PPC64_BOOT_WRAPPER
-
 /* Global OPAL struct used by opal-call.S */
 struct opal {
 	u64 base;
@@ -101,9 +99,3 @@ int opal_console_init(void *devp, struct serial_console_data *scdp)
 
 	return 0;
 }
-#else
-int opal_console_init(void *devp, struct serial_console_data *scdp)
-{
-	return -1;
-}
-#endif /* __powerpc64__ */
-- 
2.9.5

^ permalink raw reply related

* Re: [RFC PATCH 4/6] mm: provide generic compat_sys_readahead() implementation
From: Al Viro @ 2018-03-19  4:23 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Dominik Brodowski, Linux Kernel Mailing List, Arnd Bergmann,
	linux-arch, Ralf Baechle, James Hogan, linux-mips,
	Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, ppc-dev,
	Martin Schwidefsky, Heiko Carstens, linux-s390, David S . Miller,
	sparclinux, Ingo Molnar, Jiri Slaby, the arch/x86 maintainers
In-Reply-To: <20180318181848.GU30522@ZenIV.linux.org.uk>

On Sun, Mar 18, 2018 at 06:18:48PM +0000, Al Viro wrote:

> I'd done some digging in that area, will find the notes and post.

OK, found:

We have two ABIs in the game - syscall and normal C.  The latter
(for all targets we support) can be described in the following
terms:
	* there is a series of word-sized objects used to pass
arguments, some in registers, some on stack.  Arguments are
mapped on that sequence.  Anything up to word size simply takes
the next available word, so on 64bit it's all there is - nth
argument goes into the nth object, types simply do not matter.
On 32bit it's not that simple - there 64bit arguments occupy
two objects.  They are still picked from the same sequence;
on little-endian the lower half goes first, on big-endian -
the higher one.  For some architectures that's all there is to it.
However, on quite a few there's an extra complication - not every
pair can be used for 64bit value.  Rules for those are arch-dependent.
One variety is "pairs should be aligned", i.e. "if we'd consumed
an odd number of slots, add a dummy one before eating a pair".
Another is "don't let a pair span the registers/stack boundary";
surprisingly enough, that's not universal.

The syscall ABI can considerably differ from C one.  First of all,
we really do *not* want to pass anything on stack - it's a major
headache at syscall entry.  See mips/o32 for the scale of that headache.
Not fun.  OTOH, the registers that can be used are a limited resource.
i386 can't pass more than 6 words and that has served as an upper
limit.  If we encode the argument sizes (W - word, D - 64bit) we
have the following variants among the syscalls:
	* no arguments at all (SYSCALL_DEFINE0)
	* W (SYSCALL_DEFINE1)
	* WW (SYSCALL_DEFINE2)
	* WWW (SYSCALL_DEFINE3)
	* WWWW (SYSCALL_DEFINE4)
	* WWWWW (SYSCALL_DEFINE5)
	* WWWWWW (SYSCALL_DEFINE6)
	* WD (SYSCALL_DEFINE2, truncate64, ftruncate64)
	* DWW (SYSCALL_DEFINE3, lookup_dcookie)
	* WDW (SYSCALL_DEFINE3, readahead)
	* WWWD (SYSCALL_DEFINE4, pread64, pwrite64)
	* WWDD (SYSCALL_DEFINE4, fallocate, sync_file_range2)
	* WDDW (SYSCALL_DEFINE4, sync_file_range, fadvise64_64)
	* WDWW (SYSCALL_DEFINE4, fadvise64)
	* WWDWW (SYSCALL_DEFINE5, fanotify_mark)

The list for each long long-passing variant might be incomplete, but
they really are rare.  And a source of headache; not just for biarch
architectures - e.g. c6x and metag are not biarch and these syscalls
are exactly what stinks them up.

One thing we *really* don't want is syscall-dependent mapping from
syscall ABI registers to C ABI sequence.  Inside a syscall (or in
per-syscall glue) - sure, we can do it (if not happily).  In the
syscall dispatcher - fuck no, too much PITA.  mips/o32 used to be
a horrible example of why not, then they went for "copy from userland
stack whether we need it or not"...

For simple syscalls (first 7 classes in the above, each argument fits
into word) we simply map registers to the first 6 slots of the sequence
and we are done.  It's bloody tempting to use the same mapping for
the rest - use the same code that calls simple syscalls and have it
call sys_readahead() instead of sys_mkdir(), etc.  For something like
x86 or sparc that works perfectly - these guys have no padding for 64bit
arguments, so we are good (provided that userland sets those registers
sanely, that is).

OTOH, consider arm.  There we have
	* r0, r1, r2, r3, [sp,#8], [sp,#12], [sp,#16]... is the sequence
of objects used to pass arguments
	* 32bit and less - pick the next available slot
	* 64bit - skip a slot if we'd already taken an odd number, then use
the next two slots for lower and upper 32 bits of the argument.

So our classes take
simple n-argument:	0 to 6 slots
WD			4 slots
DWW			4 slots
WDW			5 slots
WWDD			6 slots
WDWW			5 slots
WWWD			6 slots
WWDWW			6 slots
WDDW			7 slots (!)  Also ****, !!!!, !@#!@#!@#!# and other nice
and well-deserved comments from arch maintainers, some of them even printable:
/* It would be nice if people remember that not all the world's an i386
   when they introduce new system calls */
SYSCALL_DEFINE4(sync_file_range2, int, fd, unsigned int, flags,
                                 loff_t, offset, loff_t, nbytes)

No idea why that hadn't been done to fadvise64_64() - that got
/*
 * Since loff_t is a 64 bit type we avoid a lot of ABI hassle
 * with a different argument ordering.
 */
asmlinkage long sys_arm_fadvise64_64(int fd, int advice,
                                     loff_t offset, loff_t len)
{
        return sys_fadvise64_64(fd, offset, len, advice);
}
long ppc_fadvise64_64(int fd, int advice, u32 offset_high, u32 offset_low,
                      u32 len_high, u32 len_low)
{
        return sys_fadvise64(fd, (u64)offset_high << 32 | offset_low,
                             (u64)len_high << 32 | len_low, advice);
}
and
asmlinkage long parisc_fadvise64_64(int fd,
                        unsigned int high_off, unsigned int low_off,
                        unsigned int high_len, unsigned int low_len, int advice)
{
        return sys_fadvise64_64(fd, (loff_t)high_off << 32 | low_off,
                        (loff_t)high_len << 32 | low_len, advice);
}
consistency, shmonsistency...  And yes, glibc has
#ifdef __ASSUME_FADVISE64_64_6ARG
  int ret = INTERNAL_SYSCALL_CALL (fadvise64_64, err, fd, advise,
                                   SYSCALL_LL64 (offset), SYSCALL_LL64 (len));
#else
  int ret = INTERNAL_SYSCALL_CALL (fadvise64_64, err, fd,
                                   __ALIGNMENT_ARG SYSCALL_LL64 (offset),
                                   SYSCALL_LL64 (len), advise);
#endif
with __ASSUME_FADVISE64_64_6ARG defined for arm and ppc...
xtensa, BTW, has
asmlinkage long xtensa_fadvise64_64(int fd, int advice,
                unsigned long long offset, unsigned long long len)
{
        return sys_fadvise64_64(fd, offset, len, advice);
}
In other words, same solution as arm and ppc.  No xtensa support in
glibc, AFAICS...

Anyway, padding rules for 32bit ones, with not too many arguments:

* arc cris frv h8300 i386 m32r m68k microblaze mn10300 nios2 openrisc
riscv sparc - no padding.
Note that e.g. frv *does* have a "don't let it cross regs/stack
boundary" rule, but it has 6 register slots, so for syscalls it
doesn't matter.  OTOH, sparc (and m32r, and...) really won't
care about regs/stack boundary.

* arm mips parisc ppc xtensa - pad to even number of words consumed;
since the number of registers in the sequence is even for all those,
there is nothing special going on at the edge.  WDDW is a bitch for
that group; mips goes for "fuck it, we copy userland stack anyway",
the rest try to cope in various ways.

* s390 - 5 register slots, padding on the crossing the regs/stack
boundary and nowhere else (i.e. pad if exactly 4 words had been
consumed).  WWDD runs afoul of that one.

* bfin - similar, except that we pad on exactly 2 words.  Same
story as with s390, only we have 3 register slots.  Headache
on: WWDWW and WWDD.

* c6x - strange beast, that; registers are 32bit, but for the
argument passing purposes it's using 64bit register pairs, filling
only the lower half when passing a 32bit argument.

* sh - really weird.  If 64bit would span the registers/stack
boundary (4 register slots there), it's done on stack... and
the first 32bit argument after that will eat the remaining
register.  Out of our classes it affects WWWD (as if there
had been a padding) and WDDW (as if the last two arguments had
switched places).

FWIW, it looks like we have several different issues mixed here

1) teaching COMPAT_SYSCALL_DEFINE to do the right thing in case
when 64bit arguments are present.  It's not terribly hard (sh
is not biarch, thankfully), but we'd better watch out carefully
when unifying - "fucker eats 7 slots" cases are irregular by
definition and solutions used for those differ between the
architectures.  It won't be pretty, though - we can get something
like
asmlinkage long compat_sys_truncate64(long a0, long a1, long a2, long a3, long a4, long a5)
{
	CS_truncate64((const char __user *)a0, ((u64)a2<<32)|(u64)a3);
}
static inline long CS_truncate64(const char __user *path, loff_t length)
{
	return sys_truncate64(path, length);
}
out of
COMPAT_SYSCALL_DEFINE2(trucate64, const char __user *, path, loff_t, length)
{
	return sys_truncate(path, length);
}
but the damn thing will be 6-argument and expressions for arguments will
expand to something equal to the forms above, but they'll be choke-full
of conditional expressions with constant 0 or 1 for selectors.  What we can't
do is evaluation of "is it long long" at macro expansion time - we can't tell
int, loff_t from int, pid_t until typedefs had been handled, at which point
the parse tree is cast in stone.  It will have to be 6-argument.
Alternatively, we could do COMPAT_SYSCALL_DEFINE_WD et.al. so that preprocessor
would get that information directly from us.

[snip the preprocessor horrors - the sketches I've got there are downright obscene]

2) s390 compat wrappers ;-/  I still wonder if we would be better
off with SYSCALL_DEFINE on s390 defining a wrapper with COMPAT_SYSCALL_DEFINE
(if present) redefining it, throwing the original away.  That could be done
with some amount of linker trickery.

3) sparc wrappers in sys32.S.  Most of those got killed off back in 2012,
what remained was
SIGN1(sys32_getrusage, compat_sys_getrusage, %o0)
SIGN1(sys32_readahead, compat_sys_readahead, %o0)
SIGN2(sys32_fadvise64, compat_sys_fadvise64, %o0, %o4)
SIGN2(sys32_fadvise64_64, compat_sys_fadvise64_64, %o0, %o5)
SIGN1(sys32_clock_nanosleep, compat_sys_clock_nanosleep, %o1)
SIGN1(sys32_timer_settime, compat_sys_timer_settime, %o1)
SIGN1(sys32_io_submit, compat_sys_io_submit, %o1)
SIGN1(sys32_mq_open, compat_sys_mq_open, %o1)
SIGN1(sys32_select, compat_sys_select, %o0)
SIGN3(sys32_futex, compat_sys_futex, %o1, %o2, %o5)
SIGN2(sys32_sendfile, compat_sys_sendfile, %o0, %o1)
SIGN1(sys32_recvfrom, compat_sys_recvfrom, %o0)
SIGN1(sys32_recvmsg, compat_sys_recvmsg, %o0)
SIGN1(sys32_sendmsg, compat_sys_sendmsg, %o0)
SIGN2(sys32_sync_file_range, compat_sync_file_range, %o0, %o5)
SIGN1(sys32_vmsplice, compat_sys_vmsplice, %o0)
with some of those killed off later and (completely pointless)
SIGN2(sys32_renameat2, sys_renameat2, %o0, %o2)
added.

Since then clock_nanosleep(), timer_settime(), io_submit(), mq_open(),
select(), recvfrom(), recvmsg(), sendmsg(), getrusage(), sync_file_range(),
futex() and vmsplice() got switched to COMPAT_SYSCALL_DEFINE, making those
SIGN... wrappers pointless.  That leaves
SIGN1(sys32_readahead, compat_sys_readahead, %o0)
SIGN2(sys32_fadvise64, compat_sys_fadvise64, %o0, %o4)
SIGN2(sys32_fadvise64_64, compat_sys_fadvise64_64, %o0, %o5)
all of which are right there in arch/sparc/kernel/sys_sparc32.c and
trivially converted to COMPAT_SYSCALL_DEFINE.  Which would kill all the
SIGN... bunch off, leaving there sys_mmap() and sys_socketcall()

4) stuff in arch really needs a good look.  And not just biarch -
I could've easily missed some "takes a 64bit argument" cases in
that area.  Moreover, it might be a good idea to have all syscall
tables generated a-la x86 and s390 ones are, ideally with the same
format of input data for all of them...

^ permalink raw reply

* [PATCH v2 9/9] powerpc/eeh: Add eeh_state_active() helper
From: Sam Bobroff @ 2018-03-19  2:49 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <cover.1521427331.git.sam.bobroff@au1.ibm.com>

Checking for a "fully active" device state requires testing two flag
bits, which is open coded in several places, so add a function to do
it.

Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
---
 arch/powerpc/include/asm/eeh.h               |  6 ++++++
 arch/powerpc/kernel/eeh.c                    | 19 ++++++-------------
 arch/powerpc/platforms/powernv/eeh-powernv.c |  9 ++-------
 3 files changed, 14 insertions(+), 20 deletions(-)

diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index fd37cc101f4f..c2266ca61853 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -256,6 +256,12 @@ static inline void eeh_serialize_unlock(unsigned long flags)
 	raw_spin_unlock_irqrestore(&confirm_error_lock, flags);
 }
 
+static inline bool eeh_state_active(int state)
+{
+	return (state & (EEH_STATE_MMIO_ACTIVE | EEH_STATE_DMA_ACTIVE))
+	== (EEH_STATE_MMIO_ACTIVE | EEH_STATE_DMA_ACTIVE);
+}
+
 typedef void *(*eeh_traverse_func)(void *data, void *flag);
 void eeh_set_pe_aux_size(int size);
 int eeh_phb_pe_create(struct pci_controller *phb);
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 2b9df0040d6b..bc640e4c5ca5 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -394,9 +394,7 @@ static int eeh_phb_check_failure(struct eeh_pe *pe)
 	/* Check PHB state */
 	ret = eeh_ops->get_state(phb_pe, NULL);
 	if ((ret < 0) ||
-	    (ret == EEH_STATE_NOT_SUPPORT) ||
-	    (ret & (EEH_STATE_MMIO_ACTIVE | EEH_STATE_DMA_ACTIVE)) ==
-	    (EEH_STATE_MMIO_ACTIVE | EEH_STATE_DMA_ACTIVE)) {
+	    (ret == EEH_STATE_NOT_SUPPORT) || eeh_state_active(ret)) {
 		ret = 0;
 		goto out;
 	}
@@ -433,7 +431,6 @@ static int eeh_phb_check_failure(struct eeh_pe *pe)
 int eeh_dev_check_failure(struct eeh_dev *edev)
 {
 	int ret;
-	int active_flags = (EEH_STATE_MMIO_ACTIVE | EEH_STATE_DMA_ACTIVE);
 	unsigned long flags;
 	struct device_node *dn;
 	struct pci_dev *dev;
@@ -525,8 +522,7 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
 	 * state, PE is in good state.
 	 */
 	if ((ret < 0) ||
-	    (ret == EEH_STATE_NOT_SUPPORT) ||
-	    ((ret & active_flags) == active_flags)) {
+	    (ret == EEH_STATE_NOT_SUPPORT) || eeh_state_active(ret)) {
 		eeh_stats.false_positives++;
 		pe->false_positives++;
 		rc = 0;
@@ -546,8 +542,7 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
 
 		/* Frozen parent PE ? */
 		ret = eeh_ops->get_state(parent_pe, NULL);
-		if (ret > 0 &&
-		    (ret & active_flags) != active_flags)
+		if (ret > 0 && !eeh_state_active(ret))
 			pe = parent_pe;
 
 		/* Next parent level */
@@ -888,7 +883,6 @@ static void *eeh_set_dev_freset(void *data, void *flag)
  */
 int eeh_pe_reset_full(struct eeh_pe *pe)
 {
-	int active_flags = (EEH_STATE_MMIO_ACTIVE | EEH_STATE_DMA_ACTIVE);
 	int reset_state = (EEH_PE_RESET | EEH_PE_CFG_BLOCKED);
 	int type = EEH_RESET_HOT;
 	unsigned int freset = 0;
@@ -919,7 +913,7 @@ int eeh_pe_reset_full(struct eeh_pe *pe)
 
 		/* Wait until the PE is in a functioning state */
 		state = eeh_ops->wait_state(pe, PCI_BUS_RESET_WAIT_MSEC);
-		if ((state & active_flags) == active_flags)
+		if (eeh_state_active(state))
 			break;
 
 		if (state < 0) {
@@ -1352,16 +1346,15 @@ static int eeh_pe_change_owner(struct eeh_pe *pe)
 	struct eeh_dev *edev, *tmp;
 	struct pci_dev *pdev;
 	struct pci_device_id *id;
-	int flags, ret;
+	int ret;
 
 	/* Check PE state */
-	flags = (EEH_STATE_MMIO_ACTIVE | EEH_STATE_DMA_ACTIVE);
 	ret = eeh_ops->get_state(pe, NULL);
 	if (ret < 0 || ret == EEH_STATE_NOT_SUPPORT)
 		return 0;
 
 	/* Unfrozen PE, nothing to do */
-	if ((ret & flags) == flags)
+	if (eeh_state_active(ret))
 		return 0;
 
 	/* Frozen PE, check if it needs PE level reset */
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index 33c86c1a1720..ddfc3544d285 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -1425,11 +1425,8 @@ static int pnv_eeh_get_pe(struct pci_controller *hose,
 	dev_pe = dev_pe->parent;
 	while (dev_pe && !(dev_pe->type & EEH_PE_PHB)) {
 		int ret;
-		int active_flags = (EEH_STATE_MMIO_ACTIVE |
-				    EEH_STATE_DMA_ACTIVE);
-
 		ret = eeh_ops->get_state(dev_pe, NULL);
-		if (ret <= 0 || (ret & active_flags) == active_flags) {
+		if (ret <= 0 || eeh_state_active(ret)) {
 			dev_pe = dev_pe->parent;
 			continue;
 		}
@@ -1463,7 +1460,6 @@ static int pnv_eeh_next_error(struct eeh_pe **pe)
 	struct eeh_pe *phb_pe, *parent_pe;
 	__be64 frozen_pe_no;
 	__be16 err_type, severity;
-	int active_flags = (EEH_STATE_MMIO_ACTIVE | EEH_STATE_DMA_ACTIVE);
 	long rc;
 	int state, ret = EEH_NEXT_ERR_NONE;
 
@@ -1626,8 +1622,7 @@ static int pnv_eeh_next_error(struct eeh_pe **pe)
 
 				/* Frozen parent PE ? */
 				state = eeh_ops->get_state(parent_pe, NULL);
-				if (state > 0 &&
-				    (state & active_flags) != active_flags)
+				if (state > 0 && !eeh_state_active(state))
 					*pe = parent_pe;
 
 				/* Next parent level */
-- 
2.16.1.74.g9b0b1f47b

^ permalink raw reply related

* [PATCH v2 8/9] powerpc/eeh: Factor out common code eeh_reset_device()
From: Sam Bobroff @ 2018-03-19  2:49 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <cover.1521427331.git.sam.bobroff@au1.ibm.com>

The caller will always pass NULL for 'rmv_data' when
'eeh_aware_driver' is true, so the first two calls to
eeh_pe_dev_traverse() can be combined without changing behaviour as
can the two arms of the final 'if' block.

This should not change behaviour.

Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
---
====== v1 -> v2: ======

 arch/powerpc/kernel/eeh_driver.c | 32 ++++++++++----------------------
 1 file changed, 10 insertions(+), 22 deletions(-)

diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 93fc22e791fa..aa3a2b08aec9 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -647,16 +647,12 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
 	 * into pci_hp_add_devices().
 	 */
 	eeh_pe_state_mark(pe, EEH_PE_KEEP);
-	if (!driver_eeh_aware) {
-		if (pe->type & EEH_PE_VF) {
-			eeh_pe_dev_traverse(pe, eeh_rmv_device, NULL);
-		} else {
-			pci_lock_rescan_remove();
-			pci_hp_remove_devices(bus);
-			pci_unlock_rescan_remove();
-		}
-	} else {
+	if (driver_eeh_aware || (pe->type & EEH_PE_VF)) {
 		eeh_pe_dev_traverse(pe, eeh_rmv_device, rmv_data);
+	} else {
+		pci_lock_rescan_remove();
+		pci_hp_remove_devices(bus);
+		pci_unlock_rescan_remove();
 	}
 
 	/*
@@ -691,8 +687,9 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
 	 * the device up before the scripts have taken it down,
 	 * potentially weird things happen.
 	 */
-	if (!driver_eeh_aware) {
-		pr_info("EEH: Sleep 5s ahead of complete hotplug\n");
+	if (!driver_eeh_aware || rmv_data->removed) {
+		pr_info("EEH: Sleep 5s ahead of %s hotplug\n",
+			(eeh_aware_driver ? "partial" : "complete"));
 		ssleep(5);
 
 		/*
@@ -705,19 +702,10 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
 		if (pe->type & EEH_PE_VF) {
 			eeh_add_virt_device(edev, NULL);
 		} else {
-			eeh_pe_state_clear(pe, EEH_PE_PRI_BUS);
+			if (!eeh_aware_driver)
+				eeh_pe_state_clear(pe, EEH_PE_PRI_BUS);
 			pci_hp_add_devices(bus);
 		}
-	} else if (rmv_data->removed) {
-		pr_info("EEH: Sleep 5s ahead of partial hotplug\n");
-		ssleep(5);
-
-		edev = list_first_entry(&pe->edevs, struct eeh_dev, list);
-		eeh_pe_traverse(pe, eeh_pe_detach_dev, NULL);
-		if (pe->type & EEH_PE_VF)
-			eeh_add_virt_device(edev, NULL);
-		else
-			pci_hp_add_devices(bus);
 	}
 	eeh_pe_state_clear(pe, EEH_PE_KEEP);
 
-- 
2.16.1.74.g9b0b1f47b

^ permalink raw reply related

* [PATCH v2 7/9] powerpc/eeh: Remove always-true tests in eeh_reset_device()
From: Sam Bobroff @ 2018-03-19  2:49 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <cover.1521427331.git.sam.bobroff@au1.ibm.com>

eeh_reset_device() tests the value of 'bus' more than once but the
only caller, eeh_handle_normal_device() does this test itself and will
never pass NULL.

So, remove the dead tests.

This should not change behaviour.

Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
---
 arch/powerpc/kernel/eeh_driver.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 07437d765434..93fc22e791fa 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -655,7 +655,7 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
 			pci_hp_remove_devices(bus);
 			pci_unlock_rescan_remove();
 		}
-	} else if (bus) {
+	} else {
 		eeh_pe_dev_traverse(pe, eeh_rmv_device, rmv_data);
 	}
 
@@ -708,7 +708,7 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
 			eeh_pe_state_clear(pe, EEH_PE_PRI_BUS);
 			pci_hp_add_devices(bus);
 		}
-	} else if (bus && rmv_data->removed) {
+	} else if (rmv_data->removed) {
 		pr_info("EEH: Sleep 5s ahead of partial hotplug\n");
 		ssleep(5);
 
-- 
2.16.1.74.g9b0b1f47b

^ permalink raw reply related

* [PATCH v2 6/9] powerpc/eeh: Clarify arguments to eeh_reset_device()
From: Sam Bobroff @ 2018-03-19  2:48 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <cover.1521427331.git.sam.bobroff@au1.ibm.com>

It is currently difficult to understand the behaviour of
eeh_reset_device() due to the way it's parameters are used. In
particular, when 'bus' is NULL, it's value is still necessary so the
same value is looked up again locally under a different name
('frozen_bus') but behaviour is changed.

To clarify this, add a new parameter 'driver_eeh_aware', and have the
caller set it when it would have passed NULL for 'bus' and always pass
a value for 'bus'. Then change any test that was on 'bus' to one on
'!driver_eeh_aware' and replace uses of 'frozen_bus' with 'bus'.

Also update the function's comment.

This should not change behaviour.

Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
---
 arch/powerpc/kernel/eeh_driver.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index cb584d72b0a5..07437d765434 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -619,17 +619,19 @@ int eeh_pe_reset_and_recover(struct eeh_pe *pe)
 
 /**
  * eeh_reset_device - Perform actual reset of a pci slot
+ * @driver_eeh_aware: Does the device's driver provide EEH support?
  * @pe: EEH PE
  * @bus: PCI bus corresponding to the isolcated slot
+ * @rmv_data: Optional, list to record removed devices
  *
  * This routine must be called to do reset on the indicated PE.
  * During the reset, udev might be invoked because those affected
  * PCI devices will be removed and then added.
  */
 static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
-				struct eeh_rmv_data *rmv_data)
+			    struct eeh_rmv_data *rmv_data,
+			    bool driver_eeh_aware)
 {
-	struct pci_bus *frozen_bus = eeh_pe_bus_get(pe);
 	time64_t tstamp;
 	int cnt, rc;
 	struct eeh_dev *edev;
@@ -645,7 +647,7 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
 	 * into pci_hp_add_devices().
 	 */
 	eeh_pe_state_mark(pe, EEH_PE_KEEP);
-	if (bus) {
+	if (!driver_eeh_aware) {
 		if (pe->type & EEH_PE_VF) {
 			eeh_pe_dev_traverse(pe, eeh_rmv_device, NULL);
 		} else {
@@ -653,7 +655,7 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
 			pci_hp_remove_devices(bus);
 			pci_unlock_rescan_remove();
 		}
-	} else if (frozen_bus) {
+	} else if (bus) {
 		eeh_pe_dev_traverse(pe, eeh_rmv_device, rmv_data);
 	}
 
@@ -689,7 +691,7 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
 	 * the device up before the scripts have taken it down,
 	 * potentially weird things happen.
 	 */
-	if (bus) {
+	if (!driver_eeh_aware) {
 		pr_info("EEH: Sleep 5s ahead of complete hotplug\n");
 		ssleep(5);
 
@@ -706,7 +708,7 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
 			eeh_pe_state_clear(pe, EEH_PE_PRI_BUS);
 			pci_hp_add_devices(bus);
 		}
-	} else if (frozen_bus && rmv_data->removed) {
+	} else if (bus && rmv_data->removed) {
 		pr_info("EEH: Sleep 5s ahead of partial hotplug\n");
 		ssleep(5);
 
@@ -715,7 +717,7 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
 		if (pe->type & EEH_PE_VF)
 			eeh_add_virt_device(edev, NULL);
 		else
-			pci_hp_add_devices(frozen_bus);
+			pci_hp_add_devices(bus);
 	}
 	eeh_pe_state_clear(pe, EEH_PE_KEEP);
 
@@ -820,7 +822,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
 	 */
 	if (result == PCI_ERS_RESULT_NONE) {
 		pr_info("EEH: Reset with hotplug activity\n");
-		rc = eeh_reset_device(pe, bus, NULL);
+		rc = eeh_reset_device(pe, bus, NULL, false);
 		if (rc) {
 			pr_warn("%s: Unable to reset, err=%d\n",
 				__func__, rc);
@@ -872,7 +874,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
 	/* If any device called out for a reset, then reset the slot */
 	if (result == PCI_ERS_RESULT_NEED_RESET) {
 		pr_info("EEH: Reset without hotplug activity\n");
-		rc = eeh_reset_device(pe, NULL, &rmv_data);
+		rc = eeh_reset_device(pe, bus, &rmv_data, true);
 		if (rc) {
 			pr_warn("%s: Cannot reset, err=%d\n",
 				__func__, rc);
-- 
2.16.1.74.g9b0b1f47b

^ permalink raw reply related

* [PATCH v2 5/9] powerpc/eeh: Rename frozen_bus to bus in eeh_handle_normal_event()
From: Sam Bobroff @ 2018-03-19  2:47 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <cover.1521427331.git.sam.bobroff@au1.ibm.com>

The name "frozen_bus" is misleading: it's not necessarily frozen, it's
just the PE's PCI bus.

Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
---
 arch/powerpc/kernel/eeh_driver.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 04a5d9db5499..cb584d72b0a5 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -754,14 +754,14 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
  */
 void eeh_handle_normal_event(struct eeh_pe *pe)
 {
-	struct pci_bus *frozen_bus;
+	struct pci_bus *bus;
 	struct eeh_dev *edev, *tmp;
 	int rc = 0;
 	enum pci_ers_result result = PCI_ERS_RESULT_NONE;
 	struct eeh_rmv_data rmv_data = {LIST_HEAD_INIT(rmv_data.edev_list), 0};
 
-	frozen_bus = eeh_pe_bus_get(pe);
-	if (!frozen_bus) {
+	bus = eeh_pe_bus_get(pe);
+	if (!bus) {
 		pr_err("%s: Cannot find PCI bus for PHB#%x-PE#%x\n",
 			__func__, pe->phb->global_number, pe->addr);
 		return;
@@ -820,7 +820,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
 	 */
 	if (result == PCI_ERS_RESULT_NONE) {
 		pr_info("EEH: Reset with hotplug activity\n");
-		rc = eeh_reset_device(pe, frozen_bus, NULL);
+		rc = eeh_reset_device(pe, bus, NULL);
 		if (rc) {
 			pr_warn("%s: Unable to reset, err=%d\n",
 				__func__, rc);
@@ -938,7 +938,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
 		eeh_pe_dev_mode_mark(pe, EEH_DEV_REMOVED);
 
 		pci_lock_rescan_remove();
-		pci_hp_remove_devices(frozen_bus);
+		pci_hp_remove_devices(bus);
 		pci_unlock_rescan_remove();
 		/* The passed PE should no longer be used */
 		return;
-- 
2.16.1.74.g9b0b1f47b

^ permalink raw reply related

* [PATCH v2 4/9] powerpc/eeh: Remove misleading test in eeh_handle_normal_event()
From: Sam Bobroff @ 2018-03-19  2:46 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <cover.1521427331.git.sam.bobroff@au1.ibm.com>

Remove a test that checks if "frozen_bus" is NULL, because it cannot
have changed since it was tested at the start of the function and so
must be true here.

Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
---
 arch/powerpc/kernel/eeh_driver.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 5b7a5ed4db4d..04a5d9db5499 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -930,20 +930,18 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
 	 * all removed devices correctly to avoid access
 	 * the their PCI config any more.
 	 */
-	if (frozen_bus) {
-		if (pe->type & EEH_PE_VF) {
-			eeh_pe_dev_traverse(pe, eeh_rmv_device, NULL);
-			eeh_pe_dev_mode_mark(pe, EEH_DEV_REMOVED);
-		} else {
-			eeh_pe_state_clear(pe, EEH_PE_PRI_BUS);
-			eeh_pe_dev_mode_mark(pe, EEH_DEV_REMOVED);
+	if (pe->type & EEH_PE_VF) {
+		eeh_pe_dev_traverse(pe, eeh_rmv_device, NULL);
+		eeh_pe_dev_mode_mark(pe, EEH_DEV_REMOVED);
+	} else {
+		eeh_pe_state_clear(pe, EEH_PE_PRI_BUS);
+		eeh_pe_dev_mode_mark(pe, EEH_DEV_REMOVED);
 
-			pci_lock_rescan_remove();
-			pci_hp_remove_devices(frozen_bus);
-			pci_unlock_rescan_remove();
-			/* The passed PE should no longer be used */
-			return;
-		}
+		pci_lock_rescan_remove();
+		pci_hp_remove_devices(frozen_bus);
+		pci_unlock_rescan_remove();
+		/* The passed PE should no longer be used */
+		return;
 	}
 final:
 	eeh_pe_state_clear(pe, EEH_PE_RECOVERING);
-- 
2.16.1.74.g9b0b1f47b

^ permalink raw reply related

* [PATCH v2 3/9] powerpc/eeh: Fix misleading comment in __eeh_addr_cache_get_device()
From: Sam Bobroff @ 2018-03-19  2:46 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <cover.1521427331.git.sam.bobroff@au1.ibm.com>

Commit "0ba178888b05 powerpc/eeh: Remove reference to PCI device"
removed a call to pci_dev_get() from __eeh_addr_cache_get_device() but
did not update the comment to match.

Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
---
 arch/powerpc/kernel/eeh_cache.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/eeh_cache.c b/arch/powerpc/kernel/eeh_cache.c
index d4cc26618809..201943d54a6e 100644
--- a/arch/powerpc/kernel/eeh_cache.c
+++ b/arch/powerpc/kernel/eeh_cache.c
@@ -84,8 +84,7 @@ static inline struct eeh_dev *__eeh_addr_cache_get_device(unsigned long addr)
  * @addr: mmio (PIO) phys address or i/o port number
  *
  * Given an mmio phys address, or a port number, find a pci device
- * that implements this address.  Be sure to pci_dev_put the device
- * when finished.  I/O port numbers are assumed to be offset
+ * that implements this address.  I/O port numbers are assumed to be offset
  * from zero (that is, they do *not* have pci_io_addr added in).
  * It is safe to call this function within an interrupt.
  */
-- 
2.16.1.74.g9b0b1f47b

^ permalink raw reply related

* [PATCH v2 2/9] powerpc/eeh: Manage EEH_PE_RECOVERING inside eeh_handle_normal_event()
From: Sam Bobroff @ 2018-03-19  2:46 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <cover.1521427331.git.sam.bobroff@au1.ibm.com>

Currently the EEH_PE_RECOVERING flag for a PE is managed by both the
caller and callee of eeh_handle_normal_event() (among other places not
considered here). This is complicated by the fact that the PE may
or may not have been invalidated by the call.

So move the callee's handling into eeh_handle_normal_event(), which
clarifies it and allows the return type to be changed to void (because
it no longer needs to indicate at the PE has been invalidated).

This should not change behaviour except in eeh_event_handler() where
it was previously possible to cause eeh_pe_state_clear() to be called
on an invalid PE, which is now avoided.

Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
---
 arch/powerpc/include/asm/eeh_event.h |  2 +-
 arch/powerpc/kernel/eeh_driver.c     | 29 +++++++++++------------------
 arch/powerpc/kernel/eeh_event.c      |  2 --
 3 files changed, 12 insertions(+), 21 deletions(-)

diff --git a/arch/powerpc/include/asm/eeh_event.h b/arch/powerpc/include/asm/eeh_event.h
index 0a168038882d..9884e872686f 100644
--- a/arch/powerpc/include/asm/eeh_event.h
+++ b/arch/powerpc/include/asm/eeh_event.h
@@ -34,7 +34,7 @@ struct eeh_event {
 int eeh_event_init(void);
 int eeh_send_failure_event(struct eeh_pe *pe);
 void eeh_remove_event(struct eeh_pe *pe, bool force);
-bool eeh_handle_normal_event(struct eeh_pe *pe);
+void eeh_handle_normal_event(struct eeh_pe *pe);
 void eeh_handle_special_event(void);
 
 #endif /* __KERNEL__ */
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 51b21c97910f..5b7a5ed4db4d 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -733,7 +733,8 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
 
 /**
  * eeh_handle_normal_event - Handle EEH events on a specific PE
- * @pe: EEH PE
+ * @pe: EEH PE - which should not be used after we return, as it may
+ * have been invalidated.
  *
  * Attempts to recover the given PE.  If recovery fails or the PE has failed
  * too many times, remove the PE.
@@ -750,10 +751,8 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
  * & devices under this slot, and then finally restarting the device
  * drivers (which cause a second set of hotplug events to go out to
  * userspace).
- *
- * Returns true if @pe should no longer be used, else false.
  */
-bool eeh_handle_normal_event(struct eeh_pe *pe)
+void eeh_handle_normal_event(struct eeh_pe *pe)
 {
 	struct pci_bus *frozen_bus;
 	struct eeh_dev *edev, *tmp;
@@ -765,9 +764,11 @@ bool eeh_handle_normal_event(struct eeh_pe *pe)
 	if (!frozen_bus) {
 		pr_err("%s: Cannot find PCI bus for PHB#%x-PE#%x\n",
 			__func__, pe->phb->global_number, pe->addr);
-		return false;
+		return;
 	}
 
+	eeh_pe_state_mark(pe, EEH_PE_RECOVERING);
+
 	eeh_pe_update_time_stamp(pe);
 	pe->freeze_count++;
 	if (pe->freeze_count > eeh_max_freezes) {
@@ -904,7 +905,7 @@ bool eeh_handle_normal_event(struct eeh_pe *pe)
 	pr_info("EEH: Notify device driver to resume\n");
 	eeh_pe_dev_traverse(pe, eeh_report_resume, NULL);
 
-	return false;
+	goto final;
 
 hard_fail:
 	/*
@@ -940,12 +941,12 @@ bool eeh_handle_normal_event(struct eeh_pe *pe)
 			pci_lock_rescan_remove();
 			pci_hp_remove_devices(frozen_bus);
 			pci_unlock_rescan_remove();
-
 			/* The passed PE should no longer be used */
-			return true;
+			return;
 		}
 	}
-	return false;
+final:
+	eeh_pe_state_clear(pe, EEH_PE_RECOVERING);
 }
 
 /**
@@ -1018,15 +1019,7 @@ void eeh_handle_special_event(void)
 		 */
 		if (rc == EEH_NEXT_ERR_FROZEN_PE ||
 		    rc == EEH_NEXT_ERR_FENCED_PHB) {
-			/*
-			 * eeh_handle_normal_event() can make the PE stale if it
-			 * determines that the PE cannot possibly be recovered.
-			 * Don't modify the PE state if that's the case.
-			 */
-			if (eeh_handle_normal_event(pe))
-				continue;
-
-			eeh_pe_state_clear(pe, EEH_PE_RECOVERING);
+			eeh_handle_normal_event(pe);
 		} else {
 			pci_lock_rescan_remove();
 			list_for_each_entry(hose, &hose_list, list_node) {
diff --git a/arch/powerpc/kernel/eeh_event.c b/arch/powerpc/kernel/eeh_event.c
index 872bcfe8f90e..61c9356bf9c9 100644
--- a/arch/powerpc/kernel/eeh_event.c
+++ b/arch/powerpc/kernel/eeh_event.c
@@ -73,7 +73,6 @@ static int eeh_event_handler(void * dummy)
 		/* We might have event without binding PE */
 		pe = event->pe;
 		if (pe) {
-			eeh_pe_state_mark(pe, EEH_PE_RECOVERING);
 			if (pe->type & EEH_PE_PHB)
 				pr_info("EEH: Detected error on PHB#%x\n",
 					 pe->phb->global_number);
@@ -82,7 +81,6 @@ static int eeh_event_handler(void * dummy)
 					"PHB#%x-PE#%x\n",
 					pe->phb->global_number, pe->addr);
 			eeh_handle_normal_event(pe);
-			eeh_pe_state_clear(pe, EEH_PE_RECOVERING);
 		} else {
 			eeh_handle_special_event();
 		}
-- 
2.16.1.74.g9b0b1f47b

^ permalink raw reply related

* [PATCH v2 1/9] powerpc/eeh: Remove eeh_handle_event()
From: Sam Bobroff @ 2018-03-19  2:46 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <cover.1521427331.git.sam.bobroff@au1.ibm.com>

The function eeh_handle_event(pe) does nothing other than switching
between calling eeh_handle_normal_event(pe) and
eeh_handle_special_event(). However it is only called in two places,
one where pe can't be NULL and the other where it must be NULL (see
eeh_event_handler()) so it does nothing but obscure the flow of
control.

So, remove it.

Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
---
 arch/powerpc/include/asm/eeh_event.h |  3 ++-
 arch/powerpc/kernel/eeh_driver.c     | 42 +++++++++++++-----------------------
 arch/powerpc/kernel/eeh_event.c      |  4 ++--
 3 files changed, 19 insertions(+), 30 deletions(-)

diff --git a/arch/powerpc/include/asm/eeh_event.h b/arch/powerpc/include/asm/eeh_event.h
index 1e551a2d6f82..0a168038882d 100644
--- a/arch/powerpc/include/asm/eeh_event.h
+++ b/arch/powerpc/include/asm/eeh_event.h
@@ -34,7 +34,8 @@ struct eeh_event {
 int eeh_event_init(void);
 int eeh_send_failure_event(struct eeh_pe *pe);
 void eeh_remove_event(struct eeh_pe *pe, bool force);
-void eeh_handle_event(struct eeh_pe *pe);
+bool eeh_handle_normal_event(struct eeh_pe *pe);
+void eeh_handle_special_event(void);
 
 #endif /* __KERNEL__ */
 #endif /* ASM_POWERPC_EEH_EVENT_H */
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 0c0b66fc5bfb..51b21c97910f 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -738,9 +738,22 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
  * Attempts to recover the given PE.  If recovery fails or the PE has failed
  * too many times, remove the PE.
  *
+ * While PHB detects address or data parity errors on particular PCI
+ * slot, the associated PE will be frozen. Besides, DMA's occurring
+ * to wild addresses (which usually happen due to bugs in device
+ * drivers or in PCI adapter firmware) can cause EEH error. #SERR,
+ * #PERR or other misc PCI-related errors also can trigger EEH errors.
+ *
+ * Recovery process consists of unplugging the device driver (which
+ * generated hotplug events to userspace), then issuing a PCI #RST to
+ * the device, then reconfiguring the PCI config space for all bridges
+ * & devices under this slot, and then finally restarting the device
+ * drivers (which cause a second set of hotplug events to go out to
+ * userspace).
+ *
  * Returns true if @pe should no longer be used, else false.
  */
-static bool eeh_handle_normal_event(struct eeh_pe *pe)
+bool eeh_handle_normal_event(struct eeh_pe *pe)
 {
 	struct pci_bus *frozen_bus;
 	struct eeh_dev *edev, *tmp;
@@ -942,7 +955,7 @@ static bool eeh_handle_normal_event(struct eeh_pe *pe)
  * specific PE.  Iterates through possible failures and handles them as
  * necessary.
  */
-static void eeh_handle_special_event(void)
+void eeh_handle_special_event(void)
 {
 	struct eeh_pe *pe, *phb_pe;
 	struct pci_bus *bus;
@@ -1049,28 +1062,3 @@ static void eeh_handle_special_event(void)
 			break;
 	} while (rc != EEH_NEXT_ERR_NONE);
 }
-
-/**
- * eeh_handle_event - Reset a PCI device after hard lockup.
- * @pe: EEH PE
- *
- * While PHB detects address or data parity errors on particular PCI
- * slot, the associated PE will be frozen. Besides, DMA's occurring
- * to wild addresses (which usually happen due to bugs in device
- * drivers or in PCI adapter firmware) can cause EEH error. #SERR,
- * #PERR or other misc PCI-related errors also can trigger EEH errors.
- *
- * Recovery process consists of unplugging the device driver (which
- * generated hotplug events to userspace), then issuing a PCI #RST to
- * the device, then reconfiguring the PCI config space for all bridges
- * & devices under this slot, and then finally restarting the device
- * drivers (which cause a second set of hotplug events to go out to
- * userspace).
- */
-void eeh_handle_event(struct eeh_pe *pe)
-{
-	if (pe)
-		eeh_handle_normal_event(pe);
-	else
-		eeh_handle_special_event();
-}
diff --git a/arch/powerpc/kernel/eeh_event.c b/arch/powerpc/kernel/eeh_event.c
index accbf8b5fd46..872bcfe8f90e 100644
--- a/arch/powerpc/kernel/eeh_event.c
+++ b/arch/powerpc/kernel/eeh_event.c
@@ -81,10 +81,10 @@ static int eeh_event_handler(void * dummy)
 				pr_info("EEH: Detected PCI bus error on "
 					"PHB#%x-PE#%x\n",
 					pe->phb->global_number, pe->addr);
-			eeh_handle_event(pe);
+			eeh_handle_normal_event(pe);
 			eeh_pe_state_clear(pe, EEH_PE_RECOVERING);
 		} else {
-			eeh_handle_event(NULL);
+			eeh_handle_special_event();
 		}
 
 		kfree(event);
-- 
2.16.1.74.g9b0b1f47b

^ permalink raw reply related

* [PATCH v2 0/9] EEH refactoring 1
From: Sam Bobroff @ 2018-03-19  2:45 UTC (permalink / raw)
  To: linuxppc-dev

Hello everyone,

Here is a set of some small, mostly idempotent, changes to improve
maintainability in some of the EEH code, primarily in eeh_driver.c.

I've kept them all small to aid review but perhaps they should be squashed down
before being applied.

Cheers,
Sam.

Patch set changelog follows:

====== v1 -> v2: ======

Patch 1/9: powerpc/eeh: Remove eeh_handle_event()

Patch 2/9: powerpc/eeh: Manage EEH_PE_RECOVERING inside eeh_handle_normal_event()

Patch 3/9: powerpc/eeh: Fix misleading comment in __eeh_addr_cache_get_device()

Patch 4/9: powerpc/eeh: Remove misleading test in eeh_handle_normal_event()

Patch 5/9: powerpc/eeh: Rename frozen_bus to bus in eeh_handle_normal_event()

Patch 6/9: powerpc/eeh: Clarify arguments to eeh_reset_device()
* Re-ordered parameters to eeh_reset_device() to keep pe first.
* Changed eeh_aware_driver to driver_eeh_aware.

Patch 7/9: powerpc/eeh: Remove always-true tests in eeh_reset_device()

Patch 8/9: powerpc/eeh: Factor out common code eeh_reset_device()
* In one case, added braces to "if" to match "else".

Patch 9/9: powerpc/eeh: Add eeh_state_active() helper

====== v1: ======

Patch 1/9: powerpc/eeh: Remove eeh_handle_event()
Patch 2/9: powerpc/eeh: Manage EEH_PE_RECOVERING inside eeh_handle_normal_event()
Patch 3/9: powerpc/eeh: Fix misleading comment in __eeh_addr_cache_get_device()
Patch 4/9: powerpc/eeh: Remove misleading test in eeh_handle_normal_event()
Patch 5/9: powerpc/eeh: Rename frozen_bus to bus in eeh_handle_normal_event()
Patch 6/9: powerpc/eeh: Clarify arguments to eeh_reset_device()
Patch 7/9: powerpc/eeh: Remove always-true tests in eeh_reset_device()
Patch 8/9: powerpc/eeh: Factor out common code eeh_reset_device()
Patch 9/9: powerpc/eeh: Add eeh_state_active() helper

Sam Bobroff (9):
  powerpc/eeh: Remove eeh_handle_event()
  powerpc/eeh: Manage EEH_PE_RECOVERING inside eeh_handle_normal_event()
  powerpc/eeh: Fix misleading comment in __eeh_addr_cache_get_device()
  powerpc/eeh: Remove misleading test in eeh_handle_normal_event()
  powerpc/eeh: Rename frozen_bus to bus in eeh_handle_normal_event()
  powerpc/eeh: Clarify arguments to eeh_reset_device()
  powerpc/eeh: Remove always-true tests in eeh_reset_device()
  powerpc/eeh: Factor out common code eeh_reset_device()
  powerpc/eeh: Add eeh_state_active() helper

 arch/powerpc/include/asm/eeh.h               |   6 ++
 arch/powerpc/include/asm/eeh_event.h         |   3 +-
 arch/powerpc/kernel/eeh.c                    |  19 ++--
 arch/powerpc/kernel/eeh_cache.c              |   3 +-
 arch/powerpc/kernel/eeh_driver.c             | 137 +++++++++++----------------
 arch/powerpc/kernel/eeh_event.c              |   6 +-
 arch/powerpc/platforms/powernv/eeh-powernv.c |   9 +-
 7 files changed, 72 insertions(+), 111 deletions(-)

-- 
2.16.1.74.g9b0b1f47b

^ permalink raw reply

* Re: [PATCH v3 18/18] infiniband: cxgb4: Eliminate duplicate barriers on weakly-ordered archs
From: Jason Gunthorpe @ 2018-03-19  1:48 UTC (permalink / raw)
  To: Sinan Kaya
  Cc: Steve Wise, netdev, timur, sulrich, linux-arm-msm,
	linux-arm-kernel, 'Steve Wise', 'Doug Ledford',
	linux-rdma, linux-kernel, 'Michael Werner',
	'Casey Leedom',
	open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)
In-Reply-To: <ac9c34e1-0a81-a494-c7c7-ac9c3922095a@codeaurora.org>

On Sat, Mar 17, 2018 at 02:30:10PM -0400, Sinan Kaya wrote:

> Somebody also has to take a task and work very hard to get rid of __raw_writeX()
> APIs in drivers/net directory. It looked like a very common practice though
> it clearly violates multiarch portability concerns Jason and Deve highlighted.

When you posted your list I thought most of the hits were in what I'd
think of 'one-arch drivers', eg an IRQ controller or clock driver or
something.. Some might have a reason for it (eg avoiding the swap, for
instance), maybe it is a hold over from before writel_relaxed, or
maybe it is just a cargo-cult behavior..

It is the obviously multi-arch drivers that probably need some
attention..

Jason

^ permalink raw reply

* Re: [RFC PATCH 2/2] powerpc: Only support DYNAMIC_FTRACE not static
From: Michael Ellerman @ 2018-03-19  1:08 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: linuxppc-dev, linux-kernel, linux-arch, naveen.n.rao
In-Reply-To: <20180316104248.6aa11792@gandalf.local.home>

Steven Rostedt <rostedt@goodmis.org> writes:

> On Sat, 17 Mar 2018 00:46:33 +1100
> Michael Ellerman <mpe@ellerman.id.au> wrote:
>
>> We've had dynamic ftrace support for over 9 years since Steve first
>> wrote it, all the distros use dynamic, and static is basically
>> untested these days, so drop support for static ftrace.
>> 
>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>> ---
>>  arch/powerpc/Kconfig                           |  1 +
>>  arch/powerpc/include/asm/ftrace.h              |  4 +---
>>  arch/powerpc/include/asm/module.h              |  5 -----
>>  arch/powerpc/kernel/trace/ftrace.c             |  2 --
>>  arch/powerpc/kernel/trace/ftrace_32.S          | 20 ------------------
>>  arch/powerpc/kernel/trace/ftrace_64.S          | 29 --------------------------
>>  arch/powerpc/kernel/trace/ftrace_64_mprofile.S |  3 ---
>>  arch/powerpc/kernel/trace/ftrace_64_pg.S       |  2 --
>>  8 files changed, 2 insertions(+), 64 deletions(-)
>> 
>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> index 73ce5dd07642..23a325df784a 100644
>> --- a/arch/powerpc/Kconfig
>> +++ b/arch/powerpc/Kconfig
>> @@ -189,6 +189,7 @@ config PPC
>>  	select HAVE_DEBUG_STACKOVERFLOW
>>  	select HAVE_DMA_API_DEBUG
>>  	select HAVE_DYNAMIC_FTRACE
>> +	select HAVE_DYNAMIC_FTRACE_ONLY
>
> I still think adding:
>
> 	select DYNAMIC_FTRACE			if FUNCTION_TRACER
>
> is the better approach.

OK. As I said in my other reply it's a bit fragile, but it does work.

I'll do a version for powerpc using the above approach.

> But I'm all for this patch. I've debated doing the same thing for x86,
> but the only reason I have not, was because it's the only way I test
> the !DYNAMIC_FTRACE code. I've broken the static function tracing
> several times and only find out during my test suite that still tests
> that case. But yeah, it would be nice to just nuke static function
> tracing for all archs. Perhaps after we finish removing unused archs,
> that may be the way to go forward.

Yeah I did look and we still have some arches that support ftrace but
not dynamic ftrace, but there's not many.

cheers

^ permalink raw reply

* Re: [RFC PATCH 1/2] ftrace: Allow arches to opt-out of static ftrace
From: Michael Ellerman @ 2018-03-19  1:02 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: linuxppc-dev, linux-kernel, linux-arch, naveen.n.rao
In-Reply-To: <20180316104030.0df39af7@gandalf.local.home>

Steven Rostedt <rostedt@goodmis.org> writes:

> On Sat, 17 Mar 2018 00:46:32 +1100
> Michael Ellerman <mpe@ellerman.id.au> wrote:
>
>> There is a small but non-zero amount of code required by arches to
>> suppory non-dynamic (static) ftrace, and more importantly there is the
>> added work of testing both configurations.
>> 
>> There are also almost no down sides to dynamic ftrace once it's well
>> tested, other than a small increase in code/data size.
>> 
>> So give arches the option to opt-out of supporting static ftrace.
>> 
>> This is implemented as a DYNAMIC_FTRACE_CHOICE option, which controls
>> whether DYNAMIC_FTRACE is presented as a user-selectable option or if
>> it is just enabled based on its dependencies being enabled (because
>> it's already default y).
>> 
>> Then the CHOICE option depends on an arch *not* selecting
>> HAVE_DYNAMIC_FTRACE_ONLY. This would be more natural in reverse, as a
>> HAVE_STATIC_FTRACE option, but that would require updating every arch.
>> 
>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>
> Why not just add in arch/powerpc/Kconfig:
>
> config PPC
> 	[..]
> 	select DYNAMIC_FTRACE			if FUNCTION_TRACER
>
> ?
>
> It seems to work for me.

It does work, but it's a bit fragile. It requires duplicating the
dependencies of DYNAMIC_FTRACE in the 'if' condition.

Currently that's:

config DYNAMIC_FTRACE
	depends on FUNCTION_TRACER
	depends on HAVE_DYNAMIC_FTRACE

So technically we should use:

 	select DYNAMIC_FTRACE			if FUNCTION_TRACER and HAVE_DYNAMIC_FTRACE

Though we happen to know we just selected HAVE_DYNAMIC_FTRACE so we can
leave that out.

As long as the dependencies of DYNAMIC_FTRACE don't change, or the 'if'
clause is updated when they do, then it's OK and it is certainly simpler.

cheers

^ permalink raw reply

* Re: [PATCH 1/5] mtd: Initialize ->fail_addr early in mtd_erase()
From: Boris Brezillon @ 2018-03-18 21:22 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
	Richard Weinberger, Cyrille Pitchen, linux-mtd
  Cc: Joern Engel, Robert Jarzmik, Benjamin Herrenschmidt,
	Paul Mackerras, Michael Ellerman, Kyungmin Park, Artem Bityutskiy,
	Solarflare linux maintainers, Edward Cree, Bert Kenward,
	Greg Kroah-Hartman, linuxppc-dev, netdev, devel, Miquel Raynal
In-Reply-To: <20180212210311.23244-2-boris.brezillon@bootlin.com>

On Mon, 12 Feb 2018 22:03:07 +0100
Boris Brezillon <boris.brezillon@bootlin.com> wrote:

> mtd_erase() can return an error before ->fail_addr is initialized to
> MTD_FAIL_ADDR_UNKNOWN. Move this initialization at the very beginning
> of the function.

Applied the patchset after addressing Miquel's comments.

> 
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> ---
>  drivers/mtd/mtdcore.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index a1c94526fb88..c87859ff338b 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -953,6 +953,8 @@ EXPORT_SYMBOL_GPL(__put_mtd_device);
>   */
>  int mtd_erase(struct mtd_info *mtd, struct erase_info *instr)
>  {
> +	instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
> +
>  	if (!mtd->erasesize || !mtd->_erase)
>  		return -ENOTSUPP;
>  
> @@ -961,7 +963,6 @@ int mtd_erase(struct mtd_info *mtd, struct erase_info *instr)
>  	if (!(mtd->flags & MTD_WRITEABLE))
>  		return -EROFS;
>  
> -	instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
>  	if (!instr->len) {
>  		instr->state = MTD_ERASE_DONE;
>  		mtd_erase_callback(instr);



-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply

* Re: [RFC PATCH 2/6] fs: provide a generic compat_sys_truncate64() implementation
From: Linus Torvalds @ 2018-03-18 18:21 UTC (permalink / raw)
  To: Dominik Brodowski
  Cc: Linux Kernel Mailing List, Arnd Bergmann, Al Viro, linux-arch,
	Ralf Baechle, James Hogan, linux-mips, Benjamin Herrenschmidt,
	Paul Mackerras, Michael Ellerman, ppc-dev, Martin Schwidefsky,
	Heiko Carstens, linux-s390, David S . Miller, sparclinux,
	Ingo Molnar, Jiri Slaby, the arch/x86 maintainers
In-Reply-To: <20180318161056.5377-3-linux@dominikbrodowski.net>

On Sun, Mar 18, 2018 at 9:10 AM, Dominik Brodowski
<linux@dominikbrodowski.net> wrote:
> +#ifdef __ARCH_WANT_COMPAT_SYS_TRUNCATE64
> +#if defined(__ARCH_WANT_COMPAT_SYS_WITH_PADDING) && \
> +       defined(__ARCH_WANT_LE_COMPAT_SYS)
> +COMPAT_SYSCALL_DEFINE4(truncate64, const char __user *, filename, u32 padding,
> +                      unsigned int, offset_low, unsigned int, offset_high)
> +#elif defined(__ARCH_WANT_COMPAT_SYS_WITH_PADDING) && \
> +       !defined(__ARCH_WANT_LE_COMPAT_SYS)
> +COMPAT_SYSCALL_DEFINE4(truncate64, const char __user *, filename, u32 padding,
> +                      unsigned int, offset_high, unsigned int, offset_low)
> +#elif !defined(__ARCH_WANT_COMPAT_SYS_WITH_PADDING) && \
> +       defined(__ARCH_WANT_LE_COMPAT_SYS)
> +COMPAT_SYSCALL_DEFINE3(truncate64, const char __user *, filename,
> +                      unsigned int, offset_low, unsigned int, offset_high)
> +#else /* no padding, big endian */
> +COMPAT_SYSCALL_DEFINE3(truncate64, const char __user *, filename,
> +                      unsigned int, offset_high, unsigned int, offset_low)
> +#endif
> +{
> +#ifdef CONFIG_SPARC
> +       if ((int) offset_high < 0)
> +               return -EINVAL;
> +#endif
> +       return do_sys_truncate(filename,
> +                              ((loff_t) offset_high << 32) | offset_low);
> +}
> +#endif /* __ARCH_WANT_COMPAT_SYS_TRUNCATE64 */

This really screams out for a sparc-specific wrapper, or maybe that
#ifdef CONFIG_SPARC should just happen for everybody.

But regardless, code like the above is completely unacceptable.

And yes, it also shows that my suggested alternative doesn't really
work, because of the way the padding changes the number of arguments,
giving that whole COMPAT_SYSCALL_DEFINE 3-vs-4 argument version.

So I think just making it be arch-specific is the right thing.

Alternatively, the COMPAT_SYSCALL_DEFINE() macro could be made smarter
and do self-counting. There are generally tricks to count macro
arguments, particularly when you know that they are limited. Things
like

    #define NARG(...) __NARG(__VA_ARGS__##, 6,5,4,3,2,1)
    #define _NARG(a,b,c,d,e,f,...) f

or something (I may have screwed that up, you get the idea).

                Linus

^ permalink raw reply

* Re: [RFC PATCH 4/6] mm: provide generic compat_sys_readahead() implementation
From: Al Viro @ 2018-03-18 18:18 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Dominik Brodowski, Linux Kernel Mailing List, Arnd Bergmann,
	linux-arch, Ralf Baechle, James Hogan, linux-mips,
	Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, ppc-dev,
	Martin Schwidefsky, Heiko Carstens, linux-s390, David S . Miller,
	sparclinux, Ingo Molnar, Jiri Slaby, the arch/x86 maintainers
In-Reply-To: <CA+55aFwuZCpAZRpsTGiUmG065ZHHpj+03_NeWiy-OGkMGw7e3g@mail.gmail.com>

On Sun, Mar 18, 2018 at 11:06:42AM -0700, Linus Torvalds wrote:

> and then we can do
> 
>   COMPAT_SYSCALL_DEFINE5(readahead, int, fd,
> COMPAT_ARG_64BIT_ODD(off), compat_size_t, count)
>   {
>       return do_readahead(fd, off_lo + ((u64)off_hi << 64), count);
>   }
> 
> which at least looks reasonably legible, and has *zero* ifdef's anywhere.

It's a bit more complicated, but...

> I do *not* want to see those disgusting __ARCH_WANT_LE_COMPAT_SYS
> things and crazy #ifdef's in code.

Absolutely.  Those piles of ifdefs are unreadable garbage.

> So either let the architectures do their own trivial wrappers
> entirely, or do something clean like the above. Do *not* do
> #ifdef'fery at the system call declaration time.
> 
> Also note that the "ODD" arguments may not be the ones that need
> padding. I could easily see a system call argument numbering scheme
> like
> 
>    r0 - system call number
>    r1 - first argument
>    r2 - second argument
>    ...
> 
> and then it's the *EVEN* 64-bit arguments that would need the padding
> (because they are actually odd in the register numbers). The above
> COMPAT_ARG_64BIT[_ODD]() model allows for that too.
> 
> Of course, if some architecture then has some other arbitrary rules (I
> could see register pairing rules that aren't the usual "even register"
> ones), then such an architecture would really have to have its own
> wrapper, but the above at least would handle the simple cases, and
> doesn't look disgusting to use.

I'd done some digging in that area, will find the notes and post.
Basically, we can even avoid the odd/even annotations and have
COMPAT_SYSCALL_DEFINE... sort it out.  It's a bit more hairy than
I would like at this stage in the cycle, though.  I'll see if it can
be done without too much PITA.

However, there still are genuinely speci^Wfucked in head cases - see
e.g. this sad story:
commit ab8a261ba5e2dd9206da640de5870cc31d568a7c
Author: Helge Deller <deller@gmx.de>
Date:   Thu Jul 10 18:07:17 2014 +0200

    parisc: fix fanotify_mark() syscall on 32bit compat kernel

Those certainly ought to stay in arch/*

^ permalink raw reply

* Re: [RFC PATCH 4/6] mm: provide generic compat_sys_readahead() implementation
From: Al Viro @ 2018-03-18 17:40 UTC (permalink / raw)
  To: Dominik Brodowski
  Cc: linux-kernel, torvalds, arnd, linux-arch, Ralf Baechle,
	James Hogan, linux-mips, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, linuxppc-dev, Martin Schwidefsky,
	Heiko Carstens, linux-s390, David S . Miller, sparclinux,
	Ingo Molnar, Jiri Slaby, x86
In-Reply-To: <20180318161056.5377-5-linux@dominikbrodowski.net>

On Sun, Mar 18, 2018 at 05:10:54PM +0100, Dominik Brodowski wrote:

> +#ifdef __ARCH_WANT_COMPAT_SYS_READAHEAD
> +#if defined(__ARCH_WANT_COMPAT_SYS_WITH_PADDING) && \
> +	defined(__ARCH_WANT_LE_COMPAT_SYS)
> +COMPAT_SYSCALL_DEFINE5(readahead, int, fd, unsigned int, padding,
> +		       unsigned int, off_lo, unsigned int, off_hi,
> +		       size_t, count)
> +#elif defined(__ARCH_WANT_COMPAT_SYS_WITH_PADDING) && \
> +	!defined(__ARCH_WANT_LE_COMPAT_SYS)
> +COMPAT_SYSCALL_DEFINE5(readahead, int, fd, unsigned int, padding,
> +		       unsigned int, off_hi, unsigned int, off_lo,
> +		       size_t, count)
> +#elif !defined(__ARCH_WANT_COMPAT_SYS_WITH_PADDING) && \
> +	defined(__ARCH_WANT_LE_COMPAT_SYS)
> +COMPAT_SYSCALL_DEFINE4(readahead, int, fd,
> +		       unsigned int, off_lo, unsigned int, off_hi,
> +		       size_t, count)
> +#else /* no padding, big endian */
> +COMPAT_SYSCALL_DEFINE4(readahead, int, fd,
> +		       unsigned int, off_hi, unsigned int, off_lo,
> +		       size_t, count)
> +#endif
> +{
> +	return do_readahead(fd, ((u64) off_hi << 32) | off_lo, count);
>  }

*UGH*

static inline compat_to_u64(u32 w0, u32 w1)
{
#ifdef __BIG_ENDIAN
	return ((u64)w0 << 32) | w1;
#else
	return ((u64)w1 << 32) | w0;
#endif
}

in compat.h, then this turns into

#ifdef __ARCH_WANT_COMPAT_SYS_WITH_PADDING
COMPAT_SYSCALL_DEFINE5(readahead, int, fd, unsigned int, padding,
		       u32, off0, u32 off1,
		       compat_size_t, count)
#else
COMPAT_SYSCALL_DEFINE4(readahead, int, fd,
		       u32, off0, u32 off1,
		       compat_size_t, count)
#endif
{
	return do_readahead(fd, compat_to_u64(off0, off1), count);
}

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox