All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Bjarke Istrup Pedersen <gurligebis@gentoo.org>,
	Anup Patel <anup.patel@linaro.org>,
	rusty@au1.ibm.com,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	virtualization@lists.linux-foundation.org,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	linux-api@vger.kernel.org, Paolo Bonzini <pbonzini@redhat.com>,
	Pranavkumar Sawargaonkar <pranavkumar@linaro.org>,
	David Miller <davem@davemloft.net>,
	Alexei Starovoitov <ast@plumgrid.com>
Subject: Re: [PATCH v3 04/41] virtio: memory access APIs
Date: Mon, 24 Nov 2014 20:49:09 +0200	[thread overview]
Message-ID: <20141124184909.GA19382@redhat.com> (raw)
In-Reply-To: <CAMuHMdWJA4aUyHo8wq=RX-+0w+PX3NtG1ZgvWgcMwKT5KYT_Tg@mail.gmail.com>

On Mon, Nov 24, 2014 at 01:58:43PM +0100, Geert Uytterhoeven wrote:
> On Mon, Nov 24, 2014 at 1:15 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Mon, Nov 24, 2014 at 01:03:24PM +0100, Geert Uytterhoeven wrote:
> >> On Mon, Nov 24, 2014 at 12:52 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> > virtio 1.0 makes all memory structures LE, so
> >> > we need APIs to conditionally do a byteswap on BE
> >> > architectures.
> >> >
> >> > To make it easier to check code statically,
> >> > add virtio specific types for multi-byte integers
> >> > in memory.
> >> >
> >> > Add low level wrappers that do a byteswap conditionally, these will be
> >> > useful e.g. for vhost.  Add high level wrappers that
> >> > query device endian-ness and act accordingly.
> >>
> >> > diff --git a/include/linux/virtio_byteorder.h b/include/linux/virtio_byteorder.h
> >> > new file mode 100644
> >> > index 0000000..824ed0b
> >> > --- /dev/null
> >> > +++ b/include/linux/virtio_byteorder.h
> >>
> >> > +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;
> >> > +}
> >>
> >> What's wrong with just using le16-to_cpu() ...
> >
> > le16-to_cpu() is simply wrong: virtio needs to be
> > LE or native endian, depending on whether it's running
> > in 0.9 or 1.0 mode.
> 
> IC, that was not clear from the description for this patch.
> I thought it was dependent on BE architectures.
> 
> Nevertheless, any chance you can get rid of the "conditional"?

I don't see how - this is fundamental to any virtio device
that wants to support both 0.9 and 1.0 from the same
codebase.

> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

WARNING: multiple messages have this Message-ID (diff)
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	David Miller <davem@davemloft.net>,
	cornelia.huck@de.ibm.com, rusty@au1.ibm.com,
	Nicholas Bellinger <nab@linux-iscsi.org>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Rusty Russell <rusty@rustcorp.com.au>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Alexei Starovoitov <ast@plumgrid.com>,
	Pranavkumar Sawargaonkar <pranavkumar@linaro.org>,
	Anup Patel <anup.patel@linaro.org>,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	Bjarke Istrup Pedersen <gurligebis@gentoo.org>,
	stephen hemminger <stephen@networkplumber.org>,
	virtualization@lists.linux-foundation.org,
	linux-api@vger.kernel.org
Subject: Re: [PATCH v3 04/41] virtio: memory access APIs
Date: Mon, 24 Nov 2014 20:49:09 +0200	[thread overview]
Message-ID: <20141124184909.GA19382@redhat.com> (raw)
In-Reply-To: <CAMuHMdWJA4aUyHo8wq=RX-+0w+PX3NtG1ZgvWgcMwKT5KYT_Tg@mail.gmail.com>

On Mon, Nov 24, 2014 at 01:58:43PM +0100, Geert Uytterhoeven wrote:
> On Mon, Nov 24, 2014 at 1:15 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Mon, Nov 24, 2014 at 01:03:24PM +0100, Geert Uytterhoeven wrote:
> >> On Mon, Nov 24, 2014 at 12:52 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> > virtio 1.0 makes all memory structures LE, so
> >> > we need APIs to conditionally do a byteswap on BE
> >> > architectures.
> >> >
> >> > To make it easier to check code statically,
> >> > add virtio specific types for multi-byte integers
> >> > in memory.
> >> >
> >> > Add low level wrappers that do a byteswap conditionally, these will be
> >> > useful e.g. for vhost.  Add high level wrappers that
> >> > query device endian-ness and act accordingly.
> >>
> >> > diff --git a/include/linux/virtio_byteorder.h b/include/linux/virtio_byteorder.h
> >> > new file mode 100644
> >> > index 0000000..824ed0b
> >> > --- /dev/null
> >> > +++ b/include/linux/virtio_byteorder.h
> >>
> >> > +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;
> >> > +}
> >>
> >> What's wrong with just using le16-to_cpu() ...
> >
> > le16-to_cpu() is simply wrong: virtio needs to be
> > LE or native endian, depending on whether it's running
> > in 0.9 or 1.0 mode.
> 
> IC, that was not clear from the description for this patch.
> I thought it was dependent on BE architectures.
> 
> Nevertheless, any chance you can get rid of the "conditional"?

