Skip to contentSkip to conversation

virtual-bus

RFC PATCH Superseded Gregory Haskins 9 participants 49 messages Apr 9, 2009 – Aug 10, 2009

Subsystems (13)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

v2 cover virtual-bus

This is release v2. Changes since v1:

*) Incorporated review feedback from Stephen Hemminger on vbus-enet driver *) Added support for connecting to vbus devices from userspace *) Added support for a virtio-vbus transport to allow virtio drivers to
work with vbus (needs testing and backend models).

(Avi, I know I still owe you a reply re the PCI debate)

Todo:
*) Develop some kind of hypercall registration mechanism for KVM so that
we can use that as an integration point instead of directly hooking kvm hypercalls
*) Beef up the userspace event channel ABI to support different event types *) Add memory-registration support
*) Integrate with qemu PCI device model to render vbus objects as PCI *) Develop some virtio backend devices.
*) Support ethtool_ops for venet.

---------------------------------------

RFC: Virtual-bus

applies to v2.6.29 (will port to git HEAD soon)

FIRST OFF: Let me state that this is _not_ a KVM or networking specific technology. Virtual-Bus is a mechanism for defining and deploying software “devices” directly in a Linux kernel. These devices are designed to be directly accessed from a variety of environments in an arbitrarly nested fashion. The goal is provide for the potential for maxium IO performance by providing the shortest and most efficient path to the "bare metal" kernel, and thus the actual IO resources. For instance, an application can be written to run the same on baremetal as it does in guest userspace nested 10 levels deep, all the while providing direct access to the resource, thus reducing latency and boosting throughput. A good way to think of this is perhaps like software based SR-IOV that supports nesting of the pass-through.

Due to its design as an in-kernel resource, it also provides very strong notions of protection and isolation so as to not introduce a security compromise when compared to traditional/alternative models where such guarantees are provided by something like userspace or hardware.

The example use-case we have provided supports a “virtual-ethernet” device being utilized in a KVM guest environment, so comparisons to virtio-net will be natural. However, please note that this is but one use-case, of many we have planned for the future (such as userspace bypass and RT guest support). The goal for right now is to describe what a virual-bus is and why we believe it is useful.

We are intent to get this core technology merged, even if the networking components are not accepted as is. It should be noted that, in many ways, virtio could be considered complimentary to the technology. We could in fact, have implemented the virtual-ethernet using a virtio-ring, but it would have required ABI changes that we didn't want to yet propose without having the concept in general vetted and accepted by the community.

[Update: this release includes a virtio-vbus transport, so virtio-net and other such drivers can now run over vbus in addition to the venet system provided]

To cut to the chase, we recently measured our virtual-ethernet on v2.6.29 on two 8-core x86_64 boxes with Chelsio T3 10GE connected back to back via cross over. We measured bare-metal performance, as well as a kvm guest (running the same kernel) connected to the T3 via a linux-bridge+tap configuration with a 1500 MTU. The results are as follows:

Bare metal: tput = 4078Mb/s, round-trip = 25593pps (39us rtt)
Virtio-net: tput = 4003Mb/s, round-trip = 320pps (3125us rtt)
Venet: tput = 4050Mb/s, round-trip = 15255 (65us rtt)

As you can see, all three technologies can achieve (MTU limited) line-rate, but the virtio-net solution is severely limited on the latency front (by a factor of 48:1)

Note that the 320pps is technically artificially low in virtio-net, caused by a a known design limitation to use a timer for tx-mitigation. However, note that even when removing the timer from the path the best we could achieve was 350us-450us of latency, and doing so causes the tput to drop to 1300Mb/s. So even in this case, I think the in-kernel results presents a compelling argument for the new model presented.

[Update: Anthony Ligouri is working on this userspace implementation problem currently and has obtained significant performance gains by utilizing some of the techniques we use in this patch set as well. More details to come.]

When we jump to 9000 byte MTU, the situation looks similar

Bare metal: tput = 9717Mb/s, round-trip = 30396pps (33us rtt)
Virtio-net: tput = 4578Mb/s, round-trip = 249pps (4016us rtt)
Venet: tput = 5802Mb/s, round-trip = 15127 (66us rtt)

Note that even the throughput was slightly better in this test for venet, though neither venet nor virtio-net could achieve line-rate. I suspect some tuning may allow these numbers to improve, TBD.

So with that said, lets jump into the description:

Virtual-Bus: What is it?
--------------------

Virtual-Bus is a kernel based IO resource container technology. It is modeled on a concept similar to the Linux Device-Model (LDM), where we have buses, devices, and drivers as the primary actors. However, VBUS has several distinctions when contrasted with LDM:

1) "Busses" in LDM are relatively static and global to the kernel (e.g. "PCI", "USB", etc). VBUS buses are arbitrarily created and destroyed dynamically, and are not globally visible. Instead they are defined as visible only to a specific subset of the system (the contained context).
2) "Devices" in LDM are typically tangible physical (or sometimes logical) devices. VBUS devices are purely software abstractions (which may or may not have one or more physical devices behind them). Devices may also be arbitrarily created or destroyed by software/administrative action as opposed to by a hardware discovery mechanism.
3) "Drivers" in LDM sit within the same kernel context as the busses and devices they interact with. VBUS drivers live in a foreign context (such as userspace, or a virtual-machine guest).

The idea is that a vbus is created to contain access to some IO services. Virtual devices are then instantiated and linked to a bus to grant access to drivers actively present on the bus. Drivers will only have visibility to devices present on their respective bus, and nothing else.

Virtual devices are defined by modules which register a deviceclass with the system. A deviceclass simply represents a type of device that _may_ be instantiated into a device, should an administrator wish to do so. Once this has happened, the device may be associated with one or more buses where it will become visible to all clients of those respective buses.

Why do we need this?
----------------------

There are various reasons why such a construct may be useful. One of the most interesting use cases is for virtualization, such as KVM. Hypervisors today provide virtualized IO resources to a guest, but this is often at a cost in both latency and throughput compared to bare metal performance. Utilizing para-virtual resources instead of emulated devices helps to mitigate this penalty, but even these techniques to date have not fully realized the potential of the underlying bare-metal hardware.

Some of the performance differential is unavoidable just given the extra processing that occurs due to the deeper stack (guest+host). However, some of this overhead is a direct result of the rather indirect path most hypervisors use to route IO. For instance, KVM uses PIO faults from the guest to trigger a guest->host-kernel->host-userspace->host-kernel sequence of events. Contrast this to a typical userspace application on the host which must only traverse app->kernel for most IO.

The fact is that the linux kernel is already great at managing access to IO resources. Therefore, if you have a hypervisor that is based on the linux kernel, is there some way that we can allow the hypervisor to manage IO directly instead of forcing this convoluted path?

The short answer is: "not yet" ;)

In order to use such a concept, we need some new facilties. For one, we need to be able to define containers with their corresponding access-control so that guests do not have unmitigated access to anything they wish. Second, we also need to define some forms of memory access that is uniform in the face of various clients (e.g. "copy_to_user()" cannot be assumed to work for, say, a KVM vcpu context). Lastly, we need to provide access to these resources in a way that makes sense for the application, such as asynchronous communication paths and minimizing context switches.

For more details, please visit our wiki at:

http://developer.novell.com/wiki/index.php/Virtual-bus

Regards,
-Greg

---

Gregory Haskins (19):
virtio: add a vbus transport
vbus: add a userspace connector
kvm: Add guest-side support for VBUS
kvm: Add VBUS support to the host
kvm: add dynamic IRQ support
kvm: add a reset capability
x86: allow the irq->vector translation to be determined outside of ioapic
venettap: add scatter-gather support
venet: add scatter-gather support
venet-tap: Adds a "venet" compatible "tap" device to VBUS
net: Add vbus_enet driver
venet: add the ABI definitions for an 802.x packet interface
ioq: add vbus helpers
ioq: Add basic definitions for a shared-memory, lockless queue
vbus: add a "vbus-proxy" bus model for vbus_driver objects
vbus: add bus-registration notifiers
vbus: add connection-client helper infrastructure
vbus: add virtual-bus definitions
shm-signal: shared-memory signals

 Documentation/vbus.txt           |  386 +++++++++
 arch/x86/Kconfig                 |   16 
 arch/x86/Makefile                |    3 
 arch/x86/include/asm/irq.h       |    6 
 arch/x86/include/asm/kvm_host.h  |    9 
 arch/x86/include/asm/kvm_para.h  |   12 
 arch/x86/kernel/io_apic.c        |   25 +
 arch/x86/kvm/Kconfig             |    9 
 arch/x86/kvm/Makefile            |    6 
 arch/x86/kvm/dynirq.c            |  329 ++++++++
 arch/x86/kvm/guest/Makefile      |    2 
 arch/x86/kvm/guest/dynirq.c      |   95 ++
 arch/x86/kvm/x86.c               |   13 
 arch/x86/kvm/x86.h               |   12 
 drivers/Makefile                 |    2 
 drivers/net/Kconfig              |   13 
 drivers/net/Makefile             |    1 
 drivers/net/vbus-enet.c          |  907 +++++++++++++++++++++
 drivers/vbus/devices/Kconfig     |   17 
 drivers/vbus/devices/Makefile    |    1 
 drivers/vbus/devices/venet-tap.c | 1609 ++++++++++++++++++++++++++++++++++++++
 drivers/vbus/proxy/Makefile      |    2 
 drivers/vbus/proxy/kvm.c         |  726 +++++++++++++++++
 drivers/virtio/Kconfig           |   15 
 drivers/virtio/Makefile          |    1 
 drivers/virtio/virtio_vbus.c     |  496 ++++++++++++
 fs/proc/base.c                   |   96 ++
 include/linux/ioq.h              |  410 ++++++++++
 include/linux/kvm.h              |    4 
 include/linux/kvm_guest.h        |    7 
 include/linux/kvm_host.h         |   27 +
 include/linux/kvm_para.h         |   60 +
 include/linux/sched.h            |    4 
 include/linux/shm_signal.h       |  188 ++++
 include/linux/vbus.h             |  166 ++++
 include/linux/vbus_client.h      |  115 +++
 include/linux/vbus_device.h      |  424 ++++++++++
 include/linux/vbus_driver.h      |   80 ++
 include/linux/vbus_userspace.h   |   48 +
 include/linux/venet.h            |   82 ++
 include/linux/virtio_vbus.h      |  163 ++++
 kernel/Makefile                  |    1 
 kernel/exit.c                    |    2 
 kernel/fork.c                    |    2 
 kernel/vbus/Kconfig              |   55 +
 kernel/vbus/Makefile             |   11 
 kernel/vbus/attribute.c          |   52 +
 kernel/vbus/client.c             |  543 +++++++++++++
 kernel/vbus/config.c             |  275 ++++++
 kernel/vbus/core.c               |  626 +++++++++++++++
 kernel/vbus/devclass.c           |  124 +++
 kernel/vbus/map.c                |   72 ++
 kernel/vbus/map.h                |   41 +
 kernel/vbus/proxy.c              |  216 +++++
 kernel/vbus/shm-ioq.c            |   89 ++
 kernel/vbus/userspace-client.c   |  485 +++++++++++
 kernel/vbus/vbus.h               |  117 +++
 kernel/vbus/virtio.c             |  628 +++++++++++++++
 lib/Kconfig                      |   22 +
 lib/Makefile                     |    2 
 lib/ioq.c                        |  298 +++++++
 lib/shm_signal.c                 |  186 ++++
 virt/kvm/kvm_main.c              |   37 +
 virt/kvm/vbus.c                  | 1307 +++++++++++++++++++++++++++++++
 64 files changed, 11777 insertions(+), 1 deletions(-)
 create mode 100644 Documentation/vbus.txt
 create mode 100644 arch/x86/kvm/dynirq.c
 create mode 100644 arch/x86/kvm/guest/Makefile
 create mode 100644 arch/x86/kvm/guest/dynirq.c
 create mode 100644 drivers/net/vbus-enet.c
 create mode 100644 drivers/vbus/devices/Kconfig
 create mode 100644 drivers/vbus/devices/Makefile
 create mode 100644 drivers/vbus/devices/venet-tap.c
 create mode 100644 drivers/vbus/proxy/Makefile
 create mode 100644 drivers/vbus/proxy/kvm.c
 create mode 100644 drivers/virtio/virtio_vbus.c
 create mode 100644 include/linux/ioq.h
 create mode 100644 include/linux/kvm_guest.h
 create mode 100644 include/linux/shm_signal.h
 create mode 100644 include/linux/vbus.h
 create mode 100644 include/linux/vbus_client.h
 create mode 100644 include/linux/vbus_device.h
 create mode 100644 include/linux/vbus_driver.h
 create mode 100644 include/linux/vbus_userspace.h
 create mode 100644 include/linux/venet.h
 create mode 100644 include/linux/virtio_vbus.h
 create mode 100644 kernel/vbus/Kconfig
 create mode 100644 kernel/vbus/Makefile
 create mode 100644 kernel/vbus/attribute.c
 create mode 100644 kernel/vbus/client.c
 create mode 100644 kernel/vbus/config.c
 create mode 100644 kernel/vbus/core.c
 create mode 100644 kernel/vbus/devclass.c
 create mode 100644 kernel/vbus/map.c
 create mode 100644 kernel/vbus/map.h
 create mode 100644 kernel/vbus/proxy.c
 create mode 100644 kernel/vbus/shm-ioq.c
 create mode 100644 kernel/vbus/userspace-client.c
 create mode 100644 kernel/vbus/vbus.h
 create mode 100644 kernel/vbus/virtio.c
 create mode 100644 lib/ioq.c
 create mode 100644 lib/shm_signal.c
 create mode 100644 virt/kvm/vbus.c
#1 lore raw

RFC PATCH v2 1/19 shm-signal: shared-memory signals

This interface provides a bidirectional shared-memory based signaling mechanism. It can be used by any entities which desire efficient communication via shared memory. The implementation details of the signaling are abstracted so that they may transcend a wide variety of locale boundaries (e.g. userspace/kernel, guest/host, etc).

The shm_signal mechanism supports event masking as well as spurious event delivery mitigation.

---

 include/linux/shm_signal.h |  188 ++++++++++++++++++++++++++++++++++++++++++++
 lib/Kconfig                |   10 ++
 lib/Makefile               |    1 
 lib/shm_signal.c           |  186 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 385 insertions(+), 0 deletions(-)
 create mode 100644 include/linux/shm_signal.h
 create mode 100644 lib/shm_signal.c
