Re: [KVM PATCH v7 2/2] KVM: add iosignalfd support

From: Avi Kivity
Date: Thu Jun 18 2009 - 08:21:38 EST

On 06/18/2009 03:09 PM, Gregory Haskins wrote:
+ 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.

Yeah, its already naturally limited by the maximum number of MMIO/PIO
devices we can register (today this is 6 per VM). I should have
documented that fact somewhere, tho.

We need to raise this limit drastically and to expose it. I suggest counting an all iosignalfd_items as part of the iodevice limit, so we don't have a bunch of little limits which no one understands.

+struct _iosignalfd_item {
+ struct list_head list;
+ struct file *file;
+ unsigned char *match;
+ struct rcu_head rcu;

Why not u64 match?

Well, tbh it was primarily because it was starting to make my head hurt
w.r.t. endianness ;). For instance, if someone wanted a u16 match, I
would presumably have to understand the relevant endianess of the u64 so
I compare the appropriate bytes against the data-register coming in from
the [MM|P]IO. Using a pointer, I simply copy/memcmp the specified
number of bytes and never have to worry about endianness.

No, a u16 will naturally expand to a u64, and the emulator will generate the correct value. As long as we don't allow mismatched access sizes, we should be fine.

As a minor bonus, item->match == NULL tells me its a wildcard. If I had
item->match as a u64, I'd need a different state flag for "wildcard".
NBD, but thought I would point it out.

True, a pointer also supplies extra information. But until we get garbage collection as part of the Java rewrite, resource management is a pain and I prefer as few pointers as possible.

+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).

Perhaps, but my thinking is that group->length only matters for
data-matching. You could conceivably have a larger window registered if
you are using all wildcards. Not sure if this is really useful, but its
the reason the code is that way today.

My thinking is to have the code behave the same way. If you require matching lengths on data match, require it on wildcard as well.

error compiling committee.c: too many arguments to function

the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at