* [PATCH] kvm tools: virtio-mmio: init_ioeventfd should use MMIO for ioeventfd__add_event() @ 2013-08-30 13:58 Ying-Shiuan Pan 2013-09-04 9:21 ` Pekka Enberg 0 siblings, 1 reply; 5+ messages in thread From: Ying-Shiuan Pan @ 2013-08-30 13:58 UTC (permalink / raw) To: kvm; +Cc: yspan, penberg, asias.hejun, Ying-Shiuan Pan From: Ying-Shiuan Pan <yingshiuan.pan@gmail.com> 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 <yspan@itri.org.tw> --- tools/kvm/virtio/mmio.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/kvm/virtio/mmio.c b/tools/kvm/virtio/mmio.c index afa2692..3838774 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, false, false); else /* Need to poll in userspace. */ - err = ioeventfd__add_event(&ioevent, true, true); + err = ioeventfd__add_event(&ioevent, false, true); if (err) return err; -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] kvm tools: virtio-mmio: init_ioeventfd should use MMIO for ioeventfd__add_event() 2013-08-30 13:58 [PATCH] kvm tools: virtio-mmio: init_ioeventfd should use MMIO for ioeventfd__add_event() Ying-Shiuan Pan @ 2013-09-04 9:21 ` Pekka Enberg 2013-09-04 10:07 ` Will Deacon 0 siblings, 1 reply; 5+ messages in thread From: Pekka Enberg @ 2013-09-04 9:21 UTC (permalink / raw) To: Ying-Shiuan Pan Cc: kvm, yspan, Asias He, Sasha Levin, Will Deacon, Marc Zyngier On 8/30/13 4:58 PM, Ying-Shiuan Pan wrote: > From: Ying-Shiuan Pan <yingshiuan.pan@gmail.com> > > 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 <yspan@itri.org.tw> Will, Marc? It would probably be good to change the two boolean arguments into one "flags" argument to avoid future bugs. > --- > tools/kvm/virtio/mmio.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tools/kvm/virtio/mmio.c b/tools/kvm/virtio/mmio.c > index afa2692..3838774 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, false, false); > else > /* Need to poll in userspace. */ > - err = ioeventfd__add_event(&ioevent, true, true); > + err = ioeventfd__add_event(&ioevent, false, true); > if (err) > return err; > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] kvm tools: virtio-mmio: init_ioeventfd should use MMIO for ioeventfd__add_event() 2013-09-04 9:21 ` Pekka Enberg @ 2013-09-04 10:07 ` Will Deacon 2013-09-04 10:13 ` Pekka Enberg 0 siblings, 1 reply; 5+ messages in thread From: Will Deacon @ 2013-09-04 10:07 UTC (permalink / raw) To: Pekka Enberg Cc: Ying-Shiuan Pan, kvm@vger.kernel.org, yspan@itri.org.tw, Asias He, Sasha Levin, Marc Zyngier 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 <yingshiuan.pan@gmail.com> > > > > 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 <yspan@itri.org.tw> > > 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 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; ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] kvm tools: virtio-mmio: init_ioeventfd should use MMIO for ioeventfd__add_event() 2013-09-04 10:07 ` Will Deacon @ 2013-09-04 10:13 ` Pekka Enberg 2013-09-04 10:20 ` Will Deacon 0 siblings, 1 reply; 5+ messages in thread From: Pekka Enberg @ 2013-09-04 10:13 UTC (permalink / raw) To: Will Deacon Cc: Ying-Shiuan Pan, kvm@vger.kernel.org, yspan@itri.org.tw, Asias He, Sasha Levin, Marc Zyngier On Wed, Sep 4, 2013 at 1:07 PM, Will Deacon <will.deacon@arm.com> 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? ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] kvm tools: virtio-mmio: init_ioeventfd should use MMIO for ioeventfd__add_event() 2013-09-04 10:13 ` Pekka Enberg @ 2013-09-04 10:20 ` Will Deacon 0 siblings, 0 replies; 5+ messages in thread From: Will Deacon @ 2013-09-04 10:20 UTC (permalink / raw) To: Pekka Enberg Cc: Ying-Shiuan Pan, kvm@vger.kernel.org, yspan@itri.org.tw, Asias He, Sasha Levin, Marc Zyngier On Wed, Sep 04, 2013 at 11:13:55AM +0100, Pekka Enberg wrote: > On Wed, Sep 4, 2013 at 1:07 PM, Will Deacon <will.deacon@arm.com> 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-09-04 10:20 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-08-30 13:58 [PATCH] kvm tools: virtio-mmio: init_ioeventfd should use MMIO for ioeventfd__add_event() Ying-Shiuan Pan 2013-09-04 9:21 ` Pekka Enberg 2013-09-04 10:07 ` Will Deacon 2013-09-04 10:13 ` Pekka Enberg 2013-09-04 10:20 ` Will Deacon
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.