From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50489) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XI3M2-00041d-02 for qemu-devel@nongnu.org; Thu, 14 Aug 2014 18:18:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XI3Lw-00080Y-SE for qemu-devel@nongnu.org; Thu, 14 Aug 2014 18:18:41 -0400 Message-ID: <53ED35B7.1080105@redhat.com> Date: Fri, 15 Aug 2014 00:18:31 +0200 From: Laszlo Ersek MIME-Version: 1.0 References: <20140814213751.16881.91600.stgit@gimli.home> <20140814213933.16881.58240.stgit@gimli.home> In-Reply-To: <20140814213933.16881.58240.stgit@gimli.home> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 2/3] x86: kvm: Add MTRR support for kvm_get|put_msrs() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Williamson , qemu-devel@nongnu.org, kvm@vger.kernel.org Cc: qemu-stable@nongnu.org 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 > Cc: Laszlo Ersek > Cc: qemu-stable@nongnu.org > --- > > 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 Thanks! Laszlo