[Qemu-devel] Re: [PATCH v2 4/8] qemu:virtio-net: Add a virtqueue for control commands from the guest

[Anthony Liguori <anthony@codemonkey.ws>]

Alex Williamson wrote:
> This will be used for RX mode, MAC table, VLAN table control, etc...
>
> The control transaction consists of one or more "out" sg entries and
> one or more "in" sg entries.  The first out entry contains a header
> defining the class and command.  Additional out entries may provide
> data for the command.  A response via the ack entry is required
> and the guest will typically be waiting for it.
>
> Signed-off-by: Alex Williamson <alex.williamson@hp.com>
> ---
>
>  hw/virtio-net.c |   33 ++++++++++++++++++++++++++++++++-
>  hw/virtio-net.h |   18 ++++++++++++++++++
>  2 files changed, 50 insertions(+), 1 deletions(-)
>
> diff --git a/hw/virtio-net.c b/hw/virtio-net.c
> index 073f23a..3378cc2 100644
> --- a/hw/virtio-net.c
> +++ b/hw/virtio-net.c
> @@ -25,6 +25,7 @@ typedef struct VirtIONet
>      uint16_t status;
>      VirtQueue *rx_vq;
>      VirtQueue *tx_vq;
> +    VirtQueue *ctrl_vq;
>      VLANClientState *vc;
>      QEMUTimer *tx_timer;
>      int tx_timer_active;
> @@ -79,7 +80,9 @@ static void virtio_net_set_link_status(VLANClientState *vc)
>  
>  static uint32_t virtio_net_get_features(VirtIODevice *vdev)
>  {
> -    uint32_t features = (1 << VIRTIO_NET_F_MAC) | (1 << VIRTIO_NET_F_STATUS);
> +    uint32_t features = (1 << VIRTIO_NET_F_MAC) |
> +                        (1 << VIRTIO_NET_F_STATUS) |
> +                        (1 << VIRTIO_NET_F_CTRL_VQ);
>  
>      return features;
>  }
> @@ -91,6 +94,33 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features)
>      n->mergeable_rx_bufs = !!(features & (1 << VIRTIO_NET_F_MRG_RXBUF));
>  }
>  
> +static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
> +{
> +    struct virtio_net_ctrl_hdr *ctrl;
> +    virtio_net_ctrl_ack *status;
> +    VirtQueueElement elem;
> +
> +    while (virtqueue_pop(vq, &elem)) {
> +        if ((elem.in_num < 1) | (elem.out_num < 1)) {
> +            fprintf(stderr, "virtio-net ctrl missing headers\n");
> +            exit(1);
> +        }
> +
> +        if (elem.out_sg[0].iov_len < sizeof(*ctrl) ||
> +            elem.out_sg[elem.in_num - 1].iov_len < sizeof(*status)) {
> +            fprintf(stderr, "virtio-net ctrl header not in correct element\n");
> +            exit(1);
> +        }
> +
> +        ctrl = elem.out_sg[0].iov_base;
> +        status = elem.in_sg[elem.in_num - 1].iov_base;
>   

This works only because we're dealing with uint8_ts but it's broken 
elsewhere in virtio-net.  iov_base is guest memory which means it may 
have different endianness.  We really should be using ldub_p() here.

> +        *status = VIRTIO_NET_ERR;
>   

And stb_p() here.

Regards,

Anthony Liguori

> +        virtqueue_push(vq, &elem, sizeof(*status));
> +        virtio_notify(vdev, vq);
> +    }
> +}
> +
>  /* RX */
>  
>  static void virtio_net_handle_rx(VirtIODevice *vdev, VirtQueue *vq)
> @@ -356,6 +386,7 @@ void virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn)
>      n->vdev.set_features = virtio_net_set_features;
>      n->rx_vq = virtio_add_queue(&n->vdev, 256, virtio_net_handle_rx);
>      n->tx_vq = virtio_add_queue(&n->vdev, 256, virtio_net_handle_tx);
> +    n->ctrl_vq = virtio_add_queue(&n->vdev, 16, virtio_net_handle_ctrl);
>      memcpy(n->mac, nd->macaddr, ETH_ALEN);
>      n->status = VIRTIO_NET_S_LINK_UP;
>      n->vc = qemu_new_vlan_client(nd->vlan, nd->model, nd->name,
> diff --git a/hw/virtio-net.h b/hw/virtio-net.h
> index 9dce663..119e38d 100644
> --- a/hw/virtio-net.h
> +++ b/hw/virtio-net.h
> @@ -40,6 +40,7 @@
>  #define VIRTIO_NET_F_HOST_UFO   14      /* Host can handle UFO in. */
>  #define VIRTIO_NET_F_MRG_RXBUF  15      /* Host can merge receive buffers. */
>  #define VIRTIO_NET_F_STATUS     16      /* virtio_net_config.status available */
> +#define VIRTIO_NET_F_CTRL_VQ    17      /* Control channel available */
>  
>  #define VIRTIO_NET_S_LINK_UP    1       /* Link is up */
>  
> @@ -84,4 +85,21 @@ struct virtio_net_hdr_mrg_rxbuf
>  
>  void virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn);
>  
> +/*
> + * Control virtqueue data structures
> + *
> + * The control virtqueue expects a header in the first sg entry
> + * and an ack/status response in the last entry.  Data for the
> + * command goes in between.
> + */
> +struct virtio_net_ctrl_hdr {
> +    uint8_t class;
> +    uint8_t cmd;
> +};
> +
> +typedef uint8_t virtio_net_ctrl_ack;
> +
> +#define VIRTIO_NET_OK     0
> +#define VIRTIO_NET_ERR    1
> +
>  #endif
>
>