* [Qemu-devel] [PATCH 0/8] virtio for endian curious guests Take #2 @ 2013-08-12 7:59 Rusty Russell 2013-08-12 7:59 ` [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio Rusty Russell ` (2 more replies) 0 siblings, 3 replies; 36+ messages in thread From: Rusty Russell @ 2013-08-12 7:59 UTC (permalink / raw) To: qemu-devel Cc: Peter Maydell, Rusty Russell, Andreas Färber, Anthony Liguori The driver parts (patches 3-8) are unchanged, so didn't repost. 1) Rebased onto more recent git tree. 2) New stub is virtio_get_byteswap() 3) PPC64 uses endianness of first CPU interrupt vectors, not CPU endiannes. Hope this one is closer... Rusty. Rusty Russell (8): virtio_get_byteswap: function for endian-ambivalent targets using virtio. target-ppc: ppc64 target's virtio can be either endian. virtio: allow byte swapping for vring and config access hw/net/virtio-net: use virtio wrappers to access headers. hw/net/virtio-balloon: use virtio wrappers to access page frame numbers. hw/block/virtio-blk: use virtio wrappers to access headers. hw/scsi/virtio-scsi: use virtio wrappers to access headers. hw/char/virtio-serial-bus: use virtio wrappers to access headers. hw/block/virtio-blk.c | 35 +++++----- hw/char/virtio-serial-bus.c | 34 +++++----- hw/net/virtio-net.c | 15 +++-- hw/scsi/virtio-scsi.c | 33 +++++----- hw/virtio/virtio-balloon.c | 3 +- hw/virtio/virtio.c | 29 +++++---- include/hw/virtio/virtio-access.h | 130 ++++++++++++++++++++++++++++++++++++++ include/hw/virtio/virtio.h | 2 + stubs/Makefile.objs | 1 + stubs/virtio_get_byteswap.c | 6 ++ target-ppc/misc_helper.c | 9 +++ 11 files changed, 226 insertions(+), 71 deletions(-) create mode 100644 include/hw/virtio/virtio-access.h create mode 100644 stubs/virtio_get_byteswap.c -- 1.8.1.2 ^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio. 2013-08-12 7:59 [Qemu-devel] [PATCH 0/8] virtio for endian curious guests Take #2 Rusty Russell @ 2013-08-12 7:59 ` Rusty Russell 2013-08-12 9:28 ` Benjamin Herrenschmidt 2013-08-12 7:59 ` [Qemu-devel] [PATCH 2/8] target-ppc: ppc64 target's virtio can be either endian Rusty Russell 2013-08-12 7:59 ` [Qemu-devel] [PATCH 3/8] virtio: allow byte swapping for vring and config access Rusty Russell 2 siblings, 1 reply; 36+ messages in thread From: Rusty Russell @ 2013-08-12 7:59 UTC (permalink / raw) To: qemu-devel; +Cc: Rusty Russell virtio data structures are defined as "target endian", which assumes that's a fixed value. In fact, that actually means it's platform-specific. Hopefully the OASIS virtio 1.0 spec will fix this. Meanwhile, create a hook for little endian ppc. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> --- include/hw/virtio/virtio-access.h | 130 ++++++++++++++++++++++++++++++++++++++ include/hw/virtio/virtio.h | 2 + stubs/Makefile.objs | 1 + stubs/virtio_get_byteswap.c | 6 ++ 4 files changed, 139 insertions(+) create mode 100644 include/hw/virtio/virtio-access.h create mode 100644 stubs/virtio_get_byteswap.c diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h new file mode 100644 index 0000000..01d7dd6 --- /dev/null +++ b/include/hw/virtio/virtio-access.h @@ -0,0 +1,130 @@ +/* + * Virtio Accessor Support: In case your target can change endian. + * + * Copyright IBM, Corp. 2013 + * + * Authors: + * Rusty Russell <rusty@au.ibm.com> + * + * This work is licensed under the terms of the GNU GPL, version 2. See + * the COPYING file in the top-level directory. + * + */ +#ifndef _QEMU_VIRTIO_ACCESS_H +#define _QEMU_VIRTIO_ACCESS_H +#include "hw/virtio/virtio.h" + +static inline uint16_t virtio_lduw_phys(hwaddr pa) +{ + if (virtio_get_byteswap()) { + return bswap16(lduw_phys(pa)); + } + return lduw_phys(pa); + +} + +static inline uint32_t virtio_ldl_phys(hwaddr pa) +{ + if (virtio_get_byteswap()) { + return bswap32(ldl_phys(pa)); + } + return ldl_phys(pa); +} + +static inline uint64_t virtio_ldq_phys(hwaddr pa) +{ + if (virtio_get_byteswap()) { + return bswap64(ldq_phys(pa)); + } + return ldq_phys(pa); +} + +static inline void virtio_stw_phys(hwaddr pa, uint16_t value) +{ + if (virtio_get_byteswap()) { + stw_phys(pa, bswap16(value)); + } else { + stw_phys(pa, value); + } +} + +static inline void virtio_stl_phys(hwaddr pa, uint32_t value) +{ + if (virtio_get_byteswap()) { + stl_phys(pa, bswap32(value)); + } else { + stl_phys(pa, value); + } +} + +static inline void virtio_stw_p(void *ptr, uint16_t v) +{ + if (virtio_get_byteswap()) { + stw_p(ptr, bswap16(v)); + } else { + stw_p(ptr, v); + } +} + +static inline void virtio_stl_p(void *ptr, uint32_t v) +{ + if (virtio_get_byteswap()) { + stl_p(ptr, bswap32(v)); + } else { + stl_p(ptr, v); + } +} + +static inline void virtio_stq_p(void *ptr, uint64_t v) +{ + if (virtio_get_byteswap()) { + stq_p(ptr, bswap64(v)); + } else { + stq_p(ptr, v); + } +} + +static inline int virtio_lduw_p(const void *ptr) +{ + if (virtio_get_byteswap()) { + return bswap16(lduw_p(ptr)); + } else { + return lduw_p(ptr); + } +} + +static inline int virtio_ldl_p(const void *ptr) +{ + if (virtio_get_byteswap()) { + return bswap32(ldl_p(ptr)); + } else { + return ldl_p(ptr); + } +} + +static inline uint64_t virtio_ldq_p(const void *ptr) +{ + if (virtio_get_byteswap()) { + return bswap64(ldq_p(ptr)); + } else { + return ldq_p(ptr); + } +} + +static inline uint32_t virtio_tswap32(uint32_t s) +{ + if (virtio_get_byteswap()) { + return bswap32(tswap32(s)); + } else { + return tswap32(s); + } +} + +static inline void virtio_tswap32s(uint32_t *s) +{ + tswap32s(s); + if (virtio_get_byteswap()) { + *s = bswap32(*s); + } +} +#endif /* _QEMU_VIRTIO_ACCESS_H */ diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index d7e9e0f..18ca725 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -248,4 +248,6 @@ void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign, bool set_handler); void virtio_queue_notify_vq(VirtQueue *vq); void virtio_irq(VirtQueue *vq); + +extern bool virtio_get_byteswap(void); #endif diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs index f306cba..05f7bab 100644 --- a/stubs/Makefile.objs +++ b/stubs/Makefile.objs @@ -26,3 +26,4 @@ stub-obj-y += vm-stop.o stub-obj-y += vmstate.o stub-obj-$(CONFIG_WIN32) += fd-register.o stub-obj-y += cpus.o +stub-obj-y += virtio_get_byteswap.o diff --git a/stubs/virtio_get_byteswap.c b/stubs/virtio_get_byteswap.c new file mode 100644 index 0000000..7cf764d --- /dev/null +++ b/stubs/virtio_get_byteswap.c @@ -0,0 +1,6 @@ +#include "hw/virtio/virtio.h" + +bool virtio_get_byteswap(void) +{ + return false; +} -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio. 2013-08-12 7:59 ` [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio Rusty Russell @ 2013-08-12 9:28 ` Benjamin Herrenschmidt 2013-08-12 9:39 ` Peter Maydell ` (2 more replies) 0 siblings, 3 replies; 36+ messages in thread From: Benjamin Herrenschmidt @ 2013-08-12 9:28 UTC (permalink / raw) To: Rusty Russell; +Cc: qemu-devel On Mon, 2013-08-12 at 17:29 +0930, Rusty Russell wrote: > virtio data structures are defined as "target endian", which assumes > that's a fixed value. In fact, that actually means it's > platform-specific. > > Hopefully the OASIS virtio 1.0 spec will fix this. Meanwhile, create > a hook for little endian ppc. Ok, sorry if I missed a previous debate on that one but why do you do a call-out to architecture specific stuff (that is not even inline) on every access ? If we consider that virtio byte order is global, can't you make it a global that is *set* by the architecture rather than *polled* by virtio ? We have explicit knowledge of when our endianness change (we get the hcall or a write to some SPR), we can call virtio *then* to adjust the endianness rather than having a call-out to the platform on every access. Ben. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio. 2013-08-12 9:28 ` Benjamin Herrenschmidt @ 2013-08-12 9:39 ` Peter Maydell 2013-08-12 9:43 ` Benjamin Herrenschmidt 2013-08-12 12:56 ` Anthony Liguori 2013-09-06 2:27 ` Rusty Russell 2 siblings, 1 reply; 36+ messages in thread From: Peter Maydell @ 2013-08-12 9:39 UTC (permalink / raw) To: Benjamin Herrenschmidt; +Cc: Rusty Russell, qemu-devel On 12 August 2013 10:28, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > We have explicit knowledge of when our endianness change (we get the > hcall or a write to some SPR), we can call virtio *then* to adjust the > endianness rather than having a call-out to the platform on every > access. ARM doesn't -- I wouldn't expect changing the endianness of exceptions via writing to the SCTLR to trap to the hypervisor (and in any case it certainly won't result in a return to userspace). -- PMM ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio. 2013-08-12 9:39 ` Peter Maydell @ 2013-08-12 9:43 ` Benjamin Herrenschmidt 2013-08-12 9:45 ` Peter Maydell 0 siblings, 1 reply; 36+ messages in thread From: Benjamin Herrenschmidt @ 2013-08-12 9:43 UTC (permalink / raw) To: Peter Maydell; +Cc: Rusty Russell, qemu-devel On Mon, 2013-08-12 at 10:39 +0100, Peter Maydell wrote: > On 12 August 2013 10:28, Benjamin Herrenschmidt > <benh@kernel.crashing.org> wrote: > > We have explicit knowledge of when our endianness change (we get the > > hcall or a write to some SPR), we can call virtio *then* to adjust the > > endianness rather than having a call-out to the platform on every > > access. > > ARM doesn't -- I wouldn't expect changing the endianness of > exceptions via writing to the SCTLR to trap to the hypervisor > (and in any case it certainly won't result in a return to > userspace). But don't you need to reconfigure the bridge (as per our previous discussion) ? In that case you do need to call out to qemu ... Cheers, Ben. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio. 2013-08-12 9:43 ` Benjamin Herrenschmidt @ 2013-08-12 9:45 ` Peter Maydell 2013-08-12 9:50 ` Benjamin Herrenschmidt 0 siblings, 1 reply; 36+ messages in thread From: Peter Maydell @ 2013-08-12 9:45 UTC (permalink / raw) To: Benjamin Herrenschmidt; +Cc: Rusty Russell, qemu-devel On 12 August 2013 10:43, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > On Mon, 2013-08-12 at 10:39 +0100, Peter Maydell wrote: >> ARM doesn't -- I wouldn't expect changing the endianness of >> exceptions via writing to the SCTLR to trap to the hypervisor >> (and in any case it certainly won't result in a return to >> userspace). > > But don't you need to reconfigure the bridge (as per our previous > discussion) ? In that case you do need to call out to qemu ... Bridge? You've lost me, I'm afraid. -- PMM ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio. 2013-08-12 9:45 ` Peter Maydell @ 2013-08-12 9:50 ` Benjamin Herrenschmidt 2013-08-12 9:52 ` Peter Maydell 0 siblings, 1 reply; 36+ messages in thread From: Benjamin Herrenschmidt @ 2013-08-12 9:50 UTC (permalink / raw) To: Peter Maydell; +Cc: Rusty Russell, qemu-devel On Mon, 2013-08-12 at 10:45 +0100, Peter Maydell wrote: > On 12 August 2013 10:43, Benjamin Herrenschmidt > <benh@kernel.crashing.org> wrote: > > On Mon, 2013-08-12 at 10:39 +0100, Peter Maydell wrote: > >> ARM doesn't -- I wouldn't expect changing the endianness of > >> exceptions via writing to the SCTLR to trap to the hypervisor > >> (and in any case it certainly won't result in a return to > >> userspace). > > > > But don't you need to reconfigure the bridge (as per our previous > > discussion) ? In that case you do need to call out to qemu ... > > Bridge? You've lost me, I'm afraid. I must be confused ... you mentioned in a previous discussion around endianness that on some ARM cores at least, when changing the OS endianness, you had to configure a different lane swapping in the bridge to the the IO devices (AXI ?) But I might be confusing with something else. Ben. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio. 2013-08-12 9:50 ` Benjamin Herrenschmidt @ 2013-08-12 9:52 ` Peter Maydell 2013-08-12 9:56 ` Benjamin Herrenschmidt 0 siblings, 1 reply; 36+ messages in thread From: Peter Maydell @ 2013-08-12 9:52 UTC (permalink / raw) To: Benjamin Herrenschmidt; +Cc: Rusty Russell, qemu-devel On 12 August 2013 10:50, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > I must be confused ... you mentioned in a previous discussion around > endianness that on some ARM cores at least, when changing the OS > endianness, you had to configure a different lane swapping in the bridge > to the the IO devices (AXI ?) No, that's just the implementation -- the bit in the control register is effectively controlling whether there is byte lane swapping in the part of the CPU which is the data path between it and its bus to the outside world. -- PMM ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio. 2013-08-12 9:52 ` Peter Maydell @ 2013-08-12 9:56 ` Benjamin Herrenschmidt 2013-08-12 10:36 ` Peter Maydell 0 siblings, 1 reply; 36+ messages in thread From: Benjamin Herrenschmidt @ 2013-08-12 9:56 UTC (permalink / raw) To: Peter Maydell; +Cc: Rusty Russell, qemu-devel On Mon, 2013-08-12 at 10:52 +0100, Peter Maydell wrote: > On 12 August 2013 10:50, Benjamin Herrenschmidt > <benh@kernel.crashing.org> wrote: > > I must be confused ... you mentioned in a previous discussion around > > endianness that on some ARM cores at least, when changing the OS > > endianness, you had to configure a different lane swapping in the bridge > > to the the IO devices (AXI ?) > > No, that's just the implementation -- the bit in the control > register is effectively controlling whether there is byte lane > swapping in the part of the CPU which is the data path between > it and its bus to the outside world. I find it amazing that an OS can touch that without hitting the hypervisor :-) Anyway, ok, we do need to poll from virtio then, but we probably need to cache as well, no ? When do you sample it in qemu ? Cheers, Ben. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio. 2013-08-12 9:56 ` Benjamin Herrenschmidt @ 2013-08-12 10:36 ` Peter Maydell 0 siblings, 0 replies; 36+ messages in thread From: Peter Maydell @ 2013-08-12 10:36 UTC (permalink / raw) To: Benjamin Herrenschmidt; +Cc: Rusty Russell, qemu-devel On 12 August 2013 10:56, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > On Mon, 2013-08-12 at 10:52 +0100, Peter Maydell wrote: >> On 12 August 2013 10:50, Benjamin Herrenschmidt >> <benh@kernel.crashing.org> wrote: >> > I must be confused ... you mentioned in a previous discussion around >> > endianness that on some ARM cores at least, when changing the OS >> > endianness, you had to configure a different lane swapping in the bridge >> > to the the IO devices (AXI ?) >> >> No, that's just the implementation -- the bit in the control >> register is effectively controlling whether there is byte lane >> swapping in the part of the CPU which is the data path between >> it and its bus to the outside world. > > I find it amazing that an OS can touch that without hitting the > hypervisor :-) It's no different to having a userspace process able to have a different setting from the OS, really. (There is an equivalent bit in another register that controls what endianness we use if we trap to hyp mode.) > Anyway, ok, we do need to poll from virtio then, but we > probably need to cache as well, no ? > > When do you sample it in qemu ? It's a bit theoretical at the moment since QEMU's ARM code kind of assumes little endian. I would expect that at the point when virtio was in an MMIO callback the CPUState struct would have been updated via the usual sync process in kvm_arch_get_registers(). -- PMM ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio. 2013-08-12 9:28 ` Benjamin Herrenschmidt 2013-08-12 9:39 ` Peter Maydell @ 2013-08-12 12:56 ` Anthony Liguori 2013-08-13 4:20 ` Rusty Russell 2013-09-06 2:27 ` Rusty Russell 2 siblings, 1 reply; 36+ messages in thread From: Anthony Liguori @ 2013-08-12 12:56 UTC (permalink / raw) To: Benjamin Herrenschmidt, Rusty Russell; +Cc: qemu-devel Benjamin Herrenschmidt <benh@kernel.crashing.org> writes: > On Mon, 2013-08-12 at 17:29 +0930, Rusty Russell wrote: >> virtio data structures are defined as "target endian", which assumes >> that's a fixed value. In fact, that actually means it's >> platform-specific. >> >> Hopefully the OASIS virtio 1.0 spec will fix this. Meanwhile, create >> a hook for little endian ppc. > > Ok, sorry if I missed a previous debate on that one but why do you do a > call-out to architecture specific stuff (that is not even inline) on > every access ? Let's focus on getting something merged. Then we can muck around with it down the road. Having target-ppc call into virtio is a layering violation. This approach keeps the dependencies cleaner. Regards, Anthony Liguori > > If we consider that virtio byte order is global, can't you make it a > global that is *set* by the architecture rather than *polled* by > virtio ? > > We have explicit knowledge of when our endianness change (we get the > hcall or a write to some SPR), we can call virtio *then* to adjust the > endianness rather than having a call-out to the platform on every > access. > > Ben. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio. 2013-08-12 12:56 ` Anthony Liguori @ 2013-08-13 4:20 ` Rusty Russell 2013-08-13 5:30 ` Benjamin Herrenschmidt 0 siblings, 1 reply; 36+ messages in thread From: Rusty Russell @ 2013-08-13 4:20 UTC (permalink / raw) To: Anthony Liguori, Benjamin Herrenschmidt; +Cc: qemu-devel Anthony Liguori <anthony@codemonkey.ws> writes: > Benjamin Herrenschmidt <benh@kernel.crashing.org> writes: > >> On Mon, 2013-08-12 at 17:29 +0930, Rusty Russell wrote: >>> virtio data structures are defined as "target endian", which assumes >>> that's a fixed value. In fact, that actually means it's >>> platform-specific. >>> >>> Hopefully the OASIS virtio 1.0 spec will fix this. Meanwhile, create >>> a hook for little endian ppc. >> >> Ok, sorry if I missed a previous debate on that one but why do you do a >> call-out to architecture specific stuff (that is not even inline) on >> every access ? Anthony said he wanted it that way: my initial patch was more optimized. > Let's focus on getting something merged. Then we can muck around with > it down the road. > > Having target-ppc call into virtio is a layering violation. This > approach keeps the dependencies cleaner. We can have it call once (eg. when the first and storing the status word) and store the result. Cheers, Rusty. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio. 2013-08-13 4:20 ` Rusty Russell @ 2013-08-13 5:30 ` Benjamin Herrenschmidt 2013-08-14 0:03 ` Rusty Russell 0 siblings, 1 reply; 36+ messages in thread From: Benjamin Herrenschmidt @ 2013-08-13 5:30 UTC (permalink / raw) To: Rusty Russell; +Cc: qemu-devel, Anthony Liguori On Tue, 2013-08-13 at 13:50 +0930, Rusty Russell wrote: > We can have it call once (eg. when the first and storing the status > word) and store the result. And fail with kexec of a different endian kernel :-) Let's not bother yet. Merge it and then we see if we can optimize. Cheers, Ben. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio. 2013-08-13 5:30 ` Benjamin Herrenschmidt @ 2013-08-14 0:03 ` Rusty Russell 0 siblings, 0 replies; 36+ messages in thread From: Rusty Russell @ 2013-08-14 0:03 UTC (permalink / raw) To: Benjamin Herrenschmidt; +Cc: qemu-devel, Anthony Liguori Benjamin Herrenschmidt <benh@kernel.crashing.org> writes: > On Tue, 2013-08-13 at 13:50 +0930, Rusty Russell wrote: >> We can have it call once (eg. when the first and storing the status >> word) and store the result. > > And fail with kexec of a different endian kernel :-) Let's not bother > yet. Merge it and then we see if we can optimize. Yeah, my code actually did it every status bye write, which unintentionally solved this. But agreed, let's let these patches stand... Cheers, Rusty. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio. 2013-08-12 9:28 ` Benjamin Herrenschmidt 2013-08-12 9:39 ` Peter Maydell 2013-08-12 12:56 ` Anthony Liguori @ 2013-09-06 2:27 ` Rusty Russell 2 siblings, 0 replies; 36+ messages in thread From: Rusty Russell @ 2013-09-06 2:27 UTC (permalink / raw) To: Benjamin Herrenschmidt; +Cc: Michael Neuling, qemu-devel Benjamin Herrenschmidt <benh@kernel.crashing.org> writes: > On Mon, 2013-08-12 at 17:29 +0930, Rusty Russell wrote: >> virtio data structures are defined as "target endian", which assumes >> that's a fixed value. In fact, that actually means it's >> platform-specific. >> >> Hopefully the OASIS virtio 1.0 spec will fix this. Meanwhile, create >> a hook for little endian ppc. > > Ok, sorry if I missed a previous debate on that one but why do you do a > call-out to architecture specific stuff (that is not even inline) on > every access ? > > If we consider that virtio byte order is global, can't you make it a > global that is *set* by the architecture rather than *polled* by > virtio ? OK, so after some more offline discussion it turns out these patches won't really work reliably, since powerpc kvm doesn't reflect the register into qemu. Mikey N has promised me a KVM_GET_ONE_REG to get the register, and I'll rework on top of that: we will query that whenever a device is reset (which Linux does on every device init, so it captures the kexec case too). Cheers, Rusty. ^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 2/8] target-ppc: ppc64 target's virtio can be either endian. 2013-08-12 7:59 [Qemu-devel] [PATCH 0/8] virtio for endian curious guests Take #2 Rusty Russell 2013-08-12 7:59 ` [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio Rusty Russell @ 2013-08-12 7:59 ` Rusty Russell 2013-08-12 7:59 ` [Qemu-devel] [PATCH 3/8] virtio: allow byte swapping for vring and config access Rusty Russell 2 siblings, 0 replies; 36+ messages in thread From: Rusty Russell @ 2013-08-12 7:59 UTC (permalink / raw) To: qemu-devel; +Cc: Rusty Russell We base it on the OS endian, as reflected by the endianness of the interrupt vectors. Suggested-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> --- target-ppc/misc_helper.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/target-ppc/misc_helper.c b/target-ppc/misc_helper.c index 616aab6..6c97c81 100644 --- a/target-ppc/misc_helper.c +++ b/target-ppc/misc_helper.c @@ -20,6 +20,7 @@ #include "helper.h" #include "helper_regs.h" +#include "hw/virtio/virtio.h" /*****************************************************************************/ /* SPR accesses */ @@ -116,3 +117,11 @@ void ppc_store_msr(CPUPPCState *env, target_ulong value) { hreg_store_msr(env, value, 0); } + +bool virtio_get_byteswap(void) +{ + PowerPCCPU *cp = POWERPC_CPU(first_cpu); + CPUPPCState *env = &cp->env; + + return env->spr[SPR_LPCR] & LPCR_ILE; +} -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 3/8] virtio: allow byte swapping for vring and config access 2013-08-12 7:59 [Qemu-devel] [PATCH 0/8] virtio for endian curious guests Take #2 Rusty Russell 2013-08-12 7:59 ` [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio Rusty Russell 2013-08-12 7:59 ` [Qemu-devel] [PATCH 2/8] target-ppc: ppc64 target's virtio can be either endian Rusty Russell @ 2013-08-12 7:59 ` Rusty Russell 2013-09-09 12:44 ` [Qemu-devel] [PATCH] hw/9pfs/virtio_9p_device: use virtio wrappers to access headers Greg Kurz 2 siblings, 1 reply; 36+ messages in thread From: Rusty Russell @ 2013-08-12 7:59 UTC (permalink / raw) To: qemu-devel; +Cc: Rusty Russell This is based on a simpler patch by Anthony Liguouri, which only handled the vring accesses. We also need some drivers to access these helpers, eg. for data which contains headers. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> --- hw/virtio/virtio.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 09f62c6..178647b 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -18,6 +18,7 @@ #include "hw/virtio/virtio.h" #include "qemu/atomic.h" #include "hw/virtio/virtio-bus.h" +#include "hw/virtio/virtio-access.h" /* * The alignment to use between consumer and producer parts of vring. @@ -104,49 +105,49 @@ static inline uint64_t vring_desc_addr(hwaddr desc_pa, int i) { hwaddr pa; pa = desc_pa + sizeof(VRingDesc) * i + offsetof(VRingDesc, addr); - return ldq_phys(pa); + return virtio_ldq_phys(pa); } static inline uint32_t vring_desc_len(hwaddr desc_pa, int i) { hwaddr pa; pa = desc_pa + sizeof(VRingDesc) * i + offsetof(VRingDesc, len); - return ldl_phys(pa); + return virtio_ldl_phys(pa); } static inline uint16_t vring_desc_flags(hwaddr desc_pa, int i) { hwaddr pa; pa = desc_pa + sizeof(VRingDesc) * i + offsetof(VRingDesc, flags); - return lduw_phys(pa); + return virtio_lduw_phys(pa); } static inline uint16_t vring_desc_next(hwaddr desc_pa, int i) { hwaddr pa; pa = desc_pa + sizeof(VRingDesc) * i + offsetof(VRingDesc, next); - return lduw_phys(pa); + return virtio_lduw_phys(pa); } static inline uint16_t vring_avail_flags(VirtQueue *vq) { hwaddr pa; pa = vq->vring.avail + offsetof(VRingAvail, flags); - return lduw_phys(pa); + return virtio_lduw_phys(pa); } static inline uint16_t vring_avail_idx(VirtQueue *vq) { hwaddr pa; pa = vq->vring.avail + offsetof(VRingAvail, idx); - return lduw_phys(pa); + return virtio_lduw_phys(pa); } static inline uint16_t vring_avail_ring(VirtQueue *vq, int i) { hwaddr pa; pa = vq->vring.avail + offsetof(VRingAvail, ring[i]); - return lduw_phys(pa); + return virtio_lduw_phys(pa); } static inline uint16_t vring_used_event(VirtQueue *vq) @@ -158,42 +159,42 @@ static inline void vring_used_ring_id(VirtQueue *vq, int i, uint32_t val) { hwaddr pa; pa = vq->vring.used + offsetof(VRingUsed, ring[i].id); - stl_phys(pa, val); + virtio_stl_phys(pa, val); } static inline void vring_used_ring_len(VirtQueue *vq, int i, uint32_t val) { hwaddr pa; pa = vq->vring.used + offsetof(VRingUsed, ring[i].len); - stl_phys(pa, val); + virtio_stl_phys(pa, val); } static uint16_t vring_used_idx(VirtQueue *vq) { hwaddr pa; pa = vq->vring.used + offsetof(VRingUsed, idx); - return lduw_phys(pa); + return virtio_lduw_phys(pa); } static inline void vring_used_idx_set(VirtQueue *vq, uint16_t val) { hwaddr pa; pa = vq->vring.used + offsetof(VRingUsed, idx); - stw_phys(pa, val); + virtio_stw_phys(pa, val); } static inline void vring_used_flags_set_bit(VirtQueue *vq, int mask) { hwaddr pa; pa = vq->vring.used + offsetof(VRingUsed, flags); - stw_phys(pa, lduw_phys(pa) | mask); + virtio_stw_phys(pa, virtio_lduw_phys(pa) | mask); } static inline void vring_used_flags_unset_bit(VirtQueue *vq, int mask) { hwaddr pa; pa = vq->vring.used + offsetof(VRingUsed, flags); - stw_phys(pa, lduw_phys(pa) & ~mask); + virtio_stw_phys(pa, virtio_lduw_phys(pa) & ~mask); } static inline void vring_avail_event(VirtQueue *vq, uint16_t val) @@ -203,7 +204,7 @@ static inline void vring_avail_event(VirtQueue *vq, uint16_t val) return; } pa = vq->vring.used + offsetof(VRingUsed, ring[vq->vring.num]); - stw_phys(pa, val); + virtio_stw_phys(pa, val); } void virtio_queue_set_notification(VirtQueue *vq, int enable) -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH] hw/9pfs/virtio_9p_device: use virtio wrappers to access headers. 2013-08-12 7:59 ` [Qemu-devel] [PATCH 3/8] virtio: allow byte swapping for vring and config access Rusty Russell @ 2013-09-09 12:44 ` Greg Kurz 2013-09-10 5:21 ` Rusty Russell 0 siblings, 1 reply; 36+ messages in thread From: Greg Kurz @ 2013-09-09 12:44 UTC (permalink / raw) To: qemu-devel; +Cc: rusty Follow-up to Rusty's virtio endianness serie: enough to get a working virtfs mount. Note that st*_raw and ld*_raw are effectively replaced by st*_p and ld*_p. Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> --- hw/9pfs/virtio-9p-device.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c index f0ffbe8..bc2d0da 100644 --- a/hw/9pfs/virtio-9p-device.c +++ b/hw/9pfs/virtio-9p-device.c @@ -19,6 +19,7 @@ #include "fsdev/qemu-fsdev.h" #include "virtio-9p-xattr.h" #include "virtio-9p-coth.h" +#include "hw/virtio/virtio-access.h" static uint32_t virtio_9p_get_features(VirtIODevice *vdev, uint32_t features) { @@ -34,7 +35,7 @@ static void virtio_9p_get_config(VirtIODevice *vdev, uint8_t *config) len = strlen(s->tag); cfg = g_malloc0(sizeof(struct virtio_9p_config) + len); - stw_raw(&cfg->tag_len, len); + virtio_stl_p(&cfg->tag_len, len); /* We don't copy the terminating null to config space */ memcpy(cfg->tag, s->tag, len); memcpy(config, cfg, s->config_size); ^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/9pfs/virtio_9p_device: use virtio wrappers to access headers. 2013-09-09 12:44 ` [Qemu-devel] [PATCH] hw/9pfs/virtio_9p_device: use virtio wrappers to access headers Greg Kurz @ 2013-09-10 5:21 ` Rusty Russell 0 siblings, 0 replies; 36+ messages in thread From: Rusty Russell @ 2013-09-10 5:21 UTC (permalink / raw) To: Greg Kurz, qemu-devel; +Cc: Michael Neuling Greg Kurz <gkurz@linux.vnet.ibm.com> writes: > Follow-up to Rusty's virtio endianness serie: enough to get a working > virtfs mount. > > Note that st*_raw and ld*_raw are effectively replaced by st*_p and ld*_p. > > Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> Thanks! I've reworked my patches in line with the anticipated KVM_GET_ONE_REG from Mikey, and put this into the series. Mikey, here's the template I assumed (needs CONFIG_KVM implementation of kvmppc_update_spr_lpcr). Cheers, Rusty. FIXME: Implement for KVM using KVM_GET_ONE_REG! diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h index 771cfbe..30d8af6 100644 --- a/target-ppc/kvm_ppc.h +++ b/target-ppc/kvm_ppc.h @@ -29,6 +29,7 @@ int kvmppc_clear_tsr_bits(PowerPCCPU *cpu, uint32_t tsr_bits); int kvmppc_or_tsr_bits(PowerPCCPU *cpu, uint32_t tsr_bits); int kvmppc_set_tcr(PowerPCCPU *cpu); int kvmppc_booke_watchdog_enable(PowerPCCPU *cpu); +void kvmppc_update_spr_lpcr(PowerPCCPU *cpu); #ifndef CONFIG_USER_ONLY off_t kvmppc_alloc_rma(const char *name, MemoryRegion *sysmem); void *kvmppc_create_spapr_tce(uint32_t liobn, uint32_t window_size, int *pfd); @@ -159,6 +160,10 @@ static inline bool kvmppc_has_cap_epr(void) { return false; } + +static inline void kvmppc_update_spr_lpcr(PowerPCCPU *cpu) +{ +} #endif #ifndef CONFIG_KVM ^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 0/8] virtio endian-ambivalent target fixes (rebased)
@ 2014-02-18 12:38 Greg Kurz
2014-02-18 12:38 ` [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio Greg Kurz
0 siblings, 1 reply; 36+ messages in thread
From: Greg Kurz @ 2014-02-18 12:38 UTC (permalink / raw)
To: afaerber
Cc: kwolf, peter.maydell, thuth, mst, marc.zyngier, rusty, agraf,
qemu-devel, stefanha, anthony, pbonzini
On Fri, 14 Feb 2014 12:59:49 +0100
Andreas Färber <afaerber@suse.de> wrote:
> Hi,
>
> [...]
>
> It might've helped if Rusty had actually used our
> scripts/get_maintainer.pl script to CC people. While Anthony seems to
> have reviewed some patches (usually Reviewed-by should be before the
> final Signed-off-by fwiw), neither Stefan (virtio-net) nor Kevin
> (virtio-blk) nor Paolo (virtio-scsi) were CC'ed, and recently Michael
> stepped up as virtio maintainer, so maybe he can take them once ready.
>
> 1/7 looks okay to me; 3-7 are rather mechanical - people will need to
> review that those changes are sufficient for the current code base.
> We've since converted virtio devices to QOM realize, so a rebase is
> likely needed for such an "old" series.
>
> Regards,
> Andreas
Hi,
Thank you Andreas for your support. :)
Here is a rebased patchset. Only the two first patches ("virtio_get_byteswap:"
and "virtio:") had to be updated actually. FWIW, this the very same patchset
I use, along with some ppc64 specific enablement code, to have functionnal
ppc64 LE guests.
---
Greg Kurz (1):
hw/9pfs/virtio_9p_device: use virtio wrappers to access headers.
Rusty Russell (7):
virtio_get_byteswap: function for endian-ambivalent targets using virtio.
virtio: allow byte swapping for vring and config access
hw/net/virtio-net: use virtio wrappers to access headers.
hw/net/virtio-balloon: use virtio wrappers to access page frame numbers.
hw/block/virtio-blk: use virtio wrappers to access headers.
hw/scsi/virtio-scsi: use virtio wrappers to access headers.
hw/char/virtio-serial-bus: use virtio wrappers to access headers.
hw/9pfs/virtio-9p-device.c | 3 +
hw/block/virtio-blk.c | 35 +++++-----
hw/char/virtio-serial-bus.c | 34 +++++-----
hw/net/virtio-net.c | 15 ++--
hw/scsi/virtio-scsi.c | 33 +++++----
hw/virtio/virtio-balloon.c | 3 +
hw/virtio/virtio.c | 38 ++++++-----
include/hw/virtio/virtio-access.h | 132 +++++++++++++++++++++++++++++++++++++
include/hw/virtio/virtio.h | 2 +
stubs/Makefile.objs | 1
stubs/virtio_get_byteswap.c | 6 ++
11 files changed, 228 insertions(+), 74 deletions(-)
create mode 100644 include/hw/virtio/virtio-access.h
create mode 100644 stubs/virtio_get_byteswap.c
Best Regards.
--
Greg
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio. 2014-02-18 12:38 [Qemu-devel] [PATCH 0/8] virtio endian-ambivalent target fixes (rebased) Greg Kurz @ 2014-02-18 12:38 ` Greg Kurz 2014-02-18 14:48 ` Alexander Graf 2014-02-18 19:25 ` Andreas Färber 0 siblings, 2 replies; 36+ messages in thread From: Greg Kurz @ 2014-02-18 12:38 UTC (permalink / raw) To: afaerber Cc: kwolf, peter.maydell, thuth, mst, marc.zyngier, rusty, agraf, qemu-devel, stefanha, anthony, pbonzini From: Rusty Russell <rusty@rustcorp.com.au> virtio data structures are defined as "target endian", which assumes that's a fixed value. In fact, that actually means it's platform-specific. The OASIS virtio 1.0 spec will fix this. Meanwhile, create a hook for little endian ppc (and potentially ARM). This is called at device reset time (which is done before any driver is loaded) since it may involve a system call to get the status when running under kvm. [ fixed checkpatch.pl error with the virtio_byteswap initialisation, ldq_phys() API change, Greg Kurz <gkurz@linux.vnet.ibm.com> ] Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> --- hw/virtio/virtio.c | 6 ++ include/hw/virtio/virtio-access.h | 132 +++++++++++++++++++++++++++++++++++++ include/hw/virtio/virtio.h | 2 + stubs/Makefile.objs | 1 stubs/virtio_get_byteswap.c | 6 ++ 5 files changed, 147 insertions(+) create mode 100644 include/hw/virtio/virtio-access.h create mode 100644 stubs/virtio_get_byteswap.c diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index aeabf3a..4fd6ac2 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -19,6 +19,9 @@ #include "hw/virtio/virtio.h" #include "qemu/atomic.h" #include "hw/virtio/virtio-bus.h" +#include "hw/virtio/virtio-access.h" + +bool virtio_byteswap; /* * The alignment to use between consumer and producer parts of vring. @@ -546,6 +549,9 @@ void virtio_reset(void *opaque) virtio_set_status(vdev, 0); + /* We assume all devices are the same endian. */ + virtio_byteswap = virtio_get_byteswap(); + if (k->reset) { k->reset(vdev); } diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h new file mode 100644 index 0000000..2e22a47 --- /dev/null +++ b/include/hw/virtio/virtio-access.h @@ -0,0 +1,132 @@ +/* + * Virtio Accessor Support: In case your target can change endian. + * + * Copyright IBM, Corp. 2013 + * + * Authors: + * Rusty Russell <rusty@au.ibm.com> + * + * This work is licensed under the terms of the GNU GPL, version 2. See + * the COPYING file in the top-level directory. + * + */ +#ifndef _QEMU_VIRTIO_ACCESS_H +#define _QEMU_VIRTIO_ACCESS_H +#include "hw/virtio/virtio.h" + +/* Initialized by virtio_get_byteswap() at any virtio device reset. */ +extern bool virtio_byteswap; + +static inline uint16_t virtio_lduw_phys(AddressSpace *as, hwaddr pa) +{ + if (virtio_byteswap) { + return bswap16(lduw_phys(as, pa)); + } + return lduw_phys(as, pa); +} + +static inline uint32_t virtio_ldl_phys(AddressSpace *as, hwaddr pa) +{ + if (virtio_byteswap) { + return bswap32(ldl_phys(as, pa)); + } + return ldl_phys(as, pa); +} + +static inline uint64_t virtio_ldq_phys(AddressSpace *as, hwaddr pa) +{ + if (virtio_byteswap) { + return bswap64(ldq_phys(as, pa)); + } + return ldq_phys(as, pa); +} + +static inline void virtio_stw_phys(AddressSpace *as, hwaddr pa, uint16_t value) +{ + if (virtio_byteswap) { + stw_phys(as, pa, bswap16(value)); + } else { + stw_phys(as, pa, value); + } +} + +static inline void virtio_stl_phys(AddressSpace *as, hwaddr pa, uint32_t value) +{ + if (virtio_byteswap) { + stl_phys(as, pa, bswap32(value)); + } else { + stl_phys(as, pa, value); + } +} + +static inline void virtio_stw_p(void *ptr, uint16_t v) +{ + if (virtio_byteswap) { + stw_p(ptr, bswap16(v)); + } else { + stw_p(ptr, v); + } +} + +static inline void virtio_stl_p(void *ptr, uint32_t v) +{ + if (virtio_byteswap) { + stl_p(ptr, bswap32(v)); + } else { + stl_p(ptr, v); + } +} + +static inline void virtio_stq_p(void *ptr, uint64_t v) +{ + if (virtio_byteswap) { + stq_p(ptr, bswap64(v)); + } else { + stq_p(ptr, v); + } +} + +static inline int virtio_lduw_p(const void *ptr) +{ + if (virtio_byteswap) { + return bswap16(lduw_p(ptr)); + } else { + return lduw_p(ptr); + } +} + +static inline int virtio_ldl_p(const void *ptr) +{ + if (virtio_byteswap) { + return bswap32(ldl_p(ptr)); + } else { + return ldl_p(ptr); + } +} + +static inline uint64_t virtio_ldq_p(const void *ptr) +{ + if (virtio_byteswap) { + return bswap64(ldq_p(ptr)); + } else { + return ldq_p(ptr); + } +} + +static inline uint32_t virtio_tswap32(uint32_t s) +{ + if (virtio_byteswap) { + return bswap32(tswap32(s)); + } else { + return tswap32(s); + } +} + +static inline void virtio_tswap32s(uint32_t *s) +{ + tswap32s(s); + if (virtio_byteswap) { + *s = bswap32(*s); + } +} +#endif /* _QEMU_VIRTIO_ACCESS_H */ diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index 3e54e90..5009945 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -253,4 +253,6 @@ void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign, bool set_handler); void virtio_queue_notify_vq(VirtQueue *vq); void virtio_irq(VirtQueue *vq); + +extern bool virtio_get_byteswap(void); #endif diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs index df92fe5..7e7a9c8 100644 --- a/stubs/Makefile.objs +++ b/stubs/Makefile.objs @@ -27,3 +27,4 @@ stub-obj-y += vm-stop.o stub-obj-y += vmstate.o stub-obj-$(CONFIG_WIN32) += fd-register.o stub-obj-y += cpus.o +stub-obj-y += virtio_get_byteswap.o diff --git a/stubs/virtio_get_byteswap.c b/stubs/virtio_get_byteswap.c new file mode 100644 index 0000000..7cf764d --- /dev/null +++ b/stubs/virtio_get_byteswap.c @@ -0,0 +1,6 @@ +#include "hw/virtio/virtio.h" + +bool virtio_get_byteswap(void) +{ + return false; +} ^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio. 2014-02-18 12:38 ` [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio Greg Kurz @ 2014-02-18 14:48 ` Alexander Graf 2014-02-18 15:03 ` Michael S. Tsirkin 2014-02-18 23:02 ` Andreas Färber 2014-02-18 19:25 ` Andreas Färber 1 sibling, 2 replies; 36+ messages in thread From: Alexander Graf @ 2014-02-18 14:48 UTC (permalink / raw) To: Greg Kurz Cc: kwolf, peter.maydell, thuth, mst, marc.zyngier, rusty, qemu-devel, stefanha, anthony, pbonzini, afaerber On 02/18/2014 01:38 PM, Greg Kurz wrote: > From: Rusty Russell <rusty@rustcorp.com.au> > > virtio data structures are defined as "target endian", which assumes > that's a fixed value. In fact, that actually means it's > platform-specific. > > The OASIS virtio 1.0 spec will fix this. Meanwhile, create a hook for > little endian ppc (and potentially ARM). This is called at device > reset time (which is done before any driver is loaded) since it > may involve a system call to get the status when running under kvm. > > [ fixed checkpatch.pl error with the virtio_byteswap initialisation, > ldq_phys() API change, Greg Kurz <gkurz@linux.vnet.ibm.com> ] > Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> > Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> > --- > hw/virtio/virtio.c | 6 ++ > include/hw/virtio/virtio-access.h | 132 +++++++++++++++++++++++++++++++++++++ > include/hw/virtio/virtio.h | 2 + > stubs/Makefile.objs | 1 > stubs/virtio_get_byteswap.c | 6 ++ > 5 files changed, 147 insertions(+) > create mode 100644 include/hw/virtio/virtio-access.h > create mode 100644 stubs/virtio_get_byteswap.c > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > index aeabf3a..4fd6ac2 100644 > --- a/hw/virtio/virtio.c > +++ b/hw/virtio/virtio.c > @@ -19,6 +19,9 @@ > #include "hw/virtio/virtio.h" > #include "qemu/atomic.h" > #include "hw/virtio/virtio-bus.h" > +#include "hw/virtio/virtio-access.h" > + > +bool virtio_byteswap; Could this be a virtio object property rather than a global? Imagine an AMP guest system with a BE and an LE system running in parallel accessing two separate virtio devices. With a single global that would break. Alex ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio. 2014-02-18 14:48 ` Alexander Graf @ 2014-02-18 15:03 ` Michael S. Tsirkin 2014-02-18 15:02 ` Alexander Graf ` (2 more replies) 2014-02-18 23:02 ` Andreas Färber 1 sibling, 3 replies; 36+ messages in thread From: Michael S. Tsirkin @ 2014-02-18 15:03 UTC (permalink / raw) To: Alexander Graf Cc: kwolf, peter.maydell, thuth, marc.zyngier, rusty, qemu-devel, stefanha, anthony, pbonzini, afaerber, Greg Kurz On Tue, Feb 18, 2014 at 03:48:38PM +0100, Alexander Graf wrote: > On 02/18/2014 01:38 PM, Greg Kurz wrote: > >From: Rusty Russell <rusty@rustcorp.com.au> > > > >virtio data structures are defined as "target endian", which assumes > >that's a fixed value. In fact, that actually means it's > >platform-specific. > > > >The OASIS virtio 1.0 spec will fix this. Meanwhile, create a hook for > >little endian ppc (and potentially ARM). This is called at device > >reset time (which is done before any driver is loaded) since it > >may involve a system call to get the status when running under kvm. > > > >[ fixed checkpatch.pl error with the virtio_byteswap initialisation, > > ldq_phys() API change, Greg Kurz <gkurz@linux.vnet.ibm.com> ] > >Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> > >Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> > >--- > > hw/virtio/virtio.c | 6 ++ > > include/hw/virtio/virtio-access.h | 132 +++++++++++++++++++++++++++++++++++++ > > include/hw/virtio/virtio.h | 2 + > > stubs/Makefile.objs | 1 > > stubs/virtio_get_byteswap.c | 6 ++ > > 5 files changed, 147 insertions(+) > > create mode 100644 include/hw/virtio/virtio-access.h > > create mode 100644 stubs/virtio_get_byteswap.c > > > >diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > >index aeabf3a..4fd6ac2 100644 > >--- a/hw/virtio/virtio.c > >+++ b/hw/virtio/virtio.c > >@@ -19,6 +19,9 @@ > > #include "hw/virtio/virtio.h" > > #include "qemu/atomic.h" > > #include "hw/virtio/virtio-bus.h" > >+#include "hw/virtio/virtio-access.h" > >+ > >+bool virtio_byteswap; > > Could this be a virtio object property rather than a global? Imagine > an AMP guest system with a BE and an LE system running in parallel > accessing two separate virtio devices. With a single global that > would break. > > > Alex Well, how does a device know which CPU uses it? I suspect we are better off waiting for 1.0 with this one. -- MST ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio. 2014-02-18 15:03 ` Michael S. Tsirkin @ 2014-02-18 15:02 ` Alexander Graf 2014-02-18 15:11 ` Michael S. Tsirkin 2014-02-18 15:04 ` Michael S. Tsirkin 2014-02-18 15:12 ` Cornelia Huck 2 siblings, 1 reply; 36+ messages in thread From: Alexander Graf @ 2014-02-18 15:02 UTC (permalink / raw) To: Michael S. Tsirkin Cc: kwolf, peter.maydell, thuth, marc.zyngier, rusty, qemu-devel, stefanha, anthony, pbonzini, afaerber, Greg Kurz On 02/18/2014 04:03 PM, Michael S. Tsirkin wrote: > On Tue, Feb 18, 2014 at 03:48:38PM +0100, Alexander Graf wrote: >> On 02/18/2014 01:38 PM, Greg Kurz wrote: >>> From: Rusty Russell <rusty@rustcorp.com.au> >>> >>> virtio data structures are defined as "target endian", which assumes >>> that's a fixed value. In fact, that actually means it's >>> platform-specific. >>> >>> The OASIS virtio 1.0 spec will fix this. Meanwhile, create a hook for >>> little endian ppc (and potentially ARM). This is called at device >>> reset time (which is done before any driver is loaded) since it >>> may involve a system call to get the status when running under kvm. >>> >>> [ fixed checkpatch.pl error with the virtio_byteswap initialisation, >>> ldq_phys() API change, Greg Kurz <gkurz@linux.vnet.ibm.com> ] >>> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> >>> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> >>> --- >>> hw/virtio/virtio.c | 6 ++ >>> include/hw/virtio/virtio-access.h | 132 +++++++++++++++++++++++++++++++++++++ >>> include/hw/virtio/virtio.h | 2 + >>> stubs/Makefile.objs | 1 >>> stubs/virtio_get_byteswap.c | 6 ++ >>> 5 files changed, 147 insertions(+) >>> create mode 100644 include/hw/virtio/virtio-access.h >>> create mode 100644 stubs/virtio_get_byteswap.c >>> >>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c >>> index aeabf3a..4fd6ac2 100644 >>> --- a/hw/virtio/virtio.c >>> +++ b/hw/virtio/virtio.c >>> @@ -19,6 +19,9 @@ >>> #include "hw/virtio/virtio.h" >>> #include "qemu/atomic.h" >>> #include "hw/virtio/virtio-bus.h" >>> +#include "hw/virtio/virtio-access.h" >>> + >>> +bool virtio_byteswap; >> Could this be a virtio object property rather than a global? Imagine >> an AMP guest system with a BE and an LE system running in parallel >> accessing two separate virtio devices. With a single global that >> would break. >> >> >> Alex > Well, how does a device know which CPU uses it? > I suspect we are better off waiting for 1.0 with this one. Good point. It just feels like a heavy layering violation to have a global variable for all virtio devices. What if we start mixing legacy and 1.0 devices? We could reuse the same flag, but it would be initialized differently per device. Alex ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio. 2014-02-18 15:02 ` Alexander Graf @ 2014-02-18 15:11 ` Michael S. Tsirkin 2014-02-18 15:07 ` Alexander Graf 0 siblings, 1 reply; 36+ messages in thread From: Michael S. Tsirkin @ 2014-02-18 15:11 UTC (permalink / raw) To: Alexander Graf Cc: kwolf, peter.maydell, thuth, marc.zyngier, rusty, qemu-devel, stefanha, anthony, pbonzini, afaerber, Greg Kurz On Tue, Feb 18, 2014 at 04:02:18PM +0100, Alexander Graf wrote: > On 02/18/2014 04:03 PM, Michael S. Tsirkin wrote: > >On Tue, Feb 18, 2014 at 03:48:38PM +0100, Alexander Graf wrote: > >>On 02/18/2014 01:38 PM, Greg Kurz wrote: > >>>From: Rusty Russell <rusty@rustcorp.com.au> > >>> > >>>virtio data structures are defined as "target endian", which assumes > >>>that's a fixed value. In fact, that actually means it's > >>>platform-specific. > >>> > >>>The OASIS virtio 1.0 spec will fix this. Meanwhile, create a hook for > >>>little endian ppc (and potentially ARM). This is called at device > >>>reset time (which is done before any driver is loaded) since it > >>>may involve a system call to get the status when running under kvm. > >>> > >>>[ fixed checkpatch.pl error with the virtio_byteswap initialisation, > >>> ldq_phys() API change, Greg Kurz <gkurz@linux.vnet.ibm.com> ] > >>>Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> > >>>Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> > >>>--- > >>> hw/virtio/virtio.c | 6 ++ > >>> include/hw/virtio/virtio-access.h | 132 +++++++++++++++++++++++++++++++++++++ > >>> include/hw/virtio/virtio.h | 2 + > >>> stubs/Makefile.objs | 1 > >>> stubs/virtio_get_byteswap.c | 6 ++ > >>> 5 files changed, 147 insertions(+) > >>> create mode 100644 include/hw/virtio/virtio-access.h > >>> create mode 100644 stubs/virtio_get_byteswap.c > >>> > >>>diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > >>>index aeabf3a..4fd6ac2 100644 > >>>--- a/hw/virtio/virtio.c > >>>+++ b/hw/virtio/virtio.c > >>>@@ -19,6 +19,9 @@ > >>> #include "hw/virtio/virtio.h" > >>> #include "qemu/atomic.h" > >>> #include "hw/virtio/virtio-bus.h" > >>>+#include "hw/virtio/virtio-access.h" > >>>+ > >>>+bool virtio_byteswap; > >>Could this be a virtio object property rather than a global? Imagine > >>an AMP guest system with a BE and an LE system running in parallel > >>accessing two separate virtio devices. With a single global that > >>would break. > >> > >> > >>Alex > >Well, how does a device know which CPU uses it? > >I suspect we are better off waiting for 1.0 with this one. > > Good point. It just feels like a heavy layering violation to have a > global variable for all virtio devices. What if we start mixing > legacy and 1.0 devices? We could reuse the same flag, but it would > be initialized differently per device. > > > Alex 1.0 won't use the flag, it's all LE. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio. 2014-02-18 15:11 ` Michael S. Tsirkin @ 2014-02-18 15:07 ` Alexander Graf 0 siblings, 0 replies; 36+ messages in thread From: Alexander Graf @ 2014-02-18 15:07 UTC (permalink / raw) To: Michael S. Tsirkin Cc: kwolf, peter.maydell, thuth, marc.zyngier, rusty, qemu-devel, stefanha, anthony, pbonzini, afaerber, Greg Kurz On 02/18/2014 04:11 PM, Michael S. Tsirkin wrote: > On Tue, Feb 18, 2014 at 04:02:18PM +0100, Alexander Graf wrote: >> On 02/18/2014 04:03 PM, Michael S. Tsirkin wrote: >>> On Tue, Feb 18, 2014 at 03:48:38PM +0100, Alexander Graf wrote: >>>> On 02/18/2014 01:38 PM, Greg Kurz wrote: >>>>> From: Rusty Russell <rusty@rustcorp.com.au> >>>>> >>>>> virtio data structures are defined as "target endian", which assumes >>>>> that's a fixed value. In fact, that actually means it's >>>>> platform-specific. >>>>> >>>>> The OASIS virtio 1.0 spec will fix this. Meanwhile, create a hook for >>>>> little endian ppc (and potentially ARM). This is called at device >>>>> reset time (which is done before any driver is loaded) since it >>>>> may involve a system call to get the status when running under kvm. >>>>> >>>>> [ fixed checkpatch.pl error with the virtio_byteswap initialisation, >>>>> ldq_phys() API change, Greg Kurz <gkurz@linux.vnet.ibm.com> ] >>>>> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> >>>>> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> >>>>> --- >>>>> hw/virtio/virtio.c | 6 ++ >>>>> include/hw/virtio/virtio-access.h | 132 +++++++++++++++++++++++++++++++++++++ >>>>> include/hw/virtio/virtio.h | 2 + >>>>> stubs/Makefile.objs | 1 >>>>> stubs/virtio_get_byteswap.c | 6 ++ >>>>> 5 files changed, 147 insertions(+) >>>>> create mode 100644 include/hw/virtio/virtio-access.h >>>>> create mode 100644 stubs/virtio_get_byteswap.c >>>>> >>>>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c >>>>> index aeabf3a..4fd6ac2 100644 >>>>> --- a/hw/virtio/virtio.c >>>>> +++ b/hw/virtio/virtio.c >>>>> @@ -19,6 +19,9 @@ >>>>> #include "hw/virtio/virtio.h" >>>>> #include "qemu/atomic.h" >>>>> #include "hw/virtio/virtio-bus.h" >>>>> +#include "hw/virtio/virtio-access.h" >>>>> + >>>>> +bool virtio_byteswap; >>>> Could this be a virtio object property rather than a global? Imagine >>>> an AMP guest system with a BE and an LE system running in parallel >>>> accessing two separate virtio devices. With a single global that >>>> would break. >>>> >>>> >>>> Alex >>> Well, how does a device know which CPU uses it? >>> I suspect we are better off waiting for 1.0 with this one. >> Good point. It just feels like a heavy layering violation to have a >> global variable for all virtio devices. What if we start mixing >> legacy and 1.0 devices? We could reuse the same flag, but it would >> be initialized differently per device. >> >> >> Alex > 1.0 won't use the flag, it's all LE. At which point we need another flag indicating that the fields are always LE which means we can reuse this flag, no? Alex ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio. 2014-02-18 15:03 ` Michael S. Tsirkin 2014-02-18 15:02 ` Alexander Graf @ 2014-02-18 15:04 ` Michael S. Tsirkin 2014-02-18 15:12 ` Cornelia Huck 2 siblings, 0 replies; 36+ messages in thread From: Michael S. Tsirkin @ 2014-02-18 15:04 UTC (permalink / raw) To: Alexander Graf Cc: kwolf, peter.maydell, thuth, marc.zyngier, rusty, qemu-devel, stefanha, anthony, pbonzini, afaerber, Greg Kurz On Tue, Feb 18, 2014 at 05:03:27PM +0200, Michael S. Tsirkin wrote: > On Tue, Feb 18, 2014 at 03:48:38PM +0100, Alexander Graf wrote: > > On 02/18/2014 01:38 PM, Greg Kurz wrote: > > >From: Rusty Russell <rusty@rustcorp.com.au> > > > > > >virtio data structures are defined as "target endian", which assumes > > >that's a fixed value. In fact, that actually means it's > > >platform-specific. > > > > > >The OASIS virtio 1.0 spec will fix this. Meanwhile, create a hook for > > >little endian ppc (and potentially ARM). This is called at device > > >reset time (which is done before any driver is loaded) since it > > >may involve a system call to get the status when running under kvm. > > > > > >[ fixed checkpatch.pl error with the virtio_byteswap initialisation, > > > ldq_phys() API change, Greg Kurz <gkurz@linux.vnet.ibm.com> ] > > >Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> > > >Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> > > >--- > > > hw/virtio/virtio.c | 6 ++ > > > include/hw/virtio/virtio-access.h | 132 +++++++++++++++++++++++++++++++++++++ > > > include/hw/virtio/virtio.h | 2 + > > > stubs/Makefile.objs | 1 > > > stubs/virtio_get_byteswap.c | 6 ++ > > > 5 files changed, 147 insertions(+) > > > create mode 100644 include/hw/virtio/virtio-access.h > > > create mode 100644 stubs/virtio_get_byteswap.c > > > > > >diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > > >index aeabf3a..4fd6ac2 100644 > > >--- a/hw/virtio/virtio.c > > >+++ b/hw/virtio/virtio.c > > >@@ -19,6 +19,9 @@ > > > #include "hw/virtio/virtio.h" > > > #include "qemu/atomic.h" > > > #include "hw/virtio/virtio-bus.h" > > >+#include "hw/virtio/virtio-access.h" > > >+ > > >+bool virtio_byteswap; > > > > Could this be a virtio object property rather than a global? Imagine > > an AMP guest system with a BE and an LE system running in parallel > > accessing two separate virtio devices. With a single global that > > would break. > > > > > > Alex > > Well, how does a device know which CPU uses it? > I suspect we are better off waiting for 1.0 with this one. I mean an AMP guest system with a BE and an LE system here. Not this patchset itself. > -- > MST ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio. 2014-02-18 15:03 ` Michael S. Tsirkin 2014-02-18 15:02 ` Alexander Graf 2014-02-18 15:04 ` Michael S. Tsirkin @ 2014-02-18 15:12 ` Cornelia Huck 2014-02-18 15:45 ` Cornelia Huck 2 siblings, 1 reply; 36+ messages in thread From: Cornelia Huck @ 2014-02-18 15:12 UTC (permalink / raw) To: Michael S. Tsirkin Cc: kwolf, peter.maydell, thuth, marc.zyngier, rusty, Alexander Graf, qemu-devel, stefanha, anthony, pbonzini, afaerber, Greg Kurz On Tue, 18 Feb 2014 17:03:27 +0200 "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Tue, Feb 18, 2014 at 03:48:38PM +0100, Alexander Graf wrote: > > On 02/18/2014 01:38 PM, Greg Kurz wrote: > > >From: Rusty Russell <rusty@rustcorp.com.au> > > > > > >virtio data structures are defined as "target endian", which assumes > > >that's a fixed value. In fact, that actually means it's > > >platform-specific. > > > > > >The OASIS virtio 1.0 spec will fix this. Meanwhile, create a hook for > > >little endian ppc (and potentially ARM). This is called at device > > >reset time (which is done before any driver is loaded) since it > > >may involve a system call to get the status when running under kvm. > > > > > >[ fixed checkpatch.pl error with the virtio_byteswap initialisation, > > > ldq_phys() API change, Greg Kurz <gkurz@linux.vnet.ibm.com> ] > > >Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> > > >Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> > > >--- > > > hw/virtio/virtio.c | 6 ++ > > > include/hw/virtio/virtio-access.h | 132 +++++++++++++++++++++++++++++++++++++ > > > include/hw/virtio/virtio.h | 2 + > > > stubs/Makefile.objs | 1 > > > stubs/virtio_get_byteswap.c | 6 ++ > > > 5 files changed, 147 insertions(+) > > > create mode 100644 include/hw/virtio/virtio-access.h > > > create mode 100644 stubs/virtio_get_byteswap.c > > > > > >diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > > >index aeabf3a..4fd6ac2 100644 > > >--- a/hw/virtio/virtio.c > > >+++ b/hw/virtio/virtio.c > > >@@ -19,6 +19,9 @@ > > > #include "hw/virtio/virtio.h" > > > #include "qemu/atomic.h" > > > #include "hw/virtio/virtio-bus.h" > > >+#include "hw/virtio/virtio-access.h" > > >+ > > >+bool virtio_byteswap; > > > > Could this be a virtio object property rather than a global? Imagine > > an AMP guest system with a BE and an LE system running in parallel > > accessing two separate virtio devices. With a single global that > > would break. > > > > > > Alex > > Well, how does a device know which CPU uses it? > I suspect we are better off waiting for 1.0 with this one. > 1.0 makes this a bit more complex, no? virtio-endian accessors are defined by the endianness of host and guest (doing a bswap depends on the host/guest combination). This needs to be per qemu instance. (ioctl under kvm? machine option?) For 1.0, we'll have everything le, so a be host will always do a bswap (as will a be guest). But whether a device is 1.0 or legacy is not something that can be decided globally, or we can't have transitional devices with qemu. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio. 2014-02-18 15:12 ` Cornelia Huck @ 2014-02-18 15:45 ` Cornelia Huck 2014-02-18 16:02 ` Alexander Graf 0 siblings, 1 reply; 36+ messages in thread From: Cornelia Huck @ 2014-02-18 15:45 UTC (permalink / raw) To: qemu-devel Cc: kwolf, peter.maydell, thuth, Michael S. Tsirkin, marc.zyngier, rusty, Alexander Graf, stefanha, anthony, pbonzini, afaerber, Greg Kurz On Tue, 18 Feb 2014 16:12:08 +0100 Cornelia Huck <cornelia.huck@de.ibm.com> wrote: > On Tue, 18 Feb 2014 17:03:27 +0200 > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > On Tue, Feb 18, 2014 at 03:48:38PM +0100, Alexander Graf wrote: > > > On 02/18/2014 01:38 PM, Greg Kurz wrote: > > > >From: Rusty Russell <rusty@rustcorp.com.au> > > > > > > > >virtio data structures are defined as "target endian", which assumes > > > >that's a fixed value. In fact, that actually means it's > > > >platform-specific. > > > > > > > >The OASIS virtio 1.0 spec will fix this. Meanwhile, create a hook for > > > >little endian ppc (and potentially ARM). This is called at device > > > >reset time (which is done before any driver is loaded) since it > > > >may involve a system call to get the status when running under kvm. > > > > > > > >[ fixed checkpatch.pl error with the virtio_byteswap initialisation, > > > > ldq_phys() API change, Greg Kurz <gkurz@linux.vnet.ibm.com> ] > > > >Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> > > > >Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> > > > >--- > > > > hw/virtio/virtio.c | 6 ++ > > > > include/hw/virtio/virtio-access.h | 132 +++++++++++++++++++++++++++++++++++++ > > > > include/hw/virtio/virtio.h | 2 + > > > > stubs/Makefile.objs | 1 > > > > stubs/virtio_get_byteswap.c | 6 ++ > > > > 5 files changed, 147 insertions(+) > > > > create mode 100644 include/hw/virtio/virtio-access.h > > > > create mode 100644 stubs/virtio_get_byteswap.c > > > > > > > >diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > > > >index aeabf3a..4fd6ac2 100644 > > > >--- a/hw/virtio/virtio.c > > > >+++ b/hw/virtio/virtio.c > > > >@@ -19,6 +19,9 @@ > > > > #include "hw/virtio/virtio.h" > > > > #include "qemu/atomic.h" > > > > #include "hw/virtio/virtio-bus.h" > > > >+#include "hw/virtio/virtio-access.h" > > > >+ > > > >+bool virtio_byteswap; > > > > > > Could this be a virtio object property rather than a global? Imagine > > > an AMP guest system with a BE and an LE system running in parallel > > > accessing two separate virtio devices. With a single global that > > > would break. > > > > > > > > > Alex > > > > Well, how does a device know which CPU uses it? > > I suspect we are better off waiting for 1.0 with this one. > > > > 1.0 makes this a bit more complex, no? > > virtio-endian accessors are defined by the endianness of host and guest > (doing a bswap depends on the host/guest combination). This needs to be > per qemu instance. (ioctl under kvm? machine option?) > > For 1.0, we'll have everything le, so a be host will always do a bswap > (as will a be guest). But whether a device is 1.0 or legacy is not > something that can be decided globally, or we can't have transitional > devices with qemu. > So here are two stupid tables on who needs to do byteswaps, one for legacy devices, one for 1.0 devices: legacy devices: host be le g be host no host yes u guest no guest no e s le host yes host no t guest no guest no virtio 1.0 devices: host be le g be host yes host no u guest yes guest yes e s le host yes host no t guest no guest no This means byteswaps in qemu always depend on guest-endianness for legacy and on host-endianness for 1.0. If we want to support transitional devices with a mixture of legacy/1.0, we'll need both a per-machine and per-device swap flag: virtio_whatever(device, parameters...) { if (device->legacy) { if (guest_needs_byteswap) { whatever_byteswap(parameters...); } else { whatever(parameters...); } } else { /* 1.0 */ whatever_le(parameters...); } } Comments? ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio. 2014-02-18 15:45 ` Cornelia Huck @ 2014-02-18 16:02 ` Alexander Graf 2014-02-18 16:17 ` Cornelia Huck 0 siblings, 1 reply; 36+ messages in thread From: Alexander Graf @ 2014-02-18 16:02 UTC (permalink / raw) To: Cornelia Huck Cc: kwolf, peter.maydell, thuth, Michael S. Tsirkin, marc.zyngier, rusty, qemu-devel, stefanha, anthony, pbonzini, afaerber, Greg Kurz > Am 18.02.2014 um 16:45 schrieb Cornelia Huck <cornelia.huck@de.ibm.com>: > > On Tue, 18 Feb 2014 16:12:08 +0100 > Cornelia Huck <cornelia.huck@de.ibm.com> wrote: > >> On Tue, 18 Feb 2014 17:03:27 +0200 >> "Michael S. Tsirkin" <mst@redhat.com> wrote: >> >>>> On Tue, Feb 18, 2014 at 03:48:38PM +0100, Alexander Graf wrote: >>>>> On 02/18/2014 01:38 PM, Greg Kurz wrote: >>>>> From: Rusty Russell <rusty@rustcorp.com.au> >>>>> >>>>> virtio data structures are defined as "target endian", which assumes >>>>> that's a fixed value. In fact, that actually means it's >>>>> platform-specific. >>>>> >>>>> The OASIS virtio 1.0 spec will fix this. Meanwhile, create a hook for >>>>> little endian ppc (and potentially ARM). This is called at device >>>>> reset time (which is done before any driver is loaded) since it >>>>> may involve a system call to get the status when running under kvm. >>>>> >>>>> [ fixed checkpatch.pl error with the virtio_byteswap initialisation, >>>>> ldq_phys() API change, Greg Kurz <gkurz@linux.vnet.ibm.com> ] >>>>> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> >>>>> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> >>>>> --- >>>>> hw/virtio/virtio.c | 6 ++ >>>>> include/hw/virtio/virtio-access.h | 132 +++++++++++++++++++++++++++++++++++++ >>>>> include/hw/virtio/virtio.h | 2 + >>>>> stubs/Makefile.objs | 1 >>>>> stubs/virtio_get_byteswap.c | 6 ++ >>>>> 5 files changed, 147 insertions(+) >>>>> create mode 100644 include/hw/virtio/virtio-access.h >>>>> create mode 100644 stubs/virtio_get_byteswap.c >>>>> >>>>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c >>>>> index aeabf3a..4fd6ac2 100644 >>>>> --- a/hw/virtio/virtio.c >>>>> +++ b/hw/virtio/virtio.c >>>>> @@ -19,6 +19,9 @@ >>>>> #include "hw/virtio/virtio.h" >>>>> #include "qemu/atomic.h" >>>>> #include "hw/virtio/virtio-bus.h" >>>>> +#include "hw/virtio/virtio-access.h" >>>>> + >>>>> +bool virtio_byteswap; >>>> >>>> Could this be a virtio object property rather than a global? Imagine >>>> an AMP guest system with a BE and an LE system running in parallel >>>> accessing two separate virtio devices. With a single global that >>>> would break. >>>> >>>> >>>> Alex >>> >>> Well, how does a device know which CPU uses it? >>> I suspect we are better off waiting for 1.0 with this one. >> >> 1.0 makes this a bit more complex, no? >> >> virtio-endian accessors are defined by the endianness of host and guest >> (doing a bswap depends on the host/guest combination). This needs to be >> per qemu instance. (ioctl under kvm? machine option?) >> >> For 1.0, we'll have everything le, so a be host will always do a bswap >> (as will a be guest). But whether a device is 1.0 or legacy is not >> something that can be decided globally, or we can't have transitional >> devices with qemu. > > So here are two stupid tables on who needs to do byteswaps, one for > legacy devices, one for 1.0 devices: > > legacy devices: > > host > be le > > g be host no host yes > u guest no guest no > e > s le host yes host no > t guest no guest no > > > > virtio 1.0 devices: > > host > be le > > g be host yes host no > u guest yes guest yes > e > s le host yes host no > t guest no guest no > > > This means byteswaps in qemu always depend on guest-endianness for > legacy and on host-endianness for 1.0. If we want to support > transitional devices with a mixture of legacy/1.0, we'll need both a > per-machine and per-device swap flag: > > virtio_whatever(device, parameters...) > { > if (device->legacy) { > if (guest_needs_byteswap) { > whatever_byteswap(parameters...); > } else { > whatever(parameters...); > } > } else { /* 1.0 */ > whatever_le(parameters...); > } > } > > Comments? Yes. My point was that we could move all of the ifery above into the device reset function and from then on only use the swap bool property. Alex > ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio. 2014-02-18 16:02 ` Alexander Graf @ 2014-02-18 16:17 ` Cornelia Huck 2014-02-18 16:21 ` Alexander Graf 0 siblings, 1 reply; 36+ messages in thread From: Cornelia Huck @ 2014-02-18 16:17 UTC (permalink / raw) To: Alexander Graf Cc: kwolf, peter.maydell, thuth, Michael S. Tsirkin, marc.zyngier, rusty, qemu-devel, stefanha, anthony, pbonzini, afaerber, Greg Kurz On Tue, 18 Feb 2014 17:02:18 +0100 Alexander Graf <agraf@suse.de> wrote: > > > > Am 18.02.2014 um 16:45 schrieb Cornelia Huck <cornelia.huck@de.ibm.com>: > > > > On Tue, 18 Feb 2014 16:12:08 +0100 > > Cornelia Huck <cornelia.huck@de.ibm.com> wrote: > > > >> On Tue, 18 Feb 2014 17:03:27 +0200 > >> "Michael S. Tsirkin" <mst@redhat.com> wrote: > >> > >>>> On Tue, Feb 18, 2014 at 03:48:38PM +0100, Alexander Graf wrote: > >>>>> On 02/18/2014 01:38 PM, Greg Kurz wrote: > >>>>> From: Rusty Russell <rusty@rustcorp.com.au> > >>>>> > >>>>> virtio data structures are defined as "target endian", which assumes > >>>>> that's a fixed value. In fact, that actually means it's > >>>>> platform-specific. > >>>>> > >>>>> The OASIS virtio 1.0 spec will fix this. Meanwhile, create a hook for > >>>>> little endian ppc (and potentially ARM). This is called at device > >>>>> reset time (which is done before any driver is loaded) since it > >>>>> may involve a system call to get the status when running under kvm. > >>>>> > >>>>> [ fixed checkpatch.pl error with the virtio_byteswap initialisation, > >>>>> ldq_phys() API change, Greg Kurz <gkurz@linux.vnet.ibm.com> ] > >>>>> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> > >>>>> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> > >>>>> --- > >>>>> hw/virtio/virtio.c | 6 ++ > >>>>> include/hw/virtio/virtio-access.h | 132 +++++++++++++++++++++++++++++++++++++ > >>>>> include/hw/virtio/virtio.h | 2 + > >>>>> stubs/Makefile.objs | 1 > >>>>> stubs/virtio_get_byteswap.c | 6 ++ > >>>>> 5 files changed, 147 insertions(+) > >>>>> create mode 100644 include/hw/virtio/virtio-access.h > >>>>> create mode 100644 stubs/virtio_get_byteswap.c > >>>>> > >>>>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > >>>>> index aeabf3a..4fd6ac2 100644 > >>>>> --- a/hw/virtio/virtio.c > >>>>> +++ b/hw/virtio/virtio.c > >>>>> @@ -19,6 +19,9 @@ > >>>>> #include "hw/virtio/virtio.h" > >>>>> #include "qemu/atomic.h" > >>>>> #include "hw/virtio/virtio-bus.h" > >>>>> +#include "hw/virtio/virtio-access.h" > >>>>> + > >>>>> +bool virtio_byteswap; > >>>> > >>>> Could this be a virtio object property rather than a global? Imagine > >>>> an AMP guest system with a BE and an LE system running in parallel > >>>> accessing two separate virtio devices. With a single global that > >>>> would break. > >>>> > >>>> > >>>> Alex > >>> > >>> Well, how does a device know which CPU uses it? > >>> I suspect we are better off waiting for 1.0 with this one. > >> > >> 1.0 makes this a bit more complex, no? > >> > >> virtio-endian accessors are defined by the endianness of host and guest > >> (doing a bswap depends on the host/guest combination). This needs to be > >> per qemu instance. (ioctl under kvm? machine option?) > >> > >> For 1.0, we'll have everything le, so a be host will always do a bswap > >> (as will a be guest). But whether a device is 1.0 or legacy is not > >> something that can be decided globally, or we can't have transitional > >> devices with qemu. > > > > So here are two stupid tables on who needs to do byteswaps, one for > > legacy devices, one for 1.0 devices: > > > > legacy devices: > > > > host > > be le > > > > g be host no host yes > > u guest no guest no > > e > > s le host yes host no > > t guest no guest no > > > > > > > > virtio 1.0 devices: > > > > host > > be le > > > > g be host yes host no > > u guest yes guest yes > > e > > s le host yes host no > > t guest no guest no > > > > > > This means byteswaps in qemu always depend on guest-endianness for > > legacy and on host-endianness for 1.0. If we want to support > > transitional devices with a mixture of legacy/1.0, we'll need both a > > per-machine and per-device swap flag: > > > > virtio_whatever(device, parameters...) > > { > > if (device->legacy) { > > if (guest_needs_byteswap) { > > whatever_byteswap(parameters...); > > } else { > > whatever(parameters...); > > } > > } else { /* 1.0 */ > > whatever_le(parameters...); > > } > > } > > > > Comments? > > Yes. My point was that we could move all of the ifery above into the device reset function and from then on only use the swap bool property. > > Alex > Hm. So whatever_le for 1.0 devices, and virtio_whatever (checking the byteswap value) for legacy devices? The device implementation will be aware of the virtio version anyway. (Btw., can some of those architectures supporting both le/be run with mixed le/be cpus? That would be a mess for legacy devices.) ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio. 2014-02-18 16:17 ` Cornelia Huck @ 2014-02-18 16:21 ` Alexander Graf 2014-02-20 23:26 ` Rusty Russell 0 siblings, 1 reply; 36+ messages in thread From: Alexander Graf @ 2014-02-18 16:21 UTC (permalink / raw) To: Cornelia Huck Cc: kwolf, peter.maydell, thuth, Michael S. Tsirkin, marc.zyngier, rusty, qemu-devel, stefanha, anthony, pbonzini, afaerber, Greg Kurz On 02/18/2014 05:17 PM, Cornelia Huck wrote: > On Tue, 18 Feb 2014 17:02:18 +0100 > Alexander Graf <agraf@suse.de> wrote: > >> >>> Am 18.02.2014 um 16:45 schrieb Cornelia Huck <cornelia.huck@de.ibm.com>: >>> >>> On Tue, 18 Feb 2014 16:12:08 +0100 >>> Cornelia Huck <cornelia.huck@de.ibm.com> wrote: >>> >>>> On Tue, 18 Feb 2014 17:03:27 +0200 >>>> "Michael S. Tsirkin" <mst@redhat.com> wrote: >>>> >>>>>> On Tue, Feb 18, 2014 at 03:48:38PM +0100, Alexander Graf wrote: >>>>>>> On 02/18/2014 01:38 PM, Greg Kurz wrote: >>>>>>> From: Rusty Russell <rusty@rustcorp.com.au> >>>>>>> >>>>>>> virtio data structures are defined as "target endian", which assumes >>>>>>> that's a fixed value. In fact, that actually means it's >>>>>>> platform-specific. >>>>>>> >>>>>>> The OASIS virtio 1.0 spec will fix this. Meanwhile, create a hook for >>>>>>> little endian ppc (and potentially ARM). This is called at device >>>>>>> reset time (which is done before any driver is loaded) since it >>>>>>> may involve a system call to get the status when running under kvm. >>>>>>> >>>>>>> [ fixed checkpatch.pl error with the virtio_byteswap initialisation, >>>>>>> ldq_phys() API change, Greg Kurz <gkurz@linux.vnet.ibm.com> ] >>>>>>> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> >>>>>>> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> >>>>>>> --- >>>>>>> hw/virtio/virtio.c | 6 ++ >>>>>>> include/hw/virtio/virtio-access.h | 132 +++++++++++++++++++++++++++++++++++++ >>>>>>> include/hw/virtio/virtio.h | 2 + >>>>>>> stubs/Makefile.objs | 1 >>>>>>> stubs/virtio_get_byteswap.c | 6 ++ >>>>>>> 5 files changed, 147 insertions(+) >>>>>>> create mode 100644 include/hw/virtio/virtio-access.h >>>>>>> create mode 100644 stubs/virtio_get_byteswap.c >>>>>>> >>>>>>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c >>>>>>> index aeabf3a..4fd6ac2 100644 >>>>>>> --- a/hw/virtio/virtio.c >>>>>>> +++ b/hw/virtio/virtio.c >>>>>>> @@ -19,6 +19,9 @@ >>>>>>> #include "hw/virtio/virtio.h" >>>>>>> #include "qemu/atomic.h" >>>>>>> #include "hw/virtio/virtio-bus.h" >>>>>>> +#include "hw/virtio/virtio-access.h" >>>>>>> + >>>>>>> +bool virtio_byteswap; >>>>>> Could this be a virtio object property rather than a global? Imagine >>>>>> an AMP guest system with a BE and an LE system running in parallel >>>>>> accessing two separate virtio devices. With a single global that >>>>>> would break. >>>>>> >>>>>> >>>>>> Alex >>>>> Well, how does a device know which CPU uses it? >>>>> I suspect we are better off waiting for 1.0 with this one. >>>> 1.0 makes this a bit more complex, no? >>>> >>>> virtio-endian accessors are defined by the endianness of host and guest >>>> (doing a bswap depends on the host/guest combination). This needs to be >>>> per qemu instance. (ioctl under kvm? machine option?) >>>> >>>> For 1.0, we'll have everything le, so a be host will always do a bswap >>>> (as will a be guest). But whether a device is 1.0 or legacy is not >>>> something that can be decided globally, or we can't have transitional >>>> devices with qemu. >>> So here are two stupid tables on who needs to do byteswaps, one for >>> legacy devices, one for 1.0 devices: >>> >>> legacy devices: >>> >>> host >>> be le >>> >>> g be host no host yes >>> u guest no guest no >>> e >>> s le host yes host no >>> t guest no guest no >>> >>> >>> >>> virtio 1.0 devices: >>> >>> host >>> be le >>> >>> g be host yes host no >>> u guest yes guest yes >>> e >>> s le host yes host no >>> t guest no guest no >>> >>> >>> This means byteswaps in qemu always depend on guest-endianness for >>> legacy and on host-endianness for 1.0. If we want to support >>> transitional devices with a mixture of legacy/1.0, we'll need both a >>> per-machine and per-device swap flag: >>> >>> virtio_whatever(device, parameters...) >>> { >>> if (device->legacy) { >>> if (guest_needs_byteswap) { >>> whatever_byteswap(parameters...); >>> } else { >>> whatever(parameters...); >>> } >>> } else { /* 1.0 */ >>> whatever_le(parameters...); >>> } >>> } >>> >>> Comments? >> Yes. My point was that we could move all of the ifery above into the device reset function and from then on only use the swap bool property. >> >> Alex >> > Hm. So whatever_le for 1.0 devices, and virtio_whatever (checking the > byteswap value) for legacy devices? The device implementation will be > aware of the virtio version anyway. Yeah, but I would hope we want to share as much code as possible here, so that config accessors can be shared between legacy virtio and 1.0 virtio. And in that case we want to have a generic helper to read/write pieces of that config space - which this patch introduces for us :). > (Btw., can some of those architectures supporting both le/be run with > mixed le/be cpus? That would be a mess for legacy devices.) Yes, they can. No, we don't care :). Alex ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio. 2014-02-18 16:21 ` Alexander Graf @ 2014-02-20 23:26 ` Rusty Russell 0 siblings, 0 replies; 36+ messages in thread From: Rusty Russell @ 2014-02-20 23:26 UTC (permalink / raw) To: Alexander Graf, Cornelia Huck Cc: kwolf, peter.maydell, thuth, Michael S. Tsirkin, marc.zyngier, qemu-devel, stefanha, anthony, pbonzini, afaerber, Greg Kurz Alexander Graf <agraf@suse.de> writes: > On 02/18/2014 05:17 PM, Cornelia Huck wrote: >> Hm. So whatever_le for 1.0 devices, and virtio_whatever (checking the >> byteswap value) for legacy devices? The device implementation will be >> aware of the virtio version anyway. > > Yeah, but I would hope we want to share as much code as possible here, > so that config accessors can be shared between legacy virtio and 1.0 > virtio. And in that case we want to have a generic helper to read/write > pieces of that config space - which this patch introduces for us :). > >> (Btw., can some of those architectures supporting both le/be run with >> mixed le/be cpus? That would be a mess for legacy devices.) > > Yes, they can. No, we don't care :). There's per-cpu (what endian) and per-device (legacy or no). To do this Right, we'd need to consult both, but we don't always have that information. So this the minimal viable solution. We'll need a per-device legacy flag eventually (there are other changes than just endian). But getting the wrappers in place is a good first step. Cheers, Rusty. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio. 2014-02-18 14:48 ` Alexander Graf 2014-02-18 15:03 ` Michael S. Tsirkin @ 2014-02-18 23:02 ` Andreas Färber 1 sibling, 0 replies; 36+ messages in thread From: Andreas Färber @ 2014-02-18 23:02 UTC (permalink / raw) To: Alexander Graf Cc: kwolf, peter.maydell, thuth, mst, marc.zyngier, rusty, qemu-devel, stefanha, anthony, pbonzini, Greg Kurz Am 18.02.2014 15:48, schrieb Alexander Graf: > On 02/18/2014 01:38 PM, Greg Kurz wrote: >> From: Rusty Russell <rusty@rustcorp.com.au> >> >> virtio data structures are defined as "target endian", which assumes >> that's a fixed value. In fact, that actually means it's >> platform-specific. >> >> The OASIS virtio 1.0 spec will fix this. Meanwhile, create a hook for >> little endian ppc (and potentially ARM). This is called at device >> reset time (which is done before any driver is loaded) since it >> may involve a system call to get the status when running under kvm. >> >> [ fixed checkpatch.pl error with the virtio_byteswap initialisation, >> ldq_phys() API change, Greg Kurz <gkurz@linux.vnet.ibm.com> ] >> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> >> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> >> --- >> hw/virtio/virtio.c | 6 ++ >> include/hw/virtio/virtio-access.h | 132 >> +++++++++++++++++++++++++++++++++++++ >> include/hw/virtio/virtio.h | 2 + >> stubs/Makefile.objs | 1 >> stubs/virtio_get_byteswap.c | 6 ++ >> 5 files changed, 147 insertions(+) >> create mode 100644 include/hw/virtio/virtio-access.h >> create mode 100644 stubs/virtio_get_byteswap.c >> >> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c >> index aeabf3a..4fd6ac2 100644 >> --- a/hw/virtio/virtio.c >> +++ b/hw/virtio/virtio.c >> @@ -19,6 +19,9 @@ >> #include "hw/virtio/virtio.h" >> #include "qemu/atomic.h" >> #include "hw/virtio/virtio-bus.h" >> +#include "hw/virtio/virtio-access.h" >> + >> +bool virtio_byteswap; > > Could this be a virtio object property rather than a global? Imagine an > AMP guest system with a BE and an LE system running in parallel > accessing two separate virtio devices. With a single global that would > break. Déjà vu... ;) http://lists.nongnu.org/archive/html/qemu-devel/2013-08/msg01043.html Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio. 2014-02-18 12:38 ` [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio Greg Kurz 2014-02-18 14:48 ` Alexander Graf @ 2014-02-18 19:25 ` Andreas Färber 2014-02-19 10:06 ` Greg Kurz 1 sibling, 1 reply; 36+ messages in thread From: Andreas Färber @ 2014-02-18 19:25 UTC (permalink / raw) To: Greg Kurz, Rusty Russell Cc: kwolf, peter.maydell, thuth, mst, marc.zyngier, agraf, qemu-devel, stefanha, anthony, pbonzini Am 18.02.2014 13:38, schrieb Greg Kurz: > diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h > new file mode 100644 > index 0000000..2e22a47 > --- /dev/null > +++ b/include/hw/virtio/virtio-access.h > @@ -0,0 +1,132 @@ > +/* > + * Virtio Accessor Support: In case your target can change endian. > + * > + * Copyright IBM, Corp. 2013 > + * > + * Authors: > + * Rusty Russell <rusty@au.ibm.com> > + * > + * This work is licensed under the terms of the GNU GPL, version 2. See > + * the COPYING file in the top-level directory. [snip] I notice that this has been GPL-2.0 from Rusty's first series on. Is there a reason not to make the new file GPL-2.0+? Cf. http://wiki.qemu.org/Relicensing Thanks, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio. 2014-02-18 19:25 ` Andreas Färber @ 2014-02-19 10:06 ` Greg Kurz 2014-02-20 23:19 ` Rusty Russell 0 siblings, 1 reply; 36+ messages in thread From: Greg Kurz @ 2014-02-19 10:06 UTC (permalink / raw) To: Rusty Russell Cc: kwolf, peter.maydell, thuth, mst, marc.zyngier, agraf, qemu-devel, stefanha, anthony, pbonzini, Andreas Färber On Tue, 18 Feb 2014 20:25:15 +0100 Andreas Färber <afaerber@suse.de> wrote: > Am 18.02.2014 13:38, schrieb Greg Kurz: > > diff --git a/include/hw/virtio/virtio-access.h > > b/include/hw/virtio/virtio-access.h new file mode 100644 > > index 0000000..2e22a47 > > --- /dev/null > > +++ b/include/hw/virtio/virtio-access.h > > @@ -0,0 +1,132 @@ > > +/* > > + * Virtio Accessor Support: In case your target can change endian. > > + * > > + * Copyright IBM, Corp. 2013 > > + * > > + * Authors: > > + * Rusty Russell <rusty@au.ibm.com> > > + * > > + * This work is licensed under the terms of the GNU GPL, version 2. > > See > > + * the COPYING file in the top-level directory. > [snip] > > I notice that this has been GPL-2.0 from Rusty's first series on. Is > there a reason not to make the new file GPL-2.0+? > > Cf. http://wiki.qemu.org/Relicensing > > Thanks, > Andreas > Rusty ? It is your call. :) -- Gregory Kurz kurzgreg@fr.ibm.com gkurz@linux.vnet.ibm.com Software Engineer @ IBM/Meiosys http://www.ibm.com Tel +33 (0)562 165 496 "Anarchy is about taking complete responsibility for yourself." Alan Moore. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio. 2014-02-19 10:06 ` Greg Kurz @ 2014-02-20 23:19 ` Rusty Russell 0 siblings, 0 replies; 36+ messages in thread From: Rusty Russell @ 2014-02-20 23:19 UTC (permalink / raw) To: Greg Kurz Cc: kwolf, peter.maydell, thuth, mst, marc.zyngier, agraf, qemu-devel, stefanha, anthony, pbonzini, Andreas Färber Greg Kurz <gkurz@linux.vnet.ibm.com> writes: > On Tue, 18 Feb 2014 20:25:15 +0100 > Andreas Färber <afaerber@suse.de> wrote: >> Am 18.02.2014 13:38, schrieb Greg Kurz: >> > diff --git a/include/hw/virtio/virtio-access.h >> > b/include/hw/virtio/virtio-access.h new file mode 100644 >> > index 0000000..2e22a47 >> > --- /dev/null >> > +++ b/include/hw/virtio/virtio-access.h >> > @@ -0,0 +1,132 @@ >> > +/* >> > + * Virtio Accessor Support: In case your target can change endian. >> > + * >> > + * Copyright IBM, Corp. 2013 >> > + * >> > + * Authors: >> > + * Rusty Russell <rusty@au.ibm.com> >> > + * >> > + * This work is licensed under the terms of the GNU GPL, version 2. >> > See >> > + * the COPYING file in the top-level directory. >> [snip] >> >> I notice that this has been GPL-2.0 from Rusty's first series on. Is >> there a reason not to make the new file GPL-2.0+? >> >> Cf. http://wiki.qemu.org/Relicensing >> >> Thanks, >> Andreas >> > > Rusty ? It is your call. :) I cut & paste that header. I always prefer 2+ to 2 anyway. Please fix: * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by * the Free Software Foundation, either version 2 of the License, or * (at your option) any later version. Thanks, Rusty. ^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2014-02-21 2:31 UTC | newest] Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-08-12 7:59 [Qemu-devel] [PATCH 0/8] virtio for endian curious guests Take #2 Rusty Russell 2013-08-12 7:59 ` [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio Rusty Russell 2013-08-12 9:28 ` Benjamin Herrenschmidt 2013-08-12 9:39 ` Peter Maydell 2013-08-12 9:43 ` Benjamin Herrenschmidt 2013-08-12 9:45 ` Peter Maydell 2013-08-12 9:50 ` Benjamin Herrenschmidt 2013-08-12 9:52 ` Peter Maydell 2013-08-12 9:56 ` Benjamin Herrenschmidt 2013-08-12 10:36 ` Peter Maydell 2013-08-12 12:56 ` Anthony Liguori 2013-08-13 4:20 ` Rusty Russell 2013-08-13 5:30 ` Benjamin Herrenschmidt 2013-08-14 0:03 ` Rusty Russell 2013-09-06 2:27 ` Rusty Russell 2013-08-12 7:59 ` [Qemu-devel] [PATCH 2/8] target-ppc: ppc64 target's virtio can be either endian Rusty Russell 2013-08-12 7:59 ` [Qemu-devel] [PATCH 3/8] virtio: allow byte swapping for vring and config access Rusty Russell 2013-09-09 12:44 ` [Qemu-devel] [PATCH] hw/9pfs/virtio_9p_device: use virtio wrappers to access headers Greg Kurz 2013-09-10 5:21 ` Rusty Russell 2014-02-18 12:38 [Qemu-devel] [PATCH 0/8] virtio endian-ambivalent target fixes (rebased) Greg Kurz 2014-02-18 12:38 ` [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio Greg Kurz 2014-02-18 14:48 ` Alexander Graf 2014-02-18 15:03 ` Michael S. Tsirkin 2014-02-18 15:02 ` Alexander Graf 2014-02-18 15:11 ` Michael S. Tsirkin 2014-02-18 15:07 ` Alexander Graf 2014-02-18 15:04 ` Michael S. Tsirkin 2014-02-18 15:12 ` Cornelia Huck 2014-02-18 15:45 ` Cornelia Huck 2014-02-18 16:02 ` Alexander Graf 2014-02-18 16:17 ` Cornelia Huck 2014-02-18 16:21 ` Alexander Graf 2014-02-20 23:26 ` Rusty Russell 2014-02-18 23:02 ` Andreas Färber 2014-02-18 19:25 ` Andreas Färber 2014-02-19 10:06 ` Greg Kurz 2014-02-20 23:19 ` Rusty Russell
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.