public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH] virtio: change config to guest endian.
@ 2008-04-22  3:57 Rusty Russell
  2008-04-22  7:44 ` Christian Borntraeger
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Rusty Russell @ 2008-04-22  3:57 UTC (permalink / raw)
  To: virtualization; +Cc: kvm-devel, Christian Borntraeger, Hollis Blanchard

[Christian, Hollis, how much is this ABI breakage going to hurt you?]

A recent proposed feature addition to the virtio block driver revealed
some flaws in the API, in particular how easy it is to break big
endian machines.

The virtio config space was originally chosen to be little-endian,
because we thought the config might be part of the PCI config space
for virtio_pci.  It's actually a separate mmio region, so that
argument holds little water; as only x86 is currently using the virtio
mechanism, we can change this (but must do so now, before the
impending s390 and ppc merges).

API changes:
- __virtio_config_val() just becomes a striaght vdev->config_get() call.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
 drivers/block/virtio_blk.c      |    4 +--
 drivers/virtio/virtio_balloon.c |    6 ++---
 include/linux/virtio_config.h   |   47 +++++++++++++---------------------------
 3 files changed, 21 insertions(+), 36 deletions(-)

diff -r a098f19a6da5 drivers/block/virtio_blk.c
--- a/drivers/block/virtio_blk.c	Sun Apr 20 14:41:02 2008 +1000
+++ b/drivers/block/virtio_blk.c	Sun Apr 20 15:07:45 2008 +1000
@@ -246,8 +246,8 @@ static int virtblk_probe(struct virtio_d
 		blk_queue_ordered(vblk->disk->queue, QUEUE_ORDERED_TAG, NULL);
 
 	/* Host must always specify the capacity. */
-	__virtio_config_val(vdev, offsetof(struct virtio_blk_config, capacity),
-			    &cap);
+	vdev->config->get(vdev, offsetof(struct virtio_blk_config, capacity),
+			  &cap, sizeof(cap));
 
 	/* If capacity is too big, truncate with warning. */
 	if ((sector_t)cap != cap) {
diff -r a098f19a6da5 drivers/virtio/virtio_balloon.c
--- a/drivers/virtio/virtio_balloon.c	Sun Apr 20 14:41:02 2008 +1000
+++ b/drivers/virtio/virtio_balloon.c	Sun Apr 20 15:07:45 2008 +1000
@@ -155,9 +155,9 @@ static inline s64 towards_target(struct 
 static inline s64 towards_target(struct virtio_balloon *vb)
 {
 	u32 v;
-	__virtio_config_val(vb->vdev,
-			    offsetof(struct virtio_balloon_config, num_pages),
-			    &v);
+	vb->vdev->config->get(vb->vdev,
+			      offsetof(struct virtio_balloon_config, num_pages),
+			      &v);
 	return v - vb->num_pages;
 }
 
diff -r a098f19a6da5 include/linux/virtio_config.h
--- a/include/linux/virtio_config.h	Sun Apr 20 14:41:02 2008 +1000
+++ b/include/linux/virtio_config.h	Sun Apr 20 15:07:45 2008 +1000
@@ -16,7 +16,7 @@
 #define VIRTIO_CONFIG_S_FAILED		0x80
 
 #ifdef __KERNEL__
-struct virtio_device;
+#include <linux/virtio.h>
 
 /**
  * virtio_config_ops - operations for configuring a virtio device
@@ -30,13 +30,11 @@ struct virtio_device;
  *	offset: the offset of the configuration field
  *	buf: the buffer to write the field value into.
  *	len: the length of the buffer
- *	Note that contents are conventionally little-endian.
  * @set: write the value of a configuration field
  *	vdev: the virtio_device
  *	offset: the offset of the configuration field
  *	buf: the buffer to read the field value from.
  *	len: the length of the buffer
- *	Note that contents are conventionally little-endian.
  * @get_status: read the status byte
  *	vdev: the virtio_device
  *	Returns the status byte
@@ -70,40 +68,27 @@ struct virtio_config_ops
 };
 
 /**
- * virtio_config_val - look for a feature and get a single virtio config.
+ * virtio_config_val - look for a feature and get a virtio config entry.
  * @vdev: the virtio device
  * @fbit: the feature bit
  * @offset: the type to search for.
  * @val: a pointer to the value to fill in.
  *
  * The return value is -ENOENT if the feature doesn't exist.  Otherwise
- * the value is endian-corrected and returned in v. */
-#define virtio_config_val(vdev, fbit, offset, v) ({			\
-	int _err;							\
-	if ((vdev)->config->feature((vdev), (fbit))) {			\
-		__virtio_config_val((vdev), (offset), (v));		\
-		_err = 0;						\
-	} else								\
-		_err = -ENOENT;						\
-	_err;								\
-})
+ * the config value is copied into whatever is pointed to by v. */
+#define virtio_config_val(vdev, fbit, offset, v) \
+	virtio_config_buf((vdev), (fbit), (offset), (v), sizeof(v))
 
-/**
- * __virtio_config_val - get a single virtio config without feature check.
- * @vdev: the virtio device
- * @offset: the type to search for.
- * @val: a pointer to the value to fill in.
- *
- * The value is endian-corrected and returned in v. */
-#define __virtio_config_val(vdev, offset, v) do {			\
-	BUILD_BUG_ON(sizeof(*(v)) != 1 && sizeof(*(v)) != 2		\
-		     && sizeof(*(v)) != 4 && sizeof(*(v)) != 8);	\
-	(vdev)->config->get((vdev), (offset), (v), sizeof(*(v)));	\
-	switch (sizeof(*(v))) {						\
-	case 2: le16_to_cpus((__u16 *) v); break;			\
-	case 4: le32_to_cpus((__u32 *) v); break;			\
-	case 8: le64_to_cpus((__u64 *) v); break;			\
-	}								\
-} while(0)
+static inline int virtio_config_buf(struct virtio_device *vdev,
+				    unsigned int fbit,
+				    unsigned int offset,
+				    void *buf, unsigned len)
+{
+	if (!vdev->config->feature(vdev, fbit))
+		return -ENOENT;
+
+	vdev->config->get(vdev, offset, buf, len);
+	return 0;
+}
 #endif /* __KERNEL__ */
 #endif /* _LINUX_VIRTIO_CONFIG_H */

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

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

* Re: [RFC PATCH] virtio: change config to guest endian.
  2008-04-22  3:57 [RFC PATCH] virtio: change config to guest endian Rusty Russell
@ 2008-04-22  7:44 ` Christian Borntraeger
  2008-04-22 14:40   ` Rusty Russell
  2008-04-22 11:22 ` Avi Kivity
  2008-04-23 10:55 ` Christian Borntraeger
  2 siblings, 1 reply; 16+ messages in thread
From: Christian Borntraeger @ 2008-04-22  7:44 UTC (permalink / raw)
  To: Rusty Russell; +Cc: kvm-devel, Hollis Blanchard, virtualization

Am Dienstag, 22. April 2008 schrieb Rusty Russell:
> [Christian, Hollis, how much is this ABI breakage going to hurt you?]

It is ok for s390 at the moment. We are still working on making userspace 
ready and I plan to change the guest<->host for s390 anyway. I try to make 
these changes for drivers/s390/kvm/kvm_virtio.c before 2.6.26. The main 
reason is, that we are currently limited to around 80 devices. I am not sure, 
if I should change the allocation of the virtqueues and descriptors to guest 
memory as well. 

Back to your patch:
I have still some ideas about virtio between little endian and big endian 
systems, but it requires more and different marshalling anyway - even on 
driver level. No idea yet how to solve that properly.

Consider your change
Acked-by: Christian Bornraeger <borntraeger@de.ibm.com>
given that you fix the issue below:

[...]
> --- a/drivers/virtio/virtio_balloon.c	Sun Apr 20 14:41:02 2008 +1000
> +++ b/drivers/virtio/virtio_balloon.c	Sun Apr 20 15:07:45 2008 +1000
> @@ -155,9 +155,9 @@ static inline s64 towards_target(struct 
>  static inline s64 towards_target(struct virtio_balloon *vb)
>  {
>  	u32 v;
> -	__virtio_config_val(vb->vdev,
> -			    offsetof(struct virtio_balloon_config, num_pages),
> -			    &v);
> +	vb->vdev->config->get(vb->vdev,
> +			      offsetof(struct virtio_balloon_config, num_pages),
> +			      &v);

this is missing a sizeof(v), no?

Christian

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

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

* Re: [RFC PATCH] virtio: change config to guest endian.
  2008-04-22  3:57 [RFC PATCH] virtio: change config to guest endian Rusty Russell
  2008-04-22  7:44 ` Christian Borntraeger
