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
msi_lock.patch
Description: msi_lock.patch
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home