[PATCH v2 0/6]

[PATCH v2 0/6]

[Glauber Costa]

Marcelo,

since you told me that you were not confrotable with the --enable-kvm
change without avi's ack, I'm dropping it, and sending the rest of the 
series again. On top of that, I'm also including some more cleanup patches:
one removes kvm_out{b,w,l}, in the same way I've already done with kvm_in{b,w,l},
and the other removes specific mmio functions.

Thanks!

[PATCH V2 0/6]

[Marc Kleine-Budde]

Hello,

this series of patches prepares the i.mx tree for the flexcan driver.
Clock support is added or adjusted. Missing defines for the base addresses
and interrupts are added.

The 5th patch uses the newly introduced dynamic registration scheme to
provide the flexcan devices. This patch contains two ifdefs to replace the
"imx_add_flexcan()" function by a noop if the flexcan driver is not
activated. This is needed to compile this series before the actual flexcan
driver and it's header file have been merged. The ifdefs can be removed
after merging.

The last patch registers flexcan to the pcm043.

please consider to apply.

cheers, Marc


P.S.: this can be pulled:

The following changes since commit ed300d7a8d115ef721b1f69ddf733e4c4dba4eac:

  iomux-mx51: add 4 pin definitions (2010-07-22 15:19:45 +0200)

are available in the git repository at:
  git://git.pengutronix.de/git/mkl/linux-2.6.git for-imx-for-2.6.36

Marc Kleine-Budde (5):
      mx25: add flexcan address and interrupt definition
      mx35: adjust flexcan clock definition
      mx35: add flexcan address
      imx: dynamically register flexcan devices for mx25 and mx35
      pcm043: register flexcan device

Sascha Hauer (1):
      mx25: flexcan clock support

[PATCH v2 0/6]

[Andre Przywara]

This is a major rework of last week's ARM PSCI host support. The code
has been moved into a separate file and the code flow has been
changed substantially (see below for more details).
  ---------

Xen did not make use of the host provided ARM PSCI (Power State
Coordination Interface [1]) functionality so far, but relied on
platform specific SMP bringup functions.
This series adds support for PSCI on the host by reading the required
information from the DTB and invoking the appropriate handler when
bringing up each single CPU.
Since PSCI is defined for both ARM32 and ARM64, I added code for both
architectures, though only ARM32 is tested on Midway and VExpress
(without any PSCI support on the latter, just a regression test).
ARM64 code was compile tested only.

The ARM32 SMP boot flow is now as following:
The DTB is scanned for a node announcing PSCI support. If that is
available, call the PSCI handler via SMC to bringup the secondary
cores. If no PSCI has been found, call the platform specific cpu_up()
function. It is now the responsibility of those functions to do the
GIC SGI call to kick the secondary CPUs. For that a function is now
provided which does this, so three platforms now reference this
generic function instead of coding up an empty one and relying on the
GIC kick in Xen code later.

The ARM64 SMP boot flow is different: PSCI is not only described in a
root DTB node, but also advertised in each core's node as the
preferred SMP bringup method. So the PSCI call wrapper function is
registered as the cpu_up function when the DTB is scanned.

This patch series is split up as follows:
1/6) rename psci.c to vpsci.c to make room for the new, host-related
     code
2/6) move the GIC SGI kick into a separate function and call it now
     directly from the platform code, removing the empty cpu_up()
3/6) parse the DTB for a PSCI node and store the needed function index
4/6) add a function to actually invoke the firmware's PSCI handler
5/6) enable PSCI on ARM32 by adding the call to the PSCI handler
6/6) enable PSCI on ARM64 by registering the PSCI handler function


Changes from v1:
- much rework, PSCI host DTB scanning unchanged (v1: 1/4, v2: 3/6)
- moved host PSCI code to psci.c, renaming the old one to vpsci.c
- have a separate psci_available variable
- removing explicit "host" from function names
- returning standard error codes on DTB scanning
- do the ARM64 PSCI call from a registered function pointer
- move the GIC kick into a separate function
- more change to the code flow in general


Signed-off-by: Andre Przywara <andre.przywara@linaro.org>

[1]: http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0022b/index.html

Andre Przywara (6):
  arm: rename xen/arch/arm/psci.c into vpsci.c
  arm: move GIC SGI kicking into separate function
  arm: parse PSCI node from the host device-tree
  arm: add a function to invoke the PSCI handler
  arm32: enable PSCI secondary CPU bringup
  arm64: enable PSCI secondary CPU bringup

 xen/arch/arm/Makefile             |   1 +
 xen/arch/arm/arm32/smpboot.c      |   4 +-
 xen/arch/arm/arm64/smpboot.c      |  23 ++++++-
 xen/arch/arm/platform.c           |   6 +-
 xen/arch/arm/platforms/exynos5.c  |  11 +---
 xen/arch/arm/platforms/omap5.c    |  11 +---
 xen/arch/arm/platforms/vexpress.c |  10 +--
 xen/arch/arm/psci.c               | 127 +++++++++++++++++++-------------------
 xen/arch/arm/smpboot.c            |  22 +++++--
 xen/arch/arm/vpsci.c              |  93 ++++++++++++++++++++++++++++
 xen/include/asm-arm/psci.h        |   7 +++
 xen/include/asm-arm/smp.h         |   2 +
 12 files changed, 215 insertions(+), 102 deletions(-)
 create mode 100644 xen/arch/arm/vpsci.c

-- 
1.7.12.1

Re: [PATCH v2 0/6]

[Ian Campbell]

On Mon, 2013-12-02 at 12:08 +0100, Andre Przywara wrote:
> This is a major rework of last week's ARM PSCI host support. The code
> has been moved into a separate file and the code flow has been
> changed substantially (see below for more details).
>   ---------
> 
> Xen did not make use of the host provided ARM PSCI (Power State
> Coordination Interface [1]) functionality so far, but relied on
> platform specific SMP bringup functions.
> This series adds support for PSCI on the host by reading the required
> information from the DTB and invoking the appropriate handler when
> bringing up each single CPU.
> Since PSCI is defined for both ARM32 and ARM64, I added code for both
> architectures, though only ARM32 is tested on Midway and VExpress
> (without any PSCI support on the latter, just a regression test).
> ARM64 code was compile tested only.
> 
> The ARM32 SMP boot flow is now as following:
> The DTB is scanned for a node announcing PSCI support. If that is
> available, call the PSCI handler via SMC to bringup the secondary
> cores. If no PSCI has been found, call the platform specific cpu_up()
> function.

Is this the most useful way round? If both PSCI and a hook are present
might it be expected that the hook might want to make the choice to
either go manual or call back to PSCI?

Perhaps to handle some quirk (aka bug) in the platform which needs
something else before/after the PSCI stuff.

>  It is now the responsibility of those functions to do the
> GIC SGI call to kick the secondary CPUs. For that a function is now
> provided which does this, so three platforms now reference this
> generic function instead of coding up an empty one and relying on the
> GIC kick in Xen code later.

Note that the existing kick serves two purposes. The first is to wake up
the processor from the firmware on platforms which need that. The second
is to wake up secondaries from the loop in head.S under:
        /* Non-boot CPUs wait here until __cpu_up is ready for them */
where they wait for smp_up_cpu to become them.

I should go read the patches to see what you've actually done here.

> The ARM64 SMP boot flow is different: PSCI is not only described in a
> root DTB node, but also advertised in each core's node as the
> preferred SMP bringup method. So the PSCI call wrapper function is
> registered as the cpu_up function when the DTB is scanned.
> 
> This patch series is split up as follows:
> 1/6) rename psci.c to vpsci.c to make room for the new, host-related
>      code
> 2/6) move the GIC SGI kick into a separate function and call it now
>      directly from the platform code, removing the empty cpu_up()
> 3/6) parse the DTB for a PSCI node and store the needed function index
> 4/6) add a function to actually invoke the firmware's PSCI handler
> 5/6) enable PSCI on ARM32 by adding the call to the PSCI handler
> 6/6) enable PSCI on ARM64 by registering the PSCI handler function
> 
> 
> Changes from v1:
> - much rework, PSCI host DTB scanning unchanged (v1: 1/4, v2: 3/6)
> - moved host PSCI code to psci.c, renaming the old one to vpsci.c
> - have a separate psci_available variable
> - removing explicit "host" from function names
> - returning standard error codes on DTB scanning
> - do the ARM64 PSCI call from a registered function pointer
> - move the GIC kick into a separate function
> - more change to the code flow in general
> 
> 
> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
> 
> [1]: http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0022b/index.html
> 
> Andre Przywara (6):
>   arm: rename xen/arch/arm/psci.c into vpsci.c
>   arm: move GIC SGI kicking into separate function
>   arm: parse PSCI node from the host device-tree
>   arm: add a function to invoke the PSCI handler
>   arm32: enable PSCI secondary CPU bringup
>   arm64: enable PSCI secondary CPU bringup
> 
>  xen/arch/arm/Makefile             |   1 +
>  xen/arch/arm/arm32/smpboot.c      |   4 +-
>  xen/arch/arm/arm64/smpboot.c      |  23 ++++++-
>  xen/arch/arm/platform.c           |   6 +-
>  xen/arch/arm/platforms/exynos5.c  |  11 +---
>  xen/arch/arm/platforms/omap5.c    |  11 +---
>  xen/arch/arm/platforms/vexpress.c |  10 +--
>  xen/arch/arm/psci.c               | 127 +++++++++++++++++++-------------------
>  xen/arch/arm/smpboot.c            |  22 +++++--
>  xen/arch/arm/vpsci.c              |  93 ++++++++++++++++++++++++++++
>  xen/include/asm-arm/psci.h        |   7 +++
>  xen/include/asm-arm/smp.h         |   2 +
>  12 files changed, 215 insertions(+), 102 deletions(-)
>  create mode 100644 xen/arch/arm/vpsci.c
> 

Re: [PATCH v2 0/6]

[Andre Przywara]

On 12/02/2013 03:52 PM, Ian Campbell wrote:
> On Mon, 2013-12-02 at 12:08 +0100, Andre Przywara wrote:
>> This is a major rework of last week's ARM PSCI host support. The code
>> has been moved into a separate file and the code flow has been
>> changed substantially (see below for more details).
>>    ---------
>>
>> Xen did not make use of the host provided ARM PSCI (Power State
>> Coordination Interface [1]) functionality so far, but relied on
>> platform specific SMP bringup functions.
>> This series adds support for PSCI on the host by reading the required
>> information from the DTB and invoking the appropriate handler when
>> bringing up each single CPU.
>> Since PSCI is defined for both ARM32 and ARM64, I added code for both
>> architectures, though only ARM32 is tested on Midway and VExpress
>> (without any PSCI support on the latter, just a regression test).
>> ARM64 code was compile tested only.
>>
>> The ARM32 SMP boot flow is now as following:
>> The DTB is scanned for a node announcing PSCI support. If that is
>> available, call the PSCI handler via SMC to bringup the secondary
>> cores. If no PSCI has been found, call the platform specific cpu_up()
>> function.
>
> Is this the most useful way round? If both PSCI and a hook are present
> might it be expected that the hook might want to make the choice to
> either go manual or call back to PSCI?
>
> Perhaps to handle some quirk (aka bug) in the platform which needs
> something else before/after the PSCI stuff.

I thought about that too and had it actually that way in the first place.
My reason to change it was: What happens if platforms get PSCI support?
Thinking about sunxi or Arndale. If the device tree advertises PSCI, 
then this is supposed to work. If it doesn't work, the DTB shouldn't 
carry the PSCI node or u-boot & friends have to remove it.

In general I agree on the "fix" idea, but I'd ask for that any bug in 
PSCI kicking should be fixed in the PSCI firmware and not in Xen or 
other kernels. If we are really in need for SMP fixes, we could still 
put them into smp_init().

I can easily change it to prefer platform code, of course.

>
>>   It is now the responsibility of those functions to do the
>> GIC SGI call to kick the secondary CPUs. For that a function is now
>> provided which does this, so three platforms now reference this
>> generic function instead of coding up an empty one and relying on the
>> GIC kick in Xen code later.
>
> Note that the existing kick serves two purposes. The first is to wake up
> the processor from the firmware on platforms which need that. The second
> is to wake up secondaries from the loop in head.S under:
>          /* Non-boot CPUs wait here until __cpu_up is ready for them */
> where they wait for smp_up_cpu to become them.
>
> I should go read the patches to see what you've actually done here.

But currently there is only one kick per CPU, right? I didn't change 
anything in this respect, just moved the code into a function and called 
that a bit earlier.

Regards,
Andre.

>
>> The ARM64 SMP boot flow is different: PSCI is not only described in a
>> root DTB node, but also advertised in each core's node as the
>> preferred SMP bringup method. So the PSCI call wrapper function is
>> registered as the cpu_up function when the DTB is scanned.
>>
>> This patch series is split up as follows:
>> 1/6) rename psci.c to vpsci.c to make room for the new, host-related
>>       code
>> 2/6) move the GIC SGI kick into a separate function and call it now
>>       directly from the platform code, removing the empty cpu_up()
>> 3/6) parse the DTB for a PSCI node and store the needed function index
>> 4/6) add a function to actually invoke the firmware's PSCI handler
>> 5/6) enable PSCI on ARM32 by adding the call to the PSCI handler
>> 6/6) enable PSCI on ARM64 by registering the PSCI handler function
>>
>>
>> Changes from v1:
>> - much rework, PSCI host DTB scanning unchanged (v1: 1/4, v2: 3/6)
>> - moved host PSCI code to psci.c, renaming the old one to vpsci.c
>> - have a separate psci_available variable
>> - removing explicit "host" from function names
>> - returning standard error codes on DTB scanning
>> - do the ARM64 PSCI call from a registered function pointer
>> - move the GIC kick into a separate function
>> - more change to the code flow in general
>>
>>
>> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
>>
>> [1]: http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0022b/index.html
>>
>> Andre Przywara (6):
>>    arm: rename xen/arch/arm/psci.c into vpsci.c
>>    arm: move GIC SGI kicking into separate function
>>    arm: parse PSCI node from the host device-tree
>>    arm: add a function to invoke the PSCI handler
>>    arm32: enable PSCI secondary CPU bringup
>>    arm64: enable PSCI secondary CPU bringup
>>
>>   xen/arch/arm/Makefile             |   1 +
>>   xen/arch/arm/arm32/smpboot.c      |   4 +-
>>   xen/arch/arm/arm64/smpboot.c      |  23 ++++++-
>>   xen/arch/arm/platform.c           |   6 +-
>>   xen/arch/arm/platforms/exynos5.c  |  11 +---
>>   xen/arch/arm/platforms/omap5.c    |  11 +---
>>   xen/arch/arm/platforms/vexpress.c |  10 +--
>>   xen/arch/arm/psci.c               | 127 +++++++++++++++++++-------------------
>>   xen/arch/arm/smpboot.c            |  22 +++++--
>>   xen/arch/arm/vpsci.c              |  93 ++++++++++++++++++++++++++++
>>   xen/include/asm-arm/psci.h        |   7 +++
>>   xen/include/asm-arm/smp.h         |   2 +
>>   12 files changed, 215 insertions(+), 102 deletions(-)
>>   create mode 100644 xen/arch/arm/vpsci.c
>>
>
>

