From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55881) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z7deA-0003Nj-MG for qemu-devel@nongnu.org; Wed, 24 Jun 2015 01:54:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z7de6-0006mI-FW for qemu-devel@nongnu.org; Wed, 24 Jun 2015 01:54:54 -0400 Date: Wed, 24 Jun 2015 15:55:08 +1000 From: David Gibson Message-ID: <20150624055508.GB10837@voom.redhat.com> References: <1434709077-17491-1-git-send-email-bharata@linux.vnet.ibm.com> <1434709077-17491-4-git-send-email-bharata@linux.vnet.ibm.com> <20150623015429.GX13352@voom.redhat.com> <20150624022544.GD26051@in.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="aM3YZ0Iwxop3KEKx" Content-Disposition: inline In-Reply-To: <20150624022544.GD26051@in.ibm.com> Subject: Re: [Qemu-devel] [RFC PATCH v4 3/5] spapr: Support ibm, dynamic-reconfiguration-memory 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 --aM3YZ0Iwxop3KEKx Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jun 24, 2015 at 07:55:44AM +0530, Bharata B Rao wrote: > On Tue, Jun 23, 2015 at 11:54:29AM +1000, David Gibson wrote: > > On Fri, Jun 19, 2015 at 03:47:55PM +0530, Bharata B Rao wrote: > > > Parse ibm,architecture.vec table obtained from the guest and enable > > > memory node configuration via ibm,dynamic-reconfiguration-memory if g= uest > > > supports it. This is in preparation to support memory hotplug for > > > sPAPR guests. > > >=20 > > > This changes the way memory node configuration is done. Currently all > > > memory nodes are built upfront. But after this patch, only memory@0 n= ode > > > for RMA is built upfront. Guest kernel boots with just that and rest = of > > > the memory nodes (via memory@XXX or ibm,dynamic-reconfiguration-memor= y) > > > are built when guest does ibm,client-architecture-support call. > > >=20 > > > Note: This patch needs a SLOF enhancement which is already part of > > > SLOF binary in QEMU. > > >=20 > > > Signed-off-by: Bharata B Rao > >=20 > > [snip] > > > +int spapr_h_cas_compose_response(sPAPRMachineState *spapr, > > > + target_ulong addr, target_ulong siz= e, > > > + bool cpu_update, bool memory_update) > > > +{ > > > + void *fdt, *fdt_skel; > > > + sPAPRDeviceTreeUpdateHeader hdr =3D { .version_id =3D 1 }; > > > + > > > + size -=3D sizeof(hdr); > > > + > > > + /* Create sceleton */ > > > + fdt_skel =3D g_malloc0(size); > > > + _FDT((fdt_create(fdt_skel, size))); > > > + _FDT((fdt_begin_node(fdt_skel, ""))); > > > + _FDT((fdt_end_node(fdt_skel))); > > > + _FDT((fdt_finish(fdt_skel))); > > > + fdt =3D g_malloc0(size); > > > + _FDT((fdt_open_into(fdt_skel, fdt, size))); > > > + g_free(fdt_skel); > > > + > > > + /* Fixup cpu nodes */ > > > + if (cpu_update) { > > > + _FDT((spapr_fixup_cpu_dt(fdt, spapr))); > > > + } > >=20 > > The cpu_update parameter seems like its not related to memory hotplug > > at all. I'm guessing it relates to CPU hotplug, in which case please > > defer it until those patches are ready to go. >=20 > This change isn't related to cpu hotplug. Earlier this compose response > routine only did CPU device tree fixup based on some conditions. I have > enabled it to check for availability DRCONF_MEMORY feature and accordingly > fixup memory DT. So this change just checks if cpu fixup is necessary > or not. Essentially we aren't changing any behaviour wrt cpu dt > fixup here. Hm, ok. Would there be any problem with just unconditionally doing both fixups? This is about as far from a hot path as its possible to get. >=20 > >=20 > > > + > > > + /* Generate memory nodes or ibm,dynamic-reconfiguration-memory n= ode */ > > > + if (memory_update && spapr->dr_lmb_enabled) { > > > + _FDT((spapr_populate_drconf_memory(spapr, fdt))); > > > + } else { > > > + _FDT((spapr_populate_memory(spapr, fdt))); > > > + } > > > + > > > + /* Pack resulting tree */ > > > + _FDT((fdt_pack(fdt))); > > > + > > > + if (fdt_totalsize(fdt) + sizeof(hdr) > size) { > > > + trace_spapr_cas_failed(size); > > > + return -1; > > > + } > > > + > > > + cpu_physical_memory_write(addr, &hdr, sizeof(hdr)); > > > + cpu_physical_memory_write(addr + sizeof(hdr), fdt, fdt_totalsize= (fdt)); > > > + trace_spapr_cas_continue(fdt_totalsize(fdt) + sizeof(hdr)); > > > + g_free(fdt); > > > + > > > + return 0; > > > +} > > > + > > > static void spapr_finalize_fdt(sPAPRMachineState *spapr, > > > hwaddr fdt_addr, > > > hwaddr rtas_addr, > > > @@ -756,10 +866,16 @@ static void spapr_finalize_fdt(sPAPRMachineStat= e *spapr, > > > /* open out the base tree into a temp buffer for the final tweak= s */ > > > _FDT((fdt_open_into(spapr->fdt_skel, fdt, FDT_MAX_SIZE))); > > > =20 > > > - ret =3D spapr_populate_memory(spapr, fdt); > > > - if (ret < 0) { > > > - fprintf(stderr, "couldn't setup memory nodes in fdt\n"); > > > - exit(1); > > > + /* > > > + * Add memory@0 node to represent RMA. Rest of the memory is eit= her > > > + * represented by memory nodes or ibm,dynamic-reconfiguration-me= mory > > > + * node later during ibm,client-architecture-support call. > > > + */ > > > + for (i =3D 0; i < nb_numa_nodes; ++i) { > > > + if (numa_info[i].node_mem) { > > > + spapr_populate_memory_node(fdt, i, 0, spapr->rma_size); > > > + break; > > > + } > >=20 > > ?? The code doesn't seem to match the comment - you appear to be > > creating a memory@0 node for every NUMA node, not just for the RMA, > > which doesn't make much sense. >=20 > I have a break there to ensure memory@0 is created only once from the 1st > memory-less node. I am slightly changing this in next version to ensure > that this works correctly even when -numa isn't specified. Ah, sorry, I missed the break;. That should be ok then. --=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 --aM3YZ0Iwxop3KEKx Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJVikY8AAoJEGw4ysog2bOSk6EP/3aBcLJB7O0ZPeNu5Rm+S7d+ dQ72B4OPGewKvLljwksqJvosWVXvuLvyraDY8ygTh6UCTKegHMpI8VkmzIaTI5B3 3jbxp7uyNxOskbl9lTlSb7jZyKna6JuGdvA2Ygio/99+oS3EoNetrbvDayv9UkQn fwjxo2ApJZRP7SJclMqTuPZ3afnmgOsLN58u0Ib3ZFuWvmTvT8kREHszS6P2jWZS WchVnxWCMIORmGYYIjLAfPKzMs/+qtA53h12u+qZdE7DcADabYE7hrYl1w4paEJ+ p4nRBoU5fyLf89IJ+NamdUJZCqZyTJ9ouekJ21+U/gUMgrRfv4t8g7tLz0wZ6GFY Id6A0o3+eUvL7+gzw5HLtJ9BnT3sRAbJOdAvORBXjWX+4qYkp+9V+1VQlrAfYuEi 6XWdxDaQ42spCXUrcAAsLIPTuYBsdFgnAZ+253o8e4u8m1V38bNpvApctq/7skrU HGghKW5PP3tYroh5Pjhv2Fnd6nobuRLKKyLwopErZF6tjDFO45KBhgzfDNybt8NH Psps/jmFiRQ5xyJdfxduRYqAizWciDkb7TUyOegqB4unvOofziE1lO93nFLhuof9 I/KA5RLnvWh0pemNJAooBYQsIb2IbRkLNsyE+crblo7NkEqqgTcMHfSuZAmoqlGp ZG2+2NCNpObYDqjYsH74 =WY/o -----END PGP SIGNATURE----- --aM3YZ0Iwxop3KEKx--