On Fri, 28 Mar 2014 18:59:22 +0100 Andreas Färber <afaer...@suse.de> wrote:
> Am 28.03.2014 11:57, schrieb Greg Kurz: > > From: Rusty Russell <ru...@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, by making all little endian. > > > > We need to support both implementations and we want to share as much code > > as possible. > > > > A good way to do it is to introduce a per-device boolean property to tell > > memory accessors whether they should swap bytes or not. This flag should > > be set at device reset time, because: > > - endianness won't change while the device is in use, and if we reboot > > into a different endianness, a new device reset will occur > > - as suggested by Alexander Graf, we can keep all the logic to set the > > property in a single place and share all the virtio memory accessors > > between the two implementations > > > > For legacy devices, we rely on a per-platform hook to set the flag. The > > virtio 1.0 implementation will just have to add some more logic in > > virtio_reset() instead of calling the hook: > > > > if (vdev->legacy) { > > vdev->needs_byteswap = virtio_legacy_get_byteswap(); > > } else { > > #ifdef HOST_WORDS_BIGENDIAN > > vdev->needs_byteswap = true; > > #else > > vdev->needs_byteswap = false; > > #endif > > } > > > > The needs_byteswap flag is preserved accross migrations. > > "across" > > Why? :) For all targets except ppc this field does not change during > runtime. Do you intend to support ppc64 <-> ppc64le migration, i.e. > protection against changing HOST_WORDS_BIGENDIAN? > Because we only set this property at virtio_reset() time... how can we ensure it still has the correct value after a migration then ? And no, I don't intend to support cross-endian migration... The only endianness change that we can support is rebooting into a different endianness. > Since you're setting the field on reset rather than in instance_init or > realize, resetting the device on one host with changing ILE may lead to > weird endianness settings: Devices are initially reset before the VM > starts. ILE will always be Big Endian then IIUC, since all PowerPCCPU > models default to Big Endian and SLOF runs Big Endian. SLOF might use a > virtio-blk device to boot from. We then boot into SLES12 ppc64le - ILE > indicates Little Endian now. As soon as the guest triggers a reset of > the device, such as by resetting the whole PCI bus, endianness changes > to Little Endian. Now you indeed have an endianness on the source that > is different from that of the newly Big Endian reset device on the > destination. Is this desired, or did you accidentally initialize the > flag in the wrong place? > Hmmm... the assumption is that ALL virtio devices get reset after the guest kernel switches to LE. Are you saying this is not the case if SLOF uses a virtio-blk device to boot from ? This device would be handed over to the guest kernel to be used as is ? If yes, then I don't see how we can cope with that... The way legacy virtio is implemented, we cannot reasonably support an endianness change without fully reseting the device. I guess this is the main motivation behind virtio 1.0 :) > If we do need it, maybe you can place the field into a subsection to > avoid imposing it on x86? > I think it is needed anyway as long as we want to support a ppc64 guest that can change endianness and uses legacy virtio devices, even with a x86 host. > Regards, > Andreas > > > > > Signed-off-by: Rusty Russell <ru...@rustcorp.com.au> > > [ fixed checkpatch.pl error with the virtio_byteswap initialisation, > > ldq_phys() API change, > > relicensed virtio-access.h to GPLv2+ on Rusty's request, > > introduce a per-device needs_byteswap flag, > > add VirtIODevice * arg to virtio helpers, > > rename virtio_get_byteswap to virtio_legacy_get_byteswap, > > Greg Kurz <gk...@linux.vnet.ibm.com> ] > > Signed-off-by: Greg Kurz <gk...@linux.vnet.ibm.com> > Cheers. -- Gregory Kurz kurzg...@fr.ibm.com gk...@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.