linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] powerpc/xive: PCI hotplug fixes under PowerVM
@ 2020-04-29  7:51 Cédric Le Goater
  2020-04-29  7:51 ` [PATCH 1/3] powerpc/xive: Clear the page tables for the ESB IO mapping Cédric Le Goater
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Cédric Le Goater @ 2020-04-29  7:51 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev, Cédric Le Goater

Hello,

Here are a couple of fixes for PCI hotplug issues for machines running
under the POWER hypervisor using hash MMU and the XIVE interrupt mode.

Commit 1ca3dec2b2df ("powerpc/xive: Prevent page fault issues in the
machine crash handler") forced the mapping of the XIVE ESB page and
this is now blocking the removal of a passthrough IO adapter because
the PCI isolation fails with "valid outstanding translations". Under
KVM, the ESB pages for the adapter interrupts are un-mapped from the
guest by the hypervisor in the KVM XIVE native device. This is is now
redundant but it's harmless.

Last is a fix to disable the XIVE debugfs file when XIVE is disabled.

Thanks,

C.

Cédric Le Goater (3):
  powerpc/xive: Clear the page tables for the ESB IO mapping
  powerpc/pci: unmap legacy INTx interrupts of passthrough IO adapters
  powerpc/xive: Do not expose a debugfs file when XIVE is disabled

 arch/powerpc/kernel/pci-hotplug.c |  2 ++
 arch/powerpc/sysdev/xive/common.c | 10 ++++++++--
 2 files changed, 10 insertions(+), 2 deletions(-)

-- 
2.25.4


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/3] powerpc/xive: Clear the page tables for the ESB IO mapping
  2020-04-29  7:51 [PATCH 0/3] powerpc/xive: PCI hotplug fixes under PowerVM Cédric Le Goater
@ 2020-04-29  7:51 ` Cédric Le Goater
  2020-04-29  7:51 ` [PATCH 2/3] powerpc/pci: unmap legacy INTx interrupts of passthrough IO adapters Cédric Le Goater
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Cédric Le Goater @ 2020-04-29  7:51 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev, Cédric Le Goater, stable

Commit 1ca3dec2b2df ("powerpc/xive: Prevent page fault issues in the
machine crash handler") fixed an issue in the FW assisted dump of
machines using hash MMU and the XIVE interrupt mode under the POWER
hypervisor. It forced the mapping of the ESB page of interrupts being
mapped in the Linux IRQ number space to make sure the 'crash kexec'
sequence worked during such an event. But it didn't handle the
un-mapping.

This mapping is now blocking the removal of a passthrough IO adapter
under the POWER hypervisor because it expects the guest OS to have
cleared all page table entries related to the adapter. If some are
still present, the RTAS call which isolates the PCI slot returns error
9001 "valid outstanding translations".

Remove these mapping in the IRQ data cleanup routine.

Under KVM, this cleanup is not required because the ESB pages for the
adapter interrupts are un-mapped from the guest by the hypervisor in
the KVM XIVE native device. This is now redundant but it's harmless.

Fixes: 1ca3dec2b2df ("powerpc/xive: Prevent page fault issues in the machine crash handler")
Cc: stable@vger.kernel.org # v5.5+
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 arch/powerpc/sysdev/xive/common.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
index 9603b2830d03..3dbc94cb4380 100644
--- a/arch/powerpc/sysdev/xive/common.c
+++ b/arch/powerpc/sysdev/xive/common.c
@@ -19,6 +19,7 @@
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/msi.h>
+#include <linux/vmalloc.h>
 
 #include <asm/debugfs.h>
 #include <asm/prom.h>
@@ -1020,12 +1021,16 @@ EXPORT_SYMBOL_GPL(is_xive_irq);
 void xive_cleanup_irq_data(struct xive_irq_data *xd)
 {
 	if (xd->eoi_mmio) {
+		unmap_kernel_range((unsigned long)xd->eoi_mmio,
+				   1u << xd->esb_shift);
 		iounmap(xd->eoi_mmio);
 		if (xd->eoi_mmio == xd->trig_mmio)
 			xd->trig_mmio = NULL;
 		xd->eoi_mmio = NULL;
 	}
 	if (xd->trig_mmio) {
+		unmap_kernel_range((unsigned long)xd->trig_mmio,
+				   1u << xd->esb_shift);
 		iounmap(xd->trig_mmio);
 		xd->trig_mmio = NULL;
 	}
-- 
2.25.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/3] powerpc/pci: unmap legacy INTx interrupts of passthrough IO adapters
  2020-04-29  7:51 [PATCH 0/3] powerpc/xive: PCI hotplug fixes under PowerVM Cédric Le Goater
  2020-04-29  7:51 ` [PATCH 1/3] powerpc/xive: Clear the page tables for the ESB IO mapping Cédric Le Goater
