linux-kernel - Re: [announce] [patch] KVM paravirtualization for Linux

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: <20070108083935.GB18259@elte.hu>
Date:	Mon, 8 Jan 2007 09:39:35 +0100
From:	Ingo Molnar <mingo@...e.hu>
To:	Avi Kivity <avi@...ranet.com>
Cc:	kvm-devel <kvm-devel@...ts.sourceforge.net>,
	linux-kernel@...r.kernel.org
Subject: Re: [announce] [patch] KVM paravirtualization for Linux

* Avi Kivity <avi@...ranet.com> wrote:

> > the cache is zapped upon pagefaults anyway, so unpinning ought to be 
> > possible. Which one would you prefer?
> 
> It's zapped by the equivalent of mmu_free_roots(), right?  That's 
> effectively unpinning it (by zeroing ->root_count).

no, right now only the guest-visible cache is zapped - the roots are 
zapped by natural rotation. I guess they should be zapped in 
kvm_cr3_cache_clear() - but i wanted to keep that function an invariant 
to the other MMU state, to make it easier to call it from whatever mmu 
codepath.

> However, kvm takes pagefaults even for silly things like setting (in 
> hardware) or clearing (in software) the dirty bit.

yeah. I think it also does some TLB flushes that are not needed. For 
example in rmap_write_protect() we do this:

                rmap_remove(vcpu, spte);
                kvm_arch_ops->tlb_flush(vcpu);

but AFAICS rmap_write_protect() is only ever called if we write a new 
cr3 - hence a TLB flush will happen anyway, because we do a 
vmcs_writel(GUEST_CR3, new_cr3). Am i missing something? I didnt want to 
remove it as part of the cr3 patches (to keep things simpler), but that 
flush looks quite unnecessary to me. The patch below seems to work in 
light testing.

	Ingo

Index: linux/drivers/kvm/mmu.c
===================================================================
--- linux.orig/drivers/kvm/mmu.c
+++ linux/drivers/kvm/mmu.c
@@ -404,7 +404,11 @@ static void rmap_write_protect(struct kv
 		BUG_ON(!(*spte & PT_WRITABLE_MASK));
 		rmap_printk("rmap_write_protect: spte %p %llx\n", spte, *spte);
 		rmap_remove(vcpu, spte);
-		kvm_arch_ops->tlb_flush(vcpu);
+		/*
+		 * While we removed a mapping there's no need to explicitly
+		 * flush the TLB here, because this codepath only triggers
+		 * if we write a new cr3 - which will flush the TLB anyway.
+		 */
 		*spte &= ~(u64)PT_WRITABLE_MASK;
 	}
 }
-
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/