Re: [PATCH] xen: fix __set_phys_to_machine

[Wei Liu <wei.liu2@citrix.com>]

On Thu, Aug 22, 2013 at 01:08:29PM +0100, David Vrabel wrote:
> 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?
> 

Indeed, this is a simple and straightforward fix. Patch on the way.

Wei.

> > --- 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