AlacrityVM guest drivers

shm-signal provides a generic shared-memory based bidirectional signaling mechanism. It is used in conjunction with an existing signal transport (such as posix-signals, interrupts, pipes, etc) to increase the efficiency of the transport since the state information is directly accessible to both sides of the link. The shared-memory design provides very cheap access to features such as event-masking and spurious delivery mititgation, and is useful implementing higher level shared-memory constructs such as rings.

lib/shm_signal.c new file +9 -0

+	/*
+	 * We always mark the remote side as dirty regardless of whether
+	 * they need to be notified.
+	 */
+	irq->dirty = 1;
+	wmb();   /* dirty must be visible before we test the pending state */
+
+	if (irq->enabled && !irq->pending) {
+		rmb();

Quoted 1 line

Quoted 1 line

On 8/6/2009 at 9:56 AM, in message <[email protected]>, Arnd

Quoted 8 lines · Gregory Haskins

shm-signal provides a generic shared-memory based bidirectional signaling mechanism. It is used in conjunction with an existing signal transport (such as posix-signals, interrupts, pipes, etc) to increase the efficiency of the transport since the state information is directly accessible to both sides of the link. The shared-memory design provides very cheap access to features such as event-masking and spurious delivery mititgation, and is useful implementing higher level shared-memory constructs such as rings.

Looks like a very useful feature in general.

include/linux/shm_signal.h line 42

Quoted 6 lines

include/linux/shm_signal.h new file +5 -0

+struct shm_signal_irq {
+       __u8                  enabled;
+       __u8                  pending;
+       __u8                  dirty;
+};

Won't this layout cause cache line ping pong? Other schemes I have seen try to separate the bits so that each cache line is written to by only one side.

This gets much more interesting if the two sides
are on remote ends of an I/O link, e.g. using a nontransparent PCI bridge, where you only want to send stores over the wire, but never fetches or even read-modify-write cycles.

Your code is probably optimal if you only communicate between host and guest code on the same CPU, but not so good if it crosses NUMA nodes or worse.

include/linux/shm_signal.h line 53

Quoted 6 lines

include/linux/shm_signal.h new file +5 -0

+struct shm_signal_desc {
+       __u32                 magic;
+       __u32                 ver;
+       struct shm_signal_irq irq[2];
+};

This data structure has implicit padding of two bytes at the end. How about adding another '__u16 reserved' to make it explicit?

Quoted 16 lines

lib/shm_signal.c new file +15 -0

+	/*
+	 * We always mark the remote side as dirty regardless of whether
+	 * they need to be notified.
+	 */
+	irq->dirty = 1;
+	wmb();   /* dirty must be visible before we test the pending state */
+
+	if (irq->enabled && !irq->pending) {
+		rmb();
+
+		/*
+		 * If the remote side has enabled notifications, and we do
+		 * not see a notification pending, we must inject a new one.
+		 */
+		irq->pending = 1;

lib/shm_signal.c line 98

Quoted 4 lines

lib/shm_signal.c new file +4 -0

+		wmb(); /* make it visible before we do the injection */
+
+		s->ops->inject(s);
+	}

Barriers always confuse me, but the rmb() looks slightly wrong. AFAIU it only prevents reads after the barrier from being done before the barrier, but you don't do any reads after it.

The (irq->enabled && !irq->pending) check could be done before the irq->dirty = 1 arrives at the bus, but that does not seem to hurt, it would at most cause a duplicate ->inject().

Regarding the scope of the barrier, did you intentionally use the global versions (rmb()/wmb()) and not the lighter single-system (smp_rmb()/smp_wmb()) versions? Your version should cope with remote links over PCI but looks otherwise optimized for local use, as I wrote above.

Hi Arnd,

Quoted 1 line

Quoted 1 line

Quoted 1 line

On 8/6/2009 at 9:56 AM, in message <[email protected]>, Arnd

Quoted 11 lines · Bergmann

Quoted 8 lines · Gregory Haskins

shm-signal provides a generic shared-memory based bidirectional signaling mechanism. It is used in conjunction with an existing signal transport (such as posix-signals, interrupts, pipes, etc) to increase the efficiency of the transport since the state information is directly accessible to both sides of the link. The shared-memory design provides very cheap access to features such as event-masking and spurious delivery mititgation, and is useful implementing higher level shared-memory constructs such as rings.

Looks like a very useful feature in general.

Thanks, I was hoping that would be the case.

Quoted 10 lines

include/linux/shm_signal.h line 42

Quoted 6 lines

include/linux/shm_signal.h new file +5 -0

+struct shm_signal_irq {
+       __u8                  enabled;
+       __u8                  pending;
+       __u8                  dirty;
+};

Won't this layout cause cache line ping pong? Other schemes I have seen try to separate the bits so that each cache line is written to by only one side.

It could possibly use some optimization in that regard. I generally consider myself an expert at concurrent programming, but this lockless stuff is, um, hard ;) I was going for correctness first.

Long story short, any suggestions on ways to split this up are welcome (particularly now, before the ABI is sealed ;)

Quoted 4 lines

This gets much more interesting if the two sides
are on remote ends of an I/O link, e.g. using a nontransparent PCI bridge, where you only want to send stores over the wire, but never fetches or even read-modify-write cycles.

/me head explodes ;)

This patch adds a pci-based driver to interface between the a host VBUS and the guest's vbus-proxy bus model.

Signed-off-by Gregory Haskins <[email protected]>

drivers/vbus/pci-bridge.c new file +10 -0

+static int
+vbus_pci_hypercall(unsigned long nr, void *data, unsigned long len)
+{
+	struct vbus_pci_hypercall params = {
+		.vector = nr,
+		.len    = len,
+		.datap  = __pa(data),
+	};
+	unsigned long flags;
+	int ret;

drivers/vbus/pci-bridge.c new file +13 -0

+
+static int
+vbus_pci_device_open(struct vbus_device_proxy *vdev, int version, int flags)
+{
+	struct vbus_pci_device *dev = to_dev(vdev);
+	struct vbus_pci_deviceopen params;
+	int ret;
+
+	if (dev->handle)
+		return -EINVAL;
+
+	params.devid   = vdev->id;
+	params.version = version;

drivers/vbus/pci-bridge.c new file +71 -0

+static int
+vbus_pci_device_shm(struct vbus_device_proxy *vdev, int id, int prio,
+		    void *ptr, size_t len,
+		    struct shm_signal_desc *sdesc, struct shm_signal **signal,
+		    int flags)
+{
+	struct vbus_pci_device *dev = to_dev(vdev);
+	struct _signal *_signal = NULL;
+	struct vbus_pci_deviceshm params;
+	unsigned long iflags;
+	int ret;
+
+	if (!dev->handle)
+		return -EINVAL;
+
+	params.devh   = dev->handle;
+	params.id     = id;
+	params.flags  = flags;
+	params.datap  = (u64)__pa(ptr);
+	params.len    = len;
+
+	if (signal) {
+		/*
+		 * The signal descriptor must be embedded within the
+		 * provided ptr
+		 */
+		if (!sdesc
+		    || (len < sizeof(*sdesc))
+		    || ((void *)sdesc < ptr)
+		    || ((void *)sdesc > (ptr + len - sizeof(*sdesc))))
+			return -EINVAL;
+
+		_signal = kzalloc(sizeof(*_signal), GFP_KERNEL);
+		if (!_signal)
+			return -ENOMEM;
+
+		_signal_init(&_signal->signal, sdesc, &_signal_ops);
+
+		/*
+		 * take another reference for the host.  This is dropped
+		 * by a SHMCLOSE event
+		 */
+		shm_signal_get(&_signal->signal);
+
+		params.signal.offset = (u64)sdesc - (u64)ptr;
+		params.signal.prio   = prio;
+		params.signal.cookie = (u64)_signal;
+
+	} else
+		params.signal.offset = -1; /* yes, this is a u32, but its ok */
+
+	ret = vbus_pci_hypercall(VBUS_PCI_HC_DEVSHM,
+				 &params, sizeof(params));
+	if (ret < 0) {
+		if (_signal) {
+			/*
+			 * We held two references above, so we need to drop
+			 * both of them
+			 */
+			shm_signal_put(&_signal->signal);
+			shm_signal_put(&_signal->signal);
+		}
+
+		return ret;
+	}
+
+	if (signal) {
+		_signal->handle = ret;
+
+		spin_lock_irqsave(&vbus_pci.lock, iflags);
+

drivers/vbus/pci-bridge.c new file +8 -0

+static int
+vbus_pci_device_call(struct vbus_device_proxy *vdev, u32 func, void *data,
+		     size_t len, int flags)
+{
+	struct vbus_pci_device *dev = to_dev(vdev);
+	struct vbus_pci_devicecall params = {
+		.devh  = dev->handle,
+		.func  = func,

drivers/vbus/pci-bridge.c new file +49 -0

+
+static struct ioq_notifier eventq_notifier;
 
...
 
+/* Invoked whenever the hypervisor ioq_signal()s our eventq */
+static void
+eventq_wakeup(struct ioq_notifier *notifier)
+{
+	struct ioq_iterator iter;
+	int ret;
+
+	/* We want to iterate on the head of the in-use index */
+	ret = ioq_iter_init(&vbus_pci.eventq, &iter, ioq_idxtype_inuse, 0);
+	BUG_ON(ret < 0);
+
+	ret = ioq_iter_seek(&iter, ioq_seek_head, 0, 0);
+	BUG_ON(ret < 0);
+
+	/*
+	 * The EOM is indicated by finding a packet that is still owned by
+	 * the south side.
+	 *
+	 * FIXME: This in theory could run indefinitely if the host keeps
+	 * feeding us events since there is nothing like a NAPI budget.  We
+	 * might need to address that
+	 */
+	while (!iter.desc->sown) {
+		struct ioq_ring_desc *desc  = iter.desc;
+		struct vbus_pci_event *event;
+
+		event = (struct vbus_pci_event *)desc->cookie;
+
+		switch (event->eventid) {
+		case VBUS_PCI_EVENT_DEVADD:
+			event_devadd(&event->data.add);
+			break;
+		case VBUS_PCI_EVENT_DEVDROP:
+			event_devdrop(&event->data.handle);
+			break;
+		case VBUS_PCI_EVENT_SHMSIGNAL:
+			event_shmsignal(&event->data.handle);
+			break;
+		case VBUS_PCI_EVENT_SHMCLOSE:
+			event_shmclose(&event->data.handle);
+			break;
+		default:
+			printk(KERN_WARNING "VBUS_PCI: Unexpected event %d\n",
+			       event->eventid);
+			break;
+		};
+

Quoted 1 line

Quoted 1 line

On 8/6/2009 at 10:42 AM, in message <[email protected]>, Arnd

Quoted 4 lines · Gregory Haskins

This patch adds a pci-based driver to interface between the a host VBUS and the guest's vbus-proxy bus model.

Signed-off-by Gregory Haskins <[email protected]>

This seems to be duplicating parts of virtio-pci that could be kept common by extending the virtio code. Layering on top of virtio would also make it possible to use the same features you add on top of other transports (e.g. the s390 virtio code) without adding yet another backend for each of them.

Quoted 15 lines

drivers/vbus/pci-bridge.c new file +14 -0

+static int
+vbus_pci_hypercall(unsigned long nr, void *data, unsigned long len)
+{
+	struct vbus_pci_hypercall params = {
+		.vector = nr,
+		.len    = len,
+		.datap  = __pa(data),
+	};
+	unsigned long flags;
+	int ret;
+
+	spin_lock_irqsave(&vbus_pci.lock, flags);
+
+	memcpy_toio(&vbus_pci.regs->hypercall.data, &params, sizeof(params));

drivers/vbus/pci-bridge.c line 83

Quoted 6 lines

drivers/vbus/pci-bridge.c new file +6 -0

+	ret = ioread32(&vbus_pci.regs->hypercall.result);
+
+	spin_unlock_irqrestore(&vbus_pci.lock, flags);
+
+	return ret;
+}

The functionality looks reasonable but please don't call this a hypercall.

A hypercall would be hypervisor specific by definition while this one is device specific if I understand it correctly. How about "command queue", "mailbox", "message queue", "devcall" or something else that we have in existing PCI devices?

drivers/vbus/pci-bridge.c

Quoted 3 lines

drivers/vbus/pci-bridge.c new file +3 -0

+
+static int
+vbus_pci_device_open(struct vbus_device_proxy *vdev, int version, int 

flags)

Quoted 15 lines

drivers/vbus/pci-bridge.c new file +15 -0

+{
+	struct vbus_pci_device *dev = to_dev(vdev);
+	struct vbus_pci_deviceopen params;
+	int ret;
+
+	if (dev->handle)
+		return -EINVAL;
+
+	params.devid   = vdev->id;
+	params.version = version;
+
+	ret = vbus_pci_hypercall(VBUS_PCI_HC_DEVOPEN,
+				 &params, sizeof(params));
+	if (ret < 0)
+		return ret;

drivers/vbus/pci-bridge.c line 171

Quoted 5 lines

drivers/vbus/pci-bridge.c new file +5 -0

+
+	dev->handle = params.handle;
+
+	return 0;
+}

This seems to add an artificial abstraction that does not make sense if you stick to the PCI abstraction.

The two sensible and common models
for virtual devices that I've seen are:

* The hypervisor knows what virtual resources exist and provides them to the guest. The guest owns them as soon as they show up in the bus (e.g. PCI) probe. The 'handle' is preexisting.

* The guest starts without any devices and asks for resources it wants to access. There is no probing of resources but the guest issues a hypercall to get a handle to a newly created virtual device (or -ENODEV).

What is your reasoning for requiring both a probe and an allocation?

Quoted 76 lines

drivers/vbus/pci-bridge.c new file +75 -0

+static int
+vbus_pci_device_shm(struct vbus_device_proxy *vdev, int id, int prio,
+		    void *ptr, size_t len,
+		    struct shm_signal_desc *sdesc, struct shm_signal **signal,
+		    int flags)
+{
+	struct vbus_pci_device *dev = to_dev(vdev);
+	struct _signal *_signal = NULL;
+	struct vbus_pci_deviceshm params;
+	unsigned long iflags;
+	int ret;
+
+	if (!dev->handle)
+		return -EINVAL;
+
+	params.devh   = dev->handle;
+	params.id     = id;
+	params.flags  = flags;
+	params.datap  = (u64)__pa(ptr);
+	params.len    = len;
+
+	if (signal) {
+		/*
+		 * The signal descriptor must be embedded within the
+		 * provided ptr
+		 */
+		if (!sdesc
+		    || (len < sizeof(*sdesc))
+		    || ((void *)sdesc < ptr)
+		    || ((void *)sdesc > (ptr + len - sizeof(*sdesc))))
+			return -EINVAL;
+
+		_signal = kzalloc(sizeof(*_signal), GFP_KERNEL);
+		if (!_signal)
+			return -ENOMEM;
+
+		_signal_init(&_signal->signal, sdesc, &_signal_ops);
+
+		/*
+		 * take another reference for the host.  This is dropped
+		 * by a SHMCLOSE event
+		 */
+		shm_signal_get(&_signal->signal);
+
+		params.signal.offset = (u64)sdesc - (u64)ptr;
+		params.signal.prio   = prio;
+		params.signal.cookie = (u64)_signal;
+
+	} else
+		params.signal.offset = -1; /* yes, this is a u32, but its ok */
+
+	ret = vbus_pci_hypercall(VBUS_PCI_HC_DEVSHM,
+				 &params, sizeof(params));
+	if (ret < 0) {
+		if (_signal) {
+			/*
+			 * We held two references above, so we need to drop
+			 * both of them
+			 */
+			shm_signal_put(&_signal->signal);
+			shm_signal_put(&_signal->signal);
+		}
+
+		return ret;
+	}
+
+	if (signal) {
+		_signal->handle = ret;
+
+		spin_lock_irqsave(&vbus_pci.lock, iflags);
+
+		list_add_tail(&_signal->list, &dev->shms);
+
+		spin_unlock_irqrestore(&vbus_pci.lock, iflags);
+

drivers/vbus/pci-bridge.c line 296

Quoted 6 lines

drivers/vbus/pci-bridge.c new file +6 -0

+		shm_signal_get(&_signal->signal);
+		*signal = &_signal->signal;
+	}
+
+	return 0;
+}

This could be implemented by virtio devices as well, right?

Quoted 13 lines

drivers/vbus/pci-bridge.c new file +12 -0

+static int
+vbus_pci_device_call(struct vbus_device_proxy *vdev, u32 func, void *data,
+		     size_t len, int flags)
+{
+	struct vbus_pci_device *dev = to_dev(vdev);
+	struct vbus_pci_devicecall params = {
+		.devh  = dev->handle,
+		.func  = func,
+		.datap = (u64)__pa(data),
+		.len   = len,
+		.flags = flags,
+	};

drivers/vbus/pci-bridge.c line 315

Quoted 6 lines

drivers/vbus/pci-bridge.c new file +6 -0

+
+	if (!dev->handle)
+		return -EINVAL;
+
+	return vbus_pci_hypercall(VBUS_PCI_HC_DEVCALL, &params, sizeof(params));
+}

Why the indirection? It seems to me that you could do the simpler

static int
vbus_pci_device_call(struct vbus_device_proxy *vdev, u32 func, void *data,
size_t len, int flags)

{
	struct vbus_pci_device *dev = to_dev(vdev);
	struct vbus_pci_hypercall params = {
		.vector = func,
		.len    = len,
		.datap  = __pa(data),
	};
	spin_lock_irqsave(&dev.lock, flags);
	memcpy_toio(&dev.regs->hypercall.data, &params, sizeof(params));
	ret = ioread32(&dev.regs->hypercall.result);
	spin_unlock_irqrestore(&dev.lock, flags);

	return ret;
}

This gets rid of your 'handle' and the unwinding through an extra pointer indirection. You just need to make sure that the device specific call numbers
don't conflict with any global ones.

Quoted 58 lines

drivers/vbus/pci-bridge.c new file +54 -0

+
+static struct ioq_notifier eventq_notifier;
 
...
 
+/* Invoked whenever the hypervisor ioq_signal()s our eventq */
+static void
+eventq_wakeup(struct ioq_notifier *notifier)
+{
+	struct ioq_iterator iter;
+	int ret;
+
+	/* We want to iterate on the head of the in-use index */
+	ret = ioq_iter_init(&vbus_pci.eventq, &iter, ioq_idxtype_inuse, 0);
+	BUG_ON(ret < 0);
+
+	ret = ioq_iter_seek(&iter, ioq_seek_head, 0, 0);
+	BUG_ON(ret < 0);
+
+	/*
+	 * The EOM is indicated by finding a packet that is still owned by
+	 * the south side.
+	 *
+	 * FIXME: This in theory could run indefinitely if the host keeps
+	 * feeding us events since there is nothing like a NAPI budget.  We
+	 * might need to address that
+	 */
+	while (!iter.desc->sown) {
+		struct ioq_ring_desc *desc  = iter.desc;
+		struct vbus_pci_event *event;
+
+		event = (struct vbus_pci_event *)desc->cookie;
+
+		switch (event->eventid) {
+		case VBUS_PCI_EVENT_DEVADD:
+			event_devadd(&event->data.add);
+			break;
+		case VBUS_PCI_EVENT_DEVDROP:
+			event_devdrop(&event->data.handle);
+			break;
+		case VBUS_PCI_EVENT_SHMSIGNAL:
+			event_shmsignal(&event->data.handle);
+			break;
+		case VBUS_PCI_EVENT_SHMCLOSE:
+			event_shmclose(&event->data.handle);
+			break;
+		default:
+			printk(KERN_WARNING "VBUS_PCI: Unexpected event %d\n",
+			       event->eventid);
+			break;
+		};
+
+		memset(event, 0, sizeof(*event));
+
+		/* Advance the in-use head */
+		ret = ioq_iter_pop(&iter, 0);
+		BUG_ON(ret < 0);

drivers/vbus/pci-bridge.c line 550

Quoted 5 lines

drivers/vbus/pci-bridge.c new file +5 -0

+	}
+
+	/* And let the south side know that we changed the queue */
+	ioq_signal(&vbus_pci.eventq, 0);
+}

Ah, so you have a global event queue and your own device hotplug mechanism. But why would you then still use PCI to back it?

We already have PCI hotplug to add and remove devices

and you have defined per device notifier queues that you can use for waking up the device, right?

Quoted 1 line

Quoted 1 line

Quoted 1 line

On 8/6/2009 at 10:42 AM, in message <[email protected]>, Arnd

Quoted 11 lines · Bergmann

Quoted 4 lines · Gregory Haskins

This patch adds a pci-based driver to interface between the a host VBUS and the guest's vbus-proxy bus model.

Signed-off-by Gregory Haskins <[email protected]>

This seems to be duplicating parts of virtio-pci that could be kept common by extending the virtio code. Layering on top of virtio would also make it possible to use the same features you add on top of other transports (e.g. the s390 virtio code) without adding yet another backend for each of them.

This doesn't make sense to me, but I suspect we are both looking at what this code does differently. I am under the impression that you may believe that there is one of these objects per vbus device. Note that this is just a bridge to vbus, so there is only one of these per system with potentially many vbus devices behind it.

In essence, this driver's job is to populate the "vbus-proxy" LDM bus with objects that it finds across the PCI-OTHER bridge. This would actually sit below the virtio components in the stack, so it doesnt make sense (to me) to turn around and build this on top of virtio. But perhaps I am missing something you are seeing.

Can you elaborate?

Quoted 2 lines

This seems to add an artificial abstraction that does not make sense if you stick to the PCI abstraction.

I think there may be confusion about what is going on here. The "device-open" pertains to a vbus device *beyond* the bridge, not the PCI device (the bridge) itself. Nor is the vbus device a PCI device.

Whats happening here is somewhat analogous to a PCI config-cycle. Its a way to open a channel to a device beyond the bridge in _response_ to a probe.

We have a way to enumerate devices present beyond the bridge (this yields a "device-id") but in order to actually talk to the device, you must first call DEVOPEN(id). When a device-id is enumerated, it generates a probe() event on vbus-proxy. The responding driver in question would then turn around and issue the handle = dev->open(VERSION) to see if it is compatible with the device, and to establish a context for further communication.

The reason why DEVOPEN returns a unique handle is to help ensure that the driver has established proper context before allowing other calls.

Quoted 2 lines

This could be implemented by virtio devices as well, right?

The big difference with dev->shm() is that it is not bound to a particular ABI within the shared-memory (as opposed to virtio, which assumes a virtio ABI). This just creates a n empty shared-memory region (with a bidirectional signaling path) which you can overlay a variety of structures (virtio included). You can of course also use non-ring based structures, such as, say, an array of idempotent state.

The point is that, once this is done, you have a shared-memory region and a way (via the shm-signal) to bidirectionally signal changes to that memory region. You can then build bigger things with it, like virtqueues.

Quoted 18 lines

static int
vbus_pci_device_call(struct vbus_device_proxy *vdev, u32 func, void *data,
size_t len, int flags)

{
	struct vbus_pci_device *dev = to_dev(vdev);
	struct vbus_pci_hypercall params = {
		.vector = func,
		.len    = len,
		.datap  = __pa(data),
	};
	spin_lock_irqsave(&dev.lock, flags);
	memcpy_toio(&dev.regs->hypercall.data, &params, sizeof(params));
	ret = ioread32(&dev.regs->hypercall.result);
	spin_unlock_irqrestore(&dev.lock, flags);

	return ret;
}

Quoted 4 lines

This gets rid of your 'handle' and the unwinding through an extra pointer indirection. You just need to make sure that the device specific call numbers
don't conflict with any global ones.

Ah, now I see the confusion...

DEVCALL is sending a synchronous call to a specific device beyond the bridge. The MMIO going on here against dev.regs->hypercall.data is sending a synchronous call to the bridge itself. They are distinctly different ;)

Quoted 1 line

Quoted 1 line

On 8/6/2009 at 1:03 PM, in message <[email protected]>, Arnd

Quoted 8 lines · you

Quoted 1 line

Quoted 1 line

Quoted 1 line

On 8/6/2009 at 10:42 AM, in message <[email protected]>, Arnd

Quoted 6 lines · Bergmann

Quoted 4 lines · Gregory Haskins

This patch adds a pci-based driver to interface between the a host VBUS and the guest's vbus-proxy bus model.

Signed-off-by Gregory Haskins <[email protected]>

Quoted 7 lines

Quoted 5 lines

This seems to be duplicating parts of virtio-pci that could be kept common by extending the virtio code. Layering on top of virtio would also make it possible to use the same features you add on top of other transports (e.g. the s390 virtio code) without adding yet another backend for each of them.

This doesn't make sense to me, but I suspect we are both looking at what

this

Quoted 2 lines

code does differently. I am under the impression that you may believe that there is one of these objects per vbus device. Note that this is just a

bridge

Quoted 2 lines

to vbus, so there is only one of these per system with potentially many vbus devices behind it.

Right, this did not become clear from the posting. For virtio, we discussed a model like this in the beginning and then rejected it in favour of a "one PCI device per virtio device" model, which I now think is a better approach than your pci-to-vbus bridge.

Quoted 8 lines

In essence, this driver's job is to populate the "vbus-proxy" LDM bus with objects that it finds across the PCI-OTHER bridge. This would actually sit below the virtio components in the stack, so it doesnt make sense (to me) to turn around and build this on top of virtio. But perhaps I am missing something you are seeing.

Can you elaborate?

Your PCI device does not serve any real purpose as far as I can tell

, you could just as well have a root device as a parent for all the vbus devices if you do your device probing like this.

However, assuming that you do the IMHO right thing to do probing like virtio with a PCI device for each slave, the code will be almost the same as virtio-pci and the two can be the same.

Quoted 5 lines

Quoted 3 lines

This seems to add an artificial abstraction that does not make sense if you stick to the PCI abstraction.

I think there may be confusion about what is going on here. The

"device-open"

Quoted 1 line

pertains to a vbus device *beyond* the bridge, not the PCI device (the

bridge)

Quoted 13 lines

itself. Nor is the vbus device a PCI device.

Whats happening here is somewhat analogous to a PCI config-cycle. Its a way to open a channel to a device beyond the bridge in _response_ to a probe.

We have a way to enumerate devices present beyond the bridge (this yields a "device-id") but in order to actually talk to the device, you must first call DEVOPEN(id). When a device-id is enumerated, it generates a probe() event on vbus-proxy. The responding driver in question would then turn around and issue the handle = dev->open(VERSION) to see if it is compatible with the device, and to establish a context for further communication.

Quoted 2 lines

The reason why DEVOPEN returns a unique handle is to help ensure that the driver has established proper context before allowing other calls.

So assuming this kind of bus is the right idea (which I think it's not), why can't the host assume they are open to start with

and you go and enumerate the devices on the bridge, creating a vbus_device for each one as you go.

Then you just need to match the vbus drivers with the devices by some string or vendor/device ID tuple.

Quoted 11 lines

Quoted 2 lines

This could be implemented by virtio devices as well, right?

The big difference with dev->shm() is that it is not bound to a particular ABI within the shared-memory (as opposed to virtio, which assumes a virtio ABI). This just creates a n empty shared-memory region (with a bidirectional signaling path) which you can overlay a variety of structures (virtio included). You can of course also use non-ring based structures, such as, say, an array of idempotent state.

Quoted 4 lines

The point is that, once this is done, you have a shared-memory region and a way (via the shm-signal) to bidirectionally signal changes to that memory region. You can then build bigger things with it, like virtqueues.

Let me try to rephrase my point: I believe you can implement the shm/ioq data transport on top of the virtio bus level, by adding shm and signal functions to struct virtio_config_ops alongside find_vqs() so that a virtio_device can have either any combination of virtqueues, shm and ioq.

Quoted 24 lines

Quoted 23 lines

static int
vbus_pci_device_call(struct vbus_device_proxy *vdev, u32 func, void *data,
size_t len, int flags)

{
	struct vbus_pci_device *dev = to_dev(vdev);
	struct vbus_pci_hypercall params = {
		.vector = func,
		.len    = len,
		.datap  = __pa(data),
	};
	spin_lock_irqsave(&dev.lock, flags);
	memcpy_toio(&dev.regs->hypercall.data, &params, sizeof(params));
	ret = ioread32(&dev.regs->hypercall.result);
	spin_unlock_irqrestore(&dev.lock, flags);

	return ret;
}

This gets rid of your 'handle' and the unwinding through an extra pointer indirection. You just need to make sure that the device specific call numbers
don't conflict with any global ones.

Quoted 3 lines

Ah, now I see the confusion...

DEVCALL is sending a synchronous call to a specific device beyond the

bridge. The MMIO going on here against dev.regs->hypercall.data is sending a synchronous call to the bridge itself. They are distinctly different ;)

well, my point earlier was that they probably should not be different ;-)

Quoted 1 line

Quoted 1 line

Quoted 1 line · Arnd Bergmann

Here are some of my arguments against it:

1) there is an ample PCI model that is easy to work with when you are in QEMU and using its device model (and you get it for free). Its the path of least resistance. For something in kernel, it is more awkward to try to coordinate the in-kernel state with the PCI state. Afaict, you either need to have it live partially in both places, or you need some PCI emulation in the kernel.

2) The signal model for the 1:1 design is not very flexible IMO.
2a) I want to be able to allocate dynamic signal paths, not pre-allocate msi-x vectors at dev-add.

2b) I also want to collapse multiple interrupts together so as to minimize the context switch rate (inject + EIO overhead). My design effectively has "NAPI" for interrupt handling. This helps when the system needs it the most: heavy IO.

3) The 1:1 model is not buying us much in terms of hotplug. We don't really "use" PCI very much even in virtio. Its a thin-shim of uniform dev-ids to resurface to the virtio-bus as something else. With LDM, hotplug is ridiculously easy anyway, so who cares. I already need an event channel anyway for (2b) anyway, so the devadd/devdrop events are trivial to handle.

4) communicating with something efficiently in-kernel requires more finesse than basic PIO/MMIO. There are tricks you can do to get around this, but with 1:1 you would have to do this trick repeatedly for each device. Even with a library solution to help, you still have per-cpu .data overhead and cpu hotplug overhead to get maximum performance. With my "bridge" model, I do it once, which I believe is ideal.

