[libvirt] [PATCH 1/1][RESEND] ppc64 cpu features
Li Zhang
zhlcindy at gmail.com
Thu Apr 11 10:42:06 UTC 2013
On 2013年04月11日 17:50, Daniel P. Berrange wrote:
> On Thu, Mar 14, 2013 at 02:54:21PM +0800, 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
> I'm not sure what to say about this. Superficially it looks ok, but I'm
> not all that familiar with this code in libvirt, nor PPC. So I think I'd
> prefer Jiri to look at this at least from the libvirt POV.
Sure, thanks.
Jiri, would you please help review this patch?
>
>> 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;
> This change seems like it probably ought to be separate.
>
> Daniel
More information about the libvir-list
mailing list