On Thu, 2026-07-09 at 11:48 +0100, Peter Maydell wrote:
> In the STM32 RCC, there is a block of 5 "enable" registers, each of
> which has 32 bits; each bit determines the level of one of the 5 * 32
> = 160 enable_irq output lines.  The code calculates the irq to be
> worked on using
>   irq_offset = ((addr - STM32_RCC_AHB1_ENR) / 4) * 32;
> 
> This assumes that the registers are all consecutive; however, there
> is a gap between the AHB1/2/3 registers and the APB1/2 registers, so
> for the APB1/2 registers we calculate a number that is 32 too high
> and can index off the end of the enable_irq[] array.
> 
> The handling of the reset registers has an identical bug.
> 
> Adjust the calculation of irq_offset to cope with the gap, and fix
> the case labels so accesses to the gap fall into the default
> LOG_UNIMP rather than being treated as if they were an actual
> register.
> 
> Coverity CID: 1663683, 1663686
> Cc: [email protected]
> Signed-off-by: Peter Maydell <[email protected]>

Reviewed-by: Alistair Francis <[email protected]>

Alistair

> ---
>  hw/misc/stm32_rcc.c         | 31 +++++++++++++++++++++++++++----
>  include/hw/misc/stm32_rcc.h |  6 +++++-
>  2 files changed, 32 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/misc/stm32_rcc.c b/hw/misc/stm32_rcc.c
> index 74ea29b156..5cfb39e560 100644
> --- a/hw/misc/stm32_rcc.c
> +++ b/hw/misc/stm32_rcc.c
> @@ -53,6 +53,27 @@ static uint64_t stm32_rcc_read(void *opaque,
> hwaddr addr, unsigned int size)
>      return value;
>  }
>  
> +static int reg_offset_to_irq_offset(hwaddr addr)
> +{
> +    /*
> +     * The reset and enable registers aren't all consecutive. In
> getting the
> +     * irq index from the register offset, we need to account for
> the gap
> +     * between the AHB regs and the APB regs.
> +     */
> +    switch (addr) {
> +    case STM32_RCC_AHB1_RSTR ... STM32_RCC_AHB3_RSTR:
> +        return ((addr - STM32_RCC_AHB1_RSTR) / 4) * 32;
> +    case STM32_RCC_APB1_RSTR ... STM32_RCC_APB2_RSTR:
> +        return ((addr - STM32_RCC_APB1_RSTR) / 4) * 32 +
> STM32_RCC_N_AHB_IRQS;
> +    case STM32_RCC_AHB1_ENR ... STM32_RCC_AHB3_ENR:
> +        return ((addr - STM32_RCC_AHB1_ENR) / 4) * 32;
> +    case STM32_RCC_APB1_ENR ... STM32_RCC_APB2_ENR:
> +        return ((addr - STM32_RCC_APB1_ENR) / 4) * 32 +
> STM32_RCC_N_AHB_IRQS;
> +    default:
> +        g_assert_not_reached();
> +    }
> +}
> +
>  static void stm32_rcc_write(void *opaque, hwaddr addr,
>                              uint64_t val64, unsigned int size)
>  {
> @@ -69,11 +90,12 @@ static void stm32_rcc_write(void *opaque, hwaddr
> addr,
>      }
>  
>      switch (addr) {
> -    case STM32_RCC_AHB1_RSTR...STM32_RCC_APB2_RSTR:
> +    case STM32_RCC_AHB1_RSTR ... STM32_RCC_AHB3_RSTR:
> +    case STM32_RCC_APB1_RSTR ... STM32_RCC_APB2_RSTR:
>          prev_value = s->regs[addr / 4];
>          s->regs[addr / 4] = value;
>  
> -        irq_offset = ((addr - STM32_RCC_AHB1_RSTR) / 4) * 32;
> +        irq_offset = reg_offset_to_irq_offset(addr);
>          for (int i = 0; i < 32; i++) {
>              new_value = extract32(value, i, 1);
>              if (extract32(prev_value, i, 1) && !new_value) {
> @@ -82,11 +104,12 @@ static void stm32_rcc_write(void *opaque, hwaddr
> addr,
>              }
>          }
>          return;
> -    case STM32_RCC_AHB1_ENR...STM32_RCC_APB2_ENR:
> +    case STM32_RCC_AHB1_ENR ... STM32_RCC_AHB3_ENR:
> +    case STM32_RCC_APB1_ENR ... STM32_RCC_APB2_ENR:
>          prev_value = s->regs[addr / 4];
>          s->regs[addr / 4] = value;
>  
> -        irq_offset = ((addr - STM32_RCC_AHB1_ENR) / 4) * 32;
> +        irq_offset = reg_offset_to_irq_offset(addr);
>          for (int i = 0; i < 32; i++) {
>              new_value = extract32(value, i, 1);
>              if (!extract32(prev_value, i, 1) && new_value) {
> diff --git a/include/hw/misc/stm32_rcc.h
> b/include/hw/misc/stm32_rcc.h
> index 4dccacc2db..a94781bf34 100644
> --- a/include/hw/misc/stm32_rcc.h
> +++ b/include/hw/misc/stm32_rcc.h
> @@ -65,7 +65,11 @@
>  
>  #define STM32_RCC_NREGS ((STM32_RCC_DCKCFGR2 >> 2) + 1)
>  #define STM32_RCC_PERIPHERAL_SIZE 0x400
> -#define STM32_RCC_NIRQS (32 * 5) /* 32 bits per reg, 5 en/rst regs
> */
> +
> +/* 32 bits per reg, 3 AHB regs and 2 APB regs */
> +#define STM32_RCC_N_AHB_IRQS (32 * 3)
> +#define STM32_RCC_N_APB_IRQS (32 * 2)
> +#define STM32_RCC_NIRQS (STM32_RCC_N_AHB_IRQS +
> STM32_RCC_N_APB_IRQS)
>  
>  #define STM32_RCC_GPIO_IRQ_OFFSET 0
>  

Reply via email to