LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 5/7 v7] bus/fsl-mc: support dma configure for devices on fsl-mc bus
From: Nipun Gupta @ 2018-09-10 13:49 UTC (permalink / raw)
  To: joro, robin.murphy, will.deacon, robh+dt, robh, mark.rutland,
	catalin.marinas, gregkh, laurentiu.tudor, bhelgaas, hch
  Cc: m.szyprowski, shawnguo, frowand.list, iommu, linux-kernel,
	devicetree, linux-arm-kernel, linuxppc-dev, linux-pci,
	bharat.bhushan, stuyoder, leoyang.li, Nipun Gupta
In-Reply-To: <1536587361-11047-1-git-send-email-nipun.gupta@nxp.com>

This patch adds support of dma configuration for devices on fsl-mc
bus using 'dma_configure' callback for busses. Also, directly calling
arch_setup_dma_ops is removed from the fsl-mc bus.

Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
Reviewed-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/bus/fsl-mc/fsl-mc-bus.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
index 5d8266c..fa43c7d 100644
--- a/drivers/bus/fsl-mc/fsl-mc-bus.c
+++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
@@ -127,6 +127,16 @@ static int fsl_mc_bus_uevent(struct device *dev, struct kobj_uevent_env *env)
 	return 0;
 }
 
+static int fsl_mc_dma_configure(struct device *dev)
+{
+	struct device *dma_dev = dev;
+
+	while (dev_is_fsl_mc(dma_dev))
+		dma_dev = dma_dev->parent;
+
+	return of_dma_configure(dev, dma_dev->of_node, 0);
+}
+
 static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
 			     char *buf)
 {
@@ -148,6 +158,7 @@ struct bus_type fsl_mc_bus_type = {
 	.name = "fsl-mc",
 	.match = fsl_mc_bus_match,
 	.uevent = fsl_mc_bus_uevent,
+	.dma_configure  = fsl_mc_dma_configure,
 	.dev_groups = fsl_mc_dev_groups,
 };
 EXPORT_SYMBOL_GPL(fsl_mc_bus_type);
@@ -633,10 +644,6 @@ int fsl_mc_device_add(struct fsl_mc_obj_desc *obj_desc,
 			goto error_cleanup_dev;
 	}
 
-	/* Objects are coherent, unless 'no shareability' flag set. */
-	if (!(obj_desc->flags & FSL_MC_OBJ_FLAG_NO_MEM_SHAREABILITY))
-		arch_setup_dma_ops(&mc_dev->dev, 0, 0, NULL, true);
-
 	/*
 	 * The device-specific probe callback will get invoked by device_add()
 	 */
-- 
1.9.1

^ permalink raw reply related

* [PATCH 6/7 v7] bus/fsl-mc: set coherent dma mask for devices on fsl-mc bus
From: Nipun Gupta @ 2018-09-10 13:49 UTC (permalink / raw)
  To: joro, robin.murphy, will.deacon, robh+dt, robh, mark.rutland,
	catalin.marinas, gregkh, laurentiu.tudor, bhelgaas, hch
  Cc: m.szyprowski, shawnguo, frowand.list, iommu, linux-kernel,
	devicetree, linux-arm-kernel, linuxppc-dev, linux-pci,
	bharat.bhushan, stuyoder, leoyang.li, Nipun Gupta
In-Reply-To: <1536587361-11047-1-git-send-email-nipun.gupta@nxp.com>

of_dma_configure() API expects coherent_dma_mask to be correctly
set in the devices. This patch does the needful.

Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/bus/fsl-mc/fsl-mc-bus.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
index fa43c7d..624828b 100644
--- a/drivers/bus/fsl-mc/fsl-mc-bus.c
+++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
@@ -627,6 +627,7 @@ int fsl_mc_device_add(struct fsl_mc_obj_desc *obj_desc,
 		mc_dev->icid = parent_mc_dev->icid;
 		mc_dev->dma_mask = FSL_MC_DEFAULT_DMA_MASK;
 		mc_dev->dev.dma_mask = &mc_dev->dma_mask;
+		mc_dev->dev.coherent_dma_mask = mc_dev->dma_mask;
 		dev_set_msi_domain(&mc_dev->dev,
 				   dev_get_msi_domain(&parent_mc_dev->dev));
 	}
-- 
1.9.1

^ permalink raw reply related

* [PATCH 7/7 v7] arm64: dts: ls208xa: comply with the iommu map binding for fsl_mc
From: Nipun Gupta @ 2018-09-10 13:49 UTC (permalink / raw)
  To: joro, robin.murphy, will.deacon, robh+dt, robh, mark.rutland,
	catalin.marinas, gregkh, laurentiu.tudor, bhelgaas, hch
  Cc: m.szyprowski, shawnguo, frowand.list, iommu, linux-kernel,
	devicetree, linux-arm-kernel, linuxppc-dev, linux-pci,
	bharat.bhushan, stuyoder, leoyang.li, Nipun Gupta
In-Reply-To: <1536587361-11047-1-git-send-email-nipun.gupta@nxp.com>

fsl-mc bus support the new iommu-map property. Comply to this binding
for fsl_mc bus.

Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
Reviewed-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
---
 arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
index 137ef4d..3d5e049 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
@@ -184,6 +184,7 @@
 		#address-cells = <2>;
 		#size-cells = <2>;
 		ranges;
+		dma-ranges = <0x0 0x0 0x0 0x0 0x10000 0x00000000>;
 
 		clockgen: clocking@1300000 {
 			compatible = "fsl,ls2080a-clockgen";
@@ -357,6 +358,8 @@
 			reg = <0x00000008 0x0c000000 0 0x40>,	 /* MC portal base */
 			      <0x00000000 0x08340000 0 0x40000>; /* MC control reg */
 			msi-parent = <&its>;
+			iommu-map = <0 &smmu 0 0>;	/* This is fixed-up by u-boot */
+			dma-coherent;
 			#address-cells = <3>;
 			#size-cells = <1>;
 
@@ -460,6 +463,9 @@
 			compatible = "arm,mmu-500";
 			reg = <0 0x5000000 0 0x800000>;
 			#global-interrupts = <12>;
+			#iommu-cells = <1>;
+			stream-match-mask = <0x7C00>;
+			dma-coherent;
 			interrupts = <0 13 4>, /* global secure fault */
 				     <0 14 4>, /* combined secure interrupt */
 				     <0 15 4>, /* global non-secure fault */
@@ -502,7 +508,6 @@
 				     <0 204 4>, <0 205 4>,
 				     <0 206 4>, <0 207 4>,
 				     <0 208 4>, <0 209 4>;
-			mmu-masters = <&fsl_mc 0x300 0>;
 		};
 
 		dspi: dspi@2100000 {
-- 
1.9.1

^ permalink raw reply related

* Re: Conflict between sparse and commit cafa0010cd51f ("Raise the minimum required gcc version to 4.6")
From: Luc Van Oostenryck @ 2018-09-10 13:56 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Nick Desaulniers, joe, Kees Cook, LKML, linux-sparse, sparse,
	linuxppc-dev
In-Reply-To: <9409138a-dc14-2b12-edc0-cfa0b471da6e@c-s.fr>

On Mon, Sep 10, 2018 at 01:19:07PM +0000, Christophe Leroy wrote:
> 
> 
> On 09/10/2018 11:34 AM, Luc Van Oostenryck wrote:
> > On Mon, Sep 10, 2018 at 09:56:33AM +0000, Christophe Leroy wrote:
> > > 
> > > # export REAL_CC=ppc-linux-gcc
> > > # make CHECK="cgcc -target=ppc -D_CALL_ELF=2 -D__GCC__=5
> > > -D__GCC_MINOR__=4" C=2 arch/powerpc/kernel/process.o
> > > scripts/kconfig/conf  --syncconfig Kconfig
> > > #
> > > # configuration written to .config
> > > #
> > >    UPD     include/config/kernel.release
> > >    UPD     include/generated/utsrelease.h
> > >    CC      kernel/bounds.s
> > >    CC      arch/powerpc/kernel/asm-offsets.s
> > >    CALL    scripts/checksyscalls.sh
> > >    CHECK   scripts/mod/empty.c
> > > Can't exec "/bin/sh": Argument list too long at /usr/local/bin/cgcc line 86.
> > > make[2]: *** [scripts/mod/empty.o] Error 1
> > > make[1]: *** [scripts/mod] Error 2
> > > make: *** [scripts] Error 2
> > 
> > OK. Clearly nobody has ever used it so :(
> > There is an infinite loop because cgcc use the env var CHECK
> > to call sparse while kbuild use CHECK to call cgcc here.
> > 
> > The following seems to work here.
> > $ export REAL_CC=ppc-linux-gcc
> > $ make CHECK="CHECK=sparse cgcc -target=ppc ...
> 
> Not yet ...
> 
> [root@pc16082vm linux-powerpc]# export REAL_CC=ppc-linux-gcc
> [root@pc16082vm linux-powerpc]# make CHECK="CHECK=sparse cgcc
> -target=ppc -D_CALL_ELF=2 -D__GNUC__=5 -D__GNUC_MINOR__=4" C=2
> arch/powerpc/kernel/process.o
>   CALL    scripts/checksyscalls.sh
>   CHECK   scripts/mod/empty.c
> <command-line>:0:0: warning: "__STDC__" redefined
> <built-in>: note: this is the location of the previous definition
> /opt/cldk-1.4.0/lib/gcc/ppc-linux/5.4.0/../../../../ppc-linux/lib/crt1.o:(.rodata+0x4):
> undefined reference to `main'
> collect2: error: ld returned 1 exit status
> make[2]: *** [scripts/mod/empty.o] Error 1
> make[1]: *** [scripts/mod] Error 2
> make: *** [scripts] Error 2

OK. Using cgcc creates more problems that it solves and this file
scripts/mod/empty.c is weird.
Dropping cgcc and simply giving the GCC version to sparse works for
me here (the needed defines are given by arch/powerpc/Makefile) but
for sure I don't have the same environment as you have:
  $ make CHECK="sparse -D__GNUC__=5 -D__GNUC_MINOR__=4" ...

Bonne chance,
-- Luc

^ permalink raw reply

* Re: Conflict between sparse and commit cafa0010cd51f ("Raise the minimum required gcc version to 4.6")
From: Christophe LEROY @ 2018-09-10 14:05 UTC (permalink / raw)
  To: Luc Van Oostenryck
  Cc: Nick Desaulniers, joe, Kees Cook, LKML, linux-sparse, sparse,
	linuxppc-dev
In-Reply-To: <20180910135604.j2phe7r3k5vs6cik@ltop.local>



Le 10/09/2018 à 15:56, Luc Van Oostenryck a écrit :
> On Mon, Sep 10, 2018 at 01:19:07PM +0000, Christophe Leroy wrote:
>>
>>
>> On 09/10/2018 11:34 AM, Luc Van Oostenryck wrote:
>>> On Mon, Sep 10, 2018 at 09:56:33AM +0000, Christophe Leroy wrote:
>>>>
>>>> # export REAL_CC=ppc-linux-gcc
>>>> # make CHECK="cgcc -target=ppc -D_CALL_ELF=2 -D__GCC__=5
>>>> -D__GCC_MINOR__=4" C=2 arch/powerpc/kernel/process.o
>>>> scripts/kconfig/conf  --syncconfig Kconfig
>>>> #
>>>> # configuration written to .config
>>>> #
>>>>     UPD     include/config/kernel.release
>>>>     UPD     include/generated/utsrelease.h
>>>>     CC      kernel/bounds.s
>>>>     CC      arch/powerpc/kernel/asm-offsets.s
>>>>     CALL    scripts/checksyscalls.sh
>>>>     CHECK   scripts/mod/empty.c
>>>> Can't exec "/bin/sh": Argument list too long at /usr/local/bin/cgcc line 86.
>>>> make[2]: *** [scripts/mod/empty.o] Error 1
>>>> make[1]: *** [scripts/mod] Error 2
>>>> make: *** [scripts] Error 2
>>>
>>> OK. Clearly nobody has ever used it so :(
>>> There is an infinite loop because cgcc use the env var CHECK
>>> to call sparse while kbuild use CHECK to call cgcc here.
>>>
>>> The following seems to work here.
>>> $ export REAL_CC=ppc-linux-gcc
>>> $ make CHECK="CHECK=sparse cgcc -target=ppc ...
>>
>> Not yet ...
>>
>> [root@pc16082vm linux-powerpc]# export REAL_CC=ppc-linux-gcc
>> [root@pc16082vm linux-powerpc]# make CHECK="CHECK=sparse cgcc
>> -target=ppc -D_CALL_ELF=2 -D__GNUC__=5 -D__GNUC_MINOR__=4" C=2
>> arch/powerpc/kernel/process.o
>>    CALL    scripts/checksyscalls.sh
>>    CHECK   scripts/mod/empty.c
>> <command-line>:0:0: warning: "__STDC__" redefined
>> <built-in>: note: this is the location of the previous definition
>> /opt/cldk-1.4.0/lib/gcc/ppc-linux/5.4.0/../../../../ppc-linux/lib/crt1.o:(.rodata+0x4):
>> undefined reference to `main'
>> collect2: error: ld returned 1 exit status
>> make[2]: *** [scripts/mod/empty.o] Error 1
>> make[1]: *** [scripts/mod] Error 2
>> make: *** [scripts] Error 2
> 
> OK. Using cgcc creates more problems that it solves and this file
> scripts/mod/empty.c is weird.
> Dropping cgcc and simply giving the GCC version to sparse works for
> me here (the needed defines are given by arch/powerpc/Makefile) but
> for sure I don't have the same environment as you have:
>    $ make CHECK="sparse -D__GNUC__=5 -D__GNUC_MINOR__=4" ...

This time it works, thanks for your help.

Should we find a may to automate that in the Makefile when CROSS_COMPILE 
is defined ?

> 
> Bonne chance,

Merci

Christophe

^ permalink raw reply

* Re: [PATCH] powerpc: fix csum_ipv6_magic() on little endian platforms
From: Xin Long @ 2018-09-10 14:28 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: benh, paulus, Michael Ellerman, jishi, LKML, linuxppc-dev, stable
In-Reply-To: <3c4cddf185438cc7200908b4f971a8e6ed7472fd.1536558233.git.christophe.leroy@c-s.fr>

On Mon, Sep 10, 2018 at 2:09 PM Christophe Leroy
<christophe.leroy@c-s.fr> wrote:
>
> On little endian platforms, csum_ipv6_magic() keeps len and proto in
> CPU byte order. This generates a bad results leading to ICMPv6 packets
> from other hosts being dropped by powerpc64le platforms.
>
> In order to fix this, len and proto should be converted to network
> byte order ie bigendian byte order. However checksumming 0x12345678
> and 0x56341278 provide the exact same result so it is enough to
> rotate the sum of len and proto by 1 byte.
>
> PPC32 only support bigendian so the fix is needed for PPC64 only
>
> Fixes: e9c4943a107b ("powerpc: Implement csum_ipv6_magic in assembly")
> Reported-by: Jianlin Shi <jishi@redhat.com>
> Reported-by: Xin Long <lucien.xin@gmail.com>
> Cc: <stable@vger.kernel.org> # 4.18+
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
>  arch/powerpc/lib/checksum_64.S | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/arch/powerpc/lib/checksum_64.S b/arch/powerpc/lib/checksum_64.S
> index 886ed94b9c13..2a68c43e13f5 100644
> --- a/arch/powerpc/lib/checksum_64.S
> +++ b/arch/powerpc/lib/checksum_64.S
> @@ -443,6 +443,9 @@ _GLOBAL(csum_ipv6_magic)
>         addc    r0, r8, r9
>         ld      r10, 0(r4)
>         ld      r11, 8(r4)
> +#ifndef CONFIG_CPU_BIG_ENDIAN
> +       rotldi  r5, r5, 8
> +#endif
>         adde    r0, r0, r10
>         add     r5, r5, r7
>         adde    r0, r0, r11
> --
> 2.13.3
>
Tested-by: Xin Long <lucien.xin@gmail.com>

^ permalink raw reply

* Re: [PATCH v2 6/9] kbuild: consolidate Devicetree dtb build rules
From: Rob Herring @ 2018-09-10 14:35 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: devicetree, linux-kernel@vger.kernel.org, Michal Marek,
	Vineet Gupta, Russell King, Catalin Marinas, Will Deacon,
	Yoshinori Sato, Michal Simek, Ralf Baechle, Paul Burton,
	James Hogan, Ley Foon Tan, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, Chris Zankel, Max Filippov,
	Linux Kbuild mailing list, arcml,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	moderated list:H8/300 ARCHITECTURE, Linux-MIPS, nios2-dev,
	linuxppc-dev, linux-xtensa
In-Reply-To: <CAK7LNAS5uEUMDpL8oCJmFWKu1YF8tof5d=2h8jzt-MLHGAEAhg@mail.gmail.com>

On Sun, Sep 9, 2018 at 6:28 PM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> 2018-09-06 8:53 GMT+09:00 Rob Herring <robh@kernel.org>:
> > There is nothing arch specific about building dtb files other than their
> > location under /arch/*/boot/dts/. Keeping each arch aligned is a pain.
> > The dependencies and supported targets are all slightly different.
> > Also, a cross-compiler for each arch is needed, but really the host
> > compiler preprocessor is perfectly fine for building dtbs. Move the
> > build rules to a common location and remove the arch specific ones. This
> > is done in a single step to avoid warnings about overriding rules.
> >
> > The build dependencies had been a mixture of 'scripts' and/or 'prepare'.
> > These pull in several dependencies some of which need a target compiler
> > (specifically devicetable-offsets.h) and aren't needed to build dtbs.
> > All that is really needed is dtc, so adjust the dependencies to only be
> > dtc.
> >
> > This change enables support 'dtbs_install' on some arches which were
> > missing the target.
> >
> > Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> > Cc: Michal Marek <michal.lkml@markovi.net>
> > Cc: Vineet Gupta <vgupta@synopsys.com>
> > Cc: Russell King <linux@armlinux.org.uk>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
> > Cc: Michal Simek <monstr@monstr.eu>
> > Cc: Ralf Baechle <ralf@linux-mips.org>
> > Cc: Paul Burton <paul.burton@mips.com>
> > Cc: James Hogan <jhogan@kernel.org>
> > Cc: Ley Foon Tan <lftan@altera.com>
> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Cc: Paul Mackerras <paulus@samba.org>
> > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > Cc: Chris Zankel <chris@zankel.net>
> > Cc: Max Filippov <jcmvbkbc@gmail.com>
> > Cc: linux-kbuild@vger.kernel.org
> > Cc: linux-snps-arc@lists.infradead.org
> > Cc: linux-arm-kernel@lists.infradead.org
> > Cc: uclinux-h8-devel@lists.sourceforge.jp
> > Cc: linux-mips@linux-mips.org
> > Cc: nios2-dev@lists.rocketboards.org
> > Cc: linuxppc-dev@lists.ozlabs.org
> > Cc: linux-xtensa@linux-xtensa.org
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > ---
> > Please ack so I can take the whole series via the DT tree.
> >
> > v2:
> >  - Fix $arch/boot/dts path check for out of tree builds
> >  - Fix dtc dependency for building built-in dtbs
> >  - Fix microblaze built-in dtb building
> >
> >  Makefile                          | 32 +++++++++++++++++++++++++++++++
> >  arch/arc/Makefile                 |  6 ------
> >  arch/arm/Makefile                 | 20 +------------------
> >  arch/arm64/Makefile               | 17 +---------------
> >  arch/c6x/Makefile                 |  2 --
> >  arch/h8300/Makefile               | 11 +----------
> >  arch/microblaze/Makefile          |  4 +---
> >  arch/microblaze/boot/dts/Makefile |  2 ++
> >  arch/mips/Makefile                | 15 +--------------
> >  arch/nds32/Makefile               |  2 +-
> >  arch/nios2/Makefile               |  7 -------
> >  arch/nios2/boot/Makefile          |  4 ----
> >  arch/powerpc/Makefile             |  3 ---
> >  arch/xtensa/Makefile              | 12 +-----------
> >  scripts/Makefile.lib              |  2 +-
> >  15 files changed, 42 insertions(+), 97 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 2b458801ba74..bc18dbbc16c5 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1212,6 +1212,32 @@ kselftest-merge:
> >                 $(srctree)/tools/testing/selftests/*/config
> >         +$(Q)$(MAKE) -f $(srctree)/Makefile olddefconfig
> >
> > +# ---------------------------------------------------------------------------
> > +# Devicetree files
> > +
> > +ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/boot/dts/),)
> > +dtstree := arch/$(SRCARCH)/boot/dts
> > +endif
> > +
> > +ifdef CONFIG_OF_EARLY_FLATTREE
> > +
> > +%.dtb %.dtb.S %.dtb.o: | dtc
> > +       $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@
>
>
> Hmm, I was worried about '%.dtb.o: | dtc'
> but seems working.
>
> Compiling %.S -> %.o requires objtool for x86,
> but x86 does not support DT.

Well, x86 does support DT to some extent. There's 2 platforms and the
DT unittests build and run on x86.

Actually, we can remove "%.dtb.S %.dtb.o" because we don't need those
as top-level build targets. Must have been a copy-n-paste relic from
before having common rules.

>
> If CONFIG_MODVERSIONS=y, scripts/genksyms/genksyms is required,
> %.dtb.S does not contain EXPORT_SYMBOL.

Okay, but that shouldn't affect any of this. We only build *.dtb.S
when doing built-in dtbs.

> BTW, 'dtc' should be a PHONY target.

Right, I found that too.

Rob

^ permalink raw reply

* How to handle PTE tables with non contiguous entries ?
From: Christophe Leroy @ 2018-09-10 14:34 UTC (permalink / raw)
  To: akpm, linux-mm, aneesh.kumar, Nicholas Piggin, Michael Ellerman,
	linuxppc-dev
  Cc: LKML

Hi,

I'm having a hard time figuring out the best way to handle the following 
situation:

On the powerpc8xx, handling 16k size pages requires to have page tables 
with 4 identical entries.

Initially I was thinking about handling this by simply modifying 
pte_index() which changing pte_t type in order to have one entry every 
16 bytes, then replicate the PTE value at *ptep, *ptep+1,*ptep+2 and 
*ptep+3 both in set_pte_at() and pte_update().

However, this doesn't work because many many places in the mm core part 
of the kernel use loops on ptep with single ptep++ increment.

Therefore did it with the following hack:

  /* PTE level */
+#if defined(CONFIG_PPC_8xx) && defined(CONFIG_PPC_16K_PAGES)
+typedef struct { pte_basic_t pte, pte1, pte2, pte3; } pte_t;
+#else
  typedef struct { pte_basic_t pte; } pte_t;
+#endif

@@ -181,7 +192,13 @@ static inline unsigned long pte_update(pte_t *p,
         : "cc" );
  #else /* PTE_ATOMIC_UPDATES */
         unsigned long old = pte_val(*p);
-       *p = __pte((old & ~clr) | set);
+       unsigned long new = (old & ~clr) | set;
+
+#if defined(CONFIG_PPC_8xx) && defined(CONFIG_PPC_16K_PAGES)
+       p->pte = p->pte1 = p->pte2 = p->pte3 = new;
+#else
+       *p = __pte(new);
+#endif
  #endif /* !PTE_ATOMIC_UPDATES */

  #ifdef CONFIG_44x


@@ -161,7 +161,11 @@ static inline void __set_pte_at(struct mm_struct 
*mm, unsigned long addr,
         /* Anything else just stores the PTE normally. That covers all 
64-bit
          * cases, and 32-bit non-hash with 32-bit PTEs.
          */
+#if defined(CONFIG_PPC_8xx) && defined(CONFIG_PPC_16K_PAGES)
+       ptep->pte = ptep->pte1 = ptep->pte2 = ptep->pte3 = pte_val(pte);
+#else
         *ptep = pte;
+#endif



But I'm not too happy with it as it means pte_t is not a single type 
anymore so passing it from one function to the other is quite heavy.


Would someone have an idea of an elegent way to handle that ?

Thanks
Christophe

^ permalink raw reply

* [PATCH v3 0/9] Devicetree build consolidation
From: Rob Herring @ 2018-09-10 15:03 UTC (permalink / raw)
  To: devicetree, linux-kernel, Masahiro Yamada
  Cc: Frank Rowand, Ley Foon Tan, Paul Burton, Will Deacon,
	Aurelien Jacquiot, Benjamin Herrenschmidt, Catalin Marinas,
	Chris Zankel, James Hogan, Ley Foon Tan, Mark Salter,
	Max Filippov, Michael Ellerman, Michal Marek, Michal Simek,
	nios2-dev, Paul Mackerras, Ralf Baechle, Russell King,
	uclinux-h8-devel, Vineet Gupta, Yoshinori Sato, linux-arm-kernel,
	linux-c6x-dev, linux-kbuild, linux-mips, linuxppc-dev,
	linux-snps-arc, linux-xtensa

This series addresses a couple of issues I have with building dts files.

First, the ability to build all the dts files in the tree. This has been
supported on most arches for some time with powerpc being the main
exception. The reason powerpc wasn't supported was it needed a change
in the location built dtb files are put.

Secondly, it's a pain to acquire all the cross-compilers needed to build
dtbs for each arch. There's no reason to build with the cross compiler and
the host compiler is perfectly fine as we only need the pre-processor.

I started addressing just those 2 problems, but kept finding small
differences such as target dependencies and dtbs_install support across
architectures. Instead of trying to align all these, I've consolidated the
build targets moving them out of the arch makefiles.

I'd like to take the series via the DT tree.

Rob

v3:
 - Rework dtc dependency to avoid 2 entry paths to scripts/dtc/. Essentially,
   I copied 'scripts_basic'.
 - Add missing scripts_basic dependency for dtc and missing PHONY tag.
 - Drop the '|' order only from dependencies
 - Drop %.dtb.S and %.dtb.o as top-level targets
 - PPC: remove duplicate mpc5200 dtbs from image-y targets

v2:
 - Fix $arch/boot/dts path check for out of tree builds
 - Fix dtc dependency for building built-in dtbs
 - Fix microblaze built-in dtb building
 - Add dtbs target for microblaze

Rob Herring (9):
  powerpc: build .dtb files in dts directory
  nios2: build .dtb files in dts directory
  nios2: use common rules to build built-in dtb
  nios2: fix building all dtbs
  c6x: use common built-in dtb support
  kbuild: consolidate Devicetree dtb build rules
  powerpc: enable building all dtbs
  c6x: enable building all dtbs
  microblaze: enable building all dtbs

 Makefile                           | 35 ++++++++++++++++++-
 arch/arc/Makefile                  |  6 ----
 arch/arm/Makefile                  | 20 +----------
 arch/arm64/Makefile                | 17 +--------
 arch/c6x/Makefile                  |  2 --
 arch/c6x/boot/dts/Makefile         | 17 ++++-----
 arch/c6x/boot/dts/linked_dtb.S     |  2 --
 arch/c6x/include/asm/sections.h    |  1 -
 arch/c6x/kernel/setup.c            |  4 +--
 arch/c6x/kernel/vmlinux.lds.S      | 10 ------
 arch/h8300/Makefile                | 11 +-----
 arch/microblaze/Makefile           |  4 +--
 arch/microblaze/boot/dts/Makefile  |  4 +++
 arch/mips/Makefile                 | 15 +-------
 arch/nds32/Makefile                |  2 +-
 arch/nios2/Makefile                | 11 +-----
 arch/nios2/boot/Makefile           | 22 ------------
 arch/nios2/boot/dts/Makefile       |  6 ++++
 arch/nios2/boot/linked_dtb.S       | 19 -----------
 arch/powerpc/Makefile              |  3 --
 arch/powerpc/boot/Makefile         | 55 ++++++++++++++----------------
 arch/powerpc/boot/dts/Makefile     |  6 ++++
 arch/powerpc/boot/dts/fsl/Makefile |  4 +++
 arch/xtensa/Makefile               | 12 +------
 scripts/Makefile                   |  3 +-
 scripts/Makefile.lib               |  2 +-
 scripts/dtc/Makefile               |  2 +-
 27 files changed, 100 insertions(+), 195 deletions(-)
 delete mode 100644 arch/c6x/boot/dts/linked_dtb.S
 create mode 100644 arch/nios2/boot/dts/Makefile
 delete mode 100644 arch/nios2/boot/linked_dtb.S
 create mode 100644 arch/powerpc/boot/dts/Makefile
 create mode 100644 arch/powerpc/boot/dts/fsl/Makefile

--
2.17.1

^ permalink raw reply

* [PATCH v3 1/9] powerpc: build .dtb files in dts directory
From: Rob Herring @ 2018-09-10 15:03 UTC (permalink / raw)
  To: devicetree, linux-kernel, Masahiro Yamada
  Cc: Frank Rowand, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, linuxppc-dev
In-Reply-To: <20180910150403.19476-1-robh@kernel.org>

Align powerpc with other architectures which build the dtb files in the
same directory as the dts files. This is also in line with most other
build targets which are located in the same directory as the source.
This move will help enable the 'dtbs' target which builds all the dtbs
regardless of kernel config.

This transition could break some scripts if they expect dtb files in the
old location.

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
v3:
 - Remove duplicate mpc5200 dtbs from image-y targets. The dtb target already
   comes from the cuImage.<platform> target.

 arch/powerpc/Makefile          |  2 +-
 arch/powerpc/boot/Makefile     | 55 ++++++++++++++++------------------
 arch/powerpc/boot/dts/Makefile |  1 +
 3 files changed, 28 insertions(+), 30 deletions(-)
 create mode 100644 arch/powerpc/boot/dts/Makefile

diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 11a1acba164a..53ea887eb34e 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -294,7 +294,7 @@ bootwrapper_install:
 	$(Q)$(MAKE) $(build)=$(boot) $(patsubst %,$(boot)/%,$@)

 %.dtb: scripts
-	$(Q)$(MAKE) $(build)=$(boot) $(patsubst %,$(boot)/%,$@)
+	$(Q)$(MAKE) $(build)=$(boot)/dts $(patsubst %,$(boot)/dts/%,$@)

 # Used to create 'merged defconfigs'
 # To use it $(call) it with the first argument as the base defconfig
diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
index 0fb96c26136f..bca5c23767df 100644
--- a/arch/powerpc/boot/Makefile
+++ b/arch/powerpc/boot/Makefile
@@ -304,9 +304,9 @@ image-$(CONFIG_PPC_ADDER875)		+= cuImage.adder875-uboot \
 					   dtbImage.adder875-redboot

 # Board ports in arch/powerpc/platform/52xx/Kconfig
-image-$(CONFIG_PPC_LITE5200)		+= cuImage.lite5200 lite5200.dtb
-image-$(CONFIG_PPC_LITE5200)		+= cuImage.lite5200b lite5200b.dtb
-image-$(CONFIG_PPC_MEDIA5200)		+= cuImage.media5200 media5200.dtb
+image-$(CONFIG_PPC_LITE5200)		+= cuImage.lite5200
+image-$(CONFIG_PPC_LITE5200)		+= cuImage.lite5200b
+image-$(CONFIG_PPC_MEDIA5200)		+= cuImage.media5200

 # Board ports in arch/powerpc/platform/82xx/Kconfig
 image-$(CONFIG_MPC8272_ADS)		+= cuImage.mpc8272ads
@@ -381,11 +381,11 @@ $(addprefix $(obj)/, $(sort $(filter zImage.%, $(image-y)))): vmlinux $(wrapperb
 	$(call if_changed,wrap,$(subst $(obj)/zImage.,,$@))

 # dtbImage% - a dtbImage is a zImage with an embedded device tree blob
-$(obj)/dtbImage.initrd.%: vmlinux $(wrapperbits) $(obj)/%.dtb FORCE
-	$(call if_changed,wrap,$*,,$(obj)/$*.dtb,$(obj)/ramdisk.image.gz)
+$(obj)/dtbImage.initrd.%: vmlinux $(wrapperbits) $(obj)/dts/%.dtb FORCE
+	$(call if_changed,wrap,$*,,$(obj)/dts/$*.dtb,$(obj)/ramdisk.image.gz)

-$(obj)/dtbImage.%: vmlinux $(wrapperbits) $(obj)/%.dtb FORCE
-	$(call if_changed,wrap,$*,,$(obj)/$*.dtb)
+$(obj)/dtbImage.%: vmlinux $(wrapperbits) $(obj)/dts/%.dtb FORCE
+	$(call if_changed,wrap,$*,,$(obj)/dts/$*.dtb)

 # This cannot be in the root of $(src) as the zImage rule always adds a $(obj)
 # prefix
@@ -395,36 +395,33 @@ $(obj)/vmlinux.strip: vmlinux
 $(obj)/uImage: vmlinux $(wrapperbits) FORCE
 	$(call if_changed,wrap,uboot)

-$(obj)/uImage.initrd.%: vmlinux $(obj)/%.dtb $(wrapperbits) FORCE
-	$(call if_changed,wrap,uboot-$*,,$(obj)/$*.dtb,$(obj)/ramdisk.image.gz)
+$(obj)/uImage.initrd.%: vmlinux $(obj)/dts/%.dtb $(wrapperbits) FORCE
+	$(call if_changed,wrap,uboot-$*,,$(obj)/dts/$*.dtb,$(obj)/ramdisk.image.gz)

-$(obj)/uImage.%: vmlinux $(obj)/%.dtb $(wrapperbits) FORCE
-	$(call if_changed,wrap,uboot-$*,,$(obj)/$*.dtb)
+$(obj)/uImage.%: vmlinux $(obj)/dts/%.dtb $(wrapperbits) FORCE
+	$(call if_changed,wrap,uboot-$*,,$(obj)/dts/$*.dtb)

-$(obj)/cuImage.initrd.%: vmlinux $(obj)/%.dtb $(wrapperbits) FORCE
-	$(call if_changed,wrap,cuboot-$*,,$(obj)/$*.dtb,$(obj)/ramdisk.image.gz)
+$(obj)/cuImage.initrd.%: vmlinux $(obj)/dts/%.dtb $(wrapperbits) FORCE
+	$(call if_changed,wrap,cuboot-$*,,$(obj)/dts/$*.dtb,$(obj)/ramdisk.image.gz)

-$(obj)/cuImage.%: vmlinux $(obj)/%.dtb $(wrapperbits) FORCE
-	$(call if_changed,wrap,cuboot-$*,,$(obj)/$*.dtb)
+$(obj)/cuImage.%: vmlinux $(obj)/dts/%.dtb $(wrapperbits) FORCE
+	$(call if_changed,wrap,cuboot-$*,,$(obj)/dts/$*.dtb)

-$(obj)/simpleImage.initrd.%: vmlinux $(obj)/%.dtb $(wrapperbits) FORCE
-	$(call if_changed,wrap,simpleboot-$*,,$(obj)/$*.dtb,$(obj)/ramdisk.image.gz)
+$(obj)/simpleImage.initrd.%: vmlinux $(obj)/dts/%.dtb $(wrapperbits) FORCE
+	$(call if_changed,wrap,simpleboot-$*,,$(obj)/dts/$*.dtb,$(obj)/ramdisk.image.gz)

-$(obj)/simpleImage.%: vmlinux $(obj)/%.dtb $(wrapperbits) FORCE
-	$(call if_changed,wrap,simpleboot-$*,,$(obj)/$*.dtb)
+$(obj)/simpleImage.%: vmlinux $(obj)/dts/%.dtb $(wrapperbits) FORCE
+	$(call if_changed,wrap,simpleboot-$*,,$(obj)/dts/$*.dtb)

-$(obj)/treeImage.initrd.%: vmlinux $(obj)/%.dtb $(wrapperbits) FORCE
-	$(call if_changed,wrap,treeboot-$*,,$(obj)/$*.dtb,$(obj)/ramdisk.image.gz)
+$(obj)/treeImage.initrd.%: vmlinux $(obj)/dts/%.dtb $(wrapperbits) FORCE
+	$(call if_changed,wrap,treeboot-$*,,$(obj)/dts/$*.dtb,$(obj)/ramdisk.image.gz)

-$(obj)/treeImage.%: vmlinux $(obj)/%.dtb $(wrapperbits) FORCE
-	$(call if_changed,wrap,treeboot-$*,,$(obj)/$*.dtb)
+$(obj)/treeImage.%: vmlinux $(obj)/dts/%.dtb $(wrapperbits) FORCE
+	$(call if_changed,wrap,treeboot-$*,,$(obj)/dts/$*.dtb)

-# Rule to build device tree blobs
-$(obj)/%.dtb: $(src)/dts/%.dts FORCE
-	$(call if_changed_dep,dtc)
-
-$(obj)/%.dtb: $(src)/dts/fsl/%.dts FORCE
-	$(call if_changed_dep,dtc)
+# Needed for the above targets to work with dts/fsl/ files
+$(obj)/dts/%.dtb: $(obj)/dts/fsl/%.dtb
+	@cp $< $@

 # If there isn't a platform selected then just strip the vmlinux.
 ifeq (,$(image-y))
diff --git a/arch/powerpc/boot/dts/Makefile b/arch/powerpc/boot/dts/Makefile
new file mode 100644
index 000000000000..f66554cd5c45
--- /dev/null
+++ b/arch/powerpc/boot/dts/Makefile
@@ -0,0 +1 @@
+# SPDX-License-Identifier: GPL-2.0
--
2.17.1

^ permalink raw reply related

* [PATCH v3 6/9] kbuild: consolidate Devicetree dtb build rules
From: Rob Herring @ 2018-09-10 15:04 UTC (permalink / raw)
  To: devicetree, linux-kernel, Masahiro Yamada
  Cc: Frank Rowand, Michal Marek, Vineet Gupta, Russell King,
	Catalin Marinas, Yoshinori Sato, Michal Simek, Ralf Baechle,
	James Hogan, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, Chris Zankel, Max Filippov, linux-kbuild,
	linux-snps-arc, linux-arm-kernel, uclinux-h8-devel, linux-mips,
	nios2-dev, linuxppc-dev, linux-xtensa, Will Deacon, Paul Burton,
	Ley Foon Tan
In-Reply-To: <20180910150403.19476-1-robh@kernel.org>

There is nothing arch specific about building dtb files other than their
location under /arch/*/boot/dts/. Keeping each arch aligned is a pain.
The dependencies and supported targets are all slightly different.
Also, a cross-compiler for each arch is needed, but really the host
compiler preprocessor is perfectly fine for building dtbs. Move the
build rules to a common location and remove the arch specific ones. This
is done in a single step to avoid warnings about overriding rules.

The build dependencies had been a mixture of 'scripts' and/or 'prepare'.
These pull in several dependencies some of which need a target compiler
(specifically devicetable-offsets.h) and aren't needed to build dtbs.
All that is really needed is dtc, so adjust the dependencies to only be
dtc.

This change enables support 'dtbs_install' on some arches which were
missing the target.

Acked-by: Will Deacon <will.deacon@arm.com>
Acked-by: Paul Burton <paul.burton@mips.com>
Acked-by: Ley Foon Tan <ley.foon.tan@intel.com>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Michal Marek <michal.lkml@markovi.net>
Cc: Vineet Gupta <vgupta@synopsys.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Michal Simek <monstr@monstr.eu>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: James Hogan <jhogan@kernel.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Chris Zankel <chris@zankel.net>
Cc: Max Filippov <jcmvbkbc@gmail.com>
Cc: linux-kbuild@vger.kernel.org
Cc: linux-snps-arc@lists.infradead.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: uclinux-h8-devel@lists.sourceforge.jp
Cc: linux-mips@linux-mips.org
Cc: nios2-dev@lists.rocketboards.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-xtensa@linux-xtensa.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
 Makefile                          | 35 ++++++++++++++++++++++++++++++-
 arch/arc/Makefile                 |  6 ------
 arch/arm/Makefile                 | 20 +-----------------
 arch/arm64/Makefile               | 17 +--------------
 arch/c6x/Makefile                 |  2 --
 arch/h8300/Makefile               | 11 +---------
 arch/microblaze/Makefile          |  4 +---
 arch/microblaze/boot/dts/Makefile |  2 ++
 arch/mips/Makefile                | 15 +------------
 arch/nds32/Makefile               |  2 +-
 arch/nios2/Makefile               |  7 -------
 arch/nios2/boot/Makefile          |  4 ----
 arch/powerpc/Makefile             |  3 ---
 arch/xtensa/Makefile              | 12 +----------
 scripts/Makefile                  |  3 +--
 scripts/Makefile.lib              |  2 +-
 scripts/dtc/Makefile              |  2 +-
 17 files changed, 46 insertions(+), 101 deletions(-)

diff --git a/Makefile b/Makefile
index 19948e556941..c43859eba70f 100644
--- a/Makefile
+++ b/Makefile
@@ -1071,7 +1071,7 @@ include/config/kernel.release: $(srctree)/Makefile FORCE
 # Carefully list dependencies so we do not try to build scripts twice
 # in parallel
 PHONY += scripts
-scripts: scripts_basic asm-generic gcc-plugins $(autoksyms_h)
+scripts: scripts_basic scripts_dtc asm-generic gcc-plugins $(autoksyms_h)
 	$(Q)$(MAKE) $(build)=$(@)
 
 # Things we need to do before we recursively start building the kernel
@@ -1215,6 +1215,33 @@ kselftest-merge:
 		$(srctree)/tools/testing/selftests/*/config
 	+$(Q)$(MAKE) -f $(srctree)/Makefile olddefconfig
 
+# ---------------------------------------------------------------------------
+# Devicetree files
+
+ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/boot/dts/),)
+dtstree := arch/$(SRCARCH)/boot/dts
+endif
+
+ifdef CONFIG_OF_EARLY_FLATTREE
+
+%.dtb : scripts_dtc
+	$(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@
+
+PHONY += dtbs dtbs_install
+dtbs: scripts_dtc
+	$(Q)$(MAKE) $(build)=$(dtstree)
+
+dtbs_install: dtbs
+	$(Q)$(MAKE) $(dtbinst)=$(dtstree)
+
+all: dtbs
+
+endif
+
+PHONY += scripts_dtc
+scripts_dtc: scripts_basic
+	$(Q)$(MAKE) $(build)=scripts/dtc
+
 # ---------------------------------------------------------------------------
 # Modules
 
@@ -1424,6 +1451,12 @@ help:
 	@echo  '  kselftest-merge - Merge all the config dependencies of kselftest to existing'
 	@echo  '                    .config.'
 	@echo  ''
+	@$(if $(dtstree), \
+		echo 'Devicetree:'; \
+		echo '* dtbs            - Build device tree blobs for enabled boards'; \
+		echo '  dtbs_install    - Install dtbs to $(INSTALL_DTBS_PATH)'; \
+		echo '')
+
 	@echo 'Userspace tools targets:'
 	@echo '  use "make tools/help"'
 	@echo '  or  "cd tools; make help"'
diff --git a/arch/arc/Makefile b/arch/arc/Makefile
index fb026196aaab..5c7bc6d62f43 100644
--- a/arch/arc/Makefile
+++ b/arch/arc/Makefile
@@ -132,11 +132,5 @@ boot_targets += uImage uImage.bin uImage.gz
 $(boot_targets): vmlinux
 	$(Q)$(MAKE) $(build)=$(boot) $(boot)/$@
 
-%.dtb %.dtb.S %.dtb.o: scripts
-	$(Q)$(MAKE) $(build)=$(boot)/dts $(boot)/dts/$@
-
-dtbs: scripts
-	$(Q)$(MAKE) $(build)=$(boot)/dts
-
 archclean:
 	$(Q)$(MAKE) $(clean)=$(boot)
diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index d1516f85f25d..161c2df6567e 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -307,12 +307,7 @@ else
 KBUILD_IMAGE := $(boot)/zImage
 endif
 
-# Build the DT binary blobs if we have OF configured
-ifeq ($(CONFIG_USE_OF),y)
-KBUILD_DTBS := dtbs
-endif
-
-all:	$(notdir $(KBUILD_IMAGE)) $(KBUILD_DTBS)
+all:	$(notdir $(KBUILD_IMAGE))
 
 
 archheaders:
@@ -339,17 +334,6 @@ $(BOOT_TARGETS): vmlinux
 $(INSTALL_TARGETS):
 	$(Q)$(MAKE) $(build)=$(boot) MACHINE=$(MACHINE) $@
 
-%.dtb: | scripts
-	$(Q)$(MAKE) $(build)=$(boot)/dts MACHINE=$(MACHINE) $(boot)/dts/$@
-
-PHONY += dtbs dtbs_install
-
-dtbs: prepare scripts
-	$(Q)$(MAKE) $(build)=$(boot)/dts
-
-dtbs_install:
-	$(Q)$(MAKE) $(dtbinst)=$(boot)/dts
-
 PHONY += vdso_install
 vdso_install:
 ifeq ($(CONFIG_VDSO),y)
@@ -371,8 +355,6 @@ define archhelp
   echo  '  uImage        - U-Boot wrapped zImage'
   echo  '  bootpImage    - Combined zImage and initial RAM disk'
   echo  '                  (supply initrd image via make variable INITRD=<path>)'
-  echo  '* dtbs          - Build device tree blobs for enabled boards'
-  echo  '  dtbs_install  - Install dtbs to $(INSTALL_DTBS_PATH)'
   echo  '  install       - Install uncompressed kernel'
   echo  '  zinstall      - Install compressed kernel'
   echo  '  uinstall      - Install U-Boot wrapped compressed kernel'
diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index 106039d25e2f..b4e994cd3a42 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -113,9 +113,8 @@ core-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a
 # Default target when executing plain make
 boot		:= arch/arm64/boot
 KBUILD_IMAGE	:= $(boot)/Image.gz
-KBUILD_DTBS	:= dtbs
 
-all:	Image.gz $(KBUILD_DTBS)
+all:	Image.gz
 
 
 Image: vmlinux
@@ -127,17 +126,6 @@ Image.%: Image
 zinstall install:
 	$(Q)$(MAKE) $(build)=$(boot) $@
 
-%.dtb: scripts
-	$(Q)$(MAKE) $(build)=$(boot)/dts $(boot)/dts/$@
-
-PHONY += dtbs dtbs_install
-
-dtbs: prepare scripts
-	$(Q)$(MAKE) $(build)=$(boot)/dts
-
-dtbs_install:
-	$(Q)$(MAKE) $(dtbinst)=$(boot)/dts
-
 PHONY += vdso_install
 vdso_install:
 	$(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso $@
@@ -145,7 +133,6 @@ vdso_install:
 # We use MRPROPER_FILES and CLEAN_FILES now
 archclean:
 	$(Q)$(MAKE) $(clean)=$(boot)
-	$(Q)$(MAKE) $(clean)=$(boot)/dts
 
 # We need to generate vdso-offsets.h before compiling certain files in kernel/.
 # In order to do that, we should use the archprepare target, but we can't since
@@ -160,8 +147,6 @@ vdso_prepare: prepare0
 define archhelp
   echo  '* Image.gz      - Compressed kernel image (arch/$(ARCH)/boot/Image.gz)'
   echo  '  Image         - Uncompressed kernel image (arch/$(ARCH)/boot/Image)'
-  echo  '* dtbs          - Build device tree blobs for enabled boards'
-  echo  '  dtbs_install  - Install dtbs to $(INSTALL_DTBS_PATH)'
   echo  '  install       - Install uncompressed kernel'
   echo  '  zinstall      - Install compressed kernel'
   echo  '                  Install using (your) ~/bin/installkernel or'
diff --git a/arch/c6x/Makefile b/arch/c6x/Makefile
index 3fe8a948e94c..b7aa854f7008 100644
--- a/arch/c6x/Makefile
+++ b/arch/c6x/Makefile
@@ -40,9 +40,7 @@ boot := arch/$(ARCH)/boot
 DTB:=$(subst dtbImage.,,$(filter dtbImage.%, $(MAKECMDGOALS)))
 export DTB
 
-ifneq ($(DTB),)
 core-y	+= $(boot)/dts/
-endif
 
 # With make 3.82 we cannot mix normal and wildcard targets
 
diff --git a/arch/h8300/Makefile b/arch/h8300/Makefile
index 58634e6bae92..4003ddc616e1 100644
--- a/arch/h8300/Makefile
+++ b/arch/h8300/Makefile
@@ -31,21 +31,12 @@ CROSS_COMPILE := h8300-unknown-linux-
 endif
 
 core-y	+= arch/$(ARCH)/kernel/ arch/$(ARCH)/mm/
-ifneq '$(CONFIG_H8300_BUILTIN_DTB)' '""'
-core-y += arch/h8300/boot/dts/
-endif
+core-y	+= arch/$(ARCH)/boot/dts/
 
 libs-y	+= arch/$(ARCH)/lib/
 
 boot := arch/h8300/boot
 
-%.dtb %.dtb.S %.dtb.o: | scripts
-	$(Q)$(MAKE) $(build)=arch/h8300/boot/dts arch/h8300/boot/dts/$@
-
-PHONY += dtbs
-dtbs: scripts
-	$(Q)$(MAKE) $(build)=arch/h8300/boot/dts
-
 archmrproper:
 
 archclean:
diff --git a/arch/microblaze/Makefile b/arch/microblaze/Makefile
index 4f3ab5707265..0823d291fbeb 100644
--- a/arch/microblaze/Makefile
+++ b/arch/microblaze/Makefile
@@ -65,9 +65,7 @@ boot := arch/microblaze/boot
 # Are we making a simpleImage.<boardname> target? If so, crack out the boardname
 DTB:=$(subst simpleImage.,,$(filter simpleImage.%, $(MAKECMDGOALS)))
 
-ifneq ($(DTB),)
-	core-y	+= $(boot)/dts/
-endif
+core-y	+= $(boot)/dts/
 
 # defines filename extension depending memory management type
 ifeq ($(CONFIG_MMU),)
diff --git a/arch/microblaze/boot/dts/Makefile b/arch/microblaze/boot/dts/Makefile
index 1f77913d404d..fe9af267f598 100644
--- a/arch/microblaze/boot/dts/Makefile
+++ b/arch/microblaze/boot/dts/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 #
 
+ifneq ($(DTB),)
 obj-y += linked_dtb.o
 
 # Ensure system.dtb exists
@@ -11,6 +12,7 @@ ifneq ($(DTB),system)
 $(obj)/system.dtb: $(obj)/$(DTB).dtb
 	$(call if_changed,cp)
 endif
+endif
 
 quiet_cmd_cp = CP      $< $@$2
 	cmd_cp = cat $< >$@$2 || (rm -f $@ && echo false)
diff --git a/arch/mips/Makefile b/arch/mips/Makefile
index d74b3742fa5d..d43eeaa6d75b 100644
--- a/arch/mips/Makefile
+++ b/arch/mips/Makefile
@@ -407,18 +407,7 @@ endif
 CLEAN_FILES += vmlinux.32 vmlinux.64
 
 # device-trees
-core-$(CONFIG_BUILTIN_DTB) += arch/mips/boot/dts/
-
-%.dtb %.dtb.S %.dtb.o: | scripts
-	$(Q)$(MAKE) $(build)=arch/mips/boot/dts arch/mips/boot/dts/$@
-
-PHONY += dtbs
-dtbs: scripts
-	$(Q)$(MAKE) $(build)=arch/mips/boot/dts
-
-PHONY += dtbs_install
-dtbs_install:
-	$(Q)$(MAKE) $(dtbinst)=arch/mips/boot/dts
+core-y += arch/mips/boot/dts/
 
 archprepare:
 ifdef CONFIG_MIPS32_N32
@@ -461,8 +450,6 @@ define archhelp
 	echo '  uImage.lzma          - U-Boot image (lzma)'
 	echo '  uImage.lzo           - U-Boot image (lzo)'
 	echo '  uzImage.bin          - U-Boot image (self-extracting)'
-	echo '  dtbs                 - Device-tree blobs for enabled boards'
-	echo '  dtbs_install         - Install dtbs to $(INSTALL_DTBS_PATH)'
 	echo
 	echo '  These will be default as appropriate for a configured platform.'
 	echo
diff --git a/arch/nds32/Makefile b/arch/nds32/Makefile
index 63f4f173e5f4..adcac830d051 100644
--- a/arch/nds32/Makefile
+++ b/arch/nds32/Makefile
@@ -43,7 +43,7 @@ CHECKFLAGS      += -D__NDS32_EB__
 endif
 
 boot := arch/nds32/boot
-core-$(BUILTIN_DTB) += $(boot)/dts/
+core-y += $(boot)/dts/
 
 .PHONY: FORCE
 
diff --git a/arch/nios2/Makefile b/arch/nios2/Makefile
index db2e78fe65c7..52c03e60b114 100644
--- a/arch/nios2/Makefile
+++ b/arch/nios2/Makefile
@@ -56,12 +56,6 @@ all: vmImage
 archclean:
 	$(Q)$(MAKE) $(clean)=$(nios2-boot)
 
-%.dtb %.dtb.S %.dtb.o: | scripts
-	$(Q)$(MAKE) $(build)=$(nios2-boot)/dts $(nios2-boot)/dts/$@
-
-dtbs:
-	$(Q)$(MAKE) $(build)=$(nios2-boot)/dts
-
 $(BOOT_TARGETS): vmlinux
 	$(Q)$(MAKE) $(build)=$(nios2-boot) $(nios2-boot)/$@
 
@@ -74,5 +68,4 @@ define archhelp
   echo  '                     (your) ~/bin/$(INSTALLKERNEL) or'
   echo  '                     (distribution) /sbin/$(INSTALLKERNEL) or'
   echo  '                     install to $$(INSTALL_PATH)'
-  echo  '  dtbs            - Build device tree blobs for enabled boards'
 endef
diff --git a/arch/nios2/boot/Makefile b/arch/nios2/boot/Makefile
index 0b48f1bf086d..37dfc7e584bc 100644
--- a/arch/nios2/boot/Makefile
+++ b/arch/nios2/boot/Makefile
@@ -31,9 +31,5 @@ $(obj)/zImage: $(obj)/compressed/vmlinux FORCE
 $(obj)/compressed/vmlinux: $(obj)/vmlinux.gz FORCE
 	$(Q)$(MAKE) $(build)=$(obj)/compressed $@
 
-targets += $(dtb-y)
-
-$(obj)/dtbs: $(addprefix $(obj)/, $(dtb-y))
-
 install:
 	sh $(srctree)/$(src)/install.sh $(KERNELRELEASE) $(BOOTIMAGE) System.map "$(INSTALL_PATH)"
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 53ea887eb34e..42f225f6ec93 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -293,9 +293,6 @@ $(BOOT_TARGETS2): vmlinux
 bootwrapper_install:
 	$(Q)$(MAKE) $(build)=$(boot) $(patsubst %,$(boot)/%,$@)
 
-%.dtb: scripts
-	$(Q)$(MAKE) $(build)=$(boot)/dts $(patsubst %,$(boot)/dts/%,$@)
-
 # Used to create 'merged defconfigs'
 # To use it $(call) it with the first argument as the base defconfig
 # and the second argument as a space separated list of .config files to merge,
diff --git a/arch/xtensa/Makefile b/arch/xtensa/Makefile
index 295c120ed099..13fd3301c517 100644
--- a/arch/xtensa/Makefile
+++ b/arch/xtensa/Makefile
@@ -84,28 +84,18 @@ LIBGCC := $(shell $(CC) $(KBUILD_CFLAGS) -print-libgcc-file-name)
 head-y		:= arch/xtensa/kernel/head.o
 core-y		+= arch/xtensa/kernel/ arch/xtensa/mm/
 core-y		+= $(buildvar) $(buildplf)
+core-y 		+= arch/xtensa/boot/dts/
 
 libs-y		+= arch/xtensa/lib/ $(LIBGCC)
 drivers-$(CONFIG_OPROFILE)	+= arch/xtensa/oprofile/
 
-ifneq ($(CONFIG_BUILTIN_DTB),"")
-core-$(CONFIG_OF) += arch/xtensa/boot/dts/
-endif
-
 boot		:= arch/xtensa/boot
 
 all Image zImage uImage: vmlinux
 	$(Q)$(MAKE) $(build)=$(boot) $@
 
-%.dtb:
-	$(Q)$(MAKE) $(build)=$(boot)/dts $(boot)/dts/$@
-
-dtbs: scripts
-	$(Q)$(MAKE) $(build)=$(boot)/dts
-
 define archhelp
   @echo '* Image       - Kernel ELF image with reset vector'
   @echo '* zImage      - Compressed kernel image (arch/xtensa/boot/images/zImage.*)'
   @echo '* uImage      - U-Boot wrapped image'
-  @echo '  dtbs        - Build device tree blobs for enabled boards'
 endef
diff --git a/scripts/Makefile b/scripts/Makefile
index 61affa300d25..ece52ff20171 100644
--- a/scripts/Makefile
+++ b/scripts/Makefile
@@ -39,8 +39,7 @@ build_unifdef: $(obj)/unifdef
 subdir-$(CONFIG_MODVERSIONS) += genksyms
 subdir-y                     += mod
 subdir-$(CONFIG_SECURITY_SELINUX) += selinux
-subdir-$(CONFIG_DTC)         += dtc
 subdir-$(CONFIG_GDB_SCRIPTS) += gdb
 
 # Let clean descend into subdirs
-subdir-	+= basic kconfig package gcc-plugins
+subdir-	+= basic dtc kconfig package gcc-plugins
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 61e596650ed3..8fe4468f9bda 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -283,7 +283,7 @@ $(obj)/%.dtb.S: $(obj)/%.dtb FORCE
 
 quiet_cmd_dtc = DTC     $@
 cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \
-	$(CPP) $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; \
+	$(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $< ; \
 	$(DTC) -O dtb -o $@ -b 0 \
 		$(addprefix -i,$(dir $<) $(DTC_INCLUDE)) $(DTC_FLAGS) \
 		-d $(depfile).dtc.tmp $(dtc-tmp) ; \
diff --git a/scripts/dtc/Makefile b/scripts/dtc/Makefile
index 1c943e03eaf2..e535b457babb 100644
--- a/scripts/dtc/Makefile
+++ b/scripts/dtc/Makefile
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 # scripts/dtc makefile
 
-hostprogs-y	:= dtc
+hostprogs-$(CONFIG_DTC) := dtc
 always		:= $(hostprogs-y)
 
 dtc-objs	:= dtc.o flattree.o fstree.o data.o livetree.o treesource.o \
-- 
2.17.1

^ permalink raw reply related

* [PATCH v3 7/9] powerpc: enable building all dtbs
From: Rob Herring @ 2018-09-10 15:04 UTC (permalink / raw)
  To: devicetree, linux-kernel, Masahiro Yamada
  Cc: Frank Rowand, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, linuxppc-dev
In-Reply-To: <20180910150403.19476-1-robh@kernel.org>

Enable the 'dtbs' target for powerpc. This allows building all the dts
files in arch/powerpc/boot/dts/ when COMPILE_TEST and OF_ALL_DTBS are
enabled.

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
 arch/powerpc/boot/dts/Makefile     | 5 +++++
 arch/powerpc/boot/dts/fsl/Makefile | 4 ++++
 2 files changed, 9 insertions(+)
 create mode 100644 arch/powerpc/boot/dts/fsl/Makefile

diff --git a/arch/powerpc/boot/dts/Makefile b/arch/powerpc/boot/dts/Makefile
index f66554cd5c45..fb335d05aae8 100644
--- a/arch/powerpc/boot/dts/Makefile
+++ b/arch/powerpc/boot/dts/Makefile
@@ -1 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
+
+subdir-y += fsl
+
+dtstree		:= $(srctree)/$(src)
+dtb-$(CONFIG_OF_ALL_DTBS) := $(patsubst $(dtstree)/%.dts,%.dtb, $(wildcard $(dtstree)/*.dts))
diff --git a/arch/powerpc/boot/dts/fsl/Makefile b/arch/powerpc/boot/dts/fsl/Makefile
new file mode 100644
index 000000000000..3bae982641e9
--- /dev/null
+++ b/arch/powerpc/boot/dts/fsl/Makefile
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0
+
+dtstree		:= $(srctree)/$(src)
+dtb-$(CONFIG_OF_ALL_DTBS) := $(patsubst $(dtstree)/%.dts,%.dtb, $(wildcard $(dtstree)/*.dts))
-- 
2.17.1

^ permalink raw reply related

* Re: Conflict between sparse and commit cafa0010cd51f ("Raise the minimum required gcc version to 4.6")
From: Luc Van Oostenryck @ 2018-09-10 15:15 UTC (permalink / raw)
  To: Christophe LEROY
  Cc: Nick Desaulniers, joe, Kees Cook, LKML, linux-sparse, sparse,
	linuxppc-dev
In-Reply-To: <6191f6dc-1819-60e4-d4e8-f815f04325f0@c-s.fr>

On Mon, Sep 10, 2018 at 04:05:34PM +0200, Christophe LEROY wrote:
> 
> This time it works, thanks for your help.

You're welcome.
 
> Should we find a may to automate that in the Makefile when
> CROSS_COMPILE is defined ?

The situation here with an old gcc is really an oddity.
I was instead thinking to update sparse so that it repports a
GCC version of at least 4.6, maybe something even more recent.

But maybe, yes, kbuild could pass GCC_VERSION to sparse so
that both the compiler used and sparse will repport the same.
I'll see. The problem is not tied to cross-compilation, though,
just that sparse may be compiled with an older compiler.

-- Luc

^ permalink raw reply

* [PATCH v5 0/2] powerpc/pseries: Improve serialization of PRRN events
From: Nathan Fontenot @ 2018-09-10 14:56 UTC (permalink / raw)
  To: mpe, linuxppc-dev; +Cc: johnallen3991, desnesn, haren, pmac

Stress testing has uncovered issues with handling continuously queued PRRN
events. Running PRRN events in this way can seriously load the system given
the sheer volume of dlpar actions being handled, eventually resulting
in a system oops (see below). This patchset ensures that PRRN
events are handled more synchronously. It also updates dlpar invocation
so that it can be done directly instead of waiting on a workqueue.

Oops: Bad kernel stack pointer, sig: 6 [#1]
SMP NR_CPUS=2048 NUMA pSeries
Supported: Yes, External                                                     54
CPU: 7 PID: 18967 Comm: kworker/u96:0 Tainted: G                 X 4.4.126-94.22-default #1
Workqueue: pseries hotplug workque pseries_hp_work_fn
task: c000000775367790 ti: c00000001ebd4000 task.ti: c00000070d140000
NIP: 0000000000000000 LR: 000000001fb3d050 CTR: 0000000000000000
REGS: c00000001ebd7d40 TRAP: 0700   Tainted: G                 X  (4.4.126-94.22-default)
MSR: 8000000102081000 <41,VEC,ME5  CR: 28000002  XER: 20040018   4
CFAR: 000000001fb3d084 40 419   1                                3
GPR00: 000000000000000040000000000010007 000000001ffff400 000000041fffe200 
GPR04: 000000000000008050000000000000000 000000001fb15fa8 0000000500000500 
GPR08: 000000000001f40040000000000000001 0000000000000000 000005:5200040002
GPR12: 00000000000000005c000000007a05400 c0000000000e89f8 000000001ed9f668 
GPR16: 000000001fbeff944000000001fbeff94 000000001fb545e4 0000006000000060 
GPR20: ffffffffffffffff4ffffffffffffffff 0000000000000000 0000000000000000 
GPR24: 00000000000000005400000001fb3c000 0000000000000000 000000001fb1b040 
GPR28: 000000001fb240004000000001fb440d8 0000000000000008 0000000000000000 
NIP [0000000000000000] 5         (null)
LR [000000001fb3d050] 031fb3d050
Call Trace:            4
Instruction dump:      4                                       5:47 12    2
XXXXXXXX XXXXXXXX XXXXX4XX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX 
XXXXXXXX XXXXXXXX XXXXX5XX XXXXXXXX 60000000 60000000 60000000 60000000 

-Nathan
---

Nathan Fontenot (2):
      powerpc/pseries: Remove prrn_work workqueue
      powerpc/pseries:Remove unneeded uses of dlpar work queue


 arch/powerpc/kernel/rtasd.c               |   17 ++-----------
 arch/powerpc/platforms/pseries/dlpar.c    |   37 +++++++----------------------
 arch/powerpc/platforms/pseries/mobility.c |   18 +++++---------
 arch/powerpc/platforms/pseries/pseries.h  |    5 ++--
 arch/powerpc/platforms/pseries/ras.c      |    2 +-
 5 files changed, 22 insertions(+), 57 deletions(-)

^ permalink raw reply

* [PATCH v5 1/2] powerpc/pseries: Remove prrn_work workqueue
From: Nathan Fontenot @ 2018-09-10 14:57 UTC (permalink / raw)
  To: mpe, linuxppc-dev; +Cc: johnallen3991, desnesn, haren, pmac
In-Reply-To: <153659128238.7764.14440832607437622051.stgit@ltcalpine2-lp14.aus.stglabs.ibm.com>

When a PRRN event is received we are already running in a worker
thread. Instead of spawning off another worker thread on the prrn_work
workqueue to handle the PRRN event we can just call the PRRN handler
routine directly.

With this update we can also pass the scope variable for the PRRN
event directly to the handler instead of it being a global variable.

This patch fixes the following oops mnessage we are seeing in PRRN testing:

Oops: Bad kernel stack pointer, sig: 6 [#1]
SMP NR_CPUS=2048 NUMA pSeries
Modules linked in: nfsv3 nfs_acl rpcsec_gss_krb5 auth_rpcgss nfsv4 dns_resolver nfs lockd grace sunrpc fscache binfmt_misc reiserfs vfat fat rpadlpar_io(X) rpaphp(X) tcp_diag udp_diag inet_diag unix_diag af_packet_diag netlink_diag af_packet xfs libcrc32c dm_service_time ibmveth(X) ses enclosure scsi_transport_sas rtc_generic btrfs xor raid6_pq sd_mod ibmvscsi(X) scsi_transport_srp ipr(X) libata sg dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua scsi_mod autofs4
Supported: Yes, External                                                     54
CPU: 7 PID: 18967 Comm: kworker/u96:0 Tainted: G                 X 4.4.126-94.22-default #1
Workqueue: pseries hotplug workque pseries_hp_work_fn
task: c000000775367790 ti: c00000001ebd4000 task.ti: c00000070d140000
NIP: 0000000000000000 LR: 000000001fb3d050 CTR: 0000000000000000
REGS: c00000001ebd7d40 TRAP: 0700   Tainted: G                 X  (4.4.126-94.22-default)
MSR: 8000000102081000 <41,VEC,ME5  CR: 28000002  XER: 20040018   4
CFAR: 000000001fb3d084 40 419   1                                3
GPR00: 000000000000000040000000000010007 000000001ffff400 000000041fffe200
GPR04: 000000000000008050000000000000000 000000001fb15fa8 0000000500000500
GPR08: 000000000001f40040000000000000001 0000000000000000 000005:5200040002
GPR12: 00000000000000005c000000007a05400 c0000000000e89f8 000000001ed9f668
GPR16: 000000001fbeff944000000001fbeff94 000000001fb545e4 0000006000000060
GPR20: ffffffffffffffff4ffffffffffffffff 0000000000000000 0000000000000000
GPR24: 00000000000000005400000001fb3c000 0000000000000000 000000001fb1b040
GPR28: 000000001fb240004000000001fb440d8 0000000000000008 0000000000000000
NIP [0000000000000000] 5         (null)
LR [000000001fb3d050] 031fb3d050
Call Trace:            4
Instruction dump:      4                                       5:47 12    2
XXXXXXXX XXXXXXXX XXXXX4XX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX
XXXXXXXX XXXXXXXX XXXXX5XX XXXXXXXX 60000000 60000000 60000000 60000000
---[ end trace aa5627b04a7d9d6b ]---                                       3NMI watchdog: BUG: soft lockup - CPU#27 stuck for 23s! [kworker/27:0:13903]
Modules linked in: nfsv3 nfs_acl rpcsec_gss_krb5 auth_rpcgss nfsv4 dns_resolver nfs lockd grace sunrpc fscache binfmt_misc reiserfs vfat fat rpadlpar_io(X) rpaphp(X) tcp_diag udp_diag inet_diag unix_diag af_packet_diag netlink_diag af_packet xfs libcrc32c dm_service_time ibmveth(X) ses enclosure scsi_transport_sas rtc_generic btrfs xor raid6_pq sd_mod ibmvscsi(X) scsi_transport_srp ipr(X) libata sg dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua scsi_mod autofs4
Supported: Yes, External
CPU: 27 PID: 13903 Comm: kworker/27:0 Tainted: G      D          X 4.4.126-94.22-default #1
Workqueue: events prrn_work_fn
task: c000000747cfa390 ti: c00000074712c000 task.ti: c00000074712c000
NIP: c0000000008002a8 LR: c000000000090770 CTR: 000000000032e088
REGS: c00000074712f7b0 TRAP: 0901   Tainted: G      D          X  (4.4.126-94.22-default)
MSR: 8000000100009033 <SF,EE,ME,IR,DR,RI,LE>  CR: 22482044  XER: 20040000
CFAR: c0000000008002c4 SOFTE: 1
GPR00: c000000000090770 c00000074712fa30 c000000000f09800 c000000000fa1928 6:02
GPR04: c000000775f5e000 fffffffffffffffe 0000000000000001 c000000000f42db8
GPR08: 0000000000000001 0000000080000007 0000000000000000 0000000000000000
GPR12: 8006210083180000 c000000007a14400
NIP [c0000000008002a8] _raw_spin_lock+0x68/0xd0
LR [c000000000090770] mobility_rtas_call+0x50/0x100
Call Trace:            59                                        5
[c00000074712fa60] [c000000000090770] mobility_rtas_call+0x50/0x100
[c00000074712faf0] [c000000000090b08] pseries_devicetree_update+0xf8/0x530
[c00000074712fc20] [c000000000031ba4] prrn_work_fn+0x34/0x50
[c00000074712fc40] [c0000000000e0390] process_one_work+0x1a0/0x4e0
[c00000074712fcd0] [c0000000000e0870] worker_thread+0x1a0/0x6105:57       2
[c00000074712fd80] [c0000000000e8b18] kthread+0x128/0x150
[c00000074712fe30] [c0000000000096f8] ret_from_kernel_thread+0x5c/0x64
Instruction dump:
2c090000 40c20010 7d40192d 40c2fff0 7c2004ac 2fa90000 40de0018 5:540030   3
e8010010 ebe1fff8 7c0803a6 4e800020 <7c210b78> e92d0000 89290009 792affe3

Signed-off-by: John Allen <jallen@linux.ibm.com>
Signed-off-by: Haren Myneni <haren@us.ibm.com>
---
v5:
  - Update commit message to include oops message
v4:
  - Remove prrn_work workqueue as suggested by Michael Ellerman
  - Make the PRRN event scope passed in as opposed to a global, suggested
    by Michael Ellerman
v3:
  -Scrap the mutex as it only replicates existing workqueue behavior.
v2:
  -Unlock prrn_lock when PRRN operations are complete, not after handler is
   scheduled.
  -Remove call to flush_work, the previous broken method of serializing
   PRRN events.
---
 arch/powerpc/kernel/rtasd.c |   17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/arch/powerpc/kernel/rtasd.c b/arch/powerpc/kernel/rtasd.c
index 44d66c33d59d..23b88b923f06 100644
--- a/arch/powerpc/kernel/rtasd.c
+++ b/arch/powerpc/kernel/rtasd.c
@@ -274,27 +274,16 @@ void pSeries_log_error(char *buf, unsigned int err_type, int fatal)
 }
 
 #ifdef CONFIG_PPC_PSERIES
-static s32 prrn_update_scope;
-
-static void prrn_work_fn(struct work_struct *work)
+static void handle_prrn_event(s32 scope)
 {
 	/*
 	 * For PRRN, we must pass the negative of the scope value in
 	 * the RTAS event.
 	 */
-	pseries_devicetree_update(-prrn_update_scope);
+	pseries_devicetree_update(-scope);
 	numa_update_cpu_topology(false);
 }
 
-static DECLARE_WORK(prrn_work, prrn_work_fn);
-
-static void prrn_schedule_update(u32 scope)
-{
-	flush_work(&prrn_work);
-	prrn_update_scope = scope;
-	schedule_work(&prrn_work);
-}
-
 static void handle_rtas_event(const struct rtas_error_log *log)
 {
 	if (rtas_error_type(log) != RTAS_TYPE_PRRN || !prrn_is_enabled())
@@ -303,7 +292,7 @@ static void handle_rtas_event(const struct rtas_error_log *log)
 	/* For PRRN Events the extended log length is used to denote
 	 * the scope for calling rtas update-nodes.
 	 */
-	prrn_schedule_update(rtas_error_extended_log_length(log));
+	handle_prrn_event(rtas_error_extended_log_length(log));
 }
 
 #else

^ permalink raw reply related

* [PATCH v5 2/2] powerpc/pseries:Remove unneeded uses of dlpar work queue
From: Nathan Fontenot @ 2018-09-10 14:57 UTC (permalink / raw)
  To: mpe, linuxppc-dev; +Cc: johnallen3991, desnesn, haren, pmac
In-Reply-To: <153659128238.7764.14440832607437622051.stgit@ltcalpine2-lp14.aus.stglabs.ibm.com>

There are three instances in which dlpar hotplug events are invoked;
handling a hotplug interrupt (in a kvm guest), handling a dlpar
request through sysfs, and updating LMB affinity when handling a
PRRN event. Only in the case of handling a hotplug interrupt do we
have to put the work on a workqueue, the other cases can handle the
dlpar request directly.

This patch exports the handle_dlpar_errorlog() function so that
dlpar hotplug events can be handled directly and updates the two
instances mentioned above to use the direct invocation.

Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/pseries/dlpar.c    |   37 +++++++----------------------
 arch/powerpc/platforms/pseries/mobility.c |   18 +++++---------
 arch/powerpc/platforms/pseries/pseries.h  |    5 ++--
 arch/powerpc/platforms/pseries/ras.c      |    2 +-
 4 files changed, 19 insertions(+), 43 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
index a0b20c03f078..052c4f2ba0a0 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -32,8 +32,6 @@ static struct workqueue_struct *pseries_hp_wq;
 struct pseries_hp_work {
 	struct work_struct work;
 	struct pseries_hp_errorlog *errlog;
-	struct completion *hp_completion;
-	int *rc;
 };
 
 struct cc_workarea {
@@ -329,7 +327,7 @@ int dlpar_release_drc(u32 drc_index)
 	return 0;
 }
 
-static int handle_dlpar_errorlog(struct pseries_hp_errorlog *hp_elog)
+int handle_dlpar_errorlog(struct pseries_hp_errorlog *hp_elog)
 {
 	int rc;
 
@@ -371,20 +369,13 @@ static void pseries_hp_work_fn(struct work_struct *work)
 	struct pseries_hp_work *hp_work =
 			container_of(work, struct pseries_hp_work, work);
 
-	if (hp_work->rc)
-		*(hp_work->rc) = handle_dlpar_errorlog(hp_work->errlog);
-	else
-		handle_dlpar_errorlog(hp_work->errlog);
-
-	if (hp_work->hp_completion)
-		complete(hp_work->hp_completion);
+	handle_dlpar_errorlog(hp_work->errlog);
 
 	kfree(hp_work->errlog);
 	kfree((void *)work);
 }
 
-void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog,
-			 struct completion *hotplug_done, int *rc)
+void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog)
 {
 	struct pseries_hp_work *work;
 	struct pseries_hp_errorlog *hp_errlog_copy;
@@ -397,13 +388,9 @@ void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog,
 	if (work) {
 		INIT_WORK((struct work_struct *)work, pseries_hp_work_fn);
 		work->errlog = hp_errlog_copy;
-		work->hp_completion = hotplug_done;
-		work->rc = rc;
 		queue_work(pseries_hp_wq, (struct work_struct *)work);
 	} else {
-		*rc = -ENOMEM;
 		kfree(hp_errlog_copy);
-		complete(hotplug_done);
 	}
 }
 
@@ -521,18 +508,15 @@ static int dlpar_parse_id_type(char **cmd, struct pseries_hp_errorlog *hp_elog)
 static ssize_t dlpar_store(struct class *class, struct class_attribute *attr,
 			   const char *buf, size_t count)
 {
-	struct pseries_hp_errorlog *hp_elog;
-	struct completion hotplug_done;
+	struct pseries_hp_errorlog hp_elog;
 	char *argbuf;
 	char *args;
 	int rc;
 
 	args = argbuf = kstrdup(buf, GFP_KERNEL);
-	hp_elog = kzalloc(sizeof(*hp_elog), GFP_KERNEL);
-	if (!hp_elog || !argbuf) {
+	if (!argbuf) {
 		pr_info("Could not allocate resources for DLPAR operation\n");
 		kfree(argbuf);
-		kfree(hp_elog);
 		return -ENOMEM;
 	}
 
@@ -540,25 +524,22 @@ static ssize_t dlpar_store(struct class *class, struct class_attribute *attr,
 	 * Parse out the request from the user, this will be in the form:
 	 * <resource> <action> <id_type> <id>
 	 */
-	rc = dlpar_parse_resource(&args, hp_elog);
+	rc = dlpar_parse_resource(&args, &hp_elog);
 	if (rc)
 		goto dlpar_store_out;
 
-	rc = dlpar_parse_action(&args, hp_elog);
+	rc = dlpar_parse_action(&args, &hp_elog);
 	if (rc)
 		goto dlpar_store_out;
 
-	rc = dlpar_parse_id_type(&args, hp_elog);
+	rc = dlpar_parse_id_type(&args, &hp_elog);
 	if (rc)
 		goto dlpar_store_out;
 
-	init_completion(&hotplug_done);
-	queue_hotplug_event(hp_elog, &hotplug_done, &rc);
-	wait_for_completion(&hotplug_done);
+	rc = handle_dlpar_errorlog(&hp_elog);
 
 dlpar_store_out:
 	kfree(argbuf);
-	kfree(hp_elog);
 
 	if (rc)
 		pr_err("Could not handle DLPAR request \"%s\"\n", buf);
diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
index f0e30dc94988..6f27d00505cf 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -242,7 +242,7 @@ static int add_dt_node(__be32 parent_phandle, __be32 drc_index)
 
 static void prrn_update_node(__be32 phandle)
 {
-	struct pseries_hp_errorlog *hp_elog;
+	struct pseries_hp_errorlog hp_elog;
 	struct device_node *dn;
 
 	/*
@@ -255,18 +255,12 @@ static void prrn_update_node(__be32 phandle)
 		return;
 	}
 
-	hp_elog = kzalloc(sizeof(*hp_elog), GFP_KERNEL);
-	if(!hp_elog)
-		return;
-
-	hp_elog->resource = PSERIES_HP_ELOG_RESOURCE_MEM;
-	hp_elog->action = PSERIES_HP_ELOG_ACTION_READD;
-	hp_elog->id_type = PSERIES_HP_ELOG_ID_DRC_INDEX;
-	hp_elog->_drc_u.drc_index = phandle;
-
-	queue_hotplug_event(hp_elog, NULL, NULL);
+	hp_elog.resource = PSERIES_HP_ELOG_RESOURCE_MEM;
+	hp_elog.action = PSERIES_HP_ELOG_ACTION_READD;
+	hp_elog.id_type = PSERIES_HP_ELOG_ID_DRC_INDEX;
+	hp_elog._drc_u.drc_index = phandle;
 
-	kfree(hp_elog);
+	handle_dlpar_errorlog(&hp_elog);
 }
 
 int pseries_devicetree_update(s32 scope)
diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
index 60db2ee511fb..9310a20aef44 100644
--- a/arch/powerpc/platforms/pseries/pseries.h
+++ b/arch/powerpc/platforms/pseries/pseries.h
@@ -59,8 +59,9 @@ extern int dlpar_detach_node(struct device_node *);
 extern int dlpar_acquire_drc(u32 drc_index);
 extern int dlpar_release_drc(u32 drc_index);
 
-void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog,
-			 struct completion *hotplug_done, int *rc);
+void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog);
+int handle_dlpar_errorlog(struct pseries_hp_errorlog *hp_errlog);
+
 #ifdef CONFIG_MEMORY_HOTPLUG
 int dlpar_memory(struct pseries_hp_errorlog *hp_elog);
 #else
diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
index 851ce326874a..85a607d52014 100644
--- a/arch/powerpc/platforms/pseries/ras.c
+++ b/arch/powerpc/platforms/pseries/ras.c
@@ -238,7 +238,7 @@ static irqreturn_t ras_hotplug_interrupt(int irq, void *dev_id)
 	 */
 	if (hp_elog->resource == PSERIES_HP_ELOG_RESOURCE_MEM ||
 	    hp_elog->resource == PSERIES_HP_ELOG_RESOURCE_CPU)
-		queue_hotplug_event(hp_elog, NULL, NULL);
+		queue_hotplug_event(hp_elog);
 	else
 		log_error(ras_log_buf, ERR_TYPE_RTAS_LOG, 0);
 

^ permalink raw reply related

* Re: [PATCH v2 1/5] powerpc/pci: Access PCI config space directly w/o pci_dn
From: Sergey Miroshnichenko @ 2018-09-10 15:46 UTC (permalink / raw)
  To: Sam Bobroff; +Cc: linuxppc-dev, linux
In-Reply-To: <20180910042344.GA14370@tungsten.ozlabs.ibm.com>


[-- Attachment #1.1: Type: text/plain, Size: 11004 bytes --]

Hello Sam,

On 9/10/18 7:23 AM, Sam Bobroff wrote:
> Hi Sergey,
> 
> On Thu, Sep 06, 2018 at 02:57:48PM +0300, Sergey Miroshnichenko wrote:
>> The pci_dn structures are retrieved from a DT, but hot-plugged PCIe
>> devices don't have them. Don't stop PCIe I/O in absence of pci_dn, so
>> it is now possible to discover new devices.
>>
>> Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
>> ---
>>  arch/powerpc/kernel/rtas_pci.c       | 97 +++++++++++++++++++---------
>>  arch/powerpc/platforms/powernv/pci.c | 64 ++++++++++++------
>>  2 files changed, 109 insertions(+), 52 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/rtas_pci.c b/arch/powerpc/kernel/rtas_pci.c
>> index c2b148b1634a..0611b46d9b5f 100644
>> --- a/arch/powerpc/kernel/rtas_pci.c
>> +++ b/arch/powerpc/kernel/rtas_pci.c
>> @@ -55,10 +55,26 @@ static inline int config_access_valid(struct pci_dn *dn, int where)
>>  	return 0;
>>  }
>>  
>> -int rtas_read_config(struct pci_dn *pdn, int where, int size, u32 *val)
>> +static int rtas_read_raw_config(unsigned long buid, int busno, unsigned int devfn,
>> +				int where, int size, u32 *val)
>>  {
>>  	int returnval = -1;
>> -	unsigned long buid, addr;
>> +	unsigned long addr = rtas_config_addr(busno, devfn, where);
>> +	int ret;
>> +
>> +	if (buid) {
>> +		ret = rtas_call(ibm_read_pci_config, 4, 2, &returnval,
>> +				addr, BUID_HI(buid), BUID_LO(buid), size);
>> +	} else {
>> +		ret = rtas_call(read_pci_config, 2, 2, &returnval, addr, size);
>> +	}
>> +	*val = returnval;
>> +
>> +	return ret;
>> +}
>> +
>> +int rtas_read_config(struct pci_dn *pdn, int where, int size, u32 *val)
>> +{
>>  	int ret;
>>  
>>  	if (!pdn)
>> @@ -71,16 +87,8 @@ int rtas_read_config(struct pci_dn *pdn, int where, int size, u32 *val)
>>  		return PCIBIOS_SET_FAILED;
>>  #endif
>>  
>> -	addr = rtas_config_addr(pdn->busno, pdn->devfn, where);
>> -	buid = pdn->phb->buid;
>> -	if (buid) {
>> -		ret = rtas_call(ibm_read_pci_config, 4, 2, &returnval,
>> -				addr, BUID_HI(buid), BUID_LO(buid), size);
>> -	} else {
>> -		ret = rtas_call(read_pci_config, 2, 2, &returnval, addr, size);
>> -	}
>> -	*val = returnval;
>> -
>> +	ret = rtas_read_raw_config(pdn->phb->buid, pdn->busno, pdn->devfn,
>> +				   where, size, val);
>>  	if (ret)
>>  		return PCIBIOS_DEVICE_NOT_FOUND;
>>  
>> @@ -98,18 +106,44 @@ static int rtas_pci_read_config(struct pci_bus *bus,
>>  
>>  	pdn = pci_get_pdn_by_devfn(bus, devfn);
>>  
>> -	/* Validity of pdn is checked in here */
>> -	ret = rtas_read_config(pdn, where, size, val);
>> -	if (*val == EEH_IO_ERROR_VALUE(size) &&
>> -	    eeh_dev_check_failure(pdn_to_eeh_dev(pdn)))
>> -		return PCIBIOS_DEVICE_NOT_FOUND;
>> +	if (pdn && eeh_enabled()) {
>> +		/* Validity of pdn is checked in here */
>> +		ret = rtas_read_config(pdn, where, size, val);
>> +
>> +		if (*val == EEH_IO_ERROR_VALUE(size) &&
>> +		    eeh_dev_check_failure(pdn_to_eeh_dev(pdn)))
>> +			ret = PCIBIOS_DEVICE_NOT_FOUND;
>> +	} else {
>> +		struct pci_controller *phb = pci_bus_to_host(bus);
>> +
>> +		ret = rtas_read_raw_config(phb->buid, bus->number, devfn,
>> +					   where, size, val);
>> +	}
> 
> In the above block, if pdn is valid but EEH isn't enabled,
> rtas_read_raw_config() will be used instead of rtas_read_config(), so
> config_access_valid() won't be tested. Is that correct?
> 

Thank you for the review!

This was the original intention, but now I can see that if a pdn is
valid, the EEH-branch should be taken even if EEH is disabled, as it was
before this patch; and functions there have checks for eeh_enabled()
inside. I'll fix that in v3 as follows:

-	if (pdn && eeh_enabled()) {
+	if (pdn) {

>>  
>>  	return ret;
>>  }
>>  
>> +static int rtas_write_raw_config(unsigned long buid, int busno, unsigned int devfn,
>> +				 int where, int size, u32 val)
>> +{
>> +	unsigned long addr = rtas_config_addr(busno, devfn, where);
>> +	int ret;
>> +
>> +	if (buid) {
>> +		ret = rtas_call(ibm_write_pci_config, 5, 1, NULL, addr,
>> +				BUID_HI(buid), BUID_LO(buid), size, (ulong)val);
>> +	} else {
>> +		ret = rtas_call(write_pci_config, 3, 1, NULL, addr, size, (ulong)val);
>> +	}
>> +
>> +	if (ret)
>> +		return PCIBIOS_DEVICE_NOT_FOUND;
>> +
>> +	return PCIBIOS_SUCCESSFUL;
>> +}
>> +
>>  int rtas_write_config(struct pci_dn *pdn, int where, int size, u32 val)
>>  {
>> -	unsigned long buid, addr;
>>  	int ret;
>>  
>>  	if (!pdn)
>> @@ -122,15 +156,8 @@ int rtas_write_config(struct pci_dn *pdn, int where, int size, u32 val)
>>  		return PCIBIOS_SET_FAILED;
>>  #endif
>>  
>> -	addr = rtas_config_addr(pdn->busno, pdn->devfn, where);
>> -	buid = pdn->phb->buid;
>> -	if (buid) {
>> -		ret = rtas_call(ibm_write_pci_config, 5, 1, NULL, addr,
>> -			BUID_HI(buid), BUID_LO(buid), size, (ulong) val);
>> -	} else {
>> -		ret = rtas_call(write_pci_config, 3, 1, NULL, addr, size, (ulong)val);
>> -	}
>> -
>> +	ret = rtas_write_raw_config(pdn->phb->buid, pdn->busno, pdn->devfn,
>> +				    where, size, val);
>>  	if (ret)
>>  		return PCIBIOS_DEVICE_NOT_FOUND;
>>  
>> @@ -141,12 +168,20 @@ static int rtas_pci_write_config(struct pci_bus *bus,
>>  				 unsigned int devfn,
>>  				 int where, int size, u32 val)
>>  {
>> -	struct pci_dn *pdn;
>> +	struct pci_dn *pdn = pci_get_pdn_by_devfn(bus, devfn);
>> +	int ret;
>>  
>> -	pdn = pci_get_pdn_by_devfn(bus, devfn);
>> +	if (pdn && eeh_enabled()) {
>> +		/* Validity of pdn is checked in here. */
>> +		ret = rtas_write_config(pdn, where, size, val);
>> +	} else {
>> +		struct pci_controller *phb = pci_bus_to_host(bus);
> 
> Same comment as rtas_pci_read_config() above.
> 

I'll fix that symmetrically.

>>  
>> -	/* Validity of pdn is checked in here. */
>> -	return rtas_write_config(pdn, where, size, val);
>> +		ret = rtas_write_raw_config(phb->buid, bus->number, devfn,
>> +					    where, size, val);
>> +	}
>> +
>> +	return ret;
>>  }
>>  
>>  static struct pci_ops rtas_pci_ops = {
>> diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
>> index 13aef2323bbc..3f87a2dc6578 100644
>> --- a/arch/powerpc/platforms/powernv/pci.c
>> +++ b/arch/powerpc/platforms/powernv/pci.c
>> @@ -654,30 +654,29 @@ static void pnv_pci_config_check_eeh(struct pci_dn *pdn)
>>  	}
>>  }
>>  
>> -int pnv_pci_cfg_read(struct pci_dn *pdn,
>> -		     int where, int size, u32 *val)
>> +int pnv_pci_cfg_read_raw(u64 phb_id, int busno, unsigned int devfn,
>> +			 int where, int size, u32 *val)
>>  {
>> -	struct pnv_phb *phb = pdn->phb->private_data;
>> -	u32 bdfn = (pdn->busno << 8) | pdn->devfn;
>> +	u32 bdfn = (busno << 8) | devfn;
>>  	s64 rc;
>>  
>>  	switch (size) {
>>  	case 1: {
>>  		u8 v8;
>> -		rc = opal_pci_config_read_byte(phb->opal_id, bdfn, where, &v8);
>> +		rc = opal_pci_config_read_byte(phb_id, bdfn, where, &v8);
>>  		*val = (rc == OPAL_SUCCESS) ? v8 : 0xff;
>>  		break;
>>  	}
>>  	case 2: {
>>  		__be16 v16;
>> -		rc = opal_pci_config_read_half_word(phb->opal_id, bdfn, where,
>> -						   &v16);
>> +		rc = opal_pci_config_read_half_word(phb_id, bdfn, where,
>> +						    &v16);
>>  		*val = (rc == OPAL_SUCCESS) ? be16_to_cpu(v16) : 0xffff;
>>  		break;
>>  	}
>>  	case 4: {
>>  		__be32 v32;
>> -		rc = opal_pci_config_read_word(phb->opal_id, bdfn, where, &v32);
>> +		rc = opal_pci_config_read_word(phb_id, bdfn, where, &v32);
>>  		*val = (rc == OPAL_SUCCESS) ? be32_to_cpu(v32) : 0xffffffff;
>>  		break;
>>  	}
>> @@ -686,27 +685,28 @@ int pnv_pci_cfg_read(struct pci_dn *pdn,
>>  	}
>>  
>>  	pr_devel("%s: bus: %x devfn: %x +%x/%x -> %08x\n",
>> -		 __func__, pdn->busno, pdn->devfn, where, size, *val);
>> +		 __func__, busno, devfn, where, size, *val);
>> +
>>  	return PCIBIOS_SUCCESSFUL;
>>  }
>>  
>> -int pnv_pci_cfg_write(struct pci_dn *pdn,
>> -		      int where, int size, u32 val)
>> +int pnv_pci_cfg_write_raw(u64 phb_id, int busno, unsigned int devfn,
>> +			  int where, int size, u32 val)
>>  {
>> -	struct pnv_phb *phb = pdn->phb->private_data;
>> -	u32 bdfn = (pdn->busno << 8) | pdn->devfn;
>> +	u32 bdfn = (busno << 8) | devfn;
>>  
>>  	pr_devel("%s: bus: %x devfn: %x +%x/%x -> %08x\n",
>> -		 __func__, pdn->busno, pdn->devfn, where, size, val);
>> +		 __func__, busno, devfn, where, size, val);
>> +
>>  	switch (size) {
>>  	case 1:
>> -		opal_pci_config_write_byte(phb->opal_id, bdfn, where, val);
>> +		opal_pci_config_write_byte(phb_id, bdfn, where, val);
>>  		break;
>>  	case 2:
>> -		opal_pci_config_write_half_word(phb->opal_id, bdfn, where, val);
>> +		opal_pci_config_write_half_word(phb_id, bdfn, where, val);
>>  		break;
>>  	case 4:
>> -		opal_pci_config_write_word(phb->opal_id, bdfn, where, val);
>> +		opal_pci_config_write_word(phb_id, bdfn, where, val);
>>  		break;
>>  	default:
>>  		return PCIBIOS_FUNC_NOT_SUPPORTED;
>> @@ -715,6 +715,24 @@ int pnv_pci_cfg_write(struct pci_dn *pdn,
>>  	return PCIBIOS_SUCCESSFUL;
>>  }
>>  
>> +int pnv_pci_cfg_read(struct pci_dn *pdn,
>> +		     int where, int size, u32 *val)
>> +{
>> +	struct pnv_phb *phb = pdn->phb->private_data;
>> +
>> +	return pnv_pci_cfg_read_raw(phb->opal_id, pdn->busno, pdn->devfn,
>> +				    where, size, val);
>> +}
>> +
>> +int pnv_pci_cfg_write(struct pci_dn *pdn,
>> +		      int where, int size, u32 val)
>> +{
>> +	struct pnv_phb *phb = pdn->phb->private_data;
>> +
>> +	return pnv_pci_cfg_write_raw(phb->opal_id, pdn->busno, pdn->devfn,
>> +				     where, size, val);
>> +}
>> +
>>  #if CONFIG_EEH
>>  static bool pnv_pci_cfg_check(struct pci_dn *pdn)
>>  {
>> @@ -750,13 +768,15 @@ static int pnv_pci_read_config(struct pci_bus *bus,
>>  			       int where, int size, u32 *val)
>>  {
>>  	struct pci_dn *pdn;
>> -	struct pnv_phb *phb;
>> +	struct pci_controller *hose = pci_bus_to_host(bus);
>> +	struct pnv_phb *phb = hose->private_data;
>>  	int ret;
>>  
>>  	*val = 0xFFFFFFFF;
>>  	pdn = pci_get_pdn_by_devfn(bus, devfn);
>>  	if (!pdn)
>> -		return PCIBIOS_DEVICE_NOT_FOUND;
>> +		return pnv_pci_cfg_read_raw(phb->opal_id, bus->number, devfn,
>> +					    where, size, val);
>>  
>>  	if (!pnv_pci_cfg_check(pdn))
>>  		return PCIBIOS_DEVICE_NOT_FOUND;
>> @@ -779,12 +799,14 @@ static int pnv_pci_write_config(struct pci_bus *bus,
>>  				int where, int size, u32 val)
>>  {
>>  	struct pci_dn *pdn;
>> -	struct pnv_phb *phb;
>> +	struct pci_controller *hose = pci_bus_to_host(bus);
>> +	struct pnv_phb *phb = hose->private_data;
>>  	int ret;
>>  
>>  	pdn = pci_get_pdn_by_devfn(bus, devfn);
>>  	if (!pdn)
>> -		return PCIBIOS_DEVICE_NOT_FOUND;
>> +		return pnv_pci_cfg_write_raw(phb->opal_id, bus->number, devfn,
>> +					     where, size, val);
>>  
>>  	if (!pnv_pci_cfg_check(pdn))
>>  		return PCIBIOS_DEVICE_NOT_FOUND;
>> -- 
>> 2.17.1
>>

Best regards,
Serge


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v2 2/5] powerpc/pci: Create pci_dn on demand
From: Sergey Miroshnichenko @ 2018-09-10 15:51 UTC (permalink / raw)
  To: Sam Bobroff; +Cc: linuxppc-dev, linux
In-Reply-To: <20180910044735.GB14370@tungsten.ozlabs.ibm.com>


[-- Attachment #1.1: Type: text/plain, Size: 6246 bytes --]

Hello Sam,

On 9/10/18 7:47 AM, Sam Bobroff wrote:
> Hi Sergey,
> 
> On Thu, Sep 06, 2018 at 02:57:49PM +0300, Sergey Miroshnichenko wrote:
>> The pci_dn structures can be created not only from DT, but also
>> directly from newly discovered PCIe devices, so allocate them
>> dynamically.
>>
>> Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
>> ---
>>  arch/powerpc/kernel/pci_dn.c | 76 ++++++++++++++++++++++++++++--------
>>  1 file changed, 59 insertions(+), 17 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
>> index ab147a1909c8..48ec16407835 100644
>> --- a/arch/powerpc/kernel/pci_dn.c
>> +++ b/arch/powerpc/kernel/pci_dn.c
>> @@ -33,6 +33,8 @@
>>  #include <asm/firmware.h>
>>  #include <asm/eeh.h>
>>  
>> +static struct pci_dn *create_pdn(struct pci_dev *pdev, struct pci_dn *parent);
>> +
>>  /*
>>   * The function is used to find the firmware data of one
>>   * specific PCI device, which is attached to the indicated
>> @@ -58,6 +60,9 @@ static struct pci_dn *pci_bus_to_pdn(struct pci_bus *bus)
>>  		pbus = pbus->parent;
>>  	}
>>  
>> +	if (!pbus->self && !pci_is_root_bus(pbus))
>> +		return NULL;
>> +
>>  	/*
>>  	 * Except virtual bus, all PCI buses should
>>  	 * have device nodes.
>> @@ -65,13 +70,15 @@ static struct pci_dn *pci_bus_to_pdn(struct pci_bus *bus)
>>  	dn = pci_bus_to_OF_node(pbus);
>>  	pdn = dn ? PCI_DN(dn) : NULL;
>>  
>> +	if (!pdn && pbus->self)
>> +		pdn = pbus->self->dev.archdata.pci_data;
>> +
>>  	return pdn;
>>  }
>>  
>>  struct pci_dn *pci_get_pdn_by_devfn(struct pci_bus *bus,
>>  				    int devfn)
>>  {
>> -	struct device_node *dn = NULL;
>>  	struct pci_dn *parent, *pdn;
>>  	struct pci_dev *pdev = NULL;
>>  
>> @@ -80,17 +87,10 @@ struct pci_dn *pci_get_pdn_by_devfn(struct pci_bus *bus,
>>  		if (pdev->devfn == devfn) {
>>  			if (pdev->dev.archdata.pci_data)
>>  				return pdev->dev.archdata.pci_data;
>> -
>> -			dn = pci_device_to_OF_node(pdev);
>>  			break;
>>  		}
>>  	}
>>  
>> -	/* Fast path: fetch from device node */
>> -	pdn = dn ? PCI_DN(dn) : NULL;
>> -	if (pdn)
>> -		return pdn;
>> -
> 
> Why is it necessary to remove the above fast-path?
> 

It is not, actually - this had leaked from early stages of debugging,
when I've found that after hotplug+rescan or hotplug+kexec the kernel
took pdns from device nodes that do not represent the actual PCIe
topology anymore. But after patches 3 and 4 the PCI_DN() is NULL for
every PCIe device except root on PowerNV, so this block is safe.

It will remain in version 3 of this patchset.

>>  	/* Slow path: fetch from firmware data hierarchy */
>>  	parent = pci_bus_to_pdn(bus);
>>  	if (!parent)
>> @@ -128,16 +128,9 @@ struct pci_dn *pci_get_pdn(struct pci_dev *pdev)
>>  	if (!parent)
>>  		return NULL;
>>  
>> -	list_for_each_entry(pdn, &parent->child_list, list) {
>> -		if (pdn->busno == pdev->bus->number &&
>> -		    pdn->devfn == pdev->devfn)
>> -			return pdn;
>> -	}
> 
> Could you explain why the above block was removed? Is it now impossible
> for it to find a pdn?
> 

I see now that this block was also removed too early: on PowerNV with
this patchset it is impossible to have pdn being present in
parent->child_list but not in pdev->dev.archdata.pci_data; but this may
be not the case for pSeries.

>> -
>> -	return NULL;
>> +	return create_pdn(pdev, parent);
>>  }
>>  
>> -#ifdef CONFIG_PCI_IOV
>>  static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
>>  					   int vf_index,
>>  					   int busno, int devfn)
>> @@ -156,7 +149,9 @@ static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
>>  	pdn->parent = parent;
>>  	pdn->busno = busno;
>>  	pdn->devfn = devfn;
>> +	#ifdef CONFIG_PCI_IOV
>>  	pdn->vf_index = vf_index;
>> +	#endif /* CONFIG_PCI_IOV */
>>  	pdn->pe_number = IODA_INVALID_PE;
>>  	INIT_LIST_HEAD(&pdn->child_list);
>>  	INIT_LIST_HEAD(&pdn->list);
> 
> I can see that this change allows you to re-use this to set up a pdn in
> create_pdn(). Perhaps you should refactor pci_add_device_node_info() to
> use it as well, now that it's possible?
> 

Sure, will do.

>> @@ -164,7 +159,54 @@ static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
>>  
>>  	return pdn;
>>  }
>> -#endif
>> +
>> +static struct pci_dn *create_pdn(struct pci_dev *pdev, struct pci_dn *parent)
>> +{
>> +	struct pci_dn *pdn = NULL;
>> +
>> +	pdn = add_one_dev_pci_data(parent, 0, pdev->bus->number, pdev->devfn);
>> +	dev_info(&pdev->dev, "Create a new pdn for devfn %2x\n", pdev->devfn / 8);
>> +
>> +	if (pdn) {
>> +		#ifdef CONFIG_EEH
>> +		struct eeh_dev *edev;
>> +		#endif /* CONFIG_EEH */
>> +		u32 class_code;
>> +		u16 device_id;
>> +		u16 vendor_id;
>> +
>> +		#ifdef CONFIG_EEH
>> +		edev = eeh_dev_init(pdn);
>> +		if (!edev) {
>> +			kfree(pdn);
>> +			dev_err(&pdev->dev, "%s: Failed to allocate edev\n", __func__);
>> +			return NULL;
>> +		}
>> +		#endif /* CONFIG_EEH */
>> +
>> +		pdn->busno = pdev->bus->busn_res.start;
> 
> It seems strange that pdn->busno is set by the call to
> add_one_dev_pci_data() above (to pdev->bus->number) and then overwritten
> here with a different value. Should add_one_dev_pci_data() use
> pdev->bus->busn_res.start and this line be removed?
> 

Thanks for capturing that! I'll prepare v3 with the fixes.

>> +
>> +		pci_bus_read_config_word(pdev->bus, pdev->devfn,
>> +					 PCI_VENDOR_ID, &vendor_id);
>> +		pdn->vendor_id = vendor_id;
>> +
>> +		pci_bus_read_config_word(pdev->bus, pdev->devfn,
>> +					 PCI_DEVICE_ID, &device_id);
>> +		pdn->device_id = device_id;
>> +
>> +		pci_bus_read_config_dword(pdev->bus, pdev->devfn,
>> +					  PCI_CLASS_REVISION, &class_code);
>> +		class_code >>= 8;
>> +		pdn->class_code = class_code;
>> +
>> +		pdn->pci_ext_config_space = 0;
>> +		pdev->dev.archdata.pci_data = pdn;
>> +	} else {
>> +		dev_err(&pdev->dev, "%s: Failed to allocate pdn\n", __func__);
>> +	}
>> +
>> +	return pdn;
>> +}
> 
> 
>>  
>>  struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
>>  {
>> -- 
>> 2.17.1
>>

Best regards,
Serge


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v2 5/5] PCI/powerpc/eeh: Add pcibios hooks for preparing to rescan
From: Sergey Miroshnichenko @ 2018-09-10 16:00 UTC (permalink / raw)
  To: Sam Bobroff; +Cc: linuxppc-dev, linux
In-Reply-To: <20180910050328.GC14370@tungsten.ozlabs.ibm.com>


[-- Attachment #1.1: Type: text/plain, Size: 5892 bytes --]

Hello Sam,

On 9/10/18 8:03 AM, Sam Bobroff wrote:
> Hi Sergey,
> 
> On Thu, Sep 06, 2018 at 02:57:52PM +0300, Sergey Miroshnichenko wrote:
>> Reading an empty slot returns all ones, which triggers a false
>> EEH error event on PowerNV.
>>
>> New callbacks pcibios_rescan_prepare/done are introduced to
>> pause/resume the EEH during rescan.
> 
> If I understand it correctly, this temporarily disables EEH for config space
> accesses on the whole PHB while the rescan runs. Is it possible that a
> real EEH event could be missed if it occurred during the rescan?
> 
> Even if it's not possible, I think it would be good to mention that in a
> comment.

Yes, missing a real EEH event is possible, unfortunately, and it is
indeed worth mentioning.

To reduce this probability the next patchset I'll post in a few days
among other things puts all the affected device drivers to pause during
rescan, mainly because of moving BARs and bridge windows, but it will
also help here a bit.

> 
>> Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
>> ---
>>  arch/powerpc/include/asm/eeh.h               |  2 ++
>>  arch/powerpc/kernel/eeh.c                    | 12 +++++++++++
>>  arch/powerpc/platforms/powernv/eeh-powernv.c | 22 ++++++++++++++++++++
>>  drivers/pci/probe.c                          | 14 +++++++++++++
>>  include/linux/pci.h                          |  2 ++
>>  5 files changed, 52 insertions(+)
>>
>> diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
>> index 219637ea69a1..926c3e31df99 100644
>> --- a/arch/powerpc/include/asm/eeh.h
>> +++ b/arch/powerpc/include/asm/eeh.h
>> @@ -219,6 +219,8 @@ struct eeh_ops {
>>  	int (*next_error)(struct eeh_pe **pe);
>>  	int (*restore_config)(struct pci_dn *pdn);
>>  	int (*notify_resume)(struct pci_dn *pdn);
>> +	int (*pause)(struct pci_bus *bus);
>> +	int (*resume)(struct pci_bus *bus);
> 
> I think these names are a bit too generic, what about naming them
> pause_bus()/resume_bus() or even prepare_rescan()/rescan_done()?
> 

Thanks! I will rename them to rescan_prepare/rescan_done to make friends
with reset_prepare/reset_done from struct pci_error_handlers.

>>  };
>>  
>>  extern int eeh_subsystem_flags;
>> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
>> index 6ebba3e48b01..9fb5012f389d 100644
>> --- a/arch/powerpc/kernel/eeh.c
>> +++ b/arch/powerpc/kernel/eeh.c
>> @@ -1831,3 +1831,15 @@ static int __init eeh_init_proc(void)
>>  	return 0;
>>  }
>>  __initcall(eeh_init_proc);
>> +
>> +void pcibios_rescan_prepare(struct pci_bus *bus)
>> +{
>> +	if (eeh_ops && eeh_ops->pause)
>> +		eeh_ops->pause(bus);
>> +}
>> +
>> +void pcibios_rescan_done(struct pci_bus *bus)
>> +{
>> +	if (eeh_ops && eeh_ops->resume)
>> +		eeh_ops->resume(bus);
>> +}
>> diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
>> index 3c1beae29f2d..9724a58afcd2 100644
>> --- a/arch/powerpc/platforms/powernv/eeh-powernv.c
>> +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
>> @@ -59,6 +59,26 @@ void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
>>  	eeh_sysfs_add_device(pdev);
>>  }
>>  
>> +static int pnv_eeh_pause(struct pci_bus *bus)
>> +{
>> +	struct pci_controller *hose = pci_bus_to_host(bus);
>> +	struct pnv_phb *phb = hose->private_data;
>> +
>> +	phb->flags &= ~PNV_PHB_FLAG_EEH;
>> +	disable_irq(eeh_event_irq);
>> +	return 0;
>> +}
>> +
>> +static int pnv_eeh_resume(struct pci_bus *bus)
>> +{
>> +	struct pci_controller *hose = pci_bus_to_host(bus);
>> +	struct pnv_phb *phb = hose->private_data;
>> +
>> +	enable_irq(eeh_event_irq);
>> +	phb->flags |= PNV_PHB_FLAG_EEH;
>> +	return 0;
>> +}
>> +
>>  static int pnv_eeh_init(void)
>>  {
>>  	struct pci_controller *hose;
>> @@ -1710,6 +1730,8 @@ static struct eeh_ops pnv_eeh_ops = {
>>  	.write_config           = pnv_eeh_write_config,
>>  	.next_error		= pnv_eeh_next_error,
>>  	.restore_config		= pnv_eeh_restore_config,
>> +	.pause			= pnv_eeh_pause,
>> +	.resume			= pnv_eeh_resume,
>>  	.notify_resume		= NULL
>>  };
>>  
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index ac876e32de4b..4a9045364809 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -2801,6 +2801,14 @@ void __weak pcibios_remove_bus(struct pci_bus *bus)
>>  {
>>  }
>>  
>> +void __weak pcibios_rescan_prepare(struct pci_bus *bus)
>> +{
>> +}
>> +
>> +void __weak pcibios_rescan_done(struct pci_bus *bus)
>> +{
>> +}
>> +
>>  struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
>>  		struct pci_ops *ops, void *sysdata, struct list_head *resources)
>>  {
>> @@ -3055,9 +3063,15 @@ unsigned int pci_rescan_bus_bridge_resize(struct pci_dev *bridge)
>>  unsigned int pci_rescan_bus(struct pci_bus *bus)
>>  {
>>  	unsigned int max;
>> +	struct pci_bus *root = bus;
>> +
>> +	while (!pci_is_root_bus(root))
>> +		root = root->parent;
>>  
>> +	pcibios_rescan_prepare(root);
>>  	max = pci_scan_child_bus(bus);
>>  	pci_assign_unassigned_bus_resources(bus);
>> +	pcibios_rescan_done(root);
>>  	pci_bus_add_devices(bus);
>>  
>>  	return max;
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index 340029b2fb38..42930731c5a7 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -1929,6 +1929,8 @@ void pcibios_penalize_isa_irq(int irq, int active);
>>  int pcibios_alloc_irq(struct pci_dev *dev);
>>  void pcibios_free_irq(struct pci_dev *dev);
>>  resource_size_t pcibios_default_alignment(void);
>> +void pcibios_rescan_prepare(struct pci_bus *bus);
>> +void pcibios_rescan_done(struct pci_bus *bus);
>>  
>>  #ifdef CONFIG_HIBERNATE_CALLBACKS
>>  extern struct dev_pm_ops pcibios_pm_ops;
>> -- 
>> 2.17.1
>>

Best regards,
Serge


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v2 0/3] tty: hvc: latency break regression fixes
From: Greg Kroah-Hartman @ 2018-09-10 16:04 UTC (permalink / raw)
  To: Nicholas Piggin
  Cc: Jiri Slaby, Michael Ellerman, Matteo Croce, Jason Gunthorpe,
	Leon Romanovsky, linuxppc-dev, op
In-Reply-To: <20180909053916.14139-1-npiggin@gmail.com>

On Sun, Sep 09, 2018 at 03:39:13PM +1000, Nicholas Piggin wrote:
> Re-sending this one with the used-uinitialized warning in patch
> 3 fixed.
> 
> Greg these patches are needed to fix regressions in this merge
> window, please consider them for your tty tree.

All now queued up, thanks.

greg k-h

^ permalink raw reply

* Re: [PATCH 2/5] include: add setbits32/clrbits32/clrsetbits32/setbits64/clrbits64/clrsetbits64 in linux/setbits.h
From: LABBE Corentin @ 2018-09-10 18:49 UTC (permalink / raw)
  To: Scott Wood
  Cc: Gilles.Muller, Julia.Lawall, agust, alexandre.torgue, alistair,
	benh, carlo, davem, galak, joabreu, khilman, maxime.ripard,
	michal.lkml, mpe, mporter, nicolas.palix, paulus, peppe.cavallaro,
	tj, vitb, wens, cocci, linux-amlogic, linux-arm-kernel, linux-ide,
	linux-kernel, linuxppc-dev, netdev, linux-sunxi
In-Reply-To: <8bfa1740800ca494028350addd7c874a8b4804bb.camel@buserror.net>

On Fri, Sep 07, 2018 at 03:00:40PM -0500, Scott Wood wrote:
> On Fri, 2018-09-07 at 19:41 +0000, Corentin Labbe wrote:
> > This patch adds setbits32/clrbits32/clrsetbits32 and
> > setbits64/clrbits64/clrsetbits64 in linux/setbits.h header.
> > 
> > Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> > ---
> >  include/linux/setbits.h | 55
> > +++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 55 insertions(+)
> >  create mode 100644 include/linux/setbits.h
> > 
> > diff --git a/include/linux/setbits.h b/include/linux/setbits.h
> > new file mode 100644
> > index 000000000000..3e1e273551bb
> > --- /dev/null
> > +++ b/include/linux/setbits.h
> > @@ -0,0 +1,55 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef __LINUX_SETBITS_H
> > +#define __LINUX_SETBITS_H
> > +
> > +#include <linux/io.h>
> > +
> > +#define __setbits(readfunction, writefunction, addr, set) \
> > +	writefunction((readfunction(addr) | (set)), addr)
> > +#define __clrbits(readfunction, writefunction, addr, mask) \
> > +	writefunction((readfunction(addr) & ~(mask)), addr)
> > +#define __clrsetbits(readfunction, writefunction, addr, mask, set) \
> > +	writefunction(((readfunction(addr) & ~(mask)) | (set)), addr)
> > +#define __setclrbits(readfunction, writefunction, addr, mask, set) \
> > +	writefunction(((readfunction(addr) | (seti)) & ~(mask)), addr)
> > +
> > +#define setbits32(addr, set) __setbits(readl, writel, addr, set)
> > +#define setbits32_relaxed(addr, set) __setbits(readl_relaxed,
> > writel_relaxed, \
> > +					       addr, set)
> > +
> > +#define clrbits32(addr, mask) __clrbits(readl, writel, addr, mask)
> > +#define clrbits32_relaxed(addr, mask) __clrbits(readl_relaxed,
> > writel_relaxed, \
> > +						addr, mask)
> 
> So now setbits32/clrbits32 is implicitly little-endian?  Introducing new
> implicit-endian accessors is probably a bad thing in general, but doing it
> with a name that until this patchset was implicitly big-endian (at least on
> powerpc) seems worse.  Why not setbits32_le()?
> 

I believed that writel/readl was endian agnostic, but It seems that I was wrong.
So I will use _le32.

> 
> > +
> > +#define clrsetbits32(addr, mask, set) __clrsetbits(readl, writel, addr,
> > mask, set)
> > +#define clrsetbits32_relaxed(addr, mask, set) __clrsetbits(readl_relaxed, \
> > +							   writel_relaxed,
> > \
> > +							   addr, mask, set)
> > +
> > +#define setclrbits32(addr, mask, set) __setclrbits(readl, writel, addr,
> > mask, set)
> > +#define setclrbits32_relaxed(addr, mask, set) __setclrbits(readl_relaxed, \
> > +							   writel_relaxed,
> > \
> > +							   addr, mask, set)
> 
> What's the use case for setclrbits?  I don't see it used anywhere in this
> patchset (not even in the coccinelle patterns), it doesn't seem like it would
> be a common pattern, and it could easily get confused with clrsetbits.
> 

It is absent from the coccinelle script due to copy/paste error.
And absent from patchset since it is only two possible example that I can test.

If you run the next fixed coccinelle script, you will find some setclrbits.
Since I fear that mask and set could have some common bits sometimes, I prefer to keep separate clrsetbits and setclrbits.

Regards

^ permalink raw reply

* Re: [PATCH 1/5] powerpc: rename setbits32/clrbits32 to setbits32_be/clrbits32_be
From: LABBE Corentin @ 2018-09-10 18:50 UTC (permalink / raw)
  To: Christophe LEROY
  Cc: Gilles.Muller, Julia.Lawall, agust, alexandre.torgue, alistair,
	benh, carlo, davem, galak, joabreu, khilman, maxime.ripard,
	michal.lkml, mpe, mporter, nicolas.palix, oss, paulus,
	peppe.cavallaro, tj, vitb, wens, netdev, linux-kernel, linux-ide,
	linux-sunxi, linux-amlogic, linuxppc-dev, cocci, linux-arm-kernel
In-Reply-To: <d16bf608-5b97-4baf-2ac1-a6d98656d716@c-s.fr>

On Mon, Sep 10, 2018 at 07:16:56AM +0200, Christophe LEROY wrote:
> 
> 
> Le 07/09/2018 à 21:41, Corentin Labbe a écrit :
> > Since setbits32/clrbits32 work on be32, it's better to remove ambiguity on
> > the used data type.
> 
> Wouldn't it be better to call them setbits_be32() / clrbits_be32() to 
> have something looking similar to in_be32() / ou_be32() ?
> 

I agree, I will update the patch.

Thanks

^ permalink raw reply

* Re: [PATCH 2/5] include: add setbits32/clrbits32/clrsetbits32/setbits64/clrbits64/clrsetbits64 in linux/setbits.h
From: LABBE Corentin @ 2018-09-10 18:53 UTC (permalink / raw)
  To: Christophe LEROY
  Cc: Gilles.Muller, Julia.Lawall, agust, alexandre.torgue, alistair,
	benh, carlo, davem, galak, joabreu, khilman, maxime.ripard,
	michal.lkml, mpe, mporter, nicolas.palix, oss, paulus,
	peppe.cavallaro, tj, vitb, wens, netdev, linux-kernel, linux-ide,
	linux-sunxi, linux-amlogic, linuxppc-dev, cocci, linux-arm-kernel
In-Reply-To: <a1795266-5679-289d-5cce-1111babf3180@c-s.fr>

On Mon, Sep 10, 2018 at 07:22:04AM +0200, Christophe LEROY wrote:
> 
> 
> Le 07/09/2018 à 21:41, Corentin Labbe a écrit :
> > This patch adds setbits32/clrbits32/clrsetbits32 and
> > setbits64/clrbits64/clrsetbits64 in linux/setbits.h header.
> 
> So you changed the name of setbits32() ... to setbits32_be() and now you 
> are adding new functions called setbits32() ... which do something 
> different ?
> 
> What will happen if any file has been forgotten during the conversion, 
> or if anybody has outoftree drivers and missed this change ?
> They will silently successfully compile without any error or warning, 
> and the result will be crap buggy.
> 
> And why would it be more legitim to have setbits32() be implicitely LE 
> instead of implicitely BE ?
> 
> I really think those new functions should be called something like 
> setbits_le32() ...
> 

I believed that writel/readl was endian agnostic so it explain my mistake.

I will use xxxbits_le32 as you requests.

Thanks
Regards

^ permalink raw reply

* Re: How to handle PTE tables with non contiguous entries ?
From: Dan Malek @ 2018-09-10 20:05 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: akpm, linux-mm, aneesh.kumar, Nicholas Piggin, Michael Ellerman,
	linuxppc-dev, LKML
In-Reply-To: <ddc3bb56-4da0-c093-256f-185d4a612b5c@c-s.fr>


Hello Cristophe.

> On Sep 10, 2018, at 7:34 AM, Christophe Leroy =
<christophe.leroy@c-s.fr> wrote:
>=20
> On the powerpc8xx, handling 16k size pages requires to have page =
tables with 4 identical entries.

Do you think a 16k page is useful?  Back in the day, the goal was to =
keep the fault handling and management overhead as simple and generic as =
possible, as you know this affects the system performance.  I understand =
there would be fewer page faults and more efficient use of the MMU =
resources with 16k, but if this comes at an overhead cost, is it really =
worth it?

In addition to the normal 4k mapping, I had thought about using 512k =
mapping, which could be easily detected at level 2 (PMD), with a single =
entry loaded into the MMU.  We would need an aux header or something =
from the executable/library to assist with knowing when this could be =
done.  I never got around to it. :)

The 8xx platforms tended to have smaller memory resources, so the 4k =
granularity was also useful in making better use of the available space.

> Would someone have an idea of an elegent way to handle that ?

My suggestion would be to not change the PTE table, but have the fault =
handler detect a 16k page and load any one of the four entries based =
upon miss offset.  Kinda use the same 4k miss hander, but with 16k =
knowledge.  You wouldn=E2=80=99t save any PTE table space, but the MMU =
efficiency may be worth it.  As I recall, the hardware may ignore/mask =
any LS bits, and there is PMD level information to utilize as well.

It=E2=80=99s been a long time since I=E2=80=99ve investigated how things =
have evolved, glad it=E2=80=99s still in use, and I hope you at least =
have some fun with the development :)

Thanks.

	=E2=80=94 Dan

^ permalink raw reply

* Re: How to handle PTE tables with non contiguous entries ?
From: Nicholas Piggin @ 2018-09-10 21:06 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: akpm, linux-mm, aneesh.kumar, Michael Ellerman, linuxppc-dev,
	LKML
In-Reply-To: <ddc3bb56-4da0-c093-256f-185d4a612b5c@c-s.fr>

On Mon, 10 Sep 2018 14:34:37 +0000
Christophe Leroy <christophe.leroy@c-s.fr> wrote:

> Hi,
> 
> I'm having a hard time figuring out the best way to handle the following 
> situation:
> 
> On the powerpc8xx, handling 16k size pages requires to have page tables 
> with 4 identical entries.
> 
> Initially I was thinking about handling this by simply modifying 
> pte_index() which changing pte_t type in order to have one entry every 
> 16 bytes, then replicate the PTE value at *ptep, *ptep+1,*ptep+2 and 
> *ptep+3 both in set_pte_at() and pte_update().
> 
> However, this doesn't work because many many places in the mm core part 
> of the kernel use loops on ptep with single ptep++ increment.
> 
> Therefore did it with the following hack:
> 
>   /* PTE level */
> +#if defined(CONFIG_PPC_8xx) && defined(CONFIG_PPC_16K_PAGES)
> +typedef struct { pte_basic_t pte, pte1, pte2, pte3; } pte_t;
> +#else
>   typedef struct { pte_basic_t pte; } pte_t;
> +#endif
> 
> @@ -181,7 +192,13 @@ static inline unsigned long pte_update(pte_t *p,
>          : "cc" );
>   #else /* PTE_ATOMIC_UPDATES */
>          unsigned long old = pte_val(*p);
> -       *p = __pte((old & ~clr) | set);
> +       unsigned long new = (old & ~clr) | set;
> +
> +#if defined(CONFIG_PPC_8xx) && defined(CONFIG_PPC_16K_PAGES)
> +       p->pte = p->pte1 = p->pte2 = p->pte3 = new;
> +#else
> +       *p = __pte(new);
> +#endif
>   #endif /* !PTE_ATOMIC_UPDATES */
> 
>   #ifdef CONFIG_44x
> 
> 
> @@ -161,7 +161,11 @@ static inline void __set_pte_at(struct mm_struct 
> *mm, unsigned long addr,
>          /* Anything else just stores the PTE normally. That covers all 
> 64-bit
>           * cases, and 32-bit non-hash with 32-bit PTEs.
>           */
> +#if defined(CONFIG_PPC_8xx) && defined(CONFIG_PPC_16K_PAGES)
> +       ptep->pte = ptep->pte1 = ptep->pte2 = ptep->pte3 = pte_val(pte);
> +#else
>          *ptep = pte;
> +#endif
> 
> 
> 
> But I'm not too happy with it as it means pte_t is not a single type 
> anymore so passing it from one function to the other is quite heavy.
> 
> 
> Would someone have an idea of an elegent way to handle that ?

I can't think of anything better. Do we pass pte by value to a lot of
non inlined functions? Possible to inline the important ones?

Other option, try to get an iterator like pte = pte_next(pte) into core
code.

Thanks,
Nick

^ 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