[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v8 11/20] virtio: add endian-ambivalent support
From: |
Greg Kurz |
Subject: |
Re: [Qemu-devel] [PATCH v8 11/20] virtio: add endian-ambivalent support to VirtIODevice |
Date: |
Fri, 13 Jun 2014 14:52:46 +0200 |
On Fri, 13 Jun 2014 14:41:30 +0200
Alexander Graf <address@hidden> wrote:
>
> On 13.06.14 14:14, Greg Kurz wrote:
> > On Fri, 13 Jun 2014 13:46:39 +0200
> > Alexander Graf <address@hidden> 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 <address@hidden>
> >>> ---
> >>>
> >>> 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.
>
> Yeah, just combine it with virtio_set_endian_target_default.
>
> static enum foo virtio_default_endian(vdev)
> {
> if (target_words_bigendian()) {
> return VIRTIO_DEVICE_ENDIAN_BIG;
> ....
> }
>
> Instead of calling virtio_set_endian_target_default, you do
>
> vdev->device_endian = virtio_default_endian(vdev);
>
> and
>
> static bool virtio_biendian_section_needed(vdev)
> {
> assert(unknown);
> return vdev->device_endian != virtio_default_endian(vdev);
> }
>
>
> Alex
>
It sure looks better. Thanks !
--
Gregory Kurz address@hidden
address@hidden
Software Engineer @ IBM/Meiosys http://www.ibm.com
Tel +33 (0)562 165 496
"Anarchy is about taking complete responsibility for yourself."
Alan Moore.
- [Qemu-devel] [PATCH v8 09/20] exec: introduce target_words_bigendian() helper, (continued)
[Qemu-devel] [PATCH v8 12/20] virtio: memory accessors for endian-ambivalent targets, Greg Kurz, 2014/06/13
[Qemu-devel] [PATCH v8 13/20] virtio: allow byte swapping for vring, Greg Kurz, 2014/06/13
[Qemu-devel] [PATCH v8 14/20] virtio-net: use virtio wrappers to access headers, Greg Kurz, 2014/06/13
[Qemu-devel] [PATCH v8 15/20] virtio-balloon: use virtio wrappers to access page frame numbers, Greg Kurz, 2014/06/13
[Qemu-devel] [PATCH v8 16/20] virtio-blk: use virtio wrappers to access headers, Greg Kurz, 2014/06/13
[Qemu-devel] [PATCH v8 17/20] virtio-scsi: use virtio wrappers to access headers, Greg Kurz, 2014/06/13