On Wed, 13 Sep 2017 22:23:29 +1000 David Gibson <da...@gibson.dropbear.id.au> wrote:
[...snip...] > > > > 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 ? > > > > > > Actually, if we're binding it back to index, which has a hard limit, > > > then it no longer makes sense to have it as a property and we should > > > go back to a constant (well, it could vary by machine type version). > > Sorry I've taken so long to reply. > Oh, don't mention it. :) > > But I guess that the hard limit of 31 as described in the changelog of > > commit 357d1e3bc7d2d80e5271bc4f3ac8537e30dc8046 still holds, doesn't > > it ? > > That's right. Note that that is a limit of *31* PHBs (numbered > 0..30), not 32 PHBs numbered 0..31. > Yeah I saw that. > > Because some guest versions (including most current distro kernels) > > can't > > access PCI MMIO above 64 TiB, we put all the PCI windows between 32 TiB > > and > > 64 TiB. This is broken into 1 TiB chunks. The first 1 TiB contains the > > PIO (64 kiB) and 32-bit MMIO (2 GiB) windows for all of the PHBs. Each > > subsequent TiB chunk contains a naturally aligned 64-bit MMIO window for > > one PHB each. > > > > This reduces the number of allowed PHBs (without full manual > > configuration > > of all the windows) from 256 to 31, but this should still be plenty in > > practice. > > > > Not sure why a machine type version would have a different limit. Can > > you think of a use case ? > > Well, the older machine types had a different layout. It allowed for > more indexes, but had smaller windows, which meant certain cards (e.g. > GPGPUs with huge BARs) didn't work properly. It also had some weird > alignments that meant we were a bit wasteful of address space. > > But we can't change the location of PHB windows on migration, so we > had to maintain that old layout for old machine types. That's why > there's a different limit depending on machine type version. > Ok, so we *just* have 2 different maximum number of PHBs: - 256 for pseries <= 2.7 - 31 for newer machine types I sent a RFC patch last monday: https://lists.nongnu.org/archive/html/qemu-ppc/2017-09/msg00203.html > > > > > > > > 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 | > > > > > > Ah, my mistake. > > > > > > > > > > > > > + .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 > > > > > > * > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
pgp7UuwuOdZuE.pgp
Description: OpenPGP digital signature