On Thu, Apr 02, 2020 at 05:03:55PM +0800, Zhenyu Zheng wrote:

Introduce vendor and model struct and related cleanup functions for ARM cpu.

Signed-off-by: Zhenyu Zheng <zhengzhenyulixi@gmail.com> --- src/cpu/cpu_arm.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+)

diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c index ee5802198f..c757c24a37 100644 --- a/src/cpu/cpu_arm.c +++ b/src/cpu/cpu_arm.c @@ -1,6 +1,7 @@ /* * cpu_arm.c: CPU driver for arm CPUs * + * Copyright (C) 2020 Huawei Technologies Co., Ltd. * Copyright (C) 2013 Red Hat, Inc. * Copyright (C) Canonical Ltd. 2012 * @@ -23,12 +24,16 @@

#include "viralloc.h" #include "cpu.h" +#include "cpu_arm.h" #include "cpu_map.h" +#include "virlog.h" #include "virstring.h" #include "virxml.h"

#define VIR_FROM_THIS VIR_FROM_CPU

+VIR_LOG_INIT("cpu.cpu_arm"); + static const virArch archs[] = { VIR_ARCH_ARMV6L, VIR_ARCH_ARMV7B, @@ -36,6 +41,21 @@ static const virArch archs[] = { VIR_ARCH_AARCH64, };

+typedef struct _virCPUarmVendor virCPUarmVendor; +typedef virCPUarmVendor *virCPUarmVendorPtr; +struct _virCPUarmVendor { + char *name; + unsigned long value; +}; + +typedef struct _virCPUarmModel virCPUarmModel; +typedef virCPUarmModel *virCPUarmModelPtr; +struct _virCPUarmModel { + char *name; + virCPUarmVendorPtr vendor; + virCPUarmData data; +}; + typedef struct _virCPUarmFeature virCPUarmFeature; typedef virCPUarmFeature *virCPUarmFeaturePtr; struct _virCPUarmFeature { @@ -64,6 +84,10 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(virCPUarmFeature, virCPUarmFeatureFree); typedef struct _virCPUarmMap virCPUarmMap; typedef virCPUarmMap *virCPUarmMapPtr; struct _virCPUarmMap { + size_t nvendors; + virCPUarmVendorPtr *vendors; + size_t nmodels; + virCPUarmModelPtr *models; GPtrArray *features; };

@@ -81,12 +105,62 @@ virCPUarmMapNew(void) return map; }

+static void +virCPUarmDataClear(virCPUarmData *data) +{ + if (!data) + return; + + VIR_FREE(data->features); +} + +static void +virCPUarmDataFree(virCPUDataPtr cpuData) +{ + if (!cpuData) + return; + + virCPUarmDataClear(&cpuData->data.arm); + VIR_FREE(cpuData); +} + +static void +armModelFree(virCPUarmModelPtr model)

Normally we want the method name prefix match the struct name eg virCPUarmModelFree()

+{ + if (!model) + return; + + virCPUarmDataClear(&model->data); + g_free(model->name); + g_free(model); +} + +static void +armVendorFree(virCPUarmVendorPtr vendor)

virCPUarmVendorFree()

+{ + if (!vendor) + return; + + g_free(vendor->name); + g_free(vendor); +} +

With those two changes Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

Introduce vendor and model struct and related cleanup functions for ARM cpu. Signed-off-by: Zhenyu Zheng <zhengzhenyulixi@gmail.com> --- src/cpu/cpu_arm.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c index ee5802198f..d8f571cae3 100644 --- a/src/cpu/cpu_arm.c +++ b/src/cpu/cpu_arm.c @@ -1,6 +1,7 @@ /* * cpu_arm.c: CPU driver for arm CPUs * + * Copyright (C) 2020 Huawei Technologies Co., Ltd. * Copyright (C) 2013 Red Hat, Inc. * Copyright (C) Canonical Ltd. 2012 * @@ -23,12 +24,16 @@ #include "viralloc.h" #include "cpu.h" +#include "cpu_arm.h" #include "cpu_map.h" +#include "virlog.h" #include "virstring.h" #include "virxml.h" #define VIR_FROM_THIS VIR_FROM_CPU +VIR_LOG_INIT("cpu.cpu_arm"); + static const virArch archs[] = { VIR_ARCH_ARMV6L, VIR_ARCH_ARMV7B, @@ -36,6 +41,21 @@ static const virArch archs[] = { VIR_ARCH_AARCH64, }; +typedef struct _virCPUarmVendor virCPUarmVendor; +typedef virCPUarmVendor *virCPUarmVendorPtr; +struct _virCPUarmVendor { + char *name; + unsigned long value; +}; + +typedef struct _virCPUarmModel virCPUarmModel; +typedef virCPUarmModel *virCPUarmModelPtr; +struct _virCPUarmModel { + char *name; + virCPUarmVendorPtr vendor; + virCPUarmData data; +}; + typedef struct _virCPUarmFeature virCPUarmFeature; typedef virCPUarmFeature *virCPUarmFeaturePtr; struct _virCPUarmFeature { @@ -64,6 +84,10 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(virCPUarmFeature, virCPUarmFeatureFree); typedef struct _virCPUarmMap virCPUarmMap; typedef virCPUarmMap *virCPUarmMapPtr; struct _virCPUarmMap { + size_t nvendors; + virCPUarmVendorPtr *vendors; + size_t nmodels; + virCPUarmModelPtr *models; GPtrArray *features; }; @@ -81,12 +105,62 @@ virCPUarmMapNew(void) return map; } +static void +virCPUarmDataClear(virCPUarmData *data) +{ + if (!data) + return; + + VIR_FREE(data->features); +} + +static void +virCPUarmDataFree(virCPUDataPtr cpuData) +{ + if (!cpuData) + return; + + virCPUarmDataClear(&cpuData->data.arm); + VIR_FREE(cpuData); +} + +static void +virCPUarmModelFree(virCPUarmModelPtr model) +{ + if (!model) + return; + + virCPUarmDataClear(&model->data); + g_free(model->name); + g_free(model); +} + +static void +virCPUarmVendorFree(virCPUarmVendorPtr vendor) +{ + if (!vendor) + return; + + g_free(vendor->name); + g_free(vendor); +} + static void virCPUarmMapFree(virCPUarmMapPtr map) { if (!map) return; + size_t i; + + for (i = 0; i < map->nmodels; i++) + virCPUarmModelFree(map->models[i]); + g_free(map->models); + + for (i = 0; i < map->nvendors; i++) + virCPUarmVendorFree(map->vendors[i]); + g_free(map->vendors); + g_ptr_array_free(map->features, TRUE); g_free(map); @@ -259,6 +333,7 @@ struct cpuArchDriver cpuDriverArm = { .compare = virCPUarmCompare, .decode = NULL, .encode = NULL, + .dataFree = virCPUarmDataFree, .baseline = virCPUarmBaseline, .update = virCPUarmUpdate, .validateFeatures = virCPUarmValidateFeatures, -- 2.26.0.windows.1

On Thu, Apr 02, 2020 at 05:03:57PM +0800, Zhenyu Zheng wrote:

Add helper functions to parse vendor and model from xml for ARM arch, and use them as callbacks when load cpu maps.

Signed-off-by: Zhenyu Zheng <zhengzhenyulixi@gmail.com> --- src/cpu/cpu_arm.c | 176 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 174 insertions(+), 2 deletions(-)

diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c index c757c24a37..88b4d91946 100644 --- a/src/cpu/cpu_arm.c +++ b/src/cpu/cpu_arm.c @@ -206,6 +206,178 @@ virCPUarmMapFeatureParse(xmlXPathContextPtr ctxt G_GNUC_UNUSED, return 0; }

+static virCPUarmVendorPtr +armVendorFindByID(virCPUarmMapPtr map, + unsigned long vendor_id)

As with previous patch, we'd expect the name to be virCPUarmVendorFindByID() and apply same naming scheme for other methods in this patch, so I won't repeat this.

+static int +armVendorParse(xmlXPathContextPtr ctxt, + const char *name, + void *data) +{ + virCPUarmMapPtr map = (virCPUarmMapPtr)data;

No need for the (virCPUarmMapPtr) cast, as 'void *' will happily cast to any other pointer in C. Only C++ needs the explicit casts.

+ virCPUarmVendorPtr vendor = NULL; + int ret = -1; + + if (VIR_ALLOC(vendor) < 0) + return ret; + + vendor->name = g_strdup(name); + + if (armVendorFindByName(map, vendor->name)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("CPU vendor %s already defined"), + vendor->name); + goto cleanup; + } + + if (virXPathULongHex("string(@value)", ctxt, &vendor->value) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Missing CPU vendor value")); + goto cleanup; + } + + if (armVendorFindByID(map, vendor->value)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("CPU vendor value 0x%2lx already defined"), + vendor->value); + goto cleanup; + } + + if (VIR_APPEND_ELEMENT(map->vendors, map->nvendors, vendor) < 0) + goto cleanup; + + ret = 0; + + cleanup: + armVendorFree(vendor); + return ret; + +}

+static int +armModelParse(xmlXPathContextPtr ctxt, + const char *name, + void *data) +{ + virCPUarmMapPtr map = (virCPUarmMapPtr)data;

Again, no need for a cast

+ virCPUarmModel *model; + xmlNodePtr *nodes = NULL;

g_autofree xmlNodePtr *nodes = NULL;

+ char *vendor = NULL;

g_autofree char *vendor = NULL;

+ int ret = -1; + + if (VIR_ALLOC(model) < 0) + goto error; + + model->name = g_strdup(name); + + if (armModelFind(map, model->name)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("CPU model %s already defined"), + model->name); + goto error; + } + + if (virXPathBoolean("boolean(./vendor)", ctxt)) { + vendor = virXPathString("string(./vendor/@name)", ctxt); + if (!vendor) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid vendor element in CPU model %s"), + model->name); + goto error; + } + + if (!(model->vendor = armVendorFindByName(map, vendor))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown vendor %s referenced by CPU model %s"), + vendor, model->name); + goto error; + } + } + + if (!virXPathBoolean("boolean(./pvr)", ctxt)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Missing PVR information for CPU model %s"), + model->name); + goto error; + } + + if (virXPathULongHex("string(./pvr/@value)", ctxt, &model->data.pvr) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Missing or invalid PVR value in CPU model %s"), + model->name); + goto error; + } + + if (VIR_APPEND_ELEMENT(map->models, map->nmodels, model) < 0) + goto error; + + ret = 0; + + cleanup: + VIR_FREE(vendor); + VIR_FREE(nodes);

...you can drop these two thanks to the g_autofree, as well as the "cleanup:" label and "ret" variable.

+ return ret;

"return 0";

+ + error: + armModelFree(model); + goto cleanup;

...and just "return -1" here instead of goto

+} + static virCPUarmMapPtr virCPUarmLoadMap(void) { @@ -213,8 +385,8 @@ virCPUarmLoadMap(void)

map = virCPUarmMapNew();

- if (cpuMapLoad("arm", NULL, virCPUarmMapFeatureParse, NULL, map) < 0) - return NULL; + if (cpuMapLoad("arm", armVendorParse, virCPUarmMapFeatureParse, + armModelParse, map) < 0)

return g_steal_pointer(&map); } -- 2.26.0.windows.1

Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

Subject:[PATCH V2 4/5] cpu: Add helper funtions to parse vendor and model Add helper functions to parse vendor and model from xml for ARM arch, and use them as callbacks when load cpu maps. Signed-off-by: Zhenyu Zheng <zhengzhenyulixi@gmail.com> --- src/cpu/cpu_arm.c | 171 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 170 insertions(+), 1 deletion(-) diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c index d8f571cae3..24404eac2c 100644 --- a/src/cpu/cpu_arm.c +++ b/src/cpu/cpu_arm.c @@ -206,6 +206,174 @@ virCPUarmMapFeatureParse(xmlXPathContextPtr ctxt G_GNUC_UNUSED, return 0; } +static virCPUarmVendorPtr +virCPUarmVendorFindByID(virCPUarmMapPtr map, + unsigned long vendor_id) +{ + size_t i; + + for (i = 0; i < map->nvendors; i++) { + if (map->vendors[i]->value == vendor_id) + return map->vendors[i]; + } + + return NULL; +} + + +static virCPUarmVendorPtr +virCPUarmVendorFindByName(virCPUarmMapPtr map, + const char *name) +{ + size_t i; + + for (i = 0; i < map->nvendors; i++) { + if (STREQ(map->vendors[i]->name, name)) + return map->vendors[i]; + } + + return NULL; +} + + +static int +virCPUarmVendorParse(xmlXPathContextPtr ctxt, + const char *name, + void *data) +{ + virCPUarmMapPtr map = data; + virCPUarmVendorPtr vendor = NULL; + int ret = -1; + + if (VIR_ALLOC(vendor) < 0) + return ret; + + vendor->name = g_strdup(name); + + if (virCPUarmVendorFindByName(map, vendor->name)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("CPU vendor %s already defined"), + vendor->name); + goto cleanup; + } + + if (virXPathULongHex("string(@value)", ctxt, &vendor->value) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Missing CPU vendor value")); + goto cleanup; + } + + if (virCPUarmVendorFindByID(map, vendor->value)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("CPU vendor value 0x%2lx already defined"), + vendor->value); + goto cleanup; + } + + if (VIR_APPEND_ELEMENT(map->vendors, map->nvendors, vendor) < 0) + goto cleanup; + + ret = 0; + + cleanup: + virCPUarmVendorFree(vendor); + return ret; + +} + +static virCPUarmModelPtr +virCPUarmModelFind(virCPUarmMapPtr map, + const char *name) +{ + size_t i; + + for (i = 0; i < map->nmodels; i++) { + if (STREQ(map->models[i]->name, name)) + return map->models[i]; + } + + return NULL; +} + +#if defined(__aarch64__) +static virCPUarmModelPtr +virCPUarmModelFindByPVR(virCPUarmMapPtr map, + unsigned long pvr) +{ + size_t i; + + for (i = 0; i < map->nmodels; i++) { + if (map->models[i]->data.pvr == pvr) + return map->models[i]; + } + + return NULL; +} +#endif + +static int +virCPUarmModelParse(xmlXPathContextPtr ctxt, + const char *name, + void *data) +{ + virCPUarmMapPtr map = data; + virCPUarmModel *model; + g_autofree xmlNodePtr *nodes = NULL; + g_autofree char *vendor = NULL; + + if (VIR_ALLOC(model) < 0) + goto error; + + model->name = g_strdup(name); + + if (virCPUarmModelFind(map, model->name)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("CPU model %s already defined"), + model->name); + goto error; + } + + if (virXPathBoolean("boolean(./vendor)", ctxt)) { + vendor = virXPathString("string(./vendor/@name)", ctxt); + if (!vendor) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid vendor element in CPU model %s"), + model->name); + goto error; + } + + if (!(model->vendor = virCPUarmVendorFindByName(map, vendor))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown vendor %s referenced by CPU model %s"), + vendor, model->name); + goto error; + } + } + + if (!virXPathBoolean("boolean(./pvr)", ctxt)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Missing PVR information for CPU model %s"), + model->name); + goto error; + } + + if (virXPathULongHex("string(./pvr/@value)", ctxt, &model->data.pvr) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Missing or invalid PVR value in CPU model %s"), + model->name); + goto error; + } + + if (VIR_APPEND_ELEMENT(map->models, map->nmodels, model) < 0) + goto error; + + return 0; + + error: + virCPUarmModelFree(model); + return -1; +} + static virCPUarmMapPtr virCPUarmLoadMap(void) { @@ -213,7 +381,8 @@ virCPUarmLoadMap(void) map = virCPUarmMapNew(); - if (cpuMapLoad("arm", NULL, virCPUarmMapFeatureParse, NULL, map) < 0) + if (cpuMapLoad("arm", virCPUarmVendorParse, virCPUarmMapFeatureParse, + virCPUarmModelParse, map) < 0) return NULL; return g_steal_pointer(&map); -- 2.26.0.windows.1

ping for reviews On Fri, Apr 10, 2020 at 3:58 PM Zhenyu Zheng <zhengzhenyulixi@gmail.com> wrote:

...

Add helper functions to parse vendor and model from xml for ARM arch, and use them as callbacks when load cpu maps.

Signed-off-by: Zhenyu Zheng <zhengzhenyulixi@gmail.com>

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

diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c index d8f571cae3..24404eac2c 100644 --- a/src/cpu/cpu_arm.c +++ b/src/cpu/cpu_arm.c @@ -206,6 +206,174 @@ virCPUarmMapFeatureParse(xmlXPathContextPtr ctxt G_GNUC_UNUSED, return 0; }

+static virCPUarmVendorPtr +virCPUarmVendorFindByID(virCPUarmMapPtr map, + unsigned long vendor_id) +{ + size_t i; + + for (i = 0; i < map->nvendors; i++) { + if (map->vendors[i]->value == vendor_id) + return map->vendors[i]; + } + + return NULL; +} + + +static virCPUarmVendorPtr +virCPUarmVendorFindByName(virCPUarmMapPtr map, + const char *name) +{ + size_t i; + + for (i = 0; i < map->nvendors; i++) { + if (STREQ(map->vendors[i]->name, name)) + return map->vendors[i]; + } + + return NULL; +} + + +static int +virCPUarmVendorParse(xmlXPathContextPtr ctxt, + const char *name, + void *data) +{ + virCPUarmMapPtr map = data; + virCPUarmVendorPtr vendor = NULL; + int ret = -1; + + if (VIR_ALLOC(vendor) < 0) + return ret; + + vendor->name = g_strdup(name); + + if (virCPUarmVendorFindByName(map, vendor->name)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("CPU vendor %s already defined"), + vendor->name); + goto cleanup; + } + + if (virXPathULongHex("string(@value)", ctxt, &vendor->value) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Missing CPU vendor value")); + goto cleanup; + } + + if (virCPUarmVendorFindByID(map, vendor->value)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("CPU vendor value 0x%2lx already defined"), + vendor->value); + goto cleanup; + } + + if (VIR_APPEND_ELEMENT(map->vendors, map->nvendors, vendor) < 0) + goto cleanup; + + ret = 0; + + cleanup: + virCPUarmVendorFree(vendor); + return ret; + +} + +static virCPUarmModelPtr +virCPUarmModelFind(virCPUarmMapPtr map, + const char *name) +{ + size_t i; + + for (i = 0; i < map->nmodels; i++) { + if (STREQ(map->models[i]->name, name)) + return map->models[i]; + } + + return NULL; +} + +#if defined(__aarch64__) +static virCPUarmModelPtr +virCPUarmModelFindByPVR(virCPUarmMapPtr map, + unsigned long pvr) +{ + size_t i; + + for (i = 0; i < map->nmodels; i++) { + if (map->models[i]->data.pvr == pvr) + return map->models[i]; + } + + return NULL; +} +#endif + +static int +virCPUarmModelParse(xmlXPathContextPtr ctxt, + const char *name, + void *data) +{ + virCPUarmMapPtr map = data; + virCPUarmModel *model; + g_autofree xmlNodePtr *nodes = NULL; + g_autofree char *vendor = NULL; + + if (VIR_ALLOC(model) < 0) + goto error; + + model->name = g_strdup(name); + + if (virCPUarmModelFind(map, model->name)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("CPU model %s already defined"), + model->name); + goto error; + } + + if (virXPathBoolean("boolean(./vendor)", ctxt)) { + vendor = virXPathString("string(./vendor/@name)", ctxt); + if (!vendor) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid vendor element in CPU model %s"), + model->name); + goto error; + } + + if (!(model->vendor = virCPUarmVendorFindByName(map, vendor))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown vendor %s referenced by CPU model %s"), + vendor, model->name); + goto error; + } + } + + if (!virXPathBoolean("boolean(./pvr)", ctxt)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Missing PVR information for CPU model %s"), + model->name); + goto error; + } + + if (virXPathULongHex("string(./pvr/@value)", ctxt, &model->data.pvr) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Missing or invalid PVR value in CPU model %s"), + model->name); + goto error; + } + + if (VIR_APPEND_ELEMENT(map->models, map->nmodels, model) < 0) + goto error; + + return 0; + + error: + virCPUarmModelFree(model); + return -1; +} + static virCPUarmMapPtr virCPUarmLoadMap(void) { @@ -213,7 +381,8 @@ virCPUarmLoadMap(void)

map = virCPUarmMapNew();

- if (cpuMapLoad("arm", NULL, virCPUarmMapFeatureParse, NULL, map) < 0) + if (cpuMapLoad("arm", virCPUarmVendorParse, virCPUarmMapFeatureParse, + virCPUarmModelParse, map) < 0) return NULL;

return g_steal_pointer(&map); -- 2.26.0.windows.1

Add helper functions to parse vendor and model from xml for ARM arch, and use them as callbacks when load cpu maps. Signed-off-by: Zhenyu Zheng <zhengzhenyulixi@gmail.com> --- src/cpu/cpu_arm.c | 171 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 170 insertions(+), 1 deletion(-) diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c index d8f571cae3..24404eac2c 100644 --- a/src/cpu/cpu_arm.c +++ b/src/cpu/cpu_arm.c @@ -206,6 +206,174 @@ virCPUarmMapFeatureParse(xmlXPathContextPtr ctxt G_GNUC_UNUSED, return 0; } +static virCPUarmVendorPtr +virCPUarmVendorFindByID(virCPUarmMapPtr map, + unsigned long vendor_id) +{ + size_t i; + + for (i = 0; i < map->nvendors; i++) { + if (map->vendors[i]->value == vendor_id) + return map->vendors[i]; + } + + return NULL; +} + + +static virCPUarmVendorPtr +virCPUarmVendorFindByName(virCPUarmMapPtr map, + const char *name) +{ + size_t i; + + for (i = 0; i < map->nvendors; i++) { + if (STREQ(map->vendors[i]->name, name)) + return map->vendors[i]; + } + + return NULL; +} + + +static int +virCPUarmVendorParse(xmlXPathContextPtr ctxt, + const char *name, + void *data) +{ + virCPUarmMapPtr map = data; + virCPUarmVendorPtr vendor = NULL; + int ret = -1; + + if (VIR_ALLOC(vendor) < 0) + return ret; + + vendor->name = g_strdup(name); + + if (virCPUarmVendorFindByName(map, vendor->name)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("CPU vendor %s already defined"), + vendor->name); + goto cleanup; + } + + if (virXPathULongHex("string(@value)", ctxt, &vendor->value) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Missing CPU vendor value")); + goto cleanup; + } + + if (virCPUarmVendorFindByID(map, vendor->value)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("CPU vendor value 0x%2lx already defined"), + vendor->value); + goto cleanup; + } + + if (VIR_APPEND_ELEMENT(map->vendors, map->nvendors, vendor) < 0) + goto cleanup; + + ret = 0; + + cleanup: + virCPUarmVendorFree(vendor); + return ret; + +} + +static virCPUarmModelPtr +virCPUarmModelFind(virCPUarmMapPtr map, + const char *name) +{ + size_t i; + + for (i = 0; i < map->nmodels; i++) { + if (STREQ(map->models[i]->name, name)) + return map->models[i]; + } + + return NULL; +} + +#if defined(__aarch64__) +static virCPUarmModelPtr +virCPUarmModelFindByPVR(virCPUarmMapPtr map, + unsigned long pvr) +{ + size_t i; + + for (i = 0; i < map->nmodels; i++) { + if (map->models[i]->data.pvr == pvr) + return map->models[i]; + } + + return NULL; +} +#endif + +static int +virCPUarmModelParse(xmlXPathContextPtr ctxt, + const char *name, + void *data) +{ + virCPUarmMapPtr map = data; + virCPUarmModel *model; + g_autofree xmlNodePtr *nodes = NULL; + g_autofree char *vendor = NULL; + + if (VIR_ALLOC(model) < 0) + goto error; + + model->name = g_strdup(name); + + if (virCPUarmModelFind(map, model->name)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("CPU model %s already defined"), + model->name); + goto error; + } + + if (virXPathBoolean("boolean(./vendor)", ctxt)) { + vendor = virXPathString("string(./vendor/@name)", ctxt); + if (!vendor) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid vendor element in CPU model %s"), + model->name); + goto error; + } + + if (!(model->vendor = virCPUarmVendorFindByName(map, vendor))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown vendor %s referenced by CPU model %s"), + vendor, model->name); + goto error; + } + } + + if (!virXPathBoolean("boolean(./pvr)", ctxt)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Missing PVR information for CPU model %s"), + model->name); + goto error; + } + + if (virXPathULongHex("string(./pvr/@value)", ctxt, &model->data.pvr) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Missing or invalid PVR value in CPU model %s"), + model->name); + goto error; + } + + if (VIR_APPEND_ELEMENT(map->models, map->nmodels, model) < 0) + goto error; + + return 0; + + error: + virCPUarmModelFree(model); + return -1; +} + static virCPUarmMapPtr virCPUarmLoadMap(void) { @@ -213,7 +381,8 @@ virCPUarmLoadMap(void) map = virCPUarmMapNew(); - if (cpuMapLoad("arm", NULL, virCPUarmMapFeatureParse, NULL, map) < 0) + if (cpuMapLoad("arm", virCPUarmVendorParse, virCPUarmMapFeatureParse, + virCPUarmModelParse, map) < 0) return NULL; return g_steal_pointer(&map); -- 2.26.0.windows.1

On Thu, Apr 02, 2020 at 05:03:59PM +0800, Zhenyu Zheng wrote:

Introduce getHost support for ARM CPU driver, read CPU vendor_id, part_id and flags from registers directly.

Signed-off-by: Zhenyu Zheng <zhengzhenyulixi@gmail.com> --- src/cpu/cpu_arm.c | 194 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 193 insertions(+), 1 deletion(-)

diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c index 88b4d91946..c8f5ce7e8a 100644 --- a/src/cpu/cpu_arm.c +++ b/src/cpu/cpu_arm.c @@ -21,6 +21,8 @@ */

