From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52408) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a3xQl-0000Zg-4l for qemu-devel@nongnu.org; Tue, 01 Dec 2015 21:46:08 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a3xQg-0000wF-RA for qemu-devel@nongnu.org; Tue, 01 Dec 2015 21:46:07 -0500 Date: Wed, 2 Dec 2015 13:32:57 +1100 From: David Gibson Message-ID: <20151202023257.GB3107@voom.redhat.com> References: <1447467188-4458-1-git-send-email-sukadev@linux.vnet.ibm.com> <1447467188-4458-2-git-send-email-sukadev@linux.vnet.ibm.com> <20151127064738.GD25500@voom.redhat.com> <20151201194931.GA22634@us.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="cvVnyQ+4j833TQvp" Content-Disposition: inline In-Reply-To: <20151201194931.GA22634@us.ibm.com> Subject: Re: [Qemu-devel] [PATCH v3 2/2] target-ppc: Implement rtas_get_sysparm(PROCESSOR_MODULE_INFO) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sukadev Bhattiprolu Cc: stewart@linux.vnet.ibm.com, thuth@redhat.com, benh@au1.ibm.com, aik@ozlabs.ru, nacc@linux.vnet.ibm.com, agraf@suse.de, qemu-devel@nongnu.org, qemu-ppc@nongnu.org, paulus@au1.ibm.com --cvVnyQ+4j833TQvp Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Dec 01, 2015 at 11:49:31AM -0800, Sukadev Bhattiprolu wrote: > David Gibson [david@gibson.dropbear.id.au] wrote: > | > @@ -240,6 +241,36 @@ static void rtas_ibm_get_system_parameter(PowerP= CCPU *cpu, > | > target_ulong ret =3D RTAS_OUT_SUCCESS; > | > =20 > | > switch (parameter) { > | > + case RTAS_SYSPARM_PROCESSOR_MODULE_INFO: { > | > + struct sPAPRRTASModuleInfo modinfo; > | > + int i, size =3D sizeof(modinfo), offset =3D 0; > | > + > | > + memset(&modinfo, 0, size); > | > + if (kvmppc_rtas_get_module_info(&modinfo)) { > | > + ret =3D RTAS_OUT_HW_ERROR; > | > + break; > | > + } > | > + > | > + stw_be_phys(&address_space_memory, buffer+offset, size); > |=20 > | You're still advertising the full structure size to the guest, even > | though it may be only partially populated. > |=20 > | That will probably work in practice, but I think we should be > | PAPRishly correct and only output the size that we actually use here. >=20 > Ok. Will have kvmppc_rtas_get_module_info() take/update a size parameter > and use that here. >=20 > | >=20 > >=20 > | > +/* Each core in the system is represented by a directory with the pr= efix > | > + * 'PowerPC,POWER' in directory /proc/device-tree/cpus/. Process th= at > | > + * directory and count the number of cores in the system. > | > + * > | > + * Return 0 if one or more cores are found. Return -1 otherwise. > | > + */ > | > +static int kvmppc_count_cores_dt(int *num_cores) > | > +{ > | > + int rc; > | > + glob_t dtglob; > | > + const char *cpus_pattern =3D "/proc/device-tree/cpus/PowerPC,POW= ER*"; > |=20 > | Under KVM PR, this could still be too specific to IBM machines. I > | think it's probably safer to just use /proc/device-tree/cpus/*, I > | don't *think* we get anything under /cpus that isn't a cpu node. >=20 > Well, on my Tuleta system (3.18.22-355.el7_1.pkvm3_1_0.3700.3.ppc64le) > I see several l2-cache, l3-cache entries as well as some properties > (like phandle, #size-cells) besides the PowerPC,POWER* entries. >=20 > $ cd /proc/device-tree/cpus >=20 > $ lsprop l3-cache@30000020/device_type > l3-cache@30000020/device_type > "cache" >=20 > $ lsprop l2-cache@200008f0/device_type > l2-cache@200008f0/device_type > "cache" >=20 > $ lsprop PowerPC,POWER8@860/device_type > PowerPC,POWER8@860/device_type > "cpu" Ah.. right, guess I was wrong. > Should we walk the /proc/device-tree/cpus/ tree and count only dirs with > device-type "cpu" (rather than relying on the pattern PowerPC,POWER*)? Yes, I think you'll have to. > |=20 > | In a number of ways I'd actually prefer to move to /cpus/cpu@NNN in > | general, since that follows the OF generic names recommendation we > | follow for most other nodes. >=20 > Do you mean rename '/proc/device-tree/cpus/PowerPC,POWER8@NNN' to > /proc/device-tree/cpus/cpu@NNN? Yes. This is a firmware matter, so it's not something that can simply be changed everywhere, but it's the approach that I'd prefer to encourage for people making future machines and firmwares. --=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 --cvVnyQ+4j833TQvp Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWXlhZAAoJEGw4ysog2bOSpJUQAKIOigoCKLzPNyiVnQr/BrbH fRZVUqmpw4Afr8Buy4NwgYNsPwrxFNssaXWImVA8g/ZF82LjQjpEooqWX8cko9Qf /1RK/I5p4ma+mzeyNnsrj6OEHiX64y33bWukHBTAj6SaW+aTN8n0TDBcsSgjh+F8 xGLXsTsln5Wb7UzA8eiAUDUD23x8KTh1oGU5Alm0Gk+6mBoEmpTxds8M3gYK3hAB WE8jc171J+bucMLClIM76n3qYPsKIJKoNE/cbw7mLOc3VciKdmqFRbq3BqWQKNKi vOC4bYoGrRzsEETOdZjrqNHBJMduPwUPBAjxZyRYTsMv6eucRzUd8n3wX6na1oUr 3FpebVImNsiEYwLSuQt3vclSE6LqFV6oTyPfw1simDvNA5V24ayCUiCA91SARoU6 P04fmEecp/6z7v8taAtJcva1J8GUrQgg2A4JVAcFtQkE6FY+g5cL6jwrOaqfT/Ir hRtCicbeC/0h4vclAbXNRceZxqlemrb1AS+izXRnppaQtfogV7BVrt7Drp2TU7En hbpZYUvzadIuqIJut2zq95+06tTSWbuInFsbEw0HiJSLTN9/IgAnCBIqQpy9NUbd euwqj2bXzb0ZQK+cocMQWLKbbVLzV6PvhRCEhZ32hPRT6YOf0iYDxI9OwLjl+Svv JMXWCyBgkUx4L16rmFDl =/O5I -----END PGP SIGNATURE----- --cvVnyQ+4j833TQvp--