| Message ID | 20230915170322.3076956-4-eperezma@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Follow VirtIO initialization properly at vdpa net cvq isolation probing | expand |
On Sat, Sep 16, 2023 at 1:03 AM Eugenio Pérez <eperezma@redhat.com> wrote: > > This patch solves a few issues. The most obvious is that the feature > set was done previous to ACKNOWLEDGE | DRIVER status bit set. Current > vdpa devices are permissive with this, but it is better to follow the > standard. > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com> > --- > net/vhost-vdpa.c | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c > index 51d8144070..4b30325977 100644 > --- a/net/vhost-vdpa.c > +++ b/net/vhost-vdpa.c > @@ -1270,8 +1270,7 @@ static int vhost_vdpa_probe_cvq_isolation(int device_fd, uint64_t features, > uint64_t backend_features; > int64_t cvq_group; > uint8_t status = VIRTIO_CONFIG_S_ACKNOWLEDGE | > - VIRTIO_CONFIG_S_DRIVER | > - VIRTIO_CONFIG_S_FEATURES_OK; > + VIRTIO_CONFIG_S_DRIVER; > int r; > > ERRP_GUARD(); > @@ -1286,15 +1285,22 @@ static int vhost_vdpa_probe_cvq_isolation(int device_fd, uint64_t features, > return 0; > } > > + r = ioctl(device_fd, VHOST_VDPA_SET_STATUS, &status); > + if (unlikely(r)) { > + error_setg_errno(errp, -r, "Cannot set device status"); > + goto out; > + } > + > r = ioctl(device_fd, VHOST_SET_FEATURES, &features); > if (unlikely(r)) { > - error_setg_errno(errp, errno, "Cannot set features"); > + error_setg_errno(errp, -r, "Cannot set features"); > goto out; > } > Spec requires a re-read ? " Re-read device status to ensure the FEATURES_OK bit is still set: otherwise, the device does not support our subset of features and the device is unusable. " Thanks > + status |= VIRTIO_CONFIG_S_FEATURES_OK; > r = ioctl(device_fd, VHOST_VDPA_SET_STATUS, &status); > if (unlikely(r)) { > - error_setg_errno(errp, -r, "Cannot set status"); > + error_setg_errno(errp, -r, "Cannot set device status"); > goto out; > } > > -- > 2.39.3 >
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c index 51d8144070..4b30325977 100644 --- a/net/vhost-vdpa.c +++ b/net/vhost-vdpa.c @@ -1270,8 +1270,7 @@ static int vhost_vdpa_probe_cvq_isolation(int device_fd, uint64_t features, uint64_t backend_features; int64_t cvq_group; uint8_t status = VIRTIO_CONFIG_S_ACKNOWLEDGE | - VIRTIO_CONFIG_S_DRIVER | - VIRTIO_CONFIG_S_FEATURES_OK; + VIRTIO_CONFIG_S_DRIVER; int r; ERRP_GUARD(); @@ -1286,15 +1285,22 @@ static int vhost_vdpa_probe_cvq_isolation(int device_fd, uint64_t features, return 0; } + r = ioctl(device_fd, VHOST_VDPA_SET_STATUS, &status); + if (unlikely(r)) { + error_setg_errno(errp, -r, "Cannot set device status"); + goto out; + } + r = ioctl(device_fd, VHOST_SET_FEATURES, &features); if (unlikely(r)) { - error_setg_errno(errp, errno, "Cannot set features"); + error_setg_errno(errp, -r, "Cannot set features"); goto out; } + status |= VIRTIO_CONFIG_S_FEATURES_OK; r = ioctl(device_fd, VHOST_VDPA_SET_STATUS, &status); if (unlikely(r)) { - error_setg_errno(errp, -r, "Cannot set status"); + error_setg_errno(errp, -r, "Cannot set device status"); goto out; }
This patch solves a few issues. The most obvious is that the feature set was done previous to ACKNOWLEDGE | DRIVER status bit set. Current vdpa devices are permissive with this, but it is better to follow the standard. Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- net/vhost-vdpa.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)