From mboxrd@z Thu Jan  1 00:00:00 1970
From: David Vrabel <david.vrabel@citrix.com>
Subject: Re: [PATCH] xen: fix __set_phys_to_machine
Date: Thu, 22 Aug 2013 13:08:29 +0100
Message-ID: <5215FF3D.80804@citrix.com>
References: <1377166214-31577-1-git-send-email-wei.liu2@citrix.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <1377166214-31577-1-git-send-email-wei.liu2@citrix.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Wei Liu <wei.liu2@citrix.com>
Cc: boris.ostrovsky@oracle.com, Stefano Stabellini <stefano.stabellini@eu.citrix.com>, xen-devel@lists.xen.org
List-Id: xen-devel@lists.xenproject.org

On 22/08/13 11:10, Wei Liu wrote:
> In commit cd9151e2: xen/balloon: set a mapping for ballooned out pages
> we have the ballooned out page's mapping set to a scratch page. When the
> page is ballooned in again its P2M entry can be the MFN of the scratch
> page, hitting the BUG_ONs in __set_phys_to_machine.

Looking at the commit that introduced this bug I wonder if the the
correct fix is to restore the original call of
__set_phys_to_machine(pfn, INVALID_P2M_ENTRY) in decrease_reservation().

We only need a valid kernel mapping for the ballooned out page, the p2m
should still be invalid for the ballooned out page, right?

> --- a/arch/x86/xen/p2m.c
> +++ b/arch/x86/xen/p2m.c
> @@ -793,17 +793,27 @@ unsigned long __init set_phys_range_identity(unsigned long pfn_s,
>  	return pfn - pfn_s;
>  }
>  
> +DECLARE_PER_CPU(struct page *, balloon_scratch_page);
>  /* Try to install p2m mapping; fail if intermediate bits missing */
>  bool __set_phys_to_machine(unsigned long pfn, unsigned long mfn)
>  {
>  	unsigned topidx, mididx, idx;
> +	unsigned long balloon_scratch_pfn;
> +	unsigned long balloon_scratch_mfn;
> +
> +	balloon_scratch_pfn = page_to_pfn(__get_cpu_var(balloon_scratch_page));
> +	balloon_scratch_mfn = pfn_to_mfn(balloon_scratch_pfn);
>  
>  	if (unlikely(xen_feature(XENFEAT_auto_translated_physmap))) {
> -		BUG_ON(pfn != mfn && mfn != INVALID_P2M_ENTRY);
> +		BUG_ON(pfn != mfn &&
> +		       pfn != balloon_scratch_mfn &&
> +		       mfn != INVALID_P2M_ENTRY &&
> +		       mfn != balloon_scratch_mfn);
>  		return true;
>  	}
>  	if (unlikely(pfn >= MAX_P2M_PFN)) {
> -		BUG_ON(mfn != INVALID_P2M_ENTRY);
> +		BUG_ON(mfn != INVALID_P2M_ENTRY &&
> +		       mfn != balloon_scratch_mfn);

This bit looks wrong/unnecessary.

David