Re: Re: [PATCH] kvm: Use a bitmap for tracking used GSIs

From: Alex Williamson <alex.williamson@hp.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: "Yang, Sheng" <sheng.yang@intel.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>
Subject: Re: Re: [PATCH] kvm: Use a bitmap for tracking used GSIs
Date: Tue, 12 May 2009 13:23:21 -0600	[thread overview]
Message-ID: <1242156201.23680.176.camel@bling> (raw)
In-Reply-To: <20090512191053.GA28935@redhat.com>

On Tue, 2009-05-12 at 22:10 +0300, Michael S. Tsirkin wrote:
> On Tue, May 12, 2009 at 01:06:54PM -0600, Alex Williamson wrote:
> > On Tue, 2009-05-12 at 12:45 -0600, Alex Williamson wrote:
> > > On Mon, 2009-05-11 at 12:00 +0000, Yang, Sheng wrote:
> > > > On Friday 08 May 2009 06:22:20 Alex Williamson wrote:
> > > > > +		/* 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?
> > > 
> > > 3, ie. /8 (bits to bytes)
> > > 
> > > > I am a little confused by these magic numbers, including 0x1f...
> > > 
> > > The 5 shift gives us the index into the array of ints, the 0x1f gives us
> > > the bit index into a specific int.  This is very similar to the code in
> > > hw/acpi.c.
> > > 
> > > > 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?
> > > 
> > > We'd probably want to use ffsll() so we can ignore 32b vs 64b longs.
> > > There's HOST_LONG_BITS, but that doesn't actually help defining a shift
> > > value.  Thanks,
> > 
> > Hmm, neither ffsl() or ffsll() are standard.  I'm inclined to stick with
> > 32bits.  We'll likely only be using the first index on x86_64 anyway
> > (2nd on ia64).  Thanks,
> > 
> > Alex
> 
> With MSI, we start with GSI 24, so we'll be using more than just the
> first index.

Right, that leaves 7 GSIs free in the first 32 bit index for device
assignment, plus 999 GSIs free in the entire table.  If we're worried
about the overhead of using a 4 or 8 byte index into the bitmap, maybe
we should be caching GSIs for specific devices.  This feels like a bit
of an overkill for now though.  Thanks,

Alex