@ 2008-04-22 11:22 ` Avi Kivity
  2008-04-22 14:31   ` Rusty Russell
  2008-04-22 16:21   ` [kvm-devel] " Hollis Blanchard
  2008-04-23 10:55 ` Christian Borntraeger
  2 siblings, 2 replies; 16+ messages in thread
From: Avi Kivity @ 2008-04-22 11:22 UTC (permalink / raw)
  To: Rusty Russell
  Cc: kvm-devel, Christian Borntraeger, Hollis Blanchard,
	virtualization

Rusty Russell wrote:
> [Christian, Hollis, how much is this ABI breakage going to hurt you?]
>
> A recent proposed feature addition to the virtio block driver revealed
> some flaws in the API, in particular how easy it is to break big
> endian machines.
>
> The virtio config space was originally chosen to be little-endian,
> because we thought the config might be part of the PCI config space
> for virtio_pci.  It's actually a separate mmio region, so that
> argument holds little water; as only x86 is currently using the virtio
> mechanism, we can change this (but must do so now, before the
> impending s390 and ppc merges).
>
>   

This will probably annoy Hollis which has guests that can go both ways.

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


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

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

* Re: [RFC PATCH] virtio: change config to guest endian.
  2008-04-22 11:22 ` Avi Kivity
@ 2008-04-22 14:31   ` Rusty Russell
  2008-04-22 20:29     ` [kvm-devel] " Hollis Blanchard
  2008-04-22 16:21   ` [kvm-devel] " Hollis Blanchard
  1 sibling, 1 reply; 16+ messages in thread
