Re: [PATCH v5 6/8] virtio: add explicit big-endian support to memory accessors
On Fri, 24 Apr 2015 09:04:21 +0200
Cornelia Huck wrote:
> On Thu, 23 Apr 2015 21:27:19 +0200
> Thomas Huth wrote:
>
Thomas's e-mail did not make it to my mailbox... weird. :-\
> > On Thu, 23 Apr 2015 17:29:06 +0200
> > Greg Kurz wrote:
> >
> > > The current memory accessors logic is:
> > > - little endian if little_endian
> > > - native endian (i.e. no byteswap) if !little_endian
> > >
> > > If we want to fully support cross-endian vhost, we also need to be
> > > able to convert to big endian.
> > >
> > > Instead of changing the little_endian argument to some 3-value enum, this
> > > patch changes the logic to:
> > > - little endian if little_endian
> > > - big endian if !little_endian
> > >
> > > The native endian case is handled by all users with a trivial helper. This
> > > patch doesn't change any functionality, nor it does add overhead.
> > >
> > > Signed-off-by: Greg Kurz
> > > ---
> > >
> > > Changes since v4:
> > > - style fixes (I have chosen if ... else in most places to stay below
> > > 80 columns, with the notable exception of the vhost helper which gets
> > > shorten in a later patch)
> > >
> > > drivers/net/macvtap.c|5 -
> > > drivers/net/tun.c|5 -
> > > drivers/vhost/vhost.h|2 +-
> > > include/linux/virtio_byteorder.h | 24 ++--
> > > include/linux/virtio_config.h|5 -
> > > include/linux/vringh.h |2 +-
> > > 6 files changed, 28 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> > > index a2f2958..6cf6b3e 100644
> > > --- a/drivers/net/macvtap.c
> > > +++ b/drivers/net/macvtap.c
> > > @@ -51,7 +51,10 @@ struct macvtap_queue {
> > >
> > > static inline bool macvtap_is_little_endian(struct macvtap_queue *q)
> > > {
> > > - return q->flags & MACVTAP_VNET_LE;
> > > + if (q->flags & MACVTAP_VNET_LE)
> > > + return true;
> > > + else
> > > + return virtio_legacy_is_little_endian();
> >
> > simply:
> >
> > return (q->flags & MACVTAP_VNET_LE) ||
> >virtio_legacy_is_little_endian();
> >
> > ?
>
> ISTR that MST preferred the current notation, but I like either your
> way or ?: (with linebreak) even better.
>
MST did not like the initial notation I had used actually. FWIW I
like the simplicity of Thomas's suggestion... even better than ?:
which is:
some_LE_check() ? true : some_default_endianness()
> >
> > > }
> > >
> (...)
> > > diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> > > index 6a49960..954c657 100644
> > > --- a/drivers/vhost/vhost.h
> > > +++ b/drivers/vhost/vhost.h
> > > @@ -175,7 +175,7 @@ static inline bool vhost_has_feature(struct
> > > vhost_virtqueue *vq, int bit)
> > >
> > > static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq)
> > > {
> > > - return vhost_has_feature(vq, VIRTIO_F_VERSION_1);
> > > + return vhost_has_feature(vq, VIRTIO_F_VERSION_1) ? true :
> > > virtio_legacy_is_little_endian();
> > > }
> >
> > That line is way longer than 80 characters ... may I suggest to switch
> > at least here to:
> >
> > return vhost_has_feature(vq, VIRTIO_F_VERSION_1) ||
> >virtio_legacy_is_little_endian();
>
> I think the line will collapse in a further patch anyway.
>
Yes, as mentionned in the changelog :)
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 6/8] virtio: add explicit big-endian support to memory accessors
On Thu, 23 Apr 2015 21:27:19 +0200
Thomas Huth wrote:
> On Thu, 23 Apr 2015 17:29:06 +0200
> Greg Kurz wrote:
>
> > The current memory accessors logic is:
> > - little endian if little_endian
> > - native endian (i.e. no byteswap) if !little_endian
> >
> > If we want to fully support cross-endian vhost, we also need to be
> > able to convert to big endian.
> >
> > Instead of changing the little_endian argument to some 3-value enum, this
> > patch changes the logic to:
> > - little endian if little_endian
> > - big endian if !little_endian
> >
> > The native endian case is handled by all users with a trivial helper. This
> > patch doesn't change any functionality, nor it does add overhead.
> >
> > Signed-off-by: Greg Kurz
> > ---
> >
> > Changes since v4:
> > - style fixes (I have chosen if ... else in most places to stay below
> > 80 columns, with the notable exception of the vhost helper which gets
> > shorten in a later patch)
> >
> > drivers/net/macvtap.c|5 -
> > drivers/net/tun.c|5 -
> > drivers/vhost/vhost.h|2 +-
> > include/linux/virtio_byteorder.h | 24 ++--
> > include/linux/virtio_config.h|5 -
> > include/linux/vringh.h |2 +-
> > 6 files changed, 28 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> > index a2f2958..6cf6b3e 100644
> > --- a/drivers/net/macvtap.c
> > +++ b/drivers/net/macvtap.c
> > @@ -51,7 +51,10 @@ struct macvtap_queue {
> >
> > static inline bool macvtap_is_little_endian(struct macvtap_queue *q)
> > {
> > - return q->flags & MACVTAP_VNET_LE;
> > + if (q->flags & MACVTAP_VNET_LE)
> > + return true;
> > + else
> > + return virtio_legacy_is_little_endian();
>
> simply:
>
> return (q->flags & MACVTAP_VNET_LE) ||
> virtio_legacy_is_little_endian();
>
> ?
ISTR that MST preferred the current notation, but I like either your
way or ?: (with linebreak) even better.
>
> > }
> >
(...)
> > diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> > index 6a49960..954c657 100644
> > --- a/drivers/vhost/vhost.h
> > +++ b/drivers/vhost/vhost.h
> > @@ -175,7 +175,7 @@ static inline bool vhost_has_feature(struct
> > vhost_virtqueue *vq, int bit)
> >
> > static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq)
> > {
> > - return vhost_has_feature(vq, VIRTIO_F_VERSION_1);
> > + return vhost_has_feature(vq, VIRTIO_F_VERSION_1) ? true :
> > virtio_legacy_is_little_endian();
> > }
>
> That line is way longer than 80 characters ... may I suggest to switch
> at least here to:
>
> return vhost_has_feature(vq, VIRTIO_F_VERSION_1) ||
> virtio_legacy_is_little_endian();
I think the line will collapse in a further patch anyway.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 6/8] virtio: add explicit big-endian support to memory accessors
On Thu, 23 Apr 2015 21:27:19 +0200
Thomas Huth th...@redhat.com wrote:
On Thu, 23 Apr 2015 17:29:06 +0200
Greg Kurz gk...@linux.vnet.ibm.com wrote:
The current memory accessors logic is:
- little endian if little_endian
- native endian (i.e. no byteswap) if !little_endian
If we want to fully support cross-endian vhost, we also need to be
able to convert to big endian.
Instead of changing the little_endian argument to some 3-value enum, this
patch changes the logic to:
- little endian if little_endian
- big endian if !little_endian
The native endian case is handled by all users with a trivial helper. This
patch doesn't change any functionality, nor it does add overhead.
Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com
---
Changes since v4:
- style fixes (I have chosen if ... else in most places to stay below
80 columns, with the notable exception of the vhost helper which gets
shorten in a later patch)
drivers/net/macvtap.c|5 -
drivers/net/tun.c|5 -
drivers/vhost/vhost.h|2 +-
include/linux/virtio_byteorder.h | 24 ++--
include/linux/virtio_config.h|5 -
include/linux/vringh.h |2 +-
6 files changed, 28 insertions(+), 15 deletions(-)
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index a2f2958..6cf6b3e 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -51,7 +51,10 @@ struct macvtap_queue {
static inline bool macvtap_is_little_endian(struct macvtap_queue *q)
{
- return q-flags MACVTAP_VNET_LE;
+ if (q-flags MACVTAP_VNET_LE)
+ return true;
+ else
+ return virtio_legacy_is_little_endian();
simply:
return (q-flags MACVTAP_VNET_LE) ||
virtio_legacy_is_little_endian();
?
ISTR that MST preferred the current notation, but I like either your
way or ?: (with linebreak) even better.
}
(...)
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 6a49960..954c657 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -175,7 +175,7 @@ static inline bool vhost_has_feature(struct
vhost_virtqueue *vq, int bit)
static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq)
{
- return vhost_has_feature(vq, VIRTIO_F_VERSION_1);
+ return vhost_has_feature(vq, VIRTIO_F_VERSION_1) ? true :
virtio_legacy_is_little_endian();
}
That line is way longer than 80 characters ... may I suggest to switch
at least here to:
return vhost_has_feature(vq, VIRTIO_F_VERSION_1) ||
virtio_legacy_is_little_endian();
I think the line will collapse in a further patch anyway.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 6/8] virtio: add explicit big-endian support to memory accessors
On Fri, 24 Apr 2015 09:04:21 +0200
Cornelia Huck cornelia.h...@de.ibm.com wrote:
On Thu, 23 Apr 2015 21:27:19 +0200
Thomas Huth th...@redhat.com wrote:
Thomas's e-mail did not make it to my mailbox... weird. :-\
On Thu, 23 Apr 2015 17:29:06 +0200
Greg Kurz gk...@linux.vnet.ibm.com wrote:
The current memory accessors logic is:
- little endian if little_endian
- native endian (i.e. no byteswap) if !little_endian
If we want to fully support cross-endian vhost, we also need to be
able to convert to big endian.
Instead of changing the little_endian argument to some 3-value enum, this
patch changes the logic to:
- little endian if little_endian
- big endian if !little_endian
The native endian case is handled by all users with a trivial helper. This
patch doesn't change any functionality, nor it does add overhead.
Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com
---
Changes since v4:
- style fixes (I have chosen if ... else in most places to stay below
80 columns, with the notable exception of the vhost helper which gets
shorten in a later patch)
drivers/net/macvtap.c|5 -
drivers/net/tun.c|5 -
drivers/vhost/vhost.h|2 +-
include/linux/virtio_byteorder.h | 24 ++--
include/linux/virtio_config.h|5 -
include/linux/vringh.h |2 +-
6 files changed, 28 insertions(+), 15 deletions(-)
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index a2f2958..6cf6b3e 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -51,7 +51,10 @@ struct macvtap_queue {
static inline bool macvtap_is_little_endian(struct macvtap_queue *q)
{
- return q-flags MACVTAP_VNET_LE;
+ if (q-flags MACVTAP_VNET_LE)
+ return true;
+ else
+ return virtio_legacy_is_little_endian();
simply:
return (q-flags MACVTAP_VNET_LE) ||
virtio_legacy_is_little_endian();
?
ISTR that MST preferred the current notation, but I like either your
way or ?: (with linebreak) even better.
MST did not like the initial notation I had used actually. FWIW I
like the simplicity of Thomas's suggestion... even better than ?:
which is:
some_LE_check() ? true : some_default_endianness()
}
(...)
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 6a49960..954c657 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -175,7 +175,7 @@ static inline bool vhost_has_feature(struct
vhost_virtqueue *vq, int bit)
static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq)
{
- return vhost_has_feature(vq, VIRTIO_F_VERSION_1);
+ return vhost_has_feature(vq, VIRTIO_F_VERSION_1) ? true :
virtio_legacy_is_little_endian();
}
That line is way longer than 80 characters ... may I suggest to switch
at least here to:
return vhost_has_feature(vq, VIRTIO_F_VERSION_1) ||
virtio_legacy_is_little_endian();
I think the line will collapse in a further patch anyway.
Yes, as mentionned in the changelog :)
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 6/8] virtio: add explicit big-endian support to memory accessors
On Thu, 23 Apr 2015 17:29:06 +0200
Greg Kurz wrote:
> The current memory accessors logic is:
> - little endian if little_endian
> - native endian (i.e. no byteswap) if !little_endian
>
> If we want to fully support cross-endian vhost, we also need to be
> able to convert to big endian.
>
> Instead of changing the little_endian argument to some 3-value enum, this
> patch changes the logic to:
> - little endian if little_endian
> - big endian if !little_endian
>
> The native endian case is handled by all users with a trivial helper. This
> patch doesn't change any functionality, nor it does add overhead.
>
> Signed-off-by: Greg Kurz
> ---
>
> Changes since v4:
> - style fixes (I have chosen if ... else in most places to stay below
> 80 columns, with the notable exception of the vhost helper which gets
> shorten in a later patch)
>
> drivers/net/macvtap.c|5 -
> drivers/net/tun.c|5 -
> drivers/vhost/vhost.h|2 +-
> include/linux/virtio_byteorder.h | 24 ++--
> include/linux/virtio_config.h|5 -
> include/linux/vringh.h |2 +-
> 6 files changed, 28 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> index a2f2958..6cf6b3e 100644
> --- a/drivers/net/macvtap.c
> +++ b/drivers/net/macvtap.c
> @@ -51,7 +51,10 @@ struct macvtap_queue {
>
> static inline bool macvtap_is_little_endian(struct macvtap_queue *q)
> {
> - return q->flags & MACVTAP_VNET_LE;
> + if (q->flags & MACVTAP_VNET_LE)
> + return true;
> + else
> + return virtio_legacy_is_little_endian();
simply:
return (q->flags & MACVTAP_VNET_LE) ||
virtio_legacy_is_little_endian();
?
> }
>
> static inline u16 macvtap16_to_cpu(struct macvtap_queue *q, __virtio16 val)
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 3c3d6c0..5b044d4 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -208,7 +208,10 @@ struct tun_struct {
>
> static inline bool tun_is_little_endian(struct tun_struct *tun)
> {
> - return tun->flags & TUN_VNET_LE;
> + if (tun->flags & TUN_VNET_LE)
> + return true;
> + else
> + return virtio_legacy_is_little_endian();
dito?
> }
>
> static inline u16 tun16_to_cpu(struct tun_struct *tun, __virtio16 val)
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index 6a49960..954c657 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -175,7 +175,7 @@ static inline bool vhost_has_feature(struct
> vhost_virtqueue *vq, int bit)
>
> static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq)
> {
> - return vhost_has_feature(vq, VIRTIO_F_VERSION_1);
> + return vhost_has_feature(vq, VIRTIO_F_VERSION_1) ? true :
> virtio_legacy_is_little_endian();
> }
That line is way longer than 80 characters ... may I suggest to switch
at least here to:
return vhost_has_feature(vq, VIRTIO_F_VERSION_1) ||
virtio_legacy_is_little_endian();
?
Apart from the cosmetics, the patch looks good to me.
Thomas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v5 6/8] virtio: add explicit big-endian support to memory accessors
The current memory accessors logic is:
- little endian if little_endian
- native endian (i.e. no byteswap) if !little_endian
If we want to fully support cross-endian vhost, we also need to be
able to convert to big endian.
Instead of changing the little_endian argument to some 3-value enum, this
patch changes the logic to:
- little endian if little_endian
- big endian if !little_endian
The native endian case is handled by all users with a trivial helper. This
patch doesn't change any functionality, nor it does add overhead.
Signed-off-by: Greg Kurz
---
Changes since v4:
- style fixes (I have chosen if ... else in most places to stay below
80 columns, with the notable exception of the vhost helper which gets
shorten in a later patch)
drivers/net/macvtap.c|5 -
drivers/net/tun.c|5 -
drivers/vhost/vhost.h|2 +-
include/linux/virtio_byteorder.h | 24 ++--
include/linux/virtio_config.h|5 -
include/linux/vringh.h |2 +-
6 files changed, 28 insertions(+), 15 deletions(-)
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index a2f2958..6cf6b3e 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -51,7 +51,10 @@ struct macvtap_queue {
static inline bool macvtap_is_little_endian(struct macvtap_queue *q)
{
- return q->flags & MACVTAP_VNET_LE;
+ if (q->flags & MACVTAP_VNET_LE)
+ return true;
+ else
+ return virtio_legacy_is_little_endian();
}
static inline u16 macvtap16_to_cpu(struct macvtap_queue *q, __virtio16 val)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 3c3d6c0..5b044d4 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -208,7 +208,10 @@ struct tun_struct {
static inline bool tun_is_little_endian(struct tun_struct *tun)
{
- return tun->flags & TUN_VNET_LE;
+ if (tun->flags & TUN_VNET_LE)
+ return true;
+ else
+ return virtio_legacy_is_little_endian();
}
static inline u16 tun16_to_cpu(struct tun_struct *tun, __virtio16 val)
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 6a49960..954c657 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -175,7 +175,7 @@ static inline bool vhost_has_feature(struct vhost_virtqueue
*vq, int bit)
static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq)
{
- return vhost_has_feature(vq, VIRTIO_F_VERSION_1);
+ return vhost_has_feature(vq, VIRTIO_F_VERSION_1) ? true :
virtio_legacy_is_little_endian();
}
/* Memory accessors */
diff --git a/include/linux/virtio_byteorder.h b/include/linux/virtio_byteorder.h
index 51865d0..ce63a2c 100644
--- a/include/linux/virtio_byteorder.h
+++ b/include/linux/virtio_byteorder.h
@@ -3,17 +3,21 @@
#include
#include
-/*
- * Low-level memory accessors for handling virtio in modern little endian and
in
- * compatibility native endian format.
- */
+static inline bool virtio_legacy_is_little_endian(void)
+{
+#ifdef __LITTLE_ENDIAN
+ return true;
+#else
+ return false;
+#endif
+}
static inline u16 __virtio16_to_cpu(bool little_endian, __virtio16 val)
{
if (little_endian)
return le16_to_cpu((__force __le16)val);
else
- return (__force u16)val;
+ return be16_to_cpu((__force __be16)val);
}
static inline __virtio16 __cpu_to_virtio16(bool little_endian, u16 val)
@@ -21,7 +25,7 @@ static inline __virtio16 __cpu_to_virtio16(bool
little_endian, u16 val)
if (little_endian)
return (__force __virtio16)cpu_to_le16(val);
else
- return (__force __virtio16)val;
+ return (__force __virtio16)cpu_to_be16(val);
}
static inline u32 __virtio32_to_cpu(bool little_endian, __virtio32 val)
@@ -29,7 +33,7 @@ static inline u32 __virtio32_to_cpu(bool little_endian,
__virtio32 val)
if (little_endian)
return le32_to_cpu((__force __le32)val);
else
- return (__force u32)val;
+ return be32_to_cpu((__force __be32)val);
}
static inline __virtio32 __cpu_to_virtio32(bool little_endian, u32 val)
@@ -37,7 +41,7 @@ static inline __virtio32 __cpu_to_virtio32(bool
little_endian, u32 val)
if (little_endian)
return (__force __virtio32)cpu_to_le32(val);
else
- return (__force __virtio32)val;
+ return (__force __virtio32)cpu_to_be32(val);
}
static inline u64 __virtio64_to_cpu(bool little_endian, __virtio64 val)
@@ -45,7 +49,7 @@ static inline u64 __virtio64_to_cpu(bool little_endian,
__virtio64 val)
if (little_endian)
return le64_to_cpu((__force __le64)val);
else
- return (__force u64)val;
+ return be64_to_cpu((__force __be64)val);
}
static inline __virtio64 __cpu_to_virtio64(bool little_endian, u64 val)
@@ -53,7 +57,7 @@ static
[PATCH v5 6/8] virtio: add explicit big-endian support to memory accessors
The current memory accessors logic is:
- little endian if little_endian
- native endian (i.e. no byteswap) if !little_endian
If we want to fully support cross-endian vhost, we also need to be
able to convert to big endian.
Instead of changing the little_endian argument to some 3-value enum, this
patch changes the logic to:
- little endian if little_endian
- big endian if !little_endian
The native endian case is handled by all users with a trivial helper. This
patch doesn't change any functionality, nor it does add overhead.
Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com
---
Changes since v4:
- style fixes (I have chosen if ... else in most places to stay below
80 columns, with the notable exception of the vhost helper which gets
shorten in a later patch)
drivers/net/macvtap.c|5 -
drivers/net/tun.c|5 -
drivers/vhost/vhost.h|2 +-
include/linux/virtio_byteorder.h | 24 ++--
include/linux/virtio_config.h|5 -
include/linux/vringh.h |2 +-
6 files changed, 28 insertions(+), 15 deletions(-)
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index a2f2958..6cf6b3e 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -51,7 +51,10 @@ struct macvtap_queue {
static inline bool macvtap_is_little_endian(struct macvtap_queue *q)
{
- return q-flags MACVTAP_VNET_LE;
+ if (q-flags MACVTAP_VNET_LE)
+ return true;
+ else
+ return virtio_legacy_is_little_endian();
}
static inline u16 macvtap16_to_cpu(struct macvtap_queue *q, __virtio16 val)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 3c3d6c0..5b044d4 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -208,7 +208,10 @@ struct tun_struct {
static inline bool tun_is_little_endian(struct tun_struct *tun)
{
- return tun-flags TUN_VNET_LE;
+ if (tun-flags TUN_VNET_LE)
+ return true;
+ else
+ return virtio_legacy_is_little_endian();
}
static inline u16 tun16_to_cpu(struct tun_struct *tun, __virtio16 val)
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 6a49960..954c657 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -175,7 +175,7 @@ static inline bool vhost_has_feature(struct vhost_virtqueue
*vq, int bit)
static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq)
{
- return vhost_has_feature(vq, VIRTIO_F_VERSION_1);
+ return vhost_has_feature(vq, VIRTIO_F_VERSION_1) ? true :
virtio_legacy_is_little_endian();
}
/* Memory accessors */
diff --git a/include/linux/virtio_byteorder.h b/include/linux/virtio_byteorder.h
index 51865d0..ce63a2c 100644
--- a/include/linux/virtio_byteorder.h
+++ b/include/linux/virtio_byteorder.h
@@ -3,17 +3,21 @@
#include linux/types.h
#include uapi/linux/virtio_types.h
-/*
- * Low-level memory accessors for handling virtio in modern little endian and
in
- * compatibility native endian format.
- */
+static inline bool virtio_legacy_is_little_endian(void)
+{
+#ifdef __LITTLE_ENDIAN
+ return true;
+#else
+ return false;
+#endif
+}
static inline u16 __virtio16_to_cpu(bool little_endian, __virtio16 val)
{
if (little_endian)
return le16_to_cpu((__force __le16)val);
else
- return (__force u16)val;
+ return be16_to_cpu((__force __be16)val);
}
static inline __virtio16 __cpu_to_virtio16(bool little_endian, u16 val)
@@ -21,7 +25,7 @@ static inline __virtio16 __cpu_to_virtio16(bool
little_endian, u16 val)
if (little_endian)
return (__force __virtio16)cpu_to_le16(val);
else
- return (__force __virtio16)val;
+ return (__force __virtio16)cpu_to_be16(val);
}
static inline u32 __virtio32_to_cpu(bool little_endian, __virtio32 val)
@@ -29,7 +33,7 @@ static inline u32 __virtio32_to_cpu(bool little_endian,
__virtio32 val)
if (little_endian)
return le32_to_cpu((__force __le32)val);
else
- return (__force u32)val;
+ return be32_to_cpu((__force __be32)val);
}
static inline __virtio32 __cpu_to_virtio32(bool little_endian, u32 val)
@@ -37,7 +41,7 @@ static inline __virtio32 __cpu_to_virtio32(bool
little_endian, u32 val)
if (little_endian)
return (__force __virtio32)cpu_to_le32(val);
else
- return (__force __virtio32)val;
+ return (__force __virtio32)cpu_to_be32(val);
}
static inline u64 __virtio64_to_cpu(bool little_endian, __virtio64 val)
@@ -45,7 +49,7 @@ static inline u64 __virtio64_to_cpu(bool little_endian,
__virtio64 val)
if (little_endian)
return le64_to_cpu((__force __le64)val);
else
- return (__force u64)val;
+ return be64_to_cpu((__force __be64)val);
}
static inline __virtio64
Re: [PATCH v5 6/8] virtio: add explicit big-endian support to memory accessors
On Thu, 23 Apr 2015 17:29:06 +0200
Greg Kurz gk...@linux.vnet.ibm.com wrote:
The current memory accessors logic is:
- little endian if little_endian
- native endian (i.e. no byteswap) if !little_endian
If we want to fully support cross-endian vhost, we also need to be
able to convert to big endian.
Instead of changing the little_endian argument to some 3-value enum, this
patch changes the logic to:
- little endian if little_endian
- big endian if !little_endian
The native endian case is handled by all users with a trivial helper. This
patch doesn't change any functionality, nor it does add overhead.
Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com
---
Changes since v4:
- style fixes (I have chosen if ... else in most places to stay below
80 columns, with the notable exception of the vhost helper which gets
shorten in a later patch)
drivers/net/macvtap.c|5 -
drivers/net/tun.c|5 -
drivers/vhost/vhost.h|2 +-
include/linux/virtio_byteorder.h | 24 ++--
include/linux/virtio_config.h|5 -
include/linux/vringh.h |2 +-
6 files changed, 28 insertions(+), 15 deletions(-)
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index a2f2958..6cf6b3e 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -51,7 +51,10 @@ struct macvtap_queue {
static inline bool macvtap_is_little_endian(struct macvtap_queue *q)
{
- return q-flags MACVTAP_VNET_LE;
+ if (q-flags MACVTAP_VNET_LE)
+ return true;
+ else
+ return virtio_legacy_is_little_endian();
simply:
return (q-flags MACVTAP_VNET_LE) ||
virtio_legacy_is_little_endian();
?
}
static inline u16 macvtap16_to_cpu(struct macvtap_queue *q, __virtio16 val)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 3c3d6c0..5b044d4 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -208,7 +208,10 @@ struct tun_struct {
static inline bool tun_is_little_endian(struct tun_struct *tun)
{
- return tun-flags TUN_VNET_LE;
+ if (tun-flags TUN_VNET_LE)
+ return true;
+ else
+ return virtio_legacy_is_little_endian();
dito?
}
static inline u16 tun16_to_cpu(struct tun_struct *tun, __virtio16 val)
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 6a49960..954c657 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -175,7 +175,7 @@ static inline bool vhost_has_feature(struct
vhost_virtqueue *vq, int bit)
static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq)
{
- return vhost_has_feature(vq, VIRTIO_F_VERSION_1);
+ return vhost_has_feature(vq, VIRTIO_F_VERSION_1) ? true :
virtio_legacy_is_little_endian();
}
That line is way longer than 80 characters ... may I suggest to switch
at least here to:
return vhost_has_feature(vq, VIRTIO_F_VERSION_1) ||
virtio_legacy_is_little_endian();
?
Apart from the cosmetics, the patch looks good to me.
Thomas
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
