5:53 p.m.
As promised in v1 this series contains a few additional patches which
limit usage of MSR features to QEMU that supports "unavailable-features"
CPU property.
This series intentionally enables MSR features for all QEMU versions
before restricting the usage to make this restriction more visible in
the test results.
Version 2:
- all but 2 patches from version 1 were pushed
- 5 new patches
Jiri Denemark (7):
conf: Introduce virCPUDefCheckFeatures
cpu_x86: Turn virCPUx86DataIteratorInit into a function
cpu_x86: Introduce virCPUx86FeatureIsMSR
cpu_x86: Read CPU features from IA32_ARCH_CAPABILITIES MSR
cpu_map: Introduce IA32_ARCH_CAPABILITIES MSR features
qemu: Forbid MSR features with old QEMU
qemu: Drop MSR features from host-model with old QEMU
src/conf/cpu_conf.c | 33 +++++++
src/conf/cpu_conf.h | 6 ++
src/cpu/cpu_x86.c | 96 +++++++++++++++++--
src/cpu/cpu_x86.h | 3 +
src/cpu_map/x86_features.xml | 20 ++++
src/libvirt_private.syms | 2 +
src/qemu/qemu_capabilities.c | 16 ++++
src/qemu/qemu_process.c | 29 +++++-
.../x86_64-cpuid-Core-i7-7600U-enabled.xml | 1 +
.../x86_64-cpuid-Core-i7-7600U-json.xml | 1 +
...86_64-cpuid-Xeon-Platinum-8268-enabled.xml | 1 +
.../x86_64-cpuid-Xeon-Platinum-8268-guest.xml | 4 +
.../x86_64-cpuid-Xeon-Platinum-8268-host.xml | 4 +
.../x86_64-cpuid-Xeon-Platinum-8268-json.xml | 3 +
.../qemu_4.1.0.x86_64.xml | 1 +
15 files changed, 207 insertions(+), 13 deletions(-)
--
2.22.0
5:53 p.m.
New subject: [libvirt] [PATCH v2 1/7] conf: Introduce virCPUDefCheckFeatures
This API can be used to check whether a CPU definition contains features
matching a given filter.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/conf/cpu_conf.c | 33 +++++++++++++++++++++++++++++++++
src/conf/cpu_conf.h | 6 ++++++
src/libvirt_private.syms | 1 +
3 files changed, 40 insertions(+)
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 675d214c50..7d16a05283 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -930,6 +930,39 @@ virCPUDefFilterFeatures(virCPUDefPtr cpu,
}
+/**
+ * virCPUDefCheckFeatures:
+ *
+ * Check CPU features for which @filter reports true and store them in a NULL
+ * terminated list returned via @features.
+ *
+ * Returns the number of features matching @filter or -1 on error.
+ */
+int
+virCPUDefCheckFeatures(virCPUDefPtr cpu,
+ virCPUDefFeatureFilter filter,
+ void *opaque,
+ char ***features)
+{
+ VIR_AUTOSTRINGLIST list = NULL;
+ size_t n = 0;
+ size_t i;
+
+ *features = NULL;
+
+ for (i = 0; i < cpu->nfeatures; i++) {
+ if (filter(cpu->features[i].name, opaque)) {
+ if (virStringListAdd(&list, cpu->features[i].name) < 0)
+ return -1;
+ n++;
+ }
+ }
+
+ VIR_STEAL_PTR(*features, list);
+ return n;
+}
+
+
bool
virCPUDefIsEqual(virCPUDefPtr src,
virCPUDefPtr dst,
diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h
index 906ef5368e..19ce816ec2 100644
--- a/src/conf/cpu_conf.h
+++ b/src/conf/cpu_conf.h
@@ -224,6 +224,12 @@ virCPUDefFilterFeatures(virCPUDefPtr cpu,
virCPUDefFeatureFilter filter,
void *opaque);
+int
+virCPUDefCheckFeatures(virCPUDefPtr cpu,
+ virCPUDefFeatureFilter filter,
+ void *opaque,
+ char ***features);
+
virCPUDefPtr *
virCPUDefListParse(const char **xmlCPUs,
unsigned int ncpus,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ab2e4bc6fe..90a6d10666 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -73,6 +73,7 @@ virCapabilitiesSetNetPrefix;
virCPUCacheModeTypeFromString;
virCPUCacheModeTypeToString;
virCPUDefAddFeature;
+virCPUDefCheckFeatures;
virCPUDefCopy;
virCPUDefCopyModel;
virCPUDefCopyModelFilter;
--
2.22.0
2:41 a.m.
New subject: [libvirt] [PATCH v2 1/7] conf: Introduce virCPUDefCheckFeatures
On Thu, Jun 20, 2019 at 12:53:36AM +0200, Jiri Denemark wrote:
This API can be used to check whether a CPU definition contains
features
matching a given filter.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/conf/cpu_conf.c | 33 +++++++++++++++++++++++++++++++++
src/conf/cpu_conf.h | 6 ++++++
src/libvirt_private.syms | 1 +
3 files changed, 40 insertions(+)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
5:53 p.m.
New subject: [libvirt] [PATCH v2 2/7] cpu_x86: Turn virCPUx86DataIteratorInit into a function
Until now, this was a macro usable for direct initialization when a
variable is defined. Turning the macro into a function makes it more
general.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/cpu/cpu_x86.c | 34 ++++++++++++++++++++++++----------
1 file changed, 24 insertions(+), 10 deletions(-)
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index b6a94d483a..6eef5cef00 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -189,8 +189,13 @@ struct _virCPUx86DataIterator {
};
-#define virCPUx86DataIteratorInit(data) \
- { data, -1 }
+static void
+virCPUx86DataIteratorInit(virCPUx86DataIteratorPtr iterator,
+ const virCPUx86Data *data)
+{
+ virCPUx86DataIterator iter = { data, -1 };
+ *iterator = iter;
+}
static bool
@@ -540,9 +545,10 @@ static int
x86DataAdd(virCPUx86Data *data1,
const virCPUx86Data *data2)
{
- virCPUx86DataIterator iter = virCPUx86DataIteratorInit(data2);
+ virCPUx86DataIterator iter;
virCPUx86DataItemPtr item;
+ virCPUx86DataIteratorInit(&iter, data2);
while ((item = virCPUx86DataNext(&iter))) {
if (virCPUx86DataAddItem(data1, item) < 0)
return -1;
@@ -556,10 +562,11 @@ static void
x86DataSubtract(virCPUx86Data *data1,
const virCPUx86Data *data2)
{
- virCPUx86DataIterator iter = virCPUx86DataIteratorInit(data1);
+ virCPUx86DataIterator iter;
virCPUx86DataItemPtr item1;
virCPUx86DataItemPtr item2;
+ virCPUx86DataIteratorInit(&iter, data1);
while ((item1 = virCPUx86DataNext(&iter))) {
item2 = virCPUx86DataGet(data2, item1);
virCPUx86DataItemClearBits(item1, item2);
@@ -571,10 +578,11 @@ static void
x86DataIntersect(virCPUx86Data *data1,
const virCPUx86Data *data2)
{
- virCPUx86DataIterator iter = virCPUx86DataIteratorInit(data1);
+ virCPUx86DataIterator iter;
virCPUx86DataItemPtr item1;
virCPUx86DataItemPtr item2;
+ virCPUx86DataIteratorInit(&iter, data1);
while ((item1 = virCPUx86DataNext(&iter))) {
item2 = virCPUx86DataGet(data2, item1);
if (item2)
@@ -588,8 +596,9 @@ x86DataIntersect(virCPUx86Data *data1,
static bool
x86DataIsEmpty(virCPUx86Data *data)
{
- virCPUx86DataIterator iter = virCPUx86DataIteratorInit(data);
+ virCPUx86DataIterator iter;
+ virCPUx86DataIteratorInit(&iter, data);
return !virCPUx86DataNext(&iter);
}
@@ -598,10 +607,11 @@ static bool
x86DataIsSubset(const virCPUx86Data *data,
const virCPUx86Data *subset)
{
- virCPUx86DataIterator iter = virCPUx86DataIteratorInit((virCPUx86Data *)subset);
+ virCPUx86DataIterator iter;
const virCPUx86DataItem *item;
const virCPUx86DataItem *itemSubset;
+ virCPUx86DataIteratorInit(&iter, subset);
while ((itemSubset = virCPUx86DataNext(&iter))) {
if (!(item = virCPUx86DataGet(data, itemSubset)) ||
!virCPUx86DataItemMatchMasked(item, itemSubset))
@@ -1314,11 +1324,13 @@ x86ModelCompare(virCPUx86ModelPtr model1,
virCPUx86ModelPtr model2)
{
virCPUx86CompareResult result = EQUAL;
- virCPUx86DataIterator iter1 = virCPUx86DataIteratorInit(&model1->data);
- virCPUx86DataIterator iter2 = virCPUx86DataIteratorInit(&model2->data);
+ virCPUx86DataIterator iter1;
+ virCPUx86DataIterator iter2;
virCPUx86DataItemPtr item1;
virCPUx86DataItemPtr item2;
+ virCPUx86DataIteratorInit(&iter1, &model1->data);
+ virCPUx86DataIteratorInit(&iter2, &model2->data);
while ((item1 = virCPUx86DataNext(&iter1))) {
virCPUx86CompareResult match = SUPERSET;
@@ -1624,10 +1636,12 @@ virCPUx86GetMap(void)
static char *
virCPUx86DataFormat(const virCPUData *data)
{
- virCPUx86DataIterator iter = virCPUx86DataIteratorInit(&data->data.x86);
+ virCPUx86DataIterator iter;
virCPUx86DataItemPtr item;
virBuffer buf = VIR_BUFFER_INITIALIZER;
+ virCPUx86DataIteratorInit(&iter, &data->data.x86);
+
virBufferAddLit(&buf, "<cpudata arch='x86'>\n");
while ((item = virCPUx86DataNext(&iter))) {
virCPUx86CPUIDPtr cpuid;
--
2.22.0
2:45 a.m.
New subject: [libvirt] [PATCH v2 2/7] cpu_x86: Turn virCPUx86DataIteratorInit into a function
On Thu, Jun 20, 2019 at 12:53:37AM +0200, Jiri Denemark wrote:
Until now, this was a macro usable for direct initialization when a
variable is defined. Turning the macro into a function makes it more
general.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/cpu/cpu_x86.c | 34 ++++++++++++++++++++++++----------
1 file changed, 24 insertions(+), 10 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
5:53 p.m.
New subject: [libvirt] [PATCH v2 3/7] cpu_x86: Introduce virCPUx86FeatureIsMSR
This function may be used as a virCPUDefFeatureFilter callback for
virCPUDefCheckFeatures, virCPUDefFilerFeatures, and similar functions to
filter or pick out features reported via MSR.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/cpu/cpu_x86.c | 40 ++++++++++++++++++++++++++++++++++++++++
src/cpu/cpu_x86.h | 3 +++
src/libvirt_private.syms | 1 +
3 files changed, 44 insertions(+)
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 6eef5cef00..a7ec0f7095 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -3362,6 +3362,46 @@ virCPUx86DataAddFeature(virCPUDataPtr cpuData,
}
+/**
+ * virCPUx86FeatureIsMSR:
+ * @name CPU feature name
+ * @opaque NULL or a pointer to bool
+ *
+ * This is a callback for functions filtering features in virCPUDef.
+ *
+ * Checks whether a given CPU feature is reported via MSR. When @opaque is NULL
+ * or a pointer to true, the function will pick out (return true for) MSR
+ * features. If @opaque is a pointer to false, the logic will be inverted and
+ * the function will filter out (return false for) MSR features.
+ */
+bool
+virCPUx86FeatureIsMSR(const char *name,
+ void *opaque)
+{
+ virCPUx86FeaturePtr feature;
+ virCPUx86DataIterator iter;
+ virCPUx86DataItemPtr item;
+ virCPUx86MapPtr map;
+ bool inverted = false;
+
+ if (opaque)
+ inverted = !*(bool *)opaque;
+
+ if ((!(map = virCPUx86GetMap()) ||
+ !(feature = x86FeatureFind(map, name))) &&
+ !(feature = x86FeatureFindInternal(name)))
+ return inverted ? true : false;
+
+ virCPUx86DataIteratorInit(&iter, &feature->data);
+ while ((item = virCPUx86DataNext(&iter))) {
+ if (item->type == VIR_CPU_X86_DATA_MSR)
+ return inverted ? false : true;
+ }
+
+ return inverted ? true : false;
+}
+
+
struct cpuArchDriver cpuDriverX86 = {
.name = "x86",
.arch = archs,
diff --git a/src/cpu/cpu_x86.h b/src/cpu/cpu_x86.h
index 28ae46647a..8ae78d5e34 100644
--- a/src/cpu/cpu_x86.h
+++ b/src/cpu/cpu_x86.h
@@ -40,3 +40,6 @@ uint32_t virCPUx86DataGetSignature(virCPUDataPtr cpuData,
int virCPUx86DataSetVendor(virCPUDataPtr cpuData,
const char *vendor);
+
+bool virCPUx86FeatureIsMSR(const char *name,
+ void *opaque);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 90a6d10666..aa64580d63 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1268,6 +1268,7 @@ virCPUx86DataAdd;
virCPUx86DataGetSignature;
virCPUx86DataSetSignature;
virCPUx86DataSetVendor;
+virCPUx86FeatureIsMSR;
# datatypes.h
--
2.22.0
3:16 a.m.
New subject: [libvirt] [PATCH v2 3/7] cpu_x86: Introduce virCPUx86FeatureIsMSR
On Thu, Jun 20, 2019 at 12:53:38AM +0200, Jiri Denemark wrote:
This function may be used as a virCPUDefFeatureFilter callback for
virCPUDefCheckFeatures, virCPUDefFilerFeatures, and similar functions to
filter or pick out features reported via MSR.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/cpu/cpu_x86.c | 40 ++++++++++++++++++++++++++++++++++++++++
src/cpu/cpu_x86.h | 3 +++
src/libvirt_private.syms | 1 +
3 files changed, 44 insertions(+)
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 6eef5cef00..a7ec0f7095 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -3362,6 +3362,46 @@ virCPUx86DataAddFeature(virCPUDataPtr cpuData,
}
+/**
+ * virCPUx86FeatureIsMSR:
+ * @name CPU feature name
+ * @opaque NULL or a pointer to bool
+ *
+ * This is a callback for functions filtering features in virCPUDef.
+ *
+ * Checks whether a given CPU feature is reported via MSR. When @opaque is NULL
+ * or a pointer to true, the function will pick out (return true for) MSR
+ * features. If @opaque is a pointer to false, the logic will be inverted and
+ * the function will filter out (return false for) MSR features.
+ */
Uhm. What? Do I understand it correctly?
@opaque | *opaque | IsMSR | return
--------+---------+-------+--------
NULL | SEGV | true | true
0xBEEF | true | true | true
0xBEEF | false | true | false
--------+---------+-------+--------
First, it feels odd that 'false' is the value resulting in
changed behavior. I'd rather see it act differently when the bool
pointer is present and points to true.
Second, by using a pointer to bool AND not requiring opaque to be
passed, we essentially have a tri-state bool.
I'd suggest (ab)using the void pointer to store the value directly,
we do have the
bool var = opaque != NULL;
pattern elsewhere in the code.
+bool
+virCPUx86FeatureIsMSR(const char *name,
+ void *opaque)
Can you name this virCPUx86FeatureFilterMSR, leaving virCPUx86FeatureIsMSR
for a static helper function that will perform the actual lookup and
this function would (possibly) invert the result? All the ternary
operators make the function harder to follow.
+{
+ virCPUx86FeaturePtr feature;
+ virCPUx86DataIterator iter;
+ virCPUx86DataItemPtr item;
+ virCPUx86MapPtr map;
+ bool inverted = false;
+
+ if (opaque)
+ inverted = !*(bool *)opaque;
+
+ if ((!(map = virCPUx86GetMap()) ||
This condition would be more readable if it were split in two.
+ !(feature = x86FeatureFind(map, name))) &&
+ !(feature = x86FeatureFindInternal(name)))
+ return inverted ? true : false;
+
+ virCPUx86DataIteratorInit(&iter, &feature->data);
+ while ((item = virCPUx86DataNext(&iter))) {
+ if (item->type == VIR_CPU_X86_DATA_MSR)
+ return inverted ? false : true;
+ }
+
+ return inverted ? true : false;
+}
+
+
struct cpuArchDriver cpuDriverX86 = {
.name = "x86",
.arch = archs,
Jano
6:47 a.m.
New subject: [libvirt] [PATCH v2 3/7] cpu_x86: Introduce virCPUx86FeatureIsMSR
On Thu, Jun 20, 2019 at 10:16:36 +0200, Ján Tomko wrote:
On Thu, Jun 20, 2019 at 12:53:38AM +0200, Jiri Denemark wrote:
>This function may be used as a virCPUDefFeatureFilter callback for
>virCPUDefCheckFeatures, virCPUDefFilerFeatures, and similar functions to
>filter or pick out features reported via MSR.
>
>Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
>---
> src/cpu/cpu_x86.c | 40 ++++++++++++++++++++++++++++++++++++++++
> src/cpu/cpu_x86.h | 3 +++
> src/libvirt_private.syms | 1 +
> 3 files changed, 44 insertions(+)
>
>diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
>index 6eef5cef00..a7ec0f7095 100644
>--- a/src/cpu/cpu_x86.c
>+++ b/src/cpu/cpu_x86.c
>@@ -3362,6 +3362,46 @@ virCPUx86DataAddFeature(virCPUDataPtr cpuData,
> }
>
>
>+/**
>+ * virCPUx86FeatureIsMSR:
>+ * @name CPU feature name
>+ * @opaque NULL or a pointer to bool
>+ *
>+ * This is a callback for functions filtering features in virCPUDef.
>+ *
>+ * Checks whether a given CPU feature is reported via MSR. When @opaque is NULL
>+ * or a pointer to true, the function will pick out (return true for) MSR
>+ * features. If @opaque is a pointer to false, the logic will be inverted and
>+ * the function will filter out (return false for) MSR features.
>+ */
Uhm. What? Do I understand it correctly?
@opaque | *opaque | IsMSR | return
--------+---------+-------+--------
NULL | SEGV | true | true
0xBEEF | true | true | true
0xBEEF | false | true | false
--------+---------+-------+--------
First, it feels odd that 'false' is the value resulting in
changed behavior. I'd rather see it act differently when the bool
pointer is present and points to true.
I actually tried both variants and could not decide which one is better.
And this means both are in fact ugly :-) So when changing
virCPUx86FeatureIsMSR into a helper, I created two simple wrappers
instead of playing with opaque pointer.
Jirka
6:48 a.m.
New subject: [libvirt] [PATCH v3 3/7] cpu_x86: Introduce virCPUx86FeatureFilter*MSR
This functions may be used as a virCPUDefFeatureFilter callbacks for
virCPUDefCheckFeatures, virCPUDefFilerFeatures, and similar functions to
select (virCPUx86FeatureFilterSelectMSR) or drop
(virCPUx86FeatureFilterDropMSR) features reported via MSR.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 2:
- new patch
Version 3:
- former virCPUx86FeatureIsMSR function was split in two filters
and a common helper
src/cpu/cpu_x86.c | 57 ++++++++++++++++++++++++++++++++++++++++
src/cpu/cpu_x86.h | 6 +++++
src/libvirt_private.syms | 3 ++-
3 files changed, 65 insertions(+), 1 deletion(-)
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 6eef5cef00..4624ebaff5 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -3362,6 +3362,63 @@ virCPUx86DataAddFeature(virCPUDataPtr cpuData,
}
+static bool
+virCPUx86FeatureIsMSR(const char *name)
+{
+ virCPUx86FeaturePtr feature;
+ virCPUx86DataIterator iter;
+ virCPUx86DataItemPtr item;
+ virCPUx86MapPtr map;
+
+ if (!(map = virCPUx86GetMap()))
+ return false;
+
+ if (!(feature = x86FeatureFind(map, name)) &&
+ !(feature = x86FeatureFindInternal(name)))
+ return false;
+
+ virCPUx86DataIteratorInit(&iter, &feature->data);
+ while ((item = virCPUx86DataNext(&iter))) {
+ if (item->type == VIR_CPU_X86_DATA_MSR)
+ return true;
+ }
+
+ return false;
+}
+
+
+/**
+ * virCPUx86FeatureFilterSelectMSR:
+ *
+ * This is a callback for functions filtering features in virCPUDef. The result
+ * will contain only MSR features.
+ *
+ * Returns true if @name is an MSR feature, false otherwise.
+ */
+bool
+virCPUx86FeatureFilterSelectMSR(const char *name,
+ void *opaque ATTRIBUTE_UNUSED)
+{
+ return virCPUx86FeatureIsMSR(name);
+}
+
+
+/**
+ * virCPUx86FeatureFilterDropMSR:
+ *
+ * This is a callback for functions filtering features in virCPUDef. The result
+ * will not contain any MSR feature.
+ *
+ * Returns true if @name is not an MSR feature, false otherwise.
+ */
+bool
+virCPUx86FeatureFilterDropMSR(const char *name,
+ void *opaque ATTRIBUTE_UNUSED)
+{
+ return !virCPUx86FeatureIsMSR(name);
+}
+
+
struct cpuArchDriver cpuDriverX86 = {
.name = "x86",
.arch = archs,
diff --git a/src/cpu/cpu_x86.h b/src/cpu/cpu_x86.h
index 28ae46647a..3ea38d1701 100644
--- a/src/cpu/cpu_x86.h
+++ b/src/cpu/cpu_x86.h
@@ -40,3 +40,9 @@ uint32_t virCPUx86DataGetSignature(virCPUDataPtr cpuData,
int virCPUx86DataSetVendor(virCPUDataPtr cpuData,
const char *vendor);
+
+bool virCPUx86FeatureFilterSelectMSR(const char *name,
+ void *opaque);
+
+bool virCPUx86FeatureFilterDropMSR(const char *name,
+ void *opaque);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 90a6d10666..c06fa67d6a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1268,7 +1268,8 @@ virCPUx86DataAdd;
virCPUx86DataGetSignature;
virCPUx86DataSetSignature;
virCPUx86DataSetVendor;
-
+virCPUx86FeatureFilterDropMSR;
+virCPUx86FeatureFilterSelectMSR;
# datatypes.h
virConnectClass;
--
2.22.0
6:56 a.m.
New subject: [libvirt] [PATCH v3 3/7] cpu_x86: Introduce virCPUx86FeatureFilter*MSR
On Thu, Jun 20, 2019 at 01:48:30PM +0200, Jiri Denemark wrote:
This functions may be used as a virCPUDefFeatureFilter callbacks for
virCPUDefCheckFeatures, virCPUDefFilerFeatures, and similar functions to
select (virCPUx86FeatureFilterSelectMSR) or drop
(virCPUx86FeatureFilterDropMSR) features reported via MSR.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 2:
- new patch
Version 3:
- former virCPUx86FeatureIsMSR function was split in two filters
and a common helper
src/cpu/cpu_x86.c | 57 ++++++++++++++++++++++++++++++++++++++++
src/cpu/cpu_x86.h | 6 +++++
src/libvirt_private.syms | 3 ++-
3 files changed, 65 insertions(+), 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
5:53 p.m.
New subject: [libvirt] [PATCH v2 4/7] cpu_x86: Read CPU features from IA32_ARCH_CAPABILITIES MSR
This is used by the host capabilities code to construct host CPU
definition.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
---
src/cpu/cpu_x86.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index a7ec0f7095..5f051950de 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -2773,6 +2773,28 @@ virCPUx86GetHost(virCPUDefPtr cpu,
cpuidSet(CPUX86_EXTENDED, cpuData) < 0)
goto cleanup;
+ /* Read the IA32_ARCH_CAPABILITIES MSR (0x10a) if supported.
+ * This is best effort since there might be no way to read the MSR
+ * when we are not running as root. */
+ if (virCPUx86DataCheckFeature(cpuData, "arch-capabilities") == 1) {
+ uint64_t msr;
+ unsigned long index = 0x10a;
+
+ if (virHostCPUGetMSR(index, &msr) == 0) {
+ virCPUx86DataItem item = {
+ .type = VIR_CPU_X86_DATA_MSR,
+ .data.msr = {
+ .index = index,
+ .eax = msr & 0xffffffff,
+ .edx = msr >> 32,
+ },
+ };
+
+ if (virCPUx86DataAdd(cpuData, &item) < 0)
+ return -1;
+ }
+ }
+
ret = x86DecodeCPUData(cpu, cpuData, models);
cpu->microcodeVersion = virHostCPUGetMicrocodeVersion();
--
2.22.0
3:17 a.m.
New subject: [libvirt] [PATCH v2 4/7] cpu_x86: Read CPU features from IA32_ARCH_CAPABILITIES MSR
On Thu, Jun 20, 2019 at 12:53:39AM +0200, Jiri Denemark wrote:
This is used by the host capabilities code to construct host CPU
definition.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
---
src/cpu/cpu_x86.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
12:04 p.m.
New subject: [libvirt] [PATCH v2 4/7] cpu_x86: Read CPU features from IA32_ARCH_CAPABILITIES MSR
On 6/19/19 6:53 PM, Jiri Denemark wrote:
This is used by the host capabilities code to construct host CPU
definition.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
---
src/cpu/cpu_x86.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index a7ec0f7095..5f051950de 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -2773,6 +2773,28 @@ virCPUx86GetHost(virCPUDefPtr cpu,
cpuidSet(CPUX86_EXTENDED, cpuData) < 0)
goto cleanup;
+ /* Read the IA32_ARCH_CAPABILITIES MSR (0x10a) if supported.
+ * This is best effort since there might be no way to read the MSR
+ * when we are not running as root. */
+ if (virCPUx86DataCheckFeature(cpuData, "arch-capabilities") == 1) {
+ uint64_t msr;
+ unsigned long index = 0x10a;
+
+ if (virHostCPUGetMSR(index, &msr) == 0) {
+ virCPUx86DataItem item = {
+ .type = VIR_CPU_X86_DATA_MSR,
+ .data.msr = {
+ .index = index,
+ .eax = msr & 0xffffffff,
+ .edx = msr >> 32,
+ },
+ };
+
+ if (virCPUx86DataAdd(cpuData, &item) < 0)
+ return -1;
Coverity notes that @cpuData will be leaked in this case.
John
+ }
+ }
+
ret = x86DecodeCPUData(cpu, cpuData, models);
cpu->microcodeVersion = virHostCPUGetMicrocodeVersion();
5:53 p.m.
New subject: [libvirt] [PATCH v2 5/7] cpu_map: Introduce IA32_ARCH_CAPABILITIES MSR features
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
---
src/cpu_map/x86_features.xml | 20 +++++++++++++++++++
.../x86_64-cpuid-Core-i7-7600U-enabled.xml | 1 +
.../x86_64-cpuid-Core-i7-7600U-json.xml | 1 +
...86_64-cpuid-Xeon-Platinum-8268-enabled.xml | 1 +
.../x86_64-cpuid-Xeon-Platinum-8268-guest.xml | 4 ++++
.../x86_64-cpuid-Xeon-Platinum-8268-host.xml | 4 ++++
.../x86_64-cpuid-Xeon-Platinum-8268-json.xml | 3 +++
.../qemu_3.1.0.x86_64.xml | 1 +
.../qemu_4.0.0.x86_64.xml | 1 +
.../qemu_4.1.0.x86_64.xml | 1 +
10 files changed, 37 insertions(+)
diff --git a/src/cpu_map/x86_features.xml b/src/cpu_map/x86_features.xml
index 370807f88e..2bed1e0372 100644
--- a/src/cpu_map/x86_features.xml
+++ b/src/cpu_map/x86_features.xml
@@ -482,4 +482,24 @@
<feature name='amd-no-ssb'>
<cpuid eax_in='0x80000008' ebx='0x04000000'/>
</feature>
+
+ <!-- IA32_ARCH_CAPABILITIES features -->
+ <feature name='rdctl-no'>
+ <msr index='0x10a' edx='0x00000000' eax='0x00000001'/>
+ </feature>
+ <feature name='ibrs-all'>
+ <msr index='0x10a' edx='0x00000000' eax='0x00000002'/>
+ </feature>
+ <feature name='rsba'>
+ <msr index='0x10a' edx='0x00000000' eax='0x00000004'/>
+ </feature>
+ <feature name='skip-l1dfl-vmentry'>
+ <msr index='0x10a' edx='0x00000000' eax='0x00000008'/>
+ </feature>
+ <feature name='ssb-no'>
+ <msr index='0x10a' edx='0x00000000' eax='0x00000010'/>
+ </feature>
+ <feature name='mds-no'>
+ <msr index='0x10a' edx='0x00000000' eax='0x00000020'/>
+ </feature>
</cpus>
diff --git a/tests/cputestdata/x86_64-cpuid-Core-i7-7600U-enabled.xml
b/tests/cputestdata/x86_64-cpuid-Core-i7-7600U-enabled.xml
index b1cdaa802a..58bc84577c 100644
--- a/tests/cputestdata/x86_64-cpuid-Core-i7-7600U-enabled.xml
+++ b/tests/cputestdata/x86_64-cpuid-Core-i7-7600U-enabled.xml
@@ -5,4 +5,5 @@
<cpuid eax_in='0x00000007' ecx_in='0x00' eax='0x00000000'
ebx='0x009c4fbb' ecx='0x00000004' edx='0x84000000'/>
<cpuid eax_in='0x0000000d' ecx_in='0x01' eax='0x0000000f'
ebx='0x00000000' ecx='0x00000000' edx='0x00000000'/>
<cpuid eax_in='0x80000001' ecx_in='0x00' eax='0x00000000'
ebx='0x00000000' ecx='0x00000121' edx='0x2c100800'/>
+ <msr index='0x10a' edx='0x00000000' eax='0x00000008'/>
</cpudata>
diff --git a/tests/cputestdata/x86_64-cpuid-Core-i7-7600U-json.xml
b/tests/cputestdata/x86_64-cpuid-Core-i7-7600U-json.xml
index 48089c6003..690081493b 100644
--- a/tests/cputestdata/x86_64-cpuid-Core-i7-7600U-json.xml
+++ b/tests/cputestdata/x86_64-cpuid-Core-i7-7600U-json.xml
@@ -10,4 +10,5 @@
<feature policy='require' name='ssbd'/>
<feature policy='require' name='xsaves'/>
<feature policy='require' name='pdpe1gb'/>
+ <feature policy='require' name='skip-l1dfl-vmentry'/>
</cpu>
diff --git a/tests/cputestdata/x86_64-cpuid-Xeon-Platinum-8268-enabled.xml
b/tests/cputestdata/x86_64-cpuid-Xeon-Platinum-8268-enabled.xml
index 434ac1956a..313009b156 100644
--- a/tests/cputestdata/x86_64-cpuid-Xeon-Platinum-8268-enabled.xml
+++ b/tests/cputestdata/x86_64-cpuid-Xeon-Platinum-8268-enabled.xml
@@ -5,4 +5,5 @@
<cpuid eax_in='0x00000007' ecx_in='0x00' eax='0x00000000'
ebx='0xd19f4fbb' ecx='0x0000080c' edx='0x84000000'/>
<cpuid eax_in='0x0000000d' ecx_in='0x01' eax='0x0000000f'
ebx='0x00000000' ecx='0x00000000' edx='0x00000000'/>
<cpuid eax_in='0x80000001' ecx_in='0x00' eax='0x00000000'
ebx='0x00000000' ecx='0x00000121' edx='0x2c100800'/>
+ <msr index='0x10a' edx='0x00000000' eax='0x0000000b'/>
</cpudata>
diff --git a/tests/cputestdata/x86_64-cpuid-Xeon-Platinum-8268-guest.xml
b/tests/cputestdata/x86_64-cpuid-Xeon-Platinum-8268-guest.xml
index c7e8a1fccf..988fb1dbdc 100644
--- a/tests/cputestdata/x86_64-cpuid-Xeon-Platinum-8268-guest.xml
+++ b/tests/cputestdata/x86_64-cpuid-Xeon-Platinum-8268-guest.xml
@@ -30,4 +30,8 @@
<feature policy='require' name='mbm_total'/>
<feature policy='require' name='mbm_local'/>
<feature policy='require' name='invtsc'/>
+ <feature policy='require' name='rdctl-no'/>
+ <feature policy='require' name='ibrs-all'/>
+ <feature policy='require' name='skip-l1dfl-vmentry'/>
+ <feature policy='require' name='mds-no'/>
</cpu>
diff --git a/tests/cputestdata/x86_64-cpuid-Xeon-Platinum-8268-host.xml
b/tests/cputestdata/x86_64-cpuid-Xeon-Platinum-8268-host.xml
index d7482751b4..fdeafc4870 100644
--- a/tests/cputestdata/x86_64-cpuid-Xeon-Platinum-8268-host.xml
+++ b/tests/cputestdata/x86_64-cpuid-Xeon-Platinum-8268-host.xml
@@ -31,4 +31,8 @@
<feature name='mbm_total'/>
<feature name='mbm_local'/>
<feature name='invtsc'/>
+ <feature name='rdctl-no'/>
+ <feature name='ibrs-all'/>
+ <feature name='skip-l1dfl-vmentry'/>
+ <feature name='mds-no'/>
</cpu>
diff --git a/tests/cputestdata/x86_64-cpuid-Xeon-Platinum-8268-json.xml
b/tests/cputestdata/x86_64-cpuid-Xeon-Platinum-8268-json.xml
index b7d12dced7..78863c61d1 100644
--- a/tests/cputestdata/x86_64-cpuid-Xeon-Platinum-8268-json.xml
+++ b/tests/cputestdata/x86_64-cpuid-Xeon-Platinum-8268-json.xml
@@ -7,4 +7,7 @@
<feature policy='require' name='umip'/>
<feature policy='require' name='pku'/>
<feature policy='require' name='xsaves'/>
+ <feature policy='require' name='rdctl-no'/>
+ <feature policy='require' name='ibrs-all'/>
+ <feature policy='require' name='skip-l1dfl-vmentry'/>
</cpu>
diff --git a/tests/domaincapsschemadata/qemu_3.1.0.x86_64.xml
b/tests/domaincapsschemadata/qemu_3.1.0.x86_64.xml
index ca3baab88c..dfd186afba 100644
--- a/tests/domaincapsschemadata/qemu_3.1.0.x86_64.xml
+++ b/tests/domaincapsschemadata/qemu_3.1.0.x86_64.xml
@@ -42,6 +42,7 @@
<feature policy='require' name='xsaves'/>
<feature policy='require' name='pdpe1gb'/>
<feature policy='require' name='invtsc'/>
+ <feature policy='require' name='skip-l1dfl-vmentry'/>
</mode>
<mode name='custom' supported='yes'>
<model usable='yes'>qemu64</model>
diff --git a/tests/domaincapsschemadata/qemu_4.0.0.x86_64.xml
b/tests/domaincapsschemadata/qemu_4.0.0.x86_64.xml
index cba841d844..36f6f1e94d 100644
--- a/tests/domaincapsschemadata/qemu_4.0.0.x86_64.xml
+++ b/tests/domaincapsschemadata/qemu_4.0.0.x86_64.xml
@@ -42,6 +42,7 @@
<feature policy='require' name='xsaves'/>
<feature policy='require' name='pdpe1gb'/>
<feature policy='require' name='invtsc'/>
+ <feature policy='require' name='skip-l1dfl-vmentry'/>
</mode>
<mode name='custom' supported='yes'>
<model usable='yes'>qemu64</model>
diff --git a/tests/domaincapsschemadata/qemu_4.1.0.x86_64.xml
b/tests/domaincapsschemadata/qemu_4.1.0.x86_64.xml
index 389e641bbb..d3fc1ce9e7 100644
--- a/tests/domaincapsschemadata/qemu_4.1.0.x86_64.xml
+++ b/tests/domaincapsschemadata/qemu_4.1.0.x86_64.xml
@@ -41,6 +41,7 @@
<feature policy='require' name='arch-capabilities'/>
<feature policy='require' name='xsaves'/>
<feature policy='require' name='pdpe1gb'/>
+ <feature policy='require' name='skip-l1dfl-vmentry'/>
</mode>
<mode name='custom' supported='yes'>
<model usable='yes'>qemu64</model>
--
2.22.0
5:53 p.m.
New subject: [libvirt] [PATCH v2 6/7] qemu: Forbid MSR features with old QEMU
Without "unavailable-features" CPU property we cannot properly detect
whether a specific MSR feature we asked for (either explicitly or
implicitly via a CPU model) was disabled by QEMU for some reason.
Because this could break migration, snapshots, and save/restore
operaions, it's better to just forbid any use of MSR features with QEMU
which lacks "unavailable-features" CPU property.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_process.c | 29 ++++++++++++++++++++++++++---
1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index fa82adbc1e..45997dad49 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -59,6 +59,7 @@
#include "qemu_firmware.h"
#include "cpu/cpu.h"
+#include "cpu/cpu_x86.h"
#include "datatypes.h"
#include "virlog.h"
#include "virerror.h"
@@ -5407,9 +5408,31 @@ qemuProcessStartValidate(virQEMUDriverPtr driver,
if (qemuProcessStartValidateShmem(vm) < 0)
return -1;
- if (vm->def->cpu &&
- virCPUValidateFeatures(vm->def->os.arch, vm->def->cpu) < 0)
- return -1;
+ if (vm->def->cpu) {
+ if (virCPUValidateFeatures(vm->def->os.arch, vm->def->cpu) < 0)
+ return -1;
+
+ if (ARCH_IS_X86(vm->def->os.arch) &&
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_UNAVAILABLE_FEATURES)) {
+ VIR_AUTOSTRINGLIST features = NULL;
+ int n;
+
+ if ((n = virCPUDefCheckFeatures(vm->def->cpu,
+ virCPUx86FeatureIsMSR, NULL,
+ &features)) < 0)
+ return -1;
+
+ if (n > 0) {
+ VIR_AUTOFREE(char *) str = NULL;
+
+ str = virStringListJoin((const char **)features, ", ");
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Some features cannot be reliably used "
+ "with this QEMU: %s"), str);
+ return -1;
+ }
+ }
+ }
if (qemuProcessStartValidateDisks(vm, qemuCaps) < 0)
return -1;
--
2.22.0
3:23 a.m.
New subject: [libvirt] [PATCH v2 6/7] qemu: Forbid MSR features with old QEMU
On Thu, Jun 20, 2019 at 12:53:41AM +0200, Jiri Denemark wrote:
Without "unavailable-features" CPU property we cannot
properly detect
whether a specific MSR feature we asked for (either explicitly or
implicitly via a CPU model) was disabled by QEMU for some reason.
Because this could break migration, snapshots, and save/restore
operaions, it's better to just forbid any use of MSR features with QEMU
which lacks "unavailable-features" CPU property.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_process.c | 29 ++++++++++++++++++++++++++---
1 file changed, 26 insertions(+), 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
5:53 p.m.
New subject: [libvirt] [PATCH v2 7/7] qemu: Drop MSR features from host-model with old QEMU
With QEMU versions which lack "unavailable-features" we use CPUID based
detection of features which were enabled or disabled once QEMU starts.
Thus using MSR features with host-model would result in all of them
being marked as disabled in the active domain definition even though
QEMU did not actually disable them.
Let's make sure we add MSR features to host-model only when
"unavailable-features" property is supported by QEMU.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 16 ++++++++++++++++
tests/domaincapsschemadata/qemu_3.1.0.x86_64.xml | 1 -
tests/domaincapsschemadata/qemu_4.0.0.x86_64.xml | 1 -
3 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 4134f319ac..b7c20f3e3e 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3193,6 +3193,22 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
goto error;
}
+ if (ARCH_IS_X86(qemuCaps->arch) &&
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_UNAVAILABLE_FEATURES)) {
+ bool selecting = false;
+ if (cpu &&
+ virCPUDefFilterFeatures(cpu, virCPUx86FeatureIsMSR, &selecting) < 0)
+ goto error;
+
+ if (migCPU &&
+ virCPUDefFilterFeatures(migCPU, virCPUx86FeatureIsMSR, &selecting) <
0)
+ goto error;
+
+ if (fullCPU &&
+ virCPUDefFilterFeatures(fullCPU, virCPUx86FeatureIsMSR, &selecting) <
0)
+ goto error;
+ }
+
virQEMUCapsSetHostModel(qemuCaps, type, cpu, migCPU, fullCPU);
cleanup:
diff --git a/tests/domaincapsschemadata/qemu_3.1.0.x86_64.xml
b/tests/domaincapsschemadata/qemu_3.1.0.x86_64.xml
index dfd186afba..ca3baab88c 100644
--- a/tests/domaincapsschemadata/qemu_3.1.0.x86_64.xml
+++ b/tests/domaincapsschemadata/qemu_3.1.0.x86_64.xml
@@ -42,7 +42,6 @@
<feature policy='require' name='xsaves'/>
<feature policy='require' name='pdpe1gb'/>
<feature policy='require' name='invtsc'/>
- <feature policy='require' name='skip-l1dfl-vmentry'/>
</mode>
<mode name='custom' supported='yes'>
<model usable='yes'>qemu64</model>
diff --git a/tests/domaincapsschemadata/qemu_4.0.0.x86_64.xml
b/tests/domaincapsschemadata/qemu_4.0.0.x86_64.xml
index 36f6f1e94d..cba841d844 100644
--- a/tests/domaincapsschemadata/qemu_4.0.0.x86_64.xml
+++ b/tests/domaincapsschemadata/qemu_4.0.0.x86_64.xml
@@ -42,7 +42,6 @@
<feature policy='require' name='xsaves'/>
<feature policy='require' name='pdpe1gb'/>
<feature policy='require' name='invtsc'/>
- <feature policy='require' name='skip-l1dfl-vmentry'/>
</mode>
<mode name='custom' supported='yes'>
<model usable='yes'>qemu64</model>
--
2.22.0
3:27 a.m.
New subject: [libvirt] [PATCH v2 7/7] qemu: Drop MSR features from host-model with old QEMU
On Thu, Jun 20, 2019 at 12:53:42AM +0200, Jiri Denemark wrote:
With QEMU versions which lack "unavailable-features" we use
CPUID based
detection of features which were enabled or disabled once QEMU starts.
Thus using MSR features with host-model would result in all of them
being marked as disabled in the active domain definition even though
QEMU did not actually disable them.
Let's make sure we add MSR features to host-model only when
"unavailable-features" property is supported by QEMU.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 16 ++++++++++++++++
tests/domaincapsschemadata/qemu_3.1.0.x86_64.xml | 1 -
tests/domaincapsschemadata/qemu_4.0.0.x86_64.xml | 1 -
3 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 4134f319ac..b7c20f3e3e 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3193,6 +3193,22 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
goto error;
}
+ if (ARCH_IS_X86(qemuCaps->arch) &&
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_UNAVAILABLE_FEATURES)) {
+ bool selecting = false;
void *invert = (void *)(intptr_t)0x1;
or (if you don't like the pointer fun)
bool invert = true;
+ if (cpu &&
+ virCPUDefFilterFeatures(cpu, virCPUx86FeatureIsMSR, &selecting) < 0)
+ goto error;
+
+ if (migCPU &&
+ virCPUDefFilterFeatures(migCPU, virCPUx86FeatureIsMSR, &selecting) <
0)
+ goto error;
+
+ if (fullCPU &&
+ virCPUDefFilterFeatures(fullCPU, virCPUx86FeatureIsMSR, &selecting) <
0)
+ goto error;
+ }
+
virQEMUCapsSetHostModel(qemuCaps, type, cpu, migCPU, fullCPU);
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
1981
days inactive
1983
days old
17 comments
3 participants
participants (3)
-
Jiri Denemark -
John Ferlan -
Ján Tomko