@ 2020-04-29  7:51 ` Cédric Le Goater
  2020-05-21  7:13   ` Cédric Le Goater
                     ` (2 more replies)
  2020-04-29  7:51 ` [PATCH 3/3] powerpc/xive: Do not expose a debugfs file when XIVE is disabled Cédric Le Goater
  2020-06-09  5:28 ` [PATCH 0/3] powerpc/xive: PCI hotplug fixes under PowerVM Michael Ellerman
  3 siblings, 3 replies; 11+ messages in thread
From: Cédric Le Goater @ 2020-04-29  7:51 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Oliver O'Halloran, linuxppc-dev, Cédric Le Goater

When a passthrough IO adapter is removed from a pseries machine using
hash MMU and the XIVE interrupt mode, the POWER hypervisor, pHyp,
expects the guest OS to have cleared all page table entries related to
the adapter. If some are still present, the RTAS call which isolates
the PCI slot returns error 9001 "valid outstanding translations" and
the removal of the IO adapter fails.

INTx interrupt numbers need special care because Linux maps the
interrupts automatically in the Linux interrupt number space if they
are presented in the device tree node describing the IO adapter. These
interrupts are not un-mapped automatically and in case of an hot-plug
adapter, the PCI hot-plug layer needs to handle the cleanup to make
sure that all the page table entries of the XIVE ESB pages are
cleared.

Cc: "Oliver O'Halloran" <oohall@gmail.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 arch/powerpc/kernel/pci-hotplug.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/kernel/pci-hotplug.c b/arch/powerpc/kernel/pci-hotplug.c
index bf83f76563a3..9e9c6befd7ea 100644
--- a/arch/powerpc/kernel/pci-hotplug.c
+++ b/arch/powerpc/kernel/pci-hotplug.c
@@ -57,6 +57,8 @@ void pcibios_release_device(struct pci_dev *dev)
 	struct pci_controller *phb = pci_bus_to_host(dev->bus);
 	struct pci_dn *pdn = pci_get_pdn(dev);
 
+	irq_dispose_mapping(dev->irq);
+
 	eeh_remove_device(dev);
 
 	if (phb->controller_ops.release_device)
-- 
2.25.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/3] powerpc/xive: Do not expose a debugfs file when XIVE is disabled
  2020-04-29  7:51 [PATCH 0/3] powerpc/xive: PCI hotplug fixes under PowerVM Cédric Le Goater
  2020-04-29  7:51 ` [PATCH 1/3] powerpc/xive: Clear the page tables for the ESB IO mapping Cédric Le Goater
  2020-04-29  7:51 ` [PATCH 2/3] powerpc/pci: unmap legacy INTx interrupts of passthrough IO adapters Cédric Le Goater
@ 2020-04-29  7:51 ` Cédric Le Goater
  2020-06-09  5:28 ` [PATCH 0/3] powerpc/xive: PCI hotplug fixes under PowerVM Michael Ellerman
  3 siblings, 0 replies; 11+ messages in thread
From: Cédric Le Goater @ 2020-04-29  7:51 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev, Cédric Le Goater

The XIVE interrupt mode can be disabled with the "xive=off" kernel
parameter, in which case there is nothing to present to the user in the
associated /sys/kernel/debug/powerpc/xive file.

Fixes: 930914b7d528 ("powerpc/xive: Add a debugfs file to dump internal XIVE state")
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 arch/powerpc/sysdev/xive/common.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
index 3dbc94cb4380..f591be9f01f4 100644
--- a/arch/powerpc/sysdev/xive/common.c
+++ b/arch/powerpc/sysdev/xive/common.c
@@ -1664,7 +1664,8 @@ DEFINE_SHOW_ATTRIBUTE(xive_core_debug);
 
 int xive_core_debug_init(void)
 {
-	debugfs_create_file("xive", 0400, powerpc_debugfs_root,
-			    NULL, &xive_core_debug_fops);
+	if (xive_enabled())
+		debugfs_create_file("xive", 0400, powerpc_debugfs_root,
+				    NULL, &xive_core_debug_fops);
 	return 0;
 }
-- 
2.25.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] powerpc/pci: unmap legacy INTx interrupts of passthrough IO adapters
  2020-04-29  7:51 ` [PATCH 2/3] powerpc/pci: unmap legacy INTx interrupts of passthrough IO adapters Cédric Le Goater
