From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47933) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z7EhX-0005Zo-4u for qemu-devel@nongnu.org; Mon, 22 Jun 2015 23:16:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z7EhS-0005yG-M5 for qemu-devel@nongnu.org; Mon, 22 Jun 2015 23:16:43 -0400 Date: Tue, 23 Jun 2015 11:32:34 +1000 From: David Gibson Message-ID: <20150623013234.GV13352@voom.redhat.com> References: <1434709077-17491-1-git-send-email-bharata@linux.vnet.ibm.com> <1434709077-17491-3-git-send-email-bharata@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="/X7s3i+qRyIJm2VP" Content-Disposition: inline In-Reply-To: <1434709077-17491-3-git-send-email-bharata@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [RFC PATCH v4 2/5] spapr: Add LMB DR connectors List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bharata B Rao Cc: mdroth@linux.vnet.ibm.com, aik@ozlabs.ru, agraf@suse.de, qemu-devel@nongnu.org, qemu-ppc@nongnu.org, tyreld@linux.vnet.ibm.com, nfont@linux.vnet.ibm.com, imammedo@redhat.com --/X7s3i+qRyIJm2VP Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Jun 19, 2015 at 03:47:54PM +0530, Bharata B Rao wrote: > Enable memory hotplug for pseries 2.4 and add LMB DR connectors. > With memory hotplug, enforce 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. >=20 > LMB DR connectors will be used by the memory hotplug code. >=20 > Signed-off-by: Bharata B Rao > Signed-off-by: Michael Roth > [spapr_drc_reset implementation] > --- > hw/ppc/spapr.c | 78 ++++++++++++++++++++++++++++++++++++++++++++= ++++++ > include/hw/ppc/spapr.h | 2 ++ > 2 files changed, 80 insertions(+) >=20 > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 87a29dc..f9af89b 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -59,6 +59,7 @@ > #include "hw/nmi.h" > =20 > #include "hw/compat.h" > +#include "qemu-common.h" > =20 > #include > =20 > @@ -1436,10 +1437,76 @@ static void spapr_cpu_init(sPAPRMachineState *spa= pr, PowerPCCPU *cpu) > qemu_register_reset(spapr_cpu_reset, cpu); > } > =20 > +static void spapr_drc_reset(void *opaque) This function needs a different name, since it's only called for LMB drcs, not all drcs. > +{ > + sPAPRDRConnector *drc =3D opaque; > + DeviceState *d =3D DEVICE(drc); > + > + if (d) { > + device_reset(d); > + } > +} > + > +static void spapr_create_lmb_dr_connectors(sPAPRMachineState *spapr) > +{ > + MachineState *machine =3D MACHINE(qdev_get_machine()); > + uint64_t lmb_size =3D SPAPR_MEMORY_BLOCK_SIZE; > + uint32_t nr_rma_lmbs =3D spapr->rma_size/lmb_size; > + uint32_t nr_lmbs =3D machine->maxram_size/lmb_size - nr_rma_lmbs; > + uint32_t nr_assigned_lmbs =3D machine->ram_size/lmb_size - nr_rma_lm= bs; > + int i; > + > + for (i =3D 0; i < nr_lmbs; i++) { > + sPAPRDRConnector *drc; > + uint64_t addr; > + > + if (i < nr_assigned_lmbs) { > + addr =3D (i + nr_rma_lmbs) * lmb_size; > + } else { > + addr =3D (i - nr_assigned_lmbs) * lmb_size + > + SPAPR_MACHINE(qdev_get_machine())->hotplug_memory.base; > + } > + > + drc =3D spapr_dr_connector_new(qdev_get_machine(), > + SPAPR_DR_CONNECTOR_TYPE_LMB, addr/lmb_size); > + qemu_register_reset(spapr_drc_reset, drc); Actually.. I'm not sure what spapr_drc_reset is needed for at all. Won't the device reset hook get called through the normal qdev path anyway? The PCI hotplug code doesn't have an explicit register_reset, so why does the memory hotplug code need it? > + } > +} > + > +/* > + * If LMB DR is enabled node memory size and max memory size should > + * be a multiple of SPAPR_MEMORY_BLOCK_SIZE (256M). > + */ > +static void spapr_validate_node_memory(sPAPRMachineState *spapr) > +{ > + int i; > + MachineState *machine =3D MACHINE(qdev_get_machine()); > + > + if (!spapr->dr_lmb_enabled) { > + return; > + } > + > + if (machine->maxram_size % SPAPR_MEMORY_BLOCK_SIZE) { > + error_report("maxmem should be a multiple of %lld MB", > + SPAPR_MEMORY_BLOCK_SIZE/M_BYTE); > + exit(EXIT_FAILURE); > + } > + > + for (i =3D 0; i < nb_numa_nodes; i++) { > + if (numa_info[i].node_mem && > + numa_info[i].node_mem % SPAPR_MEMORY_BLOCK_SIZE) { > + error_report("Memory size on node %d should be a multiple " > + "of %lld MB", i, SPAPR_MEMORY_BLOCK_SIZE/M_BYTE= ); > + exit(EXIT_FAILURE); > + } > + } > +} > + > /* pSeries LPAR / sPAPR hardware init */ > static void ppc_spapr_init(MachineState *machine) > { > sPAPRMachineState *spapr =3D SPAPR_MACHINE(machine); > + sPAPRMachineClass *smc =3D SPAPR_MACHINE_GET_CLASS(machine); > const char *kernel_filename =3D machine->kernel_filename; > const char *kernel_cmdline =3D machine->kernel_cmdline; > const char *initrd_filename =3D machine->initrd_filename; > @@ -1518,6 +1585,9 @@ static void ppc_spapr_init(MachineState *machine) > smp_threads), > XICS_IRQS); > =20 > + spapr->dr_lmb_enabled =3D smc->dr_lmb_enabled; I don't see any point to copying this value into the MachineState - I'm guessing this is a leftover from sPAPREnvironment. Anywhere you have the MachineState you can get to the MachineClass and use the value directly from there. > + spapr_validate_node_memory(spapr); > + > /* init CPUs */ > if (machine->cpu_model =3D=3D NULL) { > machine->cpu_model =3D kvm_enabled() ? "host" : "POWER7"; > @@ -1577,6 +1647,10 @@ static void ppc_spapr_init(MachineState *machine) > &spapr->hotplug_memory.mr); > } > =20 > + if (spapr->dr_lmb_enabled) { > + spapr_create_lmb_dr_connectors(spapr); > + } > + > filename =3D qemu_find_file(QEMU_FILE_TYPE_BIOS, "spapr-rtas.bin"); > if (!filename) { > error_report("Could not find LPAR rtas '%s'", "spapr-rtas.bin"); > @@ -1850,6 +1924,7 @@ static void spapr_nmi(NMIState *n, int cpu_index, E= rror **errp) > static void spapr_machine_class_init(ObjectClass *oc, void *data) > { > MachineClass *mc =3D MACHINE_CLASS(oc); > + sPAPRMachineClass *smc =3D SPAPR_MACHINE_CLASS(oc); > FWPathProviderClass *fwc =3D FW_PATH_PROVIDER_CLASS(oc); > NMIClass *nc =3D NMI_CLASS(oc); > =20 > @@ -1863,6 +1938,7 @@ static void spapr_machine_class_init(ObjectClass *o= c, void *data) > mc->kvm_type =3D spapr_kvm_type; > mc->has_dynamic_sysbus =3D true; > =20 > + smc->dr_lmb_enabled =3D false; > fwc->get_dev_path =3D spapr_get_fw_dev_path; > nc->nmi_monitor_handler =3D spapr_nmi; > } > @@ -1998,11 +2074,13 @@ static const TypeInfo spapr_machine_2_3_info =3D { > static void spapr_machine_2_4_class_init(ObjectClass *oc, void *data) > { > MachineClass *mc =3D MACHINE_CLASS(oc); > + sPAPRMachineClass *smc =3D SPAPR_MACHINE_CLASS(oc); > =20 > mc->name =3D "pseries-2.4"; > mc->desc =3D "pSeries Logical Partition (PAPR compliant) v2.4"; > mc->alias =3D "pseries"; > mc->is_default =3D 1; > + smc->dr_lmb_enabled =3D true; > } > =20 > static const TypeInfo spapr_machine_2_4_info =3D { > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index 8a1929b..b3fba76 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -35,6 +35,7 @@ struct sPAPRMachineClass { > MachineClass parent_class; > =20 > /*< public >*/ > + bool dr_lmb_enabled; /* enable dynamic-reconfig/hotplug of LMBs */ > }; > =20 > /** > @@ -74,6 +75,7 @@ struct sPAPRMachineState { > =20 > /* RTAS state */ > QTAILQ_HEAD(, sPAPRConfigureConnectorState) ccs_list; > + bool dr_lmb_enabled; /* hotplug / dynamic-reconfiguration of LMBs */ > =20 > /*< public >*/ > char *kvm_type; --=20 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 --/X7s3i+qRyIJm2VP Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJViLcyAAoJEGw4ysog2bOSolEP/2y46gzi0Z+A05tnzch4N1R5 6lInTB5cOBw5DA3fFkqM4p3fKFw1WIPuX99tsUFrHvCKRjJwFOd1x7P353J9OH8L jcVOrwtxfkWym9y0JMSkocbclYp3eX+sEVxIWL5mFBDIEgg1cm3XZmGk+csh5aj1 K0prI2W2bgSYE9x/uNqcKPatAKbfsgRK/UJa/1TECT0TFImtYLtF+4U1iHWwe0KR a0m0WojLCQsMrhRmBlKBUmbB65/DSpEOSm4vJ41zNyeq/kuQaloI2OlOV/c57Okm 0rH1X7e98VyrGf49jQrSLKiFUxu5o3DBYJv9qT19JUMOONSsR1Hfjz0aMWhc+3NQ SsV5mrYaNzyG7cuBVAr/cNBYwMWqGOvC6isajzwAeSurLqBLHxWK2XKCxEkQgjVd ysa2MAg1F+JJNDmCLW8/gJg9MYnELgWdx9yjHJgJRccB7munEIFoz9BWfawZ1OG0 Aot3FpDhwrVuTXpe2Q22WOGodf4vZRhsKzYSHG0LrKrRwQ67fDFbnArsGOVa+eWn wK45jSXleOHP1IVkuH7JMMlOYpXEPEDay2Tz1FqkayBF94T0Nxfv1hsq6ezm5+7a VtCoLsi/jjXfsmHVr1n2AdYi2PHg0N0WClMXd21WsGGmhQ43wXOGFVwQN/M9toE6 x6KV1a2Uh0eDsdLLeVdK =ezYQ -----END PGP SIGNATURE----- --/X7s3i+qRyIJm2VP--