[RFC,v5,2/3] memory: add depth assert in address_space_to_flatview

Commit Message

Chuang Xu Jan. 17, 2023, 11:55 a.m. UTC

Before using any flatview, sanity check we're not during a memory
region transaction or the map can be invalid.

Signed-off-by: Chuang Xu <[e-mail]>
---
 include/exec/memory.h | 15 +++++++++++++++
 softmmu/memory.c      |  5 +++++
 2 files changed, 20 insertions(+)

Comments

Juan Quintela Feb. 8, 2023, 7:31 p.m. UTC | #1

Chuang Xu <[e-mail]> wrote:
> Before using any flatview, sanity check we're not during a memory
> region transaction or the map can be invalid.
>
> Signed-off-by: Chuang Xu <[e-mail]>

Reviewed-by: Juan Quintela <[e-mail]>

I am waiting to see what Paolo thinks (specially of patch 1).

Later, JUan.

Patch

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 91f8a2395a..ce13ebb763 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -27,6 +27,7 @@ 
 #include "qemu/notify.h"
 #include "qom/object.h"
 #include "qemu/rcu.h"
+#include "qemu/main-loop.h"
 
 #define RAM_ADDR_INVALID (~(ram_addr_t)0)
 
@@ -1069,8 +1070,22 @@  struct FlatView {
     MemoryRegion *root;
 };
 
+bool memory_region_transaction_in_progress(void);
+
 static inline FlatView *address_space_to_flatview(AddressSpace *as)
 {
+    /*
+     * Before using any flatview, sanity check we're not during a memory
+     * region transaction or the map can be invalid.  Note that this can
+     * also be called during commit phase of memory transaction, but that
+     * should also only happen when the depth decreases to 0 first.
+     * Meanwhile it's safe to access current_map with RCU read lock held
+     * even if during a memory transaction. It means the user can bear
+     * with an obsolete map.
+     */
+    assert((!memory_region_transaction_in_progress() &&
+            qemu_mutex_iothread_locked()) ||
+            rcu_read_is_locked());
     return qatomic_rcu_read(&as->current_map);
 }
 
diff --git a/softmmu/memory.c b/softmmu/memory.c
index bc0be3f62c..856c37fd0a 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -1116,6 +1116,11 @@  void memory_region_transaction_commit(void)
    }
 }
 
+bool memory_region_transaction_in_progress(void)
+{
+    return memory_region_transaction_depth != 0;
+}
+
 static void memory_region_destructor_none(MemoryRegion *mr)
 {
 }