@ 2020-05-21  7:13   ` Cédric Le Goater
  2020-05-27  0:57   ` Oliver O'Halloran
  2020-05-27 11:05   ` Cédric Le Goater
  2 siblings, 0 replies; 11+ messages in thread
From: Cédric Le Goater @ 2020-05-21  7:13 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Carol L Soto, Oliver O'Halloran, linuxppc-dev, Greg Kurz,
	Alexey Kardashevskiy

On 4/29/20 9:51 AM, Cédric Le Goater wrote:
> When a passthrough IO adapter is removed from a pseries machine using
> hash MMU and the XIVE interrupt mode, the POWER hypervisor, pHyp,
> expects the guest OS to have cleared all page table entries related to
> the adapter. If some are still present, the RTAS call which isolates
> the PCI slot returns error 9001 "valid outstanding translations" and
> the removal of the IO adapter fails.
> 
> INTx interrupt numbers need special care because Linux maps the
> interrupts automatically in the Linux interrupt number space if they
> are presented in the device tree node describing the IO adapter. These
> interrupts are not un-mapped automatically and in case of an hot-plug
> adapter, the PCI hot-plug layer needs to handle the cleanup to make
> sure that all the page table entries of the XIVE ESB pages are
> cleared.
> 
> Cc: "Oliver O'Halloran" <oohall@gmail.com>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>


We did some tests with differnt passthrough adapters under LPAPRs (PowerVM) 
and KVM guests P8 (HPT) and P9 (HPT+Radix). Baremetal behaves correctly. 
But I would feel more comfortable if someone could give a try (PATCH1-2) 
on a different system.


Thanks,

C. 


> ---
>  arch/powerpc/kernel/pci-hotplug.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/powerpc/kernel/pci-hotplug.c b/arch/powerpc/kernel/pci-hotplug.c
> index bf83f76563a3..9e9c6befd7ea 100644
> --- a/arch/powerpc/kernel/pci-hotplug.c
> +++ b/arch/powerpc/kernel/pci-hotplug.c
> @@ -57,6 +57,8 @@ void pcibios_release_device(struct pci_dev *dev)
>  	struct pci_controller *phb = pci_bus_to_host(dev->bus);
>  	struct pci_dn *pdn = pci_get_pdn(dev);
>  
> +	irq_dispose_mapping(dev->irq);
> +
>  	eeh_remove_device(dev);
>  
>  	if (phb->controller_ops.release_device)
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] powerpc/pci: unmap legacy INTx interrupts of passthrough IO adapters
  2020-04-29  7:51 ` [PATCH 2/3] powerpc/pci: unmap legacy INTx interrupts of passthrough IO adapters Cédric Le Goater
  2020-05-21  7:13   ` Cédric Le Goater
@ 2020-05-27  0:57   ` Oliver O'Halloran
  2020-05-27  7:31     ` Cédric Le Goater
  2020-06-10 18:10     ` Cédric Le Goater
  2020-05-27 11:05   ` Cédric Le Goater
  2 siblings, 2 replies; 11+ messages in thread
From: Oliver O'Halloran @ 2020-05-27  0:57 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: linuxppc-dev

On Wed, Apr 29, 2020 at 5:51 PM Cédric Le Goater <clg@kaod.org> wrote:
>
> When a passthrough IO adapter is removed from a pseries machine using
> hash MMU and the XIVE interrupt mode, the POWER hypervisor, pHyp,
> expects the guest OS to have cleared all page table entries related to
> the adapter. If some are still present, the RTAS call which isolates
> the PCI slot returns error 9001 "valid outstanding translations" and
> the removal of the IO adapter fails.
>
> INTx interrupt numbers need special care because Linux maps the
> interrupts automatically in the Linux interrupt number space if they
> are presented in the device tree node describing the IO adapter. These
> interrupts are not un-mapped automatically and in case of an hot-plug
> adapter, the PCI hot-plug layer needs to handle the cleanup to make
> sure that all the page table entries of the XIVE ESB pages are
> cleared.
>
> Cc: "Oliver O'Halloran" <oohall@gmail.com>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  arch/powerpc/kernel/pci-hotplug.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/powerpc/kernel/pci-hotplug.c b/arch/powerpc/kernel/pci-hotplug.c
> index bf83f76563a3..9e9c6befd7ea 100644
> --- a/arch/powerpc/kernel/pci-hotplug.c
> +++ b/arch/powerpc/kernel/pci-hotplug.c
> @@ -57,6 +57,8 @@ void pcibios_release_device(struct pci_dev *dev)
>         struct pci_controller *phb = pci_bus_to_host(dev->bus);
>         struct pci_dn *pdn = pci_get_pdn(dev);
>
> +       irq_dispose_mapping(dev->irq);

What does the original mapping? Powerpc arch code or the PCI core?
Tearing down the mapping in pcibios_release_device() seems a bit fishy
to me since the PCI core has already torn down the device state at
that point. If the release is delayed it's possible that another
pci_dev has mapped the IRQ before we get here, but maybe that's ok.

> +
>         eeh_remove_device(dev);
>
>         if (phb->controller_ops.release_device)
> --
> 2.25.4
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] powerpc/pci: unmap legacy INTx interrupts of passthrough IO adapters
  2020-05-27  0:57   ` Oliver O'Halloran