Re: [PATCH v2 0/6]

[Ian Campbell]

On Wed, 2013-12-04 at 13:16 +0100, Andre Przywara wrote:
> On 12/02/2013 03:52 PM, Ian Campbell wrote:
> > On Mon, 2013-12-02 at 12:08 +0100, Andre Przywara wrote:
> >> This is a major rework of last week's ARM PSCI host support. The code
> >> has been moved into a separate file and the code flow has been
> >> changed substantially (see below for more details).
> >>    ---------
> >>
> >> Xen did not make use of the host provided ARM PSCI (Power State
> >> Coordination Interface [1]) functionality so far, but relied on
> >> platform specific SMP bringup functions.
> >> This series adds support for PSCI on the host by reading the required
> >> information from the DTB and invoking the appropriate handler when
> >> bringing up each single CPU.
> >> Since PSCI is defined for both ARM32 and ARM64, I added code for both
> >> architectures, though only ARM32 is tested on Midway and VExpress
> >> (without any PSCI support on the latter, just a regression test).
> >> ARM64 code was compile tested only.
> >>
> >> The ARM32 SMP boot flow is now as following:
> >> The DTB is scanned for a node announcing PSCI support. If that is
> >> available, call the PSCI handler via SMC to bringup the secondary
> >> cores. If no PSCI has been found, call the platform specific cpu_up()
> >> function.
> >
> > Is this the most useful way round? If both PSCI and a hook are present
> > might it be expected that the hook might want to make the choice to
> > either go manual or call back to PSCI?
> >
> > Perhaps to handle some quirk (aka bug) in the platform which needs
> > something else before/after the PSCI stuff.
> 
> I thought about that too and had it actually that way in the first place.
> My reason to change it was: What happens if platforms get PSCI support?
> Thinking about sunxi or Arndale. If the device tree advertises PSCI, 
> then this is supposed to work. If it doesn't work, the DTB shouldn't 
> carry the PSCI node or u-boot & friends have to remove it.
> 
> In general I agree on the "fix" idea, but I'd ask for that any bug in 
> PSCI kicking should be fixed in the PSCI firmware and not in Xen or 
> other kernels. If we are really in need for SMP fixes, we could still 
> put them into smp_init().
> 
> I can easily change it to prefer platform code, of course.

It's ok, I think I'm convinced by your argument.

> >>   It is now the responsibility of those functions to do the
> >> GIC SGI call to kick the secondary CPUs. For that a function is now
> >> provided which does this, so three platforms now reference this
> >> generic function instead of coding up an empty one and relying on the
> >> GIC kick in Xen code later.
> >
> > Note that the existing kick serves two purposes. The first is to wake up
> > the processor from the firmware on platforms which need that. The second
> > is to wake up secondaries from the loop in head.S under:
> >          /* Non-boot CPUs wait here until __cpu_up is ready for them */
> > where they wait for smp_up_cpu to become them.
> >
> > I should go read the patches to see what you've actually done here.
> 
> But currently there is only one kick per CPU, right? I didn't change 
> anything in this respect, just moved the code into a function and called 
> that a bit earlier.

I think we're covering this in the other subthread, so I won't repeat
here.

Ian

[PATCH v2 0/6]

[Sam Bobroff]

Hi all,

Here is v2, addressing feedback from v1.

Original cover letter follows, slightly updated for v2:

This patch set adds support for EEH recovery of hot plugged devices on pSeries
machines. Specifically, devices discovered by PCI rescanning using
/sys/bus/pci/rescan, which includes devices hotplugged by QEMU's device_add
command. (Upstream Linux pSeries guests running under QEMU/KVM don't currently
use slot power control for hotplugging.)

As a side effect this also provides EEH support for devices removed by
/sys/bus/pci/devices/*/remove and re-discovered by writing to /sys/bus/pci/rescan,
on all platforms.

The approach I've taken is to use the fact that the existing
pcibios_bus_add_device() platform hooks (which are used to set up EEH on
Virtual Function devices (VFs)) are actually called for all devices, so I've
widened their scope and made other adjustments necessary to allow them to work
for hotplugged and boot-time devices as well.

Because some of the changes are in generic PowerPC code, it's
possible that I've disturbed something for another PowerPC platform. I've tried
to minimize this by leaving that code alone as much as possible and so there
are a few cases where eeh_add_device_{early,late}() or eeh_add_sysfs_files() is
called more than once. I think these can be looked at later, as duplicate calls
are not harmful.

The first patch is a rework of the pcibios_init reordering patch I posted
earlier, which I've included here because it's necessary for this set.

I have done some testing for PowerNV on Power9 using a modified pnv_php module
and some testing on pSeries with slot power control using a modified rpaphp
module, and the EEH-related parts seem to work.

Cheers,
Sam.

Patch set changelog follows:

Patch set v2: 
Patch 1/6: powerpc/64: Adjust order in pcibios_init()
Patch 2/6: powerpc/eeh: Clear stale EEH_DEV_NO_HANDLER flag
* Also clear EEH_DEV_NO_HANDLER in eeh_handle_special_event().
Patch 3/6 (was 4/8): powerpc/eeh: Improve debug messages around device addition
Patch 4/6 (was 6/8): powerpc/eeh: Initialize EEH address cache earlier
Patch 5/6 (was 3/8 and 7/8): powerpc/eeh: EEH for pSeries hot plug
- Dropped changes to the PowerNV PHB EEH flag, instead refactor just enough to
  use the existing flag from multiple places.
- Merge the little remaining work from the above change into the patch where
  it's used.
Patch 6/6 (was 5/8 and 8/8): powerpc/eeh: Refactor around eeh_probe_devices()
- As it's so small, merged the enablement message patch into this one (where it's used).
- Reworked enablement messages.

Patch set v1:
Patch 1/8: powerpc/64: Adjust order in pcibios_init()
Patch 2/8: powerpc/eeh: Clear stale EEH_DEV_NO_HANDLER flag
Patch 3/8: powerpc/eeh: Convert PNV_PHB_FLAG_EEH to global flag
Patch 4/8: powerpc/eeh: Improve debug messages around device addition
Patch 5/8: powerpc/eeh: Add eeh_show_enabled()
Patch 6/8: powerpc/eeh: Initialize EEH address cache earlier
Patch 7/8: powerpc/eeh: EEH for pSeries hot plug
Patch 8/8: powerpc/eeh: Remove eeh_probe_devices() and eeh_addr_cache_build()

Sam Bobroff (6):
  powerpc/64: Adjust order in pcibios_init()
  powerpc/eeh: Clear stale EEH_DEV_NO_HANDLER flag
  powerpc/eeh: Improve debug messages around device addition
  powerpc/eeh: Initialize EEH address cache earlier
  powerpc/eeh: EEH for pSeries hot plug
  powerpc/eeh: Refactor around eeh_probe_devices()

 arch/powerpc/include/asm/eeh.h               |  8 +--
 arch/powerpc/kernel/eeh.c                    | 33 ++++-----
 arch/powerpc/kernel/eeh_cache.c              | 29 +-------
 arch/powerpc/kernel/eeh_driver.c             | 11 ++-
 arch/powerpc/kernel/of_platform.c            |  3 +-
 arch/powerpc/kernel/pci-common.c             |  4 --
 arch/powerpc/kernel/pci_32.c                 |  4 ++
 arch/powerpc/kernel/pci_64.c                 | 12 +++-
 arch/powerpc/platforms/powernv/eeh-powernv.c | 57 ++++++++++-----
 arch/powerpc/platforms/pseries/eeh_pseries.c | 75 +++++++++++---------
 arch/powerpc/platforms/pseries/pci.c         |  3 +-
 11 files changed, 127 insertions(+), 112 deletions(-)

-- 
2.19.0.2.gcad72f5712

[PATCH v2 1/6] powerpc/64: Adjust order in pcibios_init()

[Sam Bobroff]

The pcibios_init() function for 64 bit PowerPC currently calls
pci_bus_add_devices() before pcibios_resource_survey(), which seems
incorrect because it adds devices and attempts to bind their drivers
before allocating their resources (although no problems seem to be
apparent).

So move the call to pci_bus_add_devices() to after
pcibios_resource_survey(), while extracting call to the
pcibios_fixup() hook so that it remains in the same location.

This will also allow the ppc_md.pcibios_bus_add_device() hooks to
perform actions that depend on PCI resources, both during rescanning
(where this is already the case) and at boot time, to support future
work.

Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 arch/powerpc/kernel/pci-common.c |  4 ----
 arch/powerpc/kernel/pci_32.c     |  4 ++++
 arch/powerpc/kernel/pci_64.c     | 12 +++++++++---
 3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index ff4b7539cbdf..3146eb73e3b3 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1383,10 +1383,6 @@ void __init pcibios_resource_survey(void)
 		pr_debug("PCI: Assigning unassigned resources...\n");
 		pci_assign_unassigned_resources();
 	}
-
-	/* Call machine dependent fixup */
-	if (ppc_md.pcibios_fixup)
-		ppc_md.pcibios_fixup();
 }
 
 /* This is used by the PCI hotplug driver to allocate resource
diff --git a/arch/powerpc/kernel/pci_32.c b/arch/powerpc/kernel/pci_32.c
index 0417fda13636..7078122e9cea 100644
--- a/arch/powerpc/kernel/pci_32.c
+++ b/arch/powerpc/kernel/pci_32.c
@@ -262,6 +262,10 @@ static int __init pcibios_init(void)
 	/* Call common code to handle resource allocation */
 	pcibios_resource_survey();
 
+	/* Call machine dependent fixup */
+	if (ppc_md.pcibios_fixup)
+		ppc_md.pcibios_fixup();
+
 	/* Call machine dependent post-init code */
 	if (ppc_md.pcibios_after_init)
 		ppc_md.pcibios_after_init();
diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
index 9d8c10d55407..6f16f30031d7 100644
--- a/arch/powerpc/kernel/pci_64.c
+++ b/arch/powerpc/kernel/pci_64.c
@@ -58,14 +58,20 @@ static int __init pcibios_init(void)
 	pci_add_flags(PCI_ENABLE_PROC_DOMAINS | PCI_COMPAT_DOMAIN_0);
 
 	/* Scan all of the recorded PCI controllers.  */
-	list_for_each_entry_safe(hose, tmp, &hose_list, list_node) {
+	list_for_each_entry_safe(hose, tmp, &hose_list, list_node)
 		pcibios_scan_phb(hose);
-		pci_bus_add_devices(hose->bus);
-	}
 
 	/* Call common code to handle resource allocation */
 	pcibios_resource_survey();
 
+	/* Add devices. */
+	list_for_each_entry_safe(hose, tmp, &hose_list, list_node)
+		pci_bus_add_devices(hose->bus);
+
+	/* Call machine dependent fixup */
+	if (ppc_md.pcibios_fixup)
+		ppc_md.pcibios_fixup();
+
 	printk(KERN_DEBUG "PCI: Probing PCI hardware done\n");
 
 	return 0;
-- 
2.19.0.2.gcad72f5712

[PATCH v2 2/6] powerpc/eeh: Clear stale EEH_DEV_NO_HANDLER flag

[Sam Bobroff]

The EEH_DEV_NO_HANDLER flag is used by the EEH system to prevent the
use of driver callbacks in drivers that have been bound part way
through the recovery process. This is necessary to prevent later stage
handlers from being called when the earlier stage handlers haven't,
which can be confusing for drivers.

However, the flag is set for all devices that are added after boot
time and only cleared at the end of the EEH recovery process. This
results in hot plugged devices erroneously having the flag set during
the first recovery after they are added (causing their driver's
handlers to be incorrectly ignored).

To remedy this, clear the flag at the beginning of recovery
processing. The flag is still cleared at the end of recovery
processing, although it is no longer really necessary.

Also clear the flag during eeh_handle_special_event(), for the same
reasons.

Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
v2 * Also clear EEH_DEV_NO_HANDLER in eeh_handle_special_event().

 arch/powerpc/kernel/eeh_driver.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 6f3ee30565dd..d6f54840a3a9 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -819,6 +819,10 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
 		result = PCI_ERS_RESULT_DISCONNECT;
 	}
 
+	eeh_for_each_pe(pe, tmp_pe)
+		eeh_pe_for_each_dev(tmp_pe, edev, tmp)
+			edev->mode &= ~EEH_DEV_NO_HANDLER;
+
 	/* Walk the various device drivers attached to this slot through
 	 * a reset sequence, giving each an opportunity to do what it needs
 	 * to accomplish the reset.  Each child gets a report of the
@@ -1009,7 +1013,8 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
  */
 void eeh_handle_special_event(void)
 {
-	struct eeh_pe *pe, *phb_pe;
+	struct eeh_pe *pe, *phb_pe, *tmp_pe;
+	struct eeh_dev *edev, *tmp_edev;
 	struct pci_bus *bus;
 	struct pci_controller *hose;
 	unsigned long flags;
@@ -1078,6 +1083,10 @@ void eeh_handle_special_event(void)
 				    (phb_pe->state & EEH_PE_RECOVERING))
 					continue;
 
+				eeh_for_each_pe(pe, tmp_pe)
+					eeh_pe_for_each_dev(tmp_pe, edev, tmp_edev)
+						edev->mode &= ~EEH_DEV_NO_HANDLER;
+
 				/* Notify all devices to be down */
 				eeh_pe_state_clear(pe, EEH_PE_PRI_BUS, true);
 				eeh_set_channel_state(pe, pci_channel_io_perm_failure);
-- 
2.19.0.2.gcad72f5712

[PATCH v2 3/6] powerpc/eeh: Improve debug messages around device addition

[Sam Bobroff]

Also remove useless comment.

Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 arch/powerpc/kernel/eeh.c                    |  2 +-
 arch/powerpc/platforms/powernv/eeh-powernv.c | 14 ++++++++----
 arch/powerpc/platforms/pseries/eeh_pseries.c | 23 +++++++++++++++-----
 3 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 8d3c36a1f194..b14d89547895 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -1291,7 +1291,7 @@ void eeh_add_device_late(struct pci_dev *dev)
 	pdn = pci_get_pdn_by_devfn(dev->bus, dev->devfn);
 	edev = pdn_to_eeh_dev(pdn);
 	if (edev->pdev == dev) {
-		pr_debug("EEH: Already referenced !\n");
+		pr_debug("EEH: Device %s already referenced!\n", pci_name(dev));
 		return;
 	}
 
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index 6fc1a463b796..0e374cdba961 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -50,10 +50,7 @@ void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
 	if (!pdev->is_virtfn)
 		return;
 
