* [PATCH v6 01/21] linux-user/arm: Mark the commpage executable
2022-08-19 3:25 [PATCH v6 00/21] linux-user: Fix siginfo_t contents when jumping to non-readable pages Richard Henderson
@ 2022-08-19 3:25 ` Richard Henderson
2022-08-19 3:25 ` [PATCH v6 02/21] linux-user/hppa: Allocate page zero as a commpage Richard Henderson
` (20 subsequent siblings)
21 siblings, 0 replies; 35+ messages in thread
From: Richard Henderson @ 2022-08-19 3:25 UTC (permalink / raw)
To: qemu-devel; +Cc: laurent, iii, dramforever, alistair.francis, alex.bennee
We're about to start validating PAGE_EXEC, which means
that we've got to mark the commpage executable. We had
been placing the commpage outside of reserved_va, which
was incorrect and lead to an abort.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
linux-user/arm/target_cpu.h | 4 ++--
linux-user/elfload.c | 6 +++++-
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/linux-user/arm/target_cpu.h b/linux-user/arm/target_cpu.h
index 709d19bc9e..89ba274cfc 100644
--- a/linux-user/arm/target_cpu.h
+++ b/linux-user/arm/target_cpu.h
@@ -34,9 +34,9 @@ static inline unsigned long arm_max_reserved_va(CPUState *cs)
} else {
/*
* We need to be able to map the commpage.
- * See validate_guest_space in linux-user/elfload.c.
+ * See init_guest_commpage in linux-user/elfload.c.
*/
- return 0xffff0000ul;
+ return 0xfffffffful;
}
}
#define MAX_RESERVED_VA arm_max_reserved_va
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index ce902dbd56..3e3dc02499 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -398,7 +398,8 @@ enum {
static bool init_guest_commpage(void)
{
- void *want = g2h_untagged(HI_COMMPAGE & -qemu_host_page_size);
+ abi_ptr commpage = HI_COMMPAGE & -qemu_host_page_size;
+ void *want = g2h_untagged(commpage);
void *addr = mmap(want, qemu_host_page_size, PROT_READ | PROT_WRITE,
MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, -1, 0);
@@ -417,6 +418,9 @@ static bool init_guest_commpage(void)
perror("Protecting guest commpage");
exit(EXIT_FAILURE);
}
+
+ page_set_flags(commpage, commpage + qemu_host_page_size,
+ PAGE_READ | PAGE_EXEC | PAGE_VALID);
return true;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v6 02/21] linux-user/hppa: Allocate page zero as a commpage
2022-08-19 3:25 [PATCH v6 00/21] linux-user: Fix siginfo_t contents when jumping to non-readable pages Richard Henderson
2022-08-19 3:25 ` [PATCH v6 01/21] linux-user/arm: Mark the commpage executable Richard Henderson
@ 2022-08-19 3:25 ` Richard Henderson
2022-08-19 3:25 ` [PATCH v6 03/21] linux-user/x86_64: Allocate vsyscall page " Richard Henderson
` (19 subsequent siblings)
21 siblings, 0 replies; 35+ messages in thread
From: Richard Henderson @ 2022-08-19 3:25 UTC (permalink / raw)
To: qemu-devel; +Cc: laurent, iii, dramforever, alistair.francis, alex.bennee
We're about to start validating PAGE_EXEC, which means that we've
got to mark page zero executable. We had been special casing this
entirely within translate.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
linux-user/elfload.c | 34 +++++++++++++++++++++++++++++++---
1 file changed, 31 insertions(+), 3 deletions(-)
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 3e3dc02499..29d910c4cc 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -1646,6 +1646,34 @@ static inline void init_thread(struct target_pt_regs *regs,
regs->gr[31] = infop->entry;
}
+#define LO_COMMPAGE 0
+
+static bool init_guest_commpage(void)
+{
+ void *want = g2h_untagged(LO_COMMPAGE);
+ void *addr = mmap(want, qemu_host_page_size, PROT_NONE,
+ MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, -1, 0);
+
+ if (addr == MAP_FAILED) {
+ perror("Allocating guest commpage");
+ exit(EXIT_FAILURE);
+ }
+ if (addr != want) {
+ return false;
+ }
+
+ /*
+ * On Linux, page zero is normally marked execute only + gateway.
+ * Normal read or write is supposed to fail (thus PROT_NONE above),
+ * but specific offsets have kernel code mapped to raise permissions
+ * and implement syscalls. Here, simply mark the page executable.
+ * Special case the entry points during translation (see do_page_zero).
+ */
+ page_set_flags(LO_COMMPAGE, LO_COMMPAGE + TARGET_PAGE_SIZE,
+ PAGE_EXEC | PAGE_VALID);
+ return true;
+}
+
#endif /* TARGET_HPPA */
#ifdef TARGET_XTENSA
@@ -2326,12 +2354,12 @@ static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc,
}
#if defined(HI_COMMPAGE)
-#define LO_COMMPAGE 0
+#define LO_COMMPAGE -1
#elif defined(LO_COMMPAGE)
#define HI_COMMPAGE 0
#else
#define HI_COMMPAGE 0
-#define LO_COMMPAGE 0
+#define LO_COMMPAGE -1
#define init_guest_commpage() true
#endif
@@ -2555,7 +2583,7 @@ static void pgb_static(const char *image_name, abi_ulong orig_loaddr,
} else {
offset = -(HI_COMMPAGE & -align);
}
- } else if (LO_COMMPAGE != 0) {
+ } else if (LO_COMMPAGE != -1) {
loaddr = MIN(loaddr, LO_COMMPAGE & -align);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v6 03/21] linux-user/x86_64: Allocate vsyscall page as a commpage
2022-08-19 3:25 [PATCH v6 00/21] linux-user: Fix siginfo_t contents when jumping to non-readable pages Richard Henderson
2022-08-19 3:25 ` [PATCH v6 01/21] linux-user/arm: Mark the commpage executable Richard Henderson
2022-08-19 3:25 ` [PATCH v6 02/21] linux-user/hppa: Allocate page zero as a commpage Richard Henderson
@ 2022-08-19 3:25 ` Richard Henderson
2022-08-19 3:25 ` [PATCH v6 04/21] linux-user: Honor PT_GNU_STACK Richard Henderson
` (18 subsequent siblings)
21 siblings, 0 replies; 35+ messages in thread
From: Richard Henderson @ 2022-08-19 3:25 UTC (permalink / raw)
To: qemu-devel; +Cc: laurent, iii, dramforever, alistair.francis, alex.bennee
We're about to start validating PAGE_EXEC, which means that we've
got to the vsyscall page executable. We had been special casing
this entirely within translate.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
linux-user/elfload.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 29d910c4cc..b20d513929 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -195,6 +195,27 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs, const CPUX86State *en
(*regs)[26] = tswapreg(env->segs[R_GS].selector & 0xffff);
}
+#if ULONG_MAX >= TARGET_VSYSCALL_PAGE
+#define INIT_GUEST_COMMPAGE
+static bool init_guest_commpage(void)
+{
+ /*
+ * The vsyscall page is at a high negative address aka kernel space,
+ * which means that we cannot actually allocate it with target_mmap.
+ * We still should be able to use page_set_flags, unless the user
+ * has specified -R reserved_va, which would trigger an assert().
+ */
+ if (reserved_va != 0 &&
+ TARGET_VSYSCALL_PAGE + TARGET_PAGE_SIZE >= reserved_va) {
+ error_report("Cannot allocate vsyscall page");
+ exit(EXIT_FAILURE);
+ }
+ page_set_flags(TARGET_VSYSCALL_PAGE,
+ TARGET_VSYSCALL_PAGE + TARGET_PAGE_SIZE,
+ PAGE_EXEC | PAGE_VALID);
+ return true;
+}
+#endif
#else
#define ELF_START_MMAP 0x80000000
@@ -2360,8 +2381,10 @@ static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc,
#else
#define HI_COMMPAGE 0
#define LO_COMMPAGE -1
+#ifndef INIT_GUEST_COMMPAGE
#define init_guest_commpage() true
#endif
+#endif
static void pgb_fail_in_use(const char *image_name)
{
--
2.34.1
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v6 04/21] linux-user: Honor PT_GNU_STACK
2022-08-19 3:25 [PATCH v6 00/21] linux-user: Fix siginfo_t contents when jumping to non-readable pages Richard Henderson
` (2 preceding siblings ...)
2022-08-19 3:25 ` [PATCH v6 03/21] linux-user/x86_64: Allocate vsyscall page " Richard Henderson
@ 2022-08-19 3:25 ` Richard Henderson
2022-08-19 3:25 ` [PATCH v6 05/21] linux-user: Clear translations and tb_jmp_cache on mprotect() Richard Henderson
` (17 subsequent siblings)
21 siblings, 0 replies; 35+ messages in thread
From: Richard Henderson @ 2022-08-19 3:25 UTC (permalink / raw)
To: qemu-devel; +Cc: laurent, iii, dramforever, alistair.francis, alex.bennee
Map the stack executable if required by default or on demand.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/elf.h | 1 +
linux-user/qemu.h | 1 +
linux-user/elfload.c | 19 ++++++++++++++++++-
3 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/include/elf.h b/include/elf.h
index 3a4bcb646a..3d6b9062c0 100644
--- a/include/elf.h
+++ b/include/elf.h
@@ -31,6 +31,7 @@ typedef int64_t Elf64_Sxword;
#define PT_LOPROC 0x70000000
#define PT_HIPROC 0x7fffffff
+#define PT_GNU_STACK (PT_LOOS + 0x474e551)
#define PT_GNU_PROPERTY (PT_LOOS + 0x474e553)
#define PT_MIPS_REGINFO 0x70000000
diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 7d90de1b15..e2e93fbd1d 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -48,6 +48,7 @@ struct image_info {
uint32_t elf_flags;
int personality;
abi_ulong alignment;
+ bool exec_stack;
/* Generic semihosting knows about these pointers. */
abi_ulong arg_strings; /* strings for argv */
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index b20d513929..90375c6b74 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -232,6 +232,7 @@ static bool init_guest_commpage(void)
#define ELF_ARCH EM_386
#define ELF_PLATFORM get_elf_platform()
+#define EXSTACK_DEFAULT true
static const char *get_elf_platform(void)
{
@@ -308,6 +309,7 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs, const CPUX86State *en
#define ELF_ARCH EM_ARM
#define ELF_CLASS ELFCLASS32
+#define EXSTACK_DEFAULT true
static inline void init_thread(struct target_pt_regs *regs,
struct image_info *infop)
@@ -776,6 +778,7 @@ static inline void init_thread(struct target_pt_regs *regs,
#else
#define ELF_CLASS ELFCLASS32
+#define EXSTACK_DEFAULT true
#endif
@@ -973,6 +976,7 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs, const CPUPPCState *en
#define ELF_CLASS ELFCLASS64
#define ELF_ARCH EM_LOONGARCH
+#define EXSTACK_DEFAULT true
#define elf_check_arch(x) ((x) == EM_LOONGARCH)
@@ -1068,6 +1072,7 @@ static uint32_t get_elf_hwcap(void)
#define ELF_CLASS ELFCLASS32
#endif
#define ELF_ARCH EM_MIPS
+#define EXSTACK_DEFAULT true
#ifdef TARGET_ABI_MIPSN32
#define elf_check_abi(x) ((x) & EF_MIPS_ABI2)
@@ -1806,6 +1811,10 @@ static inline void init_thread(struct target_pt_regs *regs,
#define bswaptls(ptr) bswap32s(ptr)
#endif
+#ifndef EXSTACK_DEFAULT
+#define EXSTACK_DEFAULT false
+#endif
+
#include "elf.h"
/* We must delay the following stanzas until after "elf.h". */
@@ -2081,6 +2090,7 @@ static abi_ulong setup_arg_pages(struct linux_binprm *bprm,
struct image_info *info)
{
abi_ulong size, error, guard;
+ int prot;
size = guest_stack_size;
if (size < STACK_LOWER_LIMIT) {
@@ -2091,7 +2101,11 @@ static abi_ulong setup_arg_pages(struct linux_binprm *bprm,
guard = qemu_real_host_page_size();
}
- error = target_mmap(0, size + guard, PROT_READ | PROT_WRITE,
+ prot = PROT_READ | PROT_WRITE;
+ if (info->exec_stack) {
+ prot |= PROT_EXEC;
+ }
+ error = target_mmap(0, size + guard, prot,
MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
if (error == -1) {
perror("mmap stack");
@@ -2921,6 +2935,7 @@ static void load_elf_image(const char *image_name, int image_fd,
*/
loaddr = -1, hiaddr = 0;
info->alignment = 0;
+ info->exec_stack = EXSTACK_DEFAULT;
for (i = 0; i < ehdr->e_phnum; ++i) {
struct elf_phdr *eppnt = phdr + i;
if (eppnt->p_type == PT_LOAD) {
@@ -2963,6 +2978,8 @@ static void load_elf_image(const char *image_name, int image_fd,
if (!parse_elf_properties(image_fd, info, eppnt, bprm_buf, &err)) {
goto exit_errmsg;
}
+ } else if (eppnt->p_type == PT_GNU_STACK) {
+ info->exec_stack = eppnt->p_flags & PF_X;
}
}
--
2.34.1
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v6 05/21] linux-user: Clear translations and tb_jmp_cache on mprotect()
2022-08-19 3:25 [PATCH v6 00/21] linux-user: Fix siginfo_t contents when jumping to non-readable pages Richard Henderson
` (3 preceding siblings ...)
2022-08-19 3:25 ` [PATCH v6 04/21] linux-user: Honor PT_GNU_STACK Richard Henderson
@ 2022-08-19 3:25 ` Richard Henderson
2022-08-19 3:26 ` [PATCH v6 06/21] tests/tcg/i386: Move smc_code2 to an executable section Richard Henderson
` (16 subsequent siblings)
21 siblings, 0 replies; 35+ messages in thread
From: Richard Henderson @ 2022-08-19 3:25 UTC (permalink / raw)
To: qemu-devel; +Cc: laurent, iii, dramforever, alistair.francis, alex.bennee
From: Ilya Leoshkevich <iii@linux.ibm.com>
Currently it's possible to execute pages that do not have PAGE_EXEC
if there is an existing translation block. Fix by clearing tb_jmp_cache
and invalidating TBs, which forces recheck of permission bits.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Message-Id: <20220817150506.592862-2-iii@linux.ibm.com>
[rth: Invalidate is required -- e.g. riscv fallthrough cross test]
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
fixup mprotect
---
linux-user/mmap.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 048c4135af..e9dc8848be 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -115,6 +115,7 @@ int target_mprotect(abi_ulong start, abi_ulong len, int target_prot)
{
abi_ulong end, host_start, host_end, addr;
int prot1, ret, page_flags, host_prot;
+ CPUState *cpu;
trace_target_mprotect(start, len, target_prot);
@@ -177,7 +178,14 @@ int target_mprotect(abi_ulong start, abi_ulong len, int target_prot)
goto error;
}
}
+
page_set_flags(start, start + len, page_flags);
+ tb_invalidate_phys_range(start, start + len);
+
+ CPU_FOREACH(cpu) {
+ cpu_tb_jmp_cache_clear(cpu);
+ }
+
mmap_unlock();
return 0;
error:
--
2.34.1
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v6 06/21] tests/tcg/i386: Move smc_code2 to an executable section
2022-08-19 3:25 [PATCH v6 00/21] linux-user: Fix siginfo_t contents when jumping to non-readable pages Richard Henderson
` (4 preceding siblings ...)
2022-08-19 3:25 ` [PATCH v6 05/21] linux-user: Clear translations and tb_jmp_cache on mprotect() Richard Henderson
@ 2022-08-19 3:26 ` Richard Henderson
2022-08-19 3:26 ` [PATCH v6 07/21] accel/tcg: Introduce is_same_page() Richard Henderson
` (15 subsequent siblings)
21 siblings, 0 replies; 35+ messages in thread
From: Richard Henderson @ 2022-08-19 3:26 UTC (permalink / raw)
To: qemu-devel; +Cc: laurent, iii, dramforever, alistair.francis, alex.bennee
We're about to start validating PAGE_EXEC, which means
that we've got to put this code into a section that is
both writable and executable.
Note that this test did not run on hardware beforehand either.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
tests/tcg/i386/test-i386.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/tcg/i386/test-i386.c b/tests/tcg/i386/test-i386.c
index ac8d5a3c1f..e6b308a2c0 100644
--- a/tests/tcg/i386/test-i386.c
+++ b/tests/tcg/i386/test-i386.c
@@ -1998,7 +1998,7 @@ uint8_t code[] = {
0xc3, /* ret */
};
-asm(".section \".data\"\n"
+asm(".section \".data_x\",\"awx\"\n"
"smc_code2:\n"
"movl 4(%esp), %eax\n"
"movl %eax, smc_patch_addr2 + 1\n"
--
2.34.1
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v6 07/21] accel/tcg: Introduce is_same_page()
2022-08-19 3:25 [PATCH v6 00/21] linux-user: Fix siginfo_t contents when jumping to non-readable pages Richard Henderson
` (5 preceding siblings ...)
2022-08-19 3:26 ` [PATCH v6 06/21] tests/tcg/i386: Move smc_code2 to an executable section Richard Henderson
@ 2022-08-19 3:26 ` Richard Henderson
2022-08-21 23:27 ` Alistair Francis
2022-08-19 3:26 ` [PATCH v6 08/21] accel/tcg: Properly implement get_page_addr_code for user-only Richard Henderson
` (14 subsequent siblings)
21 siblings, 1 reply; 35+ messages in thread
From: Richard Henderson @ 2022-08-19 3:26 UTC (permalink / raw)
To: qemu-devel; +Cc: laurent, iii, dramforever, alistair.francis, alex.bennee
From: Ilya Leoshkevich <iii@linux.ibm.com>
Introduce a function that checks whether a given address is on the same
page as where disassembly started. Having it improves readability of
the following patches.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Message-Id: <20220811095534.241224-3-iii@linux.ibm.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
[rth: Make the DisasContextBase parameter const.]
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/translator.h | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/include/exec/translator.h b/include/exec/translator.h
index 7db6845535..0d0bf3a31e 100644
--- a/include/exec/translator.h
+++ b/include/exec/translator.h
@@ -187,4 +187,14 @@ FOR_EACH_TRANSLATOR_LD(GEN_TRANSLATOR_LD)
#undef GEN_TRANSLATOR_LD
+/*
+ * Return whether addr is on the same page as where disassembly started.
+ * Translators can use this to enforce the rule that only single-insn
+ * translation blocks are allowed to cross page boundaries.
+ */
+static inline bool is_same_page(const DisasContextBase *db, target_ulong addr)
+{
+ return ((addr ^ db->pc_first) & TARGET_PAGE_MASK) == 0;
+}
+
#endif /* EXEC__TRANSLATOR_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v6 07/21] accel/tcg: Introduce is_same_page()
2022-08-19 3:26 ` [PATCH v6 07/21] accel/tcg: Introduce is_same_page() Richard Henderson
@ 2022-08-21 23:27 ` Alistair Francis
0 siblings, 0 replies; 35+ messages in thread
From: Alistair Francis @ 2022-08-21 23:27 UTC (permalink / raw)
To: Richard Henderson
Cc: qemu-devel@nongnu.org Developers, Laurent Vivier, iii,
dramforever, Alistair Francis, Alex Bennée
On Fri, Aug 19, 2022 at 1:26 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> From: Ilya Leoshkevich <iii@linux.ibm.com>
>
> Introduce a function that checks whether a given address is on the same
> page as where disassembly started. Having it improves readability of
> the following patches.
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> Message-Id: <20220811095534.241224-3-iii@linux.ibm.com>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> [rth: Make the DisasContextBase parameter const.]
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> include/exec/translator.h | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/include/exec/translator.h b/include/exec/translator.h
> index 7db6845535..0d0bf3a31e 100644
> --- a/include/exec/translator.h
> +++ b/include/exec/translator.h
> @@ -187,4 +187,14 @@ FOR_EACH_TRANSLATOR_LD(GEN_TRANSLATOR_LD)
>
> #undef GEN_TRANSLATOR_LD
>
> +/*
> + * Return whether addr is on the same page as where disassembly started.
> + * Translators can use this to enforce the rule that only single-insn
> + * translation blocks are allowed to cross page boundaries.
> + */
> +static inline bool is_same_page(const DisasContextBase *db, target_ulong addr)
> +{
> + return ((addr ^ db->pc_first) & TARGET_PAGE_MASK) == 0;
> +}
> +
> #endif /* EXEC__TRANSLATOR_H */
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v6 08/21] accel/tcg: Properly implement get_page_addr_code for user-only
2022-08-19 3:25 [PATCH v6 00/21] linux-user: Fix siginfo_t contents when jumping to non-readable pages Richard Henderson
` (6 preceding siblings ...)
2022-08-19 3:26 ` [PATCH v6 07/21] accel/tcg: Introduce is_same_page() Richard Henderson
@ 2022-08-19 3:26 ` Richard Henderson
2022-08-21 23:39 ` Alistair Francis
2022-08-19 3:26 ` [PATCH v6 09/21] accel/tcg: Unlock mmap_lock after longjmp Richard Henderson
` (13 subsequent siblings)
21 siblings, 1 reply; 35+ messages in thread
From: Richard Henderson @ 2022-08-19 3:26 UTC (permalink / raw)
To: qemu-devel; +Cc: laurent, iii, dramforever, alistair.francis, alex.bennee
The current implementation is a no-op, simply returning addr.
This is incorrect, because we ought to be checking the page
permissions for execution.
Make get_page_addr_code inline for both implementations.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/exec-all.h | 85 ++++++++++++++---------------------------
accel/tcg/cputlb.c | 5 ---
accel/tcg/user-exec.c | 15 ++++++++
3 files changed, 43 insertions(+), 62 deletions(-)
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 311e5fb422..0475ec6007 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -598,43 +598,44 @@ struct MemoryRegionSection *iotlb_to_section(CPUState *cpu,
hwaddr index, MemTxAttrs attrs);
#endif
-#if defined(CONFIG_USER_ONLY)
-void mmap_lock(void);
-void mmap_unlock(void);
-bool have_mmap_lock(void);
-
/**
- * get_page_addr_code() - user-mode version
+ * get_page_addr_code_hostp()
* @env: CPUArchState
* @addr: guest virtual address of guest code
*
- * Returns @addr.
+ * See get_page_addr_code() (full-system version) for documentation on the
+ * return value.
+ *
+ * Sets *@hostp (when @hostp is non-NULL) as follows.
+ * If the return value is -1, sets *@hostp to NULL. Otherwise, sets *@hostp
+ * to the host address where @addr's content is kept.
+ *
+ * Note: this function can trigger an exception.
+ */
+tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, target_ulong addr,
+ void **hostp);
+
+/**
+ * get_page_addr_code()
+ * @env: CPUArchState
+ * @addr: guest virtual address of guest code
+ *
+ * If we cannot translate and execute from the entire RAM page, or if
+ * the region is not backed by RAM, returns -1. Otherwise, returns the
+ * ram_addr_t corresponding to the guest code at @addr.
+ *
+ * Note: this function can trigger an exception.
*/
static inline tb_page_addr_t get_page_addr_code(CPUArchState *env,
target_ulong addr)
{
- return addr;
+ return get_page_addr_code_hostp(env, addr, NULL);
}
-/**
- * get_page_addr_code_hostp() - user-mode version
- * @env: CPUArchState
- * @addr: guest virtual address of guest code
- *
- * Returns @addr.
- *
- * If @hostp is non-NULL, sets *@hostp to the host address where @addr's content
- * is kept.
- */
-static inline tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env,
- target_ulong addr,
- void **hostp)
-{
- if (hostp) {
- *hostp = g2h_untagged(addr);
- }
- return addr;
-}
+#if defined(CONFIG_USER_ONLY)
+void mmap_lock(void);
+void mmap_unlock(void);
+bool have_mmap_lock(void);
/**
* adjust_signal_pc:
@@ -691,36 +692,6 @@ G_NORETURN void cpu_loop_exit_sigbus(CPUState *cpu, target_ulong addr,
static inline void mmap_lock(void) {}
static inline void mmap_unlock(void) {}
-/**
- * get_page_addr_code() - full-system version
- * @env: CPUArchState
- * @addr: guest virtual address of guest code
- *
- * If we cannot translate and execute from the entire RAM page, or if
- * the region is not backed by RAM, returns -1. Otherwise, returns the
- * ram_addr_t corresponding to the guest code at @addr.
- *
- * Note: this function can trigger an exception.
- */
-tb_page_addr_t get_page_addr_code(CPUArchState *env, target_ulong addr);
-
-/**
- * get_page_addr_code_hostp() - full-system version
- * @env: CPUArchState
- * @addr: guest virtual address of guest code
- *
- * See get_page_addr_code() (full-system version) for documentation on the
- * return value.
- *
- * Sets *@hostp (when @hostp is non-NULL) as follows.
- * If the return value is -1, sets *@hostp to NULL. Otherwise, sets *@hostp
- * to the host address where @addr's content is kept.
- *
- * Note: this function can trigger an exception.
- */
-tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, target_ulong addr,
- void **hostp);
-
void tlb_reset_dirty(CPUState *cpu, ram_addr_t start1, ram_addr_t length);
void tlb_set_dirty(CPUState *cpu, target_ulong vaddr);
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index a46f3a654d..43bd65c973 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1544,11 +1544,6 @@ tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, target_ulong addr,
return qemu_ram_addr_from_host_nofail(p);
}
-tb_page_addr_t get_page_addr_code(CPUArchState *env, target_ulong addr)
-{
- return get_page_addr_code_hostp(env, addr, NULL);
-}
-
static void notdirty_write(CPUState *cpu, vaddr mem_vaddr, unsigned size,
CPUIOTLBEntry *iotlbentry, uintptr_t retaddr)
{
diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index 20ada5472b..a20234fb02 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -199,6 +199,21 @@ void *probe_access(CPUArchState *env, target_ulong addr, int size,
return size ? g2h(env_cpu(env), addr) : NULL;
}
+tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, target_ulong addr,
+ void **hostp)
+{
+ int flags;
+
+ flags = probe_access_internal(env, addr, 1, MMU_INST_FETCH, true, 0);
+ if (unlikely(flags)) {
+ return -1;
+ }
+ if (hostp) {
+ *hostp = g2h_untagged(addr);
+ }
+ return addr;
+}
+
/* The softmmu versions of these helpers are in cputlb.c. */
/*
--
2.34.1
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v6 08/21] accel/tcg: Properly implement get_page_addr_code for user-only
2022-08-19 3:26 ` [PATCH v6 08/21] accel/tcg: Properly implement get_page_addr_code for user-only Richard Henderson
@ 2022-08-21 23:39 ` Alistair Francis
0 siblings, 0 replies; 35+ messages in thread
From: Alistair Francis @ 2022-08-21 23:39 UTC (permalink / raw)
To: Richard Henderson
Cc: qemu-devel@nongnu.org Developers, Laurent Vivier, iii,
dramforever, Alistair Francis, Alex Bennée
On Fri, Aug 19, 2022 at 1:40 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> The current implementation is a no-op, simply returning addr.
> This is incorrect, because we ought to be checking the page
> permissions for execution.
>
> Make get_page_addr_code inline for both implementations.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> include/exec/exec-all.h | 85 ++++++++++++++---------------------------
> accel/tcg/cputlb.c | 5 ---
> accel/tcg/user-exec.c | 15 ++++++++
> 3 files changed, 43 insertions(+), 62 deletions(-)
>
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index 311e5fb422..0475ec6007 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -598,43 +598,44 @@ struct MemoryRegionSection *iotlb_to_section(CPUState *cpu,
> hwaddr index, MemTxAttrs attrs);
> #endif
>
> -#if defined(CONFIG_USER_ONLY)
> -void mmap_lock(void);
> -void mmap_unlock(void);
> -bool have_mmap_lock(void);
> -
> /**
> - * get_page_addr_code() - user-mode version
> + * get_page_addr_code_hostp()
> * @env: CPUArchState
> * @addr: guest virtual address of guest code
> *
> - * Returns @addr.
> + * See get_page_addr_code() (full-system version) for documentation on the
> + * return value.
> + *
> + * Sets *@hostp (when @hostp is non-NULL) as follows.
> + * If the return value is -1, sets *@hostp to NULL. Otherwise, sets *@hostp
> + * to the host address where @addr's content is kept.
> + *
> + * Note: this function can trigger an exception.
> + */
> +tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, target_ulong addr,
> + void **hostp);
> +
> +/**
> + * get_page_addr_code()
> + * @env: CPUArchState
> + * @addr: guest virtual address of guest code
> + *
> + * If we cannot translate and execute from the entire RAM page, or if
> + * the region is not backed by RAM, returns -1. Otherwise, returns the
> + * ram_addr_t corresponding to the guest code at @addr.
> + *
> + * Note: this function can trigger an exception.
> */
> static inline tb_page_addr_t get_page_addr_code(CPUArchState *env,
> target_ulong addr)
> {
> - return addr;
> + return get_page_addr_code_hostp(env, addr, NULL);
> }
>
> -/**
> - * get_page_addr_code_hostp() - user-mode version
> - * @env: CPUArchState
> - * @addr: guest virtual address of guest code
> - *
> - * Returns @addr.
> - *
> - * If @hostp is non-NULL, sets *@hostp to the host address where @addr's content
> - * is kept.
> - */
> -static inline tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env,
> - target_ulong addr,
> - void **hostp)
> -{
> - if (hostp) {
> - *hostp = g2h_untagged(addr);
> - }
> - return addr;
> -}
> +#if defined(CONFIG_USER_ONLY)
> +void mmap_lock(void);
> +void mmap_unlock(void);
> +bool have_mmap_lock(void);
>
> /**
> * adjust_signal_pc:
> @@ -691,36 +692,6 @@ G_NORETURN void cpu_loop_exit_sigbus(CPUState *cpu, target_ulong addr,
> static inline void mmap_lock(void) {}
> static inline void mmap_unlock(void) {}
>
> -/**
> - * get_page_addr_code() - full-system version
> - * @env: CPUArchState
> - * @addr: guest virtual address of guest code
> - *
> - * If we cannot translate and execute from the entire RAM page, or if
> - * the region is not backed by RAM, returns -1. Otherwise, returns the
> - * ram_addr_t corresponding to the guest code at @addr.
> - *
> - * Note: this function can trigger an exception.
> - */
> -tb_page_addr_t get_page_addr_code(CPUArchState *env, target_ulong addr);
> -
> -/**
> - * get_page_addr_code_hostp() - full-system version
> - * @env: CPUArchState
> - * @addr: guest virtual address of guest code
> - *
> - * See get_page_addr_code() (full-system version) for documentation on the
> - * return value.
> - *
> - * Sets *@hostp (when @hostp is non-NULL) as follows.
> - * If the return value is -1, sets *@hostp to NULL. Otherwise, sets *@hostp
> - * to the host address where @addr's content is kept.
> - *
> - * Note: this function can trigger an exception.
> - */
> -tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, target_ulong addr,
> - void **hostp);
> -
> void tlb_reset_dirty(CPUState *cpu, ram_addr_t start1, ram_addr_t length);
> void tlb_set_dirty(CPUState *cpu, target_ulong vaddr);
>
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index a46f3a654d..43bd65c973 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -1544,11 +1544,6 @@ tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, target_ulong addr,
> return qemu_ram_addr_from_host_nofail(p);
> }
>
> -tb_page_addr_t get_page_addr_code(CPUArchState *env, target_ulong addr)
> -{
> - return get_page_addr_code_hostp(env, addr, NULL);
> -}
> -
> static void notdirty_write(CPUState *cpu, vaddr mem_vaddr, unsigned size,
> CPUIOTLBEntry *iotlbentry, uintptr_t retaddr)
> {
> diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
> index 20ada5472b..a20234fb02 100644
> --- a/accel/tcg/user-exec.c
> +++ b/accel/tcg/user-exec.c
> @@ -199,6 +199,21 @@ void *probe_access(CPUArchState *env, target_ulong addr, int size,
> return size ? g2h(env_cpu(env), addr) : NULL;
> }
>
> +tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, target_ulong addr,
> + void **hostp)
> +{
> + int flags;
> +
> + flags = probe_access_internal(env, addr, 1, MMU_INST_FETCH, true, 0);
> + if (unlikely(flags)) {
> + return -1;
> + }
> + if (hostp) {
> + *hostp = g2h_untagged(addr);
> + }
> + return addr;
> +}
> +
> /* The softmmu versions of these helpers are in cputlb.c. */
>
> /*
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v6 09/21] accel/tcg: Unlock mmap_lock after longjmp
2022-08-19 3:25 [PATCH v6 00/21] linux-user: Fix siginfo_t contents when jumping to non-readable pages Richard Henderson
` (7 preceding siblings ...)
2022-08-19 3:26 ` [PATCH v6 08/21] accel/tcg: Properly implement get_page_addr_code for user-only Richard Henderson
@ 2022-08-19 3:26 ` Richard Henderson
2022-08-21 23:30 ` Alistair Francis
2022-08-19 3:26 ` [PATCH v6 10/21] accel/tcg: Make tb_htable_lookup static Richard Henderson
` (12 subsequent siblings)
21 siblings, 1 reply; 35+ messages in thread
From: Richard Henderson @ 2022-08-19 3:26 UTC (permalink / raw)
To: qemu-devel; +Cc: laurent, iii, dramforever, alistair.francis, alex.bennee
The mmap_lock is held around tb_gen_code. While the comment
is correct that the lock is dropped when tb_gen_code runs out
of memory, the lock is *not* dropped when an exception is
raised reading code for translation.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
accel/tcg/cpu-exec.c | 12 ++++++------
accel/tcg/user-exec.c | 3 ---
2 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index a565a3f8ec..d18081ca6f 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -462,13 +462,11 @@ void cpu_exec_step_atomic(CPUState *cpu)
cpu_tb_exec(cpu, tb, &tb_exit);
cpu_exec_exit(cpu);
} else {
- /*
- * The mmap_lock is dropped by tb_gen_code if it runs out of
- * memory.
- */
#ifndef CONFIG_SOFTMMU
clear_helper_retaddr();
- tcg_debug_assert(!have_mmap_lock());
+ if (have_mmap_lock()) {
+ mmap_unlock();
+ }
#endif
if (qemu_mutex_iothread_locked()) {
qemu_mutex_unlock_iothread();
@@ -936,7 +934,9 @@ int cpu_exec(CPUState *cpu)
#ifndef CONFIG_SOFTMMU
clear_helper_retaddr();
- tcg_debug_assert(!have_mmap_lock());
+ if (have_mmap_lock()) {
+ mmap_unlock();
+ }
#endif
if (qemu_mutex_iothread_locked()) {
qemu_mutex_unlock_iothread();
diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index a20234fb02..58edd33896 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -80,10 +80,7 @@ MMUAccessType adjust_signal_pc(uintptr_t *pc, bool is_write)
* (and if the translator doesn't handle page boundaries correctly
* there's little we can do about that here). Therefore, do not
* trigger the unwinder.
- *
- * Like tb_gen_code, release the memory lock before cpu_loop_exit.
*/
- mmap_unlock();
*pc = 0;
return MMU_INST_FETCH;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v6 09/21] accel/tcg: Unlock mmap_lock after longjmp
2022-08-19 3:26 ` [PATCH v6 09/21] accel/tcg: Unlock mmap_lock after longjmp Richard Henderson
@ 2022-08-21 23:30 ` Alistair Francis
0 siblings, 0 replies; 35+ messages in thread
From: Alistair Francis @ 2022-08-21 23:30 UTC (permalink / raw)
To: Richard Henderson
Cc: qemu-devel@nongnu.org Developers, Laurent Vivier, iii,
dramforever, Alistair Francis, Alex Bennée
On Fri, Aug 19, 2022 at 1:29 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> The mmap_lock is held around tb_gen_code. While the comment
> is correct that the lock is dropped when tb_gen_code runs out
> of memory, the lock is *not* dropped when an exception is
> raised reading code for translation.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> accel/tcg/cpu-exec.c | 12 ++++++------
> accel/tcg/user-exec.c | 3 ---
> 2 files changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index a565a3f8ec..d18081ca6f 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -462,13 +462,11 @@ void cpu_exec_step_atomic(CPUState *cpu)
> cpu_tb_exec(cpu, tb, &tb_exit);
> cpu_exec_exit(cpu);
> } else {
> - /*
> - * The mmap_lock is dropped by tb_gen_code if it runs out of
> - * memory.
> - */
> #ifndef CONFIG_SOFTMMU
> clear_helper_retaddr();
> - tcg_debug_assert(!have_mmap_lock());
> + if (have_mmap_lock()) {
> + mmap_unlock();
> + }
> #endif
> if (qemu_mutex_iothread_locked()) {
> qemu_mutex_unlock_iothread();
> @@ -936,7 +934,9 @@ int cpu_exec(CPUState *cpu)
>
> #ifndef CONFIG_SOFTMMU
> clear_helper_retaddr();
> - tcg_debug_assert(!have_mmap_lock());
> + if (have_mmap_lock()) {
> + mmap_unlock();
> + }
> #endif
> if (qemu_mutex_iothread_locked()) {
> qemu_mutex_unlock_iothread();
> diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
> index a20234fb02..58edd33896 100644
> --- a/accel/tcg/user-exec.c
> +++ b/accel/tcg/user-exec.c
> @@ -80,10 +80,7 @@ MMUAccessType adjust_signal_pc(uintptr_t *pc, bool is_write)
> * (and if the translator doesn't handle page boundaries correctly
> * there's little we can do about that here). Therefore, do not
> * trigger the unwinder.
> - *
> - * Like tb_gen_code, release the memory lock before cpu_loop_exit.
> */
> - mmap_unlock();
> *pc = 0;
> return MMU_INST_FETCH;
> }
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v6 10/21] accel/tcg: Make tb_htable_lookup static
2022-08-19 3:25 [PATCH v6 00/21] linux-user: Fix siginfo_t contents when jumping to non-readable pages Richard Henderson
` (8 preceding siblings ...)
2022-08-19 3:26 ` [PATCH v6 09/21] accel/tcg: Unlock mmap_lock after longjmp Richard Henderson
@ 2022-08-19 3:26 ` Richard Henderson
2022-08-21 23:33 ` Alistair Francis
2022-08-19 3:26 ` [PATCH v6 11/21] accel/tcg: Move qemu_ram_addr_from_host_nofail to physmem.c Richard Henderson
` (11 subsequent siblings)
21 siblings, 1 reply; 35+ messages in thread
From: Richard Henderson @ 2022-08-19 3:26 UTC (permalink / raw)
To: qemu-devel; +Cc: laurent, iii, dramforever, alistair.francis, alex.bennee
The function is not used outside of cpu-exec.c. Move it and
its subroutines up in the file, before the first use.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/exec-all.h | 3 -
accel/tcg/cpu-exec.c | 122 ++++++++++++++++++++--------------------
2 files changed, 61 insertions(+), 64 deletions(-)
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 0475ec6007..9f35e3b7a9 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -552,9 +552,6 @@ void tb_invalidate_phys_addr(AddressSpace *as, hwaddr addr, MemTxAttrs attrs);
#endif
void tb_flush(CPUState *cpu);
void tb_phys_invalidate(TranslationBlock *tb, tb_page_addr_t page_addr);
-TranslationBlock *tb_htable_lookup(CPUState *cpu, target_ulong pc,
- target_ulong cs_base, uint32_t flags,
- uint32_t cflags);
void tb_set_jmp_target(TranslationBlock *tb, int n, uintptr_t addr);
/* GETPC is the true target of the return instruction that we'll execute. */
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index d18081ca6f..7887af6f45 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -170,6 +170,67 @@ uint32_t curr_cflags(CPUState *cpu)
return cflags;
}
+struct tb_desc {
+ target_ulong pc;
+ target_ulong cs_base;
+ CPUArchState *env;
+ tb_page_addr_t phys_page1;
+ uint32_t flags;
+ uint32_t cflags;
+ uint32_t trace_vcpu_dstate;
+};
+
+static bool tb_lookup_cmp(const void *p, const void *d)
+{
+ const TranslationBlock *tb = p;
+ const struct tb_desc *desc = d;
+
+ if (tb->pc == desc->pc &&
+ tb->page_addr[0] == desc->phys_page1 &&
+ tb->cs_base == desc->cs_base &&
+ tb->flags == desc->flags &&
+ tb->trace_vcpu_dstate == desc->trace_vcpu_dstate &&
+ tb_cflags(tb) == desc->cflags) {
+ /* check next page if needed */
+ if (tb->page_addr[1] == -1) {
+ return true;
+ } else {
+ tb_page_addr_t phys_page2;
+ target_ulong virt_page2;
+
+ virt_page2 = (desc->pc & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
+ phys_page2 = get_page_addr_code(desc->env, virt_page2);
+ if (tb->page_addr[1] == phys_page2) {
+ return true;
+ }
+ }
+ }
+ return false;
+}
+
+static TranslationBlock *tb_htable_lookup(CPUState *cpu, target_ulong pc,
+ target_ulong cs_base, uint32_t flags,
+ uint32_t cflags)
+{
+ tb_page_addr_t phys_pc;
+ struct tb_desc desc;
+ uint32_t h;
+
+ desc.env = cpu->env_ptr;
+ desc.cs_base = cs_base;
+ desc.flags = flags;
+ desc.cflags = cflags;
+ desc.trace_vcpu_dstate = *cpu->trace_dstate;
+ desc.pc = pc;
+ phys_pc = get_page_addr_code(desc.env, pc);
+ if (phys_pc == -1) {
+ return NULL;
+ }
+ desc.phys_page1 = phys_pc & TARGET_PAGE_MASK;
+ h = tb_hash_func(phys_pc, pc, flags, cflags, *cpu->trace_dstate);
+ return qht_lookup_custom(&tb_ctx.htable, &desc, h, tb_lookup_cmp);
+}
+
/* Might cause an exception, so have a longjmp destination ready */
static inline TranslationBlock *tb_lookup(CPUState *cpu, target_ulong pc,
target_ulong cs_base,
@@ -485,67 +546,6 @@ void cpu_exec_step_atomic(CPUState *cpu)
end_exclusive();
}
-struct tb_desc {
- target_ulong pc;
- target_ulong cs_base;
- CPUArchState *env;
- tb_page_addr_t phys_page1;
- uint32_t flags;
- uint32_t cflags;
- uint32_t trace_vcpu_dstate;
-};
-
-static bool tb_lookup_cmp(const void *p, const void *d)
-{
- const TranslationBlock *tb = p;
- const struct tb_desc *desc = d;
-
- if (tb->pc == desc->pc &&
- tb->page_addr[0] == desc->phys_page1 &&
- tb->cs_base == desc->cs_base &&
- tb->flags == desc->flags &&
- tb->trace_vcpu_dstate == desc->trace_vcpu_dstate &&
- tb_cflags(tb) == desc->cflags) {
- /* check next page if needed */
- if (tb->page_addr[1] == -1) {
- return true;
- } else {
- tb_page_addr_t phys_page2;
- target_ulong virt_page2;
-
- virt_page2 = (desc->pc & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
- phys_page2 = get_page_addr_code(desc->env, virt_page2);
- if (tb->page_addr[1] == phys_page2) {
- return true;
- }
- }
- }
- return false;
-}
-
-TranslationBlock *tb_htable_lookup(CPUState *cpu, target_ulong pc,
- target_ulong cs_base, uint32_t flags,
- uint32_t cflags)
-{
- tb_page_addr_t phys_pc;
- struct tb_desc desc;
- uint32_t h;
-
- desc.env = cpu->env_ptr;
- desc.cs_base = cs_base;
- desc.flags = flags;
- desc.cflags = cflags;
- desc.trace_vcpu_dstate = *cpu->trace_dstate;
- desc.pc = pc;
- phys_pc = get_page_addr_code(desc.env, pc);
- if (phys_pc == -1) {
- return NULL;
- }
- desc.phys_page1 = phys_pc & TARGET_PAGE_MASK;
- h = tb_hash_func(phys_pc, pc, flags, cflags, *cpu->trace_dstate);
- return qht_lookup_custom(&tb_ctx.htable, &desc, h, tb_lookup_cmp);
-}
-
void tb_set_jmp_target(TranslationBlock *tb, int n, uintptr_t addr)
{
if (TCG_TARGET_HAS_direct_jump) {
--
2.34.1
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v6 10/21] accel/tcg: Make tb_htable_lookup static
2022-08-19 3:26 ` [PATCH v6 10/21] accel/tcg: Make tb_htable_lookup static Richard Henderson
@ 2022-08-21 23:33 ` Alistair Francis
0 siblings, 0 replies; 35+ messages in thread
From: Alistair Francis @ 2022-08-21 23:33 UTC (permalink / raw)
To: Richard Henderson
Cc: qemu-devel@nongnu.org Developers, Laurent Vivier, iii,
dramforever, Alistair Francis, Alex Bennée
On Fri, Aug 19, 2022 at 1:33 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> The function is not used outside of cpu-exec.c. Move it and
> its subroutines up in the file, before the first use.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> include/exec/exec-all.h | 3 -
> accel/tcg/cpu-exec.c | 122 ++++++++++++++++++++--------------------
> 2 files changed, 61 insertions(+), 64 deletions(-)
>
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index 0475ec6007..9f35e3b7a9 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -552,9 +552,6 @@ void tb_invalidate_phys_addr(AddressSpace *as, hwaddr addr, MemTxAttrs attrs);
> #endif
> void tb_flush(CPUState *cpu);
> void tb_phys_invalidate(TranslationBlock *tb, tb_page_addr_t page_addr);
> -TranslationBlock *tb_htable_lookup(CPUState *cpu, target_ulong pc,
> - target_ulong cs_base, uint32_t flags,
> - uint32_t cflags);
> void tb_set_jmp_target(TranslationBlock *tb, int n, uintptr_t addr);
>
> /* GETPC is the true target of the return instruction that we'll execute. */
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index d18081ca6f..7887af6f45 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -170,6 +170,67 @@ uint32_t curr_cflags(CPUState *cpu)
> return cflags;
> }
>
> +struct tb_desc {
> + target_ulong pc;
> + target_ulong cs_base;
> + CPUArchState *env;
> + tb_page_addr_t phys_page1;
> + uint32_t flags;
> + uint32_t cflags;
> + uint32_t trace_vcpu_dstate;
> +};
> +
> +static bool tb_lookup_cmp(const void *p, const void *d)
> +{
> + const TranslationBlock *tb = p;
> + const struct tb_desc *desc = d;
> +
> + if (tb->pc == desc->pc &&
> + tb->page_addr[0] == desc->phys_page1 &&
> + tb->cs_base == desc->cs_base &&
> + tb->flags == desc->flags &&
> + tb->trace_vcpu_dstate == desc->trace_vcpu_dstate &&
> + tb_cflags(tb) == desc->cflags) {
> + /* check next page if needed */
> + if (tb->page_addr[1] == -1) {
> + return true;
> + } else {
> + tb_page_addr_t phys_page2;
> + target_ulong virt_page2;
> +
> + virt_page2 = (desc->pc & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
> + phys_page2 = get_page_addr_code(desc->env, virt_page2);
> + if (tb->page_addr[1] == phys_page2) {
> + return true;
> + }
> + }
> + }
> + return false;
> +}
> +
> +static TranslationBlock *tb_htable_lookup(CPUState *cpu, target_ulong pc,
> + target_ulong cs_base, uint32_t flags,
> + uint32_t cflags)
> +{
> + tb_page_addr_t phys_pc;
> + struct tb_desc desc;
> + uint32_t h;
> +
> + desc.env = cpu->env_ptr;
> + desc.cs_base = cs_base;
> + desc.flags = flags;
> + desc.cflags = cflags;
> + desc.trace_vcpu_dstate = *cpu->trace_dstate;
> + desc.pc = pc;
> + phys_pc = get_page_addr_code(desc.env, pc);
> + if (phys_pc == -1) {
> + return NULL;
> + }
> + desc.phys_page1 = phys_pc & TARGET_PAGE_MASK;
> + h = tb_hash_func(phys_pc, pc, flags, cflags, *cpu->trace_dstate);
> + return qht_lookup_custom(&tb_ctx.htable, &desc, h, tb_lookup_cmp);
> +}
> +
> /* Might cause an exception, so have a longjmp destination ready */
> static inline TranslationBlock *tb_lookup(CPUState *cpu, target_ulong pc,
> target_ulong cs_base,
> @@ -485,67 +546,6 @@ void cpu_exec_step_atomic(CPUState *cpu)
> end_exclusive();
> }
>
> -struct tb_desc {
> - target_ulong pc;
> - target_ulong cs_base;
> - CPUArchState *env;
> - tb_page_addr_t phys_page1;
> - uint32_t flags;
> - uint32_t cflags;
> - uint32_t trace_vcpu_dstate;
> -};
> -
> -static bool tb_lookup_cmp(const void *p, const void *d)
> -{
> - const TranslationBlock *tb = p;
> - const struct tb_desc *desc = d;
> -
> - if (tb->pc == desc->pc &&
> - tb->page_addr[0] == desc->phys_page1 &&
> - tb->cs_base == desc->cs_base &&
> - tb->flags == desc->flags &&
> - tb->trace_vcpu_dstate == desc->trace_vcpu_dstate &&
> - tb_cflags(tb) == desc->cflags) {
> - /* check next page if needed */
> - if (tb->page_addr[1] == -1) {
> - return true;
> - } else {
> - tb_page_addr_t phys_page2;
> - target_ulong virt_page2;
> -
> - virt_page2 = (desc->pc & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
> - phys_page2 = get_page_addr_code(desc->env, virt_page2);
> - if (tb->page_addr[1] == phys_page2) {
> - return true;
> - }
> - }
> - }
> - return false;
> -}
> -
> -TranslationBlock *tb_htable_lookup(CPUState *cpu, target_ulong pc,
> - target_ulong cs_base, uint32_t flags,
> - uint32_t cflags)
> -{
> - tb_page_addr_t phys_pc;
> - struct tb_desc desc;
> - uint32_t h;
> -
> - desc.env = cpu->env_ptr;
> - desc.cs_base = cs_base;
> - desc.flags = flags;
> - desc.cflags = cflags;
> - desc.trace_vcpu_dstate = *cpu->trace_dstate;
> - desc.pc = pc;
> - phys_pc = get_page_addr_code(desc.env, pc);
> - if (phys_pc == -1) {
> - return NULL;
> - }
> - desc.phys_page1 = phys_pc & TARGET_PAGE_MASK;
> - h = tb_hash_func(phys_pc, pc, flags, cflags, *cpu->trace_dstate);
> - return qht_lookup_custom(&tb_ctx.htable, &desc, h, tb_lookup_cmp);
> -}
> -
> void tb_set_jmp_target(TranslationBlock *tb, int n, uintptr_t addr)
> {
> if (TCG_TARGET_HAS_direct_jump) {
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v6 11/21] accel/tcg: Move qemu_ram_addr_from_host_nofail to physmem.c
2022-08-19 3:25 [PATCH v6 00/21] linux-user: Fix siginfo_t contents when jumping to non-readable pages Richard Henderson
` (9 preceding siblings ...)
2022-08-19 3:26 ` [PATCH v6 10/21] accel/tcg: Make tb_htable_lookup static Richard Henderson
@ 2022-08-19 3:26 ` Richard Henderson
2022-08-21 23:34 ` Alistair Francis
2022-08-19 3:26 ` [PATCH v6 12/21] accel/tcg: Use probe_access_internal for softmmu get_page_addr_code_hostp Richard Henderson
` (10 subsequent siblings)
21 siblings, 1 reply; 35+ messages in thread
From: Richard Henderson @ 2022-08-19 3:26 UTC (permalink / raw)
To: qemu-devel; +Cc: laurent, iii, dramforever, alistair.francis, alex.bennee
The base qemu_ram_addr_from_host function is already in
softmmu/physmem.c; move the nofail version to be adjacent.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/cpu-common.h | 1 +
accel/tcg/cputlb.c | 12 ------------
softmmu/physmem.c | 12 ++++++++++++
3 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 2281be4e10..d909429427 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -72,6 +72,7 @@ typedef uintptr_t ram_addr_t;
void qemu_ram_remap(ram_addr_t addr, ram_addr_t length);
/* This should not be used by devices. */
ram_addr_t qemu_ram_addr_from_host(void *ptr);
+ram_addr_t qemu_ram_addr_from_host_nofail(void *ptr);
RAMBlock *qemu_ram_block_by_name(const char *name);
RAMBlock *qemu_ram_block_from_host(void *ptr, bool round_offset,
ram_addr_t *offset);
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 43bd65c973..80a3eb4f1c 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1283,18 +1283,6 @@ void tlb_set_page(CPUState *cpu, target_ulong vaddr,
prot, mmu_idx, size);
}
-static inline ram_addr_t qemu_ram_addr_from_host_nofail(void *ptr)
-{
- ram_addr_t ram_addr;
-
- ram_addr = qemu_ram_addr_from_host(ptr);
- if (ram_addr == RAM_ADDR_INVALID) {
- error_report("Bad ram pointer %p", ptr);
- abort();
- }
- return ram_addr;
-}
-
/*
* Note: tlb_fill() can trigger a resize of the TLB. This means that all of the
* caller's prior references to the TLB table (e.g. CPUTLBEntry pointers) must
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index dc3c3e5f2e..d4c30e99ea 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -2460,6 +2460,18 @@ ram_addr_t qemu_ram_addr_from_host(void *ptr)
return block->offset + offset;
}
+ram_addr_t qemu_ram_addr_from_host_nofail(void *ptr)
+{
+ ram_addr_t ram_addr;
+
+ ram_addr = qemu_ram_addr_from_host(ptr);
+ if (ram_addr == RAM_ADDR_INVALID) {
+ error_report("Bad ram pointer %p", ptr);
+ abort();
+ }
+ return ram_addr;
+}
+
static MemTxResult flatview_read(FlatView *fv, hwaddr addr,
MemTxAttrs attrs, void *buf, hwaddr len);
static MemTxResult flatview_write(FlatView *fv, hwaddr addr, MemTxAttrs attrs,
--
2.34.1
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v6 11/21] accel/tcg: Move qemu_ram_addr_from_host_nofail to physmem.c
2022-08-19 3:26 ` [PATCH v6 11/21] accel/tcg: Move qemu_ram_addr_from_host_nofail to physmem.c Richard Henderson
@ 2022-08-21 23:34 ` Alistair Francis
0 siblings, 0 replies; 35+ messages in thread
From: Alistair Francis @ 2022-08-21 23:34 UTC (permalink / raw)
To: Richard Henderson
Cc: qemu-devel@nongnu.org Developers, Laurent Vivier, iii,
dramforever, Alistair Francis, Alex Bennée
On Fri, Aug 19, 2022 at 1:34 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> The base qemu_ram_addr_from_host function is already in
> softmmu/physmem.c; move the nofail version to be adjacent.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> include/exec/cpu-common.h | 1 +
> accel/tcg/cputlb.c | 12 ------------
> softmmu/physmem.c | 12 ++++++++++++
> 3 files changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
> index 2281be4e10..d909429427 100644
> --- a/include/exec/cpu-common.h
> +++ b/include/exec/cpu-common.h
> @@ -72,6 +72,7 @@ typedef uintptr_t ram_addr_t;
> void qemu_ram_remap(ram_addr_t addr, ram_addr_t length);
> /* This should not be used by devices. */
> ram_addr_t qemu_ram_addr_from_host(void *ptr);
> +ram_addr_t qemu_ram_addr_from_host_nofail(void *ptr);
> RAMBlock *qemu_ram_block_by_name(const char *name);
> RAMBlock *qemu_ram_block_from_host(void *ptr, bool round_offset,
> ram_addr_t *offset);
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index 43bd65c973..80a3eb4f1c 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -1283,18 +1283,6 @@ void tlb_set_page(CPUState *cpu, target_ulong vaddr,
> prot, mmu_idx, size);
> }
>
> -static inline ram_addr_t qemu_ram_addr_from_host_nofail(void *ptr)
> -{
> - ram_addr_t ram_addr;
> -
> - ram_addr = qemu_ram_addr_from_host(ptr);
> - if (ram_addr == RAM_ADDR_INVALID) {
> - error_report("Bad ram pointer %p", ptr);
> - abort();
> - }
> - return ram_addr;
> -}
> -
> /*
> * Note: tlb_fill() can trigger a resize of the TLB. This means that all of the
> * caller's prior references to the TLB table (e.g. CPUTLBEntry pointers) must
> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
> index dc3c3e5f2e..d4c30e99ea 100644
> --- a/softmmu/physmem.c
> +++ b/softmmu/physmem.c
> @@ -2460,6 +2460,18 @@ ram_addr_t qemu_ram_addr_from_host(void *ptr)
> return block->offset + offset;
> }
>
> +ram_addr_t qemu_ram_addr_from_host_nofail(void *ptr)
> +{
> + ram_addr_t ram_addr;
> +
> + ram_addr = qemu_ram_addr_from_host(ptr);
> + if (ram_addr == RAM_ADDR_INVALID) {
> + error_report("Bad ram pointer %p", ptr);
> + abort();
> + }
> + return ram_addr;
> +}
> +
> static MemTxResult flatview_read(FlatView *fv, hwaddr addr,
> MemTxAttrs attrs, void *buf, hwaddr len);
> static MemTxResult flatview_write(FlatView *fv, hwaddr addr, MemTxAttrs attrs,
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v6 12/21] accel/tcg: Use probe_access_internal for softmmu get_page_addr_code_hostp
2022-08-19 3:25 [PATCH v6 00/21] linux-user: Fix siginfo_t contents when jumping to non-readable pages Richard Henderson
` (10 preceding siblings ...)
2022-08-19 3:26 ` [PATCH v6 11/21] accel/tcg: Move qemu_ram_addr_from_host_nofail to physmem.c Richard Henderson
@ 2022-08-19 3:26 ` Richard Henderson
2022-08-19 3:26 ` [PATCH v6 13/21] accel/tcg: Add nofault parameter to get_page_addr_code_hostp Richard Henderson
` (9 subsequent siblings)
21 siblings, 0 replies; 35+ messages in thread
From: Richard Henderson @ 2022-08-19 3:26 UTC (permalink / raw)
To: qemu-devel; +Cc: laurent, iii, dramforever, alistair.francis, alex.bennee
Simplify the implementation of get_page_addr_code_hostp
by reusing the existing probe_access infrastructure.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
accel/tcg/cputlb.c | 76 ++++++++++++++++------------------------------
1 file changed, 26 insertions(+), 50 deletions(-)
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 80a3eb4f1c..2dc2affa12 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1482,56 +1482,6 @@ static bool victim_tlb_hit(CPUArchState *env, size_t mmu_idx, size_t index,
victim_tlb_hit(env, mmu_idx, index, offsetof(CPUTLBEntry, TY), \
(ADDR) & TARGET_PAGE_MASK)
-/*
- * Return a ram_addr_t for the virtual address for execution.
- *
- * Return -1 if we can't translate and execute from an entire page
- * of RAM. This will force us to execute by loading and translating
- * one insn at a time, without caching.
- *
- * NOTE: This function will trigger an exception if the page is
- * not executable.
- */
-tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, target_ulong addr,
- void **hostp)
-{
- uintptr_t mmu_idx = cpu_mmu_index(env, true);
- uintptr_t index = tlb_index(env, mmu_idx, addr);
- CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr);
- void *p;
-
- if (unlikely(!tlb_hit(entry->addr_code, addr))) {
- if (!VICTIM_TLB_HIT(addr_code, addr)) {
- tlb_fill(env_cpu(env), addr, 0, MMU_INST_FETCH, mmu_idx, 0);
- index = tlb_index(env, mmu_idx, addr);
- entry = tlb_entry(env, mmu_idx, addr);
-
- if (unlikely(entry->addr_code & TLB_INVALID_MASK)) {
- /*
- * The MMU protection covers a smaller range than a target
- * page, so we must redo the MMU check for every insn.
- */
- return -1;
- }
- }
- assert(tlb_hit(entry->addr_code, addr));
- }
-
- if (unlikely(entry->addr_code & TLB_MMIO)) {
- /* The region is not backed by RAM. */
- if (hostp) {
- *hostp = NULL;
- }
- return -1;
- }
-
- p = (void *)((uintptr_t)addr + entry->addend);
- if (hostp) {
- *hostp = p;
- }
- return qemu_ram_addr_from_host_nofail(p);
-}
-
static void notdirty_write(CPUState *cpu, vaddr mem_vaddr, unsigned size,
CPUIOTLBEntry *iotlbentry, uintptr_t retaddr)
{
@@ -1687,6 +1637,32 @@ void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
return flags ? NULL : host;
}
+/*
+ * Return a ram_addr_t for the virtual address for execution.
+ *
+ * Return -1 if we can't translate and execute from an entire page
+ * of RAM. This will force us to execute by loading and translating
+ * one insn at a time, without caching.
+ *
+ * NOTE: This function will trigger an exception if the page is
+ * not executable.
+ */
+tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, target_ulong addr,
+ void **hostp)
+{
+ void *p;
+
+ (void)probe_access_internal(env, addr, 1, MMU_INST_FETCH,
+ cpu_mmu_index(env, true), true, &p, 0);
+ if (p == NULL) {
+ return -1;
+ }
+ if (hostp) {
+ *hostp = p;
+ }
+ return qemu_ram_addr_from_host_nofail(p);
+}
+
#ifdef CONFIG_PLUGIN
/*
* Perform a TLB lookup and populate the qemu_plugin_hwaddr structure.
--
2.34.1
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v6 13/21] accel/tcg: Add nofault parameter to get_page_addr_code_hostp
2022-08-19 3:25 [PATCH v6 00/21] linux-user: Fix siginfo_t contents when jumping to non-readable pages Richard Henderson
` (11 preceding siblings ...)
2022-08-19 3:26 ` [PATCH v6 12/21] accel/tcg: Use probe_access_internal for softmmu get_page_addr_code_hostp Richard Henderson
@ 2022-08-19 3:26 ` Richard Henderson
2022-08-21 23:37 ` Alistair Francis
2022-08-19 3:26 ` [PATCH v6 14/21] accel/tcg: Raise PROT_EXEC exception early Richard Henderson
` (8 subsequent siblings)
21 siblings, 1 reply; 35+ messages in thread
From: Richard Henderson @ 2022-08-19 3:26 UTC (permalink / raw)
To: qemu-devel; +Cc: laurent, iii, dramforever, alistair.francis, alex.bennee
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/exec-all.h | 10 +++++-----
accel/tcg/cputlb.c | 8 ++++----
accel/tcg/plugin-gen.c | 4 ++--
accel/tcg/user-exec.c | 4 ++--
4 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 9f35e3b7a9..7a6dc44d86 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -599,6 +599,8 @@ struct MemoryRegionSection *iotlb_to_section(CPUState *cpu,
* get_page_addr_code_hostp()
* @env: CPUArchState
* @addr: guest virtual address of guest code
+ * @nofault: do not raise an exception
+ * @hostp: output for host pointer
*
* See get_page_addr_code() (full-system version) for documentation on the
* return value.
@@ -607,10 +609,10 @@ struct MemoryRegionSection *iotlb_to_section(CPUState *cpu,
* If the return value is -1, sets *@hostp to NULL. Otherwise, sets *@hostp
* to the host address where @addr's content is kept.
*
- * Note: this function can trigger an exception.
+ * Note: Unless @nofault, this function can trigger an exception.
*/
tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, target_ulong addr,
- void **hostp);
+ bool nofault, void **hostp);
/**
* get_page_addr_code()
@@ -620,13 +622,11 @@ tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, target_ulong addr,
* If we cannot translate and execute from the entire RAM page, or if
* the region is not backed by RAM, returns -1. Otherwise, returns the
* ram_addr_t corresponding to the guest code at @addr.
- *
- * Note: this function can trigger an exception.
*/
static inline tb_page_addr_t get_page_addr_code(CPUArchState *env,
target_ulong addr)
{
- return get_page_addr_code_hostp(env, addr, NULL);
+ return get_page_addr_code_hostp(env, addr, true, NULL);
}
#if defined(CONFIG_USER_ONLY)
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 2dc2affa12..ae7b40dd51 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1644,16 +1644,16 @@ void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
* of RAM. This will force us to execute by loading and translating
* one insn at a time, without caching.
*
- * NOTE: This function will trigger an exception if the page is
- * not executable.
+ * NOTE: Unless @nofault, this function will trigger an exception
+ * if the page is not executable.
*/
tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, target_ulong addr,
- void **hostp)
+ bool nofault, void **hostp)
{
void *p;
(void)probe_access_internal(env, addr, 1, MMU_INST_FETCH,
- cpu_mmu_index(env, true), true, &p, 0);
+ cpu_mmu_index(env, true), nofault, &p, 0);
if (p == NULL) {
return -1;
}
diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index 3d0b101e34..8377c15383 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -872,7 +872,7 @@ bool plugin_gen_tb_start(CPUState *cpu, const TranslationBlock *tb, bool mem_onl
ptb->vaddr = tb->pc;
ptb->vaddr2 = -1;
- get_page_addr_code_hostp(cpu->env_ptr, tb->pc, &ptb->haddr1);
+ get_page_addr_code_hostp(cpu->env_ptr, tb->pc, true, &ptb->haddr1);
ptb->haddr2 = NULL;
ptb->mem_only = mem_only;
@@ -902,7 +902,7 @@ void plugin_gen_insn_start(CPUState *cpu, const DisasContextBase *db)
unlikely((db->pc_next & TARGET_PAGE_MASK) !=
(db->pc_first & TARGET_PAGE_MASK))) {
get_page_addr_code_hostp(cpu->env_ptr, db->pc_next,
- &ptb->haddr2);
+ true, &ptb->haddr2);
ptb->vaddr2 = db->pc_next;
}
if (likely(ptb->vaddr2 == -1)) {
diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index 58edd33896..e7fec960c2 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -197,11 +197,11 @@ void *probe_access(CPUArchState *env, target_ulong addr, int size,
}
tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, target_ulong addr,
- void **hostp)
+ bool nofault, void **hostp)
{
int flags;
- flags = probe_access_internal(env, addr, 1, MMU_INST_FETCH, true, 0);
+ flags = probe_access_internal(env, addr, 1, MMU_INST_FETCH, nofault, 0);
if (unlikely(flags)) {
return -1;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v6 13/21] accel/tcg: Add nofault parameter to get_page_addr_code_hostp
2022-08-19 3:26 ` [PATCH v6 13/21] accel/tcg: Add nofault parameter to get_page_addr_code_hostp Richard Henderson
@ 2022-08-21 23:37 ` Alistair Francis
0 siblings, 0 replies; 35+ messages in thread
From: Alistair Francis @ 2022-08-21 23:37 UTC (permalink / raw)
To: Richard Henderson
Cc: qemu-devel@nongnu.org Developers, Laurent Vivier, iii,
dramforever, Alistair Francis, Alex Bennée
On Fri, Aug 19, 2022 at 1:36 PM 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
> ---
> include/exec/exec-all.h | 10 +++++-----
> accel/tcg/cputlb.c | 8 ++++----
> accel/tcg/plugin-gen.c | 4 ++--
> accel/tcg/user-exec.c | 4 ++--
> 4 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index 9f35e3b7a9..7a6dc44d86 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -599,6 +599,8 @@ struct MemoryRegionSection *iotlb_to_section(CPUState *cpu,
> * get_page_addr_code_hostp()
> * @env: CPUArchState
> * @addr: guest virtual address of guest code
> + * @nofault: do not raise an exception
> + * @hostp: output for host pointer
> *
> * See get_page_addr_code() (full-system version) for documentation on the
> * return value.
> @@ -607,10 +609,10 @@ struct MemoryRegionSection *iotlb_to_section(CPUState *cpu,
> * If the return value is -1, sets *@hostp to NULL. Otherwise, sets *@hostp
> * to the host address where @addr's content is kept.
> *
> - * Note: this function can trigger an exception.
> + * Note: Unless @nofault, this function can trigger an exception.
> */
> tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, target_ulong addr,
> - void **hostp);
> + bool nofault, void **hostp);
>
> /**
> * get_page_addr_code()
> @@ -620,13 +622,11 @@ tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, target_ulong addr,
> * If we cannot translate and execute from the entire RAM page, or if
> * the region is not backed by RAM, returns -1. Otherwise, returns the
> * ram_addr_t corresponding to the guest code at @addr.
> - *
> - * Note: this function can trigger an exception.
> */
> static inline tb_page_addr_t get_page_addr_code(CPUArchState *env,
> target_ulong addr)
> {
> - return get_page_addr_code_hostp(env, addr, NULL);
> + return get_page_addr_code_hostp(env, addr, true, NULL);
> }
>
> #if defined(CONFIG_USER_ONLY)
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index 2dc2affa12..ae7b40dd51 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -1644,16 +1644,16 @@ void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
> * of RAM. This will force us to execute by loading and translating
> * one insn at a time, without caching.
> *
> - * NOTE: This function will trigger an exception if the page is
> - * not executable.
> + * NOTE: Unless @nofault, this function will trigger an exception
> + * if the page is not executable.
> */
> tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, target_ulong addr,
> - void **hostp)
> + bool nofault, void **hostp)
> {
> void *p;
>
> (void)probe_access_internal(env, addr, 1, MMU_INST_FETCH,
> - cpu_mmu_index(env, true), true, &p, 0);
> + cpu_mmu_index(env, true), nofault, &p, 0);
> if (p == NULL) {
> return -1;
> }
> diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
> index 3d0b101e34..8377c15383 100644
> --- a/accel/tcg/plugin-gen.c
> +++ b/accel/tcg/plugin-gen.c
> @@ -872,7 +872,7 @@ bool plugin_gen_tb_start(CPUState *cpu, const TranslationBlock *tb, bool mem_onl
>
> ptb->vaddr = tb->pc;
> ptb->vaddr2 = -1;
> - get_page_addr_code_hostp(cpu->env_ptr, tb->pc, &ptb->haddr1);
> + get_page_addr_code_hostp(cpu->env_ptr, tb->pc, true, &ptb->haddr1);
> ptb->haddr2 = NULL;
> ptb->mem_only = mem_only;
>
> @@ -902,7 +902,7 @@ void plugin_gen_insn_start(CPUState *cpu, const DisasContextBase *db)
> unlikely((db->pc_next & TARGET_PAGE_MASK) !=
> (db->pc_first & TARGET_PAGE_MASK))) {
> get_page_addr_code_hostp(cpu->env_ptr, db->pc_next,
> - &ptb->haddr2);
> + true, &ptb->haddr2);
> ptb->vaddr2 = db->pc_next;
> }
> if (likely(ptb->vaddr2 == -1)) {
> diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
> index 58edd33896..e7fec960c2 100644
> --- a/accel/tcg/user-exec.c
> +++ b/accel/tcg/user-exec.c
> @@ -197,11 +197,11 @@ void *probe_access(CPUArchState *env, target_ulong addr, int size,
> }
>
> tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, target_ulong addr,
> - void **hostp)
> + bool nofault, void **hostp)
> {
> int flags;
>
> - flags = probe_access_internal(env, addr, 1, MMU_INST_FETCH, true, 0);
> + flags = probe_access_internal(env, addr, 1, MMU_INST_FETCH, nofault, 0);
> if (unlikely(flags)) {
> return -1;
> }
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v6 14/21] accel/tcg: Raise PROT_EXEC exception early
2022-08-19 3:25 [PATCH v6 00/21] linux-user: Fix siginfo_t contents when jumping to non-readable pages Richard Henderson
` (12 preceding siblings ...)
2022-08-19 3:26 ` [PATCH v6 13/21] accel/tcg: Add nofault parameter to get_page_addr_code_hostp Richard Henderson
@ 2022-08-19 3:26 ` Richard Henderson
2022-08-21 23:40 ` Alistair Francis
2022-08-19 3:26 ` [PATCH v6 15/21] accel/tcg: Remove translator_ldsw Richard Henderson
` (7 subsequent siblings)
21 siblings, 1 reply; 35+ messages in thread
From: Richard Henderson @ 2022-08-19 3:26 UTC (permalink / raw)
To: qemu-devel; +Cc: laurent, iii, dramforever, alistair.francis, alex.bennee
We currently ignore PROT_EXEC on the initial lookup, and
defer raising the exception until cpu_ld*_code().
It makes more sense to raise the exception early.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
accel/tcg/cpu-exec.c | 2 +-
accel/tcg/translate-all.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 7887af6f45..7b8977a0a4 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -222,7 +222,7 @@ static TranslationBlock *tb_htable_lookup(CPUState *cpu, target_ulong pc,
desc.cflags = cflags;
desc.trace_vcpu_dstate = *cpu->trace_dstate;
desc.pc = pc;
- phys_pc = get_page_addr_code(desc.env, pc);
+ phys_pc = get_page_addr_code_hostp(desc.env, pc, false, NULL);
if (phys_pc == -1) {
return NULL;
}
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index b83161a081..069ed67bac 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -1396,7 +1396,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
assert_memory_lock();
qemu_thread_jit_write();
- phys_pc = get_page_addr_code(env, pc);
+ phys_pc = get_page_addr_code_hostp(env, pc, false, NULL);
if (phys_pc == -1) {
/* Generate a one-shot TB with 1 insn in it */
--
2.34.1
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v6 14/21] accel/tcg: Raise PROT_EXEC exception early
2022-08-19 3:26 ` [PATCH v6 14/21] accel/tcg: Raise PROT_EXEC exception early Richard Henderson
@ 2022-08-21 23:40 ` Alistair Francis
0 siblings, 0 replies; 35+ messages in thread
From: Alistair Francis @ 2022-08-21 23:40 UTC (permalink / raw)
To: Richard Henderson
Cc: qemu-devel@nongnu.org Developers, Laurent Vivier, iii,
dramforever, Alistair Francis, Alex Bennée
On Fri, Aug 19, 2022 at 1:40 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> We currently ignore PROT_EXEC on the initial lookup, and
> defer raising the exception until cpu_ld*_code().
> It makes more sense to raise the exception early.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> accel/tcg/cpu-exec.c | 2 +-
> accel/tcg/translate-all.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index 7887af6f45..7b8977a0a4 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -222,7 +222,7 @@ static TranslationBlock *tb_htable_lookup(CPUState *cpu, target_ulong pc,
> desc.cflags = cflags;
> desc.trace_vcpu_dstate = *cpu->trace_dstate;
> desc.pc = pc;
> - phys_pc = get_page_addr_code(desc.env, pc);
> + phys_pc = get_page_addr_code_hostp(desc.env, pc, false, NULL);
> if (phys_pc == -1) {
> return NULL;
> }
> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> index b83161a081..069ed67bac 100644
> --- a/accel/tcg/translate-all.c
> +++ b/accel/tcg/translate-all.c
> @@ -1396,7 +1396,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
> assert_memory_lock();
> qemu_thread_jit_write();
>
> - phys_pc = get_page_addr_code(env, pc);
> + phys_pc = get_page_addr_code_hostp(env, pc, false, NULL);
>
> if (phys_pc == -1) {
> /* Generate a one-shot TB with 1 insn in it */
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v6 15/21] accel/tcg: Remove translator_ldsw
2022-08-19 3:25 [PATCH v6 00/21] linux-user: Fix siginfo_t contents when jumping to non-readable pages Richard Henderson
` (13 preceding siblings ...)
2022-08-19 3:26 ` [PATCH v6 14/21] accel/tcg: Raise PROT_EXEC exception early Richard Henderson
@ 2022-08-19 3:26 ` Richard Henderson
2022-08-21 23:41 ` Alistair Francis
2022-08-19 3:26 ` [PATCH v6 16/21] accel/tcg: Add pc and host_pc params to gen_intermediate_code Richard Henderson
` (6 subsequent siblings)
21 siblings, 1 reply; 35+ messages in thread
From: Richard Henderson @ 2022-08-19 3:26 UTC (permalink / raw)
To: qemu-devel; +Cc: laurent, iii, dramforever, alistair.francis, alex.bennee
The only user can easily use translator_lduw and
adjust the type to signed during the return.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/translator.h | 1 -
target/i386/tcg/translate.c | 2 +-
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/include/exec/translator.h b/include/exec/translator.h
index 0d0bf3a31e..45b9268ca4 100644
--- a/include/exec/translator.h
+++ b/include/exec/translator.h
@@ -178,7 +178,6 @@ bool translator_use_goto_tb(DisasContextBase *db, target_ulong dest);
#define FOR_EACH_TRANSLATOR_LD(F) \
F(translator_ldub, uint8_t, cpu_ldub_code, /* no swap */) \
- F(translator_ldsw, int16_t, cpu_ldsw_code, bswap16) \
F(translator_lduw, uint16_t, cpu_lduw_code, bswap16) \
F(translator_ldl, uint32_t, cpu_ldl_code, bswap32) \
F(translator_ldq, uint64_t, cpu_ldq_code, bswap64)
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index b7972f0ff5..a23417d058 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -2033,7 +2033,7 @@ static inline uint8_t x86_ldub_code(CPUX86State *env, DisasContext *s)
static inline int16_t x86_ldsw_code(CPUX86State *env, DisasContext *s)
{
- return translator_ldsw(env, &s->base, advance_pc(env, s, 2));
+ return translator_lduw(env, &s->base, advance_pc(env, s, 2));
}
static inline uint16_t x86_lduw_code(CPUX86State *env, DisasContext *s)
--
2.34.1
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v6 15/21] accel/tcg: Remove translator_ldsw
2022-08-19 3:26 ` [PATCH v6 15/21] accel/tcg: Remove translator_ldsw Richard Henderson
@ 2022-08-21 23:41 ` Alistair Francis
0 siblings, 0 replies; 35+ messages in thread
From: Alistair Francis @ 2022-08-21 23:41 UTC (permalink / raw)
To: Richard Henderson
Cc: qemu-devel@nongnu.org Developers, Laurent Vivier, iii,
dramforever, Alistair Francis, Alex Bennée
On Fri, Aug 19, 2022 at 1:36 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> The only user can easily use translator_lduw and
> adjust the type to signed during the return.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> include/exec/translator.h | 1 -
> target/i386/tcg/translate.c | 2 +-
> 2 files changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/include/exec/translator.h b/include/exec/translator.h
> index 0d0bf3a31e..45b9268ca4 100644
> --- a/include/exec/translator.h
> +++ b/include/exec/translator.h
> @@ -178,7 +178,6 @@ bool translator_use_goto_tb(DisasContextBase *db, target_ulong dest);
>
> #define FOR_EACH_TRANSLATOR_LD(F) \
> F(translator_ldub, uint8_t, cpu_ldub_code, /* no swap */) \
> - F(translator_ldsw, int16_t, cpu_ldsw_code, bswap16) \
> F(translator_lduw, uint16_t, cpu_lduw_code, bswap16) \
> F(translator_ldl, uint32_t, cpu_ldl_code, bswap32) \
> F(translator_ldq, uint64_t, cpu_ldq_code, bswap64)
> diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
> index b7972f0ff5..a23417d058 100644
> --- a/target/i386/tcg/translate.c
> +++ b/target/i386/tcg/translate.c
> @@ -2033,7 +2033,7 @@ static inline uint8_t x86_ldub_code(CPUX86State *env, DisasContext *s)
>
> static inline int16_t x86_ldsw_code(CPUX86State *env, DisasContext *s)
> {
> - return translator_ldsw(env, &s->base, advance_pc(env, s, 2));
> + return translator_lduw(env, &s->base, advance_pc(env, s, 2));
> }
>
> static inline uint16_t x86_lduw_code(CPUX86State *env, DisasContext *s)
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v6 16/21] accel/tcg: Add pc and host_pc params to gen_intermediate_code
2022-08-19 3:25 [PATCH v6 00/21] linux-user: Fix siginfo_t contents when jumping to non-readable pages Richard Henderson
` (14 preceding siblings ...)
2022-08-19 3:26 ` [PATCH v6 15/21] accel/tcg: Remove translator_ldsw Richard Henderson
@ 2022-08-19 3:26 ` Richard Henderson
2022-08-21 23:42 ` Alistair Francis
2022-08-19 3:26 ` [PATCH v6 17/21] accel/tcg: Add fast path for translator_ld* Richard Henderson
` (5 subsequent siblings)
21 siblings, 1 reply; 35+ messages in thread
From: Richard Henderson @ 2022-08-19 3:26 UTC (permalink / raw)
To: qemu-devel; +Cc: laurent, iii, dramforever, alistair.francis, alex.bennee
Pass these along to translator_loop -- pc may be used instead
of tb->pc, and host_pc is currently unused. Adjust all targets
at one time.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/exec-all.h | 1 -
include/exec/translator.h | 24 ++++++++++++++++++++----
accel/tcg/translate-all.c | 3 ++-
accel/tcg/translator.c | 9 +++++----
target/alpha/translate.c | 5 +++--
target/arm/translate.c | 5 +++--
target/avr/translate.c | 5 +++--
target/cris/translate.c | 5 +++--
target/hexagon/translate.c | 6 ++++--
target/hppa/translate.c | 5 +++--
target/i386/tcg/translate.c | 5 +++--
target/loongarch/translate.c | 6 ++++--
target/m68k/translate.c | 5 +++--
target/microblaze/translate.c | 5 +++--
target/mips/tcg/translate.c | 5 +++--
target/nios2/translate.c | 5 +++--
target/openrisc/translate.c | 6 ++++--
target/ppc/translate.c | 5 +++--
target/riscv/translate.c | 5 +++--
target/rx/translate.c | 5 +++--
target/s390x/tcg/translate.c | 5 +++--
target/sh4/translate.c | 5 +++--
target/sparc/translate.c | 5 +++--
target/tricore/translate.c | 6 ++++--
target/xtensa/translate.c | 6 ++++--
25 files changed, 95 insertions(+), 52 deletions(-)
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 7a6dc44d86..4ad166966b 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -39,7 +39,6 @@ typedef ram_addr_t tb_page_addr_t;
#define TB_PAGE_ADDR_FMT RAM_ADDR_FMT
#endif
-void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int max_insns);
void restore_state_to_opc(CPUArchState *env, TranslationBlock *tb,
target_ulong *data);
diff --git a/include/exec/translator.h b/include/exec/translator.h
index 45b9268ca4..69db0f5c21 100644
--- a/include/exec/translator.h
+++ b/include/exec/translator.h
@@ -26,6 +26,19 @@
#include "exec/translate-all.h"
#include "tcg/tcg.h"
+/**
+ * gen_intermediate_code
+ * @cpu: cpu context
+ * @tb: translation block
+ * @max_insns: max number of instructions to translate
+ * @pc: guest virtual program counter address
+ * @host_pc: host physical program counter address
+ *
+ * This function must be provided by the target, which should create
+ * the target-specific DisasContext, and then invoke translator_loop.
+ */
+void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int max_insns,
+ target_ulong pc, void *host_pc);
/**
* DisasJumpType:
@@ -123,11 +136,13 @@ typedef struct TranslatorOps {
/**
* translator_loop:
- * @ops: Target-specific operations.
- * @db: Disassembly context.
* @cpu: Target vCPU.
* @tb: Translation block.
* @max_insns: Maximum number of insns to translate.
+ * @pc: guest virtual program counter address
+ * @host_pc: host physical program counter address
+ * @ops: Target-specific operations.
+ * @db: Disassembly context.
*
* Generic translator loop.
*
@@ -141,8 +156,9 @@ typedef struct TranslatorOps {
* - When single-stepping is enabled (system-wide or on the current vCPU).
* - When too many instructions have been translated.
*/
-void translator_loop(const TranslatorOps *ops, DisasContextBase *db,
- CPUState *cpu, TranslationBlock *tb, int max_insns);
+void translator_loop(CPUState *cpu, TranslationBlock *tb, int max_insns,
+ target_ulong pc, void *host_pc,
+ const TranslatorOps *ops, DisasContextBase *db);
void translator_loop_temp_check(DisasContextBase *db);
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 069ed67bac..b224f856d0 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -46,6 +46,7 @@
#include "exec/cputlb.h"
#include "exec/translate-all.h"
+#include "exec/translator.h"
#include "qemu/bitmap.h"
#include "qemu/qemu-print.h"
#include "qemu/timer.h"
@@ -1444,7 +1445,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
tcg_func_start(tcg_ctx);
tcg_ctx->cpu = env_cpu(env);
- gen_intermediate_code(cpu, tb, max_insns);
+ gen_intermediate_code(cpu, tb, max_insns, pc, host_pc);
assert(tb->size != 0);
tcg_ctx->cpu = NULL;
max_insns = tb->icount;
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index fe7af9b943..3eef30d93a 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -51,16 +51,17 @@ static inline void translator_page_protect(DisasContextBase *dcbase,
#endif
}
-void translator_loop(const TranslatorOps *ops, DisasContextBase *db,
- CPUState *cpu, TranslationBlock *tb, int max_insns)
+void translator_loop(CPUState *cpu, TranslationBlock *tb, int max_insns,
+ target_ulong pc, void *host_pc,
+ const TranslatorOps *ops, DisasContextBase *db)
{
uint32_t cflags = tb_cflags(tb);
bool plugin_enabled;
/* Initialize DisasContext */
db->tb = tb;
- db->pc_first = tb->pc;
- db->pc_next = db->pc_first;
+ db->pc_first = pc;
+ db->pc_next = pc;
db->is_jmp = DISAS_NEXT;
db->num_insns = 0;
db->max_insns = max_insns;
diff --git a/target/alpha/translate.c b/target/alpha/translate.c
index 9af1627079..6766350f56 100644
--- a/target/alpha/translate.c
+++ b/target/alpha/translate.c
@@ -3043,10 +3043,11 @@ static const TranslatorOps alpha_tr_ops = {
.disas_log = alpha_tr_disas_log,
};
-void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int max_insns)
+void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int max_insns,
+ target_ulong pc, void *host_pc)
{
DisasContext dc;
- translator_loop(&alpha_tr_ops, &dc.base, cpu, tb, max_insns);
+ translator_loop(cpu, tb, max_insns, pc, host_pc, &alpha_tr_ops, &dc.base);
}
void restore_state_to_opc(CPUAlphaState *env, TranslationBlock *tb,
diff --git a/target/arm/translate.c b/target/arm/translate.c
index ad617b9948..9474e4b44b 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -9892,7 +9892,8 @@ static const TranslatorOps thumb_translator_ops = {
};
/* generate intermediate code for basic block 'tb'. */
-void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int max_insns)
+void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int max_insns,
+ target_ulong pc, void *host_pc)
{
DisasContext dc = { };
const TranslatorOps *ops = &arm_translator_ops;
@@ -9907,7 +9908,7 @@ void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int max_insns)
}
#endif
- translator_loop(ops, &dc.base, cpu, tb, max_insns);
+ translator_loop(cpu, tb, max_insns, pc, host_pc, ops, &dc.base);
}
void restore_state_to_opc(CPUARMState *env, TranslationBlock *tb,
diff --git a/target/avr/translate.c b/target/avr/translate.c
index dc9c3d6bcc..1da34da103 100644
--- a/target/avr/translate.c
+++ b/target/avr/translate.c
@@ -3031,10 +3031,11 @@ static const TranslatorOps avr_tr_ops = {
.disas_log = avr_tr_disas_log,
};
-void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
+void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns,
+ target_ulong pc, void *host_pc)
{
DisasContext dc = { };
- translator_loop(&avr_tr_ops, &dc.base, cs, tb, max_insns);
+ translator_loop(cs, tb, max_insns, pc, host_pc, &avr_tr_ops, &dc.base);
}
void restore_state_to_opc(CPUAVRState *env, TranslationBlock *tb,
diff --git a/target/cris/translate.c b/target/cris/translate.c
index ac101344a3..73385b0b3c 100644
--- a/target/cris/translate.c
+++ b/target/cris/translate.c
@@ -3286,10 +3286,11 @@ static const TranslatorOps cris_tr_ops = {
.disas_log = cris_tr_disas_log,
};
-void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
+void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns,
+ target_ulong pc, void *host_pc)
{
DisasContext dc;
- translator_loop(&cris_tr_ops, &dc.base, cs, tb, max_insns);
+ translator_loop(cs, tb, max_insns, pc, host_pc, &cris_tr_ops, &dc.base);
}
void cris_cpu_dump_state(CPUState *cs, FILE *f, int flags)
diff --git a/target/hexagon/translate.c b/target/hexagon/translate.c
index d4fc92f7e9..0e8a0772f7 100644
--- a/target/hexagon/translate.c
+++ b/target/hexagon/translate.c
@@ -850,11 +850,13 @@ static const TranslatorOps hexagon_tr_ops = {
.disas_log = hexagon_tr_disas_log,
};
-void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
+void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns,
+ target_ulong pc, void *host_pc)
{
DisasContext ctx;
- translator_loop(&hexagon_tr_ops, &ctx.base, cs, tb, max_insns);
+ translator_loop(cs, tb, max_insns, pc, host_pc,
+ &hexagon_tr_ops, &ctx.base);
}
#define NAME_LEN 64
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index b8dbfee5e9..8b861957e0 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -4340,10 +4340,11 @@ static const TranslatorOps hppa_tr_ops = {
.disas_log = hppa_tr_disas_log,
};
-void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
+void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns,
+ target_ulong pc, void *host_pc)
{
DisasContext ctx;
- translator_loop(&hppa_tr_ops, &ctx.base, cs, tb, max_insns);
+ translator_loop(cs, tb, max_insns, pc, host_pc, &hppa_tr_ops, &ctx.base);
}
void restore_state_to_opc(CPUHPPAState *env, TranslationBlock *tb,
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index a23417d058..4836c889e0 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -8708,11 +8708,12 @@ static const TranslatorOps i386_tr_ops = {
};
/* generate intermediate code for basic block 'tb'. */
-void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int max_insns)
+void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int max_insns,
+ target_ulong pc, void *host_pc)
{
DisasContext dc;
- translator_loop(&i386_tr_ops, &dc.base, cpu, tb, max_insns);
+ translator_loop(cpu, tb, max_insns, pc, host_pc, &i386_tr_ops, &dc.base);
}
void restore_state_to_opc(CPUX86State *env, TranslationBlock *tb,
diff --git a/target/loongarch/translate.c b/target/loongarch/translate.c
index 51ba291430..95b37ea180 100644
--- a/target/loongarch/translate.c
+++ b/target/loongarch/translate.c
@@ -241,11 +241,13 @@ static const TranslatorOps loongarch_tr_ops = {
.disas_log = loongarch_tr_disas_log,
};
-void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
+void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns,
+ target_ulong pc, void *host_pc)
{
DisasContext ctx;
- translator_loop(&loongarch_tr_ops, &ctx.base, cs, tb, max_insns);
+ translator_loop(cs, tb, max_insns, pc, host_pc,
+ &loongarch_tr_ops, &ctx.base);
}
void loongarch_translate_init(void)
diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 8f3c298ad0..5098f7e570 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -6361,10 +6361,11 @@ static const TranslatorOps m68k_tr_ops = {
.disas_log = m68k_tr_disas_log,
};
-void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int max_insns)
+void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int max_insns,
+ target_ulong pc, void *host_pc)
{
DisasContext dc;
- translator_loop(&m68k_tr_ops, &dc.base, cpu, tb, max_insns);
+ translator_loop(cpu, tb, max_insns, pc, host_pc, &m68k_tr_ops, &dc.base);
}
static double floatx80_to_double(CPUM68KState *env, uint16_t high, uint64_t low)
diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index bf01384d33..c5546f93aa 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -1849,10 +1849,11 @@ static const TranslatorOps mb_tr_ops = {
.disas_log = mb_tr_disas_log,
};
-void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int max_insns)
+void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int max_insns,
+ target_ulong pc, void *host_pc)
{
DisasContext dc;
- translator_loop(&mb_tr_ops, &dc.base, cpu, tb, max_insns);
+ translator_loop(cpu, tb, max_insns, pc, host_pc, &mb_tr_ops, &dc.base);
}
void mb_cpu_dump_state(CPUState *cs, FILE *f, int flags)
diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index de1511baaf..0d936e2648 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -16155,11 +16155,12 @@ static const TranslatorOps mips_tr_ops = {
.disas_log = mips_tr_disas_log,
};
-void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
+void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns,
+ target_ulong pc, void *host_pc)
{
DisasContext ctx;
- translator_loop(&mips_tr_ops, &ctx.base, cs, tb, max_insns);
+ translator_loop(cs, tb, max_insns, pc, host_pc, &mips_tr_ops, &ctx.base);
}
void mips_tcg_init(void)
diff --git a/target/nios2/translate.c b/target/nios2/translate.c
index 3a037a68cc..c588e8e885 100644
--- a/target/nios2/translate.c
+++ b/target/nios2/translate.c
@@ -1038,10 +1038,11 @@ static const TranslatorOps nios2_tr_ops = {
.disas_log = nios2_tr_disas_log,
};
-void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
+void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns,
+ target_ulong pc, void *host_pc)
{
DisasContext dc;
- translator_loop(&nios2_tr_ops, &dc.base, cs, tb, max_insns);
+ translator_loop(cs, tb, max_insns, pc, host_pc, &nios2_tr_ops, &dc.base);
}
void nios2_cpu_dump_state(CPUState *cs, FILE *f, int flags)
diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c
index 7b8ad43d5f..8154f9d744 100644
--- a/target/openrisc/translate.c
+++ b/target/openrisc/translate.c
@@ -1705,11 +1705,13 @@ static const TranslatorOps openrisc_tr_ops = {
.disas_log = openrisc_tr_disas_log,
};
-void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
+void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns,
+ target_ulong pc, void *host_pc)
{
DisasContext ctx;
- translator_loop(&openrisc_tr_ops, &ctx.base, cs, tb, max_insns);
+ translator_loop(cs, tb, max_insns, pc, host_pc,
+ &openrisc_tr_ops, &ctx.base);
}
void openrisc_cpu_dump_state(CPUState *cs, FILE *f, int flags)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 388337f81b..000b1e518d 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -7719,11 +7719,12 @@ static const TranslatorOps ppc_tr_ops = {
.disas_log = ppc_tr_disas_log,
};
-void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
+void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns,
+ target_ulong pc, void *host_pc)
{
DisasContext ctx;
- translator_loop(&ppc_tr_ops, &ctx.base, cs, tb, max_insns);
+ translator_loop(cs, tb, max_insns, pc, host_pc, &ppc_tr_ops, &ctx.base);
}
void restore_state_to_opc(CPUPPCState *env, TranslationBlock *tb,
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 63b04e8a94..38666ddc91 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -1196,11 +1196,12 @@ static const TranslatorOps riscv_tr_ops = {
.disas_log = riscv_tr_disas_log,
};
-void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
+void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns,
+ target_ulong pc, void *host_pc)
{
DisasContext ctx;
- translator_loop(&riscv_tr_ops, &ctx.base, cs, tb, max_insns);
+ translator_loop(cs, tb, max_insns, pc, host_pc, &riscv_tr_ops, &ctx.base);
}
void riscv_translate_init(void)
diff --git a/target/rx/translate.c b/target/rx/translate.c
index 62aee66937..ea5653bc95 100644
--- a/target/rx/translate.c
+++ b/target/rx/translate.c
@@ -2363,11 +2363,12 @@ static const TranslatorOps rx_tr_ops = {
.disas_log = rx_tr_disas_log,
};
-void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
+void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns,
+ target_ulong pc, void *host_pc)
{
DisasContext dc;
- translator_loop(&rx_tr_ops, &dc.base, cs, tb, max_insns);
+ translator_loop(cs, tb, max_insns, pc, host_pc, &rx_tr_ops, &dc.base);
}
void restore_state_to_opc(CPURXState *env, TranslationBlock *tb,
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index e2ee005671..d4c0b9b3a2 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -6676,11 +6676,12 @@ static const TranslatorOps s390x_tr_ops = {
.disas_log = s390x_tr_disas_log,
};
-void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
+void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns,
+ target_ulong pc, void *host_pc)
{
DisasContext dc;
- translator_loop(&s390x_tr_ops, &dc.base, cs, tb, max_insns);
+ translator_loop(cs, tb, max_insns, pc, host_pc, &s390x_tr_ops, &dc.base);
}
void restore_state_to_opc(CPUS390XState *env, TranslationBlock *tb,
diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index f1b190e7cf..01056571c3 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -2368,11 +2368,12 @@ static const TranslatorOps sh4_tr_ops = {
.disas_log = sh4_tr_disas_log,
};
-void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
+void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns,
+ target_ulong pc, void *host_pc)
{
DisasContext ctx;
- translator_loop(&sh4_tr_ops, &ctx.base, cs, tb, max_insns);
+ translator_loop(cs, tb, max_insns, pc, host_pc, &sh4_tr_ops, &ctx.base);
}
void restore_state_to_opc(CPUSH4State *env, TranslationBlock *tb,
diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index 2e28222d31..2cbbe2396a 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -5917,11 +5917,12 @@ static const TranslatorOps sparc_tr_ops = {
.disas_log = sparc_tr_disas_log,
};
-void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
+void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns,
+ target_ulong pc, void *host_pc)
{
DisasContext dc = {};
- translator_loop(&sparc_tr_ops, &dc.base, cs, tb, max_insns);
+ translator_loop(cs, tb, max_insns, pc, host_pc, &sparc_tr_ops, &dc.base);
}
void sparc_tcg_init(void)
diff --git a/target/tricore/translate.c b/target/tricore/translate.c
index d170500fa5..a0558ead71 100644
--- a/target/tricore/translate.c
+++ b/target/tricore/translate.c
@@ -8878,10 +8878,12 @@ static const TranslatorOps tricore_tr_ops = {
};
-void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
+void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns,
+ target_ulong pc, void *host_pc)
{
DisasContext ctx;
- translator_loop(&tricore_tr_ops, &ctx.base, cs, tb, max_insns);
+ translator_loop(cs, tb, max_insns, pc, host_pc,
+ &tricore_tr_ops, &ctx.base);
}
void
diff --git a/target/xtensa/translate.c b/target/xtensa/translate.c
index 70e11eeb45..8b864ef925 100644
--- a/target/xtensa/translate.c
+++ b/target/xtensa/translate.c
@@ -1279,10 +1279,12 @@ static const TranslatorOps xtensa_translator_ops = {
.disas_log = xtensa_tr_disas_log,
};
-void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int max_insns)
+void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int max_insns,
+ target_ulong pc, void *host_pc)
{
DisasContext dc = {};
- translator_loop(&xtensa_translator_ops, &dc.base, cpu, tb, max_insns);
+ translator_loop(cpu, tb, max_insns, pc, host_pc,
+ &xtensa_translator_ops, &dc.base);
}
void xtensa_cpu_dump_state(CPUState *cs, FILE *f, int flags)
--
2.34.1
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v6 16/21] accel/tcg: Add pc and host_pc params to gen_intermediate_code
2022-08-19 3:26 ` [PATCH v6 16/21] accel/tcg: Add pc and host_pc params to gen_intermediate_code Richard Henderson
@ 2022-08-21 23:42 ` Alistair Francis
0 siblings, 0 replies; 35+ messages in thread
From: Alistair Francis @ 2022-08-21 23:42 UTC (permalink / raw)
To: Richard Henderson
Cc: qemu-devel@nongnu.org Developers, Laurent Vivier, iii,
dramforever, Alistair Francis, Alex Bennée
On Fri, Aug 19, 2022 at 1:42 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Pass these along to translator_loop -- pc may be used instead
> of tb->pc, and host_pc is currently unused. Adjust all targets
> at one time.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> include/exec/exec-all.h | 1 -
> include/exec/translator.h | 24 ++++++++++++++++++++----
> accel/tcg/translate-all.c | 3 ++-
> accel/tcg/translator.c | 9 +++++----
> target/alpha/translate.c | 5 +++--
> target/arm/translate.c | 5 +++--
> target/avr/translate.c | 5 +++--
> target/cris/translate.c | 5 +++--
> target/hexagon/translate.c | 6 ++++--
> target/hppa/translate.c | 5 +++--
> target/i386/tcg/translate.c | 5 +++--
> target/loongarch/translate.c | 6 ++++--
> target/m68k/translate.c | 5 +++--
> target/microblaze/translate.c | 5 +++--
> target/mips/tcg/translate.c | 5 +++--
> target/nios2/translate.c | 5 +++--
> target/openrisc/translate.c | 6 ++++--
> target/ppc/translate.c | 5 +++--
> target/riscv/translate.c | 5 +++--
> target/rx/translate.c | 5 +++--
> target/s390x/tcg/translate.c | 5 +++--
> target/sh4/translate.c | 5 +++--
> target/sparc/translate.c | 5 +++--
> target/tricore/translate.c | 6 ++++--
> target/xtensa/translate.c | 6 ++++--
> 25 files changed, 95 insertions(+), 52 deletions(-)
>
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index 7a6dc44d86..4ad166966b 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -39,7 +39,6 @@ typedef ram_addr_t tb_page_addr_t;
> #define TB_PAGE_ADDR_FMT RAM_ADDR_FMT
> #endif
>
> -void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int max_insns);
> void restore_state_to_opc(CPUArchState *env, TranslationBlock *tb,
> target_ulong *data);
>
> diff --git a/include/exec/translator.h b/include/exec/translator.h
> index 45b9268ca4..69db0f5c21 100644
> --- a/include/exec/translator.h
> +++ b/include/exec/translator.h
> @@ -26,6 +26,19 @@
> #include "exec/translate-all.h"
> #include "tcg/tcg.h"
>
> +/**
> + * gen_intermediate_code
> + * @cpu: cpu context
> + * @tb: translation block
> + * @max_insns: max number of instructions to translate
> + * @pc: guest virtual program counter address
> + * @host_pc: host physical program counter address
> + *
> + * This function must be provided by the target, which should create
> + * the target-specific DisasContext, and then invoke translator_loop.
> + */
> +void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int max_insns,
> + target_ulong pc, void *host_pc);
>
> /**
> * DisasJumpType:
> @@ -123,11 +136,13 @@ typedef struct TranslatorOps {
>
> /**
> * translator_loop:
> - * @ops: Target-specific operations.
> - * @db: Disassembly context.
> * @cpu: Target vCPU.
> * @tb: Translation block.
> * @max_insns: Maximum number of insns to translate.
> + * @pc: guest virtual program counter address
> + * @host_pc: host physical program counter address
> + * @ops: Target-specific operations.
> + * @db: Disassembly context.
> *
> * Generic translator loop.
> *
> @@ -141,8 +156,9 @@ typedef struct TranslatorOps {
> * - When single-stepping is enabled (system-wide or on the current vCPU).
> * - When too many instructions have been translated.
> */
> -void translator_loop(const TranslatorOps *ops, DisasContextBase *db,
> - CPUState *cpu, TranslationBlock *tb, int max_insns);
> +void translator_loop(CPUState *cpu, TranslationBlock *tb, int max_insns,
> + target_ulong pc, void *host_pc,
> + const TranslatorOps *ops, DisasContextBase *db);
>
> void translator_loop_temp_check(DisasContextBase *db);
>
> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> index 069ed67bac..b224f856d0 100644
> --- a/accel/tcg/translate-all.c
> +++ b/accel/tcg/translate-all.c
> @@ -46,6 +46,7 @@
>
> #include "exec/cputlb.h"
> #include "exec/translate-all.h"
> +#include "exec/translator.h"
> #include "qemu/bitmap.h"
> #include "qemu/qemu-print.h"
> #include "qemu/timer.h"
> @@ -1444,7 +1445,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
> tcg_func_start(tcg_ctx);
>
> tcg_ctx->cpu = env_cpu(env);
> - gen_intermediate_code(cpu, tb, max_insns);
> + gen_intermediate_code(cpu, tb, max_insns, pc, host_pc);
> assert(tb->size != 0);
> tcg_ctx->cpu = NULL;
> max_insns = tb->icount;
> diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
> index fe7af9b943..3eef30d93a 100644
> --- a/accel/tcg/translator.c
> +++ b/accel/tcg/translator.c
> @@ -51,16 +51,17 @@ static inline void translator_page_protect(DisasContextBase *dcbase,
> #endif
> }
>
> -void translator_loop(const TranslatorOps *ops, DisasContextBase *db,
> - CPUState *cpu, TranslationBlock *tb, int max_insns)
> +void translator_loop(CPUState *cpu, TranslationBlock *tb, int max_insns,
> + target_ulong pc, void *host_pc,
> + const TranslatorOps *ops, DisasContextBase *db)
> {
> uint32_t cflags = tb_cflags(tb);
> bool plugin_enabled;
>
> /* Initialize DisasContext */
> db->tb = tb;
> - db->pc_first = tb->pc;
> - db->pc_next = db->pc_first;
> + db->pc_first = pc;
> + db->pc_next = pc;
> db->is_jmp = DISAS_NEXT;
> db->num_insns = 0;
> db->max_insns = max_insns;
> diff --git a/target/alpha/translate.c b/target/alpha/translate.c
> index 9af1627079..6766350f56 100644
> --- a/target/alpha/translate.c
> +++ b/target/alpha/translate.c
> @@ -3043,10 +3043,11 @@ static const TranslatorOps alpha_tr_ops = {
> .disas_log = alpha_tr_disas_log,
> };
>
> -void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int max_insns)
> +void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int max_insns,
> + target_ulong pc, void *host_pc)
> {
> DisasContext dc;
> - translator_loop(&alpha_tr_ops, &dc.base, cpu, tb, max_insns);
> + translator_loop(cpu, tb, max_insns, pc, host_pc, &alpha_tr_ops, &dc.base);
> }
>
> void restore_state_to_opc(CPUAlphaState *env, TranslationBlock *tb,
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index ad617b9948..9474e4b44b 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -9892,7 +9892,8 @@ static const TranslatorOps thumb_translator_ops = {
> };
>
> /* generate intermediate code for basic block 'tb'. */
> -void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int max_insns)
> +void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int max_insns,
> + target_ulong pc, void *host_pc)
> {
> DisasContext dc = { };
> const TranslatorOps *ops = &arm_translator_ops;
> @@ -9907,7 +9908,7 @@ void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int max_insns)
> }
> #endif
>
> - translator_loop(ops, &dc.base, cpu, tb, max_insns);
> + translator_loop(cpu, tb, max_insns, pc, host_pc, ops, &dc.base);
> }
>
> void restore_state_to_opc(CPUARMState *env, TranslationBlock *tb,
> diff --git a/target/avr/translate.c b/target/avr/translate.c
> index dc9c3d6bcc..1da34da103 100644
> --- a/target/avr/translate.c
> +++ b/target/avr/translate.c
> @@ -3031,10 +3031,11 @@ static const TranslatorOps avr_tr_ops = {
> .disas_log = avr_tr_disas_log,
> };
>
> -void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
> +void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns,
> + target_ulong pc, void *host_pc)
> {
> DisasContext dc = { };
> - translator_loop(&avr_tr_ops, &dc.base, cs, tb, max_insns);
> + translator_loop(cs, tb, max_insns, pc, host_pc, &avr_tr_ops, &dc.base);
> }
>
> void restore_state_to_opc(CPUAVRState *env, TranslationBlock *tb,
> diff --git a/target/cris/translate.c b/target/cris/translate.c
> index ac101344a3..73385b0b3c 100644
> --- a/target/cris/translate.c
> +++ b/target/cris/translate.c
> @@ -3286,10 +3286,11 @@ static const TranslatorOps cris_tr_ops = {
> .disas_log = cris_tr_disas_log,
> };
>
> -void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
> +void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns,
> + target_ulong pc, void *host_pc)
> {
> DisasContext dc;
> - translator_loop(&cris_tr_ops, &dc.base, cs, tb, max_insns);
> + translator_loop(cs, tb, max_insns, pc, host_pc, &cris_tr_ops, &dc.base);
> }
>
> void cris_cpu_dump_state(CPUState *cs, FILE *f, int flags)
> diff --git a/target/hexagon/translate.c b/target/hexagon/translate.c
> index d4fc92f7e9..0e8a0772f7 100644
> --- a/target/hexagon/translate.c
> +++ b/target/hexagon/translate.c
> @@ -850,11 +850,13 @@ static const TranslatorOps hexagon_tr_ops = {
> .disas_log = hexagon_tr_disas_log,
> };
>
> -void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
> +void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns,
> + target_ulong pc, void *host_pc)
> {
> DisasContext ctx;
>
> - translator_loop(&hexagon_tr_ops, &ctx.base, cs, tb, max_insns);
> + translator_loop(cs, tb, max_insns, pc, host_pc,
> + &hexagon_tr_ops, &ctx.base);
> }
>
> #define NAME_LEN 64
> diff --git a/target/hppa/translate.c b/target/hppa/translate.c
> index b8dbfee5e9..8b861957e0 100644
> --- a/target/hppa/translate.c
> +++ b/target/hppa/translate.c
> @@ -4340,10 +4340,11 @@ static const TranslatorOps hppa_tr_ops = {
> .disas_log = hppa_tr_disas_log,
> };
>
> -void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
> +void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns,
> + target_ulong pc, void *host_pc)
> {
> DisasContext ctx;
> - translator_loop(&hppa_tr_ops, &ctx.base, cs, tb, max_insns);
> + translator_loop(cs, tb, max_insns, pc, host_pc, &hppa_tr_ops, &ctx.base);
> }
>
> void restore_state_to_opc(CPUHPPAState *env, TranslationBlock *tb,
> diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
> index a23417d058..4836c889e0 100644
> --- a/target/i386/tcg/translate.c
> +++ b/target/i386/tcg/translate.c
> @@ -8708,11 +8708,12 @@ static const TranslatorOps i386_tr_ops = {
> };
>
> /* generate intermediate code for basic block 'tb'. */
> -void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int max_insns)
> +void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int max_insns,
> + target_ulong pc, void *host_pc)
> {
> DisasContext dc;
>
> - translator_loop(&i386_tr_ops, &dc.base, cpu, tb, max_insns);
> + translator_loop(cpu, tb, max_insns, pc, host_pc, &i386_tr_ops, &dc.base);
> }
>
> void restore_state_to_opc(CPUX86State *env, TranslationBlock *tb,
> diff --git a/target/loongarch/translate.c b/target/loongarch/translate.c
> index 51ba291430..95b37ea180 100644
> --- a/target/loongarch/translate.c
> +++ b/target/loongarch/translate.c
> @@ -241,11 +241,13 @@ static const TranslatorOps loongarch_tr_ops = {
> .disas_log = loongarch_tr_disas_log,
> };
>
> -void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
> +void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns,
> + target_ulong pc, void *host_pc)
> {
> DisasContext ctx;
>
> - translator_loop(&loongarch_tr_ops, &ctx.base, cs, tb, max_insns);
> + translator_loop(cs, tb, max_insns, pc, host_pc,
> + &loongarch_tr_ops, &ctx.base);
> }
>
> void loongarch_translate_init(void)
> diff --git a/target/m68k/translate.c b/target/m68k/translate.c
> index 8f3c298ad0..5098f7e570 100644
> --- a/target/m68k/translate.c
> +++ b/target/m68k/translate.c
> @@ -6361,10 +6361,11 @@ static const TranslatorOps m68k_tr_ops = {
> .disas_log = m68k_tr_disas_log,
> };
>
> -void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int max_insns)
> +void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int max_insns,
> + target_ulong pc, void *host_pc)
> {
> DisasContext dc;
> - translator_loop(&m68k_tr_ops, &dc.base, cpu, tb, max_insns);
> + translator_loop(cpu, tb, max_insns, pc, host_pc, &m68k_tr_ops, &dc.base);
> }
>
> static double floatx80_to_double(CPUM68KState *env, uint16_t high, uint64_t low)
> diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
> index bf01384d33..c5546f93aa 100644
> --- a/target/microblaze/translate.c
> +++ b/target/microblaze/translate.c
> @@ -1849,10 +1849,11 @@ static const TranslatorOps mb_tr_ops = {
> .disas_log = mb_tr_disas_log,
> };
>
> -void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int max_insns)
> +void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int max_insns,
> + target_ulong pc, void *host_pc)
> {
> DisasContext dc;
> - translator_loop(&mb_tr_ops, &dc.base, cpu, tb, max_insns);
> + translator_loop(cpu, tb, max_insns, pc, host_pc, &mb_tr_ops, &dc.base);
> }
>
> void mb_cpu_dump_state(CPUState *cs, FILE *f, int flags)
> diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
> index de1511baaf..0d936e2648 100644
> --- a/target/mips/tcg/translate.c
> +++ b/target/mips/tcg/translate.c
> @@ -16155,11 +16155,12 @@ static const TranslatorOps mips_tr_ops = {
> .disas_log = mips_tr_disas_log,
> };
>
> -void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
> +void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns,
> + target_ulong pc, void *host_pc)
> {
> DisasContext ctx;
>
> - translator_loop(&mips_tr_ops, &ctx.base, cs, tb, max_insns);
> + translator_loop(cs, tb, max_insns, pc, host_pc, &mips_tr_ops, &ctx.base);
> }
>
> void mips_tcg_init(void)
> diff --git a/target/nios2/translate.c b/target/nios2/translate.c
> index 3a037a68cc..c588e8e885 100644
> --- a/target/nios2/translate.c
> +++ b/target/nios2/translate.c
> @@ -1038,10 +1038,11 @@ static const TranslatorOps nios2_tr_ops = {
> .disas_log = nios2_tr_disas_log,
> };
>
> -void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
> +void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns,
> + target_ulong pc, void *host_pc)
> {
> DisasContext dc;
> - translator_loop(&nios2_tr_ops, &dc.base, cs, tb, max_insns);
> + translator_loop(cs, tb, max_insns, pc, host_pc, &nios2_tr_ops, &dc.base);
> }
>
> void nios2_cpu_dump_state(CPUState *cs, FILE *f, int flags)
> diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c
> index 7b8ad43d5f..8154f9d744 100644
> --- a/target/openrisc/translate.c
> +++ b/target/openrisc/translate.c
> @@ -1705,11 +1705,13 @@ static const TranslatorOps openrisc_tr_ops = {
> .disas_log = openrisc_tr_disas_log,
> };
>
> -void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
> +void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns,
> + target_ulong pc, void *host_pc)
> {
> DisasContext ctx;
>
> - translator_loop(&openrisc_tr_ops, &ctx.base, cs, tb, max_insns);
> + translator_loop(cs, tb, max_insns, pc, host_pc,
> + &openrisc_tr_ops, &ctx.base);
> }
>
> void openrisc_cpu_dump_state(CPUState *cs, FILE *f, int flags)
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 388337f81b..000b1e518d 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -7719,11 +7719,12 @@ static const TranslatorOps ppc_tr_ops = {
> .disas_log = ppc_tr_disas_log,
> };
>
> -void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
> +void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns,
> + target_ulong pc, void *host_pc)
> {
> DisasContext ctx;
>
> - translator_loop(&ppc_tr_ops, &ctx.base, cs, tb, max_insns);
> + translator_loop(cs, tb, max_insns, pc, host_pc, &ppc_tr_ops, &ctx.base);
> }
>
> void restore_state_to_opc(CPUPPCState *env, TranslationBlock *tb,
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 63b04e8a94..38666ddc91 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -1196,11 +1196,12 @@ static const TranslatorOps riscv_tr_ops = {
> .disas_log = riscv_tr_disas_log,
> };
>
> -void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
> +void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns,
> + target_ulong pc, void *host_pc)
> {
> DisasContext ctx;
>
> - translator_loop(&riscv_tr_ops, &ctx.base, cs, tb, max_insns);
> + translator_loop(cs, tb, max_insns, pc, host_pc, &riscv_tr_ops, &ctx.base);
> }
>
> void riscv_translate_init(void)
> diff --git a/target/rx/translate.c b/target/rx/translate.c
> index 62aee66937..ea5653bc95 100644
> --- a/target/rx/translate.c
> +++ b/target/rx/translate.c
> @@ -2363,11 +2363,12 @@ static const TranslatorOps rx_tr_ops = {
> .disas_log = rx_tr_disas_log,
> };
>
> -void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
> +void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns,
> + target_ulong pc, void *host_pc)
> {
> DisasContext dc;
>
> - translator_loop(&rx_tr_ops, &dc.base, cs, tb, max_insns);
> + translator_loop(cs, tb, max_insns, pc, host_pc, &rx_tr_ops, &dc.base);
> }
>
> void restore_state_to_opc(CPURXState *env, TranslationBlock *tb,
> diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
> index e2ee005671..d4c0b9b3a2 100644
> --- a/target/s390x/tcg/translate.c
> +++ b/target/s390x/tcg/translate.c
> @@ -6676,11 +6676,12 @@ static const TranslatorOps s390x_tr_ops = {
> .disas_log = s390x_tr_disas_log,
> };
>
> -void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
> +void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns,
> + target_ulong pc, void *host_pc)
> {
> DisasContext dc;
>
> - translator_loop(&s390x_tr_ops, &dc.base, cs, tb, max_insns);
> + translator_loop(cs, tb, max_insns, pc, host_pc, &s390x_tr_ops, &dc.base);
> }
>
> void restore_state_to_opc(CPUS390XState *env, TranslationBlock *tb,
> diff --git a/target/sh4/translate.c b/target/sh4/translate.c
> index f1b190e7cf..01056571c3 100644
> --- a/target/sh4/translate.c
> +++ b/target/sh4/translate.c
> @@ -2368,11 +2368,12 @@ static const TranslatorOps sh4_tr_ops = {
> .disas_log = sh4_tr_disas_log,
> };
>
> -void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
> +void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns,
> + target_ulong pc, void *host_pc)
> {
> DisasContext ctx;
>
> - translator_loop(&sh4_tr_ops, &ctx.base, cs, tb, max_insns);
> + translator_loop(cs, tb, max_insns, pc, host_pc, &sh4_tr_ops, &ctx.base);
> }
>
> void restore_state_to_opc(CPUSH4State *env, TranslationBlock *tb,
> diff --git a/target/sparc/translate.c b/target/sparc/translate.c
> index 2e28222d31..2cbbe2396a 100644
> --- a/target/sparc/translate.c
> +++ b/target/sparc/translate.c
> @@ -5917,11 +5917,12 @@ static const TranslatorOps sparc_tr_ops = {
> .disas_log = sparc_tr_disas_log,
> };
>
> -void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
> +void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns,
> + target_ulong pc, void *host_pc)
> {
> DisasContext dc = {};
>
> - translator_loop(&sparc_tr_ops, &dc.base, cs, tb, max_insns);
> + translator_loop(cs, tb, max_insns, pc, host_pc, &sparc_tr_ops, &dc.base);
> }
>
> void sparc_tcg_init(void)
> diff --git a/target/tricore/translate.c b/target/tricore/translate.c
> index d170500fa5..a0558ead71 100644
> --- a/target/tricore/translate.c
> +++ b/target/tricore/translate.c
> @@ -8878,10 +8878,12 @@ static const TranslatorOps tricore_tr_ops = {
> };
>
>
> -void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
> +void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns,
> + target_ulong pc, void *host_pc)
> {
> DisasContext ctx;
> - translator_loop(&tricore_tr_ops, &ctx.base, cs, tb, max_insns);
> + translator_loop(cs, tb, max_insns, pc, host_pc,
> + &tricore_tr_ops, &ctx.base);
> }
>
> void
> diff --git a/target/xtensa/translate.c b/target/xtensa/translate.c
> index 70e11eeb45..8b864ef925 100644
> --- a/target/xtensa/translate.c
> +++ b/target/xtensa/translate.c
> @@ -1279,10 +1279,12 @@ static const TranslatorOps xtensa_translator_ops = {
> .disas_log = xtensa_tr_disas_log,
> };
>
> -void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int max_insns)
> +void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb, int max_insns,
> + target_ulong pc, void *host_pc)
> {
> DisasContext dc = {};
> - translator_loop(&xtensa_translator_ops, &dc.base, cpu, tb, max_insns);
> + translator_loop(cpu, tb, max_insns, pc, host_pc,
> + &xtensa_translator_ops, &dc.base);
> }
>
> void xtensa_cpu_dump_state(CPUState *cs, FILE *f, int flags)
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v6 17/21] accel/tcg: Add fast path for translator_ld*
2022-08-19 3:25 [PATCH v6 00/21] linux-user: Fix siginfo_t contents when jumping to non-readable pages Richard Henderson
` (15 preceding siblings ...)
2022-08-19 3:26 ` [PATCH v6 16/21] accel/tcg: Add pc and host_pc params to gen_intermediate_code Richard Henderson
@ 2022-08-19 3:26 ` Richard Henderson
2022-08-22 23:15 ` Ilya Leoshkevich
2022-08-19 3:26 ` [PATCH v6 18/21] target/s390x: Make translator stop before the end of a page Richard Henderson
` (4 subsequent siblings)
21 siblings, 1 reply; 35+ messages in thread
From: Richard Henderson @ 2022-08-19 3:26 UTC (permalink / raw)
To: qemu-devel; +Cc: laurent, iii, dramforever, alistair.francis, alex.bennee
Cache the translation from guest to host address, so we may
use direct loads when we hit on the primary translation page.
Look up the second translation page only once, during translation.
This obviates another lookup of the second page within tb_gen_code
after translation.
Fixes a bug in that plugin_insn_append should be passed the bytes
in the original memory order, not bswapped by pieces.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/translator.h | 63 +++++++++++--------
accel/tcg/translate-all.c | 26 +++-----
accel/tcg/translator.c | 127 +++++++++++++++++++++++++++++---------
3 files changed, 144 insertions(+), 72 deletions(-)
diff --git a/include/exec/translator.h b/include/exec/translator.h
index 69db0f5c21..329a42fe46 100644
--- a/include/exec/translator.h
+++ b/include/exec/translator.h
@@ -81,24 +81,14 @@ typedef enum DisasJumpType {
* Architecture-agnostic disassembly context.
*/
typedef struct DisasContextBase {
- const TranslationBlock *tb;
+ TranslationBlock *tb;
target_ulong pc_first;
target_ulong pc_next;
DisasJumpType is_jmp;
int num_insns;
int max_insns;
bool singlestep_enabled;
-#ifdef CONFIG_USER_ONLY
- /*
- * Guest address of the last byte of the last protected page.
- *
- * Pages containing the translated instructions are made non-writable in
- * order to achieve consistency in case another thread is modifying the
- * code while translate_insn() fetches the instruction bytes piecemeal.
- * Such writer threads are blocked on mmap_lock() in page_unprotect().
- */
- target_ulong page_protect_end;
-#endif
+ void *host_addr[2];
} DisasContextBase;
/**
@@ -183,24 +173,43 @@ bool translator_use_goto_tb(DisasContextBase *db, target_ulong dest);
* the relevant information at translation time.
*/
-#define GEN_TRANSLATOR_LD(fullname, type, load_fn, swap_fn) \
- type fullname ## _swap(CPUArchState *env, DisasContextBase *dcbase, \
- abi_ptr pc, bool do_swap); \
- static inline type fullname(CPUArchState *env, \
- DisasContextBase *dcbase, abi_ptr pc) \
- { \
- return fullname ## _swap(env, dcbase, pc, false); \
+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);
+
+static inline uint16_t
+translator_lduw_swap(CPUArchState *env, DisasContextBase *db,
+ abi_ptr pc, bool do_swap)
+{
+ uint16_t ret = translator_lduw(env, db, pc);
+ if (do_swap) {
+ ret = bswap16(ret);
}
+ return ret;
+}
-#define FOR_EACH_TRANSLATOR_LD(F) \
- F(translator_ldub, uint8_t, cpu_ldub_code, /* no swap */) \
- F(translator_lduw, uint16_t, cpu_lduw_code, bswap16) \
- F(translator_ldl, uint32_t, cpu_ldl_code, bswap32) \
- F(translator_ldq, uint64_t, cpu_ldq_code, bswap64)
+static inline uint32_t
+translator_ldl_swap(CPUArchState *env, DisasContextBase *db,
+ abi_ptr pc, bool do_swap)
+{
+ uint32_t ret = translator_ldl(env, db, pc);
+ if (do_swap) {
+ ret = bswap32(ret);
+ }
+ return ret;
+}
-FOR_EACH_TRANSLATOR_LD(GEN_TRANSLATOR_LD)
-
-#undef GEN_TRANSLATOR_LD
+static inline uint64_t
+translator_ldq_swap(CPUArchState *env, DisasContextBase *db,
+ abi_ptr pc, bool do_swap)
+{
+ uint64_t ret = translator_ldq_swap(env, db, pc, false);
+ if (do_swap) {
+ ret = bswap64(ret);
+ }
+ return ret;
+}
/*
* Return whether addr is on the same page as where disassembly started.
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index b224f856d0..e44f40b234 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -1385,10 +1385,10 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
{
CPUArchState *env = cpu->env_ptr;
TranslationBlock *tb, *existing_tb;
- tb_page_addr_t phys_pc, phys_page2;
- target_ulong virt_page2;
+ tb_page_addr_t phys_pc;
tcg_insn_unit *gen_code_buf;
int gen_code_size, search_size, max_insns;
+ void *host_pc;
#ifdef CONFIG_PROFILER
TCGProfile *prof = &tcg_ctx->prof;
int64_t ti;
@@ -1397,7 +1397,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
assert_memory_lock();
qemu_thread_jit_write();
- phys_pc = get_page_addr_code_hostp(env, pc, false, NULL);
+ phys_pc = get_page_addr_code_hostp(env, pc, false, &host_pc);
if (phys_pc == -1) {
/* Generate a one-shot TB with 1 insn in it */
@@ -1428,6 +1428,8 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
tb->flags = flags;
tb->cflags = cflags;
tb->trace_vcpu_dstate = *cpu->trace_dstate;
+ tb->page_addr[0] = phys_pc;
+ tb->page_addr[1] = -1;
tcg_ctx->tb_cflags = cflags;
tb_overflow:
@@ -1621,13 +1623,11 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
}
/*
- * If the TB is not associated with a physical RAM page then
- * it must be a temporary one-insn TB, and we have nothing to do
- * except fill in the page_addr[] fields. Return early before
- * attempting to link to other TBs or add to the lookup table.
+ * If the TB is not associated with a physical RAM page then it must be
+ * a temporary one-insn TB, and we have nothing left to do. Return early
+ * before attempting to link to other TBs or add to the lookup table.
*/
- if (phys_pc == -1) {
- tb->page_addr[0] = tb->page_addr[1] = -1;
+ if (tb->page_addr[0] == -1) {
return tb;
}
@@ -1638,17 +1638,11 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
*/
tcg_tb_insert(tb);
- /* check next page if needed */
- virt_page2 = (pc + tb->size - 1) & TARGET_PAGE_MASK;
- phys_page2 = -1;
- if ((pc & TARGET_PAGE_MASK) != virt_page2) {
- phys_page2 = get_page_addr_code(env, virt_page2);
- }
/*
* No explicit memory barrier is required -- tb_link_page() makes the
* TB visible in a consistent state.
*/
- existing_tb = tb_link_page(tb, phys_pc, phys_page2);
+ existing_tb = tb_link_page(tb, tb->page_addr[0], tb->page_addr[1]);
/* if the TB already exists, discard what we just translated */
if (unlikely(existing_tb != tb)) {
uintptr_t orig_aligned = (uintptr_t)gen_code_buf;
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index 3eef30d93a..c8e9523e52 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -42,15 +42,6 @@ bool translator_use_goto_tb(DisasContextBase *db, target_ulong dest)
return ((db->pc_first ^ dest) & TARGET_PAGE_MASK) == 0;
}
-static inline void translator_page_protect(DisasContextBase *dcbase,
- target_ulong pc)
-{
-#ifdef CONFIG_USER_ONLY
- dcbase->page_protect_end = pc | ~TARGET_PAGE_MASK;
- page_protect(pc);
-#endif
-}
-
void translator_loop(CPUState *cpu, TranslationBlock *tb, int max_insns,
target_ulong pc, void *host_pc,
const TranslatorOps *ops, DisasContextBase *db)
@@ -66,7 +57,12 @@ void translator_loop(CPUState *cpu, TranslationBlock *tb, int max_insns,
db->num_insns = 0;
db->max_insns = max_insns;
db->singlestep_enabled = cflags & CF_SINGLE_STEP;
- translator_page_protect(db, db->pc_next);
+ db->host_addr[0] = host_pc;
+ db->host_addr[1] = NULL;
+
+#ifdef CONFIG_USER_ONLY
+ page_protect(pc);
+#endif
ops->init_disas_context(db, cpu);
tcg_debug_assert(db->is_jmp == DISAS_NEXT); /* no early exit */
@@ -151,31 +147,104 @@ void translator_loop(CPUState *cpu, TranslationBlock *tb, int max_insns,
#endif
}
-static inline void translator_maybe_page_protect(DisasContextBase *dcbase,
- target_ulong pc, size_t len)
+static void *translator_access(CPUArchState *env, DisasContextBase *db,
+ target_ulong pc, size_t len)
{
-#ifdef CONFIG_USER_ONLY
- target_ulong end = pc + len - 1;
+ void *host;
+ target_ulong base, end;
+ TranslationBlock *tb;
- if (end > dcbase->page_protect_end) {
- translator_page_protect(dcbase, end);
+ tb = db->tb;
+
+ /* Use slow path if first page is MMIO. */
+ if (unlikely(tb->page_addr[0] == -1)) {
+ return NULL;
}
+
+ 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[1];
+ base = TARGET_PAGE_ALIGN(db->pc_first);
+ if (host == NULL) {
+ tb->page_addr[1] =
+ get_page_addr_code_hostp(env, base, false,
+ &db->host_addr[1]);
+#ifdef CONFIG_USER_ONLY
+ page_protect(end);
#endif
+ /* We cannot handle MMIO as second page. */
+ assert(tb->page_addr[1] != -1);
+ 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);
}
-#define GEN_TRANSLATOR_LD(fullname, type, load_fn, swap_fn) \
- type fullname ## _swap(CPUArchState *env, DisasContextBase *dcbase, \
- abi_ptr pc, bool do_swap) \
- { \
- translator_maybe_page_protect(dcbase, pc, sizeof(type)); \
- type ret = load_fn(env, pc); \
- if (do_swap) { \
- ret = swap_fn(ret); \
- } \
- plugin_insn_append(pc, &ret, sizeof(ret)); \
- return ret; \
+uint8_t translator_ldub(CPUArchState *env, DisasContextBase *db, abi_ptr pc)
+{
+ uint8_t ret;
+ void *p = translator_access(env, db, pc, sizeof(ret));
+
+ if (p) {
+ plugin_insn_append(pc, p, sizeof(ret));
+ return ldub_p(p);
}
+ ret = cpu_ldub_code(env, pc);
+ plugin_insn_append(pc, &ret, sizeof(ret));
+ return ret;
+}
-FOR_EACH_TRANSLATOR_LD(GEN_TRANSLATOR_LD)
+uint16_t translator_lduw(CPUArchState *env, DisasContextBase *db, abi_ptr pc)
+{
+ uint16_t ret, plug;
+ void *p = translator_access(env, db, pc, sizeof(ret));
-#undef GEN_TRANSLATOR_LD
+ if (p) {
+ plugin_insn_append(pc, p, sizeof(ret));
+ return lduw_p(p);
+ }
+ ret = cpu_lduw_code(env, pc);
+ plug = tswap16(ret);
+ plugin_insn_append(pc, &plug, sizeof(ret));
+ return ret;
+}
+
+uint32_t translator_ldl(CPUArchState *env, DisasContextBase *db, abi_ptr pc)
+{
+ uint32_t ret, plug;
+ void *p = translator_access(env, db, pc, sizeof(ret));
+
+ if (p) {
+ plugin_insn_append(pc, p, sizeof(ret));
+ return ldl_p(p);
+ }
+ ret = cpu_ldl_code(env, pc);
+ plug = tswap32(ret);
+ plugin_insn_append(pc, &plug, sizeof(ret));
+ return ret;
+}
+
+uint64_t translator_ldq(CPUArchState *env, DisasContextBase *db, abi_ptr pc)
+{
+ uint64_t ret, plug;
+ void *p = translator_access(env, db, pc, sizeof(ret));
+
+ if (p) {
+ plugin_insn_append(pc, p, sizeof(ret));
+ return ldq_p(p);
+ }
+ ret = cpu_ldq_code(env, pc);
+ plug = tswap64(ret);
+ plugin_insn_append(pc, &plug, sizeof(ret));
+ return ret;
+}
--
2.34.1
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v6 17/21] accel/tcg: Add fast path for translator_ld*
2022-08-19 3:26 ` [PATCH v6 17/21] accel/tcg: Add fast path for translator_ld* Richard Henderson
@ 2022-08-22 23:15 ` Ilya Leoshkevich
0 siblings, 0 replies; 35+ messages in thread
From: Ilya Leoshkevich @ 2022-08-22 23:15 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: laurent, dramforever, alistair.francis, alex.bennee
On Thu, 2022-08-18 at 20:26 -0700, Richard Henderson wrote:
> Cache the translation from guest to host address, so we may
> use direct loads when we hit on the primary translation page.
>
> Look up the second translation page only once, during translation.
> This obviates another lookup of the second page within tb_gen_code
> after translation.
>
> Fixes a bug in that plugin_insn_append should be passed the bytes
> in the original memory order, not bswapped by pieces.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> include/exec/translator.h | 63 +++++++++++--------
> accel/tcg/translate-all.c | 26 +++-----
> accel/tcg/translator.c | 127 +++++++++++++++++++++++++++++-------
> --
> 3 files changed, 144 insertions(+), 72 deletions(-)
>
> diff --git a/include/exec/translator.h b/include/exec/translator.h
> index 69db0f5c21..329a42fe46 100644
> --- a/include/exec/translator.h
> +++ b/include/exec/translator.h
> @@ -81,24 +81,14 @@ typedef enum DisasJumpType {
> * Architecture-agnostic disassembly context.
> */
> typedef struct DisasContextBase {
> - const TranslationBlock *tb;
> + TranslationBlock *tb;
> target_ulong pc_first;
> target_ulong pc_next;
> DisasJumpType is_jmp;
> int num_insns;
> int max_insns;
> bool singlestep_enabled;
> -#ifdef CONFIG_USER_ONLY
> - /*
> - * Guest address of the last byte of the last protected page.
> - *
> - * Pages containing the translated instructions are made non-
> writable in
> - * order to achieve consistency in case another thread is
> modifying the
> - * code while translate_insn() fetches the instruction bytes
> piecemeal.
> - * Such writer threads are blocked on mmap_lock() in
> page_unprotect().
> - */
> - target_ulong page_protect_end;
> -#endif
> + void *host_addr[2];
> } DisasContextBase;
>
> /**
> @@ -183,24 +173,43 @@ bool translator_use_goto_tb(DisasContextBase
> *db, target_ulong dest);
> * the relevant information at translation time.
> */
>
> -#define GEN_TRANSLATOR_LD(fullname, type, load_fn,
> swap_fn) \
> - type fullname ## _swap(CPUArchState *env, DisasContextBase
> *dcbase, \
> - abi_ptr pc, bool
> do_swap); \
> - static inline type fullname(CPUArchState
> *env, \
> - DisasContextBase *dcbase, abi_ptr
> pc) \
> - {
> \
> - return fullname ## _swap(env, dcbase, pc,
> false); \
> +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);
> +
> +static inline uint16_t
> +translator_lduw_swap(CPUArchState *env, DisasContextBase *db,
> + abi_ptr pc, bool do_swap)
> +{
> + uint16_t ret = translator_lduw(env, db, pc);
> + if (do_swap) {
> + ret = bswap16(ret);
> }
> + return ret;
> +}
>
> -#define
> FOR_EACH_TRANSLATOR_LD(F) \
> - F(translator_ldub, uint8_t, cpu_ldub_code, /* no swap
> */) \
> - F(translator_lduw, uint16_t, cpu_lduw_code,
> bswap16) \
> - F(translator_ldl, uint32_t, cpu_ldl_code,
> bswap32) \
> - F(translator_ldq, uint64_t, cpu_ldq_code, bswap64)
> +static inline uint32_t
> +translator_ldl_swap(CPUArchState *env, DisasContextBase *db,
> + abi_ptr pc, bool do_swap)
> +{
> + uint32_t ret = translator_ldl(env, db, pc);
> + if (do_swap) {
> + ret = bswap32(ret);
> + }
> + return ret;
> +}
>
> -FOR_EACH_TRANSLATOR_LD(GEN_TRANSLATOR_LD)
> -
> -#undef GEN_TRANSLATOR_LD
> +static inline uint64_t
> +translator_ldq_swap(CPUArchState *env, DisasContextBase *db,
> + abi_ptr pc, bool do_swap)
> +{
> + uint64_t ret = translator_ldq_swap(env, db, pc, false);
> + if (do_swap) {
> + ret = bswap64(ret);
> + }
> + return ret;
> +}
>
> /*
> * Return whether addr is on the same page as where disassembly
> started.
> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> index b224f856d0..e44f40b234 100644
> --- a/accel/tcg/translate-all.c
> +++ b/accel/tcg/translate-all.c
> @@ -1385,10 +1385,10 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
> {
> CPUArchState *env = cpu->env_ptr;
> TranslationBlock *tb, *existing_tb;
> - tb_page_addr_t phys_pc, phys_page2;
> - target_ulong virt_page2;
> + tb_page_addr_t phys_pc;
> tcg_insn_unit *gen_code_buf;
> int gen_code_size, search_size, max_insns;
> + void *host_pc;
> #ifdef CONFIG_PROFILER
> TCGProfile *prof = &tcg_ctx->prof;
> int64_t ti;
> @@ -1397,7 +1397,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
> assert_memory_lock();
> qemu_thread_jit_write();
>
> - phys_pc = get_page_addr_code_hostp(env, pc, false, NULL);
> + phys_pc = get_page_addr_code_hostp(env, pc, false, &host_pc);
>
> if (phys_pc == -1) {
> /* Generate a one-shot TB with 1 insn in it */
> @@ -1428,6 +1428,8 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
> tb->flags = flags;
> tb->cflags = cflags;
> tb->trace_vcpu_dstate = *cpu->trace_dstate;
> + tb->page_addr[0] = phys_pc;
> + tb->page_addr[1] = -1;
> tcg_ctx->tb_cflags = cflags;
> tb_overflow:
>
> @@ -1621,13 +1623,11 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
> }
>
> /*
> - * If the TB is not associated with a physical RAM page then
> - * it must be a temporary one-insn TB, and we have nothing to do
> - * except fill in the page_addr[] fields. Return early before
> - * attempting to link to other TBs or add to the lookup table.
> + * If the TB is not associated with a physical RAM page then it
> must be
> + * a temporary one-insn TB, and we have nothing left to do.
> Return early
> + * before attempting to link to other TBs or add to the lookup
> table.
> */
> - if (phys_pc == -1) {
> - tb->page_addr[0] = tb->page_addr[1] = -1;
> + if (tb->page_addr[0] == -1) {
> return tb;
> }
>
> @@ -1638,17 +1638,11 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
> */
> tcg_tb_insert(tb);
>
> - /* check next page if needed */
> - virt_page2 = (pc + tb->size - 1) & TARGET_PAGE_MASK;
> - phys_page2 = -1;
> - if ((pc & TARGET_PAGE_MASK) != virt_page2) {
> - phys_page2 = get_page_addr_code(env, virt_page2);
> - }
> /*
> * No explicit memory barrier is required -- tb_link_page()
> makes the
> * TB visible in a consistent state.
> */
> - existing_tb = tb_link_page(tb, phys_pc, phys_page2);
> + existing_tb = tb_link_page(tb, tb->page_addr[0], tb-
> >page_addr[1]);
> /* if the TB already exists, discard what we just translated */
> if (unlikely(existing_tb != tb)) {
> uintptr_t orig_aligned = (uintptr_t)gen_code_buf;
> diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
> index 3eef30d93a..c8e9523e52 100644
> --- a/accel/tcg/translator.c
> +++ b/accel/tcg/translator.c
> @@ -42,15 +42,6 @@ bool translator_use_goto_tb(DisasContextBase *db,
> target_ulong dest)
> return ((db->pc_first ^ dest) & TARGET_PAGE_MASK) == 0;
> }
>
> -static inline void translator_page_protect(DisasContextBase *dcbase,
> - target_ulong pc)
> -{
> -#ifdef CONFIG_USER_ONLY
> - dcbase->page_protect_end = pc | ~TARGET_PAGE_MASK;
> - page_protect(pc);
> -#endif
> -}
> -
> void translator_loop(CPUState *cpu, TranslationBlock *tb, int
> max_insns,
> target_ulong pc, void *host_pc,
> const TranslatorOps *ops, DisasContextBase *db)
> @@ -66,7 +57,12 @@ void translator_loop(CPUState *cpu,
> TranslationBlock *tb, int max_insns,
> db->num_insns = 0;
> db->max_insns = max_insns;
> db->singlestep_enabled = cflags & CF_SINGLE_STEP;
> - translator_page_protect(db, db->pc_next);
> + db->host_addr[0] = host_pc;
> + db->host_addr[1] = NULL;
> +
> +#ifdef CONFIG_USER_ONLY
> + page_protect(pc);
> +#endif
>
> ops->init_disas_context(db, cpu);
> tcg_debug_assert(db->is_jmp == DISAS_NEXT); /* no early exit */
> @@ -151,31 +147,104 @@ void translator_loop(CPUState *cpu,
> TranslationBlock *tb, int max_insns,
> #endif
> }
>
> -static inline void translator_maybe_page_protect(DisasContextBase
> *dcbase,
> - target_ulong pc,
> size_t len)
> +static void *translator_access(CPUArchState *env, DisasContextBase
> *db,
> + target_ulong pc, size_t len)
> {
> -#ifdef CONFIG_USER_ONLY
> - target_ulong end = pc + len - 1;
> + void *host;
> + target_ulong base, end;
> + TranslationBlock *tb;
>
> - if (end > dcbase->page_protect_end) {
> - translator_page_protect(dcbase, end);
> + tb = db->tb;
> +
> + /* Use slow path if first page is MMIO. */
> + if (unlikely(tb->page_addr[0] == -1)) {
> + return NULL;
> }
> +
> + 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[1];
> + base = TARGET_PAGE_ALIGN(db->pc_first);
> + if (host == NULL) {
> + tb->page_addr[1] =
> + get_page_addr_code_hostp(env, base, false,
> + &db->host_addr[1]);
> +#ifdef CONFIG_USER_ONLY
> + page_protect(end);
> #endif
> + /* We cannot handle MMIO as second page. */
> + assert(tb->page_addr[1] != -1);
> + 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);
> }
>
> -#define GEN_TRANSLATOR_LD(fullname, type, load_fn,
> swap_fn) \
> - type fullname ## _swap(CPUArchState *env, DisasContextBase
> *dcbase, \
> - abi_ptr pc, bool
> do_swap) \
> - {
> \
> - translator_maybe_page_protect(dcbase, pc,
> sizeof(type)); \
> - type ret = load_fn(env,
> pc); \
> - if (do_swap)
> { \
> - ret =
> swap_fn(ret); \
> - }
> \
> - plugin_insn_append(pc, &ret,
> sizeof(ret)); \
> - return
> ret; \
> +uint8_t translator_ldub(CPUArchState *env, DisasContextBase *db,
> abi_ptr pc)
> +{
> + uint8_t ret;
> + void *p = translator_access(env, db, pc, sizeof(ret));
> +
> + if (p) {
> + plugin_insn_append(pc, p, sizeof(ret));
> + return ldub_p(p);
> }
> + ret = cpu_ldub_code(env, pc);
> + plugin_insn_append(pc, &ret, sizeof(ret));
> + return ret;
> +}
>
> -FOR_EACH_TRANSLATOR_LD(GEN_TRANSLATOR_LD)
> +uint16_t translator_lduw(CPUArchState *env, DisasContextBase *db,
> abi_ptr pc)
> +{
> + uint16_t ret, plug;
> + void *p = translator_access(env, db, pc, sizeof(ret));
>
> -#undef GEN_TRANSLATOR_LD
> + if (p) {
> + plugin_insn_append(pc, p, sizeof(ret));
> + return lduw_p(p);
> + }
> + ret = cpu_lduw_code(env, pc);
> + plug = tswap16(ret);
> + plugin_insn_append(pc, &plug, sizeof(ret));
> + return ret;
> +}
> +
> +uint32_t translator_ldl(CPUArchState *env, DisasContextBase *db,
> abi_ptr pc)
> +{
> + uint32_t ret, plug;
> + void *p = translator_access(env, db, pc, sizeof(ret));
> +
> + if (p) {
> + plugin_insn_append(pc, p, sizeof(ret));
> + return ldl_p(p);
> + }
> + ret = cpu_ldl_code(env, pc);
> + plug = tswap32(ret);
> + plugin_insn_append(pc, &plug, sizeof(ret));
> + return ret;
> +}
> +
> +uint64_t translator_ldq(CPUArchState *env, DisasContextBase *db,
> abi_ptr pc)
> +{
> + uint64_t ret, plug;
> + void *p = translator_access(env, db, pc, sizeof(ret));
> +
> + if (p) {
> + plugin_insn_append(pc, p, sizeof(ret));
> + return ldq_p(p);
> + }
> + ret = cpu_ldq_code(env, pc);
> + plug = tswap64(ret);
> + plugin_insn_append(pc, &plug, sizeof(ret));
> + return ret;
> +}
Hi,
I think you need the following fixup here:
--- a/tests/tcg/multiarch/noexec.c.inc
+++ b/tests/tcg/multiarch/noexec.c.inc
@@ -1,8 +1,5 @@
/*
* Common code for arch-specific MMU_INST_FETCH fault testing.
- *
- * Declare struct arch_noexec_test before including this file and
define
- * arch_check_mcontext() after that.
*/
#define _GNU_SOURCE
@@ -13,6 +10,7 @@
#include <stdlib.h>
#include <string.h>
#include <errno.h>
+#include <unistd.h>
#include <sys/mman.h>
#include <sys/ucontext.h>
After the simplifications the comment is no longer true or useful;
unistd.h is needed for getpagesize().
With that:
Acked-by: Ilya Leoshkevich <iii@linux.ibm.com>
Tested-by: Ilya Leoshkevich <iii@linux.ibm.com>
for the series.
Best regards,
Ilya
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v6 18/21] target/s390x: Make translator stop before the end of a page
2022-08-19 3:25 [PATCH v6 00/21] linux-user: Fix siginfo_t contents when jumping to non-readable pages Richard Henderson
` (16 preceding siblings ...)
2022-08-19 3:26 ` [PATCH v6 17/21] accel/tcg: Add fast path for translator_ld* Richard Henderson
@ 2022-08-19 3:26 ` Richard Henderson
2022-08-19 3:26 ` [PATCH v6 19/21] target/i386: " Richard Henderson
` (3 subsequent siblings)
21 siblings, 0 replies; 35+ messages in thread
From: Richard Henderson @ 2022-08-19 3:26 UTC (permalink / raw)
To: qemu-devel; +Cc: laurent, iii, dramforever, alistair.francis, alex.bennee
From: Ilya Leoshkevich <iii@linux.ibm.com>
Right now translator stops right *after* the end of a page, which
breaks reporting of fault locations when the last instruction of a
multi-insn translation block crosses a page boundary.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20220817150506.592862-3-iii@linux.ibm.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/s390x/tcg/translate.c | 15 +++-
tests/tcg/s390x/noexec.c | 106 +++++++++++++++++++++++
tests/tcg/multiarch/noexec.c.inc | 141 +++++++++++++++++++++++++++++++
tests/tcg/s390x/Makefile.target | 1 +
4 files changed, 259 insertions(+), 4 deletions(-)
create mode 100644 tests/tcg/s390x/noexec.c
create mode 100644 tests/tcg/multiarch/noexec.c.inc
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index d4c0b9b3a2..1d2dddab1c 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -6609,6 +6609,14 @@ static void s390x_tr_insn_start(DisasContextBase *dcbase, CPUState *cs)
dc->insn_start = tcg_last_op();
}
+static target_ulong get_next_pc(CPUS390XState *env, DisasContext *s,
+ uint64_t pc)
+{
+ uint64_t insn = ld_code2(env, s, pc);
+
+ return pc + get_ilen((insn >> 8) & 0xff);
+}
+
static void s390x_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
{
CPUS390XState *env = cs->env_ptr;
@@ -6616,10 +6624,9 @@ static void s390x_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
dc->base.is_jmp = translate_one(env, dc);
if (dc->base.is_jmp == DISAS_NEXT) {
- uint64_t page_start;
-
- page_start = dc->base.pc_first & TARGET_PAGE_MASK;
- if (dc->base.pc_next - page_start >= TARGET_PAGE_SIZE || dc->ex_value) {
+ if (!is_same_page(dcbase, dc->base.pc_next) ||
+ !is_same_page(dcbase, get_next_pc(env, dc, dc->base.pc_next)) ||
+ dc->ex_value) {
dc->base.is_jmp = DISAS_TOO_MANY;
}
}
diff --git a/tests/tcg/s390x/noexec.c b/tests/tcg/s390x/noexec.c
new file mode 100644
index 0000000000..15d007d07f
--- /dev/null
+++ b/tests/tcg/s390x/noexec.c
@@ -0,0 +1,106 @@
+#include "../multiarch/noexec.c.inc"
+
+static void *arch_mcontext_pc(const mcontext_t *ctx)
+{
+ return (void *)ctx->psw.addr;
+}
+
+static int arch_mcontext_arg(const mcontext_t *ctx)
+{
+ return ctx->gregs[2];
+}
+
+static void arch_flush(void *p, int len)
+{
+}
+
+extern char noexec_1[];
+extern char noexec_2[];
+extern char noexec_end[];
+
+asm("noexec_1:\n"
+ " lgfi %r2,1\n" /* %r2 is 0 on entry, set 1. */
+ "noexec_2:\n"
+ " lgfi %r2,2\n" /* %r2 is 0/1; set 2. */
+ " br %r14\n" /* return */
+ "noexec_end:");
+
+extern char exrl_1[];
+extern char exrl_2[];
+extern char exrl_end[];
+
+asm("exrl_1:\n"
+ " exrl %r0, exrl_2\n"
+ " br %r14\n"
+ "exrl_2:\n"
+ " lgfi %r2,2\n"
+ "exrl_end:");
+
+int main(void)
+{
+ struct noexec_test noexec_tests[] = {
+ {
+ .name = "fallthrough",
+ .test_code = noexec_1,
+ .test_len = noexec_end - noexec_1,
+ .page_ofs = noexec_1 - noexec_2,
+ .entry_ofs = noexec_1 - noexec_2,
+ .expected_si_ofs = 0,
+ .expected_pc_ofs = 0,
+ .expected_arg = 1,
+ },
+ {
+ .name = "jump",
+ .test_code = noexec_1,
+ .test_len = noexec_end - noexec_1,
+ .page_ofs = noexec_1 - noexec_2,
+ .entry_ofs = 0,
+ .expected_si_ofs = 0,
+ .expected_pc_ofs = 0,
+ .expected_arg = 0,
+ },
+ {
+ .name = "exrl",
+ .test_code = exrl_1,
+ .test_len = exrl_end - exrl_1,
+ .page_ofs = exrl_1 - exrl_2,
+ .entry_ofs = exrl_1 - exrl_2,
+ .expected_si_ofs = 0,
+ .expected_pc_ofs = exrl_1 - exrl_2,
+ .expected_arg = 0,
+ },
+ {
+ .name = "fallthrough [cross]",
+ .test_code = noexec_1,
+ .test_len = noexec_end - noexec_1,
+ .page_ofs = noexec_1 - noexec_2 - 2,
+ .entry_ofs = noexec_1 - noexec_2 - 2,
+ .expected_si_ofs = 0,
+ .expected_pc_ofs = -2,
+ .expected_arg = 1,
+ },
+ {
+ .name = "jump [cross]",
+ .test_code = noexec_1,
+ .test_len = noexec_end - noexec_1,
+ .page_ofs = noexec_1 - noexec_2 - 2,
+ .entry_ofs = -2,
+ .expected_si_ofs = 0,
+ .expected_pc_ofs = -2,
+ .expected_arg = 0,
+ },
+ {
+ .name = "exrl [cross]",
+ .test_code = exrl_1,
+ .test_len = exrl_end - exrl_1,
+ .page_ofs = exrl_1 - exrl_2 - 2,
+ .entry_ofs = exrl_1 - exrl_2 - 2,
+ .expected_si_ofs = 0,
+ .expected_pc_ofs = exrl_1 - exrl_2 - 2,
+ .expected_arg = 0,
+ },
+ };
+
+ return test_noexec(noexec_tests,
+ sizeof(noexec_tests) / sizeof(noexec_tests[0]));
+}
diff --git a/tests/tcg/multiarch/noexec.c.inc b/tests/tcg/multiarch/noexec.c.inc
new file mode 100644
index 0000000000..bed1186f05
--- /dev/null
+++ b/tests/tcg/multiarch/noexec.c.inc
@@ -0,0 +1,141 @@
+/*
+ * Common code for arch-specific MMU_INST_FETCH fault testing.
+ *
+ * Declare struct arch_noexec_test before including this file and define
+ * arch_check_mcontext() after that.
+ */
+
+#define _GNU_SOURCE
+
+#include <assert.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <sys/mman.h>
+#include <sys/ucontext.h>
+
+/* Forward declarations. */
+
+static void *arch_mcontext_pc(const mcontext_t *ctx);
+static int arch_mcontext_arg(const mcontext_t *ctx);
+static void arch_flush(void *p, int len);
+
+/* Testing infrastructure. */
+
+struct noexec_test {
+ const char *name;
+ const char *test_code;
+ int test_len;
+ int page_ofs;
+ int entry_ofs;
+ int expected_si_ofs;
+ int expected_pc_ofs;
+ int expected_arg;
+};
+
+static void *page_base;
+static int page_size;
+static const struct noexec_test *current_noexec_test;
+
+static void handle_err(const char *syscall)
+{
+ printf("[ FAILED ] %s: %s\n", syscall, strerror(errno));
+ exit(EXIT_FAILURE);
+}
+
+static void handle_segv(int sig, siginfo_t *info, void *ucontext)
+{
+ const struct noexec_test *test = current_noexec_test;
+ const mcontext_t *mc = &((ucontext_t *)ucontext)->uc_mcontext;
+ void *expected_si;
+ void *expected_pc;
+ void *pc;
+ int arg;
+
+ if (test == NULL) {
+ printf("[ FAILED ] unexpected SEGV\n");
+ exit(EXIT_FAILURE);
+ }
+ current_noexec_test = NULL;
+
+ expected_si = page_base + test->expected_si_ofs;
+ if (info->si_addr != expected_si) {
+ printf("[ FAILED ] wrong si_addr (%p != %p)\n",
+ info->si_addr, expected_si);
+ exit(EXIT_FAILURE);
+ }
+
+ pc = arch_mcontext_pc(mc);
+ expected_pc = page_base + test->expected_pc_ofs;
+ if (pc != expected_pc) {
+ printf("[ FAILED ] wrong pc (%p != %p)\n", pc, expected_pc);
+ exit(EXIT_FAILURE);
+ }
+
+ arg = arch_mcontext_arg(mc);
+ if (arg != test->expected_arg) {
+ printf("[ FAILED ] wrong arg (%d != %d)\n", arg, test->expected_arg);
+ exit(EXIT_FAILURE);
+ }
+
+ if (mprotect(page_base, page_size,
+ PROT_READ | PROT_WRITE | PROT_EXEC) < 0) {
+ handle_err("mprotect");
+ }
+}
+
+static void test_noexec_1(const struct noexec_test *test)
+{
+ void *start = page_base + test->page_ofs;
+ void (*fn)(int arg) = page_base + test->entry_ofs;
+
+ memcpy(start, test->test_code, test->test_len);
+ arch_flush(start, test->test_len);
+
+ /* Trigger TB creation in order to test invalidation. */
+ fn(0);
+
+ if (mprotect(page_base, page_size, PROT_NONE) < 0) {
+ handle_err("mprotect");
+ }
+
+ /* Trigger SEGV and check that handle_segv() ran. */
+ current_noexec_test = test;
+ fn(0);
+ assert(current_noexec_test == NULL);
+}
+
+static int test_noexec(struct noexec_test *tests, size_t n_tests)
+{
+ struct sigaction act;
+ size_t i;
+
+ memset(&act, 0, sizeof(act));
+ act.sa_sigaction = handle_segv;
+ act.sa_flags = SA_SIGINFO;
+ if (sigaction(SIGSEGV, &act, NULL) < 0) {
+ handle_err("sigaction");
+ }
+
+ page_size = getpagesize();
+ page_base = mmap(NULL, 2 * page_size,
+ PROT_READ | PROT_WRITE | PROT_EXEC,
+ MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
+ if (page_base == MAP_FAILED) {
+ handle_err("mmap");
+ }
+ page_base += page_size;
+
+ for (i = 0; i < n_tests; i++) {
+ struct noexec_test *test = &tests[i];
+
+ printf("[ RUN ] %s\n", test->name);
+ test_noexec_1(test);
+ printf("[ OK ]\n");
+ }
+
+ printf("[ PASSED ]\n");
+ return EXIT_SUCCESS;
+}
diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index 1a7a4a2f59..5e13a41c3f 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -16,6 +16,7 @@ TESTS+=shift
TESTS+=trap
TESTS+=signals-s390x
TESTS+=branch-relative-long
+TESTS+=noexec
Z14_TESTS=vfminmax
vfminmax: LDFLAGS+=-lm
--
2.34.1
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v6 19/21] target/i386: Make translator stop before the end of a page
2022-08-19 3:25 [PATCH v6 00/21] linux-user: Fix siginfo_t contents when jumping to non-readable pages Richard Henderson
` (17 preceding siblings ...)
2022-08-19 3:26 ` [PATCH v6 18/21] target/s390x: Make translator stop before the end of a page Richard Henderson
@ 2022-08-19 3:26 ` Richard Henderson
2022-08-19 3:26 ` [PATCH v6 20/21] target/riscv: Add MAX_INSN_LEN and insn_len Richard Henderson
` (2 subsequent siblings)
21 siblings, 0 replies; 35+ messages in thread
From: Richard Henderson @ 2022-08-19 3:26 UTC (permalink / raw)
To: qemu-devel; +Cc: laurent, iii, dramforever, alistair.francis, alex.bennee
From: Ilya Leoshkevich <iii@linux.ibm.com>
Right now translator stops right *after* the end of a page, which
breaks reporting of fault locations when the last instruction of a
multi-insn translation block crosses a page boundary.
An implementation, like the one arm and s390x have, would require an
i386 length disassembler, which is burdensome to maintain. Another
alternative would be to single-step at the end of a guest page, but
this may come with a performance impact.
Fix by snapshotting disassembly state and restoring it after we figure
out we crossed a page boundary. This includes rolling back cc_op
updates and emitted ops.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1143
Message-Id: <20220817150506.592862-4-iii@linux.ibm.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/i386/tcg/translate.c | 25 ++++++++++-
tests/tcg/x86_64/noexec.c | 75 ++++++++++++++++++++++++++++++++
tests/tcg/x86_64/Makefile.target | 3 +-
3 files changed, 101 insertions(+), 2 deletions(-)
create mode 100644 tests/tcg/x86_64/noexec.c
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 4836c889e0..6481ae5c24 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -130,6 +130,7 @@ typedef struct DisasContext {
TCGv_i64 tmp1_i64;
sigjmp_buf jmpbuf;
+ TCGOp *prev_insn_end;
} DisasContext;
/* The environment in which user-only runs is constrained. */
@@ -2008,6 +2009,12 @@ static uint64_t advance_pc(CPUX86State *env, DisasContext *s, int num_bytes)
{
uint64_t pc = s->pc;
+ /* This is a subsequent insn that crosses a page boundary. */
+ if (s->base.num_insns > 1 &&
+ !is_same_page(&s->base, s->pc + num_bytes - 1)) {
+ siglongjmp(s->jmpbuf, 2);
+ }
+
s->pc += num_bytes;
if (unlikely(s->pc - s->pc_start > X86_MAX_INSN_LENGTH)) {
/* If the instruction's 16th byte is on a different page than the 1st, a
@@ -4556,6 +4563,8 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
int modrm, reg, rm, mod, op, opreg, val;
target_ulong next_eip, tval;
target_ulong pc_start = s->base.pc_next;
+ bool orig_cc_op_dirty = s->cc_op_dirty;
+ CCOp orig_cc_op = s->cc_op;
s->pc_start = s->pc = pc_start;
s->override = -1;
@@ -4568,9 +4577,22 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
s->rip_offset = 0; /* for relative ip address */
s->vex_l = 0;
s->vex_v = 0;
- if (sigsetjmp(s->jmpbuf, 0) != 0) {
+ switch (sigsetjmp(s->jmpbuf, 0)) {
+ case 0:
+ break;
+ case 1:
gen_exception_gpf(s);
return s->pc;
+ case 2:
+ /* Restore state that may affect the next instruction. */
+ s->cc_op_dirty = orig_cc_op_dirty;
+ s->cc_op = orig_cc_op;
+ s->base.num_insns--;
+ tcg_remove_ops_after(s->prev_insn_end);
+ s->base.is_jmp = DISAS_TOO_MANY;
+ return pc_start;
+ default:
+ g_assert_not_reached();
}
prefixes = 0;
@@ -8632,6 +8654,7 @@ static void i386_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu)
{
DisasContext *dc = container_of(dcbase, DisasContext, base);
+ dc->prev_insn_end = tcg_last_op();
tcg_gen_insn_start(dc->base.pc_next, dc->cc_op);
}
diff --git a/tests/tcg/x86_64/noexec.c b/tests/tcg/x86_64/noexec.c
new file mode 100644
index 0000000000..9b124901be
--- /dev/null
+++ b/tests/tcg/x86_64/noexec.c
@@ -0,0 +1,75 @@
+#include "../multiarch/noexec.c.inc"
+
+static void *arch_mcontext_pc(const mcontext_t *ctx)
+{
+ return (void *)ctx->gregs[REG_RIP];
+}
+
+int arch_mcontext_arg(const mcontext_t *ctx)
+{
+ return ctx->gregs[REG_RDI];
+}
+
+static void arch_flush(void *p, int len)
+{
+}
+
+extern char noexec_1[];
+extern char noexec_2[];
+extern char noexec_end[];
+
+asm("noexec_1:\n"
+ " movq $1,%rdi\n" /* %rdi is 0 on entry, set 1. */
+ "noexec_2:\n"
+ " movq $2,%rdi\n" /* %rdi is 0/1; set 2. */
+ " ret\n"
+ "noexec_end:");
+
+int main(void)
+{
+ struct noexec_test noexec_tests[] = {
+ {
+ .name = "fallthrough",
+ .test_code = noexec_1,
+ .test_len = noexec_end - noexec_1,
+ .page_ofs = noexec_1 - noexec_2,
+ .entry_ofs = noexec_1 - noexec_2,
+ .expected_si_ofs = 0,
+ .expected_pc_ofs = 0,
+ .expected_arg = 1,
+ },
+ {
+ .name = "jump",
+ .test_code = noexec_1,
+ .test_len = noexec_end - noexec_1,
+ .page_ofs = noexec_1 - noexec_2,
+ .entry_ofs = 0,
+ .expected_si_ofs = 0,
+ .expected_pc_ofs = 0,
+ .expected_arg = 0,
+ },
+ {
+ .name = "fallthrough [cross]",
+ .test_code = noexec_1,
+ .test_len = noexec_end - noexec_1,
+ .page_ofs = noexec_1 - noexec_2 - 2,
+ .entry_ofs = noexec_1 - noexec_2 - 2,
+ .expected_si_ofs = 0,
+ .expected_pc_ofs = -2,
+ .expected_arg = 1,
+ },
+ {
+ .name = "jump [cross]",
+ .test_code = noexec_1,
+ .test_len = noexec_end - noexec_1,
+ .page_ofs = noexec_1 - noexec_2 - 2,
+ .entry_ofs = -2,
+ .expected_si_ofs = 0,
+ .expected_pc_ofs = -2,
+ .expected_arg = 0,
+ },
+ };
+
+ return test_noexec(noexec_tests,
+ sizeof(noexec_tests) / sizeof(noexec_tests[0]));
+}
diff --git a/tests/tcg/x86_64/Makefile.target b/tests/tcg/x86_64/Makefile.target
index b71a6bcd5e..c0e7e5b005 100644
--- a/tests/tcg/x86_64/Makefile.target
+++ b/tests/tcg/x86_64/Makefile.target
@@ -10,6 +10,7 @@ include $(SRC_PATH)/tests/tcg/i386/Makefile.target
ifeq ($(filter %-linux-user, $(TARGET)),$(TARGET))
X86_64_TESTS += vsyscall
+X86_64_TESTS += noexec
TESTS=$(MULTIARCH_TESTS) $(X86_64_TESTS) test-x86_64
else
TESTS=$(MULTIARCH_TESTS)
@@ -20,5 +21,5 @@ test-x86_64: LDFLAGS+=-lm -lc
test-x86_64: test-i386.c test-i386.h test-i386-shift.h test-i386-muldiv.h
$(CC) $(CFLAGS) $< -o $@ $(LDFLAGS)
-vsyscall: $(SRC_PATH)/tests/tcg/x86_64/vsyscall.c
+%: $(SRC_PATH)/tests/tcg/x86_64/%.c
$(CC) $(CFLAGS) $< -o $@ $(LDFLAGS)
--
2.34.1
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v6 20/21] target/riscv: Add MAX_INSN_LEN and insn_len
2022-08-19 3:25 [PATCH v6 00/21] linux-user: Fix siginfo_t contents when jumping to non-readable pages Richard Henderson
` (18 preceding siblings ...)
2022-08-19 3:26 ` [PATCH v6 19/21] target/i386: " Richard Henderson
@ 2022-08-19 3:26 ` Richard Henderson
2022-08-21 23:44 ` Alistair Francis
2022-08-19 3:26 ` [PATCH v6 21/21] target/riscv: Make translator stop before the end of a page Richard Henderson
2022-08-19 17:14 ` [PATCH v6 00/21] linux-user: Fix siginfo_t contents when jumping to non-readable pages Vivian Wang
21 siblings, 1 reply; 35+ messages in thread
From: Richard Henderson @ 2022-08-19 3:26 UTC (permalink / raw)
To: qemu-devel; +Cc: laurent, iii, dramforever, alistair.francis, alex.bennee
These will be useful in properly ending the TB.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/riscv/translate.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 38666ddc91..a719aa6e63 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -1022,6 +1022,14 @@ static uint32_t opcode_at(DisasContextBase *dcbase, target_ulong pc)
/* Include decoders for factored-out extensions */
#include "decode-XVentanaCondOps.c.inc"
+/* The specification allows for longer insns, but not supported by qemu. */
+#define MAX_INSN_LEN 4
+
+static inline int insn_len(uint16_t first_word)
+{
+ return (first_word & 3) == 3 ? 4 : 2;
+}
+
static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
{
/*
@@ -1037,7 +1045,7 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
};
/* Check for compressed insn */
- if (extract16(opcode, 0, 2) != 3) {
+ if (insn_len(opcode) == 2) {
if (!has_ext(ctx, RVC)) {
gen_exception_illegal(ctx);
} else {
--
2.34.1
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v6 20/21] target/riscv: Add MAX_INSN_LEN and insn_len
2022-08-19 3:26 ` [PATCH v6 20/21] target/riscv: Add MAX_INSN_LEN and insn_len Richard Henderson
@ 2022-08-21 23:44 ` Alistair Francis
0 siblings, 0 replies; 35+ messages in thread
From: Alistair Francis @ 2022-08-21 23:44 UTC (permalink / raw)
To: Richard Henderson
Cc: qemu-devel@nongnu.org Developers, Laurent Vivier, iii,
dramforever, Alistair Francis, Alex Bennée
On Fri, Aug 19, 2022 at 1:42 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> These will be useful in properly ending the TB.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/translate.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 38666ddc91..a719aa6e63 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -1022,6 +1022,14 @@ static uint32_t opcode_at(DisasContextBase *dcbase, target_ulong pc)
> /* Include decoders for factored-out extensions */
> #include "decode-XVentanaCondOps.c.inc"
>
> +/* The specification allows for longer insns, but not supported by qemu. */
> +#define MAX_INSN_LEN 4
> +
> +static inline int insn_len(uint16_t first_word)
> +{
> + return (first_word & 3) == 3 ? 4 : 2;
> +}
> +
> static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
> {
> /*
> @@ -1037,7 +1045,7 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
> };
>
> /* Check for compressed insn */
> - if (extract16(opcode, 0, 2) != 3) {
> + if (insn_len(opcode) == 2) {
> if (!has_ext(ctx, RVC)) {
> gen_exception_illegal(ctx);
> } else {
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v6 21/21] target/riscv: Make translator stop before the end of a page
2022-08-19 3:25 [PATCH v6 00/21] linux-user: Fix siginfo_t contents when jumping to non-readable pages Richard Henderson
` (19 preceding siblings ...)
2022-08-19 3:26 ` [PATCH v6 20/21] target/riscv: Add MAX_INSN_LEN and insn_len Richard Henderson
@ 2022-08-19 3:26 ` Richard Henderson
2022-08-21 23:47 ` Alistair Francis
2022-08-19 17:14 ` [PATCH v6 00/21] linux-user: Fix siginfo_t contents when jumping to non-readable pages Vivian Wang
21 siblings, 1 reply; 35+ messages in thread
From: Richard Henderson @ 2022-08-19 3:26 UTC (permalink / raw)
To: qemu-devel; +Cc: laurent, iii, dramforever, alistair.francis, alex.bennee
Right now the translator stops right *after* the end of a page, which
breaks reporting of fault locations when the last instruction of a
multi-insn translation block crosses a page boundary.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1155
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/riscv/translate.c | 17 +++++--
tests/tcg/riscv64/noexec.c | 79 +++++++++++++++++++++++++++++++
tests/tcg/riscv64/Makefile.target | 1 +
3 files changed, 93 insertions(+), 4 deletions(-)
create mode 100644 tests/tcg/riscv64/noexec.c
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index a719aa6e63..f8af6daa70 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -1154,12 +1154,21 @@ static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
}
ctx->nftemp = 0;
+ /* Only the first insn within a TB is allowed to cross a page boundary. */
if (ctx->base.is_jmp == DISAS_NEXT) {
- target_ulong page_start;
-
- page_start = ctx->base.pc_first & TARGET_PAGE_MASK;
- if (ctx->base.pc_next - page_start >= TARGET_PAGE_SIZE) {
+ if (!is_same_page(&ctx->base, ctx->base.pc_next)) {
ctx->base.is_jmp = DISAS_TOO_MANY;
+ } else {
+ 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);
+ int len = insn_len(next_insn);
+
+ if (!is_same_page(&ctx->base, ctx->base.pc_next + len)) {
+ ctx->base.is_jmp = DISAS_TOO_MANY;
+ }
+ }
}
}
}
diff --git a/tests/tcg/riscv64/noexec.c b/tests/tcg/riscv64/noexec.c
new file mode 100644
index 0000000000..86f64b28db
--- /dev/null
+++ b/tests/tcg/riscv64/noexec.c
@@ -0,0 +1,79 @@
+#include "../multiarch/noexec.c.inc"
+
+static void *arch_mcontext_pc(const mcontext_t *ctx)
+{
+ return (void *)ctx->__gregs[REG_PC];
+}
+
+static int arch_mcontext_arg(const mcontext_t *ctx)
+{
+ return ctx->__gregs[REG_A0];
+}
+
+static void arch_flush(void *p, int len)
+{
+ __builtin___clear_cache(p, p + len);
+}
+
+extern char noexec_1[];
+extern char noexec_2[];
+extern char noexec_end[];
+
+asm(".option push\n"
+ ".option norvc\n"
+ "noexec_1:\n"
+ " li a0,1\n" /* a0 is 0 on entry, set 1. */
+ "noexec_2:\n"
+ " li a0,2\n" /* a0 is 0/1; set 2. */
+ " ret\n"
+ "noexec_end:\n"
+ ".option pop");
+
+int main(void)
+{
+ struct noexec_test noexec_tests[] = {
+ {
+ .name = "fallthrough",
+ .test_code = noexec_1,
+ .test_len = noexec_end - noexec_1,
+ .page_ofs = noexec_1 - noexec_2,
+ .entry_ofs = noexec_1 - noexec_2,
+ .expected_si_ofs = 0,
+ .expected_pc_ofs = 0,
+ .expected_arg = 1,
+ },
+ {
+ .name = "jump",
+ .test_code = noexec_1,
+ .test_len = noexec_end - noexec_1,
+ .page_ofs = noexec_1 - noexec_2,
+ .entry_ofs = 0,
+ .expected_si_ofs = 0,
+ .expected_pc_ofs = 0,
+ .expected_arg = 0,
+ },
+ {
+ .name = "fallthrough [cross]",
+ .test_code = noexec_1,
+ .test_len = noexec_end - noexec_1,
+ .page_ofs = noexec_1 - noexec_2 - 2,
+ .entry_ofs = noexec_1 - noexec_2 - 2,
+ .expected_si_ofs = 0,
+ .expected_pc_ofs = -2,
+ .expected_arg = 1,
+ },
+ {
+ .name = "jump [cross]",
+ .test_code = noexec_1,
+ .test_len = noexec_end - noexec_1,
+ .page_ofs = noexec_1 - noexec_2 - 2,
+ .entry_ofs = -2,
+ .expected_si_ofs = 0,
+ .expected_pc_ofs = -2,
+ .expected_arg = 0,
+ },
+ };
+
+ return test_noexec(noexec_tests,
+ sizeof(noexec_tests) / sizeof(noexec_tests[0]));
+}
diff --git a/tests/tcg/riscv64/Makefile.target b/tests/tcg/riscv64/Makefile.target
index d41bf6d60d..b5b89dfb0e 100644
--- a/tests/tcg/riscv64/Makefile.target
+++ b/tests/tcg/riscv64/Makefile.target
@@ -3,3 +3,4 @@
VPATH += $(SRC_PATH)/tests/tcg/riscv64
TESTS += test-div
+TESTS += noexec
--
2.34.1
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v6 21/21] target/riscv: Make translator stop before the end of a page
2022-08-19 3:26 ` [PATCH v6 21/21] target/riscv: Make translator stop before the end of a page Richard Henderson
@ 2022-08-21 23:47 ` Alistair Francis
0 siblings, 0 replies; 35+ messages in thread
From: Alistair Francis @ 2022-08-21 23:47 UTC (permalink / raw)
To: Richard Henderson
Cc: qemu-devel@nongnu.org Developers, Laurent Vivier, iii,
dramforever, Alistair Francis, Alex Bennée
On Fri, Aug 19, 2022 at 1:39 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Right now the translator stops right *after* the end of a page, which
> breaks reporting of fault locations when the last instruction of a
> multi-insn translation block crosses a page boundary.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1155
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/translate.c | 17 +++++--
> tests/tcg/riscv64/noexec.c | 79 +++++++++++++++++++++++++++++++
> tests/tcg/riscv64/Makefile.target | 1 +
> 3 files changed, 93 insertions(+), 4 deletions(-)
> create mode 100644 tests/tcg/riscv64/noexec.c
>
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index a719aa6e63..f8af6daa70 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -1154,12 +1154,21 @@ static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
> }
> ctx->nftemp = 0;
>
> + /* Only the first insn within a TB is allowed to cross a page boundary. */
> if (ctx->base.is_jmp == DISAS_NEXT) {
> - target_ulong page_start;
> -
> - page_start = ctx->base.pc_first & TARGET_PAGE_MASK;
> - if (ctx->base.pc_next - page_start >= TARGET_PAGE_SIZE) {
> + if (!is_same_page(&ctx->base, ctx->base.pc_next)) {
> ctx->base.is_jmp = DISAS_TOO_MANY;
> + } else {
> + 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);
> + int len = insn_len(next_insn);
> +
> + if (!is_same_page(&ctx->base, ctx->base.pc_next + len)) {
> + ctx->base.is_jmp = DISAS_TOO_MANY;
> + }
> + }
> }
> }
> }
> diff --git a/tests/tcg/riscv64/noexec.c b/tests/tcg/riscv64/noexec.c
> new file mode 100644
> index 0000000000..86f64b28db
> --- /dev/null
> +++ b/tests/tcg/riscv64/noexec.c
> @@ -0,0 +1,79 @@
> +#include "../multiarch/noexec.c.inc"
> +
> +static void *arch_mcontext_pc(const mcontext_t *ctx)
> +{
> + return (void *)ctx->__gregs[REG_PC];
> +}
> +
> +static int arch_mcontext_arg(const mcontext_t *ctx)
> +{
> + return ctx->__gregs[REG_A0];
> +}
> +
> +static void arch_flush(void *p, int len)
> +{
> + __builtin___clear_cache(p, p + len);
> +}
> +
> +extern char noexec_1[];
> +extern char noexec_2[];
> +extern char noexec_end[];
> +
> +asm(".option push\n"
> + ".option norvc\n"
> + "noexec_1:\n"
> + " li a0,1\n" /* a0 is 0 on entry, set 1. */
> + "noexec_2:\n"
> + " li a0,2\n" /* a0 is 0/1; set 2. */
> + " ret\n"
> + "noexec_end:\n"
> + ".option pop");
> +
> +int main(void)
> +{
> + struct noexec_test noexec_tests[] = {
> + {
> + .name = "fallthrough",
> + .test_code = noexec_1,
> + .test_len = noexec_end - noexec_1,
> + .page_ofs = noexec_1 - noexec_2,
> + .entry_ofs = noexec_1 - noexec_2,
> + .expected_si_ofs = 0,
> + .expected_pc_ofs = 0,
> + .expected_arg = 1,
> + },
> + {
> + .name = "jump",
> + .test_code = noexec_1,
> + .test_len = noexec_end - noexec_1,
> + .page_ofs = noexec_1 - noexec_2,
> + .entry_ofs = 0,
> + .expected_si_ofs = 0,
> + .expected_pc_ofs = 0,
> + .expected_arg = 0,
> + },
> + {
> + .name = "fallthrough [cross]",
> + .test_code = noexec_1,
> + .test_len = noexec_end - noexec_1,
> + .page_ofs = noexec_1 - noexec_2 - 2,
> + .entry_ofs = noexec_1 - noexec_2 - 2,
> + .expected_si_ofs = 0,
> + .expected_pc_ofs = -2,
> + .expected_arg = 1,
> + },
> + {
> + .name = "jump [cross]",
> + .test_code = noexec_1,
> + .test_len = noexec_end - noexec_1,
> + .page_ofs = noexec_1 - noexec_2 - 2,
> + .entry_ofs = -2,
> + .expected_si_ofs = 0,
> + .expected_pc_ofs = -2,
> + .expected_arg = 0,
> + },
> + };
> +
> + return test_noexec(noexec_tests,
> + sizeof(noexec_tests) / sizeof(noexec_tests[0]));
> +}
> diff --git a/tests/tcg/riscv64/Makefile.target b/tests/tcg/riscv64/Makefile.target
> index d41bf6d60d..b5b89dfb0e 100644
> --- a/tests/tcg/riscv64/Makefile.target
> +++ b/tests/tcg/riscv64/Makefile.target
> @@ -3,3 +3,4 @@
>
> VPATH += $(SRC_PATH)/tests/tcg/riscv64
> TESTS += test-div
> +TESTS += noexec
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v6 00/21] linux-user: Fix siginfo_t contents when jumping to non-readable pages
2022-08-19 3:25 [PATCH v6 00/21] linux-user: Fix siginfo_t contents when jumping to non-readable pages Richard Henderson
` (20 preceding siblings ...)
2022-08-19 3:26 ` [PATCH v6 21/21] target/riscv: Make translator stop before the end of a page Richard Henderson
@ 2022-08-19 17:14 ` Vivian Wang
21 siblings, 0 replies; 35+ messages in thread
From: Vivian Wang @ 2022-08-19 17:14 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: laurent, iii, alistair.francis, alex.bennee
On 8/19/22 11:25, Richard Henderson wrote:
> Hi Ilya,
>
> After adding support for riscv (similar to s390x, in that we can
> find the total insn length from the first couple of bits, so, easy),
> I find that the test case doesn't work without all of the other
> changes for PROT_EXEC, including the translator_ld changes.
>
> Other changes from your v5:
> - mprotect invalidates tbs. The test case is riscv, with a
> 4-byte insn at offset 0xffe, which was chained to from the
> insn at offset 0xffa. The fact that the 0xffe tb was not
> invalidated meant that we chained to it and re-executed
> without revalidating page protections.
>
> - rewrote the test framework to be agnostic of page size, which
> reduces some of the repetition. I ran into trouble with the
> riscv linker, which relaxed the segment such that .align+.org
> wasn't actually honored. This new form doesn't require the
> test bytes to be aligned in the binary.
>
>
> r~
I've confirmed that this fixes #1155
Tested-by: Vivian Wang <dramforever@live.com>
> Ilya Leoshkevich (4):
> linux-user: Clear translations and tb_jmp_cache on mprotect()
> accel/tcg: Introduce is_same_page()
> target/s390x: Make translator stop before the end of a page
> target/i386: Make translator stop before the end of a page
>
> Richard Henderson (17):
> linux-user/arm: Mark the commpage executable
> linux-user/hppa: Allocate page zero as a commpage
> linux-user/x86_64: Allocate vsyscall page as a commpage
> linux-user: Honor PT_GNU_STACK
> tests/tcg/i386: Move smc_code2 to an executable section
> accel/tcg: Properly implement get_page_addr_code for user-only
> accel/tcg: Unlock mmap_lock after longjmp
> accel/tcg: Make tb_htable_lookup static
> accel/tcg: Move qemu_ram_addr_from_host_nofail to physmem.c
> accel/tcg: Use probe_access_internal for softmmu
> get_page_addr_code_hostp
> accel/tcg: Add nofault parameter to get_page_addr_code_hostp
> accel/tcg: Raise PROT_EXEC exception early
> accel/tcg: Remove translator_ldsw
> accel/tcg: Add pc and host_pc params to gen_intermediate_code
> accel/tcg: Add fast path for translator_ld*
> target/riscv: Add MAX_INSN_LEN and insn_len
> target/riscv: Make translator stop before the end of a page
>
> include/elf.h | 1 +
> include/exec/cpu-common.h | 1 +
> include/exec/exec-all.h | 87 ++++++------------
> include/exec/translator.h | 96 +++++++++++++-------
> linux-user/arm/target_cpu.h | 4 +-
> linux-user/qemu.h | 1 +
> accel/tcg/cpu-exec.c | 134 ++++++++++++++--------------
> accel/tcg/cputlb.c | 93 ++++++--------------
> accel/tcg/plugin-gen.c | 4 +-
> accel/tcg/translate-all.c | 29 +++---
> accel/tcg/translator.c | 136 +++++++++++++++++++++-------
> accel/tcg/user-exec.c | 18 +++-
> linux-user/elfload.c | 82 +++++++++++++++--
> linux-user/mmap.c | 8 ++
> softmmu/physmem.c | 12 +++
> target/alpha/translate.c | 5 +-
> target/arm/translate.c | 5 +-
> target/avr/translate.c | 5 +-
> target/cris/translate.c | 5 +-
> target/hexagon/translate.c | 6 +-
> target/hppa/translate.c | 5 +-
> target/i386/tcg/translate.c | 32 ++++++-
> target/loongarch/translate.c | 6 +-
> target/m68k/translate.c | 5 +-
> target/microblaze/translate.c | 5 +-
> target/mips/tcg/translate.c | 5 +-
> target/nios2/translate.c | 5 +-
> target/openrisc/translate.c | 6 +-
> target/ppc/translate.c | 5 +-
> target/riscv/translate.c | 32 +++++--
> target/rx/translate.c | 5 +-
> target/s390x/tcg/translate.c | 20 +++--
> target/sh4/translate.c | 5 +-
> target/sparc/translate.c | 5 +-
> target/tricore/translate.c | 6 +-
> target/xtensa/translate.c | 6 +-
> tests/tcg/i386/test-i386.c | 2 +-
> tests/tcg/riscv64/noexec.c | 79 +++++++++++++++++
> tests/tcg/s390x/noexec.c | 106 ++++++++++++++++++++++
> tests/tcg/x86_64/noexec.c | 75 ++++++++++++++++
> tests/tcg/multiarch/noexec.c.inc | 141 ++++++++++++++++++++++++++++++
> tests/tcg/riscv64/Makefile.target | 1 +
> tests/tcg/s390x/Makefile.target | 1 +
> tests/tcg/x86_64/Makefile.target | 3 +-
> 44 files changed, 951 insertions(+), 342 deletions(-)
> create mode 100644 tests/tcg/riscv64/noexec.c
> create mode 100644 tests/tcg/s390x/noexec.c
> create mode 100644 tests/tcg/x86_64/noexec.c
> create mode 100644 tests/tcg/multiarch/noexec.c.inc
>
^ permalink raw reply [flat|nested] 35+ messages in thread