@ 2020-05-27  7:31     ` Cédric Le Goater
  2020-06-10 18:10     ` Cédric Le Goater
  1 sibling, 0 replies; 11+ messages in thread
From: Cédric Le Goater @ 2020-05-27  7:31 UTC (permalink / raw)
  To: Oliver O'Halloran; +Cc: linuxppc-dev

On 5/27/20 2:57 AM, Oliver O'Halloran wrote:
> On Wed, Apr 29, 2020 at 5:51 PM Cédric Le Goater <clg@kaod.org> wrote:
>>
>> When a passthrough IO adapter is removed from a pseries machine using
>> hash MMU and the XIVE interrupt mode, the POWER hypervisor, pHyp,
>> expects the guest OS to have cleared all page table entries related to
>> the adapter. If some are still present, the RTAS call which isolates
>> the PCI slot returns error 9001 "valid outstanding translations" and
>> the removal of the IO adapter fails.
>>
>> INTx interrupt numbers need special care because Linux maps the
>> interrupts automatically in the Linux interrupt number space if they
>> are presented in the device tree node describing the IO adapter. These
>> interrupts are not un-mapped automatically and in case of an hot-plug
>> adapter, the PCI hot-plug layer needs to handle the cleanup to make
>> sure that all the page table entries of the XIVE ESB pages are
>> cleared.
>>
>> Cc: "Oliver O'Halloran" <oohall@gmail.com>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  arch/powerpc/kernel/pci-hotplug.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/powerpc/kernel/pci-hotplug.c b/arch/powerpc/kernel/pci-hotplug.c
>> index bf83f76563a3..9e9c6befd7ea 100644
>> --- a/arch/powerpc/kernel/pci-hotplug.c
>> +++ b/arch/powerpc/kernel/pci-hotplug.c
>> @@ -57,6 +57,8 @@ void pcibios_release_device(struct pci_dev *dev)
>>         struct pci_controller *phb = pci_bus_to_host(dev->bus);
>>         struct pci_dn *pdn = pci_get_pdn(dev);
>>
>> +       irq_dispose_mapping(dev->irq);
> 
> What does the original mapping? Powerpc arch code or the PCI core?

Powerpc. In pci_read_irq_line() when a device is added.

> Tearing down the mapping in pcibios_release_device() seems a bit fishy
> to me since the PCI core has already torn down the device state at
> that point. If the release is delayed it's possible that another
> pci_dev has mapped the IRQ before we get here, but maybe that's ok.

Which scenario would that be ? multiple devices mapping the same INTx
interrupt because all are used already ? 

Where should we drop the mapping ?

Thanks,

C.

