Re: [PATCH for-8.1] accel/tcg: Avoid reading too much in load_atom_{2,4}

[Philippe Mathieu-Daudé]

On 10/8/23 17:48, Richard Henderson wrote:
> When load_atom_extract_al16_or_al8 is inexpensive, we want to use
> it early, in order to avoid the overhead of required_atomicity.
> However, we must not read past the end of the page.
>
> Reported-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---


> diff --git a/accel/tcg/ldst_atomicity.c.inc b/accel/tcg/ldst_atomicity.c.inc
> index e5c590a499..5d92485a49 100644
> --- a/accel/tcg/ldst_atomicity.c.inc
> +++ b/accel/tcg/ldst_atomicity.c.inc
> @@ -404,7 +404,10 @@ static uint16_t load_atom_2(CPUArchState *env, uintptr_t 
> ra,
>           return load_atomic2(pv);
>       }
>       if (HAVE_ATOMIC128_RO) {
> -        return load_atom_extract_al16_or_al8(pv, 2);
> +        intptr_t left_in_page = pi | TARGET_PAGE_MASK;
> +        if (likely(left_in_page <= -16)) {
> +            return load_atom_extract_al16_or_al8(pv, 2);
> +        }
>       }
>   
>       atmax = required_atomicity(env, pi, memop);
> @@ -443,7 +446,10 @@ static uint32_t load_atom_4(CPUArchState *env, uintptr_t 
> ra,
>           return load_atomic4(pv);
>       }
>       if (HAVE_ATOMIC128_RO) {
> -        return load_atom_extract_al16_or_al8(pv, 4);
> +        intptr_t left_in_page = pi | TARGET_PAGE_MASK;
> +        if (likely(left_in_page <= -16)) {
> +            return load_atom_extract_al16_or_al8(pv, 4);
> +        }
>       }

Makes sense, so to the best of my knowledge:

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>