On Wed, Jul 12, 2017 at 01:56:37PM +0530, Bharata B Rao wrote:
> On Tue, Jul 11, 2017 at 02:39:09PM +1000, David Gibson wrote:
> > spapr_drc_attach() has a 'coldplug' parameter which sets the DRC into
> > configured state initially, instead of the usual ISOLATED/UNUSABLE state.
> > It turns out this is unnecessary: although coldplugged devices do need to
> > be in CONFIGURED state once the guest starts, that will already be
> > accomplished by the reset code which will move DRCs for already plugged
> > devices into a coldplug equivalent state.
> >
> > Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
> > Reviewed-by: Laurent Vivier <lviv...@redhat.com>
> > Reviewed-by: Greg Kurz <gr...@kaod.org>
> > ---
> > hw/ppc/spapr.c | 13 +++----------
> > hw/ppc/spapr_drc.c | 5 ++---
> > hw/ppc/spapr_pci.c | 3 +--
> > include/hw/ppc/spapr_drc.h | 2 +-
> > 4 files changed, 7 insertions(+), 16 deletions(-)
> >
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 4fa982d..70b3fd3 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -2611,7 +2611,7 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t
> > addr_start, uint64_t size,
> > fdt_offset = spapr_populate_memory_node(fdt, node, addr,
> > SPAPR_MEMORY_BLOCK_SIZE);
> >
> > - spapr_drc_attach(drc, dev, fdt, fdt_offset, !dev->hotplugged,
> > errp);
> > + spapr_drc_attach(drc, dev, fdt, fdt_offset, errp);
> > addr += SPAPR_MEMORY_BLOCK_SIZE;
> > }
> > /* send hotplug notification to the
> > @@ -2956,17 +2956,10 @@ static void spapr_core_plug(HotplugHandler
> > *hotplug_dev, DeviceState *dev,
> >
> > g_assert(drc || !mc->has_hotpluggable_cpus);
> >
> > - /*
> > - * Setup CPU DT entries only for hotplugged CPUs. For boot time or
> > - * coldplugged CPUs DT entries are setup in spapr_build_fdt().
> > - */
> > - if (dev->hotplugged) {
> > - fdt = spapr_populate_hotplug_cpu_dt(cs, &fdt_offset, spapr);
> > - }
> > + fdt = spapr_populate_hotplug_cpu_dt(cs, &fdt_offset, spapr);
>
> A harmless (but still good to get rid of) side effect of this change is
> that we are now building the CPU device tree for boot time and cold-plugged
> CPUs twice:
>
> first from spapr_core_plug() -> spapr_populate_hotplug_cpu_dt()
> second from spapr_build_fdt() -> spapr_populate_cpus_dt_node()
>
> The first one is not used while the 2nd one acutally ends up building
> the CPUs DT entries for the boot time and cold-plugged CPUs.
Yeah, I know. I think simpler code is more important than the handful
of wasted cycles. We might be able to avoid this with some of the DT
generation cleanups I have on the long term radar.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