>> +
>>         eeh_remove_device(dev);
>>
>>         if (phb->controller_ops.release_device)
>> --
>> 2.25.4
>>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] powerpc/pci: unmap legacy INTx interrupts of passthrough IO adapters
  2020-04-29  7:51 ` [PATCH 2/3] powerpc/pci: unmap legacy INTx interrupts of passthrough IO adapters Cédric Le Goater
  2020-05-21  7:13   ` Cédric Le Goater
  2020-05-27  0:57   ` Oliver O'Halloran
@ 2020-05-27 11:05   ` Cédric Le Goater
  2020-05-28 13:25     ` Michael Ellerman
  2 siblings, 1 reply; 11+ messages in thread
From: Cédric Le Goater @ 2020-05-27 11:05 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: Oliver O'Halloran, linuxppc-dev

Hello Michael,

On 4/29/20 9:51 AM, Cédric Le Goater wrote:
> When a passthrough IO adapter is removed from a pseries machine using
> hash MMU and the XIVE interrupt mode, the POWER hypervisor, pHyp,
> expects the guest OS to have cleared all page table entries related to
> the adapter. If some are still present, the RTAS call which isolates
> the PCI slot returns error 9001 "valid outstanding translations" and
> the removal of the IO adapter fails.
> 
> INTx interrupt numbers need special care because Linux maps the
> interrupts automatically in the Linux interrupt number space if they
> are presented in the device tree node describing the IO adapter. These
> interrupts are not un-mapped automatically and in case of an hot-plug
> adapter, the PCI hot-plug layer needs to handle the cleanup to make
> sure that all the page table entries of the XIVE ESB pages are
> cleared.

It seems this patch needs more digging to make sure we are handling
the IRQ unmapping in the correct PCI handler. Could you please keep
it back for the moment ? 

Thanks,

C.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] powerpc/pci: unmap legacy INTx interrupts of passthrough IO adapters
  2020-05-27 11:05   ` Cédric Le Goater
@ 2020-05-28 13:25     ` Michael Ellerman
  0 siblings, 0 replies; 11+ messages in thread
From: Michael Ellerman @ 2020-05-28 13:25 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: Oliver O'Halloran, linuxppc-dev

Cédric Le Goater <clg@kaod.org> writes:
> On 4/29/20 9:51 AM, Cédric Le Goater wrote:
>> When a passthrough IO adapter is removed from a pseries machine using
>> hash MMU and the XIVE interrupt mode, the POWER hypervisor, pHyp,
>> expects the guest OS to have cleared all page table entries related to
>> the adapter. If some are still present, the RTAS call which isolates
>> the PCI slot returns error 9001 "valid outstanding translations" and
>> the removal of the IO adapter fails.
>> 
>> INTx interrupt numbers need special care because Linux maps the
>> interrupts automatically in the Linux interrupt number space if they
>> are presented in the device tree node describing the IO adapter. These
>> interrupts are not un-mapped automatically and in case of an hot-plug
>> adapter, the PCI hot-plug layer needs to handle the cleanup to make
>> sure that all the page table entries of the XIVE ESB pages are
>> cleared.
>
> It seems this patch needs more digging to make sure we are handling
> the IRQ unmapping in the correct PCI handler. Could you please keep
> it back for the moment ? 

Yep no worries.

cheers

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/3] powerpc/xive: PCI hotplug fixes under PowerVM
  2020-04-29  7:51 [PATCH 0/3] powerpc/xive: PCI hotplug fixes under PowerVM Cédric Le Goater
                   ` (2 preceding siblings ...)
  2020-04-29  7:51 ` [PATCH 3/3] powerpc/xive: Do not expose a debugfs file when XIVE is disabled Cédric Le Goater
@ 2020-06-09  5:28 ` Michael Ellerman
  3 siblings, 0 replies; 11+ messages in thread
From: Michael Ellerman @ 2020-06-09  5:28 UTC (permalink / raw)
  To: Cédric Le Goater, Michael Ellerman; +Cc: linuxppc-dev

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1022 bytes --]

