Re: [Qemu-devel] [PATCH] virtio: fix vring_align() on 64-bit win32 platforms

[Stefan Weil]

Am 22.03.2017 um 00:06 schrieb Andrew Baumann:
> > From: Eric Blake [mailto:address@hidden
> > Sent: Tuesday, 21 March 2017 15:52
> >
> > On 03/21/2017 05:31 PM, Andrew Baumann wrote:
> > > "long" is 32-bits on win32, but we need to promote it to a 64-bit hwaddr
> > > before negating, or else the top half of the address is truncated
> > >
> > > Signed-off-by: Andrew Baumann <address@hidden>
> > > ---
> > >  include/hw/virtio/virtio.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> > > index 15efcf2..a0a8543 100644
> > > --- a/include/hw/virtio/virtio.h
> > > +++ b/include/hw/virtio/virtio.h
> > > @@ -34,7 +34,7 @@ struct VirtQueue;
> > >  static inline hwaddr vring_align(hwaddr addr,
> > >                                               unsigned long align)
> > >  {
> > > -    return (addr + align - 1) & ~(align - 1);
> > > +    return (addr + align - 1) & ~(hwaddr)(align - 1);
> >
> > Why not just use the QEMU_ALIGN_DOWN macro, instead of open-coding it?
>
> Well, this code is aligning up, but yes the ALIGN_UP macro looks like it should 
> also avoid the type promotion problem. This patch is just the 
> minimally-invasive change after discovering the bug.
>
> Let me know if you want me to spin another patch with the macro.
>
> Andrew

Yes, please use QEMU_ALIGN_UP in an updated patch.
This is a bug fix needed for v2.9.0.

Fixing all other code locations which round up or down
with Coccinelle is a separate task, nothing which is
needed for the next QEMU version.

Thanks,
Stefan