5) 1:1 is going to quickly populate the available MMIO/PIO and IDT slots for any kind of medium to large configuration. The bridge model scales better in this regard.

So based on that, I think the bridge model works better for vbus. Perhaps you can convince me otherwise ;)

Quoted 6 lines

Quoted 6 lines

In essence, this driver's job is to populate the "vbus-proxy" LDM bus with objects that it finds across the PCI-OTHER bridge. This would actually sit below the virtio components in the stack, so it doesnt make sense (to me) to turn around and build this on top of virtio. But perhaps I am missing something you are seeing.

Quoted 3 lines

Quoted 1 line

Can you elaborate?

Your PCI device does not serve any real purpose as far as I can tell

That is certainly debatable. Its purpose is as follows:

1) Allows a guest to discover the vbus feature (fwiw: I used to do this with cpuid)

2) Allows the guest to establish proper context to communicate with the feature (mmio, pio, and msi) (fwiw: i used to use hypercalls)
3) Access the virtual-devices that have been configured for the feature

Correct me if I am wrong: Isn't this more of less the exact intent of something like an LDM bus (vbus-proxy) and a PCI-BRIDGE? Other than the possibility that there might be some mergable overlap (still debatable), I don't think its fair to say that this does not serve a purpose.

Quoted 2 lines

, you could just as well have a root device as a parent for all the vbus devices if you do your device probing like this.

Yes, I suppose the "bridge" could have been advertised as a virtio-based root device. In this way, the virtio probe() would replace my pci probe() for feature discovery, and a virtqueue could replace my msi+ioq for the eventq channel.

I see a few issues with that, however:

1) The virtqueue library, while a perfectly nice ring design at the metadata level, does not have an API that is friendly to kernel-to-kernel communication. It was designed more for frontend use to some remote backend. The IOQ library on the other hand, was specifically designed to support use as kernel-to-kernel (see north/south designations). So this made life easier for me. To do what you propose, the eventq channel would need to terminate in kernel, and I would thus be forced to deal that the potential API problems.

2) I would need to have Avi et. al. allocate a virtio vector to use from their namespace, which I am sure they wont be willing to do until they accept my design. Today, I have a nice conflict free PCI ID to use as I see fit.

Im sure both of these hurdles are not insurmountable, but I am left scratching my head as to why its worth the effort. It seems to me its a "six of one, half-dozen of the other" kind of scenario. Either I write a qemu PCI device and pci-bridge driver, or I write a qemu virtio-devicve and virtio root driver.

In short: What does this buy us, or did you mean something else?

Quoted 3 lines

However, assuming that you do the IMHO right thing to do probing like virtio with a PCI device for each slave, the code will be almost the same as virtio-pci and the two can be the same.

Can you elaborate?

Quoted 2 lines

and you go and enumerate the devices on the bridge, creating a vbus_device for each one as you go.

Thats exactly what it does.

Quoted 3 lines

Then you just need to match the vbus drivers with the devices by some string or vendor/device ID tuple.

Yep, thats right too. Then, when the driver gets a ->probe(), it does an dev->open() to check various state:

a) can the device be opened? if it has an max-open policy (most will have a max-open = 1 policy) and something else already has the device open, it will fail (this will not be common).
b) is the driver ABI revision compatible with the device ABI revision? This is like checking the pci config-space revision number.

For an example, see drivers/net/vbus-enet.c, line 764:

http://git.kernel.org/?p=linux/kernel/git/ghaskins/alacrityvm/linux-2.6.git;a=blob;f=drivers/net/vbus-enet.c;h=7220f43723adc5b0bece1bc37974fae1b034cd9e;hb=b3b2339efbd4e754b1c85f8bc8f85f21a1a1f509#l764

Its simple a check to see if the driver and device are compatible, and therefore the probe should succeed. Nothing more. I think what I have done is similar to how most buses (like PCI) work today (ala revision number checks with a config-cycle).

Regarding the id->handle indirection:

Internally, the DEVOPEN call translates an "id" to a "handle". The handle is just a token to help ensure that the caller actually opened the device successfully. Note that the "id" namespace is 0 based. Therefore, something like an errant DEVCALL(0) would be indistinguishable from a legit request. Using the handle abstraction gives me a slightly more robust mechanism to ensure the caller actually meant to call the host, and was in the proper context to do so. For one thing, if the device had never been opened, this would have failed before it ever reached the model. Its one more check I can do at the infrastructure level, and one less thing each model has to look out for.

Is the id->handle translation critical? No, i'm sure we could live without it, but I also don't think it hurts anything. It allows the overall code to be slightly more robust, and the individual model code to be slightly less complicated. Therefore, I don't see a problem.

Quoted 1 line

Quoted 1 line

On 8/6/2009 at 6:57 PM, in message <[email protected]>, Arnd

Quoted 1 line · Gregory Haskins

Quoted 1 line

Quoted 1 line

Quoted 1 line

On 8/6/2009 at 1:03 PM, in message <[email protected]>, Arnd

Quoted 3 lines · Bergmann

Here are some of my arguments against it:

1) there is an ample PCI model that is easy to work with when you are in

QEMU and using its device model (and you get it for free). Its the path of least resistance. For something in kernel, it is more awkward to try to coordinate the in-kernel state with the PCI state. Afaict, you either need to have it live partially in both places, or you need some PCI emulation in the kernel.

True, if the whole hypervisor is in the host kernel, then doing full PCI emulation would be
insane.

I was assuming that all of the setup code still lived in host user space.

What is the reason why it cannot?

Do you want to use something other than qemu,

do you think this will impact performance, or something else?

Quoted 3 lines

2) The signal model for the 1:1 design is not very flexible IMO. 2a) I want to be able to allocate dynamic signal paths, not pre-allocate

msi-x vectors at dev-add.

I believe msi-x implies that the interrupt vectors get added by the device driver
at run time, unlike legacy interrupts or msi. It's been a while since I dealt with
that though.

Quoted 2 lines

2b) I also want to collapse multiple interrupts together so as to

minimize the context switch rate (inject + EIO overhead). My design effectively has "NAPI" for interrupt handling. This helps when the system needs it the most: heavy IO.

That sounds like a very useful concept in general, but this seems to be a detail of the interrupt controller implementation. If the IO-APIC cannot do what you want here, maybe we just need a paravirtual IRQ controller driver, like e.g. the PS3 has.

Quoted 2 lines

3) The 1:1 model is not buying us much in terms of hotplug. We don't really

"use" PCI very much even in virtio. Its a thin-shim of uniform dev-ids to resurface to the virtio-bus as something else. With LDM, hotplug is ridiculously easy anyway, so who cares. I already need an event channel anyway for (2b) anyway, so the devadd/devdrop events are trivial to handle.

I agree for Linux guests, but when you want to run other guest operating systems,
PCI hotplug is probably the most common interface for this. AFAIK, the windows
virtio-net driver does not at all have a concept of a virtio layer but is simply
a network driver for a PCI card. The same could be applied any other device, possibly with some library code doing all the queue handling in a common way.l

Quoted 2 lines

4) communicating with something efficiently in-kernel requires more finesse

than basic PIO/MMIO. There are tricks you can do to get around this, but with 1:1 you would have to do this trick repeatedly for each device. Even with a library solution to help, you still have per-cpu .data overhead and cpu hotplug overhead to get maximum performance. With my "bridge" model, I do it once, which I believe is ideal.

Quoted 2 lines

5) 1:1 is going to quickly populate the available MMIO/PIO and IDT slots for

any kind of medium to large configuration. The bridge model scales better in this regard.

We don't need to rely on PIO, it's just the common interface that all hypervisors
can easily support. We could have different underlying methods for the communication
if space or performance becomes a bottleneck because of this.

Quoted 2 lines

So based on that, I think the bridge model works better for vbus. Perhaps

you can convince me otherwise ;)

Being able to define all of it in the host kernel seems to be the major advantage of your approach, the other points you mentioned are less important IMHO. The question is whether that is indeed a worthy goal, or if it should just live in user space as with the qemu PCI code.

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

In essence, this driver's job is to populate the "vbus-proxy" LDM bus with objects that it finds across the PCI-OTHER bridge. This would actually sit below the virtio components in the stack, so it doesnt make sense (to me)

to

Quoted 10 lines

Quoted 6 lines

Quoted 4 lines

turn around and build this on top of virtio. But perhaps I am missing something you are seeing.

Can you elaborate?

Your PCI device does not serve any real purpose as far as I can tell

That is certainly debatable. Its purpose is as follows:

1) Allows a guest to discover the vbus feature (fwiw: I used to do this with

cpuid)

true, I missed that.

Quoted 1 line

2) Allows the guest to establish proper context to communicate with the

feature (mmio, pio, and msi) (fwiw: i used to use hypercalls)

Quoted 3 lines

3) Access the virtual-devices that have been configured for the feature

Correct me if I am wrong: Isn't this more of less the exact intent of

something like an LDM bus (vbus-proxy) and a PCI-BRIDGE? Other than the possibility that there might be some mergable overlap (still debatable), I don't think its fair to say that this does not serve a purpose.

I guess you are right on that. An interesting variation of that would be make the
child devices of it virtio devices again though: Instead of the PCI emulation code
in the host kernel, you could define a simpler interface to the same effect. So the
root device would be a virtio-pci device, below which you can have virtio-virtio
devices.

Quoted 2 lines

Quoted 2 lines

, you could just as well have a root device as a parent for all the vbus

devices

Quoted 3 lines

Quoted 1 line

if you do your device probing like this.

Yes, I suppose the "bridge" could have been advertised as a virtio-based root

device. In this way, the virtio probe() would replace my pci probe() for feature discovery, and a virtqueue could replace my msi+ioq for the eventq channel.

Quoted 4 lines

I see a few issues with that, however:

1) The virtqueue library, while a perfectly nice ring design at the metadata

level, does not have an API that is friendly to kernel-to-kernel communication.
It was designed more for frontend use to some remote backend. The IOQ
library on the other hand, was specifically designed to support use as kernel-to-kernel (see north/south designations). So this made life easier for me. To do what you propose, the eventq channel would need to terminate in kernel, and I would thus be forced to deal that the potential API problems.

