On Friday 13 February 2009 04:40:05 Marcelo Tosatti wrote: > On Wed, Feb 11, 2009 at 04:08:51PM +0800, Sheng Yang wrote: > > This patch finally enable MSI-X. > > > > What we need for MSI-X: > > 1. Intercept one page in MMIO region of device. So that we can get guest > > desired MSI-X table and set up the real one. Now this have been done by > > guest, and transfer to kernel using ioctl KVM_SET_MSIX_NR and > > KVM_SET_MSIX_ENTRY. > > > > 2. Information for incoming interrupt. Now one device can have more than > > one interrupt, and they are all handled by one workqueue structure. So we > > need to identify them. The previous patch enable gsi_msg_pending_bitmap > > get this done. > > > > 3. Mapping from host IRQ to guest gsi as well as guest gsi to real > > MSI/MSI-X message address/data. We used same entry number for the host > > and guest here, so that it's easy to find the correlated guest gsi. > > > > What we lack for now: > > 1. The PCI spec said nothing can existed with MSI-X table in the same > > page of MMIO region, except pending bits. The patch ignore pending bits > > as the first step (so they are always 0 - no pending). > > > > 2. The PCI spec allowed to change MSI-X table dynamically. That means, > > the OS can enable MSI-X, then mask one MSI-X entry, modify it, and unmask > > it. The patch didn't support this, and Linux also don't work in this way. > > Have you thought about supporting this with the current ioctl interface? > Point is that once ioctl's are in and userspace code uses it, you can't > change. So: > > - If userspace calls kvm_vm_ioctl_set_msix_entry twice don't you leak > memory (that is a bug actually unless I'm mistaken)? So in the future > kvm_vm_ioctl_set_msix_entry will deallocate the currently allocated > guest/host entries and replace with userspace supplied entries? > The allocation only happen once, at the second time it would report error in current code. But allocate/deallocate is also acceptable for future. > - interrupt context can read the table while kvm_vm_ioctl_set_msix_entry > is modifying it. So you either need to forbid more than one > kvm_vm_ioctl_set_msix_entry call in the lifetime of a guest (which > you can later allow when you support MSI table change), or handle > accesses from multiple contexes now. It seems forbidding is enough for > the moment from what you said. Yeah. But for the modifying the MSI-X table, the most critical problem is, current Linux didn't support it IIRC. So I have to disable MSI-X then enable it again with new table, and it would result in lost interrupt. So seems the most reasonable method is to modify pci_enable_msix() and related function's action to support this... -- regards Yang, Sheng > > But in general looks good to me (not familiar with the internals of > pci / msi). -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
