On 10/25/22 11:23, Claudio Fontana wrote:
> On 10/24/22 15:24, Richard Henderson wrote:
>> Add a way to examine the unwind data without actually
>> restoring the data back into env.
>>
>> Signed-off-by: Richard Henderson <[email protected]>
>> ---
>> include/exec/exec-all.h | 13 ++++++++
>> accel/tcg/translate-all.c | 68 ++++++++++++++++++++++++++-------------
>> 2 files changed, 58 insertions(+), 23 deletions(-)
>>
>> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
>> index 300832bd0b..d49cf113dd 100644
>> --- a/include/exec/exec-all.h
>> +++ b/include/exec/exec-all.h
>> @@ -39,6 +39,19 @@ typedef ram_addr_t tb_page_addr_t;
>> #define TB_PAGE_ADDR_FMT RAM_ADDR_FMT
>> #endif
>>
>> +/**
>> + * cpu_unwind_state_data:
>> + * @cpu: the vCPU state is to be restore to
"the vCPU state to be restored to" ?
>> + * @host_pc: the host PC the fault occurred at
>> + * @data: output data
>> + *
>> + * Attempt to load the the unwind state for a host pc occurring in
>> + * translated code. If the searched_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
>> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
>> index e4386b3198..c772e3769c 100644
>> --- a/accel/tcg/translate-all.c
>> +++ b/accel/tcg/translate-all.c
>> @@ -320,29 +320,20 @@ 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.
>> - */
>> -static 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);
>> }
>
> It's not visible in this hunk, but what follows is:
>
> /* Reconstruct the stored insn data while looking for the point at
>
> which the end of the insn exceeds the searched_pc. */
>
> Should this comment be adapted, minimally with s,searched_pc,host_pc, ?
>
>
>> @@ -353,19 +344,40 @@ static int cpu_restore_state_from_tb(CPUState *cpu,
>> TranslationBlock *tb,
>> 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.
>> + */
>> +static 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;
>> + }
>
> Is the -1 return value some error condition to do anything about, log, tcg
> assert, or ...,
> under some DEBUG_* condition, or ignored as done here?
>
> Thanks,
>
> Claudio
>
>>
>> - 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);
>> @@ -375,7 +387,6 @@ static 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)
>> @@ -408,6 +419,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();
>
