Re: [Qemu-devel] [PATCH 1/4] virtio-mmio: introduce set_host_notifier()

[Shannon Zhao <zhaoshenglong@huawei.com>]

On 2014/11/19 15:47, Fam Zheng wrote:
> On Tue, 11/04 20:47, Shannon Zhao wrote:
>> set_host_notifier() is introduced into virtio-mmio now. Most of codes came
>> from virtio-pci.
>>
>> Signed-off-by: Ying-Shiuan Pan <yingshiuan.pan@gmail.com>
>> Signed-off-by: Li Liu <john.liuli@huawei.com>
>> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
>> ---
>>  hw/virtio/virtio-mmio.c |   70 +++++++++++++++++++++++++++++++++++++++++++++++
>>  1 files changed, 70 insertions(+), 0 deletions(-)
>>
>> diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
>> index 2450c13..d8ec2d1 100644
>> --- a/hw/virtio/virtio-mmio.c
>> +++ b/hw/virtio/virtio-mmio.c
>> @@ -23,6 +23,7 @@
>>  #include "hw/virtio/virtio.h"
>>  #include "qemu/host-utils.h"
>>  #include "hw/virtio/virtio-bus.h"
>> +#include "qemu/error-report.h"
>>  
>>  /* #define DEBUG_VIRTIO_MMIO */
>>  
>> @@ -87,8 +88,58 @@ typedef struct {
>>      uint32_t guest_page_shift;
>>      /* virtio-bus */
>>      VirtioBusState bus;
>> +    bool ioeventfd_disabled;
>> +    bool ioeventfd_started;
>>  } VirtIOMMIOProxy;
>>  
>> +static int virtio_mmio_set_host_notifier_internal(VirtIOMMIOProxy *proxy,
>> +                                                 int n, bool assign, bool set_handler)
> 
> I didn't review the code, but checkpatch.pl noticed this line and one more
> below [*] is too long (over 80 columes).
> 
>> +{
>> +    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
>> +    VirtQueue *vq = virtio_get_queue(vdev, n);
>> +    EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
>> +    int r = 0;
>> +
>> +    if (assign) {
>> +        r = event_notifier_init(notifier, 1);
>> +        if (r < 0) {
>> +            error_report("%s: unable to init event notifier: %d",
>> +                         __func__, r);
>> +            return r;
>> +        }
>> +        virtio_queue_set_host_notifier_fd_handler(vq, true, set_handler);
>> +        memory_region_add_eventfd(&proxy->iomem, VIRTIO_MMIO_QUEUENOTIFY, 4,
>> +                                  true, n, notifier);
>> +    } else {
>> +        memory_region_del_eventfd(&proxy->iomem, VIRTIO_MMIO_QUEUENOTIFY, 4,
>> +                                  true, n, notifier);
>> +        virtio_queue_set_host_notifier_fd_handler(vq, false, false);
>> +        event_notifier_cleanup(notifier);
>> +    }
>> +    return r;
>> +}
>> +
>> +static void virtio_mmio_stop_ioeventfd(VirtIOMMIOProxy *proxy)
>> +{
>> +    int r;
>> +    int n;
>> +    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
>> +
>> +    if (!proxy->ioeventfd_started) {
>> +        return;
>> +    }
>> +
>> +    for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) {
>> +        if (!virtio_queue_get_num(vdev, n)) {
>> +            continue;
>> +        }
>> +
>> +        r = virtio_mmio_set_host_notifier_internal(proxy, n, false, false);
>> +        assert(r >= 0);
>> +    }
>> +    proxy->ioeventfd_started = false;
>> +}
>> +
>>  static uint64_t virtio_mmio_read(void *opaque, hwaddr offset, unsigned size)
>>  {
>>      VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque;
>> @@ -342,6 +393,24 @@ static void virtio_mmio_reset(DeviceState *d)
>>      proxy->guest_page_shift = 0;
>>  }
>>  
>> +static int virtio_mmio_set_host_notifier(DeviceState *opaque, int n, bool assign)
> 
> [*]
> 
> No need to respin yet just for this. Please wait for a serious review.
> 

Ok,thanks,
Shannon

> Thanks,
> 
> Fam
> 
> .
> 


-- 
Shannon