Re: [PATCH V3 4/5] cpu: Add helper funtions to parse vendor and model

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Apr 22, 2020 at 15:11:22 +0800, ZhengZhenyu 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@xxxxxxxxx>
> ---
>  src/cpu/cpu_arm.c | 173 +++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 170 insertions(+), 3 deletions(-)
> 
> diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c
> index 2009904cc9..6e9ff9bf11 100644
> --- a/src/cpu/cpu_arm.c
> +++ b/src/cpu/cpu_arm.c
> @@ -204,6 +204,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)

Wrong indentation.

> +{
> +    virCPUarmMapPtr map = data;
> +    virCPUarmVendorPtr vendor = NULL;

This should be declared as

    g_autoptr(virCPUarmVendor) vendor = NULL;

> +    int ret = -1;

Remove this variable, please.

> +
> +    if (VIR_ALLOC(vendor) < 0)
> +        return ret;

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);
> +        goto cleanup;

return -1;

> +    }
> +
> +    if (virXPathULongHex("string(@value)", ctxt, &vendor->value) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       "%s", _("Missing CPU vendor value"));
> +        goto cleanup;

return -1;

> +    }
> +
> +    if (virCPUarmVendorFindByID(map, vendor->value)) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("CPU vendor value 0x%2lx already defined"),
> +                       vendor->value);
> +        goto cleanup;

return -1;

> +    }
> +
> +    if (VIR_APPEND_ELEMENT(map->vendors, map->nvendors, vendor) < 0)
> +        goto cleanup;

return -1;

> +
> +    ret = 0;

return 0;

> +
> + cleanup:
> +    virCPUarmVendorFree(vendor);
> +    return ret;
> +

The cleanup section can be dropped.

> +}
> +
> +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_autoptr(virCPUArmModel) model = NULL;

> +    g_autofree xmlNodePtr *nodes = NULL;
> +    g_autofree char *vendor = NULL;
> +
> +    if (VIR_ALLOC(model) < 0)
> +        goto error;

model = g_new0(...)

And the rest of this function should be modified in similarly to
VendorParse, i.e., s/goto error/return -1/ and drop the error section.

> +
> +    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)
>  {
> @@ -211,8 +379,8 @@ virCPUarmLoadMap(void)
>  
>      map = virCPUarmMapNew();
>  
> -    if (cpuMapLoad("arm", NULL, virCPUarmMapFeatureParse, NULL, map) < 0)
> -        return NULL;
> +    if (cpuMapLoad("arm", virCPUarmVendorParse, virCPUarmMapFeatureParse,
> +                   virCPUarmModelParse, map) < 0)

return NULL;

>  
>      return g_steal_pointer(&map);
>  }
> @@ -273,7 +441,6 @@ virCPUarmUpdate(virCPUDefPtr guest,
>      return ret;
>  }
>  
> -
>  static virCPUDefPtr
>  virCPUarmBaseline(virCPUDefPtr *cpus,
>                    unsigned int ncpus G_GNUC_UNUSED,

This looks like an unintentional change.

Jirka




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux