From patchwork Wed May 14 15:42:30 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Greg Kurz X-Patchwork-Id: 348875 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 11E64140077 for ; Thu, 15 May 2014 01:44:59 +1000 (EST) Received: from localhost ([::1]:52519 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WkbMW-0002QY-Tp for incoming@patchwork.ozlabs.org; Wed, 14 May 2014 11:44:56 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58226) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WkbKT-00071t-MP for qemu-devel@nongnu.org; Wed, 14 May 2014 11:42:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WkbKK-00078o-3a for qemu-devel@nongnu.org; Wed, 14 May 2014 11:42:49 -0400 Received: from e06smtp12.uk.ibm.com ([195.75.94.108]:47626) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WkbKJ-00078Y-Ot for qemu-devel@nongnu.org; Wed, 14 May 2014 11:42:40 -0400 Received: from /spool/local by e06smtp12.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 14 May 2014 16:42:38 +0100 Received: from d06dlp02.portsmouth.uk.ibm.com (9.149.20.14) by e06smtp12.uk.ibm.com (192.168.101.142) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Wed, 14 May 2014 16:42:36 +0100 Received: from b06cxnps4074.portsmouth.uk.ibm.com (d06relay11.portsmouth.uk.ibm.com [9.149.109.196]) by d06dlp02.portsmouth.uk.ibm.com (Postfix) with ESMTP id E0DC72190042 for ; Wed, 14 May 2014 16:42:24 +0100 (BST) Received: from d06av06.portsmouth.uk.ibm.com (d06av06.portsmouth.uk.ibm.com [9.149.37.217]) by b06cxnps4074.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id s4EFgXV03670404 for ; Wed, 14 May 2014 15:42:33 GMT Received: from d06av06.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av06.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id s4EGgX51004980 for ; Wed, 14 May 2014 10:42:33 -0600 Received: from smtp.lab.toulouse-stg.fr.ibm.com (srv01.lab.toulouse-stg.fr.ibm.com [9.101.4.1]) by d06av06.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id s4EGgW0s004957; Wed, 14 May 2014 10:42:32 -0600 Received: from bahia.local (icon-9-164-165-242.megacenter.de.ibm.com [9.164.165.242]) by smtp.lab.toulouse-stg.fr.ibm.com (Postfix) with ESMTP id 709B1210FF4; Wed, 14 May 2014 17:42:31 +0200 (CEST) To: Kevin Wolf , Anthony Liguori , "Michael S. Tsirkin" , Stefan Hajnoczi , Amit Shah , Paolo Bonzini From: Greg Kurz Date: Wed, 14 May 2014 17:42:30 +0200 Message-ID: <20140514154230.10746.56297.stgit@bahia.local> In-Reply-To: <20140514154130.10746.1412.stgit@bahia.local> References: <20140514154130.10746.1412.stgit@bahia.local> User-Agent: StGit/0.16 MIME-Version: 1.0 X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 14051415-8372-0000-0000-000009ADADC7 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.4.x-2.6.x [generic] X-Received-From: 195.75.94.108 Cc: Juan Quintela , Fam Zheng , Alexander Graf , Andreas =?utf-8?q?F=C3=A4rber?= , qemu-devel@nongnu.org Subject: [Qemu-devel] [PATCH RFC 8/8] virtio: add endian-ambivalent support to VirtIODevice X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org 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. Of course this property must be part of the migration stream as most of the virtio code will depend on it. It is hence migrated in a subsection to preserve compatibility with older migration streams. The tricky part is that the endianness gets known quite late and we must ensure no access is made to virtio data before that. It is for example invalid to call vring_avail_idx() as does the actual migration code: the VQ indexes sanity checks had to be moved from virtio_load() to virtio_load_subsections() because of that. In fact, we have two conditions where the endianness property is stale: - from initialization time (virtio_init) to first reset time (virtio_reset) - from incoming migration time (virtio_load) to the time the property is restored (virtio_load_device_endian) We enforce some paranoia by making the endianness a 3-value enum so that we can poison it when it should not be used. Signed-off-by: Greg Kurz --- exec.c | 8 +----- hw/virtio/virtio-pci.c | 11 +++----- hw/virtio/virtio.c | 64 ++++++++++++++++++++++++++++++++++++-------- include/exec/cpu-common.h | 1 + include/hw/virtio/virtio.h | 13 +++++++++ 5 files changed, 72 insertions(+), 25 deletions(-) diff --git a/exec.c b/exec.c index 91513c6..4e83588 100644 --- a/exec.c +++ b/exec.c @@ -2740,13 +2740,7 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr, #endif #if !defined(CONFIG_USER_ONLY) - -/* - * A helper function for the _utterly broken_ virtio device model to find out if - * it's running on a big endian machine. Don't do this at home kids! - */ -bool virtio_is_big_endian(void); -bool virtio_is_big_endian(void) +bool target_words_bigendian(void) { #if defined(TARGET_WORDS_BIGENDIAN) return true; diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index ce97514..2ffceb8 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -89,9 +89,6 @@ /* Flags track per-device state like workarounds for quirks in older guests. */ #define VIRTIO_PCI_FLAG_BUS_MASTER_BUG (1 << 0) -/* HACK for virtio to determine if it's running a big endian guest */ -bool virtio_is_big_endian(void); - static void virtio_pci_bus_new(VirtioBusState *bus, size_t bus_size, VirtIOPCIProxy *dev); @@ -409,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; @@ -443,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 f4eaa3f..9018b6c 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -547,6 +547,12 @@ void virtio_reset(void *opaque) virtio_set_status(vdev, 0); + if (target_words_bigendian()) { + vdev->device_endian = VIRTIO_DEVICE_ENDIAN_BIG; + } else { + vdev->device_endian = VIRTIO_DEVICE_ENDIAN_LITTLE; + } + if (k->reset) { k->reset(vdev); } @@ -834,12 +840,28 @@ void virtio_notify_config(VirtIODevice *vdev) virtio_notify_vector(vdev, vdev->config_vector); } +static void virtio_save_device_endian(VirtIODevice *vdev, QEMUFile *f) +{ + qemu_put_byte(f, vdev->device_endian); +} + +static int virtio_load_device_endian(VirtIODevice *vdev, QEMUFile *f) +{ + vdev->device_endian = qemu_get_byte(f); + return 0; +} + static const struct VirtIOSubsectionDescStruct { const char *name; int version_id; void (*save)(VirtIODevice *vdev, QEMUFile *f); int (*load)(VirtIODevice *vdev, QEMUFile *f); } virtio_subsection[] = { + { .name = "virtio/is_big_endian", + .version_id = 1, + .save = virtio_save_device_endian, + .load = virtio_load_device_endian, + }, { .name = NULL } }; @@ -861,6 +883,8 @@ void virtio_save_subsections(VirtIODevice *vdev, QEMUFile *f) int virtio_load_subsections(VirtIODevice *vdev, QEMUFile *f) { + int i; + while (qemu_peek_byte(f, 0) == QEMU_VM_SUBSECTION) { char idstr[256]; uint8_t len, size; @@ -895,6 +919,26 @@ int virtio_load_subsections(VirtIODevice *vdev, QEMUFile *f) } } + for (i = 0; i < VIRTIO_PCI_QUEUE_MAX; i++) { + if (vdev->vq[i].vring.num == 0) { + break; + } + + if (vdev->vq[i].pa) { + uint16_t nheads; + nheads = vring_avail_idx(&vdev->vq[i]) - vdev->vq[i].last_avail_idx; + /* Check it isn't doing strange things with descriptor numbers. */ + if (nheads > vdev->vq[i].vring.num) { + error_report("VQ %d size 0x%x Guest index 0x%x " + "inconsistent with Host index 0x%x: delta 0x%x", + i, vdev->vq[i].vring.num, + vring_avail_idx(&vdev->vq[i]), + vdev->vq[i].last_avail_idx, nheads); + return -1; + } + } + } + return 0; } @@ -962,6 +1006,11 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f) BusState *qbus = qdev_get_parent_bus(DEVICE(vdev)); VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); + /* We poison the endianness to ensure it does not get used before + * subsections have been loaded. + */ + vdev->device_endian = VIRTIO_DEVICE_ENDIAN_UNKNOWN; + if (k->load_config) { ret = k->load_config(qbus->parent, f); if (ret) @@ -995,18 +1044,7 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f) vdev->vq[i].notification = true; if (vdev->vq[i].pa) { - uint16_t nheads; virtqueue_init(&vdev->vq[i]); - nheads = vring_avail_idx(&vdev->vq[i]) - vdev->vq[i].last_avail_idx; - /* Check it isn't doing very strange things with descriptor numbers. */ - if (nheads > vdev->vq[i].vring.num) { - error_report("VQ %d size 0x%x Guest index 0x%x " - "inconsistent with Host index 0x%x: delta 0x%x", - i, vdev->vq[i].vring.num, - vring_avail_idx(&vdev->vq[i]), - vdev->vq[i].last_avail_idx, nheads); - return -1; - } } else if (vdev->vq[i].last_avail_idx) { error_report("VQ %d address 0x0 " "inconsistent with Host index 0x%x", @@ -1078,6 +1116,10 @@ void virtio_init(VirtIODevice *vdev, const char *name, } vdev->vmstate = qemu_add_vm_change_state_handler(virtio_vmstate_change, vdev); + /* We poison the endianness to ensure it does not get used before + * the device is reset. + */ + vdev->device_endian = VIRTIO_DEVICE_ENDIAN_UNKNOWN; } hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n) diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h index a21b65a..eb798c1 100644 --- a/include/exec/cpu-common.h +++ b/include/exec/cpu-common.h @@ -122,4 +122,5 @@ void qemu_ram_foreach_block(RAMBlockIterFunc func, void *opaque); #endif +bool target_words_bigendian(void); #endif /* !CPU_COMMON_H */ diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index 82f852f..0012470 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -104,6 +104,12 @@ typedef struct VirtQueueElement #define VIRTIO_DEVICE(obj) \ OBJECT_CHECK(VirtIODevice, (obj), TYPE_VIRTIO_DEVICE) +enum virtio_device_endian { + VIRTIO_DEVICE_ENDIAN_UNKNOWN, + VIRTIO_DEVICE_ENDIAN_LITTLE, + VIRTIO_DEVICE_ENDIAN_BIG, +}; + struct VirtIODevice { DeviceState parent_obj; @@ -121,6 +127,7 @@ struct VirtIODevice bool vm_running; VMChangeStateEntry *vmstate; char *bus_name; + enum virtio_device_endian device_endian; }; typedef struct VirtioDeviceClass { @@ -257,4 +264,10 @@ 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); + +static inline bool virtio_is_big_endian(VirtIODevice *vdev) +{ + assert(vdev->device_endian != VIRTIO_DEVICE_ENDIAN_UNKNOWN); + return vdev->device_endian == VIRTIO_DEVICE_ENDIAN_BIG; +} #endif