Re: [Qemu-devel] [for-2.11 PATCH 18/26] spapr: create DR connectors for PHBs

[Greg Kurz]

On Fri, 28 Jul 2017 13:49:25 +1000
David Gibson <address@hidden> wrote:

> On Tue, Jul 25, 2017 at 08:01:50PM +0200, Greg Kurz wrote:
> > From: Michael Roth <address@hidden>
> > 
> > Signed-off-by: Michael Roth <address@hidden>
> > Reviewed-by: David Gibson <address@hidden>
> > Signed-off-by: Greg Kurz <address@hidden>  
> 
> 
> 
> > ---
> > Changes since RFC:
> > - rebased against ppc-for-2.10 (reset hooks registering already merged)
> > - added new DRC type for PHB
> > ---
> >  hw/ppc/spapr.c             |   15 +++++++++++++++
> >  hw/ppc/spapr_drc.c         |   17 +++++++++++++++++
> >  include/hw/ppc/spapr_drc.h |    8 ++++++++
> >  3 files changed, 40 insertions(+)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 8dc505343c0f..5950c009ab7e 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -98,6 +98,9 @@
> >  
> >  #define PHANDLE_XICP            0x00001111
> >  
> > +/* maximum number of hotpluggable PHBs */
> > +#define SPAPR_DRC_MAX_PHB       256  
> 
> I wonder if we should actually make this a machine property.
> 

It makes sense.

Also, if all PHBs are instanciated with index != -1, we're limited to 31.
Maybe this could be the default value for the machine property instead of
256 then ?

> >  static ICSState *spapr_ics_create(sPAPRMachineState *spapr,
> >                                    const char *type_ics,
> >                                    int nr_irqs, Error **errp)
> > @@ -2384,6 +2387,18 @@ static void ppc_spapr_init(MachineState *machine)
> >  
> >      spapr->dr_phb_enabled = smc->dr_phb_enabled;
> >  
> > +    /* Setup hotplug / dynamic-reconfiguration connectors. top-level
> > +     * connectors (described in root DT node's "ibm,drc-types" property)
> > +     * are pre-initialized here. additional child connectors (such as
> > +     * connectors for a PHBs PCI slots) are added as needed during their
> > +     * parent's realization.
> > +     */
> > +    if (spapr->dr_phb_enabled) {
> > +        for (i = 0; i < SPAPR_DRC_MAX_PHB; i++) {
> > +            spapr_dr_connector_new(OBJECT(machine), TYPE_SPAPR_DRC_PHB, i);
> > +        }
> > +    }
> > +
> >      /* Set up PCI */
> >      spapr_pci_rtas_init();
> >  
> > diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> > index eb8024d37c54..2e1049ce61c7 100644
> > --- a/hw/ppc/spapr_drc.c
> > +++ b/hw/ppc/spapr_drc.c
> > @@ -697,6 +697,15 @@ static void spapr_drc_lmb_class_init(ObjectClass *k, 
> > void *data)
> >      drck->release = spapr_lmb_release;
> >  }
> >  
> > +static void spapr_drc_phb_class_init(ObjectClass *k, void *data)
> > +{
> > +    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_CLASS(k);
> > +
> > +    drck->typeshift = SPAPR_DR_CONNECTOR_TYPE_SHIFT_PHB;
> > +    drck->typename = "PHB";
> > +    drck->drc_name_prefix = "PHB ";
> > +}
> > +
> >  static const TypeInfo spapr_dr_connector_info = {
> >      .name          = TYPE_SPAPR_DR_CONNECTOR,
> >      .parent        = TYPE_DEVICE,
> > @@ -740,6 +749,13 @@ static const TypeInfo spapr_drc_lmb_info = {
> >      .class_init    = spapr_drc_lmb_class_init,
> >  };
> >  
> > +static const TypeInfo spapr_drc_phb_info = {
> > +    .name          = TYPE_SPAPR_DRC_PHB,
> > +    .parent        = TYPE_SPAPR_DRC_LOGICAL,  
> 
> I thought PHB DRCs were physical..
> 

My understanding is that only PCI IOAs need a physical DRC.

From LoPAPR v1.1 (March 24, 2016):

13.7 Logical Resource Dynamic Reconfiguration (LRDR)

The Logical Resource Dynamic Reconfiguration option allows a platform to make 
available and recover platform re-
sources such as CPUs, Memory Regions, Processor Host Bridges, and I/O slots 
to/from its operating OS image(s).

...

The device tree contains logical resource DR connectors for the maximum number 
of resources that the platform can
allocate to the specific OS. In some cases such as for processors and PHBs...

and

Table 240. Currently Defined DR Connector Types

| PHB | Logical PCI Host Bridge |

> > +    .instance_size = sizeof(sPAPRDRConnector),
> > +    .class_init    = spapr_drc_phb_class_init,
> > +};
> > +
> >  /* helper functions for external users */
> >  
> >  sPAPRDRConnector *spapr_drc_by_index(uint32_t index)
> > @@ -1179,6 +1195,7 @@ static void spapr_drc_register_types(void)
> >      type_register_static(&spapr_drc_cpu_info);
> >      type_register_static(&spapr_drc_pci_info);
> >      type_register_static(&spapr_drc_lmb_info);
> > +    type_register_static(&spapr_drc_phb_info);
> >  
> >      spapr_rtas_register(RTAS_SET_INDICATOR, "set-indicator",
> >                          rtas_set_indicator);
> > diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
> > index a7958d0a8d14..535fc61b98a8 100644
> > --- a/include/hw/ppc/spapr_drc.h
> > +++ b/include/hw/ppc/spapr_drc.h
> > @@ -69,6 +69,14 @@
> >  #define SPAPR_DRC_LMB(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \
> >                                          TYPE_SPAPR_DRC_LMB)
> >  
> > +#define TYPE_SPAPR_DRC_PHB "spapr-drc-phb"
> > +#define SPAPR_DRC_PHB_GET_CLASS(obj) \
> > +        OBJECT_GET_CLASS(sPAPRDRConnectorClass, obj, TYPE_SPAPR_DRC_PHB)
> > +#define SPAPR_DRC_PHB_CLASS(klass) \
> > +        OBJECT_CLASS_CHECK(sPAPRDRConnectorClass, klass, 
> > TYPE_SPAPR_DRC_PHB)
> > +#define SPAPR_DRC_PHB(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \
> > +                                        TYPE_SPAPR_DRC_PHB)
> > +
> >  /*
> >   * Various hotplug types managed by sPAPRDRConnector
> >   *
> >   
>

pgpLUl73oMhSH.pgp
Description: OpenPGP digital signature