kvm tools: virtio-mmio: init_ioeventfd should use MMIO for ioeventfd__add_event()

Commit Message

Will Deacon Sept. 4, 2013, 10:07 a.m. UTC

On Wed, Sep 04, 2013 at 10:21:26AM +0100, Pekka Enberg wrote:
> On 8/30/13 4:58 PM, Ying-Shiuan Pan wrote:
> > From: Ying-Shiuan Pan <[e-mail]>
> >
> > This patch fixes a bug that vtirtio_mmio_init_ioeventfd() passed a wrong
> > value when it invoked ioeventfd__add_event(). True value of 2nd parameter
> > indicates the eventfd uses PIO bus which is used by virito-pci, however,
> > for virtio-mmio, the value should be false.
> >
> > Signed-off-by: Ying-Shiuan Pan <[e-mail]>
> 
> Will, Marc? It would probably be good to change the two boolean
> arguments into one "flags" argument to avoid future bugs.

Like this? It gets a bit confusing, because there is a KVM_IOEVENTFD_FLAG_*
namespace as part of the kernel KVM API, but which doesn't have the flags we
need (e.g. userspace polling).

Will

--->8

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

Comments

Pekka Enberg Sept. 4, 2013, 10:13 a.m. UTC | #1

On Wed, Sep 4, 2013 at 1:07 PM, Will Deacon <[e-mail]> wrote:
> Like this? It gets a bit confusing, because there is a KVM_IOEVENTFD_FLAG_*
> namespace as part of the kernel KVM API, but which doesn't have the flags we
> need (e.g. userspace polling).

Looks good. I applied the fix so can you please redo this on top of tip?
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Will Deacon Sept. 4, 2013, 10:20 a.m. UTC | #2

On Wed, Sep 04, 2013 at 11:13:55AM +0100, Pekka Enberg wrote:
> On Wed, Sep 4, 2013 at 1:07 PM, Will Deacon <[e-mail]> wrote:
> > Like this? It gets a bit confusing, because there is a KVM_IOEVENTFD_FLAG_*
> > namespace as part of the kernel KVM API, but which doesn't have the flags we
> > need (e.g. userspace polling).
> 
> Looks good. I applied the fix so can you please redo this on top of tip?

Sure, I'll add a commit message too and send as a new thread.

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

Patch

diff --git a/tools/kvm/include/kvm/ioeventfd.h b/tools/kvm/include/kvm/ioeventfd.h
index d71fa40..bb1f78d 100644
--- a/tools/kvm/include/kvm/ioeventfd.h
+++ b/tools/kvm/include/kvm/ioeventfd.h
@@ -20,9 +20,12 @@  struct ioevent {
 	struct list_head	list;
 };
 
+#define IOEVENTFD_FLAG_PIO		(1 << 0)
+#define IOEVENTFD_FLAG_USER_POLL	(1 << 1)
+
 int ioeventfd__init(struct kvm *kvm);
 int ioeventfd__exit(struct kvm *kvm);
-int ioeventfd__add_event(struct ioevent *ioevent, bool is_pio, bool poll_in_userspace);
+int ioeventfd__add_event(struct ioevent *ioevent, int flags);
 int ioeventfd__del_event(u64 addr, u64 datamatch);
 
 #endif
diff --git a/tools/kvm/ioeventfd.c b/tools/kvm/ioeventfd.c
index ff665d4..bce6861 100644
--- a/tools/kvm/ioeventfd.c
+++ b/tools/kvm/ioeventfd.c
@@ -120,7 +120,7 @@  int ioeventfd__exit(struct kvm *kvm)
 }
 base_exit(ioeventfd__exit);
 
-int ioeventfd__add_event(struct ioevent *ioevent, bool is_pio, bool poll_in_userspace)
+int ioeventfd__add_event(struct ioevent *ioevent, int flags)
 {
 	struct kvm_ioeventfd kvm_ioevent;
 	struct epoll_event epoll_event;
@@ -145,7 +145,7 @@  int ioeventfd__add_event(struct ioevent *ioevent, bool is_pio, bool poll_in_user
 		.flags		= KVM_IOEVENTFD_FLAG_DATAMATCH,
 	};
 
-	if (is_pio)
+	if (flags & IOEVENTFD_FLAG_PIO)
 		kvm_ioevent.flags |= KVM_IOEVENTFD_FLAG_PIO;
 
 	r = ioctl(ioevent->fn_kvm->vm_fd, KVM_IOEVENTFD, &kvm_ioevent);
@@ -154,7 +154,7 @@  int ioeventfd__add_event(struct ioevent *ioevent, bool is_pio, bool poll_in_user
 		goto cleanup;
 	}
 
-	if (!poll_in_userspace)
+	if (!(flags & IOEVENTFD_FLAG_USER_POLL))
 		return 0;
 
 	epoll_event = (struct epoll_event) {
diff --git a/tools/kvm/virtio/mmio.c b/tools/kvm/virtio/mmio.c
index afa2692..afae6a7 100644
--- a/tools/kvm/virtio/mmio.c
+++ b/tools/kvm/virtio/mmio.c
@@ -55,10 +55,10 @@  static int virtio_mmio_init_ioeventfd(struct kvm *kvm,
 		 * Vhost will poll the eventfd in host kernel side,
 		 * no need to poll in userspace.
 		 */
-		err = ioeventfd__add_event(&ioevent, true, false);
+		err = ioeventfd__add_event(&ioevent, 0);
 	else
 		/* Need to poll in userspace. */
-		err = ioeventfd__add_event(&ioevent, true, true);
+		err = ioeventfd__add_event(&ioevent, IOEVENTFD_FLAG_USER_POLL);
 	if (err)
 		return err;
 
diff --git a/tools/kvm/virtio/pci.c b/tools/kvm/virtio/pci.c
index fec8ce0..bb6e7c4 100644
--- a/tools/kvm/virtio/pci.c
+++ b/tools/kvm/virtio/pci.c
@@ -46,10 +46,11 @@  static int virtio_pci__init_ioeventfd(struct kvm *kvm, struct virtio_device *vde
 		 * Vhost will poll the eventfd in host kernel side,
 		 * no need to poll in userspace.
 		 */
-		r = ioeventfd__add_event(&ioevent, true, false);
+		r = ioeventfd__add_event(&ioevent, IOEVENTFD_FLAG_PIO);
 	else
 		/* Need to poll in userspace. */
-		r = ioeventfd__add_event(&ioevent, true, true);
+		r = ioeventfd__add_event(&ioevent, IOEVENTFD_FLAG_PIO |
+						   IOEVENTFD_FLAG_USER_POLL);
 	if (r)
 		return r;