:p
atchew
Login
This series 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. Apart from that it fixes two issues reported by Peter Maydell: * Stop probing CVQ isolation if cannot set features (goto missed). * Fix incorrect error message statis "error setting features", while it should say status. Eugenio Pérez (3): vdpa net: fix error message setting virtio status vdpa net: stop probing if cannot set features vdpa net: follow VirtIO initialization properly at cvq isolation probing net/vhost-vdpa.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) -- 2.39.3
It incorrectly prints "error setting features", probably because a copy paste miss. Reported-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- net/vhost-vdpa.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c index XXXXXXX..XXXXXXX 100644 --- a/net/vhost-vdpa.c +++ b/net/vhost-vdpa.c @@ -XXX,XX +XXX,XX @@ static int vhost_vdpa_probe_cvq_isolation(int device_fd, uint64_t features, r = ioctl(device_fd, VHOST_VDPA_SET_STATUS, &status); if (unlikely(r)) { - error_setg_errno(errp, -r, "Cannot set device features"); + error_setg_errno(errp, -r, "Cannot set status"); goto out; } -- 2.39.3
Otherwise it continues the CVQ isolation probing. Reported-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- net/vhost-vdpa.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c index XXXXXXX..XXXXXXX 100644 --- a/net/vhost-vdpa.c +++ b/net/vhost-vdpa.c @@ -XXX,XX +XXX,XX @@ static int vhost_vdpa_probe_cvq_isolation(int device_fd, uint64_t features, r = ioctl(device_fd, VHOST_SET_FEATURES, &features); if (unlikely(r)) { error_setg_errno(errp, errno, "Cannot set features"); + goto out; } r = ioctl(device_fd, VHOST_VDPA_SET_STATUS, &status); -- 2.39.3
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 XXXXXXX..XXXXXXX 100644 --- a/net/vhost-vdpa.c +++ b/net/vhost-vdpa.c @@ -XXX,XX +XXX,XX @@ 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(); @@ -XXX,XX +XXX,XX @@ 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; } -- 2.39.3
This series 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. Apart from that it fixes two issues reported by Peter Maydell: * Stop probing CVQ isolation if cannot set features (goto missed). * Fix incorrect error message statis "error setting features", while it should say status. v2: add forgotten Fixes tag Eugenio Pérez (3): vdpa net: fix error message setting virtio status vdpa net: stop probing if cannot set features vdpa net: follow VirtIO initialization properly at cvq isolation probing net/vhost-vdpa.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) -- 2.39.3
It incorrectly prints "error setting features", probably because a copy paste miss. Fixes: 152128d646 ("vdpa: move CVQ isolation check to net_init_vhost_vdpa") Reported-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- net/vhost-vdpa.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c index XXXXXXX..XXXXXXX 100644 --- a/net/vhost-vdpa.c +++ b/net/vhost-vdpa.c @@ -XXX,XX +XXX,XX @@ static int vhost_vdpa_probe_cvq_isolation(int device_fd, uint64_t features, r = ioctl(device_fd, VHOST_VDPA_SET_STATUS, &status); if (unlikely(r)) { - error_setg_errno(errp, -r, "Cannot set device features"); + error_setg_errno(errp, -r, "Cannot set status"); goto out; } -- 2.39.3
Otherwise it continues the CVQ isolation probing. Fixes: 152128d646 ("vdpa: move CVQ isolation check to net_init_vhost_vdpa") Reported-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- net/vhost-vdpa.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c index XXXXXXX..XXXXXXX 100644 --- a/net/vhost-vdpa.c +++ b/net/vhost-vdpa.c @@ -XXX,XX +XXX,XX @@ static int vhost_vdpa_probe_cvq_isolation(int device_fd, uint64_t features, r = ioctl(device_fd, VHOST_SET_FEATURES, &features); if (unlikely(r)) { error_setg_errno(errp, errno, "Cannot set features"); + goto out; } r = ioctl(device_fd, VHOST_VDPA_SET_STATUS, &status); -- 2.39.3
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. Fixes: 152128d646 ("vdpa: move CVQ isolation check to net_init_vhost_vdpa") 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 XXXXXXX..XXXXXXX 100644 --- a/net/vhost-vdpa.c +++ b/net/vhost-vdpa.c @@ -XXX,XX +XXX,XX @@ 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(); @@ -XXX,XX +XXX,XX @@ 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; } -- 2.39.3