I don't see how - this is fundamental to any virtio device
that wants to support both 0.9 and 1.0 from the same
codebase.

> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

  reply	other threads:[~2014-11-24 18:49 UTC|newest]

Thread overview: 91+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-24 11:52 [PATCH v3 00/41] linux: towards virtio-1 guest support Michael S. Tsirkin
2014-11-24 11:52 ` [PATCH v3 01/41] virtio: use u32, not bitmap for struct virtio_device's features Michael S. Tsirkin
2014-11-24 11:52   ` Michael S. Tsirkin
2014-11-24 11:52 ` [PATCH v3 02/41] virtio: add support for 64 bit features Michael S. Tsirkin
2014-11-24 11:52   ` Michael S. Tsirkin
2014-11-24 11:52 ` [PATCH v3 03/41] virtio: add virtio 1.0 feature bit Michael S. Tsirkin
2014-11-24 11:52   ` Michael S. Tsirkin
2014-11-24 11:52 ` [PATCH v3 04/41] virtio: memory access APIs Michael S. Tsirkin
2014-11-24 11:52   ` Michael S. Tsirkin
2014-11-24 12:03   ` Geert Uytterhoeven
2014-11-24 12:03     ` Geert Uytterhoeven
2014-11-24 12:15     ` Michael S. Tsirkin
2014-11-24 12:15       ` Michael S. Tsirkin
2014-11-24 12:58       ` Geert Uytterhoeven
     [not found]       ` <20141124121503.GB14353-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2014-11-24 12:58         ` Geert Uytterhoeven
2014-11-24 12:58           ` Geert Uytterhoeven
2014-11-24 18:49           ` Michael S. Tsirkin [this message]
2014-11-24 18:49             ` Michael S. Tsirkin
2014-11-24 11:53 ` [PATCH v3 05/41] virtio_ring: switch to new " Michael S. Tsirkin
2014-11-24 11:53   ` Michael S. Tsirkin
2014-11-24 11:53 ` [PATCH v3 06/41] virtio_config: endian conversion for v1.0 Michael S. Tsirkin
2014-11-24 11:53   ` Michael S. Tsirkin
2014-11-24 11:53 ` [PATCH v3 07/41] virtio: allow transports to get avail/used addresses Michael S. Tsirkin
2014-11-24 11:53   ` Michael S. Tsirkin
2014-11-24 11:53 ` [PATCH v3 08/41] virtio: set FEATURES_OK Michael S. Tsirkin
2014-11-24 11:53 ` Michael S. Tsirkin
2014-11-24 11:53 ` [PATCH v3 09/41] virtio: simplify feature bit handling Michael S. Tsirkin
2014-11-24 11:53 ` Michael S. Tsirkin
2014-11-24 11:53 ` [PATCH v3 10/41] virtio: add legacy feature table support Michael S. Tsirkin
2014-11-24 11:53 ` Michael S. Tsirkin
2014-11-24 11:53 ` [PATCH v3 11/41] virtio_net: v1.0 endianness Michael S. Tsirkin
2014-11-24 11:53   ` Michael S. Tsirkin
2014-11-24 11:53 ` [PATCH v3 12/41] virtio_blk: v1.0 support Michael S. Tsirkin
2014-11-24 11:53 ` Michael S. Tsirkin
2014-11-24 11:53 ` [PATCH v3 13/41] KVM: s390: Set virtio-ccw transport revision Michael S. Tsirkin
2014-11-24 11:53 ` [PATCH v3 14/41] KVM: s390: virtio-ccw revision 1 SET_VQ Michael S. Tsirkin
2014-11-24 11:53 ` [PATCH v3 15/41] KVM: s390 allow virtio_ccw status writes to fail Michael S. Tsirkin
2014-11-24 11:53 ` [PATCH v3 16/41] KVM: s390: enable virtio-ccw revision 1 Michael S. Tsirkin
2014-11-24 11:53 ` [PATCH v3 17/41] virtio_blk: make serial attribute static Michael S. Tsirkin
2014-11-24 11:53   ` Michael S. Tsirkin
2014-11-24 11:53 ` [PATCH v3 18/41] virtio_blk: fix race at module removal Michael S. Tsirkin
2014-11-24 11:53   ` Michael S. Tsirkin
2014-11-24 11:54 ` [PATCH v3 19/41] virtio_net: pass vi around Michael S. Tsirkin
2014-11-24 11:54   ` Michael S. Tsirkin
2014-11-24 11:54 ` [PATCH v3 20/41] virtio_net: get rid of virtio_net_hdr/skb_vnet_hdr Michael S. Tsirkin
2014-11-24 11:54 ` Michael S. Tsirkin
2014-11-24 11:54 ` [PATCH v3 21/41] virtio_net: stricter short buffer length checks Michael S. Tsirkin
2014-11-24 11:54   ` Michael S. Tsirkin
2014-11-24 11:54 ` [PATCH v3 22/41] virtio_net: bigger header when VERSION_1 is set Michael S. Tsirkin
2014-11-24 11:54   ` Michael S. Tsirkin
2014-11-24 11:54 ` [PATCH v3 23/41] virtio_net: enable v1.0 support Michael S. Tsirkin
2014-11-24 11:54 ` Michael S. Tsirkin
2014-11-24 11:54 ` [PATCH v3 24/41] vhost: add memory access wrappers Michael S. Tsirkin
2014-11-24 11:54   ` Michael S. Tsirkin
2014-11-24 11:54 ` [PATCH v3 25/41] vhost/net: force len for TX to host endian Michael S. Tsirkin
2014-11-24 11:54 ` Michael S. Tsirkin
2014-11-24 11:54 ` [PATCH v3 26/41] vhost: virtio 1.0 endian-ness support Michael S. Tsirkin
2014-11-24 11:54 ` Michael S. Tsirkin
2014-11-24 14:28   ` Cedric Le Goater
2014-11-24 14:28   ` Cedric Le Goater
2014-11-24 20:28     ` Michael S. Tsirkin
2014-11-24 20:28       ` Michael S. Tsirkin
2014-11-24 11:54 ` [PATCH v3 27/41] vhost: make features 64 bit Michael S. Tsirkin
2014-11-24 11:54   ` Michael S. Tsirkin
2014-11-24 11:54 ` [PATCH v3 28/41] vhost/net: virtio 1.0 byte swap Michael S. Tsirkin
2014-11-24 11:54 ` Michael S. Tsirkin
2014-11-24 11:54 ` [PATCH v3 29/41] vhost/net: larger header for virtio 1.0 Michael S. Tsirkin
2014-11-24 11:54 ` Michael S. Tsirkin
2014-11-24 11:54 ` [PATCH v3 30/41] vhost/net: enable " Michael S. Tsirkin
2014-11-24 11:54   ` Michael S. Tsirkin
2014-11-24 11:54 ` [PATCH v3 31/41] vhost/net: suppress compiler warning Michael S. Tsirkin
2014-11-24 11:54 ` Michael S. Tsirkin
2014-11-24 11:54 ` [PATCH v3 32/41] tun: move internal flag defines out of uapi Michael S. Tsirkin
2014-11-24 11:54 ` [PATCH v3 33/41] tun: drop most type defines Michael S. Tsirkin
2014-11-24 11:55 ` [PATCH v3 34/41] tun: add VNET_LE flag Michael S. Tsirkin
2014-11-24 11:55 ` [PATCH v3 35/41] tun: TUN_VNET_LE support, fix sparse warnings for virtio headers Michael S. Tsirkin
2014-11-24 11:55 ` [PATCH v3 36/41] macvtap: TUN_VNET_HDR support Michael S. Tsirkin
2014-11-24 11:55 ` [PATCH v3 37/41] virtio_scsi: v1.0 support Michael S. Tsirkin
2014-11-24 11:55 ` Michael S. Tsirkin
2014-11-24 11:55 ` [PATCH v3 38/41] virtio_scsi: move to uapi Michael S. Tsirkin
2014-11-24 11:55 ` [PATCH v3 39/41] virtio_scsi: export to userspace Michael S. Tsirkin
2014-11-24 11:55 ` Michael S. Tsirkin
2014-11-24 11:55 ` [PATCH v3 40/41] vhost/scsi: partial virtio 1.0 support Michael S. Tsirkin
2014-11-24 11:55 ` Michael S. Tsirkin
2014-11-24 11:55 ` [PATCH v3 41/41] af_packet: virtio 1.0 stubs Michael S. Tsirkin
2014-11-24 12:15 ` [PATCH v3 00/41] linux: towards virtio-1 guest support Paolo Bonzini
2014-11-24 18:35 ` David Miller
2014-11-24 18:48 ` Cornelia Huck
2014-11-24 19:12   ` Michael S. Tsirkin
2014-11-25  9:05     ` Cornelia Huck
2014-11-24 21:25 ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141124184909.GA19382@redhat.com \
    --to=mst@redhat.com \
    --cc=anup.patel@linaro.org \
    --cc=ast@plumgrid.com \
    --cc=davem@davemloft.net \
    --cc=geert@linux-m68k.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=gurligebis@gentoo.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=pranavkumar@linaro.org \
    --cc=rusty@au1.ibm.com \
    --cc=sakari.ailus@linux.intel.com \
    --cc=virtualization@lists.linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.