On 08/14/14 23:39, Alex Williamson wrote: > The MTRR state in KVM currently runs completely independent of the > QEMU state in CPUX86State.mtrr_*. This means that on migration, the > target loses MTRR state from the source. Generally that's ok though > because KVM ignores it and maps everything as write-back anyway. The > exception to this rule is when we have an assigned device and an IOMMU > that doesn't promote NoSnoop transactions from that device to be cache > coherent. In that case KVM trusts the guest mapping of memory as > configured in the MTRR. > > This patch updates kvm_get|put_msrs() so that we retrieve the actual > vCPU MTRR settings and therefore keep CPUX86State synchronized for > migration. kvm_put_msrs() is also used on vCPU reset and therefore > allows future modificaitons of MTRR state at reset to be realized. > > Note that the entries array used by both functions was already > slightly undersized for holding every possible MSR, so this patch > increases it beyond the 28 new entries necessary for MTRR state. > > Signed-off-by: Alex Williamson <alex.williamson@xxxxxxxxxx> > Cc: Laszlo Ersek <lersek@xxxxxxxxxx> > Cc: qemu-stable@xxxxxxxxxx > --- > > target-i386/cpu.h | 2 + > target-i386/kvm.c | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 101 insertions(+), 2 deletions(-) > > diff --git a/target-i386/cpu.h b/target-i386/cpu.h > index d37d857..3460b12 100644 > --- a/target-i386/cpu.h > +++ b/target-i386/cpu.h > @@ -337,6 +337,8 @@ > #define MSR_MTRRphysBase(reg) (0x200 + 2 * (reg)) > #define MSR_MTRRphysMask(reg) (0x200 + 2 * (reg) + 1) > > +#define MSR_MTRRphysIndex(addr) ((((addr) & ~1u) - 0x200) / 2) > + > #define MSR_MTRRfix64K_00000 0x250 > #define MSR_MTRRfix16K_80000 0x258 > #define MSR_MTRRfix16K_A0000 0x259 > diff --git a/target-i386/kvm.c b/target-i386/kvm.c > index 097fe11..ddedc73 100644 > --- a/target-i386/kvm.c > +++ b/target-i386/kvm.c > @@ -79,6 +79,7 @@ static int lm_capable_kernel; > static bool has_msr_hv_hypercall; > static bool has_msr_hv_vapic; > static bool has_msr_hv_tsc; > +static bool has_msr_mtrr; > > static bool has_msr_architectural_pmu; > static uint32_t num_architectural_pmu_counters; > @@ -739,6 +740,10 @@ int kvm_arch_init_vcpu(CPUState *cs) > env->kvm_xsave_buf = qemu_memalign(4096, sizeof(struct kvm_xsave)); > } > > + if (env->features[FEAT_1_EDX] & CPUID_MTRR) { > + has_msr_mtrr = true; > + } > + > return 0; > } > > @@ -1183,7 +1188,7 @@ static int kvm_put_msrs(X86CPU *cpu, int level) > CPUX86State *env = &cpu->env; > struct { > struct kvm_msrs info; > - struct kvm_msr_entry entries[100]; > + struct kvm_msr_entry entries[150]; > } msr_data; > struct kvm_msr_entry *msrs = msr_data.entries; > int n = 0, i; > @@ -1278,6 +1283,37 @@ static int kvm_put_msrs(X86CPU *cpu, int level) > kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_REFERENCE_TSC, > env->msr_hv_tsc); > } > + if (has_msr_mtrr) { > + kvm_msr_entry_set(&msrs[n++], MSR_MTRRdefType, env->mtrr_deftype); > + kvm_msr_entry_set(&msrs[n++], > + MSR_MTRRfix64K_00000, env->mtrr_fixed[0]); > + kvm_msr_entry_set(&msrs[n++], > + MSR_MTRRfix16K_80000, env->mtrr_fixed[1]); > + kvm_msr_entry_set(&msrs[n++], > + MSR_MTRRfix16K_A0000, env->mtrr_fixed[2]); > + kvm_msr_entry_set(&msrs[n++], > + MSR_MTRRfix4K_C0000, env->mtrr_fixed[3]); > + kvm_msr_entry_set(&msrs[n++], > + MSR_MTRRfix4K_C8000, env->mtrr_fixed[4]); > + kvm_msr_entry_set(&msrs[n++], > + MSR_MTRRfix4K_D0000, env->mtrr_fixed[5]); > + kvm_msr_entry_set(&msrs[n++], > + MSR_MTRRfix4K_D8000, env->mtrr_fixed[6]); > + kvm_msr_entry_set(&msrs[n++], > + MSR_MTRRfix4K_E0000, env->mtrr_fixed[7]); > + kvm_msr_entry_set(&msrs[n++], > + MSR_MTRRfix4K_E8000, env->mtrr_fixed[8]); > + kvm_msr_entry_set(&msrs[n++], > + MSR_MTRRfix4K_F0000, env->mtrr_fixed[9]); > + kvm_msr_entry_set(&msrs[n++], > + MSR_MTRRfix4K_F8000, env->mtrr_fixed[10]); > + for (i = 0; i < MSR_MTRRcap_VCNT; i++) { > + kvm_msr_entry_set(&msrs[n++], > + MSR_MTRRphysBase(i), env->mtrr_var[i].base); > + kvm_msr_entry_set(&msrs[n++], > + MSR_MTRRphysMask(i), env->mtrr_var[i].mask); > + } > + } > > /* Note: MSR_IA32_FEATURE_CONTROL is written separately, see > * kvm_put_msr_feature_control. */ > @@ -1484,7 +1520,7 @@ static int kvm_get_msrs(X86CPU *cpu) > CPUX86State *env = &cpu->env; > struct { > struct kvm_msrs info; > - struct kvm_msr_entry entries[100]; > + struct kvm_msr_entry entries[150]; > } msr_data; > struct kvm_msr_entry *msrs = msr_data.entries; > int ret, i, n; > @@ -1572,6 +1608,24 @@ static int kvm_get_msrs(X86CPU *cpu) > if (has_msr_hv_tsc) { > msrs[n++].index = HV_X64_MSR_REFERENCE_TSC; > } > + if (has_msr_mtrr) { > + msrs[n++].index = MSR_MTRRdefType; > + msrs[n++].index = MSR_MTRRfix64K_00000; > + msrs[n++].index = MSR_MTRRfix16K_80000; > + msrs[n++].index = MSR_MTRRfix16K_A0000; > + msrs[n++].index = MSR_MTRRfix4K_C0000; > + msrs[n++].index = MSR_MTRRfix4K_C8000; > + msrs[n++].index = MSR_MTRRfix4K_D0000; > + msrs[n++].index = MSR_MTRRfix4K_D8000; > + msrs[n++].index = MSR_MTRRfix4K_E0000; > + msrs[n++].index = MSR_MTRRfix4K_E8000; > + msrs[n++].index = MSR_MTRRfix4K_F0000; > + msrs[n++].index = MSR_MTRRfix4K_F8000; > + for (i = 0; i < MSR_MTRRcap_VCNT; i++) { > + msrs[n++].index = MSR_MTRRphysBase(i); > + msrs[n++].index = MSR_MTRRphysMask(i); > + } > + } > > msr_data.info.nmsrs = n; > ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_MSRS, &msr_data); > @@ -1692,6 +1746,49 @@ static int kvm_get_msrs(X86CPU *cpu) > case HV_X64_MSR_REFERENCE_TSC: > env->msr_hv_tsc = msrs[i].data; > break; > + case MSR_MTRRdefType: > + env->mtrr_deftype = msrs[i].data; > + break; > + case MSR_MTRRfix64K_00000: > + env->mtrr_fixed[0] = msrs[i].data; > + break; > + case MSR_MTRRfix16K_80000: > + env->mtrr_fixed[1] = msrs[i].data; > + break; > + case MSR_MTRRfix16K_A0000: > + env->mtrr_fixed[2] = msrs[i].data; > + break; > + case MSR_MTRRfix4K_C0000: > + env->mtrr_fixed[3] = msrs[i].data; > + break; > + case MSR_MTRRfix4K_C8000: > + env->mtrr_fixed[4] = msrs[i].data; > + break; > + case MSR_MTRRfix4K_D0000: > + env->mtrr_fixed[5] = msrs[i].data; > + break; > + case MSR_MTRRfix4K_D8000: > + env->mtrr_fixed[6] = msrs[i].data; > + break; > + case MSR_MTRRfix4K_E0000: > + env->mtrr_fixed[7] = msrs[i].data; > + break; > + case MSR_MTRRfix4K_E8000: > + env->mtrr_fixed[8] = msrs[i].data; > + break; > + case MSR_MTRRfix4K_F0000: > + env->mtrr_fixed[9] = msrs[i].data; > + break; > + case MSR_MTRRfix4K_F8000: > + env->mtrr_fixed[10] = msrs[i].data; > + break; > + case MSR_MTRRphysBase(0) ... MSR_MTRRphysMask(MSR_MTRRcap_VCNT - 1): > + if (index & 1) { > + env->mtrr_var[MSR_MTRRphysIndex(index)].mask = msrs[i].data; > + } else { > + env->mtrr_var[MSR_MTRRphysIndex(index)].base = msrs[i].data; > + } > + break; > } > } > > Compared it with v2 2/3. Reviewed-by: Laszlo Ersek <lersek@xxxxxxxxxx> Thanks! Laszlo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html