[libvirt] [PATCH 16/18] cpu: Parse and use PVR masks in the ppc64 driver

[Jiri Denemark]

On Tue, Aug 04, 2015 at 11:38:07 +0200, Andrea Bolognani wrote:
> Instead of relying on a hard-coded mask value, read it from the CPU
> map XML and use it when looking up models by PVR.
> 
> Rewrite ppc64DriverNodeData() to use this feature.
> ---
>  src/cpu/cpu_ppc64.c      | 74 ++++++++++++++++++++++++++++--------------------
>  src/cpu/cpu_ppc64_data.h |  1 +
>  2 files changed, 44 insertions(+), 31 deletions(-)
> 
> diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
> index 9351729..add5ede 100644
> --- a/src/cpu/cpu_ppc64.c
> +++ b/src/cpu/cpu_ppc64.c
> @@ -84,8 +84,10 @@ ppc64DataCopy(virCPUppc64Data *data)
>  
>      copy->len = data->len;
>  
> -    for (i = 0; i < data->len; i++)
> +    for (i = 0; i < data->len; i++) {
>          copy->pvr[i].value = data->pvr[i].value;
> +        copy->pvr[i].mask = data->pvr[i].mask;
> +    }
>  
>      return copy;
>  
> @@ -185,20 +187,12 @@ ppc64ModelFindPVR(const struct ppc64_map *map,
>      model = map->models;
>      while (model) {
>          for (i = 0; i < model->data->len; i++)
> -            if (model->data->pvr[i].value == pvr)
> +            if ((pvr & model->data->pvr[i].mask) == model->data->pvr[i].value)
>                  return model;
>  
>          model = model->next;
>      }
>  
> -    /* PowerPC Processor Version Register is interpreted as follows :
> -     * Higher order 16 bits : Power ISA generation.
> -     * Lower order 16 bits : CPU chip version number.
> -     * If the exact CPU isn't found, return the nearest matching CPU generation
> -     */
> -    if (pvr & 0x0000FFFFul)
> -        return ppc64ModelFindPVR(map, (pvr & 0xFFFF0000ul));
> -
>      return NULL;
>  }
>  
> @@ -364,6 +358,24 @@ ppc64ModelLoad(xmlXPathContextPtr ctxt,
>          model->data->pvr[i].value = pvr;
>  
>          VIR_FREE(prop);
> +
> +        if (!(prop = virXMLPropString(nodes[i], "mask"))) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("Missing PVR mask in CPU model %s"),
> +                           model->name);
> +            goto ignore;
> +        }
> +
> +        if (virStrToLong_ul(prop, NULL, 16, &pvr) < 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("Invalid PVR mask in CPU model %s"),
> +                           model->name);
> +            goto ignore;
> +        }
> +
> +        model->data->pvr[i].mask = pvr;

Wouldn't virXPathULongHex be good enough?

> +
> +        VIR_FREE(prop);
>      }
>  
>      if (!map->models) {
> @@ -607,33 +619,33 @@ ppc64DriverFree(virCPUDataPtr data)
>  static virCPUDataPtr
>  ppc64DriverNodeData(virArch arch)
>  {
> -    virCPUDataPtr cpuData;
> -
> -    if (VIR_ALLOC(cpuData) < 0)
> -        goto error;
> -
> -    if (VIR_ALLOC(cpuData->data.ppc64) < 0)
> -        goto error;
> -
> -    if (VIR_ALLOC_N(cpuData->data.ppc64->pvr, 1) < 0)
> -        goto error;
> -
> -    cpuData->data.ppc64->len = 1;
> -
> -    cpuData->arch = arch;
> +    virCPUDataPtr nodeData = NULL;
> +    struct ppc64_map *map = NULL;
> +    struct ppc64_model *model = NULL;
> +    uint32_t pvr = 0;
>  
>  #if defined(__powerpc__) || defined(__powerpc64__)
>      asm("mfpvr %0"
> -        : "=r" (cpuData->data.ppc64->pvr[0].value));
> +        : "=r" (pvr));
>  #endif
>  
> -    return cpuData;
> +    if (!(map = ppc64LoadMap()))
> +        goto cleanup;
>  
> - error:
> -    if (cpuData)
> -        ppc64DataFree(cpuData->data.ppc64);
> -    VIR_FREE(cpuData);
> -    return NULL;
> +    if (!(model = ppc64ModelFindPVR(map, pvr))) {
> +        virReportError(VIR_ERR_OPERATION_FAILED,
> +                       _("Cannot find CPU model with PVR 0x%08x"),
> +                       pvr);
> +        goto cleanup;
> +    }
> +
> +    if (!(nodeData = ppc64MakeCPUData(arch, model->data)))
> +        goto cleanup;
> +
> + cleanup:
> +    ppc64MapFree(map);
> +
> +    return nodeData;
>  }

The ppc64DriverNodeData API is supposed to return raw data from the
host, which is then passed to ppc64DriverDecode to be translated into a
CPU model name. In other words, you copied most of the internals of
ppc64DriverDecode here while you in fact only need to add a single line

    cpuData->data.ppc64->pvr[0].mask = 0xffffffff;

Jirka