Re: [Qemu-ppc] [PULL 14/14] ppc4xx: Add device models found in PPC440 core SoCs

[BALATON Zoltan]

On Fri, 27 Apr 2018, Peter Maydell wrote:
> On 16 February 2018 at 10:06, David Gibson <address@hidden> wrote:
> > From: BALATON Zoltan <address@hidden>
> >
> > These devices are found in newer SoCs based on 440 core e.g. the 460EX
> > (http://www.embeddeddeveloper.com/assets/processors/amcc/datasheets/
> > PP460EX_DS2063.pdf)
> >
> > Signed-off-by: BALATON Zoltan <address@hidden>
> > Signed-off-by: David Gibson <address@hidden>
> > ---
>
> > +static target_ulong sdram_size(uint32_t bcr)
> > +{
> > +    target_ulong size;
> > +    int sh;
> > +
> > +    sh = 1024 - ((bcr >> 6) & 0x3ff);
> > +    if (sh == 0) {
> > +        size = -1;
> > +    } else {
> > +        size = 8 * M_BYTE * sh;
> > +    }
> > +
> > +    return size;
> > +}
>
> Hi. Coverity (CID 1390588) points out that the calculation
> "1024 - ((bcr >> 6) & 0x3ff" must result in a value of sh
> between 1 and 1024, and therefore the "sh == 0" branch of
> the if() is dead code.
>
> Is there an error in the size calculation here?

Likely this is not entirely correct (see also the FIXME comment in 
sam460ex.c:73) but I still could not obtain the user manual of the SoC 
where I think this is documented so I don't really know what's the correct 
way here and I don't have time at the moment to try to guess from 
accessible sources (such as Linux and U-Boot). So for now I'd leave it as 
it is until I can find out what would be correct here but if the warning 
from Coverity is in your way feel free to patch this to remove the sh == 0 
which should be OK if it can't ever happen.

Regards,
BALATON Zoltan