All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/2] kvm tools: Fix VHOST_SET_MEM_TABLE failure
@ 2012-07-11 16:08 Asias He
  2012-07-11 16:08 ` [PATCH v2 2/2] kvm tools: Do not poll ioeventfd if vhost is enabled Asias He
  2012-07-11 16:09 ` [PATCH v2 1/2] kvm tools: Fix VHOST_SET_MEM_TABLE failure Avi Kivity
  0 siblings, 2 replies; 6+ messages in thread
From: Asias He @ 2012-07-11 16:08 UTC (permalink / raw)
  To: Pekka Enberg; +Cc: Sasha Levin, Ingo Molnar, Cyrill Gorcunov, kvm, Asias He

VHOST_SET_MEM_TABLE failed: Operation not supported

In vhost_set_memory(), We have

        if (mem.padding)
                return -EOPNOTSUPP;

So, we need to zero struct vhost_memory.

Signed-off-by: Asias He <asias.hejun@gmail.com>
---
 tools/kvm/virtio/net.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/kvm/virtio/net.c b/tools/kvm/virtio/net.c
index ae17eb5..aa769d9 100644
--- a/tools/kvm/virtio/net.c
+++ b/tools/kvm/virtio/net.c
@@ -471,7 +471,7 @@ static void virtio_net__vhost_init(struct kvm *kvm, struct net_dev *ndev)
 	if (ndev->vhost_fd < 0)
 		die_perror("Failed openning vhost-net device");
 
-	mem = malloc(sizeof(*mem) + sizeof(struct vhost_memory_region));
+	mem = calloc(1, sizeof(*mem) + sizeof(struct vhost_memory_region));
 	if (mem == NULL)
 		die("Failed allocating memory for vhost memory map");
 
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 2/2] kvm tools: Do not poll ioeventfd if vhost is enabled
  2012-07-11 16:08 [PATCH v2 1/2] kvm tools: Fix VHOST_SET_MEM_TABLE failure Asias He
@ 2012-07-11 16:08 ` Asias He
  2012-07-11 16:09 ` [PATCH v2 1/2] kvm tools: Fix VHOST_SET_MEM_TABLE failure Avi Kivity
  1 sibling, 0 replies; 6+ messages in thread
From: Asias He @ 2012-07-11 16:08 UTC (permalink / raw)
  To: Pekka Enberg; +Cc: Sasha Levin, Ingo Molnar, Cyrill Gorcunov, kvm, Asias He

If vhost is enabled for a virtio device, vhost will poll the ioeventfd
in kernel side and there is no need to poll it in userspace. Otherwise,
both vhost kernel and userspace will race to poll.

Signed-off-by: Asias He <asias.hejun@gmail.com>
---
 tools/kvm/include/kvm/ioeventfd.h |    2 +-
 tools/kvm/include/kvm/virtio.h    |    1 +
 tools/kvm/ioeventfd.c             |    5 ++++-
 tools/kvm/virtio/mmio.c           |   10 +++++++++-
 tools/kvm/virtio/net.c            |    3 +++
 tools/kvm/virtio/pci.c            |   10 +++++++++-
 6 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/tools/kvm/include/kvm/ioeventfd.h b/tools/kvm/include/kvm/ioeventfd.h
index 70cce9a..d71fa40 100644
--- a/tools/kvm/include/kvm/ioeventfd.h
+++ b/tools/kvm/include/kvm/ioeventfd.h
@@ -22,7 +22,7 @@ struct ioevent {
 
 int ioeventfd__init(struct kvm *kvm);
 int ioeventfd__exit(struct kvm *kvm);
-int ioeventfd__add_event(struct ioevent *ioevent, bool is_pio);
+int ioeventfd__add_event(struct ioevent *ioevent, bool is_pio, bool poll_in_userspace);
 int ioeventfd__del_event(u64 addr, u64 datamatch);
 
 #endif
diff --git a/tools/kvm/include/kvm/virtio.h b/tools/kvm/include/kvm/virtio.h
index a957a5b..71b6bad 100644
--- a/tools/kvm/include/kvm/virtio.h
+++ b/tools/kvm/include/kvm/virtio.h
@@ -72,6 +72,7 @@ enum virtio_trans {
 };
 
 struct virtio_device {
+	bool			use_vhost;
 	void			*virtio;
 	struct virtio_ops	*ops;
 };
diff --git a/tools/kvm/ioeventfd.c b/tools/kvm/ioeventfd.c
index 97deb06..226876f 100644
--- a/tools/kvm/ioeventfd.c
+++ b/tools/kvm/ioeventfd.c
@@ -117,7 +117,7 @@ int ioeventfd__exit(struct kvm *kvm)
 	return 0;
 }
 
-int ioeventfd__add_event(struct ioevent *ioevent, bool is_pio)
+int ioeventfd__add_event(struct ioevent *ioevent, bool is_pio, bool poll_in_userspace)
 {
 	struct kvm_ioeventfd kvm_ioevent;
 	struct epoll_event epoll_event;
@@ -151,6 +151,9 @@ int ioeventfd__add_event(struct ioevent *ioevent, bool is_pio)
 		goto cleanup;
 	}
 
+	if (!poll_in_userspace)
+		return 0;
+
 	epoll_event = (struct epoll_event) {
 		.events		= EPOLLIN,
 		.data.ptr	= new_ioevent,
diff --git a/tools/kvm/virtio/mmio.c b/tools/kvm/virtio/mmio.c
index 8319954..1cf0815 100644
--- a/tools/kvm/virtio/mmio.c
+++ b/tools/kvm/virtio/mmio.c
@@ -48,7 +48,15 @@ static int virtio_mmio_init_ioeventfd(struct kvm *kvm,
 		.fd		= eventfd(0, 0),
 	};
 
-	err = ioeventfd__add_event(&ioevent, false);
+	if (vdev->use_vhost)
+		/*
+		 * Vhost will poll the eventfd in host kernel side,
+		 * no need to poll in userspace.
+		 */
+		err = ioeventfd__add_event(&ioevent, true, false);
+	else
+		/* Need to poll in userspace. */
+		err = ioeventfd__add_event(&ioevent, true, true);
 	if (err)
 		return err;
 
diff --git a/tools/kvm/virtio/net.c b/tools/kvm/virtio/net.c
index aa769d9..10420ae 100644
--- a/tools/kvm/virtio/net.c
+++ b/tools/kvm/virtio/net.c
@@ -492,6 +492,9 @@ static void virtio_net__vhost_init(struct kvm *kvm, struct net_dev *ndev)
 	r = ioctl(ndev->vhost_fd, VHOST_SET_MEM_TABLE, mem);
 	if (r != 0)
 		die_perror("VHOST_SET_MEM_TABLE failed");
+
+	ndev->vdev.use_vhost = true;
+
 	free(mem);
 }
 
diff --git a/tools/kvm/virtio/pci.c b/tools/kvm/virtio/pci.c
index 8558341e..f17cd8a 100644
--- a/tools/kvm/virtio/pci.c
+++ b/tools/kvm/virtio/pci.c
@@ -40,7 +40,15 @@ static int virtio_pci__init_ioeventfd(struct kvm *kvm, struct virtio_device *vde
 		.fd		= eventfd(0, 0),
 	};
 
-	r = ioeventfd__add_event(&ioevent, true);
+	if (vdev->use_vhost)
+		/*
+		 * Vhost will poll the eventfd in host kernel side,
+		 * no need to poll in userspace.
+		 */
+		r = ioeventfd__add_event(&ioevent, true, false);
+	else
+		/* Need to poll in userspace. */
+		r = ioeventfd__add_event(&ioevent, true, true);
 	if (r)
 		return r;
 
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] kvm tools: Fix VHOST_SET_MEM_TABLE failure
  2012-07-11 16:08 [PATCH v2 1/2] kvm tools: Fix VHOST_SET_MEM_TABLE failure Asias He
  2012-07-11 16:08 ` [PATCH v2 2/2] kvm tools: Do not poll ioeventfd if vhost is enabled Asias He
@ 2012-07-11 16:09 ` Avi Kivity
  2012-07-12  2:46   ` Asias He
  1 sibling, 1 reply; 6+ messages in thread
From: Avi Kivity @ 2012-07-11 16:09 UTC (permalink / raw)
  To: Asias He
  Cc: Pekka Enberg, Sasha Levin, Ingo Molnar, Cyrill Gorcunov, kvm,
	Michael S. Tsirkin

On 07/11/2012 07:08 PM, Asias He wrote:
> VHOST_SET_MEM_TABLE failed: Operation not supported
> 
> In vhost_set_memory(), We have
> 
>         if (mem.padding)
>                 return -EOPNOTSUPP;
> 
> So, we need to zero struct vhost_memory.
> 

Is this due to a change in vhost?


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



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] kvm tools: Fix VHOST_SET_MEM_TABLE failure
  2012-07-11 16:09 ` [PATCH v2 1/2] kvm tools: Fix VHOST_SET_MEM_TABLE failure Avi Kivity
@ 2012-07-12  2:46   ` Asias He
  2012-07-12  8:19     ` Avi Kivity
  0 siblings, 1 reply; 6+ messages in thread
From: Asias He @ 2012-07-12  2:46 UTC (permalink / raw)
  To: Avi Kivity
  Cc: Pekka Enberg, Sasha Levin, Ingo Molnar, Cyrill Gorcunov, kvm,
	Michael S. Tsirkin

On Thu, Jul 12, 2012 at 12:09 AM, Avi Kivity <avi@redhat.com> wrote:
> On 07/11/2012 07:08 PM, Asias He wrote:
>> VHOST_SET_MEM_TABLE failed: Operation not supported
>>
>> In vhost_set_memory(), We have
>>
>>         if (mem.padding)
>>                 return -EOPNOTSUPP;
>>
>> So, we need to zero struct vhost_memory.
>>
>
> Is this due to a change in vhost?

Seems we have this bit in the very beginning (commit 3a4d5c94).

-- 
Asias He

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] kvm tools: Fix VHOST_SET_MEM_TABLE failure
  2012-07-12  2:46   ` Asias He
@ 2012-07-12  8:19     ` Avi Kivity
  2012-07-12 23:27       ` Michael S. Tsirkin
  0 siblings, 1 reply; 6+ messages in thread
From: Avi Kivity @ 2012-07-12  8:19 UTC (permalink / raw)
  To: Asias He
  Cc: Pekka Enberg, Sasha Levin, Ingo Molnar, Cyrill Gorcunov, kvm,
	Michael S. Tsirkin

On 07/12/2012 05:46 AM, Asias He wrote:
> On Thu, Jul 12, 2012 at 12:09 AM, Avi Kivity <avi@redhat.com> wrote:
>> On 07/11/2012 07:08 PM, Asias He wrote:
>>> VHOST_SET_MEM_TABLE failed: Operation not supported
>>>
>>> In vhost_set_memory(), We have
>>>
>>>         if (mem.padding)
>>>                 return -EOPNOTSUPP;
>>>
>>> So, we need to zero struct vhost_memory.
>>>
>>
>> Is this due to a change in vhost?
> 
> Seems we have this bit in the very beginning (commit 3a4d5c94).

Okay, so it's a documentation problem.  Michael, where is the
documentation for vhost-net?

Note we have to initialize it with memset(); presumably when we
repurpose it the name will change, and anonymous unions are not very
portable.

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



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] kvm tools: Fix VHOST_SET_MEM_TABLE failure
  2012-07-12  8:19     ` Avi Kivity
@ 2012-07-12 23:27       ` Michael S. Tsirkin
  0 siblings, 0 replies; 6+ messages in thread
From: Michael S. Tsirkin @ 2012-07-12 23:27 UTC (permalink / raw)
  To: Avi Kivity
  Cc: Asias He, Pekka Enberg, Sasha Levin, Ingo Molnar, Cyrill Gorcunov, kvm

On Thu, Jul 12, 2012 at 11:19:47AM +0300, Avi Kivity wrote:
> On 07/12/2012 05:46 AM, Asias He wrote:
> > On Thu, Jul 12, 2012 at 12:09 AM, Avi Kivity <avi@redhat.com> wrote:
> >> On 07/11/2012 07:08 PM, Asias He wrote:
> >>> VHOST_SET_MEM_TABLE failed: Operation not supported
> >>>
> >>> In vhost_set_memory(), We have
> >>>
> >>>         if (mem.padding)
> >>>                 return -EOPNOTSUPP;
> >>>
> >>> So, we need to zero struct vhost_memory.
> >>>
> >>
> >> Is this due to a change in vhost?
> > 
> > Seems we have this bit in the very beginning (commit 3a4d5c94).
> 
> Okay, so it's a documentation problem.  Michael, where is the
> documentation for vhost-net?

Most fields are documented in include/linux/vhost.h
Yes the approach vhost consistently takes is to require all unused
fields to be zeroed out.

> Note we have to initialize it with memset();

We can also use = {} if we want to avoid naming it.

> presumably when we
> repurpose it the name will change, and anonymous unions are not very
> portable.

Looks like in the new C standard they are :)
I'm not sure what we'll do, exactly, if we need to reuse this padding
for something else, but not breaking build for old userspace
will be a priority.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-07-12 23:27 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-11 16:08 [PATCH v2 1/2] kvm tools: Fix VHOST_SET_MEM_TABLE failure Asias He
2012-07-11 16:08 ` [PATCH v2 2/2] kvm tools: Do not poll ioeventfd if vhost is enabled Asias He
2012-07-11 16:09 ` [PATCH v2 1/2] kvm tools: Fix VHOST_SET_MEM_TABLE failure Avi Kivity
2012-07-12  2:46   ` Asias He
2012-07-12  8:19     ` Avi Kivity
2012-07-12 23:27       ` Michael S. Tsirkin

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.