* [PATCH v2 0/2] virtio: remove unnecessary host_features in ->get_features()
@ 2022-08-03 17:36 Stefan Hajnoczi
2022-08-03 17:36 ` [PATCH v2 1/2] virtio: document vdc->get_features() callback Stefan Hajnoczi
2022-08-03 17:36 ` [PATCH v2 2/2] virtio: remove unnecessary host_features in ->get_features() Stefan Hajnoczi
0 siblings, 2 replies; 5+ messages in thread
From: Stefan Hajnoczi @ 2022-08-03 17:36 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Kevin Wolf, Fam Zheng, Amit Shah, Laurent Vivier,
Hanna Reitz, Michael S . Tsirkin, Stefan Hajnoczi,
Marc-André Lureau, Jason Wang, Paolo Bonzini,
David Hildenbrand
v2:
- Document vdv->get_features() callback [Cornelia]
The vdc->get_features() callbacks are a little inconsistent in how they use
vdev->host_features. This is because the function's behavior changed over time.
Clean things up.
Stefan Hajnoczi (2):
virtio: document vdc->get_features() callback
virtio: remove unnecessary host_features in ->get_features()
include/hw/virtio/virtio.h | 20 ++++++++++++++++++++
hw/block/virtio-blk.c | 3 ---
hw/char/virtio-serial-bus.c | 1 -
hw/net/virtio-net.c | 3 ---
hw/scsi/vhost-scsi-common.c | 3 ---
hw/scsi/virtio-scsi.c | 4 ----
hw/virtio/virtio-balloon.c | 2 --
7 files changed, 20 insertions(+), 16 deletions(-)
--
2.37.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] virtio: document vdc->get_features() callback
2022-08-03 17:36 [PATCH v2 0/2] virtio: remove unnecessary host_features in ->get_features() Stefan Hajnoczi
@ 2022-08-03 17:36 ` Stefan Hajnoczi
2022-08-04 10:14 ` Cornelia Huck
2022-08-03 17:36 ` [PATCH v2 2/2] virtio: remove unnecessary host_features in ->get_features() Stefan Hajnoczi
1 sibling, 1 reply; 5+ messages in thread
From: Stefan Hajnoczi @ 2022-08-03 17:36 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Kevin Wolf, Fam Zheng, Amit Shah, Laurent Vivier,
Hanna Reitz, Michael S . Tsirkin, Stefan Hajnoczi,
Marc-André Lureau, Jason Wang, Paolo Bonzini,
David Hildenbrand, Cornelia Huck
Suggested-by: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
include/hw/virtio/virtio.h | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index db1c0ddf6b..8d27fe1824 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -120,9 +120,29 @@ struct VirtioDeviceClass {
/* This is what a VirtioDevice must implement */
DeviceRealize realize;
DeviceUnrealize unrealize;
+
+ /**
+ * get_features:
+ * @vdev: the VirtIODevice
+ * @requested_features: existing device feature bits from
+ * vdev->host_features
+ * @errp: pointer to error object
+ *
+ * Get the device feature bits.
+ *
+ * The ->get_features() function typically sets always-on device feature
+ * bits as well as conditional feature bits that require some logic to
+ * compute.
+ *
+ * Device feature bits can also be set in vdev->host_features before this
+ * function is called using DEFINE_PROP_BIT64() qdev properties.
+ *
+ * Returns: the final device feature bits to store in vdev->host_features.
+ */
uint64_t (*get_features)(VirtIODevice *vdev,
uint64_t requested_features,
Error **errp);
+
uint64_t (*bad_features)(VirtIODevice *vdev);
void (*set_features)(VirtIODevice *vdev, uint64_t val);
int (*validate_features)(VirtIODevice *vdev);
--
2.37.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] virtio: remove unnecessary host_features in ->get_features()
2022-08-03 17:36 [PATCH v2 0/2] virtio: remove unnecessary host_features in ->get_features() Stefan Hajnoczi
2022-08-03 17:36 ` [PATCH v2 1/2] virtio: document vdc->get_features() callback Stefan Hajnoczi
@ 2022-08-03 17:36 ` Stefan Hajnoczi
2022-08-04 10:14 ` Cornelia Huck
1 sibling, 1 reply; 5+ messages in thread
From: Stefan Hajnoczi @ 2022-08-03 17:36 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Kevin Wolf, Fam Zheng, Amit Shah, Laurent Vivier,
Hanna Reitz, Michael S . Tsirkin, Stefan Hajnoczi,
Marc-André Lureau, Jason Wang, Paolo Bonzini,
David Hildenbrand, Cornelia Huck
Since at least commit 6b8f1020540c27246277377aa2c3331ad2bfb160 ("virtio:
move host_features") the ->get_features() function has been called with
host_features as an argument.
Some devices manually add host_features in ->get_features() although the
features argument already contains host_features. Make all devices
consistent by dropping the unnecessary code.
Cc: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/block/virtio-blk.c | 3 ---
hw/char/virtio-serial-bus.c | 1 -
hw/net/virtio-net.c | 3 ---
hw/scsi/vhost-scsi-common.c | 3 ---
hw/scsi/virtio-scsi.c | 4 ----
hw/virtio/virtio-balloon.c | 2 --
6 files changed, 16 deletions(-)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index e9ba752f6b..429aedcf2b 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -996,9 +996,6 @@ static uint64_t virtio_blk_get_features(VirtIODevice *vdev, uint64_t features,
{
VirtIOBlock *s = VIRTIO_BLK(vdev);
- /* Firstly sync all virtio-blk possible supported features */
- features |= s->host_features;
-
virtio_add_feature(&features, VIRTIO_BLK_F_SEG_MAX);
virtio_add_feature(&features, VIRTIO_BLK_F_GEOMETRY);
virtio_add_feature(&features, VIRTIO_BLK_F_TOPOLOGY);
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 7d4601cb5d..1414fb85ae 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -557,7 +557,6 @@ static uint64_t get_features(VirtIODevice *vdev, uint64_t features,
vser = VIRTIO_SERIAL(vdev);
- features |= vser->host_features;
if (vser->bus.max_nr_ports > 1) {
virtio_add_feature(&features, VIRTIO_CONSOLE_F_MULTIPORT);
}
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index dd0d056fde..8ecdc1cd83 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -715,9 +715,6 @@ static uint64_t virtio_net_get_features(VirtIODevice *vdev, uint64_t features,
VirtIONet *n = VIRTIO_NET(vdev);
NetClientState *nc = qemu_get_queue(n->nic);
- /* Firstly sync all virtio-net possible supported features */
- features |= n->host_features;
-
virtio_add_feature(&features, VIRTIO_NET_F_MAC);
if (!peer_has_vnet_hdr(n)) {
diff --git a/hw/scsi/vhost-scsi-common.c b/hw/scsi/vhost-scsi-common.c
index 767f827e55..8b26f90aa1 100644
--- a/hw/scsi/vhost-scsi-common.c
+++ b/hw/scsi/vhost-scsi-common.c
@@ -124,9 +124,6 @@ uint64_t vhost_scsi_common_get_features(VirtIODevice *vdev, uint64_t features,
{
VHostSCSICommon *vsc = VHOST_SCSI_COMMON(vdev);
- /* Turn on predefined features supported by this device */
- features |= vsc->host_features;
-
return vhost_get_features(&vsc->dev, vsc->feature_bits, features);
}
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 4141dddd51..f754611dfe 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -816,10 +816,6 @@ static uint64_t virtio_scsi_get_features(VirtIODevice *vdev,
uint64_t requested_features,
Error **errp)
{
- VirtIOSCSI *s = VIRTIO_SCSI(vdev);
-
- /* Firstly sync all virtio-scsi possible supported features */
- requested_features |= s->host_features;
return requested_features;
}
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 73ac5eb675..0e9ca71b15 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -796,8 +796,6 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
static uint64_t virtio_balloon_get_features(VirtIODevice *vdev, uint64_t f,
Error **errp)
{
- VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
- f |= dev->host_features;
virtio_add_feature(&f, VIRTIO_BALLOON_F_STATS_VQ);
return f;
--
2.37.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] virtio: document vdc->get_features() callback
2022-08-03 17:36 ` [PATCH v2 1/2] virtio: document vdc->get_features() callback Stefan Hajnoczi
@ 2022-08-04 10:14 ` Cornelia Huck
0 siblings, 0 replies; 5+ messages in thread
From: Cornelia Huck @ 2022-08-04 10:14 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel
Cc: qemu-block, Kevin Wolf, Fam Zheng, Amit Shah, Laurent Vivier,
Hanna Reitz, Michael S . Tsirkin, Stefan Hajnoczi,
Marc-André Lureau, Jason Wang, Paolo Bonzini,
David Hildenbrand
On Wed, Aug 03 2022, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> Suggested-by: Cornelia Huck <cohuck@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> include/hw/virtio/virtio.h | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index db1c0ddf6b..8d27fe1824 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -120,9 +120,29 @@ struct VirtioDeviceClass {
> /* This is what a VirtioDevice must implement */
> DeviceRealize realize;
> DeviceUnrealize unrealize;
> +
> + /**
> + * get_features:
> + * @vdev: the VirtIODevice
> + * @requested_features: existing device feature bits from
> + * vdev->host_features
> + * @errp: pointer to error object
> + *
> + * Get the device feature bits.
> + *
> + * The ->get_features() function typically sets always-on device feature
> + * bits as well as conditional feature bits that require some logic to
> + * compute.
> + *
> + * Device feature bits can also be set in vdev->host_features before this
> + * function is called using DEFINE_PROP_BIT64() qdev properties.
> + *
> + * Returns: the final device feature bits to store in vdev->host_features.
> + */
Not sure if we want to go full function doc for features, as none of the
other callbacks have it...
I thought about something like
"Called with vdev->host_features in requested_features. Returns device
feature bits to be stored in vdev->host_features after factoring in
device-specific feature bits."
The important part IMHO is that requested_features contains
vdev->host_features, so no need to merge them in.
> uint64_t (*get_features)(VirtIODevice *vdev,
> uint64_t requested_features,
> Error **errp);
> +
> uint64_t (*bad_features)(VirtIODevice *vdev);
> void (*set_features)(VirtIODevice *vdev, uint64_t val);
> int (*validate_features)(VirtIODevice *vdev);
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] virtio: remove unnecessary host_features in ->get_features()
2022-08-03 17:36 ` [PATCH v2 2/2] virtio: remove unnecessary host_features in ->get_features() Stefan Hajnoczi
@ 2022-08-04 10:14 ` Cornelia Huck
0 siblings, 0 replies; 5+ messages in thread
From: Cornelia Huck @ 2022-08-04 10:14 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel
Cc: qemu-block, Kevin Wolf, Fam Zheng, Amit Shah, Laurent Vivier,
Hanna Reitz, Michael S . Tsirkin, Stefan Hajnoczi,
Marc-André Lureau, Jason Wang, Paolo Bonzini,
David Hildenbrand
On Wed, Aug 03 2022, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> Since at least commit 6b8f1020540c27246277377aa2c3331ad2bfb160 ("virtio:
> move host_features") the ->get_features() function has been called with
> host_features as an argument.
>
> Some devices manually add host_features in ->get_features() although the
> features argument already contains host_features. Make all devices
> consistent by dropping the unnecessary code.
>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> hw/block/virtio-blk.c | 3 ---
> hw/char/virtio-serial-bus.c | 1 -
> hw/net/virtio-net.c | 3 ---
> hw/scsi/vhost-scsi-common.c | 3 ---
> hw/scsi/virtio-scsi.c | 4 ----
> hw/virtio/virtio-balloon.c | 2 --
> 6 files changed, 16 deletions(-)
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-08-04 10:22 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-03 17:36 [PATCH v2 0/2] virtio: remove unnecessary host_features in ->get_features() Stefan Hajnoczi
2022-08-03 17:36 ` [PATCH v2 1/2] virtio: document vdc->get_features() callback Stefan Hajnoczi
2022-08-04 10:14 ` Cornelia Huck
2022-08-03 17:36 ` [PATCH v2 2/2] virtio: remove unnecessary host_features in ->get_features() Stefan Hajnoczi
2022-08-04 10:14 ` Cornelia Huck
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.