[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 08/16] libqos: implement VIRTIO 1.0 FEATURES_OK step
From: |
Stefan Hajnoczi |
Subject: |
Re: [PATCH v3 08/16] libqos: implement VIRTIO 1.0 FEATURES_OK step |
Date: |
Tue, 22 Oct 2019 16:53:26 +0100 |
User-agent: |
Mutt/1.12.1 (2019-06-15) |
On Mon, Oct 21, 2019 at 02:23:15PM +0200, Thomas Huth wrote:
> On 19/10/2019 08.38, Stefan Hajnoczi wrote:
> > 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.
> >
> > Signed-off-by: Stefan Hajnoczi <address@hidden>
> > ---
> > tests/libqos/virtio.c | 20 +++++++++++++++++---
> > 1 file changed, 17 insertions(+), 3 deletions(-)
> [...]
> > @@ -88,9 +101,10 @@ void qvirtio_set_driver(QVirtioDevice *d)
> >
> > 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);
> > + uint8_t status = d->bus->get_status(d) | VIRTIO_CONFIG_S_DRIVER_OK;
> > +
> > + d->bus->set_status(d, status);
> > + g_assert_cmphex(d->bus->get_status(d), ==, status);
> > }
>
> The changes to qvirtio_set_driver_ok() are not mentioned in the patch
> description. Please either mention them there, or move this to a
> separate patch.
Will fix in v4.
We can no longer hardcode the status register value for DRIVER_OK since
it now depends on whether VIRTIO 1.0 is used or not.
Here is an alternative that is smaller and makes the VIRTIO 1.0
dependency obvious:
- g_assert_cmphex(d->bus->get_status(d), ==, VIRTIO_CONFIG_S_DRIVER_OK |
- VIRTIO_CONFIG_S_DRIVER | VIRTIO_CONFIG_S_ACKNOWLEDGE);
+ g_assert_cmphex(d->bus->get_status(d), ==, VIRTIO_CONFIG_S_DRIVER_OK |
+ VIRTIO_CONFIG_S_DRIVER | VIRTIO_CONFIG_S_ACKNOWLEDGE |
+ (d->features & (1ull << VIRTIO_F_VERSION_1) ?
+ VIRTIO_CONFIG_S_FEATURES_OK : 0));
Stefan
signature.asc
Description: PGP signature
- Re: [PATCH v3 05/16] tests/virtio-blk-test: set up virtqueue after feature negotiation, (continued)
[PATCH v3 08/16] libqos: implement VIRTIO 1.0 FEATURES_OK step, Stefan Hajnoczi, 2019/10/19
[PATCH v3 09/16] libqos: access VIRTIO 1.0 vring in little-endian, Stefan Hajnoczi, 2019/10/19
[PATCH v3 10/16] libqos: add iteration support to qpci_find_capability(), Stefan Hajnoczi, 2019/10/19
[PATCH v3 11/16] libqos: pass full QVirtQueue to set_queue_address(), Stefan Hajnoczi, 2019/10/19
[PATCH v3 12/16] libqos: add MSI-X callbacks to QVirtioPCIDevice, Stefan Hajnoczi, 2019/10/19