Well, virtqueues are not that bad for kernel-to-kernel communication, as Ira mentioned
referring to his virtio-over-PCI driver. You can have virtqueues on both sides, having
the host kernel create a pair of virtqueues (one in user aka guest space, one in the host
kernel), with the host virtqueue_ops doing copy_{to,from}_user to move data between them.

If you have that, you can actually use the same virtio_net driver in both guest and
host kernel, just communicating over different virtio implementations. Interestingly,
that would mean that you no longer need a separation between guest and host device
drivers (vbus and vbus-proxy in your case) but could use the same device abstraction
with just different transports to back the shm-signal or virtqueue.

Quoted 2 lines

2) I would need to have Avi et. al. allocate a virtio vector to use from

their namespace, which I am sure they wont be willing to do until they accept my design. Today, I have a nice conflict free PCI ID to use as I see fit.

My impression is the opposite: as long as you try to reinvent everything at once,
you face opposition, but if you just improve parts of the existing design one
by one (like eventfd), I think you will find lots of support.

Quoted 1 line

Im sure both of these hurdles are not insurmountable, but I am left

scratching my head as to why its worth the effort. It seems to me its a "six of one, half-dozen of the other" kind of scenario. Either I write a qemu PCI device and pci-bridge driver, or I write a qemu virtio-devicve and virtio root driver.

Quoted 1 line

Quoted 1 line

In short: What does this buy us, or did you mean something else?

In my last reply, I was thinking of a root device that can not be probed like a PCI device.

Quoted 6 lines

Quoted 4 lines

However, assuming that you do the IMHO right thing to do probing like virtio with a PCI device for each slave, the code will be almost the same as virtio-pci and the two can be the same.

Can you elaborate?

Well, let me revise based on the discussion:

The main point that remains is that I think a vbus-proxy should be the same as a
virtio device. This could be done by having (as in my earlier mails) a PCI device
per vbus-proxy, with devcall implemented in PIO or config-space and additional shm/shm-signal,

or it could be a single virtio device from virtio-pci or one
of the other existing provides that connects you with a new virtio provider sitting in the host kernel. This provider has child devices for any endpoint (virtio-net, venet, ...) that is implemented in the host kernel.

Quoted 4 lines

Regarding the id->handle indirection:

Internally, the DEVOPEN call translates an "id" to a "handle". The handle

is just a token to help ensure that the caller actually opened the device successfully. Note that the "id" namespace is 0 based. Therefore, something like an errant DEVCALL(0) would be indistinguishable from a legit request. Using the handle abstraction gives me a slightly more robust mechanism to ensure the caller actually meant to call the host, and was in the proper context to do so. For one thing, if the device had never been opened, this would have failed before it ever reached the model. Its one more check I can do at the infrastructure level, and one less thing each model has to look out for.

Quoted 2 lines

Is the id->handle translation critical? No, i'm sure we could live without

it, but I also don't think it hurts anything. It allows the overall code to be slightly more robust, and the individual model code to be slightly less complicated. Therefore, I don't see a problem.

Right, assuming your model with all vbus devices behind a single PCI device, your
handle does not hurt, it's the equivalent of a bus/dev/fn number or an MMIO address.

Quoted 8 lines

Quoted 1 line

Quoted 1 line

Arnd Bergmann <[email protected]> wrote:

Quoted 4 lines · Gregory Haskins

2b) I also want to collapse multiple interrupts together so as to
minimize the context switch rate (inject + EIO overhead). My design effectively has "NAPI" for interrupt handling. This helps when the system needs it the most: heavy IO.

Quoted 4 lines

That sounds like a very useful concept in general, but this seems to be a detail of the interrupt controller implementation. If the IO-APIC cannot do what you want here, maybe we just need a paravirtual IRQ controller driver, like e.g. the PS3 has.

Yeah, I agree this could be a function of the APIC code. Do note that I mentioned this in passing to Avi a few months ago but FWIW he indicated at that time that he is not interested in making the APIC PV.

Also, I almost forgot an important one. Add:

2c) Interrupt prioritization. I want to be able to assign priority to interrupts and handle them in priority order.

Quoted 1 line

Quoted 1 line

On 8/7/2009 at 10:57 AM, in message <[email protected]>, Arnd

Quoted 14 lines · Gregory Haskins

Quoted 7 lines

Quoted 1 line

Quoted 1 line

Arnd Bergmann <[email protected]> wrote:

Quoted 3 lines · Gregory Haskins

2b) I also want to collapse multiple interrupts together so as to
minimize the context switch rate (inject + EIO overhead). My design effectively has "NAPI" for interrupt handling. This helps when the system

Quoted 6 lines

Quoted 1 line

needs it the most: heavy IO.

That sounds like a very useful concept in general, but this seems to be a detail of the interrupt controller implementation. If the IO-APIC cannot do what you want here, maybe we just need a paravirtual IRQ controller driver, like e.g. the PS3 has.

Quoted 8 lines

Yeah, I agree this could be a function of the APIC code. Do note that I mentioned this in passing to Avi a few months ago but FWIW he indicated at that time that he is not interested in making the APIC PV.

Also, I almost forgot an important one. Add:

2c) Interrupt prioritization. I want to be able to assign priority to interrupts and handle them in priority order.

I think this part of the interface has developed into the wrong direction because you confused two questions:

1. should you build an advanced interrupt mechanism for virtual drivers?
2. how should you build an advanced interrupt mechanism for virtual drivers?

My guess is that when Avi said he did not want a paravirtual IO-APIC, he implied that the existing one is good enough (maybe Avi can clarify that point himself) answering question 1, while you took that as an indication that the code should live elsewhere instead, answering question 2.

What you built with the shm-signal code is essentially a paravirtual nested interrupt controller by another name, and deeply integrated into a new bigger subsystem. I believe that this has significant disadvantages over the approach of making it a standard interrupt controller driver:

* It completely avoids the infrastructure that we have built into Linux to deal with interrupts, e.g. /proc/interrupts statistics, IRQ balancing and CPU affinity.

* It makes it impossible to quantify the value of the feature to start with, which could be used to answer question 1 above.

* Less importantly, it does not work with any other drivers that might also benefit from a new interrupt controller -- if it is indeed better than the one we already have.

	Arnd <><

Quoted 1 line

Quoted 1 line

Quoted 1 line

On 8/6/2009 at 6:57 PM, in message <[email protected]>, Arnd

Bergmann <[email protected]> wrote:

Quoted 11 lines

Quoted 3 lines

I see a few issues with that, however:

1) The virtqueue library, while a perfectly nice ring design at the metadata

level, does not have an API that is friendly to kernel-to-kernel communication.
It was designed more for frontend use to some remote backend. The IOQ
library on the other hand, was specifically designed to support use as kernel-to-kernel (see north/south designations). So this made life easier for me. To do what you propose, the eventq channel would need to terminate in kernel, and I would thus be forced to deal that the potential API problems.

Quoted 8 lines

Well, virtqueues are not that bad for kernel-to-kernel communication, as Ira mentioned
referring to his virtio-over-PCI driver. You can have virtqueues on both sides, having
the host kernel create a pair of virtqueues (one in user aka guest space, one in the host
kernel), with the host virtqueue_ops doing copy_{to,from}_user to move data between them.

Its been a while since I looked, so perhaps I am wrong here. I will look again.

Quoted 10 lines

If you have that, you can actually use the same virtio_net driver in both guest and
host kernel, just communicating over different virtio implementations. Interestingly,
that would mean that you no longer need a separation between guest and host device
drivers (vbus and vbus-proxy in your case) but could use the same device abstraction
with just different transports to back the shm-signal or virtqueue.

Actually, I think there are some problems with that model (such as management of the interface). virtio-net really wants to connect to a virtio-net-backend (such as the one in qemu or vbus). It wasn't designed to connect back to back like that. I think you will quickly run into problems similar to what Ira faced with virtio-over-PCI with that model.

With my powerpc hardware, it should be easily possible to have at least 6 devices, each with two virtqueues, one for tx and one for rx. (The limit is caused by the amount of distinct kick() events I can generate.) This could allow many combinations of devices, such as:

* 1 virtio-net, 1 virtio-console
* 3 virtio-net, 2 virtio-console
* 6 virtio-net
* etc.

drivers/net/vbus-enet.c line 548

Quoted 9 lines · On Mon, 03 Aug 2009 13:18:02 -0400 Gregory Haskins <[email protected]> wrote:

drivers/net/vbus-enet.c new file +9 -0

+
+static const struct net_device_ops vbus_enet_netdev_ops = {
+	.ndo_open          = vbus_enet_open,
+	.ndo_stop          = vbus_enet_stop,
+	.ndo_set_config    = vbus_enet_config,
+	.ndo_start_xmit    = vbus_enet_tx_start,
+	.ndo_change_mtu	   = vbus_enet_change_mtu,
+	.ndo_tx_timeout    = vbus_enet_timeout,
+};

Missing
.ndo_set_mac_address = eth_mac_addr,
.ndo_validate_addr = eth_validate_addr,

Also, should have change_mtu.

Quoted 18 lines · Stephen Hemminger

drivers/net/vbus-enet.c line 548

Quoted 9 lines · On Mon, 03 Aug 2009 13:18:02 -0400 Gregory Haskins <[email protected]> wrote:

drivers/net/vbus-enet.c new file +9 -0

+
+static const struct net_device_ops vbus_enet_netdev_ops = {
+	.ndo_open          = vbus_enet_open,
+	.ndo_stop          = vbus_enet_stop,
+	.ndo_set_config    = vbus_enet_config,
+	.ndo_start_xmit    = vbus_enet_tx_start,
+	.ndo_change_mtu	   = vbus_enet_change_mtu,
+	.ndo_tx_timeout    = vbus_enet_timeout,
+};

Missing
.ndo_set_mac_address = eth_mac_addr,
.ndo_validate_addr = eth_validate_addr,

Ack.

Quoted 1 line

Also, should have change_mtu.

note that I do have .ndo_change_mtu. I assume this is what you are referring to and just missed it. If there is something else I need there, let me know.

