Reset PCIe devices to address DMA problem on kdump with iommu

PATCH Superseded Takao Indoh 2 participants 8 messages Nov 13, 2012

Subsystems (4)

x86 PCI Live update kexec handover (KHO)

lore

Share & tools

lore

Takao Indoh

author

2012-11-13 09:07 UTC #0 lore raw

v6 cover Reset PCIe devices to address DMA problem on kdump with iommu

These patches reset PCIe devices at boot time to address DMA problem on kdump with iommu. When "reset_devices" is specified, a hot reset is triggered on each PCIe root port and downstream port to reset its downstream endpoint.

Background:
A kdump problem about DMA has been discussed for a long time. That is, when a kernel is switched to the kdump kernel, DMA derived from first kernel affects second kernel. Especially this problem surfaces when iommu is used for PCI passthrough on KVM guest. In the case of the machine I use, when intel_iommu=on is specified, DMAR error is detected in kdump kernel and PCI SERR is also detected. Finally kdump fails because some devices does not work correctly.

The root cause is that ongoing DMA from first kernel causes DMAR fault because page table of DMAR is initialized while kdump kernel is booting up. Therefore to solve this problem DMA needs to be stopped before DMAR is initialized at kdump kernel boot time. By these patches, PCIe devices are reset by hot reset and its DMA is stopped when reset_devices is specified. One problem of this solution is that the monitor blacks out when VGA controller is reset. So this patch does not reset the port whose child endpoint is VGA device.

What I tried:
- Clearing bus master bit and INTx disable bit at boot time This did not solve this problem. I still got DMAR error on devices.
- Resetting devices in fixup_final(v1 patch)
DMAR error disappeared, but sometimes PCI SERR was detected. This is well explained here.
https://lkml.org/lkml/2012/9/9/245
This PCI SERR seems to be related to interrupt remapping.
- Clearing bus master in setup_arch() and resetting devices in fixup_final
Neither DMAR error nor PCI SERR occurred. But on certain machine kdump kernel hung up when resetting devices. It seems to be a problem specific to the platform.
- Resetting devices in setup_arch() (v2 and later patch) This solution solves all problems I found so far.

Changelog:
v6:
Rewrite using Yinghai's dummy-pci patch

v5:
Do bus reset after all devices are scanned and its config registers are saved. This fixes a bug that config register is accessed without delay after reset.
https://lkml.org/lkml/2012/10/17/47

v4:
Reduce waiting time after resetting devices. A previous patch does reset like this:
for (each device) {
save config registers
reset
wait for 500 ms
restore config registers
}

If there are N devices to be reset, it takes N*500 ms. On the other hand, the v4 patch does:
for (each device) {
save config registers
reset
}
wait 500 ms
for (each device) {
restore config registers
}
Though it needs more memory space to save config registers, the waiting time is always 500ms.
https://lkml.org/lkml/2012/10/15/49

v3:
Move alloc_bootmem and free_bootmem to early_reset_pcie_devices so that they are called only once.
https://lkml.org/lkml/2012/10/10/57

v2:
Reset devices in setup_arch() because reset need to be done before interrupt remapping is initialized.
https://lkml.org/lkml/2012/10/2/54

v1:
Add fixup_final quirk to reset PCIe devices
https://lkml.org/lkml/2012/8/3/160

Takao Indoh (4):
PCI: Define the maximum number of PCI function
Make reset_devices available at early stage
x86, pci: Reset PCIe devices at boot time
x86, pci: Enable PCI INTx when MSI is disabled

Yinghai Lu (1):
x86, pci: add dummy pci device for early stage

 arch/x86/include/asm/pci-direct.h |    3 +
 arch/x86/kernel/setup.c           |    3 +
 arch/x86/pci/common.c             |    4 +-
 arch/x86/pci/early.c              |  303 +++++++++++++++++++++++++++++++++++++
 include/linux/pci.h               |    2 +
 init/main.c                       |    4 +-
 6 files changed, 316 insertions(+), 3 deletions(-)

Takao Indoh

author

2012-11-13 09:07 UTC #1 lore raw

PATCH v6 1/5 x86, pci: add dummy pci device for early stage

From: Yinghai Lu <[email protected]>

So we can pass pci_dev *dev to reuse some generic pci functions.

Signed-off-by Yinghai Lu <[email protected]>

Signed-off-by Takao Indoh <[email protected]>

---
 arch/x86/include/asm/pci-direct.h |    2 +
 arch/x86/pci/early.c              |   75 +++++++++++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+), 0 deletions(-)

arch/x86/include/asm/pci-direct.h +2 -0

