On Thu, Oct 19, 2017 at 04:09:35PM +0200, Stefan Hajnoczi wrote:
> On Thu, Oct 19, 2017 at 01:24:07PM +0800, Changpeng Liu wrote:
> > @@ -922,6 +931,91 @@ static void vhost_user_set_iotlb_callback(struct
> > vhost_dev *dev, int enabled)
> > /* No-op as the receive channel is not dedicated to IOTLB messages. */
> > }
> >
> > +static int vhost_user_get_config(struct vhost_dev *dev, uint8_t *config,
> > + size_t config_len)
> > +{
> > + VhostUserMsg msg = {
> > + .request = VHOST_USER_GET_CONFIG,
> > + .flags = VHOST_USER_VERSION,
> > + .size = config_len,
> > + };
> > +
> > + if (config_len == 0 || config_len > VHOST_USER_PAYLOAD_SIZE) {
>
> config_len should be limited to 256 bytes:
>
> if (config_len == 0 || config_len > sizeof(msg.payload.config) {
I would just limit it to a reasonable value, acceptable to
both master and slave, not fail if it's bigger.
> > + error_report("bad config length");
> > + return -1;
> > + }
> > +
> > + if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
> > + return -1;
> > + }
> > +
> > + if (vhost_user_read(dev, &msg) < 0) {
> > + return -1;
> > + }
> > +
> > + if (msg.request != VHOST_USER_GET_CONFIG) {
> > + error_report("Received unexpected msg type. Expected %d received
> > %d",
> > + VHOST_USER_GET_CONFIG, msg.request);
> > + return -1;
> > + }
> > +
> > + if (msg.size != config_len) {
> > + error_report("Received bad msg size.");
> > + return -1;
> > + }
> > +
> > + memcpy(config, &msg.payload.config, config_len);
>
> There is some complexity here: different virtio devices use different
> amounts of config space. Devices may append new fields to the config
> space to support new features.
>
> Therefore I think the simplest protocol is to always fetch the full
> 256-byte configuration space. This way the vhost-user slave process can
> implement feature bits that the master process does not know about.
>
> In other words, I don't think the master process knows how much of the
> config space is used so it should always request 256 bytes.
Each device knows the max config space size.
vdev->config_len = config_size;
I don't think we need to hard-code 256 bytes in there.
> > + return 0;
> > +}
> > +
> > +static int vhost_user_set_config(struct vhost_dev *dev, const uint8_t
> > *config,
> > + size_t config_len)
> > +{
> > + bool reply_supported = virtio_has_feature(dev->protocol_features,
> > +
> > VHOST_USER_PROTOCOL_F_REPLY_ACK);
> > +
> > + VhostUserMsg msg = {
> > + .request = VHOST_USER_SET_CONFIG,
> > + .flags = VHOST_USER_VERSION,
> > + .size = config_len,
> > + };
> > +
> > + if (reply_supported) {
> > + msg.flags |= VHOST_USER_NEED_REPLY_MASK;
> > + }
> > +
> > + if (config_len == 0 || config_len > VHOST_USER_PAYLOAD_SIZE) {
>
> Same thing here: config_len > sizeof(msg.payload.config)