From: Rusty Russell @ 2008-04-22 14:31 UTC (permalink / raw)
  To: Avi Kivity
  Cc: kvm-devel, Christian Borntraeger, Hollis Blanchard,
	virtualization

On Tuesday 22 April 2008 21:22:48 Avi Kivity wrote:
> > The virtio config space was originally chosen to be little-endian,
> > because we thought the config might be part of the PCI config space
> > for virtio_pci.  It's actually a separate mmio region, so that
> > argument holds little water; as only x86 is currently using the virtio
> > mechanism, we can change this (but must do so now, before the
> > impending s390 and ppc merges).
>
> This will probably annoy Hollis which has guests that can go both ways.

Yes, I discussed this with Hollis.  But the virtio rings themselves already 
have this issue: we don't do any endian conversion on them and assume 
they're "our" endian in the guest.

We may still regret not doing *everything* little-endian, but this doesn't 
make it worse.

Thanks,
Rusty.


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

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

* Re: [RFC PATCH] virtio: change config to guest endian.
  2008-04-22  7:44 ` Christian Borntraeger
@ 2008-04-22 14:40   ` Rusty Russell
  0 siblings, 0 replies; 16+ messages in thread
From: Rusty Russell @ 2008-04-22 14:40 UTC (permalink / raw)
  To: Christian Borntraeger; +Cc: kvm-devel, Hollis Blanchard, virtualization

On Tuesday 22 April 2008 17:44:08 Christian Borntraeger wrote:
> Am Dienstag, 22. April 2008 schrieb Rusty Russell:
> > [Christian, Hollis, how much is this ABI breakage going to hurt you?]
>
> It is ok for s390 at the moment. We are still working on making userspace
> ready and I plan to change the guest<->host for s390 anyway. I try to make
> these changes for drivers/s390/kvm/kvm_virtio.c before 2.6.26. The main
> reason is, that we are currently limited to around 80 devices. I am not
> sure, if I should change the allocation of the virtqueues and descriptors
> to guest memory as well.

Large rings require contiguous memory, which makes guest allocation 
problematic.  512 elems at 4k pages == 5 pages.

> Back to your patch:
> I have still some ideas about virtio between little endian and big endian
> systems, but it requires more and different marshalling anyway - even on
> driver level. No idea yet how to solve that properly.

So far we've pushed such considerations onto the host.  This does mean that 
you can't virtio connect two guests directly without understanding the 
contents of the buffers so you can endian correct (eg. direct inter-guest 
networking).  inter-guest virtio is currently a party trick anyway, so I'm 
not sure it's a real issue.

> > +	vb->vdev->config->get(vb->vdev,
> > +			      offsetof(struct virtio_balloon_config, num_pages),
> > +			      &v);
>
> this is missing a sizeof(v), no?

