Re: [PULL v2 00/47] virtio,pc,pci: features, cleanups, fixes

[Philippe Mathieu-Daudé]

On 8/3/22 12:18, Peter Maydell wrote:
> On Tue, 8 Mar 2022 at 11:01, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Tue, Mar 08, 2022 at 09:05:27AM +0000, Peter Maydell wrote:
> > > On Mon, 7 Mar 2022 at 22:52, Michael S. Tsirkin <mst@redhat.com> wrote:

> > > > Now, I could maybe get behind this if it simply warned about a cast that
> > > > loses information (cast to a smaller integer) or integer/pointer cast
> > > > that does not go through uintptr_t without regard to size.
> > >
> > > This *is* warning about losing information. On 64-bit Windows
> > > pointers are 64 bits but 'long' is 32 bits, so the path
> > > pointer -> long -> uint64_t drops the top half of the pointer.
>
> > Yes obviously. My point is that this:
> > (uint64_t)hdev->vqs[queue].avail
> > is always harmless but it warns on a 32 bit system.
>
> True, I suppose. But compiler warnings are often like that: we
> take the hit of having to tweak some things we know to be OK in
> order to catch the real bugs in other cases.
>
> > And someone trying to fix that *is* what resulted in
> > (uint64_t)(unsigned long)hdev->vqs[queue].avail
>
> Using 'unsigned long' in a cast (or anything else) is often
> the wrong thing in QEMU...

$ git grep -F '(unsigned long)' | wc -l
     273

Ouch :/

These require cleanup:

target/i386/sev.c:170:    input.data = (__u64)(unsigned long)data;
target/i386/sev.c:188:    arg.data = (unsigned long)data;
target/i386/sev.c:243:    range.addr = (__u64)(unsigned long)host;
target/i386/sev.c:273:    range.addr = (__u64)(unsigned long)host;
target/i386/sev.c:730:    update.uaddr = (__u64)(unsigned long)addr;

And we might add a Gitlab issue to look at the hw/ ones:

$ git grep -F '(unsigned long)' hw | wc -l
      76