diff mbox series

[v4,07/43] tcg: Add in_code_gen_buffer

Message ID 20201214140314.18544-8-richard.henderson@linaro.org
State New
Headers show
Series Mirror map JIT memory for TCG | expand

Commit Message

Richard Henderson Dec. 14, 2020, 2:02 p.m. UTC
Create a function to determine if a pointer is within the buffer.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/tcg/tcg.h         |  6 ++++++
 accel/tcg/translate-all.c | 26 ++++++++------------------
 2 files changed, 14 insertions(+), 18 deletions(-)

Comments

Philippe Mathieu-Daudé Dec. 14, 2020, 10:09 p.m. UTC | #1
On 12/14/20 3:02 PM, Richard Henderson wrote:
> Create a function to determine if a pointer is within the buffer.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/tcg/tcg.h         |  6 ++++++
>  accel/tcg/translate-all.c | 26 ++++++++------------------
>  2 files changed, 14 insertions(+), 18 deletions(-)
> 
> diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
> index bb1e97b13b..e4d0ace44b 100644
> --- a/include/tcg/tcg.h
> +++ b/include/tcg/tcg.h
> @@ -680,6 +680,12 @@ extern __thread TCGContext *tcg_ctx;
>  extern void *tcg_code_gen_epilogue;
>  extern TCGv_env cpu_env;
>  
> +static inline bool in_code_gen_buffer(const void *p)
> +{
> +    const TCGContext *s = &tcg_init_ctx;
> +    return (size_t)(p - s->code_gen_buffer) <= s->code_gen_buffer_size;

If 'p == s->code_gen_buffer + s->code_gen_buffer_size',
is it really "in" the buffer?

> +}
> +
>  static inline size_t temp_idx(TCGTemp *ts)
>  {
>      ptrdiff_t n = ts - tcg_ctx->temps;
> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> index 4572b4901f..744f97a717 100644
> --- a/accel/tcg/translate-all.c
> +++ b/accel/tcg/translate-all.c
> @@ -392,27 +392,18 @@ void tb_destroy(TranslationBlock *tb)
>  
>  bool cpu_restore_state(CPUState *cpu, uintptr_t host_pc, bool will_exit)
>  {
> -    TranslationBlock *tb;
> -    bool r = false;
> -    uintptr_t check_offset;
> -
> -    /* The host_pc has to be in the region of current code buffer. If
> -     * it is not we will not be able to resolve it here. The two cases
> -     * where host_pc will not be correct are:
> +    /*
> +     * The host_pc has to be in the region of the code buffer.
> +     * If it is not we will not be able to resolve it here.
> +     * The two cases where host_pc will not be correct are:
>       *
>       *  - fault during translation (instruction fetch)
>       *  - fault from helper (not using GETPC() macro)
>       *
>       * Either way we need return early as we can't resolve it here.
> -     *
> -     * We are using unsigned arithmetic so if host_pc <
> -     * tcg_init_ctx.code_gen_buffer check_offset will wrap to way
> -     * above the code_gen_buffer_size
>       */
> -    check_offset = host_pc - (uintptr_t) tcg_init_ctx.code_gen_buffer;
> -
> -    if (check_offset < tcg_init_ctx.code_gen_buffer_size) {
> -        tb = tcg_tb_lookup(host_pc);
> +    if (in_code_gen_buffer((const void *)host_pc)) {
> +        TranslationBlock *tb = tcg_tb_lookup(host_pc);
>          if (tb) {
>              cpu_restore_state_from_tb(cpu, tb, host_pc, will_exit);
>              if (tb_cflags(tb) & CF_NOCACHE) {
> @@ -421,11 +412,10 @@ bool cpu_restore_state(CPUState *cpu, uintptr_t host_pc, bool will_exit)
>                  tcg_tb_remove(tb);
>                  tb_destroy(tb);
>              }
> -            r = true;
> +            return true;
>          }
>      }
> -
> -    return r;
> +    return false;
>  }
>  
>  static void page_init(void)
>
Richard Henderson Dec. 15, 2020, 10:43 p.m. UTC | #2
On 12/14/20 4:09 PM, Philippe Mathieu-Daudé wrote:
> On 12/14/20 3:02 PM, Richard Henderson wrote:
>> Create a function to determine if a pointer is within the buffer.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  include/tcg/tcg.h         |  6 ++++++
>>  accel/tcg/translate-all.c | 26 ++++++++------------------
>>  2 files changed, 14 insertions(+), 18 deletions(-)
>>
>> diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
>> index bb1e97b13b..e4d0ace44b 100644
>> --- a/include/tcg/tcg.h
>> +++ b/include/tcg/tcg.h
>> @@ -680,6 +680,12 @@ extern __thread TCGContext *tcg_ctx;
>>  extern void *tcg_code_gen_epilogue;
>>  extern TCGv_env cpu_env;
>>  
>> +static inline bool in_code_gen_buffer(const void *p)
>> +{
>> +    const TCGContext *s = &tcg_init_ctx;
>> +    return (size_t)(p - s->code_gen_buffer) <= s->code_gen_buffer_size;
> 
> If 'p == s->code_gen_buffer + s->code_gen_buffer_size',
> is it really "in" the buffer?

Well, sort of.

Compare the fact that in C, a pointer to the end of an array is valid as a
pointer even though it can't be dereferenced.  This is a pointer to the end of
the buffer.

Extra commentary required?


r~
Philippe Mathieu-Daudé Dec. 15, 2020, 11:15 p.m. UTC | #3
On 12/15/20 11:43 PM, Richard Henderson wrote:
> On 12/14/20 4:09 PM, Philippe Mathieu-Daudé wrote:
>> On 12/14/20 3:02 PM, Richard Henderson wrote:
>>> Create a function to determine if a pointer is within the buffer.
>>>
>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>> ---
>>>  include/tcg/tcg.h         |  6 ++++++
>>>  accel/tcg/translate-all.c | 26 ++++++++------------------
>>>  2 files changed, 14 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
>>> index bb1e97b13b..e4d0ace44b 100644
>>> --- a/include/tcg/tcg.h
>>> +++ b/include/tcg/tcg.h
>>> @@ -680,6 +680,12 @@ extern __thread TCGContext *tcg_ctx;
>>>  extern void *tcg_code_gen_epilogue;
>>>  extern TCGv_env cpu_env;
>>>  
>>> +static inline bool in_code_gen_buffer(const void *p)
>>> +{
>>> +    const TCGContext *s = &tcg_init_ctx;
>>> +    return (size_t)(p - s->code_gen_buffer) <= s->code_gen_buffer_size;
>>
>> If 'p == s->code_gen_buffer + s->code_gen_buffer_size',
>> is it really "in" the buffer?
> 
> Well, sort of.
> 
> Compare the fact that in C, a pointer to the end of an array is valid as a
> pointer even though it can't be dereferenced.  This is a pointer to the end of
> the buffer.
> 
> Extra commentary required?

Preferably, since you change from '<' to '<=', this would
make it clearer, then no question asked :)

With it:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Thanks,

Phil.
diff mbox series

Patch

diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
index bb1e97b13b..e4d0ace44b 100644
--- a/include/tcg/tcg.h
+++ b/include/tcg/tcg.h
@@ -680,6 +680,12 @@  extern __thread TCGContext *tcg_ctx;
 extern void *tcg_code_gen_epilogue;
 extern TCGv_env cpu_env;
 
+static inline bool in_code_gen_buffer(const void *p)
+{
+    const TCGContext *s = &tcg_init_ctx;
+    return (size_t)(p - s->code_gen_buffer) <= s->code_gen_buffer_size;
+}
+
 static inline size_t temp_idx(TCGTemp *ts)
 {
     ptrdiff_t n = ts - tcg_ctx->temps;
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 4572b4901f..744f97a717 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -392,27 +392,18 @@  void tb_destroy(TranslationBlock *tb)
 
 bool cpu_restore_state(CPUState *cpu, uintptr_t host_pc, bool will_exit)
 {
-    TranslationBlock *tb;
-    bool r = false;
-    uintptr_t check_offset;
-
-    /* The host_pc has to be in the region of current code buffer. If
-     * it is not we will not be able to resolve it here. The two cases
-     * where host_pc will not be correct are:
+    /*
+     * The host_pc has to be in the region of the code buffer.
+     * If it is not we will not be able to resolve it here.
+     * The two cases where host_pc will not be correct are:
      *
      *  - fault during translation (instruction fetch)
      *  - fault from helper (not using GETPC() macro)
      *
      * Either way we need return early as we can't resolve it here.
-     *
-     * We are using unsigned arithmetic so if host_pc <
-     * tcg_init_ctx.code_gen_buffer check_offset will wrap to way
-     * above the code_gen_buffer_size
      */
-    check_offset = host_pc - (uintptr_t) tcg_init_ctx.code_gen_buffer;
-
-    if (check_offset < tcg_init_ctx.code_gen_buffer_size) {
-        tb = tcg_tb_lookup(host_pc);
+    if (in_code_gen_buffer((const void *)host_pc)) {
+        TranslationBlock *tb = tcg_tb_lookup(host_pc);
         if (tb) {
             cpu_restore_state_from_tb(cpu, tb, host_pc, will_exit);
             if (tb_cflags(tb) & CF_NOCACHE) {
@@ -421,11 +412,10 @@  bool cpu_restore_state(CPUState *cpu, uintptr_t host_pc, bool will_exit)
                 tcg_tb_remove(tb);
                 tb_destroy(tb);
             }
-            r = true;
+            return true;
         }
     }
-
-    return r;
+    return false;
 }
 
 static void page_init(void)