From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43287) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YpHT8-0005z7-Ma for qemu-devel@nongnu.org; Mon, 04 May 2015 10:35:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YpHT4-0007JK-VL for qemu-devel@nongnu.org; Mon, 04 May 2015 10:35:38 -0400 Date: Mon, 4 May 2015 22:01:56 +1000 From: David Gibson Message-ID: <20150504120156.GE14090@voom.redhat.com> References: <1429858066-12088-1-git-send-email-bharata@linux.vnet.ibm.com> <1429858066-12088-6-git-send-email-bharata@linux.vnet.ibm.com> <20150426114748.GB18380@in.ibm.com> <20150427053607.GC18380@in.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="9l24NVCWtSuIVIod" Content-Disposition: inline In-Reply-To: <20150427053607.GC18380@in.ibm.com> Subject: Re: [Qemu-devel] [RFC PATCH v3 05/24] spapr: Reorganize CPU dt generation code List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bharata B Rao Cc: mdroth@linux.vnet.ibm.com, "Nikunj A. Dadhania" , 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, afaerber@suse.de --9l24NVCWtSuIVIod Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Apr 27, 2015 at 11:06:07AM +0530, Bharata B Rao wrote: > On Sun, Apr 26, 2015 at 05:17:48PM +0530, Bharata B Rao wrote: > > On Fri, Apr 24, 2015 at 12:17:27PM +0530, Bharata B Rao wrote: > > > Reorganize CPU device tree generation code so that it be reused from > > > hotplug path. CPU dt entries are now generated from spapr_finalize_fd= t() > > > instead of spapr_create_fdt_skel(). > >=20 > > Creating CPU DT entries from spapr_finalize_fdt() instead of > > spapr_create_fdt_skel() has an interesting side effect. > >=20 > > =20 > >=20 > > In both the cases, I am adding CPU DT nodes from QEMU in the same order, > > but not sure why the guest kernel discovers them in different orders in > > each case. >=20 > Nikunj and I tracked this down to the difference in device tree APIs that > we are using in two cases. >=20 > When CPU DT nodes are created from spapr_create_fdt_skel(), we are using > fdt_begin_node() API which does sequential write and hence CPU DT nodes > end up in the same order in which they are created. >=20 > However in my patch when I create CPU DT entries in spapr_finalize_fdt(), > I am using fdt_add_subnode() which ends up writing the CPU DT node at the > same parent offset for all the CPUs. This results in CPU DT nodes being > generated in reverse order in FDT. >=20 > >=20 > > > +static void spapr_populate_cpus_dt_node(void *fdt, sPAPREnvironment = *spapr) > > > +{ > > > + CPUState *cs; > > > + int cpus_offset; > > > + char *nodename; > > > + int smt =3D kvmppc_smt_threads(); > > > + > > > + cpus_offset =3D fdt_add_subnode(fdt, 0, "cpus"); > > > + _FDT(cpus_offset); > > > + _FDT((fdt_setprop_cell(fdt, cpus_offset, "#address-cells", 0x1))= ); > > > + _FDT((fdt_setprop_cell(fdt, cpus_offset, "#size-cells", 0x0))); > > > + > > > + CPU_FOREACH(cs) { > > > + PowerPCCPU *cpu =3D POWERPC_CPU(cs); > > > + int index =3D ppc_get_vcpu_dt_id(cpu); > > > + DeviceClass *dc =3D DEVICE_GET_CLASS(cs); > > > + int offset; > > > + > > > + if ((index % smt) !=3D 0) { > > > + continue; > > > + } > > > + > > > + nodename =3D g_strdup_printf("%s@%x", dc->fw_name, index); > > > + offset =3D fdt_add_subnode(fdt, cpus_offset, nodename); > > > + g_free(nodename); > > > + _FDT(offset); > > > + spapr_populate_cpu_dt(cs, fdt, offset); > > > + } > >=20 > > I can simply fix this by walking the CPUs in reverse order in the above > > code which makes the guest kernel to discover the CPU DT nodes in the > > right order. > >=20 > > s/CPU_FOREACH(cs)/CPU_FOREACH_REVERSE(cs) will solve this problem. Woul= d this > > be the right approach or should we just leave it to the guest kernel to > > discover and enumerate CPUs in whatever order it finds the DT nodes in = FDT ? >=20 > So using CPU_FOREACH_REVERSE(cs) appears to be right way to handle this. Yes, I think so. In theory it shouldn't matter, but I think it's safer to retain the device tree order. --=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 --9l24NVCWtSuIVIod Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJVR1+zAAoJEGw4ysog2bOSb4YQAJqeXHAFcahh2W/AJNyr/tfL QIaZj0DLtl7OCE79KuPLeq58AVwz7XnVjgTWLWpsESrlc768i+9iDnhgrbh/pNer XPQqCmLe7sET4qt8iOYi4SAcHlDXrKxYQzLv2eZHu0zGg4OxloqE5gINANUenLxZ i3Q7eQQ15GaqRxQT1DZdJAdrXQKGztryuPWD55c58Gdy9GkIl9DsRXHIYt06RmrJ HKr+OSjMSij58ZPs/n8LpUjsvT9rea+AjVVldTb4dAUjvMnzjTUECJDzuLLS/DYX 2sf+qY6/GiVOWTV8gWK/1HPpVdbZ3wmj/mNYv9g4T3sKCFXXHUA2UbRQv4dJlmYz ejCnzED4WR4wUUfM+ceNEVTAmm2hpRGrm+O8SeTWByencTaVSN/l96632fpDZ2q4 Np7Jn9ThMuP09Np6wAgI9HL/jblJqMXz58es/11gJT9lgjubwBlqfg9IdmMpZ0lA hgqx5WT6AeVJ2C2pWFqYHNSqtRmbeFVwumNZnSytPe96trBTkRtAFf2llN3DU4IN zgFIfCC6oRlZKJIW5TmtNHCgCP+m5ZsDVaZ2zzn4fQJvAU8ed8h41sYLLw6/G2gG UDIfeu0to0+nZ0SKNZl10Yk0lp32+Ku0BOqO9WddyiZkyTs7g0ytqJLJEDETp97w u0TiyBML9KkUwxMvjDuy =8hMi -----END PGP SIGNATURE----- --9l24NVCWtSuIVIod--