diff mbox

kvm: device-assignment: Catch GSI overflow

Message ID 20090507170834.26367.91907.stgit@dl380g6-3.ned.telco.ned.telco (mailing list archive)
State New, archived
Headers show

Commit Message

Alex Williamson May 7, 2009, 5:09 p.m. UTC
Fix the index at which we return -ENOSPC since the kernel side will
reject a GSI >= KVM_MAX_IRQ_ROUTES.  Also, mask as a signed int before
testing for error.

Signed-off-by: Alex Williamson <alex.williamson@hp.com>
---

 hw/device-assignment.c |    2 +-
 kvm/libkvm/libkvm.c    |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Alex Williamson May 7, 2009, 5:16 p.m. UTC | #1
On Thu, 2009-05-07 at 11:09 -0600, Alex Williamson wrote:
> Fix the index at which we return -ENOSPC since the kernel side will
> reject a GSI >= KVM_MAX_IRQ_ROUTES.  Also, mask as a signed int before
> testing for error.

Even with this, there still seems to be a fundamental problem with our
consumption of GSIs in kvm.  For example, every time a guest writes to
the MSI capabilities area and enables MSI support, we call
kvm_get_irq_route_gsi() to get a new max_used_gsi + 1 value, then call
kvm_add_routing_entry(), which updates max_used_gsi.  It doesn't take
too long before we exhaust the GSI space and the device no longer works.
This seems to happen within a minute or two of booting a guest with an
e1000e device sitting idle on a busy network.  Do we need to keep a
bitmap of used GSIs or maybe just attempt to reuse the GSI we've gotten
previously for the device?  Thanks,

Alex

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sheng Yang May 8, 2009, 9:28 a.m. UTC | #2
On Friday 08 May 2009 01:16:53 Alex Williamson wrote:
> On Thu, 2009-05-07 at 11:09 -0600, Alex Williamson wrote:
> > Fix the index at which we return -ENOSPC since the kernel side will
> > reject a GSI >= KVM_MAX_IRQ_ROUTES.  Also, mask as a signed int before
> > testing for error.
>
> Even with this, there still seems to be a fundamental problem with our
> consumption of GSIs in kvm.  For example, every time a guest writes to
> the MSI capabilities area and enables MSI support, we call
> kvm_get_irq_route_gsi() to get a new max_used_gsi + 1 value, then call
> kvm_add_routing_entry(), which updates max_used_gsi.  It doesn't take
> too long before we exhaust the GSI space and the device no longer works.
> This seems to happen within a minute or two of booting a guest with an
> e1000e device sitting idle on a busy network.  Do we need to keep a
> bitmap of used GSIs or maybe just attempt to reuse the GSI we've gotten
> previously for the device?  Thanks,

Yes, I think a bitmap is much better here.

Thanks.
diff mbox

Patch

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index a7365c8..e06dd08 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -796,7 +796,7 @@  static void assigned_dev_update_msi(PCIDevice *pci_dev, unsigned int ctrl_pos)
                 pci_dev->cap.start + PCI_MSI_DATA_32);
         assigned_dev->entry->type = KVM_IRQ_ROUTING_MSI;
         assigned_dev->entry->gsi = kvm_get_irq_route_gsi(kvm_context);
-        if (assigned_dev->entry->gsi < 0) {
+        if ((int)(assigned_dev->entry->gsi) < 0) {
             perror("assigned_dev_update_msi: kvm_get_irq_route_gsi");
             return;
         }
diff --git a/kvm/libkvm/libkvm.c b/kvm/libkvm/libkvm.c
index ba0a5d1..2a4165a 100644
--- a/kvm/libkvm/libkvm.c
+++ b/kvm/libkvm/libkvm.c
@@ -1408,7 +1408,7 @@  int kvm_get_irq_route_gsi(kvm_context_t kvm)
 {
 #ifdef KVM_CAP_IRQ_ROUTING
 	if (kvm->max_used_gsi >= KVM_IOAPIC_NUM_PINS)  {
-	    if (kvm->max_used_gsi <= kvm_get_gsi_count(kvm))
+	    if (kvm->max_used_gsi + 1 < kvm_get_gsi_count(kvm))
                 return kvm->max_used_gsi + 1;
             else
                 return -ENOSPC;