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

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 06/07/11 13:43, Jan Beulich wrote:
>>>> On 06.07.11 at 14:39, Andrew Cooper <andrew.cooper3@citrix.com> 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@citrix.com>
>>
>> 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

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