Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1243227 > unrolled thread

[PATCH] x86/PCI: Don't alloc pcibios-irq when MSI is enabled

Back to article view | Back to linux.kernel

Contents

  [PATCH] x86/PCI: Don't alloc pcibios-irq when MSI is enabled Joerg Roedel <joro@8bytes.org> - 2015-10-09 12:30 +0200
    [Bugfix v4 0/2] Prevent binding PCI drivers to PCI devices used by non-pci drivers Jiang Liu <jiang.liu@linux.intel.com> - 2015-10-09 16:10 +0200
      [Bugfix v4 1/2] iommu/amd: Prevent binding other PCI drivers to IOMMU PCI devices Jiang Liu <jiang.liu@linux.intel.com> - 2015-10-09 16:10 +0200
        Re: [Bugfix v4 1/2] iommu/amd: Prevent binding other PCI drivers to  IOMMU PCI devices Joerg Roedel <joro@8bytes.org> - 2015-10-09 18:00 +0200
      [Bugfix v4 2/2] ACPI, PCI: Prevent binding other PCI drivers to IOAPIC PCI devices Jiang Liu <jiang.liu@linux.intel.com> - 2015-10-09 16:10 +0200
        Re: [Bugfix v4 2/2] ACPI, PCI: Prevent binding other PCI drivers to  IOAPIC PCI devices Joerg Roedel <joro@8bytes.org> - 2015-10-09 17:50 +0200

#1243227 — [PATCH] x86/PCI: Don't alloc pcibios-irq when MSI is enabled

From: Joerg Roedel <jroedel@suse.de>

The pcibios-irq and MSI both use dev->irq to store the IRQ
number. While the MSI code checks for that and frees the
pcibios-irq before overwriting dev->irq, the
pcibios_alloc_irq function does not.

Usually this is not a problem, as the pcibios-irq is
allocated before probe time of the device and the MSI irq is
allocted from the drivers probe path.

But there are PCI devices handled by the core kernel and not
by a standard pci driver, like the AMD IOMMU for example.
For the AMD IOMMU a normal pci device driver does not make
sense, because a driver can be forcibly unbound from its
device, which is not a good idea for an IOMMU.

Nevertheless the PCI core code tries to match the PCI device
implementing the AMD IOMMU against drivers, and
allocates/frees a pcibios IRQ every time it tries out a new
driver. This overwrites the dev->irq field set by
pci_enable_msi() and sets it to 0 in the end (because the
probe fails and the pcibios-irq is freed again).

On suspend/resume this breaks the kernel, because the irq
descriptor for irq 0 is NULL.

Fix this by not allocating a pcibios-irq when MSI is
already active. This also has the benefit, that a device
claimed by the core kernel can not be probed by a pci driver
later.

Cc: Jiang Liu <jiang.liu@linux.intel.com>
Reported-by: Borislav Petkov <bp@alien8.de>
Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 arch/x86/pci/common.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index dc78a4a..6254c06 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -675,6 +675,14 @@ int pcibios_add_device(struct pci_dev *dev)
 
 int pcibios_alloc_irq(struct pci_dev *dev)
 {
+	/*
+	 * If the PCI device was already claimed by core code and has
+	 * MSI enabled, probing of the pcibios irq will overwrite
+	 * dev->irq.  So bail out if MSI is already enabled.
+	 */
+	if (pci_dev_msi_enabled(dev))
+		return -EBUSY;
+
 	return pcibios_enable_irq(dev);
 }
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1243397 — [Bugfix v4 0/2] Prevent binding PCI drivers to PCI devices used by non-pci drivers

Hi Joerg,
	I prepared this patchset yesterday but forgot to send them out.
I think it still worth sending out for review, so here we go:)

---
Commit 991de2e59090 ("PCI, x86: Implement pcibios_alloc_irq() and
pcibios_free_irq()") breaks suspend/resume on some AMD platforms.
The root cause is:
1) AMD IOMMU drivers enables MSI on IOMMU PCI devices at early boot stage
2) PCI driver binding code tries to allocate/free PCI legacy IRQ when
   binding other PCI drivers to IOMMU PCI devices at later boot stage,
   and breaks PCI MSI allocation information.
3) System resume breaks when restoring PCI MSI state using the damaged
   data.

We have tried on solution to detect that a PCI is in use by IOMMU driver
by checking pci_msi_enabled(). But that's too specific, actually we should
prevent binding PCI drivers to PCI devices used by non-PCI drivers.

Fortunately, we could prevent binding PCI drivers to PCI devices by setting
pci_dev->match_driver to false. If needed, we could implement a helper
function to manipulate pci_dev->match_driver.

Jiang Liu (2):
  iommu/amd: Prevent binding other PCI drivers to IOMMU PCI devices
  ACPI, PCI: Prevent binding other PCI drivers to IOAPIC PCI devices

 drivers/acpi/ioapic.c          |    7 +++++--
 drivers/iommu/amd_iommu_init.c |    3 +++
 2 files changed, 8 insertions(+), 2 deletions(-)

-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1243399 — [Bugfix v4 1/2] iommu/amd: Prevent binding other PCI drivers to IOMMU PCI devices

AMD IOMMU driver makes use of IOMMU PCI devices, so prevent binding other
PCI drivers to IOMMU PCI devices.

This fixes a bug reported by Boris that system suspend/resume gets broken
on AMD platforms. For more information, please refer to:
	https://lkml.org/lkml/2015/9/26/89

Fixes: 991de2e59090 ("PCI, x86: Implement pcibios_alloc_irq() and pcibios_free_irq()")
Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
Cc: Borislav Petkov <bp@alien8.de>
---
 drivers/iommu/amd_iommu_init.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 5ef347a13cb5..1b066e7d144d 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -1256,6 +1256,9 @@ static int iommu_init_pci(struct amd_iommu *iommu)
 	if (!iommu->dev)
 		return -ENODEV;
 
+	/* Prevent binding other PCI device drivers to IOMMU devices */
+	iommu->dev->match_driver = false;
+
 	pci_read_config_dword(iommu->dev, cap_ptr + MMIO_CAP_HDR_OFFSET,
 			      &iommu->cap);
 	pci_read_config_dword(iommu->dev, cap_ptr + MMIO_RANGE_OFFSET,
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1243494 — Re: [Bugfix v4 1/2] iommu/amd: Prevent binding other PCI drivers to IOMMU PCI devices

On Fri, Oct 09, 2015 at 10:07:31PM +0800, Jiang Liu wrote:
> AMD IOMMU driver makes use of IOMMU PCI devices, so prevent binding other
> PCI drivers to IOMMU PCI devices.
> 
> This fixes a bug reported by Boris that system suspend/resume gets broken
> on AMD platforms. For more information, please refer to:
> 	https://lkml.org/lkml/2015/9/26/89
> 
> Fixes: 991de2e59090 ("PCI, x86: Implement pcibios_alloc_irq() and pcibios_free_irq()")
> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> Cc: Borislav Petkov <bp@alien8.de>

Applied to iommu/fixes, thanks Jiang.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1243400 — [Bugfix v4 2/2] ACPI, PCI: Prevent binding other PCI drivers to IOAPIC PCI devices

The ACPI IOAPIC driver makes use of IOAPIC PCI devices, so prevent
binding other PCI drivers to IOAPIC PCI devices used by ACPI IOAPIC
driver.

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 drivers/acpi/ioapic.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/ioapic.c b/drivers/acpi/ioapic.c
index ccdc8db16bb8..da71b274fc21 100644
--- a/drivers/acpi/ioapic.c
+++ b/drivers/acpi/ioapic.c
@@ -162,12 +162,14 @@ done:
 	list_add(&ioapic->list, &ioapic_list);
 	mutex_unlock(&ioapic_list_lock);
 
-	if (dev)
+	if (dev) {
+		dev->match_driver = false;
 		dev_info(&dev->dev, "%s at %pR, GSI %u\n",
 			 type, res, (u32)gsi_base);
-	else
+	} else {
 		acpi_handle_info(handle, "%s at %pR, GSI %u\n",
 				 type, res, (u32)gsi_base);
+	}
 
 	return AE_OK;
 
@@ -216,6 +218,7 @@ int acpi_ioapic_remove(struct acpi_pci_root *root)
 		if (ioapic->pdev) {
 			pci_release_region(ioapic->pdev, 0);
 			pci_disable_device(ioapic->pdev);
+			ioapic->pdev->match_driver = true;
 			pci_dev_put(ioapic->pdev);
 		} else if (ioapic->res.flags && ioapic->res.parent) {
 			release_resource(&ioapic->res);
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1243482 — Re: [Bugfix v4 2/2] ACPI, PCI: Prevent binding other PCI drivers to IOAPIC PCI devices

On Fri, Oct 09, 2015 at 10:07:32PM +0800, Jiang Liu wrote:
> The ACPI IOAPIC driver makes use of IOAPIC PCI devices, so prevent
> binding other PCI drivers to IOAPIC PCI devices used by ACPI IOAPIC
> driver.
> 
> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> ---
>  drivers/acpi/ioapic.c |    7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)

Reviewed-by: Joerg Roedel <jroedel@suse.de>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web