<div dir="ltr">Hi Daniel,<div> </div><div>Thanks for thre review and reply, my first implementation was going to gather data from /proc/cpuinfo, but unlike X86, we can only get this kind of info:</div><div> </div><div> processor : 0 BogoMIPS : 200.00 Features : fp asimd evtstrm aes pmull sha1 sha2 crc32 cpuid CPU implementer : 0x43 CPU architecture: 8 CPU variant : 0x1 CPU part : 0x0a1 CPU revision : 1 </div><div> </div><div>so we have to perform some translation to perform human readable information, and I mentioned that 'lscpu' has done that too. So Andrea Bolognani</div><div>suggested that maybe we can use it directly, to avoid re-implement the translation. Here is the discussion: <a href="https://www.redhat.com/archives/libvir-list/2020-March/msg00812.html">https://www.redhat.com/archives/libvir-list/2020-March/msg00812.html</a></div><div> </div><div>BR,</div><div> </div><div>Zhenyu</div></div> <div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Mar 30, 2020 at 7:27 PM Daniel P. Berrangé <<a href="mailto:berrange@redhat.com">berrange@redhat.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Fri, Mar 27, 2020 at 04:52:25PM +0800, Zhenyu Zheng wrote: > Introduce getHost support for ARM, use data from 'lscpu' cmd > result. 'util-linux/lscpu' provided a very good translation of > ARM cpu data start from release v2.32, use it directly to avoid > re-implement the translation. I'm a bit wary of this approach, because parsing the output of command line tools is generally fragile / liable to break in future releases, since few tools guarantee that their output format is stable for machine parsing. How hard would it be to probe this directly in libvirt, as we do on x86 arch. > > Signed-of-by: Zhenyu Zheng <<a href="mailto:zhengzhenyulixi@gmail.com" target="_blank">zhengzhenyulixi@gmail.com</a>> > --- > src/cpu/cpu_arm.c | 198 +++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 197 insertions(+), 1 deletion(-) > > diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c > index 969025b5cf..30d9c0ae2e 100644 > --- a/src/cpu/cpu_arm.c > +++ b/src/cpu/cpu_arm.c > @@ -22,8 +22,11 @@ > #include <config.h> > > #include "viralloc.h" > +#include "virlog.h" > #include "cpu.h" > #include "cpu_map.h" > +#include "vircommand.h" > +#include "virfile.h" > #include "virstring.h" > #include "virxml.h" > #include "cpu_map.h" > @@ -31,6 +34,14 @@ > > #define VIR_FROM_THIS VIR_FROM_CPU > > +VIR_LOG_INIT("cpu.cpu_arm"); > + > +static const char *lsCpuPath = "/usr/bin/lscpu"; > + > +#define LSCPU lsCpuPath > +#define MAX_LSCPU_SIZE = (1024*1024) /* 1MB limit for lscpu output */ > + > + > static const virArch archs[] = { > VIR_ARCH_ARMV6L, > VIR_ARCH_ARMV7B, > @@ -464,13 +475,198 @@ virCPUarmValidateFeatures(virCPUDefPtr cpu) > return 0; > } > > +static int > +armCpuDataFromLsCpu(virCPUarmData *data) > +{ > + int ret = -1; > + char *outbuf = NULL; > + char *eol = NULL; > + const char *cur; > + virCommandPtr cmd = NULL; > + g_autofree char *lscpu = NULL; > + > + if (!data) > + return ret; > + > + lscpu = virFindFileInPath("lscpu"); > + cmd = virCommandNew(lscpu); > + virCommandSetOutputBuffer(cmd, &outbuf); > + > + if (virCommandRun(cmd, NULL) < 0) > + goto cleanup; > + > + if ((cur = strstr(outbuf, "Vendor ID")) == NULL) { > + virReportError(VIR_ERR_INTERNAL_ERROR, > + _("there is no \"Vendor ID\" info in %s command result"), LSCPU); > + goto cleanup; > + } > + cur = strchr(cur, ':') + 1; > + eol = strchr(cur, '\n'); > + virSkipSpaces(&cur); > + if (!eol) > + goto cleanup; > + > + data->vendor_id = g_strndup(cur, eol - cur); > + > + if ((cur = strstr(outbuf, "Model name")) == NULL) { > + virReportError(VIR_ERR_INTERNAL_ERROR, > + _("there is no \"Model name\" info in %s command result"), LSCPU); > + goto cleanup; > + } > + cur = strchr(cur, ':') + 1; > + eol = strchr(cur, '\n'); > + virSkipSpaces(&cur); > + if (!eol) > + goto cleanup; > + > + data->model_name = g_strndup(cur, eol - cur); > + > + if ((cur = strstr(outbuf, "Flags")) == NULL) { > + virReportError(VIR_ERR_INTERNAL_ERROR, > + _("there is no \"Flags\" info in %s command result"), LSCPU); > + goto cleanup; > + } > + cur = strchr(cur, ':') + 1; > + eol = strchr(cur, '\n'); > + virSkipSpaces(&cur); > + if (!eol) > + goto cleanup; > + > + data->features = g_strndup(cur, eol - cur); > + > + ret = 0; > + > + cleanup: > + virCommandFree(cmd); > + VIR_FREE(outbuf); > + return ret; > +} > + > +static int > +armCpuDataParseFeatures(virCPUDefPtr cpu, > + const virCPUarmData *cpuData) > +{ > + int ret = -1; > + size_t i; > + char **features; > + > + if (!cpu || !cpuData) > + return ret; > + > + if (!(features = virStringSplitCount(cpuData->features, " ", > + 0, &cpu->nfeatures))) > + return ret; > + if (cpu->nfeatures) { > + if (VIR_ALLOC_N(cpu->features, cpu->nfeatures) < 0) > + goto error; > + > + for (i = 0; i < cpu->nfeatures; i++) { > + cpu->features[i].policy = VIR_CPU_FEATURE_REQUIRE; > + cpu->features[i].name = g_strdup(features[i]); > + } > + } > + > + ret = 0; > + > + cleanup: > + virStringListFree(features); > + return ret; > + > + error: > + for (i = 0; i < cpu->nfeatures; i++) > + VIR_FREE(cpu->features[i].name); > + VIR_FREE(cpu->features); > + cpu->nfeatures = 0; > + goto cleanup; > +} > + > +static int > +armDecode(virCPUDefPtr cpu, > + const virCPUarmData *cpuData, > + virDomainCapsCPUModelsPtr models) > +{ > + virCPUarmMapPtr map; > + virCPUarmModelPtr model; > + virCPUarmVendorPtr vendor = NULL; > + > + if (!cpuData || !(map = virCPUarmGetMap())) > + return -1; > + > + if (!(model = armModelFind(map, cpuData->model_name))) { > + virReportError(VIR_ERR_OPERATION_FAILED, > + _("Cannot find CPU model with name %s"), > + cpuData->model_name); > + return -1; > + } > + > + if (!virCPUModelIsAllowed(model->name, models)) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("CPU model %s is not supported by hypervisor"), > + model->name); > + return -1; > + } > + > + cpu->model = g_strdup(model->name); > + > + if (cpuData->vendor_id && > + !(vendor = armVendorFindByName(map, cpuData->vendor_id))) { > + virReportError(VIR_ERR_OPERATION_FAILED, > + _("Cannot find CPU vendor with vendor id %s"), > + cpuData->vendor_id); > + return -1; > + } > + > + > + if (vendor) > + cpu->vendor = g_strdup(vendor->name); > + > + if (cpuData->features && > + armCpuDataParseFeatures(cpu, cpuData) < 0) > + return -1; > + > + return 0; > +} > + > +static int > +armDecodeCPUData(virCPUDefPtr cpu, > + const virCPUData *data, > + virDomainCapsCPUModelsPtr models) > +{ > + return armDecode(cpu, &data->data.arm, models); > +} > + > +static int > +virCPUarmGetHost(virCPUDefPtr cpu, > + virDomainCapsCPUModelsPtr models) > +{ > + virCPUDataPtr cpuData = NULL; > + int ret = -1; > + > + if (virCPUarmDriverInitialize() < 0) > + goto cleanup; > + > + if (!(cpuData = virCPUDataNew(archs[0]))) > + goto cleanup; > + > + if (armCpuDataFromLsCpu(&cpuData->data.arm) < 0) > + goto cleanup; > + > + ret = armDecodeCPUData(cpu, cpuData, models); > + > + cleanup: > + virCPUarmDataFree(cpuData); > + return ret; > +} > + > struct cpuArchDriver cpuDriverArm = { > .name = "arm", > .arch = archs, > .narch = G_N_ELEMENTS(archs), > .compare = virCPUarmCompare, > - .decode = NULL, > + .decode = armDecodeCPUData, > .encode = NULL, > + .dataFree = virCPUarmDataFree, > + .getHost = virCPUarmGetHost, > .baseline = virCPUarmBaseline, > .update = virCPUarmUpdate, > .validateFeatures = virCPUarmValidateFeatures, > -- > 2.26.0.windows.1 > > Regards, Daniel -- |: <a href="https://berrange.com" rel="noreferrer" target="_blank">https://berrange.com</a> -o- <a href="https://www.flickr.com/photos/dberrange" rel="noreferrer" target="_blank">https://www.flickr.com/photos/dberrange</a> :| |: <a href="https://libvirt.org" rel="noreferrer" target="_blank">https://libvirt.org</a> -o- <a href="https://fstop138.berrange.com" rel="noreferrer" target="_blank">https://fstop138.berrange.com</a> :| |: <a href="https://entangle-photo.org" rel="noreferrer" target="_blank">https://entangle-photo.org</a> -o- <a href="https://www.instagram.com/dberrange" rel="noreferrer" target="_blank">https://www.instagram.com/dberrange</a> :| </blockquote></div>