All of lore.kernel.org
 help / color / mirror / Atom feed
From: Takao Indoh <indou.takao@jp.fujitsu.com>
To: kexec@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-pci@vger.kernel.org
Cc: bhelgaas@google.com, vgoyal@redhat.com, hbabu@us.ibm.com,
	ishii.hironobu@jp.fujitsu.com, martin.wilck@ts.fujitsu.com
Subject: [RFC][PATCH] Reset PCIe devices to address DMA problem on kdump with iommu
Date: Fri, 03 Aug 2012 20:24:31 +0900	[thread overview]
Message-ID: <501BB4EF.7080909@jp.fujitsu.com> (raw)

Hi all,

This patch adds kernel parameter "reset_pcie_devices" which resets PCIe
devices at boot time to address DMA problem on kdump with iommu. When
this parameter 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. Recently 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 address this problem DMA needs to be stopped before DMAR
is initialized at kdump kernel boot time. By this patch, PCIe devices
are reset by hot reset and its DMA is stopped when reset_pcie_devices is
specified. One problem of this solution is that VGA is reset and the
monitor blacks out when the link between the port and VGA controller was
reset. So this patch does not reset the port whose child endpoint is VGA
device.

Any comments would be appreciated.

Signed-off-by: Takao Indoh <indou.takao@jp.fujitsu.com>
---
 Documentation/kernel-parameters.txt |    4 +
 drivers/pci/quirks.c                |   59 ++++++++++++++++++++++++++
 2 files changed, 63 insertions(+)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index e714a02..e694e9f 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2489,6 +2489,10 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 	reset_devices	[KNL] Force drivers to reset the underlying device
 			during initialization.
 
