Re: [Qemu-devel] [PATCH 3/7] dataplane: add virtqueue vring code

[Stefan Hajnoczi]

On Sat, Nov 17, 2012 at 5:15 PM, Blue Swirl <address@hidden> wrote:
> On Thu, Nov 15, 2012 at 3:19 PM, Stefan Hajnoczi <address@hidden> wrote:
>> The virtio-blk-data-plane cannot access memory using the usual QEMU
>> functions since it executes outside the global mutex and the memory APIs
>> are this time are not thread-safe.
>>
>> This patch introduces a virtqueue module based on the kernel's vhost
>> vring code.  The trick is that we map guest memory ahead of time and
>> access it cheaply outside the global mutex.
>>
>> Once the hardware emulation code can execute outside the global mutex it
>> will be possible to drop this code.
>>
>> Signed-off-by: Stefan Hajnoczi <address@hidden>
>> ---
>>  hw/Makefile.objs           |   2 +-
>>  hw/dataplane/Makefile.objs |   3 +
>>  hw/dataplane/vring.c       | 321 
>> +++++++++++++++++++++++++++++++++++++++++++++
>>  hw/dataplane/vring.h       |  54 ++++++++
>>  trace-events               |   3 +
>>  5 files changed, 382 insertions(+), 1 deletion(-)
>>  create mode 100644 hw/dataplane/Makefile.objs
>>  create mode 100644 hw/dataplane/vring.c
>>  create mode 100644 hw/dataplane/vring.h
>>
>> diff --git a/hw/Makefile.objs b/hw/Makefile.objs
>> index af4ab0c..da8ef0c 100644
>> --- a/hw/Makefile.objs
>> +++ b/hw/Makefile.objs
>> @@ -1,4 +1,4 @@
>> -common-obj-y = usb/ ide/
>> +common-obj-y = usb/ ide/ dataplane/
>>  common-obj-y += loader.o
>>  common-obj-$(CONFIG_VIRTIO) += virtio-console.o
>>  common-obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o
>> diff --git a/hw/dataplane/Makefile.objs b/hw/dataplane/Makefile.objs
>> new file mode 100644
>> index 0000000..b58544f
>> --- /dev/null
>> +++ b/hw/dataplane/Makefile.objs
>> @@ -0,0 +1,3 @@
>> +ifeq ($(CONFIG_VIRTIO), y)
>> +common-obj-$(CONFIG_VIRTIO_BLK_DATA_PLANE) += vring.o
>> +endif
>> diff --git a/hw/dataplane/vring.c b/hw/dataplane/vring.c
>> new file mode 100644
>> index 0000000..6aacce8
>> --- /dev/null
>> +++ b/hw/dataplane/vring.c
>> @@ -0,0 +1,321 @@
>> +/* Copyright 2012 Red Hat, Inc.
>> + * Copyright IBM, Corp. 2012
>> + *
>> + * Based on Linux vhost code:
>> + * Copyright (C) 2009 Red Hat, Inc.
>> + * Copyright (C) 2006 Rusty Russell IBM Corporation
>> + *
>> + * Author: Michael S. Tsirkin <address@hidden>
>> + *         Stefan Hajnoczi <address@hidden>
>> + *
>> + * Inspiration, some code, and most witty comments come from
>> + * Documentation/virtual/lguest/lguest.c, by Rusty Russell
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2.
>> + */
>> +
>> +#include "trace.h"
>> +#include "hw/dataplane/vring.h"
>> +
>> +/* Map target physical address to host address
>> + */
>> +static inline void *phys_to_host(Vring *vring, hwaddr phys)
>> +{
>> +    /* Adjust for 3.6-4 GB PCI memory range */
>> +    if (phys >= 0x100000000) {
>> +        phys -= 0x100000000 - 0xe0000000;
>> +    } else if (phys >= 0xe0000000) {
>> +        fprintf(stderr, "phys_to_host bad physical address in "
>> +                "PCI range %#lx\n", phys);
>> +        exit(1);
>
> Exiting is rather drastic. Is this guest's error or QEMU's?

This can be triggered by the guest - the guest could pass an invalid
physical memory address.  I agree it's better to have an error state
but hw/virtio.c doesn't do this any better today either.  I think the
safest option is to enter an error state where we ignore the vring
until the next (guest-initiated) reset.

>> +int vring_pop(VirtIODevice *vdev, Vring *vring,
>> +              struct iovec iov[], struct iovec *iov_end,
>> +              unsigned int *out_num, unsigned int *in_num)
>> +{
>> +    struct vring_desc desc;
>> +    unsigned int i, head, found = 0, num = vring->vr.num;
>> +    __u16 avail_idx, last_avail_idx;
>
> Please use uint16_t in QEMU code.

Sure, will convert to QEMU types.

Stefan