* [PATCH v2 01/33] accel/tcg: Use vaddr in translator_ld*
2024-04-24 23:30 [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Richard Henderson
@ 2024-04-24 23:30 ` Richard Henderson
2024-04-24 23:31 ` [PATCH v2 02/33] accel/tcg: Hide in_same_page outside of a target-specific context Richard Henderson
` (32 subsequent siblings)
33 siblings, 0 replies; 58+ messages in thread
From: Richard Henderson @ 2024-04-24 23:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Philippe Mathieu-Daudé
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/translator.h | 18 +++++++++---------
accel/tcg/translator.c | 15 ++++++++-------
target/hexagon/translate.c | 1 +
target/microblaze/translate.c | 1 +
4 files changed, 19 insertions(+), 16 deletions(-)
diff --git a/include/exec/translator.h b/include/exec/translator.h
index 2c4fb818e7..51489c181c 100644
--- a/include/exec/translator.h
+++ b/include/exec/translator.h
@@ -19,7 +19,7 @@
*/
#include "qemu/bswap.h"
-#include "exec/cpu_ldst.h" /* for abi_ptr */
+#include "exec/vaddr.h"
/**
* gen_intermediate_code
@@ -182,14 +182,14 @@ bool translator_io_start(DisasContextBase *db);
* the relevant information at translation time.
*/
-uint8_t translator_ldub(CPUArchState *env, DisasContextBase *db, abi_ptr pc);
-uint16_t translator_lduw(CPUArchState *env, DisasContextBase *db, abi_ptr pc);
-uint32_t translator_ldl(CPUArchState *env, DisasContextBase *db, abi_ptr pc);
-uint64_t translator_ldq(CPUArchState *env, DisasContextBase *db, abi_ptr pc);
+uint8_t translator_ldub(CPUArchState *env, DisasContextBase *db, vaddr pc);
+uint16_t translator_lduw(CPUArchState *env, DisasContextBase *db, vaddr pc);
+uint32_t translator_ldl(CPUArchState *env, DisasContextBase *db, vaddr pc);
+uint64_t translator_ldq(CPUArchState *env, DisasContextBase *db, vaddr pc);
static inline uint16_t
translator_lduw_swap(CPUArchState *env, DisasContextBase *db,
- abi_ptr pc, bool do_swap)
+ vaddr pc, bool do_swap)
{
uint16_t ret = translator_lduw(env, db, pc);
if (do_swap) {
@@ -200,7 +200,7 @@ translator_lduw_swap(CPUArchState *env, DisasContextBase *db,
static inline uint32_t
translator_ldl_swap(CPUArchState *env, DisasContextBase *db,
- abi_ptr pc, bool do_swap)
+ vaddr pc, bool do_swap)
{
uint32_t ret = translator_ldl(env, db, pc);
if (do_swap) {
@@ -211,7 +211,7 @@ translator_ldl_swap(CPUArchState *env, DisasContextBase *db,
static inline uint64_t
translator_ldq_swap(CPUArchState *env, DisasContextBase *db,
- abi_ptr pc, bool do_swap)
+ vaddr pc, bool do_swap)
{
uint64_t ret = translator_ldq(env, db, pc);
if (do_swap) {
@@ -230,7 +230,7 @@ translator_ldq_swap(CPUArchState *env, DisasContextBase *db,
* re-synthesised for s390x "ex"). It ensures we update other areas of
* the translator with details of the executed instruction.
*/
-void translator_fake_ldb(uint8_t insn8, abi_ptr pc);
+void translator_fake_ldb(uint8_t insn8, vaddr pc);
/*
* Return whether addr is on the same page as where disassembly started.
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index 9de0bc34c8..42fa977e45 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -13,6 +13,7 @@
#include "exec/exec-all.h"
#include "exec/translator.h"
#include "exec/plugin-gen.h"
+#include "exec/cpu_ldst.h"
#include "tcg/tcg-op-common.h"
#include "internal-target.h"
@@ -293,11 +294,11 @@ static void *translator_access(CPUArchState *env, DisasContextBase *db,
return host + (pc - base);
}
-static void plugin_insn_append(abi_ptr pc, const void *from, size_t size)
+static void plugin_insn_append(vaddr pc, const void *from, size_t size)
{
#ifdef CONFIG_PLUGIN
struct qemu_plugin_insn *insn = tcg_ctx->plugin_insn;
- abi_ptr off;
+ size_t off;
if (insn == NULL) {
return;
@@ -314,7 +315,7 @@ static void plugin_insn_append(abi_ptr pc, const void *from, size_t size)
#endif
}
-uint8_t translator_ldub(CPUArchState *env, DisasContextBase *db, abi_ptr pc)
+uint8_t translator_ldub(CPUArchState *env, DisasContextBase *db, vaddr pc)
{
uint8_t ret;
void *p = translator_access(env, db, pc, sizeof(ret));
@@ -328,7 +329,7 @@ uint8_t translator_ldub(CPUArchState *env, DisasContextBase *db, abi_ptr pc)
return ret;
}
-uint16_t translator_lduw(CPUArchState *env, DisasContextBase *db, abi_ptr pc)
+uint16_t translator_lduw(CPUArchState *env, DisasContextBase *db, vaddr pc)
{
uint16_t ret, plug;
void *p = translator_access(env, db, pc, sizeof(ret));
@@ -343,7 +344,7 @@ uint16_t translator_lduw(CPUArchState *env, DisasContextBase *db, abi_ptr pc)
return ret;
}
-uint32_t translator_ldl(CPUArchState *env, DisasContextBase *db, abi_ptr pc)
+uint32_t translator_ldl(CPUArchState *env, DisasContextBase *db, vaddr pc)
{
uint32_t ret, plug;
void *p = translator_access(env, db, pc, sizeof(ret));
@@ -358,7 +359,7 @@ uint32_t translator_ldl(CPUArchState *env, DisasContextBase *db, abi_ptr pc)
return ret;
}
-uint64_t translator_ldq(CPUArchState *env, DisasContextBase *db, abi_ptr pc)
+uint64_t translator_ldq(CPUArchState *env, DisasContextBase *db, vaddr pc)
{
uint64_t ret, plug;
void *p = translator_access(env, db, pc, sizeof(ret));
@@ -373,7 +374,7 @@ uint64_t translator_ldq(CPUArchState *env, DisasContextBase *db, abi_ptr pc)
return ret;
}
-void translator_fake_ldb(uint8_t insn8, abi_ptr pc)
+void translator_fake_ldb(uint8_t insn8, vaddr pc)
{
plugin_insn_append(pc, &insn8, sizeof(insn8));
}
diff --git a/target/hexagon/translate.c b/target/hexagon/translate.c
index f163eefe97..c9bf0e7508 100644
--- a/target/hexagon/translate.c
+++ b/target/hexagon/translate.c
@@ -24,6 +24,7 @@
#include "exec/helper-proto.h"
#include "exec/translation-block.h"
#include "exec/log.h"
+#include "exec/cpu_ldst.h"
#include "internal.h"
#include "attribs.h"
#include "insn.h"
diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index fc451befae..0941aa67c0 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -25,6 +25,7 @@
#include "tcg/tcg-op.h"
#include "exec/helper-proto.h"
#include "exec/helper-gen.h"
+#include "exec/cpu_ldst.h"
#include "exec/translator.h"
#include "qemu/qemu-print.h"
--
2.34.1
^ permalink raw reply related [flat|nested] 58+ messages in thread* [PATCH v2 02/33] accel/tcg: Hide in_same_page outside of a target-specific context
2024-04-24 23:30 [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Richard Henderson
2024-04-24 23:30 ` [PATCH v2 01/33] accel/tcg: Use vaddr in translator_ld* Richard Henderson
@ 2024-04-24 23:31 ` Richard Henderson
2024-04-24 23:31 ` [PATCH v2 03/33] accel/tcg: Pass DisasContextBase to translator_fake_ldb Richard Henderson
` (31 subsequent siblings)
33 siblings, 0 replies; 58+ messages in thread
From: Richard Henderson @ 2024-04-24 23:31 UTC (permalink / raw)
To: qemu-devel; +Cc: Philippe Mathieu-Daudé
While there are other methods that could be used to replace
TARGET_PAGE_MASK, the function is not really required outside
the context of target-specific translation.
This makes the header usable by target independent code.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/translator.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/exec/translator.h b/include/exec/translator.h
index 51489c181c..c6a9e4b69a 100644
--- a/include/exec/translator.h
+++ b/include/exec/translator.h
@@ -232,6 +232,7 @@ translator_ldq_swap(CPUArchState *env, DisasContextBase *db,
*/
void translator_fake_ldb(uint8_t insn8, vaddr pc);
+#ifdef NEED_CPU_H
/*
* Return whether addr is on the same page as where disassembly started.
* Translators can use this to enforce the rule that only single-insn
@@ -241,5 +242,6 @@ static inline bool is_same_page(const DisasContextBase *db, vaddr addr)
{
return ((addr ^ db->pc_first) & TARGET_PAGE_MASK) == 0;
}
+#endif
#endif /* EXEC__TRANSLATOR_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 58+ messages in thread* [PATCH v2 03/33] accel/tcg: Pass DisasContextBase to translator_fake_ldb
2024-04-24 23:30 [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Richard Henderson
2024-04-24 23:30 ` [PATCH v2 01/33] accel/tcg: Use vaddr in translator_ld* Richard Henderson
2024-04-24 23:31 ` [PATCH v2 02/33] accel/tcg: Hide in_same_page outside of a target-specific context Richard Henderson
@ 2024-04-24 23:31 ` Richard Henderson
2024-04-24 23:31 ` [PATCH v2 04/33] accel/tcg: Reorg translator_ld* Richard Henderson
` (30 subsequent siblings)
33 siblings, 0 replies; 58+ messages in thread
From: Richard Henderson @ 2024-04-24 23:31 UTC (permalink / raw)
To: qemu-devel; +Cc: Philippe Mathieu-Daudé
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/translator.h | 5 +++--
accel/tcg/translator.c | 2 +-
target/s390x/tcg/translate.c | 2 +-
3 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/include/exec/translator.h b/include/exec/translator.h
index c6a9e4b69a..83fe66cba0 100644
--- a/include/exec/translator.h
+++ b/include/exec/translator.h
@@ -222,15 +222,16 @@ translator_ldq_swap(CPUArchState *env, DisasContextBase *db,
/**
* translator_fake_ldb - fake instruction load
- * @insn8: byte of instruction
+ * @db: Disassembly context
* @pc: program counter of instruction
+ * @insn8: byte of instruction
*
* This is a special case helper used where the instruction we are
* about to translate comes from somewhere else (e.g. being
* re-synthesised for s390x "ex"). It ensures we update other areas of
* the translator with details of the executed instruction.
*/
-void translator_fake_ldb(uint8_t insn8, vaddr pc);
+void translator_fake_ldb(DisasContextBase *db, vaddr pc, uint8_t insn8);
#ifdef NEED_CPU_H
/*
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index 42fa977e45..92eb77c3a0 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -374,7 +374,7 @@ uint64_t translator_ldq(CPUArchState *env, DisasContextBase *db, vaddr pc)
return ret;
}
-void translator_fake_ldb(uint8_t insn8, vaddr pc)
+void translator_fake_ldb(DisasContextBase *db, vaddr pc, uint8_t insn8)
{
plugin_insn_append(pc, &insn8, sizeof(insn8));
}
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index 90a74ee795..6d7f6e7064 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -6203,7 +6203,7 @@ static const DisasInsn *extract_insn(CPUS390XState *env, DisasContext *s)
/* Register insn bytes with translator so plugins work. */
for (int i = 0; i < ilen; i++) {
uint8_t byte = extract64(insn, 56 - (i * 8), 8);
- translator_fake_ldb(byte, pc + i);
+ translator_fake_ldb(&s->base, pc + i, byte);
}
op = insn >> 56;
} else {
--
2.34.1
^ permalink raw reply related [flat|nested] 58+ messages in thread* [PATCH v2 04/33] accel/tcg: Reorg translator_ld*
2024-04-24 23:30 [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Richard Henderson
` (2 preceding siblings ...)
2024-04-24 23:31 ` [PATCH v2 03/33] accel/tcg: Pass DisasContextBase to translator_fake_ldb Richard Henderson
@ 2024-04-24 23:31 ` Richard Henderson
2024-05-06 22:47 ` Philippe Mathieu-Daudé
2024-04-24 23:31 ` [PATCH v2 05/33] accel/tcg: Cap the translation block when we encounter mmio Richard Henderson
` (29 subsequent siblings)
33 siblings, 1 reply; 58+ messages in thread
From: Richard Henderson @ 2024-04-24 23:31 UTC (permalink / raw)
To: qemu-devel
Reorg translator_access into translator_ld, with a more
memcpy-ish interface. If both pages are in ram, do not
go through the caller's slow path.
Assert that the access is within the two pages that we are
prepared to protect, per TranslationBlock. Allow access
prior to pc_first, so long as it is within the first page.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
accel/tcg/translator.c | 189 ++++++++++++++++++++++-------------------
1 file changed, 101 insertions(+), 88 deletions(-)
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index 92eb77c3a0..dbd54e25a2 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -229,69 +229,88 @@ void translator_loop(CPUState *cpu, TranslationBlock *tb, int *max_insns,
}
}
-static void *translator_access(CPUArchState *env, DisasContextBase *db,
- vaddr pc, size_t len)
+static bool translator_ld(CPUArchState *env, DisasContextBase *db,
+ void *dest, vaddr pc, size_t len)
{
+ TranslationBlock *tb = db->tb;
+ vaddr last = pc + len - 1;
void *host;
- vaddr base, end;
- TranslationBlock *tb;
-
- tb = db->tb;
+ vaddr base;
/* Use slow path if first page is MMIO. */
if (unlikely(tb_page_addr0(tb) == -1)) {
- return NULL;
+ return false;
}
- end = pc + len - 1;
- if (likely(is_same_page(db, end))) {
- host = db->host_addr[0];
- base = db->pc_first;
- } else {
+ host = db->host_addr[0];
+ base = db->pc_first;
+
+ if (likely(((base ^ last) & TARGET_PAGE_MASK) == 0)) {
+ /* Entire read is from the first page. */
+ memcpy(dest, host + (pc - base), len);
+ return true;
+ }
+
+ if (unlikely(((base ^ pc) & TARGET_PAGE_MASK) == 0)) {
+ /* Read begins on the first page and extends to the second. */
+ size_t len0 = -(pc | TARGET_PAGE_MASK);
+ memcpy(dest, host + (pc - base), len0);
+ pc += len0;
+ dest += len0;
+ len -= len0;
+ }
+
+ /*
+ * The read must conclude on the second page and not extend to a third.
+ *
+ * TODO: We could allow the two pages to be virtually discontiguous,
+ * since we already allow the two pages to be physically discontiguous.
+ * The only reasonable use case would be executing an insn at the end
+ * of the address space wrapping around to the beginning. For that,
+ * we would need to know the current width of the address space.
+ * In the meantime, assert.
+ */
+ base = (base & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
+ assert(((base ^ pc) & TARGET_PAGE_MASK) == 0);
+ assert(((base ^ last) & TARGET_PAGE_MASK) == 0);
+ host = db->host_addr[1];
+
+ if (host == NULL) {
+ tb_page_addr_t page0, old_page1, new_page1;
+
+ new_page1 = get_page_addr_code_hostp(env, base, &db->host_addr[1]);
+
+ /*
+ * If the second page is MMIO, treat as if the first page
+ * was MMIO as well, so that we do not cache the TB.
+ */
+ if (unlikely(new_page1 == -1)) {
+ tb_unlock_pages(tb);
+ tb_set_page_addr0(tb, -1);
+ return false;
+ }
+
+ /*
+ * If this is not the first time around, and page1 matches,
+ * then we already have the page locked. Alternately, we're
+ * not doing anything to prevent the PTE from changing, so
+ * we might wind up with a different page, requiring us to
+ * re-do the locking.
+ */
+ old_page1 = tb_page_addr1(tb);
+ if (likely(new_page1 != old_page1)) {
+ page0 = tb_page_addr0(tb);
+ if (unlikely(old_page1 != -1)) {
+ tb_unlock_page1(page0, old_page1);
+ }
+ tb_set_page_addr1(tb, new_page1);
+ tb_lock_page1(page0, new_page1);
+ }
host = db->host_addr[1];
- base = TARGET_PAGE_ALIGN(db->pc_first);
- if (host == NULL) {
- tb_page_addr_t page0, old_page1, new_page1;
-
- new_page1 = get_page_addr_code_hostp(env, base, &db->host_addr[1]);
-
- /*
- * If the second page is MMIO, treat as if the first page
- * was MMIO as well, so that we do not cache the TB.
- */
- if (unlikely(new_page1 == -1)) {
- tb_unlock_pages(tb);
- tb_set_page_addr0(tb, -1);
- return NULL;
- }
-
- /*
- * If this is not the first time around, and page1 matches,
- * then we already have the page locked. Alternately, we're
- * not doing anything to prevent the PTE from changing, so
- * we might wind up with a different page, requiring us to
- * re-do the locking.
- */
- old_page1 = tb_page_addr1(tb);
- if (likely(new_page1 != old_page1)) {
- page0 = tb_page_addr0(tb);
- if (unlikely(old_page1 != -1)) {
- tb_unlock_page1(page0, old_page1);
- }
- tb_set_page_addr1(tb, new_page1);
- tb_lock_page1(page0, new_page1);
- }
- host = db->host_addr[1];
- }
-
- /* Use slow path when crossing pages. */
- if (is_same_page(db, pc)) {
- return NULL;
- }
}
- tcg_debug_assert(pc >= base);
- return host + (pc - base);
+ memcpy(dest, host + (pc - base), len);
+ return true;
}
static void plugin_insn_append(vaddr pc, const void *from, size_t size)
@@ -317,61 +336,55 @@ static void plugin_insn_append(vaddr pc, const void *from, size_t size)
uint8_t translator_ldub(CPUArchState *env, DisasContextBase *db, vaddr pc)
{
- uint8_t ret;
- void *p = translator_access(env, db, pc, sizeof(ret));
+ uint8_t raw;
- if (p) {
- plugin_insn_append(pc, p, sizeof(ret));
- return ldub_p(p);
+ if (!translator_ld(env, db, &raw, pc, sizeof(raw))) {
+ raw = cpu_ldub_code(env, pc);
}
- ret = cpu_ldub_code(env, pc);
- plugin_insn_append(pc, &ret, sizeof(ret));
- return ret;
+ plugin_insn_append(pc, &raw, sizeof(raw));
+ return raw;
}
uint16_t translator_lduw(CPUArchState *env, DisasContextBase *db, vaddr pc)
{
- uint16_t ret, plug;
- void *p = translator_access(env, db, pc, sizeof(ret));
+ uint16_t raw, tgt;
- if (p) {
- plugin_insn_append(pc, p, sizeof(ret));
- return lduw_p(p);
+ if (translator_ld(env, db, &raw, pc, sizeof(raw))) {
+ tgt = tswap16(raw);
+ } else {
+ tgt = cpu_lduw_code(env, pc);
+ raw = tswap16(tgt);
}
- ret = cpu_lduw_code(env, pc);
- plug = tswap16(ret);
- plugin_insn_append(pc, &plug, sizeof(ret));
- return ret;
+ plugin_insn_append(pc, &raw, sizeof(raw));
+ return tgt;
}
uint32_t translator_ldl(CPUArchState *env, DisasContextBase *db, vaddr pc)
{
- uint32_t ret, plug;
- void *p = translator_access(env, db, pc, sizeof(ret));
+ uint32_t raw, tgt;
- if (p) {
- plugin_insn_append(pc, p, sizeof(ret));
- return ldl_p(p);
+ if (translator_ld(env, db, &raw, pc, sizeof(raw))) {
+ tgt = tswap32(raw);
+ } else {
+ tgt = cpu_ldl_code(env, pc);
+ raw = tswap32(tgt);
}
- ret = cpu_ldl_code(env, pc);
- plug = tswap32(ret);
- plugin_insn_append(pc, &plug, sizeof(ret));
- return ret;
+ plugin_insn_append(pc, &raw, sizeof(raw));
+ return tgt;
}
uint64_t translator_ldq(CPUArchState *env, DisasContextBase *db, vaddr pc)
{
- uint64_t ret, plug;
- void *p = translator_access(env, db, pc, sizeof(ret));
+ uint64_t raw, tgt;
- if (p) {
- plugin_insn_append(pc, p, sizeof(ret));
- return ldq_p(p);
+ if (translator_ld(env, db, &raw, pc, sizeof(raw))) {
+ tgt = tswap64(raw);
+ } else {
+ tgt = cpu_ldl_code(env, pc);
+ raw = tswap64(tgt);
}
- ret = cpu_ldq_code(env, pc);
- plug = tswap64(ret);
- plugin_insn_append(pc, &plug, sizeof(ret));
- return ret;
+ plugin_insn_append(pc, &raw, sizeof(raw));
+ return tgt;
}
void translator_fake_ldb(DisasContextBase *db, vaddr pc, uint8_t insn8)
--
2.34.1
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [PATCH v2 04/33] accel/tcg: Reorg translator_ld*
2024-04-24 23:31 ` [PATCH v2 04/33] accel/tcg: Reorg translator_ld* Richard Henderson
@ 2024-05-06 22:47 ` Philippe Mathieu-Daudé
2024-05-07 16:49 ` Richard Henderson
0 siblings, 1 reply; 58+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-05-06 22:47 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
On 25/4/24 01:31, Richard Henderson wrote:
> Reorg translator_access into translator_ld, with a more
> memcpy-ish interface. If both pages are in ram, do not
> go through the caller's slow path.
>
> Assert that the access is within the two pages that we are
> prepared to protect, per TranslationBlock. Allow access
> prior to pc_first, so long as it is within the first page.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> accel/tcg/translator.c | 189 ++++++++++++++++++++++-------------------
> 1 file changed, 101 insertions(+), 88 deletions(-)
> uint64_t translator_ldq(CPUArchState *env, DisasContextBase *db, vaddr pc)
> {
> - uint64_t ret, plug;
> - void *p = translator_access(env, db, pc, sizeof(ret));
> + uint64_t raw, tgt;
>
> - if (p) {
> - plugin_insn_append(pc, p, sizeof(ret));
> - return ldq_p(p);
> + if (translator_ld(env, db, &raw, pc, sizeof(raw))) {
> + tgt = tswap64(raw);
> + } else {
> + tgt = cpu_ldl_code(env, pc);
cpu_ldq_code() ?
> + raw = tswap64(tgt);
> }
> - ret = cpu_ldq_code(env, pc);
> - plug = tswap64(ret);
> - plugin_insn_append(pc, &plug, sizeof(ret));
> - return ret;
> + plugin_insn_append(pc, &raw, sizeof(raw));
> + return tgt;
> }
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [PATCH v2 04/33] accel/tcg: Reorg translator_ld*
2024-05-06 22:47 ` Philippe Mathieu-Daudé
@ 2024-05-07 16:49 ` Richard Henderson
2024-05-08 8:18 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 58+ messages in thread
From: Richard Henderson @ 2024-05-07 16:49 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
On 5/6/24 15:47, Philippe Mathieu-Daudé wrote:
> On 25/4/24 01:31, Richard Henderson wrote:
>> Reorg translator_access into translator_ld, with a more
>> memcpy-ish interface. If both pages are in ram, do not
>> go through the caller's slow path.
>>
>> Assert that the access is within the two pages that we are
>> prepared to protect, per TranslationBlock. Allow access
>> prior to pc_first, so long as it is within the first page.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>> accel/tcg/translator.c | 189 ++++++++++++++++++++++-------------------
>> 1 file changed, 101 insertions(+), 88 deletions(-)
>
>
>> uint64_t translator_ldq(CPUArchState *env, DisasContextBase *db, vaddr pc)
>> {
>> - uint64_t ret, plug;
>> - void *p = translator_access(env, db, pc, sizeof(ret));
>> + uint64_t raw, tgt;
>> - if (p) {
>> - plugin_insn_append(pc, p, sizeof(ret));
>> - return ldq_p(p);
>> + if (translator_ld(env, db, &raw, pc, sizeof(raw))) {
>> + tgt = tswap64(raw);
>> + } else {
>> + tgt = cpu_ldl_code(env, pc);
>
> cpu_ldq_code() ?
Oops, yes indeed. Fixed.
r~
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [PATCH v2 04/33] accel/tcg: Reorg translator_ld*
2024-05-07 16:49 ` Richard Henderson
@ 2024-05-08 8:18 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 58+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-05-08 8:18 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
On 7/5/24 18:49, Richard Henderson wrote:
> On 5/6/24 15:47, Philippe Mathieu-Daudé wrote:
>> On 25/4/24 01:31, Richard Henderson wrote:
>>> Reorg translator_access into translator_ld, with a more
>>> memcpy-ish interface. If both pages are in ram, do not
>>> go through the caller's slow path.
>>>
>>> Assert that the access is within the two pages that we are
>>> prepared to protect, per TranslationBlock. Allow access
>>> prior to pc_first, so long as it is within the first page.
>>>
>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>> ---
>>> accel/tcg/translator.c | 189 ++++++++++++++++++++++-------------------
>>> 1 file changed, 101 insertions(+), 88 deletions(-)
>>
>>
>>> uint64_t translator_ldq(CPUArchState *env, DisasContextBase *db,
>>> vaddr pc)
>>> {
>>> - uint64_t ret, plug;
>>> - void *p = translator_access(env, db, pc, sizeof(ret));
>>> + uint64_t raw, tgt;
>>> - if (p) {
>>> - plugin_insn_append(pc, p, sizeof(ret));
>>> - return ldq_p(p);
>>> + if (translator_ld(env, db, &raw, pc, sizeof(raw))) {
>>> + tgt = tswap64(raw);
>>> + } else {
>>> + tgt = cpu_ldl_code(env, pc);
>>
>> cpu_ldq_code() ?
>
> Oops, yes indeed. Fixed.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 58+ messages in thread
* [PATCH v2 05/33] accel/tcg: Cap the translation block when we encounter mmio
2024-04-24 23:30 [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Richard Henderson
` (3 preceding siblings ...)
2024-04-24 23:31 ` [PATCH v2 04/33] accel/tcg: Reorg translator_ld* Richard Henderson
@ 2024-04-24 23:31 ` Richard Henderson
2024-04-24 23:31 ` [PATCH v2 06/33] accel/tcg: Record mmio bytes during translation Richard Henderson
` (28 subsequent siblings)
33 siblings, 0 replies; 58+ messages in thread
From: Richard Henderson @ 2024-04-24 23:31 UTC (permalink / raw)
To: qemu-devel; +Cc: Philippe Mathieu-Daudé
Do not allow translation to proceed beyond one insn with mmio,
as we will not be caching the TranslationBlock.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
accel/tcg/translator.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index dbd54e25a2..c3f4d0e252 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -239,6 +239,8 @@ static bool translator_ld(CPUArchState *env, DisasContextBase *db,
/* Use slow path if first page is MMIO. */
if (unlikely(tb_page_addr0(tb) == -1)) {
+ /* We capped translation with first page MMIO in tb_gen_code. */
+ tcg_debug_assert(db->max_insns == 1);
return false;
}
@@ -287,6 +289,8 @@ static bool translator_ld(CPUArchState *env, DisasContextBase *db,
if (unlikely(new_page1 == -1)) {
tb_unlock_pages(tb);
tb_set_page_addr0(tb, -1);
+ /* Require that this be the final insn. */
+ db->max_insns = db->num_insns;
return false;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 58+ messages in thread* [PATCH v2 06/33] accel/tcg: Record mmio bytes during translation
2024-04-24 23:30 [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Richard Henderson
` (4 preceding siblings ...)
2024-04-24 23:31 ` [PATCH v2 05/33] accel/tcg: Cap the translation block when we encounter mmio Richard Henderson
@ 2024-04-24 23:31 ` Richard Henderson
2024-04-29 10:34 ` Philippe Mathieu-Daudé
2024-04-24 23:31 ` [PATCH v2 07/33] accel/tcg: Record when translator_fake_ldb is used Richard Henderson
` (27 subsequent siblings)
33 siblings, 1 reply; 58+ messages in thread
From: Richard Henderson @ 2024-04-24 23:31 UTC (permalink / raw)
To: qemu-devel
This will be able to replace plugin_insn_append, and will
be usable for disassembly.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/translator.h | 12 ++++++++++++
accel/tcg/translator.c | 41 +++++++++++++++++++++++++++++++++++++++
2 files changed, 53 insertions(+)
diff --git a/include/exec/translator.h b/include/exec/translator.h
index 83fe66cba0..974cc4f9c4 100644
--- a/include/exec/translator.h
+++ b/include/exec/translator.h
@@ -90,6 +90,18 @@ typedef struct DisasContextBase {
bool plugin_enabled;
struct TCGOp *insn_start;
void *host_addr[2];
+
+ /*
+ * Record insn data that we cannot read directly from host memory.
+ * There are only two reasons we cannot use host memory:
+ * (1) We are executing from I/O,
+ * (2) We are executing a synthetic instruction (s390x EX).
+ * In both cases we need record exactly one instruction,
+ * and thus the maximum amount of data we record is limited.
+ */
+ int record_start;
+ int record_len;
+ uint8_t record[32];
} DisasContextBase;
/**
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index c3f4d0e252..a3c246ea37 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -131,6 +131,8 @@ void translator_loop(CPUState *cpu, TranslationBlock *tb, int *max_insns,
db->insn_start = NULL;
db->host_addr[0] = host_pc;
db->host_addr[1] = NULL;
+ db->record_start = 0;
+ db->record_len = 0;
ops->init_disas_context(db, cpu);
tcg_debug_assert(db->is_jmp == DISAS_NEXT); /* no early exit */
@@ -317,6 +319,39 @@ static bool translator_ld(CPUArchState *env, DisasContextBase *db,
return true;
}
+static void record_save(DisasContextBase *db, vaddr pc,
+ const void *from, int size)
+{
+ int offset;
+
+ /* Do not record probes before the start of TB. */
+ if (pc < db->pc_first) {
+ return;
+ }
+
+ /*
+ * In translator_access, we verified that pc is within 2 pages
+ * of pc_first, thus this will never overflow.
+ */
+ offset = pc - db->pc_first;
+
+ /*
+ * Either the first or second page may be I/O. If it is the second,
+ * then the first byte we need to record will be at a non-zero offset.
+ * In either case, we should not need to record but a single insn.
+ */
+ if (db->record_len == 0) {
+ db->record_start = offset;
+ db->record_len = size;
+ } else {
+ assert(offset == db->record_start + db->record_len);
+ assert(db->record_len + size <= sizeof(db->record));
+ db->record_len += size;
+ }
+
+ memcpy(db->record + (offset - db->record_start), from, size);
+}
+
static void plugin_insn_append(vaddr pc, const void *from, size_t size)
{
#ifdef CONFIG_PLUGIN
@@ -344,6 +379,7 @@ uint8_t translator_ldub(CPUArchState *env, DisasContextBase *db, vaddr pc)
if (!translator_ld(env, db, &raw, pc, sizeof(raw))) {
raw = cpu_ldub_code(env, pc);
+ record_save(db, pc, &raw, sizeof(raw));
}
plugin_insn_append(pc, &raw, sizeof(raw));
return raw;
@@ -358,6 +394,7 @@ uint16_t translator_lduw(CPUArchState *env, DisasContextBase *db, vaddr pc)
} else {
tgt = cpu_lduw_code(env, pc);
raw = tswap16(tgt);
+ record_save(db, pc, &raw, sizeof(raw));
}
plugin_insn_append(pc, &raw, sizeof(raw));
return tgt;
@@ -372,6 +409,7 @@ uint32_t translator_ldl(CPUArchState *env, DisasContextBase *db, vaddr pc)
} else {
tgt = cpu_ldl_code(env, pc);
raw = tswap32(tgt);
+ record_save(db, pc, &raw, sizeof(raw));
}
plugin_insn_append(pc, &raw, sizeof(raw));
return tgt;
@@ -386,6 +424,7 @@ uint64_t translator_ldq(CPUArchState *env, DisasContextBase *db, vaddr pc)
} else {
tgt = cpu_ldl_code(env, pc);
raw = tswap64(tgt);
+ record_save(db, pc, &raw, sizeof(raw));
}
plugin_insn_append(pc, &raw, sizeof(raw));
return tgt;
@@ -393,5 +432,7 @@ uint64_t translator_ldq(CPUArchState *env, DisasContextBase *db, vaddr pc)
void translator_fake_ldb(DisasContextBase *db, vaddr pc, uint8_t insn8)
{
+ assert(pc >= db->pc_first);
+ record_save(db, pc, &insn8, sizeof(insn8));
plugin_insn_append(pc, &insn8, sizeof(insn8));
}
--
2.34.1
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [PATCH v2 06/33] accel/tcg: Record mmio bytes during translation
2024-04-24 23:31 ` [PATCH v2 06/33] accel/tcg: Record mmio bytes during translation Richard Henderson
@ 2024-04-29 10:34 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 58+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-29 10:34 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
On 25/4/24 01:31, Richard Henderson wrote:
> This will be able to replace plugin_insn_append, and will
> be usable for disassembly.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> include/exec/translator.h | 12 ++++++++++++
> accel/tcg/translator.c | 41 +++++++++++++++++++++++++++++++++++++++
> 2 files changed, 53 insertions(+)
>
> diff --git a/include/exec/translator.h b/include/exec/translator.h
> index 83fe66cba0..974cc4f9c4 100644
> --- a/include/exec/translator.h
> +++ b/include/exec/translator.h
> @@ -90,6 +90,18 @@ typedef struct DisasContextBase {
> bool plugin_enabled;
> struct TCGOp *insn_start;
> void *host_addr[2];
> +
> + /*
> + * Record insn data that we cannot read directly from host memory.
> + * There are only two reasons we cannot use host memory:
> + * (1) We are executing from I/O,
> + * (2) We are executing a synthetic instruction (s390x EX).
> + * In both cases we need record exactly one instruction,
> + * and thus the maximum amount of data we record is limited.
> + */
> + int record_start;
> + int record_len;
> + uint8_t record[32];
Alternatively (matter of style):
struct {
unsigned start;
unsigned len;
uint8_t data[32];
}
> } DisasContextBase;
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 58+ messages in thread
* [PATCH v2 07/33] accel/tcg: Record when translator_fake_ldb is used
2024-04-24 23:30 [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Richard Henderson
` (5 preceding siblings ...)
2024-04-24 23:31 ` [PATCH v2 06/33] accel/tcg: Record mmio bytes during translation Richard Henderson
@ 2024-04-24 23:31 ` Richard Henderson
2024-04-29 10:36 ` Philippe Mathieu-Daudé
2024-04-24 23:31 ` [PATCH v2 08/33] accel/tcg: Record DisasContextBase in tcg_ctx for plugins Richard Henderson
` (26 subsequent siblings)
33 siblings, 1 reply; 58+ messages in thread
From: Richard Henderson @ 2024-04-24 23:31 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/translator.h | 3 ++-
accel/tcg/translator.c | 2 ++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/include/exec/translator.h b/include/exec/translator.h
index 974cc4f9c4..e92dfba035 100644
--- a/include/exec/translator.h
+++ b/include/exec/translator.h
@@ -72,8 +72,8 @@ typedef enum DisasJumpType {
* @num_insns: Number of translated instructions (including current).
* @max_insns: Maximum number of instructions to be translated in this TB.
* @singlestep_enabled: "Hardware" single stepping enabled.
- * @saved_can_do_io: Known value of cpu->neg.can_do_io, or -1 for unknown.
* @plugin_enabled: TCG plugin enabled in this TB.
+ * @fake_insn: True if translator_fake_ldb used.
* @insn_start: The last op emitted by the insn_start hook,
* which is expected to be INDEX_op_insn_start.
*
@@ -88,6 +88,7 @@ typedef struct DisasContextBase {
int max_insns;
bool singlestep_enabled;
bool plugin_enabled;
+ bool fake_insn;
struct TCGOp *insn_start;
void *host_addr[2];
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index a3c246ea37..6863455ed9 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -129,6 +129,7 @@ void translator_loop(CPUState *cpu, TranslationBlock *tb, int *max_insns,
db->max_insns = *max_insns;
db->singlestep_enabled = cflags & CF_SINGLE_STEP;
db->insn_start = NULL;
+ db->fake_insn = false;
db->host_addr[0] = host_pc;
db->host_addr[1] = NULL;
db->record_start = 0;
@@ -433,6 +434,7 @@ uint64_t translator_ldq(CPUArchState *env, DisasContextBase *db, vaddr pc)
void translator_fake_ldb(DisasContextBase *db, vaddr pc, uint8_t insn8)
{
assert(pc >= db->pc_first);
+ db->fake_insn = true;
record_save(db, pc, &insn8, sizeof(insn8));
plugin_insn_append(pc, &insn8, sizeof(insn8));
}
--
2.34.1
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [PATCH v2 07/33] accel/tcg: Record when translator_fake_ldb is used
2024-04-24 23:31 ` [PATCH v2 07/33] accel/tcg: Record when translator_fake_ldb is used Richard Henderson
@ 2024-04-29 10:36 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 58+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-29 10:36 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
On 25/4/24 01:31, Richard Henderson wrote:
Remove left-over command from commit dcd092a063 ("accel/tcg: Improve
can_do_io management").
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> include/exec/translator.h | 3 ++-
> accel/tcg/translator.c | 2 ++
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/include/exec/translator.h b/include/exec/translator.h
> index 974cc4f9c4..e92dfba035 100644
> --- a/include/exec/translator.h
> +++ b/include/exec/translator.h
> @@ -72,8 +72,8 @@ typedef enum DisasJumpType {
> * @num_insns: Number of translated instructions (including current).
> * @max_insns: Maximum number of instructions to be translated in this TB.
> * @singlestep_enabled: "Hardware" single stepping enabled.
> - * @saved_can_do_io: Known value of cpu->neg.can_do_io, or -1 for unknown.
> * @plugin_enabled: TCG plugin enabled in this TB.
> + * @fake_insn: True if translator_fake_ldb used.
> * @insn_start: The last op emitted by the insn_start hook,
> * which is expected to be INDEX_op_insn_start.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 58+ messages in thread
* [PATCH v2 08/33] accel/tcg: Record DisasContextBase in tcg_ctx for plugins
2024-04-24 23:30 [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Richard Henderson
` (6 preceding siblings ...)
2024-04-24 23:31 ` [PATCH v2 07/33] accel/tcg: Record when translator_fake_ldb is used Richard Henderson
@ 2024-04-24 23:31 ` Richard Henderson
2024-05-03 15:11 ` Philippe Mathieu-Daudé
2024-04-24 23:31 ` [PATCH v2 09/33] plugins: Copy memory in qemu_plugin_insn_data Richard Henderson
` (25 subsequent siblings)
33 siblings, 1 reply; 58+ messages in thread
From: Richard Henderson @ 2024-04-24 23:31 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/tcg/tcg.h | 1 +
accel/tcg/plugin-gen.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
index 135e36d729..2a1c080bab 100644
--- a/include/tcg/tcg.h
+++ b/include/tcg/tcg.h
@@ -537,6 +537,7 @@ struct TCGContext {
* space for instructions (for variable-instruction-length ISAs).
*/
struct qemu_plugin_tb *plugin_tb;
+ const struct DisasContextBase *plugin_db;
/* descriptor of the instruction being translated */
struct qemu_plugin_insn *plugin_insn;
diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index 3db74ae9bf..94bbad6dc7 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -329,6 +329,7 @@ bool plugin_gen_tb_start(CPUState *cpu, const DisasContextBase *db,
tcg_gen_plugin_cb(PLUGIN_GEN_FROM_TB);
}
+ tcg_ctx->plugin_db = db;
tcg_ctx->plugin_insn = NULL;
return ret;
--
2.34.1
^ permalink raw reply related [flat|nested] 58+ messages in thread* [PATCH v2 09/33] plugins: Copy memory in qemu_plugin_insn_data
2024-04-24 23:30 [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Richard Henderson
` (7 preceding siblings ...)
2024-04-24 23:31 ` [PATCH v2 08/33] accel/tcg: Record DisasContextBase in tcg_ctx for plugins Richard Henderson
@ 2024-04-24 23:31 ` Richard Henderson
2024-04-29 10:40 ` Philippe Mathieu-Daudé
2024-04-24 23:31 ` [PATCH v2 10/33] accel/tcg: Implement translator_st Richard Henderson
` (24 subsequent siblings)
33 siblings, 1 reply; 58+ messages in thread
From: Richard Henderson @ 2024-04-24 23:31 UTC (permalink / raw)
To: qemu-devel
Instead of returning a host pointer, copy the data into
storage provided by the caller.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/qemu/qemu-plugin.h | 15 +++++++--------
contrib/plugins/execlog.c | 5 +++--
contrib/plugins/howvec.c | 4 ++--
plugins/api.c | 7 +++++--
4 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index 4fc6c3739b..5f36c2d1ac 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -61,7 +61,7 @@ typedef uint64_t qemu_plugin_id_t;
extern QEMU_PLUGIN_EXPORT int qemu_plugin_version;
-#define QEMU_PLUGIN_VERSION 2
+#define QEMU_PLUGIN_VERSION 3
/**
* struct qemu_info_t - system information for plugins
@@ -394,17 +394,16 @@ struct qemu_plugin_insn *
qemu_plugin_tb_get_insn(const struct qemu_plugin_tb *tb, size_t idx);
/**
- * qemu_plugin_insn_data() - return ptr to instruction data
+ * qemu_plugin_insn_data() - copy instruction data
* @insn: opaque instruction handle from qemu_plugin_tb_get_insn()
+ * @dest: destination into which data is copied
+ * @len: length of dest
*
- * Note: data is only valid for duration of callback. See
- * qemu_plugin_insn_size() to calculate size of stream.
- *
- * Returns: pointer to a stream of bytes containing the value of this
- * instructions opcode.
+ * Returns the number of bytes copied, minimum of @len and insn size.
*/
QEMU_PLUGIN_API
-const void *qemu_plugin_insn_data(const struct qemu_plugin_insn *insn);
+size_t qemu_plugin_insn_data(const struct qemu_plugin_insn *insn,
+ void *dest, size_t len);
/**
* qemu_plugin_insn_size() - return size of instruction
diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c
index fab18113d4..371db97eb1 100644
--- a/contrib/plugins/execlog.c
+++ b/contrib/plugins/execlog.c
@@ -258,8 +258,9 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
NULL);
}
} else {
- uint32_t insn_opcode;
- insn_opcode = *((uint32_t *)qemu_plugin_insn_data(insn));
+ uint32_t insn_opcode = 0;
+ qemu_plugin_insn_data(insn, &insn_opcode, sizeof(insn_opcode));
+
char *output = g_strdup_printf("0x%"PRIx64", 0x%"PRIx32", \"%s\"",
insn_vaddr, insn_opcode, insn_disas);
diff --git a/contrib/plugins/howvec.c b/contrib/plugins/howvec.c
index 94bbc53820..9be67f7453 100644
--- a/contrib/plugins/howvec.c
+++ b/contrib/plugins/howvec.c
@@ -252,7 +252,7 @@ static struct qemu_plugin_scoreboard *find_counter(
{
int i;
uint64_t *cnt = NULL;
- uint32_t opcode;
+ uint32_t opcode = 0;
InsnClassExecCount *class = NULL;
/*
@@ -261,7 +261,7 @@ static struct qemu_plugin_scoreboard *find_counter(
* They would probably benefit from a more tailored plugin.
* However we can fall back to individual instruction counting.
*/
- opcode = *((uint32_t *)qemu_plugin_insn_data(insn));
+ qemu_plugin_insn_data(insn, &opcode, sizeof(opcode));
for (i = 0; !cnt && i < class_table_sz; i++) {
class = &class_table[i];
diff --git a/plugins/api.c b/plugins/api.c
index 3912c9cc8f..4e9125ea29 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -216,9 +216,12 @@ qemu_plugin_tb_get_insn(const struct qemu_plugin_tb *tb, size_t idx)
* instruction being translated.
*/
-const void *qemu_plugin_insn_data(const struct qemu_plugin_insn *insn)
+size_t qemu_plugin_insn_data(const struct qemu_plugin_insn *insn,
+ void *dest, size_t len)
{
- return insn->data->data;
+ len = MIN(len, insn->data->len);
+ memcpy(dest, insn->data->data, len);
+ return len;
}
size_t qemu_plugin_insn_size(const struct qemu_plugin_insn *insn)
--
2.34.1
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [PATCH v2 09/33] plugins: Copy memory in qemu_plugin_insn_data
2024-04-24 23:31 ` [PATCH v2 09/33] plugins: Copy memory in qemu_plugin_insn_data Richard Henderson
@ 2024-04-29 10:40 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 58+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-29 10:40 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
On 25/4/24 01:31, Richard Henderson wrote:
> Instead of returning a host pointer, copy the data into
> storage provided by the caller.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> include/qemu/qemu-plugin.h | 15 +++++++--------
> contrib/plugins/execlog.c | 5 +++--
> contrib/plugins/howvec.c | 4 ++--
> plugins/api.c | 7 +++++--
> 4 files changed, 17 insertions(+), 14 deletions(-)
> /**
> * struct qemu_info_t - system information for plugins
> @@ -394,17 +394,16 @@ struct qemu_plugin_insn *
> qemu_plugin_tb_get_insn(const struct qemu_plugin_tb *tb, size_t idx);
>
> /**
> - * qemu_plugin_insn_data() - return ptr to instruction data
> + * qemu_plugin_insn_data() - copy instruction data
"copy of "?
> * @insn: opaque instruction handle from qemu_plugin_tb_get_insn()
> + * @dest: destination into which data is copied
> + * @len: length of dest
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 58+ messages in thread
* [PATCH v2 10/33] accel/tcg: Implement translator_st
2024-04-24 23:30 [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Richard Henderson
` (8 preceding siblings ...)
2024-04-24 23:31 ` [PATCH v2 09/33] plugins: Copy memory in qemu_plugin_insn_data Richard Henderson
@ 2024-04-24 23:31 ` Richard Henderson
2024-05-08 15:20 ` Philippe Mathieu-Daudé
2024-04-24 23:31 ` [PATCH v2 11/33] plugins: Use translator_st for qemu_plugin_insn_data Richard Henderson
` (23 subsequent siblings)
33 siblings, 1 reply; 58+ messages in thread
From: Richard Henderson @ 2024-04-24 23:31 UTC (permalink / raw)
To: qemu-devel
Copy data out of a completed translation. This will be used
for both plugins and disassembly.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/translator.h | 23 ++++++++++++++++
accel/tcg/translator.c | 55 +++++++++++++++++++++++++++++++++++++++
2 files changed, 78 insertions(+)
diff --git a/include/exec/translator.h b/include/exec/translator.h
index e92dfba035..3c354a4310 100644
--- a/include/exec/translator.h
+++ b/include/exec/translator.h
@@ -246,6 +246,29 @@ translator_ldq_swap(CPUArchState *env, DisasContextBase *db,
*/
void translator_fake_ldb(DisasContextBase *db, vaddr pc, uint8_t insn8);
+/**
+ * translator_st
+ * @db: disassembly context
+ * @dest: address to copy into
+ * @addr: virtual address within TB
+ * @len: length
+ *
+ * Copy @len bytes from @addr into @dest.
+ * All bytes must have been read during translation.
+ * Return true on success or false on failure.
+ */
+bool translator_st(const DisasContextBase *db, void *dest,
+ vaddr addr, size_t len);
+
+/**
+ * translator_st_len
+ * @db: disassembly context
+ *
+ * Return the number of bytes available to copy from the
+ * current translation block with translator_st.
+ */
+size_t translator_st_len(const DisasContextBase *db);
+
#ifdef NEED_CPU_H
/*
* Return whether addr is on the same page as where disassembly started.
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index 6863455ed9..7f63a8085d 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -353,6 +353,61 @@ static void record_save(DisasContextBase *db, vaddr pc,
memcpy(db->record + (offset - db->record_start), from, size);
}
+size_t translator_st_len(const DisasContextBase *db)
+{
+ return db->fake_insn ? db->record_len : db->tb->size;
+}
+
+bool translator_st(const DisasContextBase *db, void *dest,
+ vaddr addr, size_t len)
+{
+ size_t offset, offset_end;
+
+ if (addr < db->pc_first) {
+ return false;
+ }
+ offset = addr - db->pc_first;
+ offset_end = offset + len;
+ if (offset_end > translator_st_len(db)) {
+ return false;
+ }
+
+ if (!db->fake_insn) {
+ size_t offset_page1 = -(db->pc_first | TARGET_PAGE_MASK);
+
+ /* Get all the bytes from the first page. */
+ if (db->host_addr[0]) {
+ if (offset_end <= offset_page1) {
+ memcpy(dest, db->host_addr[0] + offset, len);
+ return true;
+ }
+ if (offset < offset_page1) {
+ size_t len0 = offset_page1 - offset;
+ memcpy(dest, db->host_addr[0] + offset, len0);
+ offset += len0;
+ dest += len0;
+ }
+ }
+
+ /* Get any bytes from the second page. */
+ if (db->host_addr[1] && offset >= offset_page1) {
+ memcpy(dest, db->host_addr[1] + (offset - offset_page1),
+ offset_end - offset);
+ return true;
+ }
+ }
+
+ /* Else get recorded bytes. */
+ if (db->record_len != 0 &&
+ offset >= db->record_start &&
+ offset_end <= db->record_start + db->record_len) {
+ memcpy(dest, db->record + (offset - db->record_start),
+ offset_end - offset);
+ return true;
+ }
+ return false;
+}
+
static void plugin_insn_append(vaddr pc, const void *from, size_t size)
{
#ifdef CONFIG_PLUGIN
--
2.34.1
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [PATCH v2 10/33] accel/tcg: Implement translator_st
2024-04-24 23:31 ` [PATCH v2 10/33] accel/tcg: Implement translator_st Richard Henderson
@ 2024-05-08 15:20 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 58+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-05-08 15:20 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
On 25/4/24 01:31, Richard Henderson wrote:
> Copy data out of a completed translation. This will be used
> for both plugins and disassembly.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> include/exec/translator.h | 23 ++++++++++++++++
> accel/tcg/translator.c | 55 +++++++++++++++++++++++++++++++++++++++
> 2 files changed, 78 insertions(+)
>
> diff --git a/include/exec/translator.h b/include/exec/translator.h
> index e92dfba035..3c354a4310 100644
> --- a/include/exec/translator.h
> +++ b/include/exec/translator.h
> @@ -246,6 +246,29 @@ translator_ldq_swap(CPUArchState *env, DisasContextBase *db,
> */
> void translator_fake_ldb(DisasContextBase *db, vaddr pc, uint8_t insn8);
>
> +/**
> + * translator_st
> + * @db: disassembly context
> + * @dest: address to copy into
Or s/address/buffer/
> + * @addr: virtual address within TB
> + * @len: length
> + *
> + * Copy @len bytes from @addr into @dest.
> + * All bytes must have been read during translation.
> + * Return true on success or false on failure.
> + */
> +bool translator_st(const DisasContextBase *db, void *dest,
> + vaddr addr, size_t len);
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 58+ messages in thread
* [PATCH v2 11/33] plugins: Use translator_st for qemu_plugin_insn_data
2024-04-24 23:30 [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Richard Henderson
` (9 preceding siblings ...)
2024-04-24 23:31 ` [PATCH v2 10/33] accel/tcg: Implement translator_st Richard Henderson
@ 2024-04-24 23:31 ` Richard Henderson
2024-04-29 10:44 ` Philippe Mathieu-Daudé
2024-04-24 23:31 ` [PATCH v2 12/33] plugins: Read mem_only directly from TB cflags Richard Henderson
` (22 subsequent siblings)
33 siblings, 1 reply; 58+ messages in thread
From: Richard Henderson @ 2024-04-24 23:31 UTC (permalink / raw)
To: qemu-devel
Use the bytes that we record for the entire TB, rather than
a per-insn GByteArray. Record the length of the insn in
plugin_gen_insn_end rather than infering from the length
of the array.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/qemu/plugin.h | 14 +-------------
accel/tcg/plugin-gen.c | 7 +++++--
accel/tcg/translator.c | 26 --------------------------
plugins/api.c | 12 +++++++-----
tcg/tcg.c | 3 +--
5 files changed, 14 insertions(+), 48 deletions(-)
diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
index 07b1755990..c32bb97667 100644
--- a/include/qemu/plugin.h
+++ b/include/qemu/plugin.h
@@ -97,11 +97,11 @@ struct qemu_plugin_dyn_cb {
/* Internal context for instrumenting an instruction */
struct qemu_plugin_insn {
- GByteArray *data;
uint64_t vaddr;
void *haddr;
GArray *insn_cbs;
GArray *mem_cbs;
+ uint8_t len;
bool calls_helpers;
/* if set, the instruction calls helpers that might access guest memory */
@@ -116,18 +116,6 @@ struct qemu_plugin_scoreboard {
QLIST_ENTRY(qemu_plugin_scoreboard) entry;
};
-/*
- * qemu_plugin_insn allocate and cleanup functions. We don't expect to
- * cleanup many of these structures. They are reused for each fresh
- * translation.
- */
-
-static inline void qemu_plugin_insn_cleanup_fn(gpointer data)
-{
- struct qemu_plugin_insn *insn = (struct qemu_plugin_insn *) data;
- g_byte_array_free(insn->data, true);
-}
-
/* Internal context for this TranslationBlock */
struct qemu_plugin_tb {
GPtrArray *insns;
diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index 94bbad6dc7..be2451be58 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -346,11 +346,9 @@ void plugin_gen_insn_start(CPUState *cpu, const DisasContextBase *db)
ptb->n = n;
if (n <= ptb->insns->len) {
insn = g_ptr_array_index(ptb->insns, n - 1);
- g_byte_array_set_size(insn->data, 0);
} else {
assert(n - 1 == ptb->insns->len);
insn = g_new0(struct qemu_plugin_insn, 1);
- insn->data = g_byte_array_sized_new(4);
g_ptr_array_add(ptb->insns, insn);
}
@@ -389,6 +387,11 @@ void plugin_gen_insn_start(CPUState *cpu, const DisasContextBase *db)
void plugin_gen_insn_end(void)
{
+ const DisasContextBase *db = tcg_ctx->plugin_db;
+ struct qemu_plugin_insn *pinsn = tcg_ctx->plugin_insn;
+
+ pinsn->len = db->fake_insn ? db->record_len : db->pc_next - pinsn->vaddr;
+
tcg_gen_plugin_cb(PLUGIN_GEN_AFTER_INSN);
}
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index 7f63a8085d..df73312f99 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -408,27 +408,6 @@ bool translator_st(const DisasContextBase *db, void *dest,
return false;
}
-static void plugin_insn_append(vaddr pc, const void *from, size_t size)
-{
-#ifdef CONFIG_PLUGIN
- struct qemu_plugin_insn *insn = tcg_ctx->plugin_insn;
- size_t off;
-
- if (insn == NULL) {
- return;
- }
- off = pc - insn->vaddr;
- if (off < insn->data->len) {
- g_byte_array_set_size(insn->data, off);
- } else if (off > insn->data->len) {
- /* we have an unexpected gap */
- g_assert_not_reached();
- }
-
- insn->data = g_byte_array_append(insn->data, from, size);
-#endif
-}
-
uint8_t translator_ldub(CPUArchState *env, DisasContextBase *db, vaddr pc)
{
uint8_t raw;
@@ -437,7 +416,6 @@ uint8_t translator_ldub(CPUArchState *env, DisasContextBase *db, vaddr pc)
raw = cpu_ldub_code(env, pc);
record_save(db, pc, &raw, sizeof(raw));
}
- plugin_insn_append(pc, &raw, sizeof(raw));
return raw;
}
@@ -452,7 +430,6 @@ uint16_t translator_lduw(CPUArchState *env, DisasContextBase *db, vaddr pc)
raw = tswap16(tgt);
record_save(db, pc, &raw, sizeof(raw));
}
- plugin_insn_append(pc, &raw, sizeof(raw));
return tgt;
}
@@ -467,7 +444,6 @@ uint32_t translator_ldl(CPUArchState *env, DisasContextBase *db, vaddr pc)
raw = tswap32(tgt);
record_save(db, pc, &raw, sizeof(raw));
}
- plugin_insn_append(pc, &raw, sizeof(raw));
return tgt;
}
@@ -482,7 +458,6 @@ uint64_t translator_ldq(CPUArchState *env, DisasContextBase *db, vaddr pc)
raw = tswap64(tgt);
record_save(db, pc, &raw, sizeof(raw));
}
- plugin_insn_append(pc, &raw, sizeof(raw));
return tgt;
}
@@ -491,5 +466,4 @@ void translator_fake_ldb(DisasContextBase *db, vaddr pc, uint8_t insn8)
assert(pc >= db->pc_first);
db->fake_insn = true;
record_save(db, pc, &insn8, sizeof(insn8));
- plugin_insn_append(pc, &insn8, sizeof(insn8));
}
diff --git a/plugins/api.c b/plugins/api.c
index 4e9125ea29..7b8b7523b3 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -43,6 +43,7 @@
#include "exec/exec-all.h"
#include "exec/gdbstub.h"
#include "exec/ram_addr.h"
+#include "exec/translator.h"
#include "disas/disas.h"
#include "plugin.h"
#ifndef CONFIG_USER_ONLY
@@ -219,14 +220,15 @@ qemu_plugin_tb_get_insn(const struct qemu_plugin_tb *tb, size_t idx)
size_t qemu_plugin_insn_data(const struct qemu_plugin_insn *insn,
void *dest, size_t len)
{
- len = MIN(len, insn->data->len);
- memcpy(dest, insn->data->data, len);
- return len;
+ const DisasContextBase *db = tcg_ctx->plugin_db;
+
+ len = MIN(len, insn->len);
+ return translator_st(db, dest, insn->vaddr, len) ? len : 0;
}
size_t qemu_plugin_insn_size(const struct qemu_plugin_insn *insn)
{
- return insn->data->len;
+ return insn->len;
}
uint64_t qemu_plugin_insn_vaddr(const struct qemu_plugin_insn *insn)
@@ -242,7 +244,7 @@ void *qemu_plugin_insn_haddr(const struct qemu_plugin_insn *insn)
char *qemu_plugin_insn_disas(const struct qemu_plugin_insn *insn)
{
CPUState *cpu = current_cpu;
- return plugin_disas(cpu, insn->vaddr, insn->data->len);
+ return plugin_disas(cpu, insn->vaddr, insn->len);
}
const char *qemu_plugin_insn_symbol(const struct qemu_plugin_insn *insn)
diff --git a/tcg/tcg.c b/tcg/tcg.c
index d248c52e96..691b2342a2 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -765,8 +765,7 @@ static void alloc_tcg_plugin_context(TCGContext *s)
{
#ifdef CONFIG_PLUGIN
s->plugin_tb = g_new0(struct qemu_plugin_tb, 1);
- s->plugin_tb->insns =
- g_ptr_array_new_with_free_func(qemu_plugin_insn_cleanup_fn);
+ s->plugin_tb->insns = g_ptr_array_new();
#endif
}
--
2.34.1
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [PATCH v2 11/33] plugins: Use translator_st for qemu_plugin_insn_data
2024-04-24 23:31 ` [PATCH v2 11/33] plugins: Use translator_st for qemu_plugin_insn_data Richard Henderson
@ 2024-04-29 10:44 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 58+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-29 10:44 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
On 25/4/24 01:31, Richard Henderson wrote:
> Use the bytes that we record for the entire TB, rather than
> a per-insn GByteArray. Record the length of the insn in
> plugin_gen_insn_end rather than infering from the length
> of the array.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> include/qemu/plugin.h | 14 +-------------
> accel/tcg/plugin-gen.c | 7 +++++--
> accel/tcg/translator.c | 26 --------------------------
> plugins/api.c | 12 +++++++-----
> tcg/tcg.c | 3 +--
> 5 files changed, 14 insertions(+), 48 deletions(-)
Nice!
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 58+ messages in thread
* [PATCH v2 12/33] plugins: Read mem_only directly from TB cflags
2024-04-24 23:30 [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Richard Henderson
` (10 preceding siblings ...)
2024-04-24 23:31 ` [PATCH v2 11/33] plugins: Use translator_st for qemu_plugin_insn_data Richard Henderson
@ 2024-04-24 23:31 ` Richard Henderson
2024-04-29 10:45 ` Philippe Mathieu-Daudé
2024-04-24 23:31 ` [PATCH v2 13/33] plugins: Use DisasContextBase for qemu_plugin_insn_haddr Richard Henderson
` (21 subsequent siblings)
33 siblings, 1 reply; 58+ messages in thread
From: Richard Henderson @ 2024-04-24 23:31 UTC (permalink / raw)
To: qemu-devel
Do not pass around a boolean between multiple structures,
just read it from the TranslationBlock in the TCGContext.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/plugin-gen.h | 7 +++----
include/qemu/plugin.h | 3 ---
accel/tcg/plugin-gen.c | 4 +---
accel/tcg/translator.c | 2 +-
plugins/api.c | 14 +++++++++-----
5 files changed, 14 insertions(+), 16 deletions(-)
diff --git a/include/exec/plugin-gen.h b/include/exec/plugin-gen.h
index f333f33198..cbb2ca2131 100644
--- a/include/exec/plugin-gen.h
+++ b/include/exec/plugin-gen.h
@@ -18,8 +18,7 @@ struct DisasContextBase;
#ifdef CONFIG_PLUGIN
-bool plugin_gen_tb_start(CPUState *cpu, const struct DisasContextBase *db,
- bool supress);
+bool plugin_gen_tb_start(CPUState *cpu, const struct DisasContextBase *db);
void plugin_gen_tb_end(CPUState *cpu, size_t num_insns);
void plugin_gen_insn_start(CPUState *cpu, const struct DisasContextBase *db);
void plugin_gen_insn_end(void);
@@ -28,8 +27,8 @@ void plugin_gen_disable_mem_helpers(void);
#else /* !CONFIG_PLUGIN */
-static inline bool
-plugin_gen_tb_start(CPUState *cpu, const struct DisasContextBase *db, bool sup)
+static inline
+bool plugin_gen_tb_start(CPUState *cpu, const struct DisasContextBase *db)
{
return false;
}
diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
index c32bb97667..03081be543 100644
--- a/include/qemu/plugin.h
+++ b/include/qemu/plugin.h
@@ -106,8 +106,6 @@ struct qemu_plugin_insn {
/* if set, the instruction calls helpers that might access guest memory */
bool mem_helper;
-
- bool mem_only;
};
/* A scoreboard is an array of values, indexed by vcpu_index */
@@ -124,7 +122,6 @@ struct qemu_plugin_tb {
uint64_t vaddr2;
void *haddr1;
void *haddr2;
- bool mem_only;
/* if set, the TB calls helpers that might access guest memory */
bool mem_helper;
diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index be2451be58..a4656859c6 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -303,8 +303,7 @@ static void plugin_gen_inject(struct qemu_plugin_tb *plugin_tb)
}
}
-bool plugin_gen_tb_start(CPUState *cpu, const DisasContextBase *db,
- bool mem_only)
+bool plugin_gen_tb_start(CPUState *cpu, const DisasContextBase *db)
{
bool ret = false;
@@ -323,7 +322,6 @@ bool plugin_gen_tb_start(CPUState *cpu, const DisasContextBase *db,
ptb->vaddr2 = -1;
ptb->haddr1 = db->host_addr[0];
ptb->haddr2 = NULL;
- ptb->mem_only = mem_only;
ptb->mem_helper = false;
tcg_gen_plugin_cb(PLUGIN_GEN_FROM_TB);
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index df73312f99..5a059ce83e 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -143,7 +143,7 @@ void translator_loop(CPUState *cpu, TranslationBlock *tb, int *max_insns,
ops->tb_start(db, cpu);
tcg_debug_assert(db->is_jmp == DISAS_NEXT); /* no early exit */
- plugin_enabled = plugin_gen_tb_start(cpu, db, cflags & CF_MEMI_ONLY);
+ plugin_enabled = plugin_gen_tb_start(cpu, db);
db->plugin_enabled = plugin_enabled;
while (true) {
diff --git a/plugins/api.c b/plugins/api.c
index 7b8b7523b3..39895a1cb1 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -87,12 +87,17 @@ void qemu_plugin_register_vcpu_exit_cb(qemu_plugin_id_t id,
plugin_register_cb(id, QEMU_PLUGIN_EV_VCPU_EXIT, cb);
}
+static bool tb_is_mem_only(void)
+{
+ return tb_cflags(tcg_ctx->gen_tb) & CF_MEMI_ONLY;
+}
+
void qemu_plugin_register_vcpu_tb_exec_cb(struct qemu_plugin_tb *tb,
qemu_plugin_vcpu_udata_cb_t cb,
enum qemu_plugin_cb_flags flags,
void *udata)
{
- if (!tb->mem_only) {
+ if (!tb_is_mem_only()) {
plugin_register_dyn_cb__udata(&tb->cbs, cb, flags, udata);
}
}
@@ -103,7 +108,7 @@ void qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu(
qemu_plugin_u64 entry,
uint64_t imm)
{
- if (!tb->mem_only) {
+ if (!tb_is_mem_only()) {
plugin_register_inline_op_on_entry(&tb->cbs, 0, op, entry, imm);
}
}
@@ -113,7 +118,7 @@ void qemu_plugin_register_vcpu_insn_exec_cb(struct qemu_plugin_insn *insn,
enum qemu_plugin_cb_flags flags,
void *udata)
{
- if (!insn->mem_only) {
+ if (!tb_is_mem_only()) {
plugin_register_dyn_cb__udata(&insn->insn_cbs, cb, flags, udata);
}
}
@@ -124,7 +129,7 @@ void qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(
qemu_plugin_u64 entry,
uint64_t imm)
{
- if (!insn->mem_only) {
+ if (!tb_is_mem_only()) {
plugin_register_inline_op_on_entry(&insn->insn_cbs, 0, op, entry, imm);
}
}
@@ -206,7 +211,6 @@ qemu_plugin_tb_get_insn(const struct qemu_plugin_tb *tb, size_t idx)
return NULL;
}
insn = g_ptr_array_index(tb->insns, idx);
- insn->mem_only = tb->mem_only;
return insn;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [PATCH v2 12/33] plugins: Read mem_only directly from TB cflags
2024-04-24 23:31 ` [PATCH v2 12/33] plugins: Read mem_only directly from TB cflags Richard Henderson
@ 2024-04-29 10:45 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 58+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-29 10:45 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
On 25/4/24 01:31, Richard Henderson wrote:
> Do not pass around a boolean between multiple structures,
> just read it from the TranslationBlock in the TCGContext.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> include/exec/plugin-gen.h | 7 +++----
> include/qemu/plugin.h | 3 ---
> accel/tcg/plugin-gen.c | 4 +---
> accel/tcg/translator.c | 2 +-
> plugins/api.c | 14 +++++++++-----
> 5 files changed, 14 insertions(+), 16 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 58+ messages in thread
* [PATCH v2 13/33] plugins: Use DisasContextBase for qemu_plugin_insn_haddr
2024-04-24 23:30 [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Richard Henderson
` (11 preceding siblings ...)
2024-04-24 23:31 ` [PATCH v2 12/33] plugins: Read mem_only directly from TB cflags Richard Henderson
@ 2024-04-24 23:31 ` Richard Henderson
2024-05-13 14:18 ` Philippe Mathieu-Daudé
2024-05-13 18:16 ` Pierrick Bouvier
2024-04-24 23:31 ` [PATCH v2 14/33] plugins: Use DisasContextBase for qemu_plugin_tb_vaddr Richard Henderson
` (20 subsequent siblings)
33 siblings, 2 replies; 58+ messages in thread
From: Richard Henderson @ 2024-04-24 23:31 UTC (permalink / raw)
To: qemu-devel
We can delay the computation of haddr until the plugin
actually requests it.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/qemu/plugin.h | 4 ----
accel/tcg/plugin-gen.c | 20 --------------------
plugins/api.c | 25 ++++++++++++++++++++++++-
3 files changed, 24 insertions(+), 25 deletions(-)
diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
index 03081be543..3db0e75d16 100644
--- a/include/qemu/plugin.h
+++ b/include/qemu/plugin.h
@@ -98,7 +98,6 @@ struct qemu_plugin_dyn_cb {
/* Internal context for instrumenting an instruction */
struct qemu_plugin_insn {
uint64_t vaddr;
- void *haddr;
GArray *insn_cbs;
GArray *mem_cbs;
uint8_t len;
@@ -119,9 +118,6 @@ struct qemu_plugin_tb {
GPtrArray *insns;
size_t n;
uint64_t vaddr;
- uint64_t vaddr2;
- void *haddr1;
- void *haddr2;
/* if set, the TB calls helpers that might access guest memory */
bool mem_helper;
diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index a4656859c6..b036773d3c 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -319,9 +319,6 @@ bool plugin_gen_tb_start(CPUState *cpu, const DisasContextBase *db)
ret = true;
ptb->vaddr = db->pc_first;
- ptb->vaddr2 = -1;
- ptb->haddr1 = db->host_addr[0];
- ptb->haddr2 = NULL;
ptb->mem_helper = false;
tcg_gen_plugin_cb(PLUGIN_GEN_FROM_TB);
@@ -363,23 +360,6 @@ void plugin_gen_insn_start(CPUState *cpu, const DisasContextBase *db)
pc = db->pc_next;
insn->vaddr = pc;
- /*
- * Detect page crossing to get the new host address.
- * Note that we skip this when haddr1 == NULL, e.g. when we're
- * fetching instructions from a region not backed by RAM.
- */
- if (ptb->haddr1 == NULL) {
- insn->haddr = NULL;
- } else if (is_same_page(db, db->pc_next)) {
- insn->haddr = ptb->haddr1 + pc - ptb->vaddr;
- } else {
- if (ptb->vaddr2 == -1) {
- ptb->vaddr2 = TARGET_PAGE_ALIGN(db->pc_first);
- get_page_addr_code_hostp(cpu_env(cpu), ptb->vaddr2, &ptb->haddr2);
- }
- insn->haddr = ptb->haddr2 + pc - ptb->vaddr2;
- }
-
tcg_gen_plugin_cb(PLUGIN_GEN_FROM_INSN);
}
diff --git a/plugins/api.c b/plugins/api.c
index 39895a1cb1..4b6690c7d6 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -242,7 +242,30 @@ uint64_t qemu_plugin_insn_vaddr(const struct qemu_plugin_insn *insn)
void *qemu_plugin_insn_haddr(const struct qemu_plugin_insn *insn)
{
- return insn->haddr;
+ const DisasContextBase *db = tcg_ctx->plugin_db;
+ vaddr page0_last = db->pc_first | ~TARGET_PAGE_MASK;
+
+ if (db->fake_insn) {
+ return NULL;
+ }
+
+ /*
+ * ??? The return value is not intended for use of host memory,
+ * but as a proxy for address space and physical address.
+ * Thus we are only interested in the first byte and do not
+ * care about spanning pages.
+ */
+ if (insn->vaddr <= page0_last) {
+ if (db->host_addr[0] == NULL) {
+ return NULL;
+ }
+ return db->host_addr[0] + insn->vaddr - db->pc_first;
+ } else {
+ if (db->host_addr[1] == NULL) {
+ return NULL;
+ }
+ return db->host_addr[1] + insn->vaddr - (page0_last + 1);
+ }
}
char *qemu_plugin_insn_disas(const struct qemu_plugin_insn *insn)
--
2.34.1
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [PATCH v2 13/33] plugins: Use DisasContextBase for qemu_plugin_insn_haddr
2024-04-24 23:31 ` [PATCH v2 13/33] plugins: Use DisasContextBase for qemu_plugin_insn_haddr Richard Henderson
@ 2024-05-13 14:18 ` Philippe Mathieu-Daudé
2024-05-13 18:16 ` Pierrick Bouvier
1 sibling, 0 replies; 58+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-05-13 14:18 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: Pierrick Bouvier, Alex Bennée
Cc'ing Pierrick & Alex on this last one :)
On 25/4/24 01:31, Richard Henderson wrote:
> We can delay the computation of haddr until the plugin
> actually requests it.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> include/qemu/plugin.h | 4 ----
> accel/tcg/plugin-gen.c | 20 --------------------
> plugins/api.c | 25 ++++++++++++++++++++++++-
> 3 files changed, 24 insertions(+), 25 deletions(-)
>
> diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
> index 03081be543..3db0e75d16 100644
> --- a/include/qemu/plugin.h
> +++ b/include/qemu/plugin.h
> @@ -98,7 +98,6 @@ struct qemu_plugin_dyn_cb {
> /* Internal context for instrumenting an instruction */
> struct qemu_plugin_insn {
> uint64_t vaddr;
> - void *haddr;
> GArray *insn_cbs;
> GArray *mem_cbs;
> uint8_t len;
> @@ -119,9 +118,6 @@ struct qemu_plugin_tb {
> GPtrArray *insns;
> size_t n;
> uint64_t vaddr;
> - uint64_t vaddr2;
> - void *haddr1;
> - void *haddr2;
>
> /* if set, the TB calls helpers that might access guest memory */
> bool mem_helper;
> diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
> index a4656859c6..b036773d3c 100644
> --- a/accel/tcg/plugin-gen.c
> +++ b/accel/tcg/plugin-gen.c
> @@ -319,9 +319,6 @@ bool plugin_gen_tb_start(CPUState *cpu, const DisasContextBase *db)
> ret = true;
>
> ptb->vaddr = db->pc_first;
> - ptb->vaddr2 = -1;
> - ptb->haddr1 = db->host_addr[0];
> - ptb->haddr2 = NULL;
> ptb->mem_helper = false;
>
> tcg_gen_plugin_cb(PLUGIN_GEN_FROM_TB);
> @@ -363,23 +360,6 @@ void plugin_gen_insn_start(CPUState *cpu, const DisasContextBase *db)
> pc = db->pc_next;
> insn->vaddr = pc;
>
> - /*
> - * Detect page crossing to get the new host address.
> - * Note that we skip this when haddr1 == NULL, e.g. when we're
> - * fetching instructions from a region not backed by RAM.
> - */
> - if (ptb->haddr1 == NULL) {
> - insn->haddr = NULL;
> - } else if (is_same_page(db, db->pc_next)) {
> - insn->haddr = ptb->haddr1 + pc - ptb->vaddr;
> - } else {
> - if (ptb->vaddr2 == -1) {
> - ptb->vaddr2 = TARGET_PAGE_ALIGN(db->pc_first);
> - get_page_addr_code_hostp(cpu_env(cpu), ptb->vaddr2, &ptb->haddr2);
> - }
> - insn->haddr = ptb->haddr2 + pc - ptb->vaddr2;
> - }
> -
> tcg_gen_plugin_cb(PLUGIN_GEN_FROM_INSN);
> }
>
> diff --git a/plugins/api.c b/plugins/api.c
> index 39895a1cb1..4b6690c7d6 100644
> --- a/plugins/api.c
> +++ b/plugins/api.c
> @@ -242,7 +242,30 @@ uint64_t qemu_plugin_insn_vaddr(const struct qemu_plugin_insn *insn)
>
> void *qemu_plugin_insn_haddr(const struct qemu_plugin_insn *insn)
> {
> - return insn->haddr;
> + const DisasContextBase *db = tcg_ctx->plugin_db;
> + vaddr page0_last = db->pc_first | ~TARGET_PAGE_MASK;
> +
> + if (db->fake_insn) {
> + return NULL;
> + }
> +
> + /*
> + * ??? The return value is not intended for use of host memory,
> + * but as a proxy for address space and physical address.
> + * Thus we are only interested in the first byte and do not
> + * care about spanning pages.
> + */
> + if (insn->vaddr <= page0_last) {
> + if (db->host_addr[0] == NULL) {
> + return NULL;
> + }
> + return db->host_addr[0] + insn->vaddr - db->pc_first;
> + } else {
> + if (db->host_addr[1] == NULL) {
> + return NULL;
> + }
> + return db->host_addr[1] + insn->vaddr - (page0_last + 1);
> + }
> }
>
> char *qemu_plugin_insn_disas(const struct qemu_plugin_insn *insn)
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [PATCH v2 13/33] plugins: Use DisasContextBase for qemu_plugin_insn_haddr
2024-04-24 23:31 ` [PATCH v2 13/33] plugins: Use DisasContextBase for qemu_plugin_insn_haddr Richard Henderson
2024-05-13 14:18 ` Philippe Mathieu-Daudé
@ 2024-05-13 18:16 ` Pierrick Bouvier
1 sibling, 0 replies; 58+ messages in thread
From: Pierrick Bouvier @ 2024-05-13 18:16 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
On 4/24/24 16:31, Richard Henderson wrote:
> We can delay the computation of haddr until the plugin
> actually requests it.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> include/qemu/plugin.h | 4 ----
> accel/tcg/plugin-gen.c | 20 --------------------
> plugins/api.c | 25 ++++++++++++++++++++++++-
> 3 files changed, 24 insertions(+), 25 deletions(-)
>
> diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
> index 03081be543..3db0e75d16 100644
> --- a/include/qemu/plugin.h
> +++ b/include/qemu/plugin.h
> @@ -98,7 +98,6 @@ struct qemu_plugin_dyn_cb {
> /* Internal context for instrumenting an instruction */
> struct qemu_plugin_insn {
> uint64_t vaddr;
> - void *haddr;
> GArray *insn_cbs;
> GArray *mem_cbs;
> uint8_t len;
> @@ -119,9 +118,6 @@ struct qemu_plugin_tb {
> GPtrArray *insns;
> size_t n;
> uint64_t vaddr;
> - uint64_t vaddr2;
> - void *haddr1;
> - void *haddr2;
>
> /* if set, the TB calls helpers that might access guest memory */
> bool mem_helper;
> diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
> index a4656859c6..b036773d3c 100644
> --- a/accel/tcg/plugin-gen.c
> +++ b/accel/tcg/plugin-gen.c
> @@ -319,9 +319,6 @@ bool plugin_gen_tb_start(CPUState *cpu, const DisasContextBase *db)
> ret = true;
>
> ptb->vaddr = db->pc_first;
> - ptb->vaddr2 = -1;
> - ptb->haddr1 = db->host_addr[0];
> - ptb->haddr2 = NULL;
> ptb->mem_helper = false;
>
> tcg_gen_plugin_cb(PLUGIN_GEN_FROM_TB);
> @@ -363,23 +360,6 @@ void plugin_gen_insn_start(CPUState *cpu, const DisasContextBase *db)
> pc = db->pc_next;
> insn->vaddr = pc;
>
> - /*
> - * Detect page crossing to get the new host address.
> - * Note that we skip this when haddr1 == NULL, e.g. when we're
> - * fetching instructions from a region not backed by RAM.
> - */
> - if (ptb->haddr1 == NULL) {
> - insn->haddr = NULL;
> - } else if (is_same_page(db, db->pc_next)) {
> - insn->haddr = ptb->haddr1 + pc - ptb->vaddr;
> - } else {
> - if (ptb->vaddr2 == -1) {
> - ptb->vaddr2 = TARGET_PAGE_ALIGN(db->pc_first);
> - get_page_addr_code_hostp(cpu_env(cpu), ptb->vaddr2, &ptb->haddr2);
> - }
> - insn->haddr = ptb->haddr2 + pc - ptb->vaddr2;
> - }
> -
> tcg_gen_plugin_cb(PLUGIN_GEN_FROM_INSN);
> }
>
> diff --git a/plugins/api.c b/plugins/api.c
> index 39895a1cb1..4b6690c7d6 100644
> --- a/plugins/api.c
> +++ b/plugins/api.c
> @@ -242,7 +242,30 @@ uint64_t qemu_plugin_insn_vaddr(const struct qemu_plugin_insn *insn)
>
> void *qemu_plugin_insn_haddr(const struct qemu_plugin_insn *insn)
> {
> - return insn->haddr;
> + const DisasContextBase *db = tcg_ctx->plugin_db;
> + vaddr page0_last = db->pc_first | ~TARGET_PAGE_MASK;
> +
> + if (db->fake_insn) {
> + return NULL;
> + }
> +
> + /*
> + * ??? The return value is not intended for use of host memory,
> + * but as a proxy for address space and physical address.
> + * Thus we are only interested in the first byte and do not
> + * care about spanning pages.
> + */
> + if (insn->vaddr <= page0_last) {
> + if (db->host_addr[0] == NULL) {
> + return NULL;
> + }
> + return db->host_addr[0] + insn->vaddr - db->pc_first;
> + } else {
> + if (db->host_addr[1] == NULL) {
> + return NULL;
> + }
> + return db->host_addr[1] + insn->vaddr - (page0_last + 1);
> + }
> }
>
> char *qemu_plugin_insn_disas(const struct qemu_plugin_insn *insn)
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 58+ messages in thread
* [PATCH v2 14/33] plugins: Use DisasContextBase for qemu_plugin_tb_vaddr
2024-04-24 23:30 [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Richard Henderson
` (12 preceding siblings ...)
2024-04-24 23:31 ` [PATCH v2 13/33] plugins: Use DisasContextBase for qemu_plugin_insn_haddr Richard Henderson
@ 2024-04-24 23:31 ` Richard Henderson
2024-04-24 23:31 ` [PATCH v2 15/33] plugins: Merge alloc_tcg_plugin_context into plugin_gen_tb_start Richard Henderson
` (19 subsequent siblings)
33 siblings, 0 replies; 58+ messages in thread
From: Richard Henderson @ 2024-04-24 23:31 UTC (permalink / raw)
To: qemu-devel; +Cc: Philippe Mathieu-Daudé
We do not need to separately record the start of the TB.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/qemu/plugin.h | 1 -
accel/tcg/plugin-gen.c | 3 +--
plugins/api.c | 3 ++-
3 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
index 3db0e75d16..340e10ef12 100644
--- a/include/qemu/plugin.h
+++ b/include/qemu/plugin.h
@@ -117,7 +117,6 @@ struct qemu_plugin_scoreboard {
struct qemu_plugin_tb {
GPtrArray *insns;
size_t n;
- uint64_t vaddr;
/* if set, the TB calls helpers that might access guest memory */
bool mem_helper;
diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index b036773d3c..2c52306f80 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -188,7 +188,7 @@ static void plugin_gen_inject(struct qemu_plugin_tb *plugin_tb)
int insn_idx = -1;
if (unlikely(qemu_loglevel_mask(LOG_TB_OP_PLUGIN)
- && qemu_log_in_addr_range(plugin_tb->vaddr))) {
+ && qemu_log_in_addr_range(tcg_ctx->plugin_db->pc_first))) {
FILE *logfile = qemu_log_trylock();
if (logfile) {
fprintf(logfile, "OP before plugin injection:\n");
@@ -318,7 +318,6 @@ bool plugin_gen_tb_start(CPUState *cpu, const DisasContextBase *db)
ret = true;
- ptb->vaddr = db->pc_first;
ptb->mem_helper = false;
tcg_gen_plugin_cb(PLUGIN_GEN_FROM_TB);
diff --git a/plugins/api.c b/plugins/api.c
index 4b6690c7d6..36ab47cdae 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -200,7 +200,8 @@ size_t qemu_plugin_tb_n_insns(const struct qemu_plugin_tb *tb)
uint64_t qemu_plugin_tb_vaddr(const struct qemu_plugin_tb *tb)
{
- return tb->vaddr;
+ const DisasContextBase *db = tcg_ctx->plugin_db;
+ return db->pc_first;
}
struct qemu_plugin_insn *
--
2.34.1
^ permalink raw reply related [flat|nested] 58+ messages in thread* [PATCH v2 15/33] plugins: Merge alloc_tcg_plugin_context into plugin_gen_tb_start
2024-04-24 23:30 [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Richard Henderson
` (13 preceding siblings ...)
2024-04-24 23:31 ` [PATCH v2 14/33] plugins: Use DisasContextBase for qemu_plugin_tb_vaddr Richard Henderson
@ 2024-04-24 23:31 ` Richard Henderson
2024-05-06 22:38 ` Philippe Mathieu-Daudé
2024-04-24 23:31 ` [PATCH v2 16/33] accel/tcg: Provide default implementation of disas_log Richard Henderson
` (18 subsequent siblings)
33 siblings, 1 reply; 58+ messages in thread
From: Richard Henderson @ 2024-04-24 23:31 UTC (permalink / raw)
To: qemu-devel
We don't need to allocate plugin context at startup,
we can wait until we actually use it.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
accel/tcg/plugin-gen.c | 36 ++++++++++++++++++++----------------
tcg/tcg.c | 11 -----------
2 files changed, 20 insertions(+), 27 deletions(-)
diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index 2c52306f80..8ebf215645 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -305,28 +305,32 @@ static void plugin_gen_inject(struct qemu_plugin_tb *plugin_tb)
bool plugin_gen_tb_start(CPUState *cpu, const DisasContextBase *db)
{
- bool ret = false;
+ struct qemu_plugin_tb *ptb;
- if (test_bit(QEMU_PLUGIN_EV_VCPU_TB_TRANS, cpu->plugin_state->event_mask)) {
- struct qemu_plugin_tb *ptb = tcg_ctx->plugin_tb;
-
- /* reset callbacks */
- if (ptb->cbs) {
- g_array_set_size(ptb->cbs, 0);
- }
- ptb->n = 0;
-
- ret = true;
-
- ptb->mem_helper = false;
-
- tcg_gen_plugin_cb(PLUGIN_GEN_FROM_TB);
+ if (!test_bit(QEMU_PLUGIN_EV_VCPU_TB_TRANS,
+ cpu->plugin_state->event_mask)) {
+ return false;
}
tcg_ctx->plugin_db = db;
tcg_ctx->plugin_insn = NULL;
+ ptb = tcg_ctx->plugin_tb;
- return ret;
+ if (ptb) {
+ /* Reset callbacks */
+ if (ptb->cbs) {
+ g_array_set_size(ptb->cbs, 0);
+ }
+ ptb->n = 0;
+ ptb->mem_helper = false;
+ } else {
+ ptb = g_new0(struct qemu_plugin_tb, 1);
+ tcg_ctx->plugin_tb = ptb;
+ ptb->insns = g_ptr_array_new();
+ }
+
+ tcg_gen_plugin_cb(PLUGIN_GEN_FROM_TB);
+ return true;
}
void plugin_gen_insn_start(CPUState *cpu, const DisasContextBase *db)
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 691b2342a2..6cc9f205c4 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -761,14 +761,6 @@ QEMU_BUILD_BUG_ON((int)(offsetof(CPUNegativeOffsetState, tlb.f[0]) -
< MIN_TLB_MASK_TABLE_OFS);
#endif
-static void alloc_tcg_plugin_context(TCGContext *s)
-{
-#ifdef CONFIG_PLUGIN
- s->plugin_tb = g_new0(struct qemu_plugin_tb, 1);
- s->plugin_tb->insns = g_ptr_array_new();
-#endif
-}
-
/*
* All TCG threads except the parent (i.e. the one that called tcg_context_init
* and registered the target's TCG globals) must register with this function
@@ -813,7 +805,6 @@ void tcg_register_thread(void)
qatomic_set(&tcg_ctxs[n], s);
if (n > 0) {
- alloc_tcg_plugin_context(s);
tcg_region_initial_alloc(s);
}
@@ -1360,8 +1351,6 @@ static void tcg_context_init(unsigned max_cpus)
indirect_reg_alloc_order[i] = tcg_target_reg_alloc_order[i];
}
- alloc_tcg_plugin_context(s);
-
tcg_ctx = s;
/*
* In user-mode we simply share the init context among threads, since we
--
2.34.1
^ permalink raw reply related [flat|nested] 58+ messages in thread* [PATCH v2 16/33] accel/tcg: Provide default implementation of disas_log
2024-04-24 23:30 [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Richard Henderson
` (14 preceding siblings ...)
2024-04-24 23:31 ` [PATCH v2 15/33] plugins: Merge alloc_tcg_plugin_context into plugin_gen_tb_start Richard Henderson
@ 2024-04-24 23:31 ` Richard Henderson
2024-04-24 23:31 ` [PATCH v2 17/33] accel/tcg: Return bool from TranslatorOps.disas_log Richard Henderson
` (17 subsequent siblings)
33 siblings, 0 replies; 58+ messages in thread
From: Richard Henderson @ 2024-04-24 23:31 UTC (permalink / raw)
To: qemu-devel; +Cc: Philippe Mathieu-Daudé
Almost all of the disas_log implementations are identical.
Unify them within translator_loop.
Drop extra Priv/Virt logging from target/riscv.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
accel/tcg/translator.c | 9 ++++++++-
target/alpha/translate.c | 9 ---------
target/arm/tcg/translate-a64.c | 11 -----------
target/arm/tcg/translate.c | 12 ------------
target/avr/translate.c | 8 --------
target/cris/translate.c | 11 -----------
target/hexagon/translate.c | 9 ---------
target/hppa/translate.c | 6 ++++--
target/i386/tcg/translate.c | 11 -----------
target/loongarch/tcg/translate.c | 8 --------
target/m68k/translate.c | 9 ---------
target/microblaze/translate.c | 9 ---------
target/mips/tcg/translate.c | 9 ---------
target/openrisc/translate.c | 11 -----------
target/ppc/translate.c | 9 ---------
target/riscv/translate.c | 18 ------------------
target/rx/translate.c | 8 --------
target/sh4/translate.c | 9 ---------
target/sparc/translate.c | 9 ---------
target/tricore/translate.c | 9 ---------
target/xtensa/translate.c | 9 ---------
21 files changed, 12 insertions(+), 191 deletions(-)
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index 5a059ce83e..db586b894a 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -16,6 +16,7 @@
#include "exec/cpu_ldst.h"
#include "tcg/tcg-op-common.h"
#include "internal-target.h"
+#include "disas/disas.h"
static void set_can_do_io(DisasContextBase *db, bool val)
{
@@ -225,7 +226,13 @@ void translator_loop(CPUState *cpu, TranslationBlock *tb, int *max_insns,
FILE *logfile = qemu_log_trylock();
if (logfile) {
fprintf(logfile, "----------------\n");
- ops->disas_log(db, cpu, logfile);
+
+ if (ops->disas_log) {
+ ops->disas_log(db, cpu, logfile);
+ } else {
+ fprintf(logfile, "IN: %s\n", lookup_symbol(db->pc_first));
+ target_disas(logfile, cpu, db->pc_first, db->tb->size);
+ }
fprintf(logfile, "\n");
qemu_log_unlock(logfile);
}
diff --git a/target/alpha/translate.c b/target/alpha/translate.c
index a97cd54f0c..9669334da8 100644
--- a/target/alpha/translate.c
+++ b/target/alpha/translate.c
@@ -20,7 +20,6 @@
#include "qemu/osdep.h"
#include "cpu.h"
#include "sysemu/cpus.h"
-#include "disas/disas.h"
#include "qemu/host-utils.h"
#include "exec/exec-all.h"
#include "tcg/tcg-op.h"
@@ -2940,20 +2939,12 @@ static void alpha_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
}
}
-static void alpha_tr_disas_log(const DisasContextBase *dcbase,
- CPUState *cpu, FILE *logfile)
-{
- fprintf(logfile, "IN: %s\n", lookup_symbol(dcbase->pc_first));
- target_disas(logfile, cpu, dcbase->pc_first, dcbase->tb->size);
-}
-
static const TranslatorOps alpha_tr_ops = {
.init_disas_context = alpha_tr_init_disas_context,
.tb_start = alpha_tr_tb_start,
.insn_start = alpha_tr_insn_start,
.translate_insn = alpha_tr_translate_insn,
.tb_stop = alpha_tr_tb_stop,
- .disas_log = alpha_tr_disas_log,
};
void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int *max_insns,
diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
index 2666d52711..636862870f 100644
--- a/target/arm/tcg/translate-a64.c
+++ b/target/arm/tcg/translate-a64.c
@@ -22,7 +22,6 @@
#include "translate.h"
#include "translate-a64.h"
#include "qemu/log.h"
-#include "disas/disas.h"
#include "arm_ldst.h"
#include "semihosting/semihost.h"
#include "cpregs.h"
@@ -14363,20 +14362,10 @@ static void aarch64_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
}
}
-static void aarch64_tr_disas_log(const DisasContextBase *dcbase,
- CPUState *cpu, FILE *logfile)
-{
- DisasContext *dc = container_of(dcbase, DisasContext, base);
-
- fprintf(logfile, "IN: %s\n", lookup_symbol(dc->base.pc_first));
- target_disas(logfile, cpu, dc->base.pc_first, dc->base.tb->size);
-}
-
const TranslatorOps aarch64_translator_ops = {
.init_disas_context = aarch64_tr_init_disas_context,
.tb_start = aarch64_tr_tb_start,
.insn_start = aarch64_tr_insn_start,
.translate_insn = aarch64_tr_translate_insn,
.tb_stop = aarch64_tr_tb_stop,
- .disas_log = aarch64_tr_disas_log,
};
diff --git a/target/arm/tcg/translate.c b/target/arm/tcg/translate.c
index dc49a8d806..d605e10f11 100644
--- a/target/arm/tcg/translate.c
+++ b/target/arm/tcg/translate.c
@@ -23,7 +23,6 @@
#include "translate.h"
#include "translate-a32.h"
#include "qemu/log.h"
-#include "disas/disas.h"
#include "arm_ldst.h"
#include "semihosting/semihost.h"
#include "cpregs.h"
@@ -9663,22 +9662,12 @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
}
}
-static void arm_tr_disas_log(const DisasContextBase *dcbase,
- CPUState *cpu, FILE *logfile)
-{
- DisasContext *dc = container_of(dcbase, DisasContext, base);
-
- fprintf(logfile, "IN: %s\n", lookup_symbol(dc->base.pc_first));
- target_disas(logfile, cpu, dc->base.pc_first, dc->base.tb->size);
-}
-
static const TranslatorOps arm_translator_ops = {
.init_disas_context = arm_tr_init_disas_context,
.tb_start = arm_tr_tb_start,
.insn_start = arm_tr_insn_start,
.translate_insn = arm_tr_translate_insn,
.tb_stop = arm_tr_tb_stop,
- .disas_log = arm_tr_disas_log,
};
static const TranslatorOps thumb_translator_ops = {
@@ -9687,7 +9676,6 @@ static const TranslatorOps thumb_translator_ops = {
.insn_start = arm_tr_insn_start,
.translate_insn = thumb_tr_translate_insn,
.tb_stop = arm_tr_tb_stop,
- .disas_log = arm_tr_disas_log,
};
/* generate intermediate code for basic block 'tb'. */
diff --git a/target/avr/translate.c b/target/avr/translate.c
index 87e2bd5ef1..6df93d4c77 100644
--- a/target/avr/translate.c
+++ b/target/avr/translate.c
@@ -2787,20 +2787,12 @@ static void avr_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
}
}
-static void avr_tr_disas_log(const DisasContextBase *dcbase,
- CPUState *cs, FILE *logfile)
-{
- fprintf(logfile, "IN: %s\n", lookup_symbol(dcbase->pc_first));
- target_disas(logfile, cs, dcbase->pc_first, dcbase->tb->size);
-}
-
static const TranslatorOps avr_tr_ops = {
.init_disas_context = avr_tr_init_disas_context,
.tb_start = avr_tr_tb_start,
.insn_start = avr_tr_insn_start,
.translate_insn = avr_tr_translate_insn,
.tb_stop = avr_tr_tb_stop,
- .disas_log = avr_tr_disas_log,
};
void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int *max_insns,
diff --git a/target/cris/translate.c b/target/cris/translate.c
index b3a4d61d0a..b5410189d4 100644
--- a/target/cris/translate.c
+++ b/target/cris/translate.c
@@ -25,7 +25,6 @@
#include "qemu/osdep.h"
#include "cpu.h"
-#include "disas/disas.h"
#include "exec/exec-all.h"
#include "tcg/tcg-op.h"
#include "exec/helper-proto.h"
@@ -3148,22 +3147,12 @@ static void cris_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
}
}
-static void cris_tr_disas_log(const DisasContextBase *dcbase,
- CPUState *cpu, FILE *logfile)
-{
- if (!DISAS_CRIS) {
- fprintf(logfile, "IN: %s\n", lookup_symbol(dcbase->pc_first));
- target_disas(logfile, cpu, dcbase->pc_first, dcbase->tb->size);
- }
-}
-
static const TranslatorOps cris_tr_ops = {
.init_disas_context = cris_tr_init_disas_context,
.tb_start = cris_tr_tb_start,
.insn_start = cris_tr_insn_start,
.translate_insn = cris_tr_translate_insn,
.tb_stop = cris_tr_tb_stop,
- .disas_log = cris_tr_disas_log,
};
void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int *max_insns,
diff --git a/target/hexagon/translate.c b/target/hexagon/translate.c
index c9bf0e7508..1344a3e4ab 100644
--- a/target/hexagon/translate.c
+++ b/target/hexagon/translate.c
@@ -1138,21 +1138,12 @@ static void hexagon_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
}
}
-static void hexagon_tr_disas_log(const DisasContextBase *dcbase,
- CPUState *cpu, FILE *logfile)
-{
- fprintf(logfile, "IN: %s\n", lookup_symbol(dcbase->pc_first));
- target_disas(logfile, cpu, dcbase->pc_first, dcbase->tb->size);
-}
-
-
static const TranslatorOps hexagon_tr_ops = {
.init_disas_context = hexagon_tr_init_disas_context,
.tb_start = hexagon_tr_tb_start,
.insn_start = hexagon_tr_insn_start,
.translate_insn = hexagon_tr_translate_packet,
.tb_stop = hexagon_tr_tb_stop,
- .disas_log = hexagon_tr_disas_log,
};
void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int *max_insns,
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 42fa480950..cafba84631 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -4815,12 +4815,12 @@ static void hppa_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
}
}
+#ifdef CONFIG_USER_ONLY
static void hppa_tr_disas_log(const DisasContextBase *dcbase,
CPUState *cs, FILE *logfile)
{
target_ulong pc = dcbase->pc_first;
-#ifdef CONFIG_USER_ONLY
switch (pc) {
case 0x00:
fprintf(logfile, "IN:\n0x00000000: (null)\n");
@@ -4835,11 +4835,11 @@ static void hppa_tr_disas_log(const DisasContextBase *dcbase,
fprintf(logfile, "IN:\n0x00000100: syscall\n");
return;
}
-#endif
fprintf(logfile, "IN: %s\n", lookup_symbol(pc));
target_disas(logfile, cs, pc, dcbase->tb->size);
}
+#endif
static const TranslatorOps hppa_tr_ops = {
.init_disas_context = hppa_tr_init_disas_context,
@@ -4847,7 +4847,9 @@ static const TranslatorOps hppa_tr_ops = {
.insn_start = hppa_tr_insn_start,
.translate_insn = hppa_tr_translate_insn,
.tb_stop = hppa_tr_tb_stop,
+#ifdef CONFIG_USER_ONLY
.disas_log = hppa_tr_disas_log,
+#endif
};
void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int *max_insns,
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index c05d9e5225..6658adc4bc 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -20,7 +20,6 @@
#include "qemu/host-utils.h"
#include "cpu.h"
-#include "disas/disas.h"
#include "exec/exec-all.h"
#include "tcg/tcg-op.h"
#include "tcg/tcg-op-gvec.h"
@@ -7074,22 +7073,12 @@ static void i386_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
}
}
-static void i386_tr_disas_log(const DisasContextBase *dcbase,
- CPUState *cpu, FILE *logfile)
-{
- DisasContext *dc = container_of(dcbase, DisasContext, base);
-
- fprintf(logfile, "IN: %s\n", lookup_symbol(dc->base.pc_first));
- target_disas(logfile, cpu, dc->base.pc_first, dc->base.tb->size);
-}
-
static const TranslatorOps i386_tr_ops = {
.init_disas_context = i386_tr_init_disas_context,
.tb_start = i386_tr_tb_start,
.insn_start = i386_tr_insn_start,
.translate_insn = i386_tr_translate_insn,
.tb_stop = i386_tr_tb_stop,
- .disas_log = i386_tr_disas_log,
};
/* generate intermediate code for basic block 'tb'. */
diff --git a/target/loongarch/tcg/translate.c b/target/loongarch/tcg/translate.c
index 7567712655..1fca4afc73 100644
--- a/target/loongarch/tcg/translate.c
+++ b/target/loongarch/tcg/translate.c
@@ -325,20 +325,12 @@ static void loongarch_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
}
}
-static void loongarch_tr_disas_log(const DisasContextBase *dcbase,
- CPUState *cpu, FILE *logfile)
-{
- qemu_log("IN: %s\n", lookup_symbol(dcbase->pc_first));
- target_disas(logfile, cpu, dcbase->pc_first, dcbase->tb->size);
-}
-
static const TranslatorOps loongarch_tr_ops = {
.init_disas_context = loongarch_tr_init_disas_context,
.tb_start = loongarch_tr_tb_start,
.insn_start = loongarch_tr_insn_start,
.translate_insn = loongarch_tr_translate_insn,
.tb_stop = loongarch_tr_tb_stop,
- .disas_log = loongarch_tr_disas_log,
};
void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int *max_insns,
diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 8a194f2f21..175762685c 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -20,7 +20,6 @@
#include "qemu/osdep.h"
#include "cpu.h"
-#include "disas/disas.h"
#include "exec/exec-all.h"
#include "tcg/tcg-op.h"
#include "qemu/log.h"
@@ -6063,20 +6062,12 @@ static void m68k_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
}
}
-static void m68k_tr_disas_log(const DisasContextBase *dcbase,
- CPUState *cpu, FILE *logfile)
-{
- fprintf(logfile, "IN: %s\n", lookup_symbol(dcbase->pc_first));
- target_disas(logfile, cpu, dcbase->pc_first, dcbase->tb->size);
-}
-
static const TranslatorOps m68k_tr_ops = {
.init_disas_context = m68k_tr_init_disas_context,
.tb_start = m68k_tr_tb_start,
.insn_start = m68k_tr_insn_start,
.translate_insn = m68k_tr_translate_insn,
.tb_stop = m68k_tr_tb_stop,
- .disas_log = m68k_tr_disas_log,
};
void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int *max_insns,
diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index 0941aa67c0..08d98dab07 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -20,7 +20,6 @@
#include "qemu/osdep.h"
#include "cpu.h"
-#include "disas/disas.h"
#include "exec/exec-all.h"
#include "tcg/tcg-op.h"
#include "exec/helper-proto.h"
@@ -1771,20 +1770,12 @@ static void mb_tr_tb_stop(DisasContextBase *dcb, CPUState *cs)
}
}
-static void mb_tr_disas_log(const DisasContextBase *dcb,
- CPUState *cs, FILE *logfile)
-{
- fprintf(logfile, "IN: %s\n", lookup_symbol(dcb->pc_first));
- target_disas(logfile, cs, dcb->pc_first, dcb->tb->size);
-}
-
static const TranslatorOps mb_tr_ops = {
.init_disas_context = mb_tr_init_disas_context,
.tb_start = mb_tr_tb_start,
.insn_start = mb_tr_insn_start,
.translate_insn = mb_tr_translate_insn,
.tb_stop = mb_tr_tb_stop,
- .disas_log = mb_tr_disas_log,
};
void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int *max_insns,
diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index 06c108cc9c..333469b268 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -29,7 +29,6 @@
#include "exec/translation-block.h"
#include "semihosting/semihost.h"
#include "trace.h"
-#include "disas/disas.h"
#include "fpu_helper.h"
#define HELPER_H "helper.h"
@@ -15475,20 +15474,12 @@ static void mips_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
}
}
-static void mips_tr_disas_log(const DisasContextBase *dcbase,
- CPUState *cs, FILE *logfile)
-{
- fprintf(logfile, "IN: %s\n", lookup_symbol(dcbase->pc_first));
- target_disas(logfile, cs, dcbase->pc_first, dcbase->tb->size);
-}
-
static const TranslatorOps mips_tr_ops = {
.init_disas_context = mips_tr_init_disas_context,
.tb_start = mips_tr_tb_start,
.insn_start = mips_tr_insn_start,
.translate_insn = mips_tr_translate_insn,
.tb_stop = mips_tr_tb_stop,
- .disas_log = mips_tr_disas_log,
};
void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int *max_insns,
diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c
index 23fff46084..ca566847cb 100644
--- a/target/openrisc/translate.c
+++ b/target/openrisc/translate.c
@@ -21,7 +21,6 @@
#include "qemu/osdep.h"
#include "cpu.h"
#include "exec/exec-all.h"
-#include "disas/disas.h"
#include "tcg/tcg-op.h"
#include "qemu/log.h"
#include "qemu/bitops.h"
@@ -1638,22 +1637,12 @@ static void openrisc_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
}
}
-static void openrisc_tr_disas_log(const DisasContextBase *dcbase,
- CPUState *cs, FILE *logfile)
-{
- DisasContext *s = container_of(dcbase, DisasContext, base);
-
- fprintf(logfile, "IN: %s\n", lookup_symbol(s->base.pc_first));
- target_disas(logfile, cs, s->base.pc_first, s->base.tb->size);
-}
-
static const TranslatorOps openrisc_tr_ops = {
.init_disas_context = openrisc_tr_init_disas_context,
.tb_start = openrisc_tr_tb_start,
.insn_start = openrisc_tr_insn_start,
.translate_insn = openrisc_tr_translate_insn,
.tb_stop = openrisc_tr_tb_stop,
- .disas_log = openrisc_tr_disas_log,
};
void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int *max_insns,
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 93ffec787c..49dee6cab0 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -21,7 +21,6 @@
#include "qemu/osdep.h"
#include "cpu.h"
#include "internal.h"
-#include "disas/disas.h"
#include "exec/exec-all.h"
#include "tcg/tcg-op.h"
#include "tcg/tcg-op-gvec.h"
@@ -7405,20 +7404,12 @@ static void ppc_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
}
}
-static void ppc_tr_disas_log(const DisasContextBase *dcbase,
- CPUState *cs, FILE *logfile)
-{
- fprintf(logfile, "IN: %s\n", lookup_symbol(dcbase->pc_first));
- target_disas(logfile, cs, dcbase->pc_first, dcbase->tb->size);
-}
-
static const TranslatorOps ppc_tr_ops = {
.init_disas_context = ppc_tr_init_disas_context,
.tb_start = ppc_tr_tb_start,
.insn_start = ppc_tr_insn_start,
.translate_insn = ppc_tr_translate_insn,
.tb_stop = ppc_tr_tb_stop,
- .disas_log = ppc_tr_disas_log,
};
void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int *max_insns,
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 9ff09ebdb6..c999e942e1 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -20,7 +20,6 @@
#include "qemu/log.h"
#include "cpu.h"
#include "tcg/tcg-op.h"
-#include "disas/disas.h"
#include "exec/cpu_ldst.h"
#include "exec/exec-all.h"
#include "exec/helper-proto.h"
@@ -1270,29 +1269,12 @@ static void riscv_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
}
}
-static void riscv_tr_disas_log(const DisasContextBase *dcbase,
- CPUState *cpu, FILE *logfile)
-{
-#ifndef CONFIG_USER_ONLY
- RISCVCPU *rvcpu = RISCV_CPU(cpu);
- CPURISCVState *env = &rvcpu->env;
-#endif
-
- fprintf(logfile, "IN: %s\n", lookup_symbol(dcbase->pc_first));
-#ifndef CONFIG_USER_ONLY
- fprintf(logfile, "Priv: "TARGET_FMT_ld"; Virt: %d\n",
- env->priv, env->virt_enabled);
-#endif
- target_disas(logfile, cpu, dcbase->pc_first, dcbase->tb->size);
-}
-
static const TranslatorOps riscv_tr_ops = {
.init_disas_context = riscv_tr_init_disas_context,
.tb_start = riscv_tr_tb_start,
.insn_start = riscv_tr_insn_start,
.translate_insn = riscv_tr_translate_insn,
.tb_stop = riscv_tr_tb_stop,
- .disas_log = riscv_tr_disas_log,
};
void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int *max_insns,
diff --git a/target/rx/translate.c b/target/rx/translate.c
index f6e9e0ec90..92fb2b43ad 100644
--- a/target/rx/translate.c
+++ b/target/rx/translate.c
@@ -2247,20 +2247,12 @@ static void rx_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
}
}
-static void rx_tr_disas_log(const DisasContextBase *dcbase,
- CPUState *cs, FILE *logfile)
-{
- fprintf(logfile, "IN: %s\n", lookup_symbol(dcbase->pc_first));
- target_disas(logfile, cs, dcbase->pc_first, dcbase->tb->size);
-}
-
static const TranslatorOps rx_tr_ops = {
.init_disas_context = rx_tr_init_disas_context,
.tb_start = rx_tr_tb_start,
.insn_start = rx_tr_insn_start,
.translate_insn = rx_tr_translate_insn,
.tb_stop = rx_tr_tb_stop,
- .disas_log = rx_tr_disas_log,
};
void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int *max_insns,
diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index ebb6c901bf..da5b6f046f 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -19,7 +19,6 @@
#include "qemu/osdep.h"
#include "cpu.h"
-#include "disas/disas.h"
#include "exec/exec-all.h"
#include "tcg/tcg-op.h"
#include "exec/helper-proto.h"
@@ -2301,20 +2300,12 @@ static void sh4_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
}
}
-static void sh4_tr_disas_log(const DisasContextBase *dcbase,
- CPUState *cs, FILE *logfile)
-{
- fprintf(logfile, "IN: %s\n", lookup_symbol(dcbase->pc_first));
- target_disas(logfile, cs, dcbase->pc_first, dcbase->tb->size);
-}
-
static const TranslatorOps sh4_tr_ops = {
.init_disas_context = sh4_tr_init_disas_context,
.tb_start = sh4_tr_tb_start,
.insn_start = sh4_tr_insn_start,
.translate_insn = sh4_tr_translate_insn,
.tb_stop = sh4_tr_tb_stop,
- .disas_log = sh4_tr_disas_log,
};
void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int *max_insns,
diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index 571b3e3f03..dd99be0e04 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -21,7 +21,6 @@
#include "qemu/osdep.h"
#include "cpu.h"
-#include "disas/disas.h"
#include "exec/helper-proto.h"
#include "exec/exec-all.h"
#include "tcg/tcg-op.h"
@@ -5042,20 +5041,12 @@ static void sparc_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
}
}
-static void sparc_tr_disas_log(const DisasContextBase *dcbase,
- CPUState *cpu, FILE *logfile)
-{
- fprintf(logfile, "IN: %s\n", lookup_symbol(dcbase->pc_first));
- target_disas(logfile, cpu, dcbase->pc_first, dcbase->tb->size);
-}
-
static const TranslatorOps sparc_tr_ops = {
.init_disas_context = sparc_tr_init_disas_context,
.tb_start = sparc_tr_tb_start,
.insn_start = sparc_tr_insn_start,
.translate_insn = sparc_tr_translate_insn,
.tb_stop = sparc_tr_tb_stop,
- .disas_log = sparc_tr_disas_log,
};
void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int *max_insns,
diff --git a/target/tricore/translate.c b/target/tricore/translate.c
index c45e1d992e..a46a03e1fd 100644
--- a/target/tricore/translate.c
+++ b/target/tricore/translate.c
@@ -20,7 +20,6 @@
#include "qemu/osdep.h"
#include "cpu.h"
-#include "disas/disas.h"
#include "exec/exec-all.h"
#include "tcg/tcg-op.h"
#include "exec/cpu_ldst.h"
@@ -8453,20 +8452,12 @@ static void tricore_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
}
}
-static void tricore_tr_disas_log(const DisasContextBase *dcbase,
- CPUState *cpu, FILE *logfile)
-{
- fprintf(logfile, "IN: %s\n", lookup_symbol(dcbase->pc_first));
- target_disas(logfile, cpu, dcbase->pc_first, dcbase->tb->size);
-}
-
static const TranslatorOps tricore_tr_ops = {
.init_disas_context = tricore_tr_init_disas_context,
.tb_start = tricore_tr_tb_start,
.insn_start = tricore_tr_insn_start,
.translate_insn = tricore_tr_translate_insn,
.tb_stop = tricore_tr_tb_stop,
- .disas_log = tricore_tr_disas_log,
};
diff --git a/target/xtensa/translate.c b/target/xtensa/translate.c
index b206d57fc4..42109d33ad 100644
--- a/target/xtensa/translate.c
+++ b/target/xtensa/translate.c
@@ -32,7 +32,6 @@
#include "cpu.h"
#include "exec/exec-all.h"
-#include "disas/disas.h"
#include "tcg/tcg-op.h"
#include "qemu/log.h"
#include "qemu/qemu-print.h"
@@ -1221,20 +1220,12 @@ static void xtensa_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
}
}
-static void xtensa_tr_disas_log(const DisasContextBase *dcbase,
- CPUState *cpu, FILE *logfile)
-{
- fprintf(logfile, "IN: %s\n", lookup_symbol(dcbase->pc_first));
- target_disas(logfile, cpu, dcbase->pc_first, dcbase->tb->size);
-}
-
static const TranslatorOps xtensa_translator_ops = {
.init_disas_context = xtensa_tr_init_disas_context,
.tb_start = xtensa_tr_tb_start,
.insn_start = xtensa_tr_insn_start,
.translate_insn = xtensa_tr_translate_insn,
.tb_stop = xtensa_tr_tb_stop,
- .disas_log = xtensa_tr_disas_log,
};
void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int *max_insns,
--
2.34.1
^ permalink raw reply related [flat|nested] 58+ messages in thread* [PATCH v2 17/33] accel/tcg: Return bool from TranslatorOps.disas_log
2024-04-24 23:30 [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Richard Henderson
` (15 preceding siblings ...)
2024-04-24 23:31 ` [PATCH v2 16/33] accel/tcg: Provide default implementation of disas_log Richard Henderson
@ 2024-04-24 23:31 ` Richard Henderson
2024-04-24 23:31 ` [PATCH v2 18/33] disas: Split disas.c Richard Henderson
` (16 subsequent siblings)
33 siblings, 0 replies; 58+ messages in thread
From: Richard Henderson @ 2024-04-24 23:31 UTC (permalink / raw)
To: qemu-devel; +Cc: Philippe Mathieu-Daudé
We have eliminated most uses of this hook. Reduce
further by allowing the hook to handle only the
special cases, returning false for normal processing.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/translator.h | 2 +-
accel/tcg/translator.c | 5 ++---
target/hppa/translate.c | 15 ++++++---------
target/s390x/tcg/translate.c | 8 +++-----
4 files changed, 12 insertions(+), 18 deletions(-)
diff --git a/include/exec/translator.h b/include/exec/translator.h
index 3c354a4310..bd76d6446b 100644
--- a/include/exec/translator.h
+++ b/include/exec/translator.h
@@ -135,7 +135,7 @@ typedef struct TranslatorOps {
void (*insn_start)(DisasContextBase *db, CPUState *cpu);
void (*translate_insn)(DisasContextBase *db, CPUState *cpu);
void (*tb_stop)(DisasContextBase *db, CPUState *cpu);
- void (*disas_log)(const DisasContextBase *db, CPUState *cpu, FILE *f);
+ bool (*disas_log)(const DisasContextBase *db, CPUState *cpu, FILE *f);
} TranslatorOps;
/**
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index db586b894a..8e8c4e1bf8 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -227,9 +227,8 @@ void translator_loop(CPUState *cpu, TranslationBlock *tb, int *max_insns,
if (logfile) {
fprintf(logfile, "----------------\n");
- if (ops->disas_log) {
- ops->disas_log(db, cpu, logfile);
- } else {
+ if (!ops->disas_log ||
+ !ops->disas_log(db, cpu, logfile)) {
fprintf(logfile, "IN: %s\n", lookup_symbol(db->pc_first));
target_disas(logfile, cpu, db->pc_first, db->tb->size);
}
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index cafba84631..e8a542c039 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -19,7 +19,6 @@
#include "qemu/osdep.h"
#include "cpu.h"
-#include "disas/disas.h"
#include "qemu/host-utils.h"
#include "exec/exec-all.h"
#include "tcg/tcg-op.h"
@@ -4816,7 +4815,7 @@ static void hppa_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
}
#ifdef CONFIG_USER_ONLY
-static void hppa_tr_disas_log(const DisasContextBase *dcbase,
+static bool hppa_tr_disas_log(const DisasContextBase *dcbase,
CPUState *cs, FILE *logfile)
{
target_ulong pc = dcbase->pc_first;
@@ -4824,20 +4823,18 @@ static void hppa_tr_disas_log(const DisasContextBase *dcbase,
switch (pc) {
case 0x00:
fprintf(logfile, "IN:\n0x00000000: (null)\n");
- return;
+ return true;
case 0xb0:
fprintf(logfile, "IN:\n0x000000b0: light-weight-syscall\n");
- return;
+ return true;
case 0xe0:
fprintf(logfile, "IN:\n0x000000e0: set-thread-pointer-syscall\n");
- return;
+ return true;
case 0x100:
fprintf(logfile, "IN:\n0x00000100: syscall\n");
- return;
+ return true;
}
-
- fprintf(logfile, "IN: %s\n", lookup_symbol(pc));
- target_disas(logfile, cs, pc, dcbase->tb->size);
+ return false;
}
#endif
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index 6d7f6e7064..d74939389a 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -31,7 +31,6 @@
#include "qemu/osdep.h"
#include "cpu.h"
#include "s390x-internal.h"
-#include "disas/disas.h"
#include "exec/exec-all.h"
#include "tcg/tcg-op.h"
#include "tcg/tcg-op-gvec.h"
@@ -6520,7 +6519,7 @@ static void s390x_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
}
}
-static void s390x_tr_disas_log(const DisasContextBase *dcbase,
+static bool s390x_tr_disas_log(const DisasContextBase *dcbase,
CPUState *cs, FILE *logfile)
{
DisasContext *dc = container_of(dcbase, DisasContext, base);
@@ -6528,10 +6527,9 @@ static void s390x_tr_disas_log(const DisasContextBase *dcbase,
if (unlikely(dc->ex_value)) {
/* ??? Unfortunately target_disas can't use host memory. */
fprintf(logfile, "IN: EXECUTE %016" PRIx64, dc->ex_value);
- } else {
- fprintf(logfile, "IN: %s\n", lookup_symbol(dc->base.pc_first));
- target_disas(logfile, cs, dc->base.pc_first, dc->base.tb->size);
+ return true;
}
+ return false;
}
static const TranslatorOps s390x_tr_ops = {
--
2.34.1
^ permalink raw reply related [flat|nested] 58+ messages in thread* [PATCH v2 18/33] disas: Split disas.c
2024-04-24 23:30 [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Richard Henderson
` (16 preceding siblings ...)
2024-04-24 23:31 ` [PATCH v2 17/33] accel/tcg: Return bool from TranslatorOps.disas_log Richard Henderson
@ 2024-04-24 23:31 ` Richard Henderson
2024-05-08 15:26 ` Philippe Mathieu-Daudé
2024-04-24 23:31 ` [PATCH v2 19/33] disas: Use translator_st to get disassembly data Richard Henderson
` (15 subsequent siblings)
33 siblings, 1 reply; 58+ messages in thread
From: Richard Henderson @ 2024-04-24 23:31 UTC (permalink / raw)
To: qemu-devel
The routines in disas-common.c are also used from disas-mon.c.
Otherwise the rest of disassembly is only used from tcg.
While we're at it, put host and target code into separate files.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
disas/disas-internal.h | 4 +
include/disas/disas.h | 4 +
disas/disas-common.c | 117 ++++++++++++++
disas/disas-host.c | 129 ++++++++++++++++
disas/disas-target.c | 84 ++++++++++
disas/disas.c | 337 -----------------------------------------
disas/objdump.c | 37 +++++
disas/meson.build | 8 +-
8 files changed, 381 insertions(+), 339 deletions(-)
create mode 100644 disas/disas-common.c
create mode 100644 disas/disas-host.c
create mode 100644 disas/disas-target.c
delete mode 100644 disas/disas.c
create mode 100644 disas/objdump.c
diff --git a/disas/disas-internal.h b/disas/disas-internal.h
index 84a01f126f..ed32e704cc 100644
--- a/disas/disas-internal.h
+++ b/disas/disas-internal.h
@@ -14,8 +14,12 @@ typedef struct CPUDebug {
CPUState *cpu;
} CPUDebug;
+void disas_initialize_debug(CPUDebug *s);
void disas_initialize_debug_target(CPUDebug *s, CPUState *cpu);
int disas_gstring_printf(FILE *stream, const char *fmt, ...)
G_GNUC_PRINTF(2, 3);
+int print_insn_od_host(bfd_vma pc, disassemble_info *info);
+int print_insn_od_target(bfd_vma pc, disassemble_info *info);
+
#endif
diff --git a/include/disas/disas.h b/include/disas/disas.h
index 176775eff7..54a5e68443 100644
--- a/include/disas/disas.h
+++ b/include/disas/disas.h
@@ -2,13 +2,17 @@
#define QEMU_DISAS_H
/* Disassemble this for me please... (debugging). */
+#ifdef CONFIG_TCG
void disas(FILE *out, const void *code, size_t size);
void target_disas(FILE *out, CPUState *cpu, uint64_t code, size_t size);
+#endif
void monitor_disas(Monitor *mon, CPUState *cpu, uint64_t pc,
int nb_insn, bool is_physical);
+#ifdef CONFIG_PLUGIN
char *plugin_disas(CPUState *cpu, uint64_t addr, size_t size);
+#endif
/* Look up symbol for debugging purpose. Returns "" if unknown. */
const char *lookup_symbol(uint64_t orig_addr);
diff --git a/disas/disas-common.c b/disas/disas-common.c
new file mode 100644
index 0000000000..e4118a381f
--- /dev/null
+++ b/disas/disas-common.c
@@ -0,0 +1,117 @@
+/*
+ * Common routines for disassembly.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "disas/disas.h"
+#include "disas/capstone.h"
+#include "hw/core/cpu.h"
+#include "exec/memory.h"
+#include "disas-internal.h"
+
+
+/* Filled in by elfload.c. Simplistic, but will do for now. */
+struct syminfo *syminfos = NULL;
+
+/*
+ * Get LENGTH bytes from info's buffer, at target address memaddr.
+ * Transfer them to myaddr.
+ */
+static int target_read_memory(bfd_vma memaddr, bfd_byte *myaddr, int length,
+ struct disassemble_info *info)
+{
+ CPUDebug *s = container_of(info, CPUDebug, info);
+ int r = cpu_memory_rw_debug(s->cpu, memaddr, myaddr, length, 0);
+ return r ? EIO : 0;
+}
+
+/*
+ * Print an error message. We can assume that this is in response to
+ * an error return from {host,target}_read_memory.
+ */
+static void perror_memory(int status, bfd_vma memaddr,
+ struct disassemble_info *info)
+{
+ if (status != EIO) {
+ /* Can't happen. */
+ info->fprintf_func(info->stream, "Unknown error %d\n", status);
+ } else {
+ /* Address between memaddr and memaddr + len was out of bounds. */
+ info->fprintf_func(info->stream,
+ "Address 0x%" PRIx64 " is out of bounds.\n",
+ memaddr);
+ }
+}
+
+/* Print address in hex. */
+static void print_address(bfd_vma addr, struct disassemble_info *info)
+{
+ info->fprintf_func(info->stream, "0x%" PRIx64, addr);
+}
+
+/* Stub prevents some fruitless earching in optabs disassemblers. */
+static int symbol_at_address(bfd_vma addr, struct disassemble_info *info)
+{
+ return 1;
+}
+
+void disas_initialize_debug(CPUDebug *s)
+{
+ memset(s, 0, sizeof(*s));
+ s->info.arch = bfd_arch_unknown;
+ s->info.cap_arch = -1;
+ s->info.cap_insn_unit = 4;
+ s->info.cap_insn_split = 4;
+ s->info.memory_error_func = perror_memory;
+ s->info.symbol_at_address_func = symbol_at_address;
+}
+
+void disas_initialize_debug_target(CPUDebug *s, CPUState *cpu)
+{
+ disas_initialize_debug(s);
+
+ s->cpu = cpu;
+ s->info.read_memory_func = target_read_memory;
+ s->info.print_address_func = print_address;
+ if (target_words_bigendian()) {
+ s->info.endian = BFD_ENDIAN_BIG;
+ } else {
+ s->info.endian = BFD_ENDIAN_LITTLE;
+ }
+
+ CPUClass *cc = CPU_GET_CLASS(cpu);
+ if (cc->disas_set_info) {
+ cc->disas_set_info(cpu, &s->info);
+ }
+}
+
+int disas_gstring_printf(FILE *stream, const char *fmt, ...)
+{
+ /* We abuse the FILE parameter to pass a GString. */
+ GString *s = (GString *)stream;
+ int initial_len = s->len;
+ va_list va;
+
+ va_start(va, fmt);
+ g_string_append_vprintf(s, fmt, va);
+ va_end(va);
+
+ return s->len - initial_len;
+}
+
+/* Look up symbol for debugging purpose. Returns "" if unknown. */
+const char *lookup_symbol(uint64_t orig_addr)
+{
+ const char *symbol = "";
+ struct syminfo *s;
+
+ for (s = syminfos; s; s = s->next) {
+ symbol = s->lookup_symbol(s, orig_addr);
+ if (symbol[0] != '\0') {
+ break;
+ }
+ }
+
+ return symbol;
+}
diff --git a/disas/disas-host.c b/disas/disas-host.c
new file mode 100644
index 0000000000..8146fafe80
--- /dev/null
+++ b/disas/disas-host.c
@@ -0,0 +1,129 @@
+/*
+ * Routines for host instruction disassembly.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "disas/disas.h"
+#include "disas/capstone.h"
+#include "disas-internal.h"
+
+
+/*
+ * Get LENGTH bytes from info's buffer, at host address memaddr.
+ * Transfer them to myaddr.
+ */
+static int host_read_memory(bfd_vma memaddr, bfd_byte *myaddr, int length,
+ struct disassemble_info *info)
+{
+ if (memaddr < info->buffer_vma
+ || memaddr + length > info->buffer_vma + info->buffer_length) {
+ /* Out of bounds. Use EIO because GDB uses it. */
+ return EIO;
+ }
+ memcpy (myaddr, info->buffer + (memaddr - info->buffer_vma), length);
+ return 0;
+}
+
+/* Print address in hex, truncated to the width of a host virtual address. */
+static void host_print_address(bfd_vma addr, struct disassemble_info *info)
+{
+ info->fprintf_func(info->stream, "0x%" PRIxPTR, (uintptr_t)addr);
+}
+
+static void initialize_debug_host(CPUDebug *s)
+{
+ disas_initialize_debug(s);
+
+ s->info.read_memory_func = host_read_memory;
+ s->info.print_address_func = host_print_address;
+#if HOST_BIG_ENDIAN
+ s->info.endian = BFD_ENDIAN_BIG;
+#else
+ s->info.endian = BFD_ENDIAN_LITTLE;
+#endif
+#if defined(CONFIG_TCG_INTERPRETER)
+ s->info.print_insn = print_insn_tci;
+#elif defined(__i386__)
+ s->info.mach = bfd_mach_i386_i386;
+ s->info.cap_arch = CS_ARCH_X86;
+ s->info.cap_mode = CS_MODE_32;
+ s->info.cap_insn_unit = 1;
+ s->info.cap_insn_split = 8;
+#elif defined(__x86_64__)
+ s->info.mach = bfd_mach_x86_64;
+ s->info.cap_arch = CS_ARCH_X86;
+ s->info.cap_mode = CS_MODE_64;
+ s->info.cap_insn_unit = 1;
+ s->info.cap_insn_split = 8;
+#elif defined(_ARCH_PPC)
+ s->info.cap_arch = CS_ARCH_PPC;
+# ifdef _ARCH_PPC64
+ s->info.cap_mode = CS_MODE_64;
+# endif
+#elif defined(__riscv)
+#if defined(_ILP32) || (__riscv_xlen == 32)
+ s->info.print_insn = print_insn_riscv32;
+#elif defined(_LP64)
+ s->info.print_insn = print_insn_riscv64;
+#else
+#error unsupported RISC-V ABI
+#endif
+#elif defined(__aarch64__)
+ s->info.cap_arch = CS_ARCH_ARM64;
+#elif defined(__alpha__)
+ s->info.print_insn = print_insn_alpha;
+#elif defined(__sparc__)
+ s->info.print_insn = print_insn_sparc;
+ s->info.mach = bfd_mach_sparc_v9b;
+#elif defined(__arm__)
+ /* TCG only generates code for arm mode. */
+ s->info.cap_arch = CS_ARCH_ARM;
+#elif defined(__MIPSEB__)
+ s->info.print_insn = print_insn_big_mips;
+#elif defined(__MIPSEL__)
+ s->info.print_insn = print_insn_little_mips;
+#elif defined(__m68k__)
+ s->info.print_insn = print_insn_m68k;
+#elif defined(__s390__)
+ s->info.cap_arch = CS_ARCH_SYSZ;
+ s->info.cap_insn_unit = 2;
+ s->info.cap_insn_split = 6;
+#elif defined(__hppa__)
+ s->info.print_insn = print_insn_hppa;
+#elif defined(__loongarch__)
+ s->info.print_insn = print_insn_loongarch;
+#endif
+}
+
+/* Disassemble this for me please... (debugging). */
+void disas(FILE *out, const void *code, size_t size)
+{
+ uintptr_t pc;
+ int count;
+ CPUDebug s;
+
+ initialize_debug_host(&s);
+ s.info.fprintf_func = fprintf;
+ s.info.stream = out;
+ s.info.buffer = code;
+ s.info.buffer_vma = (uintptr_t)code;
+ s.info.buffer_length = size;
+ s.info.show_opcodes = true;
+
+ if (s.info.cap_arch >= 0 && cap_disas_host(&s.info, code, size)) {
+ return;
+ }
+
+ if (s.info.print_insn == NULL) {
+ s.info.print_insn = print_insn_od_host;
+ }
+ for (pc = (uintptr_t)code; size > 0; pc += count, size -= count) {
+ fprintf(out, "0x%08" PRIxPTR ": ", pc);
+ count = s.info.print_insn(pc, &s.info);
+ fprintf(out, "\n");
+ if (count < 0) {
+ break;
+ }
+ }
+}
diff --git a/disas/disas-target.c b/disas/disas-target.c
new file mode 100644
index 0000000000..82313b2a67
--- /dev/null
+++ b/disas/disas-target.c
@@ -0,0 +1,84 @@
+/*
+ * Routines for target instruction disassembly.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "disas/disas.h"
+#include "disas/capstone.h"
+#include "disas-internal.h"
+
+
+void target_disas(FILE *out, CPUState *cpu, uint64_t code, size_t size)
+{
+ uint64_t pc;
+ int count;
+ CPUDebug s;
+
+ disas_initialize_debug_target(&s, cpu);
+ s.info.fprintf_func = fprintf;
+ s.info.stream = out;
+ s.info.buffer_vma = code;
+ s.info.buffer_length = size;
+ s.info.show_opcodes = true;
+
+ if (s.info.cap_arch >= 0 && cap_disas_target(&s.info, code, size)) {
+ return;
+ }
+
+ if (s.info.print_insn == NULL) {
+ s.info.print_insn = print_insn_od_target;
+ }
+
+ for (pc = code; size > 0; pc += count, size -= count) {
+ fprintf(out, "0x%08" PRIx64 ": ", pc);
+ count = s.info.print_insn(pc, &s.info);
+ fprintf(out, "\n");
+ if (count < 0) {
+ break;
+ }
+ if (size < count) {
+ fprintf(out,
+ "Disassembler disagrees with translator over instruction "
+ "decoding\n"
+ "Please report this to qemu-devel@nongnu.org\n");
+ break;
+ }
+ }
+}
+
+#ifdef CONFIG_PLUGIN
+static void plugin_print_address(bfd_vma addr, struct disassemble_info *info)
+{
+ /* does nothing */
+}
+
+/*
+ * We should only be dissembling one instruction at a time here. If
+ * there is left over it usually indicates the front end has read more
+ * bytes than it needed.
+ */
+char *plugin_disas(CPUState *cpu, uint64_t addr, size_t size)
+{
+ CPUDebug s;
+ GString *ds = g_string_new(NULL);
+
+ disas_initialize_debug_target(&s, cpu);
+ s.info.fprintf_func = disas_gstring_printf;
+ s.info.stream = (FILE *)ds; /* abuse this slot */
+ s.info.buffer_vma = addr;
+ s.info.buffer_length = size;
+ s.info.print_address_func = plugin_print_address;
+
+ if (s.info.cap_arch >= 0 && cap_disas_plugin(&s.info, addr, size)) {
+ ; /* done */
+ } else if (s.info.print_insn) {
+ s.info.print_insn(addr, &s.info);
+ } else {
+ ; /* cannot disassemble -- return empty string */
+ }
+
+ /* Return the buffer, freeing the GString container. */
+ return g_string_free(ds, false);
+}
+#endif /* CONFIG_PLUGIN */
diff --git a/disas/disas.c b/disas/disas.c
deleted file mode 100644
index 7e3b0bb46c..0000000000
--- a/disas/disas.c
+++ /dev/null
@@ -1,337 +0,0 @@
-/* General "disassemble this chunk" code. Used for debugging. */
-#include "qemu/osdep.h"
-#include "disas/disas-internal.h"
-#include "elf.h"
-#include "qemu/qemu-print.h"
-#include "disas/disas.h"
-#include "disas/capstone.h"
-#include "hw/core/cpu.h"
-#include "exec/memory.h"
-
-/* Filled in by elfload.c. Simplistic, but will do for now. */
-struct syminfo *syminfos = NULL;
-
-/*
- * Get LENGTH bytes from info's buffer, at host address memaddr.
- * Transfer them to myaddr.
- */
-static int host_read_memory(bfd_vma memaddr, bfd_byte *myaddr, int length,
- struct disassemble_info *info)
-{
- if (memaddr < info->buffer_vma
- || memaddr + length > info->buffer_vma + info->buffer_length) {
- /* Out of bounds. Use EIO because GDB uses it. */
- return EIO;
- }
- memcpy (myaddr, info->buffer + (memaddr - info->buffer_vma), length);
- return 0;
-}
-
-/*
- * Get LENGTH bytes from info's buffer, at target address memaddr.
- * Transfer them to myaddr.
- */
-static int target_read_memory(bfd_vma memaddr, bfd_byte *myaddr, int length,
- struct disassemble_info *info)
-{
- CPUDebug *s = container_of(info, CPUDebug, info);
- int r = cpu_memory_rw_debug(s->cpu, memaddr, myaddr, length, 0);
- return r ? EIO : 0;
-}
-
-/*
- * Print an error message. We can assume that this is in response to
- * an error return from {host,target}_read_memory.
- */
-static void perror_memory(int status, bfd_vma memaddr,
- struct disassemble_info *info)
-{
- if (status != EIO) {
- /* Can't happen. */
- info->fprintf_func(info->stream, "Unknown error %d\n", status);
- } else {
- /* Address between memaddr and memaddr + len was out of bounds. */
- info->fprintf_func(info->stream,
- "Address 0x%" PRIx64 " is out of bounds.\n",
- memaddr);
- }
-}
-
-/* Print address in hex. */
-static void print_address(bfd_vma addr, struct disassemble_info *info)
-{
- info->fprintf_func(info->stream, "0x%" PRIx64, addr);
-}
-
-/* Print address in hex, truncated to the width of a host virtual address. */
-static void host_print_address(bfd_vma addr, struct disassemble_info *info)
-{
- print_address((uintptr_t)addr, info);
-}
-
-/* Stub prevents some fruitless earching in optabs disassemblers. */
-static int symbol_at_address(bfd_vma addr, struct disassemble_info *info)
-{
- return 1;
-}
-
-static int print_insn_objdump(bfd_vma pc, disassemble_info *info,
- const char *prefix)
-{
- int i, n = info->buffer_length;
- g_autofree uint8_t *buf = g_malloc(n);
-
- if (info->read_memory_func(pc, buf, n, info) == 0) {
- for (i = 0; i < n; ++i) {
- if (i % 32 == 0) {
- info->fprintf_func(info->stream, "\n%s: ", prefix);
- }
- info->fprintf_func(info->stream, "%02x", buf[i]);
- }
- } else {
- info->fprintf_func(info->stream, "unable to read memory");
- }
- return n;
-}
-
-static int print_insn_od_host(bfd_vma pc, disassemble_info *info)
-{
- return print_insn_objdump(pc, info, "OBJD-H");
-}
-
-static int print_insn_od_target(bfd_vma pc, disassemble_info *info)
-{
- return print_insn_objdump(pc, info, "OBJD-T");
-}
-
-static void initialize_debug(CPUDebug *s)
-{
- memset(s, 0, sizeof(*s));
- s->info.arch = bfd_arch_unknown;
- s->info.cap_arch = -1;
- s->info.cap_insn_unit = 4;
- s->info.cap_insn_split = 4;
- s->info.memory_error_func = perror_memory;
- s->info.symbol_at_address_func = symbol_at_address;
-}
-
-void disas_initialize_debug_target(CPUDebug *s, CPUState *cpu)
-{
- initialize_debug(s);
-
- s->cpu = cpu;
- s->info.read_memory_func = target_read_memory;
- s->info.print_address_func = print_address;
- if (target_words_bigendian()) {
- s->info.endian = BFD_ENDIAN_BIG;
- } else {
- s->info.endian = BFD_ENDIAN_LITTLE;
- }
-
- CPUClass *cc = CPU_GET_CLASS(cpu);
- if (cc->disas_set_info) {
- cc->disas_set_info(cpu, &s->info);
- }
-}
-
-static void initialize_debug_host(CPUDebug *s)
-{
- initialize_debug(s);
-
- s->info.read_memory_func = host_read_memory;
- s->info.print_address_func = host_print_address;
-#if HOST_BIG_ENDIAN
- s->info.endian = BFD_ENDIAN_BIG;
-#else
- s->info.endian = BFD_ENDIAN_LITTLE;
-#endif
-#if defined(CONFIG_TCG_INTERPRETER)
- s->info.print_insn = print_insn_tci;
-#elif defined(__i386__)
- s->info.mach = bfd_mach_i386_i386;
- s->info.cap_arch = CS_ARCH_X86;
- s->info.cap_mode = CS_MODE_32;
- s->info.cap_insn_unit = 1;
- s->info.cap_insn_split = 8;
-#elif defined(__x86_64__)
- s->info.mach = bfd_mach_x86_64;
- s->info.cap_arch = CS_ARCH_X86;
- s->info.cap_mode = CS_MODE_64;
- s->info.cap_insn_unit = 1;
- s->info.cap_insn_split = 8;
-#elif defined(_ARCH_PPC)
- s->info.cap_arch = CS_ARCH_PPC;
-# ifdef _ARCH_PPC64
- s->info.cap_mode = CS_MODE_64;
-# endif
-#elif defined(__riscv)
-#if defined(_ILP32) || (__riscv_xlen == 32)
- s->info.print_insn = print_insn_riscv32;
-#elif defined(_LP64)
- s->info.print_insn = print_insn_riscv64;
-#else
-#error unsupported RISC-V ABI
-#endif
-#elif defined(__aarch64__)
- s->info.cap_arch = CS_ARCH_ARM64;
-#elif defined(__alpha__)
- s->info.print_insn = print_insn_alpha;
-#elif defined(__sparc__)
- s->info.print_insn = print_insn_sparc;
- s->info.mach = bfd_mach_sparc_v9b;
-#elif defined(__arm__)
- /* TCG only generates code for arm mode. */
- s->info.cap_arch = CS_ARCH_ARM;
-#elif defined(__MIPSEB__)
- s->info.print_insn = print_insn_big_mips;
-#elif defined(__MIPSEL__)
- s->info.print_insn = print_insn_little_mips;
-#elif defined(__m68k__)
- s->info.print_insn = print_insn_m68k;
-#elif defined(__s390__)
- s->info.cap_arch = CS_ARCH_SYSZ;
- s->info.cap_insn_unit = 2;
- s->info.cap_insn_split = 6;
-#elif defined(__hppa__)
- s->info.print_insn = print_insn_hppa;
-#elif defined(__loongarch__)
- s->info.print_insn = print_insn_loongarch;
-#endif
-}
-
-/* Disassemble this for me please... (debugging). */
-void target_disas(FILE *out, CPUState *cpu, uint64_t code, size_t size)
-{
- uint64_t pc;
- int count;
- CPUDebug s;
-
- disas_initialize_debug_target(&s, cpu);
- s.info.fprintf_func = fprintf;
- s.info.stream = out;
- s.info.buffer_vma = code;
- s.info.buffer_length = size;
- s.info.show_opcodes = true;
-
- if (s.info.cap_arch >= 0 && cap_disas_target(&s.info, code, size)) {
- return;
- }
-
- if (s.info.print_insn == NULL) {
- s.info.print_insn = print_insn_od_target;
- }
-
- for (pc = code; size > 0; pc += count, size -= count) {
- fprintf(out, "0x%08" PRIx64 ": ", pc);
- count = s.info.print_insn(pc, &s.info);
- fprintf(out, "\n");
- if (count < 0) {
- break;
- }
- if (size < count) {
- fprintf(out,
- "Disassembler disagrees with translator over instruction "
- "decoding\n"
- "Please report this to qemu-devel@nongnu.org\n");
- break;
- }
- }
-}
-
-int disas_gstring_printf(FILE *stream, const char *fmt, ...)
-{
- /* We abuse the FILE parameter to pass a GString. */
- GString *s = (GString *)stream;
- int initial_len = s->len;
- va_list va;
-
- va_start(va, fmt);
- g_string_append_vprintf(s, fmt, va);
- va_end(va);
-
- return s->len - initial_len;
-}
-
-static void plugin_print_address(bfd_vma addr, struct disassemble_info *info)
-{
- /* does nothing */
-}
-
-
-/*
- * We should only be dissembling one instruction at a time here. If
- * there is left over it usually indicates the front end has read more
- * bytes than it needed.
- */
-char *plugin_disas(CPUState *cpu, uint64_t addr, size_t size)
-{
- CPUDebug s;
- GString *ds = g_string_new(NULL);
-
- disas_initialize_debug_target(&s, cpu);
- s.info.fprintf_func = disas_gstring_printf;
- s.info.stream = (FILE *)ds; /* abuse this slot */
- s.info.buffer_vma = addr;
- s.info.buffer_length = size;
- s.info.print_address_func = plugin_print_address;
-
- if (s.info.cap_arch >= 0 && cap_disas_plugin(&s.info, addr, size)) {
- ; /* done */
- } else if (s.info.print_insn) {
- s.info.print_insn(addr, &s.info);
- } else {
- ; /* cannot disassemble -- return empty string */
- }
-
- /* Return the buffer, freeing the GString container. */
- return g_string_free(ds, false);
-}
-
-/* Disassemble this for me please... (debugging). */
-void disas(FILE *out, const void *code, size_t size)
-{
- uintptr_t pc;
- int count;
- CPUDebug s;
-
- initialize_debug_host(&s);
- s.info.fprintf_func = fprintf;
- s.info.stream = out;
- s.info.buffer = code;
- s.info.buffer_vma = (uintptr_t)code;
- s.info.buffer_length = size;
- s.info.show_opcodes = true;
-
- if (s.info.cap_arch >= 0 && cap_disas_host(&s.info, code, size)) {
- return;
- }
-
- if (s.info.print_insn == NULL) {
- s.info.print_insn = print_insn_od_host;
- }
- for (pc = (uintptr_t)code; size > 0; pc += count, size -= count) {
- fprintf(out, "0x%08" PRIxPTR ": ", pc);
- count = s.info.print_insn(pc, &s.info);
- fprintf(out, "\n");
- if (count < 0) {
- break;
- }
- }
-
-}
-
-/* Look up symbol for debugging purpose. Returns "" if unknown. */
-const char *lookup_symbol(uint64_t orig_addr)
-{
- const char *symbol = "";
- struct syminfo *s;
-
- for (s = syminfos; s; s = s->next) {
- symbol = s->lookup_symbol(s, orig_addr);
- if (symbol[0] != '\0') {
- break;
- }
- }
-
- return symbol;
-}
diff --git a/disas/objdump.c b/disas/objdump.c
new file mode 100644
index 0000000000..9859f23419
--- /dev/null
+++ b/disas/objdump.c
@@ -0,0 +1,37 @@
+/*
+ * Dump disassembly as text, for processing by scripts/disas-objdump.pl.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "disas-internal.h"
+
+
+static int print_insn_objdump(bfd_vma pc, disassemble_info *info,
+ const char *prefix)
+{
+ int i, n = info->buffer_length;
+ g_autofree uint8_t *buf = g_malloc(n);
+
+ if (info->read_memory_func(pc, buf, n, info) == 0) {
+ for (i = 0; i < n; ++i) {
+ if (i % 32 == 0) {
+ info->fprintf_func(info->stream, "\n%s: ", prefix);
+ }
+ info->fprintf_func(info->stream, "%02x", buf[i]);
+ }
+ } else {
+ info->fprintf_func(info->stream, "unable to read memory");
+ }
+ return n;
+}
+
+int print_insn_od_host(bfd_vma pc, disassemble_info *info)
+{
+ return print_insn_objdump(pc, info, "OBJD-H");
+}
+
+int print_insn_od_target(bfd_vma pc, disassemble_info *info)
+{
+ return print_insn_objdump(pc, info, "OBJD-T");
+}
diff --git a/disas/meson.build b/disas/meson.build
index 5c8073beb3..20d6aef9a7 100644
--- a/disas/meson.build
+++ b/disas/meson.build
@@ -14,7 +14,11 @@ common_ss.add(when: 'CONFIG_SH4_DIS', if_true: files('sh4.c'))
common_ss.add(when: 'CONFIG_SPARC_DIS', if_true: files('sparc.c'))
common_ss.add(when: 'CONFIG_XTENSA_DIS', if_true: files('xtensa.c'))
common_ss.add(when: capstone, if_true: [files('capstone.c'), capstone])
-common_ss.add(files('disas.c'))
-
+common_ss.add(when: 'CONFIG_TCG', if_true: files(
+ 'disas-host.c',
+ 'disas-target.c',
+ 'objdump.c'
+))
+common_ss.add(files('disas-common.c'))
system_ss.add(files('disas-mon.c'))
specific_ss.add(capstone)
--
2.34.1
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [PATCH v2 18/33] disas: Split disas.c
2024-04-24 23:31 ` [PATCH v2 18/33] disas: Split disas.c Richard Henderson
@ 2024-05-08 15:26 ` Philippe Mathieu-Daudé
2024-05-10 9:42 ` Richard Henderson
0 siblings, 1 reply; 58+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-05-08 15:26 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
On 25/4/24 01:31, Richard Henderson wrote:
> The routines in disas-common.c are also used from disas-mon.c.
> Otherwise the rest of disassembly is only used from tcg.
> While we're at it, put host and target code into separate files.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> disas/disas-internal.h | 4 +
> include/disas/disas.h | 4 +
> disas/disas-common.c | 117 ++++++++++++++
> disas/disas-host.c | 129 ++++++++++++++++
> disas/disas-target.c | 84 ++++++++++
> disas/disas.c | 337 -----------------------------------------
> disas/objdump.c | 37 +++++
> disas/meson.build | 8 +-
> 8 files changed, 381 insertions(+), 339 deletions(-)
> create mode 100644 disas/disas-common.c
> create mode 100644 disas/disas-host.c
> create mode 100644 disas/disas-target.c
> delete mode 100644 disas/disas.c
> create mode 100644 disas/objdump.c
> diff --git a/include/disas/disas.h b/include/disas/disas.h
> index 176775eff7..54a5e68443 100644
> --- a/include/disas/disas.h
> +++ b/include/disas/disas.h
> @@ -2,13 +2,17 @@
> #define QEMU_DISAS_H
>
> /* Disassemble this for me please... (debugging). */
> +#ifdef CONFIG_TCG
> void disas(FILE *out, const void *code, size_t size);
> void target_disas(FILE *out, CPUState *cpu, uint64_t code, size_t size);
> +#endif
>
> void monitor_disas(Monitor *mon, CPUState *cpu, uint64_t pc,
> int nb_insn, bool is_physical);
>
> +#ifdef CONFIG_PLUGIN
Although implied, could be within the previous CONFIG_TCG block.
> char *plugin_disas(CPUState *cpu, uint64_t addr, size_t size);
> +#endif
Reviewed using git-diff --color-moved=dimmed-zebra:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v2 18/33] disas: Split disas.c
2024-05-08 15:26 ` Philippe Mathieu-Daudé
@ 2024-05-10 9:42 ` Richard Henderson
0 siblings, 0 replies; 58+ messages in thread
From: Richard Henderson @ 2024-05-10 9:42 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
On 5/8/24 17:26, Philippe Mathieu-Daudé wrote:
>> @@ -2,13 +2,17 @@
>> #define QEMU_DISAS_H
>> /* Disassemble this for me please... (debugging). */
>> +#ifdef CONFIG_TCG
>> void disas(FILE *out, const void *code, size_t size);
>> void target_disas(FILE *out, CPUState *cpu, uint64_t code, size_t size);
>> +#endif
>> void monitor_disas(Monitor *mon, CPUState *cpu, uint64_t pc,
>> int nb_insn, bool is_physical);
>> +#ifdef CONFIG_PLUGIN
>
> Although implied, could be within the previous CONFIG_TCG block.
I thought about that, but this was the more minimal change.
r~
^ permalink raw reply [flat|nested] 58+ messages in thread
* [PATCH v2 19/33] disas: Use translator_st to get disassembly data
2024-04-24 23:30 [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Richard Henderson
` (17 preceding siblings ...)
2024-04-24 23:31 ` [PATCH v2 18/33] disas: Split disas.c Richard Henderson
@ 2024-04-24 23:31 ` Richard Henderson
2024-05-08 15:33 ` Philippe Mathieu-Daudé
2024-04-24 23:31 ` [PATCH v2 20/33] accel/tcg: Introduce translator_fake_ld Richard Henderson
` (14 subsequent siblings)
33 siblings, 1 reply; 58+ messages in thread
From: Richard Henderson @ 2024-04-24 23:31 UTC (permalink / raw)
To: qemu-devel
Read from already translated pages, or saved mmio data.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/disas/disas.h | 5 +++--
include/exec/translator.h | 4 ++--
include/qemu/typedefs.h | 1 +
accel/tcg/translator.c | 2 +-
disas/disas-common.c | 14 --------------
disas/disas-mon.c | 15 +++++++++++++++
disas/disas-target.c | 19 +++++++++++++++++--
plugins/api.c | 4 ++--
8 files changed, 41 insertions(+), 23 deletions(-)
diff --git a/include/disas/disas.h b/include/disas/disas.h
index 54a5e68443..c702b1effc 100644
--- a/include/disas/disas.h
+++ b/include/disas/disas.h
@@ -4,14 +4,15 @@
/* Disassemble this for me please... (debugging). */
#ifdef CONFIG_TCG
void disas(FILE *out, const void *code, size_t size);
-void target_disas(FILE *out, CPUState *cpu, uint64_t code, size_t size);
+void target_disas(FILE *out, CPUState *cpu, const DisasContextBase *db);
#endif
void monitor_disas(Monitor *mon, CPUState *cpu, uint64_t pc,
int nb_insn, bool is_physical);
#ifdef CONFIG_PLUGIN
-char *plugin_disas(CPUState *cpu, uint64_t addr, size_t size);
+char *plugin_disas(CPUState *cpu, const DisasContextBase *db,
+ uint64_t addr, size_t size);
#endif
/* Look up symbol for debugging purpose. Returns "" if unknown. */
diff --git a/include/exec/translator.h b/include/exec/translator.h
index bd76d6446b..8c3a8aa3d1 100644
--- a/include/exec/translator.h
+++ b/include/exec/translator.h
@@ -79,7 +79,7 @@ typedef enum DisasJumpType {
*
* Architecture-agnostic disassembly context.
*/
-typedef struct DisasContextBase {
+struct DisasContextBase {
TranslationBlock *tb;
vaddr pc_first;
vaddr pc_next;
@@ -103,7 +103,7 @@ typedef struct DisasContextBase {
int record_start;
int record_len;
uint8_t record[32];
-} DisasContextBase;
+};
/**
* TranslatorOps:
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index 50c277cf0b..0b3cf3f3ec 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -50,6 +50,7 @@ typedef struct CPUTLBEntryFull CPUTLBEntryFull;
typedef struct DeviceListener DeviceListener;
typedef struct DeviceState DeviceState;
typedef struct DirtyBitmapSnapshot DirtyBitmapSnapshot;
+typedef struct DisasContextBase DisasContextBase;
typedef struct DisplayChangeListener DisplayChangeListener;
typedef struct DriveInfo DriveInfo;
typedef struct DumpState DumpState;
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index 8e8c4e1bf8..e968d530d8 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -230,7 +230,7 @@ void translator_loop(CPUState *cpu, TranslationBlock *tb, int *max_insns,
if (!ops->disas_log ||
!ops->disas_log(db, cpu, logfile)) {
fprintf(logfile, "IN: %s\n", lookup_symbol(db->pc_first));
- target_disas(logfile, cpu, db->pc_first, db->tb->size);
+ target_disas(logfile, cpu, db);
}
fprintf(logfile, "\n");
qemu_log_unlock(logfile);
diff --git a/disas/disas-common.c b/disas/disas-common.c
index e4118a381f..e4118e996b 100644
--- a/disas/disas-common.c
+++ b/disas/disas-common.c
@@ -7,25 +7,12 @@
#include "disas/disas.h"
#include "disas/capstone.h"
#include "hw/core/cpu.h"
-#include "exec/memory.h"
#include "disas-internal.h"
/* Filled in by elfload.c. Simplistic, but will do for now. */
struct syminfo *syminfos = NULL;
-/*
- * Get LENGTH bytes from info's buffer, at target address memaddr.
- * Transfer them to myaddr.
- */
-static int target_read_memory(bfd_vma memaddr, bfd_byte *myaddr, int length,
- struct disassemble_info *info)
-{
- CPUDebug *s = container_of(info, CPUDebug, info);
- int r = cpu_memory_rw_debug(s->cpu, memaddr, myaddr, length, 0);
- return r ? EIO : 0;
-}
-
/*
* Print an error message. We can assume that this is in response to
* an error return from {host,target}_read_memory.
@@ -72,7 +59,6 @@ void disas_initialize_debug_target(CPUDebug *s, CPUState *cpu)
disas_initialize_debug(s);
s->cpu = cpu;
- s->info.read_memory_func = target_read_memory;
s->info.print_address_func = print_address;
if (target_words_bigendian()) {
s->info.endian = BFD_ENDIAN_BIG;
diff --git a/disas/disas-mon.c b/disas/disas-mon.c
index 5d6d9aa02d..37bf16ac79 100644
--- a/disas/disas-mon.c
+++ b/disas/disas-mon.c
@@ -11,6 +11,19 @@
#include "hw/core/cpu.h"
#include "monitor/monitor.h"
+/*
+ * Get LENGTH bytes from info's buffer, at target address memaddr.
+ * Transfer them to myaddr.
+ */
+static int
+virtual_read_memory(bfd_vma memaddr, bfd_byte *myaddr, int length,
+ struct disassemble_info *info)
+{
+ CPUDebug *s = container_of(info, CPUDebug, info);
+ int r = cpu_memory_rw_debug(s->cpu, memaddr, myaddr, length, 0);
+ return r ? EIO : 0;
+}
+
static int
physical_read_memory(bfd_vma memaddr, bfd_byte *myaddr, int length,
struct disassemble_info *info)
@@ -38,6 +51,8 @@ void monitor_disas(Monitor *mon, CPUState *cpu, uint64_t pc,
if (is_physical) {
s.info.read_memory_func = physical_read_memory;
+ } else {
+ s.info.read_memory_func = virtual_read_memory;
}
s.info.buffer_vma = pc;
diff --git a/disas/disas-target.c b/disas/disas-target.c
index 82313b2a67..48f3a365dc 100644
--- a/disas/disas-target.c
+++ b/disas/disas-target.c
@@ -6,16 +6,28 @@
#include "qemu/osdep.h"
#include "disas/disas.h"
#include "disas/capstone.h"
+#include "exec/translator.h"
#include "disas-internal.h"
-void target_disas(FILE *out, CPUState *cpu, uint64_t code, size_t size)
+static int translator_read_memory(bfd_vma memaddr, bfd_byte *myaddr,
+ int length, struct disassemble_info *info)
{
+ const DisasContextBase *db = info->application_data;
+ return translator_st(db, myaddr, memaddr, length) ? 0 : EIO;
+}
+
+void target_disas(FILE *out, CPUState *cpu, const struct DisasContextBase *db)
+{
+ uint64_t code = db->pc_first;
+ size_t size = translator_st_len(db);
uint64_t pc;
int count;
CPUDebug s;
disas_initialize_debug_target(&s, cpu);
+ s.info.read_memory_func = translator_read_memory;
+ s.info.application_data = (void *)db;
s.info.fprintf_func = fprintf;
s.info.stream = out;
s.info.buffer_vma = code;
@@ -58,12 +70,15 @@ static void plugin_print_address(bfd_vma addr, struct disassemble_info *info)
* there is left over it usually indicates the front end has read more
* bytes than it needed.
*/
-char *plugin_disas(CPUState *cpu, uint64_t addr, size_t size)
+char *plugin_disas(CPUState *cpu, const DisasContextBase *db,
+ uint64_t addr, size_t size)
{
CPUDebug s;
GString *ds = g_string_new(NULL);
disas_initialize_debug_target(&s, cpu);
+ s.info.read_memory_func = translator_read_memory;
+ s.info.application_data = (void *)db;
s.info.fprintf_func = disas_gstring_printf;
s.info.stream = (FILE *)ds; /* abuse this slot */
s.info.buffer_vma = addr;
diff --git a/plugins/api.c b/plugins/api.c
index 36ab47cdae..c084f335dd 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -271,8 +271,8 @@ void *qemu_plugin_insn_haddr(const struct qemu_plugin_insn *insn)
char *qemu_plugin_insn_disas(const struct qemu_plugin_insn *insn)
{
- CPUState *cpu = current_cpu;
- return plugin_disas(cpu, insn->vaddr, insn->len);
+ return plugin_disas(tcg_ctx->cpu, tcg_ctx->plugin_db,
+ insn->vaddr, insn->len);
}
const char *qemu_plugin_insn_symbol(const struct qemu_plugin_insn *insn)
--
2.34.1
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [PATCH v2 19/33] disas: Use translator_st to get disassembly data
2024-04-24 23:31 ` [PATCH v2 19/33] disas: Use translator_st to get disassembly data Richard Henderson
@ 2024-05-08 15:33 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 58+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-05-08 15:33 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
On 25/4/24 01:31, Richard Henderson wrote:
> Read from already translated pages, or saved mmio data.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> include/disas/disas.h | 5 +++--
> include/exec/translator.h | 4 ++--
> include/qemu/typedefs.h | 1 +
> accel/tcg/translator.c | 2 +-
> disas/disas-common.c | 14 --------------
> disas/disas-mon.c | 15 +++++++++++++++
> disas/disas-target.c | 19 +++++++++++++++++--
> plugins/api.c | 4 ++--
> 8 files changed, 41 insertions(+), 23 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 58+ messages in thread
* [PATCH v2 20/33] accel/tcg: Introduce translator_fake_ld
2024-04-24 23:30 [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Richard Henderson
` (18 preceding siblings ...)
2024-04-24 23:31 ` [PATCH v2 19/33] disas: Use translator_st to get disassembly data Richard Henderson
@ 2024-04-24 23:31 ` Richard Henderson
2024-04-24 23:31 ` [PATCH v2 21/33] target/s390x: Fix translator_fake_ld length Richard Henderson
` (13 subsequent siblings)
33 siblings, 0 replies; 58+ messages in thread
From: Richard Henderson @ 2024-04-24 23:31 UTC (permalink / raw)
To: qemu-devel; +Cc: Philippe Mathieu-Daudé
Replace translator_fake_ldb, which required multiple calls,
with translator_fake_ld, which can take all data at once.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/translator.h | 8 ++++----
accel/tcg/translator.c | 5 ++---
target/s390x/tcg/translate.c | 8 ++++----
3 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/include/exec/translator.h b/include/exec/translator.h
index 8c3a8aa3d1..fc9fc64963 100644
--- a/include/exec/translator.h
+++ b/include/exec/translator.h
@@ -234,17 +234,17 @@ translator_ldq_swap(CPUArchState *env, DisasContextBase *db,
}
/**
- * translator_fake_ldb - fake instruction load
+ * translator_fake_ld - fake instruction load
* @db: Disassembly context
- * @pc: program counter of instruction
- * @insn8: byte of instruction
+ * @data: bytes of instruction
+ * @len: number of bytes
*
* This is a special case helper used where the instruction we are
* about to translate comes from somewhere else (e.g. being
* re-synthesised for s390x "ex"). It ensures we update other areas of
* the translator with details of the executed instruction.
*/
-void translator_fake_ldb(DisasContextBase *db, vaddr pc, uint8_t insn8);
+void translator_fake_ld(DisasContextBase *db, const void *data, size_t len);
/**
* translator_st
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index e968d530d8..14622b69a8 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -467,9 +467,8 @@ uint64_t translator_ldq(CPUArchState *env, DisasContextBase *db, vaddr pc)
return tgt;
}
-void translator_fake_ldb(DisasContextBase *db, vaddr pc, uint8_t insn8)
+void translator_fake_ld(DisasContextBase *db, const void *data, size_t len)
{
- assert(pc >= db->pc_first);
db->fake_insn = true;
- record_save(db, pc, &insn8, sizeof(insn8));
+ record_save(db, db->pc_first, data, len);
}
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index d74939389a..2eb787e401 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -6191,6 +6191,8 @@ static const DisasInsn *extract_insn(CPUS390XState *env, DisasContext *s)
const DisasInsn *info;
if (unlikely(s->ex_value)) {
+ uint64_t be_insn;
+
/* Drop the EX data now, so that it's clear on exception paths. */
tcg_gen_st_i64(tcg_constant_i64(0), tcg_env,
offsetof(CPUS390XState, ex_value));
@@ -6200,10 +6202,8 @@ static const DisasInsn *extract_insn(CPUS390XState *env, DisasContext *s)
ilen = s->ex_value & 0xf;
/* Register insn bytes with translator so plugins work. */
- for (int i = 0; i < ilen; i++) {
- uint8_t byte = extract64(insn, 56 - (i * 8), 8);
- translator_fake_ldb(&s->base, pc + i, byte);
- }
+ be_insn = cpu_to_be64(insn);
+ translator_fake_ld(&s->base, &be_insn, ilen);
op = insn >> 56;
} else {
insn = ld_code2(env, s, pc);
--
2.34.1
^ permalink raw reply related [flat|nested] 58+ messages in thread* [PATCH v2 21/33] target/s390x: Fix translator_fake_ld length
2024-04-24 23:30 [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Richard Henderson
` (19 preceding siblings ...)
2024-04-24 23:31 ` [PATCH v2 20/33] accel/tcg: Introduce translator_fake_ld Richard Henderson
@ 2024-04-24 23:31 ` Richard Henderson
2024-05-08 16:33 ` Philippe Mathieu-Daudé
2024-04-24 23:31 ` [PATCH v2 22/33] target/s390x: Disassemble EXECUTEd instructions Richard Henderson
` (12 subsequent siblings)
33 siblings, 1 reply; 58+ messages in thread
From: Richard Henderson @ 2024-04-24 23:31 UTC (permalink / raw)
To: qemu-devel
The ilen value extracted from ex_value is the length of the
EXECUTE instruction itself, and so is the increment to the pc.
However, the length of the synthetic insn is located in the
opcode like all other instructions.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/s390x/tcg/translate.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index 2eb787e401..95d4d6ebc3 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -6200,11 +6200,11 @@ static const DisasInsn *extract_insn(CPUS390XState *env, DisasContext *s)
/* Extract the values saved by EXECUTE. */
insn = s->ex_value & 0xffffffffffff0000ull;
ilen = s->ex_value & 0xf;
+ op = insn >> 56;
/* Register insn bytes with translator so plugins work. */
be_insn = cpu_to_be64(insn);
- translator_fake_ld(&s->base, &be_insn, ilen);
- op = insn >> 56;
+ translator_fake_ld(&s->base, &be_insn, get_ilen(op));
} else {
insn = ld_code2(env, s, pc);
op = (insn >> 8) & 0xff;
--
2.34.1
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [PATCH v2 21/33] target/s390x: Fix translator_fake_ld length
2024-04-24 23:31 ` [PATCH v2 21/33] target/s390x: Fix translator_fake_ld length Richard Henderson
@ 2024-05-08 16:33 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 58+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-05-08 16:33 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
On 25/4/24 01:31, Richard Henderson wrote:
> The ilen value extracted from ex_value is the length of the
> EXECUTE instruction itself, and so is the increment to the pc.
> However, the length of the synthetic insn is located in the
> opcode like all other instructions.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/s390x/tcg/translate.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 58+ messages in thread
* [PATCH v2 22/33] target/s390x: Disassemble EXECUTEd instructions
2024-04-24 23:30 [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Richard Henderson
` (20 preceding siblings ...)
2024-04-24 23:31 ` [PATCH v2 21/33] target/s390x: Fix translator_fake_ld length Richard Henderson
@ 2024-04-24 23:31 ` Richard Henderson
2024-05-08 16:31 ` Philippe Mathieu-Daudé
2024-04-24 23:31 ` [PATCH v2 23/33] target/hexagon: Use translator_ldl in pkt_crosses_page Richard Henderson
` (11 subsequent siblings)
33 siblings, 1 reply; 58+ messages in thread
From: Richard Henderson @ 2024-04-24 23:31 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/s390x/tcg/translate.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index 95d4d6ebc3..bac033c63c 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -6525,8 +6525,9 @@ static bool s390x_tr_disas_log(const DisasContextBase *dcbase,
DisasContext *dc = container_of(dcbase, DisasContext, base);
if (unlikely(dc->ex_value)) {
- /* ??? Unfortunately target_disas can't use host memory. */
- fprintf(logfile, "IN: EXECUTE %016" PRIx64, dc->ex_value);
+ /* The ex_value has been recorded with translator_fake_ld. */
+ fprintf(logfile, "IN: EXECUTE\n");
+ target_disas(logfile, cs, &dc->base);
return true;
}
return false;
--
2.34.1
^ permalink raw reply related [flat|nested] 58+ messages in thread* [PATCH v2 23/33] target/hexagon: Use translator_ldl in pkt_crosses_page
2024-04-24 23:30 [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Richard Henderson
` (21 preceding siblings ...)
2024-04-24 23:31 ` [PATCH v2 22/33] target/s390x: Disassemble EXECUTEd instructions Richard Henderson
@ 2024-04-24 23:31 ` Richard Henderson
2024-04-29 10:47 ` Philippe Mathieu-Daudé
2024-04-24 23:31 ` [PATCH v2 24/33] target/microblaze: Use translator_ldl Richard Henderson
` (10 subsequent siblings)
33 siblings, 1 reply; 58+ messages in thread
From: Richard Henderson @ 2024-04-24 23:31 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/hexagon/translate.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/target/hexagon/translate.c b/target/hexagon/translate.c
index 1344a3e4ab..37dec8b5c5 100644
--- a/target/hexagon/translate.c
+++ b/target/hexagon/translate.c
@@ -24,7 +24,6 @@
#include "exec/helper-proto.h"
#include "exec/translation-block.h"
#include "exec/log.h"
-#include "exec/cpu_ldst.h"
#include "internal.h"
#include "attribs.h"
#include "insn.h"
@@ -1085,7 +1084,7 @@ static bool pkt_crosses_page(CPUHexagonState *env, DisasContext *ctx)
int nwords;
for (nwords = 0; !found_end && nwords < PACKET_WORDS_MAX; nwords++) {
- uint32_t word = cpu_ldl_code(env,
+ uint32_t word = translator_ldl(env, &ctx->base,
ctx->base.pc_next + nwords * sizeof(uint32_t));
found_end = is_packet_end(word);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 58+ messages in thread* [PATCH v2 24/33] target/microblaze: Use translator_ldl
2024-04-24 23:30 [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Richard Henderson
` (22 preceding siblings ...)
2024-04-24 23:31 ` [PATCH v2 23/33] target/hexagon: Use translator_ldl in pkt_crosses_page Richard Henderson
@ 2024-04-24 23:31 ` Richard Henderson
2024-04-24 23:31 ` [PATCH v2 25/33] target/i386: Use translator_ldub for everything Richard Henderson
` (9 subsequent siblings)
33 siblings, 0 replies; 58+ messages in thread
From: Richard Henderson @ 2024-04-24 23:31 UTC (permalink / raw)
To: qemu-devel; +Cc: Edgar E . Iglesias, Philippe Mathieu-Daudé
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/microblaze/translate.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index 08d98dab07..bac7e30cd1 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -24,7 +24,6 @@
#include "tcg/tcg-op.h"
#include "exec/helper-proto.h"
#include "exec/helper-gen.h"
-#include "exec/cpu_ldst.h"
#include "exec/translator.h"
#include "qemu/qemu-print.h"
@@ -1636,7 +1635,7 @@ static void mb_tr_translate_insn(DisasContextBase *dcb, CPUState *cs)
dc->tb_flags_to_set = 0;
- ir = cpu_ldl_code(cpu_env(cs), dc->base.pc_next);
+ ir = translator_ldl(cpu_env(cs), &dc->base, dc->base.pc_next);
if (!decode(dc, ir)) {
trap_illegal(dc, true);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 58+ messages in thread* [PATCH v2 25/33] target/i386: Use translator_ldub for everything
2024-04-24 23:30 [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Richard Henderson
` (23 preceding siblings ...)
2024-04-24 23:31 ` [PATCH v2 24/33] target/microblaze: Use translator_ldl Richard Henderson
@ 2024-04-24 23:31 ` Richard Henderson
2024-04-24 23:31 ` [PATCH v2 26/33] target/avr: Use translator_lduw Richard Henderson
` (8 subsequent siblings)
33 siblings, 0 replies; 58+ messages in thread
From: Richard Henderson @ 2024-04-24 23:31 UTC (permalink / raw)
To: qemu-devel; +Cc: Philippe Mathieu-Daudé
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/i386/tcg/translate.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 6658adc4bc..6f04a77eed 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -23,7 +23,6 @@
#include "exec/exec-all.h"
#include "tcg/tcg-op.h"
#include "tcg/tcg-op-gvec.h"
-#include "exec/cpu_ldst.h"
#include "exec/translator.h"
#include "fpu/softfloat.h"
@@ -2138,9 +2137,8 @@ static uint64_t advance_pc(CPUX86State *env, DisasContext *s, int num_bytes)
* This can happen even if the operand is only one byte long!
*/
if (((s->pc - 1) ^ (pc - 1)) & TARGET_PAGE_MASK) {
- volatile uint8_t unused =
- cpu_ldub_code(env, (s->pc - 1) & TARGET_PAGE_MASK);
- (void) unused;
+ (void)translator_ldub(env, &s->base,
+ (s->pc - 1) & TARGET_PAGE_MASK);
}
siglongjmp(s->jmpbuf, 1);
}
@@ -2718,7 +2716,7 @@ static void gen_unknown_opcode(CPUX86State *env, DisasContext *s)
fprintf(logfile, "ILLOPC: " TARGET_FMT_lx ":", pc);
for (; pc < end; ++pc) {
- fprintf(logfile, " %02x", cpu_ldub_code(env, pc));
+ fprintf(logfile, " %02x", translator_ldub(env, &s->base, pc));
}
fprintf(logfile, "\n");
qemu_log_unlock(logfile);
--
2.34.1
^ permalink raw reply related [flat|nested] 58+ messages in thread* [PATCH v2 26/33] target/avr: Use translator_lduw
2024-04-24 23:30 [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Richard Henderson
` (24 preceding siblings ...)
2024-04-24 23:31 ` [PATCH v2 25/33] target/i386: Use translator_ldub for everything Richard Henderson
@ 2024-04-24 23:31 ` Richard Henderson
2024-04-24 23:31 ` [PATCH v2 27/33] target/cris: Use translator_ld* in cris_fetch Richard Henderson
` (7 subsequent siblings)
33 siblings, 0 replies; 58+ messages in thread
From: Richard Henderson @ 2024-04-24 23:31 UTC (permalink / raw)
To: qemu-devel; +Cc: Philippe Mathieu-Daudé
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/avr/translate.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/target/avr/translate.c b/target/avr/translate.c
index 6df93d4c77..2d51892115 100644
--- a/target/avr/translate.c
+++ b/target/avr/translate.c
@@ -24,7 +24,6 @@
#include "cpu.h"
#include "exec/exec-all.h"
#include "tcg/tcg-op.h"
-#include "exec/cpu_ldst.h"
#include "exec/helper-proto.h"
#include "exec/helper-gen.h"
#include "exec/log.h"
@@ -173,7 +172,7 @@ static int to_regs_00_30_by_two(DisasContext *ctx, int indx)
static uint16_t next_word(DisasContext *ctx)
{
- return cpu_lduw_code(ctx->env, ctx->npc++ * 2);
+ return translator_lduw(ctx->env, &ctx->base, ctx->npc++ * 2);
}
static int append_16(DisasContext *ctx, int x)
--
2.34.1
^ permalink raw reply related [flat|nested] 58+ messages in thread* [PATCH v2 27/33] target/cris: Use translator_ld* in cris_fetch
2024-04-24 23:30 [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Richard Henderson
` (25 preceding siblings ...)
2024-04-24 23:31 ` [PATCH v2 26/33] target/avr: Use translator_lduw Richard Henderson
@ 2024-04-24 23:31 ` Richard Henderson
2024-04-24 23:31 ` [PATCH v2 28/33] target/cris: Use cris_fetch in translate_v10.c.inc Richard Henderson
` (6 subsequent siblings)
33 siblings, 0 replies; 58+ messages in thread
From: Richard Henderson @ 2024-04-24 23:31 UTC (permalink / raw)
To: qemu-devel; +Cc: Edgar E . Iglesias, Philippe Mathieu-Daudé
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/cris/translate.c | 25 ++++++++-----------------
1 file changed, 8 insertions(+), 17 deletions(-)
diff --git a/target/cris/translate.c b/target/cris/translate.c
index b5410189d4..bb2d6612ba 100644
--- a/target/cris/translate.c
+++ b/target/cris/translate.c
@@ -222,37 +222,28 @@ static int sign_extend(unsigned int val, unsigned int width)
}
static int cris_fetch(CPUCRISState *env, DisasContext *dc, uint32_t addr,
- unsigned int size, unsigned int sign)
+ unsigned int size, bool sign)
{
int r;
switch (size) {
case 4:
- {
- r = cpu_ldl_code(env, addr);
+ r = translator_ldl(env, &dc->base, addr);
break;
- }
case 2:
- {
+ r = translator_lduw(env, &dc->base, addr);
if (sign) {
- r = cpu_ldsw_code(env, addr);
- } else {
- r = cpu_lduw_code(env, addr);
+ r = (int16_t)r;
}
break;
- }
case 1:
- {
+ r = translator_ldub(env, &dc->base, addr);
if (sign) {
- r = cpu_ldsb_code(env, addr);
- } else {
- r = cpu_ldub_code(env, addr);
+ r = (int8_t)r;
}
break;
- }
default:
- cpu_abort(CPU(dc->cpu), "Invalid fetch size %d\n", size);
- break;
+ g_assert_not_reached();
}
return r;
}
@@ -2868,7 +2859,7 @@ static unsigned int crisv32_decoder(CPUCRISState *env, DisasContext *dc)
int i;
/* Load a halfword onto the instruction register. */
- dc->ir = cris_fetch(env, dc, dc->pc, 2, 0);
+ dc->ir = cris_fetch(env, dc, dc->pc, 2, 0);
/* Now decode it. */
dc->opcode = EXTRACT_FIELD(dc->ir, 4, 11);
--
2.34.1
^ permalink raw reply related [flat|nested] 58+ messages in thread* [PATCH v2 28/33] target/cris: Use cris_fetch in translate_v10.c.inc
2024-04-24 23:30 [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Richard Henderson
` (26 preceding siblings ...)
2024-04-24 23:31 ` [PATCH v2 27/33] target/cris: Use translator_ld* in cris_fetch Richard Henderson
@ 2024-04-24 23:31 ` Richard Henderson
2024-04-24 23:31 ` [PATCH v2 29/33] target/riscv: Use translator_ld* for everything Richard Henderson
` (5 subsequent siblings)
33 siblings, 0 replies; 58+ messages in thread
From: Richard Henderson @ 2024-04-24 23:31 UTC (permalink / raw)
To: qemu-devel; +Cc: Edgar E . Iglesias, Philippe Mathieu-Daudé
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/cris/translate.c | 1 -
target/cris/translate_v10.c.inc | 30 +++++++++---------------------
2 files changed, 9 insertions(+), 22 deletions(-)
diff --git a/target/cris/translate.c b/target/cris/translate.c
index bb2d6612ba..a30c67eb07 100644
--- a/target/cris/translate.c
+++ b/target/cris/translate.c
@@ -29,7 +29,6 @@
#include "tcg/tcg-op.h"
#include "exec/helper-proto.h"
#include "mmu.h"
-#include "exec/cpu_ldst.h"
#include "exec/translator.h"
#include "crisv32-decode.h"
#include "qemu/qemu-print.h"
diff --git a/target/cris/translate_v10.c.inc b/target/cris/translate_v10.c.inc
index 73fc27c15d..c15ff47505 100644
--- a/target/cris/translate_v10.c.inc
+++ b/target/cris/translate_v10.c.inc
@@ -165,20 +165,7 @@ static int dec10_prep_move_m(CPUCRISState *env, DisasContext *dc,
/* Load [$rs] onto T1. */
if (is_imm) {
- if (memsize != 4) {
- if (s_ext) {
- if (memsize == 1)
- imm = cpu_ldsb_code(env, dc->pc + 2);
- else
- imm = cpu_ldsw_code(env, dc->pc + 2);
- } else {
- if (memsize == 1)
- imm = cpu_ldub_code(env, dc->pc + 2);
- else
- imm = cpu_lduw_code(env, dc->pc + 2);
- }
- } else
- imm = cpu_ldl_code(env, dc->pc + 2);
+ imm = cris_fetch(env, dc, dc->pc + 2, memsize, s_ext);
tcg_gen_movi_tl(dst, imm);
@@ -929,10 +916,11 @@ static int dec10_dip(CPUCRISState *env, DisasContext *dc)
LOG_DIS("dip pc=%x opcode=%d r%d r%d\n",
dc->pc, dc->opcode, dc->src, dc->dst);
if (dc->src == 15) {
- imm = cpu_ldl_code(env, dc->pc + 2);
+ imm = cris_fetch(env, dc, dc->pc + 2, 4, 0);
tcg_gen_movi_tl(cpu_PR[PR_PREFIX], imm);
- if (dc->postinc)
+ if (dc->postinc) {
insn_len += 4;
+ }
tcg_gen_addi_tl(cpu_R[15], cpu_R[15], insn_len - 2);
} else {
gen_load(dc, cpu_PR[PR_PREFIX], cpu_R[dc->src], 4, 0);
@@ -1095,10 +1083,10 @@ static unsigned int dec10_ind(CPUCRISState *env, DisasContext *dc)
if (dc->src == 15) {
LOG_DIS("jump.%d %d r%d r%d direct\n", size,
dc->opcode, dc->src, dc->dst);
- imm = cpu_ldl_code(env, dc->pc + 2);
- if (dc->mode == CRISV10_MODE_AUTOINC)
+ imm = cris_fetch(env, dc, dc->pc + 2, size, 0);
+ if (dc->mode == CRISV10_MODE_AUTOINC) {
insn_len += size;
-
+ }
c = tcg_constant_tl(dc->pc + insn_len);
t_gen_mov_preg_TN(dc, dc->dst, c);
dc->jmp_pc = imm;
@@ -1164,7 +1152,7 @@ static unsigned int dec10_ind(CPUCRISState *env, DisasContext *dc)
case CRISV10_IND_BCC_M:
cris_cc_mask(dc, 0);
- simm = cpu_ldsw_code(env, dc->pc + 2);
+ simm = cris_fetch(env, dc, dc->pc + 2, 2, 1);
simm += 4;
LOG_DIS("bcc_m: b%s %x\n", cc_name(dc->cond), dc->pc + simm);
@@ -1185,7 +1173,7 @@ static unsigned int crisv10_decoder(CPUCRISState *env, DisasContext *dc)
unsigned int insn_len = 2;
/* Load a halfword onto the instruction register. */
- dc->ir = cpu_lduw_code(env, dc->pc);
+ dc->ir = cris_fetch(env, dc, dc->pc, 2, 0);
/* Now decode it. */
dc->opcode = EXTRACT_FIELD(dc->ir, 6, 9);
--
2.34.1
^ permalink raw reply related [flat|nested] 58+ messages in thread* [PATCH v2 29/33] target/riscv: Use translator_ld* for everything
2024-04-24 23:30 [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Richard Henderson
` (27 preceding siblings ...)
2024-04-24 23:31 ` [PATCH v2 28/33] target/cris: Use cris_fetch in translate_v10.c.inc Richard Henderson
@ 2024-04-24 23:31 ` Richard Henderson
2024-04-29 10:55 ` Philippe Mathieu-Daudé
` (2 more replies)
2024-04-24 23:31 ` [PATCH v2 30/33] target/rx: Use translator_ld* Richard Henderson
` (4 subsequent siblings)
33 siblings, 3 replies; 58+ messages in thread
From: Richard Henderson @ 2024-04-24 23:31 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/riscv/translate.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index c999e942e1..2c27fd4ce1 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -20,7 +20,6 @@
#include "qemu/log.h"
#include "cpu.h"
#include "tcg/tcg-op.h"
-#include "exec/cpu_ldst.h"
#include "exec/exec-all.h"
#include "exec/helper-proto.h"
#include "exec/helper-gen.h"
@@ -1082,7 +1081,7 @@ static uint32_t opcode_at(DisasContextBase *dcbase, target_ulong pc)
CPUState *cpu = ctx->cs;
CPURISCVState *env = cpu_env(cpu);
- return cpu_ldl_code(env, pc);
+ return translator_ldl(env, &ctx->base, pc);
}
/* Include insn module translation function */
@@ -1243,7 +1242,8 @@ static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
unsigned page_ofs = ctx->base.pc_next & ~TARGET_PAGE_MASK;
if (page_ofs > TARGET_PAGE_SIZE - MAX_INSN_LEN) {
- uint16_t next_insn = cpu_lduw_code(env, ctx->base.pc_next);
+ uint16_t next_insn =
+ translator_lduw(env, &ctx->base, ctx->base.pc_next);
int len = insn_len(next_insn);
if (!is_same_page(&ctx->base, ctx->base.pc_next + len - 1)) {
--
2.34.1
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [PATCH v2 29/33] target/riscv: Use translator_ld* for everything
2024-04-24 23:31 ` [PATCH v2 29/33] target/riscv: Use translator_ld* for everything Richard Henderson
@ 2024-04-29 10:55 ` Philippe Mathieu-Daudé
2024-05-08 16:31 ` Philippe Mathieu-Daudé
2024-05-14 6:25 ` Alistair Francis
2 siblings, 0 replies; 58+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-29 10:55 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-riscv
Cc'ing qemu-riscv@
On 25/4/24 01:31, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/riscv/translate.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index c999e942e1..2c27fd4ce1 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -20,7 +20,6 @@
> #include "qemu/log.h"
> #include "cpu.h"
> #include "tcg/tcg-op.h"
> -#include "exec/cpu_ldst.h"
> #include "exec/exec-all.h"
> #include "exec/helper-proto.h"
> #include "exec/helper-gen.h"
> @@ -1082,7 +1081,7 @@ static uint32_t opcode_at(DisasContextBase *dcbase, target_ulong pc)
> CPUState *cpu = ctx->cs;
> CPURISCVState *env = cpu_env(cpu);
>
> - return cpu_ldl_code(env, pc);
> + return translator_ldl(env, &ctx->base, pc);
> }
>
> /* Include insn module translation function */
> @@ -1243,7 +1242,8 @@ static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
> unsigned page_ofs = ctx->base.pc_next & ~TARGET_PAGE_MASK;
>
> if (page_ofs > TARGET_PAGE_SIZE - MAX_INSN_LEN) {
> - uint16_t next_insn = cpu_lduw_code(env, ctx->base.pc_next);
> + uint16_t next_insn =
> + translator_lduw(env, &ctx->base, ctx->base.pc_next);
> int len = insn_len(next_insn);
>
> if (!is_same_page(&ctx->base, ctx->base.pc_next + len - 1)) {
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [PATCH v2 29/33] target/riscv: Use translator_ld* for everything
2024-04-24 23:31 ` [PATCH v2 29/33] target/riscv: Use translator_ld* for everything Richard Henderson
2024-04-29 10:55 ` Philippe Mathieu-Daudé
@ 2024-05-08 16:31 ` Philippe Mathieu-Daudé
2024-05-14 6:25 ` Alistair Francis
2 siblings, 0 replies; 58+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-05-08 16:31 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
On 25/4/24 01:31, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/riscv/translate.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v2 29/33] target/riscv: Use translator_ld* for everything
2024-04-24 23:31 ` [PATCH v2 29/33] target/riscv: Use translator_ld* for everything Richard Henderson
2024-04-29 10:55 ` Philippe Mathieu-Daudé
2024-05-08 16:31 ` Philippe Mathieu-Daudé
@ 2024-05-14 6:25 ` Alistair Francis
2 siblings, 0 replies; 58+ messages in thread
From: Alistair Francis @ 2024-05-14 6:25 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
On Thu, Apr 25, 2024 at 9:36 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/translate.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index c999e942e1..2c27fd4ce1 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -20,7 +20,6 @@
> #include "qemu/log.h"
> #include "cpu.h"
> #include "tcg/tcg-op.h"
> -#include "exec/cpu_ldst.h"
> #include "exec/exec-all.h"
> #include "exec/helper-proto.h"
> #include "exec/helper-gen.h"
> @@ -1082,7 +1081,7 @@ static uint32_t opcode_at(DisasContextBase *dcbase, target_ulong pc)
> CPUState *cpu = ctx->cs;
> CPURISCVState *env = cpu_env(cpu);
>
> - return cpu_ldl_code(env, pc);
> + return translator_ldl(env, &ctx->base, pc);
> }
>
> /* Include insn module translation function */
> @@ -1243,7 +1242,8 @@ static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
> unsigned page_ofs = ctx->base.pc_next & ~TARGET_PAGE_MASK;
>
> if (page_ofs > TARGET_PAGE_SIZE - MAX_INSN_LEN) {
> - uint16_t next_insn = cpu_lduw_code(env, ctx->base.pc_next);
> + uint16_t next_insn =
> + translator_lduw(env, &ctx->base, ctx->base.pc_next);
> int len = insn_len(next_insn);
>
> if (!is_same_page(&ctx->base, ctx->base.pc_next + len - 1)) {
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 58+ messages in thread
* [PATCH v2 30/33] target/rx: Use translator_ld*
2024-04-24 23:30 [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Richard Henderson
` (28 preceding siblings ...)
2024-04-24 23:31 ` [PATCH v2 29/33] target/riscv: Use translator_ld* for everything Richard Henderson
@ 2024-04-24 23:31 ` Richard Henderson
2024-04-24 23:31 ` [PATCH v2 31/33] target/xtensa: Use translator_ldub in xtensa_insn_len Richard Henderson
` (3 subsequent siblings)
33 siblings, 0 replies; 58+ messages in thread
From: Richard Henderson @ 2024-04-24 23:31 UTC (permalink / raw)
To: qemu-devel; +Cc: Yoshinori Sato, Philippe Mathieu-Daudé
Reviewed-by: Yoshinori Sato <ysato@users.sourceforge.jp>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/rx/translate.c | 27 ++++++++++++++-------------
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/target/rx/translate.c b/target/rx/translate.c
index 92fb2b43ad..9b81cf20b3 100644
--- a/target/rx/translate.c
+++ b/target/rx/translate.c
@@ -22,7 +22,6 @@
#include "cpu.h"
#include "exec/exec-all.h"
#include "tcg/tcg-op.h"
-#include "exec/cpu_ldst.h"
#include "exec/helper-proto.h"
#include "exec/helper-gen.h"
#include "exec/translator.h"
@@ -75,10 +74,10 @@ static TCGv_i64 cpu_acc;
/* decoder helper */
static uint32_t decode_load_bytes(DisasContext *ctx, uint32_t insn,
- int i, int n)
+ int i, int n)
{
while (++i <= n) {
- uint8_t b = cpu_ldub_code(ctx->env, ctx->base.pc_next++);
+ uint8_t b = translator_ldub(ctx->env, &ctx->base, ctx->base.pc_next++);
insn |= b << (32 - i * 8);
}
return insn;
@@ -90,22 +89,24 @@ static uint32_t li(DisasContext *ctx, int sz)
CPURXState *env = ctx->env;
addr = ctx->base.pc_next;
- tcg_debug_assert(sz < 4);
switch (sz) {
case 1:
ctx->base.pc_next += 1;
- return cpu_ldsb_code(env, addr);
+ return (int8_t)translator_ldub(env, &ctx->base, addr);
case 2:
ctx->base.pc_next += 2;
- return cpu_ldsw_code(env, addr);
+ return (int16_t)translator_lduw(env, &ctx->base, addr);
case 3:
ctx->base.pc_next += 3;
- tmp = cpu_ldsb_code(env, addr + 2) << 16;
- tmp |= cpu_lduw_code(env, addr) & 0xffff;
+ tmp = (int8_t)translator_ldub(env, &ctx->base, addr + 2);
+ tmp <<= 16;
+ tmp |= translator_lduw(env, &ctx->base, addr);
return tmp;
case 0:
ctx->base.pc_next += 4;
- return cpu_ldl_code(env, addr);
+ return translator_ldl(env, &ctx->base, addr);
+ default:
+ g_assert_not_reached();
}
return 0;
}
@@ -190,22 +191,22 @@ static inline TCGv rx_index_addr(DisasContext *ctx, TCGv mem,
{
uint32_t dsp;
- tcg_debug_assert(ld < 3);
switch (ld) {
case 0:
return cpu_regs[reg];
case 1:
- dsp = cpu_ldub_code(ctx->env, ctx->base.pc_next) << size;
+ dsp = translator_ldub(ctx->env, &ctx->base, ctx->base.pc_next) << size;
tcg_gen_addi_i32(mem, cpu_regs[reg], dsp);
ctx->base.pc_next += 1;
return mem;
case 2:
- dsp = cpu_lduw_code(ctx->env, ctx->base.pc_next) << size;
+ dsp = translator_lduw(ctx->env, &ctx->base, ctx->base.pc_next) << size;
tcg_gen_addi_i32(mem, cpu_regs[reg], dsp);
ctx->base.pc_next += 2;
return mem;
+ default:
+ g_assert_not_reached();
}
- return NULL;
}
static inline MemOp mi_to_mop(unsigned mi)
--
2.34.1
^ permalink raw reply related [flat|nested] 58+ messages in thread* [PATCH v2 31/33] target/xtensa: Use translator_ldub in xtensa_insn_len
2024-04-24 23:30 [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Richard Henderson
` (29 preceding siblings ...)
2024-04-24 23:31 ` [PATCH v2 30/33] target/rx: Use translator_ld* Richard Henderson
@ 2024-04-24 23:31 ` Richard Henderson
2024-04-24 23:31 ` [PATCH v2 32/33] target/s390x: Use translator_lduw in get_next_pc Richard Henderson
` (2 subsequent siblings)
33 siblings, 0 replies; 58+ messages in thread
From: Richard Henderson @ 2024-04-24 23:31 UTC (permalink / raw)
To: qemu-devel; +Cc: Philippe Mathieu-Daudé
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/xtensa/translate.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/target/xtensa/translate.c b/target/xtensa/translate.c
index 42109d33ad..75b7bfda4c 100644
--- a/target/xtensa/translate.c
+++ b/target/xtensa/translate.c
@@ -35,7 +35,6 @@
#include "tcg/tcg-op.h"
#include "qemu/log.h"
#include "qemu/qemu-print.h"
-#include "exec/cpu_ldst.h"
#include "semihosting/semihost.h"
#include "exec/translator.h"
@@ -1118,7 +1117,7 @@ static void disas_xtensa_insn(CPUXtensaState *env, DisasContext *dc)
static inline unsigned xtensa_insn_len(CPUXtensaState *env, DisasContext *dc)
{
- uint8_t b0 = cpu_ldub_code(env, dc->pc);
+ uint8_t b0 = translator_ldub(env, &dc->base, dc->pc);
return xtensa_op0_insn_len(dc, b0);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 58+ messages in thread* [PATCH v2 32/33] target/s390x: Use translator_lduw in get_next_pc
2024-04-24 23:30 [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Richard Henderson
` (30 preceding siblings ...)
2024-04-24 23:31 ` [PATCH v2 31/33] target/xtensa: Use translator_ldub in xtensa_insn_len Richard Henderson
@ 2024-04-24 23:31 ` Richard Henderson
2024-05-06 22:41 ` Philippe Mathieu-Daudé
2024-04-24 23:31 ` [PATCH v2 33/33] accel/tcg: Remove cpu_ldsb_code / cpu_ldsw_code Richard Henderson
2024-04-29 10:52 ` [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Philippe Mathieu-Daudé
33 siblings, 1 reply; 58+ messages in thread
From: Richard Henderson @ 2024-04-24 23:31 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/s390x/tcg/translate.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index bac033c63c..ebd96abe6c 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -36,7 +36,6 @@
#include "tcg/tcg-op-gvec.h"
#include "qemu/log.h"
#include "qemu/host-utils.h"
-#include "exec/cpu_ldst.h"
#include "exec/helper-proto.h"
#include "exec/helper-gen.h"
@@ -6471,7 +6470,7 @@ static void s390x_tr_insn_start(DisasContextBase *dcbase, CPUState *cs)
static target_ulong get_next_pc(CPUS390XState *env, DisasContext *s,
uint64_t pc)
{
- uint64_t insn = cpu_lduw_code(env, pc);
+ uint64_t insn = translator_lduw(env, &s->base, pc);
return pc + get_ilen((insn >> 8) & 0xff);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 58+ messages in thread* [PATCH v2 33/33] accel/tcg: Remove cpu_ldsb_code / cpu_ldsw_code
2024-04-24 23:30 [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Richard Henderson
` (31 preceding siblings ...)
2024-04-24 23:31 ` [PATCH v2 32/33] target/s390x: Use translator_lduw in get_next_pc Richard Henderson
@ 2024-04-24 23:31 ` Richard Henderson
2024-04-29 10:52 ` [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Philippe Mathieu-Daudé
33 siblings, 0 replies; 58+ messages in thread
From: Richard Henderson @ 2024-04-24 23:31 UTC (permalink / raw)
To: qemu-devel; +Cc: Philippe Mathieu-Daudé
From: Philippe Mathieu-Daudé <philmd@linaro.org>
Previous commits replaced them by translator_ld* calls.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20240405131532.40913-1-philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/cpu_ldst.h | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
index eb8f3f0595..85ca104dc9 100644
--- a/include/exec/cpu_ldst.h
+++ b/include/exec/cpu_ldst.h
@@ -438,16 +438,6 @@ uint32_t cpu_lduw_code(CPUArchState *env, abi_ptr addr);
uint32_t cpu_ldl_code(CPUArchState *env, abi_ptr addr);
uint64_t cpu_ldq_code(CPUArchState *env, abi_ptr addr);
-static inline int cpu_ldsb_code(CPUArchState *env, abi_ptr addr)
-{
- return (int8_t)cpu_ldub_code(env, addr);
-}
-
-static inline int cpu_ldsw_code(CPUArchState *env, abi_ptr addr)
-{
- return (int16_t)cpu_lduw_code(env, addr);
-}
-
/**
* tlb_vaddr_to_host:
* @env: CPUArchState
--
2.34.1
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin
2024-04-24 23:30 [PATCH v2 00/33] accel/tcg: Improve disassembly for target and plugin Richard Henderson
` (32 preceding siblings ...)
2024-04-24 23:31 ` [PATCH v2 33/33] accel/tcg: Remove cpu_ldsb_code / cpu_ldsw_code Richard Henderson
@ 2024-04-29 10:52 ` Philippe Mathieu-Daudé
33 siblings, 0 replies; 58+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-04-29 10:52 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-s390x
On 25/4/24 01:30, Richard Henderson wrote:
> Based-on: 20240424230224.941028-1-richard.henderson@linaro.org
> ("[PATCH v3 00/20] Rewrite plugin code generation")
Cc'ing qemu-s390x@ for these 3 patches:
> Richard Henderson (32):
> target/s390x: Fix translator_fake_ld length
> target/s390x: Disassemble EXECUTEd instructions
> target/s390x: Use translator_lduw in get_next_pc
(could be reviewed out of the series context IMO).
Thanks,
Phil.
^ permalink raw reply [flat|nested] 58+ messages in thread