Re: [Qemu-devel] [PATCH RFC v4 03/16] virtio: support more feature bits

[Michael S. Tsirkin]

On Thu, Nov 27, 2014 at 04:40:29PM +0100, Cornelia Huck wrote:
> On Thu, 27 Nov 2014 17:34:19 +0200
> "Michael S. Tsirkin" <address@hidden> wrote:
> 
> > On Thu, Nov 27, 2014 at 04:16:36PM +0100, Cornelia Huck wrote:
> > > With virtio-1, we support more than 32 feature bits. Let's make
> > > vdev->guest_features depend on the number of supported feature bits,
> > > allowing us to grow the feature bits automatically.
> 
> ^ This was one reason why I did it this way...

Then use bitmap.h
But I think it's overdesign.


> > > 
> > > We also need to enhance the internal functions dealing with getting
> > > and setting features with an additional index field, so that all feature
> > > bits may be accessed (in chunks of 32 bits).
> > > 
> > > vhost and migration have been ignored for now.
> > > 
> > > Reviewed-by: Thomas Huth <address@hidden>
> > > Signed-off-by: Cornelia Huck <address@hidden>
> > > @@ -117,7 +125,7 @@ struct VirtIODevice
> > >      uint8_t status;
> > >      uint8_t isr;
> > >      uint16_t queue_sel;
> > > -    uint32_t guest_features;
> > > +    uint32_t guest_features[NR_VIRTIO_FEATURE_WORDS];
> > >      size_t config_len;
> > >      void *config;
> > >      uint16_t config_vector;
> > 
> > Ugh.
> > 
> > That's quite tricky to use correctly.
> > Why don't we just make it uint64_t?
> 
> ...and another one was that at least virtio-ccw reads/writes in chunks
> of 32 bits anyway.

It's quite easy to get at the high 32 bit of
64 bit integers.

> > 
> > The only real issue is that DEFINE_PROP_BIT wants
> > a uint32_t.
> > 
> > But that's easy to fix: add DEFINE_PROP64_BIT
> > that is the same but handles a 64 bit array.
> > 
> Sure, this would not really be a problem to add. But we'll stand before
> the same problem again when we want to grow past 64 bits, won't we?

It will take years till we run out of bits.
We'll handle it then, it's not like it's rocket science.

-- 
MST