Re: [PATCH V2 5/5] cpu: Introduce getHost support for ARM CPU driver
Yes, I will do that. On Mon, Apr 20, 2020 at 10:12 PM Jiri Denemark wrote: > On Fri, Apr 17, 2020 at 16:53:18 +0800, Zhenyu Zheng wrote: > > Ping for reviews > > Could you please resend the new version of patches as a separate series > (don't forget to update the subject to v3)? Hunting for the new patches > hidden in random replies to the reviewers comments or original patches > is not exactly easy. And even harder when they use the same subject as > version 2. > > Jirka >
Re: [PATCH V2 5/5] cpu: Introduce getHost support for ARM CPU driver
On Fri, Apr 17, 2020 at 16:53:18 +0800, Zhenyu Zheng wrote: > Ping for reviews Could you please resend the new version of patches as a separate series (don't forget to update the subject to v3)? Hunting for the new patches hidden in random replies to the reviewers comments or original patches is not exactly easy. And even harder when they use the same subject as version 2. Jirka
Re: [PATCH V2 5/5] cpu: Introduce getHost support for ARM CPU driver
Ping for reviews
On Fri, Apr 10, 2020 at 5:55 PM Andrea Bolognani
wrote:
> On Thu, 2020-04-09 at 13:03 +0200, Jiri Denemark wrote:
> > On Thu, Apr 02, 2020 at 17:03:59 +0800, Zhenyu Zheng wrote:
> > > +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?
>
> You tell me :)
>
> As you know, the main thing that sets x86 and ARM apart in this area
> is that on x86 you can basically mix and match CPU features at your
> heart's content, but on ARM this is not (yet) possible: the only
> features that you can really control are the SVE-related ones.
>
> I assume that, at some point further down the line, it will be
> possible to control ARM CPU features with a similar level of
> granularity as it's currently possible on x86, and at that point
> something like
>
>
>
>
>
>
> will be possible, but that's certainly not the case now, and
> attempting to do so for non-SVE features will result in QEMU refusing
> to start.
>
> With that in mind, I don't think it's a problem to include these
> features in cpu_map upfront and report them in the context of the
> host CPU: any attempt to use them in guest CPU context will be
> blocked by QEMU anyway.
>
> Perhaps we could go one step further and fail validation in libvirt
> until such a time comes when QEMU is able to accept them? That would
> make for a more pleasant user experience, I think.
>
> Do you foresee any potential issues caused by this approach? As I
> said it seems to me like it would be fine, but you're the expert when
> it comes to the CPU drivers :)
>
> --
> Andrea Bolognani / Red Hat / Virtualization
>
>
Re: [PATCH V2 5/5] cpu: Introduce getHost support for ARM CPU driver
On Thu, 2020-04-09 at 13:03 +0200, Jiri Denemark wrote:
> On Thu, Apr 02, 2020 at 17:03:59 +0800, Zhenyu Zheng wrote:
> > +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?
You tell me :)
As you know, the main thing that sets x86 and ARM apart in this area
is that on x86 you can basically mix and match CPU features at your
heart's content, but on ARM this is not (yet) possible: the only
features that you can really control are the SVE-related ones.
I assume that, at some point further down the line, it will be
possible to control ARM CPU features with a similar level of
granularity as it's currently possible on x86, and at that point
something like
will be possible, but that's certainly not the case now, and
attempting to do so for non-SVE features will result in QEMU refusing
to start.
With that in mind, I don't think it's a problem to include these
features in cpu_map upfront and report them in the context of the
host CPU: any attempt to use them in guest CPU context will be
blocked by QEMU anyway.
Perhaps we could go one step further and fail validation in libvirt
until such a time comes when QEMU is able to accept them? That would
make for a more pleasant user experience, I think.
Do you foresee any potential issues caused by this approach? As I
said it seems to me like it would be fine, but you're the expert when
it comes to the CPU drivers :)
--
Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH V2 5/5] cpu: Introduce getHost support for ARM CPU driver
Hi Daniel, Jirka
Thanks alot for the review and info, I have updated the code to only
compile on aarch64 platform, and I've done the gitlab CI testing as
suggested: https://gitlab.com/ZhengZhenyu/libvirt/pipelines/134657317 and
manually tested on aarch64.
BR,
On Fri, Apr 10, 2020 at 4:21 PM ZhengZhenyu
wrote:
> Introduce getHost support for ARM CPU driver, read
> CPU vendor_id, part_id and flags from registers
> directly.
>
> Signed-off-by: Zhenyu Zheng
> ---
> src/cpu/cpu_arm.c | 202 +-
> 1 file changed, 201 insertions(+), 1 deletion(-)
>
> diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c
> index 24404eac2c..8d6d435bee 100644
> --- a/src/cpu/cpu_arm.c
> +++ b/src/cpu/cpu_arm.c
> @@ -21,6 +21,10 @@
> */
>
> #include
> +#if defined(__aarch64__)
> +# include
> +# include
> +#endif
>
> #include "viralloc.h"
> #include "cpu.h"
> @@ -31,6 +35,12 @@
> #include "virxml.h"
>
> #define VIR_FROM_THIS VIR_FROM_CPU
> +#if defined(__aarch64__)
> +/* 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
> +#endif
>
> VIR_LOG_INIT("cpu.cpu_arm");
>
> @@ -495,13 +505,203 @@ virCPUarmValidateFeatures(virCPUDefPtr cpu)
> return 0;
> }
>
> +#if defined(__aarch64__)
> +/**
> + * virCPUarmCpuDataFromRegs:
> + *
> + * @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
> +virCPUarmCpuDataFromRegs(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"};
> +unsigned long cpuid, hwcaps;
> +char **features = NULL;
> +g_autofree 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 error;
> +}
> +data->features = g_strdup(cpu_feature_str);
> +
> +return 0;
> +
> + error:
> +virStringListFree(features);
> +return -1;
> +}
> +
> +static int
> +virCPUarmDataParseFeatures(virCPUDefPtr cpu,
> + const virCPUarmData *cpuData)
> +{
> +int ret = -1;
> +size_t i;
> +char **features;
> +
> +if (!cpu || !cpuData)
> +return ret;
> +
> +if (!(features = virStringSplitCount(cpuData->features, " ",
> + 0, >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
> +virCPUarmDecode(virCPUDefPtr cpu,
> +const virCPUarmData *cpuData,
> +virDomainCapsCPUModelsPtr models)
> +{
> +virCPUarmMapPtr map;
> +virCPUarmModelPtr model;
> +virCPUarmVendorPtr vendor = NULL;
> +
> +if (!cpuData ||
[PATCH V2 5/5] cpu: Introduce getHost support for ARM CPU driver
Introduce getHost support for ARM CPU driver, read
CPU vendor_id, part_id and flags from registers
directly.
Signed-off-by: Zhenyu Zheng
---
src/cpu/cpu_arm.c | 202 +-
1 file changed, 201 insertions(+), 1 deletion(-)
diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c
index 24404eac2c..8d6d435bee 100644
--- a/src/cpu/cpu_arm.c
+++ b/src/cpu/cpu_arm.c
@@ -21,6 +21,10 @@
*/
#include
+#if defined(__aarch64__)
+# include
+# include
+#endif
#include "viralloc.h"
#include "cpu.h"
@@ -31,6 +35,12 @@
#include "virxml.h"
#define VIR_FROM_THIS VIR_FROM_CPU
+#if defined(__aarch64__)
+/* 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
+#endif
VIR_LOG_INIT("cpu.cpu_arm");
@@ -495,13 +505,203 @@ virCPUarmValidateFeatures(virCPUDefPtr cpu)
return 0;
}
+#if defined(__aarch64__)
+/**
+ * virCPUarmCpuDataFromRegs:
+ *
+ * @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
+virCPUarmCpuDataFromRegs(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"};
+unsigned long cpuid, hwcaps;
+char **features = NULL;
+g_autofree 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 error;
+}
+data->features = g_strdup(cpu_feature_str);
+
+return 0;
+
+ error:
+virStringListFree(features);
+return -1;
+}
+
+static int
+virCPUarmDataParseFeatures(virCPUDefPtr cpu,
+ const virCPUarmData *cpuData)
+{
+int ret = -1;
+size_t i;
+char **features;
+
+if (!cpu || !cpuData)
+return ret;
+
+if (!(features = virStringSplitCount(cpuData->features, " ",
+ 0, >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
+virCPUarmDecode(virCPUDefPtr cpu,
+const virCPUarmData *cpuData,
+virDomainCapsCPUModelsPtr models)
+{
+virCPUarmMapPtr map;
+virCPUarmModelPtr model;
+virCPUarmVendorPtr vendor = NULL;
+
+if (!cpuData || !(map = virCPUarmGetMap()))
+return -1;
+
+if (!(model = virCPUarmModelFindByPVR(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
[PATCH V2 5/5] cpu: Introduce getHost support for ARM CPU driver
Introduce getHost support for ARM CPU driver, read
CPU vendor_id, part_id and flags from registers
directly.
Signed-off-by: Zhenyu Zheng
---
src/cpu/cpu_arm.c | 193 +-
1 file changed, 192 insertions(+), 1 deletion(-)
diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c
index 605e7b2d51..2ac8ad3bff 100644
--- a/src/cpu/cpu_arm.c
+++ b/src/cpu/cpu_arm.c
@@ -21,6 +21,8 @@
*/
#include
+#include
+#include
#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");
@@ -492,14 +498,199 @@ virCPUarmValidateFeatures(virCPUDefPtr cpu)
return 0;
}
+/**
+ * virCPUarmCpuDataFromRegs:
+ *
+ * @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
+virCPUarmCpuDataFromRegs(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"};
+unsigned long cpuid, hwcaps;
+char **features = NULL;
+g_autofree 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 error;
+}
+data->features = g_strdup(cpu_feature_str);
+
+return 0;
+
+ error:
+virStringListFree(features);
+return -1;
+}
+
+static int
+virCPUarmDataParseFeatures(virCPUDefPtr cpu,
+ const virCPUarmData *cpuData)
+{
+int ret = -1;
+size_t i;
+char **features;
+
+if (!cpu || !cpuData)
+return ret;
+
+if (!(features = virStringSplitCount(cpuData->features, " ",
+ 0, >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
+virCPUarmDecode(virCPUDefPtr cpu,
+const virCPUarmData *cpuData,
+virDomainCapsCPUModelsPtr models)
+{
+virCPUarmMapPtr map;
+virCPUarmModelPtr model;
+virCPUarmVendorPtr vendor = NULL;
+
+if (!cpuData || !(map = virCPUarmGetMap()))
+return -1;
+
+if (!(model = virCPUarmModelFindByPVR(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 = virCPUarmVendorFindByID(map, cpuData->vendor_id))) {
+
Re: [PATCH V2 5/5] cpu: Introduce getHost support for ARM CPU driver
Thanks for the info, as for ARM testing, we are happy to provide some machines if the community likes. BR, On Thu, Apr 9, 2020 at 8:03 PM Daniel P. Berrangé wrote: > On Thu, Apr 09, 2020 at 07:52:27PM +0800, Zhenyu Zheng wrote: > > 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 > > The key requirement from libvirt is that code must compile against our > declared set of supported platforms > > https://libvirt.org/platforms.html > > Usually RHEL-7 is the oldest platform that causes trouble. > > If code can't be made to compile on old platforms, then it is acceptable > to use conditional compilation to disable the code. > > If you want to validate your patches build on all our required platforms, > you can take advantage of our recent switch to GitLab > > Fork the libvirt repo: > > https://gitlab.com/libvirt/libvirt > > and push your patches to a branch in your personal fork. This will trigger > our automated build CI jobs across all important platforms. > > As an example here's a recent CI job run: > > https://gitlab.com/libvirt/libvirt/pipelines/134381772 > > Note, however, that most of the jobs are using x86, so if you have any > conditionally compiled arm code, we don't have good coverage for arm > on the old distros, so that may benefit from manual testing. > > > > 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. > > Regards, > Daniel > -- > |: https://berrange.com -o- > https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o- > https://fstop138.berrange.com :| > |: https://entangle-photo.org-o- > https://www.instagram.com/dberrange :| > >
Re: [PATCH V2 5/5] cpu: Introduce getHost support for ARM CPU driver
On Thu, Apr 09, 2020 at 07:52:27PM +0800, Zhenyu Zheng wrote: > 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 The key requirement from libvirt is that code must compile against our declared set of supported platforms https://libvirt.org/platforms.html Usually RHEL-7 is the oldest platform that causes trouble. If code can't be made to compile on old platforms, then it is acceptable to use conditional compilation to disable the code. If you want to validate your patches build on all our required platforms, you can take advantage of our recent switch to GitLab Fork the libvirt repo: https://gitlab.com/libvirt/libvirt and push your patches to a branch in your personal fork. This will trigger our automated build CI jobs across all important platforms. As an example here's a recent CI job run: https://gitlab.com/libvirt/libvirt/pipelines/134381772 Note, however, that most of the jobs are using x86, so if you have any conditionally compiled arm code, we don't have good coverage for arm on the old distros, so that may benefit from manual testing. > > 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. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH V2 5/5] cpu: Introduce getHost support for ARM CPU driver
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 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
> > ---
> > 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
> > +#include
> > +#include
>
> 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, >nfeatures)))
> > +return ret;
> > +if (cpu->nfeatures) {
Re: [PATCH V2 5/5] cpu: Introduce getHost support for ARM CPU driver
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
> ---
> 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
> +#include
> +#include
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, >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()))
Re: [PATCH V2 5/5] cpu: Introduce getHost support for ARM CPU driver
On Thu, Apr 09, 2020 at 11:28:04 +0100, Daniel P. Berrangé wrote:
> On Thu, Apr 02, 2020 at 05:03:59PM +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
> > ---
> > src/cpu/cpu_arm.c | 194 +-
> > 1 file changed, 193 insertions(+), 1 deletion(-)
> >
> > 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
> > +#include
> > +#include
> >
> > #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)
>
> virCPUarmDataFromRegs()
>
> > +{
> > +/* 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"};
> > +unsigned long cpuid, hwcaps;
> > +char **features = NULL;
> > +char *cpu_feature_str = NULL;
>
> g_autofree
>
> > +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;
> > +}
>
> Indent messed up here
>
> > +
> > +/* 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++;
> > +}
> > +}
>
> Indent of } messed up again.
>
> > +
> > +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:
>
> This should be called "error", since this label is only
> visited for fatal errors.
>
> > +virStringListFree(features);
> > +VIR_FREE(cpu_feature_str);
>
> Can avoid the VIR_FREE with g_autofree.
virStringListFree can be avoided too if features is declared with
VIR_AUTOSTRINGLIST.
Jirka
Re: [PATCH V2 5/5] cpu: Introduce getHost support for ARM CPU driver
On Thu, Apr 02, 2020 at 05:03:59PM +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
> ---
> src/cpu/cpu_arm.c | 194 +-
> 1 file changed, 193 insertions(+), 1 deletion(-)
>
> 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
> +#include
> +#include
>
> #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)
virCPUarmDataFromRegs()
> +{
> +/* 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"};
> +unsigned long cpuid, hwcaps;
> +char **features = NULL;
> +char *cpu_feature_str = NULL;
g_autofree
> +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;
> +}
Indent messed up here
> +
> +/* 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++;
> +}
> +}
Indent of } messed up again.
> +
> +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:
This should be called "error", since this label is only
visited for fatal errors.
> +virStringListFree(features);
> +VIR_FREE(cpu_feature_str);
Can avoid the VIR_FREE with g_autofree.
> +return -1;
> +}
> +
> +static int
> +armCpuDataParseFeatures(virCPUDefPtr cpu,
> +const virCPUarmData *cpuData)
virCPUarmDataParseFeatures()
> +{
> +int ret = -1;
> +size_t i;
> +char **features;
> +
> +if (!cpu || !cpuData)
> +return ret;
> +
> +if (!(features = virStringSplitCount(cpuData->features, " ",
> + 0, >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)
virCPUDataArmDecode
> +{
> +virCPUarmMapPtr map;
> +virCPUarmModelPtr model;
> +virCPUarmVendorPtr vendor = NULL;
> +
> +if (!cpuData || !(map = virCPUarmGetMap()))
> +return -1;
> +
> +if (!(model
[PATCH V2 5/5] cpu: Introduce getHost support for ARM CPU driver
Introduce getHost support for ARM CPU driver, read
CPU vendor_id, part_id and flags from registers
directly.
Signed-off-by: Zhenyu Zheng
---
src/cpu/cpu_arm.c | 194 +-
1 file changed, 193 insertions(+), 1 deletion(-)
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
+#include
+#include
#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"};
+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, >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))) {
+
