Re: [Qemu-devel] [PULL 09/26] target/arm: Don't clear supported PMU events when initializing PMCEID1

[Peter Maydell]

On Mon, 28 Jan 2019 at 18:11, Peter Maydell <address@hidden> wrote:
>
> From: Aaron Lindsay OS <address@hidden>
>
> A bug was introduced during a respin of:
>
>         commit 57a4a11b2b281bb548b419ca81bfafb214e4c77a
>         target/arm: Add array for supported PMU events, generate 
> PMCEID[01]_EL0



> @@ -1113,13 +1115,16 @@ uint64_t get_pmceid(CPUARMState *env, unsigned which)
>          /* We do not currently support events in the 0x40xx range */
>          assert(cnt->number <= 0x3f);
>
> -        if ((cnt->number & 0x20) == (which << 6) &&
> -                cnt->supported(env)) {
> -            pmceid |= (1 << (cnt->number & 0x1f));
> +        if (cnt->supported(&cpu->env)) {
>              supported_event_map[cnt->number] = i;
> +            uint64_t event_mask = 1 << (cnt->number & 0x1f);

Coverity complains about this line (CID 1398645). The
RHS is evaluated using 32-bit signed arithmetic (because
cnt->number is uint16_t and so integer promotion means it
ends up working with the 'int' type. If cnt->number is
31 then this means that the assignment will do an
unintended sign-extension that sets the top 32 bits
of event_mask.

Fix is probably just to use "1ULL" instead of "1" on the LHS of <<.


> +            if (cnt->number & 0x20) {
> +                cpu->pmceid1 |= event_mask;
> +            } else {
> +                cpu->pmceid0 |= event_mask;
> +            }
>          }
>      }
> -    return pmceid;
>  }

thanks
-- PMM