Re: [Xen-devel] [PATCH] KEXEC: disconnect all PCI devices from the PCI bus on crash

["Jan Beulich" <JBeulich@xxxxxxxxxx>]

>>> On 06.07.11 at 14:39, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> 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 <andrew.cooper3@xxxxxxxxxx>
> 
> 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 <asm/apic.h>
>  #include <asm/io_apic.h>
>  #include <xen/iommu.h>
> +#include <xen/pci.h>
>  
>  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

> +{
> +    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@xxxxxxxxxxxxxxxxxxx 
> http://lists.xensource.com/xen-devel 




_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel