Re: [PATCH v7 18/42] target/arm: Simplify DC_ZVA

[Peter Maydell]

On Wed, 3 Jun 2020 at 02:13, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Now that we know that the operation is on a single page,
> we need not loop over pages while probing.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/helper-a64.c | 94 +++++++++++------------------------------
>  1 file changed, 25 insertions(+), 69 deletions(-)
>
> diff --git a/target/arm/helper-a64.c b/target/arm/helper-a64.c
> index bc0649a44a..60a04dc880 100644
> --- a/target/arm/helper-a64.c
> +++ b/target/arm/helper-a64.c
> @@ -1119,85 +1119,41 @@ void HELPER(dc_zva)(CPUARMState *env, uint64_t 
> vaddr_in)
>       * (which matches the usual QEMU behaviour of not implementing either
>       * alignment faults or any memory attribute handling).
>       */
> +    int blocklen = 4 << env_archcpu(env)->dcz_blocksize;
> +    uint64_t vaddr = vaddr_in & -blocklen;

This seems unnecessarily confusing compared to keeping the old
> -    uint64_t vaddr = vaddr_in & ~(blocklen - 1);

I generally find mixing of arithmetic negation with bitwise ops
hard to read because I have to remember what the binary operation
that is 2s-complement-negation actually is.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM