11:50 p.m.
I've successfully tested these on mingw, arm and x86_64.
Almost trivial, but not pushed yet. I'll wait for review.
Michal Prívozník (3):
virhostcpu.c: Fix misalignment in virHostCPUGetMSRFromKVM comment
virhostcpu: Make virHostCPUGetMSR() work only on x86
cpu_x86: Fix placement of *CheckFeature functions
src/cpu/cpu_x86.c | 68 ++++++++++++++++++++++---------------------
src/util/virhostcpu.c | 34 ++++++++++++----------
2 files changed, 54 insertions(+), 48 deletions(-)
--
2.21.0
11:50 p.m.
New subject: [libvirt] [PATCH 1/3] virhostcpu.c: Fix misalignment in virHostCPUGetMSRFromKVM comment
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/util/virhostcpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
index 10bf3a93d5..dabe3eed82 100644
--- a/src/util/virhostcpu.c
+++ b/src/util/virhostcpu.c
@@ -1301,7 +1301,7 @@ virHostCPUGetMSRFromKVM(unsigned long index ATTRIBUTE_UNUSED,
/*
* Returns 0 on success,
* 1 when the MSR is not supported by the host CPU,
-* -1 on error.
+ * -1 on error.
*/
int
virHostCPUGetMSR(unsigned long index,
--
2.21.0
11:50 p.m.
New subject: [libvirt] [PATCH 2/3] virhostcpu: Make virHostCPUGetMSR() work only on x86
Model specific registers are a thing only on x86. Also, the
/dev/cpu/0/msr path exists only on Linux and the fallback
mechanism (asking KVM) exists on Linux and FreeBSD only.
Therefore, move the function within #ifdef that checks all
aforementioned constraints and provide a dummy stub for all
other cases.
This fixes the build on my arm box, mingw-* builds, etc.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/util/virhostcpu.c | 32 ++++++++++++++++++--------------
1 file changed, 18 insertions(+), 14 deletions(-)
diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
index dabe3eed82..f4a62c74fa 100644
--- a/src/util/virhostcpu.c
+++ b/src/util/virhostcpu.c
@@ -1257,7 +1257,9 @@ virHostCPUGetMicrocodeVersion(void)
#endif /* __linux__ */
-#if HAVE_LINUX_KVM_H && defined(KVM_GET_MSRS)
+#if HAVE_LINUX_KVM_H && defined(KVM_GET_MSRS) && \
+ (defined(__i386__) || defined(__x86_64__)) && \
+ (defined(__linux__) || defined(__FreeBSD__))
static int
virHostCPUGetMSRFromKVM(unsigned long index,
uint64_t *result)
@@ -1285,19 +1287,6 @@ virHostCPUGetMSRFromKVM(unsigned long index,
return 0;
}
-#else
-
-static int
-virHostCPUGetMSRFromKVM(unsigned long index ATTRIBUTE_UNUSED,
- uint64_t *result ATTRIBUTE_UNUSED)
-{
- virReportSystemError(ENOSYS, "%s",
- _("Reading MSRs via KVM is not supported on this
platform"));
- return -1;
-}
-#endif /* HAVE_LINUX_KVM_H && defined(KVM_GET_MSRS) */
-
-
/*
* Returns 0 on success,
* 1 when the MSR is not supported by the host CPU,
@@ -1334,3 +1323,18 @@ virHostCPUGetMSR(unsigned long index,
return virHostCPUGetMSRFromKVM(index, msr);
}
+
+#else
+
+int
+virHostCPUGetMSR(unsigned long index ATTRIBUTE_UNUSED,
+ uint64_t *msr ATTRIBUTE_UNUSED)
+{
+ virReportSystemError(ENOSYS, "%s",
+ _("Reading MSRs is not supported on this platform"));
+ return -1;
+}
+
+#endif /* HAVE_LINUX_KVM_H && defined(KVM_GET_MSRS) && \
+ (defined(__i386__) || defined(__x86_64__)) && \
+ (defined(__linux__) || defined(__FreeBSD__)) */
--
2.21.0
11:50 p.m.
New subject: [libvirt] [PATCH 3/3] cpu_x86: Fix placement of *CheckFeature functions
In e17d10386 these functions were mistakenly moved into an #ifdef
block, but remained used outside of it leaving the build broken
for platforms where #ifdef evaluated to false.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/cpu/cpu_x86.c | 68 ++++++++++++++++++++++++-----------------------
1 file changed, 35 insertions(+), 33 deletions(-)
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 1fe6770d57..f05bfa24e0 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -2707,39 +2707,6 @@ cpuidSet(uint32_t base, virCPUDataPtr data)
}
-static int
-virCPUx86CheckFeature(const virCPUDef *cpu,
- const char *name)
-{
- int ret = -1;
- virCPUx86MapPtr map;
- virCPUx86ModelPtr model = NULL;
-
- if (!(map = virCPUx86GetMap()))
- return -1;
-
- if (!(model = x86ModelFromCPU(cpu, map, -1)))
- goto cleanup;
-
- ret = x86FeatureInData(name, &model->data, map);
-
- cleanup:
- x86ModelFree(model);
- return ret;
-}
-
-
-static int
-virCPUx86DataCheckFeature(const virCPUData *data,
- const char *name)
-{
- virCPUx86MapPtr map;
-
- if (!(map = virCPUx86GetMap()))
- return -1;
-
- return x86FeatureInData(name, &data->data.x86, map);
-}
static int
@@ -2769,6 +2736,41 @@ virCPUx86GetHost(virCPUDefPtr cpu,
#endif
+static int
+virCPUx86CheckFeature(const virCPUDef *cpu,
+ const char *name)
+{
+ int ret = -1;
+ virCPUx86MapPtr map;
+ virCPUx86ModelPtr model = NULL;
+
+ if (!(map = virCPUx86GetMap()))
+ return -1;
+
+ if (!(model = x86ModelFromCPU(cpu, map, -1)))
+ goto cleanup;
+
+ ret = x86FeatureInData(name, &model->data, map);
+
+ cleanup:
+ x86ModelFree(model);
+ return ret;
+}
+
+
+static int
+virCPUx86DataCheckFeature(const virCPUData *data,
+ const char *name)
+{
+ virCPUx86MapPtr map;
+
+ if (!(map = virCPUx86GetMap()))
+ return -1;
+
+ return x86FeatureInData(name, &data->data.x86, map);
+}
+
+
static virCPUDefPtr
virCPUx86Baseline(virCPUDefPtr *cpus,
unsigned int ncpus,
--
2.21.0
1:12 a.m.
Michal Privoznik <mprivozn(a)redhat.com> [2019-04-15, 06:50AM +0200]:
I've successfully tested these on mingw, arm and x86_64.
Fixes S390 as well, thanks.
Almost trivial, but not pushed yet. I'll wait for review.
I got another incidential build error that doesn't seem related though:
../../examples/domtop/domtop.c: In function 'print_cpu_usage':
../../examples/domtop/domtop.c:242:28: error: 'pos' may be used uninitialized in
this function [-Werror=maybe-uninitialized]
usage = (now_params[pos].value.ul - then_params[pos].value.ul) /
^
cc1: all warnings being treated as errors
GCC is 8.2.1. Here's a patch:
diff --git a/examples/domtop/domtop.c b/examples/domtop/domtop.c
index c9b51aed..e463e287 100644
--- a/examples/domtop/domtop.c
+++ b/examples/domtop/domtop.c
@@ -206,7 +206,7 @@ print_cpu_usage(size_t cpu,
}
for (i = 0; i < ncpus; i++) {
- size_t pos;
+ size_t pos = 0;
double usage;
/* check if the vCPU is in the maps */
2:04 a.m.
On 4/15/19 8:12 AM, Bjoern Walk wrote:
Michal Privoznik <mprivozn(a)redhat.com> [2019-04-15, 06:50AM
+0200]:
> I've successfully tested these on mingw, arm and x86_64.
Fixes S390 as well, thanks.
> Almost trivial, but not pushed yet. I'll wait for review.
I got another incidential build error that doesn't seem related though:
Yep, it's unrelated.
../../examples/domtop/domtop.c: In function 'print_cpu_usage':
../../examples/domtop/domtop.c:242:28: error: 'pos' may be used uninitialized in
this function [-Werror=maybe-uninitialized]
usage = (now_params[pos].value.ul - then_params[pos].value.ul) /
^
cc1: all warnings being treated as errors
GCC is 8.2.1. Here's a patch:
diff --git a/examples/domtop/domtop.c b/examples/domtop/domtop.c
index c9b51aed..e463e287 100644
--- a/examples/domtop/domtop.c
+++ b/examples/domtop/domtop.c
@@ -206,7 +206,7 @@ print_cpu_usage(size_t cpu,
}
for (i = 0; i < ncpus; i++) {
- size_t pos;
+ size_t pos = 0;
double usage;
/* check if the vCPU is in the maps */
This is a false positive. Either the control cannot get to line 242 or
if it will then @pos was set. But if this patch helps you to compile
cleanly then I'm up for taking it. Mind sending it as a proper patch?
Thanks,
Michal
2:36 a.m.
On Mon, Apr 15, 2019 at 06:50:16 +0200, Michal Privoznik wrote:
I've successfully tested these on mingw, arm and x86_64.
Almost trivial, but not pushed yet. I'll wait for review.
Michal Prívozník (3):
virhostcpu.c: Fix misalignment in virHostCPUGetMSRFromKVM comment
virhostcpu: Make virHostCPUGetMSR() work only on x86
cpu_x86: Fix placement of *CheckFeature functions
src/cpu/cpu_x86.c | 68 ++++++++++++++++++++++---------------------
src/util/virhostcpu.c | 34 ++++++++++++----------
2 files changed, 54 insertions(+), 48 deletions(-)
Oops, thanks for fixing the issues.
Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com>
2048
days inactive
2048
days old
6 comments
3 participants
participants (3)
-
Bjoern Walk -
Jiri Denemark -
Michal Privoznik