Ah... sure enough, I fixed that in a followon patch.  Well-spotted, thanks!

Cheers,
Rusty.

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

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

* Re: [kvm-devel] [RFC PATCH] virtio: change config to guest endian.
  2008-04-22 11:22 ` Avi Kivity
  2008-04-22 14:31   ` Rusty Russell
@ 2008-04-22 16:21   ` Hollis Blanchard
  1 sibling, 0 replies; 16+ messages in thread
From: Hollis Blanchard @ 2008-04-22 16:21 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm-devel, Christian Borntraeger, virtualization

On Tuesday 22 April 2008 06:22:48 Avi Kivity wrote:
> Rusty Russell wrote:
> > [Christian, Hollis, how much is this ABI breakage going to hurt you?]
> >
> > A recent proposed feature addition to the virtio block driver revealed
> > some flaws in the API, in particular how easy it is to break big
> > endian machines.
> >
> > The virtio config space was originally chosen to be little-endian,
> > because we thought the config might be part of the PCI config space
> > for virtio_pci.  It's actually a separate mmio region, so that
> > argument holds little water; as only x86 is currently using the virtio
> > mechanism, we can change this (but must do so now, before the
> > impending s390 and ppc merges).
> 
> This will probably annoy Hollis which has guests that can go both ways.

Rusty and I have discussed it. Ultimately, this just takes us from a 
cross-architecture endianness definition to a per-architecture definition. 
Anyways, we've already fallen into this situation with the virtio ring data 
itself, so we're really saying "same endianness as the ring".

-- 
Hollis Blanchard
IBM Linux Technology Center

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

* Re: [kvm-devel] [RFC PATCH] virtio: change config to guest endian.
  2008-04-22 14:31   ` Rusty Russell
@ 2008-04-22 20:29     ` Hollis Blanchard
  2008-04-22 21:05       ` Rusty Russell
  0 siblings, 1 reply; 16+ messages in thread
From: Hollis Blanchard @ 2008-04-22 20:29 UTC (permalink / raw)
  To: Rusty Russell; +Cc: kvm-devel, Christian Borntraeger, virtualization

On Tuesday 22 April 2008 09:31:35 Rusty Russell wrote:
> On Tuesday 22 April 2008 21:22:48 Avi Kivity wrote:
> > > The virtio config space was originally chosen to be little-endian,
> > > because we thought the config might be part of the PCI config space
> > > for virtio_pci.  It's actually a separate mmio region, so that
> > > argument holds little water; as only x86 is currently using the virtio
> > > mechanism, we can change this (but must do so now, before the
> > > impending s390 and ppc merges).
> >
> > This will probably annoy Hollis which has guests that can go both ways.
> 
> Yes, I discussed this with Hollis.  But the virtio rings themselves already 
> have this issue: we don't do any endian conversion on them and assume 
> they're "our" endian in the guest.
> 
> We may still regret not doing *everything* little-endian, but this doesn't 
> make it worse.

Hmm, why *don't* we just do everything LE, including the ring?

-- 
Hollis Blanchard
IBM Linux Technology Center

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

* Re: [RFC PATCH] virtio: change config to guest endian.
  2008-04-22 20:29     ` [kvm-devel] " Hollis Blanchard
@ 2008-04-22 21:05       ` Rusty Russell
  2008-04-22 22:08         ` Hollis Blanchard
  0 siblings, 1 reply; 16+ messages in thread
From: Rusty Russell @ 2008-04-22 21:05 UTC (permalink / raw)
  To: Hollis Blanchard
  Cc: kvm-devel, Christian Borntraeger, Avi Kivity, virtualization

On Wednesday 23 April 2008 06:29:14 Hollis Blanchard wrote:
> On Tuesday 22 April 2008 09:31:35 Rusty Russell wrote:
> > We may still regret not doing *everything* little-endian, but this
> > doesn't make it worse.
>
> Hmm, why *don't* we just do everything LE, including the ring?

Mainly because when requirements are in doubt, simplicity wins, I think.

Cheers,
Rusty.

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

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

* Re: [RFC PATCH] virtio: change config to guest endian.
  2008-04-22 21:05       ` Rusty Russell
@ 2008-04-22 22:08         ` Hollis Blanchard
  2008-04-22 22:13           ` Anthony Liguori
  0 siblings, 1 reply; 16+ messages in thread
From: Hollis Blanchard @ 2008-04-22 22:08 UTC (permalink / raw)
  To: Rusty Russell
  Cc: kvm-devel, Christian Borntraeger, Avi Kivity, virtualization

On Tuesday 22 April 2008 16:05:38 Rusty Russell wrote:
> On Wednesday 23 April 2008 06:29:14 Hollis Blanchard wrote:
> > On Tuesday 22 April 2008 09:31:35 Rusty Russell wrote:
> > > We may still regret not doing *everything* little-endian, but this
> > > doesn't make it worse.
> >
> > Hmm, why *don't* we just do everything LE, including the ring?
> 
> Mainly because when requirements are in doubt, simplicity wins, I think.

Well, I think the definition of simplicity is up for debate in this 
case... "LE everywhere" is much simpler than "it depends", IMHO.

-- 
Hollis Blanchard
IBM Linux Technology Center

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

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

* Re: [RFC PATCH] virtio: change config to guest endian.
  2008-04-22 22:08         ` Hollis Blanchard
@ 2008-04-22 22:13           ` Anthony Liguori
  2008-04-22 22:33             ` Hollis Blanchard
  2008-04-22 23:53             ` Hollis Blanchard
  0 siblings, 2 replies; 16+ messages in thread
From: Anthony Liguori @ 2008-04-22 22:13 UTC (permalink / raw)
  To: Hollis Blanchard
  Cc: kvm-devel, Christian Borntraeger, Avi Kivity, virtualization

Hollis Blanchard wrote:
> On Tuesday 22 April 2008 16:05:38 Rusty Russell wrote:
>   
>> On Wednesday 23 April 2008 06:29:14 Hollis Blanchard wrote:
>>     
>>> On Tuesday 22 April 2008 09:31:35 Rusty Russell wrote:
>>>       
>>>> We may still regret not doing *everything* little-endian, but this
>>>> doesn't make it worse.
>>>>         
>>> Hmm, why *don't* we just do everything LE, including the ring?
>>>       
>> Mainly because when requirements are in doubt, simplicity wins, I think.
>>     
>
> Well, I think the definition of simplicity is up for debate in this 
> case... "LE everywhere" is much simpler than "it depends", IMHO.
>   

You couldn't use the vringfd direct ring mapping optimization in KVM for 
PPC without teaching the kernel to access a vring in LE format.  I'm 
pretty sure the later would get rejected on LKML anyway for vringfd as a 
generic mechanism.

Regards,

Anthony Liguori


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

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

* Re: [RFC PATCH] virtio: change config to guest endian.
  2008-04-22 22:13           ` Anthony Liguori
@ 2008-04-22 22:33             ` Hollis Blanchard
  2008-04-22 23:53             ` Hollis Blanchard
  1 sibling, 0 replies; 16+ messages in thread
From: Hollis Blanchard @ 2008-04-22 22:33 UTC (permalink / raw)
  To: Anthony Liguori
  Cc: kvm-devel, Christian Borntraeger, Avi Kivity, virtualization

On Tuesday 22 April 2008 17:13:01 Anthony Liguori wrote:
> Hollis Blanchard wrote:
> > On Tuesday 22 April 2008 16:05:38 Rusty Russell wrote:
> >   
> >> On Wednesday 23 April 2008 06:29:14 Hollis Blanchard wrote:
> >>     
> >>> On Tuesday 22 April 2008 09:31:35 Rusty Russell wrote:
> >>>       
> >>>> We may still regret not doing *everything* little-endian, but this
> >>>> doesn't make it worse.
> >>>>         
> >>> Hmm, why *don't* we just do everything LE, including the ring?
> >>>       
> >> Mainly because when requirements are in doubt, simplicity wins, I think.
> >>     
> >
> > Well, I think the definition of simplicity is up for debate in this 
> > case... "LE everywhere" is much simpler than "it depends", IMHO.
> 
> You couldn't use the vringfd direct ring mapping optimization in KVM for 
> PPC without teaching the kernel to access a vring in LE format.  I'm 
> pretty sure the later would get rejected on LKML anyway for vringfd as a 
> generic mechanism.

You mean vringfd for use cases other than virtual IO drivers? I have a poor 
imagination; can you give some examples?

Even then, it should be possible to have VIO drivers use a different set of 
accessors, just like there are swapping and non-swapping accessors for real 
IO, so I still don't see the problem.

-- 
Hollis Blanchard
IBM Linux Technology Center

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

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

* Re: [RFC PATCH] virtio: change config to guest endian.
  2008-04-22 22:13           ` Anthony Liguori
  2008-04-22 22:33             ` Hollis Blanchard
@ 2008-04-22 23:53             ` Hollis Blanchard
  1 sibling, 0 replies; 16+ messages in thread
