From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57682) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a2Coo-0003gC-D5 for qemu-devel@nongnu.org; Fri, 27 Nov 2015 01:47:44 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a2Com-0004Hh-Fn for qemu-devel@nongnu.org; Fri, 27 Nov 2015 01:47:42 -0500 Date: Fri, 27 Nov 2015 17:47:38 +1100 From: David Gibson Message-ID: <20151127064738.GD25500@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> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="0/kgSOzhNoDC5T3a" Content-Disposition: inline In-Reply-To: <1447467188-4458-2-git-send-email-sukadev@linux.vnet.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 --0/kgSOzhNoDC5T3a Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Nov 13, 2015 at 06:13:08PM -0800, Sukadev Bhattiprolu wrote: > Implement RTAS_SYSPARM_PROCESSOR_MODULE_INFO parameter to rtas_get_syspar= m() > call in qemu. This call returns the processor module (socket), chip and c= ore > information as specified in section 7.3.16.17 of LoPAPR v2.7. >=20 > We walk the /proc/device-tree to determine the number of modules(aka sock= ets), > chips and cores in the _host_ system and return this info to the guest > application that makes the rtas_get_sysparm() call. >=20 > We currently hard code the number of module_types to 1 (we currently don't > have systems with more than one module type) but we should determine that > dynamically somehow later. >=20 > Thanks to input from Nishanth Aravamudan, Alexey Kardashevskiy, David Gib= son, > Thomas Huth. >=20 > Signed-off-by: Sukadev Bhattiprolu > --- > Changelog[v3]: > [David Gibson] Use glob() to simplify pattern matching path names. > Move new code into target-ppc/{kvm.c,kvm_ppc.h} to not impact > other targets and avoid creating new files. > [Alexey Kardashevskiy] Fix indentation, error messages; fold new > code into existing files and avoid creating new files for this > parameter; since the keys are integer, use g_direct_hash() and > GINT_TO_POINTER() and avoid allocating/freeing memory. >=20 > Changelog[v2]: > [Alexey Kardashevskiy] Use existing interfaces like ldl_be_p(), > stw_be_phys(), g_hash_table_new_full(), error_report() rather > than re-inventing; fix indentation, function prottypes etc; > Drop the fts() interface (which doesn't seem to be available > on mingw32/mingw64) and use opendir() to walk specific > directories in the directory tree. > --- > hw/ppc/spapr_rtas.c | 31 +++++++++++ > include/hw/ppc/spapr.h | 13 +++++ > target-ppc/kvm.c | 141 +++++++++++++++++++++++++++++++++++++++++++= ++++++ > target-ppc/kvm_ppc.h | 8 +++ > 4 files changed, 193 insertions(+) >=20 > diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c > index 34b12a3..44b2537 100644 > --- a/hw/ppc/spapr_rtas.c > +++ b/hw/ppc/spapr_rtas.c > @@ -38,6 +38,7 @@ > =20 > #include > #include "hw/ppc/spapr_drc.h" > +#include "kvm_ppc.h" > =20 > /* #define DEBUG_SPAPR */ > =20 > @@ -240,6 +241,36 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU= *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); You're still advertising the full structure size to the guest, even though it may be only partially populated. That will probably work in practice, but I think we should be PAPRishly correct and only output the size that we actually use here. > + offset +=3D 2; > + > + stw_be_phys(&address_space_memory, buffer+offset, modinfo.module= _types); > + offset +=3D 2; > + > + for (i =3D 0; i < modinfo.module_types; i++) { > + stw_be_phys(&address_space_memory, buffer+offset, > + modinfo.si[i].sockets); > + offset +=3D 2; > + stw_be_phys(&address_space_memory, buffer+offset, > + modinfo.si[i].chips); > + offset +=3D 2; > + stw_be_phys(&address_space_memory, buffer+offset, > + modinfo.si[i].cores_per_chip); > + offset +=3D 2; > + } > + break; > + } > + > case RTAS_SYSPARM_SPLPAR_CHARACTERISTICS: { > char *param_val =3D g_strdup_printf("MaxEntCap=3D%d," > "DesMem=3D%llu," > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index 5baa906..f793465 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -463,6 +463,7 @@ int spapr_allocate_irq_block(int num, bool lsi, bool = msi); > /* RTAS ibm,get-system-parameter token values */ > #define RTAS_SYSPARM_SPLPAR_CHARACTERISTICS 20 > #define RTAS_SYSPARM_DIAGNOSTICS_RUN_MODE 42 > +#define RTAS_SYSPARM_PROCESSOR_MODULE_INFO 43 > #define RTAS_SYSPARM_UUID 48 > =20 > /* RTAS indicator/sensor types > @@ -646,4 +647,16 @@ int spapr_rng_populate_dt(void *fdt); > */ > #define SPAPR_LMB_FLAGS_ASSIGNED 0x00000008 > =20 > +#define SPAPR_MAX_MODULE_TYPES 1 > + > +struct sPAPRRTASSocketInfo { > + unsigned short sockets; > + unsigned short chips; > + unsigned short cores_per_chip; > +}; > +struct sPAPRRTASModuleInfo { > + unsigned short module_types; > + struct sPAPRRTASSocketInfo si[SPAPR_MAX_MODULE_TYPES]; > +}; > + > #endif /* !defined (__HW_SPAPR_H__) */ > diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c > index 9940a90..bbef78c 100644 7 > --- a/target-ppc/kvm.c > +++ b/target-ppc/kvm.c > @@ -15,6 +15,7 @@ > */ > =20 > #include > +#include > #include > #include > #include > @@ -2518,3 +2519,143 @@ int kvmppc_enable_hwrng(void) > =20 > return kvmppc_enable_hcall(kvm_state, H_RANDOM); > } > + > +/* Read an identifier from the file @path and add the identifier > + * to the hash table @gt unless its already in the table. > + */ > +static int kvmppc_hash_file_contents(GHashTable *gt, char *path) > +{ > + uint32_t idx; > + > + idx =3D kvmppc_read_int_dt(path); > + if (idx =3D=3D -1) { > + return -1; > + } > + > + if (g_hash_table_contains(gt, GINT_TO_POINTER(idx))) { > + return 0; > + } > + > + if (!g_hash_table_insert(gt, GINT_TO_POINTER(idx), NULL)) { > + fprintf(stderr, "%s() Unable to add key %d\n", __func__, idx); > + return -1; > + } > + > + return 0; > +} > + > +static int kvmppc_glob_count_ids_dt(const char *pattern, int *count) > +{ > + int i, rc; > + glob_t dtglob; > + GHashTable *htbl; > + > + rc =3D glob(pattern, GLOB_NOSORT, NULL, &dtglob); > + if (rc) { > + fprintf(stderr, "%s() glob(%s) returns %d, errno %d\n", __func__, > + pattern, rc, errno); > + return -1; > + } > + > + rc =3D -1; > + htbl =3D g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL, = NULL); > + > + for (i =3D 0; i < dtglob.gl_pathc; i++) { > + if (kvmppc_hash_file_contents(htbl, dtglob.gl_pathv[i])) { > + goto cleanup; > + } > + } > + > + *count =3D g_hash_table_size(htbl); > + rc =3D 0; > + > +cleanup: > + globfree(&dtglob); > + g_hash_table_remove_all(htbl); > + g_hash_table_destroy(htbl); > + > + return rc; > +} > + > +/* Each socket's (aka module's) id is contained in the 'ibm,hw-module-id' > + * file in an "xscom" directory (/proc/device-tree/xscom*). Similarly ea= ch > + * chip's id is contained in the 'ibm,chip-id' file in an xscom director= y. > + * > + * Since a module can have more than one chip and a chip can have more t= han > + * one core, there are likely to be duplicates in the module and chip ids > + * i.e more than one xscom directory can contain the same module/chip id. > + * > + * Search the xscom directories and count the number of _UNIQUE_ modules > + * and chips in the system. > + * > + * Return 0 if one or more modules and chips each are found. Return -1 > + * otherwise. > + */ > +static int kvmppc_count_sockets_chips_dt(int *num_sockets, int *num_chip= s) > +{ > + const char *chip_pattern =3D "/proc/device-tree/xscom@*/ibm,chip-id"; > + const char *module_pattern =3D "/proc/device-tree/xscom@*/ibm,hw-mod= ule-id"; > + > + if (kvmppc_glob_count_ids_dt(module_pattern, num_sockets) > + || kvmppc_glob_count_ids_dt(chip_pattern, num_chips)) { > + return -1; > + } > + > + if (*num_sockets =3D=3D 0 || *num_chips =3D=3D 0) { > + return -1; > + } > + > + return 0; > +} > + > +/* Each core in the system is represented by a directory with the prefix > + * 'PowerPC,POWER' in directory /proc/device-tree/cpus/. Process that > + * 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,POWER*"; 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. 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. > + rc =3D glob(cpus_pattern, GLOB_NOSORT, NULL, &dtglob); > + if (rc) { > + fprintf(stderr, "%s() glob(%s) returns %d, errno %d\n", __func__, > + cpus_pattern, rc, errno); > + return -1; > + } > + > + *num_cores =3D dtglob.gl_pathc; > + globfree(&dtglob); > + > + if (*num_cores =3D=3D 0) { > + return -1; > + } > + > + return 0; > +} > + > +int kvmppc_rtas_get_module_info(struct sPAPRRTASModuleInfo *modinfo) > +{ > + int cores, chips, sockets; > + > + if (kvmppc_count_sockets_chips_dt(&sockets, &chips) > + || kvmppc_count_cores_dt(&cores)) { > + return -1; > + } > + > + /* TODO: Although the interface allows multiple module types, Power > + * systems currently support only one module type per system - > + * either Single Chip Module(SCM) or Dual Chip Module(DCM), > + * but not both types in the same system. Hard code module_typ= es > + * for now till we can determine it dynamically. > + */ > + modinfo->module_types =3D 1; > + modinfo->si[0].sockets =3D sockets; > + modinfo->si[0].chips =3D chips; > + modinfo->si[0].cores_per_chip =3D cores / chips; > + > + return 0; > +} > diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h > index 309cbe0..46a2768 100644 > --- a/target-ppc/kvm_ppc.h > +++ b/target-ppc/kvm_ppc.h > @@ -10,6 +10,7 @@ > #define __KVM_PPC_H__ > =20 > #define TYPE_HOST_POWERPC_CPU "host-" TYPE_POWERPC_CPU > +#include "hw/ppc/spapr.h" > =20 > #ifdef CONFIG_KVM > =20 > @@ -55,6 +56,7 @@ void kvmppc_hash64_write_pte(CPUPPCState *env, target_u= long pte_index, > target_ulong pte0, target_ulong pte1); > bool kvmppc_has_cap_fixup_hcalls(void); > int kvmppc_enable_hwrng(void); > +int kvmppc_rtas_get_module_info(struct sPAPRRTASModuleInfo *modinfo); > =20 > #else > =20 > @@ -256,6 +258,12 @@ static inline int kvmppc_enable_hwrng(void) > { > return -1; > } > + > +static inline int kvmppc_rtas_get_module_info(struct sPAPRRTASModuleInfo= *mi) > +{ > + return -1; > +} > + > #endif > =20 > #ifndef CONFIG_KVM --=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 --0/kgSOzhNoDC5T3a Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWV/yKAAoJEGw4ysog2bOSFlQP/jGiRpokG6PDRLtSyOIfB4s2 FsKwQtJ72MYOSoZSaLewhDEoqGtnpIzG8OGXaFOn/zQvUBmM99Xl45Xe49B6PuZH QZpDfq2BT54cNxJYrarhlpFJ0+SyrLBTmi4e5RRpdBcPsqs3AF1w96+vjxY9NduY PWu+ORwLYEUbgO3Z8QvdiAYrrSMpB5B9DyDaBXpuPaDyOpn9XfM5ZBkNDHGhr/4g YR5HJAdylFgffJra1HZPARnTZfzAV6MMQoQZYCWHbVBbE90I5HDO3CsDTWTfuEJf 09RSxCoeQnMPPEoI1HlMY30Eo0oBSr/vFt1v8p4zYErURCqsjSfmpJ6SpI+Dk7BY MXyhyFjh3K7+nyceLufCVRvyhEmeiGC5U2mRHa17C4uMixLCcr9p4htzGtCyRJdN lk6oLsG/YAQY5HUw6j6V/0BuT+VqLlQ2H4q7W5exmZxRYrCj0m74H7qsHoVD91mE JEhZ5IdyPtRu8Pu96xn9j7+ni1+0tA02DDV/rxI9OYdRqSBLopQbcmjfqL+GXjX5 7f7rCsgL+4Xk4JXKstneRABsQpJTIVUdRuI+fn/pFgqySSjrFYiBShK90OaJOueW 9S2BEXe2/de9uaHjFEMgUfGt+S0dvgP/GeVjZjfwlzRxZZNfQmJGHROQzzD5nia5 Omh9qaMnK02fRPZ2LXJ+ =Abda -----END PGP SIGNATURE----- --0/kgSOzhNoDC5T3a--