On Thu, 2022-03-31 at 18:51 +0100, Peter Maydell wrote: > > Hi; Coverity has just spotted an error in this old change > (CID 1487176):
Oh my this is old ... I don't work for IBM anymore but I found the relevant doc here: https://wiki.raptorcs.com/w/images/a/a5/POWER9_PCIe_controller_v11_27JUL2018_pub.pdf So.... > > +++ b/hw/pci-host/pnv_phb4_pec.c > > +static void pnv_pec_pci_xscom_write(void *opaque, hwaddr addr, > > + uint64_t val, unsigned size) > > +{ > > + PnvPhb4PecState *pec = PNV_PHB4_PEC(opaque); > > + uint32_t reg = addr >> 3; > > + > > + switch (reg) { > > + case PEC_PCI_PBAIB_HW_CONFIG: > > + case PEC_PCI_PBAIB_READ_STK_OVR: > > + pec->pci_regs[reg] = val; > > This write function switches on 'reg' and is written assuming > that these PEC_PCI* constants are valid array indexes... They should be but... > > + break; > > + default: > > + phb_pec_error(pec, "%s @0x%"HWADDR_PRIx"=%"PRIx64"\n", > > __func__, > > + addr, val); > > + } > > +} > > +++ b/include/hw/pci-host/pnv_phb4.h > > +struct PnvPhb4PecStatimages/images/e { > > + DeviceState parent; > > + > > + /* PEC number in chip */ > > + uint32_t index; > > + uint32_t chip_id;images/ > > + > > + MemoryRegion *system_memory; > > + > > + /* Nest registers, excuding per-stack */ > > +#define PHB4_PEC_NEST_REGS_COUNT 0xf > > + uint64_t nest_regs[PHB4_PEC_NEST_REGS_COUNT]; > > + MemoryRegion nest_regs_mr; > > + > > + /* PCI registers, excluding per-stack */ > > +#define PHB4_PEC_PCI_REGS_COUNT 0x2 > > + uint64_t pci_regs[PHB4_PEC_PCI_REGS_COUNT]; > > + MemoryRegion pci_regs_mr; > > ...but we define the pci_regs[] array in PnvPhb4PecState to > have only 2 elements... > > > +++ b/include/hw/pci-host/pnv_phb4_regs.h > > +/* XSCOM PCI global registers */ > > +#define PEC_PCI_PBAIB_HW_CONFIG 0x00 > > +#define PEC_PCI_PBAIB_READ_STK_OVR 0x02 > > ...and here we define PEC_PCI_PBAIB_READ_STK_OVR as 2, which makes > it not a valid index into pci_regs[]. > > > Which of these is wrong? This one: #define PHB4_PEC_PCI_REGS_COUNT 0x2 Should be #define PHB4_PEC_PCI_REGS_COUNT 0x3 There is no register at 0x1 though. Cheers, Ben.