Messages in this thread | | | Date | Thu, 18 Jun 2009 14:45:35 +0300 | From | Avi Kivity <> | Subject | Re: [KVM PATCH v7 2/2] KVM: add iosignalfd support |
| |
On 06/16/2009 04:42 PM, Gregory Haskins wrote: > iosignalfd is a mechanism to register PIO/MMIO regions to trigger an eventfd > signal when written to by a guest. Host userspace can register any arbitrary > IO address with a corresponding eventfd and then pass the eventfd to a > specific end-point of interest for handling. > > Normal IO requires a blocking round-trip since the operation may cause > side-effects in the emulated model or may return data to the caller. > Therefore, an IO in KVM traps from the guest to the host, causes a VMX/SVM > "heavy-weight" exit back to userspace, and is ultimately serviced by qemu's > device model synchronously before returning control back to the vcpu. > > However, there is a subclass of IO which acts purely as a trigger for > other IO (such as to kick off an out-of-band DMA request, etc). For these > patterns, the synchronous call is particularly expensive since we really > only want to simply get our notification transmitted asychronously and > return as quickly as possible. All the sychronous infrastructure to ensure > proper data-dependencies are met in the normal IO case are just unecessary > overhead for signalling. This adds additional computational load on the > system, as well as latency to the signalling path. > > Therefore, we provide a mechanism for registration of an in-kernel trigger > point that allows the VCPU to only require a very brief, lightweight > exit just long enough to signal an eventfd. This also means that any > clients compatible with the eventfd interface (which includes userspace > and kernelspace equally well) can now register to be notified. The end > result should be a more flexible and higher performance notification API > for the backend KVM hypervisor and perhipheral components. > > To test this theory, we built a test-harness called "doorbell". This > module has a function called "doorbell_ring()" which simply increments a > counter for each time the doorbell is signaled. It supports signalling > from either an eventfd, or an ioctl(). > > We then wired up two paths to the doorbell: One via QEMU via a registered > io region and through the doorbell ioctl(). The other is direct via > iosignalfd. > > You can download this test harness here: > > ftp://ftp.novell.com/dev/ghaskins/doorbell.tar.bz2 > > The measured results are as follows: > > qemu-mmio: 110000 iops, 9.09us rtt > iosignalfd-mmio: 200100 iops, 5.00us rtt > iosignalfd-pio: 367300 iops, 2.72us rtt > > I didn't measure qemu-pio, because I have to figure out how to register a > PIO region with qemu's device model, and I got lazy. However, for now we > can extrapolate based on the data from the NULLIO runs of +2.56us for MMIO, > and -350ns for HC, we get: > > qemu-pio: 153139 iops, 6.53us rtt > iosignalfd-hc: 412585 iops, 2.37us rtt > > these are just for fun, for now, until I can gather more data. > > Here is a graph for your convenience: > > http://developer.novell.com/wiki/images/7/76/Iofd-chart.png > > The conclusion to draw is that we save about 4us by skipping the userspace > hop. > > > +config KVM_MAX_IOSIGNALFD_ITEMS > + int "Maximum IOSIGNALFD items per address" > + depends on KVM > + default "32" > + ---help--- > + This option influences the maximum number of fd's per PIO/MMIO > + address that are allowed to register > + >
Is there a per-vm limit on iosignalfds? if not, userspace can exhaust kernel memory in that way.
We could limit the just total number of iosignafds, it's somewhat more natural. > diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig > index daece36..a4b427f 100644 > --- a/virt/kvm/Kconfig > +++ b/virt/kvm/Kconfig > @@ -12,3 +12,5 @@ config HAVE_KVM_EVENTFD > > config KVM_APIC_ARCHITECTURE > bool > + > + >
Spurious, please drop. > +/* > + * Design note: We create one PIO/MMIO device (iosignalfd_group) which > + * aggregates one or more iosignalfd_items. Each item points to exactly one > + * eventfd, and can be registered to trigger on any write to the group > + * (wildcard), or to a write of a specific value. If more than one item is to > + * be supported, the addr/len ranges must all be identical in the group. If a > + * trigger value is to be supported on a particular item, the group range must > + * be exactly the width of the trigger. > + */ > + > +struct _iosignalfd_item { > + struct list_head list; > + struct file *file; > + unsigned char *match; > + struct rcu_head rcu; > +}; >
Why not u64 match?
> +static int > +iosignalfd_is_match(struct _iosignalfd_group *group, > + struct _iosignalfd_item *item, > + const void *val, > + int len) > +{ > + if (!item->match) > + /* wildcard is a hit */ > + return true; > + > + if (len != group->length) > + /* mis-matched length is a miss */ > + return false; >
Should check length before match (i.e. require correctly sized access).
-- error compiling committee.c: too many arguments to function
| |