index b1e7a45..b6360d3 100644
@@ -18,4 +18,6 @@ extern int early_pci_allowed(void);
 extern unsigned int pci_early_dump_regs;
 extern void early_dump_pci_device(u8 bus, u8 slot, u8 func);
 extern void early_dump_pci_devices(void);
+
+struct pci_dev *get_early_pci_dev(int num, int slot, int func);
 #endif /* _ASM_X86_PCI_DIRECT_H */

arch/x86/pci/early.c +75 -0

78 lines not on this page · raw message

Signature · 1 line

1.7.1

Andrew Murray

2012-11-13 10:01 UTC #2 lore raw

Re: [PATCH v6 1/5] x86, pci: add dummy pci device for early stage

Hello,

Some comments inline...

Quoted 30 lines · Takao Indoh

From: Yinghai Lu <[email protected]>

So we can pass pci_dev *dev to reuse some generic pci functions.

Signed-off-by Yinghai Lu <[email protected]>

Signed-off-by Takao Indoh <[email protected]>

---
 arch/x86/include/asm/pci-direct.h |    2 +
 arch/x86/pci/early.c              |   75 +++++++++++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+), 0 deletions(-)

arch/x86/include/asm/pci-direct.h +2 -0

index b1e7a45..b6360d3 100644
@@ -18,4 +18,6 @@ extern int early_pci_allowed(void);
 extern unsigned int pci_early_dump_regs;
 extern void early_dump_pci_device(u8 bus, u8 slot, u8 func);
 extern void early_dump_pci_devices(void);
+
+struct pci_dev *get_early_pci_dev(int num, int slot, int func);
 #endif /* _ASM_X86_PCI_DIRECT_H */

arch/x86/pci/early.c +0 -0

index d1067d5..aea6b2b 100644
@@ -109,3 +109,78 @@ void early_dump_pci_devices(void)
                }
        }
 }

arch/x86/pci/early.c void early_dump_pci_devices(void) · line 121

Quoted 10 lines

arch/x86/pci/early.c +10 -0

+
+static __init int
+early_pci_read(struct pci_bus *bus, unsigned int devfn, int where,
+                       int size, u32 *value)
+{
+       int num, slot, func;
+
+       num = bus->number;
+       slot = devfn >> 3;
+       func = devfn & 7;

You may want to use the PCI_SLOT and PCI_FUNC macros in include/linux/pci.h to determine values for slot and func.

Quoted 14 lines

arch/x86/pci/early.c +14 -0

+       switch (size) {
+       case 1:
+               *value = read_pci_config_byte(num, slot, func, where);
+               break;
+       case 2:
+               *value = read_pci_config_16(num, slot, func, where);
+               break;
+       case 4:
+               *value = read_pci_config(num, slot, func, where);
+               break;
+       }
+
+       return 0;
+}

arch/x86/pci/early.c void early_dump_pci_devices(void) · line 145

Quoted 10 lines

arch/x86/pci/early.c +10 -0

+
+static __init int
+early_pci_write(struct pci_bus *bus, unsigned int devfn, int where,
+                       int size, u32 value)
+{
+       int num, slot, func;
+
+       num = bus->number;
+       slot = devfn >> 3;
+       func = devfn & 7;

As above.

Quoted 26 lines

arch/x86/pci/early.c +26 -0

+       switch (size) {
+       case 1:
+               write_pci_config_byte(num, slot, func, where, (u8)value);
+               break;
+       case 2:
+               write_pci_config_16(num, slot, func, where, (u16)value);
+               break;
+       case 4:
+               write_pci_config(num, slot, func, where, (u32)value);
+               break;
+       }
+
+       return 0;
+}
+
+static __initdata struct pci_ops pci_early_ops = {
+       .read  = early_pci_read,
+       .write = early_pci_write,
+};
+static __initdata struct pci_bus pci_early_bus = {
+       .ops = &pci_early_ops,
+};
+static __initdata char pci_early_init_name[8];
+static __initdata struct pci_dev pci_early_dev = {
+       .bus = &pci_early_bus,
+       .dev = {

arch/x86/pci/early.c void early_dump_pci_devices(void) · line 181

Quoted 10 lines

arch/x86/pci/early.c +10 -0

+               .init_name = pci_early_init_name,
+       },
+};
+
+__init struct pci_dev *get_early_pci_dev(int num, int slot, int func)
+{
+       struct pci_dev *pdev;
+
+       pdev = &pci_early_dev;
+       pdev->devfn = (slot<<3) | (func & 7);

You can use PCI_DEVFN here.

Quoted 9 lines

arch/x86/pci/early.c +5 -1

+       pdev->bus->number = num;
+       sprintf((char *)pdev->dev.init_name, "%02x:%02x.%01x", num, slot, func);
+
+       return pdev;
+}
--