#include <config.h> +#include <asm/hwcap.h> +#include <sys/auxv.h>

#include "viralloc.h" #include "cpu.h" @@ -31,6 +33,10 @@ #include "virxml.h"

#define VIR_FROM_THIS VIR_FROM_CPU +/* 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

VIR_LOG_INIT("cpu.cpu_arm");

@@ -498,14 +504,200 @@ virCPUarmValidateFeatures(virCPUDefPtr cpu) return 0; }

+/** + * armCpuDataFromRegs: + * + * @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 +armCpuDataFromRegs(virCPUarmData *data)

virCPUarmDataFromRegs()

+{ + /* Generate human readable flag list according to the order of */ + /* AT_HWCAP bit map */ + const char *flag_list[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"}; + unsigned long cpuid, hwcaps; + char **features = NULL; + char *cpu_feature_str = NULL;

g_autofree

+ 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; + }

Indent messed up here

+ + /* 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); + + if (VIR_ALLOC_N(features, MAX_CPU_FLAGS) < 0) + return -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(flag_list[i]); + cpu_feature_index++; + } + }

Indent of } messed up again.

+ + if (cpu_feature_index > 1) { + cpu_feature_str = virStringListJoin((const char **)features, " "); + if (!cpu_feature_str) + goto cleanup; + } + data->features = g_strdup(cpu_feature_str); + + return 0; + + cleanup:

This should be called "error", since this label is only visited for fatal errors.

+ virStringListFree(features); + VIR_FREE(cpu_feature_str);

Can avoid the VIR_FREE with g_autofree.

+ return -1; +}

+ +static int +armCpuDataParseFeatures(virCPUDefPtr cpu, + const virCPUarmData *cpuData)

virCPUarmDataParseFeatures()

+{ + int ret = -1; + size_t i; + char **features; + + if (!cpu || !cpuData) + return ret; + + if (!(features = virStringSplitCount(cpuData->features, " ", + 0, &cpu->nfeatures))) + return ret; + if (cpu->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(features[i]); + } + } + + ret = 0; + + cleanup: + virStringListFree(features); + return ret; + + error: + for (i = 0; i < cpu->nfeatures; i++) + VIR_FREE(cpu->features[i].name); + VIR_FREE(cpu->features); + cpu->nfeatures = 0; + goto cleanup; +} + +static int +armDecode(virCPUDefPtr cpu, + const virCPUarmData *cpuData, + virDomainCapsCPUModelsPtr models)

virCPUDataArmDecode

+{ + virCPUarmMapPtr map; + virCPUarmModelPtr model; + virCPUarmVendorPtr vendor = NULL; + + if (!cpuData || !(map = virCPUarmGetMap())) + return -1; + + if (!(model = armModelFindByPVR(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 = armVendorFindByID(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->features && + armCpuDataParseFeatures(cpu, cpuData) < 0) + return -1; + + return 0; +} + +static int +armDecodeCPUData(virCPUDefPtr cpu, + const virCPUData *data, + virDomainCapsCPUModelsPtr models)

virCPUarmDecodeCPUData

+{ + return armDecode(cpu, &data->data.arm, models); +} + +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 (armCpuDataFromRegs(&cpuData->data.arm) < 0) + goto cleanup; + + ret = armDecodeCPUData(cpu, cpuData, models); + + cleanup: + virCPUarmDataFree(cpuData); + return ret; +} + struct cpuArchDriver cpuDriverArm = { .name = "arm", .arch = archs, .narch = G_N_ELEMENTS(archs), .compare = virCPUarmCompare, - .decode = NULL, + .decode = armDecodeCPUData, .encode = NULL, .dataFree = virCPUarmDataFree, + .getHost = virCPUarmGetHost, .baseline = virCPUarmBaseline, .update = virCPUarmUpdate, .validateFeatures = virCPUarmValidateFeatures, -- 2.26.0.windows.1

Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On Thu, Apr 02, 2020 at 17:03:59 +0800, Zhenyu Zheng wrote:

Introduce getHost support for ARM CPU driver, read CPU vendor_id, part_id and flags from registers directly.

Signed-off-by: Zhenyu Zheng <zhengzhenyulixi@gmail.com> --- src/cpu/cpu_arm.c | 194 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 193 insertions(+), 1 deletion(-)

In addition to all the coding style issues which Daniel pointed out...

diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c index 88b4d91946..c8f5ce7e8a 100644 --- a/src/cpu/cpu_arm.c +++ b/src/cpu/cpu_arm.c @@ -21,6 +21,8 @@ */

