[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PULL 18/21] hw/misc: Add a PWM module for NPCM7XX
From: |
Peter Maydell |
Subject: |
Re: [PULL 18/21] hw/misc: Add a PWM module for NPCM7XX |
Date: |
Wed, 13 Jan 2021 16:02:48 +0000 |
On Tue, 12 Jan 2021 at 16:58, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> From: Hao Wu <wuhaotsh@google.com>
>
> The PWM module is part of NPCM7XX module. Each NPCM7XX module has two
> identical PWM modules. Each module contains 4 PWM entries. Each PWM has
> two outputs: frequency and duty_cycle. Both are computed using inputs
> from software side.
Hi; Coverity reports a possibly-overflowing arithmetic operation here
(CID 1442342):
> +static uint32_t npcm7xx_pwm_calculate_duty(NPCM7xxPWM *p)
> +{
> + uint64_t duty;
> +
> + if (p->running) {
> + if (p->cnr == 0) {
> + duty = 0;
> + } else if (p->cmr >= p->cnr) {
> + duty = NPCM7XX_PWM_MAX_DUTY;
> + } else {
> + duty = NPCM7XX_PWM_MAX_DUTY * (p->cmr + 1) / (p->cnr + 1);
Here all of p->cmr, p->cnr and NPCM7XX_PWM_MAX_DUTY are 32-bits,
so we calculate the whole expression using 32-bit arithmetic
before assigning it to a 64-bit variable. This could be
fixed using eg a cast of NPCM7XX_PWM_MAX_DUTY to uint64_t.
Incidentally, we don't actually do any 64-bit
arithmetic calculations on 'duty' and we return
a uint32_t from this function, so 'duty' itself could
be a uint32_t, I think...
> + }
> + } else {
> + duty = 0;
> + }
> +
> + if (p->inverted) {
> + duty = NPCM7XX_PWM_MAX_DUTY - duty;
> + }
> +
> + return duty;
> +}
thanks
-- PMM
- [PULL 08/21] target/arm: add aarch32 ID register fields to cpu.h, (continued)
- [PULL 08/21] target/arm: add aarch32 ID register fields to cpu.h, Peter Maydell, 2021/01/12
- [PULL 10/21] docs: Add qemu-storage-daemon(1) manpage to meson.build, Peter Maydell, 2021/01/12
- [PULL 12/21] target/arm: Don't decode insns in the XScale/iWMMXt space as cp insns, Peter Maydell, 2021/01/12
- [PULL 13/21] hw/net/lan9118: Fix RX Status FIFO PEEK value, Peter Maydell, 2021/01/12
- [PULL 11/21] docs: Build and install all the docs in a single manual, Peter Maydell, 2021/01/12
- [PULL 09/21] ui/cocoa: Update path to docs in build tree, Peter Maydell, 2021/01/12
- [PULL 14/21] hw/net/lan9118: Add symbolic constants for register offsets, Peter Maydell, 2021/01/12
- [PULL 16/21] hw/timer: Refactor NPCM7XX Timer to use CLK clock, Peter Maydell, 2021/01/12
- [PULL 15/21] hw/misc: Add clock converter in NPCM7XX CLK module, Peter Maydell, 2021/01/12
- [PULL 18/21] hw/misc: Add a PWM module for NPCM7XX, Peter Maydell, 2021/01/12
- Re: [PULL 18/21] hw/misc: Add a PWM module for NPCM7XX,
Peter Maydell <=
- [PULL 17/21] hw/adc: Add an ADC module for NPCM7XX, Peter Maydell, 2021/01/12
[PULL 19/21] hw/misc: Add QTest for NPCM7XX PWM Module, Peter Maydell, 2021/01/12
[PULL 20/21] hw/*: Use type casting for SysBusDevice in NPCM7XX, Peter Maydell, 2021/01/12
[PULL 21/21] ui/cocoa: Fix openFile: deprecation on Big Sur, Peter Maydell, 2021/01/12