From: "Alex Bennée" <alex.bennee@linaro.org>
To: qemu-devel@nongnu.org
Cc: robert.foley@linaro.org, "Paolo Bonzini" <pbonzini@redhat.com>,
robhenry@microsoft.com, aaron@os.amperecomputing.com,
cota@braap.org, kuhn.chenqun@huawei.com, peter.puhov@linaro.org,
"Alex Bennée" <alex.bennee@linaro.org>,
"Richard Henderson" <rth@twiddle.net>
Subject: [PATCH v2 3/6] cputlb: ensure we save the IOTLB data in case of reset
Date: Wed, 10 Jun 2020 16:55:06 +0100 [thread overview]
Message-ID: <20200610155509.12850-4-alex.bennee@linaro.org> (raw)
In-Reply-To: <20200610155509.12850-1-alex.bennee@linaro.org>
Any write to a device might cause a re-arrangement of memory
triggering a TLB flush and potential re-size of the TLB invalidating
previous entries. This would cause users of qemu_plugin_get_hwaddr()
to see the warning:
invalid use of qemu_plugin_get_hwaddr
because of the failed tlb_lookup which should always succeed. To
prevent this we save the IOTLB data in case it is later needed by a
plugin doing a lookup.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
v2
- save the entry instead of re-running the tlb_fill.
squash! cputlb: ensure we save the IOTLB entry in case of reset
---
accel/tcg/cputlb.c | 63 ++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 61 insertions(+), 2 deletions(-)
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index eb2cf9de5e6..9bf9e479c7c 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1058,6 +1058,47 @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry,
return val;
}
+#ifdef CONFIG_PLUGIN
+
+typedef struct SavedIOTLB {
+ struct rcu_head rcu;
+ struct SavedIOTLB **save_loc;
+ MemoryRegionSection *section;
+ hwaddr mr_offset;
+} SavedIOTLB;
+
+static void clean_saved_entry(SavedIOTLB *s)
+{
+ atomic_rcu_set(s->save_loc, NULL);
+ g_free(s);
+}
+
+static __thread SavedIOTLB *saved_for_plugin;
+
+/*
+ * Save a potentially trashed IOTLB entry for later lookup by plugin.
+ *
+ * We also need to track the thread storage address because the RCU
+ * cleanup that runs when we leave the critical region (the current
+ * execution) is actually in a different thread.
+ */
+static void save_iotlb_data(MemoryRegionSection *section, hwaddr mr_offset)
+{
+ SavedIOTLB *s = g_new(SavedIOTLB, 1);
+ s->save_loc = &saved_for_plugin;
+ s->section = section;
+ s->mr_offset = mr_offset;
+ atomic_rcu_set(&saved_for_plugin, s);
+ call_rcu(s, clean_saved_entry, rcu);
+}
+
+#else
+static void save_iotlb_data(MemoryRegionSection *section, hwaddr mr_offset)
+{
+ /* do nothing */
+}
+#endif
+
static void io_writex(CPUArchState *env, CPUIOTLBEntry *iotlbentry,
int mmu_idx, uint64_t val, target_ulong addr,
uintptr_t retaddr, MemOp op)
@@ -1077,6 +1118,12 @@ static void io_writex(CPUArchState *env, CPUIOTLBEntry *iotlbentry,
}
cpu->mem_io_pc = retaddr;
+ /*
+ * The memory_region_dispatch may trigger a flush/resize
+ * so for plugins we save the iotlb_data just in case.
+ */
+ save_iotlb_data(section, mr_offset);
+
if (mr->global_locking && !qemu_mutex_iothread_locked()) {
qemu_mutex_lock_iothread();
locked = true;
@@ -1091,6 +1138,7 @@ static void io_writex(CPUArchState *env, CPUIOTLBEntry *iotlbentry,
MMU_DATA_STORE, mmu_idx, iotlbentry->attrs, r,
retaddr);
}
+
if (locked) {
qemu_mutex_unlock_iothread();
}
@@ -1366,8 +1414,11 @@ void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
* in the softmmu lookup code (or helper). We don't handle re-fills or
* checking the victim table. This is purely informational.
*
- * This should never fail as the memory access being instrumented
- * should have just filled the TLB.
+ * This almost never fails as the memory access being instrumented
+ * should have just filled the TLB. The one corner case is io_writex
+ * which can cause TLB flushes and potential resizing of the TLBs
+ * loosing the information we need. In those cases we need to recover
+ * data from a thread local copy of the io_tlb entry.
*/
bool tlb_plugin_lookup(CPUState *cpu, target_ulong addr, int mmu_idx,
@@ -1391,6 +1442,14 @@ bool tlb_plugin_lookup(CPUState *cpu, target_ulong addr, int mmu_idx,
data->v.ram.hostaddr = addr + tlbe->addend;
}
return true;
+ } else {
+ SavedIOTLB *saved = atomic_rcu_read(&saved_for_plugin);
+ if (saved) {
+ data->is_io = true;
+ data->v.io.section = saved->section;
+ data->v.io.offset = saved->mr_offset;
+ return true;
+ }
}
return false;
}
--
2.20.1
next prev parent reply other threads:[~2020-06-10 15:57 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-10 15:55 [PATCH v2 0/6] plugins/next (lockstep, api, hwprofile) Alex Bennée
2020-06-10 15:55 ` [PATCH v2 1/6] iotests: 194: wait migration completion on target too Alex Bennée
2020-06-10 16:38 ` Alex Bennée
2020-06-10 15:55 ` [PATCH v2 2/6] plugins: new lockstep plugin for debugging TCG changes Alex Bennée
2020-06-11 17:04 ` Robert Foley
2020-06-10 15:55 ` Alex Bennée [this message]
2020-06-21 20:33 ` [PATCH v2 3/6] cputlb: ensure we save the IOTLB data in case of reset Emilio G. Cota
2020-06-22 9:02 ` Alex Bennée
2020-06-23 1:54 ` Emilio G. Cota
2020-06-10 15:55 ` [PATCH v2 4/6] hw/virtio/pci: include vdev name in registered PCI sections Alex Bennée
2020-06-10 15:55 ` [PATCH v2 5/6] plugins: add API to return a name for a IO device Alex Bennée
2020-06-10 15:55 ` [PATCH v2 6/6] plugins: new hwprofile plugin Alex Bennée
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=20200610155509.12850-4-alex.bennee@linaro.org \
--to=alex.bennee@linaro.org \
--cc=aaron@os.amperecomputing.com \
--cc=cota@braap.org \
--cc=kuhn.chenqun@huawei.com \
--cc=pbonzini@redhat.com \
--cc=peter.puhov@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=robert.foley@linaro.org \
--cc=robhenry@microsoft.com \
--cc=rth@twiddle.net \
/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).