[libvirt] [PATCH 1/1][RESEND] ppc64 cpu features

Li Zhang zhlcindy at gmail.com
Wed Mar 27 05:16:44 UTC 2013


Hi,

Could anyone give more comment?

Thanks. :)

On 2013年03月14日 14:54, Li Zhang wrote:
> From: Li Zhang <zhlcindy at linux.vnet.ibm.com>
>
> This patch adds a CPU feature "powernv" identifying IBM Power
> processor that supports native hypervisor e.g. KVM. This can
> be used by virtualization management software to determine
> the CPU capabilities. PVR doesn't indicate whether it a
> host or a guest CPU. So, we use /proc/cpuinfo to get the
> platform information (PowerNV) and use that to set the
> "powernv" flag.
>
> Signed-off-by: Dipankar Sarma <dipankar at in.ibm.com>
> Signed-off-by: Li Zhang <zhlcindy at linux.vnet.ibm.com>
> ---
>   src/cpu/cpu_map.xml    |    9 ++
>   src/cpu/cpu_powerpc.c  |  349 ++++++++++++++++++++++++++++++++++++++----------
>   src/cpu/cpu_ppc_data.h |    4 +
>   src/util/sysinfo.c     |    2 +-
>   4 files changed, 294 insertions(+), 70 deletions(-)
>
> diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml
> index eb69a34..6b1f9b9 100644
> --- a/src/cpu/cpu_map.xml
> +++ b/src/cpu/cpu_map.xml
> @@ -587,14 +587,23 @@
>     <arch name='ppc64'>
>      <!-- vendor definitions -->
>       <vendor name='IBM' string='PowerPC'/>
> +    <feature name='powernv'> <!-- SYSTEMID_POWERNV -->
> +      <systemid platform='0x00000001'/>
> +    </feature>
>      <!-- IBM-based CPU models -->
>       <model name='POWER7'>
> +      <feature name='powernv'/>
> +      <systemid pvr='0x003f0200'/>
>         <vendor name='IBM'/>
>       </model>
>       <model name='POWER7_v2.1'>
> +      <feature name='powernv'/>
> +      <systemid pvr='0x003f0201'/>
>         <vendor name='IBM'/>
>       </model>
>       <model name='POWER7_v2.3'>
> +      <feature name='powernv'/>
> +      <systemid pvr='0x003f0203'/>
>         <vendor name='IBM'/>
>       </model>
>     </arch>
> diff --git a/src/cpu/cpu_powerpc.c b/src/cpu/cpu_powerpc.c
> index ac10789..2135944 100644
> --- a/src/cpu/cpu_powerpc.c
> +++ b/src/cpu/cpu_powerpc.c
> @@ -33,6 +33,7 @@
>   
>   #include "cpu_map.h"
>   #include "buf.h"
> +#include "sysinfo.h"
>   
>   #define VIR_FROM_THIS VIR_FROM_CPU
>   
> @@ -44,16 +45,9 @@ struct cpuPowerPC {
>       uint32_t pvr;
>   };
>   
> -static const struct cpuPowerPC cpu_defs[] = {
> -    {"POWER7", "IBM", 0x003f0200},
> -    {"POWER7_v2.1", "IBM", 0x003f0201},
> -    {"POWER7_v2.3", "IBM", 0x003f0203},
> -    {NULL, NULL, 0xffffffff}
> -};
> -
> -
>   struct ppc_vendor {
>       char *name;
> +    uint32_t pvr;
>       struct ppc_vendor *next;
>   };
>   
> @@ -64,64 +58,191 @@ struct ppc_model {
>       struct ppc_model *next;
>   };
>   
> +struct ppc_feature {
> +    char *name;
> +    union cpuData *data;
> +    struct ppc_feature *next;
> +};
> +
>   struct ppc_map {
>       struct ppc_vendor *vendors;
> +    struct ppc_feature *features;
>       struct ppc_model *models;
>   };
>   
> +static void
> +ppcDataSubtract(union cpuData *data1,
> +                const union cpuData *data2)
> +{
> +    data1->ppc.platform &= ~data2->ppc.platform;
> +}
> +
> +static bool
> +ppcDataIsSubset(const union cpuData *data,
> +                const union cpuData *subset)
> +{
> +    if (subset->ppc.platform &&
> +       (data->ppc.platform & subset->ppc.platform) == subset->ppc.platform)
> +        return true;
> +
> +    return false;
> +}
> +
> +static void
> +PowerPCDataFree(union cpuData *data)
> +{
> +    if (data == NULL)
> +        return;
> +    VIR_FREE(data);
> +}
> +
> +
> +static union cpuData *
> +ppcDataCopy(const union cpuData *data)
> +{
> +    union cpuData *copy = NULL;
> +
> +    if (VIR_ALLOC(copy) < 0) {
> +        PowerPCDataFree(copy);
> +        return NULL;
> +    }
> +    copy->ppc = data->ppc;
> +    return copy;
> +}
> +
> +
> +/* also removes all detected features from data */
>   static int
> -ConvertModelVendorFromPVR(char ***model,
> -                          char ***vendor,
> -                          uint32_t pvr)
> +ppcDataToCPUFeatures(virCPUDefPtr cpu,
> +                     int policy,
> +                     union cpuData *data,
> +                     const struct ppc_map *map)
>   {
> -    int i;
> +    const struct ppc_feature *feature = map->features;
>   
> -    for (i = 0; cpu_defs[i].name; i++) {
> -        if (cpu_defs[i].pvr == pvr) {
> -            **model = strdup(cpu_defs[i].name);
> -            **vendor = strdup(cpu_defs[i].vendor);
> -            return 0;
> +    while (feature != NULL) {
> +        if (ppcDataIsSubset(data, feature->data)) {
> +            ppcDataSubtract(data, feature->data);
> +            if (virCPUDefAddFeature(cpu, feature->name, policy) < 0)
> +                return -1;
>           }
> +        feature = feature->next;
>       }
>   
> -    virReportError(VIR_ERR_INTERNAL_ERROR,
> -                   "%s", _("Missing the definition of this model"));
> -    return -1;
> +    return 0;
>   }
>   
> -static int
> -ConvertPVRFromModel(const char *model,
> -                    uint32_t *pvr)
> +static struct ppc_feature *
> +ppcFeatureNew(void)
>   {
> -    int i;
> +    struct ppc_feature *feature;
>   
> -    for (i = 0; cpu_defs[i].name; i++) {
> -        if (STREQ(cpu_defs[i].name, model)) {
> -            *pvr = cpu_defs[i].pvr;
> -            return 0;
> -        }
> +    if (VIR_ALLOC(feature) < 0)
> +        return NULL;
> +
> +    if (VIR_ALLOC(feature->data) < 0) {
> +        VIR_FREE(feature);
> +        return NULL;
> +    }
> +
> +    return feature;
> +}
> +
> +
> +static void
> +ppcFeatureFree(struct ppc_feature *feature)
> +{
> +    if (feature == NULL)
> +        return;
> +
> +    VIR_FREE(feature->name);
> +    PowerPCDataFree(feature->data);
> +    VIR_FREE(feature);
> +}
> +
> +
> +static struct ppc_feature *
> +ppcFeatureFind(const struct ppc_map *map,
> +               const char *name)
> +{
> +    struct ppc_feature *feature;
> +
> +    feature = map->features;
> +    while (feature != NULL) {
> +        if (STREQ(feature->name, name))
> +            return feature;
> +
> +        feature = feature->next;
>       }
>   
> -    virReportError(VIR_ERR_INTERNAL_ERROR,
> -                   "%s", _("Missing the definition of this model"));
> -    return -1;
> +    return NULL;
>   }
>   
>   static int
> -cpuMatch(const union cpuData *data,
> -         char **cpu_model,
> -         char **cpu_vendor,
> -         const struct ppc_model *model)
> +ppcFeatureLoad(xmlXPathContextPtr ctxt,
> +               struct ppc_map *map)
>   {
> +    xmlNodePtr *nodes = NULL;
> +    xmlNodePtr ctxt_node = ctxt->node;
> +    struct ppc_feature *feature;
>       int ret = 0;
> +    unsigned long platform = 0;
> +    unsigned long pvr = 0;
> +    int ret_platform;
> +    int ret_pvr;
> +    int n;
> +
> +    if (!(feature = ppcFeatureNew()))
> +        goto no_memory;
> +
> +    feature->name = virXPathString("string(@name)", ctxt);
> +    if (feature->name == NULL) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       "%s", _("Missing CPU feature name"));
> +        goto ignore;
> +    }
> +
> +    if (ppcFeatureFind(map, feature->name)) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("CPU feature %s already defined"), feature->name);
> +        goto ignore;
> +    }
> +
> +    n = virXPathNodeSet("./systemid", ctxt, &nodes);
> +    if (n < 0)
> +        goto ignore;
> +
> +    ctxt->node = nodes[0];
> +    ret_platform = virXPathULongHex("string(@platform)", ctxt, &platform);
> +    ret_pvr = virXPathULongHex("string(@pvr)", ctxt, &pvr);
> +    if (ret_platform < 0 && ret_pvr < 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("Invalid systemid in %s feature"), feature->name);
> +            goto ignore;
> +    }
> +    feature->data->ppc.platform = platform;
> +    feature->data->ppc.pvr = pvr;
>   
> -    ret = ConvertModelVendorFromPVR(&cpu_model, &cpu_vendor, data->ppc.pvr);
> +    if (map->features == NULL)
> +        map->features = feature;
> +    else {
> +        feature->next = map->features;
> +        map->features = feature;
> +    }
>   
> -    if (STREQ(model->name, *cpu_model) &&
> -        STREQ(model->vendor->name, *cpu_vendor))
> -        ret = 1;
> +out:
> +    ctxt->node = ctxt_node;
> +    VIR_FREE(nodes);
>   
>       return ret;
> +
> +no_memory:
> +    virReportOOMError();
> +    ret = -1;
> +
> +ignore:
> +    ppcFeatureFree(feature);
> +    goto out;
>   }
>   
>   
> @@ -171,6 +292,66 @@ ppcModelFind(const struct ppc_map *map,
>       return NULL;
>   }
>   
> +/* also removes bits corresponding to vendor string from data */
> +static const struct ppc_vendor *
> +ppcDataToVendor(union cpuData *data,
> +                const struct ppc_map *map)
> +{
> +    const struct ppc_vendor *vendor = map->vendors;
> +
> +    while (vendor) {
> +        if (data->ppc.pvr == vendor->pvr)
> +            return vendor;
> +        vendor = vendor->next;
> +    }
> +
> +    return NULL;
> +}
> +
> +
> +static virCPUDefPtr
> +ppcDataToCPU(const union cpuData *data,
> +             const struct ppc_model *model,
> +             const struct ppc_map *map)
> +{
> +    virCPUDefPtr cpu;
> +    union cpuData *copy = NULL;
> +    union cpuData *modelData = NULL;
> +    const struct ppc_vendor *vendor;
> +
> +    if (VIR_ALLOC(cpu) < 0 ||
> +        !(cpu->model = strdup(model->name)) ||
> +        !(copy = ppcDataCopy(data)) ||
> +        !(modelData = ppcDataCopy(model->data)))
> +        goto no_memory;
> +
> +    if ((vendor = ppcDataToVendor(copy, map)) &&
> +        !(cpu->vendor = strdup(vendor->name)))
> +        goto no_memory;
> +
> +    ppcDataSubtract(copy, modelData);
> +    ppcDataSubtract(modelData, data);
> +
> +    /* because feature policy is ignored for host CPU */
> +    cpu->type = VIR_CPU_TYPE_GUEST;
> +
> +    if (ppcDataToCPUFeatures(cpu, VIR_CPU_FEATURE_REQUIRE, copy, map) ||
> +        ppcDataToCPUFeatures(cpu, VIR_CPU_FEATURE_DISABLE, modelData, map))
> +        goto error;
> +
> +cleanup:
> +    PowerPCDataFree(modelData);
> +    PowerPCDataFree(copy);
> +    return cpu;
> +
> +no_memory:
> +    virReportOOMError();
> +error:
> +    virCPUDefFree(cpu);
> +    cpu = NULL;
> +    goto cleanup;
> +}
> +
>   static struct ppc_vendor *
>   ppcVendorFind(const struct ppc_map *map,
>                 const char *name)
> @@ -256,6 +437,9 @@ ppcModelLoad(xmlXPathContextPtr ctxt,
>       xmlNodePtr *nodes = NULL;
>       struct ppc_model *model;
>       char *vendor = NULL;
> +    unsigned long pvr = 0, platform = 0;
> +    int ret_platform, ret_pvr;
> +    int n;
>       int ret = -1;
>   
>       if (!(model = ppcModelNew()))
> @@ -268,10 +452,20 @@ ppcModelLoad(xmlXPathContextPtr ctxt,
>           goto ignore;
>       }
>   
> -    ret = ConvertPVRFromModel(model->name, &model->data->ppc.pvr);
> -    if (ret < 0)
> -       goto ignore;
> +    n = virXPathNodeSet("./systemid", ctxt, &nodes);
> +    if (n < 0)
> +        goto ignore;
>   
> +    ctxt->node = nodes[0];
> +    ret_platform = virXPathULongHex("string(@platform)", ctxt, &platform);
> +    ret_pvr = virXPathULongHex("string(@pvr)", ctxt, &pvr);
> +    if (ret_platform < 0 && ret_pvr < 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("Invalid systemid in %s model"), model->name);
> +            goto ignore;
> +    }
> +    model->data->ppc.platform = platform;
> +    model->data->ppc.pvr = pvr;
>   
>       if (virXPathBoolean("boolean(./vendor)", ctxt)) {
>           vendor = virXPathString("string(./vendor/@name)", ctxt);
> @@ -324,6 +518,8 @@ ppcMapLoadCallback(enum cpuMapElement element,
>           return ppcVendorLoad(ctxt, map);
>       case CPU_MAP_ELEMENT_MODEL:
>           return ppcModelLoad(ctxt, map);
> +    case CPU_MAP_ELEMENT_FEATURE:
> +        return ppcFeatureLoad(ctxt, map);
>       default:
>           break;
>       }
> @@ -385,6 +581,7 @@ ppcModelCopy(const struct ppc_model *model)
>       }
>   
>       copy->data->ppc.pvr = model->data->ppc.pvr;
> +    copy->data->ppc.platform = model->data->ppc.platform;
>       copy->vendor = model->vendor;
>   
>       return copy;
> @@ -437,8 +634,6 @@ PowerPCDecode(virCPUDefPtr cpu,
>       const struct ppc_model *candidate;
>       virCPUDefPtr cpuCandidate;
>       virCPUDefPtr cpuModel = NULL;
> -    char *cpu_vendor = NULL;
> -    char *cpu_model = NULL;
>       unsigned int i;
>   
>       if (data == NULL || (map = ppcLoadMap()) == NULL)
> @@ -475,13 +670,30 @@ PowerPCDecode(virCPUDefPtr cpu,
>               goto next;
>           }
>   
> -        if (VIR_ALLOC(cpuCandidate) < 0) {
> -            virReportOOMError();
> +        if (!(cpuCandidate = ppcDataToCPU(data, candidate, map)))
>               goto out;
> +
> +        if (candidate->vendor && cpuCandidate->vendor &&
> +            STRNEQ(candidate->vendor->name, cpuCandidate->vendor)) {
> +            VIR_DEBUG("CPU vendor %s of model %s differs from %s; ignoring",
> +                      candidate->vendor->name, candidate->name,
> +                      cpuCandidate->vendor);
> +            virCPUDefFree(cpuCandidate);
> +            goto next;
>           }
>   
> -        cpuCandidate->model = strdup(candidate->name);
> -        cpuCandidate->vendor = strdup(candidate->vendor->name);
> +        if (cpu->type == VIR_CPU_TYPE_HOST) {
> +            cpuCandidate->type = VIR_CPU_TYPE_HOST;
> +            for (i = 0; i < cpuCandidate->nfeatures; i++) {
> +                switch (cpuCandidate->features[i].policy) {
> +                case VIR_CPU_FEATURE_DISABLE:
> +                    virCPUDefFree(cpuCandidate);
> +                    goto next;
> +                default:
> +                    cpuCandidate->features[i].policy = -1;
> +                }
> +            }
> +        }
>   
>           if (preferred && STREQ(cpuCandidate->model, preferred)) {
>               virCPUDefFree(cpuModel);
> @@ -489,19 +701,12 @@ PowerPCDecode(virCPUDefPtr cpu,
>               break;
>           }
>   
> -        ret = cpuMatch(data, &cpu_model, &cpu_vendor, candidate);
> -        if (ret < 0) {
> -            VIR_FREE(cpuCandidate);
> -            goto out;
> -        } else if (ret == 1) {
> -            cpuCandidate->model = cpu_model;
> -            cpuCandidate->vendor = cpu_vendor;
> +        if (cpuModel == NULL
> +            || cpuModel->nfeatures > cpuCandidate->nfeatures) {
>               virCPUDefFree(cpuModel);
>               cpuModel = cpuCandidate;
> -            break;
> -        }
> -
> -        virCPUDefFree(cpuCandidate);
> +        }else
> +            virCPUDefFree(cpuCandidate);
>   
>       next:
>           candidate = candidate->next;
> @@ -515,6 +720,8 @@ PowerPCDecode(virCPUDefPtr cpu,
>   
>       cpu->model = cpuModel->model;
>       cpu->vendor = cpuModel->vendor;
> +    cpu->nfeatures = cpuModel->nfeatures;
> +    cpu->features = cpuModel->features;
>       VIR_FREE(cpuModel);
>   
>       ret = 0;
> @@ -537,19 +744,11 @@ static uint32_t ppc_mfpvr(void)
>   }
>   #endif
>   
> -static void
> -PowerPCDataFree(union cpuData *data)
> -{
> -    if (data == NULL)
> -        return;
> -
> -    VIR_FREE(data);
> -}
> -
>   static union cpuData *
>   PowerPCNodeData(void)
>   {
>       union cpuData *data;
> +    virSysinfoDefPtr hostinfo;
>   
>       if (VIR_ALLOC(data) < 0) {
>           virReportOOMError();
> @@ -561,6 +760,18 @@ PowerPCNodeData(void)
>       data->ppc.pvr = ppc_mfpvr();
>   #endif
>   
> +    hostinfo = virSysinfoRead();
> +    if (hostinfo == NULL) {
> +        VIR_FREE(data);
> +        return NULL;
> +    }
> +
> +    data->ppc.platform &= ~VIR_CPU_PPC64_POWERNV;
> +
> +    if (STREQ(hostinfo->system_family, "PowerNV"))
> +        data->ppc.platform |= VIR_CPU_PPC64_POWERNV;
> +    virSysinfoDefFree(hostinfo);
> +
>       return data;
>   }
>   
> diff --git a/src/cpu/cpu_ppc_data.h b/src/cpu/cpu_ppc_data.h
> index 685332a..0e691ce 100644
> --- a/src/cpu/cpu_ppc_data.h
> +++ b/src/cpu/cpu_ppc_data.h
> @@ -28,6 +28,10 @@
>   
>   struct cpuPPCData {
>       uint32_t pvr;
> +    uint32_t platform; /* Bitflag indicating platform features */
>   };
>   
> +#define VIR_CPU_PPC64_NONE	0x0
> +#define VIR_CPU_PPC64_POWERNV	0x1
> +
>   #endif /* __VIR_CPU_PPC_DATA_H__ */
> diff --git a/src/util/sysinfo.c b/src/util/sysinfo.c
> index 6a5db80..5f98986 100644
> --- a/src/util/sysinfo.c
> +++ b/src/util/sysinfo.c
> @@ -234,7 +234,7 @@ virSysinfoRead(void) {
>       if (VIR_ALLOC(ret) < 0)
>           goto no_memory;
>   
> -    if (virFileReadAll(CPUINFO, 2048, &outbuf) < 0) {
> +    if (virFileReadAll(CPUINFO, 8192, &outbuf) < 0) {
>           virReportError(VIR_ERR_INTERNAL_ERROR,
>                          _("Failed to open %s"), CPUINFO);
>           return NULL;




More information about the libvir-list mailing list