On Mon, 28 Jan 2019 at 18:11, Peter Maydell <peter.mayd...@linaro.org> wrote: > > From: Aaron Lindsay OS <aa...@os.amperecomputing.com> > > 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