On 7 November 2017 at 16:52, Alex Bennée <alex.ben...@linaro.org> wrote:
> We are still seeing signals during translation time when we walk over
> a page protection boundary. This expands the check to ensure the
> retaddr is inside the code generation buffer. The original suggestion
> was to check versus tcg_ctx.code_gen_ptr but as we now segment the
> translation buffer we have to settle for just a general check for
> being inside.
>
> Signed-off-by: Alex Bennée <alex.ben...@linaro.org>
> Reported-by: Peter Maydell <peter.mayd...@linaro.org>
> Suggested-by: Paolo Bonzini <pbonz...@redhat.com>
> Cc: Richard Henderson <r...@twiddle.net>
> ---
> accel/tcg/translate-all.c | 20 ++++++++++++--------
> 1 file changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> index 34c5e28d07..eb255af402 100644
> --- a/accel/tcg/translate-all.c
> +++ b/accel/tcg/translate-all.c
> @@ -357,16 +357,20 @@ bool cpu_restore_state(CPUState *cpu, uintptr_t retaddr)
> TranslationBlock *tb;
> bool r = false;
>
> - /* A retaddr of zero is invalid so we really shouldn't have ended
> - * up here. The target code has likely forgotten to check retaddr
> - * != 0 before attempting to restore state. We return early to
> - * avoid blowing up on a recursive tb_lock(). The target must have
> - * previously survived a failed cpu_restore_state because
> - * tb_find_pc(0) would have failed anyway. It still should be
> - * fixed though.
> + /* The retaddr has to be in the region of current code buffer. If
> + * it's not we will not be able to resolve it here. If it is zero
> + * the calling code has likely forgotten to check retaddr before
> + * calling here.
This part of the comment isn't correct -- it's entirely expected
that we will get here with a zero retaddr, because that is
how the rest of the code tells this function "no state restoration
required".
> If it is not in the translated code we could be
> + * faulting during translation itself.
> + *
> + * Either way we need return early to avoid blowing up on a
> + * recursive tb_lock() as we can't resolve it here.
> */
>
> - if (!retaddr) {
> + if (!retaddr ||
> + (retaddr < (uintptr_t) tcg_init_ctx.code_gen_buffer) ||
> + (retaddr > (uintptr_t) (tcg_init_ctx.code_gen_buffer +
> + tcg_init_ctx.code_gen_buffer_size))) {
> return r;
> }
thanks
-- PMM
