LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: DTB build failure due to preproccessing
From: Stephen Warren @ 2013-05-31 16:37 UTC (permalink / raw)
  To: Ian Campbell
  Cc: Michal Marek, JonLoeliger, Stephen Warren, linux-kbuild,
	linux-kernel, Rob Herring, linuxppc-dev
In-Reply-To: <1369996170.5199.68.camel@zakaz.uk.xensource.com>

On 05/31/2013 04:29 AM, Ian Campbell wrote:
> This affects arch/powerpc/boot/dts/virtex440-ml510.dts but I think it is
> actually a more general issue:
> 
>         $ make ARCH=powerpc CROSS_COMPILE=powerpc-linux- virtex440-ml510.dtb 
>           CC      scripts/mod/devicetable-offsets.s
>           GEN     scripts/mod/devicetable-offsets.h
>           HOSTCC  scripts/mod/file2alias.o
>           HOSTLD  scripts/mod/modpost
>           DTC     arch/powerpc/boot/virtex440-ml510.dtb
>         Error: arch/powerpc/boot/dts/virtex440-ml510.dts:374.6-7 syntax error
>         FATAL ERROR: Unable to parse input tree
>         make[1]: *** [arch/powerpc/boot/virtex440-ml510.dtb] Error 1
>         make: *** [virtex440-ml510.dtb] Error 2
...
>            interrupt-map = <
>         # 375 "arch/powerpc/boot/dts/virtex440-ml510.dts"
>             0x3000 0 0 1 &xps_intc_0 3 2
>             0x3000 0 0 2 &xps_intc_0 2 2
>             0x3000 0 0 3 &xps_intc_0 5 2
>             0x3000 0 0 4 &xps_intc_0 4 2

I /think/ what's happening here is that dtc's rule for #line parsing
allows the formats:

# LINE FILENAME
or:
#LINE FILENAME FLAGS

where FLAGS is an integer

The lexer rule that optionally consumes flags requires some WS
(white-space) between the filename and flags. It looks like this
whitespace can actually cross a line boundary, so it ends up consuming
the first "0" of the hex constant on the next line, which then leaves
"x3000" to be parsed as cell data, which is a syntax error.

You can hack around this for testing by doing one of a few things:

a) Add an extra integer on to the end of the problematic #line directives.

b) Remove the leading "0x" from the constants following the #line
directives. I think this will still mean dtc eats the 3000 as part of
the #line directive, but hides the syntax error.

The solution here is to make dtc's #line matching regex:

<*>^"#"(line)?{WS}+[0-9]+{WS}+{STRING}({WS}+[0-9]+)? {

... use someting other than {WS} in the final instance; some kind of
{WS} that won't match/cross line boundaries. I'll see if I can cook up a
patch for this.

^ permalink raw reply

* [PATCH] dtc: Ensure #line directives don't consume data from the next line
From: Stephen Warren @ 2013-05-31 17:14 UTC (permalink / raw)
  To: David Gibson, jdl
  Cc: mmarek, Stephen Warren, linux-kbuild, devicetree-discuss,
	linux-kernel, rob.herring, linuxppc-dev

From: Stephen Warren <swarren@nvidia.com>

Previously, the #line parsing regex ended with ({WS}+[0-9]+)?. The {WS}
could match line-break characters. If the #line directive did not contain
the optional flags field at the end, this could cause any integer data on
the next line to be consumed as part of the #line directive parsing. This
could cause syntax errors (i.e. #line parsing consuming the leading 0
from a hex constant 0x1234, leaving x1234 to be parsed as cell data,
which is a syntax error), or invalid compilation results (i.e. simply
consuming integer 1234 as part of the #line processing, thus removing it
from the cell data).

Fix this by replacing {WS} with [ \t] so that it can't match line-breaks.

Reported-by: Ian Campbell <Ian.Campbell@citrix.com>
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
This is a patch for dtc upstream, in response to thread "DTB build
failure due to preproccessing". If/when it's accepted into dtc, I'll
follow up with a kernel patch that makes the same fix.

 dtc-lexer.l               |    2 +-
 tests/line_directives.dts |   10 ++++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/dtc-lexer.l b/dtc-lexer.l
index 254d5af..a3a4c69 100644
--- a/dtc-lexer.l
+++ b/dtc-lexer.l
@@ -71,7 +71,7 @@ static int pop_input_file(void);
 			push_input_file(name);
 		}
 
-<*>^"#"(line)?{WS}+[0-9]+{WS}+{STRING}({WS}+[0-9]+)? {
+<*>^"#"(line)?{WS}+[0-9]+{WS}+{STRING}([ \t]+[0-9]+)? {
 			char *line, *tmp, *fn;
 			/* skip text before line # */
 			line = yytext;
diff --git a/tests/line_directives.dts b/tests/line_directives.dts
index e9d0800..046ef37 100644
--- a/tests/line_directives.dts
+++ b/tests/line_directives.dts
@@ -8,4 +8,14 @@
 # 6 "bar.dts"
 
 / {
+/*
+ * Make sure optional flags don't consume integer data on next line. The issue
+ * was that the {WS} in the trailing ({WS}+[0-9]+)? could cross the * line-
+ * break, and consume the leading "0" of the hex constant, leaving "x12345678"
+ * to be parsed as a number, which is invalid syntax.
+ */
+	prop1 = <
+# 10 "qux.dts"
+		0x12345678
+	>;
 };
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH] dtc: Ensure #line directives don't consume data from the next line
From: Andreas Schwab @ 2013-05-31 17:38 UTC (permalink / raw)
  To: Stephen Warren
  Cc: mmarek, jdl, Stephen Warren, linux-kbuild, devicetree-discuss,
	linux-kernel, rob.herring, linuxppc-dev, David Gibson
In-Reply-To: <1370020450-18136-1-git-send-email-swarren@wwwdotorg.org>

Stephen Warren <swarren@wwwdotorg.org> writes:

> Fix this by replacing {WS} with [ \t] so that it can't match line-breaks.

I think the other uses of {WS} shouldn't span lines either.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply

* Re: [PATCH] dtc: Ensure #line directives don't consume data from the next line
From: Stephen Warren @ 2013-05-31 17:42 UTC (permalink / raw)
  To: Andreas Schwab
  Cc: mmarek, jdl, Stephen Warren, linux-kbuild, devicetree-discuss,
	linux-kernel, rob.herring, linuxppc-dev, David Gibson
In-Reply-To: <87sj13dp72.fsf@hase.home>

On 05/31/2013 11:38 AM, Andreas Schwab wrote:
> Stephen Warren <swarren@wwwdotorg.org> writes:
> 
>> Fix this by replacing {WS} with [ \t] so that it can't match line-breaks.
> 
> I think the other uses of {WS} shouldn't span lines either.

That is true, but only the optional occurrence /should/ matter. Any
changes to the other occurrences would only affect malformed #line
directives, whereas changing this one occurrence would also affect
well-formed #line directives, due to the optional nature of the trailing
flags field.

Still, it may be reasonable just to change all the occurrences anyway.
Does anyone have a strong opinion either way?

^ permalink raw reply

* [PATCH V2] dtc: ensure #line directives don't consume data from the next line
From: Stephen Warren @ 2013-05-31 18:33 UTC (permalink / raw)
  To: David Gibson, jdl
  Cc: mmarek, Stephen Warren, linux-kbuild, devicetree-discuss,
	linux-kernel, rob.herring, linuxppc-dev

From: Stephen Warren <swarren@nvidia.com>

Previously, the #line parsing regex ended with ({WS}+[0-9]+)?. The {WS}
could match line-break characters. If the #line directive did not contain
the optional flags field at the end, this could cause any integer data on
the next line to be consumed as part of the #line directive parsing. This
could cause syntax errors (i.e. #line parsing consuming the leading 0
from a hex literal 0x1234, leaving x1234 to be parsed as cell data,
which is a syntax error), or invalid compilation results (i.e. simply
consuming literal 1234 as part of the #line processing, thus removing it
from the cell data).

Fix this by replacing {WS} with [ \t] so that it can't match line-breaks.

Convert all instances of {WS}, even though the other instances should be
irrelevant for any well-formed #line directive. This is done for
consistency and ultimate safety.

Reported-by: Ian Campbell <Ian.Campbell@citrix.com>
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
v2: Convert all instances of {WS} in the regex.

This is a patch for dtc upstream, in response to thread "DTB build
failure due to preproccessing". If/when it's accepted into dtc, I'll
follow up with a kernel patch that makes the same fix.
---
 dtc-lexer.l               |    2 +-
 tests/line_directives.dts |   10 ++++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/dtc-lexer.l b/dtc-lexer.l
index 254d5af..3b41bfc 100644
--- a/dtc-lexer.l
+++ b/dtc-lexer.l
@@ -71,7 +71,7 @@ static int pop_input_file(void);
 			push_input_file(name);
 		}
 
