* [Qemu-devel] [RFC PATCH 0/7] target/i386: support VMX features in "-cpu"
@ 2019-07-02 15:01 Paolo Bonzini
2019-07-02 15:01 ` [Qemu-devel] [PATCH 1/7] target/i386: handle filtered_features in a new function mark_unavailable_features Paolo Bonzini
` (8 more replies)
0 siblings, 9 replies; 29+ messages in thread
From: Paolo Bonzini @ 2019-07-02 15:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Liran Alon, Eduardo Habkost
This series adds support for VMX feature flags so that the user can
enable and disable at will the flags. In the final version I will
also add VMX features to named CPU models, which will complete VMX
live migration support. That's somewhat tedious and I didn't want
to do that before getting a general review.
There are a few complications, which are tackled across the series:
- KVM ioctls fail for some invalid MSR settings, namely when some
controls are reported as available but the corresponding CPUID
bits have been disabled. For backwards compatibility with
e.g. "-cpu host,-rdrand", these VMX features are silently
dropped (patch 2)
- some VMX MSRs have features in the high 32 bits (patch 3)
- some VMX MSRs have values in the high 32 bits, but only
actually have 32 features; this is handled in patch 6 by
mangling the result of KVM_GET_MSRS
- KVM has a couple bugs that can be worked around relatively
easily (patch 6 and 7)
Please review!
Paolo
Paolo Bonzini (7):
target/i386: handle filtered_features in a new function
mark_unavailable_features
target/i386: introduce generic feature dependency mechanism
target/i386: expand feature words to 64 bits
target/i386: add VMX definitions
vmxcap: correct the name of the variables
target/i386: add VMX features
target/i386: work around KVM_GET_MSRS bug for secondary execution
controls
include/sysemu/kvm.h | 2 +-
scripts/kvm/vmxcap | 14 +-
target/i386/cpu.c | 433 ++++++++++++++++++++++++++++++++++++++++-----------
target/i386/cpu.h | 138 +++++++++++++++-
target/i386/kvm.c | 173 +++++++++++++++++++-
5 files changed, 656 insertions(+), 104 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH 1/7] target/i386: handle filtered_features in a new function mark_unavailable_features
2019-07-02 15:01 [Qemu-devel] [RFC PATCH 0/7] target/i386: support VMX features in "-cpu" Paolo Bonzini
@ 2019-07-02 15:01 ` Paolo Bonzini
2019-07-05 20:37 ` Eduardo Habkost
2019-07-02 15:01 ` [Qemu-devel] [PATCH 2/7] target/i386: introduce generic feature dependency mechanism Paolo Bonzini
` (7 subsequent siblings)
8 siblings, 1 reply; 29+ messages in thread
From: Paolo Bonzini @ 2019-07-02 15:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Liran Alon, Eduardo Habkost
The next patch will add a different reason for filtering features, unrelated
to host feature support. Extract a new function that takes care of disabling
the features and reporting them.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/cpu.c | 76 ++++++++++++++++++++++++++++++-------------------------
1 file changed, 41 insertions(+), 35 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index da6eb67..9149d0d 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3236,17 +3236,39 @@ static char *feature_word_description(FeatureWordInfo *f, uint32_t bit)
return NULL;
}
-static void report_unavailable_features(FeatureWord w, uint32_t mask)
+static bool x86_cpu_have_filtered_features(X86CPU *cpu)
{
+ FeatureWord w;
+
+ for (w = 0; w < ARRAY_SIZE(feature_word_info); w++) {
+ if (cpu->filtered_features[w]) {
+ return true;
+ }
+ }
+
+ return false;
+}
+
+static void mark_unavailable_features(X86CPU *cpu, FeatureWord w, uint32_t mask,
+ const char *prefix)
+{
+ CPUX86State *env = &cpu->env;
FeatureWordInfo *f = &feature_word_info[w];
int i;
char *feat_word_str;
+ env->features[w] &= ~mask;
+ cpu->filtered_features[w] |= mask;
+
+ if (!cpu->check_cpuid && !cpu->enforce_cpuid) {
+ return;
+ }
+
for (i = 0; i < 32; ++i) {
if ((1UL << i) & mask) {
feat_word_str = feature_word_description(f, i);
- warn_report("%s doesn't support requested feature: %s%s%s [bit %d]",
- accel_uses_host_cpuid() ? "host" : "TCG",
+ warn_report("%s: %s%s%s [bit %d]",
+ prefix,
feat_word_str,
f->feat_names[i] ? "." : "",
f->feat_names[i] ? f->feat_names[i] : "", i);
@@ -3691,7 +3713,7 @@ static void x86_cpu_parse_featurestr(const char *typename, char *features,
}
static void x86_cpu_expand_features(X86CPU *cpu, Error **errp);
-static int x86_cpu_filter_features(X86CPU *cpu);
+static void x86_cpu_filter_features(X86CPU *cpu);
/* Build a list with the name of all features on a feature word array */
static void x86_cpu_list_feature_names(FeatureWordArray features,
@@ -3923,15 +3945,6 @@ static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w,
return r;
}
-static void x86_cpu_report_filtered_features(X86CPU *cpu)
-{
- FeatureWord w;
-
- for (w = 0; w < FEATURE_WORDS; w++) {
- report_unavailable_features(w, cpu->filtered_features[w]);
- }
-}
-
static void x86_cpu_apply_props(X86CPU *cpu, PropValue *props)
{
PropValue *pv;
@@ -5170,21 +5183,20 @@ out:
*
* Returns: 0 if all flags are supported by the host, non-zero otherwise.
*/
-static int x86_cpu_filter_features(X86CPU *cpu)
+static void x86_cpu_filter_features(X86CPU *cpu)
{
CPUX86State *env = &cpu->env;
FeatureWord w;
- int rv = 0;
+ const char *prefix = accel_uses_host_cpuid()
+ ? "host doesn't support requested feature"
+ : "TCG doesn't support requested feature";
for (w = 0; w < FEATURE_WORDS; w++) {
uint32_t host_feat =
x86_cpu_get_supported_feature_word(w, false);
uint32_t requested_features = env->features[w];
- env->features[w] &= host_feat;
- cpu->filtered_features[w] = requested_features & ~env->features[w];
- if (cpu->filtered_features[w]) {
- rv = 1;
- }
+ uint32_t unavailable_features = requested_features & ~host_feat;
+ mark_unavailable_features(cpu, w, unavailable_features, prefix);
}
if ((env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) &&
@@ -5210,13 +5222,9 @@ static int x86_cpu_filter_features(X86CPU *cpu)
* host can't emulate the capabilities we report on
* cpu_x86_cpuid(), intel-pt can't be enabled on the current host.
*/
- env->features[FEAT_7_0_EBX] &= ~CPUID_7_0_EBX_INTEL_PT;
- cpu->filtered_features[FEAT_7_0_EBX] |= CPUID_7_0_EBX_INTEL_PT;
- rv = 1;
+ mark_unavailable_features(cpu, FEAT_7_0_EBX, CPUID_7_0_EBX_INTEL_PT, prefix);
}
}
-
- return rv;
}
static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
@@ -5257,16 +5265,14 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
goto out;
}
- if (x86_cpu_filter_features(cpu) &&
- (cpu->check_cpuid || cpu->enforce_cpuid)) {
- x86_cpu_report_filtered_features(cpu);
- if (cpu->enforce_cpuid) {
- error_setg(&local_err,
- accel_uses_host_cpuid() ?
- "Host doesn't support requested features" :
- "TCG doesn't support requested features");
- goto out;
- }
+ x86_cpu_filter_features(cpu);
+
+ if (cpu->enforce_cpuid && x86_cpu_have_filtered_features(cpu)) {
+ error_setg(&local_err,
+ accel_uses_host_cpuid() ?
+ "Host doesn't support requested features" :
+ "TCG doesn't support requested features");
+ goto out;
}
/* On AMD CPUs, some CPUID[8000_0001].EDX bits must match the bits on
--
1.8.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH 2/7] target/i386: introduce generic feature dependency mechanism
2019-07-02 15:01 [Qemu-devel] [RFC PATCH 0/7] target/i386: support VMX features in "-cpu" Paolo Bonzini
2019-07-02 15:01 ` [Qemu-devel] [PATCH 1/7] target/i386: handle filtered_features in a new function mark_unavailable_features Paolo Bonzini
@ 2019-07-02 15:01 ` Paolo Bonzini
2019-07-05 20:52 ` Eduardo Habkost
2019-07-02 15:01 ` [Qemu-devel] [PATCH 3/7] target/i386: expand feature words to 64 bits Paolo Bonzini
` (6 subsequent siblings)
8 siblings, 1 reply; 29+ messages in thread
From: Paolo Bonzini @ 2019-07-02 15:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Liran Alon, Eduardo Habkost
Sometimes a CPU feature does not make sense unless another is
present. In the case of VMX features, KVM does not even allow
setting the VMX controls to some invalid combinations.
Therefore, this patch adds a generic mechanism that looks for bits
that the user explicitly cleared, and uses them to remove other bits
from the expanded CPU definition. If these dependent bits were also
explicitly *set* by the user, this will be a warning for "-cpu check"
and an error for "-cpu enforce". If not, then the dependent bits are
cleared silently, for convenience.
With VMX features, this will be used so that for example
"-cpu host,-rdrand" will also hide support for RDRAND exiting.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/cpu.c | 77 +++++++++++++++++++++++++++++++++++--------------------
1 file changed, 49 insertions(+), 28 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 9149d0d..412e834 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -799,10 +799,6 @@ typedef struct FeatureWordInfo {
/* If type==MSR_FEATURE_WORD */
struct {
uint32_t index;
- struct { /*CPUID that enumerate this MSR*/
- FeatureWord cpuid_class;
- uint32_t cpuid_flag;
- } cpuid_dep;
} msr;
};
uint32_t tcg_features; /* Feature flags supported by TCG */
@@ -1197,10 +1193,6 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
},
.msr = {
.index = MSR_IA32_ARCH_CAPABILITIES,
- .cpuid_dep = {
- FEAT_7_0_EDX,
- CPUID_7_0_EDX_ARCH_CAPABILITIES
- }
},
},
[FEAT_CORE_CAPABILITY] = {
@@ -1217,14 +1209,26 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
},
.msr = {
.index = MSR_IA32_CORE_CAPABILITY,
- .cpuid_dep = {
- FEAT_7_0_EDX,
- CPUID_7_0_EDX_CORE_CAPABILITY,
- },
},
},
};
+typedef struct FeatureDep {
+ uint16_t from, to;
+ uint64_t from_flag, to_flags;
+} FeatureDep;
+
+static FeatureDep feature_dependencies[] = {
+ {
+ .from = FEAT_7_0_EDX, .from_flag = CPUID_7_0_EDX_ARCH_CAPABILITIES,
+ .to = FEAT_ARCH_CAPABILITIES, .to_flags = ~0ull,
+ },
+ {
+ .from = FEAT_7_0_EDX, .from_flag = CPUID_7_0_EDX_CORE_CAPABILITY,
+ .to = FEAT_CORE_CAPABILITY, .to_flags = ~0ull,
+ },
+};
+
typedef struct X86RegisterInfo32 {
/* Name of register */
const char *name;
@@ -5086,9 +5090,42 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
{
CPUX86State *env = &cpu->env;
FeatureWord w;
+ int i;
GList *l;
Error *local_err = NULL;
+ for (l = plus_features; l; l = l->next) {
+ const char *prop = l->data;
+ object_property_set_bool(OBJECT(cpu), true, prop, &local_err);
+ if (local_err) {
+ goto out;
+ }
+ }
+
+ for (l = minus_features; l; l = l->next) {
+ const char *prop = l->data;
+ object_property_set_bool(OBJECT(cpu), false, prop, &local_err);
+ if (local_err) {
+ goto out;
+ }
+ }
+
+ for (i = 0; i < ARRAY_SIZE(feature_dependencies); i++) {
+ FeatureDep *d = &feature_dependencies[i];
+ if ((env->user_features[d->from] & d->from_flag) &&
+ !(env->features[d->from] & d->from_flag)) {
+ uint64_t unavailable_features = env->features[d->to] & d->to_flags;
+
+ /* Not an error unless the dependent feature was added explicitly. */
+ mark_unavailable_features(cpu, d->to, unavailable_features & env->user_features[d->to],
+ "This feature depends on other features that were not requested");
+
+ /* Prevent adding the feature in the loop below. */
+ env->user_features[d->to] |= d->to_flags;
+ env->features[d->to] &= ~d->to_flags;
+ }
+ }
+
/*TODO: Now cpu->max_features doesn't overwrite features
* set using QOM properties, and we can convert
* plus_features & minus_features to global properties
@@ -5106,22 +5143,6 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
}
}
- for (l = plus_features; l; l = l->next) {
- const char *prop = l->data;
- object_property_set_bool(OBJECT(cpu), true, prop, &local_err);
- if (local_err) {
- goto out;
- }
- }
-
- for (l = minus_features; l; l = l->next) {
- const char *prop = l->data;
- object_property_set_bool(OBJECT(cpu), false, prop, &local_err);
- if (local_err) {
- goto out;
- }
- }
-
if (!kvm_enabled() || !cpu->expose_kvm) {
env->features[FEAT_KVM] = 0;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH 3/7] target/i386: expand feature words to 64 bits
2019-07-02 15:01 [Qemu-devel] [RFC PATCH 0/7] target/i386: support VMX features in "-cpu" Paolo Bonzini
2019-07-02 15:01 ` [Qemu-devel] [PATCH 1/7] target/i386: handle filtered_features in a new function mark_unavailable_features Paolo Bonzini
2019-07-02 15:01 ` [Qemu-devel] [PATCH 2/7] target/i386: introduce generic feature dependency mechanism Paolo Bonzini
@ 2019-07-02 15:01 ` Paolo Bonzini
2019-07-02 15:01 ` [Qemu-devel] [PATCH 4/7] target/i386: add VMX definitions Paolo Bonzini
` (5 subsequent siblings)
8 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2019-07-02 15:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Liran Alon, Eduardo Habkost
VMX requires 64-bit feature words for the IA32_VMX_EPT_VPID_CAP
and IA32_VMX_BASIC MSRs. (The VMX control MSRs are 64-bit wide but
actually have only 32 bits of information).
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/sysemu/kvm.h | 2 +-
target/i386/cpu.c | 63 +++++++++++++++++++++++++++-------------------------
target/i386/cpu.h | 4 ++--
target/i386/kvm.c | 2 +-
4 files changed, 37 insertions(+), 34 deletions(-)
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index acd90ae..f8157bc 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -463,7 +463,7 @@ int kvm_vm_check_extension(KVMState *s, unsigned int extension);
uint32_t kvm_arch_get_supported_cpuid(KVMState *env, uint32_t function,
uint32_t index, int reg);
-uint32_t kvm_arch_get_supported_msr_feature(KVMState *s, uint32_t index);
+uint64_t kvm_arch_get_supported_msr_feature(KVMState *s, uint32_t index);
void kvm_set_sigmask_len(KVMState *s, unsigned int sigmask_len);
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 412e834..4de44e4 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -787,7 +787,7 @@ typedef struct FeatureWordInfo {
* In cases of disagreement between feature naming conventions,
* aliases may be added.
*/
- const char *feat_names[32];
+ const char *feat_names[64];
union {
/* If type==CPUID_FEATURE_WORD */
struct {
@@ -801,11 +801,11 @@ typedef struct FeatureWordInfo {
uint32_t index;
} msr;
};
- uint32_t tcg_features; /* Feature flags supported by TCG */
- uint32_t unmigratable_flags; /* Feature flags known to be unmigratable */
- uint32_t migratable_flags; /* Feature flags known to be migratable */
+ uint64_t tcg_features; /* Feature flags supported by TCG */
+ uint64_t unmigratable_flags; /* Feature flags known to be unmigratable */
+ uint64_t migratable_flags; /* Feature flags known to be migratable */
/* Features that shouldn't be auto-enabled by "-cpu host" */
- uint32_t no_autoenable_flags;
+ uint64_t no_autoenable_flags;
} FeatureWordInfo;
static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
@@ -1337,14 +1337,14 @@ const char *get_register_name_32(unsigned int reg)
* Returns the set of feature flags that are supported and migratable by
* QEMU, for a given FeatureWord.
*/
-static uint32_t x86_cpu_get_migratable_flags(FeatureWord w)
+static uint64_t x86_cpu_get_migratable_flags(FeatureWord w)
{
FeatureWordInfo *wi = &feature_word_info[w];
- uint32_t r = 0;
+ uint64_t r = 0;
int i;
- for (i = 0; i < 32; i++) {
- uint32_t f = 1U << i;
+ for (i = 0; i < 64; i++) {
+ uint64_t f = 1ULL << i;
/* If the feature name is known, it is implicitly considered migratable,
* unless it is explicitly set in unmigratable_flags */
@@ -3059,7 +3059,7 @@ void x86_cpu_change_kvm_default(const char *prop, const char *value)
assert(pv->prop);
}
-static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w,
+static uint64_t x86_cpu_get_supported_feature_word(FeatureWord w,
bool migratable_only);
static bool lmce_supported(void)
@@ -3253,7 +3253,7 @@ static bool x86_cpu_have_filtered_features(X86CPU *cpu)
return false;
}
-static void mark_unavailable_features(X86CPU *cpu, FeatureWord w, uint32_t mask,
+static void mark_unavailable_features(X86CPU *cpu, FeatureWord w, uint64_t mask,
const char *prefix)
{
CPUX86State *env = &cpu->env;
@@ -3268,8 +3268,8 @@ static void mark_unavailable_features(X86CPU *cpu, FeatureWord w, uint32_t mask,
return;
}
- for (i = 0; i < 32; ++i) {
- if ((1UL << i) & mask) {
+ for (i = 0; i < 64; ++i) {
+ if ((1ULL << i) & mask) {
feat_word_str = feature_word_description(f, i);
warn_report("%s: %s%s%s [bit %d]",
prefix,
@@ -3512,7 +3512,7 @@ static void x86_cpu_get_feature_words(Object *obj, Visitor *v,
const char *name, void *opaque,
Error **errp)
{
- uint32_t *array = (uint32_t *)opaque;
+ uint64_t *array = (uint64_t *)opaque;
FeatureWord w;
X86CPUFeatureWordInfo word_infos[FEATURE_WORDS] = { };
X86CPUFeatureWordInfoList list_entries[FEATURE_WORDS] = { };
@@ -3596,6 +3596,7 @@ static inline void feat2prop(char *s)
/* Return the feature property name for a feature flag bit */
static const char *x86_cpu_feature_name(FeatureWord w, int bitnr)
{
+ const char *name;
/* XSAVE components are automatically enabled by other features,
* so return the original feature name instead
*/
@@ -3609,9 +3610,11 @@ static const char *x86_cpu_feature_name(FeatureWord w, int bitnr)
}
}
- assert(bitnr < 32);
+ assert(bitnr < 64);
assert(w < FEATURE_WORDS);
- return feature_word_info[w].feat_names[bitnr];
+ name = feature_word_info[w].feat_names[bitnr];
+ assert(bitnr < 32 || !(name && feature_word_info[w].type == CPUID_FEATURE_WORD));
+ return name;
}
/* Compatibily hack to maintain legacy +-feat semantic,
@@ -3727,10 +3730,10 @@ static void x86_cpu_list_feature_names(FeatureWordArray features,
strList **next = feat_names;
for (w = 0; w < FEATURE_WORDS; w++) {
- uint32_t filtered = features[w];
+ uint64_t filtered = features[w];
int i;
- for (i = 0; i < 32; i++) {
- if (filtered & (1UL << i)) {
+ for (i = 0; i < 64; i++) {
+ if (filtered & (1ULL << i)) {
strList *new = g_new0(strList, 1);
new->value = g_strdup(x86_cpu_feature_name(w, i));
*next = new;
@@ -3866,7 +3869,7 @@ void x86_cpu_list(void)
names = NULL;
for (i = 0; i < ARRAY_SIZE(feature_word_info); i++) {
FeatureWordInfo *fw = &feature_word_info[i];
- for (j = 0; j < 32; j++) {
+ for (j = 0; j < 64; j++) {
if (fw->feat_names[j]) {
names = g_list_append(names, (gpointer)fw->feat_names[j]);
}
@@ -3913,11 +3916,11 @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
return cpu_list;
}
-static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w,
+static uint64_t x86_cpu_get_supported_feature_word(FeatureWord w,
bool migratable_only)
{
FeatureWordInfo *wi = &feature_word_info[w];
- uint32_t r = 0;
+ uint64_t r = 0;
if (kvm_enabled()) {
switch (wi->type) {
@@ -4057,7 +4060,7 @@ static QDict *x86_cpu_static_props(void)
for (w = 0; w < FEATURE_WORDS; w++) {
FeatureWordInfo *fi = &feature_word_info[w];
int bit;
- for (bit = 0; bit < 32; bit++) {
+ for (bit = 0; bit < 64; bit++) {
if (!fi->feat_names[bit]) {
continue;
}
@@ -5213,10 +5216,10 @@ static void x86_cpu_filter_features(X86CPU *cpu)
: "TCG doesn't support requested feature";
for (w = 0; w < FEATURE_WORDS; w++) {
- uint32_t host_feat =
+ uint64_t host_feat =
x86_cpu_get_supported_feature_word(w, false);
- uint32_t requested_features = env->features[w];
- uint32_t unavailable_features = requested_features & ~host_feat;
+ uint64_t requested_features = env->features[w];
+ uint64_t unavailable_features = requested_features & ~host_feat;
mark_unavailable_features(cpu, w, unavailable_features, prefix);
}
@@ -5512,7 +5515,7 @@ static void x86_cpu_unrealizefn(DeviceState *dev, Error **errp)
typedef struct BitProperty {
FeatureWord w;
- uint32_t mask;
+ uint64_t mask;
} BitProperty;
static void x86_cpu_get_bit_prop(Object *obj, Visitor *v, const char *name,
@@ -5520,7 +5523,7 @@ static void x86_cpu_get_bit_prop(Object *obj, Visitor *v, const char *name,
{
X86CPU *cpu = X86_CPU(obj);
BitProperty *fp = opaque;
- uint32_t f = cpu->env.features[fp->w];
+ uint64_t f = cpu->env.features[fp->w];
bool value = (f & fp->mask) == fp->mask;
visit_type_bool(v, name, &value, errp);
}
@@ -5573,7 +5576,7 @@ static void x86_cpu_register_bit_prop(X86CPU *cpu,
{
BitProperty *fp;
ObjectProperty *op;
- uint32_t mask = (1UL << bitnr);
+ uint64_t mask = (1ULL << bitnr);
op = object_property_find(OBJECT(cpu), prop_name, NULL);
if (op) {
@@ -5708,7 +5711,7 @@ static void x86_cpu_initfn(Object *obj)
for (w = 0; w < FEATURE_WORDS; w++) {
int bitnr;
- for (bitnr = 0; bitnr < 32; bitnr++) {
+ for (bitnr = 0; bitnr < 64; bitnr++) {
x86_cpu_register_feature_bit_props(cpu, w, bitnr);
}
}
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 9334579..4e5dc30 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -501,7 +501,7 @@ typedef enum FeatureWord {
FEATURE_WORDS,
} FeatureWord;
-typedef uint32_t FeatureWordArray[FEATURE_WORDS];
+typedef uint64_t FeatureWordArray[FEATURE_WORDS];
/* cpuid_features bits */
#define CPUID_FP87 (1U << 0)
@@ -1438,7 +1438,7 @@ struct X86CPU {
} mwait;
/* Features that were filtered out because of missing host capabilities */
- uint32_t filtered_features[FEATURE_WORDS];
+ FeatureWordArray filtered_features;
/* Enable PMU CPUID bits. This can't be enabled by default yet because
* it doesn't have ABI stability guarantees, as it passes all PMU CPUID
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index e4b4f57..6801696 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -432,7 +432,7 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t function,
return ret;
}
-uint32_t kvm_arch_get_supported_msr_feature(KVMState *s, uint32_t index)
+uint64_t kvm_arch_get_supported_msr_feature(KVMState *s, uint32_t index)
{
struct {
struct kvm_msrs info;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH 4/7] target/i386: add VMX definitions
2019-07-02 15:01 [Qemu-devel] [RFC PATCH 0/7] target/i386: support VMX features in "-cpu" Paolo Bonzini
` (2 preceding siblings ...)
2019-07-02 15:01 ` [Qemu-devel] [PATCH 3/7] target/i386: expand feature words to 64 bits Paolo Bonzini
@ 2019-07-02 15:01 ` Paolo Bonzini
2019-07-02 15:01 ` [Qemu-devel] [PATCH 5/7] vmxcap: correct the name of the variables Paolo Bonzini
` (4 subsequent siblings)
8 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2019-07-02 15:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Liran Alon, Eduardo Habkost
These will be used to compile the list of VMX features for named
CPU models, and/or by the code that sets up the VMX MSRs.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/cpu.h | 125 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 125 insertions(+)
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 4e5dc30..ec479d5 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -452,6 +452,25 @@ typedef enum X86Seg {
#define MSR_IA32_BNDCFGS 0x00000d90
#define MSR_IA32_XSS 0x00000da0
+#define MSR_IA32_VMX_BASIC 0x00000480
+#define MSR_IA32_VMX_PINBASED_CTLS 0x00000481
+#define MSR_IA32_VMX_PROCBASED_CTLS 0x00000482
+#define MSR_IA32_VMX_EXIT_CTLS 0x00000483
+#define MSR_IA32_VMX_ENTRY_CTLS 0x00000484
+#define MSR_IA32_VMX_MISC 0x00000485
+#define MSR_IA32_VMX_CR0_FIXED0 0x00000486
+#define MSR_IA32_VMX_CR0_FIXED1 0x00000487
+#define MSR_IA32_VMX_CR4_FIXED0 0x00000488
+#define MSR_IA32_VMX_CR4_FIXED1 0x00000489
+#define MSR_IA32_VMX_VMCS_ENUM 0x0000048a
+#define MSR_IA32_VMX_PROCBASED_CTLS2 0x0000048b
+#define MSR_IA32_VMX_EPT_VPID_CAP 0x0000048c
+#define MSR_IA32_VMX_TRUE_PINBASED_CTLS 0x0000048d
+#define MSR_IA32_VMX_TRUE_PROCBASED_CTLS 0x0000048e
+#define MSR_IA32_VMX_TRUE_EXIT_CTLS 0x0000048f
+#define MSR_IA32_VMX_TRUE_ENTRY_CTLS 0x00000490
+#define MSR_IA32_VMX_VMFUNC 0x00000491
+
#define XSTATE_FP_BIT 0
#define XSTATE_SSE_BIT 1
#define XSTATE_YMM_BIT 2
@@ -746,6 +765,112 @@ typedef uint64_t FeatureWordArray[FEATURE_WORDS];
#define MSR_CORE_CAP_SPLIT_LOCK_DETECT (1U << 5)
+/* VMX MSR features */
+#define MSR_VMX_BASIC_DUAL_MONITOR (1ULL << 49)
+#define MSR_VMX_BASIC_INS_OUTS (1ULL << 54)
+#define MSR_VMX_BASIC_TRUE_CTLS (1ULL << 55)
+
+#define MSR_VMX_MISC_STORE_LMA (1ULL << 5)
+#define MSR_VMX_MISC_ACTIVITY_HLT (1ULL << 6)
+#define MSR_VMX_MISC_ACTIVITY_SHUTDOWN (1ULL << 7)
+#define MSR_VMX_MISC_ACTIVITY_WAIT_SIPI (1ULL << 8)
+#define MSR_VMX_MISC_VMWRITE_VMEXIT (1ULL << 29)
+#define MSR_VMX_MISC_ZERO_LEN_INJECT (1ULL << 30)
+
+#define MSR_VMX_EPT_EXECONLY (1ULL << 0)
+#define MSR_VMX_EPT_PAGE_WALK_LENGTH_4 (1ULL << 6)
+#define MSR_VMX_EPT_PAGE_WALK_LENGTH_5 (1ULL << 7)
+#define MSR_VMX_EPT_UC (1ULL << 8)
+#define MSR_VMX_EPT_WB (1ULL << 14)
+#define MSR_VMX_EPT_2MB (1ULL << 16)
+#define MSR_VMX_EPT_1GB (1ULL << 17)
+#define MSR_VMX_EPT_INVEPT (1ULL << 20)
+#define MSR_VMX_EPT_AD_BITS (1ULL << 21)
+#define MSR_VMX_EPT_ADVANCED_VMEXIT_INFO (1ULL << 22)
+#define MSR_VMX_EPT_INVEPT_SINGLE_CONTEXT (1ULL << 25)
+#define MSR_VMX_EPT_INVEPT_ALL_CONTEXT (1ULL << 26)
+#define MSR_VMX_EPT_INVVPID (1ULL << 32)
+#define MSR_VMX_EPT_INVVPID_SINGLE_ADDR (1ULL << 40)
+#define MSR_VMX_EPT_INVVPID_SINGLE_CONTEXT (1ULL << 41)
+#define MSR_VMX_EPT_INVVPID_ALL_CONTEXT (1ULL << 42)
+#define MSR_VMX_EPT_INVVPID_SINGLE_CONTEXT_NOGLOBALS (1ULL << 43)
+
+#define MSR_VMX_VMFUNC_EPT_SWITCHING (1ULL << 0)
+
+
+/* VMX controls */
+#define VMX_CPU_BASED_VIRTUAL_INTR_PENDING 0x00000004
+#define VMX_CPU_BASED_USE_TSC_OFFSETING 0x00000008
+#define VMX_CPU_BASED_HLT_EXITING 0x00000080
+#define VMX_CPU_BASED_INVLPG_EXITING 0x00000200
+#define VMX_CPU_BASED_MWAIT_EXITING 0x00000400
+#define VMX_CPU_BASED_RDPMC_EXITING 0x00000800
+#define VMX_CPU_BASED_RDTSC_EXITING 0x00001000
+#define VMX_CPU_BASED_CR3_LOAD_EXITING 0x00008000
+#define VMX_CPU_BASED_CR3_STORE_EXITING 0x00010000
+#define VMX_CPU_BASED_CR8_LOAD_EXITING 0x00080000
+#define VMX_CPU_BASED_CR8_STORE_EXITING 0x00100000
+#define VMX_CPU_BASED_TPR_SHADOW 0x00200000
+#define VMX_CPU_BASED_VIRTUAL_NMI_PENDING 0x00400000
+#define VMX_CPU_BASED_MOV_DR_EXITING 0x00800000
+#define VMX_CPU_BASED_UNCOND_IO_EXITING 0x01000000
+#define VMX_CPU_BASED_USE_IO_BITMAPS 0x02000000
+#define VMX_CPU_BASED_MONITOR_TRAP_FLAG 0x08000000
+#define VMX_CPU_BASED_USE_MSR_BITMAPS 0x10000000
+#define VMX_CPU_BASED_MONITOR_EXITING 0x20000000
+#define VMX_CPU_BASED_PAUSE_EXITING 0x40000000
+#define VMX_CPU_BASED_ACTIVATE_SECONDARY_CONTROLS 0x80000000
+
+#define VMX_SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES 0x00000001
+#define VMX_SECONDARY_EXEC_ENABLE_EPT 0x00000002
+#define VMX_SECONDARY_EXEC_DESC 0x00000004
+#define VMX_SECONDARY_EXEC_RDTSCP 0x00000008
+#define VMX_SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE 0x00000010
+#define VMX_SECONDARY_EXEC_ENABLE_VPID 0x00000020
+#define VMX_SECONDARY_EXEC_WBINVD_EXITING 0x00000040
+#define VMX_SECONDARY_EXEC_UNRESTRICTED_GUEST 0x00000080
+#define VMX_SECONDARY_EXEC_APIC_REGISTER_VIRT 0x00000100
+#define VMX_SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY 0x00000200
+#define VMX_SECONDARY_EXEC_PAUSE_LOOP_EXITING 0x00000400
+#define VMX_SECONDARY_EXEC_RDRAND_EXITING 0x00000800
+#define VMX_SECONDARY_EXEC_ENABLE_INVPCID 0x00001000
+#define VMX_SECONDARY_EXEC_ENABLE_VMFUNC 0x00002000
+#define VMX_SECONDARY_EXEC_SHADOW_VMCS 0x00004000
+#define VMX_SECONDARY_EXEC_ENCLS_EXITING 0x00008000
+#define VMX_SECONDARY_EXEC_RDSEED_EXITING 0x00010000
+#define VMX_SECONDARY_EXEC_ENABLE_PML 0x00020000
+#define VMX_SECONDARY_EXEC_XSAVES 0x00100000
+
+#define VMX_PIN_BASED_EXT_INTR_MASK 0x00000001
+#define VMX_PIN_BASED_NMI_EXITING 0x00000008
+#define VMX_PIN_BASED_VIRTUAL_NMIS 0x00000020
+#define VMX_PIN_BASED_VMX_PREEMPTION_TIMER 0x00000040
+#define VMX_PIN_BASED_POSTED_INTR 0x00000080
+
+#define VMX_VM_EXIT_SAVE_DEBUG_CONTROLS 0x00000004
+#define VMX_VM_EXIT_HOST_ADDR_SPACE_SIZE 0x00000200
+#define VMX_VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL 0x00001000
+#define VMX_VM_EXIT_ACK_INTR_ON_EXIT 0x00008000
+#define VMX_VM_EXIT_SAVE_IA32_PAT 0x00040000
+#define VMX_VM_EXIT_LOAD_IA32_PAT 0x00080000
+#define VMX_VM_EXIT_SAVE_IA32_EFER 0x00100000
+#define VMX_VM_EXIT_LOAD_IA32_EFER 0x00200000
+#define VMX_VM_EXIT_SAVE_VMX_PREEMPTION_TIMER 0x00400000
+#define VMX_VM_EXIT_CLEAR_BNDCFGS 0x00800000
+#define VMX_VM_EXIT_PT_CONCEAL_PIP 0x01000000
+#define VMX_VM_EXIT_CLEAR_IA32_RTIT_CTL 0x02000000
+
+#define VMX_VM_ENTRY_LOAD_DEBUG_CONTROLS 0x00000004
+#define VMX_VM_ENTRY_IA32E_MODE 0x00000200
+#define VMX_VM_ENTRY_SMM 0x00000400
+#define VMX_VM_ENTRY_DEACT_DUAL_MONITOR 0x00000800
+#define VMX_VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL 0x00002000
+#define VMX_VM_ENTRY_LOAD_IA32_PAT 0x00004000
+#define VMX_VM_ENTRY_LOAD_IA32_EFER 0x00008000
+#define VMX_VM_ENTRY_LOAD_BNDCFGS 0x00010000
+#define VMX_VM_ENTRY_PT_CONCEAL_PIP 0x00020000
+#define VMX_VM_ENTRY_LOAD_IA32_RTIT_CTL 0x00040000
+
/* Supported Hyper-V Enlightenments */
#define HYPERV_FEAT_RELAXED 0
#define HYPERV_FEAT_VAPIC 1
--
1.8.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH 5/7] vmxcap: correct the name of the variables
2019-07-02 15:01 [Qemu-devel] [RFC PATCH 0/7] target/i386: support VMX features in "-cpu" Paolo Bonzini
` (3 preceding siblings ...)
2019-07-02 15:01 ` [Qemu-devel] [PATCH 4/7] target/i386: add VMX definitions Paolo Bonzini
@ 2019-07-02 15:01 ` Paolo Bonzini
2019-07-02 15:01 ` [Qemu-devel] [PATCH 6/7] target/i386: add VMX features Paolo Bonzini
` (3 subsequent siblings)
8 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2019-07-02 15:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Liran Alon, Eduardo Habkost
The low bits are 1 if the control must be one, the high bits
are 1 if the control can be one. Correct the variable names
as they are very confusing.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
scripts/kvm/vmxcap | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/scripts/kvm/vmxcap b/scripts/kvm/vmxcap
index 99a8146..2db6832 100755
--- a/scripts/kvm/vmxcap
+++ b/scripts/kvm/vmxcap
@@ -51,15 +51,15 @@ class Control(object):
return (val & 0xffffffff, val >> 32)
def show(self):
print(self.name)
- mbz, mb1 = self.read2(self.cap_msr)
- tmbz, tmb1 = 0, 0
+ mb1, cb1 = self.read2(self.cap_msr)
+ tmb1, tcb1 = 0, 0
if self.true_cap_msr:
- tmbz, tmb1 = self.read2(self.true_cap_msr)
+ tmb1, tcb1 = self.read2(self.true_cap_msr)
for bit in sorted(self.bits.keys()):
- zero = not (mbz & (1 << bit))
- one = mb1 & (1 << bit)
- true_zero = not (tmbz & (1 << bit))
- true_one = tmb1 & (1 << bit)
+ zero = not (mb1 & (1 << bit))
+ one = cb1 & (1 << bit)
+ true_zero = not (tmb1 & (1 << bit))
+ true_one = tcb1 & (1 << bit)
s= '?'
if (self.true_cap_msr and true_zero and true_one
and one and not zero):
--
1.8.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH 6/7] target/i386: add VMX features
2019-07-02 15:01 [Qemu-devel] [RFC PATCH 0/7] target/i386: support VMX features in "-cpu" Paolo Bonzini
` (4 preceding siblings ...)
2019-07-02 15:01 ` [Qemu-devel] [PATCH 5/7] vmxcap: correct the name of the variables Paolo Bonzini
@ 2019-07-02 15:01 ` Paolo Bonzini
2019-07-05 21:22 ` Eduardo Habkost
2019-07-02 15:01 ` [Qemu-devel] [PATCH 7/7] target/i386: work around KVM_GET_MSRS bug for secondary execution controls Paolo Bonzini
` (2 subsequent siblings)
8 siblings, 1 reply; 29+ messages in thread
From: Paolo Bonzini @ 2019-07-02 15:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Liran Alon, Eduardo Habkost
Add code to convert the VMX feature words back into MSR values,
allowing the user to enable/disable VMX features as they wish. The same
infrastructure enables support for limiting VMX features in named
CPU models.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/cpu.c | 221 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
target/i386/cpu.h | 9 +++
target/i386/kvm.c | 154 ++++++++++++++++++++++++++++++++++++-
3 files changed, 382 insertions(+), 2 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 4de44e4..12f76a3 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1211,6 +1211,163 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
.index = MSR_IA32_CORE_CAPABILITY,
},
},
+
+ [FEAT_VMX_PROCBASED_CTLS] = {
+ .type = MSR_FEATURE_WORD,
+ .feat_names = {
+ NULL, NULL, "vmx-vintr-pending", "vmx-tsc-offset",
+ NULL, NULL, NULL, "vmx-hlt-exit",
+ NULL, "vmx-invlpg-exit", "vmx-mwait-exit", "vmx-rdpmc-exit",
+ "vmx-rdtsc-exit", NULL, NULL, "vmx-cr3-load-noexit",
+ "vmx-cr3-store-noexit", NULL, NULL, "vmx-cr8-load-exit",
+ "vmx-cr8-store-exit", "vmx-flexpriority", "vmx-vnmi-pending", "vmx-movdr-exit",
+ "vmx-io-exit", "vmx-io-bitmap", NULL, "vmx-mtf",
+ "vmx-msr-bitmap", "vmx-monitor-exit", "vmx-pause-exit", "vmx-secondary-ctls",
+ },
+ .msr = {
+ .index = MSR_IA32_VMX_TRUE_PROCBASED_CTLS,
+ }
+ },
+
+ [FEAT_VMX_SECONDARY_CTLS] = {
+ .type = MSR_FEATURE_WORD,
+ .feat_names = {
+ "vmx-apicv-xapic", "vmx-ept", "vmx-desc-exit", "vmx-rdtscp-exit",
+ "vmx-apicv-x2apic", "vmx-vpid", "vmx-wbinvd-exit", "vmx-unrestricted-guest",
+ "vmx-apicv-register", "vmx-apicv-vid", "vmx-ple", "vmx-rdrand-exit",
+ "vmx-invpcid-exit", "vmx-vmfunc", "vmx-shadow-vmcs", "vmx-encls-exit",
+ "vmx-rdseed-exit", "vmx-pml", NULL, NULL,
+ "vmx-xsaves", NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ },
+ .msr = {
+ .index = MSR_IA32_VMX_PROCBASED_CTLS2,
+ }
+ },
+
+ [FEAT_VMX_PINBASED_CTLS] = {
+ .type = MSR_FEATURE_WORD,
+ .feat_names = {
+ "vmx-intr-exit", NULL, NULL, "vmx-nmi-exit",
+ NULL, "vmx-vnmi", "vmx-preemption-timer", "vmx-posted-intr",
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ },
+ .msr = {
+ .index = MSR_IA32_VMX_TRUE_PINBASED_CTLS,
+ }
+ },
+
+ [FEAT_VMX_EXIT_CTLS] = {
+ .type = MSR_FEATURE_WORD,
+ /*
+ * VMX_VM_EXIT_HOST_ADDR_SPACE_SIZE is copied from
+ * the LM CPUID bit.
+ */
+ .feat_names = {
+ NULL, NULL, "vmx-exit-nosave-debugctl", NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL /* vmx-exit-host-addr-space-size */, NULL, NULL,
+ "vmx-exit-load-perf-global-ctrl", NULL, NULL, "vmx-exit-ack-intr",
+ NULL, NULL, "vmx-exit-save-pat", "vmx-exit-load-pat",
+ "vmx-exit-save-efer", "vmx-exit-load-efer",
+ "vmx-exit-save-preemption-timer", "vmx-exit-clear-bndcfgs",
+ NULL, "vmx-exit-clear-rtit-ctl", NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ },
+ .msr = {
+ .index = MSR_IA32_VMX_TRUE_EXIT_CTLS,
+ }
+ },
+
+ [FEAT_VMX_ENTRY_CTLS] = {
+ .type = MSR_FEATURE_WORD,
+ .feat_names = {
+ NULL, NULL, "vmx-entry-noload-debugctl", NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, "vmx-entry-ia32e-mode", NULL, NULL,
+ NULL, "vmx-entry-load-perf-global-ctrl", "vmx-entry-load-pat", "vmx-entry-load-efer",
+ "vmx-entry-load-bndcfgs", NULL, "vmx-entry-load-rtit-ctl", NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ },
+ .msr = {
+ .index = MSR_IA32_VMX_TRUE_ENTRY_CTLS,
+ }
+ },
+
+ [FEAT_VMX_MISC] = {
+ .type = MSR_FEATURE_WORD,
+ .feat_names = {
+ NULL, NULL, NULL, NULL,
+ NULL, "vmx-store-lma", "vmx-activity-hlt", "vmx-activity-shutdown",
+ "vmx-activity-wait-sipi", NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, "vmx-vmwrite-vmexit-fields", "vmx-zero-len-inject", NULL,
+ },
+ .msr = {
+ .index = MSR_IA32_VMX_MISC,
+ }
+ },
+
+ [FEAT_VMX_EPT_VPID_CAPS] = {
+ .type = MSR_FEATURE_WORD,
+ .feat_names = {
+ "vmx-ept-execonly", NULL, NULL, NULL,
+ NULL, NULL, "vmx-page-walk-4", "vmx-page-walk-5",
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ "vmx-ept-2mb", "vmx-ept-1gb", NULL, NULL,
+ "vmx-invept", "vmx-eptad", "vmx-ept-advanced-exitinfo", NULL,
+ NULL, "vmx-invept-single-context", "vmx-invept-all-context", NULL,
+ NULL, NULL, NULL, NULL,
+ "vmx-invvpid", NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ "vmx-invvpid-single-addr", "vmx-invept-single-context",
+ "vmx-invvpid-all-context", "vmx-invept-single-context-noglobals",
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ },
+ .msr = {
+ .index = MSR_IA32_VMX_EPT_VPID_CAP,
+ }
+ },
+
+ [FEAT_VMX_BASIC] = {
+ .type = MSR_FEATURE_WORD,
+ .feat_names = {
+ [54] = "vmx-ins-outs",
+ [55] = "vmx-true-ctls",
+ },
+ .msr = {
+ .index = MSR_IA32_VMX_BASIC,
+ },
+ /* Just to be safe - we don't support setting the MSEG version field. */
+ .no_autoenable_flags = MSR_VMX_BASIC_DUAL_MONITOR,
+ },
+
+ [FEAT_VMX_VMFUNC] = {
+ .type = MSR_FEATURE_WORD,
+ .feat_names = {
+ [0] = "vmx-eptp-switching",
+ },
+ .msr = {
+ .index = MSR_IA32_VMX_VMFUNC,
+ }
+ },
+
};
typedef struct FeatureDep {
@@ -1227,6 +1384,70 @@ static FeatureDep feature_dependencies[] = {
.from = FEAT_7_0_EDX, .from_flag = CPUID_7_0_EDX_CORE_CAPABILITY,
.to = FEAT_CORE_CAPABILITY, .to_flags = ~0ull,
},
+ {
+ .from = FEAT_1_ECX, .from_flag = CPUID_EXT_VMX,
+ .to = FEAT_VMX_PROCBASED_CTLS, .to_flags = ~0ull,
+ },
+ {
+ .from = FEAT_1_ECX, .from_flag = CPUID_EXT_VMX,
+ .to = FEAT_VMX_PINBASED_CTLS, .to_flags = ~0ull,
+ },
+ {
+ .from = FEAT_1_ECX, .from_flag = CPUID_EXT_VMX,
+ .to = FEAT_VMX_EXIT_CTLS, .to_flags = ~0ull,
+ },
+ {
+ .from = FEAT_1_ECX, .from_flag = CPUID_EXT_VMX,
+ .to = FEAT_VMX_ENTRY_CTLS, .to_flags = ~0ull,
+ },
+ {
+ .from = FEAT_1_ECX, .from_flag = CPUID_EXT_VMX,
+ .to = FEAT_VMX_MISC, .to_flags = ~0ull,
+ },
+ {
+ .from = FEAT_1_ECX, .from_flag = CPUID_EXT_VMX,
+ .to = FEAT_VMX_BASIC, .to_flags = ~0ull,
+ },
+ {
+ .from = FEAT_8000_0001_EDX, .from_flag = CPUID_EXT2_LM,
+ .to = FEAT_VMX_ENTRY_CTLS, .to_flags = VMX_VM_ENTRY_IA32E_MODE,
+ },
+ {
+ .from = FEAT_VMX_PROCBASED_CTLS, .from_flag = VMX_CPU_BASED_ACTIVATE_SECONDARY_CONTROLS,
+ .to = FEAT_VMX_SECONDARY_CTLS, .to_flags = ~0ull,
+ },
+ {
+ .from = FEAT_XSAVE, .from_flag = CPUID_XSAVE_XSAVES,
+ .to = FEAT_VMX_SECONDARY_CTLS, .to_flags = VMX_SECONDARY_EXEC_XSAVES,
+ },
+ {
+ .from = FEAT_1_ECX, .from_flag = CPUID_EXT_RDRAND,
+ .to = FEAT_VMX_SECONDARY_CTLS, .to_flags = VMX_SECONDARY_EXEC_RDRAND_EXITING,
+ },
+ {
+ .from = FEAT_7_0_EBX, .from_flag = CPUID_7_0_EBX_INVPCID,
+ .to = FEAT_VMX_SECONDARY_CTLS, .to_flags = VMX_SECONDARY_EXEC_ENABLE_INVPCID,
+ },
+ {
+ .from = FEAT_7_0_EBX, .from_flag = CPUID_7_0_EBX_RDSEED,
+ .to = FEAT_VMX_SECONDARY_CTLS, .to_flags = VMX_SECONDARY_EXEC_RDSEED_EXITING,
+ },
+ {
+ .from = FEAT_8000_0001_EDX, .from_flag = CPUID_EXT2_RDTSCP,
+ .to = FEAT_VMX_SECONDARY_CTLS, .to_flags = VMX_SECONDARY_EXEC_RDTSCP,
+ },
+ {
+ .from = FEAT_VMX_SECONDARY_CTLS, .from_flag = VMX_SECONDARY_EXEC_ENABLE_EPT,
+ .to = FEAT_VMX_EPT_VPID_CAPS, .to_flags = 0xffffffffull,
+ },
+ {
+ .from = FEAT_VMX_SECONDARY_CTLS, .from_flag = VMX_SECONDARY_EXEC_ENABLE_VPID,
+ .to = FEAT_VMX_EPT_VPID_CAPS, .to_flags = 0xffffffffull << 32,
+ },
+ {
+ .from = FEAT_VMX_SECONDARY_CTLS, .from_flag = VMX_SECONDARY_EXEC_ENABLE_VMFUNC,
+ .to = FEAT_VMX_VMFUNC, .to_flags = ~0ull,
+ },
};
typedef struct X86RegisterInfo32 {
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index ec479d5..a5710c1 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -517,6 +517,15 @@ typedef enum FeatureWord {
FEAT_XSAVE_COMP_HI, /* CPUID[EAX=0xd,ECX=0].EDX */
FEAT_ARCH_CAPABILITIES,
FEAT_CORE_CAPABILITY,
+ FEAT_VMX_PROCBASED_CTLS,
+ FEAT_VMX_SECONDARY_CTLS,
+ FEAT_VMX_PINBASED_CTLS,
+ FEAT_VMX_EXIT_CTLS,
+ FEAT_VMX_ENTRY_CTLS,
+ FEAT_VMX_MISC,
+ FEAT_VMX_EPT_VPID_CAPS,
+ FEAT_VMX_BASIC,
+ FEAT_VMX_VMFUNC,
FEATURE_WORDS,
} FeatureWord;
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 6801696..e35489c 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -96,6 +96,7 @@ static bool has_msr_virt_ssbd;
static bool has_msr_smi_count;
static bool has_msr_arch_capabs;
static bool has_msr_core_capabs;
+static bool has_msr_vmx_vmfunc;
static uint32_t has_architectural_pmu_version;
static uint32_t num_architectural_pmu_gp_counters;
@@ -438,7 +439,8 @@ uint64_t kvm_arch_get_supported_msr_feature(KVMState *s, uint32_t index)
struct kvm_msrs info;
struct kvm_msr_entry entries[1];
} msr_data;
- uint32_t ret;
+ uint64_t value;
+ uint32_t ret, can_be_one, must_be_one;
if (kvm_feature_msrs == NULL) { /* Host doesn't support feature MSRs */
return 0;
@@ -464,7 +466,25 @@ uint64_t kvm_arch_get_supported_msr_feature(KVMState *s, uint32_t index)
exit(1);
}
- return msr_data.entries[0].data;
+ value = msr_data.entries[0].data;
+ switch (index) {
+ case MSR_IA32_VMX_PROCBASED_CTLS2:
+ case MSR_IA32_VMX_TRUE_PINBASED_CTLS:
+ case MSR_IA32_VMX_TRUE_PROCBASED_CTLS:
+ case MSR_IA32_VMX_TRUE_ENTRY_CTLS:
+ case MSR_IA32_VMX_TRUE_EXIT_CTLS:
+ /*
+ * Return true for bits that can be one, but do not have to be one.
+ * The SDM tells us which bits could have a "must be one" setting,
+ * so we can do the opposite transformation in make_vmx_msr_value.
+ */
+ must_be_one = (uint32_t)value;
+ can_be_one = (uint32_t)(value >> 32);
+ return can_be_one & ~must_be_one;
+
+ default:
+ return value;
+ }
}
@@ -1933,6 +1953,9 @@ static int kvm_get_supported_msrs(KVMState *s)
case MSR_IA32_CORE_CAPABILITY:
has_msr_core_capabs = true;
break;
+ case MSR_IA32_VMX_VMFUNC:
+ has_msr_vmx_vmfunc = true;
+ break;
}
}
}
@@ -2407,6 +2430,126 @@ static int kvm_put_msr_feature_control(X86CPU *cpu)
return 0;
}
+static uint64_t make_vmx_msr_value(uint32_t index, uint32_t features)
+{
+ uint32_t default1, can_be_one, can_be_zero;
+ uint32_t must_be_one;
+
+ switch (index) {
+ case MSR_IA32_VMX_TRUE_PINBASED_CTLS:
+ default1 = 0x00000016;
+ break;
+ case MSR_IA32_VMX_TRUE_PROCBASED_CTLS:
+ default1 = 0x0401e172;
+ break;
+ case MSR_IA32_VMX_TRUE_ENTRY_CTLS:
+ default1 = 0x000011ff;
+ break;
+ case MSR_IA32_VMX_TRUE_EXIT_CTLS:
+ default1 = 0x00036dff;
+ break;
+ case MSR_IA32_VMX_PROCBASED_CTLS2:
+ default1 = 0;
+ break;
+ default:
+ abort();
+ }
+
+ /* If a feature bit is set, the control can be either set or clear.
+ * Otherwise the value is limited to either 0 or 1 by default1.
+ */
+ can_be_one = features | default1;
+ can_be_zero = features | ~default1;
+ must_be_one = ~can_be_zero;
+
+ /*
+ * Bit 0:31 -> 0 if the control bit can be zero (i.e. 1 if it must be one).
+ * Bit 32:63 -> 1 if the control bit can be one.
+ */
+ return must_be_one | (((uint64_t)can_be_one) << 32);
+}
+
+#define VMCS12_MAX_FIELD_INDEX (0x17)
+
+static void kvm_msr_entry_add_vmx(X86CPU *cpu, FeatureWordArray f)
+{
+ uint64_t kvm_vmx_basic =
+ kvm_arch_get_supported_msr_feature(kvm_state,
+ MSR_IA32_VMX_BASIC);
+ uint64_t kvm_vmx_misc =
+ kvm_arch_get_supported_msr_feature(kvm_state,
+ MSR_IA32_VMX_MISC);
+ uint64_t kvm_vmx_ept_vpid =
+ kvm_arch_get_supported_msr_feature(kvm_state,
+ MSR_IA32_VMX_EPT_VPID_CAP);
+
+ /*
+ * If the guest is 64-bit, a value of 1 is allowed for the host address
+ * space size vmexit control.
+ */
+ uint64_t fixed_vmx_exit = f[FEAT_8000_0001_EDX] & CPUID_EXT2_LM
+ ? (uint64_t)VMX_VM_EXIT_HOST_ADDR_SPACE_SIZE << 32 : 0;
+
+ /*
+ * Bits 0-30, 32-44 and 50-53 come from the host. KVM should
+ * not change them for backwards compatibility.
+ */
+ uint64_t fixed_vmx_basic = kvm_vmx_basic & 0x003c1fff7fffffffULL;
+
+ /*
+ * Same for bits 0-4 and 25-27. Bits 16-24 (CR3 target count) can
+ * change in the future but are always zero for now, clear them to be
+ * future proof. Bits 32-63 in theory could change, though KVM does
+ * not support dual-monitor treatment and probably never will; mask
+ * them out as well.
+ */
+ uint64_t fixed_vmx_misc = kvm_vmx_misc & 0x0e00001f;
+
+ /*
+ * EPT memory types should not change either, so we do not bother
+ * adding features for them.
+ */
+ uint64_t fixed_vmx_ept_mask =
+ (f[FEAT_VMX_SECONDARY_CTLS] & VMX_SECONDARY_EXEC_ENABLE_EPT ? 0x4100ull : 0);
+ uint64_t fixed_vmx_ept_vpid = kvm_vmx_ept_vpid & fixed_vmx_ept_mask;
+
+ kvm_msr_entry_add(cpu, MSR_IA32_VMX_TRUE_PROCBASED_CTLS,
+ make_vmx_msr_value(MSR_IA32_VMX_TRUE_PROCBASED_CTLS,
+ f[FEAT_VMX_PROCBASED_CTLS]));
+ kvm_msr_entry_add(cpu, MSR_IA32_VMX_TRUE_PINBASED_CTLS,
+ make_vmx_msr_value(MSR_IA32_VMX_TRUE_PINBASED_CTLS,
+ f[FEAT_VMX_PINBASED_CTLS]));
+ kvm_msr_entry_add(cpu, MSR_IA32_VMX_TRUE_EXIT_CTLS,
+ make_vmx_msr_value(MSR_IA32_VMX_TRUE_EXIT_CTLS,
+ f[FEAT_VMX_EXIT_CTLS]) | fixed_vmx_exit);
+ kvm_msr_entry_add(cpu, MSR_IA32_VMX_TRUE_ENTRY_CTLS,
+ make_vmx_msr_value(MSR_IA32_VMX_TRUE_ENTRY_CTLS,
+ f[FEAT_VMX_ENTRY_CTLS]));
+ kvm_msr_entry_add(cpu, MSR_IA32_VMX_PROCBASED_CTLS2,
+ make_vmx_msr_value(MSR_IA32_VMX_PROCBASED_CTLS2,
+ f[FEAT_VMX_SECONDARY_CTLS]));
+ kvm_msr_entry_add(cpu, MSR_IA32_VMX_EPT_VPID_CAP,
+ f[FEAT_VMX_EPT_VPID_CAPS] | fixed_vmx_ept_vpid);
+ kvm_msr_entry_add(cpu, MSR_IA32_VMX_BASIC,
+ f[FEAT_VMX_BASIC] | fixed_vmx_basic);
+ kvm_msr_entry_add(cpu, MSR_IA32_VMX_MISC,
+ f[FEAT_VMX_MISC] | fixed_vmx_misc);
+ if (has_msr_vmx_vmfunc) {
+ kvm_msr_entry_add(cpu, MSR_IA32_VMX_VMFUNC, f[FEAT_VMX_VMFUNC]);
+ }
+
+ /*
+ * Just to be safe, write these with constant values. The CRn_FIXED1
+ * MSRs are generated by KVM based on the vCPU's CPUID.
+ */
+ kvm_msr_entry_add(cpu, MSR_IA32_VMX_CR0_FIXED0,
+ CR0_PE_MASK | CR0_PG_MASK | CR0_NE_MASK);
+ kvm_msr_entry_add(cpu, MSR_IA32_VMX_CR4_FIXED0,
+ CR4_VMXE_MASK);
+ kvm_msr_entry_add(cpu, MSR_IA32_VMX_VMCS_ENUM,
+ VMCS12_MAX_FIELD_INDEX << 1);
+}
+
static int kvm_put_msrs(X86CPU *cpu, int level)
{
CPUX86State *env = &cpu->env;
@@ -2659,6 +2802,13 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
kvm_msr_entry_add(cpu, MSR_MC0_CTL + i, env->mce_banks[i]);
}
}
+ /*
+ * Older kernels do not include VMX MSRs in KVM_GET_MSR_INDEX_LIST, but
+ * all kernels with MSR features should have them.
+ */
+ if (kvm_feature_msrs && cpu_has_vmx(env)) {
+ kvm_msr_entry_add_vmx(cpu, env->features);
+ }
ret = kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MSRS, cpu->kvm_msr_buf);
if (ret < 0) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH 7/7] target/i386: work around KVM_GET_MSRS bug for secondary execution controls
2019-07-02 15:01 [Qemu-devel] [RFC PATCH 0/7] target/i386: support VMX features in "-cpu" Paolo Bonzini
` (5 preceding siblings ...)
2019-07-02 15:01 ` [Qemu-devel] [PATCH 6/7] target/i386: add VMX features Paolo Bonzini
@ 2019-07-02 15:01 ` Paolo Bonzini
2019-07-02 20:46 ` [Qemu-devel] [RFC PATCH 0/7] target/i386: support VMX features in "-cpu" no-reply
2019-07-02 21:13 ` no-reply
8 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2019-07-02 15:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Liran Alon, Eduardo Habkost
Some secondary controls are automatically enabled/disabled based on the CPUID
values that are set for the guest. However, they are still available at a
global level and therefore should be present when KVM_GET_MSRS is sent to
/dev/kvm.
Unfortunately KVM forgot to include those, so fix that.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/kvm.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index e35489c..84b42a5 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -469,6 +469,23 @@ uint64_t kvm_arch_get_supported_msr_feature(KVMState *s, uint32_t index)
value = msr_data.entries[0].data;
switch (index) {
case MSR_IA32_VMX_PROCBASED_CTLS2:
+ /* KVM forgot to add these bits for some time, do this ourselves. */
+ if (kvm_arch_get_supported_cpuid(s, 0xD, 1, R_ECX) & CPUID_XSAVE_XSAVES) {
+ value |= (uint64_t)VMX_SECONDARY_EXEC_XSAVES << 32;
+ }
+ if (kvm_arch_get_supported_cpuid(s, 1, 0, R_ECX) & CPUID_EXT_RDRAND) {
+ value |= (uint64_t)VMX_SECONDARY_EXEC_RDRAND_EXITING << 32;
+ }
+ if (kvm_arch_get_supported_cpuid(s, 7, 0, R_EBX) & CPUID_7_0_EBX_INVPCID) {
+ value |= (uint64_t)VMX_SECONDARY_EXEC_ENABLE_INVPCID << 32;
+ }
+ if (kvm_arch_get_supported_cpuid(s, 7, 0, R_EBX) & CPUID_7_0_EBX_RDSEED) {
+ value |= (uint64_t)VMX_SECONDARY_EXEC_RDSEED_EXITING << 32;
+ }
+ if (kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_EDX) & CPUID_EXT2_RDTSCP) {
+ value |= (uint64_t)VMX_SECONDARY_EXEC_RDTSCP << 32;
+ }
+ /* fall through */
case MSR_IA32_VMX_TRUE_PINBASED_CTLS:
case MSR_IA32_VMX_TRUE_PROCBASED_CTLS:
case MSR_IA32_VMX_TRUE_ENTRY_CTLS:
--
1.8.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 0/7] target/i386: support VMX features in "-cpu"
2019-07-02 15:01 [Qemu-devel] [RFC PATCH 0/7] target/i386: support VMX features in "-cpu" Paolo Bonzini
` (6 preceding siblings ...)
2019-07-02 15:01 ` [Qemu-devel] [PATCH 7/7] target/i386: work around KVM_GET_MSRS bug for secondary execution controls Paolo Bonzini
@ 2019-07-02 20:46 ` no-reply
2019-07-02 21:13 ` no-reply
8 siblings, 0 replies; 29+ messages in thread
From: no-reply @ 2019-07-02 20:46 UTC (permalink / raw)
To: pbonzini; +Cc: liran.alon, qemu-devel, ehabkost
Patchew URL: https://patchew.org/QEMU/1562079681-19204-1-git-send-email-pbonzini@redhat.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Subject: [Qemu-devel] [RFC PATCH 0/7] target/i386: support VMX features in "-cpu"
Message-id: 1562079681-19204-1-git-send-email-pbonzini@redhat.com
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Switched to a new branch 'test'
d88c7eb target/i386: work around KVM_GET_MSRS bug for secondary execution controls
c5362c7 target/i386: add VMX features
8b2be4b vmxcap: correct the name of the variables
b85f895 target/i386: add VMX definitions
3962d04 target/i386: expand feature words to 64 bits
cbeb933 target/i386: introduce generic feature dependency mechanism
9fd6230 target/i386: handle filtered_features in a new function mark_unavailable_features
=== OUTPUT BEGIN ===
1/7 Checking commit 9fd62303df59 (target/i386: handle filtered_features in a new function mark_unavailable_features)
ERROR: suspect code indent for conditional statements (4, 9)
#27: FILE: target/i386/cpu.c:3243:
+ for (w = 0; w < ARRAY_SIZE(feature_word_info); w++) {
+ if (cpu->filtered_features[w]) {
ERROR: suspect code indent for conditional statements (9, 13)
#28: FILE: target/i386/cpu.c:3244:
+ if (cpu->filtered_features[w]) {
+ return true;
WARNING: line over 80 characters
#121: FILE: target/i386/cpu.c:5225:
+ mark_unavailable_features(cpu, FEAT_7_0_EBX, CPUID_7_0_EBX_INTEL_PT, prefix);
total: 2 errors, 1 warnings, 130 lines checked
Patch 1/7 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/7 Checking commit cbeb933580d4 (target/i386: introduce generic feature dependency mechanism)
WARNING: line over 80 characters
#69: FILE: target/i386/cpu.c:1223:
+ .from = FEAT_7_0_EDX, .from_flag = CPUID_7_0_EDX_ARCH_CAPABILITIES,
WARNING: line over 80 characters
#73: FILE: target/i386/cpu.c:1227:
+ .from = FEAT_7_0_EDX, .from_flag = CPUID_7_0_EDX_CORE_CAPABILITY,
WARNING: line over 80 characters
#111: FILE: target/i386/cpu.c:5119:
+ /* Not an error unless the dependent feature was added explicitly. */
ERROR: line over 90 characters
#112: FILE: target/i386/cpu.c:5120:
+ mark_unavailable_features(cpu, d->to, unavailable_features & env->user_features[d->to],
total: 1 errors, 3 warnings, 114 lines checked
Patch 2/7 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/7 Checking commit 3962d049bcf1 (target/i386: expand feature words to 64 bits)
WARNING: line over 80 characters
#130: FILE: target/i386/cpu.c:3616:
+ assert(bitnr < 32 || !(name && feature_word_info[w].type == CPUID_FEATURE_WORD));
total: 0 errors, 1 warnings, 214 lines checked
Patch 3/7 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/7 Checking commit b85f8959ed62 (target/i386: add VMX definitions)
5/7 Checking commit 8b2be4b559cb (vmxcap: correct the name of the variables)
6/7 Checking commit c5362c7576b9 (target/i386: add VMX features)
ERROR: line over 90 characters
#32: FILE: target/i386/cpu.c:1223:
+ "vmx-cr8-store-exit", "vmx-flexpriority", "vmx-vnmi-pending", "vmx-movdr-exit",
WARNING: line over 80 characters
#34: FILE: target/i386/cpu.c:1225:
+ "vmx-msr-bitmap", "vmx-monitor-exit", "vmx-pause-exit", "vmx-secondary-ctls",
WARNING: line over 80 characters
#45: FILE: target/i386/cpu.c:1236:
+ "vmx-apicv-x2apic", "vmx-vpid", "vmx-wbinvd-exit", "vmx-unrestricted-guest",
WARNING: line over 80 characters
#47: FILE: target/i386/cpu.c:1238:
+ "vmx-invpcid-exit", "vmx-vmfunc", "vmx-shadow-vmcs", "vmx-encls-exit",
WARNING: Block comments use a leading /* on a separate line
#84: FILE: target/i386/cpu.c:1275:
+ NULL, NULL /* vmx-exit-host-addr-space-size */, NULL, NULL,
ERROR: line over 90 characters
#103: FILE: target/i386/cpu.c:1294:
+ NULL, "vmx-entry-load-perf-global-ctrl", "vmx-entry-load-pat", "vmx-entry-load-efer",
WARNING: line over 80 characters
#145: FILE: target/i386/cpu.c:1336:
+ "vmx-invvpid-all-context", "vmx-invept-single-context-noglobals",
WARNING: line over 80 characters
#166: FILE: target/i386/cpu.c:1357:
+ /* Just to be safe - we don't support setting the MSEG version field. */
ERROR: line over 90 characters
#216: FILE: target/i386/cpu.c:1416:
+ .from = FEAT_VMX_PROCBASED_CTLS, .from_flag = VMX_CPU_BASED_ACTIVATE_SECONDARY_CONTROLS,
WARNING: line over 80 characters
#225: FILE: target/i386/cpu.c:1425:
+ .to = FEAT_VMX_SECONDARY_CTLS, .to_flags = VMX_SECONDARY_EXEC_RDRAND_EXITING,
WARNING: line over 80 characters
#229: FILE: target/i386/cpu.c:1429:
+ .to = FEAT_VMX_SECONDARY_CTLS, .to_flags = VMX_SECONDARY_EXEC_ENABLE_INVPCID,
WARNING: line over 80 characters
#233: FILE: target/i386/cpu.c:1433:
+ .to = FEAT_VMX_SECONDARY_CTLS, .to_flags = VMX_SECONDARY_EXEC_RDSEED_EXITING,
WARNING: line over 80 characters
#240: FILE: target/i386/cpu.c:1440:
+ .from = FEAT_VMX_SECONDARY_CTLS, .from_flag = VMX_SECONDARY_EXEC_ENABLE_EPT,
WARNING: line over 80 characters
#244: FILE: target/i386/cpu.c:1444:
+ .from = FEAT_VMX_SECONDARY_CTLS, .from_flag = VMX_SECONDARY_EXEC_ENABLE_VPID,
WARNING: line over 80 characters
#248: FILE: target/i386/cpu.c:1448:
+ .from = FEAT_VMX_SECONDARY_CTLS, .from_flag = VMX_SECONDARY_EXEC_ENABLE_VMFUNC,
WARNING: Block comments use a leading /* on a separate line
#362: FILE: target/i386/kvm.c:2458:
+ /* If a feature bit is set, the control can be either set or clear.
WARNING: line over 80 characters
#417: FILE: target/i386/kvm.c:2513:
+ (f[FEAT_VMX_SECONDARY_CTLS] & VMX_SECONDARY_EXEC_ENABLE_EPT ? 0x4100ull : 0);
WARNING: line over 80 characters
#428: FILE: target/i386/kvm.c:2524:
+ f[FEAT_VMX_EXIT_CTLS]) | fixed_vmx_exit);
total: 3 errors, 15 warnings, 438 lines checked
Patch 6/7 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
7/7 Checking commit d88c7eb83ef2 (target/i386: work around KVM_GET_MSRS bug for secondary execution controls)
WARNING: line over 80 characters
#26: FILE: target/i386/kvm.c:473:
+ if (kvm_arch_get_supported_cpuid(s, 0xD, 1, R_ECX) & CPUID_XSAVE_XSAVES) {
WARNING: line over 80 characters
#32: FILE: target/i386/kvm.c:479:
+ if (kvm_arch_get_supported_cpuid(s, 7, 0, R_EBX) & CPUID_7_0_EBX_INVPCID) {
WARNING: line over 80 characters
#35: FILE: target/i386/kvm.c:482:
+ if (kvm_arch_get_supported_cpuid(s, 7, 0, R_EBX) & CPUID_7_0_EBX_RDSEED) {
WARNING: line over 80 characters
#38: FILE: target/i386/kvm.c:485:
+ if (kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_EDX) & CPUID_EXT2_RDTSCP) {
total: 0 errors, 4 warnings, 23 lines checked
Patch 7/7 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/1562079681-19204-1-git-send-email-pbonzini@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 0/7] target/i386: support VMX features in "-cpu"
2019-07-02 15:01 [Qemu-devel] [RFC PATCH 0/7] target/i386: support VMX features in "-cpu" Paolo Bonzini
` (7 preceding siblings ...)
2019-07-02 20:46 ` [Qemu-devel] [RFC PATCH 0/7] target/i386: support VMX features in "-cpu" no-reply
@ 2019-07-02 21:13 ` no-reply
2019-07-02 21:38 ` [Qemu-devel] No symbols in LeakSanitizer output (was Re: [RFC PATCH 0/7] target/i386: support VMX features in "-cpu") Eduardo Habkost
8 siblings, 1 reply; 29+ messages in thread
From: no-reply @ 2019-07-02 21:13 UTC (permalink / raw)
To: pbonzini; +Cc: liran.alon, qemu-devel, ehabkost
Patchew URL: https://patchew.org/QEMU/1562079681-19204-1-git-send-email-pbonzini@redhat.com/
Hi,
This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.
=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===
PASS 1 fdc-test /x86_64/fdc/cmos
PASS 2 fdc-test /x86_64/fdc/no_media_on_start
PASS 3 fdc-test /x86_64/fdc/read_without_media
==8187==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 fdc-test /x86_64/fdc/media_change
PASS 5 fdc-test /x86_64/fdc/sense_interrupt
PASS 6 fdc-test /x86_64/fdc/relative_seek
---
PASS 32 test-opts-visitor /visitor/opts/range/beyond
PASS 33 test-opts-visitor /visitor/opts/dict/unvisited
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-coroutine -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-coroutine"
==8230==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8230==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffe46319000; bottom 0x7f87c24f8000; size: 0x007683e21000 (509018771456)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 1 test-coroutine /basic/no-dangling-access
---
PASS 11 test-aio /aio/event/wait
PASS 12 test-aio /aio/event/flush
PASS 13 test-aio /aio/event/wait/no-flush-cb
==8245==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 14 test-aio /aio/timer/schedule
PASS 15 test-aio /aio/coroutine/queue-chaining
PASS 16 test-aio /aio-gsource/flush
---
PASS 27 test-aio /aio-gsource/event/wait/no-flush-cb
PASS 28 test-aio /aio-gsource/timer/schedule
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-aio-multithread -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-aio-multithread"
==8251==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-aio-multithread /aio/multi/lifecycle
PASS 12 fdc-test /x86_64/fdc/read_no_dma_19
PASS 13 fdc-test /x86_64/fdc/fuzz-registers
PASS 2 test-aio-multithread /aio/multi/schedule
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/ide-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="ide-test"
==8274==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 test-aio-multithread /aio/multi/mutex/contended
PASS 1 ide-test /x86_64/ide/identify
==8285==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 ide-test /x86_64/ide/flush
==8291==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 ide-test /x86_64/ide/bmdma/simple_rw
==8297==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 test-aio-multithread /aio/multi/mutex/handoff
PASS 4 ide-test /x86_64/ide/bmdma/trim
==8308==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 5 ide-test /x86_64/ide/bmdma/short_prdt
PASS 5 test-aio-multithread /aio/multi/mutex/mcs
==8314==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 6 ide-test /x86_64/ide/bmdma/one_sector_short_prdt
PASS 6 test-aio-multithread /aio/multi/mutex/pthread
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-throttle -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-throttle"
==8329==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-throttle /throttle/leak_bucket
PASS 2 test-throttle /throttle/compute_wait
PASS 3 test-throttle /throttle/init
---
PASS 14 test-throttle /throttle/config/max
PASS 15 test-throttle /throttle/config/iops_size
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-thread-pool -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-thread-pool"
==8326==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-thread-pool /thread-pool/submit
PASS 2 test-thread-pool /thread-pool/submit-aio
==8337==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 test-thread-pool /thread-pool/submit-co
PASS 4 test-thread-pool /thread-pool/submit-many
PASS 7 ide-test /x86_64/ide/bmdma/long_prdt
==8406==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8406==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffe17627000; bottom 0x7f37c7dfe000; size: 0x00c64f829000 (851737481216)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 8 ide-test /x86_64/ide/bmdma/no_busmaster
PASS 5 test-thread-pool /thread-pool/cancel
PASS 9 ide-test /x86_64/ide/flush/nodev
==8417==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 10 ide-test /x86_64/ide/flush/empty_drive
PASS 6 test-thread-pool /thread-pool/cancel-async
==8422==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-hbitmap -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-hbitmap"
PASS 1 test-hbitmap /hbitmap/granularity
PASS 2 test-hbitmap /hbitmap/size/0
---
PASS 4 test-hbitmap /hbitmap/iter/empty
PASS 11 ide-test /x86_64/ide/flush/retry_pci
PASS 5 test-hbitmap /hbitmap/iter/partial
==8433==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 6 test-hbitmap /hbitmap/iter/granularity
PASS 7 test-hbitmap /hbitmap/iter/iter_and_reset
PASS 8 test-hbitmap /hbitmap/get/all
---
PASS 14 test-hbitmap /hbitmap/set/twice
PASS 15 test-hbitmap /hbitmap/set/overlap
PASS 16 test-hbitmap /hbitmap/reset/empty
==8439==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 13 ide-test /x86_64/ide/cdrom/pio
PASS 17 test-hbitmap /hbitmap/reset/general
PASS 18 test-hbitmap /hbitmap/reset/all
---
PASS 28 test-hbitmap /hbitmap/truncate/shrink/medium
PASS 29 test-hbitmap /hbitmap/truncate/shrink/large
PASS 30 test-hbitmap /hbitmap/meta/zero
==8445==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 14 ide-test /x86_64/ide/cdrom/pio_large
==8451==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 15 ide-test /x86_64/ide/cdrom/dma
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/ahci-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="ahci-test"
==8465==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 ahci-test /x86_64/ahci/sanity
==8471==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 31 test-hbitmap /hbitmap/meta/one
PASS 32 test-hbitmap /hbitmap/meta/byte
PASS 33 test-hbitmap /hbitmap/meta/word
PASS 2 ahci-test /x86_64/ahci/pci_spec
==8477==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 34 test-hbitmap /hbitmap/meta/sector
PASS 35 test-hbitmap /hbitmap/serialize/align
PASS 3 ahci-test /x86_64/ahci/pci_enable
==8483==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 ahci-test /x86_64/ahci/hba_spec
PASS 36 test-hbitmap /hbitmap/serialize/basic
PASS 37 test-hbitmap /hbitmap/serialize/part
---
PASS 39 test-hbitmap /hbitmap/next_zero/next_zero_0
PASS 40 test-hbitmap /hbitmap/next_zero/next_zero_4
PASS 41 test-hbitmap /hbitmap/next_dirty_area/next_dirty_area_0
==8489==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 42 test-hbitmap /hbitmap/next_dirty_area/next_dirty_area_1
PASS 43 test-hbitmap /hbitmap/next_dirty_area/next_dirty_area_4
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-bdrv-drain -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-bdrv-drain"
==8496==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-bdrv-drain /bdrv-drain/nested
PASS 2 test-bdrv-drain /bdrv-drain/multiparent
PASS 3 test-bdrv-drain /bdrv-drain/set_aio_context
---
PASS 38 test-bdrv-drain /bdrv-drain/detach/driver_cb
PASS 39 test-bdrv-drain /bdrv-drain/attach/drain
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-bdrv-graph-mod -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-bdrv-graph-mod"
==8514==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8538==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-bdrv-graph-mod /bdrv-graph-mod/update-perm-tree
PASS 2 test-bdrv-graph-mod /bdrv-graph-mod/should-update-child
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-blockjob -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-blockjob"
==8547==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-blockjob /blockjob/ids
PASS 2 test-blockjob /blockjob/cancel/created
PASS 3 test-blockjob /blockjob/cancel/running
---
PASS 8 test-blockjob /blockjob/cancel/concluded
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-blockjob-txn -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-blockjob-txn"
PASS 6 ahci-test /x86_64/ahci/identify
==8553==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-blockjob-txn /single/success
PASS 2 test-blockjob-txn /single/failure
PASS 3 test-blockjob-txn /single/cancel
---
PASS 6 test-blockjob-txn /pair/cancel
PASS 7 test-blockjob-txn /pair/fail-cancel-race
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-block-backend -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-block-backend"
==8555==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8559==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-block-backend /block-backend/drain_aio_error
PASS 2 test-block-backend /block-backend/drain_all_aio_error
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-block-iothread -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-block-iothread"
==8568==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-block-iothread /sync-op/pread
PASS 2 test-block-iothread /sync-op/pwrite
PASS 3 test-block-iothread /sync-op/load_vmstate
---
PASS 16 test-block-iothread /propagate/mirror
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-image-locking -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-image-locking"
PASS 7 ahci-test /x86_64/ahci/max
==8590==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-image-locking /image-locking/basic
PASS 2 test-image-locking /image-locking/set-perm-abort
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-x86-cpuid -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-x86-cpuid"
==8592==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-x86-cpuid /cpuid/topology/basic
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-xbzrle -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-xbzrle"
PASS 1 test-xbzrle /xbzrle/uleb
---
PASS 10 test-int128 /int128/int128_rshift
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/rcutorture -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="rcutorture"
PASS 8 ahci-test /x86_64/ahci/reset
==8640==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 rcutorture /rcu/torture/1reader
==8640==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffd8d7d3000; bottom 0x7f1c807fe000; size: 0x00e10cfd5000 (966585569280)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 9 ahci-test /x86_64/ahci/io/pio/lba28/simple/zero
PASS 2 rcutorture /rcu/torture/10readers
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-rcu-list -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-rcu-list"
==8662==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8662==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffcc6447000; bottom 0x7f3e3f3fe000; size: 0x00be87049000 (818309009408)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 1 test-rcu-list /rcu/qlist/single-threaded
PASS 10 ahci-test /x86_64/ahci/io/pio/lba28/simple/low
==8681==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8681==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffccaf29000; bottom 0x7ff77a3fe000; size: 0x000550b2b000 (22828724224)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 2 test-rcu-list /rcu/qlist/short-few
PASS 11 ahci-test /x86_64/ahci/io/pio/lba28/simple/high
==8708==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8708==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fff0fc57000; bottom 0x7f2c471fe000; size: 0x00d2c8a59000 (905309425664)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 12 ahci-test /x86_64/ahci/io/pio/lba28/double/zero
==8714==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 test-rcu-list /rcu/qlist/long-many
==8714==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffda8520000; bottom 0x7f0b8a5fe000; size: 0x00f21df22000 (1039884492800)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-rcu-simpleq -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-rcu-simpleq"
PASS 13 ahci-test /x86_64/ahci/io/pio/lba28/double/low
==8727==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8727==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffc2466f000; bottom 0x7fe61f5fe000; size: 0x001605071000 (94573629440)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 1 test-rcu-simpleq /rcu/qsimpleq/single-threaded
PASS 14 ahci-test /x86_64/ahci/io/pio/lba28/double/high
==8739==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8739==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fffdb110000; bottom 0x7f157757c000; size: 0x00ea63b94000 (1006695432192)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 2 test-rcu-simpleq /rcu/qsimpleq/short-few
PASS 15 ahci-test /x86_64/ahci/io/pio/lba28/long/zero
==8766==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8766==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffe6cb46000; bottom 0x7fe46237c000; size: 0x001a0a7ca000 (111845089280)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 16 ahci-test /x86_64/ahci/io/pio/lba28/long/low
PASS 3 test-rcu-simpleq /rcu/qsimpleq/long-many
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-rcu-tailq -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-rcu-tailq"
==8772==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8772==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fff2a02a000; bottom 0x7f88bb97c000; size: 0x00766e6ae000 (508658638848)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 1 test-rcu-tailq /rcu/qtailq/single-threaded
PASS 17 ahci-test /x86_64/ahci/io/pio/lba28/long/high
==8791==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 test-rcu-tailq /rcu/qtailq/short-few
PASS 18 ahci-test /x86_64/ahci/io/pio/lba28/short/zero
==8818==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 19 ahci-test /x86_64/ahci/io/pio/lba28/short/low
==8824==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 test-rcu-tailq /rcu/qtailq/long-many
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-qdist -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-qdist"
PASS 1 test-qdist /qdist/none
---
PASS 8 test-qdist /qdist/binning/shrink
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-qht -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-qht"
PASS 20 ahci-test /x86_64/ahci/io/pio/lba28/short/high
==8839==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8839==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffe51e74000; bottom 0x7fc2085fe000; size: 0x003c49876000 (258931646464)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 21 ahci-test /x86_64/ahci/io/pio/lba48/simple/zero
==8845==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8845==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffc87228000; bottom 0x7fc7f6dfe000; size: 0x00349042a000 (225758584832)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 22 ahci-test /x86_64/ahci/io/pio/lba48/simple/low
==8851==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8851==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fff13fd4000; bottom 0x7f81dc7fe000; size: 0x007d377d6000 (537801875456)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 23 ahci-test /x86_64/ahci/io/pio/lba48/simple/high
==8857==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8857==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffc95f5f000; bottom 0x7fa84effe000; size: 0x005446f61000 (361967783936)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 24 ahci-test /x86_64/ahci/io/pio/lba48/double/zero
==8863==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8863==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffdd7a63000; bottom 0x7fec59dfe000; size: 0x00117dc65000 (75124592640)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 25 ahci-test /x86_64/ahci/io/pio/lba48/double/low
==8869==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8869==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffcd3d41000; bottom 0x7faa967fe000; size: 0x00523d543000 (353216245760)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 26 ahci-test /x86_64/ahci/io/pio/lba48/double/high
==8875==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8875==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fff52504000; bottom 0x7f87febfe000; size: 0x007753906000 (512503078912)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 27 ahci-test /x86_64/ahci/io/pio/lba48/long/zero
==8881==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8881==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fff51b36000; bottom 0x7fa1e99fe000; size: 0x005d68138000 (401178066944)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 28 ahci-test /x86_64/ahci/io/pio/lba48/long/low
==8887==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8887==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fff7aa37000; bottom 0x7f11b3d24000; size: 0x00edc6d13000 (1021242847232)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 29 ahci-test /x86_64/ahci/io/pio/lba48/long/high
==8893==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 30 ahci-test /x86_64/ahci/io/pio/lba48/short/zero
PASS 1 test-qht /qht/mode/default
PASS 2 test-qht /qht/mode/resize
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-qht-par -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-qht-par"
==8899==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 31 ahci-test /x86_64/ahci/io/pio/lba48/short/low
PASS 1 test-qht-par /qht/parallel/2threads-0%updates-1s
==8915==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 32 ahci-test /x86_64/ahci/io/pio/lba48/short/high
==8928==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 test-qht-par /qht/parallel/2threads-20%updates-1s
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-bitops -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-bitops"
PASS 1 test-bitops /bitops/sextract32
---
PASS 1 check-qom-interface /qom/interface/direct_impl
PASS 2 check-qom-interface /qom/interface/intermediate_impl
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/check-qom-proplist -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="check-qom-proplist"
==8952==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 check-qom-proplist /qom/proplist/createlist
PASS 2 check-qom-proplist /qom/proplist/createv
PASS 3 check-qom-proplist /qom/proplist/createcmdline
---
PASS 3 test-crypto-hmac /crypto/hmac/prealloc
PASS 4 test-crypto-hmac /crypto/hmac/digest
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-crypto-cipher -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-crypto-cipher"
==8987==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-crypto-cipher /crypto/cipher/aes-ecb-128
PASS 2 test-crypto-cipher /crypto/cipher/aes-ecb-192
PASS 3 test-crypto-cipher /crypto/cipher/aes-ecb-256
---
PASS 35 ahci-test /x86_64/ahci/io/dma/lba28/simple/zero
PASS 1 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/perfectserver
PASS 2 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/perfectclient
==9012==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodca1
PASS 36 ahci-test /x86_64/ahci/io/dma/lba28/simple/low
==9018==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 37 ahci-test /x86_64/ahci/io/dma/lba28/simple/high
PASS 4 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodca2
==9024==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 5 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodca3
PASS 6 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/badca1
PASS 7 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/badca2
---
PASS 38 ahci-test /x86_64/ahci/io/dma/lba28/double/zero
PASS 9 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodserver1
PASS 10 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodserver2
==9030==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 11 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodserver3
PASS 39 ahci-test /x86_64/ahci/io/dma/lba28/double/low
==9036==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 12 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodserver4
PASS 40 ahci-test /x86_64/ahci/io/dma/lba28/double/high
==9042==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 13 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodserver5
PASS 14 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodserver6
PASS 15 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodserver7
---
PASS 33 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/inactive2
PASS 34 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/inactive3
PASS 41 ahci-test /x86_64/ahci/io/dma/lba28/long/zero
==9048==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 35 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/chain1
PASS 36 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/chain2
PASS 37 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/missingca
---
PASS 42 ahci-test /x86_64/ahci/io/dma/lba28/long/low
PASS 1 test-crypto-tlssession /qcrypto/tlssession/psk
PASS 2 test-crypto-tlssession /qcrypto/tlssession/basicca
==9059==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 test-crypto-tlssession /qcrypto/tlssession/differentca
PASS 4 test-crypto-tlssession /qcrypto/tlssession/altname1
PASS 43 ahci-test /x86_64/ahci/io/dma/lba28/long/high
==9065==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 5 test-crypto-tlssession /qcrypto/tlssession/altname2
PASS 44 ahci-test /x86_64/ahci/io/dma/lba28/short/zero
==9071==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 6 test-crypto-tlssession /qcrypto/tlssession/altname3
PASS 45 ahci-test /x86_64/ahci/io/dma/lba28/short/low
PASS 7 test-crypto-tlssession /qcrypto/tlssession/altname4
==9077==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 46 ahci-test /x86_64/ahci/io/dma/lba28/short/high
==9083==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 47 ahci-test /x86_64/ahci/io/dma/lba48/simple/zero
==9089==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 8 test-crypto-tlssession /qcrypto/tlssession/altname5
PASS 9 test-crypto-tlssession /qcrypto/tlssession/altname6
PASS 48 ahci-test /x86_64/ahci/io/dma/lba48/simple/low
==9095==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 10 test-crypto-tlssession /qcrypto/tlssession/wildcard1
PASS 49 ahci-test /x86_64/ahci/io/dma/lba48/simple/high
==9101==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 11 test-crypto-tlssession /qcrypto/tlssession/wildcard2
PASS 50 ahci-test /x86_64/ahci/io/dma/lba48/double/zero
==9107==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 12 test-crypto-tlssession /qcrypto/tlssession/wildcard3
PASS 51 ahci-test /x86_64/ahci/io/dma/lba48/double/low
==9114==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 13 test-crypto-tlssession /qcrypto/tlssession/wildcard4
PASS 14 test-crypto-tlssession /qcrypto/tlssession/wildcard5
PASS 52 ahci-test /x86_64/ahci/io/dma/lba48/double/high
PASS 15 test-crypto-tlssession /qcrypto/tlssession/wildcard6
==9120==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 53 ahci-test /x86_64/ahci/io/dma/lba48/long/zero
PASS 16 test-crypto-tlssession /qcrypto/tlssession/cachain
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-qga -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-qga"
==9126==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-qga /qga/sync-delimited
PASS 2 test-qga /qga/sync
PASS 3 test-qga /qga/ping
---
PASS 15 test-qga /qga/invalid-cmd
PASS 16 test-qga /qga/invalid-args
PASS 17 test-qga /qga/fsfreeze-status
==9138==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 18 test-qga /qga/blacklist
PASS 19 test-qga /qga/config
PASS 20 test-qga /qga/guest-exec
PASS 21 test-qga /qga/guest-exec-invalid
PASS 55 ahci-test /x86_64/ahci/io/dma/lba48/long/high
==9152==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 22 test-qga /qga/guest-get-osinfo
PASS 23 test-qga /qga/guest-get-host-name
PASS 24 test-qga /qga/guest-get-timezone
---
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-util-filemonitor -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-util-filemonitor"
PASS 1 test-util-filemonitor /util/filemonitor
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-util-sockets -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-util-sockets"
==9166==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-util-sockets /util/socket/is-socket/bad
PASS 2 test-util-sockets /util/socket/is-socket/good
PASS 3 test-util-sockets /socket/fd-pass/name/good
---
PASS 4 test-io-task /crypto/task/thread_complete
PASS 5 test-io-task /crypto/task/thread_failure
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-io-channel-socket -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-io-channel-socket"
==9197==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-io-channel-socket /io/channel/socket/ipv4-sync
PASS 2 test-io-channel-socket /io/channel/socket/ipv4-async
PASS 3 test-io-channel-socket /io/channel/socket/ipv4-fd
---
PASS 5 test-io-channel-file /io/channel/pipe/async
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-io-channel-tls -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-io-channel-tls"
PASS 58 ahci-test /x86_64/ahci/io/dma/lba48/short/high
==9264==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-io-channel-tls /qio/channel/tls/basic
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-io-channel-command -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-io-channel-command"
PASS 59 ahci-test /x86_64/ahci/io/ncq/simple
---
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-io-channel-buffer -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-io-channel-buffer"
PASS 1 test-io-channel-buffer /io/channel/buf
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-base64 -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-base64"
==9280==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-base64 /util/base64/good
PASS 2 test-base64 /util/base64/embedded-nul
PASS 3 test-base64 /util/base64/not-nul-terminated
---
PASS 17 test-crypto-xts /crypto/xts/t-21-key-32-ptx-31/basic
PASS 18 test-crypto-xts /crypto/xts/t-21-key-32-ptx-31/unaligned
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-crypto-block -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-crypto-block"
==9310==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-crypto-block /crypto/block/qcow
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-logging -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-logging"
PASS 1 test-logging /logging/parse_range
PASS 2 test-logging /logging/parse_path
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-replication -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-replication"
==9330==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 61 ahci-test /x86_64/ahci/flush/simple
==9335==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 62 ahci-test /x86_64/ahci/flush/retry
PASS 1 test-replication /replication/primary/read
PASS 2 test-replication /replication/primary/write
==9341==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9346==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 test-replication /replication/primary/start
PASS 4 test-replication /replication/primary/stop
PASS 5 test-replication /replication/primary/do_checkpoint
PASS 6 test-replication /replication/primary/get_error_all
PASS 7 test-replication /replication/secondary/read
PASS 63 ahci-test /x86_64/ahci/flush/migrate
==9355==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 8 test-replication /replication/secondary/write
==9361==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9330==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffc7fe12000; bottom 0x7fab4b1fc000; size: 0x005134c16000 (348777439232)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 64 ahci-test /x86_64/ahci/migrate/sanity
==9398==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9403==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 9 test-replication /replication/secondary/start
PASS 10 test-replication /replication/secondary/stop
PASS 65 ahci-test /x86_64/ahci/migrate/dma/simple
==9412==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9417==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 11 test-replication /replication/secondary/do_checkpoint
PASS 12 test-replication /replication/secondary/get_error_all
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} tests/test-bufferiszero -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-bufferiszero"
PASS 66 ahci-test /x86_64/ahci/migrate/dma/halted
==9430==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9435==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 67 ahci-test /x86_64/ahci/migrate/ncq/simple
==9444==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9449==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 68 ahci-test /x86_64/ahci/migrate/ncq/halted
==9458==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 69 ahci-test /x86_64/ahci/cdrom/eject
==9463==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 70 ahci-test /x86_64/ahci/cdrom/dma/single
==9469==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 71 ahci-test /x86_64/ahci/cdrom/dma/multi
==9475==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 72 ahci-test /x86_64/ahci/cdrom/pio/single
==9481==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9481==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffd368ef000; bottom 0x7fd555dfe000; size: 0x0027e0af1000 (171273293824)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 73 ahci-test /x86_64/ahci/cdrom/pio/multi
==9487==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 74 ahci-test /x86_64/ahci/cdrom/pio/bcl
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/hd-geo-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="hd-geo-test"
PASS 1 hd-geo-test /x86_64/hd-geo/ide/none
==9501==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 hd-geo-test /x86_64/hd-geo/ide/drive/cd_0
==9507==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 hd-geo-test /x86_64/hd-geo/ide/drive/mbr/blank
==9513==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 hd-geo-test /x86_64/hd-geo/ide/drive/mbr/lba
==9519==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 5 hd-geo-test /x86_64/hd-geo/ide/drive/mbr/chs
==9525==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 6 hd-geo-test /x86_64/hd-geo/ide/device/mbr/blank
==9531==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 7 hd-geo-test /x86_64/hd-geo/ide/device/mbr/lba
==9537==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 8 hd-geo-test /x86_64/hd-geo/ide/device/mbr/chs
==9543==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 9 hd-geo-test /x86_64/hd-geo/ide/device/user/chs
==9548==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 10 hd-geo-test /x86_64/hd-geo/ide/device/user/chst
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/boot-order-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="boot-order-test"
PASS 1 test-bufferiszero /cutils/bufferiszero
---
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9633==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 bios-tables-test /x86_64/acpi/piix4
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9639==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 bios-tables-test /x86_64/acpi/q35
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9645==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 bios-tables-test /x86_64/acpi/piix4/bridge
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9651==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 bios-tables-test /x86_64/acpi/piix4/ipmi
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9657==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 5 bios-tables-test /x86_64/acpi/piix4/cpuhp
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9664==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 6 bios-tables-test /x86_64/acpi/piix4/memhp
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9670==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 7 bios-tables-test /x86_64/acpi/piix4/numamem
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9676==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 8 bios-tables-test /x86_64/acpi/piix4/dimmpxm
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9685==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 9 bios-tables-test /x86_64/acpi/q35/bridge
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9691==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 10 bios-tables-test /x86_64/acpi/q35/mmio64
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9697==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 11 bios-tables-test /x86_64/acpi/q35/ipmi
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9703==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 12 bios-tables-test /x86_64/acpi/q35/cpuhp
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9710==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 13 bios-tables-test /x86_64/acpi/q35/memhp
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9716==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 14 bios-tables-test /x86_64/acpi/q35/numamem
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9722==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 15 bios-tables-test /x86_64/acpi/q35/dimmpxm
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/boot-serial-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="boot-serial-test"
PASS 1 boot-serial-test /x86_64/boot-serial/isapc
---
PASS 1 i440fx-test /x86_64/i440fx/defaults
PASS 2 i440fx-test /x86_64/i440fx/pam
PASS 3 i440fx-test /x86_64/i440fx/firmware/bios
==9806==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 i440fx-test /x86_64/i440fx/firmware/pflash
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/fw_cfg-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="fw_cfg-test"
PASS 1 fw_cfg-test /x86_64/fw_cfg/signature
---
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/drive_del-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="drive_del-test"
PASS 1 drive_del-test /x86_64/drive_del/without-dev
PASS 2 drive_del-test /x86_64/drive_del/after_failed_device_add
==9894==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 drive_del-test /x86_64/blockdev/drive_del_device_del
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/wdt_ib700-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="wdt_ib700-test"
PASS 1 wdt_ib700-test /x86_64/wdt_ib700/pause
---
PASS 1 usb-hcd-uhci-test /x86_64/uhci/pci/init
PASS 2 usb-hcd-uhci-test /x86_64/uhci/pci/port1
PASS 3 usb-hcd-uhci-test /x86_64/uhci/pci/hotplug
==10089==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 usb-hcd-uhci-test /x86_64/uhci/pci/hotplug/usb-storage
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/usb-hcd-xhci-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="usb-hcd-xhci-test"
PASS 1 usb-hcd-xhci-test /x86_64/xhci/pci/init
PASS 2 usb-hcd-xhci-test /x86_64/xhci/pci/hotplug
==10098==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 usb-hcd-xhci-test /x86_64/xhci/pci/hotplug/usb-uas
PASS 4 usb-hcd-xhci-test /x86_64/xhci/pci/hotplug/usb-ccid
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/cpu-plug-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="cpu-plug-test"
---
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10204==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 vmgenid-test /x86_64/vmgenid/vmgenid/set-guid
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10210==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 vmgenid-test /x86_64/vmgenid/vmgenid/set-guid-auto
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10216==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 vmgenid-test /x86_64/vmgenid/vmgenid/query-monitor
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/tpm-crb-swtpm-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="tpm-crb-swtpm-test"
SKIP 1 tpm-crb-swtpm-test /x86_64/tpm/crb-swtpm/test # SKIP swtpm not in PATH or missing --tpm2 support
---
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10321==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10326==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 migration-test /x86_64/migration/fd_proto
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10334==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10339==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 migration-test /x86_64/migration/postcopy/unix
PASS 5 migration-test /x86_64/migration/postcopy/recovery
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10369==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10374==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 6 migration-test /x86_64/migration/precopy/unix
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10383==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10388==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 7 migration-test /x86_64/migration/precopy/tcp
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10397==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10402==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 8 migration-test /x86_64/migration/xbzrle/unix
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/test-x86-cpuid-compat -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-x86-cpuid-compat"
PASS 1 test-x86-cpuid-compat /x86/cpuid/parsing-plus-minus
---
PASS 6 numa-test /x86_64/numa/pc/dynamic/cpu
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/qmp-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="qmp-test"
PASS 1 qmp-test /x86_64/qmp/protocol
==10731==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 qmp-test /x86_64/qmp/oob
PASS 3 qmp-test /x86_64/qmp/preconfig
PASS 4 qmp-test /x86_64/qmp/missing-any-arg
---
PASS 5 device-introspect-test /x86_64/device/introspect/abstract-interfaces
=================================================================
==10979==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 32 byte(s) in 1 object(s) allocated from:
#0 0x558e9bbf3b1e in calloc (/tmp/qemu-test/build/x86_64-softmmu/qemu-system-x86_64+0x19f8b1e)
---
SUMMARY: AddressSanitizer: 64 byte(s) leaked in 2 allocation(s).
/tmp/qemu-test/src/tests/libqtest.c:137: kill_qemu() tried to terminate QEMU process but encountered exit status 1
ERROR - too few tests run (expected 6, got 5)
make: *** [/tmp/qemu-test/src/tests/Makefile.include:894: check-qtest-x86_64] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
The full log is available at
http://patchew.org/logs/1562079681-19204-1-git-send-email-pbonzini@redhat.com/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Qemu-devel] No symbols in LeakSanitizer output (was Re: [RFC PATCH 0/7] target/i386: support VMX features in "-cpu")
2019-07-02 21:13 ` no-reply
@ 2019-07-02 21:38 ` Eduardo Habkost
2019-07-02 23:05 ` Peter Maydell
2019-07-05 10:19 ` Paolo Bonzini
0 siblings, 2 replies; 29+ messages in thread
From: Eduardo Habkost @ 2019-07-02 21:38 UTC (permalink / raw)
To: patchew-devel; +Cc: pbonzini, liran.alon, qemu-devel
Can the asan build test in Patchew be updated to include
symbolize=1?
For reference, below is the full stack trace of the leak. It
looks like it existed for a long time.
[qemu/machine-next]$ export ASAN_OPTIONS=symbolize=1
[qemu/machine-next]$ export ASAN_SYMBOLIZER_PATH=/usr/bin/llvm-symbolizer
[qemu/machine-next]$ MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/device-introspect-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="qmp-test"
PASS 1 qmp-test /x86_64/device/introspect/list
PASS 2 qmp-test /x86_64/device/introspect/list-fields
PASS 3 qmp-test /x86_64/device/introspect/none
PASS 4 qmp-test /x86_64/device/introspect/abstract
PASS 5 qmp-test /x86_64/device/introspect/abstract-interfaces
=================================================================
==21992==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 32 byte(s) in 1 object(s) allocated from:
#0 0x55e9a04dfc6e in calloc (/home/ehabkost/rh/proj/virt/qemu/x86_64-softmmu/qemu-system-x86_64+0x18b5c6e)
#1 0x7fb604a10cf0 in g_malloc0 (/lib64/libglib-2.0.so.0+0x55cf0)
#2 0x55e9a1712c0b in gpio_i2c_init /home/ehabkost/rh/proj/virt/qemu/hw/i2c/bitbang_i2c.c:237:18
#3 0x55e9a23885d8 in object_init_with_type /home/ehabkost/rh/proj/virt/qemu/qom/object.c:359:9
#4 0x55e9a236e80f in object_initialize_with_type /home/ehabkost/rh/proj/virt/qemu/qom/object.c:463:5
#5 0x55e9a2370b95 in object_new_with_type /home/ehabkost/rh/proj/virt/qemu/qom/object.c:632:5
#6 0x55e9a23709b1 in object_new /home/ehabkost/rh/proj/virt/qemu/qom/object.c:642:12
#7 0x55e9a20b5cc8 in qmp_device_list_properties /home/ehabkost/rh/proj/virt/qemu/monitor/qmp-cmds.c:528:11
#8 0x55e9a11b5094 in qdev_device_help /home/ehabkost/rh/proj/virt/qemu/qdev-monitor.c:275:17
#9 0x55e9a11bab2a in qmp_device_add /home/ehabkost/rh/proj/virt/qemu/qdev-monitor.c:810:34
#10 0x55e9a20e31a1 in hmp_device_add /home/ehabkost/rh/proj/virt/qemu/monitor/hmp-cmds.c:2169:5
#11 0x55e9a20a2de6 in handle_hmp_command /home/ehabkost/rh/proj/virt/qemu/monitor/hmp.c:1082:5
#12 0x55e9a0d883af in qmp_human_monitor_command /home/ehabkost/rh/proj/virt/qemu/monitor/misc.c:139:5
#13 0x55e9a21c4a24 in qmp_marshal_human_monitor_command /home/ehabkost/rh/proj/virt/qemu/qapi/qapi-commands-misc.c:1172:14
#14 0x55e9a2983c28 in do_qmp_dispatch /home/ehabkost/rh/proj/virt/qemu/qapi/qmp-dispatch.c:131:5
#15 0x55e9a2982f05 in qmp_dispatch /home/ehabkost/rh/proj/virt/qemu/qapi/qmp-dispatch.c:174:11
#16 0x55e9a209d2f5 in monitor_qmp_dispatch /home/ehabkost/rh/proj/virt/qemu/monitor/qmp.c:120:11
#17 0x55e9a209b525 in monitor_qmp_bh_dispatcher /home/ehabkost/rh/proj/virt/qemu/monitor/qmp.c:209:9
#18 0x55e9a2b1942a in aio_bh_call /home/ehabkost/rh/proj/virt/qemu/util/async.c:89:5
#19 0x55e9a2b19b42 in aio_bh_poll /home/ehabkost/rh/proj/virt/qemu/util/async.c:117:13
#20 0x55e9a2b3b800 in aio_dispatch /home/ehabkost/rh/proj/virt/qemu/util/aio-posix.c:459:5
#21 0x55e9a2b1eb73 in aio_ctx_dispatch /home/ehabkost/rh/proj/virt/qemu/util/async.c:260:5
#22 0x7fb604a0aedc in g_main_context_dispatch (/lib64/libglib-2.0.so.0+0x4fedc)
Direct leak of 32 byte(s) in 1 object(s) allocated from:
#0 0x55e9a04dfc6e in calloc (/home/ehabkost/rh/proj/virt/qemu/x86_64-softmmu/qemu-system-x86_64+0x18b5c6e)
#1 0x7fb604a10cf0 in g_malloc0 (/lib64/libglib-2.0.so.0+0x55cf0)
#2 0x55e9a1712c0b in gpio_i2c_init /home/ehabkost/rh/proj/virt/qemu/hw/i2c/bitbang_i2c.c:237:18
#3 0x55e9a23885d8 in object_init_with_type /home/ehabkost/rh/proj/virt/qemu/qom/object.c:359:9
#4 0x55e9a236e80f in object_initialize_with_type /home/ehabkost/rh/proj/virt/qemu/qom/object.c:463:5
#5 0x55e9a2370b95 in object_new_with_type /home/ehabkost/rh/proj/virt/qemu/qom/object.c:632:5
#6 0x55e9a23709b1 in object_new /home/ehabkost/rh/proj/virt/qemu/qom/object.c:642:12
#7 0x55e9a20b5cc8 in qmp_device_list_properties /home/ehabkost/rh/proj/virt/qemu/monitor/qmp-cmds.c:528:11
#8 0x55e9a21c7624 in qmp_marshal_device_list_properties /home/ehabkost/rh/proj/virt/qemu/qapi/qapi-commands-misc.c:1439:14
#9 0x55e9a2983c28 in do_qmp_dispatch /home/ehabkost/rh/proj/virt/qemu/qapi/qmp-dispatch.c:131:5
#10 0x55e9a2982f05 in qmp_dispatch /home/ehabkost/rh/proj/virt/qemu/qapi/qmp-dispatch.c:174:11
#11 0x55e9a209d2f5 in monitor_qmp_dispatch /home/ehabkost/rh/proj/virt/qemu/monitor/qmp.c:120:11
#12 0x55e9a209b525 in monitor_qmp_bh_dispatcher /home/ehabkost/rh/proj/virt/qemu/monitor/qmp.c:209:9
#13 0x55e9a2b1942a in aio_bh_call /home/ehabkost/rh/proj/virt/qemu/util/async.c:89:5
#14 0x55e9a2b19b42 in aio_bh_poll /home/ehabkost/rh/proj/virt/qemu/util/async.c:117:13
#15 0x55e9a2b3b800 in aio_dispatch /home/ehabkost/rh/proj/virt/qemu/util/aio-posix.c:459:5
#16 0x55e9a2b1eb73 in aio_ctx_dispatch /home/ehabkost/rh/proj/virt/qemu/util/async.c:260:5
#17 0x7fb604a0aedc in g_main_context_dispatch (/lib64/libglib-2.0.so.0+0x4fedc)
SUMMARY: AddressSanitizer: 64 byte(s) leaked in 2 allocation(s).
tests/libqtest.c:137: kill_qemu() tried to terminate QEMU process but encountered exit status 1
ERROR - too few tests run (expected 6, got 5)
On Tue, Jul 02, 2019 at 02:13:39PM -0700, no-reply@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/1562079681-19204-1-git-send-email-pbonzini@redhat.com/
[...]
> =================================================================
> ==10979==ERROR: LeakSanitizer: detected memory leaks
>
> Direct leak of 32 byte(s) in 1 object(s) allocated from:
> #0 0x558e9bbf3b1e in calloc (/tmp/qemu-test/build/x86_64-softmmu/qemu-system-x86_64+0x19f8b1e)
> ---
>
> SUMMARY: AddressSanitizer: 64 byte(s) leaked in 2 allocation(s).
> /tmp/qemu-test/src/tests/libqtest.c:137: kill_qemu() tried to terminate QEMU process but encountered exit status 1
> ERROR - too few tests run (expected 6, got 5)
> make: *** [/tmp/qemu-test/src/tests/Makefile.include:894: check-qtest-x86_64] Error 1
> make: *** Waiting for unfinished jobs....
> Traceback (most recent call last):
>
>
> The full log is available at
> http://patchew.org/logs/1562079681-19204-1-git-send-email-pbonzini@redhat.com/testing.asan/?type=message.
> ---
> Email generated automatically by Patchew [https://patchew.org/].
> Please send your feedback to patchew-devel@redhat.com
--
Eduardo
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] No symbols in LeakSanitizer output (was Re: [RFC PATCH 0/7] target/i386: support VMX features in "-cpu")
2019-07-02 21:38 ` [Qemu-devel] No symbols in LeakSanitizer output (was Re: [RFC PATCH 0/7] target/i386: support VMX features in "-cpu") Eduardo Habkost
@ 2019-07-02 23:05 ` Peter Maydell
2019-07-05 10:19 ` Paolo Bonzini
1 sibling, 0 replies; 29+ messages in thread
From: Peter Maydell @ 2019-07-02 23:05 UTC (permalink / raw)
To: Eduardo Habkost; +Cc: patchew-devel, Paolo Bonzini, liran.alon, QEMU Developers
On Tue, 2 Jul 2019 at 22:40, Eduardo Habkost <ehabkost@redhat.com> wrote:
>
> Can the asan build test in Patchew be updated to include
> symbolize=1?
>
> For reference, below is the full stack trace of the leak. It
> looks like it existed for a long time.
Yeah. What's new is that now we build that device for
the x86-64 config that patchew runs the asan tests against.
(I sent a patch with a suggested fix earlier today.)
thanks
-- PMM
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] No symbols in LeakSanitizer output (was Re: [RFC PATCH 0/7] target/i386: support VMX features in "-cpu")
2019-07-02 21:38 ` [Qemu-devel] No symbols in LeakSanitizer output (was Re: [RFC PATCH 0/7] target/i386: support VMX features in "-cpu") Eduardo Habkost
2019-07-02 23:05 ` Peter Maydell
@ 2019-07-05 10:19 ` Paolo Bonzini
1 sibling, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2019-07-05 10:19 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel
On 02/07/19 23:38, Eduardo Habkost wrote:
> Can the asan build test in Patchew be updated to include
> symbolize=1?
>
> For reference, below is the full stack trace of the leak. It
> looks like it existed for a long time.
Sure, just add the "export" lines to tests/docker/test-debug and Patchew
will pick them up automatically.
Paolo
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 1/7] target/i386: handle filtered_features in a new function mark_unavailable_features
2019-07-02 15:01 ` [Qemu-devel] [PATCH 1/7] target/i386: handle filtered_features in a new function mark_unavailable_features Paolo Bonzini
@ 2019-07-05 20:37 ` Eduardo Habkost
2019-07-05 21:32 ` Paolo Bonzini
0 siblings, 1 reply; 29+ messages in thread
From: Eduardo Habkost @ 2019-07-05 20:37 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Liran Alon, qemu-devel
On Tue, Jul 02, 2019 at 05:01:15PM +0200, Paolo Bonzini wrote:
> The next patch will add a different reason for filtering features, unrelated
> to host feature support. Extract a new function that takes care of disabling
> the features and reporting them.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> target/i386/cpu.c | 76 ++++++++++++++++++++++++++++++-------------------------
> 1 file changed, 41 insertions(+), 35 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index da6eb67..9149d0d 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -3236,17 +3236,39 @@ static char *feature_word_description(FeatureWordInfo *f, uint32_t bit)
> return NULL;
> }
>
> -static void report_unavailable_features(FeatureWord w, uint32_t mask)
> +static bool x86_cpu_have_filtered_features(X86CPU *cpu)
> {
> + FeatureWord w;
> +
> + for (w = 0; w < ARRAY_SIZE(feature_word_info); w++) {
I prefer to use FEATURE_WORDS instead of
ARRAY_SIZE(feature_word_info), for consistency.
I'm becoming more and more inclined to transform FeatureWordArray
into a bitmap. We have too many "for (w; w < FEATURE_WORDS;
w++)" loops in the code that could be simplified using bitmap
operations.
But this is independent from this patch.
> + if (cpu->filtered_features[w]) {
> + return true;
> + }
> + }
> +
> + return false;
> +}
> +
> +static void mark_unavailable_features(X86CPU *cpu, FeatureWord w, uint32_t mask,
> + const char *prefix)
> +{
> + CPUX86State *env = &cpu->env;
> FeatureWordInfo *f = &feature_word_info[w];
> int i;
> char *feat_word_str;
>
> + env->features[w] &= ~mask;
> + cpu->filtered_features[w] |= mask;
> +
> + if (!cpu->check_cpuid && !cpu->enforce_cpuid) {
> + return;
> + }
> +
> for (i = 0; i < 32; ++i) {
> if ((1UL << i) & mask) {
> feat_word_str = feature_word_description(f, i);
> - warn_report("%s doesn't support requested feature: %s%s%s [bit %d]",
> - accel_uses_host_cpuid() ? "host" : "TCG",
> + warn_report("%s: %s%s%s [bit %d]",
> + prefix,
> feat_word_str,
> f->feat_names[i] ? "." : "",
> f->feat_names[i] ? f->feat_names[i] : "", i);
This seems to undo commit 8ca30e8673af ("target-i386: Move
warning code outside x86_cpu_filter_features()").
Filtering and reporting is separate because
x86_cpu_filter_features() is also called from a QMP command
handler that is not supposed to generate any warnings on stderr
(query-cpu-model-expansion).
> @@ -3691,7 +3713,7 @@ static void x86_cpu_parse_featurestr(const char *typename, char *features,
> }
>
> static void x86_cpu_expand_features(X86CPU *cpu, Error **errp);
> -static int x86_cpu_filter_features(X86CPU *cpu);
> +static void x86_cpu_filter_features(X86CPU *cpu);
>
> /* Build a list with the name of all features on a feature word array */
> static void x86_cpu_list_feature_names(FeatureWordArray features,
> @@ -3923,15 +3945,6 @@ static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w,
> return r;
> }
>
> -static void x86_cpu_report_filtered_features(X86CPU *cpu)
> -{
> - FeatureWord w;
> -
> - for (w = 0; w < FEATURE_WORDS; w++) {
> - report_unavailable_features(w, cpu->filtered_features[w]);
> - }
> -}
> -
> static void x86_cpu_apply_props(X86CPU *cpu, PropValue *props)
> {
> PropValue *pv;
> @@ -5170,21 +5183,20 @@ out:
> *
> * Returns: 0 if all flags are supported by the host, non-zero otherwise.
> */
> -static int x86_cpu_filter_features(X86CPU *cpu)
> +static void x86_cpu_filter_features(X86CPU *cpu)
> {
> CPUX86State *env = &cpu->env;
> FeatureWord w;
> - int rv = 0;
> + const char *prefix = accel_uses_host_cpuid()
> + ? "host doesn't support requested feature"
> + : "TCG doesn't support requested feature";
>
> for (w = 0; w < FEATURE_WORDS; w++) {
> uint32_t host_feat =
> x86_cpu_get_supported_feature_word(w, false);
> uint32_t requested_features = env->features[w];
> - env->features[w] &= host_feat;
> - cpu->filtered_features[w] = requested_features & ~env->features[w];
> - if (cpu->filtered_features[w]) {
> - rv = 1;
> - }
> + uint32_t unavailable_features = requested_features & ~host_feat;
> + mark_unavailable_features(cpu, w, unavailable_features, prefix);
> }
>
> if ((env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) &&
> @@ -5210,13 +5222,9 @@ static int x86_cpu_filter_features(X86CPU *cpu)
> * host can't emulate the capabilities we report on
> * cpu_x86_cpuid(), intel-pt can't be enabled on the current host.
> */
> - env->features[FEAT_7_0_EBX] &= ~CPUID_7_0_EBX_INTEL_PT;
> - cpu->filtered_features[FEAT_7_0_EBX] |= CPUID_7_0_EBX_INTEL_PT;
> - rv = 1;
> + mark_unavailable_features(cpu, FEAT_7_0_EBX, CPUID_7_0_EBX_INTEL_PT, prefix);
> }
> }
> -
> - return rv;
> }
>
> static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
> @@ -5257,16 +5265,14 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
> goto out;
> }
>
> - if (x86_cpu_filter_features(cpu) &&
> - (cpu->check_cpuid || cpu->enforce_cpuid)) {
> - x86_cpu_report_filtered_features(cpu);
> - if (cpu->enforce_cpuid) {
> - error_setg(&local_err,
> - accel_uses_host_cpuid() ?
> - "Host doesn't support requested features" :
> - "TCG doesn't support requested features");
> - goto out;
> - }
> + x86_cpu_filter_features(cpu);
> +
> + if (cpu->enforce_cpuid && x86_cpu_have_filtered_features(cpu)) {
> + error_setg(&local_err,
> + accel_uses_host_cpuid() ?
> + "Host doesn't support requested features" :
> + "TCG doesn't support requested features");
> + goto out;
> }
>
> /* On AMD CPUs, some CPUID[8000_0001].EDX bits must match the bits on
> --
> 1.8.3.1
>
>
>
--
Eduardo
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 2/7] target/i386: introduce generic feature dependency mechanism
2019-07-02 15:01 ` [Qemu-devel] [PATCH 2/7] target/i386: introduce generic feature dependency mechanism Paolo Bonzini
@ 2019-07-05 20:52 ` Eduardo Habkost
2019-07-05 21:12 ` Paolo Bonzini
0 siblings, 1 reply; 29+ messages in thread
From: Eduardo Habkost @ 2019-07-05 20:52 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Liran Alon, qemu-devel
On Tue, Jul 02, 2019 at 05:01:16PM +0200, Paolo Bonzini wrote:
> Sometimes a CPU feature does not make sense unless another is
> present. In the case of VMX features, KVM does not even allow
> setting the VMX controls to some invalid combinations.
>
> Therefore, this patch adds a generic mechanism that looks for bits
> that the user explicitly cleared, and uses them to remove other bits
> from the expanded CPU definition. If these dependent bits were also
> explicitly *set* by the user, this will be a warning for "-cpu check"
> and an error for "-cpu enforce". If not, then the dependent bits are
> cleared silently, for convenience.
>
> With VMX features, this will be used so that for example
> "-cpu host,-rdrand" will also hide support for RDRAND exiting.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> target/i386/cpu.c | 77 +++++++++++++++++++++++++++++++++++--------------------
> 1 file changed, 49 insertions(+), 28 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 9149d0d..412e834 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -799,10 +799,6 @@ typedef struct FeatureWordInfo {
> /* If type==MSR_FEATURE_WORD */
> struct {
> uint32_t index;
> - struct { /*CPUID that enumerate this MSR*/
> - FeatureWord cpuid_class;
> - uint32_t cpuid_flag;
> - } cpuid_dep;
> } msr;
> };
> uint32_t tcg_features; /* Feature flags supported by TCG */
> @@ -1197,10 +1193,6 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> },
> .msr = {
> .index = MSR_IA32_ARCH_CAPABILITIES,
> - .cpuid_dep = {
> - FEAT_7_0_EDX,
> - CPUID_7_0_EDX_ARCH_CAPABILITIES
> - }
> },
> },
> [FEAT_CORE_CAPABILITY] = {
> @@ -1217,14 +1209,26 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> },
> .msr = {
> .index = MSR_IA32_CORE_CAPABILITY,
> - .cpuid_dep = {
> - FEAT_7_0_EDX,
> - CPUID_7_0_EDX_CORE_CAPABILITY,
> - },
> },
> },
> };
>
> +typedef struct FeatureDep {
> + uint16_t from, to;
Why uint16_t and not FeatureWord?
> + uint64_t from_flag, to_flags;
There are other parts of the code that take a
FeatureWord/uint32_t pair (which will become uint64_t). I'd wrap
this into a typedef. I also miss documentation on the exact
meaning of those fields.
typedef struct FeatureMask {
FeatureWord w;
uint64_t mask;
};
typedef struct FeatureDependency {
/*
* Features in @to may be present only if _all_ features in @from
* present too.
*/
FeatureMask from, to;
};
static FeatureDep feature_dependencies[] = {
{
.from = { FEAT_7_0_EDX, CPUID_7_0_EDX_ARCH_CAPABILITIES
.to = { FEAT_ARCH_CAPABILITIES, ~0ull },
},
{
.from = { FEAT_7_0_EDX, CPUID_7_0_EDX_CORE_CAPABILITY },
.to = { FEAT_CORE_CAPABILITY, ~0ull },
},
};
> +} FeatureDep;
> +
> +static FeatureDep feature_dependencies[] = {
> + {
> + .from = FEAT_7_0_EDX, .from_flag = CPUID_7_0_EDX_ARCH_CAPABILITIES,
> + .to = FEAT_ARCH_CAPABILITIES, .to_flags = ~0ull,
> + },
> + {
> + .from = FEAT_7_0_EDX, .from_flag = CPUID_7_0_EDX_CORE_CAPABILITY,
> + .to = FEAT_CORE_CAPABILITY, .to_flags = ~0ull,
> + },
> +};
> +
> typedef struct X86RegisterInfo32 {
> /* Name of register */
> const char *name;
> @@ -5086,9 +5090,42 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
> {
> CPUX86State *env = &cpu->env;
> FeatureWord w;
> + int i;
> GList *l;
> Error *local_err = NULL;
>
> + for (l = plus_features; l; l = l->next) {
> + const char *prop = l->data;
> + object_property_set_bool(OBJECT(cpu), true, prop, &local_err);
> + if (local_err) {
> + goto out;
> + }
> + }
> +
> + for (l = minus_features; l; l = l->next) {
> + const char *prop = l->data;
> + object_property_set_bool(OBJECT(cpu), false, prop, &local_err);
> + if (local_err) {
> + goto out;
> + }
> + }
Maybe getting rid of plus_features/minus_features (as described
in the TODO comment below) will make things simpler.
> +
> + for (i = 0; i < ARRAY_SIZE(feature_dependencies); i++) {
> + FeatureDep *d = &feature_dependencies[i];
> + if ((env->user_features[d->from] & d->from_flag) &&
> + !(env->features[d->from] & d->from_flag)) {
Why does it matter if the feature was cleared explicitly by the
user?
> + uint64_t unavailable_features = env->features[d->to] & d->to_flags;
> +
> + /* Not an error unless the dependent feature was added explicitly. */
> + mark_unavailable_features(cpu, d->to, unavailable_features & env->user_features[d->to],
> + "This feature depends on other features that were not requested");
> +
> + /* Prevent adding the feature in the loop below. */
> + env->user_features[d->to] |= d->to_flags;
> + env->features[d->to] &= ~d->to_flags;
> + }
> + }
Maybe move this entire block inside x86_cpu_filter_features()?
> +
> /*TODO: Now cpu->max_features doesn't overwrite features
> * set using QOM properties, and we can convert
> * plus_features & minus_features to global properties
> @@ -5106,22 +5143,6 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
> }
> }
>
> - for (l = plus_features; l; l = l->next) {
> - const char *prop = l->data;
> - object_property_set_bool(OBJECT(cpu), true, prop, &local_err);
> - if (local_err) {
> - goto out;
> - }
> - }
> -
> - for (l = minus_features; l; l = l->next) {
> - const char *prop = l->data;
> - object_property_set_bool(OBJECT(cpu), false, prop, &local_err);
> - if (local_err) {
> - goto out;
> - }
> - }
> -
> if (!kvm_enabled() || !cpu->expose_kvm) {
> env->features[FEAT_KVM] = 0;
> }
> --
> 1.8.3.1
>
>
>
--
Eduardo
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 2/7] target/i386: introduce generic feature dependency mechanism
2019-07-05 20:52 ` Eduardo Habkost
@ 2019-07-05 21:12 ` Paolo Bonzini
2019-07-05 21:41 ` Eduardo Habkost
0 siblings, 1 reply; 29+ messages in thread
From: Paolo Bonzini @ 2019-07-05 21:12 UTC (permalink / raw)
To: Eduardo Habkost; +Cc: Liran Alon, qemu-devel
On 05/07/19 22:52, Eduardo Habkost wrote:
>> +typedef struct FeatureDep {
>> + uint16_t from, to;
>
> Why uint16_t and not FeatureWord?
Ok.
>> + uint64_t from_flag, to_flags;
>
> There are other parts of the code that take a
> FeatureWord/uint32_t pair (which will become uint64_t). I'd wrap
> this into a typedef. I also miss documentation on the exact
> meaning of those fields.
>
> typedef struct FeatureMask {
> FeatureWord w;
> uint64_t mask;
> };
Sounds good, I was optimizing the layout by putting small fields
together. Perhaps prematurely. :)
>> + for (l = plus_features; l; l = l->next) {
>> + const char *prop = l->data;
>> + object_property_set_bool(OBJECT(cpu), true, prop, &local_err);
>> + if (local_err) {
>> + goto out;
>> + }
>> + }
>> +
>> + for (l = minus_features; l; l = l->next) {
>> + const char *prop = l->data;
>> + object_property_set_bool(OBJECT(cpu), false, prop, &local_err);
>> + if (local_err) {
>> + goto out;
>> + }
>> + }
>
> Maybe getting rid of plus_features/minus_features (as described
> in the TODO comment below) will make things simpler.
This is just moving code. I can look at getting rid of plus_features
and minus_features but I was wary of the effects that global properties
have on query_cpu_model_expansion.
In any case, that would basically be rewriting "+foo" and "-foo" to
"foo=on" and "foo=off" respectively, right?
>> +
>> + for (i = 0; i < ARRAY_SIZE(feature_dependencies); i++) {
>> + FeatureDep *d = &feature_dependencies[i];
>> + if ((env->user_features[d->from] & d->from_flag) &&
>> + !(env->features[d->from] & d->from_flag)) {
>
> Why does it matter if the feature was cleared explicitly by the
> user?
Because the feature set of named CPU models should be internally
consistent. I thought of this mechanism as a quick "clean up user's
choices" pass to avoid having to remember a multitude of VMX features,
for example it makes "-cpu host,-rdtscp" just work.
>> + uint64_t unavailable_features = env->features[d->to] & d->to_flags;
>> +
>> + /* Not an error unless the dependent feature was added explicitly. */
>> + mark_unavailable_features(cpu, d->to, unavailable_features & env->user_features[d->to],
>> + "This feature depends on other features that were not requested");
>> +
>> + /* Prevent adding the feature in the loop below. */
>> + env->user_features[d->to] |= d->to_flags;
>> + env->features[d->to] &= ~d->to_flags;
>> + }
>> + }
>
> Maybe move this entire block inside x86_cpu_filter_features()?
It has to be done before expansion, so that env->user_features is set
properly before -cpu host is expanded.
Paolo
>> +
>> /*TODO: Now cpu->max_features doesn't overwrite features
>> * set using QOM properties, and we can convert
>> * plus_features & minus_features to global properties
>> @@ -5106,22 +5143,6 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
>> }
>> }
>>
>> - for (l = plus_features; l; l = l->next) {
>> - const char *prop = l->data;
>> - object_property_set_bool(OBJECT(cpu), true, prop, &local_err);
>> - if (local_err) {
>> - goto out;
>> - }
>> - }
>> -
>> - for (l = minus_features; l; l = l->next) {
>> - const char *prop = l->data;
>> - object_property_set_bool(OBJECT(cpu), false, prop, &local_err);
>> - if (local_err) {
>> - goto out;
>> - }
>> - }
>> -
>> if (!kvm_enabled() || !cpu->expose_kvm) {
>> env->features[FEAT_KVM] = 0;
>> }
>> --
>> 1.8.3.1
>>
>>
>>
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 6/7] target/i386: add VMX features
2019-07-02 15:01 ` [Qemu-devel] [PATCH 6/7] target/i386: add VMX features Paolo Bonzini
@ 2019-07-05 21:22 ` Eduardo Habkost
2019-07-05 22:12 ` Paolo Bonzini
0 siblings, 1 reply; 29+ messages in thread
From: Eduardo Habkost @ 2019-07-05 21:22 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Liran Alon, qemu-devel
On Tue, Jul 02, 2019 at 05:01:20PM +0200, Paolo Bonzini wrote:
> Add code to convert the VMX feature words back into MSR values,
> allowing the user to enable/disable VMX features as they wish. The same
> infrastructure enables support for limiting VMX features in named
> CPU models.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> target/i386/cpu.c | 221 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> target/i386/cpu.h | 9 +++
> target/i386/kvm.c | 154 ++++++++++++++++++++++++++++++++++++-
> 3 files changed, 382 insertions(+), 2 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 4de44e4..12f76a3 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -1211,6 +1211,163 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> .index = MSR_IA32_CORE_CAPABILITY,
> },
> },
> +
> + [FEAT_VMX_PROCBASED_CTLS] = {
> + .type = MSR_FEATURE_WORD,
> + .feat_names = {
> + NULL, NULL, "vmx-vintr-pending", "vmx-tsc-offset",
> + NULL, NULL, NULL, "vmx-hlt-exit",
> + NULL, "vmx-invlpg-exit", "vmx-mwait-exit", "vmx-rdpmc-exit",
> + "vmx-rdtsc-exit", NULL, NULL, "vmx-cr3-load-noexit",
> + "vmx-cr3-store-noexit", NULL, NULL, "vmx-cr8-load-exit",
> + "vmx-cr8-store-exit", "vmx-flexpriority", "vmx-vnmi-pending", "vmx-movdr-exit",
> + "vmx-io-exit", "vmx-io-bitmap", NULL, "vmx-mtf",
> + "vmx-msr-bitmap", "vmx-monitor-exit", "vmx-pause-exit", "vmx-secondary-ctls",
> + },
> + .msr = {
> + .index = MSR_IA32_VMX_TRUE_PROCBASED_CTLS,
> + }
> + },
> +
> + [FEAT_VMX_SECONDARY_CTLS] = {
> + .type = MSR_FEATURE_WORD,
> + .feat_names = {
> + "vmx-apicv-xapic", "vmx-ept", "vmx-desc-exit", "vmx-rdtscp-exit",
> + "vmx-apicv-x2apic", "vmx-vpid", "vmx-wbinvd-exit", "vmx-unrestricted-guest",
> + "vmx-apicv-register", "vmx-apicv-vid", "vmx-ple", "vmx-rdrand-exit",
> + "vmx-invpcid-exit", "vmx-vmfunc", "vmx-shadow-vmcs", "vmx-encls-exit",
> + "vmx-rdseed-exit", "vmx-pml", NULL, NULL,
> + "vmx-xsaves", NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + },
> + .msr = {
> + .index = MSR_IA32_VMX_PROCBASED_CTLS2,
> + }
> + },
> +
> + [FEAT_VMX_PINBASED_CTLS] = {
> + .type = MSR_FEATURE_WORD,
> + .feat_names = {
> + "vmx-intr-exit", NULL, NULL, "vmx-nmi-exit",
> + NULL, "vmx-vnmi", "vmx-preemption-timer", "vmx-posted-intr",
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + },
> + .msr = {
> + .index = MSR_IA32_VMX_TRUE_PINBASED_CTLS,
> + }
> + },
> +
> + [FEAT_VMX_EXIT_CTLS] = {
> + .type = MSR_FEATURE_WORD,
> + /*
> + * VMX_VM_EXIT_HOST_ADDR_SPACE_SIZE is copied from
> + * the LM CPUID bit.
> + */
> + .feat_names = {
> + NULL, NULL, "vmx-exit-nosave-debugctl", NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL /* vmx-exit-host-addr-space-size */, NULL, NULL,
> + "vmx-exit-load-perf-global-ctrl", NULL, NULL, "vmx-exit-ack-intr",
> + NULL, NULL, "vmx-exit-save-pat", "vmx-exit-load-pat",
> + "vmx-exit-save-efer", "vmx-exit-load-efer",
> + "vmx-exit-save-preemption-timer", "vmx-exit-clear-bndcfgs",
> + NULL, "vmx-exit-clear-rtit-ctl", NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + },
> + .msr = {
> + .index = MSR_IA32_VMX_TRUE_EXIT_CTLS,
> + }
> + },
> +
> + [FEAT_VMX_ENTRY_CTLS] = {
> + .type = MSR_FEATURE_WORD,
> + .feat_names = {
> + NULL, NULL, "vmx-entry-noload-debugctl", NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, "vmx-entry-ia32e-mode", NULL, NULL,
> + NULL, "vmx-entry-load-perf-global-ctrl", "vmx-entry-load-pat", "vmx-entry-load-efer",
> + "vmx-entry-load-bndcfgs", NULL, "vmx-entry-load-rtit-ctl", NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + },
> + .msr = {
> + .index = MSR_IA32_VMX_TRUE_ENTRY_CTLS,
> + }
> + },
> +
> + [FEAT_VMX_MISC] = {
> + .type = MSR_FEATURE_WORD,
> + .feat_names = {
> + NULL, NULL, NULL, NULL,
> + NULL, "vmx-store-lma", "vmx-activity-hlt", "vmx-activity-shutdown",
> + "vmx-activity-wait-sipi", NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, "vmx-vmwrite-vmexit-fields", "vmx-zero-len-inject", NULL,
> + },
> + .msr = {
> + .index = MSR_IA32_VMX_MISC,
> + }
> + },
> +
> + [FEAT_VMX_EPT_VPID_CAPS] = {
> + .type = MSR_FEATURE_WORD,
> + .feat_names = {
> + "vmx-ept-execonly", NULL, NULL, NULL,
> + NULL, NULL, "vmx-page-walk-4", "vmx-page-walk-5",
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + "vmx-ept-2mb", "vmx-ept-1gb", NULL, NULL,
> + "vmx-invept", "vmx-eptad", "vmx-ept-advanced-exitinfo", NULL,
> + NULL, "vmx-invept-single-context", "vmx-invept-all-context", NULL,
> + NULL, NULL, NULL, NULL,
> + "vmx-invvpid", NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + "vmx-invvpid-single-addr", "vmx-invept-single-context",
> + "vmx-invvpid-all-context", "vmx-invept-single-context-noglobals",
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL,
> + },
> + .msr = {
> + .index = MSR_IA32_VMX_EPT_VPID_CAP,
> + }
> + },
> +
> + [FEAT_VMX_BASIC] = {
> + .type = MSR_FEATURE_WORD,
> + .feat_names = {
> + [54] = "vmx-ins-outs",
> + [55] = "vmx-true-ctls",
> + },
> + .msr = {
> + .index = MSR_IA32_VMX_BASIC,
> + },
> + /* Just to be safe - we don't support setting the MSEG version field. */
> + .no_autoenable_flags = MSR_VMX_BASIC_DUAL_MONITOR,
> + },
> +
> + [FEAT_VMX_VMFUNC] = {
> + .type = MSR_FEATURE_WORD,
> + .feat_names = {
> + [0] = "vmx-eptp-switching",
> + },
> + .msr = {
> + .index = MSR_IA32_VMX_VMFUNC,
> + }
> + },
> +
> };
>
> typedef struct FeatureDep {
> @@ -1227,6 +1384,70 @@ static FeatureDep feature_dependencies[] = {
> .from = FEAT_7_0_EDX, .from_flag = CPUID_7_0_EDX_CORE_CAPABILITY,
> .to = FEAT_CORE_CAPABILITY, .to_flags = ~0ull,
> },
> + {
> + .from = FEAT_1_ECX, .from_flag = CPUID_EXT_VMX,
> + .to = FEAT_VMX_PROCBASED_CTLS, .to_flags = ~0ull,
> + },
> + {
> + .from = FEAT_1_ECX, .from_flag = CPUID_EXT_VMX,
> + .to = FEAT_VMX_PINBASED_CTLS, .to_flags = ~0ull,
> + },
> + {
> + .from = FEAT_1_ECX, .from_flag = CPUID_EXT_VMX,
> + .to = FEAT_VMX_EXIT_CTLS, .to_flags = ~0ull,
> + },
> + {
> + .from = FEAT_1_ECX, .from_flag = CPUID_EXT_VMX,
> + .to = FEAT_VMX_ENTRY_CTLS, .to_flags = ~0ull,
> + },
> + {
> + .from = FEAT_1_ECX, .from_flag = CPUID_EXT_VMX,
> + .to = FEAT_VMX_MISC, .to_flags = ~0ull,
> + },
> + {
> + .from = FEAT_1_ECX, .from_flag = CPUID_EXT_VMX,
> + .to = FEAT_VMX_BASIC, .to_flags = ~0ull,
> + },
> + {
> + .from = FEAT_8000_0001_EDX, .from_flag = CPUID_EXT2_LM,
> + .to = FEAT_VMX_ENTRY_CTLS, .to_flags = VMX_VM_ENTRY_IA32E_MODE,
> + },
> + {
> + .from = FEAT_VMX_PROCBASED_CTLS, .from_flag = VMX_CPU_BASED_ACTIVATE_SECONDARY_CONTROLS,
> + .to = FEAT_VMX_SECONDARY_CTLS, .to_flags = ~0ull,
> + },
> + {
> + .from = FEAT_XSAVE, .from_flag = CPUID_XSAVE_XSAVES,
> + .to = FEAT_VMX_SECONDARY_CTLS, .to_flags = VMX_SECONDARY_EXEC_XSAVES,
> + },
> + {
> + .from = FEAT_1_ECX, .from_flag = CPUID_EXT_RDRAND,
> + .to = FEAT_VMX_SECONDARY_CTLS, .to_flags = VMX_SECONDARY_EXEC_RDRAND_EXITING,
> + },
> + {
> + .from = FEAT_7_0_EBX, .from_flag = CPUID_7_0_EBX_INVPCID,
> + .to = FEAT_VMX_SECONDARY_CTLS, .to_flags = VMX_SECONDARY_EXEC_ENABLE_INVPCID,
> + },
> + {
> + .from = FEAT_7_0_EBX, .from_flag = CPUID_7_0_EBX_RDSEED,
> + .to = FEAT_VMX_SECONDARY_CTLS, .to_flags = VMX_SECONDARY_EXEC_RDSEED_EXITING,
> + },
> + {
> + .from = FEAT_8000_0001_EDX, .from_flag = CPUID_EXT2_RDTSCP,
> + .to = FEAT_VMX_SECONDARY_CTLS, .to_flags = VMX_SECONDARY_EXEC_RDTSCP,
> + },
> + {
> + .from = FEAT_VMX_SECONDARY_CTLS, .from_flag = VMX_SECONDARY_EXEC_ENABLE_EPT,
> + .to = FEAT_VMX_EPT_VPID_CAPS, .to_flags = 0xffffffffull,
> + },
> + {
> + .from = FEAT_VMX_SECONDARY_CTLS, .from_flag = VMX_SECONDARY_EXEC_ENABLE_VPID,
> + .to = FEAT_VMX_EPT_VPID_CAPS, .to_flags = 0xffffffffull << 32,
> + },
> + {
> + .from = FEAT_VMX_SECONDARY_CTLS, .from_flag = VMX_SECONDARY_EXEC_ENABLE_VMFUNC,
> + .to = FEAT_VMX_VMFUNC, .to_flags = ~0ull,
> + },
> };
>
> typedef struct X86RegisterInfo32 {
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index ec479d5..a5710c1 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -517,6 +517,15 @@ typedef enum FeatureWord {
> FEAT_XSAVE_COMP_HI, /* CPUID[EAX=0xd,ECX=0].EDX */
> FEAT_ARCH_CAPABILITIES,
> FEAT_CORE_CAPABILITY,
> + FEAT_VMX_PROCBASED_CTLS,
> + FEAT_VMX_SECONDARY_CTLS,
> + FEAT_VMX_PINBASED_CTLS,
> + FEAT_VMX_EXIT_CTLS,
> + FEAT_VMX_ENTRY_CTLS,
> + FEAT_VMX_MISC,
> + FEAT_VMX_EPT_VPID_CAPS,
> + FEAT_VMX_BASIC,
> + FEAT_VMX_VMFUNC,
> FEATURE_WORDS,
> } FeatureWord;
>
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 6801696..e35489c 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -96,6 +96,7 @@ static bool has_msr_virt_ssbd;
> static bool has_msr_smi_count;
> static bool has_msr_arch_capabs;
> static bool has_msr_core_capabs;
> +static bool has_msr_vmx_vmfunc;
>
> static uint32_t has_architectural_pmu_version;
> static uint32_t num_architectural_pmu_gp_counters;
> @@ -438,7 +439,8 @@ uint64_t kvm_arch_get_supported_msr_feature(KVMState *s, uint32_t index)
> struct kvm_msrs info;
> struct kvm_msr_entry entries[1];
> } msr_data;
> - uint32_t ret;
> + uint64_t value;
> + uint32_t ret, can_be_one, must_be_one;
>
> if (kvm_feature_msrs == NULL) { /* Host doesn't support feature MSRs */
> return 0;
> @@ -464,7 +466,25 @@ uint64_t kvm_arch_get_supported_msr_feature(KVMState *s, uint32_t index)
> exit(1);
> }
>
> - return msr_data.entries[0].data;
> + value = msr_data.entries[0].data;
> + switch (index) {
> + case MSR_IA32_VMX_PROCBASED_CTLS2:
> + case MSR_IA32_VMX_TRUE_PINBASED_CTLS:
> + case MSR_IA32_VMX_TRUE_PROCBASED_CTLS:
> + case MSR_IA32_VMX_TRUE_ENTRY_CTLS:
> + case MSR_IA32_VMX_TRUE_EXIT_CTLS:
> + /*
> + * Return true for bits that can be one, but do not have to be one.
> + * The SDM tells us which bits could have a "must be one" setting,
> + * so we can do the opposite transformation in make_vmx_msr_value.
> + */
> + must_be_one = (uint32_t)value;
> + can_be_one = (uint32_t)(value >> 32);
> + return can_be_one & ~must_be_one;
> +
> + default:
> + return value;
> + }
> }
>
>
> @@ -1933,6 +1953,9 @@ static int kvm_get_supported_msrs(KVMState *s)
> case MSR_IA32_CORE_CAPABILITY:
> has_msr_core_capabs = true;
> break;
> + case MSR_IA32_VMX_VMFUNC:
> + has_msr_vmx_vmfunc = true;
> + break;
> }
> }
> }
> @@ -2407,6 +2430,126 @@ static int kvm_put_msr_feature_control(X86CPU *cpu)
> return 0;
> }
>
> +static uint64_t make_vmx_msr_value(uint32_t index, uint32_t features)
> +{
> + uint32_t default1, can_be_one, can_be_zero;
> + uint32_t must_be_one;
> +
> + switch (index) {
> + case MSR_IA32_VMX_TRUE_PINBASED_CTLS:
> + default1 = 0x00000016;
> + break;
> + case MSR_IA32_VMX_TRUE_PROCBASED_CTLS:
> + default1 = 0x0401e172;
> + break;
> + case MSR_IA32_VMX_TRUE_ENTRY_CTLS:
> + default1 = 0x000011ff;
> + break;
> + case MSR_IA32_VMX_TRUE_EXIT_CTLS:
> + default1 = 0x00036dff;
> + break;
> + case MSR_IA32_VMX_PROCBASED_CTLS2:
> + default1 = 0;
> + break;
How do you plan to implement backwards compatibility if these
defaults ever change? Shouldn't these values be part of the CPU
model definitions so we can update them in the future?
> + default:
> + abort();
> + }
> +
> + /* If a feature bit is set, the control can be either set or clear.
> + * Otherwise the value is limited to either 0 or 1 by default1.
> + */
> + can_be_one = features | default1;
> + can_be_zero = features | ~default1;
> + must_be_one = ~can_be_zero;
> +
> + /*
> + * Bit 0:31 -> 0 if the control bit can be zero (i.e. 1 if it must be one).
> + * Bit 32:63 -> 1 if the control bit can be one.
> + */
> + return must_be_one | (((uint64_t)can_be_one) << 32);
> +}
> +
> +#define VMCS12_MAX_FIELD_INDEX (0x17)
> +
> +static void kvm_msr_entry_add_vmx(X86CPU *cpu, FeatureWordArray f)
> +{
> + uint64_t kvm_vmx_basic =
> + kvm_arch_get_supported_msr_feature(kvm_state,
> + MSR_IA32_VMX_BASIC);
> + uint64_t kvm_vmx_misc =
> + kvm_arch_get_supported_msr_feature(kvm_state,
> + MSR_IA32_VMX_MISC);
> + uint64_t kvm_vmx_ept_vpid =
> + kvm_arch_get_supported_msr_feature(kvm_state,
> + MSR_IA32_VMX_EPT_VPID_CAP);
If the MSR value we're exposing to the guest depends on
kvm_arch_get_supported_msr_feature(), how will we ensure this
will be safe for live migration?
If we really need to tweak the MSR values based on the host for
some reason (which is not clear to me yet), why don't we update
env->features[...] at x86_cpu_expand_features() to reflect what
the guest is really seeing?
> +
> + /*
> + * If the guest is 64-bit, a value of 1 is allowed for the host address
> + * space size vmexit control.
> + */
> + uint64_t fixed_vmx_exit = f[FEAT_8000_0001_EDX] & CPUID_EXT2_LM
> + ? (uint64_t)VMX_VM_EXIT_HOST_ADDR_SPACE_SIZE << 32 : 0;
> +
> + /*
> + * Bits 0-30, 32-44 and 50-53 come from the host. KVM should
> + * not change them for backwards compatibility.
Backwards compatibility with what?
Don't we want the MSR values to depend solely on the QEMU command
line in the future?
> + */
> + uint64_t fixed_vmx_basic = kvm_vmx_basic & 0x003c1fff7fffffffULL;
> +
> + /*
> + * Same for bits 0-4 and 25-27. Bits 16-24 (CR3 target count) can
> + * change in the future but are always zero for now, clear them to be
> + * future proof. Bits 32-63 in theory could change, though KVM does
> + * not support dual-monitor treatment and probably never will; mask
> + * them out as well.
> + */
> + uint64_t fixed_vmx_misc = kvm_vmx_misc & 0x0e00001f;
> +
> + /*
> + * EPT memory types should not change either, so we do not bother
> + * adding features for them.
> + */
> + uint64_t fixed_vmx_ept_mask =
> + (f[FEAT_VMX_SECONDARY_CTLS] & VMX_SECONDARY_EXEC_ENABLE_EPT ? 0x4100ull : 0);
> + uint64_t fixed_vmx_ept_vpid = kvm_vmx_ept_vpid & fixed_vmx_ept_mask;
> +
> + kvm_msr_entry_add(cpu, MSR_IA32_VMX_TRUE_PROCBASED_CTLS,
> + make_vmx_msr_value(MSR_IA32_VMX_TRUE_PROCBASED_CTLS,
> + f[FEAT_VMX_PROCBASED_CTLS]));
> + kvm_msr_entry_add(cpu, MSR_IA32_VMX_TRUE_PINBASED_CTLS,
> + make_vmx_msr_value(MSR_IA32_VMX_TRUE_PINBASED_CTLS,
> + f[FEAT_VMX_PINBASED_CTLS]));
> + kvm_msr_entry_add(cpu, MSR_IA32_VMX_TRUE_EXIT_CTLS,
> + make_vmx_msr_value(MSR_IA32_VMX_TRUE_EXIT_CTLS,
> + f[FEAT_VMX_EXIT_CTLS]) | fixed_vmx_exit);
> + kvm_msr_entry_add(cpu, MSR_IA32_VMX_TRUE_ENTRY_CTLS,
> + make_vmx_msr_value(MSR_IA32_VMX_TRUE_ENTRY_CTLS,
> + f[FEAT_VMX_ENTRY_CTLS]));
> + kvm_msr_entry_add(cpu, MSR_IA32_VMX_PROCBASED_CTLS2,
> + make_vmx_msr_value(MSR_IA32_VMX_PROCBASED_CTLS2,
> + f[FEAT_VMX_SECONDARY_CTLS]));
> + kvm_msr_entry_add(cpu, MSR_IA32_VMX_EPT_VPID_CAP,
> + f[FEAT_VMX_EPT_VPID_CAPS] | fixed_vmx_ept_vpid);
> + kvm_msr_entry_add(cpu, MSR_IA32_VMX_BASIC,
> + f[FEAT_VMX_BASIC] | fixed_vmx_basic);
> + kvm_msr_entry_add(cpu, MSR_IA32_VMX_MISC,
> + f[FEAT_VMX_MISC] | fixed_vmx_misc);
> + if (has_msr_vmx_vmfunc) {
> + kvm_msr_entry_add(cpu, MSR_IA32_VMX_VMFUNC, f[FEAT_VMX_VMFUNC]);
> + }
> +
> + /*
> + * Just to be safe, write these with constant values. The CRn_FIXED1
> + * MSRs are generated by KVM based on the vCPU's CPUID.
> + */
> + kvm_msr_entry_add(cpu, MSR_IA32_VMX_CR0_FIXED0,
> + CR0_PE_MASK | CR0_PG_MASK | CR0_NE_MASK);
> + kvm_msr_entry_add(cpu, MSR_IA32_VMX_CR4_FIXED0,
> + CR4_VMXE_MASK);
> + kvm_msr_entry_add(cpu, MSR_IA32_VMX_VMCS_ENUM,
> + VMCS12_MAX_FIELD_INDEX << 1);
> +}
> +
> static int kvm_put_msrs(X86CPU *cpu, int level)
> {
> CPUX86State *env = &cpu->env;
> @@ -2659,6 +2802,13 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
> kvm_msr_entry_add(cpu, MSR_MC0_CTL + i, env->mce_banks[i]);
> }
> }
> + /*
> + * Older kernels do not include VMX MSRs in KVM_GET_MSR_INDEX_LIST, but
> + * all kernels with MSR features should have them.
> + */
> + if (kvm_feature_msrs && cpu_has_vmx(env)) {
> + kvm_msr_entry_add_vmx(cpu, env->features);
> + }
>
> ret = kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MSRS, cpu->kvm_msr_buf);
> if (ret < 0) {
> --
> 1.8.3.1
>
>
>
--
Eduardo
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 1/7] target/i386: handle filtered_features in a new function mark_unavailable_features
2019-07-05 20:37 ` Eduardo Habkost
@ 2019-07-05 21:32 ` Paolo Bonzini
2019-07-05 21:44 ` Eduardo Habkost
0 siblings, 1 reply; 29+ messages in thread
From: Paolo Bonzini @ 2019-07-05 21:32 UTC (permalink / raw)
To: Eduardo Habkost; +Cc: Liran Alon, qemu-devel
On 05/07/19 22:37, Eduardo Habkost wrote:
> Filtering and reporting is separate because
> x86_cpu_filter_features() is also called from a QMP command
> handler that is not supposed to generate any warnings on stderr
> (query-cpu-model-expansion).
But that one should not set check_cpuid or enforce_cpuid, should it?
(I can still split the filtering and reporting if you prefer).
Paolo
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 2/7] target/i386: introduce generic feature dependency mechanism
2019-07-05 21:12 ` Paolo Bonzini
@ 2019-07-05 21:41 ` Eduardo Habkost
2019-07-05 22:07 ` Paolo Bonzini
0 siblings, 1 reply; 29+ messages in thread
From: Eduardo Habkost @ 2019-07-05 21:41 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Liran Alon, qemu-devel
On Fri, Jul 05, 2019 at 11:12:11PM +0200, Paolo Bonzini wrote:
> On 05/07/19 22:52, Eduardo Habkost wrote:
> >> +typedef struct FeatureDep {
> >> + uint16_t from, to;
> >
> > Why uint16_t and not FeatureWord?
>
> Ok.
>
> >> + uint64_t from_flag, to_flags;
> >
> > There are other parts of the code that take a
> > FeatureWord/uint32_t pair (which will become uint64_t). I'd wrap
> > this into a typedef. I also miss documentation on the exact
> > meaning of those fields.
> >
> > typedef struct FeatureMask {
> > FeatureWord w;
> > uint64_t mask;
> > };
>
> Sounds good, I was optimizing the layout by putting small fields
> together. Perhaps prematurely. :)
>
> >> + for (l = plus_features; l; l = l->next) {
> >> + const char *prop = l->data;
> >> + object_property_set_bool(OBJECT(cpu), true, prop, &local_err);
> >> + if (local_err) {
> >> + goto out;
> >> + }
> >> + }
> >> +
> >> + for (l = minus_features; l; l = l->next) {
> >> + const char *prop = l->data;
> >> + object_property_set_bool(OBJECT(cpu), false, prop, &local_err);
> >> + if (local_err) {
> >> + goto out;
> >> + }
> >> + }
> >
> > Maybe getting rid of plus_features/minus_features (as described
> > in the TODO comment below) will make things simpler.
>
> This is just moving code. I can look at getting rid of plus_features
> and minus_features but I was wary of the effects that global properties
> have on query_cpu_model_expansion.
Shouldn't be a problem, as query-cpu-model-expansion
documentation already advises against using "-cpu" when calling
it.
>
> In any case, that would basically be rewriting "+foo" and "-foo" to
> "foo=on" and "foo=off" respectively, right?
I don't mean changing the command line interface, but just
changing the implementation of "+foo" and "-foo".
In theory the code was already fixed to make this safe, but I
agree this might be tricky. Let's worry about
plus_features/minus_features later.
>
> >> +
> >> + for (i = 0; i < ARRAY_SIZE(feature_dependencies); i++) {
> >> + FeatureDep *d = &feature_dependencies[i];
> >> + if ((env->user_features[d->from] & d->from_flag) &&
> >> + !(env->features[d->from] & d->from_flag)) {
> >
> > Why does it matter if the feature was cleared explicitly by the
> > user?
>
> Because the feature set of named CPU models should be internally
> consistent. I thought of this mechanism as a quick "clean up user's
> choices" pass to avoid having to remember a multitude of VMX features,
> for example it makes "-cpu host,-rdtscp" just work.
If named CPU models are already consistent, ignoring
user_features shouldn't make a difference, right? It would also
be a useful mechanism to detect inconsistencies in internal CPU
model definitions.
I don't understand why the user_features check would be necessary
to make "-cpu host,-rdtscp" work.
>
> >> + uint64_t unavailable_features = env->features[d->to] & d->to_flags;
> >> +
> >> + /* Not an error unless the dependent feature was added explicitly. */
> >> + mark_unavailable_features(cpu, d->to, unavailable_features & env->user_features[d->to],
> >> + "This feature depends on other features that were not requested");
> >> +
> >> + /* Prevent adding the feature in the loop below. */
> >> + env->user_features[d->to] |= d->to_flags;
> >> + env->features[d->to] &= ~d->to_flags;
> >> + }
> >> + }
> >
> > Maybe move this entire block inside x86_cpu_filter_features()?
>
> It has to be done before expansion, so that env->user_features is set
> properly before -cpu host is expanded.
I don't get it. It looks like you only need env->user_features
to be set above because you are handling dependencies before
cpu->max_features is handled.
If you handle dependencies at x86_cpu_filter_features() instead
(after cpu->max_features was already handled), you don't even
need to worry about setting user_features.
>
> Paolo
>
> >> +
> >> /*TODO: Now cpu->max_features doesn't overwrite features
> >> * set using QOM properties, and we can convert
> >> * plus_features & minus_features to global properties
> >> @@ -5106,22 +5143,6 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
> >> }
> >> }
> >>
> >> - for (l = plus_features; l; l = l->next) {
> >> - const char *prop = l->data;
> >> - object_property_set_bool(OBJECT(cpu), true, prop, &local_err);
> >> - if (local_err) {
> >> - goto out;
> >> - }
> >> - }
> >> -
> >> - for (l = minus_features; l; l = l->next) {
> >> - const char *prop = l->data;
> >> - object_property_set_bool(OBJECT(cpu), false, prop, &local_err);
> >> - if (local_err) {
> >> - goto out;
> >> - }
> >> - }
> >> -
> >> if (!kvm_enabled() || !cpu->expose_kvm) {
> >> env->features[FEAT_KVM] = 0;
> >> }
> >> --
> >> 1.8.3.1
> >>
> >>
> >>
> >
>
--
Eduardo
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 1/7] target/i386: handle filtered_features in a new function mark_unavailable_features
2019-07-05 21:32 ` Paolo Bonzini
@ 2019-07-05 21:44 ` Eduardo Habkost
2019-07-05 22:07 ` Paolo Bonzini
0 siblings, 1 reply; 29+ messages in thread
From: Eduardo Habkost @ 2019-07-05 21:44 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Liran Alon, qemu-devel
On Fri, Jul 05, 2019 at 11:32:07PM +0200, Paolo Bonzini wrote:
> On 05/07/19 22:37, Eduardo Habkost wrote:
> > Filtering and reporting is separate because
> > x86_cpu_filter_features() is also called from a QMP command
> > handler that is not supposed to generate any warnings on stderr
> > (query-cpu-model-expansion).
>
> But that one should not set check_cpuid or enforce_cpuid, should it?
check_cpuid is set to true by default.
>
> (I can still split the filtering and reporting if you prefer).
Maybe it will work if we just add a 'bool verbose' parameter to
x86_cpu_filter_features().
x86_cpu_realizefn() would call:
x86_cpu_filter_features(cpu, cpu->check_cpuid);
x86_cpu_class_check_missing_features() would call:
x86_cpu_filter_features(cpu, false);
--
Eduardo
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 1/7] target/i386: handle filtered_features in a new function mark_unavailable_features
2019-07-05 21:44 ` Eduardo Habkost
@ 2019-07-05 22:07 ` Paolo Bonzini
2019-07-05 22:16 ` Eduardo Habkost
0 siblings, 1 reply; 29+ messages in thread
From: Paolo Bonzini @ 2019-07-05 22:07 UTC (permalink / raw)
To: Eduardo Habkost; +Cc: Liran Alon, qemu-devel
On 05/07/19 23:44, Eduardo Habkost wrote:
> On Fri, Jul 05, 2019 at 11:32:07PM +0200, Paolo Bonzini wrote:
>> On 05/07/19 22:37, Eduardo Habkost wrote:
>>> Filtering and reporting is separate because
>>> x86_cpu_filter_features() is also called from a QMP command
>>> handler that is not supposed to generate any warnings on stderr
>>> (query-cpu-model-expansion).
>>
>> But that one should not set check_cpuid or enforce_cpuid, should it?
>
> check_cpuid is set to true by default.
Ok, that's what I missed.
>>
>> (I can still split the filtering and reporting if you prefer).
>
> Maybe it will work if we just add a 'bool verbose' parameter to
> x86_cpu_filter_features().
>
> x86_cpu_realizefn() would call:
> x86_cpu_filter_features(cpu, cpu->check_cpuid);
... "|| cpu->enforce_cpuid".
> x86_cpu_class_check_missing_features() would call:
> x86_cpu_filter_features(cpu, false);
Or set check_cpuid to false there after creating the object?
Paolo
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 2/7] target/i386: introduce generic feature dependency mechanism
2019-07-05 21:41 ` Eduardo Habkost
@ 2019-07-05 22:07 ` Paolo Bonzini
2019-07-08 21:45 ` Eduardo Habkost
0 siblings, 1 reply; 29+ messages in thread
From: Paolo Bonzini @ 2019-07-05 22:07 UTC (permalink / raw)
To: Eduardo Habkost; +Cc: Liran Alon, qemu-devel
On 05/07/19 23:41, Eduardo Habkost wrote:
>>>> + for (i = 0; i < ARRAY_SIZE(feature_dependencies); i++) {
>>>> + FeatureDep *d = &feature_dependencies[i];
>>>> + if ((env->user_features[d->from] & d->from_flag) &&
>>>> + !(env->features[d->from] & d->from_flag)) {
>>> Why does it matter if the feature was cleared explicitly by the
>>> user?
>> Because the feature set of named CPU models should be internally
>> consistent. I thought of this mechanism as a quick "clean up user's
>> choices" pass to avoid having to remember a multitude of VMX features,
>> for example it makes "-cpu host,-rdtscp" just work.
> If named CPU models are already consistent, ignoring
> user_features shouldn't make a difference, right? It would also
> be a useful mechanism to detect inconsistencies in internal CPU
> model definitions.
Ok, I can drop that check.
>> It has to be done before expansion, so that env->user_features is set
>> properly before -cpu host is expanded.
>
> I don't get it. It looks like you only need env->user_features
> to be set above because you are handling dependencies before
> cpu->max_features is handled.
>
> If you handle dependencies at x86_cpu_filter_features() instead
> (after cpu->max_features was already handled), you don't even
> need to worry about setting user_features.
I think you're right, but on the other hand setting user_features is
cleaner. Effectively the dependent features have been disabled because
of something the user told QEMU. So on one hand I can move the loop to
x86_cpu_filter_features, on the other hand I'd prefer to set
user_features and then it feels more like expansion (e.g. of vmx-ept=off
to vmx-ept=off,vmx-unrestricted-guest=off) than filtering.
Paolo
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 6/7] target/i386: add VMX features
2019-07-05 21:22 ` Eduardo Habkost
@ 2019-07-05 22:12 ` Paolo Bonzini
2019-07-05 22:33 ` Eduardo Habkost
0 siblings, 1 reply; 29+ messages in thread
From: Paolo Bonzini @ 2019-07-05 22:12 UTC (permalink / raw)
To: Eduardo Habkost; +Cc: Liran Alon, qemu-devel
On 05/07/19 23:22, Eduardo Habkost wrote:
>> + switch (index) {
>> + case MSR_IA32_VMX_TRUE_PINBASED_CTLS:
>> + default1 = 0x00000016;
>> + break;
>> + case MSR_IA32_VMX_TRUE_PROCBASED_CTLS:
>> + default1 = 0x0401e172;
>> + break;
>> + case MSR_IA32_VMX_TRUE_ENTRY_CTLS:
>> + default1 = 0x000011ff;
>> + break;
>> + case MSR_IA32_VMX_TRUE_EXIT_CTLS:
>> + default1 = 0x00036dff;
>> + break;
>> + case MSR_IA32_VMX_PROCBASED_CTLS2:
>> + default1 = 0;
>> + break;
> How do you plan to implement backwards compatibility if these
> defaults ever change? Shouldn't these values be part of the CPU
> model definitions so we can update them in the future?
These are not defaults, they are "default-1 bits": if a feature is
disabled, these bits are 1 in both halves of the MSR rather than zero.
The set of default-1 bits is documented and is not going to change in
the future.
Some default-1 bits *could* however become features in the future, and
four of these already have features associated to them:
vmx-cr3-load-noexit, vmx-cr3-store-noexit, vmx-exit-nosave-debugctl,
vmx-entry-noload-debugctl. You can see that they have "no" in their
name because the feature is about the ability to "do less" rather than
"do more".
>> + uint64_t kvm_vmx_basic =
>> + kvm_arch_get_supported_msr_feature(kvm_state,
>> + MSR_IA32_VMX_BASIC);
>> + uint64_t kvm_vmx_misc =
>> + kvm_arch_get_supported_msr_feature(kvm_state,
>> + MSR_IA32_VMX_MISC);
>> + uint64_t kvm_vmx_ept_vpid =
>> + kvm_arch_get_supported_msr_feature(kvm_state,
>> + MSR_IA32_VMX_EPT_VPID_CAP);
>
> If the MSR value we're exposing to the guest depends on
> kvm_arch_get_supported_msr_feature(), how will we ensure this
> will be safe for live migration?
Because KVM guarantees that this part of the guest ABI will never
change. These values do not come from the host values of the MSRs, they
are fixed by KVM. More details below.
> If we really need to tweak the MSR values based on the host for
> some reason (which is not clear to me yet), why don't we update
> env->features[...] at x86_cpu_expand_features() to reflect what
> the guest is really seeing?
>
>
>> + /*
>> + * Bits 0-30, 32-44 and 50-53 come from the host. KVM should
>> + * not change them for backwards compatibility.
>
> Backwards compatibility with what?
>
> Don't we want the MSR values to depend solely on the QEMU command
> line in the future?
These bits are: VMCS revision, VMCS size and VMCS memory type. QEMU
cannot know them, as they depend on the internal implementation details
of KVM.
Now that KVM supports nested virt live migration they cannot change
anymore---otherwise KVM would break KVM live migration compatibility.
However, theoretically in the future KVM could add some capability
(which userspace would have to manually enable) and when the capability
is enabled the values can change.
> + /*
> + * Same for bits 0-4 and 25-27. Bits 16-24 (CR3 target count) can
> + * change in the future but are always zero for now, clear them to be
> + * future proof. Bits 32-63 in theory could change, though KVM does
> + * not support dual-monitor treatment and probably never will; mask
> + * them out as well.
> + */
The reasoning is more or less the same here. These bits are part of the
guest ABI (preemption timer scaling, CR3 target count, MSR count, MSEG
revision). Right now bits 0-4 are 5 and the others are 0; in the future:
- KVM cannot change bits 0-4 and 32-63 them without breaking guest ABI
(the values must match between what you read and what you set)
- KVM could change bits 16-24, but it always allows writing a value that
is _smaller_ than the one you read. So I'm zeroing those, ensuring no
future ABI changes.
- KVM could in theory change bits 25-27: here it also allows writing a
value that is smaller than the one you read, so guest ABI is preserved.
Such a change is very unlikely, all Intel silicon has always had 0
here. But I can change the code to zero these three bits just like bits
16-24.
Paolo
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 1/7] target/i386: handle filtered_features in a new function mark_unavailable_features
2019-07-05 22:07 ` Paolo Bonzini
@ 2019-07-05 22:16 ` Eduardo Habkost
0 siblings, 0 replies; 29+ messages in thread
From: Eduardo Habkost @ 2019-07-05 22:16 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Liran Alon, qemu-devel
On Sat, Jul 06, 2019 at 12:07:29AM +0200, Paolo Bonzini wrote:
> On 05/07/19 23:44, Eduardo Habkost wrote:
> > On Fri, Jul 05, 2019 at 11:32:07PM +0200, Paolo Bonzini wrote:
> >> On 05/07/19 22:37, Eduardo Habkost wrote:
> >>> Filtering and reporting is separate because
> >>> x86_cpu_filter_features() is also called from a QMP command
> >>> handler that is not supposed to generate any warnings on stderr
> >>> (query-cpu-model-expansion).
> >>
> >> But that one should not set check_cpuid or enforce_cpuid, should it?
> >
> > check_cpuid is set to true by default.
>
> Ok, that's what I missed.
>
> >>
> >> (I can still split the filtering and reporting if you prefer).
> >
> > Maybe it will work if we just add a 'bool verbose' parameter to
> > x86_cpu_filter_features().
> >
> > x86_cpu_realizefn() would call:
> > x86_cpu_filter_features(cpu, cpu->check_cpuid);
>
> ... "|| cpu->enforce_cpuid".
>
> > x86_cpu_class_check_missing_features() would call:
> > x86_cpu_filter_features(cpu, false);
>
> Or set check_cpuid to false there after creating the object?
It would work too, but I prefer to make the side effects of
x86_cpu_filter_features() more explicit.
--
Eduardo
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 6/7] target/i386: add VMX features
2019-07-05 22:12 ` Paolo Bonzini
@ 2019-07-05 22:33 ` Eduardo Habkost
2019-07-05 22:42 ` Paolo Bonzini
0 siblings, 1 reply; 29+ messages in thread
From: Eduardo Habkost @ 2019-07-05 22:33 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Liran Alon, qemu-devel
On Sat, Jul 06, 2019 at 12:12:49AM +0200, Paolo Bonzini wrote:
> On 05/07/19 23:22, Eduardo Habkost wrote:
> >> + switch (index) {
> >> + case MSR_IA32_VMX_TRUE_PINBASED_CTLS:
> >> + default1 = 0x00000016;
> >> + break;
> >> + case MSR_IA32_VMX_TRUE_PROCBASED_CTLS:
> >> + default1 = 0x0401e172;
> >> + break;
> >> + case MSR_IA32_VMX_TRUE_ENTRY_CTLS:
> >> + default1 = 0x000011ff;
> >> + break;
> >> + case MSR_IA32_VMX_TRUE_EXIT_CTLS:
> >> + default1 = 0x00036dff;
> >> + break;
> >> + case MSR_IA32_VMX_PROCBASED_CTLS2:
> >> + default1 = 0;
> >> + break;
> > How do you plan to implement backwards compatibility if these
> > defaults ever change? Shouldn't these values be part of the CPU
> > model definitions so we can update them in the future?
>
> These are not defaults, they are "default-1 bits": if a feature is
> disabled, these bits are 1 in both halves of the MSR rather than zero.
> The set of default-1 bits is documented and is not going to change in
> the future.
>
> Some default-1 bits *could* however become features in the future, and
> four of these already have features associated to them:
> vmx-cr3-load-noexit, vmx-cr3-store-noexit, vmx-exit-nosave-debugctl,
> vmx-entry-noload-debugctl. You can see that they have "no" in their
> name because the feature is about the ability to "do less" rather than
> "do more".
Understood. Thanks!
>
> >> + uint64_t kvm_vmx_basic =
> >> + kvm_arch_get_supported_msr_feature(kvm_state,
> >> + MSR_IA32_VMX_BASIC);
> >> + uint64_t kvm_vmx_misc =
> >> + kvm_arch_get_supported_msr_feature(kvm_state,
> >> + MSR_IA32_VMX_MISC);
> >> + uint64_t kvm_vmx_ept_vpid =
> >> + kvm_arch_get_supported_msr_feature(kvm_state,
> >> + MSR_IA32_VMX_EPT_VPID_CAP);
> >
> > If the MSR value we're exposing to the guest depends on
> > kvm_arch_get_supported_msr_feature(), how will we ensure this
> > will be safe for live migration?
>
> Because KVM guarantees that this part of the guest ABI will never
> change. These values do not come from the host values of the MSRs, they
> are fixed by KVM. More details below.
>
> > If we really need to tweak the MSR values based on the host for
> > some reason (which is not clear to me yet), why don't we update
> > env->features[...] at x86_cpu_expand_features() to reflect what
> > the guest is really seeing?
> >
> >
> >> + /*
> >> + * Bits 0-30, 32-44 and 50-53 come from the host. KVM should
> >> + * not change them for backwards compatibility.
> >
> > Backwards compatibility with what?
> >
> > Don't we want the MSR values to depend solely on the QEMU command
> > line in the future?
>
> These bits are: VMCS revision, VMCS size and VMCS memory type. QEMU
> cannot know them, as they depend on the internal implementation details
> of KVM.
>
> Now that KVM supports nested virt live migration they cannot change
> anymore---otherwise KVM would break KVM live migration compatibility.
> However, theoretically in the future KVM could add some capability
> (which userspace would have to manually enable) and when the capability
> is enabled the values can change.
Oh, that's the info I was missing. I always expected
kvm_arch_get_supported_*() to be subject to change (depending on
KVM and hardware capabilities), and not be part of guest ABI.
Now, if KVM is going to to implement the guest ABI guarantee at
KVM_GET_MSRS, that's OK. Is this going to be obvious to people
touching KVM_GET_MSRS in the future?
What if we do want the guest ABI to change in the future? How do
you expect QEMU to ask KVM to enable the new guest ABI? How do
you expect the user to ask QEMU to enable the new guest ABI?
>
> > + /*
> > + * Same for bits 0-4 and 25-27. Bits 16-24 (CR3 target count) can
> > + * change in the future but are always zero for now, clear them to be
> > + * future proof. Bits 32-63 in theory could change, though KVM does
> > + * not support dual-monitor treatment and probably never will; mask
> > + * them out as well.
> > + */
>
> The reasoning is more or less the same here. These bits are part of the
> guest ABI (preemption timer scaling, CR3 target count, MSR count, MSEG
> revision). Right now bits 0-4 are 5 and the others are 0; in the future:
>
> - KVM cannot change bits 0-4 and 32-63 them without breaking guest ABI
> (the values must match between what you read and what you set)
>
> - KVM could change bits 16-24, but it always allows writing a value that
> is _smaller_ than the one you read. So I'm zeroing those, ensuring no
> future ABI changes.
>
> - KVM could in theory change bits 25-27: here it also allows writing a
> value that is smaller than the one you read, so guest ABI is preserved.
> Such a change is very unlikely, all Intel silicon has always had 0
> here. But I can change the code to zero these three bits just like bits
> 16-24.
The complex rules above make me a bit nervous. Can we at least
make QEMU validate the values returned by
kvm_arch_get_supported_msr_feature() to catch ABI-breaking
mistakes in the future?
--
Eduardo
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 6/7] target/i386: add VMX features
2019-07-05 22:33 ` Eduardo Habkost
@ 2019-07-05 22:42 ` Paolo Bonzini
2019-07-05 22:48 ` Eduardo Habkost
0 siblings, 1 reply; 29+ messages in thread
From: Paolo Bonzini @ 2019-07-05 22:42 UTC (permalink / raw)
To: Eduardo Habkost; +Cc: Liran Alon, qemu-devel
On 06/07/19 00:33, Eduardo Habkost wrote:
> Oh, that's the info I was missing. I always expected
> kvm_arch_get_supported_*() to be subject to change (depending on
> KVM and hardware capabilities), and not be part of guest ABI.
For most bits that's true. Just not for these ones, because they are
integer values rather than bit flags.
The reason for the complex rules is that you need to know what is a
flag, what is a fixed value that the guest uses, and what is a maximum
supported value. Simpler userspace than QEMU can just use the defaults
since they don't care about maintaining the guest ABI.
> Now, if KVM is going to to implement the guest ABI guarantee at
> KVM_GET_MSRS, that's OK. Is this going to be obvious to people
> touching KVM_GET_MSRS in the future?
>
> What if we do want the guest ABI to change in the future? How do
> you expect QEMU to ask KVM to enable the new guest ABI? How do
> you expect the user to ask QEMU to enable the new guest ABI?
That would be with ioctl(KVM_ENABLE_CAP) for KVM, and with -cpu for QEMU.
>> - KVM could change bits 16-24, but it always allows writing a value that
>> is _smaller_ than the one you read. So I'm zeroing those, ensuring no
>> future ABI changes.
>>
>> - KVM could in theory change bits 25-27: here it also allows writing a
>> value that is smaller than the one you read, so guest ABI is preserved.
>> Such a change is very unlikely, all Intel silicon has always had 0
>> here. But I can change the code to zero these three bits just like bits
>> 16-24.
>
> The complex rules above make me a bit nervous. Can we at least
> make QEMU validate the values returned by
> kvm_arch_get_supported_msr_feature() to catch ABI-breaking
> mistakes in the future?
I don't know... I'm a bit wary of adding hard-coded values in QEMU,
userspace simply should not care. But I can add comments to KVM to
remind people of values that should not be changed.
Paolo
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 6/7] target/i386: add VMX features
2019-07-05 22:42 ` Paolo Bonzini
@ 2019-07-05 22:48 ` Eduardo Habkost
0 siblings, 0 replies; 29+ messages in thread
From: Eduardo Habkost @ 2019-07-05 22:48 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Liran Alon, qemu-devel
On Sat, Jul 06, 2019 at 12:42:22AM +0200, Paolo Bonzini wrote:
> On 06/07/19 00:33, Eduardo Habkost wrote:
> > Oh, that's the info I was missing. I always expected
> > kvm_arch_get_supported_*() to be subject to change (depending on
> > KVM and hardware capabilities), and not be part of guest ABI.
>
> For most bits that's true. Just not for these ones, because they are
> integer values rather than bit flags.
>
> The reason for the complex rules is that you need to know what is a
> flag, what is a fixed value that the guest uses, and what is a maximum
> supported value. Simpler userspace than QEMU can just use the defaults
> since they don't care about maintaining the guest ABI.
>
> > Now, if KVM is going to to implement the guest ABI guarantee at
> > KVM_GET_MSRS, that's OK. Is this going to be obvious to people
> > touching KVM_GET_MSRS in the future?
> >
> > What if we do want the guest ABI to change in the future? How do
> > you expect QEMU to ask KVM to enable the new guest ABI? How do
> > you expect the user to ask QEMU to enable the new guest ABI?
>
> That would be with ioctl(KVM_ENABLE_CAP) for KVM, and with -cpu for QEMU.
Makes sense to me.
>
> >> - KVM could change bits 16-24, but it always allows writing a value that
> >> is _smaller_ than the one you read. So I'm zeroing those, ensuring no
> >> future ABI changes.
> >>
> >> - KVM could in theory change bits 25-27: here it also allows writing a
> >> value that is smaller than the one you read, so guest ABI is preserved.
> >> Such a change is very unlikely, all Intel silicon has always had 0
> >> here. But I can change the code to zero these three bits just like bits
> >> 16-24.
> >
> > The complex rules above make me a bit nervous. Can we at least
> > make QEMU validate the values returned by
> > kvm_arch_get_supported_msr_feature() to catch ABI-breaking
> > mistakes in the future?
>
> I don't know... I'm a bit wary of adding hard-coded values in QEMU,
> userspace simply should not care. But I can add comments to KVM to
> remind people of values that should not be changed.
Sounds good to me. If we're worried about breaking guest ABI by
accident, we can include the MSRs in the guest ABI validation
test cases I'm working on.
--
Eduardo
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH 2/7] target/i386: introduce generic feature dependency mechanism
2019-07-05 22:07 ` Paolo Bonzini
@ 2019-07-08 21:45 ` Eduardo Habkost
0 siblings, 0 replies; 29+ messages in thread
From: Eduardo Habkost @ 2019-07-08 21:45 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Liran Alon, qemu-devel
On Sat, Jul 06, 2019 at 12:07:50AM +0200, Paolo Bonzini wrote:
> On 05/07/19 23:41, Eduardo Habkost wrote:
> >>>> + for (i = 0; i < ARRAY_SIZE(feature_dependencies); i++) {
> >>>> + FeatureDep *d = &feature_dependencies[i];
> >>>> + if ((env->user_features[d->from] & d->from_flag) &&
> >>>> + !(env->features[d->from] & d->from_flag)) {
> >>> Why does it matter if the feature was cleared explicitly by the
> >>> user?
> >> Because the feature set of named CPU models should be internally
> >> consistent. I thought of this mechanism as a quick "clean up user's
> >> choices" pass to avoid having to remember a multitude of VMX features,
> >> for example it makes "-cpu host,-rdtscp" just work.
> > If named CPU models are already consistent, ignoring
> > user_features shouldn't make a difference, right? It would also
> > be a useful mechanism to detect inconsistencies in internal CPU
> > model definitions.
>
> Ok, I can drop that check.
>
> >> It has to be done before expansion, so that env->user_features is set
> >> properly before -cpu host is expanded.
> >
> > I don't get it. It looks like you only need env->user_features
> > to be set above because you are handling dependencies before
> > cpu->max_features is handled.
> >
> > If you handle dependencies at x86_cpu_filter_features() instead
> > (after cpu->max_features was already handled), you don't even
> > need to worry about setting user_features.
>
> I think you're right, but on the other hand setting user_features is
> cleaner. Effectively the dependent features have been disabled because
> of something the user told QEMU. So on one hand I can move the loop to
> x86_cpu_filter_features, on the other hand I'd prefer to set
> user_features and then it feels more like expansion (e.g. of vmx-ept=off
> to vmx-ept=off,vmx-unrestricted-guest=off) than filtering.
I don't disagree if you really want to set user_features for
consistency. Considering it part of expansion and not filtering
makes sense, too.
The only point I disagree with is the handling feature dependency
before the "if (cpu->max_features)" block. e.g. if a feature is
being disabled by x86_cpu_get_supported_feature_word(), we also
need to disable features that depend on it.
--
Eduardo
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH 2/7] target/i386: introduce generic feature dependency mechanism
2019-09-17 10:34 [Qemu-devel] [PATCH v2 0/7] target/i386: support VMX features in "-cpu" Paolo Bonzini
@ 2019-09-17 10:34 ` Paolo Bonzini
0 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2019-09-17 10:34 UTC (permalink / raw)
To: qemu-devel; +Cc: liran.alon, ehabkost
Sometimes a CPU feature does not make sense unless another is
present. In the case of VMX features, KVM does not even allow
setting the VMX controls to some invalid combinations.
Therefore, this patch adds a generic mechanism that looks for bits
that the user explicitly cleared, and uses them to remove other bits
from the expanded CPU definition. If these dependent bits were also
explicitly *set* by the user, this will be a warning for "-cpu check"
and an error for "-cpu enforce". If not, then the dependent bits are
cleared silently, for convenience.
With VMX features, this will be used so that for example
"-cpu host,-rdrand" will also hide support for RDRAND exiting.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/cpu.c | 72 ++++++++++++++++++++++++++++++++++++-------------------
1 file changed, 48 insertions(+), 24 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 52b3f3e..fae458c 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -801,10 +801,6 @@ typedef struct FeatureWordInfo {
/* If type==MSR_FEATURE_WORD */
struct {
uint32_t index;
- struct { /*CPUID that enumerate this MSR*/
- FeatureWord cpuid_class;
- uint32_t cpuid_flag;
- } cpuid_dep;
} msr;
};
uint32_t tcg_features; /* Feature flags supported by TCG */
@@ -1218,10 +1214,6 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
},
.msr = {
.index = MSR_IA32_ARCH_CAPABILITIES,
- .cpuid_dep = {
- FEAT_7_0_EDX,
- CPUID_7_0_EDX_ARCH_CAPABILITIES
- }
},
},
[FEAT_CORE_CAPABILITY] = {
@@ -1238,14 +1230,30 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
},
.msr = {
.index = MSR_IA32_CORE_CAPABILITY,
- .cpuid_dep = {
- FEAT_7_0_EDX,
- CPUID_7_0_EDX_CORE_CAPABILITY,
- },
},
},
};
+typedef struct FeatureMask {
+ FeatureWord index;
+ uint32_t mask;
+} FeatureMask;
+
+typedef struct FeatureDep {
+ FeatureMask from, to;
+} FeatureDep;
+
+static FeatureDep feature_dependencies[] = {
+ {
+ .from = { FEAT_7_0_EDX, CPUID_7_0_EDX_ARCH_CAPABILITIES },
+ .to = { FEAT_ARCH_CAPABILITIES, ~0u },
+ },
+ {
+ .from = { FEAT_7_0_EDX, CPUID_7_0_EDX_CORE_CAPABILITY },
+ .to = { FEAT_CORE_CAPABILITY, ~0u },
+ },
+};
+
typedef struct X86RegisterInfo32 {
/* Name of register */
const char *name;
@@ -5063,9 +5071,26 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
{
CPUX86State *env = &cpu->env;
FeatureWord w;
+ int i;
GList *l;
Error *local_err = NULL;
+ for (l = plus_features; l; l = l->next) {
+ const char *prop = l->data;
+ object_property_set_bool(OBJECT(cpu), true, prop, &local_err);
+ if (local_err) {
+ goto out;
+ }
+ }
+
+ for (l = minus_features; l; l = l->next) {
+ const char *prop = l->data;
+ object_property_set_bool(OBJECT(cpu), false, prop, &local_err);
+ if (local_err) {
+ goto out;
+ }
+ }
+
/*TODO: Now cpu->max_features doesn't overwrite features
* set using QOM properties, and we can convert
* plus_features & minus_features to global properties
@@ -5083,19 +5108,18 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
}
}
- for (l = plus_features; l; l = l->next) {
- const char *prop = l->data;
- object_property_set_bool(OBJECT(cpu), true, prop, &local_err);
- if (local_err) {
- goto out;
- }
- }
+ for (i = 0; i < ARRAY_SIZE(feature_dependencies); i++) {
+ FeatureDep *d = &feature_dependencies[i];
+ if (!(env->features[d->from.index] & d->from.mask)) {
+ uint32_t unavailable_features = env->features[d->to.index] & d->to.mask;
- for (l = minus_features; l; l = l->next) {
- const char *prop = l->data;
- object_property_set_bool(OBJECT(cpu), false, prop, &local_err);
- if (local_err) {
- goto out;
+ /* Not an error unless the dependent feature was added explicitly. */
+ mark_unavailable_features(cpu, d->to.index,
+ unavailable_features & env->user_features[d->to.index],
+ "This feature depends on other features that were not requested");
+
+ env->user_features[d->to.index] |= unavailable_features;
+ env->features[d->to.index] &= ~unavailable_features;
}
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
end of thread, other threads:[~2019-09-17 10:52 UTC | newest]
Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-02 15:01 [Qemu-devel] [RFC PATCH 0/7] target/i386: support VMX features in "-cpu" Paolo Bonzini
2019-07-02 15:01 ` [Qemu-devel] [PATCH 1/7] target/i386: handle filtered_features in a new function mark_unavailable_features Paolo Bonzini
2019-07-05 20:37 ` Eduardo Habkost
2019-07-05 21:32 ` Paolo Bonzini
2019-07-05 21:44 ` Eduardo Habkost
2019-07-05 22:07 ` Paolo Bonzini
2019-07-05 22:16 ` Eduardo Habkost
2019-07-02 15:01 ` [Qemu-devel] [PATCH 2/7] target/i386: introduce generic feature dependency mechanism Paolo Bonzini
2019-07-05 20:52 ` Eduardo Habkost
2019-07-05 21:12 ` Paolo Bonzini
2019-07-05 21:41 ` Eduardo Habkost
2019-07-05 22:07 ` Paolo Bonzini
2019-07-08 21:45 ` Eduardo Habkost
2019-07-02 15:01 ` [Qemu-devel] [PATCH 3/7] target/i386: expand feature words to 64 bits Paolo Bonzini
2019-07-02 15:01 ` [Qemu-devel] [PATCH 4/7] target/i386: add VMX definitions Paolo Bonzini
2019-07-02 15:01 ` [Qemu-devel] [PATCH 5/7] vmxcap: correct the name of the variables Paolo Bonzini
2019-07-02 15:01 ` [Qemu-devel] [PATCH 6/7] target/i386: add VMX features Paolo Bonzini
2019-07-05 21:22 ` Eduardo Habkost
2019-07-05 22:12 ` Paolo Bonzini
2019-07-05 22:33 ` Eduardo Habkost
2019-07-05 22:42 ` Paolo Bonzini
2019-07-05 22:48 ` Eduardo Habkost
2019-07-02 15:01 ` [Qemu-devel] [PATCH 7/7] target/i386: work around KVM_GET_MSRS bug for secondary execution controls Paolo Bonzini
2019-07-02 20:46 ` [Qemu-devel] [RFC PATCH 0/7] target/i386: support VMX features in "-cpu" no-reply
2019-07-02 21:13 ` no-reply
2019-07-02 21:38 ` [Qemu-devel] No symbols in LeakSanitizer output (was Re: [RFC PATCH 0/7] target/i386: support VMX features in "-cpu") Eduardo Habkost
2019-07-02 23:05 ` Peter Maydell
2019-07-05 10:19 ` Paolo Bonzini
2019-09-17 10:34 [Qemu-devel] [PATCH v2 0/7] target/i386: support VMX features in "-cpu" Paolo Bonzini
2019-09-17 10:34 ` [Qemu-devel] [PATCH 2/7] target/i386: introduce generic feature dependency mechanism Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).