VIRTIO_I2C_F_ZERO_LENGTH_REQUEST is a mandatory feature, that must be implemented by everyone. Add its support.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- hw/virtio/vhost-user-i2c.c | 10 ++++++++-- include/hw/virtio/vhost-user-i2c.h | 3 +++ 2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/hw/virtio/vhost-user-i2c.c b/hw/virtio/vhost-user-i2c.c index d172632bb0cd..6323a7203ae4 100644 --- a/hw/virtio/vhost-user-i2c.c +++ b/hw/virtio/vhost-user-i2c.c @@ -19,6 +19,10 @@ #define VIRTIO_ID_I2C_ADAPTER 34 #endif
+static const int feature_bits[] = { + VIRTIO_I2C_F_ZERO_LENGTH_REQUEST +}; + static void vu_i2c_start(VirtIODevice *vdev) { BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev))); @@ -113,8 +117,10 @@ static void vu_i2c_set_status(VirtIODevice *vdev, uint8_t status) static uint64_t vu_i2c_get_features(VirtIODevice *vdev, uint64_t requested_features, Error **errp) { - /* No feature bits used yet */ - return requested_features; + VHostUserI2C *i2c = VHOST_USER_I2C(vdev); + + virtio_add_feature(&requested_features, VIRTIO_I2C_F_ZERO_LENGTH_REQUEST); + return vhost_get_features(&i2c->vhost_dev, feature_bits, requested_features); }
static void vu_i2c_handle_output(VirtIODevice *vdev, VirtQueue *vq) diff --git a/include/hw/virtio/vhost-user-i2c.h b/include/hw/virtio/vhost-user-i2c.h index deae47a76d55..d8372f3b43ea 100644 --- a/include/hw/virtio/vhost-user-i2c.h +++ b/include/hw/virtio/vhost-user-i2c.h @@ -25,4 +25,7 @@ struct VHostUserI2C { bool connected; };
+/* Virtio Feature bits */ +#define VIRTIO_I2C_F_ZERO_LENGTH_REQUEST 0 + #endif /* _QEMU_VHOST_USER_I2C_H */
On 11-01-22, 20:28, Viresh Kumar wrote:
VIRTIO_I2C_F_ZERO_LENGTH_REQUEST is a mandatory feature, that must be implemented by everyone. Add its support.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
hw/virtio/vhost-user-i2c.c | 10 ++++++++-- include/hw/virtio/vhost-user-i2c.h | 3 +++ 2 files changed, 11 insertions(+), 2 deletions(-)
Ping.
Viresh Kumar viresh.kumar@linaro.org writes:
VIRTIO_I2C_F_ZERO_LENGTH_REQUEST is a mandatory feature, that must be implemented by everyone. Add its support.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
Reviewed-by: Alex Bennée alex.bennee@linaro.org
but...
<snip>
BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));@@ -113,8 +117,10 @@ static void vu_i2c_set_status(VirtIODevice *vdev, uint8_t status) static uint64_t vu_i2c_get_features(VirtIODevice *vdev, uint64_t requested_features, Error **errp) {
- /* No feature bits used yet */
- return requested_features;
- VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
- virtio_add_feature(&requested_features, VIRTIO_I2C_F_ZERO_LENGTH_REQUEST);
- return vhost_get_features(&i2c->vhost_dev, feature_bits, requested_features);
}
It's a bit weird we set it and then pass it to the vhost-user backend. It does raise the question of why the stub actually cares about feature bits at all when really it's a negotiation with the backend.
IOW what would happen if we just called:
return vhost_get_features(&i2c->vhost_dev, feature_bits, -1);
static void vu_i2c_handle_output(VirtIODevice *vdev, VirtQueue *vq) diff --git a/include/hw/virtio/vhost-user-i2c.h b/include/hw/virtio/vhost-user-i2c.h index deae47a76d55..d8372f3b43ea 100644 --- a/include/hw/virtio/vhost-user-i2c.h +++ b/include/hw/virtio/vhost-user-i2c.h @@ -25,4 +25,7 @@ struct VHostUserI2C { bool connected; }; +/* Virtio Feature bits */ +#define VIRTIO_I2C_F_ZERO_LENGTH_REQUEST 0
#endif /* _QEMU_VHOST_USER_I2C_H */
On 10-02-22, 08:29, Alex Bennée wrote:
Viresh Kumar viresh.kumar@linaro.org writes:
@@ -113,8 +117,10 @@ static void vu_i2c_set_status(VirtIODevice *vdev, uint8_t status) static uint64_t vu_i2c_get_features(VirtIODevice *vdev, uint64_t requested_features, Error **errp) {
- /* No feature bits used yet */
- return requested_features;
- VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
- virtio_add_feature(&requested_features, VIRTIO_I2C_F_ZERO_LENGTH_REQUEST);
- return vhost_get_features(&i2c->vhost_dev, feature_bits, requested_features);
}
It's a bit weird we set it and then pass it to the vhost-user backend. It does raise the question of why the stub actually cares about feature bits at all when really it's a negotiation with the backend.
IOW what would happen if we just called:
return vhost_get_features(&i2c->vhost_dev, feature_bits, -1);
That works as well.
Also I noticed just now that I haven't added VHOST_INVALID_FEATURE_BIT at the end of the feature_bits[]. Will fix that.
stratos-dev@op-lists.linaro.org
-
Alex Bennée -
Viresh Kumar