Re: [Qemu-devel] [PATCH RFC v6 05/20] virtio: support more feature bits

[David Gibson]

On Fri, Dec 12, 2014 at 11:17:47AM +0100, Cornelia Huck wrote:
> On Fri, 12 Dec 2014 11:06:40 +0100
> Thomas Huth <address@hidden> wrote:
> 
> > On Thu, 11 Dec 2014 14:25:07 +0100
> > Cornelia Huck <address@hidden> wrote:
[snip]
> > >      vdev->queue_sel = 0;
> > >      vdev->status = 0;
> > >      vdev->isr = 0;
> > > @@ -924,7 +925,8 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
> > >      qemu_put_8s(f, &vdev->status);
> > >      qemu_put_8s(f, &vdev->isr);
> > >      qemu_put_be16s(f, &vdev->queue_sel);
> > > -    qemu_put_be32s(f, &vdev->guest_features);
> > > +    /* XXX features >= 32 */
> > > +    qemu_put_be32s(f, (uint32_t *)&vdev->guest_features);
> > 
> > Casting a uint64_t* to a uint32_t* here sounds very wrong - this likely
> > only works on little endian sytems, but certainly not on big-endian
> > systems.
> > If you do not want to extend this for 64-bit right from the beginning,
> > I think you've got to use a temporary 32-bit variable to get this right.
> 
> Hm... always store the old 32 bits here, then store the new 32 bits
> later? Should be able to get that compatible.

I think Thomas' point is that since vdev->guest_features has changed
to a uint64_t, the "old" bits are now in the second 32-bit half on a
BE system.

Or maybe I'm misunderstanding your reply.

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

pgpxzhLF5X1bu.pgp
Description: PGP signature