From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Yang, Sheng" Subject: Re: [PATCH] kvm: Use a bitmap for tracking used GSIs Date: Mon, 11 May 2009 20:00:10 +0800 Message-ID: <200905112000.10771.sheng.yang@intel.com> References: <20090507222015.5216.18027.stgit@dl380g6-3.ned.telco.ned.telco> Mime-Version: 1.0 Content-Type: Text/Plain; charset="utf-8" Content-Transfer-Encoding: 7bit Cc: Alex Williamson To: kvm@vger.kernel.org Return-path: Received: from mga03.intel.com ([143.182.124.21]:51418 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751062AbZEKL72 (ORCPT ); Mon, 11 May 2009 07:59:28 -0400 In-Reply-To: <20090507222015.5216.18027.stgit@dl380g6-3.ned.telco.ned.telco> Content-Disposition: inline Sender: kvm-owner@vger.kernel.org List-ID: On Friday 08 May 2009 06:22:20 Alex Williamson wrote: > We're currently using a counter to track the most recent GSI we've > handed out. This quickly hits KVM_MAX_IRQ_ROUTES when using device > assignment with a driver that regularly toggles the MSI enable bit. > This can mean only a few minutes of usable run time. Instead, track > used GSIs in a bitmap. > > Signed-off-by: Alex Williamson > --- > > Applies on top of "kvm: device-assignment: Catch GSI overflow" > > hw/device-assignment.c | 4 ++- > kvm/libkvm/kvm-common.h | 3 +- > kvm/libkvm/libkvm.c | 68 > +++++++++++++++++++++++++++++++++++++++++------ kvm/libkvm/libkvm.h | > 10 +++++++ > 4 files changed, 74 insertions(+), 11 deletions(-) > > diff --git a/hw/device-assignment.c b/hw/device-assignment.c > index e06dd08..5bdae24 100644 > --- a/hw/device-assignment.c > +++ b/hw/device-assignment.c > @@ -561,8 +561,10 @@ static void free_dev_irq_entries(AssignedDevice *dev) > { > int i; > > - for (i = 0; i < dev->irq_entries_nr; i++) > + for (i = 0; i < dev->irq_entries_nr; i++) { > kvm_del_routing_entry(kvm_context, &dev->entry[i]); > + kvm_free_irq_route_gsi(kvm_context, dev->entry[i].gsi); > + } > free(dev->entry); > dev->entry = NULL; > dev->irq_entries_nr = 0; > diff --git a/kvm/libkvm/kvm-common.h b/kvm/libkvm/kvm-common.h > index 591fb53..94f86e5 100644 > --- a/kvm/libkvm/kvm-common.h > +++ b/kvm/libkvm/kvm-common.h > @@ -66,8 +66,9 @@ struct kvm_context { > #ifdef KVM_CAP_IRQ_ROUTING > struct kvm_irq_routing *irq_routes; > int nr_allocated_irq_routes; > + void *used_gsi_bitmap; > + int max_gsi; > #endif > - int max_used_gsi; > }; > > int kvm_alloc_kernel_memory(kvm_context_t kvm, unsigned long memory, > diff --git a/kvm/libkvm/libkvm.c b/kvm/libkvm/libkvm.c > index 2a4165a..43abc7d 100644 > --- a/kvm/libkvm/libkvm.c > +++ b/kvm/libkvm/libkvm.c > @@ -1298,8 +1298,6 @@ int kvm_add_routing_entry(kvm_context_t kvm, > new->flags = entry->flags; > new->u = entry->u; > > - if (entry->gsi > kvm->max_used_gsi) > - kvm->max_used_gsi = entry->gsi; > return 0; > #else > return -ENOSYS; > @@ -1404,20 +1402,72 @@ int kvm_commit_irq_routes(kvm_context_t kvm) > #endif > } > > +#ifdef KVM_CAP_IRQ_ROUTING > +static inline void set_bit(unsigned int *buf, int bit) > +{ > + buf[bit >> 5] |= (1U << (bit & 0x1f)); > +} > + > +static inline void clear_bit(unsigned int *buf, int bit) > +{ > + buf[bit >> 5] &= ~(1U << (bit & 0x1f)); > +} > + > +static int kvm_find_free_gsi(kvm_context_t kvm) > +{ > + int i, bit, gsi; > + unsigned int *buf = kvm->used_gsi_bitmap; > + > + for (i = 0; i < (kvm->max_gsi >> 5); i++) { > + if (buf[i] != ~0U) > + break; > + } > + > + if (i == kvm->max_gsi >> 5) > + return -ENOSPC; > + > + bit = ffs(~buf[i]); > + if (!bit) > + return -EAGAIN; > + > + gsi = (bit - 1) | (i << 5); > + set_bit(buf, gsi); > + return gsi; > +} > +#endif > + > 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 + 1 < kvm_get_gsi_count(kvm)) > - return kvm->max_used_gsi + 1; > - else > - return -ENOSPC; > - } else > - return KVM_IOAPIC_NUM_PINS; > + if (!kvm->max_gsi) { > + int i; > + > + /* Round the number of GSIs supported to a 4 byte > + * value so we can search it using ints and ffs */ > + i = kvm_get_gsi_count(kvm) & ~0x1f; > + kvm->used_gsi_bitmap = malloc(i >> 3); 3 or 5? I am a little confused by these magic numbers, including 0x1f... I think there are something can indicate the length of unsigned long in QEmu(sorry, can't find it now...), so how about using ffsl() and get other constants based on it? -- regards Yang, Sheng > + if (!kvm->used_gsi_bitmap) > + return -ENOMEM; > + memset(kvm->used_gsi_bitmap, 0, i >> 3); > + kvm->max_gsi = i; > + > + /* Mark all the IOAPIC pin GSIs as already used */ > + for (i = 0; i <= KVM_IOAPIC_NUM_PINS; i++) > + set_bit(kvm->used_gsi_bitmap, i); > + } > + > + return kvm_find_free_gsi(kvm); > #else > return -ENOSYS; > #endif > } > + > +void kvm_free_irq_route_gsi(kvm_context_t kvm, int gsi) > +{ > +#ifdef KVM_CAP_IRQ_ROUTING > + clear_bit(kvm->used_gsi_bitmap, gsi); > +#endif > +} > > #ifdef KVM_CAP_DEVICE_MSIX > int kvm_assign_set_msix_nr(kvm_context_t kvm, > diff --git a/kvm/libkvm/libkvm.h b/kvm/libkvm/libkvm.h > index c23d37b..4e9344c 100644 > --- a/kvm/libkvm/libkvm.h > +++ b/kvm/libkvm/libkvm.h > @@ -856,6 +856,16 @@ int kvm_commit_irq_routes(kvm_context_t kvm); > */ > int kvm_get_irq_route_gsi(kvm_context_t kvm); > > +/*! > + * \brief Free used GSI number > + * > + * Free used GSI number acquired from kvm_get_irq_route_gsi() > + * > + * \param kvm Pointer to the current kvm_context > + * \param gsi GSI number to free > + */ > +void kvm_free_irq_route_gsi(kvm_context_t kvm, int gsi); > + > #ifdef KVM_CAP_DEVICE_MSIX > int kvm_assign_set_msix_nr(kvm_context_t kvm, > struct kvm_assigned_msix_nr *msix_nr); > > -- > 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