From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tim Deegan Subject: [PATCH] [RFC] VT-d: always clean up dpci timers. Date: Mon, 18 Jul 2011 17:38:48 +0100 Message-ID: <20110718163848.GD18276@whitby.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Return-path: Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: xen-devel@lists.xensource.com Cc: Allen Kay , keir@xen.org List-Id: xen-devel@lists.xenproject.org If a VM has all its PCI devices deassigned, need_iommu(d) becomes false but it might still have DPCI EOI timers that were init_timer()d but not yet kill_timer()d. That causes xen to crash later because the linked list of inactive timers gets corrupted, e.g.: (XEN) Xen call trace: (XEN) [] set_timer+0x1c2/0x24f (XEN) [] schedule+0x129/0x5dd (XEN) [] __do_softirq+0x7e/0x89 (XEN) [] do_softirq+0x26/0x28 (XEN) [] idle_loop+0x5a/0x5c (XEN) (XEN) (XEN) **************************************** (XEN) Panic on CPU 0: (XEN) Assertion 'entry->next->prev == entry' failed at /local/scratch/tdeegan/xen-unstable.hg/xen/include:172 (XEN) **************************************** The following patch makes sure that the domain destruction path always clears up the DPCI state even if !needs_iommu(d). Although it fixes the crash for me, I'm sufficiently confused by this code that I don't know whether it's enough. If the dpci timer state gets freed earlier than pci_clean_dpci_irqs() then there's still a race, and some other function (reassign_device_ownership() ?) needs to sort out the timers when the PCI card is deassigned. Allen, can you comment? Signed-off-by: Tim Deegan diff -r ab6551e30841 xen/drivers/passthrough/pci.c --- a/xen/drivers/passthrough/pci.c Mon Jul 18 10:59:44 2011 +0100 +++ b/xen/drivers/passthrough/pci.c Mon Jul 18 17:22:48 2011 +0100 @@ -269,7 +269,7 @@ static void pci_clean_dpci_irqs(struct d if ( !iommu_enabled ) return; - if ( !is_hvm_domain(d) || !need_iommu(d) ) + if ( !is_hvm_domain(d) ) return; spin_lock(&d->event_lock);