From: Hollis Blanchard @ 2008-04-22 23:53 UTC (permalink / raw)
  To: Anthony Liguori
  Cc: kvm-ia64-devel, kvm-devel, Avi Kivity, virtualization,
	Christian Borntraeger

On Tuesday 22 April 2008 17:13:01 Anthony Liguori wrote:
> Hollis Blanchard wrote:
> > On Tuesday 22 April 2008 16:05:38 Rusty Russell wrote:
> >   
> >> On Wednesday 23 April 2008 06:29:14 Hollis Blanchard wrote:
> >>     
> >>> On Tuesday 22 April 2008 09:31:35 Rusty Russell wrote:
> >>>       
> >>>> We may still regret not doing *everything* little-endian, but this
> >>>> doesn't make it worse.
> >>>>         
> >>> Hmm, why *don't* we just do everything LE, including the ring?
> >>>       
> >> Mainly because when requirements are in doubt, simplicity wins, I think.
> >>     
> >
> > Well, I think the definition of simplicity is up for debate in this 
> > case... "LE everywhere" is much simpler than "it depends", IMHO.
> >   
> 
> You couldn't use the vringfd direct ring mapping optimization in KVM for 
> PPC without teaching the kernel to access a vring in LE format.  I'm 
> pretty sure the later would get rejected on LKML anyway for vringfd as a 
> generic mechanism.

