[Qemu-devel] [PATCH 2/5] x86: Mask mtrr mask based on CPU physical address limits

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <address@hidden>

The CPU GPs if we try and set a bit in a variable MTRR mask above
the limit of physical address bits on the host.  We hit this
when loading a migration from a host with a larger physical
address limit than our destination (e.g. a Xeon->i7 of same
generation) but previously used to get away with it
until 48e1a45 started checking that msr writes actually worked.

It seems in our case the GP probably comes from KVM emulating
that GP.

Signed-off-by: Dr. David Alan Gilbert <address@hidden>
---
 target-i386/cpu.c | 13 +++++++++++++
 target-i386/cpu.h |  1 +
 target-i386/kvm.c | 13 +++++++++++--
 3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 3665fec..f9302f9 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2668,6 +2668,19 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
     }
 }
 
+/* Returns the number of physical bits supported by the guest CPU */
+unsigned int cpu_x86_guest_phys_address_bits(CPUX86State *env)
+{
+    /* the cpuid emulation code already calculates a value to give to the
+     * guest and this should match.
+     */
+    uint32_t eax, unused;
+    cpu_x86_cpuid(env, 0x80000008, 0, &eax, &unused, &unused, &unused);
+
+    /* Bottom 8 bits of leaf 0x80000008 see Table 3-17 in CPUID definition */
+    return eax & 0xff;
+}
+
 /* CPUClass::reset() */
 static void x86_cpu_reset(CPUState *s)
 {
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index d9ab884..61c06b7 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -1238,6 +1238,7 @@ X86CPU *cpu_x86_create(const char *cpu_model, Error 
**errp);
 int cpu_x86_exec(CPUState *cpu);
 void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf);
 int cpu_x86_support_mca_broadcast(CPUX86State *env);
+unsigned int cpu_x86_guest_phys_address_bits(CPUX86State *env);
 
 int cpu_get_pic_interrupt(CPUX86State *s);
 /* MSDOS compatibility mode FPU exception support */
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index abf50e6..d95d06b 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1674,6 +1674,10 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
             }
         }
         if (has_msr_mtrr) {
+            uint64_t phys_mask = BIT_RANGE(
+                                   cpu_x86_guest_phys_address_bits(env) - 1,
+                                   0);
+
             kvm_msr_entry_add(cpu, MSR_MTRRdefType, env->mtrr_deftype);
             kvm_msr_entry_add(cpu, MSR_MTRRfix64K_00000, env->mtrr_fixed[0]);
             kvm_msr_entry_add(cpu, MSR_MTRRfix16K_80000, env->mtrr_fixed[1]);
@@ -1687,10 +1691,15 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
             kvm_msr_entry_add(cpu, MSR_MTRRfix4K_F0000, env->mtrr_fixed[9]);
             kvm_msr_entry_add(cpu, MSR_MTRRfix4K_F8000, env->mtrr_fixed[10]);
             for (i = 0; i < MSR_MTRRcap_VCNT; i++) {
+                /* The CPU GPs if we write to a bit above the physical limit of
+                 * the host CPU (and KVM emulates that)
+                 */
+                uint64_t mask = env->mtrr_var[i].mask;
+                mask &= phys_mask;
+
                 kvm_msr_entry_add(cpu, MSR_MTRRphysBase(i),
                                   env->mtrr_var[i].base);
-                kvm_msr_entry_add(cpu, MSR_MTRRphysMask(i),
-                                  env->mtrr_var[i].mask);
+                kvm_msr_entry_add(cpu, MSR_MTRRphysMask(i), mask);
             }
         }
 
-- 
2.7.4