From: "Alex Bennée" <alex.bennee@linaro.org>
To: qemu-devel@nongnu.org
Cc: "Alistair Francis" <alistair.francis@wdc.com>,
"Michael Roth" <michael.roth@amd.com>,
"Palmer Dabbelt" <palmer@dabbelt.com>,
"Mahmoud Mandour" <ma.mandourr@gmail.com>,
"Pavel Dovgalyuk" <pavel.dovgaluk@ispras.ru>,
"Yoshinori Sato" <ysato@users.sourceforge.jp>,
"Weiwei Li" <liwei1518@gmail.com>,
"Eduardo Habkost" <eduardo@habkost.net>,
"Marc-André Lureau" <marcandre.lureau@redhat.com>,
"Markus Armbruster" <armbru@redhat.com>,
"David Hildenbrand" <david@redhat.com>,
"Beraldo Leal" <bleal@redhat.com>,
"Liu Zhiwei" <zhiwei_liu@linux.alibaba.com>,
"Eric Auger" <eric.auger@redhat.com>,
"Song Gao" <gaosong@loongson.cn>,
"Alex Bennée" <alex.bennee@linaro.org>,
qemu-arm@nongnu.org, "Peter Xu" <peterx@redhat.com>,
"Jiri Pirko" <jiri@resnulli.us>, "Eric Blake" <eblake@redhat.com>,
"Fabiano Rosas" <farosas@suse.de>,
qemu-s390x@nongnu.org, "Peter Maydell" <peter.maydell@linaro.org>,
"Michael S. Tsirkin" <mst@redhat.com>,
"Daniel Henrique Barboza" <dbarboza@ventanamicro.com>,
"John Snow" <jsnow@redhat.com>,
"Alexandre Iooss" <erdnaxe@crans.org>,
"Konstantin Kostiuk" <kkostiuk@redhat.com>,
"Pierrick Bouvier" <pierrick.bouvier@linaro.org>,
"Cleber Rosa" <crosa@redhat.com>,
"Ilya Leoshkevich" <iii@linux.ibm.com>,
qemu-riscv@nongnu.org, "Thomas Huth" <thuth@redhat.com>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"Richard Henderson" <richard.henderson@linaro.org>,
"Jason Wang" <jasowang@redhat.com>,
"Bin Meng" <bmeng.cn@gmail.com>,
"Philippe Mathieu-Daudé" <philmd@linaro.org>,
"Wainer dos Santos Moschetta" <wainersm@redhat.com>
Subject: [PATCH v2 07/21] target/i386: allow access_ptr to force slow path on failed probe
Date: Tue, 13 Aug 2024 21:23:15 +0100 [thread overview]
Message-ID: <20240813202329.1237572-8-alex.bennee@linaro.org> (raw)
In-Reply-To: <20240813202329.1237572-1-alex.bennee@linaro.org>
When we are using TCG plugin memory callbacks probe_access_internal
will return TLB_MMIO to force the slow path for memory access. This
results in probe_access returning NULL but the x86 access_ptr function
happily accepts an empty haddr resulting in segfault hilarity.
Check for an empty haddr to prevent the segfault and enable plugins to
track all the memory operations for the x86 save/restore helpers. As
we also want to run the slow path when instrumenting *-user we should
also not have the short cutting test_ptr macro.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2489
Fixes: 6d03226b42 (plugins: force slow path when plugins instrument memory ops)
Reviewed-by: Alexandre Iooss <erdnaxe@crans.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20240807160236.2478459-1-alex.bennee@linaro.org>
---
v2
- add comment around haddr1 test
- drop test_ptr macro
---
target/i386/tcg/access.c | 27 +++++++++++++--------------
1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/target/i386/tcg/access.c b/target/i386/tcg/access.c
index 56a1181ea5..e68b73a24b 100644
--- a/target/i386/tcg/access.c
+++ b/target/i386/tcg/access.c
@@ -58,6 +58,11 @@ static void *access_ptr(X86Access *ac, vaddr addr, unsigned len)
assert(addr >= ac->vaddr);
+ /* No haddr means probe_access wants to force slow path */
+ if (!ac->haddr1) {
+ return NULL;
+ }
+
#ifdef CONFIG_USER_ONLY
assert(offset <= ac->size1 - len);
return ac->haddr1 + offset;
@@ -78,17 +83,11 @@ static void *access_ptr(X86Access *ac, vaddr addr, unsigned len)
#endif
}
-#ifdef CONFIG_USER_ONLY
-# define test_ptr(p) true
-#else
-# define test_ptr(p) likely(p)
-#endif
-
uint8_t access_ldb(X86Access *ac, vaddr addr)
{
void *p = access_ptr(ac, addr, sizeof(uint8_t));
- if (test_ptr(p)) {
+ if (likely(p)) {
return ldub_p(p);
}
return cpu_ldub_mmuidx_ra(ac->env, addr, ac->mmu_idx, ac->ra);
@@ -98,7 +97,7 @@ uint16_t access_ldw(X86Access *ac, vaddr addr)
{
void *p = access_ptr(ac, addr, sizeof(uint16_t));
- if (test_ptr(p)) {
+ if (likely(p)) {
return lduw_le_p(p);
}
return cpu_lduw_le_mmuidx_ra(ac->env, addr, ac->mmu_idx, ac->ra);
@@ -108,7 +107,7 @@ uint32_t access_ldl(X86Access *ac, vaddr addr)
{
void *p = access_ptr(ac, addr, sizeof(uint32_t));
- if (test_ptr(p)) {
+ if (likely(p)) {
return ldl_le_p(p);
}
return cpu_ldl_le_mmuidx_ra(ac->env, addr, ac->mmu_idx, ac->ra);
@@ -118,7 +117,7 @@ uint64_t access_ldq(X86Access *ac, vaddr addr)
{
void *p = access_ptr(ac, addr, sizeof(uint64_t));
- if (test_ptr(p)) {
+ if (likely(p)) {
return ldq_le_p(p);
}
return cpu_ldq_le_mmuidx_ra(ac->env, addr, ac->mmu_idx, ac->ra);
@@ -128,7 +127,7 @@ void access_stb(X86Access *ac, vaddr addr, uint8_t val)
{
void *p = access_ptr(ac, addr, sizeof(uint8_t));
- if (test_ptr(p)) {
+ if (likely(p)) {
stb_p(p, val);
} else {
cpu_stb_mmuidx_ra(ac->env, addr, val, ac->mmu_idx, ac->ra);
@@ -139,7 +138,7 @@ void access_stw(X86Access *ac, vaddr addr, uint16_t val)
{
void *p = access_ptr(ac, addr, sizeof(uint16_t));
- if (test_ptr(p)) {
+ if (likely(p)) {
stw_le_p(p, val);
} else {
cpu_stw_le_mmuidx_ra(ac->env, addr, val, ac->mmu_idx, ac->ra);
@@ -150,7 +149,7 @@ void access_stl(X86Access *ac, vaddr addr, uint32_t val)
{
void *p = access_ptr(ac, addr, sizeof(uint32_t));
- if (test_ptr(p)) {
+ if (likely(p)) {
stl_le_p(p, val);
} else {
cpu_stl_le_mmuidx_ra(ac->env, addr, val, ac->mmu_idx, ac->ra);
@@ -161,7 +160,7 @@ void access_stq(X86Access *ac, vaddr addr, uint64_t val)
{
void *p = access_ptr(ac, addr, sizeof(uint64_t));
- if (test_ptr(p)) {
+ if (likely(p)) {
stq_le_p(p, val);
} else {
cpu_stq_le_mmuidx_ra(ac->env, addr, val, ac->mmu_idx, ac->ra);
--
2.39.2
next prev parent reply other threads:[~2024-08-13 20:25 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-13 20:23 [PATCH v2 00/21] Various fixes and tweaks for 9.1-rc2/3 Alex Bennée
2024-08-13 20:23 ` [PATCH v2 01/21] tests/avocado: Re-enable gdbsim-r5f562n8 testing U-Boot Alex Bennée
2024-08-13 20:23 ` [PATCH v2 02/21] Makefile: trigger re-configure on updated pythondeps Alex Bennée
2024-08-13 20:23 ` [PATCH v2 03/21] configure: Fix arch detection for GDB_HAS_MTE Alex Bennée
2024-08-13 20:23 ` [PATCH v2 04/21] configure: Avoid use of param. expansion when using gdb_version Alex Bennée
2024-08-13 20:23 ` [PATCH v2 05/21] configure: Fix GDB version detection for GDB_HAS_MTE Alex Bennée
2024-08-13 20:23 ` [PATCH v2 06/21] scripts/checkpatch: more checks on files imported from Linux Alex Bennée
2024-08-13 20:23 ` Alex Bennée [this message]
2024-08-14 1:55 ` [PATCH v2 07/21] target/i386: allow access_ptr to force slow path on failed probe Richard Henderson
2024-08-13 20:23 ` [PATCH v2 08/21] buildsys: Fix building without plugins on Darwin Alex Bennée
2024-08-14 8:29 ` Philippe Mathieu-Daudé
2024-08-13 20:23 ` [PATCH v2 09/21] scripts/replay-dump.py: Update to current rr record format Alex Bennée
2024-08-13 20:23 ` [PATCH v2 10/21] scripts/replay-dump.py: rejig decoders in event number order Alex Bennée
2024-08-13 20:23 ` [PATCH v2 11/21] tests/avocado: excercise scripts/replay-dump.py in replay tests Alex Bennée
2024-08-13 20:23 ` [PATCH v2 12/21] replay: allow runstate shutdown->running when replaying trace Alex Bennée
2024-08-13 20:23 ` [PATCH v2 13/21] Revert "replay: stop us hanging in rr_wait_io_event" Alex Bennée
2024-08-13 20:23 ` [PATCH v2 14/21] tests/avocado: replay_kernel.py add x86-64 q35 machine test Alex Bennée
2024-08-13 20:23 ` [PATCH v2 15/21] chardev: set record/replay on the base device of a muxed device Alex Bennée
2024-08-13 20:23 ` [PATCH v2 16/21] virtio-net: Use replay_schedule_bh_event for bhs that affect machine state Alex Bennée
2024-08-13 20:48 ` Michael S. Tsirkin
2024-08-14 6:05 ` Nicholas Piggin
2024-08-14 7:06 ` Michael S. Tsirkin
2024-08-14 17:25 ` Alex Bennée
2024-08-15 7:12 ` Nicholas Piggin
2024-08-15 14:28 ` Michael S. Tsirkin
2024-08-16 2:26 ` Nicholas Piggin
2024-08-16 2:31 ` Jason Wang
2024-08-16 12:58 ` Alex Bennée
2024-08-13 20:23 ` [PATCH v2 17/21] virtio-net: Use virtual time for RSC timers Alex Bennée
2024-08-13 20:49 ` Michael S. Tsirkin
2024-08-13 20:23 ` [PATCH v2 18/21] savevm: Fix load_snapshot error path crash Alex Bennée
2024-08-13 20:23 ` [PATCH v2 19/21] docs: Fix some typos (found by typos) and grammar issues Alex Bennée
2024-08-13 20:23 ` [PATCH v2 20/21] docs/devel: update tcg-plugins page Alex Bennée
2024-08-13 20:23 ` [PATCH v2 21/21] plugins: fix race condition with scoreboards 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=20240813202329.1237572-8-alex.bennee@linaro.org \
--to=alex.bennee@linaro.org \
--cc=alistair.francis@wdc.com \
--cc=armbru@redhat.com \
--cc=bleal@redhat.com \
--cc=bmeng.cn@gmail.com \
--cc=crosa@redhat.com \
--cc=david@redhat.com \
--cc=dbarboza@ventanamicro.com \
--cc=eblake@redhat.com \
--cc=eduardo@habkost.net \
--cc=erdnaxe@crans.org \
--cc=eric.auger@redhat.com \
--cc=farosas@suse.de \
--cc=gaosong@loongson.cn \
--cc=iii@linux.ibm.com \
--cc=jasowang@redhat.com \
--cc=jiri@resnulli.us \
--cc=jsnow@redhat.com \
--cc=kkostiuk@redhat.com \
--cc=liwei1518@gmail.com \
--cc=ma.mandourr@gmail.com \
--cc=marcandre.lureau@redhat.com \
--cc=michael.roth@amd.com \
--cc=mst@redhat.com \
--cc=palmer@dabbelt.com \
--cc=pavel.dovgaluk@ispras.ru \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=peterx@redhat.com \
--cc=philmd@linaro.org \
--cc=pierrick.bouvier@linaro.org \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=qemu-riscv@nongnu.org \
--cc=qemu-s390x@nongnu.org \
--cc=richard.henderson@linaro.org \
--cc=thuth@redhat.com \
--cc=wainersm@redhat.com \
--cc=ysato@users.sourceforge.jp \
--cc=zhiwei_liu@linux.alibaba.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).