qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Alex Bennée" <alex.bennee@linaro.org>
To: peter.maydell@linaro.org
Cc: "Michael S . Tsirkin" <mst@redhat.com>,
	"Fabiano Rosas" <farosas@linux.ibm.com>,
	qemu-devel@nongnu.org,
	"Dr . David Alan Gilbert" <dgilbert@redhat.com>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"Alex Bennée" <alex.bennee@linaro.org>
Subject: [PULL 09/17] hw/virtio/vhost: re-factor vhost-section and allow DIRTY_MEMORY_CODE
Date: Tue,  9 Jun 2020 11:38:01 +0100	[thread overview]
Message-ID: <20200609103809.23443-10-alex.bennee@linaro.org> (raw)
In-Reply-To: <20200609103809.23443-1-alex.bennee@linaro.org>

The purpose of vhost_section is to identify RAM regions that need to
be made available to a vhost client. However when running under TCG
all RAM sections have DIRTY_MEMORY_CODE set which leads to problems
down the line.

Re-factor the code so:

  - steps are clearer to follow
  - reason for rejection is recorded in the trace point
  - we allow DIRTY_MEMORY_CODE

We expand the comment to explain that kernel based vhost has specific
support for migration tracking.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Fabiano Rosas <farosas@linux.ibm.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20200605154929.26910-11-alex.bennee@linaro.org>

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index aff98a0ede5..e3e21812905 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -27,6 +27,7 @@
 #include "migration/blocker.h"
 #include "migration/qemu-file-types.h"
 #include "sysemu/dma.h"
+#include "sysemu/tcg.h"
 #include "trace.h"
 
 /* enabled until disconnected backend stabilizes */
@@ -403,26 +404,50 @@ static int vhost_verify_ring_mappings(struct vhost_dev *dev,
     return r;
 }
 
+/*
+ * vhost_section: identify sections needed for vhost access
+ *
+ * We only care about RAM sections here (where virtqueue and guest
+ * internals accessed by virtio might live). If we find one we still
+ * allow the backend to potentially filter it out of our list.
+ */
 static bool vhost_section(struct vhost_dev *dev, MemoryRegionSection *section)
 {
-    bool result;
-    bool log_dirty = memory_region_get_dirty_log_mask(section->mr) &
-                     ~(1 << DIRTY_MEMORY_MIGRATION);
-    result = memory_region_is_ram(section->mr) &&
-        !memory_region_is_rom(section->mr);
-
-    /* Vhost doesn't handle any block which is doing dirty-tracking other
-     * than migration; this typically fires on VGA areas.
-     */
-    result &= !log_dirty;
+    MemoryRegion *mr = section->mr;
+
+    if (memory_region_is_ram(mr) && !memory_region_is_rom(mr)) {
+        uint8_t dirty_mask = memory_region_get_dirty_log_mask(mr);
+        uint8_t handled_dirty;
+
+        /*
+         * Kernel based vhost doesn't handle any block which is doing
+         * dirty-tracking other than migration for which it has
+         * specific logging support. However for TCG the kernel never
+         * gets involved anyway so we can also ignore it's
+         * self-modiying code detection flags. However a vhost-user
+         * client could still confuse a TCG guest if it re-writes
+         * executable memory that has already been translated.
+         */
+        handled_dirty = (1 << DIRTY_MEMORY_MIGRATION) |
+            (1 << DIRTY_MEMORY_CODE);
 
-    if (result && dev->vhost_ops->vhost_backend_mem_section_filter) {
-        result &=
-            dev->vhost_ops->vhost_backend_mem_section_filter(dev, section);
-    }
+        if (dirty_mask & ~handled_dirty) {
+            trace_vhost_reject_section(mr->name, 1);
+            return false;
+        }
+
+        if (dev->vhost_ops->vhost_backend_mem_section_filter &&
+            !dev->vhost_ops->vhost_backend_mem_section_filter(dev, section)) {
+            trace_vhost_reject_section(mr->name, 2);
+            return false;
+        }
 
-    trace_vhost_section(section->mr->name, result);
-    return result;
+        trace_vhost_section(mr->name);
+        return true;
+    } else {
+        trace_vhost_reject_section(mr->name, 3);
+        return false;
+    }
 }
 
 static void vhost_begin(MemoryListener *listener)
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index e83500bee92..6427a0047df 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -5,7 +5,8 @@ vhost_commit(bool started, bool changed) "Started: %d Changed: %d"
 vhost_region_add_section(const char *name, uint64_t gpa, uint64_t size, uint64_t host) "%s: 0x%"PRIx64"+0x%"PRIx64" @ 0x%"PRIx64
 vhost_region_add_section_merge(const char *name, uint64_t new_size, uint64_t gpa, uint64_t owr) "%s: size: 0x%"PRIx64 " gpa: 0x%"PRIx64 " owr: 0x%"PRIx64
 vhost_region_add_section_aligned(const char *name, uint64_t gpa, uint64_t size, uint64_t host) "%s: 0x%"PRIx64"+0x%"PRIx64" @ 0x%"PRIx64
