[PATCH 0/3] target/i386: early MSR initialization + pass down host microcode revision for "-cpu host"

[PATCH 0/3] target/i386: early MSR initialization + pass down host microcode revision for "-cpu host"

[Paolo Bonzini]

As part of this, patch 1 moves feature MSR initialization to
kvm_arch_init_vcpu, which also fixes a problem related to the ordering
of kvm_put_msrs and kvm_put_nested_state.  Patch 2 adds a customizable
ucode-rev property that is supported by both TCG and HVF.  Finally patch 3
adds the KVM support, including getting the host version via KVM_GET_MSRS
and passing it back to the guest for "-cpu host" only.

Paolo Bonzini (3):
  target/i386: kvm: initialize feature MSRs very early
  target/i386: add a ucode-rev property
  target/i386: kvm: initialize microcode revision from KVM

 target/i386/cpu.c         | 14 ++++++++
 target/i386/cpu.h         |  3 ++
 target/i386/hvf/x86_emu.c |  4 +--
 target/i386/kvm.c         | 86 +++++++++++++++++++++++++++++------------------
 target/i386/kvm_i386.h    |  1 +
 target/i386/misc_helper.c |  4 +++
 6 files changed, 76 insertions(+), 36 deletions(-)

-- 
1.8.3.1

[PATCH 1/3] target/i386: kvm: initialize feature MSRs very early

[Paolo Bonzini]

Some read-only MSRs affect the behavior of ioctls such as
KVM_SET_NESTED_STATE.  We can initialize them once and for all
right after the CPU is realized, since they will never be modified
by the guest.

Reported-by: Qingua Cheng <qcheng@redhat.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/kvm.c      | 81 ++++++++++++++++++++++++++++++--------------------
 target/i386/kvm_i386.h |  1 +
 2 files changed, 49 insertions(+), 33 deletions(-)

diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 7ee3202..f6dd6b7 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -67,6 +67,8 @@
  * 255 kvm_msr_entry structs */
 #define MSR_BUF_SIZE 4096
 
+static void kvm_init_msrs(X86CPU *cpu);
+
 const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
     KVM_CAP_INFO(SET_TSS_ADDR),
     KVM_CAP_INFO(EXT_CPUID),
@@ -1842,6 +1844,8 @@ int kvm_arch_init_vcpu(CPUState *cs)
         has_msr_tsc_aux = false;
     }
 
+    kvm_init_msrs(cpu);
+
     r = hyperv_init_vcpu(cpu);
     if (r) {
         goto fail;
@@ -2660,11 +2664,53 @@ static void kvm_msr_entry_add_vmx(X86CPU *cpu, FeatureWordArray f)
                       VMCS12_MAX_FIELD_INDEX << 1);
 }
 
+static int kvm_buf_set_msrs(X86CPU *cpu)
+{
+    int ret = kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MSRS, cpu->kvm_msr_buf);
+    if (ret < 0) {
+        return ret;
+    }
+
+    if (ret < cpu->kvm_msr_buf->nmsrs) {
+        struct kvm_msr_entry *e = &cpu->kvm_msr_buf->entries[ret];
+        error_report("error: failed to set MSR 0x%" PRIx32 " to 0x%" PRIx64,
+                     (uint32_t)e->index, (uint64_t)e->data);
+    }
+
+    assert(ret == cpu->kvm_msr_buf->nmsrs);
+    return 0;
+}
+
+static void kvm_init_msrs(X86CPU *cpu)
+{
+    CPUX86State *env = &cpu->env;
+
+    kvm_msr_buf_reset(cpu);
+    if (has_msr_arch_capabs) {
+        kvm_msr_entry_add(cpu, MSR_IA32_ARCH_CAPABILITIES,
+                          env->features[FEAT_ARCH_CAPABILITIES]);
+    }
+
+    if (has_msr_core_capabs) {
+        kvm_msr_entry_add(cpu, MSR_IA32_CORE_CAPABILITY,
+                          env->features[FEAT_CORE_CAPABILITY]);
+    }
+
+    /*
+     * 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);
+    }
+
+    assert(kvm_buf_set_msrs(cpu) == 0);
+}
+
 static int kvm_put_msrs(X86CPU *cpu, int level)
 {
     CPUX86State *env = &cpu->env;
     int i;
-    int ret;
 
     kvm_msr_buf_reset(cpu);
 
@@ -2722,17 +2768,6 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
     }
 #endif
 
-    /* If host supports feature MSR, write down. */
-    if (has_msr_arch_capabs) {
-        kvm_msr_entry_add(cpu, MSR_IA32_ARCH_CAPABILITIES,
-                          env->features[FEAT_ARCH_CAPABILITIES]);
-    }
-
-    if (has_msr_core_capabs) {
-        kvm_msr_entry_add(cpu, MSR_IA32_CORE_CAPABILITY,
-                          env->features[FEAT_CORE_CAPABILITY]);
-    }
-
     /*
      * The following MSRs have side effects on the guest or are too heavy
      * for normal writeback. Limit them to reset or full state updates.
@@ -2910,14 +2945,6 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
 
         /* Note: MSR_IA32_FEATURE_CONTROL is written separately, see
          *       kvm_put_msr_feature_control. */
-
-        /*
-         * 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);
-        }
     }
 
     if (env->mcg_cap) {
@@ -2933,19 +2960,7 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
         }
     }
 
-    ret = kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MSRS, cpu->kvm_msr_buf);
-    if (ret < 0) {
-        return ret;
-    }
-
-    if (ret < cpu->kvm_msr_buf->nmsrs) {
-        struct kvm_msr_entry *e = &cpu->kvm_msr_buf->entries[ret];
-        error_report("error: failed to set MSR 0x%" PRIx32 " to 0x%" PRIx64,
-                     (uint32_t)e->index, (uint64_t)e->data);
-    }
-
-    assert(ret == cpu->kvm_msr_buf->nmsrs);
-    return 0;
+    return kvm_buf_set_msrs(cpu);
 }
 
 
diff --git a/target/i386/kvm_i386.h b/target/i386/kvm_i386.h
index 7d0242f..00bde7a 100644
--- a/target/i386/kvm_i386.h
+++ b/target/i386/kvm_i386.h
@@ -46,4 +46,5 @@ bool kvm_enable_x2apic(void);
 bool kvm_has_x2apic_api(void);
 
 bool kvm_hv_vpindex_settable(void);
+
 #endif
-- 
1.8.3.1

[PATCH 2/3] target/i386: add a ucode-rev property

[Paolo Bonzini]

Add the property and plumb it in TCG and HVF (the latter of which
tried to support returning a constant value but used the wrong MSR).

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/cpu.c         | 10 ++++++++++
 target/i386/cpu.h         |  3 +++
 target/i386/hvf/x86_emu.c |  4 +---
 target/i386/misc_helper.c |  4 ++++
 4 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 41f28ce..05ce64c 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6418,6 +6418,15 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
         }
     }
 
+    if (cpu->ucode_rev == 0) {
+        /* The default is the same as KVM's.  */
+        if (IS_AMD_CPU(env)) {
+            cpu->ucode_rev = 0x01000065;
+        } else {
+            cpu->ucode_rev = 0x100000000ULL;
+        }
+    }
+
     /* mwait extended info: needed for Core compatibility */
     /* We always wake on interrupt even if host does not have the capability */
     cpu->mwait.ecx |= CPUID_MWAIT_EMX | CPUID_MWAIT_IBE;
@@ -7100,6 +7109,7 @@ static Property x86_cpu_properties[] = {
     DEFINE_PROP_UINT32("min-level", X86CPU, env.cpuid_min_level, 0),
     DEFINE_PROP_UINT32("min-xlevel", X86CPU, env.cpuid_min_xlevel, 0),
     DEFINE_PROP_UINT32("min-xlevel2", X86CPU, env.cpuid_min_xlevel2, 0),
+    DEFINE_PROP_UINT64("ucode-rev", X86CPU, ucode_rev, 0),
     DEFINE_PROP_BOOL("full-cpuid-auto-level", X86CPU, full_cpuid_auto_level, true),
     DEFINE_PROP_STRING("hv-vendor-id", X86CPU, hyperv_vendor_id),
     DEFINE_PROP_BOOL("cpuid-0xb", X86CPU, enable_cpuid_0xb, true),
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index e6de38a..576f309 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -348,6 +348,7 @@ typedef enum X86Seg {
 #define MSR_IA32_SPEC_CTRL              0x48
 #define MSR_VIRT_SSBD                   0xc001011f
 #define MSR_IA32_PRED_CMD               0x49
+#define MSR_IA32_UCODE_REV              0x8b
 #define MSR_IA32_CORE_CAPABILITY        0xcf
 
 #define MSR_IA32_ARCH_CAPABILITIES      0x10a
@@ -1627,6 +1628,8 @@ struct X86CPU {
     CPUNegativeOffsetState neg;
     CPUX86State env;
 
+    uint64_t ucode_rev;
+
     uint32_t hyperv_spinlock_attempts;
     char *hyperv_vendor_id;
     bool hyperv_synic_kvm_only;
diff --git a/target/i386/hvf/x86_emu.c b/target/i386/hvf/x86_emu.c
index 3df7672..92ab815 100644
--- a/target/i386/hvf/x86_emu.c
+++ b/target/i386/hvf/x86_emu.c
@@ -664,8 +664,6 @@ static void exec_lods(struct CPUX86State *env, struct x86_decode *decode)
     RIP(env) += decode->len;
 }
 
-#define MSR_IA32_UCODE_REV 0x00000017
-
 void simulate_rdmsr(struct CPUState *cpu)
 {
     X86CPU *x86_cpu = X86_CPU(cpu);
@@ -681,7 +679,7 @@ void simulate_rdmsr(struct CPUState *cpu)
         val = cpu_get_apic_base(X86_CPU(cpu)->apic_state);
         break;
     case MSR_IA32_UCODE_REV:
-        val = (0x100000000ULL << 32) | 0x100000000ULL;
+        val = x86_cpu->ucode_rev;
         break;
     case MSR_EFER:
         val = rvmcs(cpu->hvf_fd, VMCS_GUEST_IA32_EFER);
diff --git a/target/i386/misc_helper.c b/target/i386/misc_helper.c
index 3eff688..aed16fe 100644
--- a/target/i386/misc_helper.c
+++ b/target/i386/misc_helper.c
@@ -229,6 +229,7 @@ void helper_rdmsr(CPUX86State *env)
 #else
 void helper_wrmsr(CPUX86State *env)
 {
+    X86CPU *x86_cpu = env_archcpu(env);
     uint64_t val;
 
     cpu_svm_check_intercept_param(env, SVM_EXIT_MSR, 1, GETPC());
@@ -371,6 +372,9 @@ void helper_wrmsr(CPUX86State *env)
         env->msr_bndcfgs = val;
         cpu_sync_bndcs_hflags(env);
         break;
+     case MSR_IA32_UCODE_REV:
+        val = x86_cpu->ucode_rev;
+        break;
     default:
         if ((uint32_t)env->regs[R_ECX] >= MSR_MC0_CTL
             && (uint32_t)env->regs[R_ECX] < MSR_MC0_CTL +
-- 
1.8.3.1

[PATCH 3/3] target/i386: kvm: initialize microcode revision from KVM

[Paolo Bonzini]

KVM can return the host microcode revision as a feature MSR.
Use it as the default value for -cpu host.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/cpu.c | 4 ++++
 target/i386/kvm.c | 5 +++++
 2 files changed, 9 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 05ce64c..1f731c1 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6416,6 +6416,10 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
                        &cpu->mwait.ecx, &cpu->mwait.edx);
             env->features[FEAT_1_ECX] |= CPUID_EXT_MONITOR;
         }
+        if (kvm_enabled() && cpu->ucode_rev == 0) {
+            cpu->ucode_rev = kvm_arch_get_supported_msr_feature(kvm_state,
+                                                                MSR_IA32_UCODE_REV);
+        }
     }
 
     if (cpu->ucode_rev == 0) {
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index f6dd6b7..26c1e78 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -2696,6 +2696,11 @@ static void kvm_init_msrs(X86CPU *cpu)
                           env->features[FEAT_CORE_CAPABILITY]);
     }
 
+    if (kvm_arch_get_supported_msr_feature(kvm_state,
+					   MSR_IA32_UCODE_REV)) {
+        kvm_msr_entry_add(cpu, MSR_IA32_UCODE_REV, cpu->ucode_rev);
+    }
+
     /*
      * Older kernels do not include VMX MSRs in KVM_GET_MSR_INDEX_LIST, but
      * all kernels with MSR features should have them.
-- 
1.8.3.1

Re: [PATCH 0/3] target/i386: early MSR initialization + pass down host microcode revision for "-cpu host"

[no-reply]

Patchew URL: https://patchew.org/QEMU/1579544504-3616-1-git-send-email-pbonzini@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 1579544504-3616-1-git-send-email-pbonzini@redhat.com
Subject: [PATCH 0/3] target/i386: early MSR initialization + pass down host microcode revision for "-cpu host"

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

Switched to a new branch 'test'
29ac94b target/i386: kvm: initialize microcode revision from KVM
54502b3 target/i386: add a ucode-rev property
7d66a0a target/i386: kvm: initialize feature MSRs very early

=== OUTPUT BEGIN ===
1/3 Checking commit 7d66a0ad44e0 (target/i386: kvm: initialize feature MSRs very early)
2/3 Checking commit 54502b34491d (target/i386: add a ucode-rev property)
3/3 Checking commit 29ac94bec151 (target/i386: kvm: initialize microcode revision from KVM)
WARNING: line over 80 characters
#23: FILE: target/i386/cpu.c:6421:
+                                                                MSR_IA32_UCODE_REV);

ERROR: code indent should never use tabs
#37: FILE: target/i386/kvm.c:2700:
+^I^I^I^I^I   MSR_IA32_UCODE_REV)) {$

total: 1 errors, 1 warnings, 21 lines checked

Patch 3/3 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/1579544504-3616-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