[PATCH V2 5/5] cpu: Introduce getHost support for ARM CPU driver
Zhenyu Zheng
zhengzhenyulixi at gmail.com
Thu Apr 9 11:52:27 UTC 2020
Hi,
Thanks alot for all the reviews, I'm updating the codes, as for those
headers,
hwcap.h is here:
https://github.com/torvalds/linux/blob/v5.6/arch/arm/include/asm/hwcap.h and
should be available for 5.6,
auxv.h is from glibc:
https://sourceware.org/git/?p=glibc.git;a=blob;f=misc/sys/auxv.h;h=1a563e1337e64e4218c1613b68dc2551a762ba00;hb=HEAD
> virCPUarmGetHost and the helpers it calls are architecture specific and
> should not be even compiled in on non-ARM.
yeah, understandable, I just don't see other archs like ppc and s390 doing
this so I didn't do it.
BR,
On Thu, Apr 9, 2020 at 7:03 PM Jiri Denemark <jdenemar at redhat.com> wrote:
> On Thu, Apr 02, 2020 at 17:03:59 +0800, Zhenyu Zheng wrote:
> > Introduce getHost support for ARM CPU driver, read
> > CPU vendor_id, part_id and flags from registers
> > directly.
> >
> > Signed-off-by: Zhenyu Zheng <zhengzhenyulixi at gmail.com>
> > ---
> > src/cpu/cpu_arm.c | 194 +++++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 193 insertions(+), 1 deletion(-)
>
> In addition to all the coding style issues which Daniel pointed out...
>
> > diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c
> > index 88b4d91946..c8f5ce7e8a 100644
> > --- a/src/cpu/cpu_arm.c
> > +++ b/src/cpu/cpu_arm.c
> > @@ -21,6 +21,8 @@
> > */
> >
> > #include <config.h>
> > +#include <asm/hwcap.h>
> > +#include <sys/auxv.h>
>
> None of these header files exist on my system with linux 5.6 headers.
>
> > #include "viralloc.h"
> > #include "cpu.h"
> > @@ -31,6 +33,10 @@
> > #include "virxml.h"
> >
> > #define VIR_FROM_THIS VIR_FROM_CPU
> > +/* Shift bit mask for parsing cpu flags */
> > +#define BIT_SHIFTS(n) (1UL << (n))
> > +/* The current max number of cpu flags on ARM is 32 */
> > +#define MAX_CPU_FLAGS 32
> >
> > VIR_LOG_INIT("cpu.cpu_arm");
> >
> > @@ -498,14 +504,200 @@ virCPUarmValidateFeatures(virCPUDefPtr cpu)
> > return 0;
> > }
> >
> > +/**
> > + * armCpuDataFromRegs:
> > + *
> > + * @data: 64-bit arm CPU specific data
> > + *
> > + * Fetches CPU vendor_id and part_id from MIDR_EL1 register, parse CPU
> > + * flags from AT_HWCAP. There are currently 32 valid flags on ARM arch
> > + * represented by each bit.
> > + */
> > +static int
> > +armCpuDataFromRegs(virCPUarmData *data)
> > +{
> > + /* Generate human readable flag list according to the order of */
> > + /* AT_HWCAP bit map */
> > + const char *flag_list[MAX_CPU_FLAGS] = {
> > + "fp", "asimd", "evtstrm", "aes", "pmull", "sha1", "sha2",
> > + "crc32", "atomics", "fphp", "asimdhp", "cpuid", "asimdrdm",
> > + "jscvt", "fcma", "lrcpc", "dcpop", "sha3", "sm3", "sm4",
> > + "asimddp", "sha512", "sve", "asimdfhm", "dit", "uscat",
> > + "ilrcpc", "flagm", "ssbs", "sb", "paca", "pacg"};
>
> I would expect these to be defined in src/cpu_map/arm_features.xml,
> although I'm not sure how well the new features would play with the
> existing ones... What do you think, Andrea?
>
> > + unsigned long cpuid, hwcaps;
> > + char **features = NULL;
> > + char *cpu_feature_str = NULL;
> > + int cpu_feature_index = 0;
> > + size_t i;
> > +
> > + if (!(getauxval(AT_HWCAP) & HWCAP_CPUID)) {
> > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > + _("CPUID registers unavailable"));
> > + return -1;
> > + }
> > +
> > + /* read the cpuid data from MIDR_EL1 register */
> > + asm("mrs %0, MIDR_EL1" : "=r" (cpuid));
> > + VIR_DEBUG("CPUID read from register: 0x%016lx", cpuid);
> > +
> > + /* parse the coresponding part_id bits */
> > + data->pvr = cpuid>>4&0xFFF;
> > + /* parse the coresponding vendor_id bits */
> > + data->vendor_id = cpuid>>24&0xFF;
> > +
> > + hwcaps = getauxval(AT_HWCAP);
> > + VIR_DEBUG("CPU flags read from register: 0x%016lx", hwcaps);
> > +
> > + if (VIR_ALLOC_N(features, MAX_CPU_FLAGS) < 0)
> > + return -1;
> > +
> > + /* shift bit map mask to parse for CPU flags */
> > + for (i = 0; i< MAX_CPU_FLAGS; i++) {
> > + if (hwcaps & BIT_SHIFTS(i)) {
> > + features[cpu_feature_index] = g_strdup(flag_list[i]);
> > + cpu_feature_index++;
> > + }
> > + }
> > +
> > + if (cpu_feature_index > 1) {
> > + cpu_feature_str = virStringListJoin((const char **)features, "
> ");
> > + if (!cpu_feature_str)
> > + goto cleanup;
> > + }
> > + data->features = g_strdup(cpu_feature_str);
> > +
> > + return 0;
> > +
> > + cleanup:
> > + virStringListFree(features);
> > + VIR_FREE(cpu_feature_str);
> > + return -1;
> > +}
> > +
> > +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 = armModelFindByPVR(map, cpuData->pvr))) {
> > + virReportError(VIR_ERR_OPERATION_FAILED,
> > + _("Cannot find CPU model with PVR 0x%03lx"),
> > + cpuData->pvr);
> > + 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 = armVendorFindByID(map, cpuData->vendor_id))) {
> > + virReportError(VIR_ERR_OPERATION_FAILED,
> > + _("Cannot find CPU vendor with vendor id
> 0x%02lx"),
> > + 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 (armCpuDataFromRegs(&cpuData->data.arm) < 0)
> > + goto cleanup;
> > +
> > + ret = armDecodeCPUData(cpu, cpuData, models);
> > +
> > + cleanup:
> > + virCPUarmDataFree(cpuData);
> > + return ret;
> > +}
>
> virCPUarmGetHost and the helpers it calls are architecture specific and
> should not be even compiled in on non-ARM.
>
> Jirka
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20200409/f5a1ea18/attachment-0001.htm>
More information about the libvir-list
mailing list