From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex Williamson Subject: Re: [PATCH v3] kvm: Use a bitmap for tracking used GSIs Date: Tue, 12 May 2009 21:42:03 -0600 Message-ID: <1242186123.4878.9.camel@2710p.home> References: <20090508222925.5119.94814.stgit@dl380g6-3.ned.telco.ned.telco> <20090512220142.5663.72948.stgit@dl380g6-3.ned.telco.ned.telco> <200905131130.01846.sheng.yang@intel.com> Mime-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: 7bit Cc: "kvm@vger.kernel.org" , "mst@redhat.com" To: "Yang, Sheng" Return-path: Received: from g4t0015.houston.hp.com ([15.201.24.18]:43551 "EHLO g4t0015.houston.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752271AbZEMDmK (ORCPT ); Tue, 12 May 2009 23:42:10 -0400 In-Reply-To: <200905131130.01846.sheng.yang@intel.com> Sender: kvm-owner@vger.kernel.org List-ID: On Wed, 2009-05-13 at 11:30 +0800, Yang, Sheng wrote: > On Wednesday 13 May 2009 06:07:15 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 > > --- > > > > v2: Added mutex to protect gsi bitmap > > v3: Updated for comments from Michael Tsirkin > > No longer depends on "[PATCH] kvm: device-assignment: Catch GSI > > overflow" > > > > hw/device-assignment.c | 4 ++ > > kvm/libkvm/kvm-common.h | 4 ++ > > kvm/libkvm/libkvm.c | 83 > > +++++++++++++++++++++++++++++++++++++++++------ kvm/libkvm/libkvm.h | > > 10 ++++++ > > 4 files changed, 88 insertions(+), 13 deletions(-) > > > > diff --git a/hw/device-assignment.c b/hw/device-assignment.c > > index a7365c8..a6cc9b9 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..4b3cb51 100644 > > --- a/kvm/libkvm/kvm-common.h > > +++ b/kvm/libkvm/kvm-common.h > > @@ -66,8 +66,10 @@ 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; > > + pthread_mutex_t gsi_mutex; > > #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 ba0a5d1..3d7ab75 100644 > > --- a/kvm/libkvm/libkvm.c > > +++ b/kvm/libkvm/libkvm.c > > @@ -35,6 +35,7 @@ > > #include > > #include > > #include > > +#include > > #include "libkvm.h" > > > > #if defined(__x86_64__) || defined(__i386__) > > @@ -65,6 +66,8 @@ > > int kvm_abi = EXPECTED_KVM_API_VERSION; > > int kvm_page_size; > > > > +static inline void set_bit(uint32_t *buf, unsigned int bit); > > + > > struct slot_info { > > unsigned long phys_addr; > > unsigned long len; > > @@ -286,6 +289,9 @@ kvm_context_t kvm_init(struct kvm_callbacks *callbacks, > > int fd; > > kvm_context_t kvm; > > int r; > > +#ifdef KVM_CAP_IRQ_ROUTING > > + int gsi_count, gsi_bytes, i; > > +#endif > > > > fd = open("/dev/kvm", O_RDWR); > > if (fd == -1) { > > @@ -322,6 +328,27 @@ kvm_context_t kvm_init(struct kvm_callbacks > > *callbacks, kvm->dirty_pages_log_all = 0; > > kvm->no_irqchip_creation = 0; > > kvm->no_pit_creation = 0; > > +#ifdef KVM_CAP_IRQ_ROUTING > > + pthread_mutex_init(&kvm->gsi_mutex, NULL); > > + > > + gsi_count = kvm_get_gsi_count(kvm); > > + /* Round up so we can search ints using ffs */ > > + gsi_bytes = (gsi_count + 31) / 32; > > CMIW, should it be gsi_bytes = (gsi_count + 7) / 8? This looks like bits-to- > int. Oops, I missed a multiplier in there. What you have would be correct for rounding up to a byte, but we really want to round up to an int, so I need to factor in a bytes/int, which gives me this: gsi_bytes = ((gsi_count + 31) / 32) * 4; Then the rest works out correctly. Thanks, Alex > > + kvm->used_gsi_bitmap = malloc(gsi_bytes); > > + if (!kvm->used_gsi_bitmap) { > > + pthread_mutex_unlock(&kvm->gsi_mutex); > > + goto out_close; > > + } > > + memset(kvm->used_gsi_bitmap, 0, gsi_bytes); > > + kvm->max_gsi = gsi_bytes * 8; > > So max_gsi = gsi_count / 4?