On Fri, 13 Sept 2019 at 16:50, Peter Maydell <peter.mayd...@linaro.org> wrote: > > From: Cédric Le Goater <c...@kaod.org> > > When doing calibration, the SPI clock rate in the CE0 Control Register > and the read delay cycles in the Read Timing Compensation Register are > set using bit[11:4] of the DMA Control Register. > > Signed-off-by: Cédric Le Goater <c...@kaod.org> > Acked-by: Joel Stanley <j...@jms.id.au> > Reviewed-by: Peter Maydell <peter.mayd...@linaro.org> > Message-id: 20190904070506.1052-7-...@kaod.org > Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
Hi; this is an old patch, but Coverity has suddenly decided it doesn't like it (CID 1547822): > +static uint8_t aspeed_smc_hclk_divisor(uint8_t hclk_mask) > +{ > + /* HCLK/1 .. HCLK/16 */ > + const uint8_t hclk_divisors[] = { > + 15, 7, 14, 6, 13, 5, 12, 4, 11, 3, 10, 2, 9, 1, 8, 0 > + }; > + int i; > + > + for (i = 0; i < ARRAY_SIZE(hclk_divisors); i++) { > + if (hclk_mask == hclk_divisors[i]) { > + return i + 1; > + } > + } > + > + qemu_log_mask(LOG_GUEST_ERROR, "invalid HCLK mask %x", hclk_mask); > + return 0; > +} > + > +/* > + * When doing calibration, the SPI clock rate in the CE0 Control > + * Register and the read delay cycles in the Read Timing Compensation > + * Register are set using bit[11:4] of the DMA Control Register. > + */ > +static void aspeed_smc_dma_calibration(AspeedSMCState *s) > +{ > + uint8_t delay = > + (s->regs[R_DMA_CTRL] >> DMA_CTRL_DELAY_SHIFT) & DMA_CTRL_DELAY_MASK; > + uint8_t hclk_mask = > + (s->regs[R_DMA_CTRL] >> DMA_CTRL_FREQ_SHIFT) & DMA_CTRL_FREQ_MASK; > + uint8_t hclk_div = aspeed_smc_hclk_divisor(hclk_mask); > + uint32_t hclk_shift = (hclk_div - 1) << 2; The code of aspeeed_smc_hclk_divisor() has a codepath where it can return 0, and this callsite doesn't check for 0, and so Coverity thinks that we might end up shifting -1 by 2 to get the hclk_shift here, which means we overflow the value, which it thinks is probably not what we meant to do. In fact this can't happen, because we always pass aspeed_smc_hclk_divisor() a value between 0 and 15, and if we do that then we always get back a value between 1 and 16. So I think the right fix would be to change the qemu_log_mask()/return 0 to be g_assert_not_reached(). thanks -- PMM