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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150513084341.GA4910@kernel>
Date:	Wed, 13 May 2015 16:43:41 +0800
From:	Wanpeng Li <wanpeng.li@...ux.intel.com>
To:	Xiao Guangrong <guangrong.xiao@...ux.intel.com>
Cc:	pbonzini@...hat.com, gleb@...nel.org, mtosatti@...hat.com,
	kvm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 09/10] KVM: MMU: fix MTRR update

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@...ux.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@...r.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/