| Message ID | 20250317052339.1108322-6-npiggin@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | ppc small fixes for 10.0 | expand |
On 3/17/25 06:23, Nicholas Piggin wrote: > Rather than use the hardcoded define throughout the tree for the > PNOR LPC address, keep it within the PnvPnor object. > > This should solve a dead code issue in the BMC HIOMAP checks where > Coverity (correctly) reported that the sanity checks are dead code. > We would like to keep the sanity checks without turning them into a > compile time assert in case we would like to make them configurable > in future. > > Fixes: 4c84a0a4a6e5 ("ppc/pnv: Add a PNOR address and size sanity checks") > Resolves: Coverity CID 1593723 > Cc: Cédric Le Goater <clg@redhat.com> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> Reviewed-by: Cédric Le Goater <clg@redhat.com> Thanks, C. > --- > include/hw/ppc/pnv_pnor.h | 1 + > hw/ppc/pnv.c | 2 +- > hw/ppc/pnv_bmc.c | 4 ++-- > hw/ppc/pnv_pnor.c | 2 ++ > 4 files changed, 6 insertions(+), 3 deletions(-) > > diff --git a/include/hw/ppc/pnv_pnor.h b/include/hw/ppc/pnv_pnor.h > index 19c2d642e82..b44cafe918d 100644 > --- a/include/hw/ppc/pnv_pnor.h > +++ b/include/hw/ppc/pnv_pnor.h > @@ -28,6 +28,7 @@ struct PnvPnor { > BlockBackend *blk; > > uint8_t *storage; > + uint32_t lpc_address; /* Offset within LPC FW space */ > int64_t size; > MemoryRegion mmio; > }; > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c > index 59365370c37..63f2232f32f 100644 > --- a/hw/ppc/pnv.c > +++ b/hw/ppc/pnv.c > @@ -1191,7 +1191,7 @@ static void pnv_init(MachineState *machine) > * Since we can not reach the remote BMC machine with LPC memops, > * map it always for now. > */ > - memory_region_add_subregion(pnv->chips[0]->fw_mr, PNOR_SPI_OFFSET, > + memory_region_add_subregion(pnv->chips[0]->fw_mr, pnv->pnor->lpc_address, > &pnv->pnor->mmio); > > /* > diff --git a/hw/ppc/pnv_bmc.c b/hw/ppc/pnv_bmc.c > index 811ba3d7a49..fb70a8c1f22 100644 > --- a/hw/ppc/pnv_bmc.c > +++ b/hw/ppc/pnv_bmc.c > @@ -174,8 +174,8 @@ static void hiomap_cmd(IPMIBmcSim *ibs, uint8_t *cmd, unsigned int cmd_len, > { > PnvPnor *pnor = PNV_PNOR(object_property_get_link(OBJECT(ibs), "pnor", > &error_abort)); > + uint32_t pnor_addr = pnor->lpc_address; > uint32_t pnor_size = pnor->size; > - uint32_t pnor_addr = PNOR_SPI_OFFSET; > bool readonly = false; > > rsp_buffer_push(rsp, cmd[2]); > @@ -251,8 +251,8 @@ static const IPMINetfn hiomap_netfn = { > > void pnv_bmc_set_pnor(IPMIBmc *bmc, PnvPnor *pnor) > { > + uint32_t pnor_addr = pnor->lpc_address; > uint32_t pnor_size = pnor->size; > - uint32_t pnor_addr = PNOR_SPI_OFFSET; > > if (!pnv_bmc_is_simulator(bmc)) { > return; > diff --git a/hw/ppc/pnv_pnor.c b/hw/ppc/pnv_pnor.c > index 863e2e70aca..9db44ca21d8 100644 > --- a/hw/ppc/pnv_pnor.c > +++ b/hw/ppc/pnv_pnor.c > @@ -108,6 +108,8 @@ static void pnv_pnor_realize(DeviceState *dev, Error **errp) > memset(s->storage, 0xFF, s->size); > } > > + s->lpc_address = PNOR_SPI_OFFSET; > + > memory_region_init_io(&s->mmio, OBJECT(s), &pnv_pnor_ops, s, > TYPE_PNV_PNOR, s->size); > }
diff --git a/include/hw/ppc/pnv_pnor.h b/include/hw/ppc/pnv_pnor.h index 19c2d642e82..b44cafe918d 100644 --- a/include/hw/ppc/pnv_pnor.h +++ b/include/hw/ppc/pnv_pnor.h @@ -28,6 +28,7 @@ struct PnvPnor { BlockBackend *blk; uint8_t *storage; + uint32_t lpc_address; /* Offset within LPC FW space */ int64_t size; MemoryRegion mmio; }; diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c index 59365370c37..63f2232f32f 100644 --- a/hw/ppc/pnv.c +++ b/hw/ppc/pnv.c @@ -1191,7 +1191,7 @@ static void pnv_init(MachineState *machine) * Since we can not reach the remote BMC machine with LPC memops, * map it always for now. */ - memory_region_add_subregion(pnv->chips[0]->fw_mr, PNOR_SPI_OFFSET, + memory_region_add_subregion(pnv->chips[0]->fw_mr, pnv->pnor->lpc_address, &pnv->pnor->mmio); /* diff --git a/hw/ppc/pnv_bmc.c b/hw/ppc/pnv_bmc.c index 811ba3d7a49..fb70a8c1f22 100644 --- a/hw/ppc/pnv_bmc.c +++ b/hw/ppc/pnv_bmc.c @@ -174,8 +174,8 @@ static void hiomap_cmd(IPMIBmcSim *ibs, uint8_t *cmd, unsigned int cmd_len, { PnvPnor *pnor = PNV_PNOR(object_property_get_link(OBJECT(ibs), "pnor", &error_abort)); + uint32_t pnor_addr = pnor->lpc_address; uint32_t pnor_size = pnor->size; - uint32_t pnor_addr = PNOR_SPI_OFFSET; bool readonly = false; rsp_buffer_push(rsp, cmd[2]); @@ -251,8 +251,8 @@ static const IPMINetfn hiomap_netfn = { void pnv_bmc_set_pnor(IPMIBmc *bmc, PnvPnor *pnor) { + uint32_t pnor_addr = pnor->lpc_address; uint32_t pnor_size = pnor->size; - uint32_t pnor_addr = PNOR_SPI_OFFSET; if (!pnv_bmc_is_simulator(bmc)) { return; diff --git a/hw/ppc/pnv_pnor.c b/hw/ppc/pnv_pnor.c index 863e2e70aca..9db44ca21d8 100644 --- a/hw/ppc/pnv_pnor.c +++ b/hw/ppc/pnv_pnor.c @@ -108,6 +108,8 @@ static void pnv_pnor_realize(DeviceState *dev, Error **errp) memset(s->storage, 0xFF, s->size); } + s->lpc_address = PNOR_SPI_OFFSET; + memory_region_init_io(&s->mmio, OBJECT(s), &pnv_pnor_ops, s, TYPE_PNV_PNOR, s->size); }
Rather than use the hardcoded define throughout the tree for the PNOR LPC address, keep it within the PnvPnor object. This should solve a dead code issue in the BMC HIOMAP checks where Coverity (correctly) reported that the sanity checks are dead code. We would like to keep the sanity checks without turning them into a compile time assert in case we would like to make them configurable in future. Fixes: 4c84a0a4a6e5 ("ppc/pnv: Add a PNOR address and size sanity checks") Resolves: Coverity CID 1593723 Cc: Cédric Le Goater <clg@redhat.com> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- include/hw/ppc/pnv_pnor.h | 1 + hw/ppc/pnv.c | 2 +- hw/ppc/pnv_bmc.c | 4 ++-- hw/ppc/pnv_pnor.c | 2 ++ 4 files changed, 6 insertions(+), 3 deletions(-)