9:37 a.m.
This changes the -cpu check/enforce code to work as it should: it will
check every single CPUID bit to make sure it is supported by the host.
The changes are a bit intrusive, but:
- The longer we take to make "enforce" strict as it should (and make libvirt
finally use it), more users will have VMs with migration-unsafe unpredictable
guest ABIs. For this reason, I would like to get this into QEMU 1.4.
- The changes in this series should affect only users that are already using
the "enforce" flag, and I believe whoever is using the "enforce"
flag really
want the strict behavior introduced by this series.
This series is based on Andreas' qom-cpu branch, and depends on the series:
Subject: [PATCH 0/2] Disable kvm_mmu_op by default on pc-1.4
Message-Id: <1357311145-16410-1-git-send-email-ehabkost(a)redhat.com>
Git tree for reference:
git://github.com/ehabkost/qemu-hacks.git cpu-enforce-all.v1
https://github.com/ehabkost/qemu-hacks/tree/cpu-enforce-all.v1
Eduardo Habkost (9):
target-i386: kvm: -cpu host: Use GET_SUPPORTED_CPUID for SVM features
target-i386: kvm: Enable all supported KVM features for -cpu host
target-i386: check/enforce: Fix CPUID leaf numbers on error messages
target-i386: check/enforce: Do not ignore "hypervisor" flag
target-i386: check/enforce: Check all CPUID.80000001H.EDX bits
target-i386: check/enforce: Check SVM flag support as well
target-i386: check/enforce: Eliminate check_feat field
target-i386: Call kvm_check_features_against_host() only if
CONFIG_KVM is set
target-i386: check/enforce: Check all feature words
target-i386/cpu.c | 73 +++++++++++++++++++++++++++++++++++++++----------------
target-i386/cpu.h | 3 +++
2 files changed, 55 insertions(+), 21 deletions(-)
--
1.7.11.7
9:37 a.m.
New subject: [libvirt] [PATCH 1/9] target-i386: kvm: -cpu host: Use GET_SUPPORTED_CPUID for SVM features
The existing -cpu host code simply set every bit inside svm_features
(initializing it to -1), and that makes it impossible to make the
enforce/check options work properly when the user asks for SVM features
explicitly in the command-line.
So, instead of initializing svm_features to -1, use GET_SUPPORTED_CPUID
to fill only the bits that are supported by the host (just like we do
for all other CPUID feature words inside kvm_cpu_fill_host()).
This will keep the existing behavior (as filter_features_for_kvm()
already uses GET_SUPPORTED_CPUID to filter svm_features), but will allow
us to properly check for KVM features inside
kvm_check_features_against_host() later.
For example, we will be able to make this:
$ qemu-system-x86_64 -cpu ...,+pfthreshold,enforce
refuse to start if the SVM "pfthreshold" feature is not supported by the
host (after we fix kvm_check_features_against_host() to check SVM flags
as well).
Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com>
---
Changes v2:
- Coding style (indentation) fix
Cc: Gleb Natapov <gleb(a)redhat.com>
Cc: Marcelo Tosatti <mtosatti(a)redhat.com>
Cc: Joerg Roedel <joro(a)8bytes.org>
Cc: kvm(a)vger.kernel.org
---
target-i386/cpu.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index ec877c7..649cfb2 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -906,13 +906,10 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
}
}
- /*
- * Every SVM feature requires emulation support in KVM - so we can't just
- * read the host features here. KVM might even support SVM features not
- * available on the host hardware. Just set all bits and mask out the
- * unsupported ones later.
- */
- x86_cpu_def->svm_features = -1;
+ /* Other KVM-specific feature fields: */
+ x86_cpu_def->svm_features =
+ kvm_arch_get_supported_cpuid(s, 0x8000000A, 0, R_EDX);
+
#endif /* CONFIG_KVM */
}
--
1.7.11.7
9:37 a.m.
New subject: [libvirt] [PATCH 2/9] target-i386: kvm: Enable all supported KVM features for -cpu host
When using -cpu host, we don't need to use the kvm_default_features
variable, as the user is explicitly asking QEMU to enable all feature
supported by the host.
This changes the kvm_cpu_fill_host() code to use GET_SUPPORTED_CPUID to
initialize the kvm_features field, so we get all host KVM features
enabled.
This will also allow use to properly check/enforce KVM features inside
kvm_check_features_against_host() later. For example, we will be able to
make this:
$ qemu-system-x86_64 -cpu ...,+kvm_pv_eoi,enforce
refuse to start if kvm_pv_eoi is not supported by the host (after we fix
kvm_check_features_against_host() to check KVM flags as well).
Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com>
---
Changes v2:
- Coding style (indentation) fix
Cc: Gleb Natapov <gleb(a)redhat.com>
Cc: Michael S. Tsirkin <mst(a)redhat.com>
Cc: Marcelo Tosatti <mtosatti(a)redhat.com>
Cc: kvm(a)vger.kernel.org
---
target-i386/cpu.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 649cfb2..4e26b11 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -909,6 +909,8 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
/* Other KVM-specific feature fields: */
x86_cpu_def->svm_features =
kvm_arch_get_supported_cpuid(s, 0x8000000A, 0, R_EDX);
+ x86_cpu_def->kvm_features =
+ kvm_arch_get_supported_cpuid(s, KVM_CPUID_FEATURES, 0, R_EAX);
#endif /* CONFIG_KVM */
}
--
1.7.11.7
9:37 a.m.
New subject: [libvirt] [PATCH 3/9] target-i386: check/enforce: Fix CPUID leaf numbers on error messages
The -cpu check/enforce warnings are printing incorrect information about the
missing flags. There are no feature flags on CPUID leaves 0 and 0x80000000, but
there were references to 0 and 0x80000000 in the table at
kvm_check_features_against_host().
This changes the model_features_t struct to contain the register number as
well, so the error messages print the correct CPUID leaf+register information,
instead of wrong CPUID leaf numbers.
This also changes the format of the error messages, so they follow the
"CPUID.<leaf>.<register>.<name> [bit <offset>]"
convention used on Intel
documentation. Example output:
$ qemu-system-x86_64 -machine pc-1.0,accel=kvm -cpu Opteron_G4,+ia64,enforce
warning: host doesn't support requested feature: CPUID.01H:EDX.ia64 [bit 30]
warning: host doesn't support requested feature: CPUID.01H:ECX.xsave [bit 26]
warning: host doesn't support requested feature: CPUID.01H:ECX.avx [bit 28]
warning: host doesn't support requested feature: CPUID.80000001H:ECX.abm [bit 5]
warning: host doesn't support requested feature: CPUID.80000001H:ECX.sse4a [bit
6]
warning: host doesn't support requested feature: CPUID.80000001H:ECX.misalignsse
[bit 7]
warning: host doesn't support requested feature: CPUID.80000001H:ECX.3dnowprefetch
[bit 8]
warning: host doesn't support requested feature: CPUID.80000001H:ECX.xop [bit 11]
warning: host doesn't support requested feature: CPUID.80000001H:ECX.fma4 [bit
16]
Unable to find x86 CPU definition
$
Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com>
---
Cc: Gleb Natapov <gleb(a)redhat.com>
Cc: Marcelo Tosatti <mtosatti(a)redhat.com>
Cc: kvm(a)vger.kernel.org
---
target-i386/cpu.c | 38 +++++++++++++++++++++++++++++---------
target-i386/cpu.h | 3 +++
2 files changed, 32 insertions(+), 9 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 4e26b11..6c43ace 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -124,6 +124,24 @@ static const char *cpuid_7_0_ebx_feature_name[] = {
NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
};
+const char *get_register_name_32(unsigned int reg)
+{
+ static const char *reg_names[CPU_NB_REGS32] = {
+ [R_EAX] = "EAX",
+ [R_ECX] = "ECX",
+ [R_EDX] = "EDX",
+ [R_EBX] = "EBX",
+ [R_ESP] = "ESP",
+ [R_EBP] = "EBP",
+ [R_ESI] = "ESI",
+ [R_EDI] = "EDI",
+ };
+
+ if (reg > CPU_NB_REGS32)
+ return NULL;
+ return reg_names[reg];
+}
+
/* collects per-function cpuid data
*/
typedef struct model_features_t {
@@ -132,7 +150,8 @@ typedef struct model_features_t {
uint32_t check_feat;
const char **flag_names;
uint32_t cpuid;
- } model_features_t;
+ int reg;
+} model_features_t;
int check_cpuid = 0;
int enforce_cpuid = 0;
@@ -921,10 +940,11 @@ static int unavailable_host_feature(struct model_features_t *f,
uint32_t mask)
for (i = 0; i < 32; ++i)
if (1 << i & mask) {
- fprintf(stderr, "warning: host cpuid %04x_%04x lacks requested"
- " flag '%s' [0x%08x]\n",
- f->cpuid >> 16, f->cpuid & 0xffff,
- f->flag_names[i] ? f->flag_names[i] : "[reserved]",
mask);
+ fprintf(stderr, "warning: host doesn't support requested feature:
"
+ "CPUID.%02XH:%s%s%s [bit %d]\n",
+ f->cpuid, get_register_name_32(f->reg),
+ f->flag_names[i] ? "." : "",
+ f->flag_names[i] ? f->flag_names[i] : "", i);
break;
}
return 0;
@@ -943,13 +963,13 @@ static int kvm_check_features_against_host(x86_def_t *guest_def)
int rv, i;
struct model_features_t ft[] = {
{&guest_def->features, &host_def.features,
- ~0, feature_name, 0x00000000},
+ ~0, feature_name, 0x00000001, R_EDX},
{&guest_def->ext_features, &host_def.ext_features,
- ~CPUID_EXT_HYPERVISOR, ext_feature_name, 0x00000001},
+ ~CPUID_EXT_HYPERVISOR, ext_feature_name, 0x00000001, R_ECX},
{&guest_def->ext2_features, &host_def.ext2_features,
- ~PPRO_FEATURES, ext2_feature_name, 0x80000000},
+ ~PPRO_FEATURES, ext2_feature_name, 0x80000001, R_EDX},
{&guest_def->ext3_features, &host_def.ext3_features,
- ~CPUID_EXT3_SVM, ext3_feature_name, 0x80000001}};
+ ~CPUID_EXT3_SVM, ext3_feature_name, 0x80000001, R_ECX}};
assert(kvm_enabled());
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 27c8d0c..ab81a5c 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -1221,4 +1221,7 @@ void cpu_report_tpr_access(CPUX86State *env, TPRAccess access);
void enable_kvm_pv_eoi(void);
void disable_kvm_mmu_op(void);
+/* Return name of 32-bit register, from a R_* constant */
+const char *get_register_name_32(unsigned int reg);
+
#endif /* CPU_I386_H */
--
1.7.11.7
2:54 p.m.
New subject: [libvirt] [Qemu-devel] [PATCH 3/9] target-i386: check/enforce: Fix CPUID leaf numbers on error messages
On Fri, Jan 4, 2013 at 3:37 PM, Eduardo Habkost <ehabkost(a)redhat.com> wrote:
The -cpu check/enforce warnings are printing incorrect information
about the
missing flags. There are no feature flags on CPUID leaves 0 and 0x80000000, but
there were references to 0 and 0x80000000 in the table at
kvm_check_features_against_host().
This changes the model_features_t struct to contain the register number as
well, so the error messages print the correct CPUID leaf+register information,
instead of wrong CPUID leaf numbers.
This also changes the format of the error messages, so they follow the
"CPUID.<leaf>.<register>.<name> [bit <offset>]"
convention used on Intel
documentation. Example output:
$ qemu-system-x86_64 -machine pc-1.0,accel=kvm -cpu Opteron_G4,+ia64,enforce
warning: host doesn't support requested feature: CPUID.01H:EDX.ia64 [bit 30]
warning: host doesn't support requested feature: CPUID.01H:ECX.xsave [bit 26]
warning: host doesn't support requested feature: CPUID.01H:ECX.avx [bit 28]
warning: host doesn't support requested feature: CPUID.80000001H:ECX.abm [bit 5]
warning: host doesn't support requested feature: CPUID.80000001H:ECX.sse4a [bit
6]
warning: host doesn't support requested feature: CPUID.80000001H:ECX.misalignsse
[bit 7]
warning: host doesn't support requested feature:
CPUID.80000001H:ECX.3dnowprefetch [bit 8]
warning: host doesn't support requested feature: CPUID.80000001H:ECX.xop [bit
11]
warning: host doesn't support requested feature: CPUID.80000001H:ECX.fma4 [bit
16]
Unable to find x86 CPU definition
$
Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com>
---
Cc: Gleb Natapov <gleb(a)redhat.com>
Cc: Marcelo Tosatti <mtosatti(a)redhat.com>
Cc: kvm(a)vger.kernel.org
---
target-i386/cpu.c | 38 +++++++++++++++++++++++++++++---------
target-i386/cpu.h | 3 +++
2 files changed, 32 insertions(+), 9 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 4e26b11..6c43ace 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -124,6 +124,24 @@ static const char *cpuid_7_0_ebx_feature_name[] = {
NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
};
+const char *get_register_name_32(unsigned int reg)
+{
+ static const char *reg_names[CPU_NB_REGS32] = {
+ [R_EAX] = "EAX",
+ [R_ECX] = "ECX",
+ [R_EDX] = "EDX",
+ [R_EBX] = "EBX",
+ [R_ESP] = "ESP",
+ [R_EBP] = "EBP",
+ [R_ESI] = "ESI",
+ [R_EDI] = "EDI",
+ };
+
+ if (reg > CPU_NB_REGS32)
Missing braces.
+ return NULL;
+ return reg_names[reg];
+}
+
/* collects per-function cpuid data
*/
typedef struct model_features_t {
@@ -132,7 +150,8 @@ typedef struct model_features_t {
uint32_t check_feat;
const char **flag_names;
uint32_t cpuid;
- } model_features_t;
+ int reg;
+} model_features_t;
int check_cpuid = 0;
int enforce_cpuid = 0;
@@ -921,10 +940,11 @@ static int unavailable_host_feature(struct model_features_t *f,
uint32_t mask)
for (i = 0; i < 32; ++i)
if (1 << i & mask) {
- fprintf(stderr, "warning: host cpuid %04x_%04x lacks requested"
- " flag '%s' [0x%08x]\n",
- f->cpuid >> 16, f->cpuid & 0xffff,
- f->flag_names[i] ? f->flag_names[i] : "[reserved]",
mask);
+ fprintf(stderr, "warning: host doesn't support requested feature:
"
+ "CPUID.%02XH:%s%s%s [bit %d]\n",
+ f->cpuid, get_register_name_32(f->reg),
This could attempt to print NULL via %s format, which is not OK with
all C libraries. If we trust that the callers always pass valid
numbers, the check above could be turned into assert().
+ f->flag_names[i] ? "." : "",
+ f->flag_names[i] ? f->flag_names[i] : "", i);
break;
}
return 0;
@@ -943,13 +963,13 @@ static int kvm_check_features_against_host(x86_def_t *guest_def)
int rv, i;
struct model_features_t ft[] = {
{&guest_def->features, &host_def.features,
- ~0, feature_name, 0x00000000},
+ ~0, feature_name, 0x00000001, R_EDX},
{&guest_def->ext_features, &host_def.ext_features,
- ~CPUID_EXT_HYPERVISOR, ext_feature_name, 0x00000001},
+ ~CPUID_EXT_HYPERVISOR, ext_feature_name, 0x00000001, R_ECX},
{&guest_def->ext2_features, &host_def.ext2_features,
- ~PPRO_FEATURES, ext2_feature_name, 0x80000000},
+ ~PPRO_FEATURES, ext2_feature_name, 0x80000001, R_EDX},
{&guest_def->ext3_features, &host_def.ext3_features,
- ~CPUID_EXT3_SVM, ext3_feature_name, 0x80000001}};
+ ~CPUID_EXT3_SVM, ext3_feature_name, 0x80000001, R_ECX}};
assert(kvm_enabled());
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 27c8d0c..ab81a5c 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -1221,4 +1221,7 @@ void cpu_report_tpr_access(CPUX86State *env, TPRAccess access);
void enable_kvm_pv_eoi(void);
void disable_kvm_mmu_op(void);
+/* Return name of 32-bit register, from a R_* constant */
+const char *get_register_name_32(unsigned int reg);
+
#endif /* CPU_I386_H */
--
1.7.11.7
9:37 a.m.
New subject: [libvirt] [PATCH 4/9] target-i386: check/enforce: Do not ignore "hypervisor" flag
We don't need any hack to ignore CPUID_EXT_HYPERVISOR anymore, because
kvm_arch_get_supported_cpuid() now set CPUID_EXT_HYPERVISOR properly.
So, this shouldn't introduce any behavior change, but it makes the code
simpler.
Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com>
---
My goal is to eliminate the check_feat field completely, as
kvm_arch_get_supported_cpuid() should now really return all the bits we
can set on all CPUID leaves.
---
target-i386/cpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 6c43ace..f1a21cf 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -965,7 +965,7 @@ static int kvm_check_features_against_host(x86_def_t *guest_def)
{&guest_def->features, &host_def.features,
~0, feature_name, 0x00000001, R_EDX},
{&guest_def->ext_features, &host_def.ext_features,
- ~CPUID_EXT_HYPERVISOR, ext_feature_name, 0x00000001, R_ECX},
+ ~0, ext_feature_name, 0x00000001, R_ECX},
{&guest_def->ext2_features, &host_def.ext2_features,
~PPRO_FEATURES, ext2_feature_name, 0x80000001, R_EDX},
{&guest_def->ext3_features, &host_def.ext3_features,
--
1.7.11.7
9:37 a.m.
New subject: [libvirt] [PATCH 5/9] target-i386: check/enforce: Check all CPUID.80000001H.EDX bits
I have no idea why PPRO_FEATURES was being ignored on the check of the
CPUID.80000001H.EDX bits. I believe it was a mistake, and it was
supposed to be ~(PPRO_FEATURES & CPUID_EXT2_AMD_ALIASES) or just
~CPUID_EXT2_AMD_ALIASES, because some time ago kvm_cpu_fill_host() used
the CPUID instruction directly (instead of
kvm_arch_get_supported_cpuid()).
But now kvm_cpu_fill_host() use kvm_arch_get_supported_cpuid(), and
kvm_arch_get_supported_cpuid() returns all supported bits for
CPUID.80000001H.EDX, even the AMD aliases (that are explicitly copied
from CPUID.01H.EDX), so we can make the code check/enforce all the
CPUID.80000001H.EDX bits.
Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com>
---
target-i386/cpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index f1a21cf..13075c7 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -967,7 +967,7 @@ static int kvm_check_features_against_host(x86_def_t *guest_def)
{&guest_def->ext_features, &host_def.ext_features,
~0, ext_feature_name, 0x00000001, R_ECX},
{&guest_def->ext2_features, &host_def.ext2_features,
- ~PPRO_FEATURES, ext2_feature_name, 0x80000001, R_EDX},
+ ~0, ext2_feature_name, 0x80000001, R_EDX},
{&guest_def->ext3_features, &host_def.ext3_features,
~CPUID_EXT3_SVM, ext3_feature_name, 0x80000001, R_ECX}};
--
1.7.11.7
9:37 a.m.
New subject: [libvirt] [PATCH 6/9] target-i386: check/enforce: Check SVM flag support as well
When nested SVM is supported, the kernel returns the SVM flag on
GET_SUPPORTED_CPUID[1], so we can check the SVM flag safely on
kvm_check_features_against_host().
I don't know why the original code ignored the SVM flag. Maybe it was
because kvm_cpu_fill_host() used the CPUID instruction directly instead
of GET_SUPPORTED_CPUID
[1] Older kernels (before v2.6.37) returned the SVM flag even if nested
SVM was _not_ supported. So the only cases where this patch should
change behavior is when SVM is being requested by the user or the
CPU model, but not supported by the host. And on these cases we
really want QEMU to abort if the "enforce" option is set.
Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com>
---
Cc: Joerg Roedel <joro(a)8bytes.org>
Cc: kvm(a)vger.kernel.org
Cc: libvir-list(a)redhat.com
Cc: Jiri Denemark <jdenemar(a)redhat.com>
I'm CCing libvirt people in case having SVM enabled by default may cause
trouble when libvirt starts using the "enforce" flag. I don't know if
libvirt expects most of the QEMU CPU models to have nested SVM enabled.
---
target-i386/cpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 13075c7..21aceed 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -969,7 +969,7 @@ static int kvm_check_features_against_host(x86_def_t *guest_def)
{&guest_def->ext2_features, &host_def.ext2_features,
~0, ext2_feature_name, 0x80000001, R_EDX},
{&guest_def->ext3_features, &host_def.ext3_features,
- ~CPUID_EXT3_SVM, ext3_feature_name, 0x80000001, R_ECX}};
+ ~0, ext3_feature_name, 0x80000001, R_ECX}};
assert(kvm_enabled());
--
1.7.11.7
9:37 a.m.
New subject: [libvirt] [PATCH 7/9] target-i386: check/enforce: Eliminate check_feat field
Now that all entries have check_feat=~0 on
kvm_check_features_against_host(), we can eliminate check_feat entirely
and make the code check all bits.
This patch shouldn't introduce any behavior change, as check_feat is set
to ~0 on all entries.
Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com>
---
target-i386/cpu.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 21aceed..05134a8 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -147,7 +147,6 @@ const char *get_register_name_32(unsigned int reg)
typedef struct model_features_t {
uint32_t *guest_feat;
uint32_t *host_feat;
- uint32_t check_feat;
const char **flag_names;
uint32_t cpuid;
int reg;
@@ -951,8 +950,7 @@ static int unavailable_host_feature(struct model_features_t *f,
uint32_t mask)
}
/* best effort attempt to inform user requested cpu flags aren't making
- * their way to the guest. Note: ft[].check_feat ideally should be
- * specified via a guest_def field to suppress report of extraneous flags.
+ * their way to the guest.
*
* This function may be called only if KVM is enabled.
*/
@@ -963,20 +961,20 @@ static int kvm_check_features_against_host(x86_def_t *guest_def)
int rv, i;
struct model_features_t ft[] = {
{&guest_def->features, &host_def.features,
- ~0, feature_name, 0x00000001, R_EDX},
+ feature_name, 0x00000001, R_EDX},
{&guest_def->ext_features, &host_def.ext_features,
- ~0, ext_feature_name, 0x00000001, R_ECX},
+ ext_feature_name, 0x00000001, R_ECX},
{&guest_def->ext2_features, &host_def.ext2_features,
- ~0, ext2_feature_name, 0x80000001, R_EDX},
+ ext2_feature_name, 0x80000001, R_EDX},
{&guest_def->ext3_features, &host_def.ext3_features,
- ~0, ext3_feature_name, 0x80000001, R_ECX}};
+ ext3_feature_name, 0x80000001, R_ECX}};
assert(kvm_enabled());
kvm_cpu_fill_host(&host_def);
for (rv = 0, i = 0; i < ARRAY_SIZE(ft); ++i)
for (mask = 1; mask; mask <<= 1)
- if (ft[i].check_feat & mask && *ft[i].guest_feat & mask
&&
+ if (*ft[i].guest_feat & mask &&
!(*ft[i].host_feat & mask)) {
unavailable_host_feature(&ft[i], mask);
rv = 1;
--
1.7.11.7
9:37 a.m.
New subject: [libvirt] [PATCH 8/9] target-i386: Call kvm_check_features_against_host() only if CONFIG_KVM is set
This will be necessary once kvm_check_features_against_host() starts
using KVM-specific definitions (so it won't compile anymore if
CONFIG_KVM is not set).
Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com>
---
target-i386/cpu.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 05134a8..02c00bf 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -933,6 +933,7 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
#endif /* CONFIG_KVM */
}
+#ifdef CONFIG_KVM
static int unavailable_host_feature(struct model_features_t *f, uint32_t mask)
{
int i;
@@ -981,6 +982,7 @@ static int kvm_check_features_against_host(x86_def_t *guest_def)
}
return rv;
}
+#endif
static void x86_cpuid_version_get_family(Object *obj, Visitor *v, void *opaque,
const char *name, Error **errp)
@@ -1404,10 +1406,12 @@ static int cpu_x86_parse_featurestr(x86_def_t *x86_cpu_def, char
*features)
x86_cpu_def->kvm_features &= ~minus_kvm_features;
x86_cpu_def->svm_features &= ~minus_svm_features;
x86_cpu_def->cpuid_7_0_ebx_features &= ~minus_7_0_ebx_features;
+#ifdef CONFIG_KVM
if (check_cpuid && kvm_enabled()) {
if (kvm_check_features_against_host(x86_cpu_def) && enforce_cpuid)
goto error;
}
+#endif
return 0;
error:
--
1.7.11.7
9:37 a.m.
New subject: [libvirt] [PATCH 9/9] target-i386: check/enforce: Check all feature words
This adds the following feature words to the list of flags to be checked
by kvm_check_features_against_host():
- cpuid_7_0_ebx_features
- ext4_features
- kvm_features
- svm_features
This will ensure the "enforce" flag works as it should: it won't allow
QEMU to be started unless every flag that was requested by the user or
defined in the CPU model is supported by the host.
This patch may cause existing configurations where "enforce" wasn't
preventing QEMU from being started to abort QEMU. But that's exactly the
point of this patch: if a flag was not supported by the host and QEMU
wasn't aborting, it was a bug in the "enforce" code.
Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com>
---
Cc: Gleb Natapov <gleb(a)redhat.com>
Cc: Marcelo Tosatti <mtosatti(a)redhat.com>
Cc: kvm(a)vger.kernel.org
Cc: libvir-list(a)redhat.com
Cc: Jiri Denemark <jdenemar(a)redhat.com>
CCing libvirt people, as this is directly related to the planned usage
of the "enforce" flag by libvirt.
The libvirt team probably has a problem in their hands: libvirt should
use "enforce" to make sure all requested flags are making their way into
the guest (so the resulting CPU is always the same, on any host), but
users may have existing working configurations where a flag is not
supported by the guest and the user really doesn't care about it. Those
configurations will necessarily break when libvirt starts using
"enforce".
One example where it may cause trouble for common setups: pc-1.3 wants
the kvm_pv_eoi flag enabled by default (so "enforce" will make sure it
is enabled), but the user may have an existing VM running on a host
without pv_eoi support. That setup is unsafe today because
live-migration between different host kernel versions may enable/disable
pv_eoi silently (that's why we need the "enforce" flag to be used by
libvirt), but the user probably would like to be able to live-migrate
that VM anyway (and have libvirt to "just do the right thing").
One possible solution to libvirt is to use "enforce" only on newer
machine-types, so existing machines with older machine-types will keep
the unsafe host-dependent-ABI behavior, but at least would keep
live-migration working in case the user is careful.
I really don't know what the libvirt team prefers, but that's the
situation today. The longer we take to make "enforce" strict as it
should and make libvirt finally use it, more users will have VMs with
migration-unsafe unpredictable guest ABIs.
---
target-i386/cpu.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 02c00bf..543ca34 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -950,8 +950,9 @@ static int unavailable_host_feature(struct model_features_t *f,
uint32_t mask)
return 0;
}
-/* best effort attempt to inform user requested cpu flags aren't making
- * their way to the guest.
+/* Check if all requested cpu flags are making their way to the guest
+ *
+ * Returns 0 if all flags are supported by the host, non-zero otherwise.
*
* This function may be called only if KVM is enabled.
*/
@@ -968,7 +969,16 @@ static int kvm_check_features_against_host(x86_def_t *guest_def)
{&guest_def->ext2_features, &host_def.ext2_features,
ext2_feature_name, 0x80000001, R_EDX},
{&guest_def->ext3_features, &host_def.ext3_features,
- ext3_feature_name, 0x80000001, R_ECX}};
+ ext3_feature_name, 0x80000001, R_ECX},
+ {&guest_def->ext4_features, &host_def.ext4_features,
+ NULL, 0xC0000001, R_EDX},
+ {&guest_def->cpuid_7_0_ebx_features,
&host_def.cpuid_7_0_ebx_features,
+ cpuid_7_0_ebx_feature_name, 7, R_EBX},
+ {&guest_def->svm_features, &host_def.svm_features,
+ svm_feature_name, 0x8000000A, R_EDX},
+ {&guest_def->kvm_features, &host_def.kvm_features,
+ kvm_feature_name, KVM_CPUID_FEATURES, R_EAX},
+ };
assert(kvm_enabled());
--
1.7.11.7
2:21 p.m.
Hi,
This is an automated message generated from the QEMU Patches.
Thank you for submitting this patch. This patch no longer applies to qemu.git.
This may have occurred due to:
1) Changes in mainline requiring your patch to be rebased and re-tested.
2) Sending the mail using a tool other than git-send-email. Please use
git-send-email to send patches to QEMU.
3) Basing this patch off of a branch that isn't tracking the QEMU
master branch. If that was done purposefully, please include the name
of the tree in the subject line in the future to prevent this message.
For instance: "[PATCH block-next 1/10] qcow3: add fancy new feature"
4) You no longer wish for this patch to be applied to QEMU. No additional
action is required on your part.
Nacked-by: QEMU Patches <aliguori(a)us.ibm.com>
Below is the output from git-am:
Applying: target-i386: kvm: -cpu host: Use GET_SUPPORTED_CPUID for SVM features
Applying: target-i386: kvm: Enable all supported KVM features for -cpu host
Applying: target-i386: check/enforce: Fix CPUID leaf numbers on error messages
fatal: sha1 information is lacking or useless (target-i386/cpu.c).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0003 target-i386: check/enforce: Fix CPUID leaf numbers on error
messages
The copy of the patch that failed is found in:
/home/aliguori/.patches/qemu-working/.git/rebase-apply/patch
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".
3 p.m.
Hi,
This is an automated message generated from the QEMU Patches.
Thank you for submitting this patch. This patch no longer applies to qemu.git.
This may have occurred due to:
1) Changes in mainline requiring your patch to be rebased and re-tested.
2) Sending the mail using a tool other than git-send-email. Please use
git-send-email to send patches to QEMU.
3) Basing this patch off of a branch that isn't tracking the QEMU
master branch. If that was done purposefully, please include the name
of the tree in the subject line in the future to prevent this message.
For instance: "[PATCH block-next 1/10] qcow3: add fancy new feature"
4) You no longer wish for this patch to be applied to QEMU. No additional
action is required on your part.
Nacked-by: QEMU Patches <aliguori(a)us.ibm.com>
Below is the output from git-am:
Applying: target-i386: kvm: -cpu host: Use GET_SUPPORTED_CPUID for SVM features
Applying: target-i386: kvm: Enable all supported KVM features for -cpu host
Applying: target-i386: check/enforce: Fix CPUID leaf numbers on error messages
fatal: sha1 information is lacking or useless (target-i386/cpu.c).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0003 target-i386: check/enforce: Fix CPUID leaf numbers on error
messages
The copy of the patch that failed is found in:
/home/aliguori/.patches/git-working/.git/rebase-apply/patch
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".
4341
days inactive
4341
days old
12 comments
3 participants
participants (3)
-
Anthony Liguori -
Blue Swirl -
Eduardo Habkost