On Wed, 29 Apr 2020 09:51:19 +0200, Cédric Le Goater wrote:
> Here are a couple of fixes for PCI hotplug issues for machines running
> under the POWER hypervisor using hash MMU and the XIVE interrupt mode.
> 
> Commit 1ca3dec2b2df ("powerpc/xive: Prevent page fault issues in the
> machine crash handler") forced the mapping of the XIVE ESB page and
> this is now blocking the removal of a passthrough IO adapter because
> the PCI isolation fails with "valid outstanding translations". Under
> KVM, the ESB pages for the adapter interrupts are un-mapped from the
> guest by the hypervisor in the KVM XIVE native device. This is is now
> redundant but it's harmless.
> 
> [...]

Patches 1 & 3 pplied to powerpc/next.

[1/3] powerpc/xive: Clear the page tables for the ESB IO mapping
      https://git.kernel.org/powerpc/c/a101950fcb78b0ba20cd487be6627dea58d55c2b
[3/3] powerpc/xive: Do not expose a debugfs file when XIVE is disabled
      https://git.kernel.org/powerpc/c/0755e85570a4615ca674ad6489d44d63916f1f3e

cheers

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] powerpc/pci: unmap legacy INTx interrupts of passthrough IO adapters
  2020-05-27  0:57   ` Oliver O'Halloran
  2020-05-27  7:31     ` Cédric Le Goater
