Re: [PATCH v3 09/10] KVM: MMU: fix MTRR update

[Wanpeng Li <wanpeng.li@linux.intel.com>]

Hi Xiao,
On Wed, May 13, 2015 at 02:42:27PM +0800, Xiao Guangrong wrote:
>Currently, whenever guest MTRR registers are changed
>kvm_mmu_reset_context is called to switch to the new root shadow page
>table, however, it's useless since:
>1) the cache type is not cached into shadow page's attribute so that
>   the original root shadow page will be reused

kvm_mmu_reset_context
  kvm_mmu_unload
    mmu_free_roots

The original root shadow page will be freed in mmu_free_roots, where I
miss?

Another question maybe not related to this patch:

If kvm_mmu_reset_context is just called to destroy the original root 
shadow page and all the sptes will remain valid? 

Regards,
Wanpeng Li 

>
>2) the cache type is set on the last spte, that means we should sync
>   the last sptes when MTRR is changed
>
>This patch fixs this issue by drop all the spte in the gfn range which
>is being updated by MTRR
>
>Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
>---
> arch/x86/kvm/x86.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 58 insertions(+), 1 deletion(-)
>
>diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>index cde5d61..bbe184f 100644
>--- a/arch/x86/kvm/x86.c
>+++ b/arch/x86/kvm/x86.c
>@@ -1852,6 +1852,63 @@ bool kvm_mtrr_valid(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> }
> EXPORT_SYMBOL_GPL(kvm_mtrr_valid);
> 
>+static void update_mtrr(struct kvm_vcpu *vcpu, u32 msr)
>+{
>+	struct mtrr_state_type *mtrr_state = &vcpu->arch.mtrr_state;
>+	unsigned char mtrr_enabled = mtrr_state->enabled;
>+	gfn_t start, end, mask;
>+	int index;
>+	bool is_fixed = true;
>+
>+	if (msr == MSR_IA32_CR_PAT || !tdp_enabled ||
>+	      !kvm_arch_has_noncoherent_dma(vcpu->kvm))
>+		return;
>+
>+	if (!(mtrr_enabled & 0x2) && msr != MSR_MTRRdefType)
>+		return;
>+
>+	switch (msr) {
>+	case MSR_MTRRfix64K_00000:
>+		start = 0x0;
>+		end = 0x80000;
>+		break;
>+	case MSR_MTRRfix16K_80000:
>+		start = 0x80000;
>+		end = 0xa0000;
>+		break;
>+	case MSR_MTRRfix16K_A0000:
>+		start = 0xa0000;
>+		end = 0xc0000;
>+		break;
>+	case MSR_MTRRfix4K_C0000 ... MSR_MTRRfix4K_F8000:
>+		index = msr - MSR_MTRRfix4K_C0000;
>+		start = 0xc0000 + index * (32 << 10);
>+		end = start + (32 << 10);
>+		break;
>+	case MSR_MTRRdefType:
>+		is_fixed = false;
>+		start = 0x0;
>+		end = ~0ULL;
>+		break;
>+	default:
>+		/* variable range MTRRs. */
>+		is_fixed = false;
>+		index = (msr - 0x200) / 2;
>+		start = (((u64)mtrr_state->var_ranges[index].base_hi) << 32) +
>+		       (mtrr_state->var_ranges[index].base_lo & PAGE_MASK);
>+		mask = (((u64)mtrr_state->var_ranges[index].mask_hi) << 32) +
>+		       (mtrr_state->var_ranges[index].mask_lo & PAGE_MASK);
>+		mask |= ~0ULL << cpuid_maxphyaddr(vcpu);
>+
>+		end = ((start & mask) | ~mask) + 1;
>+	}
>+
>+	if (is_fixed && !(mtrr_enabled & 0x1))
>+		return;
>+
>+	kvm_zap_gfn_range(vcpu->kvm, gpa_to_gfn(start), gpa_to_gfn(end));
>+}
>+
> static int set_msr_mtrr(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> {
> 	u64 *p = (u64 *)&vcpu->arch.mtrr_state.fixed_ranges;
>@@ -1885,7 +1942,7 @@ static int set_msr_mtrr(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> 		*pt = data;
> 	}
> 
>-	kvm_mmu_reset_context(vcpu);
>+	update_mtrr(vcpu, msr);
> 	return 0;
> }
> 
>-- 
>2.1.0
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at  http://www.tux.org/lkml/