[Qemu-devel] [RFC PATCH 06/15] spapr_pci: also use 'index' property as DRC index for PHBs

[Michael Roth <mdroth@linux.vnet.ibm.com>]

Prior to this patch 'index' is purely a shorthand for specifying
MMIO windows, BUIDs, and other configuration values for a PHB.

With the addition of PHB hotplug, we have a static number of DRCs
that can be used to handle hotplug/unplug operations on our PHBs,
and need a consistent way to map PHBs to these connectors, and
assign a unique identifiers for the connectors.

BUIDs would be a good choice, however, those are 64-bit values,
whereas DRC indexes are 32-bit.

'index' serves this purpose nicely, and also allows us to align
the maximum number PHBs that can be plugged with the maximum
'index' value we allow (255).

This means that when PHB hotplug is enabled (2.4+), 'index' is
now always a required value, regardless of whether or not other
configuration properties are specified explicitly. We could
potentially arrange for 'index'-less PHBs to be added in an
'unpluggable' fashion via command-line, and have checks to
generate an error when hotplugged via device_add, but the simpler
path seems to be to just make it required now.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 hw/ppc/spapr_pci.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 25a738c..e37de28 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1156,8 +1156,14 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
         if ((sphb->buid != (uint64_t)-1) || (sphb->dma_liobn != (uint32_t)-1)
             || (sphb->mem_win_addr != (hwaddr)-1)
             || (sphb->io_win_addr != (hwaddr)-1)) {
-            error_setg(errp, "Either \"index\" or other parameters must"
-                       " be specified for PAPR PHB, not both");
+            if (!spapr->dr_phb_enabled) {
+                /* if they aren't potentially using index as an identifier for
+                 * the PHB's DR connector, enforce the old semantics of index
+                 * being purely a shorthand for PHB configuration options.
+                 */
+                error_setg(errp, "Either \"index\" or other parameters must"
+                           " be specified for PAPR PHB, not both");
+            }
             return;
         }
 
@@ -1174,6 +1180,14 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
             + sphb->index * SPAPR_PCI_WINDOW_SPACING;
         sphb->mem_win_addr = windows_base + SPAPR_PCI_MMIO_WIN_OFF;
         sphb->io_win_addr = windows_base + SPAPR_PCI_IO_WIN_OFF;
+    } else {
+        if (spapr->dr_phb_enabled) {
+            error_setg(errp, "The \"index\" property is required for machine"
+                       " types that support PHB hotplug (and in such cases"
+                       " can be used alongside \"buid\" and other"
+                       " configuration properties)");
+            return;
+        }
     }
 
     if (sphb->buid == (uint64_t)-1) {
-- 
1.9.1