-	/*
-	 * The following operations will fail if VF's sysfs files
-	 * aren't created or its resources aren't finalized.
-	 */
+	pr_debug("%s: EEH: Setting up device %s.\n", __func__, pci_name(pdev));
 	eeh_add_device_early(pdn);
 	eeh_add_device_late(pdev);
 	eeh_sysfs_add_device(pdev);
@@ -397,6 +394,10 @@ static void *pnv_eeh_probe(struct pci_dn *pdn, void *data)
 	int ret;
 	int config_addr = (pdn->busno << 8) | (pdn->devfn);
 
+	pr_debug("%s: probing %04x:%02x:%02x.%01x\n",
+		__func__, hose->global_number, pdn->busno,
+		PCI_SLOT(pdn->devfn), PCI_FUNC(pdn->devfn));
+
 	/*
 	 * When probing the root bridge, which doesn't have any
 	 * subordinate PCI devices. We don't have OF node for
@@ -491,6 +492,11 @@ static void *pnv_eeh_probe(struct pci_dn *pdn, void *data)
 	/* Save memory bars */
 	eeh_save_bars(edev);
 
+	pr_debug("%s: EEH enabled on %02x:%02x.%01x PHB#%x-PE#%x\n",
+		__func__, pdn->busno, PCI_SLOT(pdn->devfn),
+		PCI_FUNC(pdn->devfn), edev->pe->phb->global_number,
+		edev->pe->addr);
+
 	return NULL;
 }
 
diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
index 7aa50258dd42..ae06878fbdea 100644
--- a/arch/powerpc/platforms/pseries/eeh_pseries.c
+++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
@@ -65,6 +65,8 @@ void pseries_pcibios_bus_add_device(struct pci_dev *pdev)
 	if (!pdev->is_virtfn)
 		return;
 
+	pr_debug("%s: EEH: Setting up device %s.\n", __func__, pci_name(pdev));
+
 	pdn->device_id  =  pdev->device;
 	pdn->vendor_id  =  pdev->vendor;
 	pdn->class_code =  pdev->class;
@@ -251,6 +253,10 @@ static void *pseries_eeh_probe(struct pci_dn *pdn, void *data)
 	int enable = 0;
 	int ret;
 
+	pr_debug("%s: probing %04x:%02x:%02x.%01x\n",
+		__func__, pdn->phb->global_number, pdn->busno,
+		PCI_SLOT(pdn->devfn), PCI_FUNC(pdn->devfn));
+
 	/* Retrieve OF node and eeh device */
 	edev = pdn_to_eeh_dev(pdn);
 	if (!edev || edev->pe)
@@ -294,7 +300,12 @@ static void *pseries_eeh_probe(struct pci_dn *pdn, void *data)
 
 	/* Enable EEH on the device */
 	ret = eeh_ops->set_option(&pe, EEH_OPT_ENABLE);
-	if (!ret) {
+	if (ret) {
+		pr_debug("%s: EEH failed to enable on %02x:%02x.%01x PHB#%x-PE#%x (code %d)\n",
+			__func__, pdn->busno, PCI_SLOT(pdn->devfn),
+			PCI_FUNC(pdn->devfn), pe.phb->global_number,
+			pe.addr, ret);
+	} else {
 		/* Retrieve PE address */
 		edev->pe_config_addr = eeh_ops->get_pe_addr(&pe);
 		pe.addr = edev->pe_config_addr;
@@ -310,11 +321,6 @@ static void *pseries_eeh_probe(struct pci_dn *pdn, void *data)
 		if (enable) {
 			eeh_add_flag(EEH_ENABLED);
 			eeh_add_to_parent_pe(edev);
-
-			pr_debug("%s: EEH enabled on %02x:%02x.%01x PHB#%x-PE#%x\n",
-				__func__, pdn->busno, PCI_SLOT(pdn->devfn),
-				PCI_FUNC(pdn->devfn), pe.phb->global_number,
-				pe.addr);
 		} else if (pdn->parent && pdn_to_eeh_dev(pdn->parent) &&
 			   (pdn_to_eeh_dev(pdn->parent))->pe) {
 			/* This device doesn't support EEH, but it may have an
@@ -323,6 +329,11 @@ static void *pseries_eeh_probe(struct pci_dn *pdn, void *data)
 			edev->pe_config_addr = pdn_to_eeh_dev(pdn->parent)->pe_config_addr;
 			eeh_add_to_parent_pe(edev);
 		}
+		pr_debug("%s: EEH %s on %02x:%02x.%01x PHB#%x-PE#%x (code %d)\n",
+			__func__, (enable ? "enabled" : "unsupported"),
+			pdn->busno, PCI_SLOT(pdn->devfn),
+			PCI_FUNC(pdn->devfn), pe.phb->global_number,
+			pe.addr, ret);
 	}
 
 	/* Save memory bars */
-- 
2.19.0.2.gcad72f5712

[PATCH v2 4/6] powerpc/eeh: Initialize EEH address cache earlier

[Sam Bobroff]

The EEH address cache is currently initialized and populated by a
single function: eeh_addr_cache_build().  While the initial population
of the cache can only be done once resources are allocated,
initialization (just setting up a spinlock) could be done much
earlier.

So move the initialization step into a separate function and call it
from a core_initcall (rather than a subsys initcall).

This will allow future work to make use of the cache during boot time
PCI scanning.

Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 arch/powerpc/include/asm/eeh.h  |  3 +++
 arch/powerpc/kernel/eeh.c       |  2 ++
 arch/powerpc/kernel/eeh_cache.c | 13 +++++++++++--
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 3613a56281f2..12baf1df134c 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -288,6 +288,7 @@ int __init eeh_ops_register(struct eeh_ops *ops);
 int __exit eeh_ops_unregister(const char *name);
 int eeh_check_failure(const volatile void __iomem *token);
 int eeh_dev_check_failure(struct eeh_dev *edev);
+void eeh_addr_cache_init(void);
 void eeh_addr_cache_build(void);
 void eeh_add_device_early(struct pci_dn *);
 void eeh_add_device_tree_early(struct pci_dn *);
@@ -348,6 +349,8 @@ static inline int eeh_check_failure(const volatile void __iomem *token)
 
 #define eeh_dev_check_failure(x) (0)
 
+static inline void eeh_addr_cache_init(void) { }
+
 static inline void eeh_addr_cache_build(void) { }
 
 static inline void eeh_add_device_early(struct pci_dn *pdn) { }
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index b14d89547895..4160514d997c 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -1213,6 +1213,8 @@ static int eeh_init(void)
 	list_for_each_entry_safe(hose, tmp, &hose_list, list_node)
 		eeh_dev_phb_init_dynamic(hose);
 
+	eeh_addr_cache_init();
+
 	/* Initialize EEH event */
 	return eeh_event_init();
 }
diff --git a/arch/powerpc/kernel/eeh_cache.c b/arch/powerpc/kernel/eeh_cache.c
index 9c68f0837385..f93dd5cf6a39 100644
--- a/arch/powerpc/kernel/eeh_cache.c
+++ b/arch/powerpc/kernel/eeh_cache.c
@@ -267,6 +267,17 @@ void eeh_addr_cache_rmv_dev(struct pci_dev *dev)
 	spin_unlock_irqrestore(&pci_io_addr_cache_root.piar_lock, flags);
 }
 
+/**
+ * eeh_addr_cache_init - Initialize a cache of I/O addresses
+ *
+ * Initialize a cache of pci i/o addresses.  This cache will be used to
+ * find the pci device that corresponds to a given address.
+ */
+void eeh_addr_cache_init(void)
+{
+	spin_lock_init(&pci_io_addr_cache_root.piar_lock);
+}
+
 /**
  * eeh_addr_cache_build - Build a cache of I/O addresses
  *
@@ -282,8 +293,6 @@ void eeh_addr_cache_build(void)
 	struct eeh_dev *edev;
 	struct pci_dev *dev = NULL;
 
-	spin_lock_init(&pci_io_addr_cache_root.piar_lock);
-
 	for_each_pci_dev(dev) {
 		pdn = pci_get_pdn_by_devfn(dev->bus, dev->devfn);
 		if (!pdn)
-- 
2.19.0.2.gcad72f5712

[PATCH v2 5/6] powerpc/eeh: EEH for pSeries hot plug

[Sam Bobroff]

On PowerNV and pSeries, devices currently acquire EEH support from
several different places: Boot-time devices from eeh_probe_devices()
and eeh_addr_cache_build(), Virtual Function devices from the pcibios
bus add device hooks and hot plugged devices from pci_hp_add_devices()
(with other platforms using other methods as well).  Unfortunately,
pSeries machines currently discover hot plugged devices using
pci_rescan_bus(), not pci_hp_add_devices(), and so those devices do
not receive EEH support.

Rather than adding another case for pci_rescan_bus(), this change
widens the scope of the pcibios bus add device hooks so that they can
handle all devices. As a side effect this also supports devices
discovered after manually rescanning via /sys/bus/pci/rescan.

Note that on PowerNV, this change allows the EEH subsystem to become
enabled after boot as long as it has not been forced off, which was
not previously possible (it was already possible on pSeries).

Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
v2 - Dropped changes to the PowerNV PHB EEH flag, instead refactor just enough to
     use the existing flag from multiple places.
   - Merge the little remaining work from the above change into the patch where
     it's used.

 arch/powerpc/kernel/eeh.c                    |  2 +-
 arch/powerpc/kernel/of_platform.c            |  3 +-
 arch/powerpc/platforms/powernv/eeh-powernv.c | 39 +++++++++-----
 arch/powerpc/platforms/pseries/eeh_pseries.c | 54 ++++++++++----------
 4 files changed, 56 insertions(+), 42 deletions(-)

diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 4160514d997c..1ed80adb40a1 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -1285,7 +1285,7 @@ void eeh_add_device_late(struct pci_dev *dev)
 	struct pci_dn *pdn;
 	struct eeh_dev *edev;
 
-	if (!dev || !eeh_enabled())
+	if (!dev)
 		return;
 
 	pr_debug("EEH: Adding device %s\n", pci_name(dev));
diff --git a/arch/powerpc/kernel/of_platform.c b/arch/powerpc/kernel/of_platform.c
index becaec990140..d5818e9c4069 100644
--- a/arch/powerpc/kernel/of_platform.c
+++ b/arch/powerpc/kernel/of_platform.c
@@ -86,7 +86,8 @@ static int of_pci_phb_probe(struct platform_device *dev)
 	pcibios_claim_one_bus(phb->bus);
 
 	/* Finish EEH setup */
-	eeh_add_device_tree_late(phb->bus);
+	if (!eeh_has_flag(EEH_FORCE_DISABLED))
+		eeh_add_device_tree_late(phb->bus);
 
 	/* Add probed PCI devices to the device model */
 	pci_bus_add_devices(phb->bus);
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index 0e374cdba961..90729d908a54 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -47,7 +47,7 @@ void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
 {
 	struct pci_dn *pdn = pci_get_pdn(pdev);
 
-	if (!pdev->is_virtfn)
+	if (eeh_has_flag(EEH_FORCE_DISABLED))
 		return;
 
 	pr_debug("%s: EEH: Setting up device %s.\n", __func__, pci_name(pdev));
@@ -226,6 +226,25 @@ static const struct file_operations eeh_tree_state_debugfs_ops = {
 
 #endif /* CONFIG_DEBUG_FS */
 
+void pnv_eeh_enable_phbs(void)
+{
+	struct pci_controller *hose;
+	struct pnv_phb *phb;
+
+	list_for_each_entry(hose, &hose_list, list_node) {
+		phb = hose->private_data;
+		/*
+		 * If EEH is enabled, we're going to rely on that.
+		 * Otherwise, we restore to conventional mechanism
+		 * to clear frozen PE during PCI config access.
+		 */
+		if (eeh_enabled())
+			phb->flags |= PNV_PHB_FLAG_EEH;
+		else
+			phb->flags &= ~PNV_PHB_FLAG_EEH;
+	}
+}
+
 /**
  * pnv_eeh_post_init - EEH platform dependent post initialization
  *
@@ -264,19 +283,11 @@ int pnv_eeh_post_init(void)
 	if (!eeh_enabled())
 		disable_irq(eeh_event_irq);
 
+	pnv_eeh_enable_phbs();
+
 	list_for_each_entry(hose, &hose_list, list_node) {
 		phb = hose->private_data;
 
-		/*
-		 * If EEH is enabled, we're going to rely on that.
-		 * Otherwise, we restore to conventional mechanism
-		 * to clear frozen PE during PCI config access.
-		 */
-		if (eeh_enabled())
-			phb->flags |= PNV_PHB_FLAG_EEH;
-		else
-			phb->flags &= ~PNV_PHB_FLAG_EEH;
-
 		/* Create debugfs entries */
 #ifdef CONFIG_DEBUG_FS
 		if (phb->has_dbgfs || !phb->dbgfs)
@@ -487,7 +498,11 @@ static void *pnv_eeh_probe(struct pci_dn *pdn, void *data)
 	 * Enable EEH explicitly so that we will do EEH check
 	 * while accessing I/O stuff
 	 */
-	eeh_add_flag(EEH_ENABLED);
+	if (!eeh_has_flag(EEH_ENABLED)) {
+		enable_irq(eeh_event_irq);
+		pnv_eeh_enable_phbs();
+		eeh_add_flag(EEH_ENABLED);
+	}
 
 	/* Save memory bars */
 	eeh_save_bars(edev);
diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
index ae06878fbdea..e68c79164974 100644
--- a/arch/powerpc/platforms/pseries/eeh_pseries.c
+++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
@@ -55,44 +55,44 @@ static int ibm_get_config_addr_info;
 static int ibm_get_config_addr_info2;
 static int ibm_configure_pe;
 
-#ifdef CONFIG_PCI_IOV
 void pseries_pcibios_bus_add_device(struct pci_dev *pdev)
 {
 	struct pci_dn *pdn = pci_get_pdn(pdev);
-	struct pci_dn *physfn_pdn;
-	struct eeh_dev *edev;
 
-	if (!pdev->is_virtfn)
+	if (eeh_has_flag(EEH_FORCE_DISABLED))
 		return;
 
 	pr_debug("%s: EEH: Setting up device %s.\n", __func__, pci_name(pdev));
+#ifdef CONFIG_PCI_IOV
+	if (pdev->is_virtfn) {
+		struct pci_dn *physfn_pdn;
 
-	pdn->device_id  =  pdev->device;
-	pdn->vendor_id  =  pdev->vendor;
-	pdn->class_code =  pdev->class;
-	/*
-	 * Last allow unfreeze return code used for retrieval
-	 * by user space in eeh-sysfs to show the last command
-	 * completion from platform.
-	 */
-	pdn->last_allow_rc =  0;
-	physfn_pdn      =  pci_get_pdn(pdev->physfn);
-	pdn->pe_number  =  physfn_pdn->pe_num_map[pdn->vf_index];
-	edev = pdn_to_eeh_dev(pdn);
-
-	/*
-	 * The following operations will fail if VF's sysfs files
-	 * aren't created or its resources aren't finalized.
-	 */
+		pdn->device_id  =  pdev->device;
+		pdn->vendor_id  =  pdev->vendor;
+		pdn->class_code =  pdev->class;
+		/*
+		 * Last allow unfreeze return code used for retrieval
+		 * by user space in eeh-sysfs to show the last command
+		 * completion from platform.
+		 */
+		pdn->last_allow_rc =  0;
+		physfn_pdn      =  pci_get_pdn(pdev->physfn);
+		pdn->pe_number  =  physfn_pdn->pe_num_map[pdn->vf_index];
+	}
+#endif
 	eeh_add_device_early(pdn);
 	eeh_add_device_late(pdev);
-	edev->pe_config_addr =  (pdn->busno << 16) | (pdn->devfn << 8);
-	eeh_rmv_from_parent_pe(edev); /* Remove as it is adding to bus pe */
-	eeh_add_to_parent_pe(edev);   /* Add as VF PE type */
-	eeh_sysfs_add_device(pdev);
+#ifdef CONFIG_PCI_IOV
+	if (pdev->is_virtfn) {
+		struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
 
-}
+		edev->pe_config_addr =  (pdn->busno << 16) | (pdn->devfn << 8);
+		eeh_rmv_from_parent_pe(edev); /* Remove as it is adding to bus pe */
+		eeh_add_to_parent_pe(edev);   /* Add as VF PE type */
+	}
 #endif
+	eeh_sysfs_add_device(pdev);
+}
 
 /*
  * Buffer for reporting slot-error-detail rtas calls. Its here
@@ -159,10 +159,8 @@ static int pseries_eeh_init(void)
 	/* Set EEH probe mode */
 	eeh_add_flag(EEH_PROBE_MODE_DEVTREE | EEH_ENABLE_IO_FOR_LOG);
 
-#ifdef CONFIG_PCI_IOV
 	/* Set EEH machine dependent code */
 	ppc_md.pcibios_bus_add_device = pseries_pcibios_bus_add_device;
-#endif
 
 	return 0;
 }
-- 
2.19.0.2.gcad72f5712

[PATCH v2 6/6] powerpc/eeh: Refactor around eeh_probe_devices()

[Sam Bobroff]

Now that EEH support for all devices (on PowerNV and pSeries) is
provided by the pcibios bus add device hooks, eeh_probe_devices() and
eeh_addr_cache_build() are redundant and can be removed.

Move the EEH enabled message into it's own function so that it can be
called from multiple places.

Note that previously on pSeries, useless EEH sysfs files were created
for some devices that did not have EEH support and this change
prevents them from being created.

Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
v2 - As it's so small, merged the enablement message patch into this one (where it's used).
   - Reworked enablement messages.

 arch/powerpc/include/asm/eeh.h               |  7 ++---
 arch/powerpc/kernel/eeh.c                    | 27 ++++++-----------
 arch/powerpc/kernel/eeh_cache.c              | 32 --------------------
 arch/powerpc/platforms/powernv/eeh-powernv.c |  4 +--
 arch/powerpc/platforms/pseries/pci.c         |  3 +-
 5 files changed, 14 insertions(+), 59 deletions(-)

diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 12baf1df134c..3994d45ae0d4 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -283,13 +283,12 @@ struct pci_bus *eeh_pe_bus_get(struct eeh_pe *pe);
 
 struct eeh_dev *eeh_dev_init(struct pci_dn *pdn);
 void eeh_dev_phb_init_dynamic(struct pci_controller *phb);
-void eeh_probe_devices(void);
+void eeh_show_enabled(void);
 int __init eeh_ops_register(struct eeh_ops *ops);
 int __exit eeh_ops_unregister(const char *name);
 int eeh_check_failure(const volatile void __iomem *token);
 int eeh_dev_check_failure(struct eeh_dev *edev);
 void eeh_addr_cache_init(void);
-void eeh_addr_cache_build(void);
 void eeh_add_device_early(struct pci_dn *);
 void eeh_add_device_tree_early(struct pci_dn *);
 void eeh_add_device_late(struct pci_dev *);
@@ -333,7 +332,7 @@ static inline bool eeh_enabled(void)
         return false;
 }
 
-static inline void eeh_probe_devices(void) { }
+static inline void eeh_show_enabled(void) { }
 
 static inline void *eeh_dev_init(struct pci_dn *pdn, void *data)
 {
@@ -351,8 +350,6 @@ static inline int eeh_check_failure(const volatile void __iomem *token)
 
 static inline void eeh_addr_cache_init(void) { }
 
-static inline void eeh_addr_cache_build(void) { }
-
 static inline void eeh_add_device_early(struct pci_dn *pdn) { }
 
 static inline void eeh_add_device_tree_early(struct pci_dn *pdn) { }
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 1ed80adb40a1..f905235f0307 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -163,6 +163,16 @@ static int __init eeh_setup(char *str)
 }
 __setup("eeh=", eeh_setup);
 
+void eeh_show_enabled(void)
+{
+	if (eeh_has_flag(EEH_FORCE_DISABLED))
+		pr_info("EEH: Recovery disabled by kernel parameter.\n");
+	else if (eeh_has_flag(EEH_ENABLED))
+		pr_info("EEH: Capable adapter found: recovery enabled.\n");
+	else
+		pr_info("EEH: No capable adapters found: recovery disabled.\n");
+}
+
 /*
  * This routine captures assorted PCI configuration space data
  * for the indicated PCI device, and puts them into a buffer
@@ -1156,23 +1166,6 @@ static struct notifier_block eeh_reboot_nb = {
 	.notifier_call = eeh_reboot_notifier,
 };
 
-void eeh_probe_devices(void)
-{
-	struct pci_controller *hose, *tmp;
-	struct pci_dn *pdn;
-
-	/* Enable EEH for all adapters */
-	list_for_each_entry_safe(hose, tmp, &hose_list, list_node) {
-		pdn = hose->pci_data;
-		traverse_pci_dn(pdn, eeh_ops->probe, NULL);
-	}
-	if (eeh_enabled())
-		pr_info("EEH: PCI Enhanced I/O Error Handling Enabled\n");
-	else
-		pr_info("EEH: No capable adapters found\n");
-
-}
-
 /**
  * eeh_init - EEH initialization
  *
diff --git a/arch/powerpc/kernel/eeh_cache.c b/arch/powerpc/kernel/eeh_cache.c
index f93dd5cf6a39..c40078d036af 100644
--- a/arch/powerpc/kernel/eeh_cache.c
+++ b/arch/powerpc/kernel/eeh_cache.c
@@ -278,38 +278,6 @@ void eeh_addr_cache_init(void)
 	spin_lock_init(&pci_io_addr_cache_root.piar_lock);
 }
 
-/**
- * eeh_addr_cache_build - Build a cache of I/O addresses
- *
- * Build a cache of pci i/o addresses.  This cache will be used to
- * find the pci device that corresponds to a given address.
- * This routine scans all pci busses to build the cache.
- * Must be run late in boot process, after the pci controllers
- * have been scanned for devices (after all device resources are known).
- */
-void eeh_addr_cache_build(void)
-{
-	struct pci_dn *pdn;
-	struct eeh_dev *edev;
-	struct pci_dev *dev = NULL;
-
-	for_each_pci_dev(dev) {
-		pdn = pci_get_pdn_by_devfn(dev->bus, dev->devfn);
-		if (!pdn)
-			continue;
-
-		edev = pdn_to_eeh_dev(pdn);
-		if (!edev)
-			continue;
-
-		dev->dev.archdata.edev = edev;
-		edev->pdev = dev;
-
-		eeh_addr_cache_insert_dev(dev);
-		eeh_sysfs_add_device(dev);
-	}
-}
-
 static int eeh_addr_cache_show(struct seq_file *s, void *v)
 {
 	struct pci_io_addr_range *piar;
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index 90729d908a54..22a94f4b8586 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -259,9 +259,7 @@ int pnv_eeh_post_init(void)
 	struct pnv_phb *phb;
 	int ret = 0;
 
-	/* Probe devices & build address cache */
-	eeh_probe_devices();
-	eeh_addr_cache_build();
+	eeh_show_enabled();
 
 	/* Register OPAL event notifier */
 	eeh_event_irq = opal_event_request(ilog2(OPAL_EVENT_PCI_ERROR));
diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c
index 37a77e57893e..d6a5f4f27507 100644
--- a/arch/powerpc/platforms/pseries/pci.c
+++ b/arch/powerpc/platforms/pseries/pci.c
@@ -242,8 +242,7 @@ void __init pSeries_final_fixup(void)
 
 	pSeries_request_regions();
 
-	eeh_probe_devices();
-	eeh_addr_cache_build();
+	eeh_show_enabled();
 
 #ifdef CONFIG_PCI_IOV
 	ppc_md.pcibios_sriov_enable = pseries_pcibios_sriov_enable;
-- 
2.19.0.2.gcad72f5712

[PATCH v2 0/6]

[vanitha.channaiah]

- Updated the patches by incorporating review comments from Takashi Iwai-san

[PATCH 1/5] pcm: direct: Add generic hw_ptr_alignment function for dmix, dshare and dsnoop
[Takashi Iwai:]
> The patch description needs rephrasing.  What actually this does is to move the code from pcm_dmix.c to pcm_direct.c
> and its header so that the helper code can be re-used by other direct-PCM plugins.

- Commit message is rephrased as suggested.
- New commit: [PATCH v2 1/6] pcm: direct: Add generic hw_ptr_alignment function for


[PATCH 2/5] pcm: dshare: Added "hw_ptr_alignment"	option in configuration for alignment of slave pointers
[Takashi Iwai:]
> Again, this patch description is too ambiguous.
> And, if it enables the hw_ptr_alignment option, update the documentation as well.

- Commit message is explained in detail for the changes done.
- Documentation updated.
- New commit: [PATCH v2 2/6] pcm: dshare: Added "hw_ptr_alignment" option in


[PATCH 3/5] pcm: dsnoop: Added "hw_ptr_alignment"	option in configuration for slave pointer alignment
[Takashi Iwai:]
> Ditto as patch 2, the description is too ambiguous, and the update of documentation is missing.
> It's not good to change the helper function semantics out of sudden, even without any description.

- Commit message is explained in detail for the changes done.
- Documentation updated.
- Divided the patch with commit ("pcm: dsnoop: Add hw_ptr_alignment option in configuration")
  into additional patch commit ("pcm: direct: Round up of slave_app_ptr pointer if buffer")
- Usecase scenario is described for the changes done in helper function.
- New commit:
    [PATCH v2 3/6] pcm: dsnoop: Added "hw_ptr_alignment" option in
    [PATCH v2 4/6] pcm: direct: Round up of slave_app_ptr pointer if buffer


[PATCH 4/5] pcm: restructuring sw params function
[Takashi Iwai:]
> I see no reason to do that.  Please describe.

- Commit message is explained in detail why reformating was done.
- New commit: [PATCH v2 5/6] pcm: restructuring sw params function


[PATCH 5/5] pcm: Update pcm->avail_min with	needed_slave_avail_min, after reading unaligned frames
[Takashi Iwai:]
> This kind of changes in the core code should be avoided as much as possible, especially if it's only relevant with the specific plugins.
> Sorry, this isn't convincing enough.  If this is a MUST, please clarify better.

- Commit message is explained in detail with the generic usecase and specific use case we came across.
- New commit: [PATCH v2 6/6] pcm: Update pcm->avail_min with needed_slave_avail_min,

Re: [PATCH v2 6/6] powerpc/eeh: Refactor around eeh_probe_devices()

[Oliver]

On Tue, May 7, 2019 at 2:30 PM Sam Bobroff <sbobroff@linux.ibm.com> wrote:
>
> Now that EEH support for all devices (on PowerNV and pSeries) is
> provided by the pcibios bus add device hooks, eeh_probe_devices() and
> eeh_addr_cache_build() are redundant and can be removed.
>
> Move the EEH enabled message into it's own function so that it can be
> called from multiple places.
>
> Note that previously on pSeries, useless EEH sysfs files were created
> for some devices that did not have EEH support and this change
> prevents them from being created.
>
> Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
> ---
> v2 - As it's so small, merged the enablement message patch into this one (where it's used).
>    - Reworked enablement messages.
>
>  arch/powerpc/include/asm/eeh.h               |  7 ++---
>  arch/powerpc/kernel/eeh.c                    | 27 ++++++-----------
>  arch/powerpc/kernel/eeh_cache.c              | 32 --------------------
>  arch/powerpc/platforms/powernv/eeh-powernv.c |  4 +--
>  arch/powerpc/platforms/pseries/pci.c         |  3 +-
>  5 files changed, 14 insertions(+), 59 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
> index 12baf1df134c..3994d45ae0d4 100644
> --- a/arch/powerpc/include/asm/eeh.h
> +++ b/arch/powerpc/include/asm/eeh.h
> @@ -283,13 +283,12 @@ struct pci_bus *eeh_pe_bus_get(struct eeh_pe *pe);
>
>  struct eeh_dev *eeh_dev_init(struct pci_dn *pdn);
>  void eeh_dev_phb_init_dynamic(struct pci_controller *phb);
> -void eeh_probe_devices(void);
> +void eeh_show_enabled(void);
>  int __init eeh_ops_register(struct eeh_ops *ops);
>  int __exit eeh_ops_unregister(const char *name);
>  int eeh_check_failure(const volatile void __iomem *token);
>  int eeh_dev_check_failure(struct eeh_dev *edev);
>  void eeh_addr_cache_init(void);
> -void eeh_addr_cache_build(void);
>  void eeh_add_device_early(struct pci_dn *);
>  void eeh_add_device_tree_early(struct pci_dn *);
>  void eeh_add_device_late(struct pci_dev *);
> @@ -333,7 +332,7 @@ static inline bool eeh_enabled(void)
>          return false;
>  }
>
> -static inline void eeh_probe_devices(void) { }
> +static inline void eeh_show_enabled(void) { }
>
>  static inline void *eeh_dev_init(struct pci_dn *pdn, void *data)
>  {
> @@ -351,8 +350,6 @@ static inline int eeh_check_failure(const volatile void __iomem *token)
>
>  static inline void eeh_addr_cache_init(void) { }
>
> -static inline void eeh_addr_cache_build(void) { }
> -
>  static inline void eeh_add_device_early(struct pci_dn *pdn) { }
>
>  static inline void eeh_add_device_tree_early(struct pci_dn *pdn) { }
> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
> index 1ed80adb40a1..f905235f0307 100644
> --- a/arch/powerpc/kernel/eeh.c
> +++ b/arch/powerpc/kernel/eeh.c
> @@ -163,6 +163,16 @@ static int __init eeh_setup(char *str)
>  }
>  __setup("eeh=", eeh_setup);
>
> +void eeh_show_enabled(void)
> +{
> +       if (eeh_has_flag(EEH_FORCE_DISABLED))
> +               pr_info("EEH: Recovery disabled by kernel parameter.\n");
> +       else if (eeh_has_flag(EEH_ENABLED))
> +               pr_info("EEH: Capable adapter found: recovery enabled.\n");
> +       else
> +               pr_info("EEH: No capable adapters found: recovery disabled.\n");
> +}
> +
>  /*
>   * This routine captures assorted PCI configuration space data
>   * for the indicated PCI device, and puts them into a buffer
> @@ -1156,23 +1166,6 @@ static struct notifier_block eeh_reboot_nb = {
>         .notifier_call = eeh_reboot_notifier,
>  };
>

> -void eeh_probe_devices(void)
> -{
> -       struct pci_controller *hose, *tmp;
> -       struct pci_dn *pdn;
> -
> -       /* Enable EEH for all adapters */
> -       list_for_each_entry_safe(hose, tmp, &hose_list, list_node) {
> -               pdn = hose->pci_data;
> -               traverse_pci_dn(pdn, eeh_ops->probe, NULL);
> -       }
> -       if (eeh_enabled())
> -               pr_info("EEH: PCI Enhanced I/O Error Handling Enabled\n");
> -       else
> -               pr_info("EEH: No capable adapters found\n");
> -
> -}