1.7.1

Quoted 4 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Andrew Murray

Takao Indoh

author

2012-11-13 11:03 UTC #3 lore raw

Re: [PATCH v6 1/5] x86, pci: add dummy pci device for early stage

Quoted 106 lines · Andrew Murray

Hello,

Some comments inline...

Quoted 30 lines · Takao Indoh

From: Yinghai Lu <[email protected]>

So we can pass pci_dev *dev to reuse some generic pci functions.

Signed-off-by Yinghai Lu <[email protected]>

Signed-off-by Takao Indoh <[email protected]>

---
  arch/x86/include/asm/pci-direct.h |    2 +
  arch/x86/pci/early.c              |   75 +++++++++++++++++++++++++++++++++++++
  2 files changed, 77 insertions(+), 0 deletions(-)

arch/x86/include/asm/pci-direct.h +2 -0

index b1e7a45..b6360d3 100644
@@ -18,4 +18,6 @@ extern int early_pci_allowed(void);
  extern unsigned int pci_early_dump_regs;
  extern void early_dump_pci_device(u8 bus, u8 slot, u8 func);
  extern void early_dump_pci_devices(void);
+
+struct pci_dev *get_early_pci_dev(int num, int slot, int func);
  #endif /* _ASM_X86_PCI_DIRECT_H */

arch/x86/pci/early.c +0 -0

index d1067d5..aea6b2b 100644
@@ -109,3 +109,78 @@ void early_dump_pci_devices(void)
                 }
         }
  }

arch/x86/pci/early.c void early_dump_pci_devices(void) · line 121

Quoted 10 lines

arch/x86/pci/early.c +10 -0

+
+static __init int
+early_pci_read(struct pci_bus *bus, unsigned int devfn, int where,
+                       int size, u32 *value)
+{
+       int num, slot, func;
+
+       num = bus->number;
+       slot = devfn >> 3;
+       func = devfn & 7;

You may want to use the PCI_SLOT and PCI_FUNC macros in include/linux/pci.h to determine values for slot and func.

Quoted 14 lines

arch/x86/pci/early.c +14 -0

+       switch (size) {
+       case 1:
+               *value = read_pci_config_byte(num, slot, func, where);
+               break;
+       case 2:
+               *value = read_pci_config_16(num, slot, func, where);
+               break;
+       case 4:
+               *value = read_pci_config(num, slot, func, where);
+               break;
+       }
+
+       return 0;
+}

arch/x86/pci/early.c void early_dump_pci_devices(void) · line 145

Quoted 10 lines

arch/x86/pci/early.c +10 -0

+
+static __init int
+early_pci_write(struct pci_bus *bus, unsigned int devfn, int where,
+                       int size, u32 value)
+{
+       int num, slot, func;
+
+       num = bus->number;
+       slot = devfn >> 3;
+       func = devfn & 7;

As above.

Quoted 30 lines

arch/x86/pci/early.c +30 -0

+       switch (size) {
+       case 1:
+               write_pci_config_byte(num, slot, func, where, (u8)value);
+               break;
+       case 2:
+               write_pci_config_16(num, slot, func, where, (u16)value);
+               break;
+       case 4:
+               write_pci_config(num, slot, func, where, (u32)value);
+               break;
+       }
+
+       return 0;
+}
+
+static __initdata struct pci_ops pci_early_ops = {
+       .read  = early_pci_read,
+       .write = early_pci_write,
+};
+static __initdata struct pci_bus pci_early_bus = {
+       .ops = &pci_early_ops,
+};
+static __initdata char pci_early_init_name[8];
+static __initdata struct pci_dev pci_early_dev = {
+       .bus = &pci_early_bus,
+       .dev = {
+               .init_name = pci_early_init_name,
+       },
+};
+

Quoted 8 lines

arch/x86/pci/early.c void early_dump_pci_devices(void) · line 181
Quoted 6 lines
arch/x86/pci/early.c +6 -0
+__init struct pci_dev *get_early_pci_dev(int num, int slot, int func)
+{
+       struct pci_dev *pdev;
+
+       pdev = &pci_early_dev;
+       pdev->devfn = (slot<<3) | (func & 7);
You can use PCI_DEVFN here.

Yeah, I forgot to use these macros. I'll fix them, thanks!

Thanks,
Takao Indoh

Quoted 18 lines

Quoted 10 lines
arch/x86/pci/early.c +5 -1
+       pdev->bus->number = num;
+       sprintf((char *)pdev->dev.init_name, "%02x:%02x.%01x", num, slot, func);
+
+       return pdev;
+}
--
1.7.1
Quoted 4 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Andrew Murray

Takao Indoh

author

