Re: [Qemu-devel] [PATCH v5 2/6] spapr: Add LMB DR connectors

[Bharata B Rao]

On Fri, Jun 26, 2015 at 03:38:41PM +1000, David Gibson wrote:
> > + */
> > +static void spapr_validate_node_memory(MachineState *machine)
> > +{
> > +    int i;
> > +    sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(machine);
> > +
> > +    if (machine->maxram_size % SPAPR_MEMORY_BLOCK_SIZE ||
> > +        machine->ram_size % SPAPR_MEMORY_BLOCK_SIZE) {
> > +        smc->dr_lmb_enabled = false;
> > +    }
> > +
> > +    for (i = 0; i < nb_numa_nodes; i++) {
> > +        if (numa_info[i].node_mem &&
> > +            numa_info[i].node_mem % SPAPR_MEMORY_BLOCK_SIZE) {
> > +            smc->dr_lmb_enabled = false;
> 
> Hrm, little nit here.
> 
> First, I'm not sure that changing a value in the MachineClass after
> initialization is a QOM-ly correct thing to do at all.
> 
> I'm also concerned that this will silently disable a feature the user
> expected, based on what may not be an obvious problem in the specified
> options.
> 
> This is only called from ppc_spapr_init() - not on every reset, so I
> think it would be better to fail out completely (with a useful error
> message), rather than carry on silently disabling memory hotplug.
> (Obviously don't fail if the user has explicitly disabled mem hotplug
> with their machine type choice).

Since we validate these memory sizes only for newer pseries machines
that support LMB DR, it should be fine to just terminate instead
of silently disabling memory hotplug. Here is the replacement patch
which does this. If you prefer a complete re-spin due to this, I can
do that as well.

Regards,
Bharata.
------------------------------

spapr: Add LMB DR connectors

From: Bharata B Rao <address@hidden>

Enable memory hotplug for pseries 2.4 and add LMB DR connectors.
With memory hotplug, enforce RAM size, NUMA node memory size and maxmem
to be a multiple of SPAPR_MEMORY_BLOCK_SIZE (256M) since that's the
granularity in which LMBs are represented and hot-added.

LMB DR connectors will be used by the memory hotplug code.

Signed-off-by: Bharata B Rao <address@hidden>
Signed-off-by: Michael Roth <address@hidden>
               [spapr_drc_reset implementation]
---
 hw/ppc/spapr.c         |   88 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/ppc/spapr.h |    1 +
 2 files changed, 89 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 241ecad..bee868c 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -59,6 +59,7 @@
 #include "hw/nmi.h"
 
 #include "hw/compat.h"
+#include "qemu-common.h"
 
 #include <libfdt.h>
 
@@ -1436,10 +1437,85 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, 
PowerPCCPU *cpu)
     qemu_register_reset(spapr_cpu_reset, cpu);
 }
 
+/*
+ * Reset routine for LMB DR devices.
+ *
+ * Unlike PCI DR devices, LMB DR devices explicitly register this reset
+ * routine. Reset for PCI DR devices will be handled by PHB reset routine
+ * when it walks all its children devices. LMB devices reset occurs
+ * as part of spapr_ppc_reset().
+ */
+static void spapr_drc_reset(void *opaque)
+{
+    sPAPRDRConnector *drc = opaque;
+    DeviceState *d = DEVICE(drc);
+
+    if (d) {
+        device_reset(d);
+    }
+}
+
+static void spapr_create_lmb_dr_connectors(sPAPRMachineState *spapr)
+{
+    MachineState *machine = MACHINE(spapr);
+    uint64_t lmb_size = SPAPR_MEMORY_BLOCK_SIZE;
+    uint32_t nr_rma_lmbs = spapr->rma_size/lmb_size;
+    uint32_t nr_lmbs = machine->maxram_size/lmb_size - nr_rma_lmbs;
+    uint32_t nr_assigned_lmbs = machine->ram_size/lmb_size - nr_rma_lmbs;
+    int i;
+
+    for (i = 0; i < nr_lmbs; i++) {
+        sPAPRDRConnector *drc;
+        uint64_t addr;
+
+        if (i < nr_assigned_lmbs) {
+            addr = (i + nr_rma_lmbs) * lmb_size;
+        } else {
+            addr = (i - nr_assigned_lmbs) * lmb_size +
+                    spapr->hotplug_memory.base;
+        }
+        drc = spapr_dr_connector_new(OBJECT(spapr), 
SPAPR_DR_CONNECTOR_TYPE_LMB,
+                                     addr/lmb_size);
+        qemu_register_reset(spapr_drc_reset, drc);
+    }
+}
+
+/*
+ * If RAM size, maxmem size and individual node mem sizes aren't aligned
+ * to SPAPR_MEMORY_BLOCK_SIZE(256MB), then refuse to start the guest
+ * since we can't support such unaligned sizes with DRCONF_MEMORY.
+ */
+static void spapr_validate_node_memory(MachineState *machine)
+{
+    int i;
+
+    if (machine->maxram_size % SPAPR_MEMORY_BLOCK_SIZE ||
+        machine->ram_size % SPAPR_MEMORY_BLOCK_SIZE) {
+        error_report("Can't support memory configuration where RAM size "
+                     "0x" RAM_ADDR_FMT " or maxmem size "
+                     "0x" RAM_ADDR_FMT " isn't aligned to %lld MB",
+                     machine->ram_size, machine->maxram_size,
+                     SPAPR_MEMORY_BLOCK_SIZE/M_BYTE);
+        exit(EXIT_FAILURE);
+    }
+
+    for (i = 0; i < nb_numa_nodes; i++) {
+        if (numa_info[i].node_mem &&
+            numa_info[i].node_mem % SPAPR_MEMORY_BLOCK_SIZE) {
+            error_report("Can't support memory configuration where memory "
+                         "size %lx of node %d isn't aligned to %lld MB",
+                         numa_info[i].node_mem, i,
+                         SPAPR_MEMORY_BLOCK_SIZE/M_BYTE);
+            exit(EXIT_FAILURE);
+        }
+    }
+}
+
 /* pSeries LPAR / sPAPR hardware init */
 static void ppc_spapr_init(MachineState *machine)
 {
     sPAPRMachineState *spapr = SPAPR_MACHINE(machine);
+    sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(machine);
     const char *kernel_filename = machine->kernel_filename;
     const char *kernel_cmdline = machine->kernel_cmdline;
     const char *initrd_filename = machine->initrd_filename;
@@ -1518,6 +1594,10 @@ static void ppc_spapr_init(MachineState *machine)
                                                smp_threads),
                                   XICS_IRQS);
 
+    if (smc->dr_lmb_enabled) {
+        spapr_validate_node_memory(machine);
+    }
+
     /* init CPUs */
     if (machine->cpu_model == NULL) {
         machine->cpu_model = kvm_enabled() ? "host" : "POWER7";
@@ -1567,6 +1647,10 @@ static void ppc_spapr_init(MachineState *machine)
                                     &spapr->hotplug_memory.mr);
     }
 
+    if (smc->dr_lmb_enabled) {
+        spapr_create_lmb_dr_connectors(spapr);
+    }
+
     filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, "spapr-rtas.bin");
     if (!filename) {
         error_report("Could not find LPAR rtas '%s'", "spapr-rtas.bin");
@@ -1840,6 +1924,7 @@ static void spapr_nmi(NMIState *n, int cpu_index, Error 
**errp)
 static void spapr_machine_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
+    sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(oc);
     FWPathProviderClass *fwc = FW_PATH_PROVIDER_CLASS(oc);
     NMIClass *nc = NMI_CLASS(oc);
 
@@ -1853,6 +1938,7 @@ static void spapr_machine_class_init(ObjectClass *oc, 
void *data)
     mc->kvm_type = spapr_kvm_type;
     mc->has_dynamic_sysbus = true;
 
+    smc->dr_lmb_enabled = false;
     fwc->get_dev_path = spapr_get_fw_dev_path;
     nc->nmi_monitor_handler = spapr_nmi;
 }
@@ -1988,11 +2074,13 @@ static const TypeInfo spapr_machine_2_3_info = {
 static void spapr_machine_2_4_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
+    sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(oc);
 
     mc->name = "pseries-2.4";
     mc->desc = "pSeries Logical Partition (PAPR compliant) v2.4";
     mc->alias = "pseries";
     mc->is_default = 1;
+    smc->dr_lmb_enabled = true;
 }
 
 static const TypeInfo spapr_machine_2_4_info = {
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 8a1929b..a142e91 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -35,6 +35,7 @@ struct sPAPRMachineClass {
     MachineClass parent_class;
 
     /*< public >*/
+    bool dr_lmb_enabled; /* enable dynamic-reconfig/hotplug of LMBs */
 };
 
 /**