Linux virtualization list
 help / color / mirror / Atom feed
* [PATCH -kernel] nvme: improve performance for virtual NVMe devices
From: Ming Lin @ 2015-11-18  5:47 UTC (permalink / raw)
  To: linux-nvme, qemu-devel
  Cc: fes, keith.busch, tytso, virtualization, axboe, Rob Nelson,
	Christoph Hellwig, Mihai Rusu
In-Reply-To: <1447825624-17011-1-git-send-email-mlin@kernel.org>

From: Rob Nelson <rlnelson@google.com>

This change provides a mechanism to reduce the number of MMIO doorbell
writes for the NVMe driver. When running in a virtualized environment
like QEMU, the cost of an MMIO is quite hefy here. The main idea for
the patch is provide the device two memory location locations:
 1) to store the doorbell values so they can be lookup without the doorbell
    MMIO write
 2) to store an event index.
I believe the doorbell value is obvious, the event index not so much.
Similar to the virtio specificaiton, the virtual device can tell the
driver (guest OS) not to write MMIO unless you are writing past this
value.

FYI: doorbell values are written by the nvme driver (guest OS) and the
event index is written by the virtual device (host OS).

The patch implements a new admin command that will communicate where
these two memory locations reside. If the command fails, the nvme
driver will work as before without any optimizations.

Contributions:
  Eric Northup <digitaleric@google.com>
  Frank Swiderski <fes@google.com>
  Ted Tso <tytso@mit.edu>
  Keith Busch <keith.busch@intel.com>

Just to give an idea on the performance boost with the vendor
extension: Running fio [1], a stock NVMe driver I get about 200K read
IOPs with my vendor patch I get about 1000K read IOPs. This was
running with a null device i.e. the backing device simply returned
success on every read IO request.

[1] Running on a 4 core machine:
  fio --time_based --name=benchmark --runtime=30
  --filename=/dev/nvme0n1 --nrfiles=1 --ioengine=libaio --iodepth=32
  --direct=1 --invalidate=1 --verify=0 --verify_fatal=0 --numjobs=4
  --rw=randread --blocksize=4k --randrepeat=false

Signed-off-by: Rob Nelson <rlnelson@google.com>
[mlin: port for upstream]
Signed-off-by: Ming Lin <mlin@kernel.org>
---
 drivers/nvme/host/Kconfig |   7 +++
 drivers/nvme/host/core.c  |   1 +
 drivers/nvme/host/pci.c   | 147 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/nvme.h      |  21 +++++++
 4 files changed, 176 insertions(+)

diff --git a/drivers/nvme/host/Kconfig b/drivers/nvme/host/Kconfig
index 002a94a..93f9438 100644
--- a/drivers/nvme/host/Kconfig
+++ b/drivers/nvme/host/Kconfig
@@ -8,3 +8,10 @@ config BLK_DEV_NVME
 
 	  To compile this driver as a module, choose M here: the
 	  module will be called nvme.
+
+config NVME_VENDOR_EXT_GOOGLE
+	tristate "NVMe Vendor Extension for Improved Virtualization"
+	depends on BLK_DEV_NVME
+	---help---
+	  Google extension to reduce the number of MMIO doorbell
+	  writes for the NVMe driver
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 400b1ea..78ac8bb 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -160,6 +160,7 @@ int nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd,
 {
 	return __nvme_submit_sync_cmd(q, cmd, buffer, bufflen, NULL, 0);
 }
+EXPORT_SYMBOL_GPL(nvme_submit_sync_cmd);
 
 int __nvme_submit_user_cmd(struct request_queue *q, struct nvme_command *cmd,
 		void __user *ubuffer, unsigned bufflen,
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 91522bb..93f1f36 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -49,6 +49,9 @@
 #define SQ_SIZE(depth)		(depth * sizeof(struct nvme_command))
 #define CQ_SIZE(depth)		(depth * sizeof(struct nvme_completion))
 
+/* Google Vendor ID is not in include/linux/pci_ids.h */
+#define PCI_VENDOR_ID_GOOGLE 0x1AE0
+
 static int use_threaded_interrupts;
 module_param(use_threaded_interrupts, int, 0);
 
@@ -106,6 +109,13 @@ struct nvme_dev {
 	unsigned long flags;
 #define NVME_CTRL_RESETTING	0
 
+#ifdef CONFIG_NVME_VENDOR_EXT_GOOGLE
+	u32 *db_mem;
+	dma_addr_t doorbell;
+	u32 *ei_mem;
+	dma_addr_t eventidx;
+#endif
+
 	struct nvme_ctrl ctrl;
 };
 
@@ -139,6 +149,12 @@ struct nvme_queue {
 	u8 cq_phase;
 	u8 cqe_seen;
 	struct async_cmd_info cmdinfo;
+#ifdef CONFIG_NVME_VENDOR_EXT_GOOGLE
+	u32 *sq_doorbell_addr;
+	u32 *sq_eventidx_addr;
+	u32 *cq_doorbell_addr;
+	u32 *cq_eventidx_addr;
+#endif
 };
 
 /*
@@ -176,6 +192,9 @@ static inline void _nvme_check_size(void)
 	BUILD_BUG_ON(sizeof(struct nvme_id_ns) != 4096);
 	BUILD_BUG_ON(sizeof(struct nvme_lba_range_type) != 64);
 	BUILD_BUG_ON(sizeof(struct nvme_smart_log) != 512);
+#ifdef CONFIG_NVME_VENDOR_EXT_GOOGLE
+	BUILD_BUG_ON(sizeof(struct nvme_doorbell_memory) != 64);
+#endif
 }
 
 /*
@@ -289,6 +308,51 @@ static void nvme_finish_aen_cmd(struct nvme_dev *dev, struct nvme_completion *cq
 	}
 }
 
+#ifdef CONFIG_NVME_VENDOR_EXT_GOOGLE
+static int nvme_vendor_memory_size(struct nvme_dev *dev)
+{
+	return ((num_possible_cpus() + 1) * 8 * dev->db_stride);
+}
+
+static int nvme_set_doorbell_memory(struct nvme_dev *dev)
+{
+	struct nvme_command c;
+
+	memset(&c, 0, sizeof(c));
+	c.doorbell_memory.opcode = nvme_admin_doorbell_memory;
+	c.doorbell_memory.prp1 = cpu_to_le64(dev->doorbell);
+	c.doorbell_memory.prp2 = cpu_to_le64(dev->eventidx);
+
+	return nvme_submit_sync_cmd(dev->ctrl.admin_q, &c, NULL, 0);
+}
+
+static inline int nvme_ext_need_event(u16 event_idx, u16 new_idx, u16 old)
+{
+	/* Borrowed from vring_need_event */
+	return (u16)(new_idx - event_idx - 1) < (u16)(new_idx - old);
+}
+
+static void nvme_ext_write_doorbell(u16 value, u32 __iomem* q_db,
+			   u32* db_addr, volatile u32* event_idx)
+{
+	u16 old_value;
+	if (!db_addr)
+		goto ring_doorbell;
+
+	old_value = *db_addr;
+	*db_addr = value;
+
+	rmb();
+	if (!nvme_ext_need_event(*event_idx, value, old_value))
+		goto no_doorbell;
+
+ring_doorbell:
+	writel(value, q_db);
+no_doorbell:
+	return;
+}
+#endif
+
 /**
  * __nvme_submit_cmd() - Copy a command into a queue and ring the doorbell
  * @nvmeq: The queue to use
@@ -306,9 +370,19 @@ static void __nvme_submit_cmd(struct nvme_queue *nvmeq,
 	else
 		memcpy(&nvmeq->sq_cmds[tail], cmd, sizeof(*cmd));
 
+#ifdef CONFIG_NVME_VENDOR_EXT_GOOGLE
+	if (nvmeq->sq_doorbell_addr)
+		wmb();
+#endif
+
 	if (++tail == nvmeq->q_depth)
 		tail = 0;
+#ifdef CONFIG_NVME_VENDOR_EXT_GOOGLE
+	nvme_ext_write_doorbell(tail, nvmeq->q_db,
+		nvmeq->sq_doorbell_addr, nvmeq->sq_eventidx_addr);
+#else
 	writel(tail, nvmeq->q_db);
+#endif
 	nvmeq->sq_tail = tail;
 }
 
@@ -719,6 +793,11 @@ static int nvme_process_cq(struct nvme_queue *nvmeq)
 		u16 status = le16_to_cpu(cqe.status);
 		struct request *req;
 
+#ifdef CONFIG_NVME_VENDOR_EXT_GOOGLE
+		if (to_pci_dev(nvmeq->dev->dev)->vendor == PCI_VENDOR_ID_GOOGLE)
+			rmb();
+#endif
+
 		if ((status & 1) != phase)
 			break;
 		nvmeq->sq_head = le16_to_cpu(cqe.sq_head);
@@ -764,7 +843,12 @@ static int nvme_process_cq(struct nvme_queue *nvmeq)
 	if (head == nvmeq->cq_head && phase == nvmeq->cq_phase)
 		return 0;
 
+#ifdef CONFIG_NVME_VENDOR_EXT_GOOGLE
+	nvme_ext_write_doorbell(head, nvmeq->q_db + nvmeq->dev->db_stride,
+		nvmeq->cq_doorbell_addr, nvmeq->cq_eventidx_addr);
+#else
 	writel(head, nvmeq->q_db + nvmeq->dev->db_stride);
+#endif
 	nvmeq->cq_head = head;
 	nvmeq->cq_phase = phase;
 
@@ -1111,6 +1195,17 @@ static struct nvme_queue *nvme_alloc_queue(struct nvme_dev *dev, int qid,
 	nvmeq->cq_vector = -1;
 	dev->queues[qid] = nvmeq;
 
+#ifdef CONFIG_NVME_VENDOR_EXT_GOOGLE
+	if (dev->db_mem && dev->ei_mem && qid != 0) {
+		nvmeq->sq_doorbell_addr = &dev->db_mem[qid * 2 * dev->db_stride];
+		nvmeq->cq_doorbell_addr =
+			&dev->db_mem[(qid * 2 + 1) * dev->db_stride];
+		nvmeq->sq_eventidx_addr = &dev->ei_mem[qid * 2 * dev->db_stride];
+		nvmeq->cq_eventidx_addr =
+			&dev->ei_mem[(qid * 2 + 1) * dev->db_stride];
+	}
+#endif
+
 	/* make sure queue descriptor is set before queue count, for kthread */
 	mb();
 	dev->queue_count++;
@@ -1145,6 +1240,16 @@ static void nvme_init_queue(struct nvme_queue *nvmeq, u16 qid)
 	nvmeq->cq_head = 0;
 	nvmeq->cq_phase = 1;
 	nvmeq->q_db = &dev->dbs[qid * 2 * dev->db_stride];
+#ifdef CONFIG_NVME_VENDOR_EXT_GOOGLE
+	if (to_pci_dev(dev->dev)->vendor == PCI_VENDOR_ID_GOOGLE && qid != 0) {
+		nvmeq->sq_doorbell_addr = &dev->db_mem[qid * 2 * dev->db_stride];
+		nvmeq->cq_doorbell_addr =
+			&dev->db_mem[(qid * 2 + 1) * dev->db_stride];
+		nvmeq->sq_eventidx_addr = &dev->ei_mem[qid * 2 * dev->db_stride];
+		nvmeq->cq_eventidx_addr =
+			&dev->ei_mem[(qid * 2 + 1) * dev->db_stride];
+	}
+#endif
 	memset((void *)nvmeq->cqes, 0, CQ_SIZE(nvmeq->q_depth));
 	dev->online_queues++;
 	spin_unlock_irq(&nvmeq->q_lock);
@@ -1565,6 +1670,19 @@ static int nvme_dev_add(struct nvme_dev *dev)
 		if (blk_mq_alloc_tag_set(&dev->tagset))
 			return 0;
 		dev->ctrl.tagset = &dev->tagset;
+
+#ifdef CONFIG_NVME_VENDOR_EXT_GOOGLE
+		if (to_pci_dev(dev->dev)->vendor == PCI_VENDOR_ID_GOOGLE) {
+			int res = nvme_set_doorbell_memory(dev);
+			if (res) {
+				// Free memory and continue on.
+				dma_free_coherent(dev->dev, 8192, dev->db_mem, dev->doorbell);
+				dma_free_coherent(dev->dev, 8192, dev->ei_mem, dev->doorbell);
+				dev->db_mem = 0;
+				dev->ei_mem = 0;
+			}
+		}
+#endif
 	}
 	queue_work(nvme_workq, &dev->scan_work);
 	return 0;
@@ -1618,7 +1736,28 @@ static int nvme_dev_map(struct nvme_dev *dev)
 	if (readl(dev->bar + NVME_REG_VS) >= NVME_VS(1, 2))
 		dev->cmb = nvme_map_cmb(dev);
 
+#ifdef CONFIG_NVME_VENDOR_EXT_GOOGLE
+	if (pdev->vendor == PCI_VENDOR_ID_GOOGLE) {
+		int mem_size = nvme_vendor_memory_size(dev);
+		dev->db_mem = dma_alloc_coherent(&pdev->dev, mem_size, &dev->doorbell, GFP_KERNEL);
+		if (!dev->db_mem) {
+			result = -ENOMEM;
+			goto unmap;
+		}
+		dev->ei_mem = dma_alloc_coherent(&pdev->dev, mem_size, &dev->eventidx, GFP_KERNEL);
+		if (!dev->ei_mem) {
+			result = -ENOMEM;
+			goto dma_free;
+		}
+	}
+
+	return 0;
+
+ dma_free:
+	dma_free_coherent(&pdev->dev, nvme_vendor_memory_size(dev), dev->db_mem, dev->doorbell);
+	dev->db_mem = 0;
 	return 0;
+#endif
 
  unmap:
 	iounmap(dev->bar);
@@ -1633,6 +1772,14 @@ static int nvme_dev_map(struct nvme_dev *dev)
 static void nvme_dev_unmap(struct nvme_dev *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev->dev);
+#ifdef CONFIG_NVME_VENDOR_EXT_GOOGLE
+	int mem_size = nvme_vendor_memory_size(dev);
+
+	if (dev->db_mem)
+		dma_free_coherent(&pdev->dev, mem_size, dev->db_mem, dev->doorbell);
+	if (dev->ei_mem)
+		dma_free_coherent(&pdev->dev, mem_size, dev->ei_mem, dev->eventidx);
+#endif
 
 	if (pdev->msi_enabled)
 		pci_disable_msi(pdev);
diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index a55986f..d3a8289 100644
--- a/include/linux/nvme.h
+++ b/include/linux/nvme.h
@@ -387,6 +387,9 @@ enum nvme_admin_opcode {
 	nvme_admin_format_nvm		= 0x80,
 	nvme_admin_security_send	= 0x81,
 	nvme_admin_security_recv	= 0x82,
+#ifdef CONFIG_NVME_VENDOR_EXT_GOOGLE
+	nvme_admin_doorbell_memory	= 0xC0,
+#endif
 };
 
 enum {
@@ -516,6 +519,18 @@ struct nvme_format_cmd {
 	__u32			rsvd11[5];
 };
 
+#ifdef CONFIG_NVME_VENDOR_EXT_GOOGLE
+struct nvme_doorbell_memory {
+	__u8			opcode;
+	__u8			flags;
+	__u16			command_id;
+	__u32			rsvd1[5];
+	__le64			prp1;
+	__le64			prp2;
+	__u32			rsvd12[6];
+};
+#endif
+
 struct nvme_command {
 	union {
 		struct nvme_common_command common;
@@ -529,6 +544,9 @@ struct nvme_command {
 		struct nvme_format_cmd format;
 		struct nvme_dsm_cmd dsm;
 		struct nvme_abort_cmd abort;
+#ifdef CONFIG_NVME_VENDOR_EXT_GOOGLE
+		struct nvme_doorbell_memory doorbell_memory;
+#endif
 	};
 };
 
@@ -575,6 +593,9 @@ enum {
 	NVME_SC_BAD_ATTRIBUTES		= 0x180,
 	NVME_SC_INVALID_PI		= 0x181,
 	NVME_SC_READ_ONLY		= 0x182,
+#ifdef CONFIG_NVME_VENDOR_EXT_GOOGLE
+	NVME_SC_DOORBELL_MEMORY_INVALID	= 0x1C0,
+#endif
 	NVME_SC_WRITE_FAULT		= 0x280,
 	NVME_SC_READ_ERROR		= 0x281,
 	NVME_SC_GUARD_CHECK		= 0x282,
-- 
1.9.1

^ permalink raw reply related

* [RFC PATCH 0/2] Google extension to improve qemu-nvme performance
From: Ming Lin @ 2015-11-18  5:47 UTC (permalink / raw)
  To: linux-nvme, qemu-devel
  Cc: fes, keith.busch, tytso, virtualization, axboe, Rob Nelson,
	Christoph Hellwig, Mihai Rusu

Hi Rob & Mihai,

I wrote vhost-nvme patches on top of Christoph's NVMe target.
vhost-nvme still uses mmio. So the guest OS can run unmodified NVMe
driver. But the tests I have done didn't show competitive performance
compared to virtio-blk/virtio-scsi. The bottleneck is in mmio. Your nvme
vendor extension patches reduces greatly the number of MMIO writes.
So I'd like to push it upstream.

I port these 2 patches to newer kernel and qemu.
I use ram disk as backend to compare performance.

qemu-nvme: 29MB/s
qemu-nvme+google-ext: 100MB/s
virtio-blk: 174MB/s
virtio-scsi: 118MB/s

I'll show you qemu-vhost-nvme+google-ext number later.

root@guest:~# cat test.job 
[global]
bs=4k
ioengine=libaio
iodepth=64
direct=1
runtime=120
time_based
rw=randread
norandommap
group_reporting
gtod_reduce=1
numjobs=2

[job1]
filename=/dev/nvme0n1
#filename=/dev/vdb
#filename=/dev/sda
rw=read

Patches also available at:
kernel:
https://git.kernel.org/cgit/linux/kernel/git/mlin/linux.git/log/?h=nvme-google-ext
qemu:
http://www.minggr.net/cgit/cgit.cgi/qemu/log/?h=nvme-google-ext 

Thanks,
Ming

^ permalink raw reply

* [RFC] kvmtool: add support for modern virtio-pci
From: Sasha Levin @ 2015-11-18  5:11 UTC (permalink / raw)
  To: kvm
  Cc: mst, andre.przywara, will.deacon, josh, virtualization, penberg,
	Sasha Levin

This is a first go at adding support for the modern (based on the 1.0 virtio
spec) virtio-pci implementation.

kvmtool makes it simple to add additional transports such as this because of
it's layering, so we are able to add it as a 3rd (after legacy virtio-pci and
virtio-mmio) transport layer, and still allow users to choose to use either
the legacy or the modern implementations (but setting the modern one as
default.

The changes to the virtio devices are mostly the result of needing to support
>32bit features, and the different initialization method for VQs.

It's worth noting that supporting v1.0 implies any_layout, but some of our
devices made assumptions about the layout - which I've fixed. But it's worth
to keep in mind that some probably went unnoticed.

To sum it up: this is a lightly tested version for feedback about the design
and to weed out major bugs people notice. Feedback is very welcome!

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 Makefile                                          |   1 +
 builtin-run.c                                     |   4 +
 include/kvm/kvm-config.h                          |   1 +
 include/kvm/pci.h                                 |   8 +-
 include/kvm/virtio-9p.h                           |   2 +-
 include/kvm/{virtio-pci.h => virtio-pci-modern.h} |  23 +-
 include/kvm/virtio-pci.h                          |   6 +-
 include/kvm/virtio.h                              |  25 +-
 include/linux/virtio_pci.h                        | 199 +++++++
 net/uip/core.c                                    |   7 +-
 virtio/9p.c                                       |  35 +-
 virtio/balloon.c                                  |  37 +-
 virtio/blk.c                                      |  50 +-
 virtio/console.c                                  |  42 +-
 virtio/core.c                                     |  16 +
 virtio/mmio.c                                     |  13 +-
 virtio/net.c                                      |  59 ++-
 virtio/pci.c                                      |   4 +-
 virtio/pci_modern.c                               | 599 ++++++++++++++++++++++
 virtio/rng.c                                      |  29 +-
 virtio/scsi.c                                     |  36 +-
 x86/include/kvm/kvm-arch.h                        |   2 +-
 22 files changed, 1109 insertions(+), 89 deletions(-)
 copy include/kvm/{virtio-pci.h => virtio-pci-modern.h} (69%)
 create mode 100644 include/linux/virtio_pci.h
 create mode 100644 virtio/pci_modern.c

diff --git a/Makefile b/Makefile
index 59622c3..13a12f8 100644
--- a/Makefile
+++ b/Makefile
@@ -67,6 +67,7 @@ OBJS	+= virtio/net.o
 OBJS	+= virtio/rng.o
 OBJS    += virtio/balloon.o
 OBJS	+= virtio/pci.o
+OBJS	+= virtio/pci_modern.o
 OBJS	+= disk/blk.o
 OBJS	+= disk/qcow.o
 OBJS	+= disk/raw.o
diff --git a/builtin-run.c b/builtin-run.c
index edcaf3e..e133b10 100644
--- a/builtin-run.c
+++ b/builtin-run.c
@@ -128,6 +128,8 @@ void kvm_run_set_wrapper_sandbox(void)
 			" rootfs"),					\
 	OPT_STRING('\0', "hugetlbfs", &(cfg)->hugetlbfs_path, "path",	\
 			"Hugetlbfs path"),				\
+	OPT_BOOLEAN('\0', "virtio-legacy", &(cfg)->old_virtio, "Use"	\
+			" legacy virtio-pci devices"),			\
 									\
 	OPT_GROUP("Kernel options:"),					\
 	OPT_STRING('k', "kernel", &(cfg)->kernel_filename, "kernel",	\
@@ -517,6 +519,8 @@ static struct kvm *kvm_cmd_run_init(int argc, const char **argv)
 	kvm->cfg.vmlinux_filename = find_vmlinux();
 	kvm->vmlinux = kvm->cfg.vmlinux_filename;
 
+	default_transport = kvm->cfg.old_virtio ? VIRTIO_PCI : VIRTIO_PCI_MODERN;
+
 	if (kvm->cfg.nrcpus == 0)
 		kvm->cfg.nrcpus = nr_online_cpus;
 
diff --git a/include/kvm/kvm-config.h b/include/kvm/kvm-config.h
index 386fa8c..b1512a1 100644
--- a/include/kvm/kvm-config.h
+++ b/include/kvm/kvm-config.h
@@ -57,6 +57,7 @@ struct kvm_config {
 	bool no_dhcp;
 	bool ioport_debug;
 	bool mmio_debug;
+	bool old_virtio;
 };
 
 #endif
diff --git a/include/kvm/pci.h b/include/kvm/pci.h
index b0c28a1..19ec56a 100644
--- a/include/kvm/pci.h
+++ b/include/kvm/pci.h
@@ -4,6 +4,7 @@
 #include <linux/types.h>
 #include <linux/kvm.h>
 #include <linux/pci_regs.h>
+#include <linux/virtio_pci.h>
 #include <endian.h>
 
 #include "kvm/devices.h"
@@ -81,7 +82,12 @@ struct pci_device_header {
 	u8		min_gnt;
 	u8		max_lat;
 	struct msix_cap msix;
-	u8		empty[136]; /* Rest of PCI config space */
+	struct virtio_pci_cap common_cap;
+	struct virtio_pci_notify_cap notify_cap;
+	struct virtio_pci_cap isr_cap;
+	struct virtio_pci_cap device_cap;
+	struct virtio_pci_cfg_cap pci_cap;
+	u8		empty[48]; /* Rest of PCI config space */
 	u32		bar_size[6];
 } __attribute__((packed));
 
diff --git a/include/kvm/virtio-9p.h b/include/kvm/virtio-9p.h
index 19ffe50..2f7e25a 100644
--- a/include/kvm/virtio-9p.h
+++ b/include/kvm/virtio-9p.h
@@ -46,7 +46,7 @@ struct p9_dev {
 	struct rb_root		fids;
 
 	struct virtio_9p_config	*config;
-	u32			features;
+	u64			features;
 
 	/* virtio queue */
 	struct virt_queue	vqs[NUM_VIRT_QUEUES];
diff --git a/include/kvm/virtio-pci.h b/include/kvm/virtio-pci-modern.h
similarity index 69%
copy from include/kvm/virtio-pci.h
copy to include/kvm/virtio-pci-modern.h
index b70cadd..f07085a 100644
--- a/include/kvm/virtio-pci.h
+++ b/include/kvm/virtio-pci-modern.h
@@ -1,8 +1,9 @@
-#ifndef KVM__VIRTIO_PCI_H
-#define KVM__VIRTIO_PCI_H
+#ifndef KVM__VIRTIO_PCI_MODERN_H
+#define KVM__VIRTIO_PCI_MODERN_H
 
 #include "kvm/devices.h"
 #include "kvm/pci.h"
+#include "kvm/virtio.h"
 
 #include <linux/types.h>
 
@@ -11,14 +12,9 @@
 
 struct kvm;
 
-struct virtio_pci_ioevent_param {
-	struct virtio_device	*vdev;
-	u32			vq;
-};
-
 #define VIRTIO_PCI_F_SIGNAL_MSI (1 << 0)
 
-struct virtio_pci {
+struct virtio_pci_modern {
 	struct pci_device_header pci_hdr;
 	struct device_header	dev_hdr;
 	void			*dev;
@@ -28,6 +24,9 @@ struct virtio_pci {
 	u32			mmio_addr;
 	u8			status;
 	u8			isr;
+	u32			device_features_sel;
+	u32			driver_features_sel;
+
 	u32			features;
 
 	/*
@@ -52,10 +51,10 @@ struct virtio_pci {
 	struct virtio_pci_ioevent_param ioeventfds[VIRTIO_PCI_MAX_VQ];
 };
 
-int virtio_pci__signal_vq(struct kvm *kvm, struct virtio_device *vdev, u32 vq);
-int virtio_pci__signal_config(struct kvm *kvm, struct virtio_device *vdev);
-int virtio_pci__exit(struct kvm *kvm, struct virtio_device *vdev);
-int virtio_pci__init(struct kvm *kvm, void *dev, struct virtio_device *vdev,
+int virtio_pcim__signal_vq(struct kvm *kvm, struct virtio_device *vdev, u32 vq);
+int virtio_pcim__signal_config(struct kvm *kvm, struct virtio_device *vdev);
+int virtio_pcim__exit(struct kvm *kvm, struct virtio_device *vdev);
+int virtio_pcim__init(struct kvm *kvm, void *dev, struct virtio_device *vdev,
 		     int device_id, int subsys_id, int class);
 
 #endif
diff --git a/include/kvm/virtio-pci.h b/include/kvm/virtio-pci.h
index b70cadd..7f2664e 100644
--- a/include/kvm/virtio-pci.h
+++ b/include/kvm/virtio-pci.h
@@ -3,6 +3,7 @@
 
 #include "kvm/devices.h"
 #include "kvm/pci.h"
+#include "kvm/virtio.h"
 
 #include <linux/types.h>
 
@@ -11,11 +12,6 @@
 
 struct kvm;
 
-struct virtio_pci_ioevent_param {
-	struct virtio_device	*vdev;
-	u32			vq;
-};
-
 #define VIRTIO_PCI_F_SIGNAL_MSI (1 << 0)
 
 struct virtio_pci {
diff --git a/include/kvm/virtio.h b/include/kvm/virtio.h
index 768ee96..3edd74f 100644
--- a/include/kvm/virtio.h
+++ b/include/kvm/virtio.h
@@ -21,6 +21,8 @@
 #define VIRTIO_ENDIAN_LE	(1 << 0)
 #define VIRTIO_ENDIAN_BE	(1 << 1)
 
+extern u8 default_transport;
+
 struct virt_queue {
 	struct vring	vring;
 	u32		pfn;
@@ -29,6 +31,7 @@ struct virt_queue {
 	u16		last_avail_idx;
 	u16		last_used_signalled;
 	u16		endian;
+	u8		enabled;
 };
 
 /*
@@ -125,6 +128,7 @@ u16 virt_queue__get_inout_iov(struct kvm *kvm, struct virt_queue *queue,
 int virtio__get_dev_specific_field(int offset, bool msix, u32 *config_off);
 
 enum virtio_trans {
+	VIRTIO_PCI_MODERN,
 	VIRTIO_PCI,
 	VIRTIO_MMIO,
 };
@@ -138,8 +142,8 @@ struct virtio_device {
 
 struct virtio_ops {
 	u8 *(*get_config)(struct kvm *kvm, void *dev);
-	u32 (*get_host_features)(struct kvm *kvm, void *dev);
-	void (*set_guest_features)(struct kvm *kvm, void *dev, u32 features);
+	u32 (*get_host_features)(struct kvm *kvm, void *dev, int sel);
+	void (*set_guest_features)(struct kvm *kvm, void *dev, u32 features, int sel);
 	int (*init_vq)(struct kvm *kvm, void *dev, u32 vq, u32 page_size,
 		       u32 align, u32 pfn);
 	int (*notify_vq)(struct kvm *kvm, void *dev, u32 vq);
@@ -154,6 +158,8 @@ struct virtio_ops {
 	int (*init)(struct kvm *kvm, void *dev, struct virtio_device *vdev,
 		    int device_id, int subsys_id, int class);
 	int (*exit)(struct kvm *kvm, struct virtio_device *vdev);
+	int (*queue_cnt)(struct virtio_device *vdev);
+	struct virt_queue *(*get_queue)(void *dev, u32 vq);
 };
 
 int virtio_init(struct kvm *kvm, void *dev, struct virtio_device *vdev,
@@ -167,10 +173,25 @@ static inline void *virtio_get_vq(struct kvm *kvm, u32 pfn, u32 page_size)
 	return guest_flat_to_host(kvm, (u64)pfn * page_size);
 }
 
+static inline void virtio_adjust_vq(struct kvm *kvm, struct virt_queue *queue, unsigned int num)
+{
+	queue->vring = (struct vring) {
+		.desc = guest_flat_to_host(kvm, (unsigned long)queue->vring.desc),
+		.used = guest_flat_to_host(kvm, (unsigned long)queue->vring.used),
+		.avail = guest_flat_to_host(kvm, (unsigned long)queue->vring.avail),
+		.num = num,
+	};
+}
+
 static inline void virtio_init_device_vq(struct virtio_device *vdev,
 					 struct virt_queue *vq)
 {
 	vq->endian = vdev->endian;
 }
 
+struct virtio_pci_ioevent_param {
+	struct virtio_device    *vdev;
+	u32                     vq;
+};
+
 #endif /* KVM__VIRTIO_H */
diff --git a/include/linux/virtio_pci.h b/include/linux/virtio_pci.h
new file mode 100644
index 0000000..90007a1
--- /dev/null
+++ b/include/linux/virtio_pci.h
@@ -0,0 +1,199 @@
+/*
+ * Virtio PCI driver
+ *
+ * This module allows virtio devices to be used over a virtual PCI device.
+ * This can be used with QEMU based VMMs like KVM or Xen.
+ *
+ * Copyright IBM Corp. 2007
+ *
+ * Authors:
+ *  Anthony Liguori  <aliguori@us.ibm.com>
+ *
+ * This header is BSD licensed so anyone can use the definitions to implement
+ * compatible drivers/servers.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of IBM nor the names of its contributors
+ *    may be used to endorse or promote products derived from this software
+ *    without specific prior written permission.
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED.  IN NO EVENT SHALL IBM OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#ifndef _LINUX_VIRTIO_PCI_H
+#define _LINUX_VIRTIO_PCI_H
+
+#include <linux/types.h>
+
+#ifndef VIRTIO_PCI_NO_LEGACY
+
+/* A 32-bit r/o bitmask of the features supported by the host */
+#define VIRTIO_PCI_HOST_FEATURES	0
+
+/* A 32-bit r/w bitmask of features activated by the guest */
+#define VIRTIO_PCI_GUEST_FEATURES	4
+
+/* A 32-bit r/w PFN for the currently selected queue */
+#define VIRTIO_PCI_QUEUE_PFN		8
+
+/* A 16-bit r/o queue size for the currently selected queue */
+#define VIRTIO_PCI_QUEUE_NUM		12
+
+/* A 16-bit r/w queue selector */
+#define VIRTIO_PCI_QUEUE_SEL		14
+
+/* A 16-bit r/w queue notifier */
+#define VIRTIO_PCI_QUEUE_NOTIFY		16
+
+/* An 8-bit device status register.  */
+#define VIRTIO_PCI_STATUS		18
+
+/* An 8-bit r/o interrupt status register.  Reading the value will return the
+ * current contents of the ISR and will also clear it.  This is effectively
+ * a read-and-acknowledge. */
+#define VIRTIO_PCI_ISR			19
+
+/* MSI-X registers: only enabled if MSI-X is enabled. */
+/* A 16-bit vector for configuration changes. */
+#define VIRTIO_MSI_CONFIG_VECTOR        20
+/* A 16-bit vector for selected queue notifications. */
+#define VIRTIO_MSI_QUEUE_VECTOR         22
+
+/* The remaining space is defined by each driver as the per-driver
+ * configuration space */
+#define VIRTIO_PCI_CONFIG_OFF(msix_enabled)	((msix_enabled) ? 24 : 20)
+/* Deprecated: please use VIRTIO_PCI_CONFIG_OFF instead */
+#define VIRTIO_PCI_CONFIG(dev)	VIRTIO_PCI_CONFIG_OFF((dev)->msix_enabled)
+
+/* Virtio ABI version, this must match exactly */
+#define VIRTIO_PCI_ABI_VERSION		0
+
+/* How many bits to shift physical queue address written to QUEUE_PFN.
+ * 12 is historical, and due to x86 page size. */
+#define VIRTIO_PCI_QUEUE_ADDR_SHIFT	12
+
+/* The alignment to use between consumer and producer parts of vring.
+ * x86 pagesize again. */
+#define VIRTIO_PCI_VRING_ALIGN		4096
+
+#endif /* VIRTIO_PCI_NO_LEGACY */
+
+/* The bit of the ISR which indicates a device configuration change. */
+#define VIRTIO_PCI_ISR_CONFIG		0x2
+/* Vector value used to disable MSI for queue */
+#define VIRTIO_MSI_NO_VECTOR            0xffff
+
+#ifndef VIRTIO_PCI_NO_MODERN
+
+/* IDs for different capabilities.  Must all exist. */
+
+/* Common configuration */
+#define VIRTIO_PCI_CAP_COMMON_CFG	1
+/* Notifications */
+#define VIRTIO_PCI_CAP_NOTIFY_CFG	2
+/* ISR access */
+#define VIRTIO_PCI_CAP_ISR_CFG		3
+/* Device specific configuration */
+#define VIRTIO_PCI_CAP_DEVICE_CFG	4
+/* PCI configuration access */
+#define VIRTIO_PCI_CAP_PCI_CFG		5
+
+/* This is the PCI capability header: */
+struct virtio_pci_cap {
+	__u8 cap_vndr;		/* Generic PCI field: PCI_CAP_ID_VNDR */
+	__u8 cap_next;		/* Generic PCI field: next ptr. */
+	__u8 cap_len;		/* Generic PCI field: capability length */
+	__u8 cfg_type;		/* Identifies the structure. */
+	__u8 bar;		/* Where to find it. */
+	__u8 padding[3];	/* Pad to full dword. */
+	__le32 offset;		/* Offset within bar. */
+	__le32 length;		/* Length of the structure, in bytes. */
+};
+
+struct virtio_pci_notify_cap {
+	struct virtio_pci_cap cap;
+	__le32 notify_off_multiplier;	/* Multiplier for queue_notify_off. */
+};
+
+/* Fields in VIRTIO_PCI_CAP_COMMON_CFG: */
+struct virtio_pci_common_cfg {
+	/* About the whole device. */
+	__le32 device_feature_select;	/* read-write */
+	__le32 device_feature;		/* read-only */
+	__le32 guest_feature_select;	/* read-write */
+	__le32 guest_feature;		/* read-write */
+	__le16 msix_config;		/* read-write */
+	__le16 num_queues;		/* read-only */
+	__u8 device_status;		/* read-write */
+	__u8 config_generation;		/* read-only */
+
+	/* About a specific virtqueue. */
+	__le16 queue_select;		/* read-write */
+	__le16 queue_size;		/* read-write, power of 2. */
+	__le16 queue_msix_vector;	/* read-write */
+	__le16 queue_enable;		/* read-write */
+	__le16 queue_notify_off;	/* read-only */
+	__le32 queue_desc_lo;		/* read-write */
+	__le32 queue_desc_hi;		/* read-write */
+	__le32 queue_avail_lo;		/* read-write */
+	__le32 queue_avail_hi;		/* read-write */
+	__le32 queue_used_lo;		/* read-write */
+	__le32 queue_used_hi;		/* read-write */
+};
+
+/* Fields in VIRTIO_PCI_CAP_PCI_CFG: */
+struct virtio_pci_cfg_cap {
+	struct virtio_pci_cap cap;
+	__u8 pci_cfg_data[4]; /* Data for BAR access. */
+};
+
+/* Macro versions of offsets for the Old Timers! */
+#define VIRTIO_PCI_CAP_VNDR		0
+#define VIRTIO_PCI_CAP_NEXT		1
+#define VIRTIO_PCI_CAP_LEN		2
+#define VIRTIO_PCI_CAP_CFG_TYPE		3
+#define VIRTIO_PCI_CAP_BAR		4
+#define VIRTIO_PCI_CAP_OFFSET		8
+#define VIRTIO_PCI_CAP_LENGTH		12
+
+#define VIRTIO_PCI_NOTIFY_CAP_MULT	16
+
+#define VIRTIO_PCI_COMMON_DFSELECT	0
+#define VIRTIO_PCI_COMMON_DF		4
+#define VIRTIO_PCI_COMMON_GFSELECT	8
+#define VIRTIO_PCI_COMMON_GF		12
+#define VIRTIO_PCI_COMMON_MSIX		16
+#define VIRTIO_PCI_COMMON_NUMQ		18
+#define VIRTIO_PCI_COMMON_STATUS	20
+#define VIRTIO_PCI_COMMON_CFGGENERATION	21
+#define VIRTIO_PCI_COMMON_Q_SELECT	22
+#define VIRTIO_PCI_COMMON_Q_SIZE	24
+#define VIRTIO_PCI_COMMON_Q_MSIX	26
+#define VIRTIO_PCI_COMMON_Q_ENABLE	28
+#define VIRTIO_PCI_COMMON_Q_NOFF	30
+#define VIRTIO_PCI_COMMON_Q_DESCLO	32
+#define VIRTIO_PCI_COMMON_Q_DESCHI	36
+#define VIRTIO_PCI_COMMON_Q_AVAILLO	40
+#define VIRTIO_PCI_COMMON_Q_AVAILHI	44
+#define VIRTIO_PCI_COMMON_Q_USEDLO	48
+#define VIRTIO_PCI_COMMON_Q_USEDHI	52
+
+#endif /* VIRTIO_PCI_NO_MODERN */
+
+#endif
diff --git a/net/uip/core.c b/net/uip/core.c
index e860f3a..7d4b19d 100644
--- a/net/uip/core.c
+++ b/net/uip/core.c
@@ -25,6 +25,12 @@ int uip_tx(struct iovec *iov, u16 out, struct uip_info *info)
 	eth_len	 = iov[1].iov_len;
 	eth	 = iov[1].iov_base;
 
+	if (out == 1) {
+		vnet_len = info->vnet_hdr_len;
+		eth = (void *)((char *)vnet + vnet_len);
+		eth_len = iov[0].iov_len - vnet_len;
+	}
+
 	/*
 	 * In case, ethernet frame is in more than one iov entry.
 	 * Copy iov buffer into one linear buffer.
@@ -87,7 +93,6 @@ int uip_rx(struct iovec *iov, u16 in, struct uip_info *info)
 
 	memcpy_toiovecend(iov, buf->vnet, 0, buf->vnet_len);
 	memcpy_toiovecend(iov, buf->eth, buf->vnet_len, buf->eth_len);
-
 	len = buf->vnet_len + buf->eth_len;
 
 	uip_buf_set_free(info, buf);
diff --git a/virtio/9p.c b/virtio/9p.c
index 49e7c5c..8dc65f3 100644
--- a/virtio/9p.c
+++ b/virtio/9p.c
@@ -1252,17 +1252,19 @@ static u8 *get_config(struct kvm *kvm, void *dev)
 	return ((u8 *)(p9dev->config));
 }
 
-static u32 get_host_features(struct kvm *kvm, void *dev)
+static u32 get_host_features(struct kvm *kvm, void *dev, int sel)
 {
-	return 1 << VIRTIO_9P_MOUNT_TAG;
+	static u64 features =	1UL << VIRTIO_9P_MOUNT_TAG;
+
+	return features >> (32 * sel);
 }
 
-static void set_guest_features(struct kvm *kvm, void *dev, u32 features)
+static void set_guest_features(struct kvm *kvm, void *dev, u32 features, int sel)
 {
 	struct p9_dev *p9dev = dev;
 	struct virtio_9p_config *conf = p9dev->config;
 
-	p9dev->features = features;
+	p9dev->features |= (u64)features << (32 * sel);
 	conf->tag_len = virtio_host_to_guest_u16(&p9dev->vdev, conf->tag_len);
 }
 
@@ -1277,11 +1279,16 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align,
 	compat__remove_message(compat_id);
 
 	queue		= &p9dev->vqs[vq];
-	queue->pfn	= pfn;
-	p		= virtio_get_vq(kvm, queue->pfn, page_size);
 	job		= &p9dev->jobs[vq];
 
-	vring_init(&queue->vring, VIRTQUEUE_NUM, p, align);
+	if (pfn) {
+		queue->pfn	= pfn;
+		p		= virtio_get_vq(kvm, queue->pfn, page_size);
+		vring_init(&queue->vring, VIRTQUEUE_NUM, p, align);
+	} else {
+		virtio_adjust_vq(kvm, queue, VIRTQUEUE_NUM);
+	}
+
 	virtio_init_device_vq(&p9dev->vdev, queue);
 
 	*job		= (struct p9_dev_job) {
@@ -1320,6 +1327,18 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
 	return size;
 }
 
+static int queue_cnt(struct virtio_device *vdev)
+{
+	return 1;
+}
+
+static struct virt_queue *get_queue(void *dev, u32 vq)
+{
+	struct p9_dev *p9dev = dev;
+
+	return &p9dev->vqs[vq];
+}
+
 struct virtio_ops p9_dev_virtio_ops = {
 	.get_config		= get_config,
 	.get_host_features	= get_host_features,
@@ -1329,6 +1348,8 @@ struct virtio_ops p9_dev_virtio_ops = {
 	.get_pfn_vq		= get_pfn_vq,
 	.get_size_vq		= get_size_vq,
 	.set_size_vq		= set_size_vq,
+	.queue_cnt		= queue_cnt,
+	.get_queue		= get_queue,
 };
 
 int virtio_9p_rootdir_parser(const struct option *opt, const char *arg, int unset)
diff --git a/virtio/balloon.c b/virtio/balloon.c
index 9564aa3..16a8aba 100644
--- a/virtio/balloon.c
+++ b/virtio/balloon.c
@@ -32,7 +32,7 @@ struct bln_dev {
 	struct list_head	list;
 	struct virtio_device	vdev;
 
-	u32			features;
+	u64			features;
 
 	/* virtio queue */
 	struct virt_queue	vqs[NUM_VIRT_QUEUES];
@@ -181,16 +181,18 @@ static u8 *get_config(struct kvm *kvm, void *dev)
 	return ((u8 *)(&bdev->config));
 }
 
-static u32 get_host_features(struct kvm *kvm, void *dev)
+static u32 get_host_features(struct kvm *kvm, void *dev, int sel)
 {
-	return 1 << VIRTIO_BALLOON_F_STATS_VQ;
+	static u64 features = 1UL << VIRTIO_BALLOON_F_STATS_VQ;
+
+	return features >> (32 * sel);
 }
 
-static void set_guest_features(struct kvm *kvm, void *dev, u32 features)
+static void set_guest_features(struct kvm *kvm, void *dev, u32 features, int sel)
 {
 	struct bln_dev *bdev = dev;
 
-	bdev->features = features;
+	bdev->features = (u64)features << (32 * sel);
 }
 
 static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align,
@@ -203,11 +205,16 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align,
 	compat__remove_message(compat_id);
 
 	queue		= &bdev->vqs[vq];
-	queue->pfn	= pfn;
-	p		= virtio_get_vq(kvm, queue->pfn, page_size);
+
+	if (pfn) {
+		queue->pfn	= pfn;
+		p		= virtio_get_vq(kvm, queue->pfn, page_size);
+		vring_init(&queue->vring, VIRTIO_BLN_QUEUE_SIZE, p, align);
+	} else {
+		virtio_adjust_vq(kvm, queue, VIRTIO_BLN_QUEUE_SIZE);
+	}
 
 	thread_pool__init_job(&bdev->jobs[vq], kvm, virtio_bln_do_io, queue);
-	vring_init(&queue->vring, VIRTIO_BLN_QUEUE_SIZE, p, align);
 
 	return 0;
 }
@@ -228,6 +235,13 @@ static int get_pfn_vq(struct kvm *kvm, void *dev, u32 vq)
 	return bdev->vqs[vq].pfn;
 }
 
+static struct virt_queue *get_queue(void *dev, u32 vq)
+{
+	struct bln_dev *bdev = dev;
+
+	return &bdev->vqs[vq];
+}
+
 static int get_size_vq(struct kvm *kvm, void *dev, u32 vq)
 {
 	return VIRTIO_BLN_QUEUE_SIZE;
@@ -239,6 +253,11 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
 	return size;
 }
 
+static int queue_cnt(struct virtio_device *vdev)
+{
+	return NUM_VIRT_QUEUES;
+}
+
 struct virtio_ops bln_dev_virtio_ops = {
 	.get_config		= get_config,
 	.get_host_features	= get_host_features,
@@ -248,6 +267,8 @@ struct virtio_ops bln_dev_virtio_ops = {
 	.get_pfn_vq		= get_pfn_vq,
 	.get_size_vq		= get_size_vq,
 	.set_size_vq            = set_size_vq,
+	.queue_cnt		= queue_cnt,
+	.get_queue		= get_queue,
 };
 
 int virtio_bln__init(struct kvm *kvm)
diff --git a/virtio/blk.c b/virtio/blk.c
index c485e4f..e036b75 100644
--- a/virtio/blk.c
+++ b/virtio/blk.c
@@ -44,7 +44,7 @@ struct blk_dev {
 	struct virtio_device		vdev;
 	struct virtio_blk_config	blk_config;
 	struct disk_image		*disk;
-	u32				features;
+	u64				features;
 
 	struct virt_queue		vqs[NUM_VIRT_QUEUES];
 	struct blk_dev_req		reqs[VIRTIO_BLK_QUEUE_SIZE];
@@ -146,21 +146,34 @@ static u8 *get_config(struct kvm *kvm, void *dev)
 	return ((u8 *)(&bdev->blk_config));
 }
 
-static u32 get_host_features(struct kvm *kvm, void *dev)
+static u32 get_host_features(struct kvm *kvm, void *dev, int sel)
 {
-	return	1UL << VIRTIO_BLK_F_SEG_MAX
+	static u64 features = 1UL << VIRTIO_BLK_F_SEG_MAX
 		| 1UL << VIRTIO_BLK_F_FLUSH
 		| 1UL << VIRTIO_RING_F_EVENT_IDX
 		| 1UL << VIRTIO_RING_F_INDIRECT_DESC;
+
+	return features >> (32 * sel);
 }
 
-static void set_guest_features(struct kvm *kvm, void *dev, u32 features)
+static void set_guest_features(struct kvm *kvm, void *dev, u32 features, int sel)
 {
 	struct blk_dev *bdev = dev;
+
+	bdev->features |= (u64)features << (32 * sel);
+}
+
+static void notify_status(struct kvm *kvm, void *dev, u8 status)
+{
+	static bool init_done;
+	struct blk_dev *bdev = dev;
 	struct virtio_blk_config *conf = &bdev->blk_config;
 	struct virtio_blk_geometry *geo = &conf->geometry;
 
-	bdev->features = features;
+	if (!(status & VIRTIO_CONFIG_S_DRIVER_OK) || init_done)
+		return;
+
+	init_done = true;
 
 	conf->capacity = virtio_host_to_guest_u64(&bdev->vdev, conf->capacity);
 	conf->size_max = virtio_host_to_guest_u32(&bdev->vdev, conf->size_max);
@@ -173,7 +186,6 @@ static void set_guest_features(struct kvm *kvm, void *dev, u32 features)
 	conf->min_io_size = virtio_host_to_guest_u16(&bdev->vdev, conf->min_io_size);
 	conf->opt_io_size = virtio_host_to_guest_u32(&bdev->vdev, conf->opt_io_size);
 }
-
 static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align,
 		   u32 pfn)
 {
@@ -184,10 +196,15 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align,
 	compat__remove_message(compat_id);
 
 	queue		= &bdev->vqs[vq];
-	queue->pfn	= pfn;
-	p		= virtio_get_vq(kvm, queue->pfn, page_size);
 
-	vring_init(&queue->vring, VIRTIO_BLK_QUEUE_SIZE, p, align);
+	if (pfn) {
+		queue->pfn	= pfn;
+		p		= virtio_get_vq(kvm, queue->pfn, page_size);
+		vring_init(&queue->vring, VIRTIO_BLK_QUEUE_SIZE, p, align);
+	} else {
+		virtio_adjust_vq(kvm, queue, VIRTIO_BLK_QUEUE_SIZE);
+	}
+
 	virtio_init_device_vq(&bdev->vdev, queue);
 
 	return 0;
@@ -232,6 +249,13 @@ static int get_pfn_vq(struct kvm *kvm, void *dev, u32 vq)
 	return bdev->vqs[vq].pfn;
 }
 
+static struct virt_queue *get_queue(void *dev, u32 vq)
+{
+	struct blk_dev *bdev = dev;
+
+	return &bdev->vqs[vq];
+}
+
 static int get_size_vq(struct kvm *kvm, void *dev, u32 vq)
 {
 	/* FIXME: dynamic */
@@ -244,6 +268,11 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
 	return size;
 }
 
+static int queue_cnt(struct virtio_device *vdev)
+{
+	return NUM_VIRT_QUEUES;
+}
+
 static struct virtio_ops blk_dev_virtio_ops = {
 	.get_config		= get_config,
 	.get_host_features	= get_host_features,
@@ -253,6 +282,9 @@ static struct virtio_ops blk_dev_virtio_ops = {
 	.get_pfn_vq		= get_pfn_vq,
 	.get_size_vq		= get_size_vq,
 	.set_size_vq		= set_size_vq,
+	.queue_cnt		= queue_cnt,
+	.get_queue		= get_queue,
+	.notify_status		= notify_status,
 };
 
 static int virtio_blk__init_one(struct kvm *kvm, struct disk_image *disk)
diff --git a/virtio/console.c b/virtio/console.c
index f1c0a19..0bdeecd 100644
--- a/virtio/console.c
+++ b/virtio/console.c
@@ -34,7 +34,7 @@ struct con_dev {
 	struct virtio_device		vdev;
 	struct virt_queue		vqs[VIRTIO_CONSOLE_NUM_QUEUES];
 	struct virtio_console_config	config;
-	u32				features;
+	u64				features;
 
 	pthread_cond_t			poll_cond;
 	int				vq_ready;
@@ -124,16 +124,26 @@ static u8 *get_config(struct kvm *kvm, void *dev)
 	return ((u8 *)(&cdev->config));
 }
 
-static u32 get_host_features(struct kvm *kvm, void *dev)
+static u32 get_host_features(struct kvm *kvm, void *dev, int sel)
 {
 	return 0;
 }
 
-static void set_guest_features(struct kvm *kvm, void *dev, u32 features)
+static void set_guest_features(struct kvm *kvm, void *dev, u32 features, int sel)
 {
+	return;
+}
+
+static void notify_status(struct kvm *kvm, void *dev, u8 status)
+{
+	static bool init_done;
 	struct con_dev *cdev = dev;
 	struct virtio_console_config *conf = &cdev->config;
 
+	if (!(status & VIRTIO_CONFIG_S_DRIVER_OK) || init_done)
+		return;
+
+	init_done = true;
 	conf->cols = virtio_host_to_guest_u16(&cdev->vdev, conf->cols);
 	conf->rows = virtio_host_to_guest_u16(&cdev->vdev, conf->rows);
 	conf->max_nr_ports = virtio_host_to_guest_u32(&cdev->vdev, conf->max_nr_ports);
@@ -150,10 +160,15 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align,
 	compat__remove_message(compat_id);
 
 	queue		= &cdev.vqs[vq];
-	queue->pfn	= pfn;
-	p		= virtio_get_vq(kvm, queue->pfn, page_size);
 
-	vring_init(&queue->vring, VIRTIO_CONSOLE_QUEUE_SIZE, p, align);
+	if (pfn) {
+		queue->pfn	= pfn;
+		p		= virtio_get_vq(kvm, queue->pfn, page_size);
+		vring_init(&queue->vring, VIRTIO_CONSOLE_QUEUE_SIZE, p, align);
+	} else {
+		virtio_adjust_vq(kvm, queue, VIRTIO_CONSOLE_NUM_QUEUES);
+	}
+
 	virtio_init_device_vq(&cdev.vdev, queue);
 
 	if (vq == VIRTIO_CONSOLE_TX_QUEUE) {
@@ -186,6 +201,13 @@ static int get_pfn_vq(struct kvm *kvm, void *dev, u32 vq)
 	return cdev->vqs[vq].pfn;
 }
 
+static struct virt_queue *get_queue(void *dev, u32 vq)
+{
+	struct con_dev *cdev = dev;
+
+	return &cdev->vqs[vq];
+}
+
 static int get_size_vq(struct kvm *kvm, void *dev, u32 vq)
 {
 	return VIRTIO_CONSOLE_QUEUE_SIZE;
@@ -197,6 +219,11 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
 	return size;
 }
 
+static int queue_cnt(struct virtio_device *vdev)
+{
+	return VIRTIO_CONSOLE_NUM_QUEUES;
+}
+
 static struct virtio_ops con_dev_virtio_ops = {
 	.get_config		= get_config,
 	.get_host_features	= get_host_features,
@@ -206,6 +233,9 @@ static struct virtio_ops con_dev_virtio_ops = {
 	.get_pfn_vq		= get_pfn_vq,
 	.get_size_vq		= get_size_vq,
 	.set_size_vq		= set_size_vq,
+	.queue_cnt		= queue_cnt,
+	.get_queue		= get_queue,
+	.notify_status		= notify_status,
 };
 
 int virtio_console__init(struct kvm *kvm)
diff --git a/virtio/core.c b/virtio/core.c
index 3b6e4d7..0225796 100644
--- a/virtio/core.c
+++ b/virtio/core.c
@@ -6,16 +6,20 @@
 #include "kvm/guest_compat.h"
 #include "kvm/barrier.h"
 #include "kvm/virtio.h"
+#include "kvm/virtio-pci-modern.h"
 #include "kvm/virtio-pci.h"
 #include "kvm/virtio-mmio.h"
 #include "kvm/util.h"
 #include "kvm/kvm.h"
 
+u8 default_transport;
 
 const char* virtio_trans_name(enum virtio_trans trans)
 {
 	if (trans == VIRTIO_PCI)
 		return "pci";
+	else if (trans == VIRTIO_PCI_MODERN)
+		return "pci-modern";
 	else if (trans == VIRTIO_MMIO)
 		return "mmio";
 	return "unknown";
@@ -187,6 +191,18 @@ int virtio_init(struct kvm *kvm, void *dev, struct virtio_device *vdev,
 	void *virtio;
 
 	switch (trans) {
+	case VIRTIO_PCI_MODERN:
+		virtio = calloc(sizeof(struct virtio_pci_modern), 1);
+		if (!virtio)
+			return -ENOMEM;
+		vdev->virtio                    = virtio;
+		vdev->ops                       = ops;
+		vdev->ops->signal_vq            = virtio_pcim__signal_vq;
+		vdev->ops->signal_config        = virtio_pcim__signal_config;
+		vdev->ops->init                 = virtio_pcim__init;
+		vdev->ops->exit                 = virtio_pcim__exit;
+		vdev->ops->init(kvm, dev, vdev, device_id, subsys_id, class);
+                break;
 	case VIRTIO_PCI:
 		virtio = calloc(sizeof(struct virtio_pci), 1);
 		if (!virtio)
diff --git a/virtio/mmio.c b/virtio/mmio.c
index 5174455..3d61bd6 100644
--- a/virtio/mmio.c
+++ b/virtio/mmio.c
@@ -123,9 +123,8 @@ static void virtio_mmio_config_in(struct kvm_cpu *vcpu,
 		ioport__write32(data, *(u32 *)(((void *)&vmmio->hdr) + addr));
 		break;
 	case VIRTIO_MMIO_HOST_FEATURES:
-		if (vmmio->hdr.host_features_sel == 0)
-			val = vdev->ops->get_host_features(vmmio->kvm,
-							   vmmio->dev);
+		val = vdev->ops->get_host_features(vmmio->kvm,
+				vmmio->dev, vmmio->hdr.host_features_sel);
 		ioport__write32(data, val);
 		break;
 	case VIRTIO_MMIO_QUEUE_PFN:
@@ -166,11 +165,9 @@ static void virtio_mmio_config_out(struct kvm_cpu *vcpu,
 			vdev->ops->notify_status(kvm, vmmio->dev, vmmio->hdr.status);
 		break;
 	case VIRTIO_MMIO_GUEST_FEATURES:
-		if (vmmio->hdr.guest_features_sel == 0) {
-			val = ioport__read32(data);
-			vdev->ops->set_guest_features(vmmio->kvm,
-						      vmmio->dev, val);
-		}
+		val = ioport__read32(data);
+		vdev->ops->set_guest_features(vmmio->kvm,
+				vmmio->dev, val, vmmio->hdr.host_features_sel);
 		break;
 	case VIRTIO_MMIO_GUEST_PAGE_SIZE:
 		val = ioport__read32(data);
diff --git a/virtio/net.c b/virtio/net.c
index 6d1be65..061ca4e 100644
--- a/virtio/net.c
+++ b/virtio/net.c
@@ -43,7 +43,8 @@ struct net_dev {
 
 	struct virt_queue		vqs[VIRTIO_NET_NUM_QUEUES * 2 + 1];
 	struct virtio_net_config	config;
-	u32				features, rx_vqs, tx_vqs, queue_pairs;
+	u64				features;
+	u32				rx_vqs, tx_vqs, queue_pairs;
 
 	pthread_t			io_thread[VIRTIO_NET_NUM_QUEUES * 2 + 1];
 	struct mutex			io_lock[VIRTIO_NET_NUM_QUEUES * 2 + 1];
@@ -431,11 +432,10 @@ static u8 *get_config(struct kvm *kvm, void *dev)
 	return ((u8 *)(&ndev->config));
 }
 
-static u32 get_host_features(struct kvm *kvm, void *dev)
+static u32 get_host_features(struct kvm *kvm, void *dev, int sel)
 {
 	struct net_dev *ndev = dev;
-
-	return 1UL << VIRTIO_NET_F_MAC
+	u64 features = 1UL << VIRTIO_NET_F_MAC
 		| 1UL << VIRTIO_NET_F_CSUM
 		| 1UL << VIRTIO_NET_F_HOST_UFO
 		| 1UL << VIRTIO_NET_F_HOST_TSO4
@@ -448,6 +448,8 @@ static u32 get_host_features(struct kvm *kvm, void *dev)
 		| 1UL << VIRTIO_NET_F_CTRL_VQ
 		| 1UL << VIRTIO_NET_F_MRG_RXBUF
 		| 1UL << (ndev->queue_pairs > 1 ? VIRTIO_NET_F_MQ : 0);
+
+	return features >> (32 * sel);
 }
 
 static int virtio_net__vhost_set_features(struct net_dev *ndev)
@@ -466,31 +468,42 @@ static int virtio_net__vhost_set_features(struct net_dev *ndev)
 	return ioctl(ndev->vhost_fd, VHOST_SET_FEATURES, &features);
 }
 
-static void set_guest_features(struct kvm *kvm, void *dev, u32 features)
+static void notify_status(struct kvm *kvm, void *dev, u8 status)
 {
+	static bool init_done;
 	struct net_dev *ndev = dev;
 	struct virtio_net_config *conf = &ndev->config;
 
-	ndev->features = features;
+	if (!(status & VIRTIO_CONFIG_S_DRIVER_OK) || init_done)
+		return;
+
+	init_done = true;
 
 	conf->status = virtio_host_to_guest_u16(&ndev->vdev, conf->status);
 	conf->max_virtqueue_pairs = virtio_host_to_guest_u16(&ndev->vdev,
-							     conf->max_virtqueue_pairs);
+							conf->max_virtqueue_pairs);
 
 	if (ndev->mode == NET_MODE_TAP) {
 		if (!virtio_net__tap_init(ndev))
 			die_perror("You have requested a TAP device, but creation of one has failed because");
 		if (ndev->vhost_fd &&
-				virtio_net__vhost_set_features(ndev) != 0)
+			virtio_net__vhost_set_features(ndev) != 0)
 			die_perror("VHOST_SET_FEATURES failed");
 	} else {
 		ndev->info.vnet_hdr_len = has_virtio_feature(ndev, VIRTIO_NET_F_MRG_RXBUF) ?
-						sizeof(struct virtio_net_hdr_mrg_rxbuf) :
-						sizeof(struct virtio_net_hdr);
+					sizeof(struct virtio_net_hdr_mrg_rxbuf) :
+					sizeof(struct virtio_net_hdr);
 		uip_init(&ndev->info);
 	}
 }
 
+static void set_guest_features(struct kvm *kvm, void *dev, u32 features, int sel)
+{
+	struct net_dev *ndev = dev;
+
+	ndev->features |= (u64)features << (32 * sel);
+}
+
 static bool is_ctrl_vq(struct net_dev *ndev, u32 vq)
 {
 	return vq == (u32)(ndev->queue_pairs * 2);
@@ -509,10 +522,15 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align,
 	compat__remove_message(compat_id);
 
 	queue		= &ndev->vqs[vq];
-	queue->pfn	= pfn;
-	p		= virtio_get_vq(kvm, queue->pfn, page_size);
 
-	vring_init(&queue->vring, VIRTIO_NET_QUEUE_SIZE, p, align);
+	if (pfn) {
+		queue->pfn	= pfn;
+		p		= virtio_get_vq(kvm, queue->pfn, page_size);
+		vring_init(&queue->vring, VIRTIO_NET_QUEUE_SIZE, p, align);
+	} else {
+		virtio_adjust_vq(kvm, queue, VIRTIO_NET_QUEUE_SIZE);
+	}
+
 	virtio_init_device_vq(&ndev->vdev, queue);
 
 	mutex_init(&ndev->io_lock[vq]);
@@ -622,6 +640,13 @@ static int get_pfn_vq(struct kvm *kvm, void *dev, u32 vq)
 	return ndev->vqs[vq].pfn;
 }
 
+static struct virt_queue *get_queue(void *dev, u32 vq)
+{
+	struct net_dev *ndev = dev;
+
+	return &ndev->vqs[vq];
+}
+
 static int get_size_vq(struct kvm *kvm, void *dev, u32 vq)
 {
 	/* FIXME: dynamic */
@@ -634,6 +659,11 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
 	return size;
 }
 
+static int queue_cnt(struct virtio_device *vdev)
+{
+	return VIRTIO_NET_NUM_QUEUES;
+}
+
 static struct virtio_ops net_dev_virtio_ops = {
 	.get_config		= get_config,
 	.get_host_features	= get_host_features,
@@ -645,6 +675,9 @@ static struct virtio_ops net_dev_virtio_ops = {
 	.notify_vq		= notify_vq,
 	.notify_vq_gsi		= notify_vq_gsi,
 	.notify_vq_eventfd	= notify_vq_eventfd,
+	.queue_cnt		= queue_cnt,
+	.get_queue		= get_queue,
+	.notify_status		= notify_status,
 };
 
 static void virtio_net__vhost_init(struct kvm *kvm, struct net_dev *ndev)
diff --git a/virtio/pci.c b/virtio/pci.c
index 90fcd64..c3a3113 100644
--- a/virtio/pci.c
+++ b/virtio/pci.c
@@ -125,7 +125,7 @@ static bool virtio_pci__io_in(struct ioport *ioport, struct kvm_cpu *vcpu, u16 p
 
 	switch (offset) {
 	case VIRTIO_PCI_HOST_FEATURES:
-		val = vdev->ops->get_host_features(kvm, vpci->dev);
+		val = vdev->ops->get_host_features(kvm, vpci->dev, 0);
 		ioport__write32(data, val);
 		break;
 	case VIRTIO_PCI_QUEUE_PFN:
@@ -211,7 +211,7 @@ static bool virtio_pci__io_out(struct ioport *ioport, struct kvm_cpu *vcpu, u16
 	switch (offset) {
 	case VIRTIO_PCI_GUEST_FEATURES:
 		val = ioport__read32(data);
-		vdev->ops->set_guest_features(kvm, vpci->dev, val);
+		vdev->ops->set_guest_features(kvm, vpci->dev, val, 0);
 		break;
 	case VIRTIO_PCI_QUEUE_PFN:
 		val = ioport__read32(data);
diff --git a/virtio/pci_modern.c b/virtio/pci_modern.c
new file mode 100644
index 0000000..6690366
--- /dev/null
+++ b/virtio/pci_modern.c
@@ -0,0 +1,599 @@
+#include "kvm/virtio-pci-modern.h"
+
+#include "kvm/ioport.h"
+#include "kvm/kvm.h"
+#include "kvm/kvm-cpu.h"
+#include "kvm/virtio-pci-dev.h"
+#include "kvm/irq.h"
+#include "kvm/virtio.h"
+#include "kvm/ioeventfd.h"
+
+#include <sys/ioctl.h>
+#include <linux/virtio_pci.h>
+#include <linux/byteorder.h>
+#include <linux/virtio_config.h>
+#include <string.h>
+
+static void virtio_pcim__ioevent_callback(struct kvm *kvm, void *param)
+{
+	struct virtio_pci_ioevent_param *ioeventfd = param;
+	struct virtio_pci_modern *vpci = ioeventfd->vdev->virtio;
+
+	ioeventfd->vdev->ops->notify_vq(kvm, vpci->dev, ioeventfd->vq);
+}
+
+__used static int virtio_pcim__init_ioeventfd(struct kvm *kvm, struct virtio_device *vdev, u32 vq)
+{
+	struct ioevent ioevent;
+	struct virtio_pci_modern *vpci = vdev->virtio;
+	int i, r, flags = 0;
+	int fds[2];
+
+	vpci->ioeventfds[vq] = (struct virtio_pci_ioevent_param) {
+		.vdev		= vdev,
+		.vq		= vq,
+	};
+
+	ioevent = (struct ioevent) {
+		.fn		= virtio_pcim__ioevent_callback,
+		.fn_ptr		= &vpci->ioeventfds[vq],
+		.datamatch	= vq,
+		.fn_kvm		= kvm,
+	};
+
+	/*
+	 * Vhost will poll the eventfd in host kernel side, otherwise we
+	 * need to poll in userspace.
+	 */
+	if (!vdev->use_vhost)
+		flags |= IOEVENTFD_FLAG_USER_POLL;
+
+	/* ioport */
+	ioevent.io_addr	= vpci->port_addr + 0x80 + vq * 2;
+	ioevent.io_len	= sizeof(u16);
+	ioevent.fd	= fds[0] = eventfd(0, 0);
+	r = ioeventfd__add_event(&ioevent, flags | IOEVENTFD_FLAG_PIO);
+	if (r)
+		return r;
+
+	/* mmio */
+	ioevent.io_addr	= vpci->mmio_addr + 0x80 + vq * 2;
+	ioevent.io_len	= sizeof(u16);
+	ioevent.fd	= fds[1] = eventfd(0, 0);
+	r = ioeventfd__add_event(&ioevent, flags);
+	if (r)
+		goto free_ioport_evt;
+
+	if (vdev->ops->notify_vq_eventfd)
+		for (i = 0; i < 2; ++i)
+			vdev->ops->notify_vq_eventfd(kvm, vpci->dev, vq,
+						     fds[i]);
+	return 0;
+
+free_ioport_evt:
+	ioeventfd__del_event(vpci->port_addr + VIRTIO_PCI_QUEUE_NOTIFY, vq);
+	return r;
+}
+
+static inline bool virtio_pcim__msix_enabled(struct virtio_pci_modern *vpci)
+{
+	return vpci->pci_hdr.msix.ctrl & cpu_to_le16(PCI_MSIX_FLAGS_ENABLE);
+}
+
+static bool virtio_pcim__notify_out(struct virtio_device *vdev, unsigned long offset, void *data, int size)
+{
+	u16 vq = ioport__read16(data);
+	struct virtio_pci_modern *vpci = vdev->virtio;
+	vdev->ops->notify_vq(vpci->kvm, vpci->dev, vq);
+
+	return true;
+}
+
+static bool virtio_pcim__config_out(struct virtio_device *vdev, unsigned long offset, void *data, int size)
+{
+	struct virtio_pci_modern *vpci = vdev->virtio;
+
+	vdev->ops->get_config(vpci->kvm, vpci->dev)[offset] = *(u8 *)data;
+
+	return true;
+}
+
+static bool virtio_pcim__common_out(struct virtio_device *vdev, unsigned long offset, void *data, int size)
+{
+	unsigned long addr;
+	u32 val, gsi, vec;
+	struct virtio_pci_modern *vpci = vdev->virtio;
+
+	switch (offset) {
+	case VIRTIO_PCI_COMMON_DFSELECT:
+		vpci->device_features_sel = ioport__read32(data);
+		break;
+	case VIRTIO_PCI_COMMON_GF:
+		val = ioport__read32(data);
+		if (vpci->driver_features_sel > 1)
+			break;
+		vdev->ops->set_guest_features(vpci->kvm, vpci->dev, val, vpci->driver_features_sel);
+		break;
+	case VIRTIO_PCI_COMMON_GFSELECT:
+		vpci->driver_features_sel = ioport__read32(data);
+		break;
+	case VIRTIO_PCI_COMMON_MSIX:
+		vec = vpci->config_vector = ioport__read16(data);
+		if (vec == VIRTIO_MSI_NO_VECTOR)
+			break;
+
+		gsi = irq__add_msix_route(vpci->kvm, &vpci->msix_table[vec].msg);
+
+		vpci->config_gsi = gsi;
+		break;
+	case VIRTIO_PCI_COMMON_STATUS:
+		vpci->status = ioport__read8(data);
+		if (vdev->ops->notify_status)
+			vdev->ops->notify_status(vpci->kvm, vpci->dev, vpci->status);
+		break;
+	case VIRTIO_PCI_COMMON_Q_SELECT:
+		vpci->queue_selector = ioport__read16(data);
+		break;
+	case VIRTIO_PCI_COMMON_Q_MSIX:
+		vec = vpci->vq_vector[vpci->queue_selector] = ioport__read16(data);
+
+		if (vec == VIRTIO_MSI_NO_VECTOR)
+			break;
+
+		gsi = irq__add_msix_route(vpci->kvm, &vpci->msix_table[vec].msg);
+		vpci->gsis[vpci->queue_selector] = gsi;
+		if (vdev->ops->notify_vq_gsi)
+			vdev->ops->notify_vq_gsi(vpci->kvm, vpci->dev,
+						vpci->queue_selector, gsi);
+		break;
+	case VIRTIO_PCI_COMMON_Q_SIZE:
+		val = ioport__read16(data);
+		break;
+	case VIRTIO_PCI_COMMON_Q_ENABLE:
+		val = ioport__read16(data);
+		if (val) {
+			virtio_pcim__init_ioeventfd(vpci->kvm, vdev, vpci->queue_selector);
+			vdev->ops->init_vq(vpci->kvm, vpci->dev, vpci->queue_selector,
+				1 << VIRTIO_PCI_QUEUE_ADDR_SHIFT,
+				VIRTIO_PCI_VRING_ALIGN, 0);
+		}
+		vdev->ops->get_queue(vpci->dev, vpci->queue_selector)->enabled = val;
+		break;
+	case VIRTIO_PCI_COMMON_Q_DESCLO:
+		val = ioport__read32(data);
+		addr = (unsigned long)vdev->ops->get_queue(vpci->dev, vpci->queue_selector)->vring.desc;
+		addr = ((addr >> 32) << 32) | val;
+		vdev->ops->get_queue(vpci->dev, vpci->queue_selector)->vring.desc = (void *)addr;
+		break;
+	case VIRTIO_PCI_COMMON_Q_DESCHI:
+		val = ioport__read32(data);
+		addr = (unsigned long)vdev->ops->get_queue(vpci->dev, vpci->queue_selector)->vring.desc;
+		addr = ((addr << 32) >> 32) | val;
+		vdev->ops->get_queue(vpci->dev, vpci->queue_selector)->vring.desc = (void *)addr;
+		break;
+	case VIRTIO_PCI_COMMON_Q_AVAILLO:
+		val = ioport__read32(data);
+		addr = (unsigned long)vdev->ops->get_queue(vpci->dev, vpci->queue_selector)->vring.avail;
+		addr = ((addr >> 32) << 32) | val;
+		vdev->ops->get_queue(vpci->dev, vpci->queue_selector)->vring.avail = (void *)addr;
+		break;
+	case VIRTIO_PCI_COMMON_Q_AVAILHI:
+		val = ioport__read32(data);
+		addr = (unsigned long)vdev->ops->get_queue(vpci->dev, vpci->queue_selector)->vring.avail;
+		addr = ((addr << 32) >> 32) | val;
+		vdev->ops->get_queue(vpci->dev, vpci->queue_selector)->vring.avail = (void *)addr;
+		break;
+	case VIRTIO_PCI_COMMON_Q_USEDLO:
+		val = ioport__read32(data);
+		addr = (unsigned long)vdev->ops->get_queue(vpci->dev, vpci->queue_selector)->vring.used;
+		addr = ((addr >> 32) << 32) | val;
+		vdev->ops->get_queue(vpci->dev, vpci->queue_selector)->vring.used = (void *)addr;
+		break;
+	case VIRTIO_PCI_COMMON_Q_USEDHI:
+		val = ioport__read32(data);
+		addr = (unsigned long)vdev->ops->get_queue(vpci->dev, vpci->queue_selector)->vring.used;
+		addr = ((addr << 32) >> 32) | val;
+		vdev->ops->get_queue(vpci->dev, vpci->queue_selector)->vring.used = (void *)addr;
+		break;
+	}
+
+	return true;
+}
+
+static bool (*io_out_map[5])(struct virtio_device *, unsigned long, void *, int) = {
+	virtio_pcim__common_out,
+	virtio_pcim__notify_out,
+	NULL,
+	virtio_pcim__config_out,
+};
+
+static bool virtio_pcim__io_out(struct ioport *ioport, struct kvm_cpu *vcpu, u16 port, void *data, int size)
+{
+        unsigned long offset;
+        struct virtio_device *vdev;
+        struct virtio_pci_modern *vpci;
+
+        vdev = ioport->priv;
+        vpci = vdev->virtio;
+        offset = port - vpci->port_addr;
+
+	return io_out_map[offset/0x80](vdev, offset - (offset/0x80) * 0x80, data, size);
+}
+
+static bool virtio_pcim__config_in(struct virtio_device *vdev, unsigned long offset, void *data, int size)
+{
+	struct virtio_pci_modern *vpci = vdev->virtio;
+
+	switch (size) {
+	case 1:
+		ioport__write8(data, vdev->ops->get_config(vpci->kvm, vpci->dev)[offset]);
+		break;
+	case 2:
+		ioport__write16(data, ((u16 *)vdev->ops->get_config(vpci->kvm, vpci->dev))[offset]);
+		break;
+	};
+
+	return true;
+}
+
+static bool virtio_pcim__common_in(struct virtio_device *vdev, unsigned long offset, void *data, int size)
+{
+	u32 val;
+	struct virtio_pci_modern *vpci = vdev->virtio;
+	static u64 features = 1UL << VIRTIO_F_VERSION_1;
+
+	switch (offset) {
+	case VIRTIO_PCI_COMMON_DFSELECT:
+		val = vpci->device_features_sel;
+		ioport__write32(data, val);
+		break;
+	case VIRTIO_PCI_COMMON_DF:
+		if (vpci->device_features_sel > 1)
+			break;
+		val = vdev->ops->get_host_features(vpci->kvm, vpci->dev, vpci->device_features_sel);
+		val |= (u32)(features >> (32 * vpci->device_features_sel));
+		ioport__write32(data, val);
+		break;
+	case VIRTIO_PCI_COMMON_GFSELECT:
+		val = vpci->driver_features_sel;
+		ioport__write32(data, val);
+		break;
+	case VIRTIO_PCI_COMMON_MSIX:
+		val = vpci->config_vector;
+		ioport__write32(data, val);
+		break;
+	case VIRTIO_PCI_COMMON_NUMQ:
+		val = vdev->ops->queue_cnt(vdev);
+		ioport__write32(data, val);
+		break;
+	case VIRTIO_PCI_COMMON_STATUS:
+		ioport__write8(data, vpci->status);
+		break;
+	case VIRTIO_PCI_COMMON_CFGGENERATION:
+		ioport__write8(data, 0); /* TODO */
+		break;
+	case VIRTIO_PCI_COMMON_Q_SELECT:
+		ioport__write16(data, vpci->queue_selector);
+		break;
+	case VIRTIO_PCI_COMMON_Q_SIZE:
+		val = vdev->ops->get_size_vq(vpci->kvm, vpci->dev, vpci->queue_selector);
+		ioport__write16(data, val);
+		break;
+	case VIRTIO_PCI_COMMON_Q_MSIX:
+		val = vpci->vq_vector[vpci->queue_selector];
+		ioport__write16(data, val);
+		break;
+	case VIRTIO_PCI_COMMON_Q_ENABLE:
+		val = vdev->ops->get_queue(vpci->dev, vpci->queue_selector)->enabled;
+		ioport__write16(data, val);
+		break;
+	case VIRTIO_PCI_COMMON_Q_NOFF:
+		val = vpci->queue_selector;
+		ioport__write16(data, val);
+		break;
+	case VIRTIO_PCI_COMMON_Q_DESCLO:
+		val = (unsigned long)vdev->ops->get_queue(vpci->dev, vpci->queue_selector)->vring.desc;
+		ioport__write32(data, val);
+		break;
+	case VIRTIO_PCI_COMMON_Q_DESCHI:
+		val = (unsigned long)vdev->ops->get_queue(vpci->dev, vpci->queue_selector)->vring.desc >> 32;
+		ioport__write32(data, val);
+		break;
+	case VIRTIO_PCI_COMMON_Q_AVAILLO:
+		val = (unsigned long)vdev->ops->get_queue(vpci->dev, vpci->queue_selector)->vring.avail;
+		ioport__write32(data, val);
+		break;
+	case VIRTIO_PCI_COMMON_Q_AVAILHI:
+		val = (unsigned long)vdev->ops->get_queue(vpci->dev, vpci->queue_selector)->vring.avail >> 32;
+		ioport__write32(data, val);
+		break;
+	case VIRTIO_PCI_COMMON_Q_USEDLO:
+		val = (unsigned long)vdev->ops->get_queue(vpci->dev, vpci->queue_selector)->vring.used;
+		ioport__write32(data, val);
+		break;
+	case VIRTIO_PCI_COMMON_Q_USEDHI:
+		val = (unsigned long)vdev->ops->get_queue(vpci->dev, vpci->queue_selector)->vring.used >> 32;
+		ioport__write32(data, val);
+		break;
+	};
+
+	return true;
+}
+
+static bool (*io_in_map[5])(struct virtio_device *, unsigned long, void *, int) = {
+	virtio_pcim__common_in,
+	NULL,
+	NULL,
+	virtio_pcim__config_in,
+};
+
+static bool virtio_pcim__io_in(struct ioport *ioport, struct kvm_cpu *vcpu, u16 port, void *data, int size)
+{
+        unsigned long offset;
+        struct virtio_device *vdev;
+        struct virtio_pci_modern *vpci;
+
+        vdev = ioport->priv;
+        vpci = vdev->virtio;
+        offset = port - vpci->port_addr;
+
+	return io_in_map[offset/0x80](vdev, offset - (offset/0x80) * 0x80, data, size);
+}
+
+static struct ioport_operations virtio_pcim__io_ops = {
+	.io_in	= virtio_pcim__io_in,
+	.io_out	= virtio_pcim__io_out,
+};
+
+static void virtio_pcim__msix_mmio_callback(struct kvm_cpu *vcpu,
+					   u64 addr, u8 *data, u32 len,
+					   u8 is_write, void *ptr)
+{
+	struct virtio_pci_modern *vpci = ptr;
+	void *table;
+	u32 offset;
+
+	if (addr > vpci->msix_io_block + PCI_IO_SIZE) {
+		table	= &vpci->msix_pba;
+		offset	= vpci->msix_io_block + PCI_IO_SIZE;
+	} else {
+		table	= &vpci->msix_table;
+		offset	= vpci->msix_io_block;
+	}
+
+	if (is_write)
+		memcpy(table + addr - offset, data, len);
+	else
+		memcpy(data, table + addr - offset, len);
+}
+
+static void virtio_pcim__signal_msi(struct kvm *kvm, struct virtio_pci_modern *vpci, int vec)
+{
+	struct kvm_msi msi = {
+		.address_lo = vpci->msix_table[vec].msg.address_lo,
+		.address_hi = vpci->msix_table[vec].msg.address_hi,
+		.data = vpci->msix_table[vec].msg.data,
+	};
+
+	ioctl(kvm->vm_fd, KVM_SIGNAL_MSI, &msi);
+}
+
+int virtio_pcim__signal_vq(struct kvm *kvm, struct virtio_device *vdev, u32 vq)
+{
+	struct virtio_pci_modern *vpci = vdev->virtio;
+	int tbl = vpci->vq_vector[vq];
+
+	if (virtio_pcim__msix_enabled(vpci) && tbl != VIRTIO_MSI_NO_VECTOR) {
+		if (vpci->pci_hdr.msix.ctrl & cpu_to_le16(PCI_MSIX_FLAGS_MASKALL) ||
+		    vpci->msix_table[tbl].ctrl & cpu_to_le16(PCI_MSIX_ENTRY_CTRL_MASKBIT)) {
+
+			vpci->msix_pba |= 1 << tbl;
+			return 0;
+		}
+
+		if (vpci->features & VIRTIO_PCI_F_SIGNAL_MSI)
+			virtio_pcim__signal_msi(kvm, vpci, vpci->vq_vector[vq]);
+		else
+			kvm__irq_trigger(kvm, vpci->gsis[vq]);
+	} else {
+		vpci->isr = VIRTIO_IRQ_HIGH;
+		kvm__irq_trigger(kvm, vpci->legacy_irq_line);
+	}
+	return 0;
+}
+
+int virtio_pcim__signal_config(struct kvm *kvm, struct virtio_device *vdev)
+{
+	struct virtio_pci_modern *vpci = vdev->virtio;
+	int tbl = vpci->config_vector;
+
+	if (virtio_pcim__msix_enabled(vpci) && tbl != VIRTIO_MSI_NO_VECTOR) {
+		if (vpci->pci_hdr.msix.ctrl & cpu_to_le16(PCI_MSIX_FLAGS_MASKALL) ||
+		    vpci->msix_table[tbl].ctrl & cpu_to_le16(PCI_MSIX_ENTRY_CTRL_MASKBIT)) {
+
+			vpci->msix_pba |= 1 << tbl;
+			return 0;
+		}
+
+		if (vpci->features & VIRTIO_PCI_F_SIGNAL_MSI)
+			virtio_pcim__signal_msi(kvm, vpci, tbl);
+		else
+			kvm__irq_trigger(kvm, vpci->config_gsi);
+	} else {
+		vpci->isr = VIRTIO_PCI_ISR_CONFIG;
+		kvm__irq_trigger(kvm, vpci->legacy_irq_line);
+	}
+
+	return 0;
+}
+
+static void virtio_pcim__io_mmio_callback(struct kvm_cpu *vcpu,
+					 u64 addr, u8 *data, u32 len,
+					 u8 is_write, void *ptr)
+{
+	struct virtio_pci_modern *vpci = ptr;
+	int direction = is_write ? KVM_EXIT_IO_OUT : KVM_EXIT_IO_IN;
+	u16 port = vpci->port_addr + (addr & (IOPORT_SIZE - 1));
+
+	kvm__emulate_io(vcpu, port, data, direction, len, 1);
+}
+
+int virtio_pcim__init(struct kvm *kvm, void *dev, struct virtio_device *vdev,
+		     int device_id, int subsys_id, int class)
+{
+	struct virtio_pci_modern *vpci = vdev->virtio;
+	int r;
+
+	vpci->kvm = kvm;
+	vpci->dev = dev;
+
+	r = ioport__register(kvm, IOPORT_EMPTY, &virtio_pcim__io_ops, IOPORT_SIZE, vdev);
+	if (r < 0)
+		return r;
+	vpci->port_addr = (u16)r;
+
+	vpci->mmio_addr = pci_get_io_space_block(IOPORT_SIZE);
+	r = kvm__register_mmio(kvm, vpci->mmio_addr, IOPORT_SIZE, false,
+			       virtio_pcim__io_mmio_callback, vpci);
+	if (r < 0)
+		goto free_ioport;
+
+	vpci->msix_io_block = pci_get_io_space_block(PCI_IO_SIZE);
+	r = kvm__register_mmio(kvm, vpci->msix_io_block, PCI_IO_SIZE, false,
+			       virtio_pcim__msix_mmio_callback, vpci);
+	if (r < 0)
+		goto free_mmio;
+
+	vpci->pci_hdr = (struct pci_device_header) {
+		.vendor_id		= cpu_to_le16(PCI_VENDOR_ID_REDHAT_QUMRANET),
+		.device_id		= cpu_to_le16(device_id),
+		.command		= PCI_COMMAND_IO | PCI_COMMAND_MEMORY,
+		.header_type		= PCI_HEADER_TYPE_NORMAL,
+		.revision_id		= 0,
+		.class[0]		= class & 0xff,
+		.class[1]		= (class >> 8) & 0xff,
+		.class[2]		= (class >> 16) & 0xff,
+		.subsys_vendor_id	= cpu_to_le16(PCI_SUBSYSTEM_VENDOR_ID_REDHAT_QUMRANET),
+		.subsys_id		= cpu_to_le16(subsys_id),
+		.bar[0]			= cpu_to_le32(vpci->mmio_addr
+							| PCI_BASE_ADDRESS_SPACE_MEMORY),
+		.bar[1]			= cpu_to_le32(vpci->port_addr
+							| PCI_BASE_ADDRESS_SPACE_IO),
+		.bar[2]			= cpu_to_le32(vpci->msix_io_block
+							| PCI_BASE_ADDRESS_SPACE_MEMORY),
+		.status			= cpu_to_le16(PCI_STATUS_CAP_LIST),
+		.capabilities		= (void *)&vpci->pci_hdr.msix - (void *)&vpci->pci_hdr,
+		.bar_size[0]		= cpu_to_le32(IOPORT_SIZE),
+		.bar_size[1]		= cpu_to_le32(IOPORT_SIZE),
+		.bar_size[2]		= cpu_to_le32(PCI_IO_SIZE*2),
+	};
+
+	vpci->dev_hdr = (struct device_header) {
+		.bus_type		= DEVICE_BUS_PCI,
+		.data			= &vpci->pci_hdr,
+	};
+
+	vpci->pci_hdr.msix.cap = PCI_CAP_ID_MSIX;
+	vpci->pci_hdr.msix.next = (void *)&vpci->pci_hdr.common_cap - (void *)&vpci->pci_hdr,
+	/*
+	 * We at most have VIRTIO_PCI_MAX_VQ entries for virt queue,
+	 * VIRTIO_PCI_MAX_CONFIG entries for config.
+	 *
+	 * To quote the PCI spec:
+	 *
+	 * System software reads this field to determine the
+	 * MSI-X Table Size N, which is encoded as N-1.
+	 * For example, a returned value of "00000000011"
+	 * indicates a table size of 4.
+	 */
+	vpci->pci_hdr.msix.ctrl = cpu_to_le16(VIRTIO_PCI_MAX_VQ + VIRTIO_PCI_MAX_CONFIG - 1);
+
+	/* Both table and PBA are mapped to the same BAR (2) */
+	vpci->pci_hdr.msix.table_offset = cpu_to_le32(2);
+	vpci->pci_hdr.msix.pba_offset = cpu_to_le32(2 | PCI_IO_SIZE);
+	vpci->config_vector = 0;
+
+	if (kvm__supports_extension(kvm, KVM_CAP_SIGNAL_MSI))
+		vpci->features |= VIRTIO_PCI_F_SIGNAL_MSI;
+
+	vpci->pci_hdr.common_cap = (struct virtio_pci_cap) {
+		.cap_vndr = 0x09,
+		.cap_next = (void *)&vpci->pci_hdr.notify_cap - (void *)&vpci->pci_hdr,
+		.cap_len = sizeof(vpci->pci_hdr.common_cap),
+		.cfg_type = VIRTIO_PCI_CAP_COMMON_CFG,
+		.bar = 0,
+		.offset = 0,
+		.length = 0x80,
+	};
+	vpci->pci_hdr.notify_cap = (struct virtio_pci_notify_cap) {
+		.cap.cap_vndr = 0x09,
+		.cap.cap_next = (void *)&vpci->pci_hdr.isr_cap - (void *)&vpci->pci_hdr,
+		.cap.cap_len = sizeof(vpci->pci_hdr.notify_cap),
+		.cap.cfg_type = VIRTIO_PCI_CAP_NOTIFY_CFG,
+		.cap.bar = 0,
+		.cap.offset = 0x80,
+		.cap.length = 0x80,
+		.notify_off_multiplier = 2,
+	};
+	vpci->pci_hdr.isr_cap = (struct virtio_pci_cap) {
+		.cap_vndr = 0x09,
+		.cap_next = (void *)&vpci->pci_hdr.device_cap - (void *)&vpci->pci_hdr,
+		.cap_len = sizeof(vpci->pci_hdr.isr_cap),
+		.cfg_type = VIRTIO_PCI_CAP_ISR_CFG,
+		.bar = 0,
+		.offset = 0x100,
+		.length = 0x80,
+	};
+	vpci->pci_hdr.device_cap = (struct virtio_pci_cap) {
+		.cap_vndr = 0x09,
+		.cap_next = (void *)&vpci->pci_hdr.pci_cap - (void *)&vpci->pci_hdr,
+		.cap_len = sizeof(vpci->pci_hdr.device_cap),
+		.cfg_type = VIRTIO_PCI_CAP_DEVICE_CFG,
+		.bar = 0,
+		.offset = 0x180,
+		.length = 0x80,
+	};
+	vpci->pci_hdr.pci_cap = (struct virtio_pci_cfg_cap) {
+		.cap.cap_vndr = 0,
+		.cap.cap_next = 0,
+		.cap.cap_len = sizeof(vpci->pci_hdr.pci_cap),
+		.cap.cfg_type = VIRTIO_PCI_CAP_PCI_CFG,
+		.cap.bar = 0,
+		.cap.offset = 0x200,
+		.cap.length = 0x80,
+	};
+
+	r = device__register(&vpci->dev_hdr);
+	if (r < 0)
+		goto free_msix_mmio;
+
+	/* save the IRQ that device__register() has allocated */
+	vpci->legacy_irq_line = vpci->pci_hdr.irq_line;
+
+	return 0;
+
+free_msix_mmio:
+	kvm__deregister_mmio(kvm, vpci->msix_io_block);
+free_mmio:
+	kvm__deregister_mmio(kvm, vpci->mmio_addr);
+free_ioport:
+	ioport__unregister(kvm, vpci->port_addr);
+	return r;
+}
+
+int virtio_pcim__exit(struct kvm *kvm, struct virtio_device *vdev)
+{
+	struct virtio_pci_modern *vpci = vdev->virtio;
+	int i;
+
+	kvm__deregister_mmio(kvm, vpci->mmio_addr);
+	kvm__deregister_mmio(kvm, vpci->msix_io_block);
+	ioport__unregister(kvm, vpci->port_addr);
+
+	for (i = 0; i < VIRTIO_PCI_MAX_VQ; i++) {
+		ioeventfd__del_event(vpci->port_addr + VIRTIO_PCI_QUEUE_NOTIFY, i);
+		ioeventfd__del_event(vpci->mmio_addr + VIRTIO_PCI_QUEUE_NOTIFY, i);
+	}
+
+	return 0;
+}
diff --git a/virtio/rng.c b/virtio/rng.c
index 9b9e128..242bfa9 100644
--- a/virtio/rng.c
+++ b/virtio/rng.c
@@ -47,13 +47,13 @@ static u8 *get_config(struct kvm *kvm, void *dev)
 	return 0;
 }
 
-static u32 get_host_features(struct kvm *kvm, void *dev)
+static u32 get_host_features(struct kvm *kvm, void *dev, int sel)
 {
 	/* Unused */
 	return 0;
 }
 
-static void set_guest_features(struct kvm *kvm, void *dev, u32 features)
+static void set_guest_features(struct kvm *kvm, void *dev, u32 features, int sel)
 {
 	/* Unused */
 }
@@ -97,12 +97,17 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align,
 	compat__remove_message(compat_id);
 
 	queue		= &rdev->vqs[vq];
-	queue->pfn	= pfn;
-	p		= virtio_get_vq(kvm, queue->pfn, page_size);
+
+	if (pfn) {
+		queue->pfn	= pfn;
+		p		= virtio_get_vq(kvm, queue->pfn, page_size);
+		vring_init(&queue->vring, VIRTIO_RNG_QUEUE_SIZE, p, align);
+	} else {
+		virtio_adjust_vq(kvm, queue, VIRTIO_RNG_QUEUE_SIZE);
+	}
 
 	job = &rdev->jobs[vq];
 
-	vring_init(&queue->vring, VIRTIO_RNG_QUEUE_SIZE, p, align);
 
 	*job = (struct rng_dev_job) {
 		.vq	= queue,
@@ -130,6 +135,13 @@ static int get_pfn_vq(struct kvm *kvm, void *dev, u32 vq)
 	return rdev->vqs[vq].pfn;
 }
 
+static struct virt_queue *get_queue(void *dev, u32 vq)
+{
+	struct rng_dev *rdev = dev;
+
+	return &rdev->vqs[vq];
+}
+
 static int get_size_vq(struct kvm *kvm, void *dev, u32 vq)
 {
 	return VIRTIO_RNG_QUEUE_SIZE;
@@ -141,6 +153,11 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
 	return size;
 }
 
+static int queue_cnt(struct virtio_device *vdev)
+{
+	return NUM_VIRT_QUEUES;
+}
+
 static struct virtio_ops rng_dev_virtio_ops = {
 	.get_config		= get_config,
 	.get_host_features	= get_host_features,
@@ -150,6 +167,8 @@ static struct virtio_ops rng_dev_virtio_ops = {
 	.get_pfn_vq		= get_pfn_vq,
 	.get_size_vq		= get_size_vq,
 	.set_size_vq		= set_size_vq,
+	.queue_cnt		= queue_cnt,
+	.get_queue		= get_queue,
 };
 
 int virtio_rng__init(struct kvm *kvm)
diff --git a/virtio/scsi.c b/virtio/scsi.c
index 58d2353..3e4bb42 100644
--- a/virtio/scsi.c
+++ b/virtio/scsi.c
@@ -22,7 +22,7 @@ struct scsi_dev {
 	struct virt_queue		vqs[NUM_VIRT_QUEUES];
 	struct virtio_scsi_config	config;
 	struct vhost_scsi_target	target;
-	u32				features;
+	u64				features;
 	int				vhost_fd;
 	struct virtio_device		vdev;
 	struct list_head		list;
@@ -36,17 +36,19 @@ static u8 *get_config(struct kvm *kvm, void *dev)
 	return ((u8 *)(&sdev->config));
 }
 
-static u32 get_host_features(struct kvm *kvm, void *dev)
+static u32 get_host_features(struct kvm *kvm, void *dev, int sel)
 {
-	return	1UL << VIRTIO_RING_F_EVENT_IDX |
+	static u64 features = 1UL << VIRTIO_RING_F_EVENT_IDX |
 		1UL << VIRTIO_RING_F_INDIRECT_DESC;
+
+	return features >> (32 * sel);
 }
 
-static void set_guest_features(struct kvm *kvm, void *dev, u32 features)
+static void set_guest_features(struct kvm *kvm, void *dev, u32 features, int sel)
 {
 	struct scsi_dev *sdev = dev;
 
-	sdev->features = features;
+	sdev->features = (u64)features << (32 * sel);
 }
 
 static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align,
@@ -62,10 +64,14 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align,
 	compat__remove_message(compat_id);
 
 	queue		= &sdev->vqs[vq];
-	queue->pfn	= pfn;
-	p		= virtio_get_vq(kvm, queue->pfn, page_size);
 
-	vring_init(&queue->vring, VIRTIO_SCSI_QUEUE_SIZE, p, align);
+	if (pfn) {
+		queue->pfn	= pfn;
+		p		= virtio_get_vq(kvm, queue->pfn, page_size);
+		vring_init(&queue->vring, VIRTIO_SCSI_QUEUE_SIZE, p, align);
+	} else {
+		virtio_adjust_vq(kvm, queue, VIRTIO_SCSI_QUEUE_SIZE);
+	}
 
 	if (sdev->vhost_fd == 0)
 		return 0;
@@ -157,6 +163,13 @@ static int get_pfn_vq(struct kvm *kvm, void *dev, u32 vq)
 	return sdev->vqs[vq].pfn;
 }
 
+static struct virt_queue *get_queue(void *dev, u32 vq)
+{
+	struct scsi_dev *sdev = dev;
+
+	return &sdev->vqs[vq];
+}
+
 static int get_size_vq(struct kvm *kvm, void *dev, u32 vq)
 {
 	return VIRTIO_SCSI_QUEUE_SIZE;
@@ -167,6 +180,11 @@ static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
 	return size;
 }
 
+static int queue_cnt(struct virtio_device *vdev)
+{
+	return NUM_VIRT_QUEUES;
+}
+
 static struct virtio_ops scsi_dev_virtio_ops = {
 	.get_config		= get_config,
 	.get_host_features	= get_host_features,
@@ -178,6 +196,8 @@ static struct virtio_ops scsi_dev_virtio_ops = {
 	.notify_vq		= notify_vq,
 	.notify_vq_gsi		= notify_vq_gsi,
 	.notify_vq_eventfd	= notify_vq_eventfd,
+	.queue_cnt		= queue_cnt,
+	.get_queue		= get_queue,
 };
 
 static void virtio_scsi_vhost_init(struct kvm *kvm, struct scsi_dev *sdev)
diff --git a/x86/include/kvm/kvm-arch.h b/x86/include/kvm/kvm-arch.h
index 50b3bfb..0b216b9 100644
--- a/x86/include/kvm/kvm-arch.h
+++ b/x86/include/kvm/kvm-arch.h
@@ -28,7 +28,7 @@
 
 #define KVM_VM_TYPE		0
 
-#define VIRTIO_DEFAULT_TRANS(kvm)	VIRTIO_PCI
+#define VIRTIO_DEFAULT_TRANS(kvm) default_transport
 
 struct kvm_arch {
 	u16			boot_selector;
-- 
2.5.0

^ permalink raw reply related

* Re: [PATCH] virtio_ring: Shadow available ring flags & index
From: Venkatesh Srinivas via Virtualization @ 2015-11-18  4:28 UTC (permalink / raw)
  To: Xie, Huawei
  Cc: KVM list, Michael S. Tsirkin,
	virtualization@lists.linux-foundation.org, Venkatesh Srinivas,
	luto@kernel.org, David Matlack, Paolo Bonzini
In-Reply-To: <20151118040818.GA17436@google.com>

On Tue, Nov 17, 2015 at 08:08:18PM -0800, Venkatesh Srinivas wrote:
> On Mon, Nov 16, 2015 at 7:46 PM, Xie, Huawei <huawei.xie@intel.com> wrote:
> 
> > On 11/14/2015 7:41 AM, Venkatesh Srinivas wrote:
> > > On Wed, Nov 11, 2015 at 02:34:33PM +0200, Michael S. Tsirkin wrote:
> > >> On Tue, Nov 10, 2015 at 04:21:07PM -0800, Venkatesh Srinivas wrote:
> > >>> Improves cacheline transfer flow of available ring header.
> > >>>
> > >>> Virtqueues are implemented as a pair of rings, one producer->consumer
> > >>> avail ring and one consumer->producer used ring; preceding the
> > >>> avail ring in memory are two contiguous u16 fields -- avail->flags
> > >>> and avail->idx. A producer posts work by writing to avail->idx and
> > >>> a consumer reads avail->idx.
> > >>>
> > >>> The flags and idx fields only need to be written by a producer CPU
> > >>> and only read by a consumer CPU; when the producer and consumer are
> > >>> running on different CPUs and the virtio_ring code is structured to
> > >>> only have source writes/sink reads, we can continuously transfer the
> > >>> avail header cacheline between 'M' states between cores. This flow
> > >>> optimizes core -> core bandwidth on certain CPUs.
> > >>>
> > >>> (see: "Software Optimization Guide for AMD Family 15h Processors",
> > >>> Section 11.6; similar language appears in the 10h guide and should
> > >>> apply to CPUs w/ exclusive caches, using LLC as a transfer cache)
> > >>>
> > >>> Unfortunately the existing virtio_ring code issued reads to the
> > >>> avail->idx and read-modify-writes to avail->flags on the producer.
> > >>>
> > >>> This change shadows the flags and index fields in producer memory;
> > >>> the vring code now reads from the shadows and only ever writes to
> > >>> avail->flags and avail->idx, allowing the cacheline to transfer
> > >>> core -> core optimally.
> > >> Sounds logical, I'll apply this after a  bit of testing
> > >> of my own, thanks!
> > > Thanks!
> >
> 
> > Venkatesh:
> > Is it that your patch only applies to CPUs w/ exclusive caches?
> 
> No --- it applies when the inter-cache coherence flow is optimized by
> 'M' -> 'M' transfers and when producer reads might interfere w/
> consumer prefetchw/reads. The AMD Optimization guides have specific
> language on this subject, but other platforms may benefit.
> (see Intel #'s below)
> 
> > Do you have perf data on Intel CPUs?
> 
> Good idea -- I ran some tests on a couple of Intel platforms:
> 
> (these are perf data from sample runs; for each I ran many runs, the
>  numbers were pretty stable except for Haswell-EP cross-socket)
> 
> One-socket Intel Xeon W3690 ("Westmere"), 3.46 GHz; core turbo disabled
> =======================================================================
> (note -- w/ core turbo disabled, performance is _very_ stable; variance of
>  < 0.5% run-to-run; figure of merit is "seconds elapsed" here)
> 
> * Producer / consumer bound to Hyperthread pairs:
> 
>  Performance counter stats for './vring_bench_noshadow 1000000000':
> 
>  343,425,166,916 L1-dcache-loads
>       21,393,148 L1-dcache-load-misses     #    0.01% of all L1-dcache hits
>   61,709,640,363 L1-dcache-stores
>        5,745,690 L1-dcache-store-misses
>   10,186,932,553 L1-dcache-prefetches
>            1,491 L1-dcache-prefetch-misses
>    121.335699344 seconds time elapsed
> 
>  Performance counter stats for './vring_bench_shadow 1000000000':
> 
>  334,766,413,861 L1-dcache-loads
>       15,787,778 L1-dcache-load-misses     #    0.00% of all L1-dcache hits
>   62,735,792,799 L1-dcache-stores
>        3,252,113 L1-dcache-store-misses
>    9,018,273,596 L1-dcache-prefetches
>              819 L1-dcache-prefetch-misses
>    121.206339656 seconds time elapsed
> 
> Effectively Performance-neutral.
> 
> * Producer / consumer bound to separate cores, same socket:
> 
>  Performance counter stats for './vring_bench_noshadow 1000000000':
> 
>    399,943,384,509 L1-dcache-loads
>      8,868,334,693 L1-dcache-load-misses     #    2.22% of all L1-dcache hits
>     62,721,376,685 L1-dcache-stores
>      2,786,806,982 L1-dcache-store-misses
>     10,915,046,967 L1-dcache-prefetches
>            328,508 L1-dcache-prefetch-misses
>      146.585969976 seconds time elapsed
> 
>  Performance counter stats for './vring_bench_shadow 1000000000':
> 
>    425,123,067,750 L1-dcache-loads 
>      6,689,318,709 L1-dcache-load-misses     #    1.57% of all L1-dcache hits
>     62,747,525,005 L1-dcache-stores 
>      2,496,274,505 L1-dcache-store-misses
>      8,627,873,397 L1-dcache-prefetches
>            146,729 L1-dcache-prefetch-misses
>      142.657327765 seconds time elapsed
> 
> 2.6% reduction in runtime; note that L1-dcache-load-misses reduced
> dramatically, 2 Billion(!) L1d misses saved.
> 
> Two-socket Intel Sandy Bridge(-EP) Xeon, 2.6 GHz; core turbo disabled
> =====================================================================
> 
> * Producer / consumer bound to Hyperthread pairs:
> 
>  Performance counter stats for './vring_bench_noshadow 100000000':
> 
>     37,129,070,402 L1-dcache-loads
>          6,416,246 L1-dcache-load-misses     #    0.02% of all L1-dcache hits
>      6,207,794,675 L1-dcache-stores
>          2,800,094 L1-dcache-store-misses
>       17.029790809 seconds time elapsed
> 
>  Performance counter stats for './vring_bench_shadow 100000000':
> 
>     36,799,559,391 L1-dcache-loads
>         10,241,080 L1-dcache-load-misses     #    0.03% of all L1-dcache hits
>      6,312,252,458 L1-dcache-stores
>          2,742,239 L1-dcache-store-misses
>       16.941001709 seconds time elapsed
> 
> Effectively Performance-neutral.
> 
> * Producer / consumer bound to separate cores, same socket:
> 
>  Performance counter stats for './vring_bench_noshadow 100000000':
> 
>     27,684,883,046 L1-dcache-loads
>        809,933,091 L1-dcache-load-misses     #    2.93% of all L1-dcache hits
>      6,219,598,352 L1-dcache-stores
>          1,758,503 L1-dcache-store-misses
>       15.020511218 seconds time elapsed
> 
>  Performance counter stats for './vring_bench_shadow 100000000':
> 
>     28,092,111,012 L1-dcache-loads                     
>        716,687,011 L1-dcache-load-misses     #    2.55% of all L1-dcache hits 
>      6,290,821,211 L1-dcache-stores 
>          1,565,583 L1-dcache-store-misses                                    
>       15.208420297 seconds time elapsed
> 
> Effectively Performance-neutral.
> 
> * Producer / consumer bound to separate cores, cross socket:
> (Sandy Bridge-EP appears to have less cross-socket variance than Haswell-EP)
> 
>  Performance counter stats for './vring_bench_noshadow 100000000':
> 
>     35,857,245,449 L1-dcache-loads
>        821,746,755 L1-dcache-load-misses     #    2.29% of all L1-dcache hits
>      6,252,551,550 L1-dcache-stores
>          4,665,405 L1-dcache-store-misses
>       46.340035651 seconds time elapsed
> 
>  Performance counter stats for './vring_bench_shadow 100000000':
> 
>     39,044,022,857 L1-dcache-loads
>        711,731,527 L1-dcache-load-misses     #    1.82% of all L1-dcache hits
>      6,349,051,557 L1-dcache-stores
>          4,292,362 L1-dcache-store-misses
>       42.593259436 seconds time elapsed
> 
> Runtimes for the cross-socket test have somewhat higher variance, but the
> pattern in counts of L1-dcache-loads and L1-dcache-load-misses for nonshadow
> vs. shadow code is very stable.
> 
> noshadow (w/o this patch) reliably clocks in at ~46 seconds, shadow ranges
> from ~48 to ~42 (-2.8% to +8.0%).
> 
> Two-socket Intel Haswell(-EP) Xeon, 2.3 GHz; core turbo disabled
> ================================================================
> 
> * Producer / consumer bound to Hyperthread pairs:
> 
>  Performance counter stats for './vring_bench_noshadow 10000000000':
> 
>    474,856,463,271 L1-dcache-loads
>         74,223,784 L1-dcache-load-misses     #    0.02% of all L1-dcache hits
>     87,274,898,671 L1-dcache-stores
>         31,869,448 L1-dcache-store-misses
>      243.290969318 seconds time elapsed
> 
>  Performance counter stats for './vring_bench_shadow 10000000000':
> 
>    466,891,993,302 L1-dcache-loads
>         80,859,208 L1-dcache-load-misses     #    0.02% of all L1-dcache hits
>     88,760,627,355 L1-dcache-stores
>         35,727,720 L1-dcache-store-misses
>      242.146970822 seconds time elapsed
> 
> Effectively Performance-neutral.
> 
> * Producer / consumer bound to separate cores, same socket:
> 
>  Performance counter stats for './vring_bench_noshadow 10000000000':
> 
>    357,657,891,797 L1-dcache-loads
>      8,760,549,978 L1-dcache-load-misses     #    2.45% of all L1-dcache hits
>     87,357,651,103 L1-dcache-stores
>         10,166,431 L1-dcache-store-misses
>      229.733047436 seconds time elapsed
> 
>  Performance counter stats for './vring_bench_shadow 10000000000':
> 
>    382,508,881,516 L1-dcache-loads
>      8,348,013,630 L1-dcache-load-misses     #    2.18% of all L1-dcache hits
>     88,756,639,931 L1-dcache-stores
>          9,842,999 L1-dcache-store-misses
>      230.850697668 seconds time elapsed
> 
> Effectively Performance-neutral.
> 
> * Producer / consumer bound to separate cores, different sockets:
> 
> Unfortunately I don't have useful numbers for this case -- even with
> core turbo disabled, runtime variance is very high (10 - 30% run-to-run).
> 
> > For the perf metric you provide, why not L1-dcache-load-misses which is
> > more meaning full?
> 
> L1-dcache-load-misses is a better metric, you're right; for the original
> AMD Piledriver run I posted:
> 
>  Performance counter stats for './vring_bench_noshadow':
>      5,451,082,016      L1-dcache-loads
>         31,690,398      L1-dcache-load-misses
>         60,288,052      L1-dcache-stores
>         60,517,840      LLC-loads
>              9,726      LLC-load-misses
>        2.221477739      seconds time elapsed
>  
>  Performance counter stats for './vring_bench_shadow':
>      5,405,701,361      L1-dcache-loads
>         31,157,235      L1-dcache-load-misses
>         59,172,380      L1-dcache-stores
>         59,398,269      LLC-loads
>             10,944      LLC-load-misses
>        2.168405376      seconds time elapsed
> 
> There is a 1.6% reduction in L1-dcache-load-misses, which lines up with
> about a 2% reduction in runtime.
> 
> Summary:
> * No workload on Westmere 1S, Sandy Bridge 2S, and Haswell 2S got worse;
> * Westmere 1S cross-core improved by ~2.5% reliably;
> * Sandy Bridge 2S cross-core cross-socket may have improved. (cross-socket
>   run variance makes it hard to tell)
> * AMD Piledriver tests improved by ~2%;
> * Other virtio implementations (over PCIe for example) should benefit;
> 
> HTH,
> -- vs;

I'm sorry -- I appear to have added an unintentional HTML draft part to my
reply. This would prevent the message from appearing on the kvm@ mailing list
at the minimum.

Re-posting with the HTML part scrubbed.

Sorry,
-- vs;

^ permalink raw reply

* Re: [PATCH] virtio_ring: Shadow available ring flags & index
From: Venkatesh Srinivas via Virtualization @ 2015-11-18  4:08 UTC (permalink / raw)
  To: Xie, Huawei
  Cc: KVM list, Michael S. Tsirkin,
	virtualization@lists.linux-foundation.org, Venkatesh Srinivas,
	luto@kernel.org, David Matlack, Paolo Bonzini
In-Reply-To: <C37D651A908B024F974696C65296B57B4B1942C3@SHSMSX101.ccr.corp.intel.com>

[-- Attachment #1: Type: text/plain, Size: 10168 bytes --]

On Mon, Nov 16, 2015 at 7:46 PM, Xie, Huawei <huawei.xie@intel.com> wrote:

> On 11/14/2015 7:41 AM, Venkatesh Srinivas wrote:
> > On Wed, Nov 11, 2015 at 02:34:33PM +0200, Michael S. Tsirkin wrote:
> >> On Tue, Nov 10, 2015 at 04:21:07PM -0800, Venkatesh Srinivas wrote:
> >>> Improves cacheline transfer flow of available ring header.
> >>>
> >>> Virtqueues are implemented as a pair of rings, one producer->consumer
> >>> avail ring and one consumer->producer used ring; preceding the
> >>> avail ring in memory are two contiguous u16 fields -- avail->flags
> >>> and avail->idx. A producer posts work by writing to avail->idx and
> >>> a consumer reads avail->idx.
> >>>
> >>> The flags and idx fields only need to be written by a producer CPU
> >>> and only read by a consumer CPU; when the producer and consumer are
> >>> running on different CPUs and the virtio_ring code is structured to
> >>> only have source writes/sink reads, we can continuously transfer the
> >>> avail header cacheline between 'M' states between cores. This flow
> >>> optimizes core -> core bandwidth on certain CPUs.
> >>>
> >>> (see: "Software Optimization Guide for AMD Family 15h Processors",
> >>> Section 11.6; similar language appears in the 10h guide and should
> >>> apply to CPUs w/ exclusive caches, using LLC as a transfer cache)
> >>>
> >>> Unfortunately the existing virtio_ring code issued reads to the
> >>> avail->idx and read-modify-writes to avail->flags on the producer.
> >>>
> >>> This change shadows the flags and index fields in producer memory;
> >>> the vring code now reads from the shadows and only ever writes to
> >>> avail->flags and avail->idx, allowing the cacheline to transfer
> >>> core -> core optimally.
> >> Sounds logical, I'll apply this after a  bit of testing
> >> of my own, thanks!
> > Thanks!
>

> Venkatesh:
> Is it that your patch only applies to CPUs w/ exclusive caches?

No --- it applies when the inter-cache coherence flow is optimized by
'M' -> 'M' transfers and when producer reads might interfere w/
consumer prefetchw/reads. The AMD Optimization guides have specific
language on this subject, but other platforms may benefit.
(see Intel #'s below)

> Do you have perf data on Intel CPUs?

Good idea -- I ran some tests on a couple of Intel platforms:

(these are perf data from sample runs; for each I ran many runs, the
 numbers were pretty stable except for Haswell-EP cross-socket)

One-socket Intel Xeon W3690 ("Westmere"), 3.46 GHz; core turbo disabled
=======================================================================
(note -- w/ core turbo disabled, performance is _very_ stable; variance of
 < 0.5% run-to-run; figure of merit is "seconds elapsed" here)

* Producer / consumer bound to Hyperthread pairs:

 Performance counter stats for './vring_bench_noshadow 1000000000':

 343,425,166,916 L1-dcache-loads
      21,393,148 L1-dcache-load-misses     #    0.01% of all L1-dcache hits
  61,709,640,363 L1-dcache-stores
       5,745,690 L1-dcache-store-misses
  10,186,932,553 L1-dcache-prefetches
           1,491 L1-dcache-prefetch-misses
   121.335699344 seconds time elapsed

 Performance counter stats for './vring_bench_shadow 1000000000':

 334,766,413,861 L1-dcache-loads
      15,787,778 L1-dcache-load-misses     #    0.00% of all L1-dcache hits
  62,735,792,799 L1-dcache-stores
       3,252,113 L1-dcache-store-misses
   9,018,273,596 L1-dcache-prefetches
             819 L1-dcache-prefetch-misses
   121.206339656 seconds time elapsed

Effectively Performance-neutral.

* Producer / consumer bound to separate cores, same socket:

 Performance counter stats for './vring_bench_noshadow 1000000000':

   399,943,384,509 L1-dcache-loads
     8,868,334,693 L1-dcache-load-misses     #    2.22% of all L1-dcache hits
    62,721,376,685 L1-dcache-stores
     2,786,806,982 L1-dcache-store-misses
    10,915,046,967 L1-dcache-prefetches
           328,508 L1-dcache-prefetch-misses
     146.585969976 seconds time elapsed

 Performance counter stats for './vring_bench_shadow 1000000000':

   425,123,067,750 L1-dcache-loads 
     6,689,318,709 L1-dcache-load-misses     #    1.57% of all L1-dcache hits
    62,747,525,005 L1-dcache-stores 
     2,496,274,505 L1-dcache-store-misses
     8,627,873,397 L1-dcache-prefetches
           146,729 L1-dcache-prefetch-misses
     142.657327765 seconds time elapsed

2.6% reduction in runtime; note that L1-dcache-load-misses reduced dramatically,
2 Billion(!) L1d misses saved.

Two-socket Intel Sandy Bridge(-EP) Xeon, 2.6 GHz; core turbo disabled
=====================================================================

* Producer / consumer bound to Hyperthread pairs:

 Performance counter stats for './vring_bench_noshadow 100000000':

    37,129,070,402 L1-dcache-loads
         6,416,246 L1-dcache-load-misses     #    0.02% of all L1-dcache hits
     6,207,794,675 L1-dcache-stores
         2,800,094 L1-dcache-store-misses
      17.029790809 seconds time elapsed

 Performance counter stats for './vring_bench_shadow 100000000':

    36,799,559,391 L1-dcache-loads
        10,241,080 L1-dcache-load-misses     #    0.03% of all L1-dcache hits
     6,312,252,458 L1-dcache-stores
         2,742,239 L1-dcache-store-misses
      16.941001709 seconds time elapsed

Effectively Performance-neutral.

* Producer / consumer bound to separate cores, same socket:

 Performance counter stats for './vring_bench_noshadow 100000000':

    27,684,883,046 L1-dcache-loads
       809,933,091 L1-dcache-load-misses     #    2.93% of all L1-dcache hits
     6,219,598,352 L1-dcache-stores
         1,758,503 L1-dcache-store-misses
      15.020511218 seconds time elapsed

 Performance counter stats for './vring_bench_shadow 100000000':

    28,092,111,012 L1-dcache-loads                                             
       716,687,011 L1-dcache-load-misses     #    2.55% of all L1-dcache hits  
     6,290,821,211 L1-dcache-stores                                            
         1,565,583 L1-dcache-store-misses                                      
      15.208420297 seconds time elapsed

Effectively Performance-neutral.

* Producer / consumer bound to separate cores, cross socket:
(Sandy Bridge-EP appears to have less cross-socket variance than Haswell-EP)

 Performance counter stats for './vring_bench_noshadow 100000000':

    35,857,245,449 L1-dcache-loads
       821,746,755 L1-dcache-load-misses     #    2.29% of all L1-dcache hits
     6,252,551,550 L1-dcache-stores
         4,665,405 L1-dcache-store-misses
      46.340035651 seconds time elapsed

 Performance counter stats for './vring_bench_shadow 100000000':

    39,044,022,857 L1-dcache-loads
       711,731,527 L1-dcache-load-misses     #    1.82% of all L1-dcache hits
     6,349,051,557 L1-dcache-stores
         4,292,362 L1-dcache-store-misses
      42.593259436 seconds time elapsed

Runtimes for the cross-socket test have somewhat higher variance, but the
pattern in counts of L1-dcache-loads and L1-dcache-load-misses for nonshadow
vs. shadow code is very stable.

noshadow (w/o this patch) reliably clocks in at ~46 seconds, shadow ranges
from ~48 to ~42 (-2.8% to +8.0%).

Two-socket Intel Haswell(-EP) Xeon, 2.3 GHz; core turbo disabled
================================================================

* Producer / consumer bound to Hyperthread pairs:

 Performance counter stats for './vring_bench_noshadow 10000000000':

   474,856,463,271 L1-dcache-loads
        74,223,784 L1-dcache-load-misses     #    0.02% of all L1-dcache hits
    87,274,898,671 L1-dcache-stores
        31,869,448 L1-dcache-store-misses
     243.290969318 seconds time elapsed

 Performance counter stats for './vring_bench_shadow 10000000000':

   466,891,993,302 L1-dcache-loads
        80,859,208 L1-dcache-load-misses     #    0.02% of all L1-dcache hits
    88,760,627,355 L1-dcache-stores
        35,727,720 L1-dcache-store-misses
     242.146970822 seconds time elapsed

Effectively Performance-neutral.

* Producer / consumer bound to separate cores, same socket:

 Performance counter stats for './vring_bench_noshadow 10000000000':

   357,657,891,797 L1-dcache-loads
     8,760,549,978 L1-dcache-load-misses     #    2.45% of all L1-dcache hits
    87,357,651,103 L1-dcache-stores
        10,166,431 L1-dcache-store-misses
     229.733047436 seconds time elapsed

 Performance counter stats for './vring_bench_shadow 10000000000':

   382,508,881,516 L1-dcache-loads
     8,348,013,630 L1-dcache-load-misses     #    2.18% of all L1-dcache hits
    88,756,639,931 L1-dcache-stores
         9,842,999 L1-dcache-store-misses
     230.850697668 seconds time elapsed

Effectively Performance-neutral.

* Producer / consumer bound to separate cores, different sockets:

Unfortunately I don't have useful numbers for this case -- even with
core turbo disabled, runtime variance is very high (10 - 30% run-to-run).

> For the perf metric you provide, why not L1-dcache-load-misses which is
> more meaning full?

L1-dcache-load-misses is a better metric, you're right; for the original
AMD Piledriver run I posted:

 Performance counter stats for './vring_bench_noshadow':
     5,451,082,016      L1-dcache-loads
        31,690,398      L1-dcache-load-misses
        60,288,052      L1-dcache-stores
        60,517,840      LLC-loads
             9,726      LLC-load-misses
       2.221477739      seconds time elapsed
 
 Performance counter stats for './vring_bench_shadow':
     5,405,701,361      L1-dcache-loads
        31,157,235      L1-dcache-load-misses
        59,172,380      L1-dcache-stores
        59,398,269      LLC-loads
            10,944      LLC-load-misses
       2.168405376      seconds time elapsed

There is a 1.6% reduction in L1-dcache-load-misses, which lines up with
about a 2% reduction in runtime.

Summary:
* No workload on Westmere 1S, Sandy Bridge 2S, and Haswell 2S got worse;
* Westmere 1S cross-core improved by ~2.5% reliably;
* Sandy Bridge 2S cross-core cross-socket may have improved. (cross-socket
  run variance makes it hard to tell)
* AMD Piledriver tests improved by ~2%;
* Other virtio implementations (over PCIe for example) should benefit;

HTH,
-- vs;

[-- Attachment #2: Type: text/html, Size: 3401 bytes --]

[-- Attachment #3: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply

* [PATCH] paravirt: remove paravirt ops pmd_update[_defer] and pte_update_defer
From: Juergen Gross @ 2015-11-17 14:51 UTC (permalink / raw)
  To: linux-kernel, x86, hpa, tglx, mingo, jeremy, chrisw, akataria,
	rusty, virtualization, xen-devel, konrad.wilk, david.vrabel,
	boris.ostrovsky
  Cc: Juergen Gross

pte_update_defer can be removed as it is always set to the same
function as pte_update. So any usage of pte_update_defer() can be
replaced by pte_update().

pmd_update and pmd_update_defer are always set to paravirt_nop, so they
can just be nuked.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 arch/x86/include/asm/paravirt.h       | 17 -----------------
 arch/x86/include/asm/paravirt_types.h |  6 ------
 arch/x86/include/asm/pgtable.h        | 15 ++-------------
 arch/x86/kernel/paravirt.c            |  3 ---
 arch/x86/lguest/boot.c                |  1 -
 arch/x86/mm/pgtable.c                 |  7 +------
 arch/x86/xen/mmu.c                    |  1 -
 7 files changed, 3 insertions(+), 47 deletions(-)

diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index 10d0596..398f068 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -375,23 +375,6 @@ static inline void pte_update(struct mm_struct *mm, unsigned long addr,
 {
 	PVOP_VCALL3(pv_mmu_ops.pte_update, mm, addr, ptep);
 }
-static inline void pmd_update(struct mm_struct *mm, unsigned long addr,
-			      pmd_t *pmdp)
-{
-	PVOP_VCALL3(pv_mmu_ops.pmd_update, mm, addr, pmdp);
-}
-
-static inline void pte_update_defer(struct mm_struct *mm, unsigned long addr,
-				    pte_t *ptep)
-{
-	PVOP_VCALL3(pv_mmu_ops.pte_update_defer, mm, addr, ptep);
-}
-
-static inline void pmd_update_defer(struct mm_struct *mm, unsigned long addr,
-				    pmd_t *pmdp)
-{
-	PVOP_VCALL3(pv_mmu_ops.pmd_update_defer, mm, addr, pmdp);
-}
 
 static inline pte_t __pte(pteval_t val)
 {
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index 31247b5..6418541 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -274,12 +274,6 @@ struct pv_mmu_ops {
 			   pmd_t *pmdp, pmd_t pmdval);
 	void (*pte_update)(struct mm_struct *mm, unsigned long addr,
 			   pte_t *ptep);
-	void (*pte_update_defer)(struct mm_struct *mm,
-				 unsigned long addr, pte_t *ptep);
-	void (*pmd_update)(struct mm_struct *mm, unsigned long addr,
-			   pmd_t *pmdp);
-	void (*pmd_update_defer)(struct mm_struct *mm,
-				 unsigned long addr, pmd_t *pmdp);
 
 	pte_t (*ptep_modify_prot_start)(struct mm_struct *mm, unsigned long addr,
 					pte_t *ptep);
diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
index 6ec0c8b..d3eee66 100644
--- a/arch/x86/include/asm/pgtable.h
+++ b/arch/x86/include/asm/pgtable.h
@@ -69,9 +69,6 @@ extern struct mm_struct *pgd_page_get_mm(struct page *page);
 #define pmd_clear(pmd)			native_pmd_clear(pmd)
 
 #define pte_update(mm, addr, ptep)              do { } while (0)
-#define pte_update_defer(mm, addr, ptep)        do { } while (0)
-#define pmd_update(mm, addr, ptep)              do { } while (0)
-#define pmd_update_defer(mm, addr, ptep)        do { } while (0)
 
 #define pgd_val(x)	native_pgd_val(x)
 #define __pgd(x)	native_make_pgd(x)
@@ -731,14 +728,9 @@ static inline void native_set_pmd_at(struct mm_struct *mm, unsigned long addr,
  * updates should either be sets, clears, or set_pte_atomic for P->P
  * transitions, which means this hook should only be called for user PTEs.
  * This hook implies a P->P protection or access change has taken place, which
- * requires a subsequent TLB flush.  The notification can optionally be delayed
- * until the TLB flush event by using the pte_update_defer form of the
- * interface, but care must be taken to assure that the flush happens while
- * still holding the same page table lock so that the shadow and primary pages
- * do not become out of sync on SMP.
+ * requires a subsequent TLB flush.
  */
 #define pte_update(mm, addr, ptep)		do { } while (0)
-#define pte_update_defer(mm, addr, ptep)	do { } while (0)
 #endif
 
 /*
@@ -830,9 +822,7 @@ static inline int pmd_write(pmd_t pmd)
 static inline pmd_t pmdp_huge_get_and_clear(struct mm_struct *mm, unsigned long addr,
 				       pmd_t *pmdp)
 {
-	pmd_t pmd = native_pmdp_get_and_clear(pmdp);
-	pmd_update(mm, addr, pmdp);
-	return pmd;
+	return native_pmdp_get_and_clear(pmdp);
 }
 
 #define __HAVE_ARCH_PMDP_SET_WRPROTECT
@@ -840,7 +830,6 @@ static inline void pmdp_set_wrprotect(struct mm_struct *mm,
 				      unsigned long addr, pmd_t *pmdp)
 {
 	clear_bit(_PAGE_BIT_RW, (unsigned long *)pmdp);
-	pmd_update(mm, addr, pmdp);
 }
 
 /*
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index c2130ae..f601250 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -444,9 +444,6 @@ struct pv_mmu_ops pv_mmu_ops = {
 	.set_pmd = native_set_pmd,
 	.set_pmd_at = native_set_pmd_at,
 	.pte_update = paravirt_nop,
-	.pte_update_defer = paravirt_nop,
-	.pmd_update = paravirt_nop,
-	.pmd_update_defer = paravirt_nop,
 
 	.ptep_modify_prot_start = __ptep_modify_prot_start,
 	.ptep_modify_prot_commit = __ptep_modify_prot_commit,
diff --git a/arch/x86/lguest/boot.c b/arch/x86/lguest/boot.c
index a0d09f6..a1900d4 100644
--- a/arch/x86/lguest/boot.c
+++ b/arch/x86/lguest/boot.c
@@ -1472,7 +1472,6 @@ __init void lguest_init(void)
 	pv_mmu_ops.lazy_mode.leave = lguest_leave_lazy_mmu_mode;
 	pv_mmu_ops.lazy_mode.flush = paravirt_flush_lazy_mmu;
 	pv_mmu_ops.pte_update = lguest_pte_update;
-	pv_mmu_ops.pte_update_defer = lguest_pte_update;
 
 #ifdef CONFIG_X86_LOCAL_APIC
 	/* APIC read/write intercepts */
diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
index fb0a9dd..ee9c2e3 100644
--- a/arch/x86/mm/pgtable.c
+++ b/arch/x86/mm/pgtable.c
@@ -414,7 +414,7 @@ int ptep_set_access_flags(struct vm_area_struct *vma,
 
 	if (changed && dirty) {
 		*ptep = entry;
-		pte_update_defer(vma->vm_mm, address, ptep);
+		pte_update(vma->vm_mm, address, ptep);
 	}
 
 	return changed;
@@ -431,7 +431,6 @@ int pmdp_set_access_flags(struct vm_area_struct *vma,
 
 	if (changed && dirty) {
 		*pmdp = entry;
-		pmd_update_defer(vma->vm_mm, address, pmdp);
 		/*
 		 * We had a write-protection fault here and changed the pmd
 		 * to to more permissive. No need to flush the TLB for that,
@@ -469,9 +468,6 @@ int pmdp_test_and_clear_young(struct vm_area_struct *vma,
 		ret = test_and_clear_bit(_PAGE_BIT_ACCESSED,
 					 (unsigned long *)pmdp);
 
-	if (ret)
-		pmd_update(vma->vm_mm, addr, pmdp);
-
 	return ret;
 }
 #endif
@@ -518,7 +514,6 @@ void pmdp_splitting_flush(struct vm_area_struct *vma,
 	set = !test_and_set_bit(_PAGE_BIT_SPLITTING,
 				(unsigned long *)pmdp);
 	if (set) {
-		pmd_update(vma->vm_mm, address, pmdp);
 		/* need tlb flush only to serialize against gup-fast */
 		flush_tlb_range(vma, address, address + HPAGE_PMD_SIZE);
 	}
diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
index ac161db..896dc14 100644
--- a/arch/x86/xen/mmu.c
+++ b/arch/x86/xen/mmu.c
@@ -2436,7 +2436,6 @@ static const struct pv_mmu_ops xen_mmu_ops __initconst = {
 	.flush_tlb_others = xen_flush_tlb_others,
 
 	.pte_update = paravirt_nop,
-	.pte_update_defer = paravirt_nop,
 
 	.pgd_alloc = xen_pgd_alloc,
 	.pgd_free = xen_pgd_free,
-- 
2.6.2

^ permalink raw reply related

* Re: [PATCH] paravirt: remove paravirt ops pmd_update_defer and pte_update_defer
From: Juergen Gross @ 2015-11-17 14:47 UTC (permalink / raw)
  To: linux-kernel, x86, hpa, tglx, mingo, jeremy, chrisw, akataria,
	rusty, virtualization, xen-devel, konrad.wilk, david.vrabel,
	boris.ostrovsky
In-Reply-To: <1447771603-9918-1-git-send-email-jgross@suse.com>

On 17/11/15 15:46, Juergen Gross wrote:
> pte_update_defer can be removed as it is always set to the same
> function as pte_update. So any usage of pte_update_defer() can be
> replaced by pte_update().
> 
> pmd_update_defer is always set to paravirt_nop, so it can just be
> nuked.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Sorry, hit send to early. Please forget this one.


Juergen

^ permalink raw reply

* [PATCH] paravirt: remove paravirt ops pmd_update_defer and pte_update_defer
From: Juergen Gross @ 2015-11-17 14:46 UTC (permalink / raw)
  To: linux-kernel, x86, hpa, tglx, mingo, jeremy, chrisw, akataria,
	rusty, virtualization, xen-devel, konrad.wilk, david.vrabel,
	boris.ostrovsky
  Cc: Juergen Gross

pte_update_defer can be removed as it is always set to the same
function as pte_update. So any usage of pte_update_defer() can be
replaced by pte_update().

pmd_update_defer is always set to paravirt_nop, so it can just be
nuked.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 arch/x86/include/asm/paravirt.h       | 12 ------------
 arch/x86/include/asm/paravirt_types.h |  4 ----
 arch/x86/include/asm/pgtable.h        |  9 +--------
 arch/x86/kernel/paravirt.c            |  2 --
 arch/x86/lguest/boot.c                |  1 -
 arch/x86/mm/pgtable.c                 |  3 +--
 arch/x86/xen/mmu.c                    |  1 -
 7 files changed, 2 insertions(+), 30 deletions(-)

diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index 10d0596..10c39b9 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -381,18 +381,6 @@ static inline void pmd_update(struct mm_struct *mm, unsigned long addr,
 	PVOP_VCALL3(pv_mmu_ops.pmd_update, mm, addr, pmdp);
 }
 
-static inline void pte_update_defer(struct mm_struct *mm, unsigned long addr,
-				    pte_t *ptep)
-{
-	PVOP_VCALL3(pv_mmu_ops.pte_update_defer, mm, addr, ptep);
-}
-
-static inline void pmd_update_defer(struct mm_struct *mm, unsigned long addr,
-				    pmd_t *pmdp)
-{
-	PVOP_VCALL3(pv_mmu_ops.pmd_update_defer, mm, addr, pmdp);
-}
-
 static inline pte_t __pte(pteval_t val)
 {
 	pteval_t ret;
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index 31247b5..274727e 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -274,12 +274,8 @@ struct pv_mmu_ops {
 			   pmd_t *pmdp, pmd_t pmdval);
 	void (*pte_update)(struct mm_struct *mm, unsigned long addr,
 			   pte_t *ptep);
-	void (*pte_update_defer)(struct mm_struct *mm,
-				 unsigned long addr, pte_t *ptep);
 	void (*pmd_update)(struct mm_struct *mm, unsigned long addr,
 			   pmd_t *pmdp);
-	void (*pmd_update_defer)(struct mm_struct *mm,
-				 unsigned long addr, pmd_t *pmdp);
 
 	pte_t (*ptep_modify_prot_start)(struct mm_struct *mm, unsigned long addr,
 					pte_t *ptep);
diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
index 6ec0c8b..5126367 100644
--- a/arch/x86/include/asm/pgtable.h
+++ b/arch/x86/include/asm/pgtable.h
@@ -69,9 +69,7 @@ extern struct mm_struct *pgd_page_get_mm(struct page *page);
 #define pmd_clear(pmd)			native_pmd_clear(pmd)
 
 #define pte_update(mm, addr, ptep)              do { } while (0)
-#define pte_update_defer(mm, addr, ptep)        do { } while (0)
 #define pmd_update(mm, addr, ptep)              do { } while (0)
-#define pmd_update_defer(mm, addr, ptep)        do { } while (0)
 
 #define pgd_val(x)	native_pgd_val(x)
 #define __pgd(x)	native_make_pgd(x)
@@ -731,14 +729,9 @@ static inline void native_set_pmd_at(struct mm_struct *mm, unsigned long addr,
  * updates should either be sets, clears, or set_pte_atomic for P->P
  * transitions, which means this hook should only be called for user PTEs.
  * This hook implies a P->P protection or access change has taken place, which
- * requires a subsequent TLB flush.  The notification can optionally be delayed
- * until the TLB flush event by using the pte_update_defer form of the
- * interface, but care must be taken to assure that the flush happens while
- * still holding the same page table lock so that the shadow and primary pages
- * do not become out of sync on SMP.
+ * requires a subsequent TLB flush.
  */
 #define pte_update(mm, addr, ptep)		do { } while (0)
-#define pte_update_defer(mm, addr, ptep)	do { } while (0)
 #endif
 
 /*
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index c2130ae..3ac7b85 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -444,9 +444,7 @@ struct pv_mmu_ops pv_mmu_ops = {
 	.set_pmd = native_set_pmd,
 	.set_pmd_at = native_set_pmd_at,
 	.pte_update = paravirt_nop,
-	.pte_update_defer = paravirt_nop,
 	.pmd_update = paravirt_nop,
-	.pmd_update_defer = paravirt_nop,
 
 	.ptep_modify_prot_start = __ptep_modify_prot_start,
 	.ptep_modify_prot_commit = __ptep_modify_prot_commit,
diff --git a/arch/x86/lguest/boot.c b/arch/x86/lguest/boot.c
index a0d09f6..a1900d4 100644
--- a/arch/x86/lguest/boot.c
+++ b/arch/x86/lguest/boot.c
@@ -1472,7 +1472,6 @@ __init void lguest_init(void)
 	pv_mmu_ops.lazy_mode.leave = lguest_leave_lazy_mmu_mode;
 	pv_mmu_ops.lazy_mode.flush = paravirt_flush_lazy_mmu;
 	pv_mmu_ops.pte_update = lguest_pte_update;
-	pv_mmu_ops.pte_update_defer = lguest_pte_update;
 
 #ifdef CONFIG_X86_LOCAL_APIC
 	/* APIC read/write intercepts */
diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
index fb0a9dd..3d8fc45 100644
--- a/arch/x86/mm/pgtable.c
+++ b/arch/x86/mm/pgtable.c
@@ -414,7 +414,7 @@ int ptep_set_access_flags(struct vm_area_struct *vma,
 
 	if (changed && dirty) {
 		*ptep = entry;
-		pte_update_defer(vma->vm_mm, address, ptep);
+		pte_update(vma->vm_mm, address, ptep);
 	}
 
 	return changed;
@@ -431,7 +431,6 @@ int pmdp_set_access_flags(struct vm_area_struct *vma,
 
 	if (changed && dirty) {
 		*pmdp = entry;
-		pmd_update_defer(vma->vm_mm, address, pmdp);
 		/*
 		 * We had a write-protection fault here and changed the pmd
 		 * to to more permissive. No need to flush the TLB for that,
diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
index ac161db..896dc14 100644
--- a/arch/x86/xen/mmu.c
+++ b/arch/x86/xen/mmu.c
@@ -2436,7 +2436,6 @@ static const struct pv_mmu_ops xen_mmu_ops __initconst = {
 	.flush_tlb_others = xen_flush_tlb_others,
 
 	.pte_update = paravirt_nop,
-	.pte_update_defer = paravirt_nop,
 
 	.pgd_alloc = xen_pgd_alloc,
 	.pgd_free = xen_pgd_free,
-- 
2.6.2

^ permalink raw reply related

* Re: [PATCH] paravirt: remove unused pv_apic_ops structure
From: David Vrabel @ 2015-11-17 13:51 UTC (permalink / raw)
  To: Juergen Gross, linux-kernel, x86, hpa, tglx, mingo, jeremy,
	chrisw, akataria, rusty, virtualization, xen-devel, konrad.wilk,
	boris.ostrovsky
In-Reply-To: <1447767872-16730-1-git-send-email-jgross@suse.com>

On 17/11/15 13:44, Juergen Gross wrote:
> The only member of that structure is startup_ipi_hook which is always
> set to paravirt_nop.

Reviewed-by: David Vrabel <david.vrabel@citrix.com>

David

^ permalink raw reply

* [PATCH] paravirt: remove unused pv_apic_ops structure
From: Juergen Gross @ 2015-11-17 13:44 UTC (permalink / raw)
  To: linux-kernel, x86, hpa, tglx, mingo, jeremy, chrisw, akataria,
	rusty, virtualization, xen-devel, konrad.wilk, david.vrabel,
	boris.ostrovsky
  Cc: Juergen Gross

The only member of that structure is startup_ipi_hook which is always
set to paravirt_nop.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 arch/x86/include/asm/paravirt.h       |  9 ---------
 arch/x86/include/asm/paravirt_types.h | 10 ----------
 arch/x86/include/asm/smp.h            |  3 ---
 arch/x86/kernel/paravirt.c            |  8 --------
 arch/x86/kernel/smpboot.c             |  7 -------
 arch/x86/xen/enlighten.c              |  7 -------
 6 files changed, 44 deletions(-)

diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index 10d0596..4d7f080 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -285,15 +285,6 @@ static inline void slow_down_io(void)
 #endif
 }
 
-#ifdef CONFIG_SMP
-static inline void startup_ipi_hook(int phys_apicid, unsigned long start_eip,
-				    unsigned long start_esp)
-{
-	PVOP_VCALL3(pv_apic_ops.startup_ipi_hook,
-		    phys_apicid, start_eip, start_esp);
-}
-#endif
-
 static inline void paravirt_activate_mm(struct mm_struct *prev,
 					struct mm_struct *next)
 {
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index 31247b5..b0e603b 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -215,14 +215,6 @@ struct pv_irq_ops {
 #endif
 };
 
-struct pv_apic_ops {
-#ifdef CONFIG_X86_LOCAL_APIC
-	void (*startup_ipi_hook)(int phys_apicid,
-				 unsigned long start_eip,
-				 unsigned long start_esp);
-#endif
-};
-
 struct pv_mmu_ops {
 	unsigned long (*read_cr2)(void);
 	void (*write_cr2)(unsigned long);
@@ -354,7 +346,6 @@ struct paravirt_patch_template {
 	struct pv_time_ops pv_time_ops;
 	struct pv_cpu_ops pv_cpu_ops;
 	struct pv_irq_ops pv_irq_ops;
-	struct pv_apic_ops pv_apic_ops;
 	struct pv_mmu_ops pv_mmu_ops;
 	struct pv_lock_ops pv_lock_ops;
 };
@@ -364,7 +355,6 @@ extern struct pv_init_ops pv_init_ops;
 extern struct pv_time_ops pv_time_ops;
 extern struct pv_cpu_ops pv_cpu_ops;
 extern struct pv_irq_ops pv_irq_ops;
-extern struct pv_apic_ops pv_apic_ops;
 extern struct pv_mmu_ops pv_mmu_ops;
 extern struct pv_lock_ops pv_lock_ops;
 
diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
index 222a6a3..c16ddf9 100644
--- a/arch/x86/include/asm/smp.h
+++ b/arch/x86/include/asm/smp.h
@@ -74,9 +74,6 @@ struct smp_ops {
 extern void set_cpu_sibling_map(int cpu);
 
 #ifdef CONFIG_SMP
-#ifndef CONFIG_PARAVIRT
-#define startup_ipi_hook(phys_apicid, start_eip, start_esp) do { } while (0)
-#endif
 extern struct smp_ops smp_ops;
 
 static inline void smp_send_stop(void)
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index c2130ae..f3b79b2 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -133,7 +133,6 @@ static void *get_call_destination(u8 type)
 		.pv_time_ops = pv_time_ops,
 		.pv_cpu_ops = pv_cpu_ops,
 		.pv_irq_ops = pv_irq_ops,
-		.pv_apic_ops = pv_apic_ops,
 		.pv_mmu_ops = pv_mmu_ops,
 #ifdef CONFIG_PARAVIRT_SPINLOCKS
 		.pv_lock_ops = pv_lock_ops,
@@ -403,12 +402,6 @@ NOKPROBE_SYMBOL(native_get_debugreg);
 NOKPROBE_SYMBOL(native_set_debugreg);
 NOKPROBE_SYMBOL(native_load_idt);
 
-struct pv_apic_ops pv_apic_ops = {
-#ifdef CONFIG_X86_LOCAL_APIC
-	.startup_ipi_hook = paravirt_nop,
-#endif
-};
-
 #if defined(CONFIG_X86_32) && !defined(CONFIG_X86_PAE)
 /* 32-bit pagetable entries */
 #define PTE_IDENT	__PV_IS_CALLEE_SAVE(_paravirt_ident_32)
@@ -492,6 +485,5 @@ struct pv_mmu_ops pv_mmu_ops = {
 EXPORT_SYMBOL_GPL(pv_time_ops);
 EXPORT_SYMBOL    (pv_cpu_ops);
 EXPORT_SYMBOL    (pv_mmu_ops);
-EXPORT_SYMBOL_GPL(pv_apic_ops);
 EXPORT_SYMBOL_GPL(pv_info);
 EXPORT_SYMBOL    (pv_irq_ops);
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 892ee2e5..4df7777 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -629,13 +629,6 @@ wakeup_secondary_cpu_via_init(int phys_apicid, unsigned long start_eip)
 		num_starts = 0;
 
 	/*
-	 * Paravirt / VMI wants a startup IPI hook here to set up the
-	 * target processor state.
-	 */
-	startup_ipi_hook(phys_apicid, (unsigned long) start_secondary,
-			 stack_start);
-
-	/*
 	 * Run STARTUP IPI loop.
 	 */
 	pr_debug("#startup loops: %d\n", num_starts);
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 5774800..4334e511 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1265,12 +1265,6 @@ static const struct pv_cpu_ops xen_cpu_ops __initconst = {
 	.end_context_switch = xen_end_context_switch,
 };
 
-static const struct pv_apic_ops xen_apic_ops __initconst = {
-#ifdef CONFIG_X86_LOCAL_APIC
-	.startup_ipi_hook = paravirt_nop,
-#endif
-};
-
 static void xen_reboot(int reason)
 {
 	struct sched_shutdown r = { .reason = reason };
@@ -1536,7 +1530,6 @@ asmlinkage __visible void __init xen_start_kernel(void)
 	/* Install Xen paravirt ops */
 	pv_info = xen_info;
 	pv_init_ops = xen_init_ops;
-	pv_apic_ops = xen_apic_ops;
 	if (!xen_pvh_domain()) {
 		pv_cpu_ops = xen_cpu_ops;
 
-- 
2.6.2

^ permalink raw reply related

* vhost-blk and qemu
From: Mohan G via Virtualization @ 2015-11-17 13:23 UTC (permalink / raw)
  To: virtualization@lists.linux-foundation.org
In-Reply-To: <1872664481.6220477.1447766581884.JavaMail.yahoo.ref@mail.yahoo.com>


[-- Attachment #1.1: Type: text/plain, Size: 302 bytes --]

Hi,I am looking to experiment the vhost-blk stack. Can some one point me to the latest code version and the corresponding qemu version location.I am on centos 7 (3.10 ) kernel. I am hoping using the vhost-blk.ko and corresponding qemu version will get me started to measure some numbers.


RegardsMohan

[-- Attachment #1.2: Type: text/html, Size: 920 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply

* Re: [PATCH net-next RFC V3 0/3] basic busy polling support for vhost_net
From: Jason Wang @ 2015-11-17  6:31 UTC (permalink / raw)
  To: Felipe Franciosi
  Cc: netdev@vger.kernel.org, virtualization@lists.linux-foundation.org,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org, mst@redhat.com
In-Reply-To: <5645AB73.7060808@redhat.com>



On 11/13/2015 05:20 PM, Jason Wang wrote:
>
> On 11/12/2015 08:02 PM, Felipe Franciosi wrote:
>> Hi Jason,
>>
>> I understand your busy loop timeout is quite conservative at 50us. Did you try any other values?
> I've also tried 20us. And results shows 50us was better in:
>
> - very small packet tx (e.g 64bytes at most 46% improvement)
> - TCP_RR (at most 11% improvement)
>
> But I will test bigger values. In fact, for net itself, we can be even
> more aggressive: make vhost poll forever but I haven't tired this.
>
>> Also, did you measure how polling affects many VMs talking to each other (e.g. 20 VMs on each host, perhaps with several vNICs each, transmitting to a corresponding VM/vNIC pair on another host)?
> Not yet, in my todo list.
>
>>
>> On a complete separate experiment (busy waiting on storage I/O rings on Xen), I have observed that bigger timeouts gave bigger benefits. On the other hand, all cases that contended for CPU were badly hurt with any sort of polling.
>>
>> The cases that contended for CPU consisted of many VMs generating workload over very fast I/O devices (in that case, several NVMe devices on a single host). And the metric that got affected was aggregate throughput from all VMs.
>>
>> The solution was to determine whether to poll depending on the host's overall CPU utilisation at that moment. That gave me the best of both worlds as polling made everything faster without slowing down any other metric.
> You mean a threshold and exit polling when it exceeds this? I use a
> simpler method: just exit the busy loop when there's more than one
> processes is in running state. I test this method in the past for socket
> busy read (http://www.gossamer-threads.com/lists/linux/kernel/1997531)
> which seems can solve the issue. But haven't tested this for vhost
> polling. Will run some simple test (e.g pin two vhost threads in one
> host cpu), and see how well it perform.
>
> Thanks

Run simple test like:

- starting two VMs, each vm has one vcpu
- pin both two vhost threads of VMs to cpu 0
- pin vcpu0 of VM1 to cpu1
- pin vcpu0 of VM2 to cpu2

Try two TCP_RR netperf tests in parallell:

/busy loop timeouts/trate1+trate2/-+%/
/no busy loop/13966.76+13987.31/+0%/
/20us/14097.89+14088.82/+0.08%)
/50us/15103.98+15103.73/+8.06%/


Busy loop can still give improvements even if two vhost threads are
contending one cpu on host.

>
>> Thanks,
>> Felipe
>>
>>
>>
>> On 12/11/2015 10:20, "kvm-owner@vger.kernel.org on behalf of Jason Wang" <kvm-owner@vger.kernel.org on behalf of jasowang@redhat.com> wrote:
>>
>>> On 11/12/2015 06:16 PM, Jason Wang wrote:
>>>> Hi all:
>>>>
>>>> This series tries to add basic busy polling for vhost net. The idea is
>>>> simple: at the end of tx/rx processing, busy polling for new tx added
>>>> descriptor and rx receive socket for a while. The maximum number of
>>>> time (in us) could be spent on busy polling was specified ioctl.
>>>>
>>>> Test were done through:
>>>>
>>>> - 50 us as busy loop timeout
>>>> - Netperf 2.6
>>>> - Two machines with back to back connected ixgbe
>>>> - Guest with 1 vcpu and 1 queue
>>>>
>>>> Results:
>>>> - For stream workload, ioexits were reduced dramatically in medium
>>>>   size (1024-2048) of tx (at most -39%) and almost all rx (at most
>>>>   -79%) as a result of polling. This compensate for the possible
>>>>   wasted cpu cycles more or less. That porbably why we can still see
>>>>   some increasing in the normalized throughput in some cases.
>>>> - Throughput of tx were increased (at most 105%) expect for the huge
>>>>   write (16384). And we can send more packets in the case (+tpkts were
>>>>   increased).
>>>> - Very minor rx regression in some cases.
>>>> - Improvemnt on TCP_RR (at most 16%).
>>> Forget to mention, the following test results by order are:
>>>
>>> 1) Guest TX
>>> 2) Guest RX
>>> 3) TCP_RR
>>>
>>>> size/session/+thu%/+normalize%/+tpkts%/+rpkts%/+ioexits%/
>>>>    64/     1/   +9%/  -17%/   +5%/  +10%/   -2%
>>>>    64/     2/   +8%/  -18%/   +6%/  +10%/   -1%
>>>>    64/     4/   +4%/  -21%/   +6%/  +10%/   -1%
>>>>    64/     8/   +9%/  -17%/   +6%/   +9%/   -2%
>>>>   256/     1/  +20%/   -1%/  +15%/  +11%/   -9%
>>>>   256/     2/  +15%/   -6%/  +15%/   +8%/   -8%
>>>>   256/     4/  +17%/   -4%/  +16%/   +8%/   -8%
>>>>   256/     8/  -61%/  -69%/  +16%/  +10%/  -10%
>>>>   512/     1/  +15%/   -3%/  +19%/  +18%/  -11%
>>>>   512/     2/  +19%/    0%/  +19%/  +13%/  -10%
>>>>   512/     4/  +18%/   -2%/  +18%/  +15%/  -10%
>>>>   512/     8/  +17%/   -1%/  +18%/  +15%/  -11%
>>>>  1024/     1/  +25%/   +4%/  +27%/  +16%/  -21%
>>>>  1024/     2/  +28%/   +8%/  +25%/  +15%/  -22%
>>>>  1024/     4/  +25%/   +5%/  +25%/  +14%/  -21%
>>>>  1024/     8/  +27%/   +7%/  +25%/  +16%/  -21%
>>>>  2048/     1/  +32%/  +12%/  +31%/  +22%/  -38%
>>>>  2048/     2/  +33%/  +12%/  +30%/  +23%/  -36%
>>>>  2048/     4/  +31%/  +10%/  +31%/  +24%/  -37%
>>>>  2048/     8/ +105%/  +75%/  +33%/  +23%/  -39%
>>>> 16384/     1/    0%/  -14%/   +2%/    0%/  +19%
>>>> 16384/     2/    0%/  -13%/  +19%/  -13%/  +17%
>>>> 16384/     4/    0%/  -12%/   +3%/    0%/   +2%
>>>> 16384/     8/    0%/  -11%/   -2%/   +1%/   +1%
>>>> size/session/+thu%/+normalize%/+tpkts%/+rpkts%/+ioexits%/
>>>>    64/     1/   -7%/  -23%/   +4%/   +6%/  -74%
>>>>    64/     2/   -2%/  -12%/   +2%/   +2%/  -55%
>>>>    64/     4/   +2%/   -5%/  +10%/   -2%/  -43%
>>>>    64/     8/   -5%/   -5%/  +11%/  -34%/  -59%
>>>>   256/     1/   -6%/  -16%/   +9%/  +11%/  -60%
>>>>   256/     2/   +3%/   -4%/   +6%/   -3%/  -28%
>>>>   256/     4/    0%/   -5%/   -9%/   -9%/  -10%
>>>>   256/     8/   -3%/   -6%/  -12%/   -9%/  -40%
>>>>   512/     1/   -4%/  -17%/  -10%/  +21%/  -34%
>>>>   512/     2/    0%/   -9%/  -14%/   -3%/  -30%
>>>>   512/     4/    0%/   -4%/  -18%/  -12%/   -4%
>>>>   512/     8/   -1%/   -4%/   -1%/   -5%/   +4%
>>>>  1024/     1/    0%/  -16%/  +12%/  +11%/  -10%
>>>>  1024/     2/    0%/  -11%/    0%/   +5%/  -31%
>>>>  1024/     4/    0%/   -4%/   -7%/   +1%/  -22%
>>>>  1024/     8/   -5%/   -6%/  -17%/  -29%/  -79%
>>>>  2048/     1/    0%/  -16%/   +1%/   +9%/  -10%
>>>>  2048/     2/    0%/  -12%/   +7%/   +9%/  -26%
>>>>  2048/     4/    0%/   -7%/   -4%/   +3%/  -64%
>>>>  2048/     8/   -1%/   -5%/   -6%/   +4%/  -20%
>>>> 16384/     1/    0%/  -12%/  +11%/   +7%/  -20%
>>>> 16384/     2/    0%/   -7%/   +1%/   +5%/  -26%
>>>> 16384/     4/    0%/   -5%/  +12%/  +22%/  -23%
>>>> 16384/     8/    0%/   -1%/   -8%/   +5%/   -3%
>>>> size/session/+thu%/+normalize%/+tpkts%/+rpkts%/+ioexits%/
>>>>     1/     1/   +9%/  -29%/   +9%/   +9%/   +9%
>>>>     1/    25/   +6%/  -18%/   +6%/   +6%/   -1%
>>>>     1/    50/   +6%/  -19%/   +5%/   +5%/   -2%
>>>>     1/   100/   +5%/  -19%/   +4%/   +4%/   -3%
>>>>    64/     1/  +10%/  -28%/  +10%/  +10%/  +10%
>>>>    64/    25/   +8%/  -18%/   +7%/   +7%/   -2%
>>>>    64/    50/   +8%/  -17%/   +8%/   +8%/   -1%
>>>>    64/   100/   +8%/  -17%/   +8%/   +8%/   -1%
>>>>   256/     1/  +10%/  -28%/  +10%/  +10%/  +10%
>>>>   256/    25/  +15%/  -13%/  +15%/  +15%/    0%
>>>>   256/    50/  +16%/  -14%/  +18%/  +18%/   +2%
>>>>   256/   100/  +15%/  -13%/  +12%/  +12%/   -2%
>>>>
>>>> Changes from V2:
>>>> - poll also at the end of rx handling
>>>> - factor out the polling logic and optimize the code a little bit
>>>> - add two ioctls to get and set the busy poll timeout
>>>> - test on ixgbe (which can give more stable and reproducable numbers)
>>>>   instead of mlx4.
>>>>
>>>> Changes from V1:
>>>> - Add a comment for vhost_has_work() to explain why it could be
>>>>   lockless
>>>> - Add param description for busyloop_timeout
>>>> - Split out the busy polling logic into a new helper
>>>> - Check and exit the loop when there's a pending signal
>>>> - Disable preemption during busy looping to make sure lock_clock() was
>>>>   correctly used.
>>>>
>>>> Jason Wang (3):
>>>>   vhost: introduce vhost_has_work()
>>>>   vhost: introduce vhost_vq_more_avail()
>>>>   vhost_net: basic polling support
>>>>
>>>>  drivers/vhost/net.c        | 77 +++++++++++++++++++++++++++++++++++++++++++---
>>>>  drivers/vhost/vhost.c      | 48 +++++++++++++++++++++++------
>>>>  drivers/vhost/vhost.h      |  3 ++
>>>>  include/uapi/linux/vhost.h | 11 +++++++
>>>>  4 files changed, 125 insertions(+), 14 deletions(-)
>>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> N�����r��y���b�X��ǧv�^�)޺{.n�+����{����zX��\x17��ܨ}���Ơz�&j:+v���\a����zZ+��+zf���h���~����i���z�\x1e�w���?����&�)ߢ^[f��^jǫy�m��@A�a��\x7f�\f0��h�\x0f�i\x7f
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply

* Re: [PATCH] vhost: relax log address alignment
From: Jason Wang @ 2015-11-17  5:40 UTC (permalink / raw)
  To: Michael S. Tsirkin, linux-kernel; +Cc: netdev, kvm, virtualization
In-Reply-To: <1447685995-16282-1-git-send-email-mst@redhat.com>



On 11/16/2015 11:00 PM, Michael S. Tsirkin wrote:
> commit 5d9a07b0de512b77bf28d2401e5fe3351f00a240 ("vhost: relax used
> address alignment") fixed the alignment for the used virtual address,
> but not for the physical address used for logging.
>
> That's a mistake: alignment should clearly be the same for virtual and
> physical addresses,
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  drivers/vhost/vhost.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index eec2f11..080422f 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -819,7 +819,7 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp)
>  		BUILD_BUG_ON(__alignof__ *vq->used > VRING_USED_ALIGN_SIZE);
>  		if ((a.avail_user_addr & (VRING_AVAIL_ALIGN_SIZE - 1)) ||
>  		    (a.used_user_addr & (VRING_USED_ALIGN_SIZE - 1)) ||
> -		    (a.log_guest_addr & (sizeof(u64) - 1))) {
> +		    (a.log_guest_addr & (VRING_USED_ALIGN_SIZE - 1))) {
>  			r = -EINVAL;
>  			break;
>  		}

Acked-by: Jason Wang <jasowang@redhat.com>

^ permalink raw reply

* Re: [PATCH] virtio_ring: Shadow available ring flags & index
From: Xie, Huawei @ 2015-11-17  3:46 UTC (permalink / raw)
  To: Venkatesh Srinivas, Michael S. Tsirkin
  Cc: KVM list, virtualization@lists.linux-foundation.org,
	Venkatesh Srinivas, luto@kernel.org, David Matlack, Paolo Bonzini
In-Reply-To: <20151113234140.GA6512@google.com>

On 11/14/2015 7:41 AM, Venkatesh Srinivas wrote:
> On Wed, Nov 11, 2015 at 02:34:33PM +0200, Michael S. Tsirkin wrote:
>> On Tue, Nov 10, 2015 at 04:21:07PM -0800, Venkatesh Srinivas wrote:
>>> Improves cacheline transfer flow of available ring header.
>>>
>>> Virtqueues are implemented as a pair of rings, one producer->consumer
>>> avail ring and one consumer->producer used ring; preceding the
>>> avail ring in memory are two contiguous u16 fields -- avail->flags
>>> and avail->idx. A producer posts work by writing to avail->idx and
>>> a consumer reads avail->idx.
>>>
>>> The flags and idx fields only need to be written by a producer CPU
>>> and only read by a consumer CPU; when the producer and consumer are
>>> running on different CPUs and the virtio_ring code is structured to
>>> only have source writes/sink reads, we can continuously transfer the
>>> avail header cacheline between 'M' states between cores. This flow
>>> optimizes core -> core bandwidth on certain CPUs.
>>>
>>> (see: "Software Optimization Guide for AMD Family 15h Processors",
>>> Section 11.6; similar language appears in the 10h guide and should
>>> apply to CPUs w/ exclusive caches, using LLC as a transfer cache)
>>>
>>> Unfortunately the existing virtio_ring code issued reads to the
>>> avail->idx and read-modify-writes to avail->flags on the producer.
>>>
>>> This change shadows the flags and index fields in producer memory;
>>> the vring code now reads from the shadows and only ever writes to
>>> avail->flags and avail->idx, allowing the cacheline to transfer
>>> core -> core optimally.
>> Sounds logical, I'll apply this after a  bit of testing
>> of my own, thanks!
> Thanks!
Venkatesh:
Is it that your patch only applies to CPUs w/ exclusive caches? Do you
have perf data on Intel CPUs?
For the perf metric you provide, why not L1-dcache-load-misses which is
more meaning full?
>
>>> In a concurrent version of vring_bench, the time required for
>>> 10,000,000 buffer checkout/returns was reduced by ~2% (average
>>> across many runs) on an AMD Piledriver (15h) CPU:
>>>
>>> (w/o shadowing):
>>>  Performance counter stats for './vring_bench':
>>>      5,451,082,016      L1-dcache-loads
>>>      ...
>>>        2.221477739 seconds time elapsed
>>>
>>> (w/ shadowing):
>>>  Performance counter stats for './vring_bench':
>>>      5,405,701,361      L1-dcache-loads
>>>      ...
>>>        2.168405376 seconds time elapsed
>> Could you supply the full command line you used
>> to test this?
> Yes --
>
> perf stat -e L1-dcache-loads,L1-dcache-load-misses,L1-dcache-stores \
> 	./vring_bench
>
> The standard version of vring_bench is single-threaded (posted on this list
> but never submitted); my tests were with a version that has a worker thread
> polling the VQ. How should I share it? Should I just attach it to an email
> here?
>
>>> The further away (in a NUMA sense) virtio producers and consumers are
>>> from each other, the more we expect to benefit. Physical implementations
>>> of virtio devices and implementations of virtio where the consumer polls
>>> vring avail indexes (vhost) should also benefit.
>>>
>>> Signed-off-by: Venkatesh Srinivas <venkateshs@google.com>
>> Here's a similar patch for the ring itself:
>> https://lkml.org/lkml/2015/9/10/111
>>
>> Does it help you as well?
> I tested your patch in our environment; our virtqueues do not support
> Indirect entries and your patch does not manage to elide many writes, so I
> do not see a performance difference. In an environment with Indirect, your
> patch will likely be a win.
>
> (My patch gets most of its win by eliminating reads on the producer; when
> the producer reads avail fields at the same time the consumer is polling,
> we see cacheline transfers that hurt performance. Your patch eliminates
> writes, which is nice, but our tests w/ polling are not as sensitive to
> writes from the producer.)
>
> I have two quick comments on your patch --
> 1) I think you need to kfree vq->avail when deleting the virtqueue.
>
> 2) Should we avoid allocating a cache for virtqueues that are not
>    performance critical? (ex: virtio-scsi eventq/controlq, virtio-net
>    controlq)
>
> Should I post comments in reply to the original patch email (given that it
> is ~2 months old)?
>
> Thanks!
> -- vs;
>

^ permalink raw reply

* [PATCH] vhost: relax log address alignment
From: Michael S. Tsirkin @ 2015-11-16 15:00 UTC (permalink / raw)
  To: linux-kernel; +Cc: netdev, kvm, virtualization

commit 5d9a07b0de512b77bf28d2401e5fe3351f00a240 ("vhost: relax used
address alignment") fixed the alignment for the used virtual address,
but not for the physical address used for logging.

That's a mistake: alignment should clearly be the same for virtual and
physical addresses,

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/vhost/vhost.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index eec2f11..080422f 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -819,7 +819,7 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp)
 		BUILD_BUG_ON(__alignof__ *vq->used > VRING_USED_ALIGN_SIZE);
 		if ((a.avail_user_addr & (VRING_AVAIL_ALIGN_SIZE - 1)) ||
 		    (a.used_user_addr & (VRING_USED_ALIGN_SIZE - 1)) ||
-		    (a.log_guest_addr & (sizeof(u64) - 1))) {
+		    (a.log_guest_addr & (VRING_USED_ALIGN_SIZE - 1))) {
 			r = -EINVAL;
 			break;
 		}
-- 
MST

^ permalink raw reply related

* Re: [PATCH] virtio_ring: Shadow available ring flags & index
From: Venkatesh Srinivas via Virtualization @ 2015-11-13 23:41 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: KVM list, virtualization, Venkatesh Srinivas, luto, David Matlack,
	Paolo Bonzini, huawei.xie
In-Reply-To: <20151111142647-mutt-send-email-mst@redhat.com>

On Wed, Nov 11, 2015 at 02:34:33PM +0200, Michael S. Tsirkin wrote:
> On Tue, Nov 10, 2015 at 04:21:07PM -0800, Venkatesh Srinivas wrote:
> > Improves cacheline transfer flow of available ring header.
> >
> > Virtqueues are implemented as a pair of rings, one producer->consumer
> > avail ring and one consumer->producer used ring; preceding the
> > avail ring in memory are two contiguous u16 fields -- avail->flags
> > and avail->idx. A producer posts work by writing to avail->idx and
> > a consumer reads avail->idx.
> >
> > The flags and idx fields only need to be written by a producer CPU
> > and only read by a consumer CPU; when the producer and consumer are
> > running on different CPUs and the virtio_ring code is structured to
> > only have source writes/sink reads, we can continuously transfer the
> > avail header cacheline between 'M' states between cores. This flow
> > optimizes core -> core bandwidth on certain CPUs.
> >
> > (see: "Software Optimization Guide for AMD Family 15h Processors",
> > Section 11.6; similar language appears in the 10h guide and should
> > apply to CPUs w/ exclusive caches, using LLC as a transfer cache)
> >
> > Unfortunately the existing virtio_ring code issued reads to the
> > avail->idx and read-modify-writes to avail->flags on the producer.
> >
> > This change shadows the flags and index fields in producer memory;
> > the vring code now reads from the shadows and only ever writes to
> > avail->flags and avail->idx, allowing the cacheline to transfer
> > core -> core optimally.
>
> Sounds logical, I'll apply this after a  bit of testing
> of my own, thanks!

Thanks!

> > In a concurrent version of vring_bench, the time required for
> > 10,000,000 buffer checkout/returns was reduced by ~2% (average
> > across many runs) on an AMD Piledriver (15h) CPU:
> >
> > (w/o shadowing):
> >  Performance counter stats for './vring_bench':
> >      5,451,082,016      L1-dcache-loads
> >      ...
> >        2.221477739 seconds time elapsed
> >
> > (w/ shadowing):
> >  Performance counter stats for './vring_bench':
> >      5,405,701,361      L1-dcache-loads
> >      ...
> >        2.168405376 seconds time elapsed
>
> Could you supply the full command line you used
> to test this?

Yes --

perf stat -e L1-dcache-loads,L1-dcache-load-misses,L1-dcache-stores \
	./vring_bench

The standard version of vring_bench is single-threaded (posted on this list
but never submitted); my tests were with a version that has a worker thread
polling the VQ. How should I share it? Should I just attach it to an email
here?

> > The further away (in a NUMA sense) virtio producers and consumers are
> > from each other, the more we expect to benefit. Physical implementations
> > of virtio devices and implementations of virtio where the consumer polls
> > vring avail indexes (vhost) should also benefit.
> >
> > Signed-off-by: Venkatesh Srinivas <venkateshs@google.com>
>
> Here's a similar patch for the ring itself:
> https://lkml.org/lkml/2015/9/10/111
>
> Does it help you as well?

I tested your patch in our environment; our virtqueues do not support
Indirect entries and your patch does not manage to elide many writes, so I
do not see a performance difference. In an environment with Indirect, your
patch will likely be a win.

(My patch gets most of its win by eliminating reads on the producer; when
the producer reads avail fields at the same time the consumer is polling,
we see cacheline transfers that hurt performance. Your patch eliminates
writes, which is nice, but our tests w/ polling are not as sensitive to
writes from the producer.)

I have two quick comments on your patch --
1) I think you need to kfree vq->avail when deleting the virtqueue.

2) Should we avoid allocating a cache for virtqueues that are not
   performance critical? (ex: virtio-scsi eventq/controlq, virtio-net
   controlq)

Should I post comments in reply to the original patch email (given that it
is ~2 months old)?

Thanks!
-- vs;

^ permalink raw reply

* Re: [PATCH net-next RFC V3 0/3] basic busy polling support for vhost_net
From: Jason Wang @ 2015-11-13  9:20 UTC (permalink / raw)
  To: Felipe Franciosi
  Cc: netdev@vger.kernel.org, virtualization@lists.linux-foundation.org,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org, mst@redhat.com
In-Reply-To: <F61F5796-38D7-42A0-9590-75840230A576@nutanix.com>



On 11/12/2015 08:02 PM, Felipe Franciosi wrote:
> Hi Jason,
>
> I understand your busy loop timeout is quite conservative at 50us. Did you try any other values?

I've also tried 20us. And results shows 50us was better in:

- very small packet tx (e.g 64bytes at most 46% improvement)
- TCP_RR (at most 11% improvement)

But I will test bigger values. In fact, for net itself, we can be even
more aggressive: make vhost poll forever but I haven't tired this.

>
> Also, did you measure how polling affects many VMs talking to each other (e.g. 20 VMs on each host, perhaps with several vNICs each, transmitting to a corresponding VM/vNIC pair on another host)?

Not yet, in my todo list.

>
>
> On a complete separate experiment (busy waiting on storage I/O rings on Xen), I have observed that bigger timeouts gave bigger benefits. On the other hand, all cases that contended for CPU were badly hurt with any sort of polling.
>
> The cases that contended for CPU consisted of many VMs generating workload over very fast I/O devices (in that case, several NVMe devices on a single host). And the metric that got affected was aggregate throughput from all VMs.
>
> The solution was to determine whether to poll depending on the host's overall CPU utilisation at that moment. That gave me the best of both worlds as polling made everything faster without slowing down any other metric.

You mean a threshold and exit polling when it exceeds this? I use a
simpler method: just exit the busy loop when there's more than one
processes is in running state. I test this method in the past for socket
busy read (http://www.gossamer-threads.com/lists/linux/kernel/1997531)
which seems can solve the issue. But haven't tested this for vhost
polling. Will run some simple test (e.g pin two vhost threads in one
host cpu), and see how well it perform.

Thanks

>
> Thanks,
> Felipe
>
>
>
> On 12/11/2015 10:20, "kvm-owner@vger.kernel.org on behalf of Jason Wang" <kvm-owner@vger.kernel.org on behalf of jasowang@redhat.com> wrote:
>
>>
>> On 11/12/2015 06:16 PM, Jason Wang wrote:
>>> Hi all:
>>>
>>> This series tries to add basic busy polling for vhost net. The idea is
>>> simple: at the end of tx/rx processing, busy polling for new tx added
>>> descriptor and rx receive socket for a while. The maximum number of
>>> time (in us) could be spent on busy polling was specified ioctl.
>>>
>>> Test were done through:
>>>
>>> - 50 us as busy loop timeout
>>> - Netperf 2.6
>>> - Two machines with back to back connected ixgbe
>>> - Guest with 1 vcpu and 1 queue
>>>
>>> Results:
>>> - For stream workload, ioexits were reduced dramatically in medium
>>>   size (1024-2048) of tx (at most -39%) and almost all rx (at most
>>>   -79%) as a result of polling. This compensate for the possible
>>>   wasted cpu cycles more or less. That porbably why we can still see
>>>   some increasing in the normalized throughput in some cases.
>>> - Throughput of tx were increased (at most 105%) expect for the huge
>>>   write (16384). And we can send more packets in the case (+tpkts were
>>>   increased).
>>> - Very minor rx regression in some cases.
>>> - Improvemnt on TCP_RR (at most 16%).
>> Forget to mention, the following test results by order are:
>>
>> 1) Guest TX
>> 2) Guest RX
>> 3) TCP_RR
>>
>>> size/session/+thu%/+normalize%/+tpkts%/+rpkts%/+ioexits%/
>>>    64/     1/   +9%/  -17%/   +5%/  +10%/   -2%
>>>    64/     2/   +8%/  -18%/   +6%/  +10%/   -1%
>>>    64/     4/   +4%/  -21%/   +6%/  +10%/   -1%
>>>    64/     8/   +9%/  -17%/   +6%/   +9%/   -2%
>>>   256/     1/  +20%/   -1%/  +15%/  +11%/   -9%
>>>   256/     2/  +15%/   -6%/  +15%/   +8%/   -8%
>>>   256/     4/  +17%/   -4%/  +16%/   +8%/   -8%
>>>   256/     8/  -61%/  -69%/  +16%/  +10%/  -10%
>>>   512/     1/  +15%/   -3%/  +19%/  +18%/  -11%
>>>   512/     2/  +19%/    0%/  +19%/  +13%/  -10%
>>>   512/     4/  +18%/   -2%/  +18%/  +15%/  -10%
>>>   512/     8/  +17%/   -1%/  +18%/  +15%/  -11%
>>>  1024/     1/  +25%/   +4%/  +27%/  +16%/  -21%
>>>  1024/     2/  +28%/   +8%/  +25%/  +15%/  -22%
>>>  1024/     4/  +25%/   +5%/  +25%/  +14%/  -21%
>>>  1024/     8/  +27%/   +7%/  +25%/  +16%/  -21%
>>>  2048/     1/  +32%/  +12%/  +31%/  +22%/  -38%
>>>  2048/     2/  +33%/  +12%/  +30%/  +23%/  -36%
>>>  2048/     4/  +31%/  +10%/  +31%/  +24%/  -37%
>>>  2048/     8/ +105%/  +75%/  +33%/  +23%/  -39%
>>> 16384/     1/    0%/  -14%/   +2%/    0%/  +19%
>>> 16384/     2/    0%/  -13%/  +19%/  -13%/  +17%
>>> 16384/     4/    0%/  -12%/   +3%/    0%/   +2%
>>> 16384/     8/    0%/  -11%/   -2%/   +1%/   +1%
>>> size/session/+thu%/+normalize%/+tpkts%/+rpkts%/+ioexits%/
>>>    64/     1/   -7%/  -23%/   +4%/   +6%/  -74%
>>>    64/     2/   -2%/  -12%/   +2%/   +2%/  -55%
>>>    64/     4/   +2%/   -5%/  +10%/   -2%/  -43%
>>>    64/     8/   -5%/   -5%/  +11%/  -34%/  -59%
>>>   256/     1/   -6%/  -16%/   +9%/  +11%/  -60%
>>>   256/     2/   +3%/   -4%/   +6%/   -3%/  -28%
>>>   256/     4/    0%/   -5%/   -9%/   -9%/  -10%
>>>   256/     8/   -3%/   -6%/  -12%/   -9%/  -40%
>>>   512/     1/   -4%/  -17%/  -10%/  +21%/  -34%
>>>   512/     2/    0%/   -9%/  -14%/   -3%/  -30%
>>>   512/     4/    0%/   -4%/  -18%/  -12%/   -4%
>>>   512/     8/   -1%/   -4%/   -1%/   -5%/   +4%
>>>  1024/     1/    0%/  -16%/  +12%/  +11%/  -10%
>>>  1024/     2/    0%/  -11%/    0%/   +5%/  -31%
>>>  1024/     4/    0%/   -4%/   -7%/   +1%/  -22%
>>>  1024/     8/   -5%/   -6%/  -17%/  -29%/  -79%
>>>  2048/     1/    0%/  -16%/   +1%/   +9%/  -10%
>>>  2048/     2/    0%/  -12%/   +7%/   +9%/  -26%
>>>  2048/     4/    0%/   -7%/   -4%/   +3%/  -64%
>>>  2048/     8/   -1%/   -5%/   -6%/   +4%/  -20%
>>> 16384/     1/    0%/  -12%/  +11%/   +7%/  -20%
>>> 16384/     2/    0%/   -7%/   +1%/   +5%/  -26%
>>> 16384/     4/    0%/   -5%/  +12%/  +22%/  -23%
>>> 16384/     8/    0%/   -1%/   -8%/   +5%/   -3%
>>> size/session/+thu%/+normalize%/+tpkts%/+rpkts%/+ioexits%/
>>>     1/     1/   +9%/  -29%/   +9%/   +9%/   +9%
>>>     1/    25/   +6%/  -18%/   +6%/   +6%/   -1%
>>>     1/    50/   +6%/  -19%/   +5%/   +5%/   -2%
>>>     1/   100/   +5%/  -19%/   +4%/   +4%/   -3%
>>>    64/     1/  +10%/  -28%/  +10%/  +10%/  +10%
>>>    64/    25/   +8%/  -18%/   +7%/   +7%/   -2%
>>>    64/    50/   +8%/  -17%/   +8%/   +8%/   -1%
>>>    64/   100/   +8%/  -17%/   +8%/   +8%/   -1%
>>>   256/     1/  +10%/  -28%/  +10%/  +10%/  +10%
>>>   256/    25/  +15%/  -13%/  +15%/  +15%/    0%
>>>   256/    50/  +16%/  -14%/  +18%/  +18%/   +2%
>>>   256/   100/  +15%/  -13%/  +12%/  +12%/   -2%
>>>
>>> Changes from V2:
>>> - poll also at the end of rx handling
>>> - factor out the polling logic and optimize the code a little bit
>>> - add two ioctls to get and set the busy poll timeout
>>> - test on ixgbe (which can give more stable and reproducable numbers)
>>>   instead of mlx4.
>>>
>>> Changes from V1:
>>> - Add a comment for vhost_has_work() to explain why it could be
>>>   lockless
>>> - Add param description for busyloop_timeout
>>> - Split out the busy polling logic into a new helper
>>> - Check and exit the loop when there's a pending signal
>>> - Disable preemption during busy looping to make sure lock_clock() was
>>>   correctly used.
>>>
>>> Jason Wang (3):
>>>   vhost: introduce vhost_has_work()
>>>   vhost: introduce vhost_vq_more_avail()
>>>   vhost_net: basic polling support
>>>
>>>  drivers/vhost/net.c        | 77 +++++++++++++++++++++++++++++++++++++++++++---
>>>  drivers/vhost/vhost.c      | 48 +++++++++++++++++++++++------
>>>  drivers/vhost/vhost.h      |  3 ++
>>>  include/uapi/linux/vhost.h | 11 +++++++
>>>  4 files changed, 125 insertions(+), 14 deletions(-)
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> N�����r��y���b�X��ǧv�^�)޺{.n�+����{����zX��\x17��ܨ}���Ơz�&j:+v���\a����zZ+��+zf���h���~����i���z�\x1e�w���?����&�)ߢ^[f��^jǫy�m��@A�a��\x7f�\f0��h�\x0f�i\x7f

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply

* Re: [PATCH] vhost: move is_le setup to the backend
From: Greg Kurz @ 2015-11-12 14:28 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20151112154610-mutt-send-email-mst@redhat.com>

On Thu, 12 Nov 2015 15:46:30 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Fri, Oct 30, 2015 at 12:42:35PM +0100, Greg Kurz wrote:
> > The vq->is_le field is used to fix endianness when accessing the vring via
> > the cpu_to_vhost16() and vhost16_to_cpu() helpers in the following cases:
> > 
> > 1) host is big endian and device is modern virtio
> > 
> > 2) host has cross-endian support and device is legacy virtio with a different
> >    endianness than the host
> > 
> > Both cases rely on the VHOST_SET_FEATURES ioctl, but 2) also needs the
> > VHOST_SET_VRING_ENDIAN ioctl to be called by userspace. Since vq->is_le
> > is only needed when the backend is active, it was decided to set it at
> > backend start.
> > 
> > This is currently done in vhost_init_used()->vhost_init_is_le() but it
> > obfuscates the core vhost code. This patch moves the is_le setup to a
> > dedicated function that is called from the backend code.
> > 
> > Note vhost_net is the only backend that can pass vq->private_data == NULL to
> > vhost_init_used(), hence the "if (sock)" branch.
> > 
> > No behaviour change.
> > 
> > Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> 
> I plan to look at this next week, busy with QEMU 2.5 now.
> 

I don't have any deadline for this since this is only a cleanup tentative.

Thanks.

> > ---
> >  drivers/vhost/net.c   |    6 ++++++
> >  drivers/vhost/scsi.c  |    3 +++
> >  drivers/vhost/test.c  |    2 ++
> >  drivers/vhost/vhost.c |   12 +++++++-----
> >  drivers/vhost/vhost.h |    1 +
> >  5 files changed, 19 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> > index 9eda69e40678..d6319cb2664c 100644
> > --- a/drivers/vhost/net.c
> > +++ b/drivers/vhost/net.c
> > @@ -917,6 +917,12 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
> >  
> >  		vhost_net_disable_vq(n, vq);
> >  		vq->private_data = sock;
> > +
> > +		if (sock)
> > +			vhost_set_is_le(vq);
> > +		else
> > +			vq->is_le = virtio_legacy_is_little_endian();
> > +
> >  		r = vhost_init_used(vq);
> >  		if (r)
> >  			goto err_used;
> > diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> > index e25a23692822..e2644a301fa5 100644
> > --- a/drivers/vhost/scsi.c
> > +++ b/drivers/vhost/scsi.c
> > @@ -1276,6 +1276,9 @@ vhost_scsi_set_endpoint(struct vhost_scsi *vs,
> >  			vq = &vs->vqs[i].vq;
> >  			mutex_lock(&vq->mutex);
> >  			vq->private_data = vs_tpg;
> > +
> > +			vhost_set_is_le(vq);
> > +
> >  			vhost_init_used(vq);
> >  			mutex_unlock(&vq->mutex);
> >  		}
> > diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c
> > index f2882ac98726..b1c7df502211 100644
> > --- a/drivers/vhost/test.c
> > +++ b/drivers/vhost/test.c
> > @@ -196,6 +196,8 @@ static long vhost_test_run(struct vhost_test *n, int test)
> >  		oldpriv = vq->private_data;
> >  		vq->private_data = priv;
> >  
> > +		vhost_set_is_le(vq);
> > +
> >  		r = vhost_init_used(&n->vqs[index]);
> >  
> >  		mutex_unlock(&vq->mutex);
> > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > index eec2f11809ff..6be863dcbd13 100644
> > --- a/drivers/vhost/vhost.c
> > +++ b/drivers/vhost/vhost.c
> > @@ -113,6 +113,12 @@ static void vhost_init_is_le(struct vhost_virtqueue *vq)
> >  }
> >  #endif /* CONFIG_VHOST_CROSS_ENDIAN_LEGACY */
> >  
> > +void vhost_set_is_le(struct vhost_virtqueue *vq)
> > +{
> > +	vhost_init_is_le(vq);
> > +}
> > +EXPORT_SYMBOL_GPL(vhost_set_is_le);
> > +
> >  static void vhost_poll_func(struct file *file, wait_queue_head_t *wqh,
> >  			    poll_table *pt)
> >  {
> > @@ -1156,12 +1162,8 @@ int vhost_init_used(struct vhost_virtqueue *vq)
> >  {
> >  	__virtio16 last_used_idx;
> >  	int r;
> > -	if (!vq->private_data) {
> > -		vq->is_le = virtio_legacy_is_little_endian();
> > +	if (!vq->private_data)
> >  		return 0;
> > -	}
> > -
> > -	vhost_init_is_le(vq);
> >  
> >  	r = vhost_update_used_flags(vq);
> >  	if (r)
> > diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> > index 4772862b71a7..8a62041959fe 100644
> > --- a/drivers/vhost/vhost.h
> > +++ b/drivers/vhost/vhost.h
> > @@ -162,6 +162,7 @@ bool vhost_enable_notify(struct vhost_dev *, struct vhost_virtqueue *);
> >  
> >  int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
> >  		    unsigned int log_num, u64 len);
> > +void vhost_set_is_le(struct vhost_virtqueue *vq);
> >  
> >  #define vq_err(vq, fmt, ...) do {                                  \
> >  		pr_debug(pr_fmt(fmt), ##__VA_ARGS__);       \
> 

^ permalink raw reply

* Re: [PATCH] vhost: move is_le setup to the backend
From: Michael S. Tsirkin @ 2015-11-12 13:46 UTC (permalink / raw)
  To: Greg Kurz; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20151030114235.28847.63465.stgit@bahia.huguette.org>

On Fri, Oct 30, 2015 at 12:42:35PM +0100, Greg Kurz wrote:
> The vq->is_le field is used to fix endianness when accessing the vring via
> the cpu_to_vhost16() and vhost16_to_cpu() helpers in the following cases:
> 
> 1) host is big endian and device is modern virtio
> 
> 2) host has cross-endian support and device is legacy virtio with a different
>    endianness than the host
> 
> Both cases rely on the VHOST_SET_FEATURES ioctl, but 2) also needs the
> VHOST_SET_VRING_ENDIAN ioctl to be called by userspace. Since vq->is_le
> is only needed when the backend is active, it was decided to set it at
> backend start.
> 
> This is currently done in vhost_init_used()->vhost_init_is_le() but it
> obfuscates the core vhost code. This patch moves the is_le setup to a
> dedicated function that is called from the backend code.
> 
> Note vhost_net is the only backend that can pass vq->private_data == NULL to
> vhost_init_used(), hence the "if (sock)" branch.
> 
> No behaviour change.
> 
> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>

I plan to look at this next week, busy with QEMU 2.5 now.

> ---
>  drivers/vhost/net.c   |    6 ++++++
>  drivers/vhost/scsi.c  |    3 +++
>  drivers/vhost/test.c  |    2 ++
>  drivers/vhost/vhost.c |   12 +++++++-----
>  drivers/vhost/vhost.h |    1 +
>  5 files changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 9eda69e40678..d6319cb2664c 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -917,6 +917,12 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
>  
>  		vhost_net_disable_vq(n, vq);
>  		vq->private_data = sock;
> +
> +		if (sock)
> +			vhost_set_is_le(vq);
> +		else
> +			vq->is_le = virtio_legacy_is_little_endian();
> +
>  		r = vhost_init_used(vq);
>  		if (r)
>  			goto err_used;
> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> index e25a23692822..e2644a301fa5 100644
> --- a/drivers/vhost/scsi.c
> +++ b/drivers/vhost/scsi.c
> @@ -1276,6 +1276,9 @@ vhost_scsi_set_endpoint(struct vhost_scsi *vs,
>  			vq = &vs->vqs[i].vq;
>  			mutex_lock(&vq->mutex);
>  			vq->private_data = vs_tpg;
> +
> +			vhost_set_is_le(vq);
> +
>  			vhost_init_used(vq);
>  			mutex_unlock(&vq->mutex);
>  		}
> diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c
> index f2882ac98726..b1c7df502211 100644
> --- a/drivers/vhost/test.c
> +++ b/drivers/vhost/test.c
> @@ -196,6 +196,8 @@ static long vhost_test_run(struct vhost_test *n, int test)
>  		oldpriv = vq->private_data;
>  		vq->private_data = priv;
>  
> +		vhost_set_is_le(vq);
> +
>  		r = vhost_init_used(&n->vqs[index]);
>  
>  		mutex_unlock(&vq->mutex);
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index eec2f11809ff..6be863dcbd13 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -113,6 +113,12 @@ static void vhost_init_is_le(struct vhost_virtqueue *vq)
>  }
>  #endif /* CONFIG_VHOST_CROSS_ENDIAN_LEGACY */
>  
> +void vhost_set_is_le(struct vhost_virtqueue *vq)
> +{
> +	vhost_init_is_le(vq);
> +}
> +EXPORT_SYMBOL_GPL(vhost_set_is_le);
> +
>  static void vhost_poll_func(struct file *file, wait_queue_head_t *wqh,
>  			    poll_table *pt)
>  {
> @@ -1156,12 +1162,8 @@ int vhost_init_used(struct vhost_virtqueue *vq)
>  {
>  	__virtio16 last_used_idx;
>  	int r;
> -	if (!vq->private_data) {
> -		vq->is_le = virtio_legacy_is_little_endian();
> +	if (!vq->private_data)
>  		return 0;
> -	}
> -
> -	vhost_init_is_le(vq);
>  
>  	r = vhost_update_used_flags(vq);
>  	if (r)
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index 4772862b71a7..8a62041959fe 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -162,6 +162,7 @@ bool vhost_enable_notify(struct vhost_dev *, struct vhost_virtqueue *);
>  
>  int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
>  		    unsigned int log_num, u64 len);
> +void vhost_set_is_le(struct vhost_virtqueue *vq);
>  
>  #define vq_err(vq, fmt, ...) do {                                  \
>  		pr_debug(pr_fmt(fmt), ##__VA_ARGS__);       \

^ permalink raw reply

* Re: [PATCH] vhost: move is_le setup to the backend
From: Cornelia Huck @ 2015-11-12 12:37 UTC (permalink / raw)
  To: Greg Kurz; +Cc: netdev, virtualization, linux-kernel, kvm, Michael S. Tsirkin
In-Reply-To: <20151030114235.28847.63465.stgit@bahia.huguette.org>

On Fri, 30 Oct 2015 12:42:35 +0100
Greg Kurz <gkurz@linux.vnet.ibm.com> wrote:

> The vq->is_le field is used to fix endianness when accessing the vring via
> the cpu_to_vhost16() and vhost16_to_cpu() helpers in the following cases:
> 
> 1) host is big endian and device is modern virtio
> 
> 2) host has cross-endian support and device is legacy virtio with a different
>    endianness than the host
> 
> Both cases rely on the VHOST_SET_FEATURES ioctl, but 2) also needs the
> VHOST_SET_VRING_ENDIAN ioctl to be called by userspace. Since vq->is_le
> is only needed when the backend is active, it was decided to set it at
> backend start.
> 
> This is currently done in vhost_init_used()->vhost_init_is_le() but it
> obfuscates the core vhost code. This patch moves the is_le setup to a
> dedicated function that is called from the backend code.
> 
> Note vhost_net is the only backend that can pass vq->private_data == NULL to
> vhost_init_used(), hence the "if (sock)" branch.
> 
> No behaviour change.
> 
> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> ---
>  drivers/vhost/net.c   |    6 ++++++
>  drivers/vhost/scsi.c  |    3 +++
>  drivers/vhost/test.c  |    2 ++
>  drivers/vhost/vhost.c |   12 +++++++-----
>  drivers/vhost/vhost.h |    1 +
>  5 files changed, 19 insertions(+), 5 deletions(-)

Makes sense.

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

^ permalink raw reply

* Re: [PATCH v3 0/3] virtio DMA API core stuff
From: David Woodhouse @ 2015-11-12 12:18 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-s390, KVM, Benjamin Herrenschmidt, Sebastian Ott,
	linux-kernel@vger.kernel.org, Andy Lutomirski,
	Christian Borntraeger, Joerg Roedel, Martin Schwidefsky,
	Paolo Bonzini, Linux Virtualization, Christoph Hellwig
In-Reply-To: <20151112124048-mutt-send-email-mst@redhat.com>


[-- Attachment #1.1: Type: text/plain, Size: 4543 bytes --]

On Thu, 2015-11-12 at 13:09 +0200, Michael S. Tsirkin wrote:
> On Wed, Nov 11, 2015 at 11:30:27PM +0100, David Woodhouse wrote:
> > 
> > If the IOMMU is exposed, and enabled, and telling the guest kernel that
> > it *does* cover the virtio devices, then those virtio devices will
> > *not* be in passthrough mode.
> 
> This we need to fix. Because in most configurations if you are
> using kernel drivers, then you don't want IOMMU with virtio,
> but if you are using VFIO then you do.

This is *absolutely* not specific to virtio. There are *plenty* of
other users (especially networking) where we only really care about the
existence of the IOMMU for VFIO purposes and assigning devices to
guests, and we are willing to dispense with the protection that it
offers for native in-kernel drivers. For that, boot with iommu=pt.

There is no way, currently, to enable the passthrough mode on a per-
device basis. Although it has been discussed right here, very recently.

Let's not conflate those issues.

> > You choosing to use the DMA API in the virtio device drivers instead of
> > being buggy, has nothing to do with whether it's actually in
> > passthrough mode or not. Whether it's in passthrough mode or not, using
> > the DMA API is technically the right thing to do — because it should
> > either *do* the translation, or return a 1:1 mapped IOVA, as
> > appropriate.
> 
> Right but first we need to actually make DMA API do the right thing
> at least on x86,ppc and arm.

It already does the right thing on x86, modulo BIOS bugs (including the
qemu ACPI table but that you said you're not too worried about).

> I'm not worried about qemu bugs that much.  I am interested in being
> able to use both VFIO and kernel drivers with virtio devices with good
> performance and without tweaking kernel parameters.

OK, then you are interested in the semi-orthogonal discussion about
DMA_ATTR_IOMMU_BYPASS. Either way, device drivers SHALL use the DMA
API.


> > Having said that, if this were real hardware I'd just be blacklisting
> > it and saying "Another BIOS with broken DMAR tables --> IOMMU
> > completely disabled". So perhaps we should just do that.
> > 
> Yes, once there is new QEMU where virtio is covered by the IOMMU,
> that would be one way to address existing QEMU bugs. 

No, that's not required. All that's required is to fix the currently-
broken ACPI table so that it *admits* that the virtio devices aren't
covered by the IOMMU. And I've never waited for a fix to be available
before, before blacklisting *other* broken firmwares...

The only reason I'm holding off for now is because ARM and PPC also
need a quirk for their platform code to realise that certain devices
actually *aren't* covered by the IOMMU, and I might be able to just use
the same thing and still enable the IOMMU in the offending qemu
versions.

Although as noted, it would need to cover assigned devices as well as
virtio — qemu currently lies to us and tells us that the emulated IOMMU
in the guest does cover *those* too.

> With virt, each device can have different priveledges:
> some are part of hypervisor so with a kernel driver
> trying to get protection from them using an IOMMU which is also
> part of hypervisor makes no sense
>  - but when using a
> userspace driver then getting protection from the userspace
> driver does make sense. Others are real devices so
> getting protection from them makes some sense.
> 
> Which is which? It's easiest for the device driver itself to
> gain that knowledge. Please note this is *not* the same
> question as whether a specific device is covered by an IOMMU.

OK. How does your device driver know whether the virtio PCI device it's
talking to is actually implemented by the hypervisor, or whether it's
one of the real PCI implementations that apparently exist?

> Linux doesn't seem to support that usecase at the moment, if this is a
> generic problem then we need to teach Linux to solve it, but if virtio
> is unique in this requirement, then we should just keep doing virtio
> specific things to solve it.

It is a generic problem. There is a discussion elsewhere about how (or
indeed whether) to solve it. It absolutely isn't virtio-specific, and
we absolutely shouldn't be doing virtio-specific things to solve it.

Nothing excuses just eschewing the correct DMA API. That's just broken,
and only ever worked in conjunction with *other* bugs elsewhere in the
platform.


-- 
dwmw2


[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5691 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply

* Re: [PATCH net-next RFC V3 0/3] basic busy polling support for vhost_net
From: Felipe Franciosi @ 2015-11-12 12:02 UTC (permalink / raw)
  To: Jason Wang
  Cc: netdev@vger.kernel.org, virtualization@lists.linux-foundation.org,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org, mst@redhat.com
In-Reply-To: <564467E9.9080409@redhat.com>

Hi Jason,

I understand your busy loop timeout is quite conservative at 50us. Did you try any other values?

Also, did you measure how polling affects many VMs talking to each other (e.g. 20 VMs on each host, perhaps with several vNICs each, transmitting to a corresponding VM/vNIC pair on another host)?


On a complete separate experiment (busy waiting on storage I/O rings on Xen), I have observed that bigger timeouts gave bigger benefits. On the other hand, all cases that contended for CPU were badly hurt with any sort of polling.

The cases that contended for CPU consisted of many VMs generating workload over very fast I/O devices (in that case, several NVMe devices on a single host). And the metric that got affected was aggregate throughput from all VMs.

The solution was to determine whether to poll depending on the host's overall CPU utilisation at that moment. That gave me the best of both worlds as polling made everything faster without slowing down any other metric.

Thanks,
Felipe



On 12/11/2015 10:20, "kvm-owner@vger.kernel.org on behalf of Jason Wang" <kvm-owner@vger.kernel.org on behalf of jasowang@redhat.com> wrote:

>
>
>On 11/12/2015 06:16 PM, Jason Wang wrote:
>> Hi all:
>>
>> This series tries to add basic busy polling for vhost net. The idea is
>> simple: at the end of tx/rx processing, busy polling for new tx added
>> descriptor and rx receive socket for a while. The maximum number of
>> time (in us) could be spent on busy polling was specified ioctl.
>>
>> Test were done through:
>>
>> - 50 us as busy loop timeout
>> - Netperf 2.6
>> - Two machines with back to back connected ixgbe
>> - Guest with 1 vcpu and 1 queue
>>
>> Results:
>> - For stream workload, ioexits were reduced dramatically in medium
>>   size (1024-2048) of tx (at most -39%) and almost all rx (at most
>>   -79%) as a result of polling. This compensate for the possible
>>   wasted cpu cycles more or less. That porbably why we can still see
>>   some increasing in the normalized throughput in some cases.
>> - Throughput of tx were increased (at most 105%) expect for the huge
>>   write (16384). And we can send more packets in the case (+tpkts were
>>   increased).
>> - Very minor rx regression in some cases.
>> - Improvemnt on TCP_RR (at most 16%).
>
>Forget to mention, the following test results by order are:
>
>1) Guest TX
>2) Guest RX
>3) TCP_RR
>
>> size/session/+thu%/+normalize%/+tpkts%/+rpkts%/+ioexits%/
>>    64/     1/   +9%/  -17%/   +5%/  +10%/   -2%
>>    64/     2/   +8%/  -18%/   +6%/  +10%/   -1%
>>    64/     4/   +4%/  -21%/   +6%/  +10%/   -1%
>>    64/     8/   +9%/  -17%/   +6%/   +9%/   -2%
>>   256/     1/  +20%/   -1%/  +15%/  +11%/   -9%
>>   256/     2/  +15%/   -6%/  +15%/   +8%/   -8%
>>   256/     4/  +17%/   -4%/  +16%/   +8%/   -8%
>>   256/     8/  -61%/  -69%/  +16%/  +10%/  -10%
>>   512/     1/  +15%/   -3%/  +19%/  +18%/  -11%
>>   512/     2/  +19%/    0%/  +19%/  +13%/  -10%
>>   512/     4/  +18%/   -2%/  +18%/  +15%/  -10%
>>   512/     8/  +17%/   -1%/  +18%/  +15%/  -11%
>>  1024/     1/  +25%/   +4%/  +27%/  +16%/  -21%
>>  1024/     2/  +28%/   +8%/  +25%/  +15%/  -22%
>>  1024/     4/  +25%/   +5%/  +25%/  +14%/  -21%
>>  1024/     8/  +27%/   +7%/  +25%/  +16%/  -21%
>>  2048/     1/  +32%/  +12%/  +31%/  +22%/  -38%
>>  2048/     2/  +33%/  +12%/  +30%/  +23%/  -36%
>>  2048/     4/  +31%/  +10%/  +31%/  +24%/  -37%
>>  2048/     8/ +105%/  +75%/  +33%/  +23%/  -39%
>> 16384/     1/    0%/  -14%/   +2%/    0%/  +19%
>> 16384/     2/    0%/  -13%/  +19%/  -13%/  +17%
>> 16384/     4/    0%/  -12%/   +3%/    0%/   +2%
>> 16384/     8/    0%/  -11%/   -2%/   +1%/   +1%
>> size/session/+thu%/+normalize%/+tpkts%/+rpkts%/+ioexits%/
>>    64/     1/   -7%/  -23%/   +4%/   +6%/  -74%
>>    64/     2/   -2%/  -12%/   +2%/   +2%/  -55%
>>    64/     4/   +2%/   -5%/  +10%/   -2%/  -43%
>>    64/     8/   -5%/   -5%/  +11%/  -34%/  -59%
>>   256/     1/   -6%/  -16%/   +9%/  +11%/  -60%
>>   256/     2/   +3%/   -4%/   +6%/   -3%/  -28%
>>   256/     4/    0%/   -5%/   -9%/   -9%/  -10%
>>   256/     8/   -3%/   -6%/  -12%/   -9%/  -40%
>>   512/     1/   -4%/  -17%/  -10%/  +21%/  -34%
>>   512/     2/    0%/   -9%/  -14%/   -3%/  -30%
>>   512/     4/    0%/   -4%/  -18%/  -12%/   -4%
>>   512/     8/   -1%/   -4%/   -1%/   -5%/   +4%
>>  1024/     1/    0%/  -16%/  +12%/  +11%/  -10%
>>  1024/     2/    0%/  -11%/    0%/   +5%/  -31%
>>  1024/     4/    0%/   -4%/   -7%/   +1%/  -22%
>>  1024/     8/   -5%/   -6%/  -17%/  -29%/  -79%
>>  2048/     1/    0%/  -16%/   +1%/   +9%/  -10%
>>  2048/     2/    0%/  -12%/   +7%/   +9%/  -26%
>>  2048/     4/    0%/   -7%/   -4%/   +3%/  -64%
>>  2048/     8/   -1%/   -5%/   -6%/   +4%/  -20%
>> 16384/     1/    0%/  -12%/  +11%/   +7%/  -20%
>> 16384/     2/    0%/   -7%/   +1%/   +5%/  -26%
>> 16384/     4/    0%/   -5%/  +12%/  +22%/  -23%
>> 16384/     8/    0%/   -1%/   -8%/   +5%/   -3%
>> size/session/+thu%/+normalize%/+tpkts%/+rpkts%/+ioexits%/
>>     1/     1/   +9%/  -29%/   +9%/   +9%/   +9%
>>     1/    25/   +6%/  -18%/   +6%/   +6%/   -1%
>>     1/    50/   +6%/  -19%/   +5%/   +5%/   -2%
>>     1/   100/   +5%/  -19%/   +4%/   +4%/   -3%
>>    64/     1/  +10%/  -28%/  +10%/  +10%/  +10%
>>    64/    25/   +8%/  -18%/   +7%/   +7%/   -2%
>>    64/    50/   +8%/  -17%/   +8%/   +8%/   -1%
>>    64/   100/   +8%/  -17%/   +8%/   +8%/   -1%
>>   256/     1/  +10%/  -28%/  +10%/  +10%/  +10%
>>   256/    25/  +15%/  -13%/  +15%/  +15%/    0%
>>   256/    50/  +16%/  -14%/  +18%/  +18%/   +2%
>>   256/   100/  +15%/  -13%/  +12%/  +12%/   -2%
>>
>> Changes from V2:
>> - poll also at the end of rx handling
>> - factor out the polling logic and optimize the code a little bit
>> - add two ioctls to get and set the busy poll timeout
>> - test on ixgbe (which can give more stable and reproducable numbers)
>>   instead of mlx4.
>>
>> Changes from V1:
>> - Add a comment for vhost_has_work() to explain why it could be
>>   lockless
>> - Add param description for busyloop_timeout
>> - Split out the busy polling logic into a new helper
>> - Check and exit the loop when there's a pending signal
>> - Disable preemption during busy looping to make sure lock_clock() was
>>   correctly used.
>>
>> Jason Wang (3):
>>   vhost: introduce vhost_has_work()
>>   vhost: introduce vhost_vq_more_avail()
>>   vhost_net: basic polling support
>>
>>  drivers/vhost/net.c        | 77 +++++++++++++++++++++++++++++++++++++++++++---
>>  drivers/vhost/vhost.c      | 48 +++++++++++++++++++++++------
>>  drivers/vhost/vhost.h      |  3 ++
>>  include/uapi/linux/vhost.h | 11 +++++++
>>  4 files changed, 125 insertions(+), 14 deletions(-)
>>
>
>--
>To unsubscribe from this list: send the line "unsubscribe kvm" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v3 0/3] virtio DMA API core stuff
From: Michael S. Tsirkin @ 2015-11-12 11:09 UTC (permalink / raw)
  To: David Woodhouse
  Cc: linux-s390, KVM, Benjamin Herrenschmidt, Sebastian Ott,
	linux-kernel@vger.kernel.org, Andy Lutomirski,
	Christian Borntraeger, Joerg Roedel, Martin Schwidefsky,
	Paolo Bonzini, Linux Virtualization, Christoph Hellwig
In-Reply-To: <1447281027.3513.11.camel@infradead.org>

On Wed, Nov 11, 2015 at 11:30:27PM +0100, David Woodhouse wrote:
> On Wed, 2015-11-11 at 07:56 -0800, Andy Lutomirski wrote:
> > 
> > Can you flesh out this trick?
> > 
> > On x86 IIUC the IOMMU more-or-less defaults to passthrough.  If the
> > kernel wants, it can switch it to a non-passthrough mode.  My patches
> > cause the virtio driver to do exactly this, except that the host
> > implementation doesn't actually exist yet, so the patches will instead
> > have no particular effect.
> 
> At some level, yes — we're compatible with a 1982 IBM PC and thus the
> IOMMU is entirely disabled at boot until the kernel turns it on —
> except in TXT mode where we abandon that compatibility.
> 
> But no, the virtio driver has *nothing* to do with switching the device
> out of passthrough mode. It is either in passthrough mode, or it isn't.
> 
> If the VMM *doesn't* expose an IOMMU to the guest, obviously the
> devices are in passthrough mode. If the guest kernel doesn't have IOMMU
> support enabled, then obviously the devices are in passthrough mode.
> And if the ACPI tables exposed to the guest kernel *tell* it that the
> virtio devices are not actually behind the IOMMU (which qemu gets
> wrong), then it'll be in passthrough mode.
> 
> If the IOMMU is exposed, and enabled, and telling the guest kernel that
> it *does* cover the virtio devices, then those virtio devices will
> *not* be in passthrough mode.

This we need to fix. Because in most configurations if you are
using kernel drivers, then you don't want IOMMU with virtio,
but if you are using VFIO then you do.

Intel's iommu can be programmed to still
do a kind of passthrough (1:1) mapping, it's
just a matter of doing this for virtio devices
when not using VFIO.

> You choosing to use the DMA API in the virtio device drivers instead of
> being buggy, has nothing to do with whether it's actually in
> passthrough mode or not. Whether it's in passthrough mode or not, using
> the DMA API is technically the right thing to do — because it should
> either *do* the translation, or return a 1:1 mapped IOVA, as
> appropriate.

Right but first we need to actually make DMA API do the right thing
at least on x86,ppc and arm.

> > On powerpc and sparc, we *already* screwed up.  The host already tells
> > the guest that there's an IOMMU and that it's *enabled* because those
> > platforms don't have selective IOMMU coverage the way that x86 does.
> > So we need to work around it.
> 
> No, we need it on x86 too because once we fix the virtio device driver
> bug and make it start using the DMA API, then we start to trip up on
> the qemu bug where it lies about which devices are covered by the
> IOMMU.
> 
> Of course, we still have that same qemu bug w.r.t. assigned devices,
> which it *also* claims are behind its IOMMU when they're not...

I'm not worried about qemu bugs that much.  I am interested in being
able to use both VFIO and kernel drivers with virtio devices with good
performance and without tweaking kernel parameters.


> > I think that, if we want fancy virt-friendly IOMMU stuff like you're
> > talking about, then the right thing to do is to create a virtio bus
> > instead of pretending to be PCI.  That bus could have a virtio IOMMU
> > and its own cross-platform enumeration mechanism for devices on the
> > bus, and everything would be peachy.
> 
> That doesn't really help very much for the x86 case where the problem
> is compatibility with *existing* (arguably broken) qemu
> implementations.
> 
> Having said that, if this were real hardware I'd just be blacklisting
> it and saying "Another BIOS with broken DMAR tables --> IOMMU
> completely disabled". So perhaps we should just do that.
> 

Yes, once there is new QEMU where virtio is covered by the IOMMU,
that would be one way to address existing QEMU bugs. 

> > I still don't understand what trick.  If we want virtio devices to be
> > assignable, then they should be translated through the IOMMU, and the
> > DMA API is the right interface for that.
> 
> The DMA API is the right interface *regardless* of whether there's
> actual translation to be done. The device driver itself should not be
> involved in any way with that decision.

With virt, each device can have different priveledges:
some are part of hypervisor so with a kernel driver
trying to get protection from them using an IOMMU which is also
part of hypervisor makes no sense - but when using a
userspace driver then getting protection from the userspace
driver does make sense. Others are real devices so
getting protection from them makes some sense.

Which is which? It's easiest for the device driver itself to
gain that knowledge. Please note this is *not* the same
question as whether a specific device is covered by an IOMMU.

> When you want to access MMIO, you use ioremap() and writel() instead of
> doing random crap for yourself. When you want DMA, you use the DMA API
> to get a bus address for your device *even* if you expect there to be
> no IOMMU and you expect it to precisely match the physical address. No
> excuses.

No problem, but the fact remains that virtio does need
per-device control over whether it's passthrough or not.

Forget the bugs, that's not the issue - the issue is
that it's sometimes part of hypervisor and
sometimes isn't.

We just can't say it's always not a part of hypervisor so you always
want maximum protection - that drops performance by to the floor.

Linux doesn't seem to support that usecase at the moment, if this is a
generic problem then we need to teach Linux to solve it, but if virtio
is unique in this requirement, then we should just keep doing virtio
specific things to solve it.


> -- 
> dwmw2
> 
> 


_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply

* Re: [PATCH net-next RFC V3 0/3] basic busy polling support for vhost_net
From: Jason Wang @ 2015-11-12 10:20 UTC (permalink / raw)
  To: mst, kvm, virtualization, netdev, linux-kernel
In-Reply-To: <1447323395-28052-1-git-send-email-jasowang@redhat.com>



On 11/12/2015 06:16 PM, Jason Wang wrote:
> Hi all:
>
> This series tries to add basic busy polling for vhost net. The idea is
> simple: at the end of tx/rx processing, busy polling for new tx added
> descriptor and rx receive socket for a while. The maximum number of
> time (in us) could be spent on busy polling was specified ioctl.
>
> Test were done through:
>
> - 50 us as busy loop timeout
> - Netperf 2.6
> - Two machines with back to back connected ixgbe
> - Guest with 1 vcpu and 1 queue
>
> Results:
> - For stream workload, ioexits were reduced dramatically in medium
>   size (1024-2048) of tx (at most -39%) and almost all rx (at most
>   -79%) as a result of polling. This compensate for the possible
>   wasted cpu cycles more or less. That porbably why we can still see
>   some increasing in the normalized throughput in some cases.
> - Throughput of tx were increased (at most 105%) expect for the huge
>   write (16384). And we can send more packets in the case (+tpkts were
>   increased).
> - Very minor rx regression in some cases.
> - Improvemnt on TCP_RR (at most 16%).

Forget to mention, the following test results by order are:

1) Guest TX
2) Guest RX
3) TCP_RR

> size/session/+thu%/+normalize%/+tpkts%/+rpkts%/+ioexits%/
>    64/     1/   +9%/  -17%/   +5%/  +10%/   -2%
>    64/     2/   +8%/  -18%/   +6%/  +10%/   -1%
>    64/     4/   +4%/  -21%/   +6%/  +10%/   -1%
>    64/     8/   +9%/  -17%/   +6%/   +9%/   -2%
>   256/     1/  +20%/   -1%/  +15%/  +11%/   -9%
>   256/     2/  +15%/   -6%/  +15%/   +8%/   -8%
>   256/     4/  +17%/   -4%/  +16%/   +8%/   -8%
>   256/     8/  -61%/  -69%/  +16%/  +10%/  -10%
>   512/     1/  +15%/   -3%/  +19%/  +18%/  -11%
>   512/     2/  +19%/    0%/  +19%/  +13%/  -10%
>   512/     4/  +18%/   -2%/  +18%/  +15%/  -10%
>   512/     8/  +17%/   -1%/  +18%/  +15%/  -11%
>  1024/     1/  +25%/   +4%/  +27%/  +16%/  -21%
>  1024/     2/  +28%/   +8%/  +25%/  +15%/  -22%
>  1024/     4/  +25%/   +5%/  +25%/  +14%/  -21%
>  1024/     8/  +27%/   +7%/  +25%/  +16%/  -21%
>  2048/     1/  +32%/  +12%/  +31%/  +22%/  -38%
>  2048/     2/  +33%/  +12%/  +30%/  +23%/  -36%
>  2048/     4/  +31%/  +10%/  +31%/  +24%/  -37%
>  2048/     8/ +105%/  +75%/  +33%/  +23%/  -39%
> 16384/     1/    0%/  -14%/   +2%/    0%/  +19%
> 16384/     2/    0%/  -13%/  +19%/  -13%/  +17%
> 16384/     4/    0%/  -12%/   +3%/    0%/   +2%
> 16384/     8/    0%/  -11%/   -2%/   +1%/   +1%
> size/session/+thu%/+normalize%/+tpkts%/+rpkts%/+ioexits%/
>    64/     1/   -7%/  -23%/   +4%/   +6%/  -74%
>    64/     2/   -2%/  -12%/   +2%/   +2%/  -55%
>    64/     4/   +2%/   -5%/  +10%/   -2%/  -43%
>    64/     8/   -5%/   -5%/  +11%/  -34%/  -59%
>   256/     1/   -6%/  -16%/   +9%/  +11%/  -60%
>   256/     2/   +3%/   -4%/   +6%/   -3%/  -28%
>   256/     4/    0%/   -5%/   -9%/   -9%/  -10%
>   256/     8/   -3%/   -6%/  -12%/   -9%/  -40%
>   512/     1/   -4%/  -17%/  -10%/  +21%/  -34%
>   512/     2/    0%/   -9%/  -14%/   -3%/  -30%
>   512/     4/    0%/   -4%/  -18%/  -12%/   -4%
>   512/     8/   -1%/   -4%/   -1%/   -5%/   +4%
>  1024/     1/    0%/  -16%/  +12%/  +11%/  -10%
>  1024/     2/    0%/  -11%/    0%/   +5%/  -31%
>  1024/     4/    0%/   -4%/   -7%/   +1%/  -22%
>  1024/     8/   -5%/   -6%/  -17%/  -29%/  -79%
>  2048/     1/    0%/  -16%/   +1%/   +9%/  -10%
>  2048/     2/    0%/  -12%/   +7%/   +9%/  -26%
>  2048/     4/    0%/   -7%/   -4%/   +3%/  -64%
>  2048/     8/   -1%/   -5%/   -6%/   +4%/  -20%
> 16384/     1/    0%/  -12%/  +11%/   +7%/  -20%
> 16384/     2/    0%/   -7%/   +1%/   +5%/  -26%
> 16384/     4/    0%/   -5%/  +12%/  +22%/  -23%
> 16384/     8/    0%/   -1%/   -8%/   +5%/   -3%
> size/session/+thu%/+normalize%/+tpkts%/+rpkts%/+ioexits%/
>     1/     1/   +9%/  -29%/   +9%/   +9%/   +9%
>     1/    25/   +6%/  -18%/   +6%/   +6%/   -1%
>     1/    50/   +6%/  -19%/   +5%/   +5%/   -2%
>     1/   100/   +5%/  -19%/   +4%/   +4%/   -3%
>    64/     1/  +10%/  -28%/  +10%/  +10%/  +10%
>    64/    25/   +8%/  -18%/   +7%/   +7%/   -2%
>    64/    50/   +8%/  -17%/   +8%/   +8%/   -1%
>    64/   100/   +8%/  -17%/   +8%/   +8%/   -1%
>   256/     1/  +10%/  -28%/  +10%/  +10%/  +10%
>   256/    25/  +15%/  -13%/  +15%/  +15%/    0%
>   256/    50/  +16%/  -14%/  +18%/  +18%/   +2%
>   256/   100/  +15%/  -13%/  +12%/  +12%/   -2%
>
> Changes from V2:
> - poll also at the end of rx handling
> - factor out the polling logic and optimize the code a little bit
> - add two ioctls to get and set the busy poll timeout
> - test on ixgbe (which can give more stable and reproducable numbers)
>   instead of mlx4.
>
> Changes from V1:
> - Add a comment for vhost_has_work() to explain why it could be
>   lockless
> - Add param description for busyloop_timeout
> - Split out the busy polling logic into a new helper
> - Check and exit the loop when there's a pending signal
> - Disable preemption during busy looping to make sure lock_clock() was
>   correctly used.
>
> Jason Wang (3):
>   vhost: introduce vhost_has_work()
>   vhost: introduce vhost_vq_more_avail()
>   vhost_net: basic polling support
>
>  drivers/vhost/net.c        | 77 +++++++++++++++++++++++++++++++++++++++++++---
>  drivers/vhost/vhost.c      | 48 +++++++++++++++++++++++------
>  drivers/vhost/vhost.h      |  3 ++
>  include/uapi/linux/vhost.h | 11 +++++++
>  4 files changed, 125 insertions(+), 14 deletions(-)
>

^ permalink raw reply

* [PATCH net-next RFC V3 3/3] vhost_net: basic polling support
From: Jason Wang @ 2015-11-12 10:16 UTC (permalink / raw)
  To: mst, kvm, virtualization, netdev, linux-kernel
In-Reply-To: <1447323395-28052-1-git-send-email-jasowang@redhat.com>

This patch tries to poll for new added tx buffer or socket receive
queue for a while at the end of tx/rx processing. The maximum time
spent on polling were specified through a new kind of vring ioctl.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/vhost/net.c        | 77 +++++++++++++++++++++++++++++++++++++++++++---
 drivers/vhost/vhost.c      | 15 +++++++++
 drivers/vhost/vhost.h      |  1 +
 include/uapi/linux/vhost.h | 11 +++++++
 4 files changed, 99 insertions(+), 5 deletions(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 9eda69e..a38fa32 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -287,6 +287,45 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success)
 	rcu_read_unlock_bh();
 }
 
+static inline unsigned long busy_clock(void)
+{
+	return local_clock() >> 10;
+}
+
+static bool vhost_can_busy_poll(struct vhost_dev *dev,
+				unsigned long endtime)
+{
+	return likely(!need_resched()) &&
+	       likely(!time_after(busy_clock(), endtime)) &&
+	       likely(!signal_pending(current)) &&
+	       !vhost_has_work(dev) &&
+	       single_task_running();
+}
+
+static int vhost_net_tx_get_vq_desc(struct vhost_net *net,
+				    struct vhost_virtqueue *vq,
+				    struct iovec iov[], unsigned int iov_size,
+				    unsigned int *out_num, unsigned int *in_num)
+{
+	unsigned long uninitialized_var(endtime);
+
+	if (vq->busyloop_timeout) {
+		preempt_disable();
+		endtime = busy_clock() + vq->busyloop_timeout;
+	}
+
+	while (vq->busyloop_timeout &&
+	       vhost_can_busy_poll(vq->dev, endtime) &&
+	       !vhost_vq_more_avail(vq->dev, vq))
+		cpu_relax();
+
+	if (vq->busyloop_timeout)
+		preempt_enable();
+
+	return vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
+				 out_num, in_num, NULL, NULL);
+}
+
 /* Expects to be always run from workqueue - which acts as
  * read-size critical section for our kind of RCU. */
 static void handle_tx(struct vhost_net *net)
@@ -331,10 +370,9 @@ static void handle_tx(struct vhost_net *net)
 			      % UIO_MAXIOV == nvq->done_idx))
 			break;
 
-		head = vhost_get_vq_desc(vq, vq->iov,
-					 ARRAY_SIZE(vq->iov),
-					 &out, &in,
-					 NULL, NULL);
+		head = vhost_net_tx_get_vq_desc(net, vq, vq->iov,
+						ARRAY_SIZE(vq->iov),
+						&out, &in);
 		/* On error, stop handling until the next kick. */
 		if (unlikely(head < 0))
 			break;
@@ -435,6 +473,35 @@ static int peek_head_len(struct sock *sk)
 	return len;
 }
 
+static int vhost_net_peek_head_len(struct vhost_net *net, struct sock *sk)
+{
+	struct vhost_net_virtqueue *nvq = &net->vqs[VHOST_NET_VQ_TX];
+	struct vhost_virtqueue *vq = &nvq->vq;
+	unsigned long uninitialized_var(endtime);
+
+	if (vq->busyloop_timeout) {
+		mutex_lock(&vq->mutex);
+		vhost_disable_notify(&net->dev, vq);
+		preempt_disable();
+		endtime = busy_clock() + vq->busyloop_timeout;
+	}
+
+	while (vq->busyloop_timeout &&
+	       vhost_can_busy_poll(&net->dev, endtime) &&
+	       skb_queue_empty(&sk->sk_receive_queue) &&
+	       !vhost_vq_more_avail(&net->dev, vq))
+		cpu_relax();
+
+	if (vq->busyloop_timeout) {
+		preempt_enable();
+		if (vhost_enable_notify(&net->dev, vq))
+			vhost_poll_queue(&vq->poll);
+		mutex_unlock(&vq->mutex);
+	}
+
+	return peek_head_len(sk);
+}
+
 /* This is a multi-buffer version of vhost_get_desc, that works if
  *	vq has read descriptors only.
  * @vq		- the relevant virtqueue
@@ -553,7 +620,7 @@ static void handle_rx(struct vhost_net *net)
 		vq->log : NULL;
 	mergeable = vhost_has_feature(vq, VIRTIO_NET_F_MRG_RXBUF);
 
-	while ((sock_len = peek_head_len(sock->sk))) {
+	while ((sock_len = vhost_net_peek_head_len(net, sock->sk))) {
 		sock_len += sock_hlen;
 		vhost_len = sock_len + vhost_hlen;
 		headcount = get_rx_bufs(vq, vq->heads, vhost_len,
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index b86c5aa..8f9a64c 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -285,6 +285,7 @@ static void vhost_vq_reset(struct vhost_dev *dev,
 	vq->memory = NULL;
 	vq->is_le = virtio_legacy_is_little_endian();
 	vhost_vq_reset_user_be(vq);
+	vq->busyloop_timeout = 0;
 }
 
 static int vhost_worker(void *data)
@@ -747,6 +748,7 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp)
 	struct vhost_vring_state s;
 	struct vhost_vring_file f;
 	struct vhost_vring_addr a;
+	struct vhost_vring_busyloop_timeout t;
 	u32 idx;
 	long r;
 
@@ -919,6 +921,19 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp)
 	case VHOST_GET_VRING_ENDIAN:
 		r = vhost_get_vring_endian(vq, idx, argp);
 		break;
+	case VHOST_SET_VRING_BUSYLOOP_TIMEOUT:
+		if (copy_from_user(&t, argp, sizeof t)) {
+			r = -EFAULT;
+			break;
+		}
+		vq->busyloop_timeout = t.timeout;
+		break;
+	case VHOST_GET_VRING_BUSYLOOP_TIMEOUT:
+		t.index = idx;
+		t.timeout = vq->busyloop_timeout;
+		if (copy_to_user(argp, &t, sizeof t))
+			r = -EFAULT;
+		break;
 	default:
 		r = -ENOIOCTLCMD;
 	}
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 5983a13..90453f0 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -115,6 +115,7 @@ struct vhost_virtqueue {
 	/* Ring endianness requested by userspace for cross-endian support. */
 	bool user_be;
 #endif
+	u32 busyloop_timeout;
 };
 
 struct vhost_dev {
diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
index ab373191..f013656 100644
--- a/include/uapi/linux/vhost.h
+++ b/include/uapi/linux/vhost.h
@@ -27,6 +27,11 @@ struct vhost_vring_file {
 
 };
 
+struct vhost_vring_busyloop_timeout {
+	unsigned int index;
+	unsigned int timeout;
+};
+
 struct vhost_vring_addr {
 	unsigned int index;
 	/* Option flags. */
@@ -126,6 +131,12 @@ struct vhost_memory {
 #define VHOST_SET_VRING_CALL _IOW(VHOST_VIRTIO, 0x21, struct vhost_vring_file)
 /* Set eventfd to signal an error */
 #define VHOST_SET_VRING_ERR _IOW(VHOST_VIRTIO, 0x22, struct vhost_vring_file)
+/* Set busy loop timeout */
+#define VHOST_SET_VRING_BUSYLOOP_TIMEOUT _IOW(VHOST_VIRTIO, 0x23,	\
+		                     struct vhost_vring_busyloop_timeout)
+/* Get busy loop timeout */
+#define VHOST_GET_VRING_BUSYLOOP_TIMEOUT _IOW(VHOST_VIRTIO, 0x24,	\
+		                     struct vhost_vring_busyloop_timeout)
 
 /* VHOST_NET specific defines */
 
-- 
2.1.4

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox