[V5,3/4] cpu: Introduce getHost support for ARM CPU driver

Message ID TY2PR01MB311380AFE294266B4E87B85699BF0@TY2PR01MB3113.jpnprd01.prod.outlook.com
State New
Headers show
Series
  • Introduce getHost support for ARM CPU driver
Related show

Commit Message

Zhenyu Zheng May 13, 2020, 10:48 a.m.
Introduce getHost support for ARM CPU driver,
read CPU vendor_id, part_id and flags from
registers directly. These codes will only be
compiled on aarch64 hardware.

Signed-off-by: Zhenyu Zheng <zheng.zhenyu@outlook.com>

---
 src/cpu/cpu_arm.c | 162 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 162 insertions(+)

-- 
2.20.1

Comments

Jiri Denemark May 15, 2020, 9:51 a.m. | #1
On Wed, May 13, 2020 at 18:48:34 +0800, Zhenyu Zheng wrote:
> Introduce getHost support for ARM CPU driver,

> read CPU vendor_id, part_id and flags from

> registers directly. These codes will only be

> compiled on aarch64 hardware.

> 

> Signed-off-by: Zhenyu Zheng <zheng.zhenyu@outlook.com>

> ---

>  src/cpu/cpu_arm.c | 162 ++++++++++++++++++++++++++++++++++++++++++++++

>  1 file changed, 162 insertions(+)

> 

> diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c

> index c7a6abfb22..6aef568503 100644

> --- a/src/cpu/cpu_arm.c

> +++ b/src/cpu/cpu_arm.c

> @@ -21,6 +21,10 @@

>   */

>  

>  #include <config.h>

> +#if defined(__aarch64__)

> +# include <asm/hwcap.h>

> +# include <sys/auxv.h>

> +#endif

>  

>  #include "viralloc.h"

>  #include "cpu.h"

> @@ -31,6 +35,13 @@

>  #include "virxml.h"

>  

>  #define VIR_FROM_THIS VIR_FROM_CPU

> +#if defined(__aarch64__)

> +/* Shift bit mask for parsing cpu flags */

> +# define BIT_SHIFTS(n) (1UL << (n))

> +/* The current max number of cpu flags on ARM is 32 */

> +# define MAX_CPU_FLAGS 32

> +#endif

> +

>  

>  VIR_LOG_INIT("cpu.cpu_arm");

>  

> @@ -486,11 +497,162 @@ virCPUarmValidateFeatures(virCPUDefPtr cpu)

>      return 0;

>  }

>  

> +#if defined(__aarch64__)

> +/* Generate human readable flag list according to the order of */

> +/* AT_HWCAP bit map */

> +const char *aarch64_cpu_flags[MAX_CPU_FLAGS] = {

> +    "fp", "asimd", "evtstrm", "aes", "pmull", "sha1", "sha2",

> +    "crc32", "atomics", "fphp", "asimdhp", "cpuid", "asimdrdm",

> +    "jscvt", "fcma", "lrcpc", "dcpop", "sha3", "sm3", "sm4",

> +    "asimddp", "sha512", "sve", "asimdfhm", "dit", "uscat",

> +    "ilrcpc", "flagm", "ssbs", "sb", "paca", "pacg"};

> +/**

> + * virCPUarmCpuDataFromRegs:

> + *

> + * @data: 64-bit arm CPU specific data

> + *

> + * Fetches CPU vendor_id and part_id from MIDR_EL1 register, parse CPU

> + * flags from AT_HWCAP. There are currently 32 valid flags  on ARM arch

> + * represented by each bit.

> + */

> +static int

> +virCPUarmCpuDataFromRegs(virCPUarmData *data)

> +{

> +    unsigned long cpuid;

> +    unsigned long hwcaps;

> +    VIR_AUTOSTRINGLIST features = NULL;

> +    int cpu_feature_index = 0;

> +    size_t i;

> +

> +    if (!(getauxval(AT_HWCAP) & HWCAP_CPUID)) {

> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",

> +                       _("CPUID registers unavailable"));

> +            return -1;

> +    }

> +

> +    /* read the cpuid data from MIDR_EL1 register */

> +    asm("mrs %0, MIDR_EL1" : "=r" (cpuid));

> +    VIR_DEBUG("CPUID read from register:  0x%016lx", cpuid);

> +

> +    /* parse the coresponding part_id bits */

> +    data->pvr = (cpuid >> 4) & 0xfff;

> +    /* parse the coresponding vendor_id bits */

> +    data->vendor_id = (cpuid >> 24) & 0xff;

> +

> +    hwcaps = getauxval(AT_HWCAP);

> +    VIR_DEBUG("CPU flags read from register:  0x%016lx", hwcaps);

> +

> +    features = g_new0(char *, MAX_CPU_FLAGS + 1);

> +

> +    /* shift bit map mask to parse for CPU flags */

> +    for (i = 0; i < MAX_CPU_FLAGS; i++) {

> +        if (hwcaps & BIT_SHIFTS(i)) {

> +            features[cpu_feature_index] = g_strdup(aarch64_cpu_flags[i]);

> +            cpu_feature_index++;

> +        }

> +    }

> +

> +    if (cpu_feature_index > 0) {

> +        data->features = g_steal_pointer(&features);

> +        data->nfeatures = cpu_feature_index;


There's no need for data->nfeatures as data->features is
NULL-terminated.

> +    }

> +

> +    return 0;

> +}

> +

> +static int

> +virCPUarmDecode(virCPUDefPtr cpu,

> +                const virCPUarmData *cpuData,

> +                virDomainCapsCPUModelsPtr models)

> +{

> +    size_t i;

> +    virCPUarmMapPtr map;

> +    virCPUarmModelPtr model;

> +    virCPUarmVendorPtr vendor = NULL;

> +

> +    if (!cpuData || !(map = virCPUarmGetMap()))

> +        return -1;

> +

> +    if (!(model = virCPUarmModelFindByPVR(map, cpuData->pvr))) {

> +        virReportError(VIR_ERR_OPERATION_FAILED,

> +                       _("Cannot find CPU model with PVR 0x%03lx"),

> +                       cpuData->pvr);

> +        return -1;

> +    }

> +

> +    if (!virCPUModelIsAllowed(model->name, models)) {

> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,

> +                       _("CPU model %s is not supported by hypervisor"),

> +                       model->name);

> +        return -1;

> +    }

> +

> +    cpu->model = g_strdup(model->name);

> +

> +    if (cpuData->vendor_id &&

> +        !(vendor = virCPUarmVendorFindByID(map, cpuData->vendor_id))) {

> +        virReportError(VIR_ERR_OPERATION_FAILED,

> +                       _("Cannot find CPU vendor with vendor id 0x%02lx"),

> +                       cpuData->vendor_id);

> +        return -1;

> +    }

> +

> +    if (vendor)

> +        cpu->vendor = g_strdup(vendor->name);

> +

> +    if (cpuData->nfeatures) {

> +        cpu->nfeatures = cpuData->nfeatures;

> +        if (VIR_ALLOC_N(cpu->features, cpu->nfeatures) < 0)

> +            goto error;


Even better (and without cpuData->nfeatures):

    if (cpuData->features) {
        cpu->nfeatures = virStringListLength((const char **)cpuData->features);
        cpu->features = g_new0(virCPUFeatureDef, cpu->nfeatures);

> +        for (i = 0; i < cpu->nfeatures; i++) {

> +            cpu->features[i].policy = VIR_CPU_FEATURE_REQUIRE;

> +            cpu->features[i].name = g_strdup(cpuData->features[i]);

> +        }

> +    }

> +

> +    return 0;

> +

> + error:

> +    for (i = 0; i < cpu->nfeatures; i++)

> +        VIR_FREE(cpu->features[i].name);

> +    VIR_FREE(cpu->features);

> +    cpu->nfeatures = 0;

> +    return -1;


The only way we could get here is when allocating cpu->features fails,
which means there's nothing to cleanup. Anyway, we can avoid the only
goto statement by using g_new0 and drop this section entirely.

> +}

> +

> +static int

> +virCPUarmGetHost(virCPUDefPtr cpu,

> +                 virDomainCapsCPUModelsPtr models)

> +{

> +    virCPUDataPtr cpuData = NULL;


I forgot to mention this in my previous review, but this function would
benefit from using g_autoptr as we could just drop the cleanup section
and return directly in call cases:

    g_autoptr(virCPUData) cpuData = NULL;

> +    int ret = -1;


Not needed anymore.

> +

> +    if (virCPUarmDriverInitialize() < 0)

> +        goto cleanup;


return -1;

> +

> +    if (!(cpuData = virCPUDataNew(archs[0])))

> +        goto cleanup;


return -1;

> +

> +    if (virCPUarmCpuDataFromRegs(&cpuData->data.arm) < 0)

> +        goto cleanup;


return -1;

> +

> +    ret = virCPUarmDecode(cpu, &cpuData->data.arm, models);


return virCPUarmDecode(cpu, &cpuData->data.arm, models);

> +

> + cleanup:

> +    virCPUarmDataFree(cpuData);

> +    return ret;

> +}


Not needed anymore.

> +#endif

> +

> +

>  struct cpuArchDriver cpuDriverArm = {

>      .name = "arm",

>      .arch = archs,

>      .narch = G_N_ELEMENTS(archs),

>      .compare = virCPUarmCompare,

> +#if defined(__aarch64__)

> +    .getHost = virCPUarmGetHost,

> +#endif

>      .decode = NULL,

>      .encode = NULL,

>      .dataFree = virCPUarmDataFree,


Jirka

Patch

diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c
index c7a6abfb22..6aef568503 100644
--- a/src/cpu/cpu_arm.c
+++ b/src/cpu/cpu_arm.c
@@ -21,6 +21,10 @@ 
  */
 
 #include <config.h>
+#if defined(__aarch64__)
+# include <asm/hwcap.h>
+# include <sys/auxv.h>
+#endif
 
 #include "viralloc.h"
 #include "cpu.h"
@@ -31,6 +35,13 @@ 
 #include "virxml.h"
 
 #define VIR_FROM_THIS VIR_FROM_CPU
+#if defined(__aarch64__)
+/* Shift bit mask for parsing cpu flags */
+# define BIT_SHIFTS(n) (1UL << (n))
+/* The current max number of cpu flags on ARM is 32 */
+# define MAX_CPU_FLAGS 32
+#endif
+
 
 VIR_LOG_INIT("cpu.cpu_arm");
 
@@ -486,11 +497,162 @@  virCPUarmValidateFeatures(virCPUDefPtr cpu)
     return 0;
 }
 
