Message ID | 20210520102822.2471710-1-philmd@redhat.com |
---|---|
State | New |
Headers | show |
Series | hw/virtio: Have virtio_bus_get_vdev_bad_features() return 64-bit value | expand |
On Thu, 20 May 2021 12:28:22 +0200 Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > In commit 019a3edbb25 ("virtio: make features 64bit wide") we > increased the 'features' field to 64-bit, but forgot to update > the virtio_bus_get_vdev_bad_features() helper. The 'bad features' > are truncated to 32-bit. The virtio_net_bad_features() handler > from the virtio-net devices is potentially affected. > > Have the virtio_bus_get_vdev_bad_features() helper return the > full 64-bit value. > > Cc: qemu-stable@nongnu.org > Fixes: 019a3edbb25 ("virtio: make features 64bit wide") > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > include/hw/virtio/virtio-bus.h | 2 +- > hw/virtio/virtio-bus.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Cornelia Huck <cohuck@redhat.com>
On Thu, May 20, 2021 at 12:28:22PM +0200, Philippe Mathieu-Daudé wrote: > In commit 019a3edbb25 ("virtio: make features 64bit wide") we > increased the 'features' field to 64-bit, but forgot to update > the virtio_bus_get_vdev_bad_features() helper. The 'bad features' > are truncated to 32-bit. The virtio_net_bad_features() handler > from the virtio-net devices is potentially affected. I'm fine with increasing it for consistency, but bad features are all legacy things aren't they? So there isn't a functional issue ... or did I miss anything? > > Have the virtio_bus_get_vdev_bad_features() helper return the > full 64-bit value. > > Cc: qemu-stable@nongnu.org > Fixes: 019a3edbb25 ("virtio: make features 64bit wide") > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > include/hw/virtio/virtio-bus.h | 2 +- > hw/virtio/virtio-bus.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h > index ef8abe49c5a..f9955ff577a 100644 > --- a/include/hw/virtio/virtio-bus.h > +++ b/include/hw/virtio/virtio-bus.h > @@ -122,7 +122,7 @@ uint16_t virtio_bus_get_vdev_id(VirtioBusState *bus); > /* Get the config_len field of the plugged device. */ > size_t virtio_bus_get_vdev_config_len(VirtioBusState *bus); > /* Get bad features of the plugged device. */ > -uint32_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus); > +uint64_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus); > /* Get config of the plugged device. */ > void virtio_bus_get_vdev_config(VirtioBusState *bus, uint8_t *config); > /* Set config of the plugged device. */ > diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c > index 859978d2487..25a2b68a234 100644 > --- a/hw/virtio/virtio-bus.c > +++ b/hw/virtio/virtio-bus.c > @@ -134,7 +134,7 @@ size_t virtio_bus_get_vdev_config_len(VirtioBusState *bus) > } > > /* Get bad features of the plugged device. */ > -uint32_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus) > +uint64_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus) > { > VirtIODevice *vdev = virtio_bus_get_device(bus); > VirtioDeviceClass *k; > -- > 2.26.3
On 5/23/21 10:07 AM, Michael S. Tsirkin wrote: > On Thu, May 20, 2021 at 12:28:22PM +0200, Philippe Mathieu-Daudé wrote: >> In commit 019a3edbb25 ("virtio: make features 64bit wide") we >> increased the 'features' field to 64-bit, but forgot to update >> the virtio_bus_get_vdev_bad_features() helper. The 'bad features' >> are truncated to 32-bit. The virtio_net_bad_features() handler >> from the virtio-net devices is potentially affected. > > I'm fine with increasing it for consistency, but bad features > are all legacy things aren't they? So there isn't a functional > issue ... or did I miss anything? Are you saying all bad legacy features are < 32-bit and there is no potential problem? > >> >> Have the virtio_bus_get_vdev_bad_features() helper return the >> full 64-bit value. >> >> Cc: qemu-stable@nongnu.org >> Fixes: 019a3edbb25 ("virtio: make features 64bit wide") >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> --- >> include/hw/virtio/virtio-bus.h | 2 +- >> hw/virtio/virtio-bus.c | 2 +- >> 2 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h >> index ef8abe49c5a..f9955ff577a 100644 >> --- a/include/hw/virtio/virtio-bus.h >> +++ b/include/hw/virtio/virtio-bus.h >> @@ -122,7 +122,7 @@ uint16_t virtio_bus_get_vdev_id(VirtioBusState *bus); >> /* Get the config_len field of the plugged device. */ >> size_t virtio_bus_get_vdev_config_len(VirtioBusState *bus); >> /* Get bad features of the plugged device. */ >> -uint32_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus); >> +uint64_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus); >> /* Get config of the plugged device. */ >> void virtio_bus_get_vdev_config(VirtioBusState *bus, uint8_t *config); >> /* Set config of the plugged device. */ >> diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c >> index 859978d2487..25a2b68a234 100644 >> --- a/hw/virtio/virtio-bus.c >> +++ b/hw/virtio/virtio-bus.c >> @@ -134,7 +134,7 @@ size_t virtio_bus_get_vdev_config_len(VirtioBusState *bus) >> } >> >> /* Get bad features of the plugged device. */ >> -uint32_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus) >> +uint64_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus) >> { >> VirtIODevice *vdev = virtio_bus_get_device(bus); >> VirtioDeviceClass *k; >> -- >> 2.26.3 >
On Mon, May 24, 2021 at 08:21:48PM +0200, Philippe Mathieu-Daudé wrote: > On 5/23/21 10:07 AM, Michael S. Tsirkin wrote: > > On Thu, May 20, 2021 at 12:28:22PM +0200, Philippe Mathieu-Daudé wrote: > >> In commit 019a3edbb25 ("virtio: make features 64bit wide") we > >> increased the 'features' field to 64-bit, but forgot to update > >> the virtio_bus_get_vdev_bad_features() helper. The 'bad features' > >> are truncated to 32-bit. The virtio_net_bad_features() handler > >> from the virtio-net devices is potentially affected. > > > > I'm fine with increasing it for consistency, but bad features > > are all legacy things aren't they? So there isn't a functional > > issue ... or did I miss anything? > > Are you saying all bad legacy features are < 32-bit and there is no > potential problem? I think so yes. I agree with the change, I jus think it's cosmetic and the commit log should say so. > > > >> > >> Have the virtio_bus_get_vdev_bad_features() helper return the > >> full 64-bit value. > >> > >> Cc: qemu-stable@nongnu.org > >> Fixes: 019a3edbb25 ("virtio: make features 64bit wide") > >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > >> --- > >> include/hw/virtio/virtio-bus.h | 2 +- > >> hw/virtio/virtio-bus.c | 2 +- > >> 2 files changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h > >> index ef8abe49c5a..f9955ff577a 100644 > >> --- a/include/hw/virtio/virtio-bus.h > >> +++ b/include/hw/virtio/virtio-bus.h > >> @@ -122,7 +122,7 @@ uint16_t virtio_bus_get_vdev_id(VirtioBusState *bus); > >> /* Get the config_len field of the plugged device. */ > >> size_t virtio_bus_get_vdev_config_len(VirtioBusState *bus); > >> /* Get bad features of the plugged device. */ > >> -uint32_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus); > >> +uint64_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus); > >> /* Get config of the plugged device. */ > >> void virtio_bus_get_vdev_config(VirtioBusState *bus, uint8_t *config); > >> /* Set config of the plugged device. */ > >> diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c > >> index 859978d2487..25a2b68a234 100644 > >> --- a/hw/virtio/virtio-bus.c > >> +++ b/hw/virtio/virtio-bus.c > >> @@ -134,7 +134,7 @@ size_t virtio_bus_get_vdev_config_len(VirtioBusState *bus) > >> } > >> > >> /* Get bad features of the plugged device. */ > >> -uint32_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus) > >> +uint64_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus) > >> { > >> VirtIODevice *vdev = virtio_bus_get_device(bus); > >> VirtioDeviceClass *k; > >> -- > >> 2.26.3 > >
diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h index ef8abe49c5a..f9955ff577a 100644 --- a/include/hw/virtio/virtio-bus.h +++ b/include/hw/virtio/virtio-bus.h @@ -122,7 +122,7 @@ uint16_t virtio_bus_get_vdev_id(VirtioBusState *bus); /* Get the config_len field of the plugged device. */ size_t virtio_bus_get_vdev_config_len(VirtioBusState *bus); /* Get bad features of the plugged device. */ -uint32_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus); +uint64_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus); /* Get config of the plugged device. */ void virtio_bus_get_vdev_config(VirtioBusState *bus, uint8_t *config); /* Set config of the plugged device. */ diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c index 859978d2487..25a2b68a234 100644 --- a/hw/virtio/virtio-bus.c +++ b/hw/virtio/virtio-bus.c @@ -134,7 +134,7 @@ size_t virtio_bus_get_vdev_config_len(VirtioBusState *bus) } /* Get bad features of the plugged device. */ -uint32_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus) +uint64_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus) { VirtIODevice *vdev = virtio_bus_get_device(bus); VirtioDeviceClass *k;
In commit 019a3edbb25 ("virtio: make features 64bit wide") we increased the 'features' field to 64-bit, but forgot to update the virtio_bus_get_vdev_bad_features() helper. The 'bad features' are truncated to 32-bit. The virtio_net_bad_features() handler from the virtio-net devices is potentially affected. Have the virtio_bus_get_vdev_bad_features() helper return the full 64-bit value. Cc: qemu-stable@nongnu.org Fixes: 019a3edbb25 ("virtio: make features 64bit wide") Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- include/hw/virtio/virtio-bus.h | 2 +- hw/virtio/virtio-bus.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)