On 09.10.2012 09:58, Li Zhang wrote:
Currently, the CPU model driver is not implemented for PowerPC.
Host's CPU information is needed to exposed to guests' XML file some
time.
This patch is to implement the callback functions of CPU model driver.
Signed-off-by: Li Zhang <zhlcindy(a)linux.vnet.ibm.com>
---
src/cpu/cpu_map.xml | 14 ++
src/cpu/cpu_powerpc.c | 585 +++++++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 583 insertions(+), 16 deletions(-)
diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml
index 9a89e2e..affcce3 100644
--- a/src/cpu/cpu_map.xml
+++ b/src/cpu/cpu_map.xml
@@ -495,4 +495,18 @@
<feature name='fma4'/>
</model>
</arch>
+ <arch name='ppc64'>
+ <!-- vendor definitions -->
+ <vendor name='IBM' string='PowerPC'/>
+ <!-- IBM-based CPU models -->
+ <model name='POWER7'>
+ <vendor name='IBM'/>
+ </model>
+ <model name='POWER7_v2.1'>
+ <vendor name='IBM'/>
+ </model>
+ <model name='POWER7_v2.3'>
+ <vendor name='IBM'/>
+ </model>
+ </arch>
</cpus>
diff --git a/src/cpu/cpu_powerpc.c b/src/cpu/cpu_powerpc.c
index 1b77caf..3a88e68 100644
--- a/src/cpu/cpu_powerpc.c
+++ b/src/cpu/cpu_powerpc.c
@@ -20,18 +20,526 @@
* Authors:
* Anton Blanchard <anton(a)au.ibm.com>
* Prerna Saxena <prerna(a)linux.vnet.ibm.com>
+ * Li Zhang <zhlcindy(a)linux.vnet.ibm.com>
*/
#include <config.h>
+#include <stdint.h>
+#include "logging.h"
#include "memory.h"
+#include "util.h"
#include "cpu.h"
+#include "cpu_map.h"
+#include "buf.h"
#define VIR_FROM_THIS VIR_FROM_CPU
+#define VENDOR_STRING_LENGTH 7
+
static const char *archs[] = { "ppc64" };
+struct cpuPowerPC {
+ const char *name;
+ const char *vendor;
+ 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 ppcVendor {
+ char *name;
+ struct ppcVendor *next;
+};
+
+struct ppcModel {
+ char *name;
+ const struct ppcVendor *vendor;
+ union cpuData *data;
+ struct ppcModel *next;
+};
+
+struct ppcMap {
+ struct ppcVendor *vendors;
+ struct ppcModel *models;
+};
+
+static int ConvertModelVendorFromPVR(char ***model, char ***vendor, uint32_t pvr)
This line is too long. Moreover, we tend to prefix functions with the
part of libvirt they live in. So this function should be
static int
cpuConvertModelVendorFromPVR(char ***model,
char ***vendor,
uint32_t pvr);
or something like that.
+{
+ int i = 0;
+ while (cpu_defs[i].name != NULL) {
+ if (cpu_defs[i].pvr == pvr) {
+ **model = strdup(cpu_defs[i].name);
+ **vendor = strdup(cpu_defs[i].vendor);
+ return 0;
+ }
+ i ++;
+ }
Yeah, we've used 'while' in this way many times. But for() loop - while
being totally exchangeable in this case is better.
+
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Missing the definition of this model"));
s/VIR_ERR_INTERNAL_ERROR,/VIR_ERR_INTERNAL_ERROR, "%s"/
+ return -1;
+}
> +
> +static int ConvertPVRFromModel(const char *model, uint32_t *pvr)
> +{
> + int i = 0;
> + while (cpu_defs[i].name !=NULL) {
> + if (STREQ(cpu_defs[i].name, model)) {
> + *pvr = cpu_defs[i].pvr;
> + return 0;
> + }
> + i ++;
> + }
+
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Missing the definition of this model"));
ditto
+ return -1;
+}
Same applies for this function.
+
+static int cpuMatch(const union cpuData *data, char **cpu_model,
+ char **cpu_vendor, const struct ppcModel *model)
+{
+ int ret = 0;
+
+ ret = ConvertModelVendorFromPVR(&cpu_model, &cpu_vendor, data->ppc.pvr);
+
+ if (STREQ(model->name, *cpu_model) &&
+ STREQ(model->vendor->name, *cpu_vendor))
+ ret = 1;
+
+ return ret;
+}
+
+
+static struct ppcModel *
+ppcModelNew(void)
+{
+ struct ppcModel *model;
+
+ if (VIR_ALLOC(model) < 0)
+ return NULL;
+
+ if (VIR_ALLOC(model->data) < 0) {
+ VIR_FREE(model);
+ return NULL;
+ }
+
+ return model;
+}
+
+static void
+ppcModelFree(struct ppcModel *model)
+{
+ if (model == NULL)
+ return;
+
+ VIR_FREE(model->name);
+
+ if (model->data)
+ VIR_FREE(model->data);
VIR_FREE(NULL) is just fine. so you don't need to test model->data.
+
+ VIR_FREE(model);
+}
+
+static struct ppcModel *
+ppcModelFind(const struct ppcMap *map,
+ const char *name)
+{
+ struct ppcModel *model;
+
+ model = map->models;
+ while (model != NULL) {
+ if (STREQ(model->name, name))
+ return model;
+
+ model = model->next;
+ }
+
+ return NULL;
+}
+
+static struct ppcVendor *
+ppcVendorFind(const struct ppcMap *map,
+ const char *name)
+{
+ struct ppcVendor *vendor;
+
+ vendor = map->vendors;
+ while (vendor) {
+ if (STREQ(vendor->name, name))
+ return vendor;
+
+ vendor = vendor->next;
+ }
+
+ return NULL;
+}
+
+static void
+ppcVendorFree(struct ppcVendor *vendor)
+{
+ if (!vendor)
+ return;
+
+ VIR_FREE(vendor->name);
+ VIR_FREE(vendor);
+}
+
+static int
+ppcVendorLoad(xmlXPathContextPtr ctxt,
+ struct ppcMap *map)
+{
+ struct ppcVendor *vendor = NULL;
+ char *string = NULL;
+ int ret = 0;
+
+ if (VIR_ALLOC(vendor) < 0)
+ goto no_memory;
+
+ vendor->name = virXPathString("string(@name)", ctxt);
I think attributes are always string, aren't they? But this doesn't hurt.
+ if (!vendor->name) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("Missing CPU vendor name"));
+ goto ignore;
+ }
+
+ if (ppcVendorFind(map, vendor->name)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("CPU vendor %s already defined"), vendor->name);
+ goto ignore;
+ }
+
+ string = virXPathString("string(@string)", ctxt);
+ if (!string) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Missing vendor string for CPU vendor %s"),
vendor->name);
+ goto ignore;
+ }
+ if (strlen(string) != VENDOR_STRING_LENGTH) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Invalid CPU vendor string '%s'"), string);
+ goto ignore;
+ }
Does PowerPC equivalent of CPUID requires 7 characters at most?
+ if (!map->vendors)
+ map->vendors = vendor;
+ else {
+ vendor->next = map->vendors;
+ map->vendors = vendor;
+ }
+
+out:
+ VIR_FREE(string);
+
+ return ret;
+
+no_memory:
+ virReportOOMError();
+ ret = -1;
we often initialize ret = -1; and when everything went okay, we set ret
= 0; in this case that would be just before out: label. However, in this
case out can be dropped and we can have the only error label. Having
VIR_FREE(string) twice there is not a problem as it is single line.
+ignore:
+ ppcVendorFree(vendor);
+ goto out;
+}
+
+static int
+ppcModelLoad(xmlXPathContextPtr ctxt,
+ struct ppcMap *map)
+{
+ xmlNodePtr *nodes = NULL;
+ struct ppcModel *model;
+ char *vendor = NULL;
+ int ret = 0;
+
+ if (!(model = ppcModelNew()))
+ goto no_memory;
+
+ model->name = virXPathString("string(@name)", ctxt);
+ if (model->name == NULL) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("Missing CPU model name"));
+ goto ignore;
+ }
+
+ ret = ConvertPVRFromModel(model->name, &model->data->ppc.pvr);
+ if (ret < 0)
+ goto ignore;
+
+
+ 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 ignore;
+ }
+
+ if (!(model->vendor = ppcVendorFind(map, vendor))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unknown vendor %s referenced by CPU model %s"),
+ vendor, model->name);
+ goto ignore;
+ }
+ }
+
+ if (map->models == NULL)
+ map->models = model;
+ else {
+ model->next = map->models;
+ map->models = model;
+ }
+
+out:
+ VIR_FREE(vendor);
+ VIR_FREE(nodes);
+ return ret;
+
+no_memory:
+ virReportOOMError();
+ ret = -1;
+
+ignore:
+ ppcModelFree(model);
+ goto out;
+}
see my comments to previous function.
+
+static int
+ppcMapLoadCallback(enum cpuMapElement element,
+ xmlXPathContextPtr ctxt,
+ void *data)
+{
+ struct ppcMap *map = data;
+
+ switch (element) {
+ case CPU_MAP_ELEMENT_VENDOR:
+ return ppcVendorLoad(ctxt, map);
+ case CPU_MAP_ELEMENT_MODEL:
+ return ppcModelLoad(ctxt, map);
+ default:
+ break;
+ }
+
+ return 0;
+}
+
+static void
+ppcMapFree(struct ppcMap *map)
+{
+ if (map == NULL)
+ return;
+
+ while (map->models != NULL) {
+ struct ppcModel *model = map->models;
+ map->models = model->next;
+ ppcModelFree(model);
+ }
+
+ while (map->vendors != NULL) {
+ struct ppcVendor *vendor = map->vendors;
+ map->vendors = vendor->next;
+ ppcVendorFree(vendor);
+ }
+
+ VIR_FREE(map);
+}
+
+static struct ppcMap *
+ppcLoadMap(void)
+{
+ struct ppcMap *map;
+
+ if (VIR_ALLOC(map) < 0) {
+ virReportOOMError();
+ return NULL;
+ }
+
+ if (cpuMapLoad("ppc64", ppcMapLoadCallback, map) < 0)
+ goto error;
+
+ return map;
+
+error:
+ ppcMapFree(map);
+ return NULL;
+}
+
+static struct ppcModel *
+ppcModelCopy(const struct ppcModel *model)
+{
+ struct ppcModel *copy;
+
+ if (VIR_ALLOC(copy) < 0
+ || VIR_ALLOC(copy->data) < 0
+ || !(copy->name = strdup(model->name))){
+ ppcModelFree(copy);
+ return NULL;
+ }
+
+ copy->data->ppc.pvr = model->data->ppc.pvr;
+ copy->vendor = model->vendor;
+
+ return copy;
+}
+
+static struct ppcModel *
+ppcModelFromCPU(const virCPUDefPtr cpu,
+ const struct ppcMap *map)
+{
+ struct ppcModel *model = NULL;
+
+ if ((model = ppcModelFind(map, cpu->model))) {
+ if ((model = ppcModelCopy(model)) == NULL) {
+ goto no_memory;
+ }
+ } else if (!(model = ppcModelNew())) {
+ goto no_memory;
+ }
+
+ return model;
+
+no_memory:
+ virReportOOMError();
+ ppcModelFree(model);
+
+ return NULL;
+}
+
+static virCPUCompareResult
+PowerPCCompare(virCPUDefPtr host,
whitespace at the EOL
+ virCPUDefPtr cpu)
+{
+ if ((cpu->arch && STRNEQ(host->arch, cpu->arch)) ||
+ STRNEQ(host->model, cpu->model))
+ return VIR_CPU_COMPARE_INCOMPATIBLE;
+
+ return VIR_CPU_COMPARE_IDENTICAL;
+}
+
+static int
+PowerPCDecode(virCPUDefPtr cpu,
+ const union cpuData *data,
+ const char **models,
+ unsigned int nmodels,
+ const char *preferred)
+{
+ int ret = -1;
+ struct ppcMap *map;
+ const struct ppcModel *candidate;
+ virCPUDefPtr cpuCandidate;
+ virCPUDefPtr cpuModel = NULL;
+ char *cpu_vendor = NULL;
+ char *cpu_model = NULL;
+ unsigned int i;
+
+ if (data == NULL || (map = ppcLoadMap()) == NULL)
+ return -1;
+
+ candidate = map->models;
+
+ while (candidate != NULL) {
+ bool allowed = (models == NULL);
+
+ for (i = 0; i < nmodels; i++) {
+ if (models && models[i] && STREQ(models[i],
candidate->name)) {
+ allowed = true;
+ break;
+ }
+ }
+
+ if (!allowed) {
+ if (preferred && STREQ(candidate->name, preferred)) {
+ if (cpu->fallback != VIR_CPU_FALLBACK_ALLOW) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("CPU model %s is not supported by
hypervisor"),
+ preferred);
+ goto out;
+ } else {
+ VIR_WARN("Preferred CPU model %s not allowed by"
+ " hypervisor; closest supported model will be"
+ " used", preferred);
+ }
+ } else {
+ VIR_DEBUG("CPU model %s not allowed by hypervisor; ignoring",
+ candidate->name);
+ }
+ goto next;
+ }
+
+ if (VIR_ALLOC(cpuCandidate) < 0) {
+ virReportOOMError();
+ goto out;
+ }
+
+ cpuCandidate->model = strdup(candidate->name);
+ cpuCandidate->vendor = strdup(candidate->vendor->name);
+
+ if (preferred && STREQ(cpuCandidate->model, preferred)) {
+ virCPUDefFree(cpuModel);
+ cpuModel = cpuCandidate;
+ 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;
+ virCPUDefFree(cpuModel);
+ cpuModel = cpuCandidate;
+ break;
+ }
+
+ virCPUDefFree(cpuCandidate);
+
+ next:
+ candidate = candidate->next;
+ }
+
+ if (cpuModel == NULL) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("Cannot find suitable CPU model for given
data"));
+ goto out;
+ }
+
+ cpu->model = cpuModel->model;
+ cpu->vendor = cpuModel->vendor;
+ VIR_FREE(cpuModel);
+
+ ret = 0;
+
+out:
+ ppcMapFree(map);
+ virCPUDefFree(cpuModel);
+
+ return ret;
+}
+
+static uint32_t ppc_mfpvr(void)
+{
+ uint32_t pvr;
+ asm ("mfpvr %0"
+ : "=r"(pvr));
+ return pvr;
+}
There is no such instruction on x86. So we need to make this function
ppc only and introduce stub for other architectures.
+
+static void
+PowerPCDataFree(union cpuData *data)
+{
+ if (data == NULL)
+ return;
+
+ VIR_FREE(data);
+}
+
static union cpuData *
PowerPCNodeData(void)
{
@@ -42,40 +550,85 @@ PowerPCNodeData(void)
return NULL;
}
+ data->ppc.pvr = ppc_mfpvr();
+
return data;
}
-
static int
-PowerPCDecode(virCPUDefPtr cpu ATTRIBUTE_UNUSED,
- const union cpuData *data ATTRIBUTE_UNUSED,
- const char **models ATTRIBUTE_UNUSED,
- unsigned int nmodels ATTRIBUTE_UNUSED,
- const char *preferred ATTRIBUTE_UNUSED)
+PowerPCUpdate(virCPUDefPtr guest ATTRIBUTE_UNUSED,
+ const virCPUDefPtr host ATTRIBUTE_UNUSED)
{
- return 0;
+ return 0;
}
-
-static void
-PowerPCDataFree(union cpuData *data)
+static virCPUDefPtr
+PowerPCBaseline(virCPUDefPtr *cpus,
+ unsigned int ncpus ATTRIBUTE_UNUSED,
+ const char **models ATTRIBUTE_UNUSED,
+ unsigned int nmodels ATTRIBUTE_UNUSED)
{
- if (data == NULL)
- return;
+ struct ppcMap *map = NULL;
+ struct ppcModel *base_model = NULL;
+ virCPUDefPtr cpu = NULL;
+ struct ppcModel *model = NULL;
+ bool outputModel = true;
+
+ if (!(map = ppcLoadMap())) {
+ goto error;
+ }
+
+ if (!(base_model = ppcModelFromCPU(cpus[0], map))) {
+ goto error;
+ }
+
+ if (VIR_ALLOC(cpu) < 0 ||
+ !(cpu->arch = strdup(cpus[0]->arch)))
+ goto no_memory;
+ cpu->type = VIR_CPU_TYPE_GUEST;
+ cpu->match = VIR_CPU_MATCH_EXACT;
+
+ if (!cpus[0]->model) {
+ outputModel = false;
+ } else if (!(model = ppcModelFind(map, cpus[0]->model))) {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("Unknown CPU vendor %s"), cpus[0]->model);
+ goto error;
+ }
+
+ base_model->data->ppc.pvr = model->data->ppc.pvr;
+ if (PowerPCDecode(cpu, base_model->data, models, nmodels, NULL) < 0)
+ goto error;
+
+ if (!outputModel)
+ VIR_FREE(cpu->model);
+
+ VIR_FREE(cpu->arch);
+
+cleanup:
+ ppcModelFree(base_model);
+ ppcMapFree(map);
- VIR_FREE(data);
+ return cpu;
+no_memory:
+ virReportOOMError();
+error:
+ ppcModelFree(model);
+ virCPUDefFree(cpu);
+ cpu = NULL;
+ goto cleanup;
}
struct cpuArchDriver cpuDriverPowerPC = {
.name = "ppc64",
.arch = archs,
.narch = ARRAY_CARDINALITY(archs),
- .compare = NULL,
+ .compare = PowerPCCompare,
.decode = PowerPCDecode,
.encode = NULL,
.free = PowerPCDataFree,
.nodeData = PowerPCNodeData,
.guestData = NULL,
- .baseline = NULL,
- .update = NULL,
+ .baseline = PowerPCBaseline,
+ .update = PowerPCUpdate,
.hasFeature = NULL,
};
Moreover, you need to add src/cpu/cpu_powerpc.c into po/POTFILES.in
make syntax-check should have warned you about this.
Michal