+#if defined(__aarch64__)
+/* Generate human readable flag list according to the order of */
+/* AT_HWCAP bit map */
+const char *aarch64_cpu_flags[MAX_CPU_FLAGS] = {
+    "fp", "asimd", "evtstrm", "aes", "pmull", "sha1", "sha2",
+    "crc32", "atomics", "fphp", "asimdhp", "cpuid", "asimdrdm",
+    "jscvt", "fcma", "lrcpc", "dcpop", "sha3", "sm3", "sm4",
+    "asimddp", "sha512", "sve", "asimdfhm", "dit", "uscat",
+    "ilrcpc", "flagm", "ssbs", "sb", "paca", "pacg"};
+/**
+ * virCPUarmCpuDataFromRegs:
+ *
+ * @data: 64-bit arm CPU specific data
+ *
+ * Fetches CPU vendor_id and part_id from MIDR_EL1 register, parse CPU
+ * flags from AT_HWCAP. There are currently 32 valid flags  on ARM arch
+ * represented by each bit.
+ */
+static int
+virCPUarmCpuDataFromRegs(virCPUarmData *data)
+{
+    unsigned long cpuid;
+    unsigned long hwcaps;
+    VIR_AUTOSTRINGLIST features = NULL;
+    int cpu_feature_index = 0;
+    size_t i;
+
+    if (!(getauxval(AT_HWCAP) & HWCAP_CPUID)) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("CPUID registers unavailable"));
+            return -1;
+    }
+
+    /* read the cpuid data from MIDR_EL1 register */
+    asm("mrs %0, MIDR_EL1" : "=r" (cpuid));
+    VIR_DEBUG("CPUID read from register:  0x%016lx", cpuid);
+
+    /* parse the coresponding part_id bits */
+    data->pvr = (cpuid >> 4) & 0xfff;
+    /* parse the coresponding vendor_id bits */
+    data->vendor_id = (cpuid >> 24) & 0xff;
+
+    hwcaps = getauxval(AT_HWCAP);
+    VIR_DEBUG("CPU flags read from register:  0x%016lx", hwcaps);
+
+    features = g_new0(char *, MAX_CPU_FLAGS + 1);
+
+    /* shift bit map mask to parse for CPU flags */
+    for (i = 0; i < MAX_CPU_FLAGS; i++) {
+        if (hwcaps & BIT_SHIFTS(i)) {
+            features[cpu_feature_index] = g_strdup(aarch64_cpu_flags[i]);
+            cpu_feature_index++;
+        }
+    }
+
+    if (cpu_feature_index > 0) {
+        data->features = g_steal_pointer(&features);
+        data->nfeatures = cpu_feature_index;
+    }
+
+    return 0;
+}
+
+static int
+virCPUarmDecode(virCPUDefPtr cpu,
+                const virCPUarmData *cpuData,
+                virDomainCapsCPUModelsPtr models)
+{
+    size_t i;
+    virCPUarmMapPtr map;
+    virCPUarmModelPtr model;
+    virCPUarmVendorPtr vendor = NULL;
+
+    if (!cpuData || !(map = virCPUarmGetMap()))
+        return -1;
+
+    if (!(model = virCPUarmModelFindByPVR(map, cpuData->pvr))) {
+        virReportError(VIR_ERR_OPERATION_FAILED,
+                       _("Cannot find CPU model with PVR 0x%03lx"),
+                       cpuData->pvr);
+        return -1;
+    }
+
+    if (!virCPUModelIsAllowed(model->name, models)) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("CPU model %s is not supported by hypervisor"),
+                       model->name);
+        return -1;
+    }
+
+    cpu->model = g_strdup(model->name);
+
+    if (cpuData->vendor_id &&
+        !(vendor = virCPUarmVendorFindByID(map, cpuData->vendor_id))) {
+        virReportError(VIR_ERR_OPERATION_FAILED,
+                       _("Cannot find CPU vendor with vendor id 0x%02lx"),
+                       cpuData->vendor_id);
+        return -1;
+    }
+
+    if (vendor)
+        cpu->vendor = g_strdup(vendor->name);
+
+    if (cpuData->nfeatures) {
+        cpu->nfeatures = cpuData->nfeatures;
+        if (VIR_ALLOC_N(cpu->features, cpu->nfeatures) < 0)
+            goto error;
+        for (i = 0; i < cpu->nfeatures; i++) {
+            cpu->features[i].policy = VIR_CPU_FEATURE_REQUIRE;
+            cpu->features[i].name = g_strdup(cpuData->features[i]);
+        }
+    }
+
+    return 0;
+
+ error:
+    for (i = 0; i < cpu->nfeatures; i++)
+        VIR_FREE(cpu->features[i].name);
+    VIR_FREE(cpu->features);
+    cpu->nfeatures = 0;
+    return -1;
+}
+
+static int
+virCPUarmGetHost(virCPUDefPtr cpu,
+                 virDomainCapsCPUModelsPtr models)
+{
+    virCPUDataPtr cpuData = NULL;
+    int ret = -1;
+
+    if (virCPUarmDriverInitialize() < 0)
+        goto cleanup;
+
+    if (!(cpuData = virCPUDataNew(archs[0])))
+        goto cleanup;
+
+    if (virCPUarmCpuDataFromRegs(&cpuData->data.arm) < 0)
+        goto cleanup;
+
+    ret = virCPUarmDecode(cpu, &cpuData->data.arm, models);
+
+ cleanup:
+    virCPUarmDataFree(cpuData);
+    return ret;
+}
+#endif
+
+
 struct cpuArchDriver cpuDriverArm = {
     .name = "arm",
     .arch = archs,
     .narch = G_N_ELEMENTS(archs),
     .compare = virCPUarmCompare,
+#if defined(__aarch64__)
+    .getHost = virCPUarmGetHost,
+#endif
     .decode = NULL,
     .encode = NULL,
     .dataFree = virCPUarmDataFree,