Viresh Kumar <[email protected]> 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 <[email protected]> Reviewed-by: Alex Bennée <[email protected]> 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 */ -- Alex Bennée
