From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marcelo Tosatti <mtosatti@redhat.com>
Subject: Re: [PATCH] kvm mmu notifier
Date: Fri, 4 Jul 2008 14:39:34 -0300
Message-ID: <20080704173934.GA4293@dmt.cnet>
References: <20080626181115.GM14329@duo.random> <20080627224317.GA28057@dmt.cnet> <20080703151742.GF21788@duo.random>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: kvm@vger.kernel.org
To: Andrea Arcangeli <andrea@qumranet.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([66.187.233.31]:50898 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1751243AbYGDRkL (ORCPT <rfc822;kvm@vger.kernel.org>);
	Fri, 4 Jul 2008 13:40:11 -0400
Content-Disposition: inline
In-Reply-To: <20080703151742.GF21788@duo.random>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

Hi Andrea,

On Thu, Jul 03, 2008 at 05:17:42PM +0200, Andrea Arcangeli wrote:

> +static int kvm_unmap_rmapp(struct kvm *kvm, unsigned long *rmapp)
> +{
> +	u64 *spte;
> +	int need_tlb_flush = 0;
> +
> +	while ((spte = rmap_next(kvm, rmapp, NULL))) {
> +		BUG_ON(!(*spte & PT_PRESENT_MASK));
> +		rmap_printk("kvm_rmap_unmap_hva: spte %p %llx\n", spte, *spte);
> +		rmap_remove(kvm, spte);

There's a locking issue with rmap_remove. Other than concurrent memslot
changes which your "read-only browsing" patch covers, there's concurrent
alias changes.

The mmu_shrink path is similarly vulnerable at the moment (see
http://article.gmane.org/gmane.comp.emulators.kvm.devel/19102), we
should change memslots and aliases to be protected by mmu_lock.

> +		set_shadow_pte(spte, shadow_trap_nonpresent_pte);
> +		need_tlb_flush = 1;
> +	}
> +	return need_tlb_flush;
> +}

Still need to nuke large page mappings on invalidate_range, right?
Presently not an issue for invalidate_page.

Other than that looks much simpler, thanks.