Re: [PATCH 1/7] ppc/pnv: Fix LPC serirq routing calculation

["Cédric Le Goater" <clg@redhat.com>]

On 8/6/24 15:13, Nicholas Piggin wrote:
> The serirq routing table is split over two registers, the calculation
> for the high irqs in the second register did not subtract the irq
> offset. This was spotted by Coverity as a shift-by-negative. Fix this
> and change the open-coded shifting and masking to use extract32()
> function so it's less error-prone.
> 
> This went unnoticed because irqs >= 14 are not used in a standard
> QEMU/OPAL boot, changing the first QEMU serial-isa irq to 14 to test
> does demonstrate serial irqs aren't received, and that this change
> fixes that.
> 
> Reported-by: Cédric Le Goater <clg@redhat.com>
> Resolves: Coverity CID 1558829 (partially)
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> ---
>   target/ppc/cpu.h |  1 +
>   hw/ppc/pnv_lpc.c | 10 ++++++++--
>   2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 321ed2da75..bd32a1a5f8 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -40,6 +40,7 @@
>   
>   #define PPC_BIT_NR(bit)         (63 - (bit))
>   #define PPC_BIT(bit)            (0x8000000000000000ULL >> (bit))
> +#define PPC_BIT32_NR(bit)       (31 - (bit))
>   #define PPC_BIT32(bit)          (0x80000000 >> (bit))
>   #define PPC_BIT8(bit)           (0x80 >> (bit))
>   #define PPC_BITMASK(bs, be)     ((PPC_BIT(bs) - PPC_BIT(be)) | PPC_BIT(bs))
> diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
> index f8aad955b5..80b79dfbbc 100644
> --- a/hw/ppc/pnv_lpc.c
> +++ b/hw/ppc/pnv_lpc.c
> @@ -435,13 +435,19 @@ static void pnv_lpc_eval_serirq_routes(PnvLpcController *lpc)
>           return;
>       }
>   
> +    /*
> +     * Each of the ISA irqs is routed to one of the 4 SERIRQ irqs with 2
> +     * bits, split across 2 OPB registers.
> +     */
>       for (irq = 0; irq <= 13; irq++) {
> -        int serirq = (lpc->opb_irq_route1 >> (31 - 5 - (irq * 2))) & 0x3;
> +        int serirq = extract32(lpc->opb_irq_route1,
> +                                    PPC_BIT32_NR(5 + irq * 2), 2);
>           lpc->irq_to_serirq_route[irq] = serirq;
>       }
>   
>       for (irq = 14; irq < ISA_NUM_IRQS; irq++) {
> -        int serirq = (lpc->opb_irq_route0 >> (31 - 9 - (irq * 2))) & 0x3;
> +        int serirq = extract32(lpc->opb_irq_route0,
> +                               PPC_BIT32_NR(9 + (irq - 14) * 2), 2);
>           lpc->irq_to_serirq_route[irq] = serirq;
>       }
>   }