#include <config.h> +#include <asm/hwcap.h> +#include <sys/auxv.h>

None of these header files exist on my system with linux 5.6 headers.

#include "viralloc.h" #include "cpu.h" @@ -31,6 +33,10 @@ #include "virxml.h"

#define VIR_FROM_THIS VIR_FROM_CPU +/* 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

VIR_LOG_INIT("cpu.cpu_arm");

@@ -498,14 +504,200 @@ virCPUarmValidateFeatures(virCPUDefPtr cpu) return 0; }

+/** + * armCpuDataFromRegs: + * + * @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 +armCpuDataFromRegs(virCPUarmData *data) +{ + /* Generate human readable flag list according to the order of */ + /* AT_HWCAP bit map */ + const char *flag_list[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"};

I would expect these to be defined in src/cpu_map/arm_features.xml, although I'm not sure how well the new features would play with the existing ones... What do you think, Andrea?

+ unsigned long cpuid, hwcaps; + char **features = NULL; + char *cpu_feature_str = 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); + + if (VIR_ALLOC_N(features, MAX_CPU_FLAGS) < 0) + return -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(flag_list[i]); + cpu_feature_index++; + } + } + + if (cpu_feature_index > 1) { + cpu_feature_str = virStringListJoin((const char **)features, " "); + if (!cpu_feature_str) + goto cleanup; + } + data->features = g_strdup(cpu_feature_str); + + return 0; + + cleanup: + virStringListFree(features); + VIR_FREE(cpu_feature_str); + return -1; +} + +static int +armCpuDataParseFeatures(virCPUDefPtr cpu, + const virCPUarmData *cpuData) +{ + int ret = -1; + size_t i; + char **features; + + if (!cpu || !cpuData) + return ret; + + if (!(features = virStringSplitCount(cpuData->features, " ", + 0, &cpu->nfeatures))) + return ret; + if (cpu->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(features[i]); + } + } + + ret = 0; + + cleanup: + virStringListFree(features); + return ret; + + error: + for (i = 0; i < cpu->nfeatures; i++) + VIR_FREE(cpu->features[i].name); + VIR_FREE(cpu->features); + cpu->nfeatures = 0; + goto cleanup; +} + +static int +armDecode(virCPUDefPtr cpu, + const virCPUarmData *cpuData, + virDomainCapsCPUModelsPtr models) +{ + virCPUarmMapPtr map; + virCPUarmModelPtr model; + virCPUarmVendorPtr vendor = NULL; + + if (!cpuData || !(map = virCPUarmGetMap())) + return -1; + + if (!(model = armModelFindByPVR(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 = armVendorFindByID(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->features && + armCpuDataParseFeatures(cpu, cpuData) < 0) + return -1; + + return 0; +} + +static int +armDecodeCPUData(virCPUDefPtr cpu, + const virCPUData *data, + virDomainCapsCPUModelsPtr models) +{ + return armDecode(cpu, &data->data.arm, models); +} + +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 (armCpuDataFromRegs(&cpuData->data.arm) < 0) + goto cleanup; + + ret = armDecodeCPUData(cpu, cpuData, models); + + cleanup: + virCPUarmDataFree(cpuData); + return ret; +}

virCPUarmGetHost and the helpers it calls are architecture specific and should not be even compiled in on non-ARM. Jirka

Introduce getHost support for ARM CPU driver, read CPU vendor_id, part_id and flags from registers directly. Signed-off-by: Zhenyu Zheng <zhengzhenyulixi@gmail.com> --- src/cpu/cpu_arm.c | 193 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 192 insertions(+), 1 deletion(-) diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c index 605e7b2d51..2ac8ad3bff 100644 --- a/src/cpu/cpu_arm.c +++ b/src/cpu/cpu_arm.c @@ -21,6 +21,8 @@ */ #include <config.h> +#include <asm/hwcap.h> +#include <sys/auxv.h> #include "viralloc.h" #include "cpu.h" @@ -31,6 +33,10 @@ #include "virxml.h" #define VIR_FROM_THIS VIR_FROM_CPU +/* 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 VIR_LOG_INIT("cpu.cpu_arm"); @@ -492,14 +498,199 @@ virCPUarmValidateFeatures(virCPUDefPtr cpu) return 0; } +/** + * 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) +{ + /* Generate human readable flag list according to the order of */ + /* AT_HWCAP bit map */ + const char *flag_list[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"}; + unsigned long cpuid, hwcaps; + char **features = NULL; + g_autofree char *cpu_feature_str = 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); + + if (VIR_ALLOC_N(features, MAX_CPU_FLAGS) < 0) + return -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(flag_list[i]); + cpu_feature_index++; + } + } + + if (cpu_feature_index > 1) { + cpu_feature_str = virStringListJoin((const char **)features, " "); + if (!cpu_feature_str) + goto error; + } + data->features = g_strdup(cpu_feature_str); + + return 0; + + error: + virStringListFree(features); + return -1; +} + +static int +virCPUarmDataParseFeatures(virCPUDefPtr cpu, + const virCPUarmData *cpuData) +{ + int ret = -1; + size_t i; + char **features; + + if (!cpu || !cpuData) + return ret; + + if (!(features = virStringSplitCount(cpuData->features, " ", + 0, &cpu->nfeatures))) + return ret; + if (cpu->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(features[i]); + } + } + + ret = 0; + + cleanup: + virStringListFree(features); + return ret; + + error: + for (i = 0; i < cpu->nfeatures; i++) + VIR_FREE(cpu->features[i].name); + VIR_FREE(cpu->features); + cpu->nfeatures = 0; + goto cleanup; +} + +static int +virCPUarmDecode(virCPUDefPtr cpu, + const virCPUarmData *cpuData, + virDomainCapsCPUModelsPtr models) +{ + 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->features && + virCPUarmDataParseFeatures(cpu, cpuData) < 0) + return -1; + + return 0; +} + +static int +virCPUarmDecodeCPUData(virCPUDefPtr cpu, + const virCPUData *data, + virDomainCapsCPUModelsPtr models) +{ + return virCPUarmDecode(cpu, &data->data.arm, models); +} + +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 = virCPUarmDecodeCPUData(cpu, cpuData, models); + + cleanup: + virCPUarmDataFree(cpuData); + return ret; +} + struct cpuArchDriver cpuDriverArm = { .name = "arm", .arch = archs, .narch = G_N_ELEMENTS(archs), .compare = virCPUarmCompare, - .decode = NULL, + .decode = virCPUarmDecodeCPUData, .encode = NULL, .dataFree = virCPUarmDataFree, + .getHost = virCPUarmGetHost, .baseline = virCPUarmBaseline, .update = virCPUarmUpdate, .validateFeatures = virCPUarmValidateFeatures, -- 2.26.0.windows.1

Hi Daniel, Jirka Thanks alot for the review and info, I have updated the code to only compile on aarch64 platform, and I've done the gitlab CI testing as suggested: https://gitlab.com/ZhengZhenyu/libvirt/pipelines/134657317 and manually tested on aarch64. BR, On Fri, Apr 10, 2020 at 4:21 PM ZhengZhenyu <zheng.zhenyu@foxmail.com> wrote:

Introduce getHost support for ARM CPU driver, read CPU vendor_id, part_id and flags from registers directly.

Signed-off-by: Zhenyu Zheng <zhengzhenyulixi@gmail.com> --- src/cpu/cpu_arm.c | 202 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 201 insertions(+), 1 deletion(-)

diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c index 24404eac2c..8d6d435bee 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,12 @@ #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");

@@ -495,13 +505,203 @@ virCPUarmValidateFeatures(virCPUDefPtr cpu) return 0; }

+#if defined(__aarch64__) +/** + * 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) +{ + /* Generate human readable flag list according to the order of */ + /* AT_HWCAP bit map */ + const char *flag_list[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"}; + unsigned long cpuid, hwcaps; + char **features = NULL; + g_autofree char *cpu_feature_str = 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); + + if (VIR_ALLOC_N(features, MAX_CPU_FLAGS) < 0) + return -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(flag_list[i]); + cpu_feature_index++; + } + } + + if (cpu_feature_index > 1) { + cpu_feature_str = virStringListJoin((const char **)features, " "); + if (!cpu_feature_str) + goto error; + } + data->features = g_strdup(cpu_feature_str); + + return 0; + + error: + virStringListFree(features); + return -1; +} + +static int +virCPUarmDataParseFeatures(virCPUDefPtr cpu, + const virCPUarmData *cpuData) +{ + int ret = -1; + size_t i; + char **features; + + if (!cpu || !cpuData) + return ret; + + if (!(features = virStringSplitCount(cpuData->features, " ", + 0, &cpu->nfeatures))) + return ret; + if (cpu->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(features[i]); + } + } + + ret = 0; + + cleanup: + virStringListFree(features); + return ret; + + error: + for (i = 0; i < cpu->nfeatures; i++) + VIR_FREE(cpu->features[i].name); + VIR_FREE(cpu->features); + cpu->nfeatures = 0; + goto cleanup; +} + +static int +virCPUarmDecode(virCPUDefPtr cpu, + const virCPUarmData *cpuData, + virDomainCapsCPUModelsPtr models) +{ + 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->features && + virCPUarmDataParseFeatures(cpu, cpuData) < 0) + return -1; + + return 0; +} + +static int +virCPUarmDecodeCPUData(virCPUDefPtr cpu, + const virCPUData *data, + virDomainCapsCPUModelsPtr models) +{ + return virCPUarmDecode(cpu, &data->data.arm, models); +} + +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 = virCPUarmDecodeCPUData(cpu, cpuData, models); + + cleanup: + virCPUarmDataFree(cpuData); + return ret; +} +#endif + struct cpuArchDriver cpuDriverArm = { .name = "arm", .arch = archs, .narch = G_N_ELEMENTS(archs), .compare = virCPUarmCompare, - .decode = NULL, +#if defined(__aarch64__) + .getHost = virCPUarmGetHost, + .decode = virCPUarmDecodeCPUData, +#else .encode = NULL, +#endif .dataFree = virCPUarmDataFree, .baseline = virCPUarmBaseline, .update = virCPUarmUpdate, -- 2.26.0.windows.1