-<*>^"#"(line)?{WS}+[0-9]+{WS}+{STRING}({WS}+[0-9]+)? {
+<*>^"#"(line)?[ \t]+[0-9]+[ \t]+{STRING}([ \t]+[0-9]+)? {
 			char *line, *tmp, *fn;
 			/* skip text before line # */
 			line = yytext;
diff --git a/tests/line_directives.dts b/tests/line_directives.dts
index e9d0800..046ef37 100644
--- a/tests/line_directives.dts
+++ b/tests/line_directives.dts
@@ -8,4 +8,14 @@
 # 6 "bar.dts"
 
 / {
+/*
+ * Make sure optional flags don't consume integer data on next line. The issue
+ * was that the {WS} in the trailing ({WS}+[0-9]+)? could cross the * line-
+ * break, and consume the leading "0" of the hex constant, leaving "x12345678"
+ * to be parsed as a number, which is invalid syntax.
+ */
+	prop1 = <
+# 10 "qux.dts"
+		0x12345678
+	>;
 };
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH] powerpc/sysfs: disable hotplug for the boot cpu
From: Benjamin Herrenschmidt @ 2013-05-31 21:49 UTC (permalink / raw)
  To: Zhao Chenhui; +Cc: scottwood, linuxppc-dev, linux-kernel
In-Reply-To: <1369727984-21505-2-git-send-email-chenhui.zhao@freescale.com>

On Tue, 2013-05-28 at 15:59 +0800, Zhao Chenhui wrote:
> Some features depend on the boot cpu, for instance, hibernate/suspend.
> So disable hotplug for the boot cpu.

Don't we have code to "move" the boot CPU around when that happens ?

Ben.

> Signed-off-by: Zhao Chenhui <chenhui.zhao@freescale.com>
> ---
>  arch/powerpc/kernel/sysfs.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
> index e68a845..294b1c4e 100644
> --- a/arch/powerpc/kernel/sysfs.c
> +++ b/arch/powerpc/kernel/sysfs.c
> @@ -655,8 +655,10 @@ static int __init topology_init(void)
>  		 * CPU.  For instance, the boot cpu might never be valid
>  		 * for hotplugging.
>  		 */
> -		if (ppc_md.cpu_die)
> +		if (ppc_md.cpu_die && cpu != boot_cpuid)
>  			c->hotpluggable = 1;
> +		else
> +			c->hotpluggable = 0;
>  
>  		if (cpu_online(cpu) || c->hotpluggable) {
>  			register_cpu(c, cpu);

^ permalink raw reply

* Re: 3.10-rc ppc64 corrupts usermem when swapping
From: Benjamin Herrenschmidt @ 2013-05-31 22:23 UTC (permalink / raw)
  To: Aneesh Kumar K.V
  Cc: Paul Mackerras, Anton Blanchard, Hugh Dickins, linuxppc-dev,
	David Gibson
In-Reply-To: <87ppw7mrx7.fsf@linux.vnet.ibm.com>

On Fri, 2013-05-31 at 14:45 +0530, Aneesh Kumar K.V wrote:

> > The patch you are running on is what I'll send to Linus for 3.10 (+/-
> > cosmetics). Aneesh second patch is a much larger rework which will be
> > needed for THP but that will wait for 3.11. I'm happy for you to test it
> > but I first want to make sure it's solid with the 3.10 fix :-)

BTW. One concern I still have is that Hugh identified the bad commit
to be:

7e74c3921ad9610c0b49f28b8fc69f7480505841
"powerpc: Fix hpte_decode to use the correct decoding for page sizes".

However, you introduce the return on HPTE not found earlier, in

b1022fbd293564de91596b8775340cf41ad5214c
"powerpc: Decode the pte-lp-encoding bits correctly."

So while I'm still happy with the current band-aid for 3.10 and am
about to send it to Linus, the above *does* seem to indicate that
there is also something wrong with the "Fix hpte_decode..." commit,
which might not actually get the page size right...

Can you investigate ?

Cheers,
Ben.

^ permalink raw reply

* [git pull] Please pull powerpc.git merge branch
From: Benjamin Herrenschmidt @ 2013-05-31 23:22 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linuxppc-dev, Linux Kernel list

Hi Linus !

Here are a few more fixes for powerpc 3.10. It's a bit more than I would
have liked this late in the game but I suppose that's what happens with
a brand new chip generation coming out.

A few regression fixes, some last minute fixes for new P8 features such
as transactional memory,...

There's also one powerpc KVM patch that I requested that adds two
missing functions to our in-kernel interrupt controller support which
is itself a new 3.10 feature. These are defined by the base hypervisor
specification. We didn't implement them originally because Linux doesn't
use them but they are simple and I'm not comfortable having a
half-implemented interface in 3.10 and having to deal with versionning
etc... later when something starts needing those calls. They cannot be
emulated in qemu when using in-kernel interrupt controller (not enough
shared state).

Cheers,
Ben.

The following changes since commit 58f8bbd2e39c3732c55698494338ee19a92c53a0:

  Merge branch 'drm-fixes' of git://people.freedesktop.org/~airlied/linux (2013-05-28 10:11:34 -0700)

are available in the git repository at:


  git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc.git merge

for you to fetch changes up to 58a032c3b106adcd2b83b7e631de3b79f238cdd2:

  powerpc/perf: Add missing SIER support (2013-06-01 08:29:29 +1000)

----------------------------------------------------------------
Aneesh Kumar K.V (1):
      powerpc/mm: Always invalidate tlb on hpte invalidate and update

Kevin Hao (2):
      powerpc/pci: Remove the stale comments of pci_process_bridge_OF_ranges
      powerpc/pci: Remove the unused variables in pci_process_bridge_OF_ranges

Michael Ellerman (2):
      powerpc/perf: Revert to original NO_SIPR logic
      powerpc/perf: Add missing SIER support

Michael Neuling (7):
      powerpc/tm: Make room for hypervisor in abort cause codes
      powerpc/tm: Update cause codes documentation
      powerpc/tm: Abort on emulation and alignment faults
      powerpc/tm: Move TM abort cause codes to uapi
      powerpc/tm: Fix userspace stack corruption on signal delivery for active transactions
      powerpc/pseries: Kill all prefetch streams on context switch
      powerpc/pseries: Improve stream generation comments in copypage/user

Nishanth Aravamudan (1):
      powerpc/cputable: Fix oprofile_cpu_type on power8

Paul Mackerras (1):
      powerpc/kvm/book3s: Add support for H_IPOLL and H_XIRR_X in XICS emulation

Priyanka Jain (1):
      powerpc/32bit:Store temporary result in r0 instead of r8

Srivatsa S. Bhat (1):
      powerpc/pseries: Always enable CONFIG_HOTPLUG_CPU on PSERIES SMP

chenhui zhao (1):
      powerpc/mpic: Fix irq distribution problem when MPIC_SINGLE_DEST_CPU

 Documentation/powerpc/transactional_memory.txt |   27 +++++++++-
 arch/powerpc/include/asm/hvcall.h              |    1 +
 arch/powerpc/include/asm/ppc_asm.h             |   11 ++++
 arch/powerpc/include/asm/processor.h           |   13 ++---
 arch/powerpc/include/asm/reg.h                 |   11 ----
 arch/powerpc/include/asm/signal.h              |    3 ++
 arch/powerpc/include/asm/tm.h                  |    2 +
 arch/powerpc/include/uapi/asm/Kbuild           |    1 +
 arch/powerpc/include/uapi/asm/tm.h             |   18 +++++++
 arch/powerpc/kernel/cputable.c                 |    4 +-
 arch/powerpc/kernel/entry_32.S                 |    2 +-
 arch/powerpc/kernel/entry_64.S                 |    7 +++
 arch/powerpc/kernel/pci-common.c               |   14 +----
 arch/powerpc/kernel/signal.c                   |   40 +++++++++++++-
 arch/powerpc/kernel/signal.h                   |    2 +-
 arch/powerpc/kernel/signal_32.c                |   10 +---
 arch/powerpc/kernel/signal_64.c                |   23 +++-----
 arch/powerpc/kernel/traps.c                    |   29 ++++++++++
 arch/powerpc/kvm/book3s_hv.c                   |    2 +
 arch/powerpc/kvm/book3s_pr_papr.c              |    2 +
 arch/powerpc/kvm/book3s_xics.c                 |   29 ++++++++++
 arch/powerpc/lib/copypage_power7.S             |   19 ++++---
 arch/powerpc/lib/copyuser_power7.S             |   12 +++--
 arch/powerpc/mm/hash_native_64.c               |   30 ++++++++---
 arch/powerpc/perf/core-book3s.c                |   67 +++++++++++-------------
 arch/powerpc/platforms/pseries/Kconfig         |    2 +
 arch/powerpc/sysdev/mpic.c                     |    4 +-
 27 files changed, 261 insertions(+), 124 deletions(-)
 create mode 100644 arch/powerpc/include/uapi/asm/tm.h

^ permalink raw reply

* Re: [PATCH 1/3] powerpc/mpc85xx: remove the unneeded pci init functions for corenet ds board
From: Scott Wood @ 2013-05-31 23:27 UTC (permalink / raw)
  To: Kevin Hao; +Cc: linuxppc
In-Reply-To: <20130531064348.GC16514@pek-khao-d1.corp.ad.wrs.com>

On 05/31/2013 01:43:49 AM, Kevin Hao wrote:
> On Thu, May 30, 2013 at 01:54:59PM -0500, Scott Wood wrote:
> > On 05/30/2013 05:20:34 AM, Kevin Hao wrote:
> > >On Tue, May 28, 2013 at 05:52:09PM -0500, Scott Wood wrote:
> > >> On 05/21/2013 07:04:58 AM, Kevin Hao wrote:
> > >> >It also seems that we don't support ISA on all the current
> > >corenet ds
> > >> >boards. So picking a primary bus seems useless, remove that
> > >function
> > >> >too.
> > >>
> > >> IIRC that was due to some bugs in the PPC PCI code in the =20
> absence of
> > >> any primary bus.
> > >
> > >Do you know more about these bugs?
> >
> > Not off the top of my head -- either search the archives or ask Ben.
> >
> > >>  fsl_pci_assign_primary() will arbitrarily pick one
> > >> to be primary if there's no ISA.  Have the bugs been fixed?
> > >
> > >I know there should be some reason that we put the
> > >fsl_pci_assign_primary()
> > >here. But frankly I am not sure what bugs this workaround try to
> > >fix. For these
> > >corenet boards picking one to be primary has no effect to the
> > >64bit kernel.
> > >And for 32bit kernel, the only effect of this is that isa_io_base
> > >is set to the
> > >io virtual base of the primary bus. But the isa_io_base only make
> > >sense when
> > >we do have a isa bus, so that we can access some well-known io
> > >ports directly
> > >by using outx/inx. But if we don't have isa bus on the board, the
> > >value of
> > >isa_io_base should make no sense at all. So we really don't need
> > >to pick a
> > >fake primary bus. Of course I may miss something, correct me if I
> > >am wrong. :-)
> >
> > outx/inx can also be used for PCI I/O BARs.
>=20
> Yes, I know there is also PIO. But the value of isa_io_base doesn't
> have any effect for this.