(Since the IA64 guys have already implemented BE guests on LE hosts, they 
should be aware of this discussion too, which is why I've CCed them.)

After a short but torturous whiteboard session, followed by a much longer but 
less painful discussion, I'm fine with the virtio device config space being 
BE for PowerPC and LE for x86.

In the future, we can use a feature bit to indicate that PCI config space 
contains an explicit endianness flag. (This will be set to BE or LE, *not* 
to "opposite of normal", because "normal" is also too vague.)

-- 
Hollis Blanchard
IBM Linux Technology Center

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

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

* Re: [RFC PATCH] virtio: change config to guest endian.
  2008-04-22  3:57 [RFC PATCH] virtio: change config to guest endian Rusty Russell
  2008-04-22  7:44 ` Christian Borntraeger
  2008-04-22 11:22 ` Avi Kivity
@ 2008-04-23 10:55 ` Christian Borntraeger
  2008-04-23 12:38   ` Avi Kivity
  2008-04-23 15:53   ` [kvm-devel] " Rusty Russell
  2 siblings, 2 replies; 16+ messages in thread
From: Christian Borntraeger @ 2008-04-23 10:55 UTC (permalink / raw)
  To: kvm-devel; +Cc: Hollis Blanchard, virtualization

Am Dienstag, 22. April 2008 schrieb Rusty Russell:
[...]
> diff -r a098f19a6da5 include/linux/virtio_config.h
> --- a/include/linux/virtio_config.h	Sun Apr 20 14:41:02 2008 +1000
> +++ b/include/linux/virtio_config.h	Sun Apr 20 15:07:45 2008 +1000
> @@ -16,7 +16,7 @@
>  #define VIRTIO_CONFIG_S_FAILED		0x80
> 
>  #ifdef __KERNEL__
> -struct virtio_device;
> +#include <linux/virtio.h>

I just realized, that this breaks make headers_check as we dont export 
virtio.h (and we dont want to export it as it relies on scatterlist.h).

Christian


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

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

* Re: [RFC PATCH] virtio: change config to guest endian.
  2008-04-23 10:55 ` Christian Borntraeger
@ 2008-04-23 12:38   ` Avi Kivity
  2008-04-23 12:47     ` Christian Borntraeger
  2008-04-23 15:53   ` [kvm-devel] " Rusty Russell
  1 sibling, 1 reply; 16+ messages in thread
From: Avi Kivity @ 2008-04-23 12:38 UTC (permalink / raw)
  To: Christian Borntraeger; +Cc: kvm-devel, Hollis Blanchard, virtualization

Christian Borntraeger wrote:
> Am Dienstag, 22. April 2008 schrieb Rusty Russell:
> [...]
>   
>> diff -r a098f19a6da5 include/linux/virtio_config.h
>> --- a/include/linux/virtio_config.h	Sun Apr 20 14:41:02 2008 +1000
>> +++ b/include/linux/virtio_config.h	Sun Apr 20 15:07:45 2008 +1000
>> @@ -16,7 +16,7 @@
>>  #define VIRTIO_CONFIG_S_FAILED		0x80
>>
>>  #ifdef __KERNEL__
>> -struct virtio_device;
>> +#include <linux/virtio.h>
>>     
>
> I just realized, that this breaks make headers_check as we dont export 
> virtio.h (and we dont want to export it as it relies on scatterlist.h).
>
>   

It's guarded by a #ifdef __KERNEL__, so it should be alright.

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


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

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

* Re: [RFC PATCH] virtio: change config to guest endian.
  2008-04-23 12:38   ` Avi Kivity
@ 2008-04-23 12:47     ` Christian Borntraeger
  0 siblings, 0 replies; 16+ messages in thread
From: Christian Borntraeger @ 2008-04-23 12:47 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm-devel, Hollis Blanchard, virtualization

Am Mittwoch, 23. April 2008 schrieb Avi Kivity:
> >>  #define VIRTIO_CONFIG_S_FAILED		0x80
> >>
> >>  #ifdef __KERNEL__
> >> -struct virtio_device;
> >> +#include <linux/virtio.h>
> >>     
> >
> > I just realized, that this breaks make headers_check as we dont export 
> > virtio.h (and we dont want to export it as it relies on scatterlist.h).
> >
> >   
> 
> It's guarded by a #ifdef __KERNEL__, so it should be alright.

Yes, you are right. Thanks

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

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

* Re: [kvm-devel] [RFC PATCH] virtio: change config to guest endian.
  2008-04-23 10:55 ` Christian Borntraeger
  2008-04-23 12:38   ` Avi Kivity
@ 2008-04-23 15:53   ` Rusty Russell
  1 sibling, 0 replies; 16+ messages in thread
From: Rusty Russell @ 2008-04-23 15:53 UTC (permalink / raw)
  To: Christian Borntraeger; +Cc: kvm-devel, Hollis Blanchard, virtualization

On Wednesday 23 April 2008 20:55:50 Christian Borntraeger wrote:
> Am Dienstag, 22. April 2008 schrieb Rusty Russell:
> [...]
>
> > diff -r a098f19a6da5 include/linux/virtio_config.h
> > --- a/include/linux/virtio_config.h	Sun Apr 20 14:41:02 2008 +1000
> > +++ b/include/linux/virtio_config.h	Sun Apr 20 15:07:45 2008 +1000
> > @@ -16,7 +16,7 @@
> >  #define VIRTIO_CONFIG_S_FAILED		0x80
> >
> >  #ifdef __KERNEL__
> > -struct virtio_device;
> > +#include <linux/virtio.h>
>
> I just realized, that this breaks make headers_check as we dont export
> virtio.h (and we dont want to export it as it relies on scatterlist.h).

It's under #ifdef __KERNEL__ though?

Cheers,
Rusty.

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

end of thread, other threads:[~2008-04-23 15:53 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-04-22  3:57 [RFC PATCH] virtio: change config to guest endian Rusty Russell
2008-04-22  7:44 ` Christian Borntraeger
2008-04-22 14:40   ` Rusty Russell
2008-04-22 11:22 ` Avi Kivity
2008-04-22 14:31   ` Rusty Russell
2008-04-22 20:29     ` [kvm-devel] " Hollis Blanchard
2008-04-22 21:05       ` Rusty Russell
2008-04-22 22:08         ` Hollis Blanchard
2008-04-22 22:13           ` Anthony Liguori
2008-04-22 22:33             ` Hollis Blanchard
2008-04-22 23:53             ` Hollis Blanchard
2008-04-22 16:21   ` [kvm-devel] " Hollis Blanchard
2008-04-23 10:55 ` Christian Borntraeger
2008-04-23 12:38   ` Avi Kivity
2008-04-23 12:47     ` Christian Borntraeger
2008-04-23 15:53   ` [kvm-devel] " Rusty Russell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox