Re: [PATCH] ioeventfd: Introduce KVM_IOEVENTFD_FLAG_PIPE

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/03/2011 08:04 PM, Sasha Levin wrote:
The new flag allows passing a write side of a pipe instead of an
eventfd to be notified of writes to the specified memory region.

Instead of signaling an event, the value written to the memory region
is written to the pipe.

Using a pipe instead of an eventfd is usefull when any value can be
written to the memory region but we're interested in recieving the
actual value instead of just a notification.

A simple example for practical use is the serial port. we are not
interested in an exit every time a char is written to the port, but
we do need to know what was written so we could handle it on the guest.

---
  include/linux/kvm.h |    2 +
  virt/kvm/eventfd.c  |   65 +++++++++++++++++++++++++++++++++++---------------

Documentation/virtua/kvm/api.txt +++++++++++++++++

@@ -424,6 +425,7 @@ struct _ioeventfd {
  	struct list_head     list;
  	u64                  addr;
  	int                  length;
+	struct file         *pipe;
  	struct eventfd_ctx  *eventfd;

In a union with eventfd please.

@@ -481,6 +487,21 @@ ioeventfd_in_range(struct _ioeventfd *p, gpa_t addr, int len, const void *val)
  	return _val == p->datamatch ? true : false;
  }

+static ssize_t kernel_write(struct file *file, const char *buf, size_t count,
+			    loff_t pos)
+{
+	mm_segment_t old_fs;
+	ssize_t res;
+
+	old_fs = get_fs();
+	set_fs(get_ds());
+	/* The cast to a user pointer is valid due to the set_fs() */
+	res = vfs_write(file, (const char __user *)buf, count,&pos);
+	set_fs(old_fs);
+
+	return res;
+}
+

Is there no generic helper for this?  Should there be?

  /* MMIO/PIO writes trigger an event if the addr/val match */
  static int
  ioeventfd_write(struct kvm_io_device *this, gpa_t addr, int len,
@@ -491,7 +512,11 @@ ioeventfd_write(struct kvm_io_device *this, gpa_t addr, int len,
  	if (!ioeventfd_in_range(p, addr, len, val))
  		return -EOPNOTSUPP;

-	eventfd_signal(p->eventfd, 1);
+	if (p->pipe)
+		kernel_write(p->pipe, val, len, 0);

You're writing potentially variable length data.

We need a protocol containing address, data, length, and supporting read accesses as well.

Is the write guaranteed atomic?  We probably need serialization here.

+	else
+		eventfd_signal(p->eventfd, 1);
+
  	return 0;
  }

@@ -555,9 +580,11 @@ kvm_assign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
  	if (args->flags&  ~KVM_IOEVENTFD_VALID_FLAG_MASK)
  		return -EINVAL;

-	eventfd = eventfd_ctx_fdget(args->fd);
-	if (IS_ERR(eventfd))
-		return PTR_ERR(eventfd);
+	if (!(args->flags&  KVM_IOEVENTFD_FLAG_PIPE)) {
+		eventfd = eventfd_ctx_fdget(args->fd);
+		if (IS_ERR(eventfd))
+			return PTR_ERR(eventfd);
+	}

  	p = kzalloc(sizeof(*p), GFP_KERNEL);
  	if (!p) {
@@ -568,7 +595,11 @@ kvm_assign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
  	INIT_LIST_HEAD(&p->list);
  	p->addr    = args->addr;
  	p->length  = args->len;
-	p->eventfd = eventfd;
+
+	if (args->flags&  KVM_IOEVENTFD_FLAG_PIPE)
+		p->pipe = fget(args->fd);
+	else
+		p->eventfd = eventfd;

The split logic with the previous hunk isn't nice. Suggest moving the 'else' there, and assigning the whole union here.

  	list_for_each_entry_safe(p, tmp,&kvm->ioeventfds, list) {
  		bool wildcard = !(args->flags&  KVM_IOEVENTFD_FLAG_DATAMATCH);

-		if (p->eventfd != eventfd  ||
-		    p->addr != args->addr  ||
+		if (p->addr != args->addr  ||
  		    p->length != args->len ||
  		    p->wildcard != wildcard)
  			continue;

Why?


--
error compiling committee.c: too many arguments to function

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux