On Mon, 24 Aug 2015 16:52:45 +0200
Greg Kurz <gk...@linux.vnet.ibm.com> wrote:
> On Fri, 21 Aug 2015 17:05:49 +0800
> Jason Wang <jasow...@redhat.com> wrote:
>
> > We used to use mmio for notification. This could be slow on some arch
> > (e.g on x86 without EPT). So this patch introduces pio bar and a pio
> > notification cap for modern device. This ability is enabled through
> > property "modern-pio-notify" for virtio pci devices and was disabled
> > by default. Management can enable when it thinks it was needed.
> >
> > Benchmarks shows almost no obvious difference with legacy device.
> > Thanks Wenli Quan <wq...@redhat.com> for the benchmarking.
> >
> > Cc: Michael S. Tsirkin <m...@redhat.com>
> > Signed-off-by: Jason Wang <jasow...@redhat.com>
> > ---
> > hw/virtio/virtio-pci.c | 122
> > ++++++++++++++++++++++++++++++++++++++++++-------
> > hw/virtio/virtio-pci.h | 10 ++++
> > 2 files changed, 115 insertions(+), 17 deletions(-)
> >
> > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> > index fbd1f1f..e399565 100644
> > --- a/hw/virtio/virtio-pci.c
> > +++ b/hw/virtio/virtio-pci.c
> > @@ -210,7 +210,9 @@ static int
> > virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
> > EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
> > bool legacy = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_LEGACY);
> > bool modern = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN);
> > + bool modern_pio = proxy->flags & VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY;
> > MemoryRegion *modern_mr = &proxy->notify.mr;
> > + MemoryRegion *modern_notify_mr = &proxy->notify_pio.mr;
> > MemoryRegion *legacy_mr = &proxy->bar;
> > hwaddr modern_addr = QEMU_VIRTIO_PCI_QUEUE_MEM_MULT *
> > virtio_get_queue_index(vq);
> > @@ -228,6 +230,10 @@ static int
> > virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
> > if (modern) {
> > memory_region_add_eventfd(modern_mr, modern_addr, 0,
> > false, n, notifier);
> > + if (modern_pio) {
> > + memory_region_add_eventfd(modern_notify_mr, 0, 2,
> > + true, n, notifier);
> > + }
>
>
> This calls for the following change in memory.c:
>
> static void adjust_endianness(MemoryRegion *mr, uint64_t *data, unsigned
> size)
> {
> - if (memory_region_wrong_endianness(mr)) {
> + if (size && memory_region_wrong_endianness(mr)) {
>
Oops... this comment was for patch 4/6... Please ignore.
>
> otherwise we abort on PPC64.
>
> > }
> > if (legacy) {
> > memory_region_add_eventfd(legacy_mr, legacy_addr, 2,
> > @@ -237,6 +243,10 @@ static int
> > virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
> > if (modern) {
> > memory_region_del_eventfd(modern_mr, modern_addr, 0,
> > false, n, notifier);
> > + if (modern_pio) {
> > + memory_region_del_eventfd(modern_notify_mr, 0, 2,
> > + true, n, notifier);
> > + }
> > }
> > if (legacy) {
> > memory_region_del_eventfd(legacy_mr, legacy_addr, 2,
> > @@ -1344,6 +1354,17 @@ static void virtio_pci_notify_write(void *opaque,
> > hwaddr addr,
> > }
> > }
> >
> > +static void virtio_pci_notify_write_pio(void *opaque, hwaddr addr,
> > + uint64_t val, unsigned size)
> > +{
> > + VirtIODevice *vdev = opaque;
> > + unsigned queue = val;
> > +
> > + if (queue < VIRTIO_QUEUE_MAX) {
> > + virtio_queue_notify(vdev, queue);
> > + }
> > +}
> > +
> > static uint64_t virtio_pci_isr_read(void *opaque, hwaddr addr,
> > unsigned size)
> > {
> > @@ -1437,6 +1458,16 @@ static void
> > virtio_pci_modern_regions_init(VirtIOPCIProxy *proxy)
> > },
> > .endianness = DEVICE_LITTLE_ENDIAN,
> > };
> > + static const MemoryRegionOps notify_pio_ops = {
> > + .read = virtio_pci_notify_read,
> > + .write = virtio_pci_notify_write_pio,
> > + .impl = {
> > + .min_access_size = 1,
> > + .max_access_size = 4,
> > + },
> > + .endianness = DEVICE_LITTLE_ENDIAN,
> > + };
> > +
> >
> > memory_region_init_io(&proxy->common.mr, OBJECT(proxy),
> > &common_ops,
> > @@ -1461,30 +1492,60 @@ static void
> > virtio_pci_modern_regions_init(VirtIOPCIProxy *proxy)
> > virtio_bus_get_device(&proxy->bus),
> > "virtio-pci-notify",
> > proxy->notify.size);
> > +
> > + memory_region_init_io(&proxy->notify_pio.mr, OBJECT(proxy),
> > + ¬ify_pio_ops,
> > + virtio_bus_get_device(&proxy->bus),
> > + "virtio-pci-notify-pio",
> > + proxy->notify.size);
> > }
> >
> > static void virtio_pci_modern_region_map(VirtIOPCIProxy *proxy,
> > VirtIOPCIRegion *region,
> > - struct virtio_pci_cap *cap)
> > + struct virtio_pci_cap *cap,
> > + MemoryRegion *mr,
> > + uint8_t bar)
> > {
> > - memory_region_add_subregion(&proxy->modern_bar,
> > - region->offset,
> > - ®ion->mr);
> > + memory_region_add_subregion(mr, region->offset, ®ion->mr);
> >
> > cap->cfg_type = region->type;
> > - cap->bar = proxy->modern_mem_bar;
> > + cap->bar = bar;
> > cap->offset = cpu_to_le32(region->offset);
> > cap->length = cpu_to_le32(region->size);
> > virtio_pci_add_mem_cap(proxy, cap);
> > +
> > +}
> > +
> > +static void virtio_pci_modern_mem_region_map(VirtIOPCIProxy *proxy,
> > + VirtIOPCIRegion *region,
> > + struct virtio_pci_cap *cap)
> > +{
> > + virtio_pci_modern_region_map(proxy, region, cap,
> > + &proxy->modern_bar,
> > proxy->modern_mem_bar);
> > }
> >
> > -static void virtio_pci_modern_region_unmap(VirtIOPCIProxy *proxy,
> > - VirtIOPCIRegion *region)
> > +static void virtio_pci_modern_io_region_map(VirtIOPCIProxy *proxy,
> > + VirtIOPCIRegion *region,
> > + struct virtio_pci_cap *cap)
> > +{
> > + virtio_pci_modern_region_map(proxy, region, cap,
> > + &proxy->io_bar, proxy->modern_io_bar);
> > +}
> > +
> > +static void virtio_pci_modern_mem_region_unmap(VirtIOPCIProxy *proxy,
> > + VirtIOPCIRegion *region)
> > {
> > memory_region_del_subregion(&proxy->modern_bar,
> > ®ion->mr);
> > }
> >
> > +static void virtio_pci_modern_io_region_unmap(VirtIOPCIProxy *proxy,
> > + VirtIOPCIRegion *region)
> > +{
> > + memory_region_del_subregion(&proxy->io_bar,
> > + ®ion->mr);
> > +}
> > +
> > /* This is called by virtio-bus just after the device is plugged. */
> > static void virtio_pci_device_plugged(DeviceState *d, Error **errp)
> > {
> > @@ -1492,6 +1553,7 @@ static void virtio_pci_device_plugged(DeviceState *d,
> > Error **errp)
> > VirtioBusState *bus = &proxy->bus;
> > bool legacy = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_LEGACY);
> > bool modern = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN);
> > + bool modern_pio = proxy->flags & VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY;
> > uint8_t *config;
> > uint32_t size;
> > VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> > @@ -1530,16 +1592,31 @@ static void virtio_pci_device_plugged(DeviceState
> > *d, Error **errp)
> > .cap.cap_len = sizeof cfg,
> > .cap.cfg_type = VIRTIO_PCI_CAP_PCI_CFG,
> > };
> > - struct virtio_pci_cfg_cap *cfg_mask;
> > + struct virtio_pci_notify_cap notify_pio = {
> > + .cap.cap_len = sizeof notify,
> > + .notify_off_multiplier = cpu_to_le32(0x0),
> > + };
> >
> > - /* TODO: add io access for speed */
> > + struct virtio_pci_cfg_cap *cfg_mask;
> >
> > virtio_add_feature(&vdev->host_features, VIRTIO_F_VERSION_1);
> > virtio_pci_modern_regions_init(proxy);
> > - virtio_pci_modern_region_map(proxy, &proxy->common, &cap);
> > - virtio_pci_modern_region_map(proxy, &proxy->isr, &cap);
> > - virtio_pci_modern_region_map(proxy, &proxy->device, &cap);
> > - virtio_pci_modern_region_map(proxy, &proxy->notify, ¬ify.cap);
> > +
> > + virtio_pci_modern_mem_region_map(proxy, &proxy->common, &cap);
> > + virtio_pci_modern_mem_region_map(proxy, &proxy->isr, &cap);
> > + virtio_pci_modern_mem_region_map(proxy, &proxy->device, &cap);
> > + virtio_pci_modern_mem_region_map(proxy, &proxy->notify,
> > ¬ify.cap);
> > +
> > + if (modern_pio) {
> > + memory_region_init(&proxy->io_bar, OBJECT(proxy),
> > + "virtio-pci-io", 0x4);
> > +
> > + pci_register_bar(&proxy->pci_dev, proxy->modern_io_bar,
> > + PCI_BASE_ADDRESS_SPACE_IO, &proxy->io_bar);
> > +
> > + virtio_pci_modern_io_region_map(proxy, &proxy->notify_pio,
> > + ¬ify_pio.cap);
> > + }
> >
> > pci_register_bar(&proxy->pci_dev, proxy->modern_mem_bar,
> > PCI_BASE_ADDRESS_SPACE_MEMORY |
> > @@ -1592,14 +1669,18 @@ static void virtio_pci_device_unplugged(DeviceState
> > *d)
> > {
> > VirtIOPCIProxy *proxy = VIRTIO_PCI(d);
> > bool modern = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN);
> > + bool modern_pio = proxy->flags & VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY;
> >
> > virtio_pci_stop_ioeventfd(proxy);
> >
> > if (modern) {
> > - virtio_pci_modern_region_unmap(proxy, &proxy->common);
> > - virtio_pci_modern_region_unmap(proxy, &proxy->isr);
> > - virtio_pci_modern_region_unmap(proxy, &proxy->device);
> > - virtio_pci_modern_region_unmap(proxy, &proxy->notify);
> > + virtio_pci_modern_mem_region_unmap(proxy, &proxy->common);
> > + virtio_pci_modern_mem_region_unmap(proxy, &proxy->isr);
> > + virtio_pci_modern_mem_region_unmap(proxy, &proxy->device);
> > + virtio_pci_modern_mem_region_unmap(proxy, &proxy->notify);
> > + if (modern_pio) {
> > + virtio_pci_modern_io_region_unmap(proxy, &proxy->notify_pio);
> > + }
> > }
> > }
> >
> > @@ -1619,6 +1700,7 @@ static void virtio_pci_realize(PCIDevice *pci_dev,
> > Error **errp)
> > */
> > proxy->legacy_io_bar = 0;
> > proxy->msix_bar = 1;
> > + proxy->modern_io_bar = 2;
> > proxy->modern_mem_bar = 4;
> >
> > proxy->common.offset = 0x0;
> > @@ -1638,6 +1720,10 @@ static void virtio_pci_realize(PCIDevice *pci_dev,
> > Error **errp)
> > QEMU_VIRTIO_PCI_QUEUE_MEM_MULT * VIRTIO_QUEUE_MAX;
> > proxy->notify.type = VIRTIO_PCI_CAP_NOTIFY_CFG;
> >
> > + proxy->notify_pio.offset = 0x0;
> > + proxy->notify_pio.size = 0x4;
> > + proxy->notify_pio.type = VIRTIO_PCI_CAP_NOTIFY_CFG;
> > +
> > /* subclasses can enforce modern, so do this unconditionally */
> > memory_region_init(&proxy->modern_bar, OBJECT(proxy), "virtio-pci",
> > 2 * QEMU_VIRTIO_PCI_QUEUE_MEM_MULT *
> > @@ -1684,6 +1770,8 @@ static Property virtio_pci_properties[] = {
> > VIRTIO_PCI_FLAG_DISABLE_MODERN_BIT, true),
> > DEFINE_PROP_BIT("migrate-modern", VirtIOPCIProxy, flags,
> > VIRTIO_PCI_FLAG_MIGRATE_MODERN_BIT, true),
> > + DEFINE_PROP_BIT("modern-pio-notify", VirtIOPCIProxy, flags,
> > + VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY_BIT, false),
> > DEFINE_PROP_END_OF_LIST(),
> > };
> >
> > diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
> > index fd2e889..491edfd 100644
> > --- a/hw/virtio/virtio-pci.h
> > +++ b/hw/virtio/virtio-pci.h
> > @@ -79,6 +79,13 @@ typedef struct VirtioBusClass VirtioPCIBusClass;
> > #define VIRTIO_PCI_FLAG_MIGRATE_MODERN_BIT 4
> > #define VIRTIO_PCI_FLAG_MIGRATE_MODERN (1 <<
> > VIRTIO_PCI_FLAG_MIGRATE_MODERN_BIT)
> >
> > +/* have pio notification for modern device ? */
> > +#define VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY_BIT 5
> > +#define VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY \
> > + (1 << VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY_BIT)
> > +
> > +
> > +
> > typedef struct {
> > MSIMessage msg;
> > int virq;
> > @@ -123,11 +130,14 @@ struct VirtIOPCIProxy {
> > VirtIOPCIRegion isr;
> > VirtIOPCIRegion device;
> > VirtIOPCIRegion notify;
> > + VirtIOPCIRegion notify_pio;
> > MemoryRegion modern_bar;
> > + MemoryRegion io_bar;
> > MemoryRegion modern_cfg;
> > AddressSpace modern_as;
> > uint32_t legacy_io_bar;
> > uint32_t msix_bar;
> > + uint32_t modern_io_bar;
> > uint32_t modern_mem_bar;
> > int config_cap;
> > uint32_t flags;
>
>
