From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34787) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z8ML7-0008IW-Q2 for qemu-devel@nongnu.org; Fri, 26 Jun 2015 01:38:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z8ML6-0002pJ-3c for qemu-devel@nongnu.org; Fri, 26 Jun 2015 01:38:13 -0400 Date: Fri, 26 Jun 2015 15:38:41 +1000 From: David Gibson Message-ID: <20150626053841.GE22737@voom.redhat.com> References: <1435212855-21685-1-git-send-email-bharata@linux.vnet.ibm.com> <1435212855-21685-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="KuLpqunXa7jZSBt+" Content-Disposition: inline In-Reply-To: <1435212855-21685-3-git-send-email-bharata@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [PATCH v5 2/6] 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 --KuLpqunXa7jZSBt+ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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. >=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 | 79 ++++++++++++++++++++++++++++++++++++++++++++= ++++++ > include/hw/ppc/spapr.h | 1 + > 2 files changed, 80 insertions(+) >=20 > 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" > =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 > +/* > + * 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 =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(spapr); > + 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->hotplug_memory.base; > + } > + drc =3D 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 =3D SPAPR_MACHINE_GET_CLASS(machine); > + > + if (machine->maxram_size % SPAPR_MEMORY_BLOCK_SIZE || > + machine->ram_size % SPAPR_MEMORY_BLOCK_SIZE) { > + smc->dr_lmb_enabled =3D false; > + } > + > + for (i =3D 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 =3D 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). --=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 --KuLpqunXa7jZSBt+ Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJVjOVhAAoJEGw4ysog2bOSOiYP/RXV2K2nuQNQZu2xq9ZO/BSN y3eIu3NfiVGTFylrbrhSKJ4UKIKLnG4Bs8l1j1b5VQ766N0KEmrt6iRKWUCcVYSD frY+d+uar4TeZYcJlE7JsrXGSl81L4vVCSM8hs88jkCsaEUmw0nbQjC55Rifjg/m weFgmrJJb3WMxrZLQofth3R+/PEp01a15/b1bUJN2/Fanq9AhShepQAvRvnP/xaq skglokk+S+tNQgTHMroUdetdGeyfHtlu6Sgjy5GZs1M/uh06NZZ4Bn9kmG7GUUdm Kbrj0rVmim9J1doW9R7GipdlI8zffqs9yCTOUbufdsJ09KP6Bfy//0bL5Pdbz5DS fdXSCIbzoTCSxfAkHWIvB6n4lpGA8A5agQZfvBDk87GTUVw4ETX9kPdeWf/M1rdo D6svytUhTPO/GPl3J9pP+ppW9UAKC6YH51y/AOyed2mXZzn0TiIw1x0pZ4ve1Z7x ojnogW2LMDvd6TTxxeNvgj6l5bnWTeN1M9XcHJcV1FYlxwfvvFBv9JMVZk2fk3Hf YJ6fe1jmatHMXEEiwPlqpVjYIxUJ20aFo8vgjZmtRfkFXqFpTH0O/dbkrEr7Ed79 7kn9SPMkrIZYeeoUg4OqiDCIEFBT7Qo9AjDbevOCI9idAWilDe7AuqeC/S1q2bd+ W9TS5VydFEoNfSk8xymq =2hSB -----END PGP SIGNATURE----- --KuLpqunXa7jZSBt+--