| Message ID | 20240806131318.275109-2-npiggin@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | various ppc fixes | expand |
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; > } > }
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; > } > }
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; } }
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> --- target/ppc/cpu.h | 1 + hw/ppc/pnv_lpc.c | 10 ++++++++-- 2 files changed, 9 insertions(+), 2 deletions(-)