On Thu, Jun 25, 2015 at 11:44:11AM +0530, Bharata B Rao wrote:
> 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 <bhar...@linux.vnet.ibm.com>
> Signed-off-by: Michael Roth <mdr...@linux.vnet.ibm.com>
> [spapr_drc_reset implementation]
> ---
> hw/ppc/spapr.c | 79
> ++++++++++++++++++++++++++++++++++++++++++++++++++
> include/hw/ppc/spapr.h | 1 +
> 2 files changed, 80 insertions(+)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 241ecad..d7e0e44 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,76 @@ 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 disable LMB DR feature.
> + */
> +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).
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
pgpYA5EZxPIfa.pgp
Description: PGP signature
