RE: [Xen-devel] [PATCH] Re-enable MSI support

["Jiang, Yunhong" <yunhong.jiang@xxxxxxxxx>]

Tian, Kevin <> wrote:
>> From: Espen Skoglund
>> Sent: Wednesday, December 10, 2008 7:34 PM
>> 
>> [Yunhong Jiang]
>>> 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.
>> 
>> So what exactly is it that pcidevs_lock is supposed to "protect" now?
>> Does it indicate that someone is holding a reference to a pci_dev?
>> Does it indicate that someone will modify some pci_dev?  Does it
>> indicate that someone is messing around with interrupts or MSI
>> descriptors?
> 
> I think it protects all above. As those operations are all rare, such a
> big lock can avoid complex lock/unlock sequence regarding to different
> paths to different resource of an assigned device.

Excactly. Maybe the name can be changed more meaningful, but I'm really poor on 
that.

> 
>> 
>> Regarding pci_enable_msi: Can you not get into race conditions with
>> the splitup you're doing?  pci_enable_msi() does afterall poke around in
>> the MSI capability record. 
>> 
>>      eSk
> 
> If I read it correctly, only one out of lock is to mask/unmask MSI
> vector. MSI-X has per-vector mask dword, and thus no contention.
> multiple-vector support for MSI is not supported yet. All other touch
> to MSI/MSI-X capabilities are protected by that lock.

Excactly. Maybe I should stated this more cleanly in my original mail. And 
that's the reason we can't support multiple entry MSI very easily.

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