Quoted 18 lines · On Mon, 03 Aug 2009 16:10:37 -0400 Gregory Haskins <[email protected]> wrote:

Quoted 17 lines · Stephen Hemminger

drivers/net/vbus-enet.c line 548

Quoted 9 lines · On Mon, 03 Aug 2009 13:18:02 -0400 Gregory Haskins <[email protected]> wrote:

drivers/net/vbus-enet.c new file +9 -0

+
+static const struct net_device_ops vbus_enet_netdev_ops = {
+	.ndo_open          = vbus_enet_open,
+	.ndo_stop          = vbus_enet_stop,
+	.ndo_set_config    = vbus_enet_config,
+	.ndo_start_xmit    = vbus_enet_tx_start,
+	.ndo_change_mtu	   = vbus_enet_change_mtu,
+	.ndo_tx_timeout    = vbus_enet_timeout,
+};

Missing
.ndo_set_mac_address = eth_mac_addr,
.ndo_validate_addr = eth_validate_addr,

Quoted 6 lines

Ack.

Quoted 1 line

Also, should have change_mtu.

note that I do have .ndo_change_mtu. I assume this is what you are referring to and just missed it. If there is something else I need there, let me know.

If you don't have a change_mtu, then MTU is unlimited.

Can the device handle 64K or larger transfers?

Quoted 28 lines · Stephen Hemminger

Quoted 24 lines · On Mon, 03 Aug 2009 16:10:37 -0400 Gregory Haskins <[email protected]> wrote:

Quoted 17 lines · Stephen Hemminger

drivers/net/vbus-enet.c line 548

Quoted 9 lines · On Mon, 03 Aug 2009 13:18:02 -0400 Gregory Haskins <[email protected]> wrote:

drivers/net/vbus-enet.c new file +9 -0

+
+static const struct net_device_ops vbus_enet_netdev_ops = {
+	.ndo_open          = vbus_enet_open,
+	.ndo_stop          = vbus_enet_stop,
+	.ndo_set_config    = vbus_enet_config,
+	.ndo_start_xmit    = vbus_enet_tx_start,
+	.ndo_change_mtu	   = vbus_enet_change_mtu,
+	.ndo_tx_timeout    = vbus_enet_timeout,
+};

Missing
.ndo_set_mac_address = eth_mac_addr,
.ndo_validate_addr = eth_validate_addr,

Ack.

Quoted 1 line

Also, should have change_mtu.

note that I do have .ndo_change_mtu. I assume this is what you are referring to and just missed it. If there is something else I need there, let me know.

Quoted 1 line

If you don't have a change_mtu, then MTU is unlimited.

Is "change_mtu" different from .ndo_change_mtu" on the ndo struct? That's whats confusing me, as I have the .ndo one already. Is there something else I need in addition, or should I be ok as is?

net: Add vbus_enet driver

A virtualized 802.x network device based on the VBUS interface. It can be used with any hypervisor/kernel that supports the virtual-ethernet/vbus protocol.

Signed-off-by Gregory Haskins <[email protected]>

From: Gregory Haskins <[email protected]>
Date: Mon, 03 Aug 2009 21:14:04 -0400

Quoted 6 lines

net: Add vbus_enet driver

A virtualized 802.x network device based on the VBUS interface. It can be used with any hypervisor/kernel that supports the virtual-ethernet/vbus protocol.

Quoted 1 line

Signed-off-by Gregory Haskins <[email protected]>

I'm fine with this, it depends upon the vbus infrastructure so whoever pulls that in can pull this in as well:

Acked-by David S. Miller <[email protected]>

SG/GSO significantly enhance the performance of network traffic under certain circumstances. We implement this feature as a separate patch to avoid intially complicating the baseline venet driver. This will presumably make the review process slightly easier, since we can focus on the basic interface first.

Signed-off-by Gregory Haskins <[email protected]>

Quoted 6 lines · On Mon, 03 Aug 2009 13:18:07 -0400 Gregory Haskins <[email protected]> wrote:

SG/GSO significantly enhance the performance of network traffic under certain circumstances. We implement this feature as a separate patch to avoid intially complicating the baseline venet driver. This will presumably make the review process slightly easier, since we can focus on the basic interface first.

Quoted 1 line

Signed-off-by Gregory Haskins <[email protected]>

Never mind, previous comment about adding TSO.

But it still would be good to have ethtool interface for this.

