From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH] KEXEC: disconnect all PCI devices from the PCI bus on crash Date: Wed, 6 Jul 2011 13:46:57 +0100 Message-ID: <4E145941.1030604@citrix.com> References: <7ea606c5ce8c8fcfc577.1309955952@andrewcoop.uk.xensource.com> <4E14748D020000780004C618@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <4E14748D020000780004C618@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Jan Beulich Cc: "xen-devel@lists.xensource.com" List-Id: xen-devel@lists.xenproject.org On 06/07/11 13:43, Jan Beulich wrote: >>>> On 06.07.11 at 14:39, Andrew Cooper wrote: >> In the case of a crash, IOMMU DMA remapping gets turned off so that >> the kdump kernel may boot. However, this is warned as being dangerous >> in the VTD specification if a DMA transaction is in progress. >> >> Also, in the case of a crash, DMA transactions and interrupts from >> peripheral devices such as network cards are likely to keep coming in. >> Without DMA remapping enabled, the transactions will be writing over >> low memory, corrupting the crash state, and perhaps even the kdump >> reserved memory. >> >> Therefore, on the crash path, we can disconnect all PCI devices from >> their respective buses so that they are no longer able to be DMA >> busmasters. This reduces the risk of DMA transactions corrupting >> state (and will also reduce spurious interrupts arriving to the kdump >> kernel) until the kdump kernel and properly reset the PCI devices. >> >> Signed-off-by: Andrew Cooper >> >> diff -r 2f63562df1c4 -r 7ea606c5ce8c xen/arch/x86/crash.c >> --- a/xen/arch/x86/crash.c Mon Jun 27 17:37:12 2011 +0100 >> +++ b/xen/arch/x86/crash.c Wed Jul 06 13:37:44 2011 +0100 >> @@ -28,6 +28,7 @@ >> #include >> #include >> #include >> +#include >> >> static atomic_t waiting_for_crash_ipi; >> static unsigned int crashing_cpu; >> @@ -78,6 +79,8 @@ static void nmi_shootdown_cpus(void) >> msecs--; >> } >> >> + disconnect_pci_devices(); >> + >> /* Crash shutdown any IOMMU functionality as the crashdump kernel is >> not >> * happy when booting if interrupt/dma remapping is still enabled */ >> iommu_crash_shutdown(); >> diff -r 2f63562df1c4 -r 7ea606c5ce8c xen/drivers/passthrough/pci.c >> --- a/xen/drivers/passthrough/pci.c Mon Jun 27 17:37:12 2011 +0100 >> +++ b/xen/drivers/passthrough/pci.c Wed Jul 06 13:37:44 2011 +0100 >> @@ -462,6 +462,32 @@ int __init scan_pci_devices(void) >> return 0; >> } >> >> +/* Disconnect a PCI device from the PCI bus. From the PCI spec: >> + * "When a 0 is written to [the COMMAND] register, the device is >> + * logically disconnected from the PCI bus for all accesses except >> + * configuration accesses. All devices are required to support >> + * this base level of functionality." >> + */ >> +void disconnect_pci_device(struct pci_dev *pdev) > Any reason this cannot be static? Or even be integrated into the > single caller? > > Jan Not specifically - I can fold them together if you think that would be better ~Andrew >> +{ >> + pci_conf_write16(pdev->bus, PCI_SLOT(pdev->devfn), >> + PCI_FUNC(pdev->devfn), PCI_COMMAND, 0); >> +} >> + >> +/* Diconnect all PCI devices from the PCI buses. >> + */ >> +void disconnect_pci_devices(void) >> +{ >> + struct pci_dev *pdev; >> + >> + spin_lock(&pcidevs_lock); >> + >> + list_for_each_entry ( pdev, &alldevs_list, alldevs_list ) >> + disconnect_pci_device(pdev); >> + >> + spin_unlock(&pcidevs_lock); >> +} >> + >> #ifdef SUPPORT_MSI_REMAPPING >> static void dump_pci_devices(unsigned char ch) >> { >> diff -r 2f63562df1c4 -r 7ea606c5ce8c xen/include/xen/pci.h >> --- a/xen/include/xen/pci.h Mon Jun 27 17:37:12 2011 +0100 >> +++ b/xen/include/xen/pci.h Wed Jul 06 13:37:44 2011 +0100 >> @@ -92,6 +92,9 @@ int pci_add_device_ext(u8 bus, u8 devfn, >> struct pci_dev *pci_get_pdev(int bus, int devfn); >> struct pci_dev *pci_get_pdev_by_domain(struct domain *d, int bus, int >> devfn); >> >> +void disconnect_pci_device(struct pci_dev * pdev); >> +void disconnect_pci_devices(void); >> + >> uint8_t pci_conf_read8( >> unsigned int bus, unsigned int dev, unsigned int func, unsigned int >> reg); >> uint16_t pci_conf_read16( >> >> _______________________________________________ >> Xen-devel mailing list >> Xen-devel@lists.xensource.com >> http://lists.xensource.com/xen-devel > > -- Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer T: +44 (0)1223 225 900, http://www.citrix.com