* [Qemu-devel] [PATCH v2 0/7] target/i386: support VMX features in "-cpu"
@ 2019-09-17 10:34 Paolo Bonzini
2019-09-17 10:34 ` [Qemu-devel] [PATCH 1/7] target/i386: handle filtered_features in a new function mark_unavailable_features Paolo Bonzini
` (7 more replies)
0 siblings, 8 replies; 12+ messages in thread
From: Paolo Bonzini @ 2019-09-17 10:34 UTC (permalink / raw)
To: qemu-devel; +Cc: liran.alon, ehabkost
This series adds support for VMX feature flags so that the user can
enable and disable at will the flags. A separate series will
also add VMX features to named CPU models, which will complete VMX
live migration support. That's orthogonal and somewhat tedious.
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)
Paolo
v1->v2: do not consult check_cpuid/enforce_cpuid in mark_unavailable_features
introduce struct FeatureMask
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 | 443 ++++++++++++++++++++++++++++++++++++++++-----------
target/i386/cpu.h | 136 +++++++++++++++-
target/i386/kvm.c | 173 +++++++++++++++++++-
5 files changed, 665 insertions(+), 103 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH 1/7] target/i386: handle filtered_features in a new function mark_unavailable_features
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
2019-09-17 10:34 ` [Qemu-devel] [PATCH 2/7] target/i386: introduce generic feature dependency mechanism Paolo Bonzini
` (6 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2019-09-17 10:34 UTC (permalink / raw)
To: qemu-devel; +Cc: liran.alon, ehabkost
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 optionally reporting them.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/cpu.c | 87 ++++++++++++++++++++++++++++++-------------------------
1 file changed, 48 insertions(+), 39 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 9e0bac3..52b3f3e 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3096,17 +3096,41 @@ 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 < FEATURE_WORDS; w++) {
+ if (cpu->filtered_features[w]) {
+ return true;
+ }
+ }
+
+ return false;
+}
+
+static void mark_unavailable_features(X86CPU *cpu, FeatureWord w, uint32_t mask,
+ const char *verbose_prefix)
+{
+ CPUX86State *env = &cpu->env;
FeatureWordInfo *f = &feature_word_info[w];
int i;
char *feat_word_str;
+ if (!cpu->force_features) {
+ env->features[w] &= ~mask;
+ }
+ cpu->filtered_features[w] |= mask;
+
+ if (!verbose_prefix) {
+ 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]",
+ verbose_prefix,
feat_word_str,
f->feat_names[i] ? "." : "",
f->feat_names[i] ? f->feat_names[i] : "", i);
@@ -3511,7 +3535,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, bool verbose);
/* Build a list with the name of all features on a feature word array */
static void x86_cpu_list_feature_names(FeatureWordArray features,
@@ -3576,7 +3600,7 @@ static void x86_cpu_class_check_missing_features(X86CPUClass *xcc,
next = &new->next;
}
- x86_cpu_filter_features(xc);
+ x86_cpu_filter_features(xc, false);
x86_cpu_list_feature_names(xc->filtered_features, next);
@@ -3784,15 +3808,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;
@@ -5154,24 +5169,24 @@ 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, bool verbose)
{
CPUX86State *env = &cpu->env;
FeatureWord w;
- int rv = 0;
+ const char *prefix = NULL;
+
+ if (verbose) {
+ 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];
- uint32_t available_features = requested_features & host_feat;
- if (!cpu->force_features) {
- env->features[w] = available_features;
- }
- cpu->filtered_features[w] = requested_features & ~available_features;
- 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) &&
@@ -5197,13 +5212,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)
@@ -5244,16 +5255,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, cpu->check_cpuid || cpu->enforce_cpuid);
+
+ 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] 12+ 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 ` [Qemu-devel] [PATCH 1/7] target/i386: handle filtered_features in a new function mark_unavailable_features Paolo Bonzini
@ 2019-09-17 10:34 ` Paolo Bonzini
2019-09-17 10:34 ` [Qemu-devel] [PATCH 3/7] target/i386: expand feature words to 64 bits Paolo Bonzini
` (5 subsequent siblings)
7 siblings, 0 replies; 12+ 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] 12+ messages in thread
* [Qemu-devel] [PATCH 3/7] target/i386: expand feature words to 64 bits
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 1/7] target/i386: handle filtered_features in a new function mark_unavailable_features Paolo Bonzini
2019-09-17 10:34 ` [Qemu-devel] [PATCH 2/7] target/i386: introduce generic feature dependency mechanism Paolo Bonzini
@ 2019-09-17 10:34 ` Paolo Bonzini
2019-09-17 10:34 ` [Qemu-devel] [PATCH 4/7] target/i386: add VMX definitions Paolo Bonzini
` (4 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2019-09-17 10:34 UTC (permalink / raw)
To: qemu-devel; +Cc: liran.alon, ehabkost
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 | 71 +++++++++++++++++++++++++++-------------------------
target/i386/cpu.h | 2 +-
target/i386/kvm.c | 2 +-
4 files changed, 40 insertions(+), 37 deletions(-)
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 909bcd7..d691e45 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -462,7 +462,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 fae458c..83a3692 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -789,7 +789,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 {
@@ -803,11 +803,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] = {
@@ -1236,7 +1236,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
typedef struct FeatureMask {
FeatureWord index;
- uint32_t mask;
+ uint64_t mask;
} FeatureMask;
typedef struct FeatureDep {
@@ -1246,11 +1246,11 @@ typedef struct FeatureDep {
static FeatureDep feature_dependencies[] = {
{
.from = { FEAT_7_0_EDX, CPUID_7_0_EDX_ARCH_CAPABILITIES },
- .to = { FEAT_ARCH_CAPABILITIES, ~0u },
+ .to = { FEAT_ARCH_CAPABILITIES, ~0ull },
},
{
.from = { FEAT_7_0_EDX, CPUID_7_0_EDX_CORE_CAPABILITY },
- .to = { FEAT_CORE_CAPABILITY, ~0u },
+ .to = { FEAT_CORE_CAPABILITY, ~0ull },
},
};
@@ -1362,14 +1362,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 */
@@ -2931,7 +2931,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)
@@ -3117,7 +3117,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 *verbose_prefix)
{
CPUX86State *env = &cpu->env;
@@ -3134,8 +3134,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]",
verbose_prefix,
@@ -3378,7 +3378,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] = { };
@@ -3422,6 +3422,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
*/
@@ -3435,9 +3436,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,
@@ -3553,10 +3556,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;
@@ -3725,7 +3728,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]);
}
@@ -3780,11 +3783,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) {
@@ -3955,7 +3958,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;
}
@@ -5111,7 +5114,7 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
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;
+ uint64_t unavailable_features = env->features[d->to.index] & d->to.mask;
/* Not an error unless the dependent feature was added explicitly. */
mark_unavailable_features(cpu, d->to.index,
@@ -5206,10 +5209,10 @@ static void x86_cpu_filter_features(X86CPU *cpu, bool verbose)
}
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);
}
@@ -5506,7 +5509,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,
@@ -5514,7 +5517,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);
}
@@ -5567,7 +5570,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) {
@@ -5701,7 +5704,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 5f6e3a0..c00cc35 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -502,7 +502,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)
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 8023c67..52f98d7 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -441,7 +441,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] 12+ messages in thread
* [Qemu-devel] [PATCH 4/7] target/i386: add VMX definitions
2019-09-17 10:34 [Qemu-devel] [PATCH v2 0/7] target/i386: support VMX features in "-cpu" Paolo Bonzini
` (2 preceding siblings ...)
2019-09-17 10:34 ` [Qemu-devel] [PATCH 3/7] target/i386: expand feature words to 64 bits Paolo Bonzini
@ 2019-09-17 10:34 ` Paolo Bonzini
2019-09-17 10:34 ` [Qemu-devel] [PATCH 5/7] vmxcap: correct the name of the variables Paolo Bonzini
` (3 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2019-09-17 10:34 UTC (permalink / raw)
To: qemu-devel; +Cc: liran.alon, ehabkost
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 c00cc35..7b7f062 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
@@ -750,6 +769,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] 12+ messages in thread
* [Qemu-devel] [PATCH 5/7] vmxcap: correct the name of the variables
2019-09-17 10:34 [Qemu-devel] [PATCH v2 0/7] target/i386: support VMX features in "-cpu" Paolo Bonzini
` (3 preceding siblings ...)
2019-09-17 10:34 ` [Qemu-devel] [PATCH 4/7] target/i386: add VMX definitions Paolo Bonzini
@ 2019-09-17 10:34 ` Paolo Bonzini
2019-09-17 10:34 ` [Qemu-devel] [PATCH 6/7] target/i386: add VMX features Paolo Bonzini
` (2 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2019-09-17 10:34 UTC (permalink / raw)
To: qemu-devel; +Cc: liran.alon, ehabkost
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 d8c7d6d..5dfeb2e 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] 12+ messages in thread
* [Qemu-devel] [PATCH 6/7] target/i386: add VMX features
2019-09-17 10:34 [Qemu-devel] [PATCH v2 0/7] target/i386: support VMX features in "-cpu" Paolo Bonzini
` (4 preceding siblings ...)
2019-09-17 10:34 ` [Qemu-devel] [PATCH 5/7] vmxcap: correct the name of the variables Paolo Bonzini
@ 2019-09-17 10:34 ` Paolo Bonzini
2019-09-19 14:32 ` Liran Alon
2019-09-17 10:34 ` [Qemu-devel] [PATCH 7/7] target/i386: work around KVM_GET_MSRS bug for secondary execution controls Paolo Bonzini
2019-09-17 12:28 ` [Qemu-devel] [PATCH v2 0/7] target/i386: support VMX features in "-cpu" no-reply
7 siblings, 1 reply; 12+ messages in thread
From: Paolo Bonzini @ 2019-09-17 10:34 UTC (permalink / raw)
To: qemu-devel; +Cc: liran.alon, ehabkost
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 | 225 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
target/i386/cpu.h | 9 +++
target/i386/kvm.c | 154 ++++++++++++++++++++++++++++++++++++-
3 files changed, 386 insertions(+), 2 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 83a3692..5b771f1 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1232,6 +1232,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 FeatureMask {
@@ -1252,6 +1409,74 @@ static FeatureDep feature_dependencies[] = {
.from = { FEAT_7_0_EDX, CPUID_7_0_EDX_CORE_CAPABILITY },
.to = { FEAT_CORE_CAPABILITY, ~0ull },
},
+ {
+ .from = { FEAT_1_ECX, CPUID_EXT_VMX },
+ .to = { FEAT_VMX_PROCBASED_CTLS, ~0ull },
+ },
+ {
+ .from = { FEAT_1_ECX, CPUID_EXT_VMX },
+ .to = { FEAT_VMX_PINBASED_CTLS, ~0ull },
+ },
+ {
+ .from = { FEAT_1_ECX, CPUID_EXT_VMX },
+ .to = { FEAT_VMX_EXIT_CTLS, ~0ull },
+ },
+ {
+ .from = { FEAT_1_ECX, CPUID_EXT_VMX },
+ .to = { FEAT_VMX_ENTRY_CTLS, ~0ull },
+ },
+ {
+ .from = { FEAT_1_ECX, CPUID_EXT_VMX },
+ .to = { FEAT_VMX_MISC, ~0ull },
+ },
+ {
+ .from = { FEAT_1_ECX, CPUID_EXT_VMX },
+ .to = { FEAT_VMX_BASIC, ~0ull },
+ },
+ {
+ .from = { FEAT_8000_0001_EDX, CPUID_EXT2_LM },
+ .to = { FEAT_VMX_ENTRY_CTLS, VMX_VM_ENTRY_IA32E_MODE },
+ },
+ {
+ .from = { FEAT_VMX_PROCBASED_CTLS, VMX_CPU_BASED_ACTIVATE_SECONDARY_CONTROLS },
+ .to = { FEAT_VMX_SECONDARY_CTLS, ~0ull },
+ },
+ {
+ .from = { FEAT_XSAVE, CPUID_XSAVE_XSAVES },
+ .to = { FEAT_VMX_SECONDARY_CTLS, VMX_SECONDARY_EXEC_XSAVES },
+ },
+ {
+ .from = { FEAT_1_ECX, CPUID_EXT_RDRAND },
+ .to = { FEAT_VMX_SECONDARY_CTLS, VMX_SECONDARY_EXEC_RDRAND_EXITING },
+ },
+ {
+ .from = { FEAT_7_0_EBX, CPUID_7_0_EBX_INVPCID },
+ .to = { FEAT_VMX_SECONDARY_CTLS, VMX_SECONDARY_EXEC_ENABLE_INVPCID },
+ },
+ {
+ .from = { FEAT_7_0_EBX, CPUID_7_0_EBX_RDSEED },
+ .to = { FEAT_VMX_SECONDARY_CTLS, VMX_SECONDARY_EXEC_RDSEED_EXITING },
+ },
+ {
+ .from = { FEAT_8000_0001_EDX, CPUID_EXT2_RDTSCP },
+ .to = { FEAT_VMX_SECONDARY_CTLS, VMX_SECONDARY_EXEC_RDTSCP },
+ },
+ {
+ .from = { FEAT_VMX_SECONDARY_CTLS, VMX_SECONDARY_EXEC_ENABLE_EPT },
+ .to = { FEAT_VMX_EPT_VPID_CAPS, 0xffffffffull },
+ },
+ {
+ .from = { FEAT_VMX_SECONDARY_CTLS, VMX_SECONDARY_EXEC_ENABLE_EPT },
+ .to = { FEAT_VMX_SECONDARY_CTLS, VMX_SECONDARY_EXEC_UNRESTRICTED_GUEST },
+ },
+ {
+ .from = { FEAT_VMX_SECONDARY_CTLS, VMX_SECONDARY_EXEC_ENABLE_VPID },
+ .to = { FEAT_VMX_EPT_VPID_CAPS, 0xffffffffull << 32 },
+ },
+ {
+ .from = { FEAT_VMX_SECONDARY_CTLS, VMX_SECONDARY_EXEC_ENABLE_VMFUNC },
+ .to = { FEAT_VMX_VMFUNC, ~0ull },
+ },
};
typedef struct X86RegisterInfo32 {
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 7b7f062..8447ece 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -518,6 +518,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 52f98d7..bba1930 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -99,6 +99,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;
@@ -447,7 +448,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;
@@ -473,7 +475,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;
+ }
}
@@ -1938,6 +1958,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;
}
}
}
@@ -2411,6 +2434,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;
@@ -2668,6 +2811,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] 12+ messages in thread
* [Qemu-devel] [PATCH 7/7] target/i386: work around KVM_GET_MSRS bug for secondary execution controls
2019-09-17 10:34 [Qemu-devel] [PATCH v2 0/7] target/i386: support VMX features in "-cpu" Paolo Bonzini
` (5 preceding siblings ...)
2019-09-17 10:34 ` [Qemu-devel] [PATCH 6/7] target/i386: add VMX features Paolo Bonzini
@ 2019-09-17 10:34 ` Paolo Bonzini
2019-09-17 12:28 ` [Qemu-devel] [PATCH v2 0/7] target/i386: support VMX features in "-cpu" no-reply
7 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2019-09-17 10:34 UTC (permalink / raw)
To: qemu-devel; +Cc: liran.alon, ehabkost
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 bba1930..4843f8c 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -478,6 +478,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] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/7] target/i386: support VMX features in "-cpu"
2019-09-17 10:34 [Qemu-devel] [PATCH v2 0/7] target/i386: support VMX features in "-cpu" Paolo Bonzini
` (6 preceding siblings ...)
2019-09-17 10:34 ` [Qemu-devel] [PATCH 7/7] target/i386: work around KVM_GET_MSRS bug for secondary execution controls Paolo Bonzini
@ 2019-09-17 12:28 ` no-reply
7 siblings, 0 replies; 12+ messages in thread
From: no-reply @ 2019-09-17 12:28 UTC (permalink / raw)
To: pbonzini; +Cc: liran.alon, qemu-devel, ehabkost
Patchew URL: https://patchew.org/QEMU/1568716480-9973-1-git-send-email-pbonzini@redhat.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Subject: [Qemu-devel] [PATCH v2 0/7] target/i386: support VMX features in "-cpu"
Message-id: 1568716480-9973-1-git-send-email-pbonzini@redhat.com
Type: series
=== 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 ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
521db80..186c0ab master -> master
- [tag update] patchew/156871562997.196432.17776290406203122029.stgit@bahia.lan -> patchew/156871562997.196432.17776290406203122029.stgit@bahia.lan
* [new tag] patchew/1568716480-9973-1-git-send-email-pbonzini@redhat.com -> patchew/1568716480-9973-1-git-send-email-pbonzini@redhat.com
* [new tag] patchew/156872146565.1757.3033215873677512474.stgit@pasha-Precision-3630-Tower -> patchew/156872146565.1757.3033215873677512474.stgit@pasha-Precision-3630-Tower
- [tag update] patchew/20190917092004.999-1-mreitz@redhat.com -> patchew/20190917092004.999-1-mreitz@redhat.com
* [new tag] patchew/20190917110443.2029-1-kwolf@redhat.com -> patchew/20190917110443.2029-1-kwolf@redhat.com
* [new tag] patchew/20190917111441.27405-1-kraxel@redhat.com -> patchew/20190917111441.27405-1-kraxel@redhat.com
Switched to a new branch 'test'
728ad01 target/i386: work around KVM_GET_MSRS bug for secondary execution controls
dd50d5d target/i386: add VMX features
14eae1d vmxcap: correct the name of the variables
6914f81 target/i386: add VMX definitions
60e92af target/i386: expand feature words to 64 bits
50ba007 target/i386: introduce generic feature dependency mechanism
2cc5119 target/i386: handle filtered_features in a new function mark_unavailable_features
=== OUTPUT BEGIN ===
1/7 Checking commit 2cc511980158 (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:3103:
+ for (w = 0; w < FEATURE_WORDS; w++) {
+ if (cpu->filtered_features[w]) {
ERROR: suspect code indent for conditional statements (9, 13)
#28: FILE: target/i386/cpu.c:3104:
+ if (cpu->filtered_features[w]) {
+ return true;
WARNING: line over 80 characters
#139: FILE: target/i386/cpu.c:5215:
+ mark_unavailable_features(cpu, FEAT_7_0_EBX, CPUID_7_0_EBX_INTEL_PT, prefix);
total: 2 errors, 1 warnings, 147 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 50ba0079516a (target/i386: introduce generic feature dependency mechanism)
WARNING: line over 80 characters
#126: FILE: target/i386/cpu.c:5114:
+ uint32_t unavailable_features = env->features[d->to.index] & d->to.mask;
WARNING: line over 80 characters
#133: FILE: target/i386/cpu.c:5116:
+ /* Not an error unless the dependent feature was added explicitly. */
ERROR: line over 90 characters
#135: FILE: target/i386/cpu.c:5118:
+ unavailable_features & env->user_features[d->to.index],
total: 1 errors, 2 warnings, 110 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 60e92afb4b86 (target/i386: expand feature words to 64 bits)
WARNING: line over 80 characters
#153: FILE: target/i386/cpu.c:3442:
+ assert(bitnr < 32 || !(name && feature_word_info[w].type == CPUID_FEATURE_WORD));
WARNING: line over 80 characters
#209: FILE: target/i386/cpu.c:5117:
+ uint64_t unavailable_features = env->features[d->to.index] & d->to.mask;
total: 0 errors, 2 warnings, 235 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 6914f81d020e (target/i386: add VMX definitions)
5/7 Checking commit 14eae1d5e8ea (vmxcap: correct the name of the variables)
6/7 Checking commit dd50d5d8984b (target/i386: add VMX features)
ERROR: line over 90 characters
#32: FILE: target/i386/cpu.c:1244:
+ "vmx-cr8-store-exit", "vmx-flexpriority", "vmx-vnmi-pending", "vmx-movdr-exit",
WARNING: line over 80 characters
#34: FILE: target/i386/cpu.c:1246:
+ "vmx-msr-bitmap", "vmx-monitor-exit", "vmx-pause-exit", "vmx-secondary-ctls",
WARNING: line over 80 characters
#45: FILE: target/i386/cpu.c:1257:
+ "vmx-apicv-x2apic", "vmx-vpid", "vmx-wbinvd-exit", "vmx-unrestricted-guest",
WARNING: line over 80 characters
#47: FILE: target/i386/cpu.c:1259:
+ "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:1296:
+ NULL, NULL /* vmx-exit-host-addr-space-size */, NULL, NULL,
ERROR: line over 90 characters
#103: FILE: target/i386/cpu.c:1315:
+ 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:1357:
+ "vmx-invvpid-all-context", "vmx-invept-single-context-noglobals",
WARNING: line over 80 characters
#166: FILE: target/i386/cpu.c:1378:
+ /* Just to be safe - we don't support setting the MSEG version field. */
WARNING: line over 80 characters
#216: FILE: target/i386/cpu.c:1441:
+ .from = { FEAT_VMX_PROCBASED_CTLS, VMX_CPU_BASED_ACTIVATE_SECONDARY_CONTROLS },
WARNING: line over 80 characters
#245: FILE: target/i386/cpu.c:1470:
+ .to = { FEAT_VMX_SECONDARY_CTLS, VMX_SECONDARY_EXEC_UNRESTRICTED_GUEST },
WARNING: Block comments use a leading /* on a separate line
#366: FILE: target/i386/kvm.c:2462:
+ /* If a feature bit is set, the control can be either set or clear.
WARNING: line over 80 characters
#421: FILE: target/i386/kvm.c:2517:
+ (f[FEAT_VMX_SECONDARY_CTLS] & VMX_SECONDARY_EXEC_ENABLE_EPT ? 0x4100ull : 0);
WARNING: line over 80 characters
#432: FILE: target/i386/kvm.c:2528:
+ f[FEAT_VMX_EXIT_CTLS]) | fixed_vmx_exit);
total: 2 errors, 11 warnings, 442 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 728ad0189d83 (target/i386: work around KVM_GET_MSRS bug for secondary execution controls)
WARNING: line over 80 characters
#26: FILE: target/i386/kvm.c:482:
+ 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:488:
+ 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:491:
+ 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:494:
+ 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/1568716480-9973-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] 12+ messages in thread
* Re: [PATCH 6/7] target/i386: add VMX features
2019-09-17 10:34 ` [Qemu-devel] [PATCH 6/7] target/i386: add VMX features Paolo Bonzini
@ 2019-09-19 14:32 ` Liran Alon
2019-09-19 14:54 ` Paolo Bonzini
0 siblings, 1 reply; 12+ messages in thread
From: Liran Alon @ 2019-09-19 14:32 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, ehabkost
> On 17 Sep 2019, at 13:34, Paolo Bonzini <pbonzini@redhat.com> 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>
> ---
> +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();
> + }
> +
See below.
> + /*
> + * 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;
I haven’t yet read deeply entire patch-series but I’m definitely against having
these hard-coded values in code instead of explicitly building proper bitmap
with well-defined bit names. This is error-prone and less readable.
(E.g. Am I suppose as a reader to convert 0x0401e172 to which processor-based controls it represents?)
-Liran
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 6/7] target/i386: add VMX features
2019-09-19 14:32 ` Liran Alon
@ 2019-09-19 14:54 ` Paolo Bonzini
2019-09-19 15:16 ` Paolo Bonzini
0 siblings, 1 reply; 12+ messages in thread
From: Paolo Bonzini @ 2019-09-19 14:54 UTC (permalink / raw)
To: Liran Alon; +Cc: qemu-devel, ehabkost
On 19/09/19 16:32, Liran Alon wrote:
>
>
>> On 17 Sep 2019, at 13:34, Paolo Bonzini <pbonzini@redhat.com> 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>
>> ---
>> +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();
>> + }
>> +
>
> See below.
>
>> + /*
>> + * 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;
>
> I haven’t yet read deeply entire patch-series but I’m definitely against having
> these hard-coded values in code instead of explicitly building proper bitmap
> with well-defined bit names. This is error-prone and less readable.
> (E.g. Am I suppose as a reader to convert 0x0401e172 to which processor-based controls it represents?)
No, you're not. :) In fact, most of the bits that are set in these
constants have no definition. Most "default1" reserved bits have
remained reserved since forever, the only exceptions are DEBUGCTL and
CR3 load/store controls.
The hex constants here correspond simply to the bits that are listed in
appendix A of the SDM:
Default settings partition the various controls into the following
classes:
[...]
* Default1. They are (or have been) reserved with a default
setting of 1.
[...]
The default1 class of pin-based VM-execution controls contains bits 1,
2, and 4
[...]
The default1 class of processor-based VM-execution controls contains
bits 1, 4–6, 8, 13–16, and 26
[...]
The default1 class of VM-exit controls contains bits 0–8, 10, 11, 13,
14, 16, and 17
[...]
The default1 class of VM-entry controls contains bits 0–8 and 12
I could add four #defines for these values, but they are used only here
and shouldn't be used elsewhere since make_vmx_msr_value is there
exactly to hide the existence of default1 reserved bits.
I will add defines for fixed_vmx_basic, fixed_vmx_misc and
fixed_vmx_ept_mask, though.
Paolo
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 6/7] target/i386: add VMX features
2019-09-19 14:54 ` Paolo Bonzini
@ 2019-09-19 15:16 ` Paolo Bonzini
0 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2019-09-19 15:16 UTC (permalink / raw)
To: Liran Alon; +Cc: qemu-devel, ehabkost
On 19/09/19 16:54, Paolo Bonzini wrote:
> I will add defines for fixed_vmx_basic, fixed_vmx_misc and
> fixed_vmx_ept_mask, though.
... like this:
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 8447ece..c62e3b6 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -779,14 +779,19 @@ typedef uint64_t FeatureWordArray[FEATURE_WORDS];
#define MSR_CORE_CAP_SPLIT_LOCK_DETECT (1U << 5)
/* VMX MSR features */
+#define MSR_VMX_BASIC_VMCS_REVISION_MASK 0x7FFFFFFFull
+#define MSR_VMX_BASIC_VMXON_REGION_SIZE_MASK (0x00001FFFull << 32)
+#define MSR_VMX_BASIC_VMCS_MEM_TYPE_MASK (0x003C0000ull << 32)
#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_PREEMPTION_TIMER_SHIFT_MASK 0x1Full
#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_MAX_MSR_LIST_SIZE_MASK 0x0E000000ull
#define MSR_VMX_MISC_VMWRITE_VMEXIT (1ULL << 29)
#define MSR_VMX_MISC_ZERO_LEN_INJECT (1ULL << 30)
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 29865c3..7c5ad26 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -2500,7 +2500,10 @@ static void kvm_msr_entry_add_vmx(X86CPU *cpu, FeatureWordArray f)
* 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;
+ uint64_t fixed_vmx_basic = kvm_vmx_basic &
+ (MSR_VMX_BASIC_VMCS_REVISION_MASK |
+ MSR_VMX_BASIC_VMXON_REGION_SIZE_MASK |
+ MSR_VMX_BASIC_VMCS_MEM_TYPE_MASK);
/*
* Same for bits 0-4 and 25-27. Bits 16-24 (CR3 target count) can
@@ -2509,14 +2512,17 @@ static void kvm_msr_entry_add_vmx(X86CPU *cpu, FeatureWordArray f)
* not support dual-monitor treatment and probably never will; mask
* them out as well.
*/
- uint64_t fixed_vmx_misc = kvm_vmx_misc & 0x0e00001f;
+ uint64_t fixed_vmx_misc = kvm_vmx_misc &
+ (MSR_VMX_MISC_PREEMPTION_TIMER_SHIFT_MASK |
+ MSR_VMX_MISC_MAX_MSR_LIST_SIZE_MASK);
/*
* 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);
+ (f[FEAT_VMX_SECONDARY_CTLS] & VMX_SECONDARY_EXEC_ENABLE_EPT ?
+ MSR_VMX_EPT_UC | MSR_VMX_EPT_WB : 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,
Paolo
^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-09-19 15:19 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 1/7] target/i386: handle filtered_features in a new function mark_unavailable_features Paolo Bonzini
2019-09-17 10:34 ` [Qemu-devel] [PATCH 2/7] target/i386: introduce generic feature dependency mechanism Paolo Bonzini
2019-09-17 10:34 ` [Qemu-devel] [PATCH 3/7] target/i386: expand feature words to 64 bits Paolo Bonzini
2019-09-17 10:34 ` [Qemu-devel] [PATCH 4/7] target/i386: add VMX definitions Paolo Bonzini
2019-09-17 10:34 ` [Qemu-devel] [PATCH 5/7] vmxcap: correct the name of the variables Paolo Bonzini
2019-09-17 10:34 ` [Qemu-devel] [PATCH 6/7] target/i386: add VMX features Paolo Bonzini
2019-09-19 14:32 ` Liran Alon
2019-09-19 14:54 ` Paolo Bonzini
2019-09-19 15:16 ` Paolo Bonzini
2019-09-17 10:34 ` [Qemu-devel] [PATCH 7/7] target/i386: work around KVM_GET_MSRS bug for secondary execution controls Paolo Bonzini
2019-09-17 12:28 ` [Qemu-devel] [PATCH v2 0/7] target/i386: support VMX features in "-cpu" no-reply
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).