See this e-mail for some of the issues I was referring to with =20
isa_io_base being zero:
https://lists.ozlabs.org/pipermail/linuxppc-dev/2012-June/098586.html

Reading it again I'm not so sure that the problem is so much that we =20
need a primary, as that somewhat bad things happen on non-primary =20
buses, such as the possibility of assigning a zero BAR.  Some hardware =20
(including QEMU's PCI emulation) cares about this, though most =20
doesn't.  We only have one PCI bus under QEMU, so when we started =20
picking an arbitrary bus to be primary, the problem went away because =20
there was only one bus and therefore there was no non-primary bus.

-Scott=

^ permalink raw reply

* Re: [git pull] Please pull powerpc.git merge branch
From: Benjamin Herrenschmidt @ 2013-05-31 23:32 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linuxppc-dev, Linux Kernel list
In-Reply-To: <1370042526.3928.162.camel@pasglop>

On Sat, 2013-06-01 at 09:22 +1000, Benjamin Herrenschmidt wrote:
> Hi Linus !
> 
> Here are a few more fixes for powerpc 3.10. It's a bit more than I would
> have liked this late in the game but I suppose that's what happens with
> a brand new chip generation coming out.
>
> A few regression fixes, some last minute fixes for new P8 features such
> as transactional memory,...
> 
> There's also one powerpc KVM patch that I requested that adds two
> missing functions to our in-kernel interrupt controller support which
> is itself a new 3.10 feature. These are defined by the base hypervisor
> specification. We didn't implement them originally because Linux doesn't
> use them but they are simple and I'm not comfortable having a
> half-implemented interface in 3.10 and having to deal with versionning
> etc... later when something starts needing those calls. They cannot be
> emulated in qemu when using in-kernel interrupt controller (not enough
> shared state).

Just added a last minute patch to fix a typo introducing a breakage
in our cputable for Power7+ processors, sorry about that, but the
regression it fixes just hurt me :-)

Sorry about that..

Cheers,
Ben.

The following changes since commit 58f8bbd2e39c3732c55698494338ee19a92c53a0:

  Merge branch 'drm-fixes' of git://people.freedesktop.org/~airlied/linux (2013-05-28 10:11:34 -0700)

are available in the git repository at:


  git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc.git merge

for you to fetch changes up to badec11b645e21acbc2411d7759e3efa559af443:

  powerpc/cputable: Fix typo on P7+ cputable entry (2013-06-01 09:30:03 +1000)

----------------------------------------------------------------
Aneesh Kumar K.V (1):
      powerpc/mm: Always invalidate tlb on hpte invalidate and update

Kevin Hao (2):
      powerpc/pci: Remove the stale comments of pci_process_bridge_OF_ranges
      powerpc/pci: Remove the unused variables in pci_process_bridge_OF_ranges

Michael Ellerman (2):
      powerpc/perf: Revert to original NO_SIPR logic
      powerpc/perf: Add missing SIER support

Michael Neuling (7):
      powerpc/tm: Make room for hypervisor in abort cause codes
      powerpc/tm: Update cause codes documentation
      powerpc/tm: Abort on emulation and alignment faults
      powerpc/tm: Move TM abort cause codes to uapi
      powerpc/tm: Fix userspace stack corruption on signal delivery for active transactions
      powerpc/pseries: Kill all prefetch streams on context switch
      powerpc/pseries: Improve stream generation comments in copypage/user

Nishanth Aravamudan (1):
      powerpc/cputable: Fix oprofile_cpu_type on power8

Paul Mackerras (1):
      powerpc/kvm/book3s: Add support for H_IPOLL and H_XIRR_X in XICS emulation

Priyanka Jain (1):
      powerpc/32bit:Store temporary result in r0 instead of r8

Srivatsa S. Bhat (1):
      powerpc/pseries: Always enable CONFIG_HOTPLUG_CPU on PSERIES SMP

Will Schmidt (1):
      powerpc/cputable: Fix typo on P7+ cputable entry

chenhui zhao (1):
      powerpc/mpic: Fix irq distribution problem when MPIC_SINGLE_DEST_CPU

 Documentation/powerpc/transactional_memory.txt |   27 +++++++++-
 arch/powerpc/include/asm/hvcall.h              |    1 +
 arch/powerpc/include/asm/ppc_asm.h             |   11 ++++
 arch/powerpc/include/asm/processor.h           |   13 ++---
 arch/powerpc/include/asm/reg.h                 |   11 ----
 arch/powerpc/include/asm/signal.h              |    3 ++
 arch/powerpc/include/asm/tm.h                  |    2 +
 arch/powerpc/include/uapi/asm/Kbuild           |    1 +
 arch/powerpc/include/uapi/asm/tm.h             |   18 +++++++
 arch/powerpc/kernel/cputable.c                 |    6 +--
 arch/powerpc/kernel/entry_32.S                 |    2 +-
 arch/powerpc/kernel/entry_64.S                 |    7 +++
 arch/powerpc/kernel/pci-common.c               |   14 +----
 arch/powerpc/kernel/signal.c                   |   40 +++++++++++++-
 arch/powerpc/kernel/signal.h                   |    2 +-
 arch/powerpc/kernel/signal_32.c                |   10 +---
 arch/powerpc/kernel/signal_64.c                |   23 +++-----
 arch/powerpc/kernel/traps.c                    |   29 ++++++++++
 arch/powerpc/kvm/book3s_hv.c                   |    2 +
 arch/powerpc/kvm/book3s_pr_papr.c              |    2 +
 arch/powerpc/kvm/book3s_xics.c                 |   29 ++++++++++
 arch/powerpc/lib/copypage_power7.S             |   19 ++++---
 arch/powerpc/lib/copyuser_power7.S             |   12 +++--
 arch/powerpc/mm/hash_native_64.c               |   30 ++++++++---
 arch/powerpc/perf/core-book3s.c                |   67 +++++++++++-------------
 arch/powerpc/platforms/pseries/Kconfig         |    2 +
 arch/powerpc/sysdev/mpic.c                     |    4 +-
 27 files changed, 262 insertions(+), 125 deletions(-)
 create mode 100644 arch/powerpc/include/uapi/asm/tm.h

