On Wed, May 13, 2020 at 18:48:32 +0800, Zhenyu Zheng wrote:

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

Signed-off-by: Zhenyu Zheng <zheng.zhenyu@outlook.com> --- src/cpu/cpu_arm.c | 159 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 158 insertions(+), 1 deletion(-)

diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c index 1bb0afb762..c7a6abfb22 100644 --- a/src/cpu/cpu_arm.c +++ b/src/cpu/cpu_arm.c @@ -165,6 +165,7 @@ virCPUarmMapFree(virCPUarmMapPtr map) virCPUarmVendorFree(map->vendors[i]); g_free(map->vendors);

+ g_ptr_array_free(map->features, TRUE);

g_free(map);

Unintentional change I guess.

@@ -210,6 +211,160 @@ 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)

The indentation is still wrong.

+{ + virCPUarmMapPtr map = data; + g_autoptr(virCPUarmVendor) vendor = NULL; + + vendor = g_new0(virCPUarmVendor, 1); + vendor->name = g_strdup(name); + + if (virCPUarmVendorFindByName(map, vendor->name)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("CPU vendor %s already defined"), + vendor->name); + return -1; + } + + if (virXPathULongHex("string(@value)", ctxt, &vendor->value) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Missing CPU vendor value")); + return -1; + } + + if (virCPUarmVendorFindByID(map, vendor->value)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("CPU vendor value 0x%2lx already defined"), + vendor->value); + return -1; + } + + if (VIR_APPEND_ELEMENT(map->vendors, map->nvendors, vendor) < 0) + return -1; + + return 0; +} + +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

Apparently I didn't notice this in my last review... virCPUarmModelFindByPVR should be moved to the next patch, otherwise compilation would fail after this patch as the function is never used here.

+ +static int +virCPUarmModelParse(xmlXPathContextPtr ctxt, + const char *name, + void *data)

Wrong indentation again.

+{ + virCPUarmMapPtr map = data; + virCPUarmModel *model;

g_autoptr(virCPUarmModel) model = NULL;

+ g_autofree xmlNodePtr *nodes = NULL; + g_autofree char *vendor = NULL; + + model = g_new0(virCPUarmModel, 1); + model->name = g_strdup(name); + + if (virCPUarmModelFind(map, model->name)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("CPU model %s already defined"), + model->name); + return -1; + } + + 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); + return -1; + } + + if (!(model->vendor = virCPUarmVendorFindByName(map, vendor))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown vendor %s referenced by CPU model %s"), + vendor, model->name); + return -1; + } + } + + if (!virXPathBoolean("boolean(./pvr)", ctxt)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Missing PVR information for CPU model %s"), + model->name); + return -1; + } + + 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); + return -1; + } + + if (VIR_APPEND_ELEMENT(map->models, map->nmodels, model) < 0) + return -1; + + return 0; +} + static virCPUarmMapPtr virCPUarmLoadMap(void) { @@ -217,7 +372,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); @@ -279,6 +435,7 @@ virCPUarmUpdate(virCPUDefPtr guest, return ret; }

+ static virCPUDefPtr virCPUarmBaseline(virCPUDefPtr *cpus, unsigned int ncpus G_GNUC_UNUSED,

This should be squashed to the previous patch to remove the unexpected change there. Jirka