+	reset_pcie_devices
+			[PCIE] Reset PCIe endpoint at boot time by sending a
+			hot reset to root port and downstream port
+
 	resume=		[SWSUSP]
 			Specify the partition device for software suspend
 			Format:
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 5155317..7f7fc02 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -32,6 +32,65 @@
 #include "pci.h"
 
 /*
+ * Reset PCIe endpoint by sending a hot reset to root port and downstream port
+ */
+unsigned int reset_pcie_devices;
+EXPORT_SYMBOL(reset_pcie_devices);
+static int __init set_reset_pcie_devices(char *str)
+{
+	reset_pcie_devices = 1;
+	return 1;
+}
+__setup("reset_pcie_devices", set_reset_pcie_devices);
+
+static void __devinit quirk_pcie_device_reset(struct pci_dev *dev)
+{
+	struct pci_bus *subordinate;
+	struct pci_dev *child;
+	u16 ctrl;
+
+	if (!reset_pcie_devices || !pci_is_pcie(dev) || !dev->subordinate ||
+	    ((dev->pcie_type != PCI_EXP_TYPE_ROOT_PORT) &&
+	     (dev->pcie_type != PCI_EXP_TYPE_DOWNSTREAM)))
+		return;
+
+	subordinate = dev->subordinate;
+	list_for_each_entry(child, &subordinate->devices, bus_list) {
+		if ((child->pcie_type == PCI_EXP_TYPE_UPSTREAM) ||
+		    (child->pcie_type == PCI_EXP_TYPE_PCI_BRIDGE) ||
+		    ((child->class >> 16) == PCI_BASE_CLASS_DISPLAY))
+		/* Don't reset switch, bridge, VGA device */
+		return;
+	}
+
+	dev_info(&dev->dev, "Reset Secondary bus\n");
+
+	list_for_each_entry(child, &subordinate->devices, bus_list) {
+		dev_info(&child->dev, "save state\n");
+		pci_save_state(child);
+	}
+
+	/* Assert Secondary Bus Reset */
+	pci_read_config_word(dev, PCI_BRIDGE_CONTROL, &ctrl);
+	ctrl |= PCI_BRIDGE_CTL_BUS_RESET;
+	pci_write_config_word(dev, PCI_BRIDGE_CONTROL, ctrl);
+
+	msleep(2);
+
+	/* De-assert Secondary Bus Reset */
+	ctrl &= ~PCI_BRIDGE_CTL_BUS_RESET;
+	pci_write_config_word(dev, PCI_BRIDGE_CONTROL, ctrl);
+
+	msleep(200);
+
+	list_for_each_entry(child, &subordinate->devices, bus_list) {
+		dev_info(&child->dev, "restore state\n");
+		pci_restore_state(child);
+	}
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_ANY_ID, PCI_ANY_ID, quirk_pcie_device_reset);
+
+/*
  * Decoding should be disabled for a PCI device during BAR sizing to avoid
  * conflict. But doing so may cause problems on host bridge and perhaps other
  * key system devices. For devices that need to have mmio decoding always-on,


WARNING: multiple messages have this Message-ID (diff)
From: Takao Indoh <indou.takao@jp.fujitsu.com>
To: kexec@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-pci@vger.kernel.org
Cc: bhelgaas@google.com, hbabu@us.ibm.com,
	martin.wilck@ts.fujitsu.com, ishii.hironobu@jp.fujitsu.com,
	vgoyal@redhat.com
Subject: [RFC][PATCH] Reset PCIe devices to address DMA problem on kdump with iommu
Date: Fri, 03 Aug 2012 20:24:31 +0900	[thread overview]
Message-ID: <501BB4EF.7080909@jp.fujitsu.com> (raw)

Hi all,

This patch adds kernel parameter "reset_pcie_devices" which resets PCIe
devices at boot time to address DMA problem on kdump with iommu. When
this parameter 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. Recently 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 address this problem DMA needs to be stopped before DMAR
is initialized at kdump kernel boot time. By this patch, PCIe devices
are reset by hot reset and its DMA is stopped when reset_pcie_devices is
specified. One problem of this solution is that VGA is reset and the
monitor blacks out when the link between the port and VGA controller was
reset. So this patch does not reset the port whose child endpoint is VGA
device.

Any comments would be appreciated.

Signed-off-by: Takao Indoh <indou.takao@jp.fujitsu.com>
---
 Documentation/kernel-parameters.txt |    4 +
 drivers/pci/quirks.c                |   59 ++++++++++++++++++++++++++
 2 files changed, 63 insertions(+)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index e714a02..e694e9f 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2489,6 +2489,10 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 	reset_devices	[KNL] Force drivers to reset the underlying device
 			during initialization.
 
+	reset_pcie_devices
+			[PCIE] Reset PCIe endpoint at boot time by sending a
+			hot reset to root port and downstream port
+
 	resume=		[SWSUSP]
 			Specify the partition device for software suspend
 			Format:
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 5155317..7f7fc02 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -32,6 +32,65 @@
 #include "pci.h"
 
 /*
+ * Reset PCIe endpoint by sending a hot reset to root port and downstream port
+ */
+unsigned int reset_pcie_devices;
+EXPORT_SYMBOL(reset_pcie_devices);
+static int __init set_reset_pcie_devices(char *str)
+{
+	reset_pcie_devices = 1;
+	return 1;
+}
+__setup("reset_pcie_devices", set_reset_pcie_devices);
+
+static void __devinit quirk_pcie_device_reset(struct pci_dev *dev)
+{
+	struct pci_bus *subordinate;
+	struct pci_dev *child;
+	u16 ctrl;
+
+	if (!reset_pcie_devices || !pci_is_pcie(dev) || !dev->subordinate ||
+	    ((dev->pcie_type != PCI_EXP_TYPE_ROOT_PORT) &&
+	     (dev->pcie_type != PCI_EXP_TYPE_DOWNSTREAM)))
+		return;
+
+	subordinate = dev->subordinate;
+	list_for_each_entry(child, &subordinate->devices, bus_list) {
+		if ((child->pcie_type == PCI_EXP_TYPE_UPSTREAM) ||
+		    (child->pcie_type == PCI_EXP_TYPE_PCI_BRIDGE) ||
+		    ((child->class >> 16) == PCI_BASE_CLASS_DISPLAY))
+		/* Don't reset switch, bridge, VGA device */
+		return;
+	}
+
+	dev_info(&dev->dev, "Reset Secondary bus\n");
+
+	list_for_each_entry(child, &subordinate->devices, bus_list) {
+		dev_info(&child->dev, "save state\n");
+		pci_save_state(child);
+	}
+
+	/* Assert Secondary Bus Reset */
+	pci_read_config_word(dev, PCI_BRIDGE_CONTROL, &ctrl);
+	ctrl |= PCI_BRIDGE_CTL_BUS_RESET;
+	pci_write_config_word(dev, PCI_BRIDGE_CONTROL, ctrl);
+
+	msleep(2);
+
+	/* De-assert Secondary Bus Reset */
+	ctrl &= ~PCI_BRIDGE_CTL_BUS_RESET;
+	pci_write_config_word(dev, PCI_BRIDGE_CONTROL, ctrl);
+
+	msleep(200);
+
+	list_for_each_entry(child, &subordinate->devices, bus_list) {
+		dev_info(&child->dev, "restore state\n");
+		pci_restore_state(child);
+	}
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_ANY_ID, PCI_ANY_ID, quirk_pcie_device_reset);
+
+/*
  * Decoding should be disabled for a PCI device during BAR sizing to avoid
  * conflict. But doing so may cause problems on host bridge and perhaps other
  * key system devices. For devices that need to have mmio decoding always-on,


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

             reply	other threads:[~2012-08-03 11:28 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-03 11:24 Takao Indoh [this message]
2012-08-03 11:24 ` [RFC][PATCH] Reset PCIe devices to address DMA problem on kdump with iommu Takao Indoh
2012-08-03 11:46 ` Vivek Goyal
2012-08-03 11:46   ` Vivek Goyal
2012-08-06  4:30   ` Takao Indoh
2012-08-06  4:30     ` Takao Indoh
2012-08-06 20:39     ` Vivek Goyal
2012-08-06 20:39       ` Vivek Goyal
2012-08-07  9:02       ` Takao Indoh
2012-08-07  9:02         ` Takao Indoh
2012-09-05 11:09       ` Takao Indoh
2012-09-05 11:09         ` Takao Indoh
2012-09-10  2:34         ` Kaneshige, Kenji
2012-09-10  2:34           ` Kaneshige, Kenji
2012-09-10  6:35           ` Takao Indoh
2012-09-10  6:35             ` Takao Indoh
2012-09-11 11:52             ` Kaneshige, Kenji
2012-09-11 11:52               ` Kaneshige, Kenji
2012-09-10 14:36         ` Vivek Goyal
2012-09-10 14:36           ` Vivek Goyal
2012-09-11 10:32           ` Takao Indoh
2012-09-11 10:32             ` Takao Indoh
2012-09-11 14:43             ` Vivek Goyal
2012-09-11 14:43               ` Vivek Goyal
2012-09-12  9:00               ` Takao Indoh
2012-09-12  9:00                 ` Takao Indoh
2012-09-14 15:48                 ` Vivek Goyal
2012-09-14 15:48                   ` Vivek Goyal
2012-09-24 11:22                   ` Takao Indoh
2012-09-24 11:22                     ` Takao Indoh
2012-09-14 20:03             ` Konrad Rzeszutek Wilk
2012-09-14 20:03               ` Konrad Rzeszutek Wilk
2012-09-19  1:52               ` Takao Indoh
2012-09-19  1:52                 ` Takao Indoh
2012-09-21 17:57               ` Don Dutile
2012-09-21 17:57                 ` Don Dutile
2012-09-24 11:12                 ` Takao Indoh
2012-09-24 11:12                   ` Takao Indoh
2012-08-06  4:09 ` Don Dutile
2012-08-06  4:09   ` Don Dutile
2012-08-06  4:45   ` Takao Indoh
2012-08-06  4:45     ` Takao Indoh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=501BB4EF.7080909@jp.fujitsu.com \
    --to=indou.takao@jp.fujitsu.com \
    --cc=bhelgaas@google.com \
    --cc=hbabu@us.ibm.com \
    --cc=ishii.hironobu@jp.fujitsu.com \
    --cc=kexec@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=martin.wilck@ts.fujitsu.com \
    --cc=vgoyal@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.