The one concern I have about this is that PAPR requires us to enable
EEH for the device before we do any config accesses. From PAPR:

R1–7.3.11.1–5. For the EEH option: If a device driver is going to
enable EEH and the platform has not defaulted
to EEH enabled, then it must do so before it does any operations with
its IOA, including any configuration
cycles or Load or Store operations.

So if we want to be strictly compatible we'd need to ensure the
set-eeh-option RTAS call happens before we read the VDID in
pci_scan_device(). The pseries eeh_probe() function does this
currently, but if we defer it until the pcibios call happens we'll
have done a pile of config accesses before then. Maybe it doesn't
matter, but we'd need to do further testing under phyp or work out
some other way to ensure it's done pre-probe.

>  /**
>   * eeh_init - EEH initialization
>   *
> diff --git a/arch/powerpc/kernel/eeh_cache.c b/arch/powerpc/kernel/eeh_cache.c
> index f93dd5cf6a39..c40078d036af 100644
> --- a/arch/powerpc/kernel/eeh_cache.c
> +++ b/arch/powerpc/kernel/eeh_cache.c
> @@ -278,38 +278,6 @@ void eeh_addr_cache_init(void)
>         spin_lock_init(&pci_io_addr_cache_root.piar_lock);
>  }
>
> -/**
> - * eeh_addr_cache_build - Build a cache of I/O addresses
> - *
> - * Build a cache of pci i/o addresses.  This cache will be used to
> - * find the pci device that corresponds to a given address.
> - * This routine scans all pci busses to build the cache.
> - * Must be run late in boot process, after the pci controllers
> - * have been scanned for devices (after all device resources are known).
> - */
> -void eeh_addr_cache_build(void)
> -{
> -       struct pci_dn *pdn;
> -       struct eeh_dev *edev;
> -       struct pci_dev *dev = NULL;
> -
> -       for_each_pci_dev(dev) {
> -               pdn = pci_get_pdn_by_devfn(dev->bus, dev->devfn);
> -               if (!pdn)
> -                       continue;
> -
> -               edev = pdn_to_eeh_dev(pdn);
> -               if (!edev)
> -                       continue;
> -
> -               dev->dev.archdata.edev = edev;
> -               edev->pdev = dev;
> -
> -               eeh_addr_cache_insert_dev(dev);
> -               eeh_sysfs_add_device(dev);
> -       }
> -}
> -
>  static int eeh_addr_cache_show(struct seq_file *s, void *v)
>  {
>         struct pci_io_addr_range *piar;
> diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
> index 90729d908a54..22a94f4b8586 100644
> --- a/arch/powerpc/platforms/powernv/eeh-powernv.c
> +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
> @@ -259,9 +259,7 @@ int pnv_eeh_post_init(void)
>         struct pnv_phb *phb;
>         int ret = 0;
>
> -       /* Probe devices & build address cache */
> -       eeh_probe_devices();
> -       eeh_addr_cache_build();
> +       eeh_show_enabled();
>
>         /* Register OPAL event notifier */
>         eeh_event_irq = opal_event_request(ilog2(OPAL_EVENT_PCI_ERROR));
> diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c
> index 37a77e57893e..d6a5f4f27507 100644
> --- a/arch/powerpc/platforms/pseries/pci.c
> +++ b/arch/powerpc/platforms/pseries/pci.c
> @@ -242,8 +242,7 @@ void __init pSeries_final_fixup(void)
>
>         pSeries_request_regions();
>
> -       eeh_probe_devices();
> -       eeh_addr_cache_build();
> +       eeh_show_enabled();
>
>  #ifdef CONFIG_PCI_IOV
>         ppc_md.pcibios_sriov_enable = pseries_pcibios_sriov_enable;
> --
> 2.19.0.2.gcad72f5712
>

Re: [PATCH v2 3/6] powerpc/eeh: Improve debug messages around device addition

[Alexey Kardashevskiy]

On 07/05/2019 14:30, Sam Bobroff wrote:
> Also remove useless comment.
> 
> Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>  arch/powerpc/kernel/eeh.c                    |  2 +-
>  arch/powerpc/platforms/powernv/eeh-powernv.c | 14 ++++++++----
>  arch/powerpc/platforms/pseries/eeh_pseries.c | 23 +++++++++++++++-----
>  3 files changed, 28 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
> index 8d3c36a1f194..b14d89547895 100644
> --- a/arch/powerpc/kernel/eeh.c
> +++ b/arch/powerpc/kernel/eeh.c
> @@ -1291,7 +1291,7 @@ void eeh_add_device_late(struct pci_dev *dev)
>  	pdn = pci_get_pdn_by_devfn(dev->bus, dev->devfn);
>  	edev = pdn_to_eeh_dev(pdn);
>  	if (edev->pdev == dev) {
> -		pr_debug("EEH: Already referenced !\n");
> +		pr_debug("EEH: Device %s already referenced!\n", pci_name(dev));
>  		return;
>  	}
>  
> diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
> index 6fc1a463b796..0e374cdba961 100644
> --- a/arch/powerpc/platforms/powernv/eeh-powernv.c
> +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
> @@ -50,10 +50,7 @@ void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
>  	if (!pdev->is_virtfn)
>  		return;
>  
> -	/*
> -	 * The following operations will fail if VF's sysfs files
> -	 * aren't created or its resources aren't finalized.
> -	 */
> +	pr_debug("%s: EEH: Setting up device %s.\n", __func__, pci_name(pdev));


dev_dbg() seems more appropriate.


>  	eeh_add_device_early(pdn);
>  	eeh_add_device_late(pdev);
>  	eeh_sysfs_add_device(pdev);
> @@ -397,6 +394,10 @@ static void *pnv_eeh_probe(struct pci_dn *pdn, void *data)
>  	int ret;
>  	int config_addr = (pdn->busno << 8) | (pdn->devfn);
>  
> +	pr_debug("%s: probing %04x:%02x:%02x.%01x\n",
> +		__func__, hose->global_number, pdn->busno,
> +		PCI_SLOT(pdn->devfn), PCI_FUNC(pdn->devfn));
> +
>  	/*
>  	 * When probing the root bridge, which doesn't have any
>  	 * subordinate PCI devices. We don't have OF node for
> @@ -491,6 +492,11 @@ static void *pnv_eeh_probe(struct pci_dn *pdn, void *data)
>  	/* Save memory bars */
>  	eeh_save_bars(edev);
>  
> +	pr_debug("%s: EEH enabled on %02x:%02x.%01x PHB#%x-PE#%x\n",
> +		__func__, pdn->busno, PCI_SLOT(pdn->devfn),
> +		PCI_FUNC(pdn->devfn), edev->pe->phb->global_number,
> +		edev->pe->addr);
> +
>  	return NULL;
>  }
>  
> diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
> index 7aa50258dd42..ae06878fbdea 100644
> --- a/arch/powerpc/platforms/pseries/eeh_pseries.c
> +++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
> @@ -65,6 +65,8 @@ void pseries_pcibios_bus_add_device(struct pci_dev *pdev)
>  	if (!pdev->is_virtfn)
>  		return;
>  
> +	pr_debug("%s: EEH: Setting up device %s.\n", __func__, pci_name(pdev));
> +
>  	pdn->device_id  =  pdev->device;
>  	pdn->vendor_id  =  pdev->vendor;
>  	pdn->class_code =  pdev->class;
> @@ -251,6 +253,10 @@ static void *pseries_eeh_probe(struct pci_dn *pdn, void *data)
>  	int enable = 0;
>  	int ret;
>  
> +	pr_debug("%s: probing %04x:%02x:%02x.%01x\n",
> +		__func__, pdn->phb->global_number, pdn->busno,
> +		PCI_SLOT(pdn->devfn), PCI_FUNC(pdn->devfn));
> +
>  	/* Retrieve OF node and eeh device */
>  	edev = pdn_to_eeh_dev(pdn);
>  	if (!edev || edev->pe)
> @@ -294,7 +300,12 @@ static void *pseries_eeh_probe(struct pci_dn *pdn, void *data)
>  
>  	/* Enable EEH on the device */
>  	ret = eeh_ops->set_option(&pe, EEH_OPT_ENABLE);
> -	if (!ret) {
> +	if (ret) {
> +		pr_debug("%s: EEH failed to enable on %02x:%02x.%01x PHB#%x-PE#%x (code %d)\n",
> +			__func__, pdn->busno, PCI_SLOT(pdn->devfn),
> +			PCI_FUNC(pdn->devfn), pe.phb->global_number,
> +			pe.addr, ret);
> +	} else {


edev!=NULL here so you could do dev_dbg(&edev->pdev->dev,...) and skip
PCI_SLOT/PCI_FUNC. Or is (edev!=NULL && edev->pdev==NULL) possible (it
could be, just asking)?


>  		/* Retrieve PE address */
>  		edev->pe_config_addr = eeh_ops->get_pe_addr(&pe);
>  		pe.addr = edev->pe_config_addr;
> @@ -310,11 +321,6 @@ static void *pseries_eeh_probe(struct pci_dn *pdn, void *data)
>  		if (enable) {
>  			eeh_add_flag(EEH_ENABLED);
>  			eeh_add_to_parent_pe(edev);
> -
> -			pr_debug("%s: EEH enabled on %02x:%02x.%01x PHB#%x-PE#%x\n",
> -				__func__, pdn->busno, PCI_SLOT(pdn->devfn),
> -				PCI_FUNC(pdn->devfn), pe.phb->global_number,
> -				pe.addr);
>  		} else if (pdn->parent && pdn_to_eeh_dev(pdn->parent) &&
>  			   (pdn_to_eeh_dev(pdn->parent))->pe) {
>  			/* This device doesn't support EEH, but it may have an
> @@ -323,6 +329,11 @@ static void *pseries_eeh_probe(struct pci_dn *pdn, void *data)
>  			edev->pe_config_addr = pdn_to_eeh_dev(pdn->parent)->pe_config_addr;
>  			eeh_add_to_parent_pe(edev);
>  		}
> +		pr_debug("%s: EEH %s on %02x:%02x.%01x PHB#%x-PE#%x (code %d)\n",
> +			__func__, (enable ? "enabled" : "unsupported"),
> +			pdn->busno, PCI_SLOT(pdn->devfn),
> +			PCI_FUNC(pdn->devfn), pe.phb->global_number,
> +			pe.addr, ret);

Same here. I understand though this one is a cut-n-paste :)


>  	}
>  
>  	/* Save memory bars */
> 

-- 
Alexey

Re: [PATCH v2 3/6] powerpc/eeh: Improve debug messages around device addition

[Sam Bobroff]

[-- Attachment #1: Type: text/plain, Size: 6225 bytes --]

On Tue, Jun 11, 2019 at 03:47:58PM +1000, Alexey Kardashevskiy wrote:
> 
> 
> On 07/05/2019 14:30, Sam Bobroff wrote:
> > Also remove useless comment.
> > 
> > Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
> > Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> > ---
> >  arch/powerpc/kernel/eeh.c                    |  2 +-
> >  arch/powerpc/platforms/powernv/eeh-powernv.c | 14 ++++++++----
> >  arch/powerpc/platforms/pseries/eeh_pseries.c | 23 +++++++++++++++-----
> >  3 files changed, 28 insertions(+), 11 deletions(-)
> > 
> > diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
> > index 8d3c36a1f194..b14d89547895 100644
> > --- a/arch/powerpc/kernel/eeh.c
> > +++ b/arch/powerpc/kernel/eeh.c
> > @@ -1291,7 +1291,7 @@ void eeh_add_device_late(struct pci_dev *dev)
> >  	pdn = pci_get_pdn_by_devfn(dev->bus, dev->devfn);
> >  	edev = pdn_to_eeh_dev(pdn);
> >  	if (edev->pdev == dev) {
> > -		pr_debug("EEH: Already referenced !\n");
> > +		pr_debug("EEH: Device %s already referenced!\n", pci_name(dev));
> >  		return;
> >  	}
> >  
> > diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
> > index 6fc1a463b796..0e374cdba961 100644
> > --- a/arch/powerpc/platforms/powernv/eeh-powernv.c
> > +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
> > @@ -50,10 +50,7 @@ void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
> >  	if (!pdev->is_virtfn)
> >  		return;
> >  
> > -	/*
> > -	 * The following operations will fail if VF's sysfs files
> > -	 * aren't created or its resources aren't finalized.
> > -	 */
> > +	pr_debug("%s: EEH: Setting up device %s.\n", __func__, pci_name(pdev));
> 
> 
> dev_dbg() seems more appropriate.

Oh! It does, or even pci_debug() :-)

I'll change it if I need to do another version, otherwise I'll clean it
up later.

> >  	eeh_add_device_early(pdn);
> >  	eeh_add_device_late(pdev);
> >  	eeh_sysfs_add_device(pdev);
> > @@ -397,6 +394,10 @@ static void *pnv_eeh_probe(struct pci_dn *pdn, void *data)
> >  	int ret;
> >  	int config_addr = (pdn->busno << 8) | (pdn->devfn);
> >  
> > +	pr_debug("%s: probing %04x:%02x:%02x.%01x\n",
> > +		__func__, hose->global_number, pdn->busno,
> > +		PCI_SLOT(pdn->devfn), PCI_FUNC(pdn->devfn));
> > +
> >  	/*
> >  	 * When probing the root bridge, which doesn't have any
> >  	 * subordinate PCI devices. We don't have OF node for
> > @@ -491,6 +492,11 @@ static void *pnv_eeh_probe(struct pci_dn *pdn, void *data)
> >  	/* Save memory bars */
> >  	eeh_save_bars(edev);
> >  
> > +	pr_debug("%s: EEH enabled on %02x:%02x.%01x PHB#%x-PE#%x\n",
> > +		__func__, pdn->busno, PCI_SLOT(pdn->devfn),
> > +		PCI_FUNC(pdn->devfn), edev->pe->phb->global_number,
> > +		edev->pe->addr);
> > +
> >  	return NULL;
> >  }
> >  
> > diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
> > index 7aa50258dd42..ae06878fbdea 100644
> > --- a/arch/powerpc/platforms/pseries/eeh_pseries.c
> > +++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
> > @@ -65,6 +65,8 @@ void pseries_pcibios_bus_add_device(struct pci_dev *pdev)
> >  	if (!pdev->is_virtfn)
> >  		return;
> >  
> > +	pr_debug("%s: EEH: Setting up device %s.\n", __func__, pci_name(pdev));
> > +
> >  	pdn->device_id  =  pdev->device;
> >  	pdn->vendor_id  =  pdev->vendor;
> >  	pdn->class_code =  pdev->class;
> > @@ -251,6 +253,10 @@ static void *pseries_eeh_probe(struct pci_dn *pdn, void *data)
> >  	int enable = 0;
> >  	int ret;
> >  
> > +	pr_debug("%s: probing %04x:%02x:%02x.%01x\n",
> > +		__func__, pdn->phb->global_number, pdn->busno,
> > +		PCI_SLOT(pdn->devfn), PCI_FUNC(pdn->devfn));
> > +
> >  	/* Retrieve OF node and eeh device */
> >  	edev = pdn_to_eeh_dev(pdn);
> >  	if (!edev || edev->pe)
> > @@ -294,7 +300,12 @@ static void *pseries_eeh_probe(struct pci_dn *pdn, void *data)
> >  
> >  	/* Enable EEH on the device */
> >  	ret = eeh_ops->set_option(&pe, EEH_OPT_ENABLE);
> > -	if (!ret) {
> > +	if (ret) {
> > +		pr_debug("%s: EEH failed to enable on %02x:%02x.%01x PHB#%x-PE#%x (code %d)\n",
> > +			__func__, pdn->busno, PCI_SLOT(pdn->devfn),
> > +			PCI_FUNC(pdn->devfn), pe.phb->global_number,
> > +			pe.addr, ret);
> > +	} else {
> 
> 
> edev!=NULL here so you could do dev_dbg(&edev->pdev->dev,...) and skip
> PCI_SLOT/PCI_FUNC. Or is (edev!=NULL && edev->pdev==NULL) possible (it
> could be, just asking)?

I can see that edev will be non-NULL here, but that pr_debug() pattern
(using the PDN information to form the PCI address) is quite common
across the EEH code, so I think rather than changing a couple of
specific cases, I should do a separate cleanup patch and introduce
something like pdn_debug(pdn, "...."). What do you think?

(I don't know exactly when edev->pdev can be NULL.)

> 
> >  		/* Retrieve PE address */
> >  		edev->pe_config_addr = eeh_ops->get_pe_addr(&pe);
> >  		pe.addr = edev->pe_config_addr;
> > @@ -310,11 +321,6 @@ static void *pseries_eeh_probe(struct pci_dn *pdn, void *data)
> >  		if (enable) {
> >  			eeh_add_flag(EEH_ENABLED);
> >  			eeh_add_to_parent_pe(edev);
> > -
> > -			pr_debug("%s: EEH enabled on %02x:%02x.%01x PHB#%x-PE#%x\n",
> > -				__func__, pdn->busno, PCI_SLOT(pdn->devfn),
> > -				PCI_FUNC(pdn->devfn), pe.phb->global_number,
> > -				pe.addr);
> >  		} else if (pdn->parent && pdn_to_eeh_dev(pdn->parent) &&
> >  			   (pdn_to_eeh_dev(pdn->parent))->pe) {
> >  			/* This device doesn't support EEH, but it may have an
> > @@ -323,6 +329,11 @@ static void *pseries_eeh_probe(struct pci_dn *pdn, void *data)
> >  			edev->pe_config_addr = pdn_to_eeh_dev(pdn->parent)->pe_config_addr;
> >  			eeh_add_to_parent_pe(edev);
> >  		}
> > +		pr_debug("%s: EEH %s on %02x:%02x.%01x PHB#%x-PE#%x (code %d)\n",
> > +			__func__, (enable ? "enabled" : "unsupported"),
> > +			pdn->busno, PCI_SLOT(pdn->devfn),
> > +			PCI_FUNC(pdn->devfn), pe.phb->global_number,
> > +			pe.addr, ret);
> 
> Same here. I understand though this one is a cut-n-paste :)
> 
> 
> >  	}
> >  
> >  	/* Save memory bars */
> > 
> 
> -- 
> Alexey

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 6/6] powerpc/eeh: Refactor around eeh_probe_devices()

[Sam Bobroff]

[-- Attachment #1: Type: text/plain, Size: 9571 bytes --]

On Wed, Jun 05, 2019 at 03:49:15PM +1000, Oliver wrote:
> On Tue, May 7, 2019 at 2:30 PM Sam Bobroff <sbobroff@linux.ibm.com> wrote:
> >
> > Now that EEH support for all devices (on PowerNV and pSeries) is
> > provided by the pcibios bus add device hooks, eeh_probe_devices() and
> > eeh_addr_cache_build() are redundant and can be removed.
> >
> > Move the EEH enabled message into it's own function so that it can be
> > called from multiple places.
> >
> > Note that previously on pSeries, useless EEH sysfs files were created
> > for some devices that did not have EEH support and this change
> > prevents them from being created.
> >
> > Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
> > ---
> > v2 - As it's so small, merged the enablement message patch into this one (where it's used).
> >    - Reworked enablement messages.
> >
> >  arch/powerpc/include/asm/eeh.h               |  7 ++---
> >  arch/powerpc/kernel/eeh.c                    | 27 ++++++-----------
> >  arch/powerpc/kernel/eeh_cache.c              | 32 --------------------
> >  arch/powerpc/platforms/powernv/eeh-powernv.c |  4 +--
> >  arch/powerpc/platforms/pseries/pci.c         |  3 +-
> >  5 files changed, 14 insertions(+), 59 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
> > index 12baf1df134c..3994d45ae0d4 100644
> > --- a/arch/powerpc/include/asm/eeh.h
> > +++ b/arch/powerpc/include/asm/eeh.h
> > @@ -283,13 +283,12 @@ struct pci_bus *eeh_pe_bus_get(struct eeh_pe *pe);
> >
> >  struct eeh_dev *eeh_dev_init(struct pci_dn *pdn);
> >  void eeh_dev_phb_init_dynamic(struct pci_controller *phb);
> > -void eeh_probe_devices(void);
> > +void eeh_show_enabled(void);
> >  int __init eeh_ops_register(struct eeh_ops *ops);
> >  int __exit eeh_ops_unregister(const char *name);
> >  int eeh_check_failure(const volatile void __iomem *token);
> >  int eeh_dev_check_failure(struct eeh_dev *edev);
> >  void eeh_addr_cache_init(void);
> > -void eeh_addr_cache_build(void);
> >  void eeh_add_device_early(struct pci_dn *);
> >  void eeh_add_device_tree_early(struct pci_dn *);
> >  void eeh_add_device_late(struct pci_dev *);
> > @@ -333,7 +332,7 @@ static inline bool eeh_enabled(void)
> >          return false;
> >  }
> >
> > -static inline void eeh_probe_devices(void) { }
> > +static inline void eeh_show_enabled(void) { }
> >
> >  static inline void *eeh_dev_init(struct pci_dn *pdn, void *data)
> >  {
> > @@ -351,8 +350,6 @@ static inline int eeh_check_failure(const volatile void __iomem *token)
> >
> >  static inline void eeh_addr_cache_init(void) { }
> >
> > -static inline void eeh_addr_cache_build(void) { }
> > -
> >  static inline void eeh_add_device_early(struct pci_dn *pdn) { }
> >
> >  static inline void eeh_add_device_tree_early(struct pci_dn *pdn) { }
> > diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
> > index 1ed80adb40a1..f905235f0307 100644
> > --- a/arch/powerpc/kernel/eeh.c
> > +++ b/arch/powerpc/kernel/eeh.c
> > @@ -163,6 +163,16 @@ static int __init eeh_setup(char *str)
> >  }
> >  __setup("eeh=", eeh_setup);
> >
> > +void eeh_show_enabled(void)
> > +{
> > +       if (eeh_has_flag(EEH_FORCE_DISABLED))
> > +               pr_info("EEH: Recovery disabled by kernel parameter.\n");
> > +       else if (eeh_has_flag(EEH_ENABLED))
> > +               pr_info("EEH: Capable adapter found: recovery enabled.\n");
> > +       else
> > +               pr_info("EEH: No capable adapters found: recovery disabled.\n");
> > +}
> > +
> >  /*
> >   * This routine captures assorted PCI configuration space data
> >   * for the indicated PCI device, and puts them into a buffer
> > @@ -1156,23 +1166,6 @@ static struct notifier_block eeh_reboot_nb = {
> >         .notifier_call = eeh_reboot_notifier,
> >  };
> >
> 
> > -void eeh_probe_devices(void)
> > -{
> > -       struct pci_controller *hose, *tmp;
> > -       struct pci_dn *pdn;
> > -
> > -       /* Enable EEH for all adapters */
> > -       list_for_each_entry_safe(hose, tmp, &hose_list, list_node) {
> > -               pdn = hose->pci_data;
> > -               traverse_pci_dn(pdn, eeh_ops->probe, NULL);
> > -       }
> > -       if (eeh_enabled())
> > -               pr_info("EEH: PCI Enhanced I/O Error Handling Enabled\n");
> > -       else
> > -               pr_info("EEH: No capable adapters found\n");
> > -
> > -}
> 
> The one concern I have about this is that PAPR requires us to enable
> EEH for the device before we do any config accesses. From PAPR:
> 
> R1–7.3.11.1–5. For the EEH option: If a device driver is going to
> enable EEH and the platform has not defaulted
> to EEH enabled, then it must do so before it does any operations with
> its IOA, including any configuration
> cycles or Load or Store operations.
> 
> So if we want to be strictly compatible we'd need to ensure the
> set-eeh-option RTAS call happens before we read the VDID in
> pci_scan_device(). The pseries eeh_probe() function does this
> currently, but if we defer it until the pcibios call happens we'll
> have done a pile of config accesses before then. Maybe it doesn't
> matter, but we'd need to do further testing under phyp or work out
> some other way to ensure it's done pre-probe.

Hmm! I had not looked at this specifically, but I don't think I've made
it any worse. The reordering is all underneath pcibios_init(), and it
changes things from this...

	for each PHB: pcibios_scan_phb() and then pci_bus_add_devices().
	pcibios_resource_survey() [calls ppc_md.pcibios_fixup()->eeh_probe_devices()]

... to this:

	for each PHB: pcibios_scan_phb()
	pcibios_resource_survey()
	for each PHB: pci_bus_add_devices()
	ppc_md.pcibios_fixup()->eeh_probe_devices()

So either way, the EEH probe (and therefore setup) happens last. Moving
the probe into pseries_pcibios_bus_add_device() actually moves it a
little earlier than before.

Just for kicks, I instrumented rtas_pci_read_config() and it shows
that there are indeed several accesses before the probe function is
called. Here's the first:

pcibios_init() ->
pcibios_scan_phb() ->
__of_scan_bus() ->
of_scan_pci_dev() ->
of_create_pci_dev() ->
set_pcie_port_type() ->
pci_find_capability()

Most of that is PowerPC specific, and there's already an EEH hack in
of_scan_pci_dev(), so I tried a quick hack to probe there and it does
seem at least boot OK. And it's before any accesses! :-)

What do you think? (Although, I think I'd prefer to leave this as follow
up work.)

> >  /**
> >   * eeh_init - EEH initialization
> >   *
> > diff --git a/arch/powerpc/kernel/eeh_cache.c b/arch/powerpc/kernel/eeh_cache.c
> > index f93dd5cf6a39..c40078d036af 100644
> > --- a/arch/powerpc/kernel/eeh_cache.c
> > +++ b/arch/powerpc/kernel/eeh_cache.c
> > @@ -278,38 +278,6 @@ void eeh_addr_cache_init(void)
> >         spin_lock_init(&pci_io_addr_cache_root.piar_lock);
> >  }
> >
> > -/**
> > - * eeh_addr_cache_build - Build a cache of I/O addresses
> > - *
> > - * Build a cache of pci i/o addresses.  This cache will be used to
> > - * find the pci device that corresponds to a given address.
> > - * This routine scans all pci busses to build the cache.
> > - * Must be run late in boot process, after the pci controllers
> > - * have been scanned for devices (after all device resources are known).
> > - */
> > -void eeh_addr_cache_build(void)
> > -{
> > -       struct pci_dn *pdn;
> > -       struct eeh_dev *edev;
> > -       struct pci_dev *dev = NULL;
> > -
> > -       for_each_pci_dev(dev) {
> > -               pdn = pci_get_pdn_by_devfn(dev->bus, dev->devfn);
> > -               if (!pdn)
> > -                       continue;
> > -
> > -               edev = pdn_to_eeh_dev(pdn);
> > -               if (!edev)
> > -                       continue;
> > -
> > -               dev->dev.archdata.edev = edev;
> > -               edev->pdev = dev;
> > -
> > -               eeh_addr_cache_insert_dev(dev);
> > -               eeh_sysfs_add_device(dev);
> > -       }
> > -}
> > -
> >  static int eeh_addr_cache_show(struct seq_file *s, void *v)
> >  {
> >         struct pci_io_addr_range *piar;
> > diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
> > index 90729d908a54..22a94f4b8586 100644
> > --- a/arch/powerpc/platforms/powernv/eeh-powernv.c
> > +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
> > @@ -259,9 +259,7 @@ int pnv_eeh_post_init(void)
> >         struct pnv_phb *phb;
> >         int ret = 0;
> >
> > -       /* Probe devices & build address cache */
> > -       eeh_probe_devices();
> > -       eeh_addr_cache_build();
> > +       eeh_show_enabled();
> >
> >         /* Register OPAL event notifier */
> >         eeh_event_irq = opal_event_request(ilog2(OPAL_EVENT_PCI_ERROR));
> > diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c
> > index 37a77e57893e..d6a5f4f27507 100644
> > --- a/arch/powerpc/platforms/pseries/pci.c
> > +++ b/arch/powerpc/platforms/pseries/pci.c
> > @@ -242,8 +242,7 @@ void __init pSeries_final_fixup(void)
> >
> >         pSeries_request_regions();
> >
> > -       eeh_probe_devices();
> > -       eeh_addr_cache_build();
> > +       eeh_show_enabled();
> >
> >  #ifdef CONFIG_PCI_IOV
> >         ppc_md.pcibios_sriov_enable = pseries_pcibios_sriov_enable;
> > --
> > 2.19.0.2.gcad72f5712
> >
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 3/6] powerpc/eeh: Improve debug messages around device addition

[Alexey Kardashevskiy]

On 19/06/2019 14:27, Sam Bobroff wrote:
> On Tue, Jun 11, 2019 at 03:47:58PM +1000, Alexey Kardashevskiy wrote:
>>
>>
>> On 07/05/2019 14:30, Sam Bobroff wrote:
>>> Also remove useless comment.
>>>
>>> Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
>>> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>> ---
>>>  arch/powerpc/kernel/eeh.c                    |  2 +-
>>>  arch/powerpc/platforms/powernv/eeh-powernv.c | 14 ++++++++----
>>>  arch/powerpc/platforms/pseries/eeh_pseries.c | 23 +++++++++++++++-----
>>>  3 files changed, 28 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
>>> index 8d3c36a1f194..b14d89547895 100644
>>> --- a/arch/powerpc/kernel/eeh.c
>>> +++ b/arch/powerpc/kernel/eeh.c
>>> @@ -1291,7 +1291,7 @@ void eeh_add_device_late(struct pci_dev *dev)
>>>  	pdn = pci_get_pdn_by_devfn(dev->bus, dev->devfn);
>>>  	edev = pdn_to_eeh_dev(pdn);
>>>  	if (edev->pdev == dev) {
>>> -		pr_debug("EEH: Already referenced !\n");
>>> +		pr_debug("EEH: Device %s already referenced!\n", pci_name(dev));
>>>  		return;
>>>  	}
>>>  
>>> diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
>>> index 6fc1a463b796..0e374cdba961 100644
>>> --- a/arch/powerpc/platforms/powernv/eeh-powernv.c
>>> +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
>>> @@ -50,10 +50,7 @@ void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
>>>  	if (!pdev->is_virtfn)
>>>  		return;
>>>  
>>> -	/*
>>> -	 * The following operations will fail if VF's sysfs files
>>> -	 * aren't created or its resources aren't finalized.
>>> -	 */
>>> +	pr_debug("%s: EEH: Setting up device %s.\n", __func__, pci_name(pdev));
>>
>>
>> dev_dbg() seems more appropriate.
> 
> Oh! It does, or even pci_debug() :-)
> 
> I'll change it if I need to do another version, otherwise I'll clean it
> up later.
> 
>>>  	eeh_add_device_early(pdn);
>>>  	eeh_add_device_late(pdev);
>>>  	eeh_sysfs_add_device(pdev);
>>> @@ -397,6 +394,10 @@ static void *pnv_eeh_probe(struct pci_dn *pdn, void *data)
>>>  	int ret;
>>>  	int config_addr = (pdn->busno << 8) | (pdn->devfn);
>>>  
>>> +	pr_debug("%s: probing %04x:%02x:%02x.%01x\n",
>>> +		__func__, hose->global_number, pdn->busno,
>>> +		PCI_SLOT(pdn->devfn), PCI_FUNC(pdn->devfn));
>>> +
>>>  	/*
>>>  	 * When probing the root bridge, which doesn't have any
>>>  	 * subordinate PCI devices. We don't have OF node for
>>> @@ -491,6 +492,11 @@ static void *pnv_eeh_probe(struct pci_dn *pdn, void *data)
>>>  	/* Save memory bars */
>>>  	eeh_save_bars(edev);
>>>  
>>> +	pr_debug("%s: EEH enabled on %02x:%02x.%01x PHB#%x-PE#%x\n",
>>> +		__func__, pdn->busno, PCI_SLOT(pdn->devfn),
>>> +		PCI_FUNC(pdn->devfn), edev->pe->phb->global_number,
>>> +		edev->pe->addr);
>>> +
>>>  	return NULL;
>>>  }
>>>  
>>> diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
>>> index 7aa50258dd42..ae06878fbdea 100644
>>> --- a/arch/powerpc/platforms/pseries/eeh_pseries.c
>>> +++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
>>> @@ -65,6 +65,8 @@ void pseries_pcibios_bus_add_device(struct pci_dev *pdev)
>>>  	if (!pdev->is_virtfn)
>>>  		return;
>>>  
>>> +	pr_debug("%s: EEH: Setting up device %s.\n", __func__, pci_name(pdev));
>>> +
>>>  	pdn->device_id  =  pdev->device;
>>>  	pdn->vendor_id  =  pdev->vendor;
>>>  	pdn->class_code =  pdev->class;
>>> @@ -251,6 +253,10 @@ static void *pseries_eeh_probe(struct pci_dn *pdn, void *data)
>>>  	int enable = 0;
>>>  	int ret;
>>>  
>>> +	pr_debug("%s: probing %04x:%02x:%02x.%01x\n",
>>> +		__func__, pdn->phb->global_number, pdn->busno,
>>> +		PCI_SLOT(pdn->devfn), PCI_FUNC(pdn->devfn));
>>> +
>>>  	/* Retrieve OF node and eeh device */
>>>  	edev = pdn_to_eeh_dev(pdn);
>>>  	if (!edev || edev->pe)
>>> @@ -294,7 +300,12 @@ static void *pseries_eeh_probe(struct pci_dn *pdn, void *data)
>>>  
>>>  	/* Enable EEH on the device */
>>>  	ret = eeh_ops->set_option(&pe, EEH_OPT_ENABLE);
>>> -	if (!ret) {
>>> +	if (ret) {
>>> +		pr_debug("%s: EEH failed to enable on %02x:%02x.%01x PHB#%x-PE#%x (code %d)\n",
>>> +			__func__, pdn->busno, PCI_SLOT(pdn->devfn),
>>> +			PCI_FUNC(pdn->devfn), pe.phb->global_number,
>>> +			pe.addr, ret);
>>> +	} else {
>>
>>
>> edev!=NULL here so you could do dev_dbg(&edev->pdev->dev,...) and skip
>> PCI_SLOT/PCI_FUNC. Or is (edev!=NULL && edev->pdev==NULL) possible (it
>> could be, just asking)?
> 
> I can see that edev will be non-NULL here, but that pr_debug() pattern
> (using the PDN information to form the PCI address) is quite common
> across the EEH code, so I think rather than changing a couple of
> specific cases, I should do a separate cleanup patch and introduce
> something like pdn_debug(pdn, "...."). What do you think?


I'd switch them all to already existing dev_dbg/pci_debug rather than
adding pdn_debug as imho it should not have been used in the first place
really...

> (I don't know exactly when edev->pdev can be NULL.)

... and if you switch to dev_dbg/pci_debug, I think quite soon you'll
know if it can or cannot be NULL :)



> 
>>
>>>  		/* Retrieve PE address */
>>>  		edev->pe_config_addr = eeh_ops->get_pe_addr(&pe);
>>>  		pe.addr = edev->pe_config_addr;
>>> @@ -310,11 +321,6 @@ static void *pseries_eeh_probe(struct pci_dn *pdn, void *data)
>>>  		if (enable) {
>>>  			eeh_add_flag(EEH_ENABLED);
>>>  			eeh_add_to_parent_pe(edev);
>>> -
>>> -			pr_debug("%s: EEH enabled on %02x:%02x.%01x PHB#%x-PE#%x\n",
>>> -				__func__, pdn->busno, PCI_SLOT(pdn->devfn),
>>> -				PCI_FUNC(pdn->devfn), pe.phb->global_number,
>>> -				pe.addr);
>>>  		} else if (pdn->parent && pdn_to_eeh_dev(pdn->parent) &&
>>>  			   (pdn_to_eeh_dev(pdn->parent))->pe) {
>>>  			/* This device doesn't support EEH, but it may have an
>>> @@ -323,6 +329,11 @@ static void *pseries_eeh_probe(struct pci_dn *pdn, void *data)
>>>  			edev->pe_config_addr = pdn_to_eeh_dev(pdn->parent)->pe_config_addr;
>>>  			eeh_add_to_parent_pe(edev);
>>>  		}
>>> +		pr_debug("%s: EEH %s on %02x:%02x.%01x PHB#%x-PE#%x (code %d)\n",
>>> +			__func__, (enable ? "enabled" : "unsupported"),
>>> +			pdn->busno, PCI_SLOT(pdn->devfn),
>>> +			PCI_FUNC(pdn->devfn), pe.phb->global_number,
>>> +			pe.addr, ret);
>>
>> Same here. I understand though this one is a cut-n-paste :)
>>
>>
>>>  	}
>>>  
>>>  	/* Save memory bars */
>>>
>>
>> -- 
>> Alexey

-- 
Alexey

Re: [PATCH v2 3/6] powerpc/eeh: Improve debug messages around device addition

[Oliver O'Halloran]

On Thu, Jun 20, 2019 at 12:40 PM Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>
> On 19/06/2019 14:27, Sam Bobroff wrote:
> > On Tue, Jun 11, 2019 at 03:47:58PM +1000, Alexey Kardashevskiy wrote:
> >>
> >> On 07/05/2019 14:30, Sam Bobroff wrote:
> >>> Also remove useless comment.
> >>>
> >>> Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
> >>> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> >>> ---
> *snip*
> >
> > I can see that edev will be non-NULL here, but that pr_debug() pattern
> > (using the PDN information to form the PCI address) is quite common
> > across the EEH code, so I think rather than changing a couple of
> > specific cases, I should do a separate cleanup patch and introduce
> > something like pdn_debug(pdn, "...."). What do you think?
>
> I'd switch them all to already existing dev_dbg/pci_debug rather than
> adding pdn_debug as imho it should not have been used in the first place
> really...
>
> > (I don't know exactly when edev->pdev can be NULL.)
>
> ... and if you switch to dev_dbg/pci_debug, I think quite soon you'll
> know if it can or cannot be NULL :)

As far as I can tell edev->pdev is NULL in two cases:

1. Before eeh_device_add_late() has been called on the pdev. The late
part of the add maps the pdev to an edev and sets the pdev's edev
pointer and vis a vis.
2. While recoverying EEH unaware devices. Unaware devices are
destroyed and rescanned and the edev->pdev pointer is cleared by
pcibios_device_release()

In most of these cases it should be safe to use the pci_*() functions
rather than making a new one up for printing pdns. In the cases where
we might not have a PCI dev i'd make a new set of prints that take an
EEH dev rather than a pci_dn since i'd like pci_dn to die sooner
rather than later.

Oliver

Re: [EXTERNAL] Re: [PATCH v2 3/6] powerpc/eeh: Improve debug messages around device addition

[Sam Bobroff]

[-- Attachment #1: Type: text/plain, Size: 2572 bytes --]

On Thu, Jun 20, 2019 at 01:45:24PM +1000, Oliver O'Halloran wrote:
> On Thu, Jun 20, 2019 at 12:40 PM Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> >
> > On 19/06/2019 14:27, Sam Bobroff wrote:
> > > On Tue, Jun 11, 2019 at 03:47:58PM +1000, Alexey Kardashevskiy wrote:
> > >>
> > >> On 07/05/2019 14:30, Sam Bobroff wrote:
> > >>> Also remove useless comment.
> > >>>
> > >>> Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
> > >>> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> > >>> ---
> > *snip*
> > >
> > > I can see that edev will be non-NULL here, but that pr_debug() pattern
> > > (using the PDN information to form the PCI address) is quite common
> > > across the EEH code, so I think rather than changing a couple of
> > > specific cases, I should do a separate cleanup patch and introduce
> > > something like pdn_debug(pdn, "...."). What do you think?
> >
> > I'd switch them all to already existing dev_dbg/pci_debug rather than
> > adding pdn_debug as imho it should not have been used in the first place
> > really...
> >
> > > (I don't know exactly when edev->pdev can be NULL.)
> >
> > ... and if you switch to dev_dbg/pci_debug, I think quite soon you'll
> > know if it can or cannot be NULL :)
> 
> As far as I can tell edev->pdev is NULL in two cases:
> 
> 1. Before eeh_device_add_late() has been called on the pdev. The late
> part of the add maps the pdev to an edev and sets the pdev's edev
> pointer and vis a vis.
> 2. While recoverying EEH unaware devices. Unaware devices are
> destroyed and rescanned and the edev->pdev pointer is cleared by
> pcibios_device_release()
> 
> In most of these cases it should be safe to use the pci_*() functions
> rather than making a new one up for printing pdns. In the cases where
> we might not have a PCI dev i'd make a new set of prints that take an
> EEH dev rather than a pci_dn since i'd like pci_dn to die sooner
> rather than later.
> 
> Oliver

I'll change the calls in {pnv,pseries}_pcibios_bus_add_device() and
eeh_add_device_late() to use dev_dbg() and post a new version.

For {pnv,pseries}_eeh_probe() I'm not sure what we can do; there's no
pci_dev available yet and while it would be nice to use the eeh_dev
rather than the pdn, it doesn't seem to have the bus/device/fn
information we need. Am I missing something there?  (The code in the
probe functions seems to get it from the pci_dn.)

If there isn't an easy way around this, would it therefore be reasonable
to just leave them open-coded as they are?

Cheers,
Sam.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [EXTERNAL] Re: [PATCH v2 3/6] powerpc/eeh: Improve debug messages around device addition

[Oliver O'Halloran]

On Tue, 2019-07-16 at 16:48 +1000, Sam Bobroff wrote:
> On Thu, Jun 20, 2019 at 01:45:24PM +1000, Oliver O'Halloran wrote:
> > On Thu, Jun 20, 2019 at 12:40 PM Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> > > On 19/06/2019 14:27, Sam Bobroff wrote:
> > > > On Tue, Jun 11, 2019 at 03:47:58PM +1000, Alexey Kardashevskiy wrote:
> > > > > On 07/05/2019 14:30, Sam Bobroff wrote:
> > > > > > Also remove useless comment.
> > > > > > 
> > > > > > Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
> > > > > > Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> > > > > > ---
> > > *snip*
> > > > I can see that edev will be non-NULL here, but that pr_debug() pattern
> > > > (using the PDN information to form the PCI address) is quite common
> > > > across the EEH code, so I think rather than changing a couple of
> > > > specific cases, I should do a separate cleanup patch and introduce
> > > > something like pdn_debug(pdn, "...."). What do you think?
> > > 
> > > I'd switch them all to already existing dev_dbg/pci_debug rather than
> > > adding pdn_debug as imho it should not have been used in the first place
> > > really...
> > > 
> > > > (I don't know exactly when edev->pdev can be NULL.)
> > > 
> > > ... and if you switch to dev_dbg/pci_debug, I think quite soon you'll
> > > know if it can or cannot be NULL :)
> > 
> > As far as I can tell edev->pdev is NULL in two cases:
> > 
> > 1. Before eeh_device_add_late() has been called on the pdev. The late
> > part of the add maps the pdev to an edev and sets the pdev's edev
> > pointer and vis a vis.
> > 2. While recoverying EEH unaware devices. Unaware devices are
> > destroyed and rescanned and the edev->pdev pointer is cleared by
> > pcibios_device_release()
> > 
> > In most of these cases it should be safe to use the pci_*() functions
> > rather than making a new one up for printing pdns. In the cases where
> > we might not have a PCI dev i'd make a new set of prints that take an
> > EEH dev rather than a pci_dn since i'd like pci_dn to die sooner
> > rather than later.
> > 
> > Oliver
> 
> I'll change the calls in {pnv,pseries}_pcibios_bus_add_device() and
> eeh_add_device_late() to use dev_dbg() and post a new version.
> 
> For {pnv,pseries}_eeh_probe() I'm not sure what we can do; there's no
> pci_dev available yet and while it would be nice to use the eeh_dev
> rather than the pdn, it doesn't seem to have the bus/device/fn
> information we need. Am I missing something there?  (The code in the
> probe functions seems to get it from the pci_dn.)

We do have a pci_dev in the powernv case since pnv_eeh_probe() isn't
called until the late probe happens (which is after the pci_dev has
been created). I've got some patches to rework the probe path to make
this a bit clearer, but they need a bit more work.

> 
> If there isn't an easy way around this, would it therefore be reasonable
> to just leave them open-coded as they are?

I've had this patch floating around a while that should do the trick.
The PCI_BUSNO macro is probably unnecessary since I'm sure there is
something that does it in generic code, but I couldn't find it.


From 61ff8c23c4d13ff640fb2d069dcedacdf2ee22dd Mon Sep 17 00:00:00 2001
From: Oliver O'Halloran <oohall@gmail.com>
Date: Thu, 18 Apr 2019 18:25:13 +1000
Subject: [PATCH] powerpc/eeh: Add bdfn field to eeh_dev

Preperation for removing pci_dn from the powernv EEH code. The only thing we
really use pci_dn for is to get the bdfn of the device for config space
accesses, so adding that information to eeh_dev reduces the need to carry
around the pci_dn.

Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
 arch/powerpc/include/asm/eeh.h     | 2 ++
 arch/powerpc/include/asm/ppc-pci.h | 2 ++
 arch/powerpc/kernel/eeh_dev.c      | 2 ++
 3 files changed, 6 insertions(+)

diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 7fd476d..a208e02 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -131,6 +131,8 @@ static inline bool eeh_pe_passed(struct eeh_pe *pe)
 struct eeh_dev {
 	int mode;			/* EEH mode			*/
 	int class_code;			/* Class code of the device	*/
+	int bdfn; 			/* bdfn of device (for cfg ops) */
+	struct pci_controller *controller;
 	int pe_config_addr;		/* PE config address		*/
 	u32 config_space[16];		/* Saved PCI config space	*/
 	int pcix_cap;			/* Saved PCIx capability	*/
diff --git a/arch/powerpc/include/asm/ppc-pci.h b/arch/powerpc/include/asm/ppc-pci.h
index cec2d64..72860de 100644
--- a/arch/powerpc/include/asm/ppc-pci.h
+++ b/arch/powerpc/include/asm/ppc-pci.h
@@ -74,6 +74,8 @@ static inline const char *eeh_driver_name(struct pci_dev *pdev)
 
 #endif /* CONFIG_EEH */
 
+#define PCI_BUSNO(bdfn) ((bdfn >> 8) & 0xff)
+
 #else /* CONFIG_PCI */
 static inline void init_pci_config_tokens(void) { }
 #endif /* !CONFIG_PCI */
diff --git a/arch/powerpc/kernel/eeh_dev.c b/arch/powerpc/kernel/eeh_dev.c
index c4317c4..7370185 100644
--- a/arch/powerpc/kernel/eeh_dev.c
+++ b/arch/powerpc/kernel/eeh_dev.c
@@ -47,6 +47,8 @@ struct eeh_dev *eeh_dev_init(struct pci_dn *pdn)
 	/* Associate EEH device with OF node */
 	pdn->edev = edev;
 	edev->pdn = pdn;
+	edev->bdfn = (pdn->busno << 8) | pdn->devfn;
+	edev->controller = pdn->phb;
 
 	return edev;
 }
-- 
2.9.5

Re: [EXTERNAL] Re: [PATCH v2 3/6] powerpc/eeh: Improve debug messages around device addition

[Sam Bobroff]

[-- Attachment #1: Type: text/plain, Size: 6095 bytes --]

On Tue, Jul 16, 2019 at 05:00:44PM +1000, Oliver O'Halloran wrote:
> On Tue, 2019-07-16 at 16:48 +1000, Sam Bobroff wrote:
> > On Thu, Jun 20, 2019 at 01:45:24PM +1000, Oliver O'Halloran wrote:
> > > On Thu, Jun 20, 2019 at 12:40 PM Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> > > > On 19/06/2019 14:27, Sam Bobroff wrote:
> > > > > On Tue, Jun 11, 2019 at 03:47:58PM +1000, Alexey Kardashevskiy wrote:
> > > > > > On 07/05/2019 14:30, Sam Bobroff wrote:
> > > > > > > Also remove useless comment.
> > > > > > > 
> > > > > > > Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
> > > > > > > Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> > > > > > > ---
> > > > *snip*
> > > > > I can see that edev will be non-NULL here, but that pr_debug() pattern
> > > > > (using the PDN information to form the PCI address) is quite common
> > > > > across the EEH code, so I think rather than changing a couple of
> > > > > specific cases, I should do a separate cleanup patch and introduce
> > > > > something like pdn_debug(pdn, "...."). What do you think?
> > > > 
> > > > I'd switch them all to already existing dev_dbg/pci_debug rather than
> > > > adding pdn_debug as imho it should not have been used in the first place
> > > > really...
> > > > 
> > > > > (I don't know exactly when edev->pdev can be NULL.)
> > > > 
> > > > ... and if you switch to dev_dbg/pci_debug, I think quite soon you'll
> > > > know if it can or cannot be NULL :)
> > > 
> > > As far as I can tell edev->pdev is NULL in two cases:
> > > 
> > > 1. Before eeh_device_add_late() has been called on the pdev. The late
> > > part of the add maps the pdev to an edev and sets the pdev's edev
> > > pointer and vis a vis.
> > > 2. While recoverying EEH unaware devices. Unaware devices are
> > > destroyed and rescanned and the edev->pdev pointer is cleared by
> > > pcibios_device_release()
> > > 
> > > In most of these cases it should be safe to use the pci_*() functions
> > > rather than making a new one up for printing pdns. In the cases where
> > > we might not have a PCI dev i'd make a new set of prints that take an
> > > EEH dev rather than a pci_dn since i'd like pci_dn to die sooner
> > > rather than later.
> > > 
> > > Oliver
> > 
> > I'll change the calls in {pnv,pseries}_pcibios_bus_add_device() and
> > eeh_add_device_late() to use dev_dbg() and post a new version.
> > 
> > For {pnv,pseries}_eeh_probe() I'm not sure what we can do; there's no
> > pci_dev available yet and while it would be nice to use the eeh_dev
> > rather than the pdn, it doesn't seem to have the bus/device/fn
> > information we need. Am I missing something there?  (The code in the
> > probe functions seems to get it from the pci_dn.)
> 
> We do have a pci_dev in the powernv case since pnv_eeh_probe() isn't
> called until the late probe happens (which is after the pci_dev has
> been created). I've got some patches to rework the probe path to make
> this a bit clearer, but they need a bit more work.
> 
> > 
> > If there isn't an easy way around this, would it therefore be reasonable
> > to just leave them open-coded as they are?
> 
> I've had this patch floating around a while that should do the trick.
> The PCI_BUSNO macro is probably unnecessary since I'm sure there is
> something that does it in generic code, but I couldn't find it.

Looks good, I'll try including it and create a dev_dbg style function
or macro that takes an edev.

I don't think I can use it in the pcibios bus add device handlers (where
there is no edev, or where it may be attached to the wrong device) but
I'll use it for all the other cases.

If it works out well I can follow up and update more of the EEH logging
to use it :-)

> From 61ff8c23c4d13ff640fb2d069dcedacdf2ee22dd Mon Sep 17 00:00:00 2001
> From: Oliver O'Halloran <oohall@gmail.com>
> Date: Thu, 18 Apr 2019 18:25:13 +1000
> Subject: [PATCH] powerpc/eeh: Add bdfn field to eeh_dev
> 
> Preperation for removing pci_dn from the powernv EEH code. The only thing we
> really use pci_dn for is to get the bdfn of the device for config space
> accesses, so adding that information to eeh_dev reduces the need to carry
> around the pci_dn.
> 
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> ---
>  arch/powerpc/include/asm/eeh.h     | 2 ++
>  arch/powerpc/include/asm/ppc-pci.h | 2 ++
>  arch/powerpc/kernel/eeh_dev.c      | 2 ++
>  3 files changed, 6 insertions(+)
> 
> diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
> index 7fd476d..a208e02 100644
> --- a/arch/powerpc/include/asm/eeh.h
> +++ b/arch/powerpc/include/asm/eeh.h
> @@ -131,6 +131,8 @@ static inline bool eeh_pe_passed(struct eeh_pe *pe)
>  struct eeh_dev {
>  	int mode;			/* EEH mode			*/
>  	int class_code;			/* Class code of the device	*/
> +	int bdfn; 			/* bdfn of device (for cfg ops) */
> +	struct pci_controller *controller;
>  	int pe_config_addr;		/* PE config address		*/
>  	u32 config_space[16];		/* Saved PCI config space	*/
>  	int pcix_cap;			/* Saved PCIx capability	*/
> diff --git a/arch/powerpc/include/asm/ppc-pci.h b/arch/powerpc/include/asm/ppc-pci.h
> index cec2d64..72860de 100644
> --- a/arch/powerpc/include/asm/ppc-pci.h
> +++ b/arch/powerpc/include/asm/ppc-pci.h
> @@ -74,6 +74,8 @@ static inline const char *eeh_driver_name(struct pci_dev *pdev)
>  
>  #endif /* CONFIG_EEH */
>  
> +#define PCI_BUSNO(bdfn) ((bdfn >> 8) & 0xff)
> +
>  #else /* CONFIG_PCI */
>  static inline void init_pci_config_tokens(void) { }
>  #endif /* !CONFIG_PCI */
> diff --git a/arch/powerpc/kernel/eeh_dev.c b/arch/powerpc/kernel/eeh_dev.c
> index c4317c4..7370185 100644
> --- a/arch/powerpc/kernel/eeh_dev.c
> +++ b/arch/powerpc/kernel/eeh_dev.c
> @@ -47,6 +47,8 @@ struct eeh_dev *eeh_dev_init(struct pci_dn *pdn)
>  	/* Associate EEH device with OF node */
>  	pdn->edev = edev;
>  	edev->pdn = pdn;
> +	edev->bdfn = (pdn->busno << 8) | pdn->devfn;
> +	edev->controller = pdn->phb;
>  
>  	return edev;
>  }
> -- 
> 2.9.5
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v2 0/6]

[Zeyu Jin]

Currently we have both multi-thread compression and multifd to optimize
live migration in Qemu. Mulit-thread compression deals with the situation
where network bandwith is limited but cpu resource adequate. Multifd instead
aims to take full advantage of network bandwith. Moreover it supports both
zlib and zstd compression on each channel.

In this patch series, we did some code refactoring on multi-thread compression
live migration and bring zstd compression method support for it.

Below is the test result of multi-thread compression live migration
with different compress methods. Test result shows that zstd outperforms
zlib by about 70%.

 Migration Configuration:
 Guest 8U 32G
 compress-threads   8
 decompress-threads 2
 compress-level 1
 bandwidth-limit 100Mbps

 Test Result:
 +---------------------+--------------+-------------+
 |  compress method    |   zlib       |    zstd     |
 +---------------------+--------------+-------------+
 |  total time (ms)    |   75256      |    44187    |
 +---------------------+--------------+-------------+
 |  downtime(ms)       |   128        |    81       |
 +---------------------+--------------+-------------+
 |  transferred ram(kB)|   1576866    |    736117   |
 +---------------------+--------------+-------------+
 |  throughput(mbps)   |   172.06     |    137.16   |
 +---------------------+--------------+-------------+
 |  total ram(kB)      |   33685952   |    33685952 |
 +---------------------+--------------+-------------+

Zeyu Jin (6):
  migration: Add multi-thread compress method
  migration: Refactoring multi-thread compress migration
  migration: Add multi-thread compress ops
  migration: Add zstd support in multi-thread compression
  migration: Add compress_level sanity check
  doc: Update multi-thread compression doc

 docs/multi-thread-compression.txt |  31 ++-
 hw/core/qdev-properties-system.c  |  11 +
 include/hw/qdev-properties.h      |   4 +
 migration/migration.c             |  56 ++++-
 migration/migration.h             |   1 +
 migration/qemu-file.c             |  62 +----
 migration/qemu-file.h             |   4 +-
 migration/ram.c                   | 381 +++++++++++++++++++++++++-----
 monitor/hmp-cmds.c                |  12 +
 qapi/migration.json               |  26 +-
 10 files changed, 465 insertions(+), 123 deletions(-)

-- 
2.27.0