^ permalink raw reply

* Re: [PATCH v2] can: flexcan: remove HAVE_CAN_FLEXCAN Kconfig symbol
From: Benjamin Herrenschmidt @ 2013-05-31 23:40 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Arnd Bergmann, Sascha Hauer, U Bhaskar-B22300, linux-kernel,
	linux-can, Shawn Guo, linuxppc-dev, linux-arm-kernel
In-Reply-To: <519A4A00.8000109@pengutronix.de>

On Mon, 2013-05-20 at 18:06 +0200, Marc Kleine-Budde wrote:
> On 05/17/2013 11:09 AM, Shawn Guo wrote:
> > On Fri, May 17, 2013 at 10:59:17AM +0200, Marc Kleine-Budde wrote:
> >> This patch removes the Kconfig symbol HAVE_CAN_FLEXCAN from arch/{arm,powerpc}
> >> and allowing compilation unconditionally on all arm and powerpc platforms.
> >>
> >> This brings a bigger compile time coverage and removes the following dependency
> >> warning found by Arnd Bergmann:
> >>
> >>     warning: (SOC_IMX28 && SOC_IMX25 && SOC_IMX35 && IMX_HAVE_PLATFORM_FLEXCAN &&
> >>         SOC_IMX53 && SOC_IMX6Q) selects HAVE_CAN_FLEXCAN
> >>     which has unmet direct dependencies (NET && CAN && CAN_DEV)
> >>
> >> Cc: Arnd Bergmann <arnd@arndb.de>
> >> Cc: Shawn Guo <shawn.guo@linaro.org>
> > 
> > Acked-by: Shawn Guo <shawn.guo@linaro.org>
> 
> Thanks.
> 
> An Acked-by by the powerpc people would be fine. However, if nobody
> doesn't object, I'm sending this patch via linux-can and net-next upstream.

Sorry, missed it, if it's still out there, add my

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH 00/12] dma: various minor clean ups for slave drivers
From: Dan Williams @ 2013-06-01  0:09 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Shawn Guo, Stephen Warren, Linux Kernel Mailing List,
	Andy Shevchenko, Zhang Wei, linux-tegra, linuxppc-dev
In-Reply-To: <20130530174727.GE3767@intel.com>

On Thu, May 30, 2013 at 10:47 AM, Vinod Koul <vinod.koul@intel.com> wrote:
> On Mon, May 27, 2013 at 03:14:30PM +0300, Andy Shevchenko wrote:
>> Here is a set of small independent patches that clean up or fix minor things
>> across DMA slave drivers.
> The series looks fine. I am going to wait a day more and apply, pls speak up if
> you disagree and ack if you agree

Looks ok to me.  Reminds we can probably take this one step further
and provide a generic implementation for the common case.  It's just a
bit inconsistent though that some engines will poll the completion
handler (try to advance the state of the last completed cookie)
whereas others just assume things will be completed asynchronously.

--
Dan

^ permalink raw reply

* Re: [PATCH 02/23] powerpc/eeh: Function to tranverse PCI devices
From: Benjamin Herrenschmidt @ 2013-06-01  4:13 UTC (permalink / raw)
  To: Gavin Shan; +Cc: linuxppc-dev
In-Reply-To: <1369902245-5886-3-git-send-email-shangw@linux.vnet.ibm.com>

On Thu, 2013-05-30 at 16:23 +0800, Gavin Shan wrote:
> For EEH on PowerNV platform, the PCI devices will be probed to
> check if they support EEH functionality. Different from the case
> of EEH for pSeries platform, we will probe real PCI device instead
> of device tree node for EEH capability on PowerNV platform.
> 
> The patch introduces function eeh_pci_dev_traverse() to traverse
> PCI devices for the indicated PCI bus from top to bottom.

This seems racy vs. hotplug etc... Any reason you can't use
pci_walk_bus() from drivers/pci/bus.c ?

Cheers,
Ben.

> Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
> ---
>  arch/powerpc/include/asm/eeh.h           |    3 ++
>  arch/powerpc/platforms/pseries/eeh_dev.c |   35 ++++++++++++++++++++++++++++++
>  2 files changed, 38 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
> index e32c3c5..eeaeab6 100644
> --- a/arch/powerpc/include/asm/eeh.h
> +++ b/arch/powerpc/include/asm/eeh.h
> @@ -183,6 +183,7 @@ static inline void eeh_unlock(void)
>  #define EEH_MAX_ALLOWED_FREEZES 5
>  
>  typedef void *(*eeh_traverse_func)(void *data, void *flag);
> +typedef void *(*eeh_pci_traverse_func)(struct pci_dev *dev, void *flag);
>  int eeh_phb_pe_create(struct pci_controller *phb);
>  int eeh_add_to_parent_pe(struct eeh_dev *edev);
>  int eeh_rmv_from_parent_pe(struct eeh_dev *edev, int purge_pe);
> @@ -191,6 +192,8 @@ void *eeh_pe_dev_traverse(struct eeh_pe *root,
>  void eeh_pe_restore_bars(struct eeh_pe *pe);
>  struct pci_bus *eeh_pe_bus_get(struct eeh_pe *pe);
>  
> +void eeh_pci_dev_traverse(struct pci_bus *bus,
> +		eeh_pci_traverse_func fn, void *flag);
>  void *eeh_dev_init(struct device_node *dn, void *data);
>  void eeh_dev_phb_init_dynamic(struct pci_controller *phb);
>  int __init eeh_ops_register(struct eeh_ops *ops);
> diff --git a/arch/powerpc/platforms/pseries/eeh_dev.c b/arch/powerpc/platforms/pseries/eeh_dev.c
> index 1efa28f..12c445d 100644
> --- a/arch/powerpc/platforms/pseries/eeh_dev.c
> +++ b/arch/powerpc/platforms/pseries/eeh_dev.c
> @@ -41,6 +41,41 @@
>  #include <asm/pci-bridge.h>
>  #include <asm/ppc-pci.h>
>  
> +
> +/**
> + * eeh_pci_dev_traverse - Traverse PCI devices for the indicated bus
> + * @bus: PCI bus
> + * @fn: callback function
> + * @flag: extra flag
> + *
> + * The function traverses the PCI devices for the indicated PCI bus
> + * from top to bottom fashion until the supplied callback function
> + * returns non-zero value on the specific PCI device.
> + */
> +void eeh_pci_dev_traverse(struct pci_bus *bus,
> +		eeh_pci_traverse_func fn, void *flag)
> +{
> +	struct pci_dev *dev;
> +	void *ret;
> +
> +	if (!bus)
> +		return;
> +
> +	/*
> +	 * We should make sure the parent devices are scanned
> +	 * prior to the child devices so that the parent PE
> +	 * could be created before the child PEs.
> +	 */
> +	list_for_each_entry(dev, &bus->devices, bus_list) {
> +		ret = fn(dev, flag);
> +		if (ret)
> +			return;
> +
> +		if (dev->subordinate)
> +			eeh_pci_dev_traverse(dev->subordinate, fn, flag);
> +	}
> +}
> +
>  /**
>   * eeh_dev_init - Create EEH device according to OF node
>   * @dn: device node

^ permalink raw reply

* Re: [PATCH 03/23] powerpc/eeh: Make eeh_phb_pe_get() public
From: Benjamin Herrenschmidt @ 2013-06-01  4:14 UTC (permalink / raw)
  To: Gavin Shan; +Cc: linuxppc-dev
In-Reply-To: <1369902245-5886-4-git-send-email-shangw@linux.vnet.ibm.com>

On Thu, 2013-05-30 at 16:23 +0800, Gavin Shan wrote:
> While processing EEH event interrupt from P7IOC, we need function
> to retrieve the PE according to the indicated PCI host controller
> (struct pci_controller). The patch makes function eeh_phb_pe_get()
> public so that other source files can call it for that purpose.

Just to make things clear to me... You always have the concept of a
"controller PE" ? What does it actually correspond to in terms of HW
setting ? Bus 0 ? Bus 0..255 ? IE, A "catch all" fallback ?

Cheers,
Ben.

> Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
> ---
>  arch/powerpc/include/asm/eeh.h          |    1 +
>  arch/powerpc/platforms/pseries/eeh_pe.c |    2 +-
>  2 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
> index eeaeab6..4b48178 100644
> --- a/arch/powerpc/include/asm/eeh.h
> +++ b/arch/powerpc/include/asm/eeh.h
> @@ -185,6 +185,7 @@ static inline void eeh_unlock(void)
>  typedef void *(*eeh_traverse_func)(void *data, void *flag);
>  typedef void *(*eeh_pci_traverse_func)(struct pci_dev *dev, void *flag);
>  int eeh_phb_pe_create(struct pci_controller *phb);
> +struct eeh_pe *eeh_phb_pe_get(struct pci_controller *phb);
>  int eeh_add_to_parent_pe(struct eeh_dev *edev);
>  int eeh_rmv_from_parent_pe(struct eeh_dev *edev, int purge_pe);
>  void *eeh_pe_dev_traverse(struct eeh_pe *root,
> diff --git a/arch/powerpc/platforms/pseries/eeh_pe.c b/arch/powerpc/platforms/pseries/eeh_pe.c
> index fe43d1a..6e3eb43 100644
> --- a/arch/powerpc/platforms/pseries/eeh_pe.c
> +++ b/arch/powerpc/platforms/pseries/eeh_pe.c
> @@ -95,7 +95,7 @@ int eeh_phb_pe_create(struct pci_controller *phb)
>   * hierarchy tree is composed of PHB PEs. The function is used
>   * to retrieve the corresponding PHB PE according to the given PHB.
>   */
> -static struct eeh_pe *eeh_phb_pe_get(struct pci_controller *phb)
> +struct eeh_pe *eeh_phb_pe_get(struct pci_controller *phb)
>  {
>  	struct eeh_pe *pe;
>  

^ permalink raw reply

* Re: [PATCH 08/23] powerpc/eeh: Refactor eeh_reset_pe_once()
From: Benjamin Herrenschmidt @ 2013-06-01  4:18 UTC (permalink / raw)
  To: Gavin Shan; +Cc: linuxppc-dev
In-Reply-To: <1369902245-5886-9-git-send-email-shangw@linux.vnet.ibm.com>

On Thu, 2013-05-30 at 16:23 +0800, Gavin Shan wrote:
> The patch changes the criteria used to judge if the PE has been
> resetted successfully. We needn't the PE status is exactly equal
> to the combo: (EEH_STATE_MMIO_ACTIVE | EEH_STATE_DMA_ACTIVE).

The comment above doesn't mean anything :-)

I assume you meant to write "we shouldn't check that the
returned PE status is exactly equal to the two flags X and Y,
but instead only check that they are both set".

Cheers,
Ben.

> Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
> ---
>  arch/powerpc/platforms/pseries/eeh.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/eeh.c b/arch/powerpc/platforms/pseries/eeh.c
> index 39d2ea6..ffe34c4 100644
> --- a/arch/powerpc/platforms/pseries/eeh.c
> +++ b/arch/powerpc/platforms/pseries/eeh.c
> @@ -527,7 +527,6 @@ static void eeh_reset_pe_once(struct eeh_pe *pe)
>  	 * Partitionable Endpoint trumps hot-reset.
>    	 */
>  	eeh_pe_dev_traverse(pe, eeh_set_dev_freset, &freset);
> -

Unrelated churn

>  	if (freset)
>  		eeh_ops->reset(pe, EEH_RESET_FUNDAMENTAL);
>  	else
> @@ -565,6 +564,7 @@ static void eeh_reset_pe_once(struct eeh_pe *pe)
>   */
>  int eeh_reset_pe(struct eeh_pe *pe)
>  {
> +	int flags = (EEH_STATE_MMIO_ACTIVE | EEH_STATE_DMA_ACTIVE);
>  	int i, rc;
>  
>  	/* Take three shots at resetting the bus */
> @@ -572,7 +572,7 @@ int eeh_reset_pe(struct eeh_pe *pe)
>  		eeh_reset_pe_once(pe);
>  
>  		rc = eeh_ops->wait_state(pe, PCI_BUS_RESET_WAIT_MSEC);
> -		if (rc == (EEH_STATE_MMIO_ACTIVE | EEH_STATE_DMA_ACTIVE))
> +		if ((rc & flags) == flags)
>  			return 0;
>  
>  		if (rc < 0) {

^ permalink raw reply

* Re: SATA hang on 8315E triggered by heavy flash write?
From: Anthony Foiani @ 2013-06-01  4:24 UTC (permalink / raw)
  To: Xie Shaohui-B21989; +Cc: Wood Scott-B07421, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <ED492CCEAF882048BC2237DE806547C90B1E8E68@039-SN2MPN1-013.039d.mgd.msft.net>


Shaohui, greetings --

Xie Shaohui-B21989 <B21989@freescale.com> writes:

> I found a MPC8315ERDB rev1.0 board and did some tests.

I bet that was fun.  :) Thanks for going the extra mile and finding
that hardware.  Were you able to unearth a 8315DS of any sort?

> First there is no limit speed issue on the board, so it seems it may
> only happen on the MPC8315DS board.

To be clear, the board we're using does boot and run just fine at
3Gbps most of the time; the CONFIG_MPC8315DS fix was one suggested by
our vendor, but even then, I suspect it was basically prophylactic.

Or, perhaps, it was conflated with the NOR / SATA issue -- see below.

> Second, the SATA can work well with NOR write operation on the ERDB
> board. So the two issues happened to you should be board issues.

Very possibly!

Our vendor has identified at least one possible error with the wiring
/ routing on this board, and have suggested a hardware modification.
Their fix makes sense, but any hardware modification introduces the
risk of breaking one of the few prototype boards.

Since we're very close to software delivery, and we have a workaround
in hand -- namely, don't write the disk during flash operations -- my
team has decided that we'll go with the software workaround until
initial delivery.

We should be able to do this modification and associated testing in
mid-July; at that point, I'll report back with our findings.

Thanks again for all your help; you and Scott have been extremely
helpful and have provided excellent support.  Apologies if it turns
out that it was all due to a wiring error.  :(

Best regards,
Anthony Foiani

^ permalink raw reply

* Re: [PATCH 16/23] powerpc/eeh: I/O chip PE reset
From: Benjamin Herrenschmidt @ 2013-06-01  4:24 UTC (permalink / raw)
  To: Gavin Shan; +Cc: linuxppc-dev
In-Reply-To: <1369902245-5886-17-git-send-email-shangw@linux.vnet.ibm.com>

On Thu, 2013-05-30 at 16:23 +0800, Gavin Shan wrote:
> The patch adds the I/O chip backend to do PE reset. For now, we
> focus on PCI bus dependent PE. If PHB PE has been put into error
> state, the PHB will take complete reset. Besides, the root bridge
> will take fundamental or hot reset accordingly if the indicated
> PE locates at the toppest of PCI hierarchy tree. Otherwise, the
> upstream p2p bridge will take hot reset.
> 
> Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>

 .../...

> +static s64 ioda_eeh_phb_poll(struct pnv_phb *phb)
> +{
> +	s64 rc = OPAL_HARDWARE;
> +
> +	while (1) {
> +		rc = opal_pci_poll(phb->opal_id);
> +		if (rc <= 0)
> +			break;
> +	}
> +
> +	return rc;
> +}

This is rude. It should at the very least cond_resched, but better,
isn't the firmware supposed to return a positive value to indicate
how long to wait for before calling again ? In that case it should
msleep() ...

> +static int ioda_eeh_phb_reset(struct pci_controller *hose, int option)
> +{
> +	struct pnv_phb *phb = hose->private_data;
> +	s64 rc = OPAL_HARDWARE;
> +
> +	pr_debug("%s: Reset PHB#%x, option=%d\n",
> +		__func__, hose->global_number, option);
> +
> +	/* Issue PHB complete reset request */
> +	if (option == EEH_RESET_FUNDAMENTAL ||
> +	    option == EEH_RESET_HOT)
> +		rc = opal_pci_reset(phb->opal_id,
> +				OPAL_PHB_COMPLETE,
> +				OPAL_ASSERT_RESET);
> +	else if (option == EEH_RESET_DEACTIVATE)
> +		rc = opal_pci_reset(phb->opal_id,
> +				OPAL_PHB_COMPLETE,
> +				OPAL_DEASSERT_RESET);
> +	if (rc < 0)
> +		goto out;
> +
> +	/*
> +	 * Poll state of the PHB until the request is done
> +	 * successfully.
> +	 */
> +	rc = ioda_eeh_phb_poll(phb);
> +out:
> +	if (rc != OPAL_SUCCESS)
> +		return -EIO;
> +
> +	return 0;
> +}
> +
> +static int ioda_eeh_root_reset(struct pci_controller *hose, int option)
> +{
> +	struct pnv_phb *phb = hose->private_data;
> +	s64 rc = OPAL_SUCCESS;
> +
> +	pr_debug("%s: Reset PHB#%x, option=%d\n",
> +		__func__, hose->global_number, option);
> +
> +	/*
> +	 * During the reset deassert time, we needn't care
> +	 * the reset scope because the firmware does nothing
> +	 * for fundamental or hot reset during deassert phase.
> +	 */
> +	if (option == EEH_RESET_FUNDAMENTAL)
> +		rc = opal_pci_reset(phb->opal_id,
> +				OPAL_PCI_FUNDAMENTAL_RESET,
> +				OPAL_ASSERT_RESET);
> +	else if (option == EEH_RESET_HOT)
> +		rc = opal_pci_reset(phb->opal_id,
> +				OPAL_PCI_HOT_RESET,
> +				OPAL_ASSERT_RESET);
> +	else if (option == EEH_RESET_DEACTIVATE)
> +		rc = opal_pci_reset(phb->opal_id,
> +				OPAL_PCI_HOT_RESET,
> +				OPAL_DEASSERT_RESET);
> +	if (rc < 0)
> +		goto out;
> +
> +	/* Poll state of the PHB until the request is done */
> +	rc = ioda_eeh_phb_poll(phb);
> +out:
> +	if (rc != OPAL_SUCCESS)
> +		return -EIO;
> +
> +	return 0;
> +}
> +
> +static int ioda_eeh_bridge_reset(struct pci_controller *hose,
> +		struct pci_dev *dev, int option)
> +{
> +	u16 ctrl;
> +
> +	pr_debug("%s: Reset device %04x:%02x:%02x.%01x with option %d\n",
> +		__func__, hose->global_number, dev->bus->number,
> +		PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn), option);
> +
> +	switch (option) {
> +	case EEH_RESET_FUNDAMENTAL:
> +	case EEH_RESET_HOT:
> +		pci_read_config_word(dev, PCI_BRIDGE_CONTROL, &ctrl);
> +		ctrl |= PCI_BRIDGE_CTL_BUS_RESET;
> +		pci_write_config_word(dev, PCI_BRIDGE_CONTROL, ctrl);
> +		break;
> +	case EEH_RESET_DEACTIVATE:
> +		pci_read_config_word(dev, PCI_BRIDGE_CONTROL, &ctrl);
> +		ctrl &= ~PCI_BRIDGE_CTL_BUS_RESET;
> +		pci_write_config_word(dev, PCI_BRIDGE_CONTROL, ctrl);
> +		break;
> +	}
> +
> +	return 0;
> +}

Is there any minimum delay by spec here ? And is there a delay to
respect after the rest or is that handled at the upper level ?

Also, it looks like nothing here checks if the link is coming back
up below the bridge, at what speed etc... this might be something
to add in the future.

> +/**
> + * ioda_eeh_reset - Reset the indicated PE
> + * @pe: EEH PE
> + * @option: reset option
> + *
> + * Do reset on the indicated PE. For PCI bus sensitive PE,
> + * we need to reset the parent p2p bridge. The PHB has to
> + * be reinitialized if the p2p bridge is root bridge. For
> + * PCI device sensitive PE, we will try to reset the device
> + * through FLR. For now, we don't have OPAL APIs to do HARD
> + * reset yet, so all reset would be SOFT (HOT) reset.
> + */
> +static int ioda_eeh_reset(struct eeh_pe *pe, int option)
> +{
> +	struct pci_controller *hose = pe->phb;
> +	struct eeh_dev *edev;
> +	struct pci_dev *dev;
> +	int ret;
> +
> +	/*
> +	 * Anyway, we have to clear the problematic state for the
> +	 * corresponding PE. However, we needn't do it if the PE
> +	 * is PHB associated. That means the PHB is having fatal
> +	 * errors and it needs reset. Further more, the AIB interface
> +	 * isn't reliable any more.
> +	 */
> +	if (!(pe->type & EEH_PE_PHB) &&
> +	    (option == EEH_RESET_HOT ||
> +	    option == EEH_RESET_FUNDAMENTAL)) {
> +		ret = ioda_eeh_pe_clear(pe);
> +		if (ret)
> +			return -EIO;
> +	}
> +
> +	/*
> +	 * The rules applied to reset, either fundamental or hot reset:
> +	 *
> +	 * We always reset the direct upstream bridge of the PE. If the
> +	 * direct upstream bridge isn't root bridge, we always take hot
> +	 * reset no matter what option (fundamental or hot) is. Otherwise,
> +	 * we should do the reset according to the required option.
> +	 */
> +	if (pe->type & EEH_PE_PHB) {
> +		ret = ioda_eeh_phb_reset(hose, option);
> +	} else {
> +		if (pe->type & EEH_PE_DEVICE) {
> +			/*
> +			 * If it's device PE, we didn't refer to the parent
> +			 * PCI bus yet. So we have to figure it out indirectly.
> +			 */
> +			edev = list_first_entry(&pe->edevs,
> +					struct eeh_dev, list);
> +			dev = eeh_dev_to_pci_dev(edev);
> +			dev = dev->bus->self;
> +		} else {
> +			/*
> +			 * If it's bus PE, the parent PCI bus is already there
> +			 * and just pick it up.
> +			 */
> +			dev = pe->bus->self;
> +		}
> +
> +		/*
> +		 * Do reset based on the fact that the direct upstream bridge
> +		 * is root bridge (port) or not.
> +		 */
> +		if (dev->bus->number == 0)
> +			ret = ioda_eeh_root_reset(hose, option);
> +		else
> +			ret = ioda_eeh_bridge_reset(hose, dev, option);
> +	}
> +
> +	return ret;
> +}
> +
>  struct pnv_eeh_ops ioda_eeh_ops = {
>  	.post_init		= ioda_eeh_post_init,
>  	.set_option		= ioda_eeh_set_option,
>  	.get_state		= ioda_eeh_get_state,
> -	.reset			= NULL,
> +	.reset			= ioda_eeh_reset,
>  	.get_log		= NULL,
>  	.configure_bridge	= NULL
>  };

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH 18/23] powerpc/eeh: PowerNV EEH backends
From: Benjamin Herrenschmidt @ 2013-06-01  4:29 UTC (permalink / raw)
  To: Gavin Shan; +Cc: linuxppc-dev
In-Reply-To: <1369902245-5886-19-git-send-email-shangw@linux.vnet.ibm.com>

On Thu, 2013-05-30 at 16:24 +0800, Gavin Shan wrote:
> The patch adds EEH backends for PowerNV platform. It's notable that
> part of those EEH backends call to the I/O chip dependent backends.

Add a check for my new OPALv3 flag before registering since you rely
on a whole bunch of new APIs that the current versions of OPAL don't
have.

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH 22/23] powerpc/eeh: Connect EEH error interrupt handle
From: Benjamin Herrenschmidt @ 2013-06-01  4:32 UTC (permalink / raw)
  To: Gavin Shan; +Cc: linuxppc-dev
In-Reply-To: <1369902245-5886-23-git-send-email-shangw@linux.vnet.ibm.com>

On Thu, 2013-05-30 at 16:24 +0800, Gavin Shan wrote:
> The EEH error interrupts should have been exported by firmware
> through device tree. The OS already installed the interrupt
> handler (opal_interrupt()) for them. The patch checks if we have
> any existing EEH errors and wakes the kernel thread up to process
> that if possible.

Instead, please create a new notifier that opal interrupt calls whenever
the return even state changes and have PCI "register" with it.

Additionally, we want any caller of opal_poll_events() to instead call
a wrapper that will also check for event changes and signal "clients".

Finally, we need a way to disable/enable that notifying via something
like an atomic counter (no need to hard synchronize with pending calls)
and use it for something like xmon to avoid calling all over the place
when xmon polls for console input via udbg for example.

This is a bit of work, I can give you a hand with it next week.

Cheers,
Ben.

> Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
> ---
>  arch/powerpc/platforms/powernv/opal.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
> index 628c564..cca78c9 100644
> --- a/arch/powerpc/platforms/powernv/opal.c
> +++ b/arch/powerpc/platforms/powernv/opal.c
> @@ -18,6 +18,8 @@
>  #include <linux/slab.h>
>  #include <asm/opal.h>
>  #include <asm/firmware.h>
> +#include <asm/io.h>
> +#include <asm/eeh.h>
>  
>  #include "powernv.h"
>  
> @@ -296,6 +298,10 @@ static irqreturn_t opal_interrupt(int irq, void *data)
>  	uint64_t events;
>  
>  	opal_handle_interrupt(virq_to_hw(irq), &events);
> +#ifdef CONFIG_EEH
> +	if (events & OPAL_EVENT_PCI_ERROR)
> +		pci_err_event();
> +#endif
>  
>  	/* XXX TODO: Do something with the events */
>  

^ permalink raw reply

* Re: [PATCH 23/23] powerpc/eeh: Add debugfs entry to inject errors
From: Benjamin Herrenschmidt @ 2013-06-01  4:34 UTC (permalink / raw)
  To: Gavin Shan; +Cc: linuxppc-dev
In-Reply-To: <1369902245-5886-24-git-send-email-shangw@linux.vnet.ibm.com>

On Thu, 2013-05-30 at 16:24 +0800, Gavin Shan wrote:
> The patch intends to add debugfs entry powerpc/EEH/PHBx so that
> the administrator can inject EEH errors to specified PCI host
> bridge for testing purpose.

Use a better naming for the debugfs files. Something like
eeh_err_inject/pciNNNN, to be consistent with the general naming
of PHBs in the system.

However, maybe it would be better to instead having something along
the lines of a directory per PHB with a file in it for error injection ?

That way we can stick more things in there that can become handy for
debugging / diagnostics, such as register dumps etc...

Cheers,
Ben.

> Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
> ---
>  arch/powerpc/platforms/powernv/eeh-ioda.c |   36 ++++++++++++++++++++++++++++-
>  1 files changed, 35 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
> index ec5c524..4cc9db7 100644
> --- a/arch/powerpc/platforms/powernv/eeh-ioda.c
> +++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
> @@ -22,6 +22,7 @@
>  
>  #include <linux/bootmem.h>
>  #include <linux/delay.h>
> +#include <linux/debugfs.h>
>  #include <linux/init.h>
>  #include <linux/io.h>
>  #include <linux/irq.h>
> @@ -43,6 +44,29 @@
>  #include "powernv.h"
>  #include "pci.h"
>  
> +static struct dentry *ioda_eeh_dbgfs = NULL;
> +
> +static int ioda_eeh_dbgfs_set(void *data, u64 val)
> +{
> +	struct pci_controller *hose = data;
> +	struct pnv_phb *phb = hose->private_data;
> +
> +	out_be64(phb->regs + 0xD10, val);
> +	return 0;
> +}
> +
> +static int ioda_eeh_dbgfs_get(void *data, u64 *val)
> +{
> +	struct pci_controller *hose = data;
> +	struct pnv_phb *phb = hose->private_data;
> +
> +	*val = in_be64(phb->regs + 0xD10);
> +	return 0;
> +}
> +
> +DEFINE_SIMPLE_ATTRIBUTE(ioda_eeh_dbgfs_ops, ioda_eeh_dbgfs_get,
> +			ioda_eeh_dbgfs_set, "0x%llx\n");
> +
>  /**
>   * ioda_eeh_post_init - Chip dependent post initialization
>   * @hose: PCI controller
> @@ -54,10 +78,20 @@
>  static int ioda_eeh_post_init(struct pci_controller *hose)
>  {
>  	struct pnv_phb *phb = hose->private_data;
> +	char name[16];
> +
> +	/* Create EEH debugfs root if possible */
> +	if (!ioda_eeh_dbgfs)
> +		ioda_eeh_dbgfs = debugfs_create_dir("EEH", powerpc_debugfs_root);
>  
>  	/* FIXME: Enable it for PHB3 later */
> -	if (phb->type == PNV_PHB_IODA1)
> +	if (phb->type == PNV_PHB_IODA1) {
> +		sprintf(name, "PHB%d", hose->global_number);
> +		debugfs_create_file(name, 0600, ioda_eeh_dbgfs,
> +				    hose, &ioda_eeh_dbgfs_ops);
> +
>  		phb->eeh_enabled = 1;
> +	}
>  
>  	return 0;
>  }

