[v4,08/16] libqos: implement VIRTIO 1.0 FEATURES_OK step
diff mbox series

Message ID 20191023100425.12168-9-stefanha@redhat.com
State New
Headers show
Series
  • libqos: add VIRTIO PCI 1.0 support
Related show

Commit Message

Stefan Hajnoczi Oct. 23, 2019, 10:04 a.m. UTC
Device initialization has an extra step in VIRTIO 1.0.  The FEATURES_OK
status bit is set to indicate that feature negotiation has completed.
The driver then reads the status register again to check that the device
agrees with the final features.

Implement this step as part of qvirtio_set_features() instead of
introducing a separate function.  This way all existing code works
without modifications.

The check in qvirtio_set_driver_ok() needs to be updated because
FEATURES_OK will be set for VIRTIO 1.0 devices.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
v4:
 * Make FEATURES_OK change in qvirtio_set_driver_ok() clearer and
   mention it in the commit description [Thomas]
---
 tests/libqos/virtio.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

Comments

Thomas Huth Oct. 23, 2019, 11:04 a.m. UTC | #1
----- Original Message -----
> From: "Stefan Hajnoczi" <stefanha@redhat.com>
> Sent: Wednesday, October 23, 2019 12:04:17 PM
> 
> Device initialization has an extra step in VIRTIO 1.0.  The FEATURES_OK
> status bit is set to indicate that feature negotiation has completed.
> The driver then reads the status register again to check that the device
> agrees with the final features.
> 
> Implement this step as part of qvirtio_set_features() instead of
> introducing a separate function.  This way all existing code works
> without modifications.
> 
> The check in qvirtio_set_driver_ok() needs to be updated because
> FEATURES_OK will be set for VIRTIO 1.0 devices.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> v4:
>  * Make FEATURES_OK change in qvirtio_set_driver_ok() clearer and
>    mention it in the commit description [Thomas]
> ---
>  tests/libqos/virtio.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)

Reviewed-by: Thomas Huth" <thuth@redhat.com>

Patch
diff mbox series

diff --git a/tests/libqos/virtio.c b/tests/libqos/virtio.c
index 6049ff3b3e..fa597c2481 100644
--- a/tests/libqos/virtio.c
+++ b/tests/libqos/virtio.c
@@ -46,6 +46,20 @@  void qvirtio_set_features(QVirtioDevice *d, uint64_t features)
 {
     d->features = features;
     d->bus->set_features(d, features);
+
+    /*
+     * This could be a separate function for drivers that want to access
+     * configuration space before setting FEATURES_OK, but no existing users
+     * need that and it's less code for callers if this is done implicitly.
+    */
+    if (features & (1ull << VIRTIO_F_VERSION_1)) {
+        uint8_t status = d->bus->get_status(d) |
+                         VIRTIO_CONFIG_S_FEATURES_OK;
+
+        d->bus->set_status(d, status);
+        g_assert_cmphex(d->bus->get_status(d), ==, status);
+    }
+
     d->features_negotiated = true;
 }
 
@@ -86,7 +100,9 @@  void qvirtio_set_driver_ok(QVirtioDevice *d)
 {
     d->bus->set_status(d, d->bus->get_status(d) | VIRTIO_CONFIG_S_DRIVER_OK);
     g_assert_cmphex(d->bus->get_status(d), ==, VIRTIO_CONFIG_S_DRIVER_OK |
-                    VIRTIO_CONFIG_S_DRIVER | VIRTIO_CONFIG_S_ACKNOWLEDGE);
+                    VIRTIO_CONFIG_S_DRIVER | VIRTIO_CONFIG_S_ACKNOWLEDGE |
+                    (d->features & (1ull << VIRTIO_F_VERSION_1) ?
+                     VIRTIO_CONFIG_S_FEATURES_OK : 0));
 }
 
 void qvirtio_wait_queue_isr(QTestState *qts, QVirtioDevice *d,