drivers/net/vbus-enet.c struct vbus_enet_priv { · line 69

Quoted 7 lines · On Mon, 03 Aug 2009 13:18:07 -0400 Gregory Haskins <[email protected]> wrote:

drivers/net/vbus-enet.c +7 -0

+	struct {
+		int                sg:1;
+		int                tso:1;
+		int                ufo:1;
+		int                tso6:1;
+		int                ecn:1;
+	} flags;

Why do you have to shadow flags that are already available in net_device? It is bad design to replicate state in a device driver. The problem with replicated state is that it has to be updated in both places.

(Applies to v2.6.31-rc5, proposed for linux-next after review is complete)

This series implements the guest-side drivers for accelerated IO when running on top of the AlacrityVM hypervisor, the details of which you can find here:

http://developer.novell.com/wiki/index.php/AlacrityVM

This series includes the basic plumbing, as well as the driver for accelerated 802.x (ethernet) networking.

Regards,
-Greg

---

Gregory Haskins (7):
venet: add scatter-gather/GSO support
net: Add vbus_enet driver
ioq: add driver-side vbus helpers
vbus-proxy: add a pci-to-vbus bridge
vbus: add a "vbus-proxy" bus model for vbus_driver objects
ioq: Add basic definitions for a shared-memory, lockless queue
shm-signal: shared-memory signals

 arch/x86/Kconfig            |    2 
 drivers/Makefile            |    1 
 drivers/net/Kconfig         |   14 +
 drivers/net/Makefile        |    1 
 drivers/net/vbus-enet.c     |  899 +++++++++++++++++++++++++++++++++++++++++++
 drivers/vbus/Kconfig        |   24 +
 drivers/vbus/Makefile       |    6 
 drivers/vbus/bus-proxy.c    |  216 ++++++++++
 drivers/vbus/pci-bridge.c   |  824 +++++++++++++++++++++++++++++++++++++++
 include/linux/Kbuild        |    4 
 include/linux/ioq.h         |  415 ++++++++++++++++++++
 include/linux/shm_signal.h  |  189 +++++++++
 include/linux/vbus_driver.h |   80 ++++
 include/linux/vbus_pci.h    |  127 ++++++
 include/linux/venet.h       |   84 ++++
 lib/Kconfig                 |   21 +
 lib/Makefile                |    2 
 lib/ioq.c                   |  294 ++++++++++++++
 lib/shm_signal.c            |  192 +++++++++
 19 files changed, 3395 insertions(+), 0 deletions(-)
 create mode 100644 drivers/net/vbus-enet.c
 create mode 100644 drivers/vbus/Kconfig
 create mode 100644 drivers/vbus/Makefile
 create mode 100644 drivers/vbus/bus-proxy.c
 create mode 100644 drivers/vbus/pci-bridge.c
 create mode 100644 include/linux/ioq.h
 create mode 100644 include/linux/shm_signal.h
 create mode 100644 include/linux/vbus_driver.h
 create mode 100644 include/linux/vbus_pci.h
 create mode 100644 include/linux/venet.h
 create mode 100644 lib/ioq.c
 create mode 100644 lib/shm_signal.c

Signature · 5 lines

Signature
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Quoted 1 line · Gregory Haskins

(Applies to v2.6.31-rc5, proposed for linux-next after review is complete)

These are guest drivers, right? Merging the guest first means relying on kernel interface from an out of tree driver, which well might change before it goes in. Would it make more sense to start merging with the host side of the project?

Quoted 5 lines

This series implements the guest-side drivers for accelerated IO when running on top of the AlacrityVM hypervisor, the details of which you can find here:

http://developer.novell.com/wiki/index.php/AlacrityVM

Since AlacrityVM is kvm based, Cc [email protected].

Quoted 2 lines

This series includes the basic plumbing, as well as the driver for accelerated 802.x (ethernet) networking.

The graphs comparing virtio with vbus look interesting. However, they do not compare apples to apples, do they? These compare userspace virtio with kernel vbus, where for apples to apples comparison one would need to compare kernel virtio with kernel vbus. Right?

Quoted 54 lines

Regards,
-Greg

---

Gregory Haskins (7):
venet: add scatter-gather/GSO support
net: Add vbus_enet driver
ioq: add driver-side vbus helpers
vbus-proxy: add a pci-to-vbus bridge
vbus: add a "vbus-proxy" bus model for vbus_driver objects
ioq: Add basic definitions for a shared-memory, lockless queue
shm-signal: shared-memory signals

 arch/x86/Kconfig            |    2 
 drivers/Makefile            |    1 
 drivers/net/Kconfig         |   14 +
 drivers/net/Makefile        |    1 
 drivers/net/vbus-enet.c     |  899 +++++++++++++++++++++++++++++++++++++++++++
 drivers/vbus/Kconfig        |   24 +
 drivers/vbus/Makefile       |    6 
 drivers/vbus/bus-proxy.c    |  216 ++++++++++
 drivers/vbus/pci-bridge.c   |  824 +++++++++++++++++++++++++++++++++++++++
 include/linux/Kbuild        |    4 
 include/linux/ioq.h         |  415 ++++++++++++++++++++
 include/linux/shm_signal.h  |  189 +++++++++
 include/linux/vbus_driver.h |   80 ++++
 include/linux/vbus_pci.h    |  127 ++++++
 include/linux/venet.h       |   84 ++++
 lib/Kconfig                 |   21 +
 lib/Makefile                |    2 
 lib/ioq.c                   |  294 ++++++++++++++
 lib/shm_signal.c            |  192 +++++++++
 19 files changed, 3395 insertions(+), 0 deletions(-)
 create mode 100644 drivers/net/vbus-enet.c
 create mode 100644 drivers/vbus/Kconfig
 create mode 100644 drivers/vbus/Makefile
 create mode 100644 drivers/vbus/bus-proxy.c
 create mode 100644 drivers/vbus/pci-bridge.c
 create mode 100644 include/linux/ioq.h
 create mode 100644 include/linux/shm_signal.h
 create mode 100644 include/linux/vbus_driver.h
 create mode 100644 include/linux/vbus_pci.h
 create mode 100644 include/linux/venet.h
 create mode 100644 lib/ioq.c
 create mode 100644 lib/shm_signal.c

Signature · 5 lines

Signature
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Quoted 1 line

Quoted 1 line

On 8/6/2009 at 6:17 AM, in message <[email protected]>,

Quoted 19 lines · Michael S. Tsirkin

Quoted 1 line · Gregory Haskins

(Applies to v2.6.31-rc5, proposed for linux-next after review is complete)

These are guest drivers, right? Merging the guest first means relying on kernel interface from an out of tree driver, which well might change before it goes in. Would it make more sense to start merging with the host side of the project?

Quoted 5 lines

This series implements the guest-side drivers for accelerated IO when running on top of the AlacrityVM hypervisor, the details of which you can find here:

http://developer.novell.com/wiki/index.php/AlacrityVM

Since AlacrityVM is kvm based, Cc [email protected].

Quoted 2 lines

This series includes the basic plumbing, as well as the driver for accelerated 802.x (ethernet) networking.

Quoted 5 lines

The graphs comparing virtio with vbus look interesting. However, they do not compare apples to apples, do they? These compare userspace virtio with kernel vbus, where for apples to apples comparison one would need to compare kernel virtio with kernel vbus. Right?

Or userspace virtio with userspace vbus.

Quoted 1 line

Quoted 1 line

On 8/6/2009 at 4:19 AM, in message <[email protected]>,

Quoted 1 line · Gregory Haskins

(Applies to v2.6.31-rc5, proposed for linux-next after review is complete)

These are guest drivers, right?

Merging the guest first means relying on
kernel interface from an out of tree driver, which well might change before it goes in.

Would it make more sense to start merging with the host side of the project?

Quoted 6 lines

This series implements the guest-side drivers for accelerated IO when running on top of the AlacrityVM hypervisor, the details of which you can find here:

http://developer.novell.com/wiki/index.php/AlacrityVM

Since AlacrityVM is kvm based, Cc [email protected].

Quoted 3 lines

This series includes the basic plumbing, as well as the driver for accelerated 802.x (ethernet) networking.

The graphs comparing virtio with vbus look interesting. However, they do not compare apples to apples, do they?

These compare userspace virtio with kernel vbus,

where for apples to apples comparison one would need to compare kernel virtio with kernel vbus. Right?

Hi Michael,

Quoted 1 line

Quoted 1 line

Quoted 1 line

On 8/6/2009 at 4:19 AM, in message <[email protected]>,

Quoted 4 lines · Michael S. Tsirkin

Quoted 1 line · Gregory Haskins

(Applies to v2.6.31-rc5, proposed for linux-next after review is complete)

These are guest drivers, right?

Yep.

Quoted 3 lines

Merging the guest first means relying on
kernel interface from an out of tree driver, which well might change before it goes in.

ABI compatibility is already addressed/handled, so even if that is true its not a problem.

Quoted 1 line

Would it make more sense to start merging with the host side of the project?

Not necessarily, no. These are drivers for a "device", so its no different than merging any other driver really. This is especially true since the hypervisor is also already published and freely available today, so anyone can start using it.

Quoted 1 line

Quoted 1 line

On 8/6/2009 at 8:24 AM, in message <[email protected]>,

Quoted 9 lines · Gregory Haskins

Hi Michael,

Quoted 1 line

Quoted 1 line

Quoted 1 line

On 8/6/2009 at 4:19 AM, in message <[email protected]>,

Quoted 4 lines · Michael S. Tsirkin

Quoted 1 line · Gregory Haskins

(Applies to v2.6.31-rc5, proposed for linux-next after review is complete)

These are guest drivers, right?

Quoted 7 lines

Yep.

Quoted 3 lines

Merging the guest first means relying on
kernel interface from an out of tree driver, which well might change before it goes in.

ABI compatibility is already addressed/handled, so even if that is true its

not a problem.

It is? With versioning? Presumably this:

Even assuming host even knows how to decode this structure (e.g. some other host module doesn't use VBUS_PCI_HC_DEVOPEN),

checks the version
and denies older guests, this might help guest not to crash, but guest still won't work.

Quoted 2 lines

Quoted 2 lines

Would it make more sense to start merging with the host side of the

project?

Quoted 1 line

Quoted 4 lines

Not necessarily, no. These are drivers for a "device", so its no different than merging any other driver really. This is especially true since the hypervisor is also already published and freely available today, so anyone can start using it.

The difference is clear to me: devices do not get to set kernel/userspace interfaces. This "device" depends on a specific interface between kernel and (guest) userspace.

Quoted 4 lines

Merging the guest first means relying on
kernel interface from an out of tree driver, which well might change before it goes in.

ABI compatibility is already addressed/handled, so even if that is true its not a problem.

Quoted 4 lines

Quoted 4 lines

This series implements the guest-side drivers for accelerated IO when running on top of the AlacrityVM hypervisor, the details of which you can find here:

Quoted 4 lines

Quoted 2 lines

http://developer.novell.com/wiki/index.php/AlacrityVM

Since AlacrityVM is kvm based, Cc [email protected].

I *can* do that, but there is nothing in these drivers that is KVM specific (its all pure PCI and VBUS). I've already made the general announcement about the project/ml cross posted to KVM for anyone that might be interested, but I figure I will spare the general KVM list the details unless something specifically pertains to, or affects, KVM. For instance, when I get to pushing the hypervisor side, I still need to work on getting that 'xinterface' patch to you guys. I would certainly be CC'ing kvm@vger when that happens since it modifies the KVM code.

So instead, I would just encourage anyone interested (such as yourself) to join the alacrity list so I don't bother the KVM community unless absolutely necessary.

Quoted 1 line

Quoted 1 line

On 8/6/2009 at 8:54 AM, in message <[email protected]>, Avi Kivity

Quoted 6 lines · Gregory Haskins

Quoted 4 lines

Merging the guest first means relying on
kernel interface from an out of tree driver, which well might change before it goes in.

ABI compatibility is already addressed/handled, so even if that is true its

not a problem.

Quoted 2 lines

Really the correct way to address the ABI is to publish a spec and write both host and guest drivers to that. Unfortunately we didn't do this with virtio.

It becomes more important when you have multiple implementations (e.g. Windows drivers).

Quoted 10 lines

Quoted 8 lines

Quoted 6 lines

This series implements the guest-side drivers for accelerated IO when running on top of the AlacrityVM hypervisor, the details of which you can find here:

http://developer.novell.com/wiki/index.php/AlacrityVM

Since AlacrityVM is kvm based, Cc [email protected].

I *can* do that, but there is nothing in these drivers that is KVM specific

(its all pure PCI and VBUS). I've already made the general announcement about the project/ml cross posted to KVM for anyone that might be interested, but I figure I will spare the general KVM list the details unless something specifically pertains to, or affects, KVM. For instance, when I get to pushing the hypervisor side, I still need to work on getting that 'xinterface' patch to you guys. I would certainly be CC'ing kvm@vger when that happens since it modifies the KVM code.

Quoted 1 line

Quoted 1 line

So instead, I would just encourage anyone interested (such as yourself) to

join the alacrity list so I don't bother the KVM community unless absolutely necessary.

Quoted 1 line

It's true that vbus is a separate project (in fact even virtio is completely separate from kvm). Still I think it would be of interest to many kvm@ readers.

Quoted 4 lines

It's true that vbus is a separate project (in fact even virtio is completely separate from kvm). Still I think it would be of interest to many kvm@ readers.

Well, my goal was to not annoy KVM readers. ;) So if you feel as though there is benefit to having all of KVM CC'd and I won't be annoying everyone, I see no problem in cross posting.

Would you like to see all conversations, or just ones related to code (and, of course, KVM relevant items)

Quoted 1 line

Quoted 1 line

On 8/6/2009 at 9:44 AM, in message <[email protected]>, Avi Kivity

Quoted 5 lines · Gregory Haskins

Quoted 4 lines

It's true that vbus is a separate project (in fact even virtio is completely separate from kvm). Still I think it would be of interest to many kvm@ readers.

Quoted 1 line

Well, my goal was to not annoy KVM readers. ;) So if you feel as though

there is benefit to having all of KVM CC'd and I won't be annoying everyone, I see no problem in cross posting.

Quoted 1 line

I can only speak for myself, I'm interested in this project

(though still rooting for virtio).

Quoted 2 lines

Would you like to see all conversations, or just ones related to code (and,

of course, KVM relevant items)

I guess internal vbus changes won't be too interesting for most readers, but new releases, benchmarks, and kvm-related stuff will be welcome on the kvm list.

Quoted 3 lines

(though still rooting for virtio).

Heh...not to belabor the point to death, but virtio is orthogonal (you keep forgetting that ;).

Its really the vbus device-model vs the qemu device-model (and possibly vs the "in-kernel pci emulation" model that I believe Michael is working on).

You can run virtio on any of those three.

Quoted 1 line

Quoted 1 line

On 8/6/2009 at 9:57 AM, in message <[email protected]>, Avi Kivity

Quoted 5 lines · Gregory Haskins

Quoted 3 lines

(though still rooting for virtio).

Heh...not to belabor the point to death, but virtio is orthogonal (you keep

forgetting that ;).

Quoted 2 lines

Its really the vbus device-model vs the qemu device-model (and possibly vs the

"in-kernel pci emulation" model that I believe Michael is working on).

Quoted 1 line

Quoted 2 lines

You can run virtio on any of those three.

It's not orthogonal. virtio is one set of ABI+guest drivers+host support to get networking on kvm guests. AlacrityVM's vbus-based drivers are another set of ABI+guest drivers+host support to get networking on kvm guests. That makes them competitors (two different ways to do one thing), not orthogonal.

We can exchange out the "virtio-pci" module like this:

(guest-side)

|--------------------------
| virtio-net
|--------------------------
| virtio-ring
|--------------------------
| virtio-bus
|--------------------------
| virtio-vbus
|--------------------------
| vbus-proxy
|--------------------------
| vbus-connector
|--------------------------
                      |
                   (vbus)
                      |
