[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH] Re-enable MSI support
Currently the MSI is disabled because of some lock issue. This patch try to cleanup the lock related to MSI lock. Because I have no AMD's iommu environment, so I didn't test AMD's platform, WeiWang, can you please have a check on AMD's code path? Signed-off-by: Jiang Yunhong <yunhong.jiang@xxxxxxxxx> The current issue related including: 1) The sequence of pci_dev's lock and irq_desc's lock is not same. For example, In pci_cleanup_msi, it will get pci_dev lock before msi_free_vectors get irq_desc's lock, while in pci_disable_msi , it will get irq_desc's lock before pci_dev lock. 2) The sequence of pci_dev's lock and pcidevs_lock is not same. For example, reassign_device_ownership will get pci_dev's lock before get pcidevs_lock, while pci_lock_domai_pdev will get pcidevs_lock before pci_dev's lock. 3) The lock to bus2bridge is not always confusing. 4) xmalloc with irq_save in several code path, which cause debug version failed. Also some minor issue exits,(can be identified by the patch), including: the spin_lock_irqsave and spin_lock for iommu->lock and hd->mapping_lock; race condition may happen in XEN_DOMCTL_assign_device and XEN_DOMCTL_deassign_device, the rangeset is freed in rangeset_domain_destroy() before the unmap_domain_pirq try to deny the irq access in arch_domain_destro, etc. This patch try to do some cleanup for these issues. 1) The basic idea is to remove the pci_dev's lock, instead, we try to use the big pcidevs_lock to protect the whole pci_dev stuff. It including both pci_dev adding/removing, and also the assignment of the devices. We checked the code and seems there is no critical code path for this. We try to use fine-grained lock and seems the code will be very tricky. 2) Split the pci_enable_msi into two step, firstly it will just construct the msi_desc through pci_enable_msi without holding the irq_desc lock, and then it will setup msi through setup_msi_irq with irq_desc holded. 3) Change the iommu->lock and hd->mapping_lock to be irq_save. 4) Fix to some minor issues. Now the lock sequence is: pcidevs_lock -> domai's event_lock -> iommu's lock -> hvm_iommu's mapping_lock. The irq_desc's lock will always be the last lock be hold for peformance consideration. Something need notice: 1) It is tricky to support multi-vector MSI. The reason is: the mask bit is in the same register shared by multiple vector, that means the ISR need compete for this register and need hold another lock, and it will cause things very tricky (the support is buggy in current code). So in fact, I think the current code does not support it. 2) I'm not sure if pci_remove_device need some improvement, to make sure the resouce is freed, since it assume the pci device is owned by other domain still. For example, it may need call pci_clean_dpci_irqs(). But because I'm not sure how will this function be used, so I didn't touch it. One thing left is the IOMMU's page fault handler, which will try to access the vt-d's mapping table and context table currently. We can simply remove the print_vtd_entries in iommu_page_fault_do_one, or place the print task in a delay work. I tried to split the patch to some smaller one, but seems the logic is something related, so I give up later. Thanks Yunhong Jiang Attachment:
msi_lock.patch _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |