login
Header Space

 
 

Re: s390 kvm_virtio.c build error

Previous thread: [PATCH 3/4] posix timers: timer_delete: remove the bogus "->it_process != NULL" check by Oleg Nesterov on Saturday, May 3, 2008 - 1:35 pm. (2 messages)

Next thread: [PATCH v2] unify sys_pipe implementation by Ulrich Drepper on Saturday, May 3, 2008 - 2:01 pm. (5 messages)
To: Christian Borntraeger <borntraeger@...>, Martin Schwidefsky <schwidefsky@...>, Carsten Otte <cotte@...>, Avi Kivity <avi@...>, Rusty Russell <rusty@...>
Cc: <kvm-devel@...>, <linux-s390@...>, <linux-kernel@...>
Date: Saturday, May 3, 2008 - 1:47 pm

Commit c45a6816c19dee67b8f725e6646d428901a6dc24
(virtio: explicit advertisement of driver features)
and commit e976a2b997fc4ad70ccc53acfe62811c4aaec851
(s390: KVM guest: virtio device support, and kvm hypercalls)
don't like each other:

&lt;--  snip  --&gt;

...
  CC      drivers/s390/kvm/kvm_virtio.o
/home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/s390/kvm/kvm_virtio.c:224: error: unknown field 'feature' specified in initializer
/home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/s390/kvm/kvm_virtio.c:224: warning: initialization from incompatible pointer type
make[3]: *** [drivers/s390/kvm/kvm_virtio.o] Error 1

&lt;--  snip  --&gt;

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

--
To: Adrian Bunk <bunk@...>
Cc: Christian Borntraeger <borntraeger@...>, Martin Schwidefsky <schwidefsky@...>, Carsten Otte <cotte@...>, Avi Kivity <avi@...>, <kvm-devel@...>, <linux-s390@...>, <linux-kernel@...>
Date: Sunday, May 4, 2008 - 8:19 pm

Yep, I broke s390.  This was kind of expected, but I didn't want to try to
fix it as I am unable to test.

It would look something like this:

virtio: Attempt to fix kvm_virtio after feature management changes

Very similar to lguest code: set and get feature are now separate callbacks.

Signed-off-by: Rusty Russell &lt;rusty@rustcorp.com.au&gt;

diff -r 219d6c116996 drivers/s390/kvm/kvm_virtio.c
--- a/drivers/s390/kvm/kvm_virtio.c	Mon May 05 10:03:16 2008 +1000
+++ b/drivers/s390/kvm/kvm_virtio.c	Mon May 05 10:17:25 2008 +1000
@@ -78,27 +78,34 @@ static unsigned desc_size(const struct k
 		+ desc-&gt;config_len;
 }
 
-/*
- * This tests (and acknowleges) a feature bit.
- */
-static bool kvm_feature(struct virtio_device *vdev, unsigned fbit)
+/* This gets the device's feature bits. */
+static u32 kvm_get_features(struct virtio_device *vdev)
 {
+	unsigned int i;
+	u32 features = 0;
 	struct kvm_device_desc *desc = to_kvmdev(vdev)-&gt;desc;
-	u8 *features;
+	u8 *in_features = kvm_vq_features(desc);
 
-	if (fbit / 8 &gt; desc-&gt;feature_len)
-		return false;
+	/* We do this the slow but generic way. */
+	for (i = 0; i &lt; min(desc-&gt;feature_len * 8, 32); i++)
+		if (in_features[i / 8] &amp; (1 &lt;&lt; (i % 8)))
+			features |= (1 &lt;&lt; i);
 
-	features = kvm_vq_features(desc);
-	if (!(features[fbit / 8] &amp; (1 &lt;&lt; (fbit % 8))))
-		return false;
+	return features;
+}
 
