On Fri, 24 Aug 2018 17:30:12 +0200 Cédric Le Goater <c...@kaod.org> wrote:
> On 08/24/2018 05:09 PM, Peter Maydell wrote: > > On 21 August 2018 at 05:33, David Gibson <da...@gibson.dropbear.id.au> > > wrote: > >> From: Cédric Le Goater <c...@kaod.org> > >> > >> It should save us some CPU cycles as these routines perform a lot of > >> checks. > >> > >> Signed-off-by: Cédric Le Goater <c...@kaod.org> > >> Signed-off-by: David Gibson <da...@gibson.dropbear.id.au> > >> --- > >> hw/ppc/spapr_pci.c | 11 ++++++----- > >> 1 file changed, 6 insertions(+), 5 deletions(-) > > > > Hi; Coverity points out in CID 1395183 that there's a bug in > > this part of this patch: > > > >> @@ -1558,6 +1559,7 @@ static void spapr_phb_realize(DeviceState *dev, > >> Error **errp) > >> sPAPRMachineState *spapr = > >> (sPAPRMachineState *) object_dynamic_cast(qdev_get_machine(), > >> TYPE_SPAPR_MACHINE); > >> + sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr); > > > > This has moved the call to SPAPR_MACHINE_GET_CLASS() above > > the check for "is spapr NULL", which is wrong, because it > > will unconditionally dereference the pointer you pass to it. > I've sent a trivial patch to fix this. > I see. This is a simple fix but the root reason for this check is > commit f7d6bfcdc0fe ("spapr_pci: fail gracefully with non-pseries > machine types"). > Yeah, we also have one in spapr_cpu_core_realize() for the very same reason. > Is there a way to specify which device type can or can not be > plugged on a machine ? > > I suppose we cannot use : > > machine_class_allow_dynamic_sysbus_dev() > > for cold plugged devices. Or can we ? That would be better. > Hmm... not sure this would help. The root problem is that many places in spapr_pci and spapr_cpu_core assume the machine is sPAPR. > Thanks, > > C.