Re: [libvirt] [PATCH 6/6] qemu: Add support for emulated CPU features

23 Sep 2013

(CCing qemu-devel and libvir-list)

On Sun, Sep 22, 2013 at 04:44:55PM +0200, Borislav Petkov wrote:
...
From: Borislav Petkov <bp@suse.de>
Add support for the KVM_GET_EMULATED_CPUID ioctl and leave feature bits
enabled, when requested by userspace, if kvm emulates them.
Signed-off-by: Borislav Petkov <bp@suse.de>
This will break some assumptions about the semantics of "-cpu host",
that today means "enable every single feature that can be enabled in the
host". This, in turn, breaks the assumption that "-cpu host" can be used
by libvirt to get information about all CPU features supported by the
host.

So, I have two questions:

1) Should "-cpu host" enable GET_EMULATED_CPUID features? (I believe the
   answer is "no");
2) If not, we need a new mechanism to let libvirt know which features
   can be enabled on a host (in a way that would allow libvirt to
   easily calculate the set of possible destination hosts to which a VM
   can be migrated, before deciding to migrate it).

This interface is getting complex enough for me to consider simply
asking libvirt developers to call GET_EMULATED_CPUID and
GET_SUPPORTED_CPUID directly, instead of asking QEMU for information.
But if libvirt does that, it needs a way to know if it's running a QEMU
version that supports GET_EMULATED_CPUID, or an old version that
supports only GET_SUPPORTED_CPUID.

Another alternative is to simply let libvirt ignore GET_EMULATED_CPUID
by now, and treat GET_EMULATED_CPUID features as not supported by the
host. But then it will break the use cases that prompted the creation of
GET_EMULATED_CPUID in the first place.
...
---
 include/sysemu/kvm.h      |  4 ++++
 linux-headers/linux/kvm.h |  4 ++++
 target-i386/cpu.c         |  7 +++++++
 target-i386/kvm.c         | 38 ++++++++++++++++++++++++++++++++++----
 4 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 9bbe3db1464e..8eda1ada848a 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -265,6 +265,10 @@ int kvm_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_emulated_cpuid(KVMState *env, uint32_t function,
+                                     uint32_t index, int reg);
+
 void kvm_cpu_synchronize_state(CPUState *cpu);
/* generic hooks - to be moved/refactored once there are more users */
diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index c614070662e1..edc8f2db1f8d 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -541,6 +541,7 @@ struct kvm_ppc_smmu_info {
 #define KVM_TRACE_ENABLE          __KVM_DEPRECATED_MAIN_W_0x06
 #define KVM_TRACE_PAUSE           __KVM_DEPRECATED_MAIN_0x07
 #define KVM_TRACE_DISABLE         __KVM_DEPRECATED_MAIN_0x08
+#define KVM_GET_EMULATED_CPUID    _IOWR(KVMIO, 0x09, struct kvm_cpuid2)
/*
  * Extension capability list.
@@ -666,6 +667,9 @@ struct kvm_ppc_smmu_info {
 #define KVM_CAP_IRQ_MPIC 90
 #define KVM_CAP_PPC_RTAS 91
 #define KVM_CAP_IRQ_XICS 92
+#define KVM_CAP_ARM_EL1_32BIT 93
+#define KVM_CAP_SPAPR_MULTITCE 94
+#define KVM_CAP_EXT_EMUL_CPUID 95
#ifdef KVM_CAP_IRQ_ROUTING
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index c36345e426d7..5406348ceb4a 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1850,7 +1850,14 @@ static void filter_features_for_kvm(X86CPU *cpu)
                                                              wi->cpuid_ecx,
                                                              wi->cpuid_reg);
         uint32_t requested_features = env->features[w];
+
+        uint32_t emul_features = kvm_arch_get_emulated_cpuid(s, wi->cpuid_eax,
+                                                                wi->cpuid_ecx,
+                                                                wi->cpuid_reg);
+
         env->features[w] &= host_feat;
+        env->features[w] |= (requested_features & emul_features);
+
I find the above logic confusing: you remove a few bits just to re-add
them later. Isn't easier and simpler to simply do:
    env->features[w] &= (host_feat | emul_features);
?
...
cpu->filtered_features[w] = requested_features & ~env->features[w];
     }
 }
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 749aa09a21a6..7f598f01bda5 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -80,7 +80,7 @@ bool kvm_allows_irq0_override(void)
     return !kvm_irqchip_in_kernel() || kvm_has_gsi_routing();
 }
-static struct kvm_cpuid2 *try_get_cpuid(KVMState *s, int max)
+static struct kvm_cpuid2 *try_get_cpuid(KVMState *s, int ioctl, int max)
 {
     struct kvm_cpuid2 *cpuid;
     int r, size;
@@ -88,7 +88,7 @@ static struct kvm_cpuid2 *try_get_cpuid(KVMState *s, int max)
     size = sizeof(*cpuid) + max * sizeof(*cpuid->entries);
     cpuid = (struct kvm_cpuid2 *)g_malloc0(size);
     cpuid->nent = max;
-    r = kvm_ioctl(s, KVM_GET_SUPPORTED_CPUID, cpuid);
+    r = kvm_ioctl(s, ioctl, cpuid);
     if (r == 0 && cpuid->nent >= max) {
         r = -E2BIG;
     }
@@ -97,7 +97,10 @@ static struct kvm_cpuid2 *try_get_cpuid(KVMState *s, int max)
             g_free(cpuid);
             return NULL;
         } else {
-            fprintf(stderr, "KVM_GET_SUPPORTED_CPUID failed: %s\n",
+            fprintf(stderr, "%s failed: %s\n",
+                    (ioctl == (int)KVM_GET_SUPPORTED_CPUID
+                     ? "KVM_GET_SUPPORTED_CPUID"
+                     : "KVM_GET_EMULATED_CPUID"),
                     strerror(-r));
             exit(1);
         }
@@ -112,7 +115,17 @@ static struct kvm_cpuid2 *get_supported_cpuid(KVMState *s)
 {
     struct kvm_cpuid2 *cpuid;
     int max = 1;
-    while ((cpuid = try_get_cpuid(s, max)) == NULL) {
+    while ((cpuid = try_get_cpuid(s, KVM_GET_SUPPORTED_CPUID, max)) == NULL) {
+        max *= 2;
+    }
+    return cpuid;
+}
+
+static struct kvm_cpuid2 *get_emulated_cpuid(KVMState *s)
+{
+    struct kvm_cpuid2 *cpuid;
+    int max = 1;
+    while ((cpuid = try_get_cpuid(s, KVM_GET_EMULATED_CPUID, max)) == NULL) {
         max *= 2;
     }
     return cpuid;
@@ -241,6 +254,23 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t function,
     return ret;
 }
+uint32_t kvm_arch_get_emulated_cpuid(KVMState *s, uint32_t function,
+                                     uint32_t index, int reg)
+{
+    struct kvm_cpuid2 *cpuid __attribute__((unused));
+
+    if (!kvm_check_extension(s, KVM_CAP_EXT_EMUL_CPUID))
+        return 0;
+
+    cpuid = get_emulated_cpuid(s);
+
+    struct kvm_cpuid_entry2 *entry = cpuid_find_entry(cpuid, function, index);
+    if (entry)
+        return cpuid_entry_get_reg(entry, reg);
+
+    return 0;
+}
+
 typedef struct HWPoisonPage {
     ram_addr_t ram_addr;
     QLIST_ENTRY(HWPoisonPage) list;
-- 
1.8.4
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
-- 
Eduardo

    

Eduardo Habkost

tags

participants (1)