@ 2020-06-10 18:10     ` Cédric Le Goater
  1 sibling, 0 replies; 11+ messages in thread
From: Cédric Le Goater @ 2020-06-10 18:10 UTC (permalink / raw)
  To: Oliver O'Halloran; +Cc: linuxppc-dev

On 5/27/20 2:57 AM, Oliver O'Halloran wrote:
> On Wed, Apr 29, 2020 at 5:51 PM Cédric Le Goater <clg@kaod.org> wrote:
>>
>> When a passthrough IO adapter is removed from a pseries machine using
>> hash MMU and the XIVE interrupt mode, the POWER hypervisor, pHyp,
>> expects the guest OS to have cleared all page table entries related to
>> the adapter. If some are still present, the RTAS call which isolates
>> the PCI slot returns error 9001 "valid outstanding translations" and
>> the removal of the IO adapter fails.
>>
>> INTx interrupt numbers need special care because Linux maps the
>> interrupts automatically in the Linux interrupt number space if they
>> are presented in the device tree node describing the IO adapter. These
>> interrupts are not un-mapped automatically and in case of an hot-plug
>> adapter, the PCI hot-plug layer needs to handle the cleanup to make
>> sure that all the page table entries of the XIVE ESB pages are
>> cleared.
>>
>> Cc: "Oliver O'Halloran" <oohall@gmail.com>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  arch/powerpc/kernel/pci-hotplug.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/powerpc/kernel/pci-hotplug.c b/arch/powerpc/kernel/pci-hotplug.c
>> index bf83f76563a3..9e9c6befd7ea 100644
>> --- a/arch/powerpc/kernel/pci-hotplug.c
>> +++ b/arch/powerpc/kernel/pci-hotplug.c
>> @@ -57,6 +57,8 @@ void pcibios_release_device(struct pci_dev *dev)
>>         struct pci_controller *phb = pci_bus_to_host(dev->bus);
>>         struct pci_dn *pdn = pci_get_pdn(dev);
>>
>> +       irq_dispose_mapping(dev->irq);
> 
> What does the original mapping? Powerpc arch code or the PCI core?
> Tearing down the mapping in pcibios_release_device() seems a bit fishy
> to me since the PCI core has already torn down the device state at
> that point. If the release is delayed it's possible that another
> pci_dev has mapped the IRQ before we get here, but maybe that's ok.

How's that below ? INTx mappings are cleared only when the PHB is removed.
It applies to all platforms but we could limit the removal to PHB hotplug
on pseries. 

C.


From 10794159567552355f87e86e24002641c54e7ab5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=A9dric=20Le=20Goater?= <clg@kaod.org>
Date: Wed, 10 Jun 2020 19:55:24 +0200
Subject: [PATCH] powerpc/pci: unmap legacy INTx interrupts of passthrough IO
 adapters
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When a passthrough IO adapter is removed from a pseries machine using
hash MMU and the XIVE interrupt mode, the POWER hypervisor, pHyp,
expects the guest OS to have cleared all page table entries related to
the adapter. If some are still present, the RTAS call which isolates
the PCI slot returns error 9001 "valid outstanding translations" and
the removal of the IO adapter fails.

INTx interrupt numbers need special care because Linux maps the
interrupts automatically in the Linux interrupt number space. These
interrupts are not un-mapped automatically and in case of an hot-plug
adapter, the PCI hot-plug layer needs to handle the cleanup to make
sure that all the page table entries of the ESB pages are cleared.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 arch/powerpc/include/asm/pci-bridge.h |  4 +++
 arch/powerpc/kernel/pci-common.c      | 47 +++++++++++++++++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
index b92e81b256e5..9960dd249079 100644
--- a/arch/powerpc/include/asm/pci-bridge.h
+++ b/arch/powerpc/include/asm/pci-bridge.h
@@ -48,6 +48,8 @@ struct pci_controller_ops {
 
 /*
  * Structure of a PCI controller (host bridge)
+ *
+ * @intx: legacy INTx mappings
  */
 struct pci_controller {
 	struct pci_bus *bus;
@@ -127,6 +129,8 @@ struct pci_controller {
 
 	void *private_data;
 	struct npu *npu;
+
+	unsigned int intx[PCI_NUM_INTX];
 };
 
 /* These are used for config access before all the PCI probing
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index be108616a721..795a9b49e0d6 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -353,6 +353,48 @@ struct pci_controller *pci_find_controller_for_domain(int domain_nr)
 	return NULL;
 }
 
+static void pci_intx_register(struct pci_dev *pdev, int virq)
+{
+	struct pci_controller *phb = pci_bus_to_host(pdev->bus);
+	int i;
+
+	for (i = 0; i < PCI_NUM_INTX; i++) {
+		/*
+		 * Look for an empty or an equivalent slot, IRQs can be
+		 * shared
+		 */
+		if (phb->intx[i] == virq || !phb->intx[i]) {
+			phb->intx[i] = virq;
+			break;
+		}
+	}
+
+	if (i == PCI_NUM_INTX)
+		pr_err("PCI:%s INTx all mapped\n", pci_name(pdev));
+}
+
+/*
+ * Clearing the mapped INTx interrupts will also clear the underlying
+ * mappings of the ESB pages of the interrupts when under XIVE. It is
+ * a requirement of PowerVM to clear all memory mappings before
+ * removing a PHB.
+ */
+static void pci_intx_dispose(struct pci_controller *phb)
+{
+	int i;
+
+	for (i = 0; i < PCI_NUM_INTX; i++)
+		irq_dispose_mapping(phb->intx[i]);
+}
+
+void pcibios_remove_bus(struct pci_bus *bus)
+{
+	pr_debug("PCI: Clearing PHB %04x:%02x...\n",
+		 pci_domain_nr(bus), bus->number);
+	pci_intx_dispose(pci_bus_to_host(bus));
+}
+EXPORT_SYMBOL_GPL(pcibios_remove_bus);
+
 /*
  * Reads the interrupt pin to determine if interrupt is use by card.
  * If the interrupt is used, then gets the interrupt line from the
@@ -401,6 +443,11 @@ static int pci_read_irq_line(struct pci_dev *pci_dev)
 
 	pci_dev->irq = virq;
 
+	/*
+	 * Record all INTx mappings to clear them if the PHB is
+	 * dynamically removed.
+	 */
+	pci_intx_register(pci_dev, virq);
 	return 0;
 }
 
-- 
2.25.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2020-06-10 20:42 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-29  7:51 [PATCH 0/3] powerpc/xive: PCI hotplug fixes under PowerVM Cédric Le Goater
2020-04-29  7:51 ` [PATCH 1/3] powerpc/xive: Clear the page tables for the ESB IO mapping Cédric Le Goater
2020-04-29  7:51 ` [PATCH 2/3] powerpc/pci: unmap legacy INTx interrupts of passthrough IO adapters Cédric Le Goater
2020-05-21  7:13   ` Cédric Le Goater
2020-05-27  0:57   ` Oliver O'Halloran
2020-05-27  7:31     ` Cédric Le Goater
2020-06-10 18:10     ` Cédric Le Goater
2020-05-27 11:05   ` Cédric Le Goater
2020-05-28 13:25     ` Michael Ellerman
2020-04-29  7:51 ` [PATCH 3/3] powerpc/xive: Do not expose a debugfs file when XIVE is disabled Cédric Le Goater
2020-06-09  5:28 ` [PATCH 0/3] powerpc/xive: PCI hotplug fixes under PowerVM Michael Ellerman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).