include/linux/shm_signal.h new file +188 -0
new file mode 100644
index 0000000..a65e54e
@@ -0,0 +1,188 @@
+/*
+ * Copyright 2009 Novell.  All Rights Reserved.
+ *
+ * Author:
+ *      Gregory Haskins <[email protected]>
+ *
+ * This file is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA.
+ */
+
+#ifndef _LINUX_SHM_SIGNAL_H
+#define _LINUX_SHM_SIGNAL_H
+
+#include <asm/types.h>
+
+/*
+ *---------
+ * The following structures represent data that is shared across boundaries
+ * which may be quite disparate from one another (e.g. Windows vs Linux,
+ * 32 vs 64 bit, etc).  Therefore, care has been taken to make sure they
+ * present data in a manner that is independent of the environment.
+ *-----------
+ */
+
+#define SHM_SIGNAL_MAGIC 0x58fa39df
+#define SHM_SIGNAL_VER   1
+
+struct shm_signal_irq {
+	__u8                  enabled;
+	__u8                  pending;
+	__u8                  dirty;
+};
+
+enum shm_signal_locality {
+	shm_locality_north,
+	shm_locality_south,
+};
+
+struct shm_signal_desc {
+	__u32                 magic;
+	__u32                 ver;
+	struct shm_signal_irq irq[2];
+};
+
+/* --- END SHARED STRUCTURES --- */
+
+#ifdef __KERNEL__
+
+#include <linux/interrupt.h>
+
+struct shm_signal_notifier {
+	void (*signal)(struct shm_signal_notifier *);
+};
+
+struct shm_signal;
+
+struct shm_signal_ops {
+	int      (*inject)(struct shm_signal *s);
+	void     (*fault)(struct shm_signal *s, const char *fmt, ...);
+	void     (*release)(struct shm_signal *s);
+};
+
+enum {
+	shm_signal_in_wakeup,
+};
+
+struct shm_signal {
+	atomic_t                    refs;
+	spinlock_t                  lock;
+	enum shm_signal_locality    locale;
+	unsigned long               flags;
+	struct shm_signal_ops      *ops;
+	struct shm_signal_desc     *desc;
+	struct shm_signal_notifier *notifier;
+	struct tasklet_struct       deferred_notify;
+};
+
+#define SHM_SIGNAL_FAULT(s, fmt, args...)  \
+  ((s)->ops->fault ? (s)->ops->fault((s), fmt, ## args) : panic(fmt, ## args))
+
+ /*
+  * These functions should only be used internally
+  */
+void _shm_signal_release(struct shm_signal *s);
+void _shm_signal_wakeup(struct shm_signal *s);
+
+/**
+ * shm_signal_init() - initialize an SHM_SIGNAL
+ * @s:        SHM_SIGNAL context
+ *
+ * Initializes SHM_SIGNAL context before first use
+ *
+ **/
+void shm_signal_init(struct shm_signal *s);
+
+/**
+ * shm_signal_get() - acquire an SHM_SIGNAL context reference
+ * @s:        SHM_SIGNAL context
+ *
+ **/
+static inline struct shm_signal *shm_signal_get(struct shm_signal *s)
+{
+	atomic_inc(&s->refs);
+
+	return s;
+}
+
+/**
+ * shm_signal_put() - release an SHM_SIGNAL context reference
+ * @s:        SHM_SIGNAL context
+ *
+ **/
+static inline void shm_signal_put(struct shm_signal *s)
+{
+	if (atomic_dec_and_test(&s->refs))
+		_shm_signal_release(s);
+}
+
+/**
+ * shm_signal_enable() - enables local notifications on an SHM_SIGNAL
+ * @s:        SHM_SIGNAL context
+ * @flags:      Reserved for future use, must be 0
+ *
+ * Enables/unmasks the registered notifier (if applicable) to receive wakeups
+ * whenever the remote side performs an shm_signal() operation. A notification
+ * will be dispatched immediately if any pending signals have already been
+ * issued prior to invoking this call.
+ *
+ * This is synonymous with unmasking an interrupt.
+ *
+ * Returns: success = 0, <0 = ERRNO
+ *
+ **/
+int shm_signal_enable(struct shm_signal *s, int flags);
+
+/**
+ * shm_signal_disable() - disable local notifications on an SHM_SIGNAL
+ * @s:        SHM_SIGNAL context
+ * @flags:      Reserved for future use, must be 0
+ *
+ * Disables/masks the registered shm_signal_notifier (if applicable) from
+ * receiving any further notifications.  Any subsequent calls to shm_signal()
+ * by the remote side will update the shm as dirty, but will not traverse the
+ * locale boundary and will not invoke the notifier callback.  Signals
+ * delivered while masked will be deferred until shm_signal_enable() is
+ * invoked.
+ *
+ * This is synonymous with masking an interrupt
+ *
+ * Returns: success = 0, <0 = ERRNO
+ *
+ **/
+int shm_signal_disable(struct shm_signal *s, int flags);
+
+/**
+ * shm_signal_inject() - notify the remote side about shm changes
+ * @s:        SHM_SIGNAL context
+ * @flags:      Reserved for future use, must be 0
+ *
+ * Marks the shm state as "dirty" and, if enabled, will traverse
+ * a locale boundary to inject a remote notification.  The remote
+ * side controls whether the notification should be delivered via
+ * the shm_signal_enable/disable() interface.
+ *
+ * The specifics of how to traverse a locale boundary are abstracted
+ * by the shm_signal_ops->signal() interface and provided by a particular
+ * implementation.  However, typically going north to south would be
+ * something like a syscall/hypercall, and going south to north would be
+ * something like a posix-signal/guest-interrupt.
+ *
+ * Returns: success = 0, <0 = ERRNO
+ *
+ **/
+int shm_signal_inject(struct shm_signal *s, int flags);
+
+#endif /* __KERNEL__ */
+
+#endif /* _LINUX_SHM_SIGNAL_H */
lib/Kconfig +10 -0
index 03c2c24..32d82fe 100644
@@ -174,4 +174,14 @@ config DISABLE_OBSOLETE_CPUMASK_FUNCTIONS
        bool "Disable obsolete cpumask functions" if DEBUG_PER_CPU_MAPS
        depends on EXPERIMENTAL && BROKEN
 
+config SHM_SIGNAL
+	boolean "SHM Signal - Generic shared-memory signaling mechanism"
+	default n
+	help
+	 Provides a shared-memory based signaling mechansim to indicate
+         memory-dirty notifications between two end-points.
+
+	 If unsure, say N
+
+
 endmenu
lib/Makefile +1 -0
index 32b0e64..bc36327 100644
@@ -71,6 +71,7 @@ obj-$(CONFIG_TEXTSEARCH_BM) += ts_bm.o
 obj-$(CONFIG_TEXTSEARCH_FSM) += ts_fsm.o
 obj-$(CONFIG_SMP) += percpu_counter.o
 obj-$(CONFIG_AUDIT_GENERIC) += audit.o
+obj-$(CONFIG_SHM_SIGNAL) += shm_signal.o
 
 obj-$(CONFIG_SWIOTLB) += swiotlb.o
 obj-$(CONFIG_IOMMU_HELPER) += iommu-helper.o
lib/shm_signal.c new file +186 -0

186 lines not on this page · raw message

#2 lore raw

RFC PATCH v2 2/19 vbus: add virtual-bus definitions

In reply to Gregory Haskins

See Documentation/vbus.txt for details

---

 Documentation/vbus.txt      |  386 +++++++++++++++++++++++++++++
 arch/x86/Kconfig            |    2 
 fs/proc/base.c              |   96 +++++++
 include/linux/sched.h       |    4 
 include/linux/vbus.h        |  147 +++++++++++
 include/linux/vbus_device.h |  417 ++++++++++++++++++++++++++++++++
 kernel/Makefile             |    1 
 kernel/exit.c               |    2 
 kernel/fork.c               |    2 
 kernel/vbus/Kconfig         |   14 +
 kernel/vbus/Makefile        |    1 
 kernel/vbus/attribute.c     |   52 ++++
 kernel/vbus/config.c        |  275 +++++++++++++++++++++
 kernel/vbus/core.c          |  567 +++++++++++++++++++++++++++++++++++++++++++
 kernel/vbus/devclass.c      |  124 +++++++++
 kernel/vbus/map.c           |   72 +++++
 kernel/vbus/map.h           |   41 +++
 kernel/vbus/vbus.h          |  116 +++++++++
 18 files changed, 2319 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/vbus.txt
 create mode 100644 include/linux/vbus.h
 create mode 100644 include/linux/vbus_device.h
 create mode 100644 kernel/vbus/Kconfig
 create mode 100644 kernel/vbus/Makefile
 create mode 100644 kernel/vbus/attribute.c
 create mode 100644 kernel/vbus/config.c
 create mode 100644 kernel/vbus/core.c
 create mode 100644 kernel/vbus/devclass.c
 create mode 100644 kernel/vbus/map.c
 create mode 100644 kernel/vbus/map.h
 create mode 100644 kernel/vbus/vbus.h
Documentation/vbus.txt new file +386 -0
new file mode 100644
index 0000000..e8a05da
@@ -0,0 +1,386 @@
+
+Virtual-Bus:
+======================
+Author: Gregory Haskins <[email protected]>
+
+
+
+
+What is it?
+--------------------
+
+Virtual-Bus is a kernel based IO resource container technology.  It is modeled
+on a concept similar to the Linux Device-Model (LDM), where we have buses,
+devices, and drivers as the primary actors.  However, VBUS has several
+distinctions when contrasted with LDM:
+
+  1) "Busses" in LDM are relatively static and global to the kernel (e.g.
+     "PCI", "USB", etc).  VBUS buses are arbitrarily created and destroyed
+     dynamically, and are not globally visible.  Instead they are defined as
+     visible only to a specific subset of the system (the contained context).
+  2) "Devices" in LDM are typically tangible physical (or sometimes logical)
+     devices.  VBUS devices are purely software abstractions (which may or
+     may not have one or more physical devices behind them).  Devices may
+     also be arbitrarily created or destroyed by software/administrative action
+     as opposed to by a hardware discovery mechanism.
+  3) "Drivers" in LDM sit within the same kernel context as the busses and
+     devices they interact with.  VBUS drivers live in a foreign
+     context (such as userspace, or a virtual-machine guest).
+
+The idea is that a vbus is created to contain access to some IO services.
+Virtual devices are then instantiated and linked to a bus to grant access to
+drivers actively present on the bus.  Drivers will only have visibility to
+devices present on their respective bus, and nothing else.
+
+Virtual devices are defined by modules which register a deviceclass with the
+system.  A deviceclass simply represents a type of device that _may_ be
+instantiated into a device, should an administrator wish to do so.  Once
+this has happened, the device may be associated with one or more buses where
+it will become visible to all clients of those respective buses.
+
+Why do we need this?
+----------------------
+
+There are various reasons why such a construct may be useful.  One of the
+most interesting use cases is for virtualization, such as KVM.  Hypervisors
+today provide virtualized IO resources to a guest, but this is often at a cost
+in both latency and throughput compared to bare metal performance.  Utilizing
+para-virtual resources instead of emulated devices helps to mitigate this
+penalty, but even these techniques to date have not fully realized the
+potential of the underlying bare-metal hardware.
+
+Some of the performance differential is unavoidable just given the extra
+processing that occurs due to the deeper stack (guest+host).  However, some of
+this overhead is a direct result of the rather indirect path most hypervisors
+use to route IO.  For instance, KVM uses PIO faults from the guest to trigger
+a guest->host-kernel->host-userspace->host-kernel sequence of events.
+Contrast this to a typical userspace application on the host which must only
+traverse app->kernel for most IO.
+
+The fact is that the linux kernel is already great at managing access to IO
+resources.  Therefore, if you have a hypervisor that is based on the linux
+kernel, is there some way that we can allow the hypervisor to manage IO
+directly instead of forcing this convoluted path?
+
+The short answer is: "not yet" ;)
+
+In order to use such a concept, we need some new facilties.  For one, we
+need to be able to define containers with their corresponding access-control so
+that guests do not have unmitigated access to anything they wish.  Second,
+we also need to define some forms of memory access that is uniform in the face
+of various clients (e.g. "copy_to_user()" cannot be assumed to work for, say,
+a KVM vcpu context).  Lastly, we need to provide access to these resources in
+a way that makes sense for the application, such as asynchronous communication
+paths and minimizing context switches.
+
+So we introduce VBUS as a framework to provide such facilities.  The net
+result is a *substantial* reduction in IO overhead, even when compared to
+state of the art para-virtualization techniques (such as virtio-net).
+
+How do I use it?
+------------------------
+
+There are two components to utilizing a virtual-bus.  One is the
+administrative function (creating and configuring a bus and its devices).  The
+other is the consumption of the resources on the bus by a client (e.g. a
+virtual machine, or a userspace application).  The former occurs on the host
+kernel by means of interacting with various special filesystems (e.g. sysfs,
+configfs, etc).  The latter occurs by means of a "vbus connector" which must
+be developed specifically to bridge a particular environment.  To date, we
+have developed such connectors for host-userspace and kvm-guests.  Conceivably
+we could develop other connectors as needs arise (e.g. lguest, xen,
+guest-userspace, etc).  This document deals with the administrative interface.
+Details about developing a connector are out of scope for this document.
+
+Interacting with vbus
+------------------------
+
+The first step is to enable virtual-bus support (CONFIG_VBUS) as well as any
+desired vbus-device modules (e.g. CONFIG_VBUS_VENETTAP), and ensure that your
+environment mounts both sysfs and configfs somewhere in the filesystem.  This
+document will assume they are mounted to /sys and /config, respectively.
+
+VBUS will create a top-level directory "vbus" in each of the two respective
+filesystems.  At boot-up, they will look like the following:
+
+/sys/vbus/
+|-- deviceclass
+|-- devices
+|-- instances
+`-- version
+
+/config/vbus/
+|-- devices
+`-- instances
+
+Following their respective roles, /config/vbus is for userspace to manage the
+lifetime of some number of objects/attributes.  This is in contrast to
+/sys/vbus which is a reflection of objects managed by the kernel.  It is
+assumed the reader is already familiar with these two facilities, so we will
+not go into depth about their general operation.  Suffice to say that vbus
+consists of objects that are managed both by userspace and the kernel.
+Modification of objects via /config/vbus will typically be reflected in the
+/sys/vbus area.
+
+It all starts with a deviceclass
+--------------------------------
+
+Before you can do anything useful with vbus, you need some registered
+deviceclasses.  A deviceclass provides the implementation of a specific type
+of virtual device.  A deviceclass will typically be registered by loading a
+kernel-module.  Once loaded, the available device types are enumerated under
+/sys/vbus/deviceclass.  For example, we will load our "venet-tap" module,
+which provides network services:
+
+# modprobe venet-tap
+# tree /sys/vbus
+/sys/vbus
+|-- deviceclass
+|   `-- venet-tap
+|-- devices
+|-- instances
+`-- version
+
+An administrative agent should be able to enumerate /sys/vbus/deviceclass to
+determine what services are available on a given platform.
+
+Create the container
+-------------------
+
+The next step is to create a new container.  In vbus, this comes in the form
+of a vbus-instance and it is created by a simple "mkdir" in the
+/config/vbus/instances area.  The only requirement is that the instance is
+given a host-wide unique name.  This may be some kind of association to the
+application (e.g. the unique VM GUID) or it can be arbitrary.  For the
+purposes of example, we will let $(uuidgen) generate a random UUID for us.
+
+# mkdir /config/vbus/instances/$(uuidgen)
+# tree /sys/vbus/
+/sys/vbus/
+|-- deviceclass
+|   `-- venet-tap
+|-- devices
+|-- instances
+|   `-- beb4df8f-7483-4028-b3f7-767512e2a18c
+|       |-- devices
+|       `-- members
+`-- version
+
+So we can see that we have now created a vbus called
+
+               "beb4df8f-7483-4028-b3f7-767512e2a18c"
+
+in the /config area, and it was immediately reflected in the
+/sys/vbus/instances area as well (with a few subobjects of its own: "devices"
+and "members").  The "devices" object denotes any devices that are present on
+the bus (in this case: none).  Likewise, "members" denotes the pids of any
+tasks that are members of the bus (in this case: none).  We will come back to
+this later.  For now, we move on to the next step
+
+Create a device instance
+------------------------
+
+Devices are instantiated by again utilizing the /config/vbus configfs area.
+At first you may suspect that devices are created as subordinate objects of a
+bus/container instance, but you would be mistaken.  Devices are actually
+root-level objects in vbus specifically to allow greater flexibility in the
+association of a device.  For instance, it may be desirable to have a single
+device that spans multiple VMs (consider an ethernet switch, or a shared disk
+for a cluster).  Therefore, device lifecycles are managed by creating/deleting
+objects in /config/vbus/devices.
+
+Note: Creating a device instance is actually a two step process:  We need to
+give the device instance a unique name, and we also need to give it a specific
+device type.  It is hard to express both parameters using standard filesystem
+operations like mkdir, so the design decision was made to require performing
+the operation in two steps.
+
+Our first step is to create a unique instance.  We will again utilize
+$(uuidgen) to yield an arbitrary name.  Any name will suffice as long as it is
+unqie on this particular host.
+
+# mkdir /config/vbus/devices/$(uuidgen)
+# tree /sys/vbus
+/sys/vbus
+|-- deviceclass
+|   `-- venet-tap
+|-- devices
+|   `-- 6a1aff24-5dc0-4aea-9c35-435daef90e55
+|       `-- interfaces
+|-- instances
+|   `-- beb4df8f-7483-4028-b3f7-767512e2a18c
+|       |-- devices
+|       `-- members
+`-- version
+
+At this point we have created a partial instance, since we have not yet
+assigned a type to the device.  Even so, we can see that some state has
+changed under /sys/vbus/devices.  We now have an instance named
+
+	      	 6a1aff24-5dc0-4aea-9c35-435daef90e55
+
+and it has a single subordinate object: "interfaces".  This object in
+particular is provided by the infrastructure, though do note that a
+deviceclass may also provide its own attributes/objects once it is created.
+
+We will go ahead and give this device a type to complete its construction.  We
+do this by setting the /config/vbus/devices/$devname/type attribute with a
+valid deviceclass type:
+
+# echo foo > /config/vbus/devices/6a1aff24-5dc0-4aea-9c35-435daef90e55/type
+bash: echo: write error: No such file or directory
+
+Oops!  What happened?  "foo" is not a valid deviceclass.  We need to consult
+the /sys/vbus/deviceclass area to find out what our options are:
+
+# tree /sys/vbus/deviceclass/
+/sys/vbus/deviceclass/
+`-- venet-tap
+
+Lets try again:
+
+# echo venet-tap > /config/vbus/devices/6a1aff24-5dc0-4aea-9c35-435daef90e55/type
+# tree /sys/vbus/
+/sys/vbus/
+|-- deviceclass
+|   `-- venet-tap
+|-- devices
+|   `-- 6a1aff24-5dc0-4aea-9c35-435daef90e55
+|       |-- class -> ../../deviceclass/venet-tap
+|       |-- client_mac
+|       |-- enabled
+|       |-- host_mac
+|       |-- ifname
+|       `-- interfaces
+|-- instances
+|   `-- beb4df8f-7483-4028-b3f7-767512e2a18c
+|       |-- devices
+|       `-- members
+`-- version
+
+Ok, that looks better.  And note that /sys/vbus/devices now has some more
+subordinate objects.  Most of those were registered when the venet-tap
+deviceclass was given a chance to create an instance of itself.  Those
+attributes are a property of the venet-tap and therefore are out of scope
+for this document.  Please see the documentation that accompanies a particular
+module for more details.
+
+Put the device on the bus
+-------------------------
+
+The next administrative step is to associate our new device with our bus.
+This is accomplished using a symbolic link from the bus instance to our device
+instance.
+
+ln -s /config/vbus/devices/6a1aff24-5dc0-4aea-9c35-435daef90e55/ /config/vbus/instances/beb4df8f-7483-4028-b3f7-767512e2a18c/
+# tree /sys/vbus/
+/sys/vbus/
+|-- deviceclass
+|   `-- venet-tap
+|-- devices
+|   `-- 6a1aff24-5dc0-4aea-9c35-435daef90e55
+|       |-- class -> ../../deviceclass/venet-tap
+|       |-- client_mac
+|       |-- enabled
+|       |-- host_mac
+|       |-- ifname
+|       `-- interfaces
+|           `-- 0 -> ../../../instances/beb4df8f-7483-4028-b3f7-767512e2a18c/devices/0
+|-- instances
+|   `-- beb4df8f-7483-4028-b3f7-767512e2a18c
+|       |-- devices
+|       |   `-- 0
+|       |       |-- device -> ../../../../devices/6a1aff24-5dc0-4aea-9c35-435daef90e55
+|       |       `-- type
+|       `-- members
+`-- version
+
+We can now see that the device indicates that it has an interface registered
+to a bus:
+
+/sys/vbus/devices/6a1aff24-5dc0-4aea-9c35-435daef90e55/interfaces/
+`-- 0 -> ../../../instances/beb4df8f-7483-4028-b3f7-767512e2a18c/devices/0
+
+Likewise, we can see that the bus has a device listed (id = "0"):
+
+/sys/vbus/instances/beb4df8f-7483-4028-b3f7-767512e2a18c/devices/
+`-- 0
+    |-- device -> ../../../../devices/6a1aff24-5dc0-4aea-9c35-435daef90e55
+    `-- type
+
+At this point, our container is ready for use.  However, it currently has 0
+members, so lets fix that
+
+Add some members
+--------------------
+
+Membership is controlled by an attribute: /proc/$pid/vbus.  A pid can only be
+a member of one (or zero) busses at a time.  To establish membership, we set
+the name of the bus, like so:
+
+# echo beb4df8f-7483-4028-b3f7-767512e2a18c > /proc/self/vbus
+# tree /sys/vbus/
+/sys/vbus/
+|-- deviceclass
+|   `-- venet-tap
+|-- devices
+|   `-- 6a1aff24-5dc0-4aea-9c35-435daef90e55
+|       |-- class -> ../../deviceclass/venet-tap
+|       |-- client_mac
+|       |-- enabled
+|       |-- host_mac
+|       |-- ifname
+|       `-- interfaces
+|           `-- 0 -> ../../../instances/beb4df8f-7483-4028-b3f7-767512e2a18c/devices/0
+|-- instances
+|   `-- beb4df8f-7483-4028-b3f7-767512e2a18c
+|       |-- devices
+|       |   `-- 0
+|       |       |-- device -> ../../../../devices/6a1aff24-5dc0-4aea-9c35-435daef90e55
+|       |       `-- type
+|       `-- members
+|           |-- 4382
+|           `-- 4588
+`-- version
+
+Woah!  Why are there two members?  VBUS membership is inherited by forked
+tasks.  Therefore 4382 is the pid of our shell (which we set via /proc/self),
+and 4588 is the pid of the forked/exec'ed "tree" process.  This property can
+be useful for having things like qemu set up the bus and then forking each
+vcpu which will inherit access.
+
+At this point, we are ready to roll.  Pid 4382 has access to a virtual-bus
+namespace with one device, id=0.  Its type is:
+
+# cat /sys/vbus/instances/beb4df8f-7483-4028-b3f7-767512e2a18c/devices/0/type
+virtual-ethernet
+
+"virtual-ethernet"?  Why is it not "venet-tap"?  Device-classes are allowed to
+register their interfaces under an id that is not required to be the same as
+their deviceclass.  This supports device polymorphism.   For instance,
+consider that an interface "virtual-ethernet" may provide basic 802.x packet
+exchange.  However, we could have various implementations of a device that
+supports the 802.x interface, while having various implementations behind
+them.
+
+For instance, "venet-tap" might act like a tuntap module, while
+"venet-loopback" would loop packets back and "venet-switch" would form a
+layer-2 domain among the participating guests.  All three modules would
+presumably support the same basic 802.x interface, yet all three have
+completely different implementations.
+
+Drivers on this particular bus would see this instance id=0 as a type
+"virtual-ethernet" even though the underlying implementation happens to be a
+tap device.  This means a single driver that supports the protocol advertised
+by the "virtual-ethernet" type would be able to support the plethera of
+available device types that we may wish to create.
+
+Teardown:
+---------------
+
+We can descontruct a vbus container doing pretty much the opposite of what we
+did to create it.  Echo "0" into /proc/self/vbus, rm the symlink between the
+bus and device, and rmdir the bus and device objects.  Once that is done, we
+can even rmmod the venet-tap module.  Note that the infrastructure will
+maintain a module-ref while it is configured in a container, so be sure to
+completely tear down the vbus/device before trying this.
arch/x86/Kconfig +2 -0
index bc2fbad..3fca247 100644
@@ -1939,6 +1939,8 @@ source "drivers/pcmcia/Kconfig"
 
 source "drivers/pci/hotplug/Kconfig"
 
+source "kernel/vbus/Kconfig"
+
 endmenu
 
 
fs/proc/base.c +96 -0

114 lines not on this page · raw message

include/linux/sched.h +4 -0
index 011db2f..cd2f9b1 100644
@@ -97,6 +97,7 @@ struct futex_pi_state;
 struct robust_list_head;
 struct bio;
 struct bts_tracer;
+struct vbus;
 
 /*
  * List of flags we want to share for kernel threads,
@@ -1329,6 +1330,9 @@ struct task_struct {
 	unsigned int lockdep_recursion;
 	struct held_lock held_locks[MAX_LOCK_DEPTH];
 #endif
+#ifdef CONFIG_VBUS
+	struct vbus *vbus;
+#endif
 
 /* journalling filesystem info */
 	void *journal_info;
include/linux/vbus.h new file +147 -0

147 lines not on this page · raw message

include/linux/vbus_device.h new file +417 -0

417 lines not on this page · raw message

kernel/Makefile +1 -0
index e4791b3..99a98a7 100644
@@ -93,6 +93,7 @@ obj-$(CONFIG_HAVE_GENERIC_DMA_COHERENT) += dma-coherent.o
 obj-$(CONFIG_FUNCTION_TRACER) += trace/
 obj-$(CONFIG_TRACING) += trace/
 obj-$(CONFIG_SMP) += sched_cpupri.o
+obj-$(CONFIG_VBUS) += vbus/
 
 ifneq ($(CONFIG_SCHED_OMIT_FRAME_POINTER),y)
 # According to Alan Modra <[email protected]>, the -fno-omit-frame-pointer is
kernel/exit.c +2 -0
index efd30cc..8736de6 100644
@@ -48,6 +48,7 @@
 #include <linux/tracehook.h>
 #include <linux/init_task.h>
 #include <trace/sched.h>
+#include <linux/vbus.h>
 
 #include <asm/uaccess.h>
 #include <asm/unistd.h>
@@ -1081,6 +1082,7 @@ NORET_TYPE void do_exit(long code)
 	check_stack_usage();
 	exit_thread();
 	cgroup_exit(tsk, 1);
+	task_vbus_disassociate(tsk);
 
 	if (group_dead && tsk->signal->leader)
 		disassociate_ctty(1);
kernel/fork.c +2 -0
index 4854c2c..5536053 100644
@@ -61,6 +61,7 @@
 #include <linux/proc_fs.h>
 #include <linux/blkdev.h>
 #include <trace/sched.h>
+#include <linux/vbus.h>
 
 #include <asm/pgtable.h>
 #include <asm/pgalloc.h>
@@ -1274,6 +1275,7 @@ static struct task_struct *copy_process(unsigned long clone_flags,
 	write_unlock_irq(&tasklist_lock);
 	proc_fork_connector(p);
 	cgroup_post_fork(p);
+	fork_vbus(p);
 	return p;
 
 bad_fork_free_graph:
kernel/vbus/Kconfig new file +14 -0
new file mode 100644
index 0000000..f2b92f5
@@ -0,0 +1,14 @@
+#
+# Virtual-Bus (VBus) configuration
+#
+
+config VBUS
+       bool "Virtual Bus"
+       select CONFIGFS_FS
+       select SHM_SIGNAL
+       default n
+       help
+        Provides a mechansism for declaring virtual-bus objects and binding
+	various tasks and devices which reside on the bus.
+
+	If unsure, say N
kernel/vbus/Makefile new file +1 -0
new file mode 100644
index 0000000..367f65b
@@ -0,0 +1 @@
+obj-$(CONFIG_VBUS) += core.o devclass.o config.o attribute.o map.o
kernel/vbus/attribute.c new file +52 -0

52 lines not on this page · raw message

kernel/vbus/config.c new file +275 -0

275 lines not on this page · raw message

kernel/vbus/core.c new file +567 -0

567 lines not on this page · raw message

kernel/vbus/devclass.c new file +124 -0

124 lines not on this page · raw message

kernel/vbus/map.c new file +72 -0

72 lines not on this page · raw message

kernel/vbus/map.h new file +41 -0

41 lines not on this page · raw message

kernel/vbus/vbus.h new file +116 -0

116 lines not on this page · raw message

#3 lore raw

RFC PATCH v2 3/19 vbus: add connection-client helper infrastructure

In reply to Gregory Haskins

We expect to have various types of connection-clients (e.g. userspace, kvm, etc), each of which is likely to have common access patterns and marshalling duties. Therefore we create a "client" API to simplify client development by helping with mundane tasks such as handle-2-pointer translation, etc.

Special thanks to Pat Mullaney for suggesting the optimization to pass a cookie object down during DEVICESHM operations to save lookup overhead on the event channel.

---

 include/linux/vbus_client.h |  115 +++++++++
 kernel/vbus/Makefile        |    2 
 kernel/vbus/client.c        |  543 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 659 insertions(+), 1 deletions(-)
 create mode 100644 include/linux/vbus_client.h
 create mode 100644 kernel/vbus/client.c
include/linux/vbus_client.h new file +115 -0
new file mode 100644
index 0000000..62dab78
@@ -0,0 +1,115 @@
+/*
+ * Copyright 2009 Novell.  All Rights Reserved.
+ *
+ * Virtual-Bus - Client interface
+ *
+ * We expect to have various types of connection-clients (e.g. userspace,
+ * kvm, etc).  Each client will be connecting from some environment outside
+ * of the kernel, and therefore will not have direct access to the API as
+ * presented in ./linux/vbus.h.  There will undoubtedly be some parameter
+ * marshalling that must occur, as well as common patterns for the handling
+ * of those marshalled parameters (e.g. translating a handle into a pointer,
+ * etc).
+ *
+ * Therefore this "client" API is provided to simplify the development
+ * of any clients.  Of course, a client is free to bypass this API entirely
+ * and communicate with the direct VBUS API if desired.
+ *
+ * Author:
+ *      Gregory Haskins <[email protected]>
+ *
+ * This file is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA.
+ */
+
+#ifndef _LINUX_VBUS_CLIENT_H
+#define _LINUX_VBUS_CLIENT_H
+
+#include <linux/types.h>
+#include <linux/compiler.h>
+
+struct vbus_deviceopen {
+	__u32 devid;
+	__u32 version; /* device ABI version */
+	__u64 handle; /* return value for devh */
+};
+
+struct vbus_devicecall {
+	__u64 devh;   /* device-handle (returned from DEVICEOPEN */
+	__u32 func;
+	__u32 len;
+	__u32 flags;
+	__u64 datap;
+};
+
+struct vbus_deviceshm {
+	__u64 devh;   /* device-handle (returned from DEVICEOPEN */
+	__u32 id;
+	__u32 len;
+	__u32 flags;
+	struct {
+		__u32 offset;
+		__u32 prio;
+		__u64 cookie; /* token to pass back when signaling client */
+	} signal;
+	__u64 datap;
+	__u64 handle; /* return value for signaling from client to kernel */
+};
+
+#ifdef __KERNEL__
+
+#include <linux/ioq.h>
+#include <linux/module.h>
+#include <asm/atomic.h>
+
+struct vbus_client;
+
+struct vbus_client_ops {
+	int (*deviceopen)(struct vbus_client *client,  struct vbus_memctx *ctx,
+			  __u32 devid, __u32 version, __u64 *devh);
+	int (*deviceclose)(struct vbus_client *client, __u64 devh);
+	int (*devicecall)(struct vbus_client *client,
+			  __u64 devh, __u32 func,
+			  void *data, __u32 len, __u32 flags);
+	int (*deviceshm)(struct vbus_client *client,
+			 __u64 devh, __u32 id,
+			 struct vbus_shm *shm, struct shm_signal *signal,
+			 __u32 flags, __u64 *handle);
+	int (*shmsignal)(struct vbus_client *client, __u64 handle);
+	void (*release)(struct vbus_client *client);
+};
+
+struct vbus_client {
+	atomic_t refs;
+	struct vbus_client_ops *ops;
+};
+
+static inline void vbus_client_get(struct vbus_client *client)
+{
+	atomic_inc(&client->refs);
+}
+
+static inline void vbus_client_put(struct vbus_client *client)
+{
+	if (atomic_dec_and_test(&client->refs))
+		client->ops->release(client);
+}
+
+struct vbus_client *vbus_client_attach(struct vbus *bus);
+
+extern struct vbus_memctx *current_memctx;
+struct vbus_memctx *task_memctx_alloc(struct task_struct *task);
+
+#endif /* __KERNEL__ */
+
+#endif /* _LINUX_VBUS_CLIENT_H */
kernel/vbus/Makefile +1 -1
index 367f65b..4d440e5 100644
@@ -1 +1 @@
-obj-$(CONFIG_VBUS) += core.o devclass.o config.o attribute.o map.o
+obj-$(CONFIG_VBUS) += core.o devclass.o config.o attribute.o map.o client.o
kernel/vbus/client.c new file +543 -0

543 lines not on this page · raw message

#4 lore raw

Re: [RFC PATCH v2 03/19] vbus: add connection-client helper infrastructure

Quoted 30 lines · Gregory Haskins
kernel/vbus/client.c new file +30 -0
+static unsigned long
+task_memctx_copy_to(struct vbus_memctx *ctx, void *dst, const void *src,
+		    unsigned long n)
+{
+	struct task_memctx *tm = to_task_memctx(ctx);
+	struct task_struct *p = tm->task;
+
+	while (n) {
+		unsigned long offset = ((unsigned long)dst)%PAGE_SIZE;
+		unsigned long len = PAGE_SIZE - offset;
+		int ret;
+		struct page *pg;
+		void *maddr;
+
+		if (len > n)
+			len = n;
+
+		down_read(&p->mm->mmap_sem);
+		ret = get_user_pages(p, p->mm,
+				     (unsigned long)dst, 1, 1, 0, &pg, NULL);
+
+		if (ret != 1) {
+			up_read(&p->mm->mmap_sem);
+			break;
+		}
+
+		maddr = kmap_atomic(pg, KM_USER0);
+		memcpy(maddr + offset, src, len);
+		kunmap_atomic(maddr, KM_USER0);
+		set_page_dirty_lock(pg);

kernel/vbus/client.c line 471

Quoted 10 lines
kernel/vbus/client.c new file +10 -0
+		put_page(pg);
+		up_read(&p->mm->mmap_sem);
+
+		src += len;
+		dst += len;
+		n -= len;
+	}
+
+	return n;
+}

BTW, why did you decide to use get_user_pages? Would switch_mm + copy_to_user work as well avoiding page walk if all pages are present?

Also - if we just had vmexit because a process executed io (or hypercall), can't we just do copy_to_user there? Avi, I think at some point you said that we can?

Signature · 1 line
MST
#5 lore raw

Re: [RFC PATCH v2 03/19] vbus: add connection-client helper infrastructure

Quoted 39 lines · Michael S. Tsirkin
Quoted 37 lines · Gregory Haskins
kernel/vbus/client.c new file +37 -0
+static unsigned long
+task_memctx_copy_to(struct vbus_memctx *ctx, void *dst, const void *src,
+		    unsigned long n)
+{
+	struct task_memctx *tm = to_task_memctx(ctx);
+	struct task_struct *p = tm->task;
+
+	while (n) {
+		unsigned long offset = ((unsigned long)dst)%PAGE_SIZE;
+		unsigned long len = PAGE_SIZE - offset;
+		int ret;
+		struct page *pg;
+		void *maddr;
+
+		if (len > n)
+			len = n;
+
+		down_read(&p->mm->mmap_sem);
+		ret = get_user_pages(p, p->mm,
+				     (unsigned long)dst, 1, 1, 0, &pg, NULL);
+
+		if (ret != 1) {
+			up_read(&p->mm->mmap_sem);
+			break;
+		}
+
+		maddr = kmap_atomic(pg, KM_USER0);
+		memcpy(maddr + offset, src, len);
+		kunmap_atomic(maddr, KM_USER0);
+		set_page_dirty_lock(pg);
+		put_page(pg);
+		up_read(&p->mm->mmap_sem);
+
+		src += len;
+		dst += len;
+		n -= len;
+	}
Quoted 9 lines

kernel/vbus/client.c line 471

Quoted 4 lines
kernel/vbus/client.c new file +3 -0
+
+	return n;
+}

BTW, why did you decide to use get_user_pages? Would switch_mm + copy_to_user work as well avoiding page walk if all pages are present?

Well, basic c_t_u() won't work because its likely not "current" if you are updating the ring from some other task, but I think you have already figured that out based on the switch_mm suggestion. The simple truth is I was not familiar with switch_mm at the time I wrote this (nor am I now). If this is a superior method that allows you to acquire c_t_u(some_other_ctx) like behavior, I see no problem in changing. I will look into this, and thanks for the suggestion!

Quoted 4 lines

Also - if we just had vmexit because a process executed io (or hypercall), can't we just do copy_to_user there? Avi, I think at some point you said that we can?

Right, and yes that will work I believe. We could always do a "if (p == current)" check to test for this. To date, I don't typically do anything mem-ops related directly in vcpu context so this wasn't an issue...but that doesn't mean someone wont try in the future. Therefore, I agree we should strive to optimize it if we can.

Quoted 2 lines

Thanks Michael,
-Greg

#6 lore raw

Re: [RFC PATCH v2 03/19] vbus: add connection-client helper infrastructure

Quoted 6 lines · Gregory Haskins
Quoted 5 lines

BTW, why did you decide to use get_user_pages?
Would switch_mm + copy_to_user work as well
avoiding page walk if all pages are present?

Quoted 8 lines

Well, basic c_t_u() won't work because its likely not "current" if you are updating the ring from some other task, but I think you have already figured that out based on the switch_mm suggestion. The simple truth is I was not familiar with switch_mm at the time I wrote this (nor am I now). If this is a superior method that allows you to acquire c_t_u(some_other_ctx) like behavior, I see no problem in changing. I will look into this, and thanks for the suggestion!

copy_to_user() is significantly faster than get_user_pages() + kmap() + memcmp() (or their variants).

Signature · 2 lines
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.
#7 lore raw

Re: [RFC PATCH v2 03/19] vbus: add connection-client helper infrastructure

Quoted 17 lines · Avi Kivity
Quoted 5 lines · Gregory Haskins
Quoted 4 lines

BTW, why did you decide to use get_user_pages?
Would switch_mm + copy_to_user work as well
avoiding page walk if all pages are present?

Quoted 8 lines

Well, basic c_t_u() won't work because its likely not "current" if you are updating the ring from some other task, but I think you have already figured that out based on the switch_mm suggestion. The simple truth is I was not familiar with switch_mm at the time I wrote this (nor am I now). If this is a superior method that allows you to acquire c_t_u(some_other_ctx) like behavior, I see no problem in changing. I will look into this, and thanks for the suggestion!

Quoted 3 lines

copy_to_user() is significantly faster than get_user_pages() + kmap()
+ memcmp() (or their variants).

Oh, I don't doubt that (in fact, I was pretty sure that was the case based on some of the optimizations I could see in studying the c_t_u() path). I just didn't realize there were other ways to do it if its a non "current" task. ;)

I guess the enigma for me right now is what cost does switch_mm have? (Thats not a slam against the suggested approach...I really do not know and am curious).

As an aside, note that we seem to be reviewing v2, where v3 is really the last set I pushed. I think this patch is more or less the same across both iterations, but FYI that I would recommend looking at v3 instead.

-Greg

#8 lore raw

Re: [RFC PATCH v2 03/19] vbus: add connection-client helper infrastructure

Quoted 8 lines · Gregory Haskins

Oh, I don't doubt that (in fact, I was pretty sure that was the case based on some of the optimizations I could see in studying the c_t_u() path). I just didn't realize there were other ways to do it if its a non "current" task. ;)

I guess the enigma for me right now is what cost does switch_mm have? (Thats not a slam against the suggested approach...I really do not know and am curious).

switch_mm() is probably very cheap (reloads cr3), but it does dirty the current cpu's tlb. When the kernel needs to flush a process' tlb, it will have to IPI that cpu in addition to all others. This takes place, for example, after munmap() or after a page is swapped out (though significant batching is done there).

It's still plenty cheaper in my estimation.

Signature · 2 lines
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.
#9 lore raw

Re: [RFC PATCH v2 03/19] vbus: add connection-client helper infrastructure

In reply to Michael S. Tsirkin
Quoted 4 lines · Michael S. Tsirkin

Also - if we just had vmexit because a process executed io (or hypercall), can't we just do copy_to_user there? Avi, I think at some point you said that we can?

You can do copy_to_user() whereever it is legal in Linux. Almost all of kvm runs in process context, preemptible, and with interrupts enabled.

Signature · 2 lines
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.
#10 lore raw

RFC PATCH v2 4/19 vbus: add bus-registration notifiers

In reply to Gregory Haskins

We need to get hotswap events in environments which cannot use existing facilities (e.g. inotify). So we add a notifier-chain to allow client callbacks whenever an interface is {un}registered.

---

 include/linux/vbus.h |   15 +++++++++++++
 kernel/vbus/core.c   |   59 ++++++++++++++++++++++++++++++++++++++++++++++++++
 kernel/vbus/vbus.h   |    1 +
 3 files changed, 75 insertions(+), 0 deletions(-)
include/linux/vbus.h +15 -0
index 5f0566c..04db4ff 100644
@@ -29,6 +29,7 @@
 #include <linux/sched.h>
 #include <linux/rcupdate.h>
 #include <linux/vbus_device.h>
+#include <linux/notifier.h>
 
 struct vbus;
 struct task_struct;
@@ -137,6 +138,20 @@ static inline void task_vbus_disassociate(struct task_struct *p)
 	}
 }
 
+enum {
+	VBUS_EVENT_DEVADD,
+	VBUS_EVENT_DEVDROP,
+};
+
+struct vbus_event_devadd {
+	const char   *type;
+	unsigned long id;
+};
+
+int vbus_notifier_register(struct vbus *vbus, struct notifier_block *nb);
+int vbus_notifier_unregister(struct vbus *vbus, struct notifier_block *nb);
+
+
 #else /* CONFIG_VBUS */
 
 #define fork_vbus(p) do { } while (0)
kernel/vbus/core.c +59 -0

89 lines not on this page · raw message

kernel/vbus/vbus.h +1 -0
index 1266d69..cd2676b 100644
@@ -51,6 +51,7 @@ struct vbus {
 	struct vbus_subdir members;
 	unsigned long next_id;
 	struct rb_node node;
+	struct raw_notifier_head notifier;
 };
 
 struct vbus_member {
#11 lore raw

RFC PATCH v2 5/19 vbus: add a "vbus-proxy" bus model for vbus_driver objects

In reply to Gregory Haskins

This will generally be used for hypervisors to publish any host-side virtual devices up to a guest. The guest will have the opportunity to consume any devices present on the vbus-proxy as if they were platform devices, similar to existing buses like PCI.

---

 include/linux/vbus_driver.h |   73 +++++++++++++++++++++
 kernel/vbus/Kconfig         |    9 +++
 kernel/vbus/Makefile        |    4 +
 kernel/vbus/proxy.c         |  152 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 238 insertions(+), 0 deletions(-)
 create mode 100644 include/linux/vbus_driver.h
 create mode 100644 kernel/vbus/proxy.c
include/linux/vbus_driver.h new file +73 -0
new file mode 100644
index 0000000..c53e13f
@@ -0,0 +1,73 @@
+/*
+ * Copyright 2009 Novell.  All Rights Reserved.
+ *
+ * Mediates access to a host VBUS from a guest kernel by providing a
+ * global view of all VBUS devices
+ *
+ * Author:
+ *      Gregory Haskins <[email protected]>
+ *
+ * This file is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA.
+ */
+
+#ifndef _LINUX_VBUS_DRIVER_H
+#define _LINUX_VBUS_DRIVER_H
+
+#include <linux/device.h>
+#include <linux/shm_signal.h>
+
+struct vbus_device_proxy;
+struct vbus_driver;
+
+struct vbus_device_proxy_ops {
+	int (*open)(struct vbus_device_proxy *dev, int version, int flags);
+	int (*close)(struct vbus_device_proxy *dev, int flags);
+	int (*shm)(struct vbus_device_proxy *dev, int id, int prio,
+		   void *ptr, size_t len,
+		   struct shm_signal_desc *sigdesc, struct shm_signal **signal,
+		   int flags);
+	int (*call)(struct vbus_device_proxy *dev, u32 func,
+		    void *data, size_t len, int flags);
+	void (*release)(struct vbus_device_proxy *dev);
+};
+
+struct vbus_device_proxy {
+	char                          *type;
+	u64                            id;
+	void                          *priv; /* Used by drivers */
+	struct vbus_device_proxy_ops  *ops;
+	struct device                  dev;
+};
+
+int vbus_device_proxy_register(struct vbus_device_proxy *dev);
+void vbus_device_proxy_unregister(struct vbus_device_proxy *dev);
+
+struct vbus_device_proxy *vbus_device_proxy_find(u64 id);
+
+struct vbus_driver_ops {
+	int (*probe)(struct vbus_device_proxy *dev);
+	int (*remove)(struct vbus_device_proxy *dev);
+};
+
+struct vbus_driver {
+	char                          *type;
+	struct module                 *owner;
+	struct vbus_driver_ops        *ops;
+	struct device_driver           drv;
+};
+
+int vbus_driver_register(struct vbus_driver *drv);
+void vbus_driver_unregister(struct vbus_driver *drv);
+
+#endif /* _LINUX_VBUS_DRIVER_H */
kernel/vbus/Kconfig +9 -0
index f2b92f5..3aaa085 100644
@@ -12,3 +12,12 @@ config VBUS
 	various tasks and devices which reside on the bus.
 
 	If unsure, say N
+
+config VBUS_DRIVERS
+       tristate "VBUS Driver support"
+       default n
+       help
+        Adds support for a virtual bus model for proxying drivers.
+
+	If unsure, say N
+
kernel/vbus/Makefile +4 -0
index 4d440e5..d028ece 100644
@@ -1 +1,5 @@
 obj-$(CONFIG_VBUS) += core.o devclass.o config.o attribute.o map.o client.o
+
+vbus-proxy-objs += proxy.o
+obj-$(CONFIG_VBUS_DRIVERS) += vbus-proxy.o
+
kernel/vbus/proxy.c new file +152 -0

152 lines not on this page · raw message

#12 lore raw

RFC PATCH v2 6/19 ioq: Add basic definitions for a shared-memory, lockless queue

In reply to Gregory Haskins

We can map these over VBUS shared memory (or really any shared-memory architecture if it supports shm-signals) to allow asynchronous communication between two end-points. Memory is synchronized using pure barriers (i.e. lockless), so IOQs are friendly in many contexts, even if the memory is remote.

---

 include/linux/ioq.h |  410 +++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/Kconfig         |   12 +
 lib/Makefile        |    1 
 lib/ioq.c           |  298 +++++++++++++++++++++++++++++++++++++
 4 files changed, 721 insertions(+), 0 deletions(-)
 create mode 100644 include/linux/ioq.h
 create mode 100644 lib/ioq.c
include/linux/ioq.h new file +410 -0
new file mode 100644
index 0000000..d450d9a
@@ -0,0 +1,410 @@
+/*
+ * Copyright 2009 Novell.  All Rights Reserved.
+ *
+ * IOQ is a generic shared-memory, lockless queue mechanism. It can be used
+ * in a variety of ways, though its intended purpose is to become the
+ * asynchronous communication path for virtual-bus drivers.
+ *
+ * The following are a list of key design points:
+ *
+ * #) All shared-memory is always allocated on explicitly one side of the
+ *    link.  This typically would be the guest side in a VM/VMM scenario.
+ * #) Each IOQ has the concept of "north" and "south" locales, where
+ *    north denotes the memory-owner side (e.g. guest).
+ * #) An IOQ is manipulated using an iterator idiom.
+ * #) Provides a bi-directional signaling/notification infrastructure on
+ *    a per-queue basis, which includes an event mitigation strategy
+ *    to reduce boundary switching.
+ * #) The signaling path is abstracted so that various technologies and
+ *    topologies can define their own specific implementation while sharing
+ *    the basic structures and code.
+ *
+ * Author:
+ *      Gregory Haskins <[email protected]>
+ *
+ * This file is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA.
+ */
+
+#ifndef _LINUX_IOQ_H
+#define _LINUX_IOQ_H
+
+#include <asm/types.h>
+#include <linux/shm_signal.h>
+
+/*
+ *---------
+ * The following structures represent data that is shared across boundaries
+ * which may be quite disparate from one another (e.g. Windows vs Linux,
+ * 32 vs 64 bit, etc).  Therefore, care has been taken to make sure they
+ * present data in a manner that is independent of the environment.
+ *-----------
+ */
+struct ioq_ring_desc {
+	__u64                 cookie; /* for arbitrary use by north-side */
+	__u64                 ptr;
+	__u64                 len;
+	__u8                  valid;
+	__u8                  sown; /* South owned = 1, North owned = 0 */
+};
+
+#define IOQ_RING_MAGIC 0x47fa2fe4
+#define IOQ_RING_VER   4
+
+struct ioq_ring_idx {
+	__u32                 head;    /* 0 based index to head of ptr array */
+	__u32                 tail;    /* 0 based index to tail of ptr array */
+	__u8                  full;
+};
+
+enum ioq_locality {
+	ioq_locality_north,
+	ioq_locality_south,
+};
+
+struct ioq_ring_head {
+	__u32                  magic;
+	__u32                  ver;
+	struct shm_signal_desc signal;
+	struct ioq_ring_idx    idx[2];
+	__u32                  count;
+	struct ioq_ring_desc   ring[1]; /* "count" elements will be allocated */
+};
+
+#define IOQ_HEAD_DESC_SIZE(count) \
+    (sizeof(struct ioq_ring_head) + sizeof(struct ioq_ring_desc) * (count - 1))
+
+/* --- END SHARED STRUCTURES --- */
+
+#ifdef __KERNEL__
+
+#include <linux/sched.h>
+#include <linux/wait.h>
+#include <linux/interrupt.h>
+#include <linux/shm_signal.h>
+#include <asm/atomic.h>
+
+enum ioq_idx_type {
+	ioq_idxtype_valid,
+	ioq_idxtype_inuse,
+	ioq_idxtype_both,
+	ioq_idxtype_invalid,
+};
+
+enum ioq_seek_type {
+	ioq_seek_tail,
+	ioq_seek_next,
+	ioq_seek_head,
+	ioq_seek_set
+};
+
+struct ioq_iterator {
+	struct ioq            *ioq;
+	struct ioq_ring_idx   *idx;
+	u32                    pos;
+	struct ioq_ring_desc  *desc;
+	int                    update:1;
+	int                    dualidx:1;
+	int                    flipowner:1;
+};
+
+struct ioq_notifier {
+	void (*signal)(struct ioq_notifier *);
+};
+
+struct ioq_ops {
+	void     (*release)(struct ioq *ioq);
+};
+
+struct ioq {
+	struct ioq_ops *ops;
+
+	atomic_t               refs;
+	enum ioq_locality      locale;
+	struct ioq_ring_head  *head_desc;
+	struct ioq_ring_desc  *ring;
+	struct shm_signal     *signal;
+	wait_queue_head_t      wq;
+	struct ioq_notifier   *notifier;
+	size_t                 count;
+	struct shm_signal_notifier shm_notifier;
+};
+
+#define IOQ_ITER_AUTOUPDATE  (1 << 0)
+#define IOQ_ITER_NOFLIPOWNER (1 << 1)
+
+/**
+ * ioq_init() - initialize an IOQ
+ * @ioq:        IOQ context
+ *
+ * Initializes IOQ context before first use
+ *
+ **/
+void ioq_init(struct ioq *ioq,
+	      struct ioq_ops *ops,
+	      enum ioq_locality locale,
+	      struct ioq_ring_head *head,
+	      struct shm_signal *signal,
+	      size_t count);
+
+/**
+ * ioq_get() - acquire an IOQ context reference
+ * @ioq:        IOQ context
+ *
+ **/
+static inline struct ioq *ioq_get(struct ioq *ioq)
+{
+	atomic_inc(&ioq->refs);
+
+	return ioq;
+}
+
+/**
+ * ioq_put() - release an IOQ context reference
+ * @ioq:        IOQ context
+ *
+ **/
+static inline void ioq_put(struct ioq *ioq)
+{
+	if (atomic_dec_and_test(&ioq->refs)) {
+		shm_signal_put(ioq->signal);
+		ioq->ops->release(ioq);
+	}
+}
+
+/**
+ * ioq_notify_enable() - enables local notifications on an IOQ
+ * @ioq:        IOQ context
+ * @flags:      Reserved for future use, must be 0
+ *
+ * Enables/unmasks the registered ioq_notifier (if applicable) and waitq to
+ * receive wakeups whenever the remote side performs an ioq_signal() operation.
+ * A notification will be dispatched immediately if any pending signals have
+ * already been issued prior to invoking this call.
+ *
+ * This is synonymous with unmasking an interrupt.
+ *
+ * Returns: success = 0, <0 = ERRNO
+ *
+ **/
+static inline int ioq_notify_enable(struct ioq *ioq, int flags)
+{
+	return shm_signal_enable(ioq->signal, 0);
+}
+
+/**
+ * ioq_notify_disable() - disable local notifications on an IOQ
+ * @ioq:        IOQ context
+ * @flags:      Reserved for future use, must be 0
+ *
+ * Disables/masks the registered ioq_notifier (if applicable) and waitq
+ * from receiving any further notifications.  Any subsequent calls to
+ * ioq_signal() by the remote side will update the ring as dirty, but
+ * will not traverse the locale boundary and will not invoke the notifier
+ * callback or wakeup the waitq.  Signals delivered while masked will
+ * be deferred until ioq_notify_enable() is invoked
+ *
+ * This is synonymous with masking an interrupt
+ *
+ * Returns: success = 0, <0 = ERRNO
+ *
+ **/
+static inline int ioq_notify_disable(struct ioq *ioq, int flags)
+{
+	return shm_signal_disable(ioq->signal, 0);
+}
+
+/**
+ * ioq_signal() - notify the remote side about ring changes
+ * @ioq:        IOQ context
+ * @flags:      Reserved for future use, must be 0
+ *
+ * Marks the ring state as "dirty" and, if enabled, will traverse
+ * a locale boundary to invoke a remote notification.  The remote
+ * side controls whether the notification should be delivered via
+ * the ioq_notify_enable/disable() interface.
+ *
+ * The specifics of how to traverse a locale boundary are abstracted
+ * by the ioq_ops->signal() interface and provided by a particular
+ * implementation.  However, typically going north to south would be
+ * something like a syscall/hypercall, and going south to north would be
+ * something like a posix-signal/guest-interrupt.
+ *
+ * Returns: success = 0, <0 = ERRNO
+ *
+ **/
+static inline int ioq_signal(struct ioq *ioq, int flags)
+{
+	return shm_signal_inject(ioq->signal, 0);
+}
+
+/**
+ * ioq_count() - counts the number of outstanding descriptors in an index
+ * @ioq:        IOQ context
+ * @type:	Specifies the index type
+ *                 (*) valid: the descriptor is valid.  This is usually
+ *                     used to keep track of descriptors that may not
+ *                     be carrying a useful payload, but still need to
+ *                     be tracked carefully.
+ *                 (*) inuse: Descriptors that carry useful payload
+ *
+ * Returns:
+ *  (*) >=0: # of descriptors outstanding in the index
+ *  (*) <0 = ERRNO
+ *
+ **/
+int ioq_count(struct ioq *ioq, enum ioq_idx_type type);
+
+/**
+ * ioq_remain() - counts the number of remaining descriptors in an index
+ * @ioq:        IOQ context
+ * @type:	Specifies the index type
+ *                 (*) valid: the descriptor is valid.  This is usually
+ *                     used to keep track of descriptors that may not
+ *                     be carrying a useful payload, but still need to
+ *                     be tracked carefully.
+ *                 (*) inuse: Descriptors that carry useful payload
+ *
+ * This is the converse of ioq_count().  This function returns the number
+ * of "free" descriptors left in a particular index
+ *
+ * Returns:
+ *  (*) >=0: # of descriptors remaining in the index
+ *  (*) <0 = ERRNO
+ *
+ **/
+int ioq_remain(struct ioq *ioq, enum ioq_idx_type type);
+
+/**
+ * ioq_size() - counts the maximum number of descriptors in an ring
+ * @ioq:        IOQ context
+ *
+ * This function returns the maximum number of descriptors supported in
+ * a ring, regardless of their current state (free or inuse).
+ *
+ * Returns:
+ *  (*) >=0: total # of descriptors in the ring
+ *  (*) <0 = ERRNO
+ *
+ **/
+int ioq_size(struct ioq *ioq);
+
+/**
+ * ioq_full() - determines if a specific index is "full"
+ * @ioq:        IOQ context
+ * @type:	Specifies the index type
+ *                 (*) valid: the descriptor is valid.  This is usually
+ *                     used to keep track of descriptors that may not
+ *                     be carrying a useful payload, but still need to
+ *                     be tracked carefully.
+ *                 (*) inuse: Descriptors that carry useful payload
+ *
+ * Returns:
+ *  (*) 0: index is not full
+ *  (*) 1: index is full
+ *  (*) <0 = ERRNO
+ *
+ **/
+int ioq_full(struct ioq *ioq, enum ioq_idx_type type);
+
+/**
+ * ioq_empty() - determines if a specific index is "empty"
+ * @ioq:        IOQ context
+ * @type:	Specifies the index type
+ *                 (*) valid: the descriptor is valid.  This is usually
+ *                     used to keep track of descriptors that may not
+ *                     be carrying a useful payload, but still need to
+ *                     be tracked carefully.
+ *                 (*) inuse: Descriptors that carry useful payload
+ *
+ * Returns:
+ *  (*) 0: index is not empty
+ *  (*) 1: index is empty
+ *  (*) <0 = ERRNO
+ *
+ **/
+static inline int ioq_empty(struct ioq *ioq, enum ioq_idx_type type)
+{
+    return !ioq_count(ioq, type);
+}
+
+/**
+ * ioq_iter_init() - initialize an iterator for IOQ descriptor traversal
+ * @ioq:        IOQ context to iterate on
+ * @iter:	Iterator context to init (usually from stack)
+ * @type:	Specifies the index type to iterate against
+ *                 (*) valid: iterate against the "valid" index
+ *                 (*) inuse: iterate against the "inuse" index
+ *                 (*) both: iterate against both indexes simultaneously
+ * @flags:      Bitfield with 0 or more bits set to alter behavior
+ *                 (*) autoupdate: automatically signal the remote side
+ *                     whenever the iterator pushes/pops to a new desc
+ *                 (*) noflipowner: do not flip the ownership bit during
+ *                     a push/pop operation
+ *
+ * Returns: success = 0, <0 = ERRNO
+ *
+ **/
+int ioq_iter_init(struct ioq *ioq, struct ioq_iterator *iter,
+		  enum ioq_idx_type type, int flags);
+
+/**
+ * ioq_iter_seek() - seek to a specific location in the IOQ ring
+ * @iter:	Iterator context (must be initialized with ioq_iter_init)
+ * @type:	Specifies the type of seek operation
+ *                 (*) tail: seek to the absolute tail, offset is ignored
+ *                 (*) next: seek to the relative next, offset is ignored
+ *                 (*) head: seek to the absolute head, offset is ignored
+ *                 (*) set: seek to the absolute offset
+ * @offset:     Offset for ioq_seek_set operations
+ * @flags:      Reserved for future use, must be 0
+ *
+ * Returns: success = 0, <0 = ERRNO
+ *
+ **/
+int  ioq_iter_seek(struct ioq_iterator *iter, enum ioq_seek_type type,
+		   long offset, int flags);
+
+/**
+ * ioq_iter_push() - push the tail pointer forward
+ * @iter:	Iterator context (must be initialized with ioq_iter_init)
+ * @flags:      Reserved for future use, must be 0
+ *
+ * This function will simultaneously advance the tail ptr in the current
+ * index (valid/inuse, as specified in the ioq_iter_init) as well as
+ * perform a seek(next) operation.  This effectively "pushes" a new pointer
+ * onto the tail of the index.
+ *
+ * Returns: success = 0, <0 = ERRNO
+ *
+ **/
+int  ioq_iter_push(struct ioq_iterator *iter, int flags);
+
+/**
+ * ioq_iter_pop() - pop the head pointer from the ring
+ * @iter:	Iterator context (must be initialized with ioq_iter_init)
+ * @flags:      Reserved for future use, must be 0
+ *
+ * This function will simultaneously advance the head ptr in the current
+ * index (valid/inuse, as specified in the ioq_iter_init) as well as
+ * perform a seek(next) operation.  This effectively "pops" a pointer
+ * from the head of the index.
+ *
+ * Returns: success = 0, <0 = ERRNO
+ *
+ **/
+int  ioq_iter_pop(struct ioq_iterator *iter,  int flags);
+
+#endif /* __KERNEL__ */
+
+#endif /* _LINUX_IOQ_H */
lib/Kconfig +12 -0
index 32d82fe..1e66f8e 100644
@@ -183,5 +183,17 @@ config SHM_SIGNAL
 
 	 If unsure, say N
 
+config IOQ
+	boolean "IO-Queue library - Generic shared-memory queue"
+	select SHM_SIGNAL
+	default n
+	help
+	 IOQ is a generic shared-memory-queue mechanism that happens to be
+	 friendly to virtualization boundaries. It can be used in a variety
+	 of ways, though its intended purpose is to become the low-level
+	 communication path for paravirtualized drivers.
+
+	 If unsure, say N
+
 
 endmenu
lib/Makefile +1 -0
index bc36327..98cd332 100644
@@ -72,6 +72,7 @@ obj-$(CONFIG_TEXTSEARCH_FSM) += ts_fsm.o
 obj-$(CONFIG_SMP) += percpu_counter.o
 obj-$(CONFIG_AUDIT_GENERIC) += audit.o
 obj-$(CONFIG_SHM_SIGNAL) += shm_signal.o
+obj-$(CONFIG_IOQ) += ioq.o
 
 obj-$(CONFIG_SWIOTLB) += swiotlb.o
 obj-$(CONFIG_IOMMU_HELPER) += iommu-helper.o
lib/ioq.c new file +298 -0

298 lines not on this page · raw message

#13 lore raw

RFC PATCH v2 7/19 ioq: add vbus helpers

In reply to Gregory Haskins

It will be common to map an IOQ over the VBUS shared-memory interfaces, so lets generalize their setup so we can reuse the pattern.

---

 include/linux/vbus_device.h |    7 +++
 include/linux/vbus_driver.h |    7 +++
 kernel/vbus/Kconfig         |    2 +
 kernel/vbus/Makefile        |    1 
 kernel/vbus/proxy.c         |   64 +++++++++++++++++++++++++++++++
 kernel/vbus/shm-ioq.c       |   89 +++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 170 insertions(+), 0 deletions(-)
 create mode 100644 kernel/vbus/shm-ioq.c
include/linux/vbus_device.h +7 -0
index f73cd86..c91bce4 100644
@@ -102,6 +102,7 @@
 #include <linux/configfs.h>
 #include <linux/rbtree.h>
 #include <linux/shm_signal.h>
+#include <linux/ioq.h>
 #include <linux/vbus.h>
 #include <asm/atomic.h>
 
@@ -414,4 +415,10 @@ static inline void vbus_connection_put(struct vbus_connection *conn)
 		conn->ops->release(conn);
 }
 
+/*
+ * device-side IOQ helper - dereferences device-shm as an IOQ
+ */
+int vbus_shm_ioq_attach(struct vbus_shm *shm, struct shm_signal *signal,
+			int maxcount, struct ioq **ioq);
+
 #endif /* _LINUX_VBUS_DEVICE_H */
include/linux/vbus_driver.h +7 -0
index c53e13f..9cfbf60 100644
@@ -26,6 +26,7 @@
 
 #include <linux/device.h>
 #include <linux/shm_signal.h>
+#include <linux/ioq.h>
 
 struct vbus_device_proxy;
 struct vbus_driver;
@@ -70,4 +71,10 @@ struct vbus_driver {
 int vbus_driver_register(struct vbus_driver *drv);
 void vbus_driver_unregister(struct vbus_driver *drv);
 
+/*
+ * driver-side IOQ helper - allocates device-shm and maps an IOQ on it
+ */
+int vbus_driver_ioq_alloc(struct vbus_device_proxy *dev, int id, int prio,
+			  size_t ringsize, struct ioq **ioq);
+
 #endif /* _LINUX_VBUS_DRIVER_H */
kernel/vbus/Kconfig +2 -0
index 3aaa085..71acd6f 100644
@@ -6,6 +6,7 @@ config VBUS
        bool "Virtual Bus"
        select CONFIGFS_FS
        select SHM_SIGNAL
+       select IOQ
        default n
        help
         Provides a mechansism for declaring virtual-bus objects and binding
@@ -15,6 +16,7 @@ config VBUS
 
 config VBUS_DRIVERS
        tristate "VBUS Driver support"
+       select IOQ
        default n
        help
         Adds support for a virtual bus model for proxying drivers.
kernel/vbus/Makefile +1 -0
index d028ece..45f6503 100644
@@ -1,4 +1,5 @@
 obj-$(CONFIG_VBUS) += core.o devclass.o config.o attribute.o map.o client.o
+obj-$(CONFIG_VBUS) += shm-ioq.o
 
 vbus-proxy-objs += proxy.o
 obj-$(CONFIG_VBUS_DRIVERS) += vbus-proxy.o
kernel/vbus/proxy.c +64 -0

67 lines not on this page · raw message

kernel/vbus/shm-ioq.c new file +89 -0

89 lines not on this page · raw message

#14 lore raw

RFC PATCH v2 8/19 venet: add the ABI definitions for an 802.x packet interface

In reply to Gregory Haskins

---

 include/linux/venet.h |   47 +++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 47 insertions(+), 0 deletions(-)
 create mode 100644 include/linux/venet.h
include/linux/venet.h new file +47 -0
new file mode 100644
index 0000000..ef6b199
@@ -0,0 +1,47 @@
+/*
+ * Copyright 2008 Novell.  All Rights Reserved.
+ *
+ * Virtual-Ethernet adapter
+ *
+ * Author:
+ *      Gregory Haskins <[email protected]>
+ *
+ * This file is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA.
+ */
+
+#ifndef _LINUX_VENET_H
+#define _LINUX_VENET_H
+
+#define VENET_VERSION 1
+
+#define VENET_TYPE "virtual-ethernet"
+
+#define VENET_QUEUE_RX 0
+#define VENET_QUEUE_TX 1
+
+struct venet_capabilities {
+	__u32 gid;
+	__u32 bits;
+};
+
+/* CAPABILITIES-GROUP 0 */
+/* #define VENET_CAP_FOO    0   (No capabilities defined yet, for now) */
+
+#define VENET_FUNC_LINKUP   0
+#define VENET_FUNC_LINKDOWN 1
+#define VENET_FUNC_MACQUERY 2
+#define VENET_FUNC_NEGCAP   3 /* negotiate capabilities */
+#define VENET_FUNC_FLUSHRX  4
+
+#endif /* _LINUX_VENET_H */
#15 lore raw

RFC PATCH v2 9/19 net: Add vbus_enet driver

In reply to Gregory Haskins

---

 drivers/net/Kconfig     |   13 +
 drivers/net/Makefile    |    1 
 drivers/net/vbus-enet.c |  680 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 694 insertions(+), 0 deletions(-)
 create mode 100644 drivers/net/vbus-enet.c
drivers/net/Kconfig +13 -0
index 62d732a..ac9dabd 100644
@@ -3099,4 +3099,17 @@ config VIRTIO_NET
 	  This is the virtual network driver for virtio.  It can be used with
           lguest or QEMU based VMMs (like KVM or Xen).  Say Y or M.
 
+config VBUS_ENET
+	tristate "Virtual Ethernet Driver"
+	depends on VBUS_DRIVERS
+	help
+	   A virtualized 802.x network device based on the VBUS interface.
+	   It can be used with any hypervisor/kernel that supports the
+	   vbus protocol.
+
+config VBUS_ENET_DEBUG
+        bool "Enable Debugging"
+	depends on VBUS_ENET
+	default n
+
 endif # NETDEVICES
drivers/net/Makefile +1 -0
index 471baaf..61db928 100644
@@ -264,6 +264,7 @@ obj-$(CONFIG_FS_ENET) += fs_enet/
 obj-$(CONFIG_NETXEN_NIC) += netxen/
 obj-$(CONFIG_NIU) += niu.o
 obj-$(CONFIG_VIRTIO_NET) += virtio_net.o
+obj-$(CONFIG_VBUS_ENET) += vbus-enet.o
 obj-$(CONFIG_SFC) += sfc/
 
 obj-$(CONFIG_WIMAX) += wimax/
drivers/net/vbus-enet.c new file +680 -0

680 lines not on this page · raw message

#16 lore raw

Re: [RFC PATCH v2 09/19] net: Add vbus_enet driver

Quoted 76 lines · On Thu, 09 Apr 2009 12:31:29 -0400 Gregory Haskins <[email protected]> wrote:

---

 drivers/net/Kconfig     |   13 +
 drivers/net/Makefile    |    1 
 drivers/net/vbus-enet.c |  680 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 694 insertions(+), 0 deletions(-)
 create mode 100644 drivers/net/vbus-enet.c
drivers/net/Kconfig +13 -0
index 62d732a..ac9dabd 100644
@@ -3099,4 +3099,17 @@ config VIRTIO_NET
 	  This is the virtual network driver for virtio.  It can be used with
           lguest or QEMU based VMMs (like KVM or Xen).  Say Y or M.
 
+config VBUS_ENET
+	tristate "Virtual Ethernet Driver"
+	depends on VBUS_DRIVERS
+	help
+	   A virtualized 802.x network device based on the VBUS interface.
+	   It can be used with any hypervisor/kernel that supports the
+	   vbus protocol.
+
+config VBUS_ENET_DEBUG
+        bool "Enable Debugging"
+	depends on VBUS_ENET
+	default n
+
 endif # NETDEVICES
drivers/net/Makefile +1 -0
index 471baaf..61db928 100644
@@ -264,6 +264,7 @@ obj-$(CONFIG_FS_ENET) += fs_enet/
 obj-$(CONFIG_NETXEN_NIC) += netxen/
 obj-$(CONFIG_NIU) += niu.o
 obj-$(CONFIG_VIRTIO_NET) += virtio_net.o
+obj-$(CONFIG_VBUS_ENET) += vbus-enet.o
 obj-$(CONFIG_SFC) += sfc/
 
 obj-$(CONFIG_WIMAX) += wimax/
drivers/net/vbus-enet.c new file +27 -0
new file mode 100644
index 0000000..3779f77
@@ -0,0 +1,680 @@
+/*
+ * vbus_enet - A virtualized 802.x network device based on the VBUS interface
+ *
+ * Copyright (C) 2009 Novell, Gregory Haskins <[email protected]>
+ *
+ * Derived from the SNULL example from the book "Linux Device Drivers" by
+ * Alessandro Rubini, Jonathan Corbet, and Greg Kroah-Hartman, published
+ * by O'Reilly & Associates.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/moduleparam.h>
+
+#include <linux/sched.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/errno.h>
+#include <linux/types.h>
+#include <linux/interrupt.h>
+
+#include <linux/in.h>
+#include <linux/netdevice.h>
+#include <linux/etherdevice.h>
+#include <linux/ip.h>
+#include <linux/tcp.h>
+#include <linux/skbuff.h>

drivers/net/vbus-enet.c line 37

Quoted 10 lines
drivers/net/vbus-enet.c new file +10 -0
+#include <linux/ioq.h>
+#include <linux/vbus_driver.h>
+
+#include <linux/in6.h>
+#include <asm/checksum.h>
+
+#include <linux/venet.h>
+
+MODULE_AUTHOR("Gregory Haskins");
+MODULE_LICENSE("GPL");

MODULE_DESCRIPTION ?
MODULE_VERSION ?

drivers/net/vbus-enet.c line 40

Quoted 2 lines
drivers/net/vbus-enet.c new file +2 -0
+static int napi_weight = 128;
+module_param(napi_weight, int, 0444);

Already accessible through sysfs

drivers/net/vbus-enet.c line 42

Quoted 2 lines
drivers/net/vbus-enet.c new file +2 -0
+static int rx_ringlen = 256;
+module_param(rx_ringlen, int, 0444);

API for ring length exists via ethtool. If you used this then there would be no need for device special parameter.

drivers/net/vbus-enet.c line 51

Quoted 9 lines
drivers/net/vbus-enet.c new file +9 -0
+static int tx_ringlen = 256;
+module_param(tx_ringlen, int, 0444);
+
+#undef PDEBUG             /* undef it, just in case */
+#ifdef VBUS_ENET_DEBUG
+#  define PDEBUG(fmt, args...) printk(KERN_DEBUG "vbus_enet: " fmt, ## args)
+#else
+#  define PDEBUG(fmt, args...) /* not debugging: nothing */
+#endif

Why reinvent pr_debug()?

Quoted 59 lines
drivers/net/vbus-enet.c new file +59 -0
+
+struct vbus_enet_queue {
+	struct ioq              *queue;
+	struct ioq_notifier      notifier;
+};
+
+struct vbus_enet_priv {
+	spinlock_t                 lock;
+	struct net_device         *dev;
+	struct vbus_device_proxy  *vdev;
+	struct napi_struct         napi;
+	struct vbus_enet_queue     rxq;
+	struct vbus_enet_queue     txq;
+	struct tasklet_struct      txtask;
+};
+
+static struct vbus_enet_priv *
+napi_to_priv(struct napi_struct *napi)
+{
+	return container_of(napi, struct vbus_enet_priv, napi);
+}
+
+static int
+queue_init(struct vbus_enet_priv *priv,
+	   struct vbus_enet_queue *q,
+	   int qid,
+	   size_t ringsize,
+	   void (*func)(struct ioq_notifier *))
+{
+	struct vbus_device_proxy *dev = priv->vdev;
+	int ret;
+
+	ret = vbus_driver_ioq_alloc(dev, qid, 0, ringsize, &q->queue);
+	if (ret < 0)
+		panic("ioq_alloc failed: %d\n", ret);
+
+	if (func) {
+		q->notifier.signal = func;
+		q->queue->notifier = &q->notifier;
+	}
+
+	return 0;
+}
+
+static int
+devcall(struct vbus_enet_priv *priv, u32 func, void *data, size_t len)
+{
+	struct vbus_device_proxy *dev = priv->vdev;
+
+	return dev->ops->call(dev, func, data, len, 0);
+}
+
+/*
+ * ---------------
+ * rx descriptors
+ * ---------------
+ */
+
+static void

drivers/net/vbus-enet.c line 120

Quoted 10 lines
drivers/net/vbus-enet.c new file +10 -0
+rxdesc_alloc(struct ioq_ring_desc *desc, size_t len)
+{
+	struct sk_buff *skb;
+
+	len += ETH_HLEN;
+
+	skb = dev_alloc_skb(len + 2);
+	BUG_ON(!skb);
+
+	skb_reserve(skb, 2); /* align IP on 16B boundary */

Use NET_IP_ALIGN rather than 2

use netdev_alloc_skb because it NUMA aware.

Quoted 14 lines
drivers/net/vbus-enet.c new file +14 -0
+
+	desc->cookie = (u64)skb;
+	desc->ptr    = (u64)__pa(skb->data);
+	desc->len    = len; /* total length  */
+	desc->valid  = 1;
+}
+
+static void
+rx_setup(struct vbus_enet_priv *priv)
+{
+	struct ioq *ioq = priv->rxq.queue;
+	struct ioq_iterator iter;
+	int ret;
+

drivers/net/vbus-enet.c line 144

Quoted 10 lines
drivers/net/vbus-enet.c new file +10 -0
+	/*
+	 * We want to iterate on the "valid" index.  By default the iterator
+	 * will not "autoupdate" which means it will not hypercall the host
+	 * with our changes.  This is good, because we are really just
+	 * initializing stuff here anyway.  Note that you can always manually
+	 * signal the host with ioq_signal() if the autoupdate feature is not
+	 * used.
+	 */
+	ret = ioq_iter_init(ioq, &iter, ioq_idxtype_valid, 0);
+	BUG_ON(ret < 0);

Why not doing proper initialization error handling, I.e fail the attempt to bring device up with error code (-ENOMEM)...

Quoted 400 lines
drivers/net/vbus-enet.c new file +400 -0
+	/*
+	 * Seek to the tail of the valid index (which should be our first
+	 * item, since the queue is brand-new)
+	 */
+	ret = ioq_iter_seek(&iter, ioq_seek_tail, 0, 0);
+	BUG_ON(ret < 0);
+
+	/*
+	 * Now populate each descriptor with an empty SKB and mark it valid
+	 */
+	while (!iter.desc->valid) {
+		rxdesc_alloc(iter.desc, priv->dev->mtu);
+
+		/*
+		 * This push operation will simultaneously advance the
+		 * valid-head index and increment our position in the queue
+		 * by one.
+		 */
+		ret = ioq_iter_push(&iter, 0);
+		BUG_ON(ret < 0);
+	}
+}
+
+static void
+rx_teardown(struct vbus_enet_priv *priv)
+{
+	struct ioq *ioq = priv->rxq.queue;
+	struct ioq_iterator iter;
+	int ret;
+
+	ret = ioq_iter_init(ioq, &iter, ioq_idxtype_valid, 0);
+	BUG_ON(ret < 0);
+
+	ret = ioq_iter_seek(&iter, ioq_seek_head, 0, 0);
+	BUG_ON(ret < 0);
+
+	/*
+	 * free each valid descriptor
+	 */
+	while (iter.desc->valid) {
+		struct sk_buff *skb = (struct sk_buff *)iter.desc->cookie;
+
+		iter.desc->valid = 0;
+		wmb();
+
+		iter.desc->ptr = 0;
+		iter.desc->cookie = 0;
+
+		ret = ioq_iter_pop(&iter, 0);
+		BUG_ON(ret < 0);
+
+		dev_kfree_skb(skb);
+	}
+}
+
+/*
+ * Open and close
+ */
+
+static int
+vbus_enet_open(struct net_device *dev)
+{
+	struct vbus_enet_priv *priv = netdev_priv(dev);
+	int ret;
+
+	ret = devcall(priv, VENET_FUNC_LINKUP, NULL, 0);
+	BUG_ON(ret < 0);
+
+	napi_enable(&priv->napi);
+
+	return 0;
+}
+
+static int
+vbus_enet_stop(struct net_device *dev)
+{
+	struct vbus_enet_priv *priv = netdev_priv(dev);
+	int ret;
+
+	napi_disable(&priv->napi);
+
+	ret = devcall(priv, VENET_FUNC_LINKDOWN, NULL, 0);
+	BUG_ON(ret < 0);
+
+	return 0;
+}
+
+/*
+ * Configuration changes (passed on by ifconfig)
+ */
+static int
+vbus_enet_config(struct net_device *dev, struct ifmap *map)
+{
+	if (dev->flags & IFF_UP) /* can't act on a running interface */
+		return -EBUSY;
+
+	/* Don't allow changing the I/O address */
+	if (map->base_addr != dev->base_addr) {
+		printk(KERN_WARNING "vbus_enet: Can't change I/O address\n");
+		return -EOPNOTSUPP;
+	}
+
+	/* ignore other fields */
+	return 0;
+}
+
+static void
+vbus_enet_schedule_rx(struct vbus_enet_priv *priv)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->lock, flags);
+
+	if (netif_rx_schedule_prep(&priv->napi)) {
+		/* Disable further interrupts */
+		ioq_notify_disable(priv->rxq.queue, 0);
+		__netif_rx_schedule(&priv->napi);
+	}
+
+	spin_unlock_irqrestore(&priv->lock, flags);
+}
+
+static int
+vbus_enet_change_mtu(struct net_device *dev, int new_mtu)
+{
+	struct vbus_enet_priv *priv = netdev_priv(dev);
+	int ret;
+
+	dev->mtu = new_mtu;
+
+	/*
+	 * FLUSHRX will cause the device to flush any outstanding
+	 * RX buffers.  They will appear to come in as 0 length
+	 * packets which we can simply discard and replace with new_mtu
+	 * buffers for the future.
+	 */
+	ret = devcall(priv, VENET_FUNC_FLUSHRX, NULL, 0);
+	BUG_ON(ret < 0);
+
+	vbus_enet_schedule_rx(priv);
+
+	return 0;
+}
+
+/*
+ * The poll implementation.
+ */
+static int
+vbus_enet_poll(struct napi_struct *napi, int budget)
+{
+	struct vbus_enet_priv *priv = napi_to_priv(napi);
+	int npackets = 0;
+	struct ioq_iterator iter;
+	int ret;
+
+	PDEBUG("%lld: polling...\n", priv->vdev->id);
+
+	/* We want to iterate on the head of the in-use index */
+	ret = ioq_iter_init(priv->rxq.queue, &iter, ioq_idxtype_inuse,
+			    IOQ_ITER_AUTOUPDATE);
+	BUG_ON(ret < 0);
+
+	ret = ioq_iter_seek(&iter, ioq_seek_head, 0, 0);
+	BUG_ON(ret < 0);
+
+	/*
+	 * We stop if we have met the quota or there are no more packets.
+	 * The EOM is indicated by finding a packet that is still owned by
+	 * the south side
+	 */
+	while ((npackets < budget) && (!iter.desc->sown)) {
+		struct sk_buff *skb = (struct sk_buff *)iter.desc->cookie;
+
+		if (iter.desc->len) {
+			skb_put(skb, iter.desc->len);
+
+			/* Maintain stats */
+			npackets++;
+			priv->dev->stats.rx_packets++;
+			priv->dev->stats.rx_bytes += iter.desc->len;
+
+			/* Pass the buffer up to the stack */
+			skb->dev      = priv->dev;
+			skb->protocol = eth_type_trans(skb, priv->dev);
+			netif_receive_skb(skb);
+
+			mb();
+		} else
+			/*
+			 * the device may send a zero-length packet when its
+			 * flushing references on the ring.  We can just drop
+			 * these on the floor
+			 */
+			dev_kfree_skb(skb);
+
+		/* Grab a new buffer to put in the ring */
+		rxdesc_alloc(iter.desc, priv->dev->mtu);
+
+		/* Advance the in-use tail */
+		ret = ioq_iter_pop(&iter, 0);
+		BUG_ON(ret < 0);
+	}
+
+	PDEBUG("%lld poll: %d packets received\n", priv->vdev->id, npackets);
+
+	/*
+	 * If we processed all packets, we're done; tell the kernel and
+	 * reenable ints
+	 */
+	if (ioq_empty(priv->rxq.queue, ioq_idxtype_inuse)) {
+		netif_rx_complete(napi);
+		ioq_notify_enable(priv->rxq.queue, 0);
+		ret = 0;
+	} else
+		/* We couldn't process everything. */
+		ret = 1;
+
+	return ret;
+}
+
+/*
+ * Transmit a packet (called by the kernel)
+ */
+static int
+vbus_enet_tx_start(struct sk_buff *skb, struct net_device *dev)
+{
+	struct vbus_enet_priv *priv = netdev_priv(dev);
+	struct ioq_iterator    iter;
+	int ret;
+	unsigned long flags;
+
+	PDEBUG("%lld: sending %d bytes\n", priv->vdev->id, skb->len);
+
+	spin_lock_irqsave(&priv->lock, flags);
+
+	if (ioq_full(priv->txq.queue, ioq_idxtype_valid)) {
+		/*
+		 * We must flow-control the kernel by disabling the
+		 * queue
+		 */
+		spin_unlock_irqrestore(&priv->lock, flags);
+		netif_stop_queue(dev);
+		printk(KERN_ERR "VBUS_ENET: tx on full queue bug "	\
+		       "on device %lld\n", priv->vdev->id);
+		return 1;
+	}
+
+	/*
+	 * We want to iterate on the tail of both the "inuse" and "valid" index
+	 * so we specify the "both" index
+	 */
+	ret = ioq_iter_init(priv->txq.queue, &iter, ioq_idxtype_both,
+			    IOQ_ITER_AUTOUPDATE);
+	BUG_ON(ret < 0);
+
+	ret = ioq_iter_seek(&iter, ioq_seek_tail, 0, 0);
+	BUG_ON(ret < 0);
+	BUG_ON(iter.desc->sown);
+
+	/*
+	 * We simply put the skb right onto the ring.  We will get an interrupt
+	 * later when the data has been consumed and we can reap the pointers
+	 * at that time
+	 */
+	iter.desc->cookie = (u64)skb;
+	iter.desc->len = (u64)skb->len;
+	iter.desc->ptr = (u64)__pa(skb->data);
+	iter.desc->valid  = 1;
+
+	priv->dev->stats.tx_packets++;
+	priv->dev->stats.tx_bytes += skb->len;
+
+	/*
+	 * This advances both indexes together implicitly, and then
+	 * signals the south side to consume the packet
+	 */
+	ret = ioq_iter_push(&iter, 0);
+	BUG_ON(ret < 0);
+
+	dev->trans_start = jiffies; /* save the timestamp */
+
+	if (ioq_full(priv->txq.queue, ioq_idxtype_valid)) {
+		/*
+		 * If the queue is congested, we must flow-control the kernel
+		 */
+		PDEBUG("%lld: backpressure tx queue\n", priv->vdev->id);
+		netif_stop_queue(dev);
+	}
+
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	return 0;
+}
+
+/*
+ * reclaim any outstanding completed tx packets
+ *
+ * assumes priv->lock held
+ */
+static void
+vbus_enet_tx_reap(struct vbus_enet_priv *priv, int force)
+{
+	struct ioq_iterator iter;
+	int ret;
+
+	/*
+	 * We want to iterate on the head of the valid index, but we
+	 * do not want the iter_pop (below) to flip the ownership, so
+	 * we set the NOFLIPOWNER option
+	 */
+	ret = ioq_iter_init(priv->txq.queue, &iter, ioq_idxtype_valid,
+			    IOQ_ITER_NOFLIPOWNER);
+	BUG_ON(ret < 0);
+
+	ret = ioq_iter_seek(&iter, ioq_seek_head, 0, 0);
+	BUG_ON(ret < 0);
+
+	/*
+	 * We are done once we find the first packet either invalid or still
+	 * owned by the south-side
+	 */
+	while (iter.desc->valid && (!iter.desc->sown || force)) {
+		struct sk_buff *skb = (struct sk_buff *)iter.desc->cookie;
+
+		PDEBUG("%lld: completed sending %d bytes\n",
+		       priv->vdev->id, skb->len);
+
+		/* Reset the descriptor */
+		iter.desc->valid  = 0;
+
+		dev_kfree_skb(skb);
+
+		/* Advance the valid-index head */
+		ret = ioq_iter_pop(&iter, 0);
+		BUG_ON(ret < 0);
+	}
+
+	/*
+	 * If we were previously stopped due to flow control, restart the
+	 * processing
+	 */
+	if (netif_queue_stopped(priv->dev)
+	    && !ioq_full(priv->txq.queue, ioq_idxtype_valid)) {
+		PDEBUG("%lld: re-enabling tx queue\n", priv->vdev->id);
+		netif_wake_queue(priv->dev);
+	}
+}
+
+static void
+vbus_enet_timeout(struct net_device *dev)
+{
+	struct vbus_enet_priv *priv = netdev_priv(dev);
+	unsigned long flags;
+
+	printk(KERN_DEBUG "VBUS_ENET %lld: Transmit timeout\n", priv->vdev->id);
+
+	spin_lock_irqsave(&priv->lock, flags);
+	vbus_enet_tx_reap(priv, 0);
+	spin_unlock_irqrestore(&priv->lock, flags);
+}
+
+static void
+rx_isr(struct ioq_notifier *notifier)
+{
+	struct vbus_enet_priv *priv;
+	struct net_device  *dev;
+
+	priv = container_of(notifier, struct vbus_enet_priv, rxq.notifier);
+	dev = priv->dev;
+
+	if (!ioq_empty(priv->rxq.queue, ioq_idxtype_inuse))
+		vbus_enet_schedule_rx(priv);
+}
+
+static void
+deferred_tx_isr(unsigned long data)
+{
+	struct vbus_enet_priv *priv = (struct vbus_enet_priv *)data;
+	unsigned long flags;
+
+	PDEBUG("deferred_tx_isr for %lld\n", priv->vdev->id);
+
+	spin_lock_irqsave(&priv->lock, flags);
+	vbus_enet_tx_reap(priv, 0);
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	ioq_notify_enable(priv->txq.queue, 0);
+}
+
+static void
+tx_isr(struct ioq_notifier *notifier)
+{
+       struct vbus_enet_priv *priv;
+       unsigned long flags;
+
+       priv = container_of(notifier, struct vbus_enet_priv, txq.notifier);
+
+       PDEBUG("tx_isr for %lld\n", priv->vdev->id);
+
+       ioq_notify_disable(priv->txq.queue, 0);

drivers/net/vbus-enet.c line 555

Quoted 10 lines
drivers/net/vbus-enet.c new file +10 -0
+       tasklet_schedule(&priv->txtask);
+}
+
+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,

add .ndo_validate_addr = eth_valid_addr?
multicast list?

Quoted 126 lines
drivers/net/vbus-enet.c new file +125 -0
+};
+
+/*
+ * This is called whenever a new vbus_device_proxy is added to the vbus
+ * with the matching VENET_ID
+ */
+static int
+vbus_enet_probe(struct vbus_device_proxy *vdev)
+{
+	struct net_device  *dev;
+	struct vbus_enet_priv *priv;
+	int ret;
+
+	printk(KERN_INFO "VBUS_ENET: Found new device at %lld\n", vdev->id);
+
+	ret = vdev->ops->open(vdev, VENET_VERSION, 0);
+	if (ret < 0)
+		return ret;
+
+	dev = alloc_etherdev(sizeof(struct vbus_enet_priv));
+	if (!dev)
+		return -ENOMEM;
+
+	priv = netdev_priv(dev);
+
+	spin_lock_init(&priv->lock);
+	priv->dev  = dev;
+	priv->vdev = vdev;
+
+	tasklet_init(&priv->txtask, deferred_tx_isr, (unsigned long)priv);
+
+	queue_init(priv, &priv->rxq, VENET_QUEUE_RX, rx_ringlen, rx_isr);
+	queue_init(priv, &priv->txq, VENET_QUEUE_TX, tx_ringlen, tx_isr);
+
+	rx_setup(priv);
+
+	ioq_notify_enable(priv->rxq.queue, 0);  /* enable interrupts */
+	ioq_notify_enable(priv->txq.queue, 0);
+
+	dev->netdev_ops     = &vbus_enet_netdev_ops;
+	dev->watchdog_timeo = 5 * HZ;
+
+	netif_napi_add(dev, &priv->napi, vbus_enet_poll, napi_weight);
+
+	ret = devcall(priv, VENET_FUNC_MACQUERY, priv->dev->dev_addr, ETH_ALEN);
+	if (ret < 0) {
+		printk(KERN_INFO "VENET: Error obtaining MAC address for " \
+		       "%lld\n",
+		       priv->vdev->id);
+		goto out_free;
+	}
+
+	dev->features |= NETIF_F_HIGHDMA;
+
+	ret = register_netdev(dev);
+	if (ret < 0) {
+		printk(KERN_INFO "VENET: error %i registering device \"%s\"\n",
+		       ret, dev->name);
+		goto out_free;
+	}
+
+	vdev->priv = priv;
+
+	return 0;
+
+ out_free:
+	free_netdev(dev);
+
+	return ret;
+}
+
+static int
+vbus_enet_remove(struct vbus_device_proxy *vdev)
+{
+	struct vbus_enet_priv *priv = (struct vbus_enet_priv *)vdev->priv;
+	struct vbus_device_proxy *dev = priv->vdev;
+
+	unregister_netdev(priv->dev);
+	napi_disable(&priv->napi);
+
+	rx_teardown(priv);
+	vbus_enet_tx_reap(priv, 1);
+
+	ioq_put(priv->rxq.queue);
+	ioq_put(priv->txq.queue);
+
+	dev->ops->close(dev, 0);
+
+	free_netdev(priv->dev);
+
+	return 0;
+}
+
+/*
+ * Finally, the module stuff
+ */
+
+static struct vbus_driver_ops vbus_enet_driver_ops = {
+	.probe  = vbus_enet_probe,
+	.remove = vbus_enet_remove,
+};
+
+static struct vbus_driver vbus_enet_driver = {
+	.type   = VENET_TYPE,
+	.owner  = THIS_MODULE,
+	.ops    = &vbus_enet_driver_ops,
+};
+
+static __init int
+vbus_enet_init_module(void)
+{
+	printk(KERN_INFO "Virtual Ethernet: Copyright (C) 2009 Novell, Gregory Haskins\n");
+	printk(KERN_DEBUG "VBUSENET: Using %d/%d queue depth\n",
+	       rx_ringlen, tx_ringlen);
+	return vbus_driver_register(&vbus_enet_driver);
+}
+
+static __exit void
+vbus_enet_cleanup(void)
+{
+	vbus_driver_unregister(&vbus_enet_driver);
+}
+
+module_init(vbus_enet_init_module);
+module_exit(vbus_enet_cleanup);
#17 lore raw

Re: [RFC PATCH v2 09/19] net: Add vbus_enet driver

Quoted 11 lines · Stephen Hemminger

drivers/net/vbus-enet.c line 51

Quoted 9 lines
drivers/net/vbus-enet.c new file +9 -0
+static int tx_ringlen = 256;
+module_param(tx_ringlen, int, 0444);
+
+#undef PDEBUG             /* undef it, just in case */
+#ifdef VBUS_ENET_DEBUG
+#  define PDEBUG(fmt, args...) printk(KERN_DEBUG "vbus_enet: " fmt, ## args)
+#else
+#  define PDEBUG(fmt, args...) /* not debugging: nothing */
+#endif

Why reinvent pr_debug()?

Even more important, use dev_dbg() instead please, that uniquly describes your device and driver together, which is what you need/want, and it ties into the dynamic debug work, so you don't need a special kernel config option.

thanks,

greg k-h

#18 lore raw

RFC PATCH v2 10/19 venet-tap: Adds a "venet" compatible "tap" device to VBUS

In reply to Gregory Haskins

This module is similar in concept to a "tuntap". A tuntap module provides a netif() interface on one side, and a char-dev interface on the other. Packets that ingress on one interface, egress on the other (and vice versa).

This module offers a similar concept, except that it substitues the char-dev for a VBUS/IOQ interface. This allows a VBUS compatible entity (e.g. userspace or a guest) to directly inject and receive packets from the host/kernel stack.

Thanks to Pat Mullaney for contributing the maxcount modification

---

 drivers/Makefile                 |    1 
 drivers/vbus/devices/Kconfig     |   17 
 drivers/vbus/devices/Makefile    |    1 
 drivers/vbus/devices/venet-tap.c | 1388 ++++++++++++++++++++++++++++++++++++++
 kernel/vbus/Kconfig              |   13 
 5 files changed, 1420 insertions(+), 0 deletions(-)
 create mode 100644 drivers/vbus/devices/Kconfig
 create mode 100644 drivers/vbus/devices/Makefile
 create mode 100644 drivers/vbus/devices/venet-tap.c
drivers/Makefile +1 -0
index c1bf417..98fab51 100644
@@ -106,3 +106,4 @@ obj-$(CONFIG_SSB)		+= ssb/
 obj-$(CONFIG_VIRTIO)		+= virtio/
 obj-$(CONFIG_STAGING)		+= staging/
 obj-y				+= platform/
+obj-$(CONFIG_VBUS_DEVICES)	+= vbus/devices/
drivers/vbus/devices/Kconfig new file +17 -0
new file mode 100644
index 0000000..64e4731
@@ -0,0 +1,17 @@
+#
+# Virtual-Bus (VBus) configuration
+#
+
+config VBUS_VENETTAP
+       tristate "Virtual-Bus Ethernet Tap Device"
+       depends on VBUS_DEVICES
+       default n
+       help
+        Provides a virtual ethernet adapter to a vbus, which in turn
+        manifests itself as a standard netif based adapter to the
+	kernel.  It can be used similarly to a "tuntap" device,
+        except that the char-dev transport is replaced with a vbus/ioq
+        interface.
+
+	If unsure, say N
+
drivers/vbus/devices/Makefile new file +1 -0
new file mode 100644
index 0000000..2ea7d2a
@@ -0,0 +1 @@
+obj-$(CONFIG_VBUS_VENETTAP) += venet-tap.o
drivers/vbus/devices/venet-tap.c new file +1388 -0

1,388 lines not on this page · raw message

kernel/vbus/Kconfig +13 -0
index 71acd6f..3ce0adc 100644
@@ -14,6 +14,17 @@ config VBUS
 
 	If unsure, say N
 
+config VBUS_DEVICES
+       bool "Virtual-Bus Devices"
+       depends on VBUS
+       default n
+       help
+         Provides device-class modules for instantiation on a virtual-bus
+
+	 If unsure, say N
+
+source "drivers/vbus/devices/Kconfig"
+
 config VBUS_DRIVERS
        tristate "VBUS Driver support"
        select IOQ
@@ -23,3 +34,5 @@ config VBUS_DRIVERS
 
 	If unsure, say N
 
+
+
#19 lore raw

RFC PATCH v2 11/19 venet: add scatter-gather support

In reply to Gregory Haskins

---

 drivers/net/vbus-enet.c |  249 +++++++++++++++++++++++++++++++++++++++++++++--
 include/linux/venet.h   |   39 +++++++
 2 files changed, 275 insertions(+), 13 deletions(-)
drivers/net/vbus-enet.c +238 -11
index 3779f77..2a190e0 100644
@@ -42,6 +42,8 @@ static int rx_ringlen = 256;
 module_param(rx_ringlen, int, 0444);
 static int tx_ringlen = 256;
 module_param(tx_ringlen, int, 0444);
+static int sg_enabled = 1;
+module_param(sg_enabled, int, 0444);
 
 #undef PDEBUG             /* undef it, just in case */
 #ifdef VBUS_ENET_DEBUG
@@ -63,8 +65,17 @@ struct vbus_enet_priv {
 	struct vbus_enet_queue     rxq;
 	struct vbus_enet_queue     txq;
 	struct tasklet_struct      txtask;
+	struct {
+		int                sg:1;
+		int                tso:1;
+		int                ufo:1;
+		int                tso6:1;
+		int                ecn:1;
+	} flags;
 };
 
+static void vbus_enet_tx_reap(struct vbus_enet_priv *priv, int force);
+
 static struct vbus_enet_priv *
 napi_to_priv(struct napi_struct *napi)
 {
@@ -198,6 +209,93 @@ rx_teardown(struct vbus_enet_priv *priv)
 	}
 }
 
+static int
+tx_setup(struct vbus_enet_priv *priv)
+{
+	struct ioq *ioq = priv->txq.queue;
+	struct ioq_iterator iter;
+	int i;
+	int ret;
+
+	if (!priv->flags.sg)
+		/*
+		 * There is nothing to do for a ring that is not using
+		 * scatter-gather
+		 */
+		return 0;
+
+	ret = ioq_iter_init(ioq, &iter, ioq_idxtype_valid, 0);
+	BUG_ON(ret < 0);
+
+	ret = ioq_iter_seek(&iter, ioq_seek_set, 0, 0);
+	BUG_ON(ret < 0);
+
+	/*
+	 * Now populate each descriptor with an empty SG descriptor
+	 */
+	for (i = 0; i < tx_ringlen; i++) {
+		struct venet_sg *vsg;
+		size_t iovlen = sizeof(struct venet_iov) * (MAX_SKB_FRAGS-1);
+		size_t len = sizeof(*vsg) + iovlen;
+
+		vsg = kzalloc(len, GFP_KERNEL);
+		if (!vsg)
+			return -ENOMEM;
+
+		iter.desc->cookie = (u64)vsg;
+		iter.desc->len    = len;
+		iter.desc->ptr    = (u64)__pa(vsg);
+
+		ret = ioq_iter_seek(&iter, ioq_seek_next, 0, 0);
+		BUG_ON(ret < 0);
+	}
+
+	return 0;
+}
+
+static void
+tx_teardown(struct vbus_enet_priv *priv)
+{
+	struct ioq *ioq = priv->txq.queue;
+	struct ioq_iterator iter;
+	int ret;
+
+	/* forcefully free all outstanding transmissions */
+	vbus_enet_tx_reap(priv, 1);
+
+	if (!priv->flags.sg)
+		/*
+		 * There is nothing else to do for a ring that is not using
+		 * scatter-gather
+		 */
+		return;
+
+	ret = ioq_iter_init(ioq, &iter, ioq_idxtype_valid, 0);
+	BUG_ON(ret < 0);
+
+	/* seek to position 0 */
+	ret = ioq_iter_seek(&iter, ioq_seek_set, 0, 0);
+	BUG_ON(ret < 0);
+
+	/*
+	 * free each valid descriptor
+	 */
+	while (iter.desc->cookie) {
+		struct venet_sg *vsg = (struct venet_sg *)iter.desc->cookie;
+
+		iter.desc->valid = 0;
+		wmb();
+
+		iter.desc->ptr = 0;
+		iter.desc->cookie = 0;
+
+		ret = ioq_iter_seek(&iter, ioq_seek_next, 0, 0);
+		BUG_ON(ret < 0);
+
+		kfree(vsg);
+	}
+}
+
 /*
  * Open and close
  */
@@ -402,14 +500,67 @@ vbus_enet_tx_start(struct sk_buff *skb, struct net_device *dev)
 	BUG_ON(ret < 0);
 	BUG_ON(iter.desc->sown);
 
-	/*
-	 * We simply put the skb right onto the ring.  We will get an interrupt
-	 * later when the data has been consumed and we can reap the pointers
-	 * at that time
-	 */
-	iter.desc->cookie = (u64)skb;
-	iter.desc->len = (u64)skb->len;
-	iter.desc->ptr = (u64)__pa(skb->data);
+	if (priv->flags.sg) {
+		struct venet_sg *vsg = (struct venet_sg *)iter.desc->cookie;
+		struct scatterlist sgl[MAX_SKB_FRAGS+1];
+		struct scatterlist *sg;
+		int count, maxcount = ARRAY_SIZE(sgl);
+
+		sg_init_table(sgl, maxcount);
+
+		memset(vsg, 0, sizeof(*vsg));
+
+		vsg->cookie = (u64)skb;
+		vsg->len    = skb->len;
+
+		if (skb->ip_summed == CHECKSUM_PARTIAL) {
+			vsg->flags      |= VENET_SG_FLAG_NEEDS_CSUM;
+			vsg->csum.start  = skb->csum_start - skb_headroom(skb);
+			vsg->csum.offset = skb->csum_offset;
+		}
+
+		if (skb_is_gso(skb)) {
+			struct skb_shared_info *sinfo = skb_shinfo(skb);
+
+			vsg->flags |= VENET_SG_FLAG_GSO;
+
+			vsg->gso.hdrlen = skb_transport_header(skb) - skb->data;
+			vsg->gso.size = sinfo->gso_size;
+			if (sinfo->gso_type & SKB_GSO_TCPV4)
+				vsg->gso.type = VENET_GSO_TYPE_TCPV4;
+			else if (sinfo->gso_type & SKB_GSO_TCPV6)
+				vsg->gso.type = VENET_GSO_TYPE_TCPV6;
+			else if (sinfo->gso_type & SKB_GSO_UDP)
+				vsg->gso.type = VENET_GSO_TYPE_UDP;
+			else
+				panic("Virtual-Ethernet: unknown GSO type " \
+				      "0x%x\n", sinfo->gso_type);
+
+			if (sinfo->gso_type & SKB_GSO_TCP_ECN)
+				vsg->flags |= VENET_SG_FLAG_ECN;
+		}
+
+		count = skb_to_sgvec(skb, sgl, 0, skb->len);
+
+		BUG_ON(count > maxcount);
+
+		for (sg = &sgl[0]; sg; sg = sg_next(sg)) {
+			struct venet_iov *iov = &vsg->iov[vsg->count++];
+
+			iov->len = sg->length;
+			iov->ptr = (u64)sg_phys(sg);
+		}
+
+	} else {
+		/*
+		 * non scatter-gather mode: simply put the skb right onto the
+		 * ring.
+		 */
+		iter.desc->cookie = (u64)skb;
+		iter.desc->len = (u64)skb->len;
+		iter.desc->ptr = (u64)__pa(skb->data);
+	}
+
 	iter.desc->valid  = 1;
 
 	priv->dev->stats.tx_packets++;
@@ -465,7 +616,17 @@ vbus_enet_tx_reap(struct vbus_enet_priv *priv, int force)
 	 * owned by the south-side
 	 */
 	while (iter.desc->valid && (!iter.desc->sown || force)) {
-		struct sk_buff *skb = (struct sk_buff *)iter.desc->cookie;
+		struct sk_buff *skb;
+
+		if (priv->flags.sg) {
+			struct venet_sg *vsg;
+
+			vsg = (struct venet_sg *)iter.desc->cookie;
+			skb = (struct sk_buff *)vsg->cookie;
+
+		} else {
+			skb = (struct sk_buff *)iter.desc->cookie;
+		}
 
 		PDEBUG("%lld: completed sending %d bytes\n",
 		       priv->vdev->id, skb->len);
@@ -546,6 +707,47 @@ tx_isr(struct ioq_notifier *notifier)
        tasklet_schedule(&priv->txtask);
 }
 
+static int
+vbus_enet_negcap(struct vbus_enet_priv *priv)
+{
+	int ret;
+	struct venet_capabilities caps;
+
+	memset(&caps, 0, sizeof(caps));
+
+	if (sg_enabled) {
+		caps.gid = VENET_CAP_GROUP_SG;
+		caps.bits |= (VENET_CAP_SG|VENET_CAP_TSO4|VENET_CAP_TSO6
+			      |VENET_CAP_ECN|VENET_CAP_UFO);
+	}
+
+	ret = devcall(priv, VENET_FUNC_NEGCAP, &caps, sizeof(caps));
+	if (ret < 0)
+		return ret;
+
+	if (caps.bits & VENET_CAP_SG) {
+		priv->flags.sg = true;
+
+		if (caps.bits & VENET_CAP_TSO4)
+			priv->flags.tso = true;
+		if (caps.bits & VENET_CAP_TSO6)
+			priv->flags.tso6 = true;
+		if (caps.bits & VENET_CAP_UFO)
+			priv->flags.ufo = true;
+		if (caps.bits & VENET_CAP_ECN)
+			priv->flags.ecn = true;
+
+		printk(KERN_INFO "VBUSENET %lld: " \
+		       "Detected GSO features %s%s%s%s\n", priv->vdev->id,
+		       priv->flags.tso  ? "t" : "-",
+		       priv->flags.tso6 ? "T" : "-",
+		       priv->flags.ufo  ? "u" : "-",
+		       priv->flags.ecn  ? "e" : "-");
+	}
+
+	return 0;
+}
+
 static const struct net_device_ops vbus_enet_netdev_ops = {
 	.ndo_open          = vbus_enet_open,
 	.ndo_stop          = vbus_enet_stop,
@@ -582,12 +784,21 @@ vbus_enet_probe(struct vbus_device_proxy *vdev)
 	priv->dev  = dev;
 	priv->vdev = vdev;
 
+	ret = vbus_enet_negcap(priv);
+	if (ret < 0) {
+		printk(KERN_INFO "VENET: Error negotiating capabilities for " \
+		       "%lld\n",
+		       priv->vdev->id);
+		goto out_free;
+	}
+
 	tasklet_init(&priv->txtask, deferred_tx_isr, (unsigned long)priv);
 
 	queue_init(priv, &priv->rxq, VENET_QUEUE_RX, rx_ringlen, rx_isr);
 	queue_init(priv, &priv->txq, VENET_QUEUE_TX, tx_ringlen, tx_isr);
 
 	rx_setup(priv);
+	tx_setup(priv);
 
 	ioq_notify_enable(priv->rxq.queue, 0);  /* enable interrupts */
 	ioq_notify_enable(priv->txq.queue, 0);
@@ -607,6 +818,22 @@ vbus_enet_probe(struct vbus_device_proxy *vdev)
 
 	dev->features |= NETIF_F_HIGHDMA;
 
+	if (priv->flags.sg) {
+		dev->features |= NETIF_F_SG|NETIF_F_HW_CSUM|NETIF_F_FRAGLIST;
+
+		if (priv->flags.tso)
+			dev->features |= NETIF_F_TSO;
+
+		if (priv->flags.ufo)
+			dev->features |= NETIF_F_UFO;
+
+		if (priv->flags.tso6)
+			dev->features |= NETIF_F_TSO6;
+
+		if (priv->flags.ecn)
+			dev->features |= NETIF_F_TSO_ECN;
+	}
+
 	ret = register_netdev(dev);
 	if (ret < 0) {
 		printk(KERN_INFO "VENET: error %i registering device \"%s\"\n",
@@ -634,9 +861,9 @@ vbus_enet_remove(struct vbus_device_proxy *vdev)
 	napi_disable(&priv->napi);
 
 	rx_teardown(priv);
-	vbus_enet_tx_reap(priv, 1);
-
 	ioq_put(priv->rxq.queue);
+
+	tx_teardown(priv);
 	ioq_put(priv->txq.queue);
 
 	dev->ops->close(dev, 0);
include/linux/venet.h +37 -2
index ef6b199..1c96b90 100644
@@ -35,8 +35,43 @@ struct venet_capabilities {
 	__u32 bits;
 };
 
-/* CAPABILITIES-GROUP 0 */
-/* #define VENET_CAP_FOO    0   (No capabilities defined yet, for now) */
+#define VENET_CAP_GROUP_SG 0
+
+/* CAPABILITIES-GROUP SG */
+#define VENET_CAP_SG     (1 << 0)
+#define VENET_CAP_TSO4   (1 << 1)
+#define VENET_CAP_TSO6   (1 << 2)
+#define VENET_CAP_ECN    (1 << 3)
+#define VENET_CAP_UFO    (1 << 4)
+
+struct venet_iov {
+	__u32 len;
+	__u64 ptr;
+};
+
+#define VENET_SG_FLAG_NEEDS_CSUM (1 << 0)
+#define VENET_SG_FLAG_GSO        (1 << 1)
+#define VENET_SG_FLAG_ECN        (1 << 2)
+
+struct venet_sg {
+	__u64            cookie;
+	__u32            flags;
+	__u32            len;     /* total length of all iovs */
+	struct {
+		__u16    start;	  /* csum starting position */
+		__u16    offset;  /* offset to place csum */
+	} csum;
+	struct {
+#define VENET_GSO_TYPE_TCPV4	0	/* IPv4 TCP (TSO) */
+#define VENET_GSO_TYPE_UDP	1	/* IPv4 UDP (UFO) */
+#define VENET_GSO_TYPE_TCPV6	2	/* IPv6 TCP */
+		__u8     type;
+		__u16    hdrlen;
+		__u16    size;
+	} gso;
+	__u32            count;   /* nr of iovs */
+	struct venet_iov iov[1];
+};
 
 #define VENET_FUNC_LINKUP   0
 #define VENET_FUNC_LINKDOWN 1
#20 lore raw

RFC PATCH v2 12/19 venettap: add scatter-gather support

In reply to Gregory Haskins

---

 drivers/vbus/devices/venet-tap.c |  235 +++++++++++++++++++++++++++++++++++++-
 1 files changed, 228 insertions(+), 7 deletions(-)
drivers/vbus/devices/venet-tap.c +228 -7
index 148e2c8..a1f2dc6 100644
@@ -81,6 +81,13 @@ enum {
 	TX_IOQ_CONGESTED,
 };
 
+struct venettap;
+
+struct venettap_rx_ops {
+	int (*decode)(struct venettap *priv, void *ptr, int len);
+	int (*import)(struct venettap *, struct sk_buff *, void *, int);
+};
+
 struct venettap {
 	spinlock_t                   lock;
 	unsigned char                hmac[ETH_ALEN]; /* host-mac */
@@ -108,7 +115,13 @@ struct venettap {
 		struct vbus_memctx          *ctx;
 		struct venettap_queue        rxq;
 		struct venettap_queue        txq;
+		struct venettap_rx_ops      *rx_ops;
 		wait_queue_head_t            rx_empty;
+		struct {
+			struct venet_sg     *desc;
+			size_t               len;
+			int                  enabled:1;
+		} sg;
 		int                          connected:1;
 		int                          opened:1;
 		int                          link:1;
@@ -290,6 +303,183 @@ venettap_change_mtu(struct net_device *dev, int new_mtu)
 }
 
 /*
+ * ---------------------------
+ * Scatter-Gather support
+ * ---------------------------
+ */
+
+/* assumes reference to priv->vbus.conn held */
+static int
+venettap_sg_decode(struct venettap *priv, void *ptr, int len)
+{
+	struct venet_sg *vsg;
+	struct vbus_memctx *ctx;
+	int ret;
+
+	/*
+	 * SG is enabled, so we need to pull in the venet_sg
+	 * header before we can interpret the rest of the
+	 * packet
+	 *
+	 * FIXME: Make sure this is not too big
+	 */
+	if (unlikely(len > priv->vbus.sg.len)) {
+		kfree(priv->vbus.sg.desc);
+		priv->vbus.sg.desc = kzalloc(len, GFP_KERNEL);
+	}
+
+	vsg = priv->vbus.sg.desc;
+	ctx = priv->vbus.ctx;
+
+	ret = ctx->ops->copy_from(ctx, vsg, ptr, len);
+	BUG_ON(ret);
+
+	/*
+	 * Non GSO type packets should be constrained by the MTU setting
+	 * on the host
+	 */
+	if (!(vsg->flags & VENET_SG_FLAG_GSO)
+	    && (vsg->len > (priv->netif.dev->mtu + ETH_HLEN)))
+		return -1;
+
+	return vsg->len;
+}
+
+/*
+ * venettap_sg_import - import an skb in scatter-gather mode
+ *
+ * assumes reference to priv->vbus.conn held
+ */
+static int
+venettap_sg_import(struct venettap *priv, struct sk_buff *skb,
+		   void *ptr, int len)
+{
+	struct venet_sg *vsg = priv->vbus.sg.desc;
+	struct vbus_memctx *ctx = priv->vbus.ctx;
+	int remain = len;
+	int ret;
+	int i;
+
+	PDEBUG("Importing %d bytes in %d segments\n", len, vsg->count);
+
+	for (i = 0; i < vsg->count; i++) {
+		struct venet_iov *iov = &vsg->iov[i];
+
+		if (remain < iov->len)
+			return -EINVAL;
+
+		PDEBUG("Segment %d: %p/%d\n", i, iov->ptr, iov->len);
+
+		ret = ctx->ops->copy_from(ctx, skb_tail_pointer(skb),
+					 (void *)iov->ptr,
+					 iov->len);
+		if (ret)
+			return -EFAULT;
+
+		skb_put(skb, iov->len);
+		remain -= iov->len;
+	}
+
+	if (vsg->flags & VENET_SG_FLAG_NEEDS_CSUM
+	    && !skb_partial_csum_set(skb, vsg->csum.start, vsg->csum.offset))
+		return -EINVAL;
+
+	if (vsg->flags & VENET_SG_FLAG_GSO) {
+		struct skb_shared_info *sinfo = skb_shinfo(skb);
+
+		PDEBUG("GSO packet detected\n");
+
+		switch (vsg->gso.type) {
+		case VENET_GSO_TYPE_TCPV4:
+			sinfo->gso_type = SKB_GSO_TCPV4;
+			break;
+		case VENET_GSO_TYPE_TCPV6:
+			sinfo->gso_type = SKB_GSO_TCPV6;
+			break;
+		case VENET_GSO_TYPE_UDP:
+			sinfo->gso_type = SKB_GSO_UDP;
+			break;
+		default:
+			PDEBUG("Illegal GSO type: %d\n", vsg->gso.type);
+			priv->netif.stats.rx_frame_errors++;
+			kfree_skb(skb);
+			return -EINVAL;
+		}
+
+		if (vsg->flags & VENET_SG_FLAG_ECN)
+			sinfo->gso_type |= SKB_GSO_TCP_ECN;
+
+		sinfo->gso_size = vsg->gso.size;
+		if (skb_shinfo(skb)->gso_size == 0) {
+			PDEBUG("Illegal GSO size: %d\n", vsg->gso.size);
+			priv->netif.stats.rx_frame_errors++;
+			kfree_skb(skb);
+			return -EINVAL;
+		}
+
+		/* Header must be checked, and gso_segs computed. */
+		skb_shinfo(skb)->gso_type |= SKB_GSO_DODGY;
+		skb_shinfo(skb)->gso_segs = 0;
+	}
+
+	return 0;
+}
+
+static struct venettap_rx_ops venettap_sg_rx_ops = {
+	.decode = venettap_sg_decode,
+	.import = venettap_sg_import,
+};
+
+/*
+ * ---------------------------
+ * Flat (non Scatter-Gather) support
+ * ---------------------------
+ */
+
+/* assumes reference to priv->vbus.conn held */
+static int
+venettap_flat_decode(struct venettap *priv, void *ptr, int len)
+{
+	size_t maxlen = priv->netif.dev->mtu + ETH_HLEN;
+
+	if (len > maxlen)
+		return -1;
+
+	/*
+	 * If SG is *not* enabled, the length is simply the
+	 * descriptor length
+	 */
+
+	return len;
+}
+
+/*
+ * venettap_rx_flat - import an skb in non scatter-gather mode
+ *
+ * assumes reference to priv->vbus.conn held
+ */
+static int
+venettap_flat_import(struct venettap *priv, struct sk_buff *skb,
+		     void *ptr, int len)
+{
+	struct vbus_memctx *ctx = priv->vbus.ctx;
+	int ret;
+
+	ret = ctx->ops->copy_from(ctx, skb_tail_pointer(skb), ptr, len);
+	if (ret)
+		return -EFAULT;
+
+	skb_put(skb, len);
+
+	return 0;
+}
+
+static struct venettap_rx_ops venettap_flat_rx_ops = {
+	.decode = venettap_flat_decode,
+	.import = venettap_flat_import,
+};
+
+/*
  * The poll implementation.
  */
 static int
@@ -303,6 +493,7 @@ venettap_rx(struct venettap *priv)
 	int                         ret;
 	unsigned long               flags;
 	struct vbus_connection     *conn;
+	struct venettap_rx_ops     *rx_ops;
 
 	PDEBUG("polling...\n");
 
@@ -326,6 +517,8 @@ venettap_rx(struct venettap *priv)
 	ioq = priv->vbus.rxq.queue;
 	ctx = priv->vbus.ctx;
 
+	rx_ops = priv->vbus.rx_ops;
+
 	spin_unlock_irqrestore(&priv->lock, flags);
 
 	/* We want to iterate on the head of the in-use index */
@@ -340,11 +533,14 @@ venettap_rx(struct venettap *priv)
 	 * the north side
 	 */
 	while (iter.desc->sown) {
-		size_t len = iter.desc->len;
-		size_t maxlen = priv->netif.dev->mtu + ETH_HLEN;
 		struct sk_buff *skb = NULL;
+		size_t len;
+
+		len = rx_ops->decode(priv,
+				     (void *)iter.desc->ptr,
+				     iter.desc->len);
 
-		if (unlikely(len > maxlen)) {
+		if (unlikely(len < 0)) {
 			priv->netif.stats.rx_errors++;
 			priv->netif.stats.rx_length_errors++;
 			goto next;
@@ -362,10 +558,8 @@ venettap_rx(struct venettap *priv)
 		/* align IP on 16B boundary */
 		skb_reserve(skb, 2);
 
-		ret = ctx->ops->copy_from(ctx, skb->data,
-					 (void *)iter.desc->ptr,
-					 len);
-		if (unlikely(ret)) {
+		ret = rx_ops->import(priv, skb, (void *)iter.desc->ptr, len);
+		if (unlikely(ret < 0)) {
 			priv->netif.stats.rx_errors++;
 			goto next;
 		}
@@ -846,6 +1040,23 @@ venettap_macquery(struct venettap *priv, void *data, unsigned long len)
 	return 0;
 }
 
+static u32
+venettap_negcap_sg(struct venettap *priv, u32 requested)
+{
+	u32 available = VENET_CAP_SG|VENET_CAP_TSO4|VENET_CAP_TSO6
+		|VENET_CAP_ECN;
+	u32 ret;
+
+	ret = available & requested;
+
+	if (ret & VENET_CAP_SG) {
+		priv->vbus.sg.enabled = true;
+		priv->vbus.rx_ops = &venettap_sg_rx_ops;
+	}
+
+	return ret;
+}
+
 /*
  * Negotiate Capabilities - This function is provided so that the
  * interface may be extended without breaking ABI compatability
@@ -873,6 +1084,9 @@ venettap_negcap(struct venettap *priv, void *data, unsigned long len)
 		return -EFAULT;
 
 	switch (caps.gid) {
+	case VENET_CAP_GROUP_SG:
+		caps.bits = venettap_negcap_sg(priv, caps.bits);
+		break;
 	default:
 		caps.bits = 0;
 		break;
@@ -1057,6 +1271,12 @@ venettap_vlink_release(struct vbus_connection *conn)
 	vbus_memctx_put(priv->vbus.ctx);
 
 	kobject_put(priv->vbus.dev.kobj);
+
+	priv->vbus.sg.enabled = false;
+	priv->vbus.rx_ops = &venettap_flat_rx_ops;
+	kfree(priv->vbus.sg.desc);
+	priv->vbus.sg.desc = NULL;
+	priv->vbus.sg.len = 0;
 }
 
 static struct vbus_connection_ops venettap_vbus_link_ops = {
@@ -1336,6 +1556,7 @@ venettap_device_create(struct vbus_devclass *dc,
 	_vdev->ops             = &venettap_device_ops;
 	_vdev->attrs           = &venettap_attr_group;
 
+	priv->vbus.rx_ops      = &venettap_flat_rx_ops;
 	init_waitqueue_head(&priv->vbus.rx_empty);
 
 	/*
#21 lore raw

RFC PATCH v2 13/19 x86: allow the irq->vector translation to be determined outside of ioapic

In reply to Gregory Haskins

The ioapic code currently privately manages the mapping between irq and vector. This results in some layering violations as the support for certain MSI operations need this info. As a result, the MSI code itself was moved to the ioapic module. This is not really optimal.

We now have another need to gain access to the vector assignment on x86. However, rather than put yet another inappropriately placed function into io-apic, lets create a way to export this simple data and therefore allow the logic to sit closer to where it belongs.

Ideally we should abstract the entire notion of irq->vector management out of io-apic, but we leave that as an excercise for another day.

---

 arch/x86/include/asm/irq.h |    6 ++++++
 arch/x86/kernel/io_apic.c  |   25 +++++++++++++++++++++++++
 2 files changed, 31 insertions(+), 0 deletions(-)
arch/x86/include/asm/irq.h +6 -0
index 592688e..b1726d8 100644
@@ -40,6 +40,12 @@ extern unsigned int do_IRQ(struct pt_regs *regs);
 extern void init_IRQ(void);
 extern void native_init_IRQ(void);
 
+#ifdef CONFIG_SMP
+extern int set_irq_affinity(int irq, cpumask_t mask);
+#endif
+
+extern int irq_to_vector(int irq);
+
 /* Interrupt vector management */
 extern DECLARE_BITMAP(used_vectors, NR_VECTORS);
 extern int vector_used_by_percpu_irq(unsigned int vector);
arch/x86/kernel/io_apic.c +25 -0
index bc7ac4d..86a2c36 100644
@@ -614,6 +614,14 @@ set_ioapic_affinity_irq(unsigned int irq, const struct cpumask *mask)
 
 	set_ioapic_affinity_irq_desc(desc, mask);
 }
+
+int set_irq_affinity(int irq, cpumask_t mask)
+{
+	set_ioapic_affinity_irq(irq, &mask);
+
+	return 0;
+}
+
 #endif /* CONFIG_SMP */
 
 /*
@@ -3249,6 +3257,23 @@ void destroy_irq(unsigned int irq)
 	spin_unlock_irqrestore(&vector_lock, flags);
 }
 
+int irq_to_vector(int irq)
+{
+	struct irq_cfg *cfg;
+	unsigned long flags;
+	int ret = -ENOENT;
+
+	spin_lock_irqsave(&vector_lock, flags);
+
+	cfg = irq_cfg(irq);
+	if (cfg && cfg->vector != 0)
+		ret = cfg->vector;
+
+	spin_unlock_irqrestore(&vector_lock, flags);
+
+	return ret;
+}
+
 /*
  * MSI message composition
  */
#22 lore raw

RFC PATCH v2 14/19 kvm: add a reset capability

In reply to Gregory Haskins

We need a way to detect if a VM is reset later in the series, so lets add a capability for userspace to signal a VM reset down to the kernel.

---

 arch/x86/kvm/x86.c       |    1 +
 include/linux/kvm.h      |    2 ++
 include/linux/kvm_host.h |    6 ++++++
 virt/kvm/kvm_main.c      |   36 ++++++++++++++++++++++++++++++++++++
 4 files changed, 45 insertions(+), 0 deletions(-)
arch/x86/kvm/x86.c +1 -0
index 758b7a1..9b0a649 100644
@@ -971,6 +971,7 @@ int kvm_dev_ioctl_check_extension(long ext)
 	case KVM_CAP_NOP_IO_DELAY:
 	case KVM_CAP_MP_STATE:
 	case KVM_CAP_SYNC_MMU:
+	case KVM_CAP_RESET:
 		r = 1;
 		break;
 	case KVM_CAP_COALESCED_MMIO:
include/linux/kvm.h +2 -0
index 0424326..7ffd8f5 100644
@@ -396,6 +396,7 @@ struct kvm_trace_rec {
 #ifdef __KVM_HAVE_USER_NMI
 #define KVM_CAP_USER_NMI 22
 #endif
+#define KVM_CAP_RESET 23
 
 /*
  * ioctls for VM fds
@@ -429,6 +430,7 @@ struct kvm_trace_rec {
 				   struct kvm_assigned_pci_dev)
 #define KVM_ASSIGN_IRQ _IOR(KVMIO, 0x70, \
 			    struct kvm_assigned_irq)
+#define KVM_RESET	          _IO(KVMIO,  0x67)
 
 /*
  * ioctls for vcpu fds
include/linux/kvm_host.h +6 -0
index bf6f703..506eca1 100644
@@ -17,6 +17,7 @@
 #include <linux/preempt.h>
 #include <linux/marker.h>
 #include <linux/msi.h>
+#include <linux/notifier.h>
 #include <asm/signal.h>
 
 #include <linux/kvm.h>
@@ -132,6 +133,8 @@ struct kvm {
 	unsigned long mmu_notifier_seq;
 	long mmu_notifier_count;
 #endif
+
+	struct raw_notifier_head reset_notifier; /* triggers when VM reboots */
 };
 
 /* The guest did something we don't support. */
@@ -158,6 +161,9 @@ void kvm_exit(void);
 void kvm_get_kvm(struct kvm *kvm);
 void kvm_put_kvm(struct kvm *kvm);
 
+int kvm_reset_notifier_register(struct kvm *kvm, struct notifier_block *nb);
+int kvm_reset_notifier_unregister(struct kvm *kvm, struct notifier_block *nb);
+
 #define HPA_MSB ((sizeof(hpa_t) * 8) - 1)
 #define HPA_ERR_MASK ((hpa_t)1 << HPA_MSB)
 static inline int is_error_hpa(hpa_t hpa) { return hpa >> HPA_MSB; }
virt/kvm/kvm_main.c +36 -0
index 29a667c..fca2d25 100644
@@ -868,6 +868,8 @@ static struct kvm *kvm_create_vm(void)
 #ifdef KVM_COALESCED_MMIO_PAGE_OFFSET
 	kvm_coalesced_mmio_init(kvm);
 #endif
+	RAW_INIT_NOTIFIER_HEAD(&kvm->reset_notifier);
+
 out:
 	return kvm;
 }
@@ -1485,6 +1487,35 @@ void mark_page_dirty(struct kvm *kvm, gfn_t gfn)
 	}
 }
 
+static void kvm_notify_reset(struct kvm *kvm)
+{
+	mutex_lock(&kvm->lock);
+	raw_notifier_call_chain(&kvm->reset_notifier, 0, kvm);
+	mutex_unlock(&kvm->lock);
+}
+
+int kvm_reset_notifier_register(struct kvm *kvm, struct notifier_block *nb)
+{
+	int ret;
+
+	mutex_lock(&kvm->lock);
+	ret = raw_notifier_chain_register(&kvm->reset_notifier, nb);
+	mutex_unlock(&kvm->lock);
+
+	return ret;
+}
+
+int kvm_reset_notifier_unregister(struct kvm *kvm, struct notifier_block *nb)
+{
+	int ret;
+
+	mutex_lock(&kvm->lock);
+	ret = raw_notifier_chain_unregister(&kvm->reset_notifier, nb);
+	mutex_unlock(&kvm->lock);
+
+	return ret;
+}
+
 /*
  * The vCPU has executed a HLT instruction with in-kernel mode enabled.
  */
@@ -1929,6 +1960,11 @@ static long kvm_vm_ioctl(struct file *filp,
 		break;
 	}
 #endif
+	case KVM_RESET: {
+		kvm_notify_reset(kvm);
+		r = 0;
+		break;
+	}
 	default:
 		r = kvm_arch_vm_ioctl(filp, ioctl, arg);
 	}
#23 lore raw

Re: [RFC PATCH v2 14/19] kvm: add a reset capability

Quoted 3 lines · Gregory Haskins

We need a way to detect if a VM is reset later in the series, so lets add a capability for userspace to signal a VM reset down to the kernel.

As I mentioned, this won't be reliable. It needs to be driven from userspace and be per-device.

Signature · 1 line
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
#24 lore raw

RFC PATCH v2 15/19 kvm: add dynamic IRQ support

In reply to Gregory Haskins

This patch provides the ability to dynamically declare and map an interrupt-request handle to an x86 8-bit vector.

Problem Statement: Emulated devices (such as PCI, ISA, etc) have interrupt routing done via standard PC mechanisms (MP-table, ACPI, etc). However, we also want to support a new class of devices which exist in a new virtualized namespace and therefore should not try to piggyback on these emulated mechanisms. Rather, we create a way to dynamically register interrupt resources that acts indepent of the emulated counterpart.

On x86, a simplistic view of the interrupt model is that each core has a local-APIC which can recieve messages from APIC-compliant routing devices (such as IO-APIC and MSI) regarding details about an interrupt (such as which vector to raise). These routing devices are controlled by the OS so they may translate a physical event (such as "e1000: raise an RX interrupt") to a logical destination (such as "inject IDT vector 46 on core 3"). A dynirq is a virtual implementation of such a router (think of it as a virtual-MSI, but without the coupling to an existing standard, such as PCI).

The model is simple: A guest OS can allocate the mapping of "IRQ" handle to "vector/core" in any way it sees fit, and provide this information to the dynirq module running in the host. The assigned IRQ then becomes the sole handle needed to inject an IDT vector to the guest from a host. A host entity that wishes to raise an interrupt simple needs to call kvm_inject_dynirq(irq) and the routing is performed transparently.

---

 arch/x86/Kconfig                |    5 +
 arch/x86/Makefile               |    3 
 arch/x86/include/asm/kvm_host.h |    9 +
 arch/x86/include/asm/kvm_para.h |   11 +
 arch/x86/kvm/Makefile           |    3 
 arch/x86/kvm/dynirq.c           |  329 +++++++++++++++++++++++++++++++++++++++
 arch/x86/kvm/guest/Makefile     |    2 
 arch/x86/kvm/guest/dynirq.c     |   95 +++++++++++
 arch/x86/kvm/x86.c              |    6 +
 include/linux/kvm.h             |    1 
 include/linux/kvm_guest.h       |    7 +
 include/linux/kvm_host.h        |    1 
 include/linux/kvm_para.h        |    1 
 13 files changed, 472 insertions(+), 1 deletions(-)
 create mode 100644 arch/x86/kvm/dynirq.c
 create mode 100644 arch/x86/kvm/guest/Makefile
 create mode 100644 arch/x86/kvm/guest/dynirq.c
 create mode 100644 include/linux/kvm_guest.h
arch/x86/Kconfig +5 -0
index 3fca247..91fefd5 100644
@@ -446,6 +446,11 @@ config KVM_GUEST
 	 This option enables various optimizations for running under the KVM
 	 hypervisor.
 
+config KVM_GUEST_DYNIRQ
+       bool "KVM Dynamic IRQ support"
+       depends on KVM_GUEST
+       default y
+
 source "arch/x86/lguest/Kconfig"
 
 config PARAVIRT
arch/x86/Makefile +3 -0
index d1a47ad..d788815 100644
@@ -147,6 +147,9 @@ core-$(CONFIG_XEN) += arch/x86/xen/
 # lguest paravirtualization support
 core-$(CONFIG_LGUEST_GUEST) += arch/x86/lguest/
 
+# kvm paravirtualization support
+core-$(CONFIG_KVM_GUEST) += arch/x86/kvm/guest/
+
 core-y += arch/x86/kernel/
 core-y += arch/x86/mm/
 
arch/x86/include/asm/kvm_host.h +9 -0
index 730843d..9ae398a 100644
@@ -346,6 +346,12 @@ struct kvm_mem_alias {
 	gfn_t target_gfn;
 };
 
+struct kvm_dynirq {
+	spinlock_t lock;
+	struct rb_root map;
+	struct kvm *kvm;
+};
+
 struct kvm_arch{
 	int naliases;
 	struct kvm_mem_alias aliases[KVM_ALIAS_SLOTS];
@@ -363,6 +369,7 @@ struct kvm_arch{
 	struct iommu_domain *iommu_domain;
 	struct kvm_pic *vpic;
 	struct kvm_ioapic *vioapic;
+	struct kvm_dynirq *dynirq;
 	struct kvm_pit *vpit;
 	struct hlist_head irq_ack_notifier_list;
 	int vapics_in_nmi_mode;
@@ -519,6 +526,8 @@ int emulator_write_phys(struct kvm_vcpu *vcpu, gpa_t gpa,
 			  const void *val, int bytes);
 int kvm_pv_mmu_op(struct kvm_vcpu *vcpu, unsigned long bytes,
 		  gpa_t addr, unsigned long *ret);
+int kvm_dynirq_hc(struct kvm_vcpu *vcpu, int nr, gpa_t gpa, size_t len);
+void kvm_free_dynirq(struct kvm *kvm);
 
 extern bool tdp_enabled;
 
arch/x86/include/asm/kvm_para.h +11 -0
index b8a3305..fba210e 100644
@@ -13,6 +13,7 @@
 #define KVM_FEATURE_CLOCKSOURCE		0
 #define KVM_FEATURE_NOP_IO_DELAY	1
 #define KVM_FEATURE_MMU_OP		2
+#define KVM_FEATURE_DYNIRQ		3
 
 #define MSR_KVM_WALL_CLOCK  0x11
 #define MSR_KVM_SYSTEM_TIME 0x12
@@ -45,6 +46,16 @@ struct kvm_mmu_op_release_pt {
 	__u64 pt_phys;
 };
 
+/* Operations for KVM_HC_DYNIRQ */
+#define KVM_DYNIRQ_OP_SET   1
+#define KVM_DYNIRQ_OP_CLEAR 2
+
+struct kvm_dynirq_set {
+	__u32 irq;
+	__u32 vec;  /* x86 IDT vector */
+	__u32 dest; /* 0-based vcpu id */
+};
+
 #ifdef __KERNEL__
 #include <asm/processor.h>
 
arch/x86/kvm/Makefile +2 -1
index d3ec292..d5676f5 100644
@@ -14,9 +14,10 @@ endif
 EXTRA_CFLAGS += -Ivirt/kvm -Iarch/x86/kvm
 
 kvm-objs := $(common-objs) x86.o mmu.o x86_emulate.o i8259.o irq.o lapic.o \
-	i8254.o
+	i8254.o dynirq.o
 obj-$(CONFIG_KVM) += kvm.o
 kvm-intel-objs = vmx.o
 obj-$(CONFIG_KVM_INTEL) += kvm-intel.o
 kvm-amd-objs = svm.o
 obj-$(CONFIG_KVM_AMD) += kvm-amd.o
+
arch/x86/kvm/dynirq.c new file +329 -0

329 lines not on this page · raw message

arch/x86/kvm/guest/Makefile new file +2 -0
new file mode 100644
index 0000000..de8f824
@@ -0,0 +1,2 @@
+
+obj-$(CONFIG_KVM_GUEST_DYNIRQ) += dynirq.o
\ No newline at end of file
arch/x86/kvm/guest/dynirq.c new file +95 -0

95 lines not on this page · raw message

arch/x86/kvm/x86.c +6 -0
index 9b0a649..e24f0a5 100644
@@ -972,6 +972,7 @@ int kvm_dev_ioctl_check_extension(long ext)
 	case KVM_CAP_MP_STATE:
 	case KVM_CAP_SYNC_MMU:
 	case KVM_CAP_RESET:
+	case KVM_CAP_DYNIRQ:
 		r = 1;
 		break;
 	case KVM_CAP_COALESCED_MMIO:
@@ -2684,6 +2685,9 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
 	case KVM_HC_MMU_OP:
 		r = kvm_pv_mmu_op(vcpu, a0, hc_gpa(vcpu, a1, a2), &ret);
 		break;
+	case KVM_HC_DYNIRQ:
+		ret = kvm_dynirq_hc(vcpu, a0, a1, a2);
+		break;
 	default:
 		ret = -KVM_ENOSYS;
 		break;
@@ -4141,6 +4145,8 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
 	kvm_free_pit(kvm);
 	kfree(kvm->arch.vpic);
 	kfree(kvm->arch.vioapic);
+	if (kvm->arch.dynirq)
+		kvm_free_dynirq(kvm);
 	kvm_free_vcpus(kvm);
 	kvm_free_physmem(kvm);
 	if (kvm->arch.apic_access_page)
include/linux/kvm.h +1 -0
index 7ffd8f5..349d273 100644
@@ -397,6 +397,7 @@ struct kvm_trace_rec {
 #define KVM_CAP_USER_NMI 22
 #endif
 #define KVM_CAP_RESET 23
+#define KVM_CAP_DYNIRQ 24
 
 /*
  * ioctls for VM fds
include/linux/kvm_guest.h new file +7 -0
new file mode 100644
index 0000000..7dd7930
@@ -0,0 +1,7 @@
+#ifndef __LINUX_KVM_GUEST_H
+#define __LINUX_KVM_GUEST_H
+
+extern int create_kvm_dynirq(int cpu);
+extern int destroy_kvm_dynirq(int irq);
+
+#endif /* __LINUX_KVM_GUEST_H */
include/linux/kvm_host.h +1 -0
index 506eca1..bec9b35 100644
@@ -297,6 +297,7 @@ int kvm_cpu_get_interrupt(struct kvm_vcpu *v);
 int kvm_cpu_has_interrupt(struct kvm_vcpu *v);
 int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu);
 void kvm_vcpu_kick(struct kvm_vcpu *vcpu);
+int kvm_inject_dynirq(struct kvm *kvm, int irq);
 
 int kvm_is_mmio_pfn(pfn_t pfn);
 
include/linux/kvm_para.h +1 -0
index 3ddce03..a2de904 100644
@@ -16,6 +16,7 @@
 
 #define KVM_HC_VAPIC_POLL_IRQ		1
 #define KVM_HC_MMU_OP			2
+#define KVM_HC_DYNIRQ			3
 
 /*
  * hypercalls use architecture specific
#25 lore raw

Re: [RFC PATCH v2 15/19] kvm: add dynamic IRQ support

Quoted 42 lines · Gregory Haskins

This patch provides the ability to dynamically declare and map an interrupt-request handle to an x86 8-bit vector.

Problem Statement: Emulated devices (such as PCI, ISA, etc) have interrupt routing done via standard PC mechanisms (MP-table, ACPI, etc). However, we also want to support a new class of devices which exist in a new virtualized namespace and therefore should not try to piggyback on these emulated mechanisms. Rather, we create a way to dynamically register interrupt resources that acts indepent of the emulated counterpart.

On x86, a simplistic view of the interrupt model is that each core has a local-APIC which can recieve messages from APIC-compliant routing devices (such as IO-APIC and MSI) regarding details about an interrupt (such as which vector to raise). These routing devices are controlled by the OS so they may translate a physical event (such as "e1000: raise an RX interrupt") to a logical destination (such as "inject IDT vector 46 on core 3"). A dynirq is a virtual implementation of such a router (think of it as a virtual-MSI, but without the coupling to an existing standard, such as PCI).

The model is simple: A guest OS can allocate the mapping of "IRQ" handle to "vector/core" in any way it sees fit, and provide this information to the dynirq module running in the host. The assigned IRQ then becomes the sole handle needed to inject an IDT vector to the guest from a host. A host entity that wishes to raise an interrupt simple needs to call kvm_inject_dynirq(irq) and the routing is performed transparently.

arch/x86/kvm/dynirq.c new file +12 -0
+static int
+_kvm_inject_dynirq(struct kvm *kvm, struct dynirq *entry)
+{
+	struct kvm_vcpu *vcpu;
+	int ret;
+
+	mutex_lock(&kvm->lock);
+
+	vcpu = kvm->vcpus[entry->dest];
+	if (!vcpu) {
+		ret = -ENOENT;
+		goto out;

arch/x86/kvm/dynirq.c line 181

Quoted 10 lines
arch/x86/kvm/dynirq.c new file +10 -0
+	}
+
+	ret = kvm_apic_set_irq(vcpu, entry->vec, 1);
+
+out:
+	mutex_unlock(&kvm->lock);
+
+	return ret;
+}
+

Given that you're using the apic to inject the IRQ, you'll need an EOI. So what's the difference between dynirq and MSI, performance wise?

Signature · 1 line
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
#26 lore raw

Re: [RFC PATCH v2 15/19] kvm: add dynamic IRQ support

Quoted 5 lines · Avi Kivity
Quoted 1 line

Given that you're using the apic to inject the IRQ, you'll need an EOI. So what's the difference between dynirq and MSI, performance wise?

I would have loved to eliminate that EOI completely, but that is a much broader problem to solve ;)

Actually dynirq wasnt introduced as a performance alternative over MSI. Rather, I was trying to eliminate the complexity of needing to sync between the userspace PCI emulation and the in kernel models.

However, its moot. Since v2 introduce virtio-vbus, I now have a much clearer picture of what is needed here, and v3 will simply integrate with MSI interrupts and drop dynirq completely. So please ignore this patch.

-Greg

#27 lore raw

RFC PATCH v2 16/19 kvm: Add VBUS support to the host

In reply to Gregory Haskins

This patch adds support for guest access to a VBUS assigned to the same context as the VM. It utilizes a IOQ+IRQ to move events from host->guest, and provides a hypercall interface to move events guest->host.

---

 arch/x86/include/asm/kvm_para.h |    1 
 arch/x86/kvm/Kconfig            |    9 
 arch/x86/kvm/Makefile           |    3 
 arch/x86/kvm/x86.c              |    6 
 arch/x86/kvm/x86.h              |   12 
 include/linux/kvm.h             |    1 
 include/linux/kvm_host.h        |   20 +
 include/linux/kvm_para.h        |   59 ++
 virt/kvm/kvm_main.c             |    1 
 virt/kvm/vbus.c                 | 1307 +++++++++++++++++++++++++++++++++++++++
 10 files changed, 1419 insertions(+), 0 deletions(-)
 create mode 100644 virt/kvm/vbus.c
arch/x86/include/asm/kvm_para.h +1 -0
index fba210e..19d81e0 100644
@@ -14,6 +14,7 @@
 #define KVM_FEATURE_NOP_IO_DELAY	1
 #define KVM_FEATURE_MMU_OP		2
 #define KVM_FEATURE_DYNIRQ		3
+#define KVM_FEATURE_VBUS                4
 
 #define MSR_KVM_WALL_CLOCK  0x11
 #define MSR_KVM_SYSTEM_TIME 0x12
arch/x86/kvm/Kconfig +9 -0
index b81125f..875e96e 100644
@@ -64,6 +64,15 @@ config KVM_TRACE
 	  relayfs.  Note the ABI is not considered stable and will be
 	  modified in future updates.
 
+config KVM_HOST_VBUS
+       bool "KVM virtual-bus (VBUS) host-side support"
+       depends on KVM
+       select VBUS
+       default n
+       ---help---
+          This option enables host-side support for accessing virtual-bus
+	  devices.
+
 # OK, it's a little counter-intuitive to do this, but it puts it neatly under
 # the virtualization menu.
 source drivers/lguest/Kconfig
arch/x86/kvm/Makefile +3 -0
index d5676f5..f749ec9 100644
@@ -15,6 +15,9 @@ EXTRA_CFLAGS += -Ivirt/kvm -Iarch/x86/kvm
 
 kvm-objs := $(common-objs) x86.o mmu.o x86_emulate.o i8259.o irq.o lapic.o \
 	i8254.o dynirq.o
+ifeq ($(CONFIG_KVM_HOST_VBUS),y)
+kvm-objs += $(addprefix ../../../virt/kvm/, vbus.o)
+endif
 obj-$(CONFIG_KVM) += kvm.o
 kvm-intel-objs = vmx.o
 obj-$(CONFIG_KVM_INTEL) += kvm-intel.o
arch/x86/kvm/x86.c +6 -0
index e24f0a5..2369d84 100644
@@ -996,6 +996,9 @@ int kvm_dev_ioctl_check_extension(long ext)
 	case KVM_CAP_CLOCKSOURCE:
 		r = boot_cpu_has(X86_FEATURE_CONSTANT_TSC);
 		break;
+	case KVM_CAP_VBUS:
+		r = kvm_vbus_support();
+		break;
 	default:
 		r = 0;
 		break;
@@ -2688,6 +2691,9 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
 	case KVM_HC_DYNIRQ:
 		ret = kvm_dynirq_hc(vcpu, a0, a1, a2);
 		break;
+	case KVM_HC_VBUS:
+		ret = kvm_vbus_hc(vcpu, a0, a1, a2);
+		break;
 	default:
 		ret = -KVM_ENOSYS;
 		break;
arch/x86/kvm/x86.h +12 -0
index 6a4be78..b6c682b 100644
@@ -3,6 +3,18 @@
 
 #include <linux/kvm_host.h>
 
+#ifdef CONFIG_KVM_HOST_VBUS
+static inline int kvm_vbus_support(void)
+{
+    return 1;
+}
+#else
+static inline int kvm_vbus_support(void)
+{
+    return 0;
+}
+#endif
+
 static inline void kvm_clear_exception_queue(struct kvm_vcpu *vcpu)
 {
 	vcpu->arch.exception.pending = false;
include/linux/kvm.h +1 -0
index 349d273..077daac 100644
@@ -398,6 +398,7 @@ struct kvm_trace_rec {
 #endif
 #define KVM_CAP_RESET 23
 #define KVM_CAP_DYNIRQ 24
+#define KVM_CAP_VBUS 25
 
 /*
  * ioctls for VM fds
include/linux/kvm_host.h +20 -0
index bec9b35..757f998 100644
@@ -120,6 +120,9 @@ struct kvm {
 	struct list_head vm_list;
 	struct kvm_io_bus mmio_bus;
 	struct kvm_io_bus pio_bus;
+#ifdef CONFIG_KVM_HOST_VBUS
+	struct kvm_vbus *kvbus;
+#endif
 	struct kvm_vm_stat stat;
 	struct kvm_arch arch;
 	atomic_t users_count;
@@ -471,4 +474,21 @@ static inline int mmu_notifier_retry(struct kvm_vcpu *vcpu, unsigned long mmu_se
 }
 #endif
 
+#ifdef CONFIG_KVM_HOST_VBUS
+
+int kvm_vbus_hc(struct kvm_vcpu *vcpu, int nr, gpa_t gpa, size_t len);
+void kvm_vbus_release(struct kvm_vbus *kvbus);
+
+#else /* CONFIG_KVM_HOST_VBUS */
+
+static inline int
+kvm_vbus_hc(struct kvm_vcpu *vcpu, int nr, gpa_t gpa, size_t len)
+{
+	return -EINVAL;
+}
+
+#define kvm_vbus_release(kvbus) do {} while (0)
+
+#endif /* CONFIG_KVM_HOST_VBUS */
+
 #endif
include/linux/kvm_para.h +59 -0

65 lines not on this page · raw message

virt/kvm/kvm_main.c +1 -0
index fca2d25..2e4ba8b 100644
@@ -942,6 +942,7 @@ static int kvm_vm_release(struct inode *inode, struct file *filp)
 {
 	struct kvm *kvm = filp->private_data;
 
+	kvm_vbus_release(kvm->kvbus);
 	kvm_put_kvm(kvm);
 	return 0;
 }
virt/kvm/vbus.c new file +1307 -0

1,307 lines not on this page · raw message

#28 lore raw

RFC PATCH v2 17/19 kvm: Add guest-side support for VBUS

In reply to Gregory Haskins

This adds a driver to interface between the host VBUS support, and the guest-vbus bus model.

---

 arch/x86/Kconfig            |    9 +
 drivers/Makefile            |    1 
 drivers/vbus/proxy/Makefile |    2 
 drivers/vbus/proxy/kvm.c    |  726 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 738 insertions(+), 0 deletions(-)
 create mode 100644 drivers/vbus/proxy/Makefile
 create mode 100644 drivers/vbus/proxy/kvm.c
arch/x86/Kconfig +9 -0
index 91fefd5..8661495 100644
@@ -451,6 +451,15 @@ config KVM_GUEST_DYNIRQ
        depends on KVM_GUEST
        default y
 
+config KVM_GUEST_VBUS
+       tristate "KVM virtual-bus (VBUS) guest-side support"
+       depends on KVM_GUEST
+       select VBUS_DRIVERS
+       default y
+       ---help---
+          This option enables guest-side support for accessing virtual-bus
+	  devices.
+
 source "arch/x86/lguest/Kconfig"
 
 config PARAVIRT
drivers/Makefile +1 -0
index 98fab51..4f2cb93 100644
@@ -107,3 +107,4 @@ obj-$(CONFIG_VIRTIO)		+= virtio/
 obj-$(CONFIG_STAGING)		+= staging/
 obj-y				+= platform/
 obj-$(CONFIG_VBUS_DEVICES)	+= vbus/devices/
+obj-$(CONFIG_VBUS_DRIVERS)	+= vbus/proxy/
drivers/vbus/proxy/Makefile new file +2 -0
new file mode 100644
index 0000000..c18d58d
@@ -0,0 +1,2 @@
+kvm-guest-vbus-objs += kvm.o
+obj-$(CONFIG_KVM_GUEST_VBUS) += kvm-guest-vbus.o
drivers/vbus/proxy/kvm.c new file +726 -0

726 lines not on this page · raw message

#29 lore raw

RFC PATCH v2 18/19 vbus: add a userspace connector

In reply to Gregory Haskins

This allows userspace applications to access vbus devices

---

 include/linux/vbus.h           |    4 
 include/linux/vbus_client.h    |    2 
 include/linux/vbus_userspace.h |   48 ++++
 kernel/vbus/Kconfig            |   10 +
 kernel/vbus/Makefile           |    2 
 kernel/vbus/userspace-client.c |  485 ++++++++++++++++++++++++++++++++++++++++
 6 files changed, 550 insertions(+), 1 deletions(-)
 create mode 100644 include/linux/vbus_userspace.h
 create mode 100644 kernel/vbus/userspace-client.c
include/linux/vbus.h +4 -0
index 04db4ff..f967e59 100644
@@ -23,6 +23,8 @@
 #ifndef _LINUX_VBUS_H
 #define _LINUX_VBUS_H
 
+#ifdef __KERNEL__
+
 #ifdef CONFIG_VBUS
 
 #include <linux/module.h>
@@ -159,4 +161,6 @@ int vbus_notifier_unregister(struct vbus *vbus, struct notifier_block *nb);
 
 #endif /* CONFIG_VBUS */
 
+#endif /* __KERNEL__ */
+
 #endif /* _LINUX_VBUS_H */
include/linux/vbus_client.h +1 -1
index 62dab78..4c82822 100644
@@ -35,7 +35,7 @@
 #ifndef _LINUX_VBUS_CLIENT_H
 #define _LINUX_VBUS_CLIENT_H
 
-#include <linux/types.h>
+#include <asm/types.h>
 #include <linux/compiler.h>
 
 struct vbus_deviceopen {
include/linux/vbus_userspace.h new file +48 -0

48 lines not on this page · raw message

kernel/vbus/Kconfig +10 -0
index 3ce0adc..b894dd1 100644
@@ -25,6 +25,16 @@ config VBUS_DEVICES
 
 source "drivers/vbus/devices/Kconfig"
 
+config VBUS_USERSPACE
+       tristate "Virtual-Bus userspace client support"
+       depends on VBUS
+       default y
+       help
+         Provides facilities for userspace applications to access virtual-
+	 bus objects.
+
+	 If unsure, say N
+
 config VBUS_DRIVERS
        tristate "VBUS Driver support"
        select IOQ
kernel/vbus/Makefile +2 -0
index 45f6503..61d0371 100644
@@ -4,3 +4,5 @@ obj-$(CONFIG_VBUS) += shm-ioq.o
 vbus-proxy-objs += proxy.o
 obj-$(CONFIG_VBUS_DRIVERS) += vbus-proxy.o
 
+vbus-userspace-objs += userspace-client.o
+obj-$(CONFIG_VBUS_USERSPACE) += vbus-userspace.o
kernel/vbus/userspace-client.c new file +485 -0

485 lines not on this page · raw message

#30 lore raw

RFC PATCH v2 19/19 virtio: add a vbus transport

In reply to Gregory Haskins

We add a new virtio transport for accessing backends located on vbus. This complements the existing transports for virtio-pci, virtio-s390, and virtio-lguest that already exist.

---

 drivers/virtio/Kconfig       |   15 +
 drivers/virtio/Makefile      |    1 
 drivers/virtio/virtio_vbus.c |  496 +++++++++++++++++++++++++++++++++
 include/linux/virtio_vbus.h  |  163 +++++++++++
 kernel/vbus/Kconfig          |    7 
 kernel/vbus/Makefile         |    3 
 kernel/vbus/virtio.c         |  628 ++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 1313 insertions(+), 0 deletions(-)
 create mode 100644 drivers/virtio/virtio_vbus.c
 create mode 100644 include/linux/virtio_vbus.h
 create mode 100644 kernel/vbus/virtio.c
drivers/virtio/Kconfig +15 -0
index 3dd6294..e8562ee 100644
@@ -23,6 +23,21 @@ config VIRTIO_PCI
 
 	  If unsure, say M.
 
+config VIRTIO_VBUS
+	tristate "VBUS driver for virtio devices (EXPERIMENTAL)"
+	depends on VBUS_DRIVERS && EXPERIMENTAL
+	select VIRTIO
+	select VIRTIO_RING
+	---help---
+	  This drivers provides support for virtio based paravirtual device
+	  drivers over VBUS.  This requires that your VMM has appropriate VBUS
+	  virtio backends.
+
+	  Currently, the ABI is not considered stable so there is no guarantee
+	  that this version of the driver will work with your VMM.
+
+	  If unsure, say M.
+
 config VIRTIO_BALLOON
 	tristate "Virtio balloon driver (EXPERIMENTAL)"
 	select VIRTIO
drivers/virtio/Makefile +1 -0
index 6738c44..0342e42 100644
@@ -1,4 +1,5 @@
 obj-$(CONFIG_VIRTIO) += virtio.o
 obj-$(CONFIG_VIRTIO_RING) += virtio_ring.o
 obj-$(CONFIG_VIRTIO_PCI) += virtio_pci.o
+obj-$(CONFIG_VIRTIO_VBUS) += virtio_vbus.o
 obj-$(CONFIG_VIRTIO_BALLOON) += virtio_balloon.o
drivers/virtio/virtio_vbus.c new file +496 -0

496 lines not on this page · raw message

include/linux/virtio_vbus.h new file +163 -0

163 lines not on this page · raw message

kernel/vbus/Kconfig +7 -0
index b894dd1..5eeced2 100644
@@ -14,6 +14,13 @@ config VBUS
 
 	If unsure, say N
 
+config VBUS_VIRTIO_BACKEND
+       tristate "Virtio VBUS Backend"
+       depends on VBUS
+       default n
+       help
+        Provides backend support for virtio devices over vbus
+
 config VBUS_DEVICES
        bool "Virtual-Bus Devices"
        depends on VBUS
kernel/vbus/Makefile +3 -0
index 61d0371..c2bd140 100644
@@ -1,6 +1,9 @@
 obj-$(CONFIG_VBUS) += core.o devclass.o config.o attribute.o map.o client.o
 obj-$(CONFIG_VBUS) += shm-ioq.o
 
+virtio-backend-objs += virtio.o
+obj-$(CONFIG_VBUS_VIRTIO_BACKEND) += virtio-backend.o
+
 vbus-proxy-objs += proxy.o
 obj-$(CONFIG_VBUS_DRIVERS) += vbus-proxy.o
 
kernel/vbus/virtio.c new file +628 -0

628 lines not on this page · raw message

#31 lore raw

Re: [RFC PATCH v2 19/19] virtio: add a vbus transport

Quoted 5 lines · Gregory Haskins

We add a new virtio transport for accessing backends located on vbus. This complements the existing transports for virtio-pci, virtio-s390, and virtio-lguest that already exist.

Very interesting...

I'm somewhat confused by what you're advocating vbus as. I'm trying to figure out how we converge vbus and virtio and become one big happy family :-)

What parts of it do you think are better than virtio? Should we forget about venet and just focus on virtio-net on top of virtio-vbus assuming that we can prove venet-tap/virtio-vbus/virtio-net is just as good as venet-tap/vbus/venet?

If we can prove that an in-kernel virtio-net backend/virtio-pci/virtio-net does just as well as venet-tap/virtio-vbus/virtio-net does that mean that vbus is no longer needed?

If you concede that the transport mechanisms can be identical, are you really advocating the discovering and configuration mechanisms in vbus? Is that what we should be focusing on? Do you care only about the host mechanisms or do you also require the guest infrastructure to be present?

I think two paravirtual I/O frameworks for KVM is a bad thing. It duplicates a ton of code and will very likely lead to user unhappiness.

Regards,

Anthony Liguori

#32 lore raw

Re: [RFC PATCH v2 19/19] virtio: add a vbus transport

Quoted 8 lines · Anthony Liguori
Quoted 6 lines · Gregory Haskins

We add a new virtio transport for accessing backends located on vbus. This
complements the existing transports for virtio-pci, virtio-s390, and virtio-lguest that already exist.

Quoted 7 lines

Very interesting...

I'm somewhat confused by what you're advocating vbus as. I'm trying to figure out how we converge vbus and virtio and become one big happy family :-)

What parts of it do you think are better than virtio?

I see them as distinctly different, and complimentary: virtio is primarily a device-interface. vbus is primarily a bus-model.

While its true that vbus has a rudimentary "device interface" (ala dev->call(), dev->shm(), etc), it will typically be layered with a more robust device model (such as virtqueue based virtio-net, or IOQ based venet).

This is akin to raw PCI. PCI has the basic mechanics (MMIO, PIO, interrupts, DMA), but generally devices overlay higher layer constructs (such as tx/rx rings for ethernet). vbus is similar in that regard.

virtio, on the other hand, is a fully fleshed device model, but it needs to be paired up with some other bus-model (such as qemu+pci via virtio-pci, or linux+vbus via virtio-vbus) to be complete.

Quoted 4 lines

Should we forget
about venet and just focus on virtio-net on top of virtio-vbus assuming that we can prove venet-tap/virtio-vbus/virtio-net is just as good as venet-tap/vbus/venet?

I think going forward, I would love to see some community weight behind virtio-vbus and the standardization of the virtio-X drivers (net, blk, console, etc) as the default core IO device model for vbus. If that were to happen, I would not be inclined to create any new native vbus drivers for where there is overlap with virtio (net, disk, etc).

I *will*, however, likely use the native vbus interfaces for some of the device models in the pipeline (such as the real-time controller, and OFED) because there isn't a virtio equivalent for that, and the vbus device model was written with them in mind.

And regardless of the final outcome of virtio-vbus as a viable virtio transport model, I will probably continue to support the native vbus "venet" protocol for AlacrityVM indefinitely only because I have people that are using it today (AlacrityVM being one example, but others are in the works), it is simple, and it works extremely well.

Quoted 5 lines

If we can prove that an in-kernel virtio-net backend/virtio-pci/virtio-net does just as well as venet-tap/virtio-vbus/virtio-net does that mean that vbus is no longer needed?

vbus is primarily about the bus model and resource-containers. It is therefore so much more than just 802.x networking and/or KVM. So no, obtaining equal network performance with virtio-net sans vbus does not invalidate vbus as a concept. It would perhaps purely be disincentive for anyone to chose venet over virtio-net within a KVM guest who doesn't have a vested interest in the venet ABI.

Quoted 3 lines

If you concede that the transport mechanisms can be identical, are you really advocating the discovering and configuration mechanisms in vbus?

The problem is that they are not truly identical, and they cannot easily be made identical.

One of the design goals is to create a re-useable, in-kernel device model with a thin-shim transport between the guest and the bus. The transport (called a "vbus-connector") in question must convey a very small verb namespace (devadd/drop, devopen/close, shmsignal, etc) to use this bus. So you have a driver for this transport (e.g. vbus-pcibridge) but the bus-model itself, device-models, and guest drivers are all reusable.

Another design goal is to create a high-performance, low-latency, software-to-software optimized IO subsystem. PCI was designed for software-to-hardware, so it has assumptions about that environment that simply do not apply in software-to-software. Most of these limitations can be worked-around with KVM surgery and/or creative guest development, but they are (at best) awkward to express in PCI.

For example, creating a high-performance, synchronous, in-kernel "call" in PCI is fairly complicated, awkward, and requires a bit of per-device heap to pull off. Yet, I need this functionality for things like RT support (scheduler state changes, etc).

As another example, the PCI model is somewhat resource rigid and therefore awkward for creating dynamic software objects in response to guest actions on demand, such as IPC sockets, due to the way MSI works.

As a third example, the PCI device model makes assumptions about signal-path delivery (MSI to APIC) which may or may not be the optimal path (e.g. see my priority+aggregation thread with Arnd).

I am open to suggestions on ways to write the "connector" code differently. I am sure that some things could be addressed at a different layer (such as the conversation with Arnd regarding the PV interrupt controller).

I think, however, that the motivations to PCI'ize the x86-KVM world are driven more by inertia (its already in qemu and many guests know it) than by intentional selection (its model is the best to express the needs of virt). Therefore, I am trying to put a stake in the ground to redirect that inertia towards a path that I believe is more amenable to virt in the long term. Consider it a long-term investment in the platform.

My approach to date is more akin to how something like USB was introduced. The USB designers were not trying to artificially constrain their design so that it looked like PCI just to avoid needing an OS driver. They (presumably) designed a bus that best achieved their goals, and then people wrote adaption layers (ala PCI-to-USB bridges) to support them on the OS's/arches that cared. Eventually most did.

I admit that, as with any new bus (e.g. USB), there will undoubtedly be some lumps during the initial roll-out phase since it is not yet universally supported. But the long term vision is that (assuming it succeeds as a project) that eventually vbus would be ubiquitous (ala USB) and it makes for a better bus-model choice for things like linux-based hypervisors (pci-based kvm (x86/ppc), s390-kvm, lguest, UML, openvz, and even something like Xen-Dom0, etc), as well as applications and even some specialized physical systems (Blade systems, clusters, etc).

Quoted 2 lines

Is that what we should be focusing on? Do you care only about the host mechanisms or do you also require the guest infrastructure to be present?

I see it as a tightly integrated portion of the stack that sits right below the virtio layer.

Quoted 2 lines

I think two paravirtual I/O frameworks for KVM is a bad thing.

So hopefully after reading my reply above you can see that I don't think this is indeed what is being proposed.

Quoted 1 line

It duplicates a ton of code and will very likely lead to user unhappiness.

I totally agree and I hope we can work together to find a solution that makes the most sense and eliminates as much duplicate effort as possible.

Kind Regards,
-Greg

#33 lore raw
In reply to Gregory Haskins

Avi,

Quoted 6 lines · Gregory Haskins

Todo:
*) Develop some kind of hypercall registration mechanism for KVM so that
we can use that as an integration point instead of directly hooking kvm hypercalls

What would you like to see here? I now remember why I removed the original patch I had for registration...it requires some kind of discovery mechanism on its own. Note that this is hard, but I figured it would make the overall series simpler if I didn't go this route and instead just integrated with a statically allocated vector. That being said, I have no problem adding this back in but figure we should discuss the approach so I don't go down a rat-hole ;)

So, one thing we could do is use a string-identifier to discover hypercall resources. In this model, we would have one additional hypercall registered with kvm (in addition to the mmu-ops, etc) called KVM_HC_DYNHC or something like that. The support for DYNHC could be indicated in the cpuid (much like I do with the RESET, DYNIRQ, and VBUS
support today. When hypercall provides register, the could provide a
string such as "vbus", and they would be allocated a hypercall id. Likewise, the HC_DYNHC interface would allow a guest to query the cpuid for the DYNHC feature, and then query the HC_DYNHC vector for a string to hc# translation. If the provider is not present, we return -1 for the hc#, otherwise we return the one that was allocated.

I know how you feel about string-ids in general, but I am not quite sure how to design this otherwise without it looking eerily similar to what I already have (which is registering a new HC vector in kvm_para.h)

Thoughts?

-Greg

#34 lore raw
Quoted 11 lines · Gregory Haskins

Avi,

Quoted 6 lines · Gregory Haskins

Todo:
*) Develop some kind of hypercall registration mechanism for KVM so that
we can use that as an integration point instead of directly hooking kvm hypercalls

Quoted 9 lines

What would you like to see here? I now remember why I removed the original patch I had for registration...it requires some kind of discovery mechanism on its own. Note that this is hard, but I figured it would make the overall series simpler if I didn't go this route and instead just integrated with a statically allocated vector. That being said, I have no problem adding this back in but figure we should discuss the approach so I don't go down a rat-hole ;)

One idea is similar to signalfd() or eventfd(). Provide a kvm ioctl that takes a gsi and returns an fd. Writes to the fd change the state of the line, possible triggering an interrupt. Another ioctl takes a hypercall number or pio port as well as an existing fd. Invocations of the hypercall or writes to the port write to the fd (using the same protocol as eventfd), so the other end can respond.

The nice thing is that this can be used by both kernel and userspace components, and for kernel components, hypercalls can be either buffered or unbuffered.

Quoted 11 lines

So, one thing we could do is use a string-identifier to discover
hypercall resources. In this model, we would have one additional
hypercall registered with kvm (in addition to the mmu-ops, etc) called
KVM_HC_DYNHC or something like that. The support for DYNHC could be
indicated in the cpuid (much like I do with the RESET, DYNIRQ, and VBUS
support today. When hypercall provides register, the could provide a
string such as "vbus", and they would be allocated a hypercall id. Likewise, the HC_DYNHC interface would allow a guest to query the cpuid for the DYNHC feature, and then query the HC_DYNHC vector for a string
to hc# translation. If the provider is not present, we return -1 for
the hc#, otherwise we return the one that was allocated.

Quoted 4 lines

I know how you feel about string-ids in general, but I am not quite sure how to design this otherwise without it looking eerily similar to what I already have (which is registering a new HC vector in kvm_para.h)

No need for a string ID. Reserve a range of hypercall numbers for dynamic IDs. Userspace allocates one and gives it to the device using its configuration space (as applies to whatever bus it is using).

Signature · 1 line
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
#35 lore raw
Quoted 30 lines · Avi Kivity
Quoted 11 lines · Gregory Haskins

Avi,

Quoted 6 lines · Gregory Haskins

Todo:
*) Develop some kind of hypercall registration mechanism for KVM so that
we can use that as an integration point instead of directly hooking kvm hypercalls

Quoted 9 lines

What would you like to see here? I now remember why I removed the original patch I had for registration...it requires some kind of discovery mechanism on its own. Note that this is hard, but I figured it would make the overall series simpler if I didn't go this route and instead just integrated with a statically allocated vector. That being said, I have no problem adding this back in but figure we should discuss the approach so I don't go down a rat-hole ;)

One idea is similar to signalfd() or eventfd(). Provide a kvm ioctl that takes a gsi and returns an fd. Writes to the fd change the state of the line, possible triggering an interrupt. Another ioctl takes a hypercall number or pio port as well as an existing fd. Invocations of the hypercall or writes to the port write to the fd (using the same protocol as eventfd), so the other end can respond.

Quoted 3 lines

The nice thing is that this can be used by both kernel and userspace components, and for kernel components, hypercalls can be either buffered or unbuffered.

And thus the "kvm-eventfd" (irqfd/iosignalfd) interface project was born. ;)

(Michael FYI: so I will be pushing a vbus-v4 series at some point in the near future that is expressed in terms of irqfd/iosignalfd, per the conversation above. The patches in v3 and earlier are more intrusive to the KVM core than they will be in final form)

-Greg

#36 lore raw
Quoted 6 lines · Gregory Haskins
Quoted 2 lines · Avi Kivity

Gregory Haskins wrote:
One idea is similar to signalfd() or eventfd()

And thus the "kvm-eventfd" (irqfd/iosignalfd) interface project was born. ;)

The lguest patch queue already has such an interface :) And I have a partially complete in-kernel virtio_pci patch with the same trick.

I switched from "kernel created eventfd" to "userspace passes in eventfd" after a while though; it lets you connect multiple virtqueues to a single fd if you want.

Combined with a minor change to allow any process with access to the lguest fd to queue interrupts, this allowed lguest to move to a thread-per-virtqueue model which was a significant speedup as well as nice code reduction.

Here's the relevant kernel patch for reading.

Thanks!
Rusty.

lguest: use eventfds for device notification

Currently, when a Guest wants to perform I/O it calls LHCALL_NOTIFY with an address: the main Launcher process returns with this address, and figures out what device to run.

A far nicer model is to let processes bind an eventfd to an address: if we find one, we simply signal the eventfd.

---
 drivers/lguest/Kconfig          |    2 -
 drivers/lguest/core.c           |    8 ++--
 drivers/lguest/lg.h             |    9 ++++
 drivers/lguest/lguest_user.c    |   73 ++++++++++++++++++++++++++++++++++++++++
 include/linux/lguest_launcher.h |    1 
 5 files changed, 89 insertions(+), 4 deletions(-)
drivers/lguest/Kconfig +1 -1
@@ -1,6 +1,6 @@
 config LGUEST
 	tristate "Linux hypervisor example code"
-	depends on X86_32 && EXPERIMENTAL && !X86_PAE && FUTEX
+	depends on X86_32 && EXPERIMENTAL && !X86_PAE && EVENTFD
 	select HVC_DRIVER
 	---help---
 	  This is a very simple module which allows you to run
drivers/lguest/core.c +5 -3
@@ -198,9 +198,11 @@ int run_guest(struct lg_cpu *cpu, unsign
 		/* It's possible the Guest did a NOTIFY hypercall to the
 		 * Launcher, in which case we return from the read() now. */
 		if (cpu->pending_notify) {
-			if (put_user(cpu->pending_notify, user))
-				return -EFAULT;
-			return sizeof(cpu->pending_notify);
+			if (!send_notify_to_eventfd(cpu)) {
+				if (put_user(cpu->pending_notify, user))
+					return -EFAULT;
+				return sizeof(cpu->pending_notify);
+			}
 		}
 
 		/* Check for signals */
drivers/lguest/lg.h +9 -0
@@ -82,6 +82,11 @@ struct lg_cpu {
 	struct lg_cpu_arch arch;
 };
 
+struct lg_eventfds {
+	unsigned long addr;
+	struct file *event;
+};
+
 /* The private info the thread maintains about the guest. */
 struct lguest
 {
@@ -102,6 +107,9 @@ struct lguest
 	unsigned int stack_pages;
 	u32 tsc_khz;
 
+	unsigned int num_eventfds;
+	struct lg_eventfds *eventfds;
+
 	/* Dead? */
 	const char *dead;
 };
@@ -152,6 +160,7 @@ void setup_default_idt_entries(struct lg
 void copy_traps(const struct lg_cpu *cpu, struct desc_struct *idt,
 		const unsigned long *def);
 void guest_set_clockevent(struct lg_cpu *cpu, unsigned long delta);
+bool send_notify_to_eventfd(struct lg_cpu *cpu);
 void init_clockdev(struct lg_cpu *cpu);
 bool check_syscall_vector(struct lguest *lg);
 int init_interrupts(void);
drivers/lguest/lguest_user.c +73 -0

97 lines not on this page · raw message

include/linux/lguest_launcher.h +1 -0
@@ -58,6 +58,7 @@ enum lguest_req
 	LHREQ_GETDMA, /* No longer used */
 	LHREQ_IRQ, /* + irq */
 	LHREQ_BREAK, /* + on/off flag (on blocks until someone does off) */
+	LHREQ_EVENTFD, /* + address, fd. */
 };
 
 /* The alignment to use between consumer and producer parts of vring.
#37 lore raw
Quoted 24 lines · Rusty Russell
Quoted 6 lines · Gregory Haskins
Quoted 2 lines · Avi Kivity

Gregory Haskins wrote:
One idea is similar to signalfd() or eventfd()

And thus the "kvm-eventfd" (irqfd/iosignalfd) interface project was born. ;)

The lguest patch queue already has such an interface :) And I have a partially complete in-kernel virtio_pci patch with the same trick.

I switched from "kernel created eventfd" to "userspace passes in eventfd" after a while though; it lets you connect multiple virtqueues to a single fd if you want.

Combined with a minor change to allow any process with access to the lguest fd to queue interrupts, this allowed lguest to move to a thread-per-virtqueue model which was a significant speedup as well as nice code reduction.

Here's the relevant kernel patch for reading.

Thanks!
Rusty.

Quoted 8 lines

lguest: use eventfds for device notification

Currently, when a Guest wants to perform I/O it calls LHCALL_NOTIFY with an address: the main Launcher process returns with this address, and figures out what device to run.

A far nicer model is to let processes bind an eventfd to an address: if we find one, we simply signal the eventfd.

A couple of (probably misguided) RCU questions/suggestions interspersed.

Quoted 85 lines
---
 drivers/lguest/Kconfig          |    2 -
 drivers/lguest/core.c           |    8 ++--
 drivers/lguest/lg.h             |    9 ++++
 drivers/lguest/lguest_user.c    |   73 ++++++++++++++++++++++++++++++++++++++++
 include/linux/lguest_launcher.h |    1 
 5 files changed, 89 insertions(+), 4 deletions(-)
drivers/lguest/Kconfig +1 -1
@@ -1,6 +1,6 @@
 config LGUEST
 	tristate "Linux hypervisor example code"
-	depends on X86_32 && EXPERIMENTAL && !X86_PAE && FUTEX
+	depends on X86_32 && EXPERIMENTAL && !X86_PAE && EVENTFD
 	select HVC_DRIVER
 	---help---
 	  This is a very simple module which allows you to run
drivers/lguest/core.c +5 -3
@@ -198,9 +198,11 @@ int run_guest(struct lg_cpu *cpu, unsign
 		/* It's possible the Guest did a NOTIFY hypercall to the
 		 * Launcher, in which case we return from the read() now. */
 		if (cpu->pending_notify) {
-			if (put_user(cpu->pending_notify, user))
-				return -EFAULT;
-			return sizeof(cpu->pending_notify);
+			if (!send_notify_to_eventfd(cpu)) {
+				if (put_user(cpu->pending_notify, user))
+					return -EFAULT;
+				return sizeof(cpu->pending_notify);
+			}
 		}
 
 		/* Check for signals */
drivers/lguest/lg.h +9 -0
@@ -82,6 +82,11 @@ struct lg_cpu {
 	struct lg_cpu_arch arch;
 };
 
+struct lg_eventfds {
+	unsigned long addr;
+	struct file *event;
+};
+
 /* The private info the thread maintains about the guest. */
 struct lguest
 {
@@ -102,6 +107,9 @@ struct lguest
 	unsigned int stack_pages;
 	u32 tsc_khz;
 
+	unsigned int num_eventfds;
+	struct lg_eventfds *eventfds;
+
 	/* Dead? */
 	const char *dead;
 };
@@ -152,6 +160,7 @@ void setup_default_idt_entries(struct lg
 void copy_traps(const struct lg_cpu *cpu, struct desc_struct *idt,
 		const unsigned long *def);
 void guest_set_clockevent(struct lg_cpu *cpu, unsigned long delta);
+bool send_notify_to_eventfd(struct lg_cpu *cpu);
 void init_clockdev(struct lg_cpu *cpu);
 bool check_syscall_vector(struct lguest *lg);
 int init_interrupts(void);
drivers/lguest/lguest_user.c +2 -0
@@ -7,6 +7,8 @@
 #include <linux/miscdevice.h>
 #include <linux/fs.h>
 #include <linux/sched.h>
+#include <linux/eventfd.h>
+#include <linux/file.h>
 #include "lg.h"
 
 /*L:055 When something happens, the Waker process needs a way to stop the

drivers/lguest/lguest_user.c static int break_guest_out(struct lg_cpu · line 45

Quoted 9 lines
drivers/lguest/lguest_user.c +6 -0
@@ -35,6 +37,70 @@ static int break_guest_out(struct lg_cpu
 	}
 }
 
+bool send_notify_to_eventfd(struct lg_cpu *cpu)
+{
+	unsigned int i;
+
+	/* lg->eventfds is RCU-protected */
+	preempt_disable();

Suggest changing to rcu_read_lock() to match the synchronize_rcu().

drivers/lguest/lguest_user.c static int break_guest_out(struct lg_cpu · line 48

Quoted 3 lines
drivers/lguest/lguest_user.c +3 -0
+	for (i = 0; i < cpu->lg->num_eventfds; i++) {
+		if (cpu->lg->eventfds[i].addr == cpu->pending_notify) {
+			eventfd_signal(cpu->lg->eventfds[i].event, 1);

Shouldn't this be something like the following?

		p = rcu_dereference(cpu->lg->eventfds);
		if (p[i].addr == cpu->pending_notify) {
			eventfd_signal(p[i].event, 1);

drivers/lguest/lguest_user.c static int break_guest_out(struct lg_cpu · line 53

Quoted 5 lines
drivers/lguest/lguest_user.c +5 -0
+			cpu->pending_notify = 0;
+			break;
+		}
+	}
+	preempt_enable();

And of course, rcu_read_unlock() here.

Quoted 19 lines
drivers/lguest/lguest_user.c +19 -0
+	return cpu->pending_notify == 0;
+}
+
+static int add_eventfd(struct lguest *lg, unsigned long addr, int fd)
+{
+	struct lg_eventfds *new, *old;
+
+	if (!addr)
+		return -EINVAL;
+
+	/* Replace the old array with the new one, carefully: others can
+	 * be accessing it at the same time */
+	new = kmalloc(sizeof(*new) * (lg->num_eventfds + 1), GFP_KERNEL);
+	if (!new)
+		return -ENOMEM;
+
+	memcpy(new, lg->eventfds, sizeof(*new) * lg->num_eventfds);
+	old = lg->eventfds;
+	lg->eventfds = new;

drivers/lguest/lguest_user.c static int break_guest_out(struct lg_cpu · line 82

Quoted 10 lines
drivers/lguest/lguest_user.c +10 -0
+	synchronize_rcu();
+	kfree(old);
+
+	lg->eventfds[lg->num_eventfds].addr = addr;
+	lg->eventfds[lg->num_eventfds].event = eventfd_fget(fd);
+	if (IS_ERR(lg->eventfds[lg->num_eventfds].event))
+		return PTR_ERR(lg->eventfds[lg->num_eventfds].event);
+
+	wmb();
+	lg->num_eventfds++;

Doesn't the synchronize_rcu() need to be synchronize_sched() to match the preempt_disable() in send_notify_to_eventfd()? Or, alternatively, use rcu_read_lock() instead of preempt_disable() in send_notify_to_eventfd(). This last is preferred.

Although you have the wmb() above, there is no ordering in send_notify_to_eventfd(). Would the following work?

	old = lg->eventfds;
	lg->eventfds = new;

	lg->eventfds[lg->num_eventfds].addr = addr;
	lg->eventfds[lg->num_eventfds].event = eventfd_fget(fd);
	if (IS_ERR(lg->eventfds[lg->num_eventfds].event))
		return PTR_ERR(lg->eventfds[lg->num_eventfds].event);

	synchronize_rcu();
	kfree(old);
	lg->num_eventfds++;

Here, synchronize_rcu() is doing two things:

1. ensuring that old readers who might be referencing "old" are
done before the kfree(), and

2. wait for the completion of all old readers who might (a) be
referencing the short "old" array and (b) be unaware of the initialization of the new element.

Or do we also need to wait for anyone who might still be using the old value of lg->num_eventfds? If so, the usual trick is to put this value behind the same pointer that references the array, so that any given rcu_dereference() is guaranteed to see matching array and size.

Quoted 64 lines
drivers/lguest/lguest_user.c +28 -0
+	return 0;
+}
+
+static int attach_eventfd(struct lguest *lg, const unsigned long __user *input)
+{
+	unsigned long addr, fd;
+	int err;
+
+	if (get_user(addr, input) != 0)
+		return -EFAULT;
+	input++;
+	if (get_user(fd, input) != 0)
+		return -EFAULT;
+
+	mutex_lock(&lguest_lock);
+	err = add_eventfd(lg, addr, fd);
+	mutex_unlock(&lguest_lock);
+
+	return 0;
+}
+
 /*L:050 Sending an interrupt is done by writing LHREQ_IRQ and an interrupt
  * number to /dev/lguest. */
 static int user_send_irq(struct lg_cpu *cpu, const unsigned long __user *input)
@@ -260,6 +326,8 @@ static ssize_t write(struct file *file, 
 		return user_send_irq(cpu, input);
 	case LHREQ_BREAK:
 		return break_guest_out(cpu, input);
+	case LHREQ_EVENTFD:
+		return attach_eventfd(lg, input);
 	default:
 		return -EINVAL;
 	}
@@ -297,6 +365,11 @@ static int close(struct inode *inode, st
 		 * the Launcher's memory management structure. */
 		mmput(lg->cpus[i].mm);
 	}
+
+	/* Release any eventfds they registered. */
+	for (i = 0; i < lg->num_eventfds; i++)
+		fput(lg->eventfds[i].event);
+
 	/* If lg->dead doesn't contain an error code it will be NULL or a
 	 * kmalloc()ed string, either of which is ok to hand to kfree(). */
 	if (!IS_ERR(lg->dead))
include/linux/lguest_launcher.h +1 -1
@@ -58,6 +58,7 @@ enum lguest_req
 	LHREQ_GETDMA, /* No longer used */
 	LHREQ_IRQ, /* + irq */
 	LHREQ_BREAK, /* + on/off flag (on blocks until someone does off) */
+	LHREQ_EVENTFD, /* + address, fd. */
 };
 
 /* The alignment to use between consumer and producer parts of vring.
 
 
 
--

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

#38 lore raw
Quoted 5 lines · Paul E. McKenney

Review

Quoted 2 lines · Rusty Russell
(quoted diff) +2 -0
+	/* lg->eventfds is RCU-protected */
+	preempt_disable();

Suggest changing to rcu_read_lock() to match the synchronize_rcu().

Ah yes, much better. As I was implementing it I warred with myself since lguest aims for simplicity above all else. But since we only ever add things to the array, RCU probably is simpler.

Quoted 9 lines

Review

Quoted 3 lines
(quoted diff) +3 -0
+	for (i = 0; i < cpu->lg->num_eventfds; i++) {
+		if (cpu->lg->eventfds[i].addr == cpu->pending_notify) {
+			eventfd_signal(cpu->lg->eventfds[i].event, 1);

Shouldn't this be something like the following?

		p = rcu_dereference(cpu->lg->eventfds);
		if (p[i].addr == cpu->pending_notify) {
			eventfd_signal(p[i].event, 1);

Hmm, need to read num_eventfds first, too. It doesn't matter if we get the old ->num_eventfds and the new ->eventfds, but the other way around would be bad.

Here's the inter-diff:

drivers/lguest/lguest_user.c +11 -5
@@ -39,18 +39,24 @@ static int break_guest_out(struct lg_cpu
 
 bool send_notify_to_eventfd(struct lg_cpu *cpu)
 {
-	unsigned int i;
+	unsigned int i, num;
+	struct lg_eventfds *eventfds;
+
+	/* Make sure we grab the total number before accessing the array. */
+	cpu->lg->num_eventfds = num;
+	rmb();
 
 	/* lg->eventfds is RCU-protected */
 	rcu_read_lock();
-	for (i = 0; i < cpu->lg->num_eventfds; i++) {
-		if (cpu->lg->eventfds[i].addr == cpu->pending_notify) {
-			eventfd_signal(cpu->lg->eventfds[i].event, 1);
+	eventfds = rcu_dereference(cpu->lg->eventfds);
+	for (i = 0; i < num; i++) {
+		if (eventfds[i].addr == cpu->pending_notify) {
+			eventfd_signal(eventfds[i].event, 1);
 			cpu->pending_notify = 0;
 			break;
 		}
 	}
-	preempt_enable();
+	rcu_read_unlock();
 	return cpu->pending_notify == 0;
 }
 

Thanks!
Rusty.

#39 lore raw
Quoted 10 lines · Rusty Russell
Quoted 5 lines · Paul E. McKenney

Review

Quoted 2 lines · Rusty Russell
(quoted diff) +2 -0
+	/* lg->eventfds is RCU-protected */
+	preempt_disable();

Suggest changing to rcu_read_lock() to match the synchronize_rcu().

Ah yes, much better. As I was implementing it I warred with myself since lguest aims for simplicity above all else. But since we only ever add things to the array, RCU probably is simpler.

;-)

Quoted 6 lines
Quoted 6 lines

Review

Quoted 3 lines
(quoted diff) +3 -0
+	for (i = 0; i < cpu->lg->num_eventfds; i++) {
+		if (cpu->lg->eventfds[i].addr == cpu->pending_notify) {
+			eventfd_signal(cpu->lg->eventfds[i].event, 1);

Shouldn't this be something like the following?

Quoted 6 lines
Quoted 3 lines
		p = rcu_dereference(cpu->lg->eventfds);
		if (p[i].addr == cpu->pending_notify) {
			eventfd_signal(p[i].event, 1);

Hmm, need to read num_eventfds first, too. It doesn't matter if we get the old ->num_eventfds and the new ->eventfds, but the other way around would be bad.

Yep!!! ;-)

Quoted 24 lines

Here's the inter-diff:

drivers/lguest/lguest_user.c +8 -4
@@ -39,18 +39,24 @@ static int break_guest_out(struct lg_cpu
 
 bool send_notify_to_eventfd(struct lg_cpu *cpu)
 {
-	unsigned int i;
+	unsigned int i, num;
+	struct lg_eventfds *eventfds;
+
+	/* Make sure we grab the total number before accessing the array. */
+	cpu->lg->num_eventfds = num;
+	rmb();
 
 	/* lg->eventfds is RCU-protected */
 	rcu_read_lock();
-	for (i = 0; i < cpu->lg->num_eventfds; i++) {
-		if (cpu->lg->eventfds[i].addr == cpu->pending_notify) {
-			eventfd_signal(cpu->lg->eventfds[i].event, 1);
+	eventfds = rcu_dereference(cpu->lg->eventfds);
+	for (i = 0; i < num; i++) {

drivers/lguest/lguest_user.c static int break_guest_out(struct lg_cpu · line 61

Quoted 10 lines
drivers/lguest/lguest_user.c +3 -1
+		if (eventfds[i].addr == cpu->pending_notify) {
+			eventfd_signal(eventfds[i].event, 1);
 			cpu->pending_notify = 0;
 			break;
 		}
 	}
-	preempt_enable();
+	rcu_read_unlock();
 	return cpu->pending_notify == 0;
 }

It is possible to get rid of the rmb() and wmb() as well, doing something like the following:

	struct lg_eventfds_num {
		unsigned int n;
		struct lg_eventfds a[0];
	}

Then the rcu_dereference() gets you a pointer to a struct lg_eventfds_num, which has the array and its length in guaranteed synchronization without the need for barriers.

Does this work for you, or is there some complication that I am missing?

Thanx, Paul

#40 lore raw
Quoted 11 lines · Paul E. McKenney

It is possible to get rid of the rmb() and wmb() as well, doing something like the following:

	struct lg_eventfds_num {
		unsigned int n;
		struct lg_eventfds a[0];
	}

Then the rcu_dereference() gets you a pointer to a struct lg_eventfds_num, which has the array and its length in guaranteed synchronization without the need for barriers.

Yep, that's actually quite nice. The only wart is that it needs to be allocated even when n == 0, but IMHO worth it for barrier avoidance.

This is what I ended up with:

lguest: use eventfds for device notification

Currently, when a Guest wants to perform I/O it calls LHCALL_NOTIFY with an address: the main Launcher process returns with this address, and figures out what device to run.

A far nicer model is to let processes bind an eventfd to an address: if we find one, we simply signal the eventfd.

---
 drivers/lguest/Kconfig          |    2 
 drivers/lguest/core.c           |    8 ++-
 drivers/lguest/lg.h             |   13 +++++
 drivers/lguest/lguest_user.c    |   98 +++++++++++++++++++++++++++++++++++++++-
 include/linux/lguest_launcher.h |    1 
 5 files changed, 116 insertions(+), 6 deletions(-)
drivers/lguest/Kconfig +1 -1
@@ -1,6 +1,6 @@
 config LGUEST
 	tristate "Linux hypervisor example code"
-	depends on X86_32 && EXPERIMENTAL && FUTEX
+	depends on X86_32 && EXPERIMENTAL && EVENTFD
 	select HVC_DRIVER
 	---help---
 	  This is a very simple module which allows you to run
drivers/lguest/core.c +5 -3
@@ -198,9 +198,11 @@ int run_guest(struct lg_cpu *cpu, unsign
 		/* It's possible the Guest did a NOTIFY hypercall to the
 		 * Launcher, in which case we return from the read() now. */
 		if (cpu->pending_notify) {
-			if (put_user(cpu->pending_notify, user))
-				return -EFAULT;
-			return sizeof(cpu->pending_notify);
+			if (!send_notify_to_eventfd(cpu)) {
+				if (put_user(cpu->pending_notify, user))
+					return -EFAULT;
+				return sizeof(cpu->pending_notify);
+			}
 		}
 
 		/* Check for signals */
drivers/lguest/lg.h +13 -0
@@ -82,6 +82,16 @@ struct lg_cpu {
 	struct lg_cpu_arch arch;
 };
 
+struct lg_eventfd {
+	unsigned long addr;
+	struct file *event;
+};
+
+struct lg_eventfd_map {
+	unsigned int num;
+	struct lg_eventfd map[];
+};
+
 /* The private info the thread maintains about the guest. */
 struct lguest
 {
@@ -102,6 +112,8 @@ struct lguest
 	unsigned int stack_pages;
 	u32 tsc_khz;
 
+	struct lg_eventfd_map *eventfds;
+
 	/* Dead? */
 	const char *dead;
 };
@@ -154,6 +166,7 @@ void setup_default_idt_entries(struct lg
 void copy_traps(const struct lg_cpu *cpu, struct desc_struct *idt,
 		const unsigned long *def);
 void guest_set_clockevent(struct lg_cpu *cpu, unsigned long delta);
+bool send_notify_to_eventfd(struct lg_cpu *cpu);
 void init_clockdev(struct lg_cpu *cpu);
 bool check_syscall_vector(struct lguest *lg);
 int init_interrupts(void);
drivers/lguest/lguest_user.c +96 -2

140 lines not on this page · raw message

include/linux/lguest_launcher.h +1 -0
@@ -58,6 +58,7 @@ enum lguest_req
 	LHREQ_GETDMA, /* No longer used */
 	LHREQ_IRQ, /* + irq */
 	LHREQ_BREAK, /* + on/off flag (on blocks until someone does off) */
+	LHREQ_EVENTFD, /* + address, fd. */
 };
 
 /* The alignment to use between consumer and producer parts of vring.
Reviewed-by
#41 lore raw
Quoted 9 lines · Rusty Russell
Quoted 8 lines · Paul E. McKenney

It is possible to get rid of the rmb() and wmb() as well, doing something like the following:

	struct lg_eventfds_num {
		unsigned int n;
		struct lg_eventfds a[0];
	}
Quoted 6 lines
Quoted 3 lines

Then the rcu_dereference() gets you a pointer to a struct lg_eventfds_num, which has the array and its length in guaranteed synchronization without the need for barriers.

Yep, that's actually quite nice. The only wart is that it needs to be allocated even when n == 0, but IMHO worth it for barrier avoidance.

Well, I suppose that you -could- statically allocate one in struct lguest, but it is not clear to me that this cure would be better than the always-allocate disease in this case. But either way, you would be allocating an instance, so your statement above is correct. ;-)

Quoted 10 lines

This is what I ended up with:

lguest: use eventfds for device notification

Currently, when a Guest wants to perform I/O it calls LHCALL_NOTIFY with an address: the main Launcher process returns with this address, and figures out what device to run.

A far nicer model is to let processes bind an eventfd to an address: if we find one, we simply signal the eventfd.

Looks very good to me from an RCU viewpoint!!!

Quoted 238 lines
---
 drivers/lguest/Kconfig          |    2 
 drivers/lguest/core.c           |    8 ++-
 drivers/lguest/lg.h             |   13 +++++
 drivers/lguest/lguest_user.c    |   98 +++++++++++++++++++++++++++++++++++++++-
 include/linux/lguest_launcher.h |    1 
 5 files changed, 116 insertions(+), 6 deletions(-)
drivers/lguest/Kconfig +1 -1
@@ -1,6 +1,6 @@
 config LGUEST
 	tristate "Linux hypervisor example code"
-	depends on X86_32 && EXPERIMENTAL && FUTEX
+	depends on X86_32 && EXPERIMENTAL && EVENTFD
 	select HVC_DRIVER
 	---help---
 	  This is a very simple module which allows you to run
drivers/lguest/core.c +5 -3
@@ -198,9 +198,11 @@ int run_guest(struct lg_cpu *cpu, unsign
 		/* It's possible the Guest did a NOTIFY hypercall to the
 		 * Launcher, in which case we return from the read() now. */
 		if (cpu->pending_notify) {
-			if (put_user(cpu->pending_notify, user))
-				return -EFAULT;
-			return sizeof(cpu->pending_notify);
+			if (!send_notify_to_eventfd(cpu)) {
+				if (put_user(cpu->pending_notify, user))
+					return -EFAULT;
+				return sizeof(cpu->pending_notify);
+			}
 		}
 
 		/* Check for signals */
drivers/lguest/lg.h +13 -0
@@ -82,6 +82,16 @@ struct lg_cpu {
 	struct lg_cpu_arch arch;
 };
 
+struct lg_eventfd {
+	unsigned long addr;
+	struct file *event;
+};
+
+struct lg_eventfd_map {
+	unsigned int num;
+	struct lg_eventfd map[];
+};
+
 /* The private info the thread maintains about the guest. */
 struct lguest
 {
@@ -102,6 +112,8 @@ struct lguest
 	unsigned int stack_pages;
 	u32 tsc_khz;
 
+	struct lg_eventfd_map *eventfds;
+
 	/* Dead? */
 	const char *dead;
 };
@@ -154,6 +166,7 @@ void setup_default_idt_entries(struct lg
 void copy_traps(const struct lg_cpu *cpu, struct desc_struct *idt,
 		const unsigned long *def);
 void guest_set_clockevent(struct lg_cpu *cpu, unsigned long delta);
+bool send_notify_to_eventfd(struct lg_cpu *cpu);
 void init_clockdev(struct lg_cpu *cpu);
 bool check_syscall_vector(struct lguest *lg);
 int init_interrupts(void);
drivers/lguest/lguest_user.c +96 -2
@@ -7,6 +7,8 @@
 #include <linux/miscdevice.h>
 #include <linux/fs.h>
 #include <linux/sched.h>
+#include <linux/eventfd.h>
+#include <linux/file.h>
 #include "lg.h"
 
 /*L:055 When something happens, the Waker process needs a way to stop the
@@ -35,6 +37,81 @@ static int break_guest_out(struct lg_cpu
 	}
 }
 
+bool send_notify_to_eventfd(struct lg_cpu *cpu)
+{
+	unsigned int i;
+	struct lg_eventfd_map *map;
+
+	/* lg->eventfds is RCU-protected */
+	rcu_read_lock();
+	map = rcu_dereference(cpu->lg->eventfds);
+	for (i = 0; i < map->num; i++) {
+		if (map->map[i].addr == cpu->pending_notify) {
+			eventfd_signal(map->map[i].event, 1);
+			cpu->pending_notify = 0;
+			break;
+		}
+	}
+	rcu_read_unlock();
+	return cpu->pending_notify == 0;
+}
+
+static int add_eventfd(struct lguest *lg, unsigned long addr, int fd)
+{
+	struct lg_eventfd_map *new, *old = lg->eventfds;
+
+	if (!addr)
+		return -EINVAL;
+
+	/* Replace the old array with the new one, carefully: others can
+	 * be accessing it at the same time */
+	new = kmalloc(sizeof(*new) + sizeof(new->map[0]) * (old->num + 1),
+		      GFP_KERNEL);
+	if (!new)
+		return -ENOMEM;
+
+	/* First make identical copy. */
+	memcpy(new->map, old->map, sizeof(old->map[0]) * old->num);
+	new->num = old->num;
+
+	/* Now append new entry. */
+	new->map[new->num].addr = addr;
+	new->map[new->num].event = eventfd_fget(fd);
+	if (IS_ERR(new->map[new->num].event)) {
+		kfree(new);
+		return PTR_ERR(new->map[new->num].event);
+	}
+	new->num++;
+
+	/* Now put new one in place. */
+	rcu_assign_pointer(lg->eventfds, new);
+
+	/* We're not in a big hurry.  Wait until noone's looking at old
+	 * version, then delete it. */
+	synchronize_rcu();
+	kfree(old);
+
+	return 0;
+}
+
+static int attach_eventfd(struct lguest *lg, const unsigned long __user *input)
+{
+	unsigned long addr, fd;
+	int err;
+
+	if (get_user(addr, input) != 0)
+		return -EFAULT;
+	input++;
+	if (get_user(fd, input) != 0)
+		return -EFAULT;
+
+	mutex_lock(&lguest_lock);
+	err = add_eventfd(lg, addr, fd);
+	mutex_unlock(&lguest_lock);
+
+	return 0;
+}
+
 /*L:050 Sending an interrupt is done by writing LHREQ_IRQ and an interrupt
  * number to /dev/lguest. */
 static int user_send_irq(struct lg_cpu *cpu, const unsigned long __user *input)
@@ -184,6 +261,13 @@ static int initialize(struct file *file,
 		goto unlock;
 	}
 
+	lg->eventfds = kmalloc(sizeof(*lg->eventfds), GFP_KERNEL);
+	if (!lg->eventfds) {
+		err = -ENOMEM;
+		goto free_lg;
+	}
+	lg->eventfds->num = 0;
+
 	/* Populate the easy fields of our "struct lguest" */
 	lg->mem_base = (void __user *)args[0];
 	lg->pfn_limit = args[1];
@@ -191,7 +275,7 @@ static int initialize(struct file *file,
 	/* This is the first cpu (cpu 0) and it will start booting at args[2] */
 	err = lg_cpu_start(&lg->cpus[0], 0, args[2]);
 	if (err)
-		goto release_guest;
+		goto free_eventfds;
 
 	/* Initialize the Guest's shadow page tables, using the toplevel
 	 * address the Launcher gave us.  This allocates memory, so can fail. */
@@ -210,7 +294,9 @@ static int initialize(struct file *file,
 free_regs:
 	/* FIXME: This should be in free_vcpu */
 	free_page(lg->cpus[0].regs_page);
-release_guest:
+free_eventfds:
+	kfree(lg->eventfds);
+free_lg:
 	kfree(lg);
 unlock:
 	mutex_unlock(&lguest_lock);
@@ -260,6 +346,8 @@ static ssize_t write(struct file *file, 
 		return user_send_irq(cpu, input);
 	case LHREQ_BREAK:
 		return break_guest_out(cpu, input);
+	case LHREQ_EVENTFD:
+		return attach_eventfd(lg, input);
 	default:
 		return -EINVAL;
 	}
@@ -297,6 +385,12 @@ static int close(struct inode *inode, st
 		 * the Launcher's memory management structure. */
 		mmput(lg->cpus[i].mm);
 	}
+
+	/* Release any eventfds they registered. */
+	for (i = 0; i < lg->eventfds->num; i++)
+		fput(lg->eventfds->map[i].event);
+	kfree(lg->eventfds);
+
 	/* If lg->dead doesn't contain an error code it will be NULL or a
 	 * kmalloc()ed string, either of which is ok to hand to kfree(). */
 	if (!IS_ERR(lg->dead))
include/linux/lguest_launcher.h +1 -0
@@ -58,6 +58,7 @@ enum lguest_req
 	LHREQ_GETDMA, /* No longer used */
 	LHREQ_IRQ, /* + irq */
 	LHREQ_BREAK, /* + on/off flag (on blocks until someone does off) */
+	LHREQ_EVENTFD, /* + address, fd. */
 };
 
 /* The alignment to use between consumer and producer parts of vring.
#42 lore raw
In reply to Rusty Russell

Hi Rusty,

Quoted 4 lines · Rusty Russell
Quoted 2 lines · Gregory Haskins

Avi Kivity wrote:

Quoted 8 lines
Quoted 6 lines
Quoted 3 lines

Gregory Haskins wrote:
One idea is similar to signalfd() or eventfd()

And thus the "kvm-eventfd" (irqfd/iosignalfd) interface project was born. ;)

The lguest patch queue already has such an interface :)

Cool! Ultimately I think it will be easier if both lguest+kvm support the same eventfd notion so this is good you are already moving in the same direction.

Quoted 2 lines

And I have a partially complete in-kernel virtio_pci patch with the same trick.

I thought lguest didn't use pci? Or do you just mean that you have an in-kernel virtio-net for lguest?

As a follow up question, I wonder if we can easily port that to vbus so that it will work in both lguest and kvm? (note to self: push a skeleton example today)

Quoted 4 lines

I switched from "kernel created eventfd" to "userspace passes in eventfd" after a while though; it lets you connect multiple virtqueues to a single fd if you want.

Yeah, actually we switched that that model, too. Aside from the limitation you point out, there were some problems that Al Viro had raised trying to do it in kernel w.r.t. fd abuse.

Quoted 4 lines

Combined with a minor change to allow any process with access to the lguest fd to queue interrupts, this allowed lguest to move to a thread-per-virtqueue model which was a significant speedup as well as nice code reduction.

Yep, that was one of my findings on venet as well so I was looking forward to trying to get virtio-net to do the same.

Quoted 2 lines

Here's the relevant kernel patch for reading.

Thanks Rusty! Will take a look.

Quoted 26 lines

Thanks!
Rusty.

lguest: use eventfds for device notification

Currently, when a Guest wants to perform I/O it calls LHCALL_NOTIFY with an address: the main Launcher process returns with this address, and figures out what device to run.

A far nicer model is to let processes bind an eventfd to an address: if we find one, we simply signal the eventfd.

---
 drivers/lguest/Kconfig          |    2 -
 drivers/lguest/core.c           |    8 ++--
 drivers/lguest/lg.h             |    9 ++++
 drivers/lguest/lguest_user.c    |   73 ++++++++++++++++++++++++++++++++++++++++
 include/linux/lguest_launcher.h |    1 
 5 files changed, 89 insertions(+), 4 deletions(-)

drivers/lguest/Kconfig line 4

Quoted 5 lines
drivers/lguest/Kconfig +1 -1
@@ -1,6 +1,6 @@
 config LGUEST
 	tristate "Linux hypervisor example code"
-	depends on X86_32 && EXPERIMENTAL && !X86_PAE && FUTEX
+	depends on X86_32 && EXPERIMENTAL && !X86_PAE && EVENTFD
 

Note to self: we probably need a similar line in KVM now.

Quoted 166 lines
drivers/lguest/Kconfig +0 -0
 	select HVC_DRIVER
 	---help---
drivers/lguest/Kconfig +0 -0
 	  This is a very simple module which allows you to run
drivers/lguest/core.c +5 -3
@@ -198,9 +198,11 @@ int run_guest(struct lg_cpu *cpu, unsign
 		/* It's possible the Guest did a NOTIFY hypercall to the
 		 * Launcher, in which case we return from the read() now. */
 		if (cpu->pending_notify) {
-			if (put_user(cpu->pending_notify, user))
-				return -EFAULT;
-			return sizeof(cpu->pending_notify);
+			if (!send_notify_to_eventfd(cpu)) {
+				if (put_user(cpu->pending_notify, user))
+					return -EFAULT;
+				return sizeof(cpu->pending_notify);
+			}
 		}
 
 		/* Check for signals */
drivers/lguest/lg.h +9 -0
@@ -82,6 +82,11 @@ struct lg_cpu {
 	struct lg_cpu_arch arch;
 };
 
+struct lg_eventfds {
+	unsigned long addr;
+	struct file *event;
+};
+
 /* The private info the thread maintains about the guest. */
 struct lguest
 {
@@ -102,6 +107,9 @@ struct lguest
 	unsigned int stack_pages;
 	u32 tsc_khz;
 
+	unsigned int num_eventfds;
+	struct lg_eventfds *eventfds;
+
 	/* Dead? */
 	const char *dead;
 };
@@ -152,6 +160,7 @@ void setup_default_idt_entries(struct lg
 void copy_traps(const struct lg_cpu *cpu, struct desc_struct *idt,
 		const unsigned long *def);
 void guest_set_clockevent(struct lg_cpu *cpu, unsigned long delta);
+bool send_notify_to_eventfd(struct lg_cpu *cpu);
 void init_clockdev(struct lg_cpu *cpu);
 bool check_syscall_vector(struct lguest *lg);
 int init_interrupts(void);
drivers/lguest/lguest_user.c +73 -0
@@ -7,6 +7,8 @@
 #include <linux/miscdevice.h>
 #include <linux/fs.h>
 #include <linux/sched.h>
+#include <linux/eventfd.h>
+#include <linux/file.h>
 #include "lg.h"
 
 /*L:055 When something happens, the Waker process needs a way to stop the
@@ -35,6 +37,70 @@ static int break_guest_out(struct lg_cpu
 	}
 }
 
+bool send_notify_to_eventfd(struct lg_cpu *cpu)
+{
+	unsigned int i;
+
+	/* lg->eventfds is RCU-protected */
+	preempt_disable();
+	for (i = 0; i < cpu->lg->num_eventfds; i++) {
+		if (cpu->lg->eventfds[i].addr == cpu->pending_notify) {
+			eventfd_signal(cpu->lg->eventfds[i].event, 1);
+			cpu->pending_notify = 0;
+			break;
+		}
+	}
+	preempt_enable();
+	return cpu->pending_notify == 0;
+}
+
+static int add_eventfd(struct lguest *lg, unsigned long addr, int fd)
+{
+	struct lg_eventfds *new, *old;
+
+	if (!addr)
+		return -EINVAL;
+
+	/* Replace the old array with the new one, carefully: others can
+	 * be accessing it at the same time */
+	new = kmalloc(sizeof(*new) * (lg->num_eventfds + 1), GFP_KERNEL);
+	if (!new)
+		return -ENOMEM;
+
+	memcpy(new, lg->eventfds, sizeof(*new) * lg->num_eventfds);
+	old = lg->eventfds;
+	lg->eventfds = new;
+	synchronize_rcu();
+	kfree(old);
+
+	lg->eventfds[lg->num_eventfds].addr = addr;
+	lg->eventfds[lg->num_eventfds].event = eventfd_fget(fd);
+	if (IS_ERR(lg->eventfds[lg->num_eventfds].event))
+		return PTR_ERR(lg->eventfds[lg->num_eventfds].event);
+
+	wmb();
+	lg->num_eventfds++;
+	return 0;
+}
+
+static int attach_eventfd(struct lguest *lg, const unsigned long __user *input)
+{
+	unsigned long addr, fd;
+	int err;
+
+	if (get_user(addr, input) != 0)
+		return -EFAULT;
+	input++;
+	if (get_user(fd, input) != 0)
+		return -EFAULT;
+
+	mutex_lock(&lguest_lock);
+	err = add_eventfd(lg, addr, fd);
+	mutex_unlock(&lguest_lock);
+
+	return 0;
+}
+
 /*L:050 Sending an interrupt is done by writing LHREQ_IRQ and an interrupt
  * number to /dev/lguest. */
 static int user_send_irq(struct lg_cpu *cpu, const unsigned long __user *input)
@@ -260,6 +326,8 @@ static ssize_t write(struct file *file, 
 		return user_send_irq(cpu, input);
 	case LHREQ_BREAK:
 		return break_guest_out(cpu, input);
+	case LHREQ_EVENTFD:
+		return attach_eventfd(lg, input);
 	default:
 		return -EINVAL;
 	}
@@ -297,6 +365,11 @@ static int close(struct inode *inode, st
 		 * the Launcher's memory management structure. */
 		mmput(lg->cpus[i].mm);
 	}
+
+	/* Release any eventfds they registered. */
+	for (i = 0; i < lg->num_eventfds; i++)
+		fput(lg->eventfds[i].event);
+
 	/* If lg->dead doesn't contain an error code it will be NULL or a
 	 * kmalloc()ed string, either of which is ok to hand to kfree(). */
 	if (!IS_ERR(lg->dead))

include/linux/lguest_launcher.h enum lguest_req · line 64

Quoted 7 lines
include/linux/lguest_launcher.h +1 -0
@@ -58,6 +58,7 @@ enum lguest_req
 	LHREQ_GETDMA, /* No longer used */
 	LHREQ_IRQ, /* + irq */
 	LHREQ_BREAK, /* + on/off flag (on blocks until someone does off) */
+	LHREQ_EVENTFD, /* + address, fd. */
 };
 
 /* The alignment to use between consumer and producer parts of vring.

Other than the potential rcu issues that Paul already addressed, looks good. FWIW: this looks like what we are calling "iosignalfd" on the kvm land (unless I am misunderstanding). Do you have the equivalent of "irqfd" going the other way?

Thanks Rusty,
-Greg

#43 lore raw
Quoted 11 lines · Gregory Haskins

drivers/lguest/Kconfig line 5

Quoted 7 lines
drivers/lguest/Kconfig +1 -1
@@ -1,6 +1,6 @@
 config LGUEST
 	tristate "Linux hypervisor example code"
-	depends on X86_32 && EXPERIMENTAL && !X86_PAE && FUTEX
+	depends on X86_32 && EXPERIMENTAL && !X86_PAE && EVENTFD
 
 

Note to self: we probably need a similar line in KVM now.

'select EVENTFD' is more appropriate.

Signature · 2 lines
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.
#44 lore raw
Quoted 12 lines · Avi Kivity
Quoted 10 lines · Gregory Haskins

drivers/lguest/Kconfig line 4

Quoted 6 lines
drivers/lguest/Kconfig +1 -1
@@ -1,6 +1,6 @@
 config LGUEST
     tristate "Linux hypervisor example code"
-    depends on X86_32 && EXPERIMENTAL && !X86_PAE && FUTEX
+    depends on X86_32 && EXPERIMENTAL && !X86_PAE && EVENTFD
 

Note to self: we probably need a similar line in KVM now.

Quoted 3 lines

'select EVENTFD' is more appropriate.

Yeah, I was thinking the same...

#45 lore raw

PATCH 1/1 kvm: make sure we select EVENTFD for any arch that declares HAVE_KVM_EVENTFD

http://lkml.org/lkml/2009/6/5/277

(It is unclear what action, if any, we also may need to address kvm-kmod
for kernels prior to eventfd support (2.6.22ish))

---

 virt/kvm/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
virt/kvm/Kconfig +1 -0
index b166f30..56c6848 100644
@@ -8,3 +8,4 @@ config HAVE_KVM_IRQCHIP
 
 config HAVE_KVM_EVENTFD
        bool
+       select EVENTFD
Applied
#46 lore raw

Re: [PATCH] kvm: make sure we select EVENTFD for any arch that declares HAVE_KVM_EVENTFD

Quoted 5 lines · Gregory Haskins

http://lkml.org/lkml/2009/6/5/277

(It is unclear what action, if any, we also may need to address kvm-kmod
for kernels prior to eventfd support (2.6.22ish))

Applied, thanks.

Signature · 1 line
error compiling committee.c: too many arguments to function
#47 lore raw
In reply to Gregory Haskins
Quoted 8 lines · Gregory Haskins

Hi Rusty,

Quoted 5 lines · Rusty Russell
Quoted 4 lines · Gregory Haskins
Quoted 2 lines · Avi Kivity

Gregory Haskins wrote:
One idea is similar to signalfd() or eventfd()

Quoted 8 lines
Quoted 4 lines
Quoted 2 lines

And thus the "kvm-eventfd" (irqfd/iosignalfd) interface project was born. ;)

The lguest patch queue already has such an interface :)

Cool! Ultimately I think it will be easier if both lguest+kvm support the same eventfd notion so this is good you are already moving in the same direction.

Not really; lguest doesn't do PCI.

Quoted 5 lines
Quoted 2 lines

And I have a partially complete in-kernel virtio_pci patch with the same trick.

I thought lguest didn't use pci? Or do you just mean that you have an in-kernel virtio-net for lguest?

No, this was for kvm. Sorry for the confusion.

Quoted 4 lines

Other than the potential rcu issues that Paul already addressed, looks good. FWIW: this looks like what we are calling "iosignalfd" on the kvm land (unless I am misunderstanding). Do you have the equivalent of "irqfd" going the other way?

Yes; lguest uses write() (offset indicates cpu #) rather than ioctls, but anyone can do the LHREQ_IRQ write to queue an interrupt for delivery.

So the threads just get the same /dev/lguest fd and it's simple.

Thanks!
Rusty.

#48 lore raw
Quoted 16 lines · Rusty Russell
Quoted 14 lines · Gregory Haskins

Hi Rusty,

Quoted 10 lines · Rusty Russell
Quoted 8 lines · Gregory Haskins
Quoted 3 lines · Avi Kivity

Gregory Haskins wrote:
One idea is similar to signalfd() or eventfd()

And thus the "kvm-eventfd" (irqfd/iosignalfd) interface project was born. ;)

Quoted 9 lines
Quoted 6 lines
Quoted 2 lines

The lguest patch queue already has such an interface :)

Cool! Ultimately I think it will be easier if both lguest+kvm support the same eventfd notion so this is good you are already moving in the same direction.

Not really; lguest doesn't do PCI.

Thats ok. I see these eventfd interfaces as somewhat orthogonal to PCI. I.e. if both lguest and kvm have an eventfd mechnism for signaling in both directions (e.g. interrupts and io), it would make it easier to support the kind of thing I am striving for with a unified backend. That is: one in-kernel virtio-net that works in both (or even many) HV environments. I see that as a higher layer abstraction than PCI, per se.

Quoted 10 lines
Quoted 7 lines
Quoted 4 lines

And I have a partially complete in-kernel virtio_pci patch with the same trick.

I thought lguest didn't use pci? Or do you just mean that you have an in-kernel virtio-net for lguest?

No, this was for kvm. Sorry for the confusion.

Ah, sorry. Well, if its in any kind of shape to see the light of day, please forward it over. Perhaps Michael and I can craft it into a working solution.

Quoted 7 lines
Quoted 6 lines

Other than the potential rcu issues that Paul already addressed, looks good. FWIW: this looks like what we are calling "iosignalfd" on the kvm land (unless I am misunderstanding). Do you have the equivalent of "irqfd" going the other way?

Quoted 5 lines

Yes; lguest uses write() (offset indicates cpu #) rather than ioctls, but anyone can do the LHREQ_IRQ write to queue an interrupt for delivery.

So the threads just get the same /dev/lguest fd and it's simple.

Ah, ok. Thats workable, too. (This kind of detail would be buried in the "lguest connector" for vbus anyway, so it doesn't have to have a uniform "eventfd_signal()" interface to work. The fd concept alone is sufficiently flexible).

Thanks Rusty,
-Greg