Re: [Qemu-devel] [PATCH v4 1/4] vhost-user: add new vhost user messages to support virtio config space

[Stefan Hajnoczi]

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) {

> +        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.

> +    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)