On Fri, 13 Jun 2014 13:46:39 +0200 Alexander Graf <ag...@suse.de> wrote:
> > On 13.06.14 13:23, Greg Kurz wrote: > > Some CPU families can dynamically change their endianness. This means we > > can have little endian ppc or big endian arm guests for example. This has > > an impact on legacy virtio data structures since they are target endian. > > We hence introduce a new property to track the endianness of each virtio > > device. It is reasonnably assumed that endianness won't change while the > > device is in use : we hence capture the device endianness when it gets > > reset. > > > > We migrate this property in a subsection, after the device descriptor. This > > means the load code must not rely on it until it is restored. As a > > consequence, > > the vring sanity checks had to be moved after the call to > > vmstate_load_state(). > > We enforce paranoia by poisoning the property at the begining of > > virtio_load(). > > > > Signed-off-by: Greg Kurz <gk...@linux.vnet.ibm.com> > > --- > > > > Changes since last version (virtio migration RFC): > > - endianness cached at device reset time > > - reworked migration support > > > > hw/virtio/virtio-pci.c | 8 ++-- > > hw/virtio/virtio.c | 100 > > +++++++++++++++++++++++++++++++++++++++----- > > include/hw/virtio/virtio.h | 12 ++++- > > 3 files changed, 102 insertions(+), 18 deletions(-) > > > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > > index 390c8d2..2ffceb8 100644 > > --- a/hw/virtio/virtio-pci.c > > +++ b/hw/virtio/virtio-pci.c > > @@ -406,13 +406,13 @@ static uint64_t virtio_pci_config_read(void *opaque, > > hwaddr addr, > > break; > > case 2: > > val = virtio_config_readw(vdev, addr); > > - if (virtio_is_big_endian()) { > > + if (virtio_is_big_endian(vdev)) { > > val = bswap16(val); > > } > > break; > > case 4: > > val = virtio_config_readl(vdev, addr); > > - if (virtio_is_big_endian()) { > > + if (virtio_is_big_endian(vdev)) { > > val = bswap32(val); > > } > > break; > > @@ -440,13 +440,13 @@ static void virtio_pci_config_write(void *opaque, > > hwaddr addr, > > virtio_config_writeb(vdev, addr, val); > > break; > > case 2: > > - if (virtio_is_big_endian()) { > > + if (virtio_is_big_endian(vdev)) { > > val = bswap16(val); > > } > > virtio_config_writew(vdev, addr, val); > > break; > > case 4: > > - if (virtio_is_big_endian()) { > > + if (virtio_is_big_endian(vdev)) { > > val = bswap32(val); > > } > > virtio_config_writel(vdev, addr, val); > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > > index 16b73d9..6235df8 100644 > > --- a/hw/virtio/virtio.c > > +++ b/hw/virtio/virtio.c > > @@ -545,6 +545,24 @@ void virtio_set_status(VirtIODevice *vdev, uint8_t val) > > vdev->status = val; > > } > > > > +static void virtio_set_endian_target_default(VirtIODevice *vdev) > > +{ > > +#ifdef TARGET_WORDS_BIGENDIAN > > + vdev->device_endian = VIRTIO_DEVICE_ENDIAN_BIG; > > +#else > > + vdev->device_endian = VIRTIO_DEVICE_ENDIAN_LITTLE; > > +#endif > > +} > > + > > +static void virtio_set_endian_cpu(VirtIODevice *vdev, CPUState *cpu) > > +{ > > + if (cpu_virtio_is_big_endian(cpu)) { > > + vdev->device_endian = VIRTIO_DEVICE_ENDIAN_BIG; > > + } else { > > + vdev->device_endian = VIRTIO_DEVICE_ENDIAN_LITTLE; > > + } > > +} > > + > > void virtio_reset(void *opaque) > > { > > VirtIODevice *vdev = opaque; > > @@ -552,6 +570,13 @@ void virtio_reset(void *opaque) > > int i; > > > > virtio_set_status(vdev, 0); > > + if (current_cpu) { > > + /* Guest initiated reset */ > > + virtio_set_endian_cpu(vdev, current_cpu); > > + } else { > > + /* System reset */ > > + virtio_set_endian_target_default(vdev); > > + } > > > > if (k->reset) { > > k->reset(vdev); > > @@ -840,6 +865,28 @@ void virtio_notify_config(VirtIODevice *vdev) > > virtio_notify_vector(vdev, vdev->config_vector); > > } > > > > +static bool virtio_device_endian_needed(void *opaque) > > +{ > > + VirtIODevice *vdev = opaque; > > + > > + assert(vdev->device_endian != VIRTIO_DEVICE_ENDIAN_UNKNOWN); > > +#ifdef TARGET_WORDS_BIGENDIAN > > + return vdev->device_endian == VIRTIO_DEVICE_ENDIAN_LITTLE; > > +#else > > + return vdev->device_endian == VIRTIO_DEVICE_ENDIAN_BIG; > > +#endif > > if (target_words_bigendian()) { > return virtio_is_big_endian(); > } else { > return !virtio_is_big_endian(); > } > > Then we have one less dependency on TARGET_WORDS_BIGENDIAN which means a > step towards compiling virtio once ;). > > > Alex > Heh ! It used to be target_words_bigendian() in the first place and I turned it into TARGET_WORDS_BIGENDIAN to avoid "hey virtio isn't soon to be common why don't you sort this out at compile time?" remarks. :) Since we are definitely not on a fastpath here, I'll gladly fix this. -- 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.