2012-11-13 09:07 UTC #4 lore raw

PATCH v6 2/5 PCI: Define the maximum number of PCI function

In reply to Takao Indoh

Define the maximum number of PCI function so that PCI functions can be enumerated without using "8".

Signed-off-by Takao Indoh <[email protected]>

---
 include/linux/pci.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

include/linux/pci.h +2 -0

index ee21795..eca3231 100644
@@ -35,6 +35,8 @@
 /* Include the ID list */
 #include <linux/pci_ids.h>
 
+#define PCI_MAX_FUNCTIONS 8
+
 /* pci_slot represents a physical slot */
 struct pci_slot {
 	struct pci_bus *bus;		/* The bus this slot is on */

Signature · 1 line

1.7.1

Takao Indoh

author

2012-11-13 09:07 UTC #5 lore raw

PATCH v6 3/5 Make reset_devices available at early stage

In reply to Takao Indoh

Change reset_devices from __setup to early_param so this parameter is available at early stage.

Signed-off-by Takao Indoh <[email protected]>

---
 init/main.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

init/main.c +2 -2

index e33e09d..f2b24cb 100644
@@ -144,10 +144,10 @@ EXPORT_SYMBOL(reset_devices);
 static int __init set_reset_devices(char *str)
 {
 	reset_devices = 1;
-	return 1;
+	return 0;
 }
 
-__setup("reset_devices", set_reset_devices);
+early_param("reset_devices", set_reset_devices);
 
 static const char * argv_init[MAX_INIT_ARGS+2] = { "init", NULL, };
 const char * envp_init[MAX_INIT_ENVS+2] = { "HOME=/", "TERM=linux", NULL, };

Signature · 1 line

1.7.1

Takao Indoh

author

2012-11-13 09:08 UTC #6 lore raw

PATCH v6 4/5 x86, pci: Reset PCIe devices at boot time

In reply to Takao Indoh

This patch resets PCIe devices at boot time by hot reset when "reset_devices" is specified.

Kdump with intel_iommu=on fails becasue ongoing DMA from first kernel causes DMAR fault when page table of DMAR is initialized while kdump kernel is booting up. To solve this problem, this patch resets PCIe devices by hot reset and its DMA is stopped when reset_devices is specified.

Signed-off-by Takao Indoh <[email protected]>

---
 arch/x86/include/asm/pci-direct.h |    1 +
 arch/x86/kernel/setup.c           |    3 +
 arch/x86/pci/early.c              |  228 +++++++++++++++++++++++++++++++++++++
 3 files changed, 232 insertions(+), 0 deletions(-)

arch/x86/include/asm/pci-direct.h +1 -0

index b6360d3..5620070 100644
@@ -18,6 +18,7 @@ extern int early_pci_allowed(void);
 extern unsigned int pci_early_dump_regs;
 extern void early_dump_pci_device(u8 bus, u8 slot, u8 func);
 extern void early_dump_pci_devices(void);
+extern void early_reset_pcie_devices(void);
 
 struct pci_dev *get_early_pci_dev(int num, int slot, int func);
 #endif /* _ASM_X86_PCI_DIRECT_H */

arch/x86/kernel/setup.c +3 -0

index ca45696..2e7928e 100644
@@ -1001,6 +1001,9 @@ void __init setup_arch(char **cmdline_p)
 	generic_apic_probe();
 
 	early_quirks();
+#ifdef CONFIG_PCI
+	early_reset_pcie_devices();
+#endif
 
 	/*
 	 * Read APIC and some other early information from ACPI tables.

arch/x86/pci/early.c +228 -0

236 lines not on this page · raw message

Signature · 1 line

1.7.1

Takao Indoh

author

2012-11-13 09:08 UTC #7 lore raw

PATCH v6 5/5 x86, pci: Enable PCI INTx when MSI is disabled

In reply to Takao Indoh

This patch enables INTx if MSI is disabled in pcibios_enable_device(). In normal case interrupt disable bit in command register is 0b on boot time, but in case of kdump, this bit may be 1b. It causes problems of some drivers. At leaset I confirmed mptsas driver does not work in such a case. This patch fix this problem.

Signed-off-by Takao Indoh <[email protected]>

---
 arch/x86/pci/common.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

arch/x86/pci/common.c +3 -1

index 720e973..2bb7ecc 100644
@@ -615,8 +615,10 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
 	if ((err = pci_enable_resources(dev, mask)) < 0)
 		return err;
 
-	if (!pci_dev_msi_enabled(dev))
+	if (!pci_dev_msi_enabled(dev)) {
+		pci_intx(dev, true);
 		return pcibios_enable_irq(dev);
+	}
 	return 0;
 }

Signature · 1 line

1.7.1