From dba04c3488c4699f5afe96f66e448b1d447cf3fb Mon Sep 17 00:00:00 2001 From: Michael Tokarev Date: Mon, 20 Jul 2020 19:06:27 +0300 Subject: [PATCH 1/9] acpi: accept byte and word access to core ACPI registers All ISA registers should be accessible as bytes, words or dwords (if wide enough). Fix the access constraints for acpi-pm-evt, acpi-pm-tmr & acpi-cnt registers. Fixes: 5d971f9e67 (memory: Revert "memory: accept mismatching sizes in memory_region_access_valid") Fixes: afafe4bbe0 (apci: switch cnt to memory api) Fixes: 77d58b1e47 (apci: switch timer to memory api) Fixes: b5a7c024d2 (apci: switch evt to memory api) Buglink: https://lore.kernel.org/xen-devel/20200630170913.123646-1-anthony.perard@citrix.com/T/ Buglink: https://bugs.debian.org/964793 BugLink: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=964247 BugLink: https://bugs.launchpad.net/bugs/1886318 Reported-By: Simon John Signed-off-by: Michael Tokarev Message-Id: <20200720160627.15491-1-mjt@msgid.tls.msk.ru> Cc: qemu-stable@nongnu.org Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/acpi/core.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/hw/acpi/core.c b/hw/acpi/core.c index f6d9ec4f13..ac06db3450 100644 --- a/hw/acpi/core.c +++ b/hw/acpi/core.c @@ -458,7 +458,8 @@ static void acpi_pm_evt_write(void *opaque, hwaddr addr, uint64_t val, static const MemoryRegionOps acpi_pm_evt_ops = { .read = acpi_pm_evt_read, .write = acpi_pm_evt_write, - .valid.min_access_size = 2, + .impl.min_access_size = 2, + .valid.min_access_size = 1, .valid.max_access_size = 2, .endianness = DEVICE_LITTLE_ENDIAN, }; @@ -527,7 +528,8 @@ static void acpi_pm_tmr_write(void *opaque, hwaddr addr, uint64_t val, static const MemoryRegionOps acpi_pm_tmr_ops = { .read = acpi_pm_tmr_read, .write = acpi_pm_tmr_write, - .valid.min_access_size = 4, + .impl.min_access_size = 4, + .valid.min_access_size = 1, .valid.max_access_size = 4, .endianness = DEVICE_LITTLE_ENDIAN, }; @@ -599,7 +601,8 @@ static void acpi_pm_cnt_write(void *opaque, hwaddr addr, uint64_t val, static const MemoryRegionOps acpi_pm_cnt_ops = { .read = acpi_pm_cnt_read, .write = acpi_pm_cnt_write, - .valid.min_access_size = 2, + .impl.min_access_size = 2, + .valid.min_access_size = 1, .valid.max_access_size = 2, .endianness = DEVICE_LITTLE_ENDIAN, }; From cf4e3d000e438d04077a56c401b41f3336a2a09d Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 21 Jul 2020 14:11:53 +0200 Subject: [PATCH 2/9] virtio: Drop broken and superfluous object_property_set_link() virtio_crypto_pci_realize() and copies the value of vcrypto->vdev's property "cryptodev" to vcrypto's property: object_property_set_link(OBJECT(vrng), "rng", OBJECT(vrng->vdev.conf.rng), NULL); Since it does so only after realize, this always fails, but the error is ignored. It's actually superfluous: vcrypto's property is an alias of vcrypto->vdev's property, created by virtio_instance_init_common(). Drop the call. Same for virtio_ccw_crypto_realize(), virtio_rng_pci_realize(), virtio_ccw_rng_realize(). Signed-off-by: Markus Armbruster Message-Id: <20200721121153.1128844-1-armbru@redhat.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/s390x/virtio-ccw-crypto.c | 3 --- hw/s390x/virtio-ccw-rng.c | 3 --- hw/virtio/virtio-crypto-pci.c | 2 -- hw/virtio/virtio-rng-pci.c | 3 --- 4 files changed, 11 deletions(-) diff --git a/hw/s390x/virtio-ccw-crypto.c b/hw/s390x/virtio-ccw-crypto.c index 570c0333fc..358c74fb4b 100644 --- a/hw/s390x/virtio-ccw-crypto.c +++ b/hw/s390x/virtio-ccw-crypto.c @@ -23,9 +23,6 @@ static void virtio_ccw_crypto_realize(VirtioCcwDevice *ccw_dev, Error **errp) if (!qdev_realize(vdev, BUS(&ccw_dev->bus), errp)) { return; } - - object_property_set_link(OBJECT(vdev), "cryptodev", - OBJECT(dev->vdev.conf.cryptodev), NULL); } static void virtio_ccw_crypto_instance_init(Object *obj) diff --git a/hw/s390x/virtio-ccw-rng.c b/hw/s390x/virtio-ccw-rng.c index 4bb8c16d79..2e3a9da5e8 100644 --- a/hw/s390x/virtio-ccw-rng.c +++ b/hw/s390x/virtio-ccw-rng.c @@ -24,9 +24,6 @@ static void virtio_ccw_rng_realize(VirtioCcwDevice *ccw_dev, Error **errp) if (!qdev_realize(vdev, BUS(&ccw_dev->bus), errp)) { return; } - - object_property_set_link(OBJECT(dev), "rng", OBJECT(dev->vdev.conf.rng), - NULL); } static void virtio_ccw_rng_instance_init(Object *obj) diff --git a/hw/virtio/virtio-crypto-pci.c b/hw/virtio/virtio-crypto-pci.c index f1cc979d33..198f86e08c 100644 --- a/hw/virtio/virtio-crypto-pci.c +++ b/hw/virtio/virtio-crypto-pci.c @@ -57,8 +57,6 @@ static void virtio_crypto_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp) if (!qdev_realize(vdev, BUS(&vpci_dev->bus), errp)) { return; } - object_property_set_link(OBJECT(vcrypto), "cryptodev", - OBJECT(vcrypto->vdev.conf.cryptodev), NULL); } static void virtio_crypto_pci_class_init(ObjectClass *klass, void *data) diff --git a/hw/virtio/virtio-rng-pci.c b/hw/virtio/virtio-rng-pci.c index 2f0b529b62..8afbb4c209 100644 --- a/hw/virtio/virtio-rng-pci.c +++ b/hw/virtio/virtio-rng-pci.c @@ -38,9 +38,6 @@ static void virtio_rng_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp) if (!qdev_realize(vdev, BUS(&vpci_dev->bus), errp)) { return; } - - object_property_set_link(OBJECT(vrng), "rng", OBJECT(vrng->vdev.conf.rng), - NULL); } static void virtio_rng_pci_class_init(ObjectClass *klass, void *data) From 20a4da0f23078deeff5ea6d1e12f47d968d7c3c9 Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Mon, 20 Jul 2020 10:51:15 -0700 Subject: [PATCH 3/9] virtio-balloon: Prevent guest from starting a report when we didn't request one Based on code review it appears possible for the driver to force the device out of a stopped state when hinting by repeating the last ID it was provided. Prevent this by only allowing a transition to the start state when we are in the requested state. This way the driver is only allowed to send one descriptor that will transition the device into the start state. All others will leave it in the stop state once it has finished. Fixes: c13c4153f76d ("virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT") Acked-by: David Hildenbrand Signed-off-by: Alexander Duyck Message-Id: <20200720175115.21935.99563.stgit@localhost.localdomain> Cc: qemu-stable@nongnu.org Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/virtio/virtio-balloon.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c index e670f1e595..ce70adcc69 100644 --- a/hw/virtio/virtio-balloon.c +++ b/hw/virtio/virtio-balloon.c @@ -526,7 +526,8 @@ static bool get_free_page_hints(VirtIOBalloon *dev) ret = false; goto out; } - if (id == dev->free_page_report_cmd_id) { + if (dev->free_page_report_status == FREE_PAGE_REPORT_S_REQUESTED && + id == dev->free_page_report_cmd_id) { dev->free_page_report_status = FREE_PAGE_REPORT_S_START; } else { /* From 1a83e0b9c492a0eaeacd6fbb858fc81d04ab9c3e Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Mon, 20 Jul 2020 10:51:22 -0700 Subject: [PATCH 4/9] virtio-balloon: Add locking to prevent possible race when starting hinting There is already locking in place when we are stopping free page hinting but there is not similar protections in place when we start. I can only assume this was overlooked as in most cases the page hinting should not be occurring when we are starting the hinting, however there is still a chance we could be processing hints by the time we get back around to restarting the hinting so we are better off making sure to protect the state with the mutex lock rather than just updating the value with no protections. Based on feedback from Peter Maydell this issue had also been spotted by Coverity: CID 1430269 Acked-by: David Hildenbrand Signed-off-by: Alexander Duyck Message-Id: <20200720175122.21935.78013.stgit@localhost.localdomain> Cc: qemu-stable@nongnu.org Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/virtio/virtio-balloon.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c index ce70adcc69..6e2d129340 100644 --- a/hw/virtio/virtio-balloon.c +++ b/hw/virtio/virtio-balloon.c @@ -592,6 +592,8 @@ static void virtio_balloon_free_page_start(VirtIOBalloon *s) return; } + qemu_mutex_lock(&s->free_page_lock); + if (s->free_page_report_cmd_id == UINT_MAX) { s->free_page_report_cmd_id = VIRTIO_BALLOON_FREE_PAGE_REPORT_CMD_ID_MIN; @@ -600,6 +602,8 @@ static void virtio_balloon_free_page_start(VirtIOBalloon *s) } s->free_page_report_status = FREE_PAGE_REPORT_S_REQUESTED; + qemu_mutex_unlock(&s->free_page_lock); + virtio_notify_config(vdev); } From 3219b42f025d4d7a9c463235e9f937ab38067de3 Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Mon, 20 Jul 2020 10:51:28 -0700 Subject: [PATCH 5/9] virtio-balloon: Replace free page hinting references to 'report' with 'hint' Recently a feature named Free Page Reporting was added to the virtio balloon. In order to avoid any confusion we should drop the use of the word 'report' when referring to Free Page Hinting. So what this patch does is go through and replace all instances of 'report' with 'hint" when we are referring to free page hinting. Acked-by: David Hildenbrand Signed-off-by: Alexander Duyck Message-Id: <20200720175128.21935.93927.stgit@localhost.localdomain> Cc: qemu-stable@nongnu.org Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/virtio/virtio-balloon.c | 76 +++++++++++++++--------------- include/hw/virtio/virtio-balloon.h | 20 ++++---- 2 files changed, 48 insertions(+), 48 deletions(-) diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c index 6e2d129340..22cb5df717 100644 --- a/hw/virtio/virtio-balloon.c +++ b/hw/virtio/virtio-balloon.c @@ -526,22 +526,22 @@ static bool get_free_page_hints(VirtIOBalloon *dev) ret = false; goto out; } - if (dev->free_page_report_status == FREE_PAGE_REPORT_S_REQUESTED && - id == dev->free_page_report_cmd_id) { - dev->free_page_report_status = FREE_PAGE_REPORT_S_START; + if (dev->free_page_hint_status == FREE_PAGE_HINT_S_REQUESTED && + id == dev->free_page_hint_cmd_id) { + dev->free_page_hint_status = FREE_PAGE_HINT_S_START; } else { /* * Stop the optimization only when it has started. This * avoids a stale stop sign for the previous command. */ - if (dev->free_page_report_status == FREE_PAGE_REPORT_S_START) { - dev->free_page_report_status = FREE_PAGE_REPORT_S_STOP; + if (dev->free_page_hint_status == FREE_PAGE_HINT_S_START) { + dev->free_page_hint_status = FREE_PAGE_HINT_S_STOP; } } } if (elem->in_num) { - if (dev->free_page_report_status == FREE_PAGE_REPORT_S_START) { + if (dev->free_page_hint_status == FREE_PAGE_HINT_S_START) { qemu_guest_free_page_hint(elem->in_sg[0].iov_base, elem->in_sg[0].iov_len); } @@ -567,11 +567,11 @@ static void virtio_ballloon_get_free_page_hints(void *opaque) qemu_mutex_unlock(&dev->free_page_lock); virtio_notify(vdev, vq); /* - * Start to poll the vq once the reporting started. Otherwise, continue + * Start to poll the vq once the hinting started. Otherwise, continue * only when there are entries on the vq, which need to be given back. */ } while (continue_to_get_hints || - dev->free_page_report_status == FREE_PAGE_REPORT_S_START); + dev->free_page_hint_status == FREE_PAGE_HINT_S_START); virtio_queue_set_notification(vq, 1); } @@ -594,14 +594,14 @@ static void virtio_balloon_free_page_start(VirtIOBalloon *s) qemu_mutex_lock(&s->free_page_lock); - if (s->free_page_report_cmd_id == UINT_MAX) { - s->free_page_report_cmd_id = - VIRTIO_BALLOON_FREE_PAGE_REPORT_CMD_ID_MIN; + if (s->free_page_hint_cmd_id == UINT_MAX) { + s->free_page_hint_cmd_id = + VIRTIO_BALLOON_FREE_PAGE_HINT_CMD_ID_MIN; } else { - s->free_page_report_cmd_id++; + s->free_page_hint_cmd_id++; } - s->free_page_report_status = FREE_PAGE_REPORT_S_REQUESTED; + s->free_page_hint_status = FREE_PAGE_HINT_S_REQUESTED; qemu_mutex_unlock(&s->free_page_lock); virtio_notify_config(vdev); @@ -611,18 +611,18 @@ static void virtio_balloon_free_page_stop(VirtIOBalloon *s) { VirtIODevice *vdev = VIRTIO_DEVICE(s); - if (s->free_page_report_status != FREE_PAGE_REPORT_S_STOP) { + if (s->free_page_hint_status != FREE_PAGE_HINT_S_STOP) { /* * The lock also guarantees us that the * virtio_ballloon_get_free_page_hints exits after the - * free_page_report_status is set to S_STOP. + * free_page_hint_status is set to S_STOP. */ qemu_mutex_lock(&s->free_page_lock); /* - * The guest hasn't done the reporting, so host sends a notification - * to the guest to actively stop the reporting. + * The guest isn't done hinting, so send a notification + * to the guest to actively stop the hinting. */ - s->free_page_report_status = FREE_PAGE_REPORT_S_STOP; + s->free_page_hint_status = FREE_PAGE_HINT_S_STOP; qemu_mutex_unlock(&s->free_page_lock); virtio_notify_config(vdev); } @@ -632,20 +632,20 @@ static void virtio_balloon_free_page_done(VirtIOBalloon *s) { VirtIODevice *vdev = VIRTIO_DEVICE(s); - if (s->free_page_report_status != FREE_PAGE_REPORT_S_DONE) { + if (s->free_page_hint_status != FREE_PAGE_HINT_S_DONE) { /* See virtio_balloon_free_page_stop() */ qemu_mutex_lock(&s->free_page_lock); - s->free_page_report_status = FREE_PAGE_REPORT_S_DONE; + s->free_page_hint_status = FREE_PAGE_HINT_S_DONE; qemu_mutex_unlock(&s->free_page_lock); virtio_notify_config(vdev); } } static int -virtio_balloon_free_page_report_notify(NotifierWithReturn *n, void *data) +virtio_balloon_free_page_hint_notify(NotifierWithReturn *n, void *data) { VirtIOBalloon *dev = container_of(n, VirtIOBalloon, - free_page_report_notify); + free_page_hint_notify); VirtIODevice *vdev = VIRTIO_DEVICE(dev); PrecopyNotifyData *pnd = data; @@ -703,7 +703,7 @@ static size_t virtio_balloon_config_size(VirtIOBalloon *s) if (virtio_has_feature(features, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) { return offsetof(struct virtio_balloon_config, poison_val); } - return offsetof(struct virtio_balloon_config, free_page_report_cmd_id); + return offsetof(struct virtio_balloon_config, free_page_hint_cmd_id); } static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data) @@ -715,14 +715,14 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data) config.actual = cpu_to_le32(dev->actual); config.poison_val = cpu_to_le32(dev->poison_val); - if (dev->free_page_report_status == FREE_PAGE_REPORT_S_REQUESTED) { - config.free_page_report_cmd_id = - cpu_to_le32(dev->free_page_report_cmd_id); - } else if (dev->free_page_report_status == FREE_PAGE_REPORT_S_STOP) { - config.free_page_report_cmd_id = + if (dev->free_page_hint_status == FREE_PAGE_HINT_S_REQUESTED) { + config.free_page_hint_cmd_id = + cpu_to_le32(dev->free_page_hint_cmd_id); + } else if (dev->free_page_hint_status == FREE_PAGE_HINT_S_STOP) { + config.free_page_hint_cmd_id = cpu_to_le32(VIRTIO_BALLOON_CMD_ID_STOP); - } else if (dev->free_page_report_status == FREE_PAGE_REPORT_S_DONE) { - config.free_page_report_cmd_id = + } else if (dev->free_page_hint_status == FREE_PAGE_HINT_S_DONE) { + config.free_page_hint_cmd_id = cpu_to_le32(VIRTIO_BALLOON_CMD_ID_DONE); } @@ -835,14 +835,14 @@ static int virtio_balloon_post_load_device(void *opaque, int version_id) return 0; } -static const VMStateDescription vmstate_virtio_balloon_free_page_report = { +static const VMStateDescription vmstate_virtio_balloon_free_page_hint = { .name = "virtio-balloon-device/free-page-report", .version_id = 1, .minimum_version_id = 1, .needed = virtio_balloon_free_page_support, .fields = (VMStateField[]) { - VMSTATE_UINT32(free_page_report_cmd_id, VirtIOBalloon), - VMSTATE_UINT32(free_page_report_status, VirtIOBalloon), + VMSTATE_UINT32(free_page_hint_cmd_id, VirtIOBalloon), + VMSTATE_UINT32(free_page_hint_status, VirtIOBalloon), VMSTATE_END_OF_LIST() } }; @@ -869,7 +869,7 @@ static const VMStateDescription vmstate_virtio_balloon_device = { VMSTATE_END_OF_LIST() }, .subsections = (const VMStateDescription * []) { - &vmstate_virtio_balloon_free_page_report, + &vmstate_virtio_balloon_free_page_hint, &vmstate_virtio_balloon_page_poison, NULL } @@ -908,7 +908,7 @@ static void virtio_balloon_device_realize(DeviceState *dev, Error **errp) VIRTIO_BALLOON_F_FREE_PAGE_HINT)) { s->free_page_vq = virtio_add_queue(vdev, VIRTQUEUE_MAX_SIZE, virtio_balloon_handle_free_page_vq); - precopy_add_notifier(&s->free_page_report_notify); + precopy_add_notifier(&s->free_page_hint_notify); object_ref(OBJECT(s->iothread)); s->free_page_bh = aio_bh_new(iothread_get_aio_context(s->iothread), @@ -932,7 +932,7 @@ static void virtio_balloon_device_unrealize(DeviceState *dev) qemu_bh_delete(s->free_page_bh); object_unref(OBJECT(s->iothread)); virtio_balloon_free_page_stop(s); - precopy_remove_notifier(&s->free_page_report_notify); + precopy_remove_notifier(&s->free_page_hint_notify); } balloon_stats_destroy_timer(s); qemu_remove_balloon_handler(s); @@ -1004,8 +1004,8 @@ static void virtio_balloon_instance_init(Object *obj) qemu_mutex_init(&s->free_page_lock); qemu_cond_init(&s->free_page_cond); - s->free_page_report_cmd_id = VIRTIO_BALLOON_FREE_PAGE_REPORT_CMD_ID_MIN; - s->free_page_report_notify.notify = virtio_balloon_free_page_report_notify; + s->free_page_hint_cmd_id = VIRTIO_BALLOON_FREE_PAGE_HINT_CMD_ID_MIN; + s->free_page_hint_notify.notify = virtio_balloon_free_page_hint_notify; object_property_add(obj, "guest-stats", "guest statistics", balloon_stats_get_all, NULL, NULL, s); diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h index d49fef00ce..28fd2b3960 100644 --- a/include/hw/virtio/virtio-balloon.h +++ b/include/hw/virtio/virtio-balloon.h @@ -23,7 +23,7 @@ #define VIRTIO_BALLOON(obj) \ OBJECT_CHECK(VirtIOBalloon, (obj), TYPE_VIRTIO_BALLOON) -#define VIRTIO_BALLOON_FREE_PAGE_REPORT_CMD_ID_MIN 0x80000000 +#define VIRTIO_BALLOON_FREE_PAGE_HINT_CMD_ID_MIN 0x80000000 typedef struct virtio_balloon_stat VirtIOBalloonStat; @@ -33,20 +33,20 @@ typedef struct virtio_balloon_stat_modern { uint64_t val; } VirtIOBalloonStatModern; -enum virtio_balloon_free_page_report_status { - FREE_PAGE_REPORT_S_STOP = 0, - FREE_PAGE_REPORT_S_REQUESTED = 1, - FREE_PAGE_REPORT_S_START = 2, - FREE_PAGE_REPORT_S_DONE = 3, +enum virtio_balloon_free_page_hint_status { + FREE_PAGE_HINT_S_STOP = 0, + FREE_PAGE_HINT_S_REQUESTED = 1, + FREE_PAGE_HINT_S_START = 2, + FREE_PAGE_HINT_S_DONE = 3, }; typedef struct VirtIOBalloon { VirtIODevice parent_obj; VirtQueue *ivq, *dvq, *svq, *free_page_vq, *reporting_vq; - uint32_t free_page_report_status; + uint32_t free_page_hint_status; uint32_t num_pages; uint32_t actual; - uint32_t free_page_report_cmd_id; + uint32_t free_page_hint_cmd_id; uint64_t stats[VIRTIO_BALLOON_S_NR]; VirtQueueElement *stats_vq_elem; size_t stats_vq_offset; @@ -55,7 +55,7 @@ typedef struct VirtIOBalloon { QEMUBH *free_page_bh; /* * Lock to synchronize threads to access the free page reporting related - * fields (e.g. free_page_report_status). + * fields (e.g. free_page_hint_status). */ QemuMutex free_page_lock; QemuCond free_page_cond; @@ -64,7 +64,7 @@ typedef struct VirtIOBalloon { * stopped. */ bool block_iothread; - NotifierWithReturn free_page_report_notify; + NotifierWithReturn free_page_hint_notify; int64_t stats_last_update; int64_t stats_poll_interval; uint32_t host_features; From 7c78bdd7a3d0086179331f10d1f6f8cdac34731a Mon Sep 17 00:00:00 2001 From: Cornelia Huck Date: Tue, 7 Jul 2020 12:54:45 +0200 Subject: [PATCH 6/9] virtio: list legacy-capable devices Several types of virtio devices had already been around before the virtio standard was specified. These devices support virtio in legacy (and transitional) mode. Devices that have been added in the virtio standard are considered non-transitional (i.e. with no support for legacy virtio). Provide a helper function so virtio transports can figure that out easily. Signed-off-by: Cornelia Huck Message-Id: <20200707105446.677966-2-cohuck@redhat.com> Cc: qemu-stable@nongnu.org Acked-by: Halil Pasic Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/virtio/virtio.c | 25 +++++++++++++++++++++++++ include/hw/virtio/virtio.h | 2 ++ 2 files changed, 27 insertions(+) diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 5bd2a2f621..546a198e79 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -27,6 +27,7 @@ #include "hw/virtio/virtio-access.h" #include "sysemu/dma.h" #include "sysemu/runstate.h" +#include "standard-headers/linux/virtio_ids.h" /* * The alignment to use between consumer and producer parts of vring. @@ -3279,6 +3280,30 @@ void virtio_init(VirtIODevice *vdev, const char *name, vdev->use_guest_notifier_mask = true; } +/* + * Only devices that have already been around prior to defining the virtio + * standard support legacy mode; this includes devices not specified in the + * standard. All newer devices conform to the virtio standard only. + */ +bool virtio_legacy_allowed(VirtIODevice *vdev) +{ + switch (vdev->device_id) { + case VIRTIO_ID_NET: + case VIRTIO_ID_BLOCK: + case VIRTIO_ID_CONSOLE: + case VIRTIO_ID_RNG: + case VIRTIO_ID_BALLOON: + case VIRTIO_ID_RPMSG: + case VIRTIO_ID_SCSI: + case VIRTIO_ID_9P: + case VIRTIO_ID_RPROC_SERIAL: + case VIRTIO_ID_CAIF: + return true; + default: + return false; + } +} + hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n) { return vdev->vq[n].vring.desc; diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index b69d517496..198ffc7626 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -396,4 +396,6 @@ static inline bool virtio_device_disabled(VirtIODevice *vdev) return unlikely(vdev->disabled || vdev->broken); } +bool virtio_legacy_allowed(VirtIODevice *vdev); + #endif From 9b3a35ec8236933ab958a4c3ad883163f1ca66e7 Mon Sep 17 00:00:00 2001 From: Cornelia Huck Date: Tue, 7 Jul 2020 12:54:46 +0200 Subject: [PATCH 7/9] virtio: verify that legacy support is not accidentally on If a virtio device does not have legacy support, make sure that it is actually off, and bail out if not. For virtio-pci, this means that any device without legacy support that has been specified to modern-only (or that has been forced to it) will work. For virtio-ccw, this duplicates the check that is currently done prior to realization for any device that explicitly specified no support for legacy. This catches devices that have not been fenced properly. Signed-off-by: Cornelia Huck Message-Id: <20200707105446.677966-3-cohuck@redhat.com> Cc: qemu-stable@nongnu.org Acked-by: Halil Pasic Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/s390x/virtio-ccw.c | 6 ++++++ hw/virtio/virtio-pci.c | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index 3c988a000b..0e60270297 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -1121,6 +1121,12 @@ static void virtio_ccw_device_plugged(DeviceState *d, Error **errp) dev->max_rev = 0; } + if (!virtio_ccw_rev_max(dev) && !virtio_legacy_allowed(vdev)) { + error_setg(errp, "Invalid value of property max_rev " + "(is %d expected >= 1)", virtio_ccw_rev_max(dev)); + return; + } + if (virtio_get_num_queues(vdev) > VIRTIO_QUEUE_MAX) { error_setg(errp, "The number of virtqueues %d " "exceeds virtio limit %d", n, diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index 8554cf2a03..db8b711b35 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -1581,6 +1581,10 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) } if (legacy) { + if (!virtio_legacy_allowed(vdev)) { + error_setg(errp, "device is modern-only, use disable-legacy=on"); + return; + } if (virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM)) { error_setg(errp, "VIRTIO_F_IOMMU_PLATFORM was supported by" " neither legacy nor transitional device"); From a4544c45e109ceee87ee8c19baff28be3890d788 Mon Sep 17 00:00:00 2001 From: Liu Yi L Date: Sat, 4 Jul 2020 01:07:15 -0700 Subject: [PATCH 8/9] intel_iommu: Use correct shift for 256 bits qi descriptor In chapter 10.4.23 of VT-d spec 3.0, Descriptor Width bit was introduced in VTD_IQA_REG. Software could set this bit to tell VT-d the QI descriptor from software would be 256 bits. Accordingly, the VTD_IQH_QH_SHIFT should be 5 when descriptor size is 256 bits. This patch adds the DW bit check when deciding the shift used to update VTD_IQH_REG. Signed-off-by: Liu Yi L Message-Id: <1593850035-35483-1-git-send-email-yi.l.liu@intel.com> Reviewed-by: Peter Xu Acked-by: Jason Wang Cc: qemu-stable@nongnu.org Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/i386/intel_iommu.c | 7 ++++++- hw/i386/intel_iommu_internal.h | 3 ++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index c56398e991..0c286635cf 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -2549,6 +2549,11 @@ static bool vtd_process_inv_desc(IntelIOMMUState *s) /* Try to fetch and process more Invalidation Descriptors */ static void vtd_fetch_inv_desc(IntelIOMMUState *s) { + int qi_shift; + + /* Refer to 10.4.23 of VT-d spec 3.0 */ + qi_shift = s->iq_dw ? VTD_IQH_QH_SHIFT_5 : VTD_IQH_QH_SHIFT_4; + trace_vtd_inv_qi_fetch(); if (s->iq_tail >= s->iq_size) { @@ -2567,7 +2572,7 @@ static void vtd_fetch_inv_desc(IntelIOMMUState *s) } /* Must update the IQH_REG in time */ vtd_set_quad_raw(s, DMAR_IQH_REG, - (((uint64_t)(s->iq_head)) << VTD_IQH_QH_SHIFT) & + (((uint64_t)(s->iq_head)) << qi_shift) & VTD_IQH_QH_MASK); } } diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h index 862033ebe6..3d5487fe2c 100644 --- a/hw/i386/intel_iommu_internal.h +++ b/hw/i386/intel_iommu_internal.h @@ -230,7 +230,8 @@ #define VTD_IQA_DW_MASK 0x800 /* IQH_REG */ -#define VTD_IQH_QH_SHIFT 4 +#define VTD_IQH_QH_SHIFT_4 4 +#define VTD_IQH_QH_SHIFT_5 5 #define VTD_IQH_QH_MASK 0x7fff0ULL /* ICS_REG */ From ccec7e9603f446fe75c6c563ba335c00cfda6a06 Mon Sep 17 00:00:00 2001 From: Andrew Melnychenko Date: Mon, 6 Jul 2020 14:21:23 +0300 Subject: [PATCH 9/9] virtio-pci: Changed vdev to proxy for VirtIO PCI BAR callbacks. There is an issue when callback may be called with invalid vdev. It happens on unplug when vdev already deleted and VirtIOPciProxy is not. So now, callbacks accept proxy device, and vdev retrieved from it. Technically memio callbacks should be removed during the flatview update, but memoryregions remain til PCI device(and it's address space) completely deleted. Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1716352 Signed-off-by: Andrew Melnychenko Message-Id: <20200706112123.971087-1-andrew@daynix.com> Cc: qemu-stable@nongnu.org Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/virtio/virtio-pci.c | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index db8b711b35..ada1101d07 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -1333,11 +1333,12 @@ static uint64_t virtio_pci_notify_read(void *opaque, hwaddr addr, static void virtio_pci_notify_write(void *opaque, hwaddr addr, uint64_t val, unsigned size) { - VirtIODevice *vdev = opaque; - VirtIOPCIProxy *proxy = VIRTIO_PCI(DEVICE(vdev)->parent_bus->parent); + VirtIOPCIProxy *proxy = opaque; + VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus); + unsigned queue = addr / virtio_pci_queue_mem_mult(proxy); - if (queue < VIRTIO_QUEUE_MAX) { + if (vdev != NULL && queue < VIRTIO_QUEUE_MAX) { virtio_queue_notify(vdev, queue); } } @@ -1345,10 +1346,12 @@ static void virtio_pci_notify_write(void *opaque, hwaddr addr, static void virtio_pci_notify_write_pio(void *opaque, hwaddr addr, uint64_t val, unsigned size) { - VirtIODevice *vdev = opaque; + VirtIOPCIProxy *proxy = opaque; + VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus); + unsigned queue = val; - if (queue < VIRTIO_QUEUE_MAX) { + if (vdev != NULL && queue < VIRTIO_QUEUE_MAX) { virtio_queue_notify(vdev, queue); } } @@ -1372,9 +1375,14 @@ static void virtio_pci_isr_write(void *opaque, hwaddr addr, static uint64_t virtio_pci_device_read(void *opaque, hwaddr addr, unsigned size) { - VirtIODevice *vdev = opaque; + VirtIOPCIProxy *proxy = opaque; + VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus); uint64_t val = 0; + if (vdev == NULL) { + return val; + } + switch (size) { case 1: val = virtio_config_modern_readb(vdev, addr); @@ -1392,7 +1400,13 @@ static uint64_t virtio_pci_device_read(void *opaque, hwaddr addr, static void virtio_pci_device_write(void *opaque, hwaddr addr, uint64_t val, unsigned size) { - VirtIODevice *vdev = opaque; + VirtIOPCIProxy *proxy = opaque; + VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus); + + if (vdev == NULL) { + return; + } + switch (size) { case 1: virtio_config_modern_writeb(vdev, addr, val); @@ -1469,19 +1483,19 @@ static void virtio_pci_modern_regions_init(VirtIOPCIProxy *proxy) memory_region_init_io(&proxy->device.mr, OBJECT(proxy), &device_ops, - virtio_bus_get_device(&proxy->bus), + proxy, "virtio-pci-device", proxy->device.size); memory_region_init_io(&proxy->notify.mr, OBJECT(proxy), ¬ify_ops, - virtio_bus_get_device(&proxy->bus), + proxy, "virtio-pci-notify", proxy->notify.size); memory_region_init_io(&proxy->notify_pio.mr, OBJECT(proxy), ¬ify_pio_ops, - virtio_bus_get_device(&proxy->bus), + proxy, "virtio-pci-notify-pio", proxy->notify_pio.size); }