| Message ID | 155059667853.1466090.16527852453054217565.stgit@bahia.lab.toulouse-stg.fr.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | spapr: Add support for PHB hotplug | expand |
On Tue, Feb 19, 2019 at 06:17:58PM +0100, Greg Kurz wrote: > All DRC subtypes have been converted to generate the FDT fragment at > configure connector time instead of attach time. The fdt and fdt_offset > arguments of spapr_drc_attach() aren't needed anymore. Drop them and > make the implementation of the dt_populate() method mandatory. > > Signed-off-by: Greg Kurz <groug@kaod.org> I've applied the first 5 patches to ppc-for-4.0, but as a followup... [...] > @@ -1113,8 +1104,6 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu, > > drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); > > - g_assert(drc->fdt || drck->dt_populate); > - > if (!drc->fdt) { ..you can now remove this conditional, since it will always be true.
On Wed, 20 Feb 2019 14:22:19 +1100 David Gibson <david@gibson.dropbear.id.au> wrote: > On Tue, Feb 19, 2019 at 06:17:58PM +0100, Greg Kurz wrote: > > All DRC subtypes have been converted to generate the FDT fragment at > > configure connector time instead of attach time. The fdt and fdt_offset > > arguments of spapr_drc_attach() aren't needed anymore. Drop them and > > make the implementation of the dt_populate() method mandatory. > > > > Signed-off-by: Greg Kurz <groug@kaod.org> > > I've applied the first 5 patches to ppc-for-4.0, but as a followup... > > [...] > > @@ -1113,8 +1104,6 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu, > > > > drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); > > > > - g_assert(drc->fdt || drck->dt_populate); > > - > > if (!drc->fdt) { > > ..you can now remove this conditional, since it will always be true. > Hmm... I'm afraid this is not true since configure-connector is supposed to be called several times according to PAPR. And this is exactly what the code does: first return the node name to the guest, then all properties and subnodes one at a time...
On Wed, Feb 20, 2019 at 10:01:23AM +0100, Greg Kurz wrote: > On Wed, 20 Feb 2019 14:22:19 +1100 > David Gibson <david@gibson.dropbear.id.au> wrote: > > > On Tue, Feb 19, 2019 at 06:17:58PM +0100, Greg Kurz wrote: > > > All DRC subtypes have been converted to generate the FDT fragment at > > > configure connector time instead of attach time. The fdt and fdt_offset > > > arguments of spapr_drc_attach() aren't needed anymore. Drop them and > > > make the implementation of the dt_populate() method mandatory. > > > > > > Signed-off-by: Greg Kurz <groug@kaod.org> > > > > I've applied the first 5 patches to ppc-for-4.0, but as a followup... > > > > [...] > > > @@ -1113,8 +1104,6 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu, > > > > > > drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); > > > > > > - g_assert(drc->fdt || drck->dt_populate); > > > - > > > if (!drc->fdt) { > > > > ..you can now remove this conditional, since it will always be true. > > > > Hmm... I'm afraid this is not true since configure-connector is supposed > to be called several times according to PAPR. And this is exactly what > the code does: first return the node name to the guest, then all properties > and subnodes one at a time... Oh, duh, of course. Forget I said that.
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 6cf7a9f5c1f2..9364d07364ac 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -3362,7 +3362,7 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size, addr / SPAPR_MEMORY_BLOCK_SIZE); g_assert(drc); - spapr_drc_attach(drc, dev, NULL, 0, &local_err); + spapr_drc_attach(drc, dev, &local_err); if (local_err) { while (addr > addr_start) { addr -= SPAPR_MEMORY_BLOCK_SIZE; @@ -3744,7 +3744,7 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev, g_assert(drc || !mc->has_hotpluggable_cpus); if (drc) { - spapr_drc_attach(drc, dev, NULL, 0, &local_err); + spapr_drc_attach(drc, dev, &local_err); if (local_err) { error_propagate(errp, local_err); return; diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c index 248eb8a93d6d..87ca7d973564 100644 --- a/hw/ppc/spapr_drc.c +++ b/hw/ppc/spapr_drc.c @@ -374,11 +374,8 @@ static void prop_get_fdt(Object *obj, Visitor *v, const char *name, } while (fdt_depth != 0); } -void spapr_drc_attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt, - int fdt_start_offset, Error **errp) +void spapr_drc_attach(sPAPRDRConnector *drc, DeviceState *d, Error **errp) { - sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); - trace_spapr_drc_attach(spapr_drc_index(drc)); if (drc->dev) { @@ -387,15 +384,9 @@ void spapr_drc_attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt, } g_assert((drc->state == SPAPR_DRC_STATE_LOGICAL_UNUSABLE) || (drc->state == SPAPR_DRC_STATE_PHYSICAL_POWERON)); - g_assert(fdt || drck->dt_populate); drc->dev = d; - if (fdt) { - drc->fdt = fdt; - drc->fdt_start_offset = fdt_start_offset; - } - object_property_add_link(OBJECT(drc), "device", object_get_typename(OBJECT(drc->dev)), (Object **)(&drc->dev), @@ -1113,8 +1104,6 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu, drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); - g_assert(drc->fdt || drck->dt_populate); - if (!drc->fdt) { Error *local_err = NULL; void *fdt; diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index b22c9f57b2dd..e2bc9fec824b 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -1457,7 +1457,7 @@ static void spapr_pci_plug(HotplugHandler *plug_handler, goto out; } - spapr_drc_attach(drc, DEVICE(pdev), NULL, 0, &local_err); + spapr_drc_attach(drc, DEVICE(pdev), &local_err); if (local_err) { goto out; } diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h index 2aa919f0cfe5..f32758ec8487 100644 --- a/include/hw/ppc/spapr_drc.h +++ b/include/hw/ppc/spapr_drc.h @@ -261,8 +261,7 @@ sPAPRDRConnector *spapr_drc_by_id(const char *type, uint32_t id); int spapr_drc_populate_dt(void *fdt, int fdt_offset, Object *owner, uint32_t drc_type_mask); -void spapr_drc_attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt, - int fdt_start_offset, Error **errp); +void spapr_drc_attach(sPAPRDRConnector *drc, DeviceState *d, Error **errp); void spapr_drc_detach(sPAPRDRConnector *drc); bool spapr_drc_needed(void *opaque);
All DRC subtypes have been converted to generate the FDT fragment at configure connector time instead of attach time. The fdt and fdt_offset arguments of spapr_drc_attach() aren't needed anymore. Drop them and make the implementation of the dt_populate() method mandatory. Signed-off-by: Greg Kurz <groug@kaod.org> --- hw/ppc/spapr.c | 4 ++-- hw/ppc/spapr_drc.c | 13 +------------ hw/ppc/spapr_pci.c | 2 +- include/hw/ppc/spapr_drc.h | 3 +-- 4 files changed, 5 insertions(+), 17 deletions(-)