* [PATCH v2 0/6] tcg: Fix x86 TARGET_TB_PCREL (#1269)
@ 2022-10-27 10:02 Richard Henderson
2022-10-27 10:02 ` [PATCH v2 1/6] accel/tcg: Introduce cpu_unwind_state_data Richard Henderson
` (6 more replies)
0 siblings, 7 replies; 17+ messages in thread
From: Richard Henderson @ 2022-10-27 10:02 UTC (permalink / raw)
To: qemu-devel; +Cc: cfontana
As per #1269, this affects NetBSD installer boot.
The problem is that one of the x86 acpi callbacks modifies
env->eip during an mmio store, which means that the tracking
that translate.c does is thrown out of whack.
Introduce a method to extract unwind data without the
writeback to env. This isn't a perfect abstraction, but I
couldn't think of anything better. There's a couple of lines
of code duplication, but probably less than any abstration
that we might put on top
Changes for v2:
* Rebase on master, 23 patches merged.
* Comments adjusted per review (claudio)
r~
Richard Henderson (6):
accel/tcg: Introduce cpu_unwind_state_data
target/i386: Use cpu_unwind_state_data for tpr access
target/openrisc: Always exit after mtspr npc
target/openrisc: Use cpu_unwind_state_data for mfspr
accel/tcg: Remove will_exit argument from cpu_restore_state
accel/tcg: Remove reset_icount argument from cpu_restore_state_from_tb
accel/tcg/internal.h | 4 +-
include/exec/exec-all.h | 24 +++++---
accel/tcg/cpu-exec-common.c | 2 +-
accel/tcg/tb-maint.c | 4 +-
accel/tcg/translate-all.c | 91 +++++++++++++++++------------
target/alpha/helper.c | 2 +-
target/alpha/mem_helper.c | 2 +-
target/arm/op_helper.c | 2 +-
target/arm/tlb_helper.c | 8 +--
target/cris/helper.c | 2 +-
target/i386/helper.c | 21 ++++++-
target/i386/tcg/sysemu/svm_helper.c | 2 +-
target/m68k/op_helper.c | 4 +-
target/microblaze/helper.c | 2 +-
target/nios2/op_helper.c | 2 +-
target/openrisc/sys_helper.c | 17 ++++--
target/ppc/excp_helper.c | 2 +-
target/s390x/tcg/excp_helper.c | 2 +-
target/tricore/op_helper.c | 2 +-
target/xtensa/helper.c | 6 +-
20 files changed, 125 insertions(+), 76 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH v2 1/6] accel/tcg: Introduce cpu_unwind_state_data 2022-10-27 10:02 [PATCH v2 0/6] tcg: Fix x86 TARGET_TB_PCREL (#1269) Richard Henderson @ 2022-10-27 10:02 ` Richard Henderson 2022-10-27 10:40 ` Claudio Fontana 2022-10-27 10:02 ` [PATCH v2 2/6] target/i386: Use cpu_unwind_state_data for tpr access Richard Henderson ` (5 subsequent siblings) 6 siblings, 1 reply; 17+ messages in thread From: Richard Henderson @ 2022-10-27 10:02 UTC (permalink / raw) To: qemu-devel; +Cc: cfontana Add a way to examine the unwind data without actually restoring the data back into env. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- accel/tcg/internal.h | 4 +-- include/exec/exec-all.h | 21 ++++++++--- accel/tcg/translate-all.c | 74 ++++++++++++++++++++++++++------------- 3 files changed, 68 insertions(+), 31 deletions(-) diff --git a/accel/tcg/internal.h b/accel/tcg/internal.h index 1227bb69bd..9c06b320b7 100644 --- a/accel/tcg/internal.h +++ b/accel/tcg/internal.h @@ -106,8 +106,8 @@ void tb_reset_jump(TranslationBlock *tb, int n); TranslationBlock *tb_link_page(TranslationBlock *tb, tb_page_addr_t phys_pc, tb_page_addr_t phys_page2); bool tb_invalidate_phys_page_unwind(tb_page_addr_t addr, uintptr_t pc); -int cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb, - uintptr_t searched_pc, bool reset_icount); +void cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb, + uintptr_t host_pc, bool reset_icount); /* Return the current PC from CPU, which may be cached in TB. */ static inline target_ulong log_pc(CPUState *cpu, const TranslationBlock *tb) diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index e948992a80..7d851f5907 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -39,20 +39,33 @@ typedef ram_addr_t tb_page_addr_t; #define TB_PAGE_ADDR_FMT RAM_ADDR_FMT #endif +/** + * cpu_unwind_state_data: + * @cpu: the cpu context + * @host_pc: the host pc within the translation + * @data: output data + * + * Attempt to load the the unwind state for a host pc occurring in + * translated code. If @host_pc is not in translated code, the + * function returns false; otherwise @data is loaded. + * This is the same unwind info as given to restore_state_to_opc. + */ +bool cpu_unwind_state_data(CPUState *cpu, uintptr_t host_pc, uint64_t *data); + /** * cpu_restore_state: - * @cpu: the vCPU state is to be restore to - * @searched_pc: the host PC the fault occurred at + * @cpu: the cpu context + * @host_pc: the host pc within the translation * @will_exit: true if the TB executed will be interrupted after some cpu adjustments. Required for maintaining the correct icount valus * @return: true if state was restored, false otherwise * * Attempt to restore the state for a fault occurring in translated - * code. If the searched_pc is not in translated code no state is + * code. If @host_pc is not in translated code no state is * restored and the function returns false. */ -bool cpu_restore_state(CPUState *cpu, uintptr_t searched_pc, bool will_exit); +bool cpu_restore_state(CPUState *cpu, uintptr_t host_pc, bool will_exit); G_NORETURN void cpu_loop_exit_noexc(CPUState *cpu); G_NORETURN void cpu_loop_exit(CPUState *cpu); diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c index f185356a36..319becb698 100644 --- a/accel/tcg/translate-all.c +++ b/accel/tcg/translate-all.c @@ -247,52 +247,66 @@ static int encode_search(TranslationBlock *tb, uint8_t *block) return p - block; } -/* The cpu state corresponding to 'searched_pc' is restored. - * When reset_icount is true, current TB will be interrupted and - * icount should be recalculated. - */ -int cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb, - uintptr_t searched_pc, bool reset_icount) +static int cpu_unwind_data_from_tb(TranslationBlock *tb, uintptr_t host_pc, + uint64_t *data) { - uint64_t data[TARGET_INSN_START_WORDS]; - uintptr_t host_pc = (uintptr_t)tb->tc.ptr; + uintptr_t iter_pc = (uintptr_t)tb->tc.ptr; const uint8_t *p = tb->tc.ptr + tb->tc.size; int i, j, num_insns = tb->icount; -#ifdef CONFIG_PROFILER - TCGProfile *prof = &tcg_ctx->prof; - int64_t ti = profile_getclock(); -#endif - searched_pc -= GETPC_ADJ; + host_pc -= GETPC_ADJ; - if (searched_pc < host_pc) { + if (host_pc < iter_pc) { return -1; } - memset(data, 0, sizeof(data)); + memset(data, 0, sizeof(uint64_t) * TARGET_INSN_START_WORDS); if (!TARGET_TB_PCREL) { data[0] = tb_pc(tb); } - /* Reconstruct the stored insn data while looking for the point at - which the end of the insn exceeds the searched_pc. */ + /* + * Reconstruct the stored insn data while looking for the point + * at which the end of the insn exceeds host_pc. + */ for (i = 0; i < num_insns; ++i) { for (j = 0; j < TARGET_INSN_START_WORDS; ++j) { data[j] += decode_sleb128(&p); } - host_pc += decode_sleb128(&p); - if (host_pc > searched_pc) { - goto found; + iter_pc += decode_sleb128(&p); + if (iter_pc > host_pc) { + return num_insns - i; } } return -1; +} + +/* + * The cpu state corresponding to 'host_pc' is restored. + * When reset_icount is true, current TB will be interrupted and + * icount should be recalculated. + */ +void cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb, + uintptr_t host_pc, bool reset_icount) +{ + uint64_t data[TARGET_INSN_START_WORDS]; +#ifdef CONFIG_PROFILER + TCGProfile *prof = &tcg_ctx->prof; + int64_t ti = profile_getclock(); +#endif + int insns_left = cpu_unwind_data_from_tb(tb, host_pc, data); + + if (insns_left < 0) { + return; + } - found: if (reset_icount && (tb_cflags(tb) & CF_USE_ICOUNT)) { assert(icount_enabled()); - /* Reset the cycle counter to the start of the block - and shift if to the number of actually executed instructions */ - cpu_neg(cpu)->icount_decr.u16.low += num_insns - i; + /* + * Reset the cycle counter to the start of the block and + * shift if to the number of actually executed instructions. + */ + cpu_neg(cpu)->icount_decr.u16.low += insns_left; } cpu->cc->tcg_ops->restore_state_to_opc(cpu, tb, data); @@ -302,7 +316,6 @@ int cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb, prof->restore_time + profile_getclock() - ti); qatomic_set(&prof->restore_count, prof->restore_count + 1); #endif - return 0; } bool cpu_restore_state(CPUState *cpu, uintptr_t host_pc, bool will_exit) @@ -335,6 +348,17 @@ bool cpu_restore_state(CPUState *cpu, uintptr_t host_pc, bool will_exit) return false; } +bool cpu_unwind_state_data(CPUState *cpu, uintptr_t host_pc, uint64_t *data) +{ + if (in_code_gen_buffer((const void *)(host_pc - tcg_splitwx_diff))) { + TranslationBlock *tb = tcg_tb_lookup(host_pc); + if (tb) { + return cpu_unwind_data_from_tb(tb, host_pc, data) >= 0; + } + } + return false; +} + void page_init(void) { page_size_init(); -- 2.34.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/6] accel/tcg: Introduce cpu_unwind_state_data 2022-10-27 10:02 ` [PATCH v2 1/6] accel/tcg: Introduce cpu_unwind_state_data Richard Henderson @ 2022-10-27 10:40 ` Claudio Fontana 2022-10-27 11:30 ` Richard Henderson 0 siblings, 1 reply; 17+ messages in thread From: Claudio Fontana @ 2022-10-27 10:40 UTC (permalink / raw) To: Richard Henderson, qemu-devel On 10/27/22 12:02, Richard Henderson wrote: > Add a way to examine the unwind data without actually > restoring the data back into env. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > accel/tcg/internal.h | 4 +-- > include/exec/exec-all.h | 21 ++++++++--- > accel/tcg/translate-all.c | 74 ++++++++++++++++++++++++++------------- > 3 files changed, 68 insertions(+), 31 deletions(-) > > diff --git a/accel/tcg/internal.h b/accel/tcg/internal.h > index 1227bb69bd..9c06b320b7 100644 > --- a/accel/tcg/internal.h > +++ b/accel/tcg/internal.h > @@ -106,8 +106,8 @@ void tb_reset_jump(TranslationBlock *tb, int n); > TranslationBlock *tb_link_page(TranslationBlock *tb, tb_page_addr_t phys_pc, > tb_page_addr_t phys_page2); > bool tb_invalidate_phys_page_unwind(tb_page_addr_t addr, uintptr_t pc); > -int cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb, > - uintptr_t searched_pc, bool reset_icount); > +void cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb, > + uintptr_t host_pc, bool reset_icount); > > /* Return the current PC from CPU, which may be cached in TB. */ > static inline target_ulong log_pc(CPUState *cpu, const TranslationBlock *tb) > diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h > index e948992a80..7d851f5907 100644 > --- a/include/exec/exec-all.h > +++ b/include/exec/exec-all.h > @@ -39,20 +39,33 @@ typedef ram_addr_t tb_page_addr_t; > #define TB_PAGE_ADDR_FMT RAM_ADDR_FMT > #endif > > +/** > + * cpu_unwind_state_data: > + * @cpu: the cpu context > + * @host_pc: the host pc within the translation > + * @data: output data > + * > + * Attempt to load the the unwind state for a host pc occurring in > + * translated code. If @host_pc is not in translated code, the > + * function returns false; otherwise @data is loaded. > + * This is the same unwind info as given to restore_state_to_opc. > + */ > +bool cpu_unwind_state_data(CPUState *cpu, uintptr_t host_pc, uint64_t *data); > + > /** > * cpu_restore_state: > - * @cpu: the vCPU state is to be restore to > - * @searched_pc: the host PC the fault occurred at > + * @cpu: the cpu context > + * @host_pc: the host pc within the translation > * @will_exit: true if the TB executed will be interrupted after some > cpu adjustments. Required for maintaining the correct > icount valus > * @return: true if state was restored, false otherwise > * > * Attempt to restore the state for a fault occurring in translated > - * code. If the searched_pc is not in translated code no state is > + * code. If @host_pc is not in translated code no state is > * restored and the function returns false. > */ > -bool cpu_restore_state(CPUState *cpu, uintptr_t searched_pc, bool will_exit); > +bool cpu_restore_state(CPUState *cpu, uintptr_t host_pc, bool will_exit); > > G_NORETURN void cpu_loop_exit_noexc(CPUState *cpu); > G_NORETURN void cpu_loop_exit(CPUState *cpu); > diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c > index f185356a36..319becb698 100644 > --- a/accel/tcg/translate-all.c > +++ b/accel/tcg/translate-all.c > @@ -247,52 +247,66 @@ static int encode_search(TranslationBlock *tb, uint8_t *block) > return p - block; > } > > -/* The cpu state corresponding to 'searched_pc' is restored. > - * When reset_icount is true, current TB will be interrupted and > - * icount should be recalculated. > - */ > -int cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb, > - uintptr_t searched_pc, bool reset_icount) Maybe add a small comment about what the return value of this static function means? It can be indirectly inferred from its point of use: + int insns_left = cpu_unwind_data_from_tb(tb, host_pc, data); But I find having the information about the meaning of a function and return value useful to be available there. IIUC for external functions the standard way is to document in the header files, but for the static functions I would think we can do it here. With that Reviewed-by: Claudio Fontana <cfontana@suse.de> > +static int cpu_unwind_data_from_tb(TranslationBlock *tb, uintptr_t host_pc, > + uint64_t *data) > { > - uint64_t data[TARGET_INSN_START_WORDS]; > - uintptr_t host_pc = (uintptr_t)tb->tc.ptr; > + uintptr_t iter_pc = (uintptr_t)tb->tc.ptr; > const uint8_t *p = tb->tc.ptr + tb->tc.size; > int i, j, num_insns = tb->icount; > -#ifdef CONFIG_PROFILER > - TCGProfile *prof = &tcg_ctx->prof; > - int64_t ti = profile_getclock(); > -#endif > > - searched_pc -= GETPC_ADJ; > + host_pc -= GETPC_ADJ; > > - if (searched_pc < host_pc) { > + if (host_pc < iter_pc) { > return -1; > } > > - memset(data, 0, sizeof(data)); > + memset(data, 0, sizeof(uint64_t) * TARGET_INSN_START_WORDS); > if (!TARGET_TB_PCREL) { > data[0] = tb_pc(tb); > } > > - /* Reconstruct the stored insn data while looking for the point at > - which the end of the insn exceeds the searched_pc. */ > + /* > + * Reconstruct the stored insn data while looking for the point > + * at which the end of the insn exceeds host_pc. > + */ > for (i = 0; i < num_insns; ++i) { > for (j = 0; j < TARGET_INSN_START_WORDS; ++j) { > data[j] += decode_sleb128(&p); > } > - host_pc += decode_sleb128(&p); > - if (host_pc > searched_pc) { > - goto found; > + iter_pc += decode_sleb128(&p); > + if (iter_pc > host_pc) { > + return num_insns - i; > } > } > return -1; > +} > + > +/* > + * The cpu state corresponding to 'host_pc' is restored. > + * When reset_icount is true, current TB will be interrupted and > + * icount should be recalculated. > + */ > +void cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb, > + uintptr_t host_pc, bool reset_icount) > +{ > + uint64_t data[TARGET_INSN_START_WORDS]; > +#ifdef CONFIG_PROFILER > + TCGProfile *prof = &tcg_ctx->prof; > + int64_t ti = profile_getclock(); > +#endif > + int insns_left = cpu_unwind_data_from_tb(tb, host_pc, data); > + > + if (insns_left < 0) { > + return; > + } > > - found: > if (reset_icount && (tb_cflags(tb) & CF_USE_ICOUNT)) { > assert(icount_enabled()); > - /* Reset the cycle counter to the start of the block > - and shift if to the number of actually executed instructions */ > - cpu_neg(cpu)->icount_decr.u16.low += num_insns - i; > + /* > + * Reset the cycle counter to the start of the block and > + * shift if to the number of actually executed instructions. > + */ > + cpu_neg(cpu)->icount_decr.u16.low += insns_left; > } > > cpu->cc->tcg_ops->restore_state_to_opc(cpu, tb, data); > @@ -302,7 +316,6 @@ int cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb, > prof->restore_time + profile_getclock() - ti); > qatomic_set(&prof->restore_count, prof->restore_count + 1); > #endif > - return 0; > } > > bool cpu_restore_state(CPUState *cpu, uintptr_t host_pc, bool will_exit) > @@ -335,6 +348,17 @@ bool cpu_restore_state(CPUState *cpu, uintptr_t host_pc, bool will_exit) > return false; > } > > +bool cpu_unwind_state_data(CPUState *cpu, uintptr_t host_pc, uint64_t *data) > +{ > + if (in_code_gen_buffer((const void *)(host_pc - tcg_splitwx_diff))) { > + TranslationBlock *tb = tcg_tb_lookup(host_pc); > + if (tb) { > + return cpu_unwind_data_from_tb(tb, host_pc, data) >= 0; > + } > + } > + return false; > +} > + > void page_init(void) > { > page_size_init(); ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/6] accel/tcg: Introduce cpu_unwind_state_data 2022-10-27 10:40 ` Claudio Fontana @ 2022-10-27 11:30 ` Richard Henderson 2022-10-27 11:39 ` Claudio Fontana 0 siblings, 1 reply; 17+ messages in thread From: Richard Henderson @ 2022-10-27 11:30 UTC (permalink / raw) To: Claudio Fontana, qemu-devel On 10/27/22 20:40, Claudio Fontana wrote: > On 10/27/22 12:02, Richard Henderson wrote: >> Add a way to examine the unwind data without actually >> restoring the data back into env. >> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >> --- >> accel/tcg/internal.h | 4 +-- >> include/exec/exec-all.h | 21 ++++++++--- >> accel/tcg/translate-all.c | 74 ++++++++++++++++++++++++++------------- >> 3 files changed, 68 insertions(+), 31 deletions(-) >> >> diff --git a/accel/tcg/internal.h b/accel/tcg/internal.h >> index 1227bb69bd..9c06b320b7 100644 >> --- a/accel/tcg/internal.h >> +++ b/accel/tcg/internal.h >> @@ -106,8 +106,8 @@ void tb_reset_jump(TranslationBlock *tb, int n); >> TranslationBlock *tb_link_page(TranslationBlock *tb, tb_page_addr_t phys_pc, >> tb_page_addr_t phys_page2); >> bool tb_invalidate_phys_page_unwind(tb_page_addr_t addr, uintptr_t pc); >> -int cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb, >> - uintptr_t searched_pc, bool reset_icount); >> +void cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb, >> + uintptr_t host_pc, bool reset_icount); >> >> /* Return the current PC from CPU, which may be cached in TB. */ >> static inline target_ulong log_pc(CPUState *cpu, const TranslationBlock *tb) >> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h >> index e948992a80..7d851f5907 100644 >> --- a/include/exec/exec-all.h >> +++ b/include/exec/exec-all.h >> @@ -39,20 +39,33 @@ typedef ram_addr_t tb_page_addr_t; >> #define TB_PAGE_ADDR_FMT RAM_ADDR_FMT >> #endif >> >> +/** >> + * cpu_unwind_state_data: >> + * @cpu: the cpu context >> + * @host_pc: the host pc within the translation >> + * @data: output data >> + * >> + * Attempt to load the the unwind state for a host pc occurring in >> + * translated code. If @host_pc is not in translated code, the >> + * function returns false; otherwise @data is loaded. >> + * This is the same unwind info as given to restore_state_to_opc. >> + */ >> +bool cpu_unwind_state_data(CPUState *cpu, uintptr_t host_pc, uint64_t *data); >> + >> /** >> * cpu_restore_state: >> - * @cpu: the vCPU state is to be restore to >> - * @searched_pc: the host PC the fault occurred at >> + * @cpu: the cpu context >> + * @host_pc: the host pc within the translation >> * @will_exit: true if the TB executed will be interrupted after some >> cpu adjustments. Required for maintaining the correct >> icount valus >> * @return: true if state was restored, false otherwise >> * >> * Attempt to restore the state for a fault occurring in translated >> - * code. If the searched_pc is not in translated code no state is >> + * code. If @host_pc is not in translated code no state is >> * restored and the function returns false. >> */ >> -bool cpu_restore_state(CPUState *cpu, uintptr_t searched_pc, bool will_exit); >> +bool cpu_restore_state(CPUState *cpu, uintptr_t host_pc, bool will_exit); >> >> G_NORETURN void cpu_loop_exit_noexc(CPUState *cpu); >> G_NORETURN void cpu_loop_exit(CPUState *cpu); >> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c >> index f185356a36..319becb698 100644 >> --- a/accel/tcg/translate-all.c >> +++ b/accel/tcg/translate-all.c >> @@ -247,52 +247,66 @@ static int encode_search(TranslationBlock *tb, uint8_t *block) >> return p - block; >> } >> >> -/* The cpu state corresponding to 'searched_pc' is restored. >> - * When reset_icount is true, current TB will be interrupted and >> - * icount should be recalculated. >> - */ >> -int cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb, >> - uintptr_t searched_pc, bool reset_icount) > > > Maybe add a small comment about what the return value of this static function means? > It can be indirectly inferred from its point of use: > > + int insns_left = cpu_unwind_data_from_tb(tb, host_pc, data); > > But I find having the information about the meaning of a function and return value useful to be available there. > > IIUC for external functions the standard way is to document in the header files, but for the static functions I would think we can do it here. > > With that Reviewed-by: Claudio Fontana <cfontana@suse.de> I added +/** + * cpu_unwind_data_from_tb: Load unwind data for TB + * @tb: translation block + * @host_pc: the host pc within translation + * @data: output array + * + * Within @tb, locate the guest insn whose translation contains @host_pc, + * then load the unwind data created by INDEX_opc_start_insn for that + * guest insn. Return the number of guest insns which remain un-executed + * within @tb -- these must be credited back to the cpu's icount budget. + * + * If we could not determine which guest insn to which @host_pc belongs, + * return -1 and do not load unwind data. + * FIXME: Such a failure is likely to break the guest, as we were not + * expecting to unwind from such a location. This may be some sort of + * backend code generation problem. Consider asserting instead. */ Which I think captures some of your v1 comments as well. r~ ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/6] accel/tcg: Introduce cpu_unwind_state_data 2022-10-27 11:30 ` Richard Henderson @ 2022-10-27 11:39 ` Claudio Fontana 0 siblings, 0 replies; 17+ messages in thread From: Claudio Fontana @ 2022-10-27 11:39 UTC (permalink / raw) To: Richard Henderson, qemu-devel On 10/27/22 13:30, Richard Henderson wrote: > On 10/27/22 20:40, Claudio Fontana wrote: >> On 10/27/22 12:02, Richard Henderson wrote: >>> Add a way to examine the unwind data without actually >>> restoring the data back into env. >>> >>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >>> --- >>> accel/tcg/internal.h | 4 +-- >>> include/exec/exec-all.h | 21 ++++++++--- >>> accel/tcg/translate-all.c | 74 ++++++++++++++++++++++++++------------- >>> 3 files changed, 68 insertions(+), 31 deletions(-) >>> >>> diff --git a/accel/tcg/internal.h b/accel/tcg/internal.h >>> index 1227bb69bd..9c06b320b7 100644 >>> --- a/accel/tcg/internal.h >>> +++ b/accel/tcg/internal.h >>> @@ -106,8 +106,8 @@ void tb_reset_jump(TranslationBlock *tb, int n); >>> TranslationBlock *tb_link_page(TranslationBlock *tb, tb_page_addr_t phys_pc, >>> tb_page_addr_t phys_page2); >>> bool tb_invalidate_phys_page_unwind(tb_page_addr_t addr, uintptr_t pc); >>> -int cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb, >>> - uintptr_t searched_pc, bool reset_icount); >>> +void cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb, >>> + uintptr_t host_pc, bool reset_icount); >>> >>> /* Return the current PC from CPU, which may be cached in TB. */ >>> static inline target_ulong log_pc(CPUState *cpu, const TranslationBlock *tb) >>> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h >>> index e948992a80..7d851f5907 100644 >>> --- a/include/exec/exec-all.h >>> +++ b/include/exec/exec-all.h >>> @@ -39,20 +39,33 @@ typedef ram_addr_t tb_page_addr_t; >>> #define TB_PAGE_ADDR_FMT RAM_ADDR_FMT >>> #endif >>> >>> +/** >>> + * cpu_unwind_state_data: >>> + * @cpu: the cpu context >>> + * @host_pc: the host pc within the translation >>> + * @data: output data >>> + * >>> + * Attempt to load the the unwind state for a host pc occurring in >>> + * translated code. If @host_pc is not in translated code, the >>> + * function returns false; otherwise @data is loaded. >>> + * This is the same unwind info as given to restore_state_to_opc. >>> + */ >>> +bool cpu_unwind_state_data(CPUState *cpu, uintptr_t host_pc, uint64_t *data); >>> + >>> /** >>> * cpu_restore_state: >>> - * @cpu: the vCPU state is to be restore to >>> - * @searched_pc: the host PC the fault occurred at >>> + * @cpu: the cpu context >>> + * @host_pc: the host pc within the translation >>> * @will_exit: true if the TB executed will be interrupted after some >>> cpu adjustments. Required for maintaining the correct >>> icount valus >>> * @return: true if state was restored, false otherwise >>> * >>> * Attempt to restore the state for a fault occurring in translated >>> - * code. If the searched_pc is not in translated code no state is >>> + * code. If @host_pc is not in translated code no state is >>> * restored and the function returns false. >>> */ >>> -bool cpu_restore_state(CPUState *cpu, uintptr_t searched_pc, bool will_exit); >>> +bool cpu_restore_state(CPUState *cpu, uintptr_t host_pc, bool will_exit); >>> >>> G_NORETURN void cpu_loop_exit_noexc(CPUState *cpu); >>> G_NORETURN void cpu_loop_exit(CPUState *cpu); >>> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c >>> index f185356a36..319becb698 100644 >>> --- a/accel/tcg/translate-all.c >>> +++ b/accel/tcg/translate-all.c >>> @@ -247,52 +247,66 @@ static int encode_search(TranslationBlock *tb, uint8_t *block) >>> return p - block; >>> } >>> >>> -/* The cpu state corresponding to 'searched_pc' is restored. >>> - * When reset_icount is true, current TB will be interrupted and >>> - * icount should be recalculated. >>> - */ >>> -int cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb, >>> - uintptr_t searched_pc, bool reset_icount) >> >> >> Maybe add a small comment about what the return value of this static function means? >> It can be indirectly inferred from its point of use: >> >> + int insns_left = cpu_unwind_data_from_tb(tb, host_pc, data); >> >> But I find having the information about the meaning of a function and return value useful to be available there. >> >> IIUC for external functions the standard way is to document in the header files, but for the static functions I would think we can do it here. >> >> With that Reviewed-by: Claudio Fontana <cfontana@suse.de> > > > I added > > +/** > + * cpu_unwind_data_from_tb: Load unwind data for TB > + * @tb: translation block > + * @host_pc: the host pc within translation > + * @data: output array > + * > + * Within @tb, locate the guest insn whose translation contains @host_pc, > + * then load the unwind data created by INDEX_opc_start_insn for that > + * guest insn. Return the number of guest insns which remain un-executed > + * within @tb -- these must be credited back to the cpu's icount budget. > + * > + * If we could not determine which guest insn to which @host_pc belongs, > + * return -1 and do not load unwind data. > + * FIXME: Such a failure is likely to break the guest, as we were not > + * expecting to unwind from such a location. This may be some sort of > + * backend code generation problem. Consider asserting instead. > */ > > Which I think captures some of your v1 comments as well. > > > r~ > Very clear thanks, Reviewed-by: Claudio Fontana <cfontana@suse.de> ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 2/6] target/i386: Use cpu_unwind_state_data for tpr access 2022-10-27 10:02 [PATCH v2 0/6] tcg: Fix x86 TARGET_TB_PCREL (#1269) Richard Henderson 2022-10-27 10:02 ` [PATCH v2 1/6] accel/tcg: Introduce cpu_unwind_state_data Richard Henderson @ 2022-10-27 10:02 ` Richard Henderson 2022-10-27 12:22 ` Claudio Fontana 2022-10-27 10:02 ` [PATCH v2 3/6] target/openrisc: Always exit after mtspr npc Richard Henderson ` (4 subsequent siblings) 6 siblings, 1 reply; 17+ messages in thread From: Richard Henderson @ 2022-10-27 10:02 UTC (permalink / raw) To: qemu-devel; +Cc: cfontana Avoid cpu_restore_state, and modifying env->eip out from underneath the translator with TARGET_TB_PCREL. There is some slight duplication from x86_restore_state_to_opc, but it's just a few lines. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1269 Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/i386/helper.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/target/i386/helper.c b/target/i386/helper.c index b62a1e48e2..2cd1756f1a 100644 --- a/target/i386/helper.c +++ b/target/i386/helper.c @@ -509,6 +509,23 @@ void cpu_x86_inject_mce(Monitor *mon, X86CPU *cpu, int bank, } } +static target_ulong get_memio_eip(CPUX86State *env) +{ + uint64_t data[TARGET_INSN_START_WORDS]; + CPUState *cs = env_cpu(env); + + if (!cpu_unwind_state_data(cs, cs->mem_io_pc, data)) { + return env->eip; + } + + /* Per x86_restore_state_to_opc. */ + if (TARGET_TB_PCREL) { + return (env->eip & TARGET_PAGE_MASK) | data[0]; + } else { + return data[0] - env->segs[R_CS].base; + } +} + void cpu_report_tpr_access(CPUX86State *env, TPRAccess access) { X86CPU *cpu = env_archcpu(env); @@ -519,9 +536,9 @@ void cpu_report_tpr_access(CPUX86State *env, TPRAccess access) cpu_interrupt(cs, CPU_INTERRUPT_TPR); } else if (tcg_enabled()) { - cpu_restore_state(cs, cs->mem_io_pc, false); + target_ulong eip = get_memio_eip(env); - apic_handle_tpr_access_report(cpu->apic_state, env->eip, access); + apic_handle_tpr_access_report(cpu->apic_state, eip, access); } } #endif /* !CONFIG_USER_ONLY */ -- 2.34.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/6] target/i386: Use cpu_unwind_state_data for tpr access 2022-10-27 10:02 ` [PATCH v2 2/6] target/i386: Use cpu_unwind_state_data for tpr access Richard Henderson @ 2022-10-27 12:22 ` Claudio Fontana 2022-10-27 20:13 ` Richard Henderson 0 siblings, 1 reply; 17+ messages in thread From: Claudio Fontana @ 2022-10-27 12:22 UTC (permalink / raw) To: Richard Henderson, qemu-devel On 10/27/22 12:02, Richard Henderson wrote: > Avoid cpu_restore_state, and modifying env->eip out from > underneath the translator with TARGET_TB_PCREL. There is > some slight duplication from x86_restore_state_to_opc, > but it's just a few lines. > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1269 > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/i386/helper.c | 21 +++++++++++++++++++-- > 1 file changed, 19 insertions(+), 2 deletions(-) > > diff --git a/target/i386/helper.c b/target/i386/helper.c > index b62a1e48e2..2cd1756f1a 100644 > --- a/target/i386/helper.c > +++ b/target/i386/helper.c > @@ -509,6 +509,23 @@ void cpu_x86_inject_mce(Monitor *mon, X86CPU *cpu, int bank, > } > } > > +static target_ulong get_memio_eip(CPUX86State *env) > +{ > + uint64_t data[TARGET_INSN_START_WORDS]; > + CPUState *cs = env_cpu(env); > + > + if (!cpu_unwind_state_data(cs, cs->mem_io_pc, data)) { > + return env->eip; > + } > + > + /* Per x86_restore_state_to_opc. */ > + if (TARGET_TB_PCREL) { > + return (env->eip & TARGET_PAGE_MASK) | data[0]; > + } else { > + return data[0] - env->segs[R_CS].base; here we switch from taking cs_base from the TranslationBlock to taking it from env-> . I traced the tb->cs_base use back to cpu_exec() and cpu_exec_step_atomic() and from there it seems ok, as the sequence is cpu_get_tb_cpu_state(cpu->env_ptr, &pc, &cs_base, &flags), which gets it from env, followed by tb_gen_code(...cs_base) which sets the TranslationBlock cs_base, and tb->cs_base does not seem to change again. I mention this in the case there can be some weird situation in which env and tb can end up not being consistent, does a TranslationBlock that is initialized with a certain cs_base from the env that contains user code to load / change the CS segment base potentially constitute a problem? Ciao, Claudio > + } > +} > + > void cpu_report_tpr_access(CPUX86State *env, TPRAccess access) > { > X86CPU *cpu = env_archcpu(env); > @@ -519,9 +536,9 @@ void cpu_report_tpr_access(CPUX86State *env, TPRAccess access) > > cpu_interrupt(cs, CPU_INTERRUPT_TPR); > } else if (tcg_enabled()) { > - cpu_restore_state(cs, cs->mem_io_pc, false); > + target_ulong eip = get_memio_eip(env); > > - apic_handle_tpr_access_report(cpu->apic_state, env->eip, access); > + apic_handle_tpr_access_report(cpu->apic_state, eip, access); > } > } > #endif /* !CONFIG_USER_ONLY */ ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/6] target/i386: Use cpu_unwind_state_data for tpr access 2022-10-27 12:22 ` Claudio Fontana @ 2022-10-27 20:13 ` Richard Henderson 2022-10-28 8:10 ` Claudio Fontana 0 siblings, 1 reply; 17+ messages in thread From: Richard Henderson @ 2022-10-27 20:13 UTC (permalink / raw) To: Claudio Fontana, qemu-devel On 10/27/22 22:22, Claudio Fontana wrote: > On 10/27/22 12:02, Richard Henderson wrote: >> + /* Per x86_restore_state_to_opc. */ >> + if (TARGET_TB_PCREL) { >> + return (env->eip & TARGET_PAGE_MASK) | data[0]; >> + } else { >> + return data[0] - env->segs[R_CS].base; > > here we switch from taking cs_base from the TranslationBlock to taking it from env-> . > > I traced the tb->cs_base use back to > > cpu_exec() and cpu_exec_step_atomic() > > and from there it seems ok, as the sequence is > > cpu_get_tb_cpu_state(cpu->env_ptr, &pc, &cs_base, &flags), which gets it from env, > > followed by > > tb_gen_code(...cs_base) which sets the TranslationBlock cs_base, and tb->cs_base does > not seem to change again. Correct. I wondered if I'd made a mistake by not returning the TB located during the search, but it doesn't seem to have mattered for the two users. > I mention this in the case there can be some weird situation in which env and tb can > end up not being consistent, does a TranslationBlock that is initialized with a certain > cs_base from the env that contains user code to load / change the CS segment base > potentially constitute a problem? The only way to load/change a CS segment base is a branch instruction, which will of course end the TB. There should be no way to change CS that continues the TB. r~ ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/6] target/i386: Use cpu_unwind_state_data for tpr access 2022-10-27 20:13 ` Richard Henderson @ 2022-10-28 8:10 ` Claudio Fontana 0 siblings, 0 replies; 17+ messages in thread From: Claudio Fontana @ 2022-10-28 8:10 UTC (permalink / raw) To: Richard Henderson, qemu-devel On 10/27/22 22:13, Richard Henderson wrote: > On 10/27/22 22:22, Claudio Fontana wrote: >> On 10/27/22 12:02, Richard Henderson wrote: >>> + /* Per x86_restore_state_to_opc. */ >>> + if (TARGET_TB_PCREL) { >>> + return (env->eip & TARGET_PAGE_MASK) | data[0]; >>> + } else { >>> + return data[0] - env->segs[R_CS].base; >> >> here we switch from taking cs_base from the TranslationBlock to taking it from env-> . >> >> I traced the tb->cs_base use back to >> >> cpu_exec() and cpu_exec_step_atomic() >> >> and from there it seems ok, as the sequence is >> >> cpu_get_tb_cpu_state(cpu->env_ptr, &pc, &cs_base, &flags), which gets it from env, >> >> followed by >> >> tb_gen_code(...cs_base) which sets the TranslationBlock cs_base, and tb->cs_base does >> not seem to change again. > Correct. I wondered if I'd made a mistake by not returning the TB located during the > search, but it doesn't seem to have mattered for the two users. > >> I mention this in the case there can be some weird situation in which env and tb can >> end up not being consistent, does a TranslationBlock that is initialized with a certain >> cs_base from the env that contains user code to load / change the CS segment base >> potentially constitute a problem? > The only way to load/change a CS segment base is a branch instruction, which will of > course end the TB. There should be no way to change CS that continues the TB. > > > r~ Reviewed-by: Claudio Fontana <cfontana@suse.de> ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 3/6] target/openrisc: Always exit after mtspr npc 2022-10-27 10:02 [PATCH v2 0/6] tcg: Fix x86 TARGET_TB_PCREL (#1269) Richard Henderson 2022-10-27 10:02 ` [PATCH v2 1/6] accel/tcg: Introduce cpu_unwind_state_data Richard Henderson 2022-10-27 10:02 ` [PATCH v2 2/6] target/i386: Use cpu_unwind_state_data for tpr access Richard Henderson @ 2022-10-27 10:02 ` Richard Henderson 2022-10-27 10:49 ` Philippe Mathieu-Daudé 2022-10-27 10:02 ` [PATCH v2 4/6] target/openrisc: Use cpu_unwind_state_data for mfspr Richard Henderson ` (3 subsequent siblings) 6 siblings, 1 reply; 17+ messages in thread From: Richard Henderson @ 2022-10-27 10:02 UTC (permalink / raw) To: qemu-devel; +Cc: cfontana We have called cpu_restore_state asserting will_exit. Do not go back on that promise. This affects icount. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/openrisc/sys_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/openrisc/sys_helper.c b/target/openrisc/sys_helper.c index 09b3c97d7c..a3508e421d 100644 --- a/target/openrisc/sys_helper.c +++ b/target/openrisc/sys_helper.c @@ -51,8 +51,8 @@ void HELPER(mtspr)(CPUOpenRISCState *env, target_ulong spr, target_ulong rb) if (env->pc != rb) { env->pc = rb; env->dflag = 0; - cpu_loop_exit(cs); } + cpu_loop_exit(cs); break; case TO_SPR(0, 17): /* SR */ -- 2.34.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 3/6] target/openrisc: Always exit after mtspr npc 2022-10-27 10:02 ` [PATCH v2 3/6] target/openrisc: Always exit after mtspr npc Richard Henderson @ 2022-10-27 10:49 ` Philippe Mathieu-Daudé 0 siblings, 0 replies; 17+ messages in thread From: Philippe Mathieu-Daudé @ 2022-10-27 10:49 UTC (permalink / raw) To: Richard Henderson, qemu-devel; +Cc: cfontana On 27/10/22 12:02, Richard Henderson wrote: > We have called cpu_restore_state asserting will_exit. > Do not go back on that promise. This affects icount. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/openrisc/sys_helper.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 4/6] target/openrisc: Use cpu_unwind_state_data for mfspr 2022-10-27 10:02 [PATCH v2 0/6] tcg: Fix x86 TARGET_TB_PCREL (#1269) Richard Henderson ` (2 preceding siblings ...) 2022-10-27 10:02 ` [PATCH v2 3/6] target/openrisc: Always exit after mtspr npc Richard Henderson @ 2022-10-27 10:02 ` Richard Henderson 2022-10-28 9:16 ` Claudio Fontana 2022-10-27 10:02 ` [PATCH v2 5/6] accel/tcg: Remove will_exit argument from cpu_restore_state Richard Henderson ` (2 subsequent siblings) 6 siblings, 1 reply; 17+ messages in thread From: Richard Henderson @ 2022-10-27 10:02 UTC (permalink / raw) To: qemu-devel; +Cc: cfontana Since we do not plan to exit, use cpu_unwind_state_data and extract exactly the data requested. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/openrisc/sys_helper.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/target/openrisc/sys_helper.c b/target/openrisc/sys_helper.c index a3508e421d..dde2fa1623 100644 --- a/target/openrisc/sys_helper.c +++ b/target/openrisc/sys_helper.c @@ -199,6 +199,7 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env, target_ulong rd, target_ulong spr) { #ifndef CONFIG_USER_ONLY + uint64_t data[TARGET_INSN_START_WORDS]; MachineState *ms = MACHINE(qdev_get_machine()); OpenRISCCPU *cpu = env_archcpu(env); CPUState *cs = env_cpu(env); @@ -232,14 +233,20 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env, target_ulong rd, return env->evbar; case TO_SPR(0, 16): /* NPC (equals PC) */ - cpu_restore_state(cs, GETPC(), false); + if (cpu_unwind_state_data(cs, GETPC(), data)) { + return data[0]; + } return env->pc; case TO_SPR(0, 17): /* SR */ return cpu_get_sr(env); case TO_SPR(0, 18): /* PPC */ - cpu_restore_state(cs, GETPC(), false); + if (cpu_unwind_state_data(cs, GETPC(), data)) { + if (data[1] & 2) { + return data[0] - 4; + } + } return env->ppc; case TO_SPR(0, 32): /* EPCR */ -- 2.34.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 4/6] target/openrisc: Use cpu_unwind_state_data for mfspr 2022-10-27 10:02 ` [PATCH v2 4/6] target/openrisc: Use cpu_unwind_state_data for mfspr Richard Henderson @ 2022-10-28 9:16 ` Claudio Fontana 2022-10-28 18:46 ` Richard Henderson 0 siblings, 1 reply; 17+ messages in thread From: Claudio Fontana @ 2022-10-28 9:16 UTC (permalink / raw) To: Richard Henderson, Philippe Mathieu-Daudé; +Cc: qemu-devel On 10/27/22 12:02, Richard Henderson wrote: > Since we do not plan to exit, use cpu_unwind_state_data > and extract exactly the data requested. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/openrisc/sys_helper.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/target/openrisc/sys_helper.c b/target/openrisc/sys_helper.c > index a3508e421d..dde2fa1623 100644 > --- a/target/openrisc/sys_helper.c > +++ b/target/openrisc/sys_helper.c > @@ -199,6 +199,7 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env, target_ulong rd, > target_ulong spr) > { > #ifndef CONFIG_USER_ONLY > + uint64_t data[TARGET_INSN_START_WORDS]; > MachineState *ms = MACHINE(qdev_get_machine()); > OpenRISCCPU *cpu = env_archcpu(env); > CPUState *cs = env_cpu(env); > @@ -232,14 +233,20 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env, target_ulong rd, > return env->evbar; > > case TO_SPR(0, 16): /* NPC (equals PC) */ > - cpu_restore_state(cs, GETPC(), false); > + if (cpu_unwind_state_data(cs, GETPC(), data)) { > + return data[0]; > + } > return env->pc; > > case TO_SPR(0, 17): /* SR */ > return cpu_get_sr(env); > > case TO_SPR(0, 18): /* PPC */ > - cpu_restore_state(cs, GETPC(), false); > + if (cpu_unwind_state_data(cs, GETPC(), data)) { > + if (data[1] & 2) { > + return data[0] - 4; > + } > + } > return env->ppc; > > case TO_SPR(0, 32): /* EPCR */ I am struggling to understand if the fact that we are not setting cpu->env.dflag anymore in the mfspr helper is fine; here I am unfamiliar with the arch, also Ccing Philippe in case he wants to step in to review this bit. Thanks, CLaudio ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 4/6] target/openrisc: Use cpu_unwind_state_data for mfspr 2022-10-28 9:16 ` Claudio Fontana @ 2022-10-28 18:46 ` Richard Henderson 0 siblings, 0 replies; 17+ messages in thread From: Richard Henderson @ 2022-10-28 18:46 UTC (permalink / raw) To: Claudio Fontana, Philippe Mathieu-Daudé; +Cc: qemu-devel, Stafford Horne On 10/28/22 20:16, Claudio Fontana wrote: > On 10/27/22 12:02, Richard Henderson wrote: >> Since we do not plan to exit, use cpu_unwind_state_data >> and extract exactly the data requested. >> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >> --- >> target/openrisc/sys_helper.c | 11 +++++++++-- >> 1 file changed, 9 insertions(+), 2 deletions(-) >> >> diff --git a/target/openrisc/sys_helper.c b/target/openrisc/sys_helper.c >> index a3508e421d..dde2fa1623 100644 >> --- a/target/openrisc/sys_helper.c >> +++ b/target/openrisc/sys_helper.c >> @@ -199,6 +199,7 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env, target_ulong rd, >> target_ulong spr) >> { >> #ifndef CONFIG_USER_ONLY >> + uint64_t data[TARGET_INSN_START_WORDS]; >> MachineState *ms = MACHINE(qdev_get_machine()); >> OpenRISCCPU *cpu = env_archcpu(env); >> CPUState *cs = env_cpu(env); >> @@ -232,14 +233,20 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env, target_ulong rd, >> return env->evbar; >> >> case TO_SPR(0, 16): /* NPC (equals PC) */ >> - cpu_restore_state(cs, GETPC(), false); >> + if (cpu_unwind_state_data(cs, GETPC(), data)) { >> + return data[0]; >> + } >> return env->pc; >> >> case TO_SPR(0, 17): /* SR */ >> return cpu_get_sr(env); >> >> case TO_SPR(0, 18): /* PPC */ >> - cpu_restore_state(cs, GETPC(), false); >> + if (cpu_unwind_state_data(cs, GETPC(), data)) { >> + if (data[1] & 2) { >> + return data[0] - 4; >> + } >> + } >> return env->ppc; >> >> case TO_SPR(0, 32): /* EPCR */ > > I am struggling to understand if the fact that we are not setting cpu->env.dflag anymore in the mfspr helper is fine; That we might do so before was buggy. We manage dflag in openrisc_tr_tb_stop expecting a particular value. I should expand the patch comment on this. Consider: l.j L2 // branch l.mfspr r1, ppc // delay L1: boom L2: l.lwa r3, (r4) With "l.mfspr reg, ppc" executed in a delay slot, dflag would be set, but it would not be cleared by openrisc_tr_tb_stop on exiting the TB, because tb_stop thinks the value is already zero. The next TB begins at L2 with dflag set when it should not. If the load has a tlb miss, then the interrupt will be delivered with DSX set in the status register, and the pc decremented (openrisc implements restart after delay slot by re-executing the branch). This will cause the return from interrupt to go to L1, and boom! r~ ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 5/6] accel/tcg: Remove will_exit argument from cpu_restore_state 2022-10-27 10:02 [PATCH v2 0/6] tcg: Fix x86 TARGET_TB_PCREL (#1269) Richard Henderson ` (3 preceding siblings ...) 2022-10-27 10:02 ` [PATCH v2 4/6] target/openrisc: Use cpu_unwind_state_data for mfspr Richard Henderson @ 2022-10-27 10:02 ` Richard Henderson 2022-10-27 10:02 ` [PATCH v2 6/6] accel/tcg: Remove reset_icount argument from cpu_restore_state_from_tb Richard Henderson 2022-10-31 0:40 ` [PATCH v2 0/6] tcg: Fix x86 TARGET_TB_PCREL (#1269) Richard Henderson 6 siblings, 0 replies; 17+ messages in thread From: Richard Henderson @ 2022-10-27 10:02 UTC (permalink / raw) To: qemu-devel; +Cc: cfontana The value passed is always true, and if the target's synchronize_from_tb hook is non-trivial, not exiting may be erroneous. Reviewed-by: Claudio Fontana <cfontana@suse.de> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- include/exec/exec-all.h | 5 +---- accel/tcg/cpu-exec-common.c | 2 +- accel/tcg/translate-all.c | 12 ++---------- target/alpha/helper.c | 2 +- target/alpha/mem_helper.c | 2 +- target/arm/op_helper.c | 2 +- target/arm/tlb_helper.c | 8 ++++---- target/cris/helper.c | 2 +- target/i386/tcg/sysemu/svm_helper.c | 2 +- target/m68k/op_helper.c | 4 ++-- target/microblaze/helper.c | 2 +- target/nios2/op_helper.c | 2 +- target/openrisc/sys_helper.c | 4 ++-- target/ppc/excp_helper.c | 2 +- target/s390x/tcg/excp_helper.c | 2 +- target/tricore/op_helper.c | 2 +- target/xtensa/helper.c | 6 +++--- 17 files changed, 25 insertions(+), 36 deletions(-) diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index 7d851f5907..9b7bfbf09a 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -56,16 +56,13 @@ bool cpu_unwind_state_data(CPUState *cpu, uintptr_t host_pc, uint64_t *data); * cpu_restore_state: * @cpu: the cpu context * @host_pc: the host pc within the translation - * @will_exit: true if the TB executed will be interrupted after some - cpu adjustments. Required for maintaining the correct - icount valus * @return: true if state was restored, false otherwise * * Attempt to restore the state for a fault occurring in translated * code. If @host_pc is not in translated code no state is * restored and the function returns false. */ -bool cpu_restore_state(CPUState *cpu, uintptr_t host_pc, bool will_exit); +bool cpu_restore_state(CPUState *cpu, uintptr_t host_pc); G_NORETURN void cpu_loop_exit_noexc(CPUState *cpu); G_NORETURN void cpu_loop_exit(CPUState *cpu); diff --git a/accel/tcg/cpu-exec-common.c b/accel/tcg/cpu-exec-common.c index be6fe45aa5..c7bc8c6efa 100644 --- a/accel/tcg/cpu-exec-common.c +++ b/accel/tcg/cpu-exec-common.c @@ -71,7 +71,7 @@ void cpu_loop_exit(CPUState *cpu) void cpu_loop_exit_restore(CPUState *cpu, uintptr_t pc) { if (pc) { - cpu_restore_state(cpu, pc, true); + cpu_restore_state(cpu, pc); } cpu_loop_exit(cpu); } diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c index 319becb698..90997fed47 100644 --- a/accel/tcg/translate-all.c +++ b/accel/tcg/translate-all.c @@ -318,16 +318,8 @@ void cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb, #endif } -bool cpu_restore_state(CPUState *cpu, uintptr_t host_pc, bool will_exit) +bool cpu_restore_state(CPUState *cpu, uintptr_t host_pc) { - /* - * The pc update associated with restore without exit will - * break the relative pc adjustments performed by TARGET_TB_PCREL. - */ - if (TARGET_TB_PCREL) { - assert(will_exit); - } - /* * The host_pc has to be in the rx region of the code buffer. * If it is not we will not be able to resolve it here. @@ -341,7 +333,7 @@ bool cpu_restore_state(CPUState *cpu, uintptr_t host_pc, bool will_exit) if (in_code_gen_buffer((const void *)(host_pc - tcg_splitwx_diff))) { TranslationBlock *tb = tcg_tb_lookup(host_pc); if (tb) { - cpu_restore_state_from_tb(cpu, tb, host_pc, will_exit); + cpu_restore_state_from_tb(cpu, tb, host_pc, true); return true; } } diff --git a/target/alpha/helper.c b/target/alpha/helper.c index a5a389b5a3..970c869771 100644 --- a/target/alpha/helper.c +++ b/target/alpha/helper.c @@ -532,7 +532,7 @@ G_NORETURN void dynamic_excp(CPUAlphaState *env, uintptr_t retaddr, cs->exception_index = excp; env->error_code = error; if (retaddr) { - cpu_restore_state(cs, retaddr, true); + cpu_restore_state(cs, retaddr); /* Floating-point exceptions (our only users) point to the next PC. */ env->pc += 4; } diff --git a/target/alpha/mem_helper.c b/target/alpha/mem_helper.c index 47283a0612..a39b52c5dd 100644 --- a/target/alpha/mem_helper.c +++ b/target/alpha/mem_helper.c @@ -28,7 +28,7 @@ static void do_unaligned_access(CPUAlphaState *env, vaddr addr, uintptr_t retadd uint64_t pc; uint32_t insn; - cpu_restore_state(env_cpu(env), retaddr, true); + cpu_restore_state(env_cpu(env), retaddr); pc = env->pc; insn = cpu_ldl_code(env, pc); diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c index c5bde1cfcc..70672bcd9f 100644 --- a/target/arm/op_helper.c +++ b/target/arm/op_helper.c @@ -78,7 +78,7 @@ void raise_exception_ra(CPUARMState *env, uint32_t excp, uint32_t syndrome, * we must restore CPU state here before setting the syndrome * the caller passed us, and cannot use cpu_loop_exit_restore(). */ - cpu_restore_state(cs, ra, true); + cpu_restore_state(cs, ra); raise_exception(env, excp, syndrome, target_el); } diff --git a/target/arm/tlb_helper.c b/target/arm/tlb_helper.c index 69b0dc69df..0f4f4fc809 100644 --- a/target/arm/tlb_helper.c +++ b/target/arm/tlb_helper.c @@ -156,7 +156,7 @@ void arm_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr, ARMMMUFaultInfo fi = {}; /* now we have a real cpu fault */ - cpu_restore_state(cs, retaddr, true); + cpu_restore_state(cs, retaddr); fi.type = ARMFault_Alignment; arm_deliver_fault(cpu, vaddr, access_type, mmu_idx, &fi); @@ -196,7 +196,7 @@ void arm_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr, ARMMMUFaultInfo fi = {}; /* now we have a real cpu fault */ - cpu_restore_state(cs, retaddr, true); + cpu_restore_state(cs, retaddr); fi.ea = arm_extabort_type(response); fi.type = ARMFault_SyncExternal; @@ -252,7 +252,7 @@ bool arm_cpu_tlb_fill(CPUState *cs, vaddr address, int size, return false; } else { /* now we have a real cpu fault */ - cpu_restore_state(cs, retaddr, true); + cpu_restore_state(cs, retaddr); arm_deliver_fault(cpu, address, access_type, mmu_idx, fi); } } @@ -271,7 +271,7 @@ void arm_cpu_record_sigsegv(CPUState *cs, vaddr addr, * We report both ESR and FAR to signal handlers. * For now, it's easiest to deliver the fault normally. */ - cpu_restore_state(cs, ra, true); + cpu_restore_state(cs, ra); arm_deliver_fault(cpu, addr, access_type, MMU_USER_IDX, &fi); } diff --git a/target/cris/helper.c b/target/cris/helper.c index 91e4aeb178..81a72699b5 100644 --- a/target/cris/helper.c +++ b/target/cris/helper.c @@ -87,7 +87,7 @@ bool cris_cpu_tlb_fill(CPUState *cs, vaddr address, int size, cs->exception_index = EXCP_BUSFAULT; env->fault_vector = res.bf_vec; if (retaddr) { - if (cpu_restore_state(cs, retaddr, true)) { + if (cpu_restore_state(cs, retaddr)) { /* Evaluate flags after retranslation. */ helper_top_evaluate_flags(env); } diff --git a/target/i386/tcg/sysemu/svm_helper.c b/target/i386/tcg/sysemu/svm_helper.c index 8e88567399..2d27731b60 100644 --- a/target/i386/tcg/sysemu/svm_helper.c +++ b/target/i386/tcg/sysemu/svm_helper.c @@ -704,7 +704,7 @@ void cpu_vmexit(CPUX86State *env, uint32_t exit_code, uint64_t exit_info_1, { CPUState *cs = env_cpu(env); - cpu_restore_state(cs, retaddr, true); + cpu_restore_state(cs, retaddr); qemu_log_mask(CPU_LOG_TB_IN_ASM, "vmexit(%08x, %016" PRIx64 ", %016" PRIx64 ", " TARGET_FMT_lx ")!\n", diff --git a/target/m68k/op_helper.c b/target/m68k/op_helper.c index 5da176d642..1ce850bbc5 100644 --- a/target/m68k/op_helper.c +++ b/target/m68k/op_helper.c @@ -460,7 +460,7 @@ void m68k_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr, M68kCPU *cpu = M68K_CPU(cs); CPUM68KState *env = &cpu->env; - cpu_restore_state(cs, retaddr, true); + cpu_restore_state(cs, retaddr); if (m68k_feature(env, M68K_FEATURE_M68040)) { env->mmu.mmusr = 0; @@ -558,7 +558,7 @@ raise_exception_format2(CPUM68KState *env, int tt, int ilen, uintptr_t raddr) cs->exception_index = tt; /* Recover PC and CC_OP for the beginning of the insn. */ - cpu_restore_state(cs, raddr, true); + cpu_restore_state(cs, raddr); /* Flags are current in env->cc_*, or are undefined. */ env->cc_op = CC_OP_FLAGS; diff --git a/target/microblaze/helper.c b/target/microblaze/helper.c index a607fe68e5..98bdb82de8 100644 --- a/target/microblaze/helper.c +++ b/target/microblaze/helper.c @@ -277,7 +277,7 @@ void mb_cpu_do_unaligned_access(CPUState *cs, vaddr addr, uint32_t esr, iflags; /* Recover the pc and iflags from the corresponding insn_start. */ - cpu_restore_state(cs, retaddr, true); + cpu_restore_state(cs, retaddr); iflags = cpu->env.iflags; qemu_log_mask(CPU_LOG_INT, diff --git a/target/nios2/op_helper.c b/target/nios2/op_helper.c index 2e30d0a908..0aaf33ffc2 100644 --- a/target/nios2/op_helper.c +++ b/target/nios2/op_helper.c @@ -40,7 +40,7 @@ void nios2_cpu_loop_exit_advance(CPUNios2State *env, uintptr_t retaddr) * Do this here, rather than in restore_state_to_opc(), * lest we affect QEMU internal exceptions, like EXCP_DEBUG. */ - cpu_restore_state(cs, retaddr, true); + cpu_restore_state(cs, retaddr); env->pc += 4; cpu_loop_exit(cs); } diff --git a/target/openrisc/sys_helper.c b/target/openrisc/sys_helper.c index dde2fa1623..ec145960e3 100644 --- a/target/openrisc/sys_helper.c +++ b/target/openrisc/sys_helper.c @@ -45,7 +45,7 @@ void HELPER(mtspr)(CPUOpenRISCState *env, target_ulong spr, target_ulong rb) break; case TO_SPR(0, 16): /* NPC */ - cpu_restore_state(cs, GETPC(), true); + cpu_restore_state(cs, GETPC()); /* ??? Mirror or1ksim in not trashing delayed branch state when "jumping" to the current instruction. */ if (env->pc != rb) { @@ -131,7 +131,7 @@ void HELPER(mtspr)(CPUOpenRISCState *env, target_ulong spr, target_ulong rb) case TO_SPR(8, 0): /* PMR */ env->pmr = rb; if (env->pmr & PMR_DME || env->pmr & PMR_SME) { - cpu_restore_state(cs, GETPC(), true); + cpu_restore_state(cs, GETPC()); env->pc += 4; cs->halted = 1; raise_exception(cpu, EXCP_HALTED); diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index 43f2480e94..3ded309265 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -2414,7 +2414,7 @@ void ppc_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr, uint32_t insn; /* Restore state and reload the insn we executed, for filling in DSISR. */ - cpu_restore_state(cs, retaddr, true); + cpu_restore_state(cs, retaddr); insn = cpu_ldl_code(env, env->nip); switch (env->mmu_model) { diff --git a/target/s390x/tcg/excp_helper.c b/target/s390x/tcg/excp_helper.c index 29ccf70df1..2cd6d062b9 100644 --- a/target/s390x/tcg/excp_helper.c +++ b/target/s390x/tcg/excp_helper.c @@ -39,7 +39,7 @@ G_NORETURN void tcg_s390_program_interrupt(CPUS390XState *env, { CPUState *cs = env_cpu(env); - cpu_restore_state(cs, ra, true); + cpu_restore_state(cs, ra); qemu_log_mask(CPU_LOG_INT, "program interrupt at %#" PRIx64 "\n", env->psw.addr); trigger_pgm_exception(env, code); diff --git a/target/tricore/op_helper.c b/target/tricore/op_helper.c index a79c838a92..532ae6b74c 100644 --- a/target/tricore/op_helper.c +++ b/target/tricore/op_helper.c @@ -31,7 +31,7 @@ void raise_exception_sync_internal(CPUTriCoreState *env, uint32_t class, int tin { CPUState *cs = env_cpu(env); /* in case we come from a helper-call we need to restore the PC */ - cpu_restore_state(cs, pc, true); + cpu_restore_state(cs, pc); /* Tin is loaded into d[15] */ env->gpr_d[15] = tin; diff --git a/target/xtensa/helper.c b/target/xtensa/helper.c index e0a9caab4b..2aa9777a8e 100644 --- a/target/xtensa/helper.c +++ b/target/xtensa/helper.c @@ -253,7 +253,7 @@ void xtensa_cpu_do_unaligned_access(CPUState *cs, assert(xtensa_option_enabled(env->config, XTENSA_OPTION_UNALIGNED_EXCEPTION)); - cpu_restore_state(CPU(cpu), retaddr, true); + cpu_restore_state(CPU(cpu), retaddr); HELPER(exception_cause_vaddr)(env, env->pc, LOAD_STORE_ALIGNMENT_CAUSE, addr); @@ -284,7 +284,7 @@ bool xtensa_cpu_tlb_fill(CPUState *cs, vaddr address, int size, } else if (probe) { return false; } else { - cpu_restore_state(cs, retaddr, true); + cpu_restore_state(cs, retaddr); HELPER(exception_cause_vaddr)(env, env->pc, ret, address); } } @@ -297,7 +297,7 @@ void xtensa_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr, XtensaCPU *cpu = XTENSA_CPU(cs); CPUXtensaState *env = &cpu->env; - cpu_restore_state(cs, retaddr, true); + cpu_restore_state(cs, retaddr); HELPER(exception_cause_vaddr)(env, env->pc, access_type == MMU_INST_FETCH ? INSTR_PIF_ADDR_ERROR_CAUSE : -- 2.34.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 6/6] accel/tcg: Remove reset_icount argument from cpu_restore_state_from_tb 2022-10-27 10:02 [PATCH v2 0/6] tcg: Fix x86 TARGET_TB_PCREL (#1269) Richard Henderson ` (4 preceding siblings ...) 2022-10-27 10:02 ` [PATCH v2 5/6] accel/tcg: Remove will_exit argument from cpu_restore_state Richard Henderson @ 2022-10-27 10:02 ` Richard Henderson 2022-10-31 0:40 ` [PATCH v2 0/6] tcg: Fix x86 TARGET_TB_PCREL (#1269) Richard Henderson 6 siblings, 0 replies; 17+ messages in thread From: Richard Henderson @ 2022-10-27 10:02 UTC (permalink / raw) To: qemu-devel; +Cc: cfontana The value passed is always true. Reviewed-by: Claudio Fontana <cfontana@suse.de> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- accel/tcg/internal.h | 2 +- accel/tcg/tb-maint.c | 4 ++-- accel/tcg/translate-all.c | 15 +++++++-------- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/accel/tcg/internal.h b/accel/tcg/internal.h index 9c06b320b7..cb13bade4f 100644 --- a/accel/tcg/internal.h +++ b/accel/tcg/internal.h @@ -107,7 +107,7 @@ TranslationBlock *tb_link_page(TranslationBlock *tb, tb_page_addr_t phys_pc, tb_page_addr_t phys_page2); bool tb_invalidate_phys_page_unwind(tb_page_addr_t addr, uintptr_t pc); void cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb, - uintptr_t host_pc, bool reset_icount); + uintptr_t host_pc); /* Return the current PC from CPU, which may be cached in TB. */ static inline target_ulong log_pc(CPUState *cpu, const TranslationBlock *tb) diff --git a/accel/tcg/tb-maint.c b/accel/tcg/tb-maint.c index c8e921089d..0cdb35548c 100644 --- a/accel/tcg/tb-maint.c +++ b/accel/tcg/tb-maint.c @@ -536,7 +536,7 @@ tb_invalidate_phys_page_range__locked(struct page_collection *pages, * restore the CPU state. */ current_tb_modified = true; - cpu_restore_state_from_tb(cpu, current_tb, retaddr, true); + cpu_restore_state_from_tb(cpu, current_tb, retaddr); } #endif /* TARGET_HAS_PRECISE_SMC */ tb_phys_invalidate__locked(tb); @@ -685,7 +685,7 @@ bool tb_invalidate_phys_page_unwind(tb_page_addr_t addr, uintptr_t pc) * function to partially restore the CPU state. */ current_tb_modified = true; - cpu_restore_state_from_tb(cpu, current_tb, pc, true); + cpu_restore_state_from_tb(cpu, current_tb, pc); } #endif /* TARGET_HAS_PRECISE_SMC */ tb_phys_invalidate(tb, addr); diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c index 90997fed47..0089578f8f 100644 --- a/accel/tcg/translate-all.c +++ b/accel/tcg/translate-all.c @@ -282,12 +282,11 @@ static int cpu_unwind_data_from_tb(TranslationBlock *tb, uintptr_t host_pc, } /* - * The cpu state corresponding to 'host_pc' is restored. - * When reset_icount is true, current TB will be interrupted and - * icount should be recalculated. + * The cpu state corresponding to 'host_pc' is restored in + * preparation for exiting the TB. */ void cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb, - uintptr_t host_pc, bool reset_icount) + uintptr_t host_pc) { uint64_t data[TARGET_INSN_START_WORDS]; #ifdef CONFIG_PROFILER @@ -300,7 +299,7 @@ void cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb, return; } - if (reset_icount && (tb_cflags(tb) & CF_USE_ICOUNT)) { + if (tb_cflags(tb) & CF_USE_ICOUNT) { assert(icount_enabled()); /* * Reset the cycle counter to the start of the block and @@ -333,7 +332,7 @@ bool cpu_restore_state(CPUState *cpu, uintptr_t host_pc) if (in_code_gen_buffer((const void *)(host_pc - tcg_splitwx_diff))) { TranslationBlock *tb = tcg_tb_lookup(host_pc); if (tb) { - cpu_restore_state_from_tb(cpu, tb, host_pc, true); + cpu_restore_state_from_tb(cpu, tb, host_pc); return true; } } @@ -1032,7 +1031,7 @@ void tb_check_watchpoint(CPUState *cpu, uintptr_t retaddr) tb = tcg_tb_lookup(retaddr); if (tb) { /* We can use retranslation to find the PC. */ - cpu_restore_state_from_tb(cpu, tb, retaddr, true); + cpu_restore_state_from_tb(cpu, tb, retaddr); tb_phys_invalidate(tb, -1); } else { /* The exception probably happened in a helper. The CPU state should @@ -1068,7 +1067,7 @@ void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr) cpu_abort(cpu, "cpu_io_recompile: could not find TB for pc=%p", (void *)retaddr); } - cpu_restore_state_from_tb(cpu, tb, retaddr, true); + cpu_restore_state_from_tb(cpu, tb, retaddr); /* * Some guests must re-execute the branch when re-executing a delay -- 2.34.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 0/6] tcg: Fix x86 TARGET_TB_PCREL (#1269) 2022-10-27 10:02 [PATCH v2 0/6] tcg: Fix x86 TARGET_TB_PCREL (#1269) Richard Henderson ` (5 preceding siblings ...) 2022-10-27 10:02 ` [PATCH v2 6/6] accel/tcg: Remove reset_icount argument from cpu_restore_state_from_tb Richard Henderson @ 2022-10-31 0:40 ` Richard Henderson 6 siblings, 0 replies; 17+ messages in thread From: Richard Henderson @ 2022-10-31 0:40 UTC (permalink / raw) To: qemu-devel; +Cc: cfontana On 10/27/22 21:02, Richard Henderson wrote: > As per #1269, this affects NetBSD installer boot. > > The problem is that one of the x86 acpi callbacks modifies > env->eip during an mmio store, which means that the tracking > that translate.c does is thrown out of whack. > > Introduce a method to extract unwind data without the > writeback to env. This isn't a perfect abstraction, but I > couldn't think of anything better. There's a couple of lines > of code duplication, but probably less than any abstration > that we might put on top > > Changes for v2: > * Rebase on master, 23 patches merged. > * Comments adjusted per review (claudio) > > > r~ Queuing to tcg-next. r~ ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2022-10-31 5:47 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-10-27 10:02 [PATCH v2 0/6] tcg: Fix x86 TARGET_TB_PCREL (#1269) Richard Henderson 2022-10-27 10:02 ` [PATCH v2 1/6] accel/tcg: Introduce cpu_unwind_state_data Richard Henderson 2022-10-27 10:40 ` Claudio Fontana 2022-10-27 11:30 ` Richard Henderson 2022-10-27 11:39 ` Claudio Fontana 2022-10-27 10:02 ` [PATCH v2 2/6] target/i386: Use cpu_unwind_state_data for tpr access Richard Henderson 2022-10-27 12:22 ` Claudio Fontana 2022-10-27 20:13 ` Richard Henderson 2022-10-28 8:10 ` Claudio Fontana 2022-10-27 10:02 ` [PATCH v2 3/6] target/openrisc: Always exit after mtspr npc Richard Henderson 2022-10-27 10:49 ` Philippe Mathieu-Daudé 2022-10-27 10:02 ` [PATCH v2 4/6] target/openrisc: Use cpu_unwind_state_data for mfspr Richard Henderson 2022-10-28 9:16 ` Claudio Fontana 2022-10-28 18:46 ` Richard Henderson 2022-10-27 10:02 ` [PATCH v2 5/6] accel/tcg: Remove will_exit argument from cpu_restore_state Richard Henderson 2022-10-27 10:02 ` [PATCH v2 6/6] accel/tcg: Remove reset_icount argument from cpu_restore_state_from_tb Richard Henderson 2022-10-31 0:40 ` [PATCH v2 0/6] tcg: Fix x86 TARGET_TB_PCREL (#1269) Richard Henderson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.