[libvirt] [PATCH qom-cpu 0/7] disable kvm_mmu + -cpu "enforce" fixes (v3)

Changes on v3: - Patches 3-9 from v2 are now already on qom-cpu tree - Remove CONFIG_KVM #ifdefs by declaring fake KVM_* #defines on sysemu/kvm.h - Refactor code that uses the feature word arrays (to make it easier to add a new feature name array) - Add feature name array for CPUID leaf 0xC0000001 Changes on v2: - Now both the kvm_mmu-disable and -cpu "enforce" changes are on the same series - Coding style fixes Git tree for reference: git://github.com/ehabkost/qemu-hacks.git cpu-enforce-all.v3 https://github.com/ehabkost/qemu-hacks/tree/cpu-enforce-all.v3 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. Eduardo Habkost (7): kvm: Add fake KVM constants to avoid #ifdefs on KVM-specific code target-i386: Don't set any KVM flag by default if KVM is disabled target-i386: Disable kvm_mmu by default target-i386/cpu: Introduce FeatureWord typedefs target-i386: kvm_check_features_against_host(): Use feature_word_info target-i386/cpu.c: Add feature name array for ext4_features target-i386: check/enforce: Check all feature words include/sysemu/kvm.h | 14 ++++ target-i386/cpu.c | 193 ++++++++++++++++++++++++++++++++------------------- target-i386/cpu.h | 15 ++++ 3 files changed, 150 insertions(+), 72 deletions(-) -- 1.7.11.7

Any KVM-specific code that use these constants must check if kvm_enabled() is true before using them. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- Cc: kvm@vger.kernel.org Cc: Michael S. Tsirkin <mst@redhat.com> Cc: Gleb Natapov <gleb@redhat.com> Cc: Marcelo Tosatti <mtosatti@redhat.com> --- include/sysemu/kvm.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h index 3db19ff..15f9658 100644 --- a/include/sysemu/kvm.h +++ b/include/sysemu/kvm.h @@ -21,6 +21,20 @@ #ifdef CONFIG_KVM #include <linux/kvm.h> #include <linux/kvm_para.h> +#else +/* These constants must never be used at runtime if kvm_enabled() is false. + * They exist so we don't need #ifdefs around KVM-specific code that already + * checks kvm_enabled() properly. + */ +#define KVM_CPUID_SIGNATURE 0 +#define KVM_CPUID_FEATURES 0 +#define KVM_FEATURE_CLOCKSOURCE 0 +#define KVM_FEATURE_NOP_IO_DELAY 0 +#define KVM_FEATURE_MMU_OP 0 +#define KVM_FEATURE_CLOCKSOURCE2 0 +#define KVM_FEATURE_ASYNC_PF 0 +#define KVM_FEATURE_STEAL_TIME 0 +#define KVM_FEATURE_PV_EOI 0 #endif extern int kvm_allowed; -- 1.7.11.7

This is a cleanup that tries to solve two small issues: - We don't need a separate kvm_pv_eoi_features variable just to keep a constant calculated at compile-time, and this style would require adding a separate variable (that's declared twice because of the CONFIG_KVM ifdef) for each feature that's going to be enabled/disable by machine-type compat code. - The pc-1.3 code is setting the kvm_pv_eoi flag on cpuid_kvm_features even when KVM is disabled at runtime. This small incosistency in the cpuid_kvm_features field isn't a problem today because cpuid_kvm_features is ignored by the TCG code, but it may cause unexpected problems later when refactoring the CPUID handling code. This patch eliminates the kvm_pv_eoi_features variable and simply uses kvm_enabled() inside the enable_kvm_pv_eoi() compat function, so it enables kvm_pv_eoi only if KVM is enabled. I believe this makes the behavior of enable_kvm_pv_eoi() clearer and easier to understand. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- Cc: kvm@vger.kernel.org Cc: Michael S. Tsirkin <mst@redhat.com> Cc: Gleb Natapov <gleb@redhat.com> Cc: Marcelo Tosatti <mtosatti@redhat.com> Changes v2: - Coding style fix Changes v3: - Eliminate #ifdef by using the fake KVM_FEATURE_PV_EOI #define --- target-i386/cpu.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 951e206..40400ac 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -164,15 +164,15 @@ static uint32_t kvm_default_features = (1 << KVM_FEATURE_CLOCKSOURCE) | (1 << KVM_FEATURE_ASYNC_PF) | (1 << KVM_FEATURE_STEAL_TIME) | (1 << KVM_FEATURE_CLOCKSOURCE_STABLE_BIT); -static const uint32_t kvm_pv_eoi_features = (0x1 << KVM_FEATURE_PV_EOI); #else static uint32_t kvm_default_features = 0; -static const uint32_t kvm_pv_eoi_features = 0; #endif void enable_kvm_pv_eoi(void) { - kvm_default_features |= kvm_pv_eoi_features; + if (kvm_enabled()) { + kvm_default_features |= (1UL << KVM_FEATURE_PV_EOI); + } } void host_cpuid(uint32_t function, uint32_t count, -- 1.7.11.7