-vhost_section(const char *name, int r) "%s:%d"
+vhost_section(const char *name) "%s"
+vhost_reject_section(const char *name, int d) "%s:%d"
 vhost_iotlb_miss(void *dev, int step) "%p step %d"
 
 # vhost-user.c
-- 
2.20.1



  parent reply	other threads:[~2020-06-09 10:41 UTC|newest]

Thread overview: 76+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-09 10:37 [PULL 00/17] testing and misc fixes Alex Bennée
2020-06-09 10:37 ` [PULL 01/17] qemu-plugin.h: add missing include <stddef.h> to define size_t Alex Bennée
2020-06-09 10:37 ` [PULL 02/17] scripts/clean-includes: Mark 'qemu/qemu-plugin.h' as special header Alex Bennée
2020-06-09 10:37 ` [PULL 03/17] tests/plugin: correctly honour io_count Alex Bennée
2020-06-09 10:37 ` [PULL 04/17] exec: flush the whole TLB if a watchpoint crosses a page boundary Alex Bennée
2020-06-09 10:37 ` [PULL 05/17] .travis.yml: allow failure for unreliable hosts Alex Bennée
2020-06-09 10:37 ` [PULL 06/17] .shippable: temporaily disable some cross builds Alex Bennée
2020-06-09 10:37 ` [PULL 07/17] tests/docker: fix pre-requisite for debian-tricore-cross Alex Bennée
2020-06-09 10:38 ` [PULL 08/17] docker: update Ubuntu to 20.04 Alex Bennée
2020-06-09 10:38 ` Alex Bennée [this message]
2020-06-09 10:38 ` [PULL 10/17] linux-user: provide fallback pgd_find_hole for bare chroots Alex Bennée
2020-06-09 10:38 ` [PULL 11/17] linux-user: deal with address wrap for ARM_COMMPAGE on 32 bit Alex Bennée
2020-06-09 10:38   ` [Bug 1880225] " Alex Bennée
2020-06-09 10:38 ` [PULL 12/17] tests/tcg: add simple commpage test case Alex Bennée
2020-06-09 10:38 ` [PULL 13/17] linux-user: detect overflow of MAP_FIXED mmap Alex Bennée
2020-06-09 10:38 ` [PULL 14/17] tests/docker: Remove flex/bison packages Alex Bennée
2020-06-09 10:38 ` [PULL 15/17] tests/vm: " Alex Bennée
2020-06-09 10:53   ` Claudio Fontana
2020-06-09 11:08     ` Alex Bennée
2020-06-09 10:38 ` [PULL 16/17] cirrus-ci: " Alex Bennée
2020-06-09 10:38 ` [PULL 17/17] scripts/coverity-scan: " Alex Bennée
2020-06-11 10:20 ` [PULL 00/17] testing and misc fixes Peter Maydell
  -- strict thread matches above, loose matches on Subject: below --
2020-06-05 15:49 [PATCH v1 00/14] various fixes for next PR (testing, vhost, guest_base fixes) Alex Bennée
2020-06-05 15:49 ` [PATCH v1 01/14] qemu-plugin.h: add missing include <stddef.h> to define size_t Alex Bennée
2020-06-05 15:49 ` [PATCH v1 02/14] scripts/clean-includes: Mark 'qemu/qemu-plugin.h' as special header Alex Bennée
2020-06-05 15:49 ` [PATCH v1 03/14] tests/plugin: correctly honour io_count Alex Bennée
2020-06-05 15:49 ` [PATCH v1 04/14] exec: flush the whole TLB if a watchpoint crosses a page boundary Alex Bennée
2020-06-05 15:49 ` [PATCH v1 05/14] .travis.yml: allow failure for unreliable hosts Alex Bennée
2020-06-05 15:49 ` [PATCH v1 06/14] .shippable: temporaily disable some cross builds Alex Bennée
2020-06-05 16:12   ` Philippe Mathieu-Daudé
2020-06-05 15:49 ` [PATCH v1 07/14] iotests: 194: wait migration completion on target too Alex Bennée
2020-06-05 15:49 ` [PATCH v1 08/14] tests/docker: fix pre-requisite for debian-tricore-cross Alex Bennée
2020-06-05 15:49 ` [PATCH v1 09/14] docker: update Ubuntu to 20.04 Alex Bennée
2020-06-05 15:49 ` [PATCH v1 10/14] hw/virtio/vhost: re-factor vhost-section and allow DIRTY_MEMORY_CODE Alex Bennée
2020-06-05 15:49 ` [PATCH v1 11/14] linux-user: provide fallback pgd_find_hole for bare chroots Alex Bennée
2020-06-05 15:49 ` [PATCH v1 12/14] linux-user: deal with address wrap for ARM_COMMPAGE on 32 bit Alex Bennée
2020-06-05 15:49   ` [Bug 1880225] " Alex Bennée
2020-06-05 15:49 ` [PATCH v1 13/14] tests/tcg: add simple commpage test case Alex Bennée
2020-06-05 15:49 ` [PATCH v1 14/14] linux-user: detect overflow of MAP_FIXED mmap Alex Bennée
2020-06-05 16:16   ` Philippe Mathieu-Daudé
2020-06-05 15:54 ` [PATCH v1 00/14] various fixes for next PR (testing, vhost, guest_base fixes) Eric Blake
2020-06-05 17:46 ` no-reply
2020-06-07  6:55 ` Thomas Huth
2020-06-08 15:58   ` Alex Bennée
2020-05-27 10:05 [PATCH v1 0/3] some linux-user guest_base fixes Alex Bennée
2020-05-27 10:05 ` [PATCH v1 1/3] linux-user: provide fallback pgd_find_hole for bare chroots Alex Bennée
2020-06-02  0:37   ` Richard Henderson
2020-05-27 10:05 ` [PATCH v1 2/3] linux-user: deal with address wrap for ARM_COMMPAGE on 32 bit Alex Bennée
2020-05-27 10:05   ` [Bug 1880225] " Alex Bennée
2020-05-27 12:05   ` Aleksandar Markovic
2020-05-27 12:05     ` [Bug 1880225] " Aleksandar Markovic
2020-05-27 14:47     ` Aleksandar Markovic
2020-05-27 14:47       ` [Bug 1880225] " Aleksandar Markovic
2020-05-27 16:14       ` Alex Bennée
2020-05-27 16:14         ` [Bug 1880225] " Alex Bennée
2020-06-02  0:28   ` Richard Henderson
2020-06-05  9:45     ` Alex Bennée
2020-06-05  9:45       ` [Bug 1880225] " Alex Bennée
2020-06-05 10:24       ` Alex Bennée
2020-06-05 10:24         ` [Bug 1880225] " Alex Bennée
2020-05-27 10:05 ` [PATCH v1 3/3] tests/tcg: add simple commpage test case Alex Bennée
2020-06-02  0:40   ` Richard Henderson
2020-05-27 14:12 ` [PATCH v1 0/3] some linux-user guest_base fixes no-reply
2020-05-22 18:43 [Bug 1880225] [NEW] Emulation of some arm programs fail with "Assertion `have_guest_base' failed." Aleksandar Markovic
2020-05-22 19:18 ` Alex Bennée
2020-05-22 19:18   ` Alex Bennée
2020-05-22 19:27 ` [Bug 1880225] " Alex Bennée
2020-05-23  1:07 ` Aleksandar Markovic
2020-05-23  1:14 ` Aleksandar Markovic
2020-05-23  7:40   ` Alex Bennée
2020-05-23  7:40     ` Alex Bennée
2020-05-23  1:31 ` Aleksandar Markovic
2020-05-23  7:50 ` Alex Bennée
2020-05-23  7:52 ` Aleksandar Markovic
2020-05-23 10:14   ` Alex Bennée
2020-08-20 15:08 ` Thomas Huth

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200609103809.23443-10-alex.bennee@linaro.org \
    --to=alex.bennee@linaro.org \
    --cc=dgilbert@redhat.com \
    --cc=farosas@linux.ibm.com \
    --cc=mst@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).