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

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();

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~

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>