[Qemu-devel] [PATCH v3 1/2] target-ppc: Define kvmppc_read_int_dt()

[Qemu-devel] [PATCH v3 1/2] target-ppc: Define kvmppc_read_int_dt()

[Sukadev Bhattiprolu]

Extract code from the function kvmppc_read_int_cpu_dt() that actually
reads the file into a separate function, so it can be called from
other places.

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
 target-ppc/kvm.c | 36 ++++++++++++++++++++++--------------
 1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index ac70f08..9940a90 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -1838,13 +1838,8 @@ static int kvmppc_find_cpu_dt(char *buf, int buf_len)
     return 0;
 }
 
-/* Read a CPU node property from the host device tree that's a single
- * integer (32-bit or 64-bit).  Returns 0 if anything goes wrong
- * (can't find or open the property, or doesn't understand the
- * format) */
-static uint64_t kvmppc_read_int_cpu_dt(const char *propname)
+static uint64_t kvmppc_read_int_dt(const char *filename)
 {
-    char buf[PATH_MAX], *tmp;
     union {
         uint32_t v32;
         uint64_t v64;
@@ -1852,14 +1847,7 @@ static uint64_t kvmppc_read_int_cpu_dt(const char *propname)
     FILE *f;
     int len;
 
-    if (kvmppc_find_cpu_dt(buf, sizeof(buf))) {
-        return -1;
-    }
-
-    tmp = g_strdup_printf("%s/%s", buf, propname);
-
-    f = fopen(tmp, "rb");
-    g_free(tmp);
+    f = fopen(filename, "rb");
     if (!f) {
         return -1;
     }
@@ -1877,6 +1865,26 @@ static uint64_t kvmppc_read_int_cpu_dt(const char *propname)
     return 0;
 }
 
+/* Read a CPU node property from the host device tree that's a single
+ * integer (32-bit or 64-bit).  Returns 0 if anything goes wrong
+ * (can't find or open the property, or doesn't understand the
+ * format) */
+static uint64_t kvmppc_read_int_cpu_dt(const char *propname)
+{
+    char buf[PATH_MAX], *tmp;
+    uint64_t val;
+
+    if (kvmppc_find_cpu_dt(buf, sizeof(buf))) {
+        return -1;
+    }
+
+    tmp = g_strdup_printf("%s/%s", buf, propname);
+    val = kvmppc_read_int_dt(tmp);
+    g_free(tmp);
+
+    return val;
+}
+
 uint64_t kvmppc_get_clockfreq(void)
 {
     return kvmppc_read_int_cpu_dt("clock-frequency");
-- 
2.1.0

[Qemu-devel] [PATCH v3 2/2] target-ppc: Implement rtas_get_sysparm(PROCESSOR_MODULE_INFO)

[Sukadev Bhattiprolu]

Implement RTAS_SYSPARM_PROCESSOR_MODULE_INFO parameter to rtas_get_sysparm()
call in qemu. This call returns the processor module (socket), chip and core
information as specified in section 7.3.16.17 of LoPAPR v2.7.

We walk the /proc/device-tree to determine the number of modules(aka sockets),
chips and cores in the _host_ system and return this info to the guest
application that makes the rtas_get_sysparm() call.

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.

Thanks to input from Nishanth Aravamudan, Alexey Kardashevskiy, David Gibson,
Thomas Huth.

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
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.

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(+)

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 @@
 
 #include <libfdt.h>
 #include "hw/ppc/spapr_drc.h"
+#include "kvm_ppc.h"
 
 /* #define DEBUG_SPAPR */
 
@@ -240,6 +241,36 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
     target_ulong ret = RTAS_OUT_SUCCESS;
 
     switch (parameter) {
+    case RTAS_SYSPARM_PROCESSOR_MODULE_INFO: {
+        struct sPAPRRTASModuleInfo modinfo;
+        int i, size = sizeof(modinfo), offset = 0;
+
+        memset(&modinfo, 0, size);
+        if (kvmppc_rtas_get_module_info(&modinfo)) {
+            ret = RTAS_OUT_HW_ERROR;
+            break;
+        }
+
+        stw_be_phys(&address_space_memory, buffer+offset, size);
+        offset += 2;
+
+        stw_be_phys(&address_space_memory, buffer+offset, modinfo.module_types);
+        offset += 2;
+
+        for (i = 0; i < modinfo.module_types; i++) {
+            stw_be_phys(&address_space_memory, buffer+offset,
+                        modinfo.si[i].sockets);
+            offset += 2;
+            stw_be_phys(&address_space_memory, buffer+offset,
+                        modinfo.si[i].chips);
+            offset += 2;
+            stw_be_phys(&address_space_memory, buffer+offset,
+                        modinfo.si[i].cores_per_chip);
+            offset += 2;
+        }
+        break;
+    }
+
     case RTAS_SYSPARM_SPLPAR_CHARACTERISTICS: {
         char *param_val = g_strdup_printf("MaxEntCap=%d,"
                                           "DesMem=%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
 
 /* RTAS indicator/sensor types
@@ -646,4 +647,16 @@ int spapr_rng_populate_dt(void *fdt);
  */
 #define SPAPR_LMB_FLAGS_ASSIGNED 0x00000008
 
+#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
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -15,6 +15,7 @@
  */
 
 #include <dirent.h>
+#include <glob.h>
 #include <sys/types.h>
 #include <sys/ioctl.h>
 #include <sys/mman.h>
@@ -2518,3 +2519,143 @@ int kvmppc_enable_hwrng(void)
 
     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 = kvmppc_read_int_dt(path);
+    if (idx == -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 = 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 = -1;
+    htbl = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL, NULL);
+
+    for (i = 0; i < dtglob.gl_pathc; i++) {
+        if (kvmppc_hash_file_contents(htbl, dtglob.gl_pathv[i])) {
+            goto cleanup;
+        }
+    }
+
+    *count = g_hash_table_size(htbl);
+    rc = 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 each
+ * chip's id is contained in the 'ibm,chip-id' file in an xscom directory.
+ *
+ * Since a module can have more than one chip and a chip can have more than
+ * 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_chips)
+{
+    const char *chip_pattern = "/proc/device-tree/xscom@*/ibm,chip-id";
+    const char *module_pattern = "/proc/device-tree/xscom@*/ibm,hw-module-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 == 0 || *num_chips == 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 = "/proc/device-tree/cpus/PowerPC,POWER*";
+
+    rc = 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 = dtglob.gl_pathc;
+    globfree(&dtglob);
+
+    if (*num_cores == 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_types
+     *       for now till we can determine it dynamically.
+     */
+    modinfo->module_types = 1;
+    modinfo->si[0].sockets = sockets;
+    modinfo->si[0].chips = chips;
+    modinfo->si[0].cores_per_chip = 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__
 
 #define TYPE_HOST_POWERPC_CPU "host-" TYPE_POWERPC_CPU
+#include "hw/ppc/spapr.h"
 
 #ifdef CONFIG_KVM
 
@@ -55,6 +56,7 @@ void kvmppc_hash64_write_pte(CPUPPCState *env, target_ulong 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);
 
 #else
 
@@ -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
 
 #ifndef CONFIG_KVM
-- 
2.1.0

Re: [Qemu-devel] [Qemu-ppc] [PATCH v3 2/2] target-ppc: Implement rtas_get_sysparm(PROCESSOR_MODULE_INFO)

[Sukadev Bhattiprolu]

David, Alexey,

Any comments on this patch?

Suka

Sukadev Bhattiprolu [sukadev@linux.vnet.ibm.com] wrote:
| Implement RTAS_SYSPARM_PROCESSOR_MODULE_INFO parameter to rtas_get_sysparm()
| call in qemu. This call returns the processor module (socket), chip and core
| information as specified in section 7.3.16.17 of LoPAPR v2.7.
| 
| We walk the /proc/device-tree to determine the number of modules(aka sockets),
| chips and cores in the _host_ system and return this info to the guest
| application that makes the rtas_get_sysparm() call.
| 
| 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.
| 
| Thanks to input from Nishanth Aravamudan, Alexey Kardashevskiy, David Gibson,
| Thomas Huth.
| 
| Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
| ---
| 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.
| 
| 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(+)
| 
| 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 @@
| 
|  #include <libfdt.h>
|  #include "hw/ppc/spapr_drc.h"
| +#include "kvm_ppc.h"
| 
|  /* #define DEBUG_SPAPR */
| 
| @@ -240,6 +241,36 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
|      target_ulong ret = RTAS_OUT_SUCCESS;
| 
|      switch (parameter) {
| +    case RTAS_SYSPARM_PROCESSOR_MODULE_INFO: {
| +        struct sPAPRRTASModuleInfo modinfo;
| +        int i, size = sizeof(modinfo), offset = 0;
| +
| +        memset(&modinfo, 0, size);
| +        if (kvmppc_rtas_get_module_info(&modinfo)) {
| +            ret = RTAS_OUT_HW_ERROR;
| +            break;
| +        }
| +
| +        stw_be_phys(&address_space_memory, buffer+offset, size);
| +        offset += 2;
| +
| +        stw_be_phys(&address_space_memory, buffer+offset, modinfo.module_types);
| +        offset += 2;
| +
| +        for (i = 0; i < modinfo.module_types; i++) {
| +            stw_be_phys(&address_space_memory, buffer+offset,
| +                        modinfo.si[i].sockets);
| +            offset += 2;
| +            stw_be_phys(&address_space_memory, buffer+offset,
| +                        modinfo.si[i].chips);
| +            offset += 2;
| +            stw_be_phys(&address_space_memory, buffer+offset,
| +                        modinfo.si[i].cores_per_chip);
| +            offset += 2;
| +        }
| +        break;
| +    }
| +
|      case RTAS_SYSPARM_SPLPAR_CHARACTERISTICS: {
|          char *param_val = g_strdup_printf("MaxEntCap=%d,"
|                                            "DesMem=%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
| 
|  /* RTAS indicator/sensor types
| @@ -646,4 +647,16 @@ int spapr_rng_populate_dt(void *fdt);
|   */
|  #define SPAPR_LMB_FLAGS_ASSIGNED 0x00000008
| 
| +#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
| --- a/target-ppc/kvm.c
| +++ b/target-ppc/kvm.c
| @@ -15,6 +15,7 @@
|   */
| 
|  #include <dirent.h>
| +#include <glob.h>
|  #include <sys/types.h>
|  #include <sys/ioctl.h>
|  #include <sys/mman.h>
| @@ -2518,3 +2519,143 @@ int kvmppc_enable_hwrng(void)
| 
|      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 = kvmppc_read_int_dt(path);
| +    if (idx == -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 = 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 = -1;
| +    htbl = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL, NULL);
| +
| +    for (i = 0; i < dtglob.gl_pathc; i++) {
| +        if (kvmppc_hash_file_contents(htbl, dtglob.gl_pathv[i])) {
| +            goto cleanup;
| +        }
| +    }
| +
| +    *count = g_hash_table_size(htbl);
| +    rc = 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 each
| + * chip's id is contained in the 'ibm,chip-id' file in an xscom directory.
| + *
| + * Since a module can have more than one chip and a chip can have more than
| + * 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_chips)
| +{
| +    const char *chip_pattern = "/proc/device-tree/xscom@*/ibm,chip-id";
| +    const char *module_pattern = "/proc/device-tree/xscom@*/ibm,hw-module-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 == 0 || *num_chips == 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 = "/proc/device-tree/cpus/PowerPC,POWER*";
| +
| +    rc = 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 = dtglob.gl_pathc;
| +    globfree(&dtglob);
| +
| +    if (*num_cores == 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_types
| +     *       for now till we can determine it dynamically.
| +     */
| +    modinfo->module_types = 1;
| +    modinfo->si[0].sockets = sockets;
| +    modinfo->si[0].chips = chips;
| +    modinfo->si[0].cores_per_chip = 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__
| 
|  #define TYPE_HOST_POWERPC_CPU "host-" TYPE_POWERPC_CPU
| +#include "hw/ppc/spapr.h"
| 
|  #ifdef CONFIG_KVM
| 
| @@ -55,6 +56,7 @@ void kvmppc_hash64_write_pte(CPUPPCState *env, target_ulong 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);
| 
|  #else
| 
| @@ -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
| 
|  #ifndef CONFIG_KVM
| -- 
| 2.1.0
| 

Re: [Qemu-devel] [Qemu-ppc] [PATCH v3 2/2] target-ppc: Implement rtas_get_sysparm(PROCESSOR_MODULE_INFO)

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 11766 bytes --]

On Tue, Nov 24, 2015 at 10:30:31PM -0800, Sukadev Bhattiprolu wrote:
> David, Alexey,
> 
> Any comments on this patch?

Sorry, I've been busy, I've sent comments now.

> 
> Suka
> 
> Sukadev Bhattiprolu [sukadev@linux.vnet.ibm.com] wrote:
> | Implement RTAS_SYSPARM_PROCESSOR_MODULE_INFO parameter to rtas_get_sysparm()
> | call in qemu. This call returns the processor module (socket), chip and core
> | information as specified in section 7.3.16.17 of LoPAPR v2.7.
> | 
> | We walk the /proc/device-tree to determine the number of modules(aka sockets),
> | chips and cores in the _host_ system and return this info to the guest
> | application that makes the rtas_get_sysparm() call.
> | 
> | 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.
> | 
> | Thanks to input from Nishanth Aravamudan, Alexey Kardashevskiy, David Gibson,
> | Thomas Huth.
> | 
> | Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> | ---
> | 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.
> | 
> | 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(+)
> | 
> | 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 @@
> | 
> |  #include <libfdt.h>
> |  #include "hw/ppc/spapr_drc.h"
> | +#include "kvm_ppc.h"
> | 
> |  /* #define DEBUG_SPAPR */
> | 
> | @@ -240,6 +241,36 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
> |      target_ulong ret = RTAS_OUT_SUCCESS;
> | 
> |      switch (parameter) {
> | +    case RTAS_SYSPARM_PROCESSOR_MODULE_INFO: {
> | +        struct sPAPRRTASModuleInfo modinfo;
> | +        int i, size = sizeof(modinfo), offset = 0;
> | +
> | +        memset(&modinfo, 0, size);
> | +        if (kvmppc_rtas_get_module_info(&modinfo)) {
> | +            ret = RTAS_OUT_HW_ERROR;
> | +            break;
> | +        }
> | +
> | +        stw_be_phys(&address_space_memory, buffer+offset, size);
> | +        offset += 2;
> | +
> | +        stw_be_phys(&address_space_memory, buffer+offset, modinfo.module_types);
> | +        offset += 2;
> | +
> | +        for (i = 0; i < modinfo.module_types; i++) {
> | +            stw_be_phys(&address_space_memory, buffer+offset,
> | +                        modinfo.si[i].sockets);
> | +            offset += 2;
> | +            stw_be_phys(&address_space_memory, buffer+offset,
> | +                        modinfo.si[i].chips);
> | +            offset += 2;
> | +            stw_be_phys(&address_space_memory, buffer+offset,
> | +                        modinfo.si[i].cores_per_chip);
> | +            offset += 2;
> | +        }
> | +        break;
> | +    }
> | +
> |      case RTAS_SYSPARM_SPLPAR_CHARACTERISTICS: {
> |          char *param_val = g_strdup_printf("MaxEntCap=%d,"
> |                                            "DesMem=%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
> | 
> |  /* RTAS indicator/sensor types
> | @@ -646,4 +647,16 @@ int spapr_rng_populate_dt(void *fdt);
> |   */
> |  #define SPAPR_LMB_FLAGS_ASSIGNED 0x00000008
> | 
> | +#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
> | --- a/target-ppc/kvm.c
> | +++ b/target-ppc/kvm.c
> | @@ -15,6 +15,7 @@
> |   */
> | 
> |  #include <dirent.h>
> | +#include <glob.h>
> |  #include <sys/types.h>
> |  #include <sys/ioctl.h>
> |  #include <sys/mman.h>
> | @@ -2518,3 +2519,143 @@ int kvmppc_enable_hwrng(void)
> | 
> |      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 = kvmppc_read_int_dt(path);
> | +    if (idx == -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 = 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 = -1;
> | +    htbl = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL, NULL);
> | +
> | +    for (i = 0; i < dtglob.gl_pathc; i++) {
> | +        if (kvmppc_hash_file_contents(htbl, dtglob.gl_pathv[i])) {
> | +            goto cleanup;
> | +        }
> | +    }
> | +
> | +    *count = g_hash_table_size(htbl);
> | +    rc = 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 each
> | + * chip's id is contained in the 'ibm,chip-id' file in an xscom directory.
> | + *
> | + * Since a module can have more than one chip and a chip can have more than
> | + * 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_chips)
> | +{
> | +    const char *chip_pattern = "/proc/device-tree/xscom@*/ibm,chip-id";
> | +    const char *module_pattern = "/proc/device-tree/xscom@*/ibm,hw-module-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 == 0 || *num_chips == 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 = "/proc/device-tree/cpus/PowerPC,POWER*";
> | +
> | +    rc = 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 = dtglob.gl_pathc;
> | +    globfree(&dtglob);
> | +
> | +    if (*num_cores == 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_types
> | +     *       for now till we can determine it dynamically.
> | +     */
> | +    modinfo->module_types = 1;
> | +    modinfo->si[0].sockets = sockets;
> | +    modinfo->si[0].chips = chips;
> | +    modinfo->si[0].cores_per_chip = 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__
> | 
> |  #define TYPE_HOST_POWERPC_CPU "host-" TYPE_POWERPC_CPU
> | +#include "hw/ppc/spapr.h"
> | 
> |  #ifdef CONFIG_KVM
> | 
> | @@ -55,6 +56,7 @@ void kvmppc_hash64_write_pte(CPUPPCState *env, target_ulong 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);
> | 
> |  #else
> | 
> | @@ -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
> | 
> |  #ifndef CONFIG_KVM
> | -- 
> | 2.1.0
> | 
> 

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH v3 2/2] target-ppc: Implement rtas_get_sysparm(PROCESSOR_MODULE_INFO)

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 11580 bytes --]

On Fri, Nov 13, 2015 at 06:13:08PM -0800, Sukadev Bhattiprolu wrote:
> Implement RTAS_SYSPARM_PROCESSOR_MODULE_INFO parameter to rtas_get_sysparm()
> call in qemu. This call returns the processor module (socket), chip and core
> information as specified in section 7.3.16.17 of LoPAPR v2.7.
> 
> We walk the /proc/device-tree to determine the number of modules(aka sockets),
> chips and cores in the _host_ system and return this info to the guest
> application that makes the rtas_get_sysparm() call.
> 
> 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.
> 
> Thanks to input from Nishanth Aravamudan, Alexey Kardashevskiy, David Gibson,
> Thomas Huth.
> 
> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> ---
> 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.
> 
> 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(+)
> 
> 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 @@
>  
>  #include <libfdt.h>
>  #include "hw/ppc/spapr_drc.h"
> +#include "kvm_ppc.h"
>  
>  /* #define DEBUG_SPAPR */
>  
> @@ -240,6 +241,36 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
>      target_ulong ret = RTAS_OUT_SUCCESS;
>  
>      switch (parameter) {
> +    case RTAS_SYSPARM_PROCESSOR_MODULE_INFO: {
> +        struct sPAPRRTASModuleInfo modinfo;
> +        int i, size = sizeof(modinfo), offset = 0;
> +
> +        memset(&modinfo, 0, size);
> +        if (kvmppc_rtas_get_module_info(&modinfo)) {
> +            ret = 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 += 2;
> +
> +        stw_be_phys(&address_space_memory, buffer+offset, modinfo.module_types);
> +        offset += 2;
> +
> +        for (i = 0; i < modinfo.module_types; i++) {
> +            stw_be_phys(&address_space_memory, buffer+offset,
> +                        modinfo.si[i].sockets);
> +            offset += 2;
> +            stw_be_phys(&address_space_memory, buffer+offset,
> +                        modinfo.si[i].chips);
> +            offset += 2;
> +            stw_be_phys(&address_space_memory, buffer+offset,
> +                        modinfo.si[i].cores_per_chip);
> +            offset += 2;
> +        }
> +        break;
> +    }
> +
>      case RTAS_SYSPARM_SPLPAR_CHARACTERISTICS: {
>          char *param_val = g_strdup_printf("MaxEntCap=%d,"
>                                            "DesMem=%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
>  
>  /* RTAS indicator/sensor types
> @@ -646,4 +647,16 @@ int spapr_rng_populate_dt(void *fdt);
>   */
>  #define SPAPR_LMB_FLAGS_ASSIGNED 0x00000008
>  
> +#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 @@
>   */
>  
>  #include <dirent.h>
> +#include <glob.h>
>  #include <sys/types.h>
>  #include <sys/ioctl.h>
>  #include <sys/mman.h>
> @@ -2518,3 +2519,143 @@ int kvmppc_enable_hwrng(void)
>  
>      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 = kvmppc_read_int_dt(path);
> +    if (idx == -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 = 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 = -1;
> +    htbl = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL, NULL);
> +
> +    for (i = 0; i < dtglob.gl_pathc; i++) {
> +        if (kvmppc_hash_file_contents(htbl, dtglob.gl_pathv[i])) {
> +            goto cleanup;
> +        }
> +    }
> +
> +    *count = g_hash_table_size(htbl);
> +    rc = 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 each
> + * chip's id is contained in the 'ibm,chip-id' file in an xscom directory.
> + *
> + * Since a module can have more than one chip and a chip can have more than
> + * 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_chips)
> +{
> +    const char *chip_pattern = "/proc/device-tree/xscom@*/ibm,chip-id";
> +    const char *module_pattern = "/proc/device-tree/xscom@*/ibm,hw-module-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 == 0 || *num_chips == 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 = "/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 = 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 = dtglob.gl_pathc;
> +    globfree(&dtglob);
> +
> +    if (*num_cores == 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_types
> +     *       for now till we can determine it dynamically.
> +     */
> +    modinfo->module_types = 1;
> +    modinfo->si[0].sockets = sockets;
> +    modinfo->si[0].chips = chips;
> +    modinfo->si[0].cores_per_chip = 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__
>  
>  #define TYPE_HOST_POWERPC_CPU "host-" TYPE_POWERPC_CPU
> +#include "hw/ppc/spapr.h"
>  
>  #ifdef CONFIG_KVM
>  
> @@ -55,6 +56,7 @@ void kvmppc_hash64_write_pte(CPUPPCState *env, target_ulong 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);
>  
>  #else
>  
> @@ -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
>  
>  #ifndef CONFIG_KVM

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH v3 2/2] target-ppc: Implement rtas_get_sysparm(PROCESSOR_MODULE_INFO)

[Sukadev Bhattiprolu]

David Gibson [david@gibson.dropbear.id.au] wrote:
| > @@ -240,6 +241,36 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
| >      target_ulong ret = RTAS_OUT_SUCCESS;
| >  
| >      switch (parameter) {
| > +    case RTAS_SYSPARM_PROCESSOR_MODULE_INFO: {
| > +        struct sPAPRRTASModuleInfo modinfo;
| > +        int i, size = sizeof(modinfo), offset = 0;
| > +
| > +        memset(&modinfo, 0, size);
| > +        if (kvmppc_rtas_get_module_info(&modinfo)) {
| > +            ret = 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.

Ok. Will have kvmppc_rtas_get_module_info() take/update a size parameter
and use that here.

|

<snip>

| > +/* 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 = "/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.

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.

$ cd /proc/device-tree/cpus

$ lsprop l3-cache@30000020/device_type
l3-cache@30000020/device_type
		 "cache"

$ lsprop l2-cache@200008f0/device_type
l2-cache@200008f0/device_type
		 "cache"

$ lsprop PowerPC,POWER8@860/device_type
PowerPC,POWER8@860/device_type
		 "cpu"

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*)?

| 
| 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.

Do you mean rename '/proc/device-tree/cpus/PowerPC,POWER8@NNN' to
/proc/device-tree/cpus/cpu@NNN?

Thanks,

Sukadev

Re: [Qemu-devel] [PATCH v3 2/2] target-ppc: Implement rtas_get_sysparm(PROCESSOR_MODULE_INFO)

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3329 bytes --]

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(PowerPCCPU *cpu,
> | >      target_ulong ret = RTAS_OUT_SUCCESS;
> | >  
> | >      switch (parameter) {
> | > +    case RTAS_SYSPARM_PROCESSOR_MODULE_INFO: {
> | > +        struct sPAPRRTASModuleInfo modinfo;
> | > +        int i, size = sizeof(modinfo), offset = 0;
> | > +
> | > +        memset(&modinfo, 0, size);
> | > +        if (kvmppc_rtas_get_module_info(&modinfo)) {
> | > +            ret = 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.
> 
> Ok. Will have kvmppc_rtas_get_module_info() take/update a size parameter
> and use that here.
> 
> |
> 
> <snip>
> 
> | > +/* 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 = "/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.
> 
> 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.
> 
> $ cd /proc/device-tree/cpus
> 
> $ lsprop l3-cache@30000020/device_type
> l3-cache@30000020/device_type
> 		 "cache"
> 
> $ lsprop l2-cache@200008f0/device_type
> l2-cache@200008f0/device_type
> 		 "cache"
> 
> $ 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.

> | 
> | 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.
> 
> 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.

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [Qemu-ppc] [PATCH v3 2/2] target-ppc: Implement rtas_get_sysparm(PROCESSOR_MODULE_INFO)

[Sukadev Bhattiprolu]

David Gibson [david@gibson.dropbear.id.au] wrote:
| > 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.
| 
Ok. Are we ok with the xscom check I have (copied below):

+static int kvmppc_count_sockets_chips_dt(int *num_sockets, int *num_chips)
+{
+    const char *chip_pattern = "/proc/device-tree/xscom@*/ibm,chip-id";
+    const char *module_pattern = "/proc/device-tree/xscom@*/ibm,hw-module-id";

---

I am not sure of a reliable way to detect number of chips besides the
xscom. I do see that sysfs has node entries:

	for i in `ls -d /sys/bus/cpu/devices/cpu*`; 
	> do
	> basename $i/node*
	> done | sort | uniq
	node0
	node1
	node16
	node17

which seems to correspond to

	$ lsprop /proc/device-tree/xscom@*/ibm,chip-id
	/proc/device-tree/xscom@3fc0000000000/ibm,chip-id
			 00000000
	/proc/device-tree/xscom@3fc0800000000/ibm,chip-id
			 00000001
	/proc/device-tree/xscom@3fc8000000000/ibm,chip-id
			 00000010 (16)
	/proc/device-tree/xscom@3fc8800000000/ibm,chip-id
			 00000011 (17)

but is that reliable?
> +
| > | 
| > | 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.
| > 
| > 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.

Sure. ppc64_cpu also seems to rely on '/proc/device-tree/cpus/PowerPC'
to count cores present and maybe there are other utilities that would
need to be updated.

Sukadev

Re: [Qemu-devel] [PATCH v3 1/2] target-ppc: Define kvmppc_read_int_dt()

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 576 bytes --]

On Fri, Nov 13, 2015 at 06:13:07PM -0800, Sukadev Bhattiprolu wrote:
> Extract code from the function kvmppc_read_int_cpu_dt() that actually
> reads the file into a separate function, so it can be called from
> other places.
> 
> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>

Looks like a nice generalization, I've applied this to my ppc-for-2.6
branch.
-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]