-	/*
-	 * We set the matching bit in the other half of the bitmap to tell the
-	 * Host we want to use this feature.
-	 */
-	features[desc-&gt;feature_len + fbit / 8] |= (1 &lt;&lt; (fbit % 8));
-	return true;
+static void kvm_set_features(struct virtio_device *vdev, u32 features)
+{
+	unsigned int i;
+	struct kvm_device_desc *desc = to_kvmdev(vdev)-&gt;desc;
+	/* Second half of bitmap is features we accept. */
+	u8 *out_features = kvm_vq_features(desc) + desc-&gt;feature_len;
+
+	memset(out_features, 0, desc-&gt;feature_len);
+	for (i = 0; i &lt; min(desc-&gt;feature_len * 8, 32...
To: Adrian Bunk <bunk@...>
Cc: Christian Borntraeger <borntraeger@...>, Martin Schwidefsky <schwidefsky@...>, Carsten Otte <cotte@...>, Avi Kivity <avi@...>, Rusty Russell <rusty@...>, <kvm-devel@...>, <linux-s390@...>, <linux-kernel@...>
Date: Sunday, May 4, 2008 - 3:25 pm

Hmm... this should help:

---
 drivers/s390/kvm/kvm_virtio.c |   40 +++++++++++++++++++++++-----------------
 1 file changed, 23 insertions(+), 17 deletions(-)

Index: linux-2.6/drivers/s390/kvm/kvm_virtio.c
===================================================================
--- linux-2.6.orig/drivers/s390/kvm/kvm_virtio.c
+++ linux-2.6/drivers/s390/kvm/kvm_virtio.c
@@ -78,27 +78,32 @@ static unsigned desc_size(const struct k
 		+ desc-&gt;config_len;
 }
 
-/*
- * This tests (and acknowleges) a feature bit.
- */
-static bool kvm_feature(struct virtio_device *vdev, unsigned fbit)
+/* This gets the device's feature bits. */
+static u32 kvm_get_features(struct virtio_device *vdev)
 {
+	unsigned int i;
+	u32 features = 0;
 	struct kvm_device_desc *desc = to_kvmdev(vdev)-&gt;desc;
-	u8 *features;
+	u8 *in_features = kvm_vq_features(desc);
 
-	if (fbit / 8 &gt; desc-&gt;feature_len)
-		return false;
+	for (i = 0; i &lt; min(desc-&gt;feature_len * 8, 32); i++)
+		if (in_features[i / 8] &amp; (1 &lt;&lt; (i % 8)))
+			features |= (1 &lt;&lt; i);
+	return features;
+}
 
-	features = kvm_vq_features(desc);
-	if (!(features[fbit / 8] &amp; (1 &lt;&lt; (fbit % 8))))
-		return false;
+static void kvm_set_features(struct virtio_device *vdev, u32 features)
+{
+	unsigned int i;
+	struct kvm_device_desc *desc = to_kvmdev(vdev)-&gt;desc;
+	/* Second half of bitmap is features we accept. */
+	u8 *out_features = kvm_vq_features(desc) + desc-&gt;feature_len;
 
-	/*
-	 * We set the matching bit in the other half of the bitmap to tell the
-	 * Host we want to use this feature.
-	 */
-	features[desc-&gt;feature_len + fbit / 8] |= (1 &lt;&lt; (fbit % 8));
-	return true;
+	memset(out_features, 0, desc-&gt;feature_len);
+	for (i = 0; i &lt; min(desc-&gt;feature_len * 8, 32); i++) {
+		if (features &amp; (1 &lt;&lt; i))
+			out_features[i / 8] |= (1 &lt;&lt; (i % 8));
+	}
 }
 
 /*
@@ -221,7 +226,8 @@ static void kvm_del_vq(struct virtqueue 
  * The config ops structure as defined...
To: Heiko Carstens <heiko.carstens@...>
Cc: Adrian Bunk <bunk@...>, Martin Schwidefsky <schwidefsky@...>, Carsten Otte <cotte@...>, Avi Kivity <avi@...>, Rusty Russell <rusty@...>, <kvm-devel@...>, <linux-s390@...>, <linux-kernel@...>
Date: Monday, May 5, 2008 - 8:29 am

Thanks Heiko.
I did a short test and it seems to work.

Acked-by: Christian Borntraeger &lt;borntraeger@de.ibm.com&gt;

This looks almost identical to Rusty's patch. Who is going to send this (or 
Rustys) patch to Linus?

Christian
--
To: Christian Borntraeger <borntraeger@...>
Cc: Heiko Carstens <heiko.carstens@...>, Adrian Bunk <bunk@...>, Martin Schwidefsky <schwidefsky@...>, Carsten Otte <cotte@...>, Rusty Russell <rusty@...>, <kvm-devel@...>, <linux-s390@...>, <linux-kernel@...>
Date: Monday, May 5, 2008 - 9:00 am

I can, but tell me which one.  Also, the patch (Heiko's) needs a 
changelog entry and a signoff.

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

--
To: Avi Kivity <avi@...>
Cc: Christian Borntraeger <borntraeger@...>, Heiko Carstens <heiko.carstens@...>, Adrian Bunk <bunk@...>, Carsten Otte <cotte@...>, Rusty Russell <rusty@...>, <kvm-devel@...>, <linux-s390@...>, <linux-kernel@...>
Date: Monday, May 5, 2008 - 9:06 am

I've added Heiko's patch to my patchqueue. But since this is
drivers/s390/kvm this should go in over the kvm.git. See patch below.

-- 
blue skies,
  Martin.

"Reality continues to ruin my life." - Calvin.

---
Subject: [PATCH] kvm/s390 compile error

From: Heiko Carstens &lt;heiko.carstens@de.ibm.com&gt;

Fix kvm compile error:

Commit c45a6816c19dee67b8f725e6646d428901a6dc24
(virtio: explicit advertisement of driver features)
and commit e976a2b997fc4ad70ccc53acfe62811c4aaec851
(s390: KVM guest: virtio device support, and kvm hypercalls)
don't like each other:

  CC      drivers/s390/kvm/kvm_virtio.o
drivers/s390/kvm/kvm_virtio.c:224: error: unknown field 'feature' specified in initializer
drivers/s390/kvm/kvm_virtio.c:224: warning: initialization from incompatible pointer type
make[3]: *** [drivers/s390/kvm/kvm_virtio.o] Error 1

Cc: Adrian Bunk &lt;bunk@kernel.org&gt;
Signed-off-by: Heiko Carstens &lt;heiko.carstens@de.ibm.com&gt;
Signed-off-by: Martin Schwidefsky &lt;schwidefsky@de.ibm.com&gt;
---

 drivers/s390/kvm/kvm_virtio.c |   40 +++++++++++++++++++++++-----------------
 1 file changed, 23 insertions(+), 17 deletions(-)

diff -urpN linux-2.6/drivers/s390/kvm/kvm_virtio.c linux-2.6-patched/drivers/s390/kvm/kvm_virtio.c
--- linux-2.6/drivers/s390/kvm/kvm_virtio.c	2008-05-05 13:20:45.000000000 +0200
+++ linux-2.6-patched/drivers/s390/kvm/kvm_virtio.c	2008-05-05 13:20:48.000000000 +0200
@@ -78,27 +78,32 @@ static unsigned desc_size(const struct k
 		+ desc-&gt;config_len;
 }
 
-/*
- * This tests (and acknowleges) a feature bit.
- */
-static bool kvm_feature(struct virtio_device *vdev, unsigned fbit)
+/* This gets the device's feature bits. */
+static u32 kvm_get_features(struct virtio_device *vdev)
 {
+	unsigned int i;
+	u32 features = 0;
 	struct kvm_device_desc *desc = to_kvmdev(vdev)-&gt;desc;
-	u8 *features;
+	u8 *in_features = kvm_vq_features(desc);
 
-	if (fbit / 8 &gt; desc-&gt;feature_len)
-		return false;
+	for (i = 0; i &lt; min(desc-&gt;f...
To: <schwidefsky@...>
Cc: Christian Borntraeger <borntraeger@...>, Heiko Carstens <heiko.carstens@...>, Adrian Bunk <bunk@...>, Carsten Otte <cotte@...>, Rusty Russell <rusty@...>, <kvm-devel@...>, <linux-s390@...>, <linux-kernel@...>
Date: Tuesday, May 6, 2008 - 10:39 am

Thanks, I added this to my queue as well.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

--
To: <mschwid2@...>
Cc: Avi Kivity <avi@...>, <borntrae@...>, <heicars2@...>, Adrian Bunk <bunk@...>, Rusty Russell <rusty@...>, <kvm-devel@...>, <linux-s390@...>, <linux-kernel@...>
Date: Monday, May 5, 2008 - 9:20 am

Acked-by: Carsten Otte &lt;cotte@de.ibm.com&gt;

--
Previous thread: [PATCH 3/4] posix timers: timer_delete: remove the bogus "->it_process != NULL" check by Oleg Nesterov on Saturday, May 3, 2008 - 1:35 pm. (2 messages)

Next thread: [PATCH v2] unify sys_pipe implementation by Ulrich Drepper on Saturday, May 3, 2008 - 2:01 pm. (5 messages)
speck-geostationary