On Mon, 7 Jan 2013 16:20:43 -0200 Eduardo Habkost <ehabkost@redhat.com> wrote:
This is a cleanup that tries to solve two small issues:
- We don't need a separate kvm_pv_eoi_features variable just to keep a constant calculated at compile-time, and this style would require adding a separate variable (that's declared twice because of the CONFIG_KVM ifdef) for each feature that's going to be enabled/disable by machine-type compat code. - The pc-1.3 code is setting the kvm_pv_eoi flag on cpuid_kvm_features even when KVM is disabled at runtime. This small incosistency in the cpuid_kvm_features field isn't a problem today because cpuid_kvm_features is ignored by the TCG code, but it may cause unexpected problems later when refactoring the CPUID handling code.
This patch eliminates the kvm_pv_eoi_features variable and simply uses kvm_enabled() inside the enable_kvm_pv_eoi() compat function, so it enables kvm_pv_eoi only if KVM is enabled. I believe this makes the behavior of enable_kvm_pv_eoi() clearer and easier to understand.
Subj doesn't match what patch actually does. Have you meant "Don't set kvm_pv_eoi flag by default if KVM is disabled"? Although "eliminate kvm_pv_eoi_features variable" might better describe what patch is doing.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- Cc: kvm@vger.kernel.org Cc: Michael S. Tsirkin <mst@redhat.com> Cc: Gleb Natapov <gleb@redhat.com> Cc: Marcelo Tosatti <mtosatti@redhat.com>
Changes v2: - Coding style fix
Changes v3: - Eliminate #ifdef by using the fake KVM_FEATURE_PV_EOI #define --- target-i386/cpu.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 951e206..40400ac 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -164,15 +164,15 @@ static uint32_t kvm_default_features = (1 << KVM_FEATURE_CLOCKSOURCE) | (1 << KVM_FEATURE_ASYNC_PF) | (1 << KVM_FEATURE_STEAL_TIME) | (1 << KVM_FEATURE_CLOCKSOURCE_STABLE_BIT); -static const uint32_t kvm_pv_eoi_features = (0x1 << KVM_FEATURE_PV_EOI); #else static uint32_t kvm_default_features = 0; -static const uint32_t kvm_pv_eoi_features = 0; #endif
void enable_kvm_pv_eoi(void) { - kvm_default_features |= kvm_pv_eoi_features; + if (kvm_enabled()) { + kvm_default_features |= (1UL << KVM_FEATURE_PV_EOI); + } }
void host_cpuid(uint32_t function, uint32_t count,

On Wed, Jan 09, 2013 at 10:46:12AM +0100, Igor Mammedov wrote:
On Mon, 7 Jan 2013 16:20:43 -0200 Eduardo Habkost <ehabkost@redhat.com> wrote:
This is a cleanup that tries to solve two small issues:
- We don't need a separate kvm_pv_eoi_features variable just to keep a constant calculated at compile-time, and this style would require adding a separate variable (that's declared twice because of the CONFIG_KVM ifdef) for each feature that's going to be enabled/disable by machine-type compat code. - The pc-1.3 code is setting the kvm_pv_eoi flag on cpuid_kvm_features even when KVM is disabled at runtime. This small incosistency in the cpuid_kvm_features field isn't a problem today because cpuid_kvm_features is ignored by the TCG code, but it may cause unexpected problems later when refactoring the CPUID handling code.
This patch eliminates the kvm_pv_eoi_features variable and simply uses kvm_enabled() inside the enable_kvm_pv_eoi() compat function, so it enables kvm_pv_eoi only if KVM is enabled. I believe this makes the behavior of enable_kvm_pv_eoi() clearer and easier to understand.
Subj doesn't match what patch actually does. Have you meant "Don't set kvm_pv_eoi flag by default if KVM is disabled"? Although "eliminate kvm_pv_eoi_features variable" might better describe what patch is doing.
True. The other flags in kvm_default_features may be already set and will be copied to cpuid_kvm_features even if kvm_enabled() is false. I had a previous version of this patch that also changed the code using kvm_default_features to check kvm_enabled(), but I removed that part and kept only the kvm_pv_eoi_features variable removal. Andreas, please ignore this patch (it is not necessary anymore as this series doesn't include machine-type compatibility code for kvm_mmu). Patches 2-7 don't depend on this patch and should apply cleanly without it. I will send a new version later, probably with a separate patch to ignore kvm_default_features if kvm_enabled() is false (so there's no need to even check kvm_enabled() inside enable_kvm_pv_eoi()).
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- Cc: kvm@vger.kernel.org Cc: Michael S. Tsirkin <mst@redhat.com> Cc: Gleb Natapov <gleb@redhat.com> Cc: Marcelo Tosatti <mtosatti@redhat.com>
Changes v2: - Coding style fix
Changes v3: - Eliminate #ifdef by using the fake KVM_FEATURE_PV_EOI #define --- target-i386/cpu.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 951e206..40400ac 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -164,15 +164,15 @@ static uint32_t kvm_default_features = (1 << KVM_FEATURE_CLOCKSOURCE) | (1 << KVM_FEATURE_ASYNC_PF) | (1 << KVM_FEATURE_STEAL_TIME) | (1 << KVM_FEATURE_CLOCKSOURCE_STABLE_BIT); -static const uint32_t kvm_pv_eoi_features = (0x1 << KVM_FEATURE_PV_EOI); #else static uint32_t kvm_default_features = 0; -static const uint32_t kvm_pv_eoi_features = 0; #endif
void enable_kvm_pv_eoi(void) { - kvm_default_features |= kvm_pv_eoi_features; + if (kvm_enabled()) { + kvm_default_features |= (1UL << KVM_FEATURE_PV_EOI); + } }
void host_cpuid(uint32_t function, uint32_t count,
-- Eduardo

On Wed, Jan 09, 2013 at 09:41:37AM -0200, Eduardo Habkost wrote: [...]
Andreas, please ignore this patch (it is not necessary anymore as this series doesn't include machine-type compatibility code for kvm_mmu). Patches 2-7 don't depend on this patch and should apply cleanly without it.
I mean: patches 1 and 3-7 don't depend on this patch and can be applied cleanly without it. -- Eduardo

KVM_CAP_PV_MMU capability reporting was removed from the kernel since v2.6.33 (see commit a68a6a7282373), and was completely removed from the kernel since v3.3 (see commit fb92045843). It doesn't make sense to keep it enabled by default, as it would cause unnecessary hassle when using the "enforce" flag. This disables kvm_mmu on all machine-types. With this fix, the possible scenarios when migrating from QEMU <= 1.3 to QEMU 1.4 are; ------------+------------+---------------------------------------------------- src kernel | dst kernel | Result ------------+------------+----------------------------------------------------
= 2.6.33 | any | kvm_mmu was already disabled and will stay disabled <= 2.6.32 | >= 3.3 | correct live migration is impossible <= 2.6.32 | <= 3.2 | kvm_mmu will be disabled on next guest reboot * ------------+------------+----------------------------------------------------
* If they are running kernel <= 2.6.32 and want kvm_mmu to be kept enabled on guest reboot, they can explicitly add +kvm_mmu to the QEMU command-line. Using 2.6.33 and higher, it is not possible to enable kvm_mmu explicitly anymore. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- Cc: kvm@vger.kernel.org Cc: Michael S. Tsirkin <mst@redhat.com> Cc: Gleb Natapov <gleb@redhat.com> Cc: Marcelo Tosatti <mtosatti@redhat.com> Cc: libvir-list@redhat.com Cc: Jiri Denemark <jdenemar@redhat.com> Changes v2: - Coding style fix - Removed redundant comments above machine init functions Changes v3: - Eliminate per-machine-type compatibility code --- target-i386/cpu.c | 1 - 1 file changed, 1 deletion(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 40400ac..b09b625 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -159,7 +159,6 @@ int enforce_cpuid = 0; #if defined(CONFIG_KVM) static uint32_t kvm_default_features = (1 << KVM_FEATURE_CLOCKSOURCE) | (1 << KVM_FEATURE_NOP_IO_DELAY) | - (1 << KVM_FEATURE_MMU_OP) | (1 << KVM_FEATURE_CLOCKSOURCE2) | (1 << KVM_FEATURE_ASYNC_PF) | (1 << KVM_FEATURE_STEAL_TIME) | -- 1.7.11.7

Am 07.01.2013 19:20, schrieb Eduardo Habkost:
KVM_CAP_PV_MMU capability reporting was removed from the kernel since v2.6.33 (see commit a68a6a7282373), and was completely removed from the kernel since v3.3 (see commit fb92045843). It doesn't make sense to keep it enabled by default, as it would cause unnecessary hassle when using the "enforce" flag.
This disables kvm_mmu on all machine-types. With this fix, the possible scenarios when migrating from QEMU <= 1.3 to QEMU 1.4 are;
------------+------------+---------------------------------------------------- src kernel | dst kernel | Result ------------+------------+----------------------------------------------------
= 2.6.33 | any | kvm_mmu was already disabled and will stay disabled <= 2.6.32 | >= 3.3 | correct live migration is impossible <= 2.6.32 | <= 3.2 | kvm_mmu will be disabled on next guest reboot * ------------+------------+----------------------------------------------------
When using ASCII art, please remember to use at most 76 characters, to avoid linewraps in git-log. Shortening the second column fixes this. Andreas
* If they are running kernel <= 2.6.32 and want kvm_mmu to be kept enabled on guest reboot, they can explicitly add +kvm_mmu to the QEMU command-line. Using 2.6.33 and higher, it is not possible to enable kvm_mmu explicitly anymore.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
-- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

This introduces a FeatureWord enum, FeatureWordInfo struct (with generation information about a feature word), and a FeatureWordArray typedef, and changes add_flagname_to_bitmaps() code and cpu_x86_parse_featurestr() to use the new typedefs instead of separate variables for each feature word. This will help us keep the code at kvm_check_features_against_host(), cpu_x86_parse_featurestr() and add_flagname_to_bitmaps() sane while adding new feature name arrays. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- target-i386/cpu.c | 97 +++++++++++++++++++++++++++---------------------------- target-i386/cpu.h | 15 +++++++++ 2 files changed, 63 insertions(+), 49 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index b09b625..7d62d48 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -124,6 +124,20 @@ static const char *cpuid_7_0_ebx_feature_name[] = { NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, }; +typedef struct FeatureWordInfo { + const char **feat_names; +} FeatureWordInfo; + +static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { + [FEAT_1_EDX] = { .feat_names = feature_name }, + [FEAT_1_ECX] = { .feat_names = ext_feature_name }, + [FEAT_8000_0001_EDX] = { .feat_names = ext2_feature_name }, + [FEAT_8000_0001_ECX] = { .feat_names = ext3_feature_name }, + [FEAT_KVM] = { .feat_names = kvm_feature_name }, + [FEAT_SVM] = { .feat_names = svm_feature_name }, + [FEAT_7_0_EBX] = { .feat_names = cpuid_7_0_ebx_feature_name }, +}; + const char *get_register_name_32(unsigned int reg) { static const char *reg_names[CPU_NB_REGS32] = { @@ -271,23 +285,20 @@ static bool lookup_feature(uint32_t *pval, const char *s, const char *e, return found; } -static void add_flagname_to_bitmaps(const char *flagname, uint32_t *features, - uint32_t *ext_features, - uint32_t *ext2_features, - uint32_t *ext3_features, - uint32_t *kvm_features, - uint32_t *svm_features, - uint32_t *cpuid_7_0_ebx_features) +static void add_flagname_to_bitmaps(const char *flagname, + FeatureWordArray words) { - if (!lookup_feature(features, flagname, NULL, feature_name) && - !lookup_feature(ext_features, flagname, NULL, ext_feature_name) && - !lookup_feature(ext2_features, flagname, NULL, ext2_feature_name) && - !lookup_feature(ext3_features, flagname, NULL, ext3_feature_name) && - !lookup_feature(kvm_features, flagname, NULL, kvm_feature_name) && - !lookup_feature(svm_features, flagname, NULL, svm_feature_name) && - !lookup_feature(cpuid_7_0_ebx_features, flagname, NULL, - cpuid_7_0_ebx_feature_name)) - fprintf(stderr, "CPU feature %s not found\n", flagname); + FeatureWord w; + for (w = 0; w < FEATURE_WORDS; w++) { + FeatureWordInfo *wi = &feature_word_info[w]; + if (wi->feat_names && + lookup_feature(&words[w], flagname, NULL, wi->feat_names)) { + break; + } + } + if (w == FEATURE_WORDS) { + fprintf(stderr, "CPU feature %s not found\n", flagname); + } } typedef struct x86_def_t { @@ -1256,35 +1267,23 @@ static int cpu_x86_parse_featurestr(x86_def_t *x86_cpu_def, char *features) unsigned int i; char *featurestr; /* Single 'key=value" string being parsed */ /* Features to be added */ - uint32_t plus_features = 0, plus_ext_features = 0; - uint32_t plus_ext2_features = 0, plus_ext3_features = 0; - uint32_t plus_kvm_features = kvm_default_features, plus_svm_features = 0; - uint32_t plus_7_0_ebx_features = 0; + FeatureWordArray plus_features = { + [FEAT_KVM] = kvm_default_features, + }; /* Features to be removed */ - uint32_t minus_features = 0, minus_ext_features = 0; - uint32_t minus_ext2_features = 0, minus_ext3_features = 0; - uint32_t minus_kvm_features = 0, minus_svm_features = 0; - uint32_t minus_7_0_ebx_features = 0; + FeatureWordArray minus_features = { 0 }; uint32_t numvalue; - add_flagname_to_bitmaps("hypervisor", &plus_features, - &plus_ext_features, &plus_ext2_features, &plus_ext3_features, - &plus_kvm_features, &plus_svm_features, &plus_7_0_ebx_features); + add_flagname_to_bitmaps("hypervisor", plus_features); featurestr = features ? strtok(features, ",") : NULL; while (featurestr) { char *val; if (featurestr[0] == '+') { - add_flagname_to_bitmaps(featurestr + 1, &plus_features, - &plus_ext_features, &plus_ext2_features, - &plus_ext3_features, &plus_kvm_features, - &plus_svm_features, &plus_7_0_ebx_features); + add_flagname_to_bitmaps(featurestr + 1, plus_features); } else if (featurestr[0] == '-') { - add_flagname_to_bitmaps(featurestr + 1, &minus_features, - &minus_ext_features, &minus_ext2_features, - &minus_ext3_features, &minus_kvm_features, - &minus_svm_features, &minus_7_0_ebx_features); + add_flagname_to_bitmaps(featurestr + 1, minus_features); } else if ((val = strchr(featurestr, '='))) { *val = 0; val++; if (!strcmp(featurestr, "family")) { @@ -1384,20 +1383,20 @@ static int cpu_x86_parse_featurestr(x86_def_t *x86_cpu_def, char *features) } featurestr = strtok(NULL, ","); } - x86_cpu_def->features |= plus_features; - x86_cpu_def->ext_features |= plus_ext_features; - x86_cpu_def->ext2_features |= plus_ext2_features; - x86_cpu_def->ext3_features |= plus_ext3_features; - x86_cpu_def->kvm_features |= plus_kvm_features; - x86_cpu_def->svm_features |= plus_svm_features; - x86_cpu_def->cpuid_7_0_ebx_features |= plus_7_0_ebx_features; - x86_cpu_def->features &= ~minus_features; - x86_cpu_def->ext_features &= ~minus_ext_features; - x86_cpu_def->ext2_features &= ~minus_ext2_features; - x86_cpu_def->ext3_features &= ~minus_ext3_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; + x86_cpu_def->features |= plus_features[FEAT_1_EDX]; + x86_cpu_def->ext_features |= plus_features[FEAT_1_ECX]; + x86_cpu_def->ext2_features |= plus_features[FEAT_8000_0001_EDX]; + x86_cpu_def->ext3_features |= plus_features[FEAT_8000_0001_ECX]; + x86_cpu_def->kvm_features |= plus_features[FEAT_KVM]; + x86_cpu_def->svm_features |= plus_features[FEAT_SVM]; + x86_cpu_def->cpuid_7_0_ebx_features |= plus_features[FEAT_7_0_EBX]; + x86_cpu_def->features &= ~minus_features[FEAT_1_EDX]; + x86_cpu_def->ext_features &= ~minus_features[FEAT_1_ECX]; + x86_cpu_def->ext2_features &= ~minus_features[FEAT_8000_0001_EDX]; + x86_cpu_def->ext3_features &= ~minus_features[FEAT_8000_0001_ECX]; + x86_cpu_def->kvm_features &= ~minus_features[FEAT_KVM]; + x86_cpu_def->svm_features &= ~minus_features[FEAT_SVM]; + x86_cpu_def->cpuid_7_0_ebx_features &= ~minus_features[FEAT_7_0_EBX]; if (check_cpuid && kvm_enabled()) { if (kvm_check_features_against_host(x86_cpu_def) && enforce_cpuid) goto error; diff --git a/target-i386/cpu.h b/target-i386/cpu.h index e56921b..e4a7c50 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -361,6 +361,21 @@ #define MSR_VM_HSAVE_PA 0xc0010117 +/* CPUID feature words */ +typedef enum FeatureWord { + FEAT_1_EDX, /* CPUID[1].EDX */ + FEAT_1_ECX, /* CPUID[1].ECX */ + FEAT_7_0_EBX, /* CPUID[EAX=7,ECX=0].EBX */ + FEAT_8000_0001_EDX, /* CPUID[8000_0001].EDX */ + FEAT_8000_0001_ECX, /* CPUID[8000_0001].ECX */ + FEAT_C000_0001_EDX, /* CPUID[C000_0001].EDX */ + FEAT_KVM, /* CPUID[4000_0001].EAX (KVM_CPUID_FEATURES) */ + FEAT_SVM, /* CPUID[8000_000A].EDX */ + FEATURE_WORDS, +} FeatureWord; + +typedef uint32_t FeatureWordArray[FEATURE_WORDS]; + /* cpuid_features bits */ #define CPUID_FP87 (1 << 0) #define CPUID_VME (1 << 1) -- 1.7.11.7

On Mon, Jan 07, 2013 at 04:20:45PM -0200, Eduardo Habkost wrote:
This introduces a FeatureWord enum, FeatureWordInfo struct (with generation information about a feature word), and a FeatureWordArray typedef, and changes add_flagname_to_bitmaps() code and cpu_x86_parse_featurestr() to use the new typedefs instead of separate variables for each feature word.
This will help us keep the code at kvm_check_features_against_host(), cpu_x86_parse_featurestr() and add_flagname_to_bitmaps() sane while adding new feature name arrays.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- target-i386/cpu.c | 97 +++++++++++++++++++++++++++---------------------------- target-i386/cpu.h | 15 +++++++++ 2 files changed, 63 insertions(+), 49 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index b09b625..7d62d48 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -124,6 +124,20 @@ static const char *cpuid_7_0_ebx_feature_name[] = { NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, };
+typedef struct FeatureWordInfo { + const char **feat_names; +} FeatureWordInfo; + +static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { + [FEAT_1_EDX] = { .feat_names = feature_name }, + [FEAT_1_ECX] = { .feat_names = ext_feature_name }, + [FEAT_8000_0001_EDX] = { .feat_names = ext2_feature_name }, + [FEAT_8000_0001_ECX] = { .feat_names = ext3_feature_name }, + [FEAT_KVM] = { .feat_names = kvm_feature_name }, + [FEAT_SVM] = { .feat_names = svm_feature_name }, + [FEAT_7_0_EBX] = { .feat_names = cpuid_7_0_ebx_feature_name }, +}; + const char *get_register_name_32(unsigned int reg) { static const char *reg_names[CPU_NB_REGS32] = { @@ -271,23 +285,20 @@ static bool lookup_feature(uint32_t *pval, const char *s, const char *e, return found; }
-static void add_flagname_to_bitmaps(const char *flagname, uint32_t *features, - uint32_t *ext_features, - uint32_t *ext2_features, - uint32_t *ext3_features, - uint32_t *kvm_features, - uint32_t *svm_features, - uint32_t *cpuid_7_0_ebx_features) +static void add_flagname_to_bitmaps(const char *flagname, + FeatureWordArray words) { - if (!lookup_feature(features, flagname, NULL, feature_name) && - !lookup_feature(ext_features, flagname, NULL, ext_feature_name) && - !lookup_feature(ext2_features, flagname, NULL, ext2_feature_name) && - !lookup_feature(ext3_features, flagname, NULL, ext3_feature_name) && - !lookup_feature(kvm_features, flagname, NULL, kvm_feature_name) && - !lookup_feature(svm_features, flagname, NULL, svm_feature_name) && - !lookup_feature(cpuid_7_0_ebx_features, flagname, NULL, - cpuid_7_0_ebx_feature_name)) - fprintf(stderr, "CPU feature %s not found\n", flagname); + FeatureWord w; + for (w = 0; w < FEATURE_WORDS; w++) { + FeatureWordInfo *wi = &feature_word_info[w]; + if (wi->feat_names && I would move patch 6 before that one and drop this check here, but if you disagree it is your call.
+ lookup_feature(&words[w], flagname, NULL, wi->feat_names)) { + break; + } + } + if (w == FEATURE_WORDS) { + fprintf(stderr, "CPU feature %s not found\n", flagname); + } }
typedef struct x86_def_t { @@ -1256,35 +1267,23 @@ static int cpu_x86_parse_featurestr(x86_def_t *x86_cpu_def, char *features) unsigned int i; char *featurestr; /* Single 'key=value" string being parsed */ /* Features to be added */ - uint32_t plus_features = 0, plus_ext_features = 0; - uint32_t plus_ext2_features = 0, plus_ext3_features = 0; - uint32_t plus_kvm_features = kvm_default_features, plus_svm_features = 0; - uint32_t plus_7_0_ebx_features = 0; + FeatureWordArray plus_features = { + [FEAT_KVM] = kvm_default_features, + }; /* Features to be removed */ - uint32_t minus_features = 0, minus_ext_features = 0; - uint32_t minus_ext2_features = 0, minus_ext3_features = 0; - uint32_t minus_kvm_features = 0, minus_svm_features = 0; - uint32_t minus_7_0_ebx_features = 0; + FeatureWordArray minus_features = { 0 }; uint32_t numvalue;
- add_flagname_to_bitmaps("hypervisor", &plus_features, - &plus_ext_features, &plus_ext2_features, &plus_ext3_features, - &plus_kvm_features, &plus_svm_features, &plus_7_0_ebx_features); + add_flagname_to_bitmaps("hypervisor", plus_features);
featurestr = features ? strtok(features, ",") : NULL;
while (featurestr) { char *val; if (featurestr[0] == '+') { - add_flagname_to_bitmaps(featurestr + 1, &plus_features, - &plus_ext_features, &plus_ext2_features, - &plus_ext3_features, &plus_kvm_features, - &plus_svm_features, &plus_7_0_ebx_features); + add_flagname_to_bitmaps(featurestr + 1, plus_features); } else if (featurestr[0] == '-') { - add_flagname_to_bitmaps(featurestr + 1, &minus_features, - &minus_ext_features, &minus_ext2_features, - &minus_ext3_features, &minus_kvm_features, - &minus_svm_features, &minus_7_0_ebx_features); + add_flagname_to_bitmaps(featurestr + 1, minus_features); } else if ((val = strchr(featurestr, '='))) { *val = 0; val++; if (!strcmp(featurestr, "family")) { @@ -1384,20 +1383,20 @@ static int cpu_x86_parse_featurestr(x86_def_t *x86_cpu_def, char *features) } featurestr = strtok(NULL, ","); } - x86_cpu_def->features |= plus_features; - x86_cpu_def->ext_features |= plus_ext_features; - x86_cpu_def->ext2_features |= plus_ext2_features; - x86_cpu_def->ext3_features |= plus_ext3_features; - x86_cpu_def->kvm_features |= plus_kvm_features; - x86_cpu_def->svm_features |= plus_svm_features; - x86_cpu_def->cpuid_7_0_ebx_features |= plus_7_0_ebx_features; - x86_cpu_def->features &= ~minus_features; - x86_cpu_def->ext_features &= ~minus_ext_features; - x86_cpu_def->ext2_features &= ~minus_ext2_features; - x86_cpu_def->ext3_features &= ~minus_ext3_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; + x86_cpu_def->features |= plus_features[FEAT_1_EDX]; + x86_cpu_def->ext_features |= plus_features[FEAT_1_ECX]; + x86_cpu_def->ext2_features |= plus_features[FEAT_8000_0001_EDX]; + x86_cpu_def->ext3_features |= plus_features[FEAT_8000_0001_ECX]; + x86_cpu_def->kvm_features |= plus_features[FEAT_KVM]; + x86_cpu_def->svm_features |= plus_features[FEAT_SVM]; + x86_cpu_def->cpuid_7_0_ebx_features |= plus_features[FEAT_7_0_EBX]; + x86_cpu_def->features &= ~minus_features[FEAT_1_EDX]; + x86_cpu_def->ext_features &= ~minus_features[FEAT_1_ECX]; + x86_cpu_def->ext2_features &= ~minus_features[FEAT_8000_0001_EDX]; + x86_cpu_def->ext3_features &= ~minus_features[FEAT_8000_0001_ECX]; + x86_cpu_def->kvm_features &= ~minus_features[FEAT_KVM]; + x86_cpu_def->svm_features &= ~minus_features[FEAT_SVM]; + x86_cpu_def->cpuid_7_0_ebx_features &= ~minus_features[FEAT_7_0_EBX]; if (check_cpuid && kvm_enabled()) { if (kvm_check_features_against_host(x86_cpu_def) && enforce_cpuid) goto error; diff --git a/target-i386/cpu.h b/target-i386/cpu.h index e56921b..e4a7c50 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -361,6 +361,21 @@
#define MSR_VM_HSAVE_PA 0xc0010117
+/* CPUID feature words */ +typedef enum FeatureWord { + FEAT_1_EDX, /* CPUID[1].EDX */ + FEAT_1_ECX, /* CPUID[1].ECX */ + FEAT_7_0_EBX, /* CPUID[EAX=7,ECX=0].EBX */ + FEAT_8000_0001_EDX, /* CPUID[8000_0001].EDX */ + FEAT_8000_0001_ECX, /* CPUID[8000_0001].ECX */ + FEAT_C000_0001_EDX, /* CPUID[C000_0001].EDX */ + FEAT_KVM, /* CPUID[4000_0001].EAX (KVM_CPUID_FEATURES) */ + FEAT_SVM, /* CPUID[8000_000A].EDX */ + FEATURE_WORDS, +} FeatureWord; + +typedef uint32_t FeatureWordArray[FEATURE_WORDS]; + /* cpuid_features bits */ #define CPUID_FP87 (1 << 0) #define CPUID_VME (1 << 1) -- 1.7.11.7
-- Gleb.

Instead of carrying the CPUID leaf/register and feature name array on the model_features_t struct, move that information into feature_word_info so it can be reused by other functions. The goal is to eventually kill model_features_t entirely, but to do that we have to either convert x86_def_t.features to an array or use offsetof() inside FeatureWordInfo (to replace the pointers inside model_features_t). So by now just move most of the model_features_t fields to FeatureWordInfo except for the two pointers to local arguments. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- target-i386/cpu.c | 73 +++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 49 insertions(+), 24 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 7d62d48..4b3ee63 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -126,16 +126,39 @@ static const char *cpuid_7_0_ebx_feature_name[] = { typedef struct FeatureWordInfo { const char **feat_names; + uint32_t cpuid_eax; /* Input EAX for CPUID */ + int cpuid_reg; /* R_* register constant */ } FeatureWordInfo; static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { - [FEAT_1_EDX] = { .feat_names = feature_name }, - [FEAT_1_ECX] = { .feat_names = ext_feature_name }, - [FEAT_8000_0001_EDX] = { .feat_names = ext2_feature_name }, - [FEAT_8000_0001_ECX] = { .feat_names = ext3_feature_name }, - [FEAT_KVM] = { .feat_names = kvm_feature_name }, - [FEAT_SVM] = { .feat_names = svm_feature_name }, - [FEAT_7_0_EBX] = { .feat_names = cpuid_7_0_ebx_feature_name }, + [FEAT_1_EDX] = { + .feat_names = feature_name, + .cpuid_eax = 1, .cpuid_reg = R_EDX, + }, + [FEAT_1_ECX] = { + .feat_names = ext_feature_name, + .cpuid_eax = 1, .cpuid_reg = R_ECX, + }, + [FEAT_8000_0001_EDX] = { + .feat_names = ext2_feature_name, + .cpuid_eax = 0x80000001, .cpuid_reg = R_EDX, + }, + [FEAT_8000_0001_ECX] = { + .feat_names = ext3_feature_name, + .cpuid_eax = 0x80000001, .cpuid_reg = R_ECX, + }, + [FEAT_KVM] = { + .feat_names = kvm_feature_name, + .cpuid_eax = KVM_CPUID_FEATURES, .cpuid_reg = R_EAX, + }, + [FEAT_SVM] = { + .feat_names = svm_feature_name, + .cpuid_eax = 0x8000000A, .cpuid_reg = R_EDX, + }, + [FEAT_7_0_EBX] = { + .feat_names = cpuid_7_0_ebx_feature_name, + .cpuid_eax = 7, .cpuid_reg = R_EBX, + }, }; const char *get_register_name_32(unsigned int reg) @@ -162,9 +185,7 @@ const char *get_register_name_32(unsigned int reg) typedef struct model_features_t { uint32_t *guest_feat; uint32_t *host_feat; - const char **flag_names; - uint32_t cpuid; - int reg; + FeatureWord feat_word; } model_features_t; int check_cpuid = 0; @@ -935,19 +956,19 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def) #endif /* CONFIG_KVM */ } -static int unavailable_host_feature(struct model_features_t *f, uint32_t mask) +static int unavailable_host_feature(FeatureWordInfo *f, uint32_t mask) { int i; for (i = 0; i < 32; ++i) if (1 << i & mask) { - const char *reg = get_register_name_32(f->reg); + const char *reg = get_register_name_32(f->cpuid_reg); assert(reg); fprintf(stderr, "warning: host doesn't support requested feature: " "CPUID.%02XH:%s%s%s [bit %d]\n", - f->cpuid, reg, - f->flag_names[i] ? "." : "", - f->flag_names[i] ? f->flag_names[i] : "", i); + f->cpuid_eax, reg, + f->feat_names[i] ? "." : "", + f->feat_names[i] ? f->feat_names[i] : "", i); break; } return 0; @@ -965,25 +986,29 @@ 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, - feature_name, 0x00000001, R_EDX}, + FEAT_1_EDX }, {&guest_def->ext_features, &host_def.ext_features, - ext_feature_name, 0x00000001, R_ECX}, + FEAT_1_ECX }, {&guest_def->ext2_features, &host_def.ext2_features, - ext2_feature_name, 0x80000001, R_EDX}, + FEAT_8000_0001_EDX }, {&guest_def->ext3_features, &host_def.ext3_features, - ext3_feature_name, 0x80000001, R_ECX} + FEAT_8000_0001_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) + for (rv = 0, i = 0; i < ARRAY_SIZE(ft); ++i) { + FeatureWord w = ft[i].feat_word; + FeatureWordInfo *wi = &feature_word_info[w]; + for (mask = 1; mask; mask <<= 1) { if (*ft[i].guest_feat & mask && !(*ft[i].host_feat & mask)) { - unavailable_host_feature(&ft[i], mask); - rv = 1; - } + unavailable_host_feature(wi, mask); + rv = 1; + } + } + } return rv; } -- 1.7.11.7

Feature names were taken from the X86_FEATURE_* constants in the Linux kernel code. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- Cc: Gleb Natapov <gleb@redhat.com> --- target-i386/cpu.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 4b3ee63..a54c464 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -95,6 +95,17 @@ static const char *ext3_feature_name[] = { NULL, NULL, NULL, NULL, }; +static const char *ext4_feature_name[] = { + NULL, NULL, "xstore","xstore-en", + NULL, NULL, "xcrypt","xcrypt-en", + "ace2", "ace2-en", "phe", "phe-en", + "pmm", "pmm-en", NULL, NULL, + NULL, NULL, NULL, NULL, + NULL, NULL, NULL, NULL, + NULL, NULL, NULL, NULL, + NULL, NULL, NULL, NULL, +}; + static const char *kvm_feature_name[] = { "kvmclock", "kvm_nopiodelay", "kvm_mmu", "kvmclock", "kvm_asyncpf", "kvm_steal_time", "kvm_pv_eoi", NULL, @@ -147,6 +158,10 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { .feat_names = ext3_feature_name, .cpuid_eax = 0x80000001, .cpuid_reg = R_ECX, }, + [FEAT_C000_0001_EDX] = { + .feat_names = ext4_feature_name, + .cpuid_eax = 0xC0000001, .cpuid_reg = R_EDX, + }, [FEAT_KVM] = { .feat_names = kvm_feature_name, .cpuid_eax = KVM_CPUID_FEATURES, .cpuid_reg = R_EAX, @@ -1412,6 +1427,7 @@ static int cpu_x86_parse_featurestr(x86_def_t *x86_cpu_def, char *features) x86_cpu_def->ext_features |= plus_features[FEAT_1_ECX]; x86_cpu_def->ext2_features |= plus_features[FEAT_8000_0001_EDX]; x86_cpu_def->ext3_features |= plus_features[FEAT_8000_0001_ECX]; + x86_cpu_def->ext4_features |= plus_features[FEAT_C000_0001_EDX]; x86_cpu_def->kvm_features |= plus_features[FEAT_KVM]; x86_cpu_def->svm_features |= plus_features[FEAT_SVM]; x86_cpu_def->cpuid_7_0_ebx_features |= plus_features[FEAT_7_0_EBX]; @@ -1419,6 +1435,7 @@ static int cpu_x86_parse_featurestr(x86_def_t *x86_cpu_def, char *features) x86_cpu_def->ext_features &= ~minus_features[FEAT_1_ECX]; x86_cpu_def->ext2_features &= ~minus_features[FEAT_8000_0001_EDX]; x86_cpu_def->ext3_features &= ~minus_features[FEAT_8000_0001_ECX]; + x86_cpu_def->ext4_features &= ~minus_features[FEAT_C000_0001_EDX]; x86_cpu_def->kvm_features &= ~minus_features[FEAT_KVM]; x86_cpu_def->svm_features &= ~minus_features[FEAT_SVM]; x86_cpu_def->cpuid_7_0_ebx_features &= ~minus_features[FEAT_7_0_EBX]; -- 1.7.11.7

Am 07.01.2013 19:20, schrieb Eduardo Habkost:
Feature names were taken from the X86_FEATURE_* constants in the Linux kernel code.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- Cc: Gleb Natapov <gleb@redhat.com> --- target-i386/cpu.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 4b3ee63..a54c464 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -95,6 +95,17 @@ static const char *ext3_feature_name[] = { NULL, NULL, NULL, NULL, };
+static const char *ext4_feature_name[] = { + NULL, NULL, "xstore","xstore-en", + NULL, NULL, "xcrypt","xcrypt-en",
Missing spaces, fixed. Andreas
+ "ace2", "ace2-en", "phe", "phe-en", + "pmm", "pmm-en", NULL, NULL, + NULL, NULL, NULL, NULL, + NULL, NULL, NULL, NULL, + NULL, NULL, NULL, NULL, + NULL, NULL, NULL, NULL, +}; + static const char *kvm_feature_name[] = { "kvmclock", "kvm_nopiodelay", "kvm_mmu", "kvmclock", "kvm_asyncpf", "kvm_steal_time", "kvm_pv_eoi", NULL, [snip]
-- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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@redhat.com> --- Cc: Gleb Natapov <gleb@redhat.com> Cc: Marcelo Tosatti <mtosatti@redhat.com> Cc: kvm@vger.kernel.org Cc: libvir-list@redhat.com Cc: Jiri Denemark <jdenemar@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. Changes v2: - Coding style fix Changes v3: - Added ext4_feature_name array --- target-i386/cpu.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index a54c464..68cabcf 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -989,8 +989,9 @@ static int unavailable_host_feature(FeatureWordInfo *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. */ @@ -1008,6 +1009,14 @@ static int kvm_check_features_against_host(x86_def_t *guest_def) FEAT_8000_0001_EDX }, {&guest_def->ext3_features, &host_def.ext3_features, FEAT_8000_0001_ECX }, + {&guest_def->ext4_features, &host_def.ext4_features, + FEAT_C000_0001_EDX }, + {&guest_def->cpuid_7_0_ebx_features, &host_def.cpuid_7_0_ebx_features, + FEAT_7_0_EBX }, + {&guest_def->svm_features, &host_def.svm_features, + FEAT_SVM }, + {&guest_def->kvm_features, &host_def.kvm_features, + FEAT_KVM }, }; assert(kvm_enabled()); -- 1.7.11.7

On Mon, Jan 07, 2013 at 04:20:41PM -0200, Eduardo Habkost wrote:
Changes on v3: - Patches 3-9 from v2 are now already on qom-cpu tree - Remove CONFIG_KVM #ifdefs by declaring fake KVM_* #defines on sysemu/kvm.h - Refactor code that uses the feature word arrays (to make it easier to add a new feature name array) - Add feature name array for CPUID leaf 0xC0000001
Changes on v2: - Now both the kvm_mmu-disable and -cpu "enforce" changes are on the same series - Coding style fixes
Git tree for reference: git://github.com/ehabkost/qemu-hacks.git cpu-enforce-all.v3 https://github.com/ehabkost/qemu-hacks/tree/cpu-enforce-all.v3
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.
Reviewed-by: Gleb Natapov <gleb@redhat.com> Small comment on patch 4. Fill free to ignore.
Eduardo Habkost (7): kvm: Add fake KVM constants to avoid #ifdefs on KVM-specific code target-i386: Don't set any KVM flag by default if KVM is disabled target-i386: Disable kvm_mmu by default target-i386/cpu: Introduce FeatureWord typedefs target-i386: kvm_check_features_against_host(): Use feature_word_info target-i386/cpu.c: Add feature name array for ext4_features target-i386: check/enforce: Check all feature words
include/sysemu/kvm.h | 14 ++++ target-i386/cpu.c | 193 ++++++++++++++++++++++++++++++++------------------- target-i386/cpu.h | 15 ++++ 3 files changed, 150 insertions(+), 72 deletions(-)
-- 1.7.11.7
-- Gleb.

Am 07.01.2013 19:20, schrieb Eduardo Habkost:
Eduardo Habkost (7): kvm: Add fake KVM constants to avoid #ifdefs on KVM-specific code [...] target-i386: Disable kvm_mmu by default target-i386/cpu: Introduce FeatureWord typedefs target-i386: kvm_check_features_against_host(): Use feature_word_info target-i386/cpu.c: Add feature name array for ext4_features target-i386: check/enforce: Check all feature words
Thanks, applied these to qom-cpu: https://github.com/afaerber/qemu-cpu/commits/qom-cpu Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
participants (4)
-
Andreas Färber
-
Eduardo Habkost
-
Gleb Natapov
-
Igor Mammedov