^ permalink raw reply

* Re: [PATCH v3 0/8] Nvram-to-pstore
From: Benjamin Herrenschmidt @ 2013-06-01  4:40 UTC (permalink / raw)
  To: Aruna Balakrishnaiah
  Cc: jkenisto, tony.luck, mahesh, cbouatmailru, linux-kernel,
	linuxppc-dev, paulus, anton, ccross, keescook
In-Reply-To: <20130425100952.21017.51799.stgit@aruna-ThinkPad-T420>

On Thu, 2013-04-25 at 15:47 +0530, Aruna Balakrishnaiah wrote:
> Currently the kernel provides the contents of p-series NVRAM only as a
> simple stream of bytes via /dev/nvram, which must be interpreted in user
> space by the nvram command in the powerpc-utils package. This patch set
> exploits the pstore subsystem to expose each partition in NVRAM as a
> separate file in /dev/pstore. For instance Oops messages will stored in a
> file named [dmesg-nvram-2].

We will need the same stuff for powernv. This is not a blocker for this
patch series which I'm happy to apply (after I give it another round of
review, hopefully today) but I would very much like you to, on top of
this, start moving some of the basic pseries partition nvram handling
to a generic place, along with your pstore bits, and use it on powernv
as well.

In fact, this applies to at least all the BookS server platforms...

