Re: [Qemu-devel] [PULL v2 07/12] target/mips: Update ITU to utilize SAARI and SAAR CP0 registers

[Aleksandar Markovic]

> From: Peter Maydell <address@hidden>
> Sent: Thursday, February 14, 2019 7:40 PM
> To: Aleksandar Markovic
> Cc: QEMU Developers; Aleksandar Markovic
> Subject: Re: [PULL v2 07/12] target/mips: Update ITU to utilize SAARI and 
> SAAR CP0 registers
> 
> On Fri, 18 Jan 2019 at 16:59, Aleksandar Markovic
> <address@hidden> wrote:
> >
> > From: Yongbok Kim <address@hidden>
> >
> > Update ITU to utilize SAARI and SAAR CP0 registers.
> 
> Hi; Coverity complains (CID 1398648) about this bit of code:
> 
> > -static void itc_reconfigure(MIPSITUState *tag)
> > +void itc_reconfigure(MIPSITUState *tag)
> >  {
> >      uint64_t *am = &tag->ITCAddressMap[0];
> >      MemoryRegion *mr = &tag->storage_io;
> > @@ -92,6 +92,12 @@ static void itc_reconfigure(MIPSITUState *tag)
> >      uint64_t size = (1 * KiB) + (am[1] & ITC_AM1_ADDR_MASK_MASK);
> >      bool is_enabled = (am[0] & ITC_AM0_EN_MASK) != 0;
> >
> > +    if (tag->saar_present) {
> > +        address = ((*(uint64_t *) tag->saar) & 0xFFFFFFFFE000ULL) << 4;
> > +        size = 1 << ((*(uint64_t *) tag->saar >> 1) & 0x1f);
> > +        is_enabled = *(uint64_t *) tag->saar & 1;
> > +    }
> > +
> 
> because the "1 << ..." calculation of size is done as a 32-bit
> signed integer which may then be unintentionally sign-extended
> into the 64-bit result. Using "1ULL" instead of "1" on the LHS
> of the shift would fix this.
> 

Thanks, I'll try to integrate the fix soon.

Aleksandar

> thanks
> -- PMM