Re: [RFC] virtio-blk PCI backend

[Dor Laor <dor.laor-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

Anthony Liguori wrote:
> This still needs quite a lot of work but I wanted to post it for reference.
>
> Regards,
>
> Anthony Liguori
>
> diff --git a/qemu/Makefile.target b/qemu/Makefile.target
>   
...
Why change Rusty's codding standard? It will be harder to track changes.
> +typedef struct VRingDesc
> +{
> +    uint64_t addr;
> +    uint32_t len;
> +    uint16_t flags;
> +    uint16_t next;
> +} VRingDesc;
> +
> +typedef struct VRingAvail
> +{
> +    uint16_t flags;
> +    uint16_t idx;
> +    uint16_t ring[0];
> +} VRingAvail;
> +
> +typedef struct VRingUsedElem
> +{
> +    uint32_t id;
> +    uint32_t len;
> +} VRingUsedElem;
> +
> +typedef struct VRingUsed
> +{
> +    uint16_t flags;
> +    uint16_t idx;
> +    VRingUsedElem ring[0];
> +} VRingUsed;
> +
> +typedef struct VRing
> +{
> +    unsigned int num;
> +    VRingDesc *desc;
> +    VRingAvail *avail;
> +    VRingUsed *used;
> +} VRing;
> +
....
> +static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)
> +{
> +    VirtIODevice *vdev = to_virtio_device(opaque);
> +    ram_addr_t pa;
> +
> +    addr -= vdev->addr;
> +
> +    switch (addr) {
> +    case VIRTIO_PCI_GUEST_FEATURES:
> +	if (vdev->set_features)
> +	    vdev->set_features(vdev->opaque, val);
> +	vdev->features = val;
> +	break;
> +    case VIRTIO_PCI_QUEUE_PFN:
> +	pa = (ram_addr_t)val << TARGET_PAGE_BITS;
> +	vdev->vq[vdev->queue_sel].pfn = val;
>   
Some validity checks are missing, you assume you have the queue_sel.
> +	if (pa < (ram_size - TARGET_PAGE_SIZE))
> +	    vring_init(&vdev->vq[vdev->queue_sel], phys_ram_base + pa);
> +	break;
> +    case VIRTIO_PCI_QUEUE_SEL:
> +	if (val < VIRTIO_PCI_QUEUE_MAX)
> +	    vdev->queue_sel = val;
> +	break;
> +    case VIRTIO_PCI_QUEUE_NOTIFY:
> +	if (val < VIRTIO_PCI_QUEUE_MAX)
> +	    virtio_ring_kick(vdev, &vdev->vq[val]);
> +	break;
> +    case VIRTIO_PCI_STATUS:
> +	vdev->status = val & 0xFF;
>   
we should keep another internal status and it will track the 
initialization of all the above fields (
pfn, queue_sel,..) the device will be active once all of them were 
initialized by the guest
> +	break;
> +    default:
> +	if (addr >= VIRTIO_PCI_CONFIG && vdev->set_config)
> +	    vdev->set_config(vdev->opaque, addr - VIRTIO_PCI_CONFIG, val);
> +	break;
> +    }
> +}
> +
>   

What about having block/net/9p.. in separate files? It will grow over time.
> +#include <linux/virtio_blk.h>
> +#include <stdbool.h>
> +
> +#define BLK_MAX_QUEUE_SIZE	127
> +
> +static bool virtio_blk_handle_request(BlockDriverState *bs,
> +				      VirtIODevice *vdev, VirtQueue *vq)
> +{
> +    struct iovec iov[vq->vring.num];
> +    unsigned int head, out_num, in_num, wlen;
> +    struct virtio_blk_inhdr *in;
> +    struct virtio_blk_outhdr *out;
> +
Great job, Dor.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/