Things that come to mind:

 - nvram_64.c duplicates generic_nvram.c as far as the user accessors
are concerned, it should be possible to get rid of code there. Either
the arch or the generic one (*)

 - The nvram partition management should move to generic. While at it
factor in the powermac variant (same stuff, mostly duplicated code)

 - powernv wants all the goodies that pseries has, as does cell.

(*) I wonder about that generic stuff... userspace might want to start
doing things like resizing the common partition if not big enough etc...
For that we might want to add more specific ioctl's. Is anybody other
than us using generic_nvram ? I don't like adding ioctl's like that
to a generic driver, maybe we could just make it call into something
like arch_nvram_ioctl() and have an empty weak variant instead of the
current ifdef game.

Cheers,
Ben.

> Changes from v2:
> 	- Fix renaming of pstore type ids in nvram.c
> 
> Changes from v1:
> 	- Reduce #ifdefs by and remove forward declarations of pstore callbacks
> 	- Handle return value of nvram_write_os_partition
> 	- Remove empty pstore callbacks and register pstore only when pstore
> 	  is configured
> ---
> 
> Aruna Balakrishnaiah (8):
>       powerpc/pseries: Remove syslog prefix in uncompressed oops text
>       powerpc/pseries: Add version and timestamp to oops header
>       powerpc/pseries: Introduce generic read function to read nvram-partitions
>       powerpc/pseries: Read/Write oops nvram partition via pstore
>       powerpc/pseries: Read rtas partition via pstore
>       powerpc/pseries: Distinguish between a os-partition and non-os partition
>       powerpc/pseries: Read of-config partition via pstore
>       powerpc/pseries: Read common partition via pstore
> 
> 
>  arch/powerpc/platforms/pseries/nvram.c |  353 +++++++++++++++++++++++++++-----
>  fs/pstore/inode.c                      |    9 +
>  include/linux/pstore.h                 |    4 
>  3 files changed, 313 insertions(+), 53 deletions(-)
> 

^ permalink raw reply

* Re: [PATCH v3 0/8] Nvram-to-pstore
From: Benjamin Herrenschmidt @ 2013-06-01  4:43 UTC (permalink / raw)
  To: Aruna Balakrishnaiah
  Cc: jkenisto, tony.luck, mahesh, cbouatmailru, linux-kernel,
	linuxppc-dev, paulus, anton, ccross, keescook