|--------------------------
| kvm.ko
|--------------------------
| vbus-connector
|--------------------------
| vbus
|--------------------------
| virtio-net-tap (vbus model)
|--------------------------
| netif
|--------------------------
     (host-side)

So virtio-net runs unmodified. What is "competing" here is "virtio-pci" vs "virtio-vbus". Also, venet vs virtio-net are technically competing. But to say "virtio vs vbus" is inaccurate, IMO.

3. The ioq method seems to be the real core of your work that makes venet perform better than virtio-net with its virtqueues. I don't see any reason to doubt that your claim is correct. My conclusion from this would be to add support for ioq to virtio devices, alongside virtqueues, but to leave out the extra bus_type and probing method.

Quoted 1 line

Quoted 1 line

On 8/6/2009 at 11:50 AM, in message <[email protected]>, Avi Kivity

Quoted 6 lines · Arnd Bergmann

3. The ioq method seems to be the real core of your work that makes venet perform better than virtio-net with its virtqueues. I don't see any reason to doubt that your claim is correct. My conclusion from this would be to add support for ioq to virtio devices, alongside virtqueues, but to leave out the extra bus_type and probing method.

The current conjecture is that ioq outperforms virtio because the host side of ioq is implemented in the host kernel, while the host side of virtio is implemented in userspace. AFAIK, no one pointed out differences in the protocol which explain the differences in performance.

Based on this, I will continue my efforts surrounding to use of vbus including its use to accelerate KVM for AlacrityVM. If I can find a way to do this in such a way that KVM upstream finds acceptable, I would be very happy and will work towards whatever that compromise might be. OTOH, if the KVM community is set against the concept of a generalized/shared backend, and thus wants to use some other approach that does not involve vbus, that is fine too. Choice is one of the great assets of open source, eh? :)

3. The ioq method seems to be the real core of your work that makes venet perform better than virtio-net with its virtqueues. I don't see any reason to doubt that your claim is correct. My conclusion from this would be to add support for ioq to virtio devices, alongside virtqueues, but to leave out the extra bus_type and probing method.

	Arnd <><

How hard would it be to implement virtio over vbus and perhaps the virtio-net backend?

This would leave only one variable in the comparison, clear misconceptions and make evaluation easier by judging each of vbus, venet etc separately on its own merits.

The way things are now, it is unclear exactly where those performance improvements are coming from (or how much each component contributes) because there are too many variables.

Replacing virtio-net by venet would be a hard proposition if only because virtio-net has (closed source) windows drivers available. There has to be shown that venet by itself does something significantly better that virtio-net can't be modified to do comparably well.

Having venet in addition to virtio-net is also difficult, given that having only one set of paravirtual drivers in the kernel was the whole point behind virtio.

Quoted 1 line

Quoted 1 line

On 8/6/2009 at 11:40 AM, in message <[email protected]>, Arnd

Quoted 33 lines · Gregory Haskins

We can exchange out the "virtio-pci" module like this:

(guest-side)

|--------------------------
| virtio-net
|--------------------------
| virtio-ring
|--------------------------
| virtio-bus
|--------------------------
| virtio-vbus
|--------------------------
| vbus-proxy
|--------------------------
| vbus-connector
|--------------------------
                      |
                   (vbus)
                      |
|--------------------------
| kvm.ko
|--------------------------
| vbus-connector
|--------------------------
| vbus
|--------------------------
| virtio-net-tap (vbus model)
|--------------------------
| netif
|--------------------------
     (host-side)

Quoted 1 line

So virtio-net runs unmodified. What is "competing" here is "virtio-pci" vs

"virtio-vbus".

Quoted 1 line

Also, venet vs virtio-net are technically competing. But to say "virtio vs

vbus" is inaccurate, IMO.

I think what's confusing everyone is that you are competing on multiple issues:

1. Implementation of bus probing: both vbus and virtio are backed by PCI devices and can be backed by something else (e.g. virtio by lguest or even by vbus).

2. Exchange of metadata: virtio uses a config space, vbus uses devcall to do the same.

3. User data transport: virtio has virtqueues, vbus has shm/ioq.

I think these three are the main differences, and the venet vs. virtio-net question comes down to which interface the drivers use for each aspect. Do you agree with this interpretation?

Now to draw conclusions from each of these is of course highly subjective, but this is how I view it:

1. The bus probing is roughly equivalent, they both work and the virtio method seems to need a little less code but that could be fixed by slimming down the vbus code as I mentioned in my comments on the pci-to-vbus bridge code. However, I would much prefer not to have both of them, and virtio came first.

2. the two methods (devcall/config space) are more or less equivalent and you should be able to implement each one through the other one. The virtio design was driven by making it look similar to PCI, the vbus design was driven by making it easy to implement in a host kernel. I don't care too much about these, as they can probably coexist without causing any trouble. For a (hypothetical) vbus-in-virtio device, a devcall can be a config-set/config-get pair, for a virtio-in-vbus, you can do a config-get and a config-set devcall and be happy. Each could be done in a trivial helper library.

3. The ioq method seems to be the real core of your work that makes venet perform better than virtio-net with its virtqueues. I don't see any reason to doubt that your claim is correct. My conclusion from this would be to add support for ioq to virtio devices, alongside virtqueues, but to leave out the extra bus_type and probing method.

Quoted 1 line

Quoted 1 line

Quoted 1 line

On 8/6/2009 at 11:40 AM, in message <[email protected]>, Arnd

Quoted 1 line · Bergmann

On Thursday 06 August 2009, Gregory Haskins wrote:

Quoted 6 lines

3. The ioq method seems to be the real core of your work that makes venet perform better than virtio-net with its virtqueues. I don't see any reason to doubt that your claim is correct. My conclusion from this would be to add support for ioq to virtio devices, alongside virtqueues, but to leave out the extra bus_type and probing method.

While I appreciate the sentiment, I doubt that is actually whats helping here.

There are a variety of factors that I poured into venet/vbus that I think contribute to its superior performance. However, the difference in the ring design I do not think is one if them. In fact, in many ways I think Rusty's design might turn out to be faster if put side by side because he was much more careful with cacheline alignment than I was. Also note that I was careful to not pick one ring vs the other ;) They both should work.

IMO, we are only looking at the tip of the iceberg when looking at this purely as the difference between virtio-pci vs virtio-vbus, or venet vs virtio-net.

Really, the big thing I am working on here is the host side device-model. The idea here was to design a bus model that was conducive to high performance, software to software IO that would work in a variety of environments (that may or may not have PCI). KVM is one such environment, but I also have people looking at building other types of containers, and even physical systems (host+blade kind of setups).

The idea is that the "connector" is modular, and then something like virtio-net or venet "just work": in kvm, in the userspace container, on the blade system.

It provides a management infrastructure that (hopefully) makes sense for these different types of containers, regardless of whether they have PCI, QEMU, etc (e.g. things that are inherent to KVM, but not others).

I hope this helps to clarify the project :)

Quoted 1 line

(though still rooting for virtio).

Heh...not to belabor the point to death, but virtio is orthogonal (you keep forgetting that ;).

Quoted 1 line

Quoted 1 line

On 8/6/2009 at 9:59 AM, in message <[email protected]>,

Quoted 3 lines · Gregory Haskins

Quoted 1 line

(though still rooting for virtio).

Heh...not to belabor the point to death, but virtio is orthogonal (you keep

forgetting that ;).

venet and virtio aren't orthogonal, are they?

Quoted 4 lines

This series includes the basic plumbing, as well as the driver for accelerated 802.x (ethernet) networking.

The graphs comparing virtio with vbus look interesting.

Quoted 5 lines · Michael S. Tsirkin

Quoted 4 lines

This series includes the basic plumbing, as well as the driver for accelerated 802.x (ethernet) networking.

Quoted 2 lines

The graphs comparing virtio with vbus look interesting.

1gbit throughput on a 10gbit link? I have a hard time believing that.

I've seen much higher myself. Can you describe your test setup in more detail?

That said, note that the graphs were from earlier kernel runs (2.6.28, 29-rc8). The most recent data I can find that I published is for 2.6.29, announced with the vbus-v3 release back in April:

http://lkml.org/lkml/2009/4/21/408

In it, the virtio-net throughput numbers are substantially higher and possibly more in line with your expectations (4.5gb/s) (though notably still lagging venet, which weighed in at 5.6gb/s).

Generally, I find that the virtio-net exhibits non-deterministic results from release to release. I suspect (as we have discussed) the tx-mitigation scheme. Some releases buffer the daylights out of the stream, and virtio gets close(r) throughput (e.g. 4.5g vs 5.8g, but absolutely terrible latency (4000us vs 65us). Other releases it seems to operate with more of a compromise (1.3gb/s vs 3.8gb/s, but 350us vs 85us).

If there is another patch-series/tree I should be using for comparison, please point me at it.

Quoted 6 lines · Gregory Haskins

That said, note that the graphs were from earlier kernel runs (2.6.28, 29-rc8). The most recent data I can find that I published is for 2.6.29, announced with the vbus-v3 release back in April:

http://lkml.org/lkml/2009/4/21/408

Quoted 4 lines

In it, the virtio-net throughput numbers are substantially higher and possibly more in line with your expectations (4.5gb/s) (though notably still lagging venet, which weighed in at 5.6gb/s).

Okay, that makes more sense. Would be nice to update the graphs as they make virtio look really, really bad :-)

Quoted 9 lines

Generally, I find that the virtio-net exhibits non-deterministic results from release to release. I suspect (as we have discussed) the tx-mitigation scheme. Some releases buffer the daylights out of the stream, and virtio gets close(r) throughput (e.g. 4.5g vs 5.8g, but absolutely terrible latency (4000us vs 65us). Other releases it seems to operate with more of a compromise (1.3gb/s vs 3.8gb/s, but 350us vs 85us).

Are you using kvm modules or a new kernel?

There was some timer
infrastructure changes around 28/29 and it's possible that the system your on is now detecting an hpet which will result in a better time source. That could have an affect on mitigation.

Quoted 4 lines

If there is another patch-series/tree I should be using for comparison, please point me at it.

No, I think it's fair to look at upstream Linux. Looking at the latest bits would be nice though because there are some virtio friendly changes recently like MSI-x and GRO.