[Qemu-devel] [PATCH 9/9] target-i386: check/enforce: Check all feature words

[Eduardo Habkost <ehabkost@redhat.com>]

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.
---
 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