In-Reply-To: <1370061602.3766.22.camel@pasglop>

On Sat, 2013-06-01 at 14:40 +1000, Benjamin Herrenschmidt wrote:

  .../...

> In fact, this applies to at least all the BookS server platforms...
> 
> Things that come to mind:
> 
>  - nvram_64.c duplicates generic_nvram.c as far as the user accessors
> are concerned, it should be possible to get rid of code there. Either
> the arch or the generic one (*)
> 
>  - The nvram partition management should move to generic. While at it
> factor in the powermac variant (same stuff, mostly duplicated code)
> 
>  - powernv wants all the goodies that pseries has, as does cell.

 - It's stupid for userspace to re-implement the whole partition scheme,
so let's add ioctl's to lookup partitions by name & type. We could turn
the whole thing into sysfs instead too which might be better .... (ie
a file per partition).

> (*) I wonder about that generic stuff... userspace might want to start
> doing things like resizing the common partition if not big enough etc...
> For that we might want to add more specific ioctl's. Is anybody other
> than us using generic_nvram ? I don't like adding ioctl's like that
> to a generic driver, maybe we could just make it call into something
> like arch_nvram_ioctl() and have an empty weak variant instead of the
> current ifdef game.
> 
> Cheers,
> Ben.
> 
> > Changes from v2:
> > 	- Fix renaming of pstore type ids in nvram.c
> > 
> > Changes from v1:
> > 	- Reduce #ifdefs by and remove forward declarations of pstore callbacks
> > 	- Handle return value of nvram_write_os_partition
> > 	- Remove empty pstore callbacks and register pstore only when pstore
> > 	  is configured
> > ---
> > 
> > Aruna Balakrishnaiah (8):
> >       powerpc/pseries: Remove syslog prefix in uncompressed oops text
> >       powerpc/pseries: Add version and timestamp to oops header
> >       powerpc/pseries: Introduce generic read function to read nvram-partitions
> >       powerpc/pseries: Read/Write oops nvram partition via pstore
> >       powerpc/pseries: Read rtas partition via pstore
> >       powerpc/pseries: Distinguish between a os-partition and non-os partition
> >       powerpc/pseries: Read of-config partition via pstore
> >       powerpc/pseries: Read common partition via pstore
> > 
> > 
> >  arch/powerpc/platforms/pseries/nvram.c |  353 +++++++++++++++++++++++++++-----
> >  fs/pstore/inode.c                      |    9 +
> >  include/linux/pstore.h                 |    4 
> >  3 files changed, 313 insertions(+), 53 deletions(-)
> > 
> 

^ permalink raw reply

* Re: [PATCH v3 5/8] powerpc/pseries: Read rtas partition via pstore
From: Benjamin Herrenschmidt @ 2013-06-01  4:49 UTC (permalink / raw)
  To: Aruna Balakrishnaiah
  Cc: jkenisto, tony.luck, mahesh, cbouatmailru, linux-kernel,
	linuxppc-dev, paulus, anton, ccross, keescook
In-Reply-To: <20130425101851.21017.90041.stgit@aruna-ThinkPad-T420>

On Thu, 2013-04-25 at 15:48 +0530, Aruna Balakrishnaiah wrote:
> This patch set exploits the pstore subsystem to read details of rtas partition
> in NVRAM to a separate file in /dev/pstore. For instance, rtas details will be
> stored in a file named [rtas-nvram-4].
> 
 .../...

> diff --git a/include/linux/pstore.h b/include/linux/pstore.h
> index 75d0176..d7a8fe9 100644
> --- a/include/linux/pstore.h
> +++ b/include/linux/pstore.h
> @@ -35,6 +35,8 @@ enum pstore_type_id {
>  	PSTORE_TYPE_MCE		= 1,
>  	PSTORE_TYPE_CONSOLE	= 2,
>  	PSTORE_TYPE_FTRACE	= 3,
> +	/* PPC64 partition types */
> +	PSTORE_TYPE_PPC_RTAS	= 4,
>  	PSTORE_TYPE_UNKNOWN	= 255
>  };
>  

Not sure about that list...

What do you mean by "RTAS" ? The error logs ? What about our "common"
partition (firmware settings ?). We should probably at least define
a generic PSTORE_TYPE_FIRMWARE for firmware private stuff...

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH v3 5/8] powerpc/pseries: Read rtas partition via pstore
From: Benjamin Herrenschmidt @ 2013-06-01  4:50 UTC (permalink / raw)
  To: Aruna Balakrishnaiah
  Cc: jkenisto, tony.luck, mahesh, cbouatmailru, linux-kernel,
	linuxppc-dev, paulus, anton, ccross, keescook
In-Reply-To: <1370062185.3766.27.camel@pasglop>

On Sat, 2013-06-01 at 14:49 +1000, Benjamin Herrenschmidt wrote:
> On Thu, 2013-04-25 at 15:48 +0530, Aruna Balakrishnaiah wrote:
> > This patch set exploits the pstore subsystem to read details of rtas partition
> > in NVRAM to a separate file in /dev/pstore. For instance, rtas details will be
> > stored in a file named [rtas-nvram-4].
> > 
>  .../...
> 
> > diff --git a/include/linux/pstore.h b/include/linux/pstore.h
> > index 75d0176..d7a8fe9 100644
> > --- a/include/linux/pstore.h
> > +++ b/include/linux/pstore.h
> > @@ -35,6 +35,8 @@ enum pstore_type_id {
> >  	PSTORE_TYPE_MCE		= 1,
> >  	PSTORE_TYPE_CONSOLE	= 2,
> >  	PSTORE_TYPE_FTRACE	= 3,
> > +	/* PPC64 partition types */
> > +	PSTORE_TYPE_PPC_RTAS	= 4,
> >  	PSTORE_TYPE_UNKNOWN	= 255
> >  };
> >  
> 
> Not sure about that list...
> 
> What do you mean by "RTAS" ? The error logs ? What about our "common"
> partition (firmware settings ?). We should probably at least define
> a generic PSTORE_TYPE_FIRMWARE for firmware private stuff...

Scrap it, I just noticed your next patch doing that,... see comment
there.

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH v3 8/8] powerpc/pseries: Read common partition via pstore
From: Benjamin Herrenschmidt @ 2013-06-01  4:52 UTC (permalink / raw)
  To: Aruna Balakrishnaiah
  Cc: jkenisto, tony.luck, mahesh, cbouatmailru, linux-kernel,
	linuxppc-dev, paulus, anton, ccross, keescook
In-Reply-To: <20130425101908.21017.32553.stgit@aruna-ThinkPad-T420>

On Thu, 2013-04-25 at 15:49 +0530, Aruna Balakrishnaiah wrote:

> diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
> index 8d4fb65..88cc050 100644
> --- a/fs/pstore/inode.c
> +++ b/fs/pstore/inode.c
> @@ -330,6 +330,9 @@ int pstore_mkfile(enum pstore_type_id type, char *psname, u64 id, int count,
>  	case PSTORE_TYPE_PPC_OF:
>  		sprintf(name, "of-%s-%lld", psname, id);
>  		break;

Call this powerpc-ofw-... Does it even contain something we use in Linux
at all ? Last I looked we only used the common one right ? Also it's
format afaik is defined in the CHRP bindings so it's not generic OFW
stuff, hence the powerpc prefix.

> +	case PSTORE_TYPE_PPC_COMMON:
> +		sprintf(name, "common-%s-%lld", psname, id);
> +		break;

Same deal, call that powerpc-common

>  	case PSTORE_TYPE_UNKNOWN:
>  		sprintf(name, "unknown-%s-%lld", psname, id);
>  		break;
> diff --git a/include/linux/pstore.h b/include/linux/pstore.h
> index 615dc18..656699f 100644
> --- a/include/linux/pstore.h
> +++ b/include/linux/pstore.h
> @@ -38,6 +38,7 @@ enum pstore_type_id {
>  	/* PPC64 partition types */
>  	PSTORE_TYPE_PPC_RTAS	= 4,
>  	PSTORE_TYPE_PPC_OF	= 5,
> +	PSTORE_TYPE_PPC_COMMON	= 6,
>  	PSTORE_TYPE_UNKNOWN	= 255
>  };

Do we expose anything else or keep it hidden ?

Cheers,
Ben.

^ 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