Re: [PATCH 2/3] xen/p2m: Using INVALID_MFN instead of mfn_valid

Re: [PATCH 2/3] xen/p2m: Using INVALID_MFN instead of mfn_valid

[Andres Lagar-Cavilla]

>
> At 11:41 +0100 on 16 Aug (1345117281), Jan Beulich wrote:
>> >>> On 16.08.12 at 12:31, "Hao, Xudong" <xudong.hao@intel.com> wrote:
>> >> >> >>> On 15.08.12 at 08:57, Xudong Hao <xudong.hao@intel.com> wrote:
>> >> >> > --- a/xen/arch/x86/mm/p2m-ept.c	Tue Jul 24 17:02:04 2012 +0200
>> >> >> > +++ b/xen/arch/x86/mm/p2m-ept.c	Thu Jul 26 15:40:01 2012 +0800
>> >> >> > @@ -428,7 +428,7 @@ ept_set_entry(struct p2m_domain *p2m, un
>> >> >> >      }
>> >> >> >
>> >> >> >      /* Track the highest gfn for which we have ever had a valid
>> mapping
>> >> */
>> >> >> > -    if ( mfn_valid(mfn_x(mfn)) &&
>> >> >> > +    if ( (mfn_x(mfn) != INVALID_MFN) &&
>> >> >> >           (gfn + (1UL << order) - 1 > p2m->max_mapped_pfn) )
>> >> >> >          p2m->max_mapped_pfn = gfn + (1UL << order) - 1;
>> >> >>
>> >> >> Depending on how the above comment gets addressed (i.e.
>> >> >> whether MMIO MFNs are to be considered here at all), this
>> >> >> might need changing anyway, as this a huge max_mapped_pfn
>> >> >> value likely wouldn't be very useful anymore.
>> >> >
>> >> > Your viewpoint is similar with us. Here max_mapped_pfn value is for
>> memory
>> >> > but not for MMIO. I think this is a simple changes, do you have
>> another
>> >> > suggestion?
>> >>
>> >> The question is why this needs to be changed at all. If this is
>> >> only about RAM, then mfn_valid() is the right thing to use. If
>> >> this is about MMIO too, then the condition is wrong already
>> >> (since, as we appear to agree, even now there can be MMIO
>> >> above RAM, provided there's little enough RAM).
>> >>
>> >
>> > The original code considered EPT only, now for the device assignment,
>> it
>> > need to consider MMIO. So how about remove the mfn_valid() here?
>>
>> I don't think it's there without reason, but I'm not sure. Tim?
>
> max_mapped_pfn should be the highest entry that's even had a mapping in
> the p2m.  Its intent was to provide a fast path exit from p2m lookups in
> the (at the time) common case where _emulated_ MMIO addresses were
> higher than all the actual p2m mappings, and the cost of a failed lookup
> (on 32-bit) was a page fault in the linear map.  Also, at the time, the
> p2m wasn't typed and we didn't support direct MMIO, so mfn_valid() was
> equivalent to 'entry is present'.
>
> These days, I'm not sure how useful max_mapped_pfn is, since (a) for any
> VM with >3GB RAM the emulated MMIO lookups are not avoided, and (b) on
> 64-bit builds there's not pagefault for a failed lookup.  Also it seems to
> have been abused in a few places to do for() loops that touch every PFN
> instead of just walking the tries.  So I might get rid of it after 4.2
> is out.

max_mapped_pfn also helps keep XENMEM_maximum_gpfn O(1).
Andres

>
> In the meantime, the patch at the top of this thread is definitely an
> improvement.  However, I think this is a better fix:
>
> diff -r c887c30a0a35 xen/arch/x86/mm/p2m-ept.c
> --- a/xen/arch/x86/mm/p2m-ept.c	Thu Aug 16 10:16:19 2012 +0200
> +++ b/xen/arch/x86/mm/p2m-ept.c	Thu Aug 16 11:57:44 2012 +0100
> @@ -428,7 +428,7 @@ ept_set_entry(struct p2m_domain *p2m, un
>      }
>
>      /* Track the highest gfn for which we have ever had a valid mapping
> */
> -    if ( mfn_valid(mfn_x(mfn)) &&
> +    if ( p2mt != p2m_invalid &&
>           (gfn + (1UL << order) - 1 > p2m->max_mapped_pfn) )
>          p2m->max_mapped_pfn = gfn + (1UL << order) - 1;
>
> diff -r c887c30a0a35 xen/arch/x86/mm/p2m-pt.c
> --- a/xen/arch/x86/mm/p2m-pt.c	Thu Aug 16 10:16:19 2012 +0200
> +++ b/xen/arch/x86/mm/p2m-pt.c	Thu Aug 16 11:57:44 2012 +0100
> @@ -454,7 +454,7 @@ p2m_set_entry(struct p2m_domain *p2m, un
>      }
>
>      /* Track the highest gfn for which we have ever had a valid mapping
> */
> -    if ( mfn_valid(mfn)
> +    if ( p2mt != p2m_invalid
>           && (gfn + (1UL << page_order) - 1 > p2m->max_mapped_pfn) )
>          p2m->max_mapped_pfn = gfn + (1UL << page_order) - 1;
>
> and I'll commit it this afternoon or tomorrow.
>
> Tim.
>

Re: [PATCH 2/3] xen/p2m: Using INVALID_MFN instead of mfn_valid

[Tim Deegan]

At 10:17 -0700 on 16 Aug (1345112267), Andres Lagar-Cavilla wrote:
> > max_mapped_pfn should be the highest entry that's even had a mapping in
> > the p2m.  Its intent was to provide a fast path exit from p2m lookups in
> > the (at the time) common case where _emulated_ MMIO addresses were
> > higher than all the actual p2m mappings, and the cost of a failed lookup
> > (on 32-bit) was a page fault in the linear map.  Also, at the time, the
> > p2m wasn't typed and we didn't support direct MMIO, so mfn_valid() was
> > equivalent to 'entry is present'.
> >
> > These days, I'm not sure how useful max_mapped_pfn is, since (a) for any
> > VM with >3GB RAM the emulated MMIO lookups are not avoided, and (b) on
> > 64-bit builds there's not pagefault for a failed lookup.  Also it seems to
> > have been abused in a few places to do for() loops that touch every PFN
> > instead of just walking the tries.  So I might get rid of it after 4.2
> > is out.
> 
> max_mapped_pfn also helps keep XENMEM_maximum_gpfn O(1).

Ergh, true.  With a little tidying up in the tree update code (to
eliminate empty nodes) it will still be O(1) - just walking
rightmost-first down a trie of fixed height.  But that's a little
more work than I hoped for. :)

Tim.

[PATCH 2/3] xen/p2m: Using INVALID_MFN instead of mfn_valid

[Xudong Hao]

64 bits big bar's MMIO address may out of the highest gfn, then mfn_valid 
may return failure, so using INVALID_MFN to measure.

Signed-off-by: Xiantao Zhang <xiantao.zhang@intel.com>
Signed-off-by: Xudong Hao <xudong.hao@intel.com>

diff -r 663eb766cdde xen/arch/x86/mm/p2m-ept.c
--- a/xen/arch/x86/mm/p2m-ept.c	Tue Jul 24 17:02:04 2012 +0200
+++ b/xen/arch/x86/mm/p2m-ept.c	Thu Jul 26 15:40:01 2012 +0800
@@ -428,7 +428,7 @@ ept_set_entry(struct p2m_domain *p2m, un
     }
 
     /* Track the highest gfn for which we have ever had a valid mapping */
-    if ( mfn_valid(mfn_x(mfn)) &&
+    if ( (mfn_x(mfn) != INVALID_MFN) &&
          (gfn + (1UL << order) - 1 > p2m->max_mapped_pfn) )
         p2m->max_mapped_pfn = gfn + (1UL << order) - 1;

Re: [PATCH 2/3] xen/p2m: Using INVALID_MFN instead of mfn_valid

[Jan Beulich]

>>> On 15.08.12 at 08:57, Xudong Hao <xudong.hao@intel.com> wrote:
> 64 bits big bar's MMIO address may out of the highest gfn, then mfn_valid 
> may return failure, so using INVALID_MFN to measure.

Hmm, that can be true for 32-bit BARs too (on systems with less
than 4Gb).

> Signed-off-by: Xiantao Zhang <xiantao.zhang@intel.com>
> Signed-off-by: Xudong Hao <xudong.hao@intel.com>
> 
> diff -r 663eb766cdde xen/arch/x86/mm/p2m-ept.c
> --- a/xen/arch/x86/mm/p2m-ept.c	Tue Jul 24 17:02:04 2012 +0200
> +++ b/xen/arch/x86/mm/p2m-ept.c	Thu Jul 26 15:40:01 2012 +0800
> @@ -428,7 +428,7 @@ ept_set_entry(struct p2m_domain *p2m, un
>      }
>  
>      /* Track the highest gfn for which we have ever had a valid mapping */
> -    if ( mfn_valid(mfn_x(mfn)) &&
> +    if ( (mfn_x(mfn) != INVALID_MFN) &&
>           (gfn + (1UL << order) - 1 > p2m->max_mapped_pfn) )
>          p2m->max_mapped_pfn = gfn + (1UL << order) - 1;

Depending on how the above comment gets addressed (i.e.
whether MMIO MFNs are to be considered here at all), this
might need changing anyway, as this a huge max_mapped_pfn
value likely wouldn't be very useful anymore.

Jan

Re: [PATCH 2/3] xen/p2m: Using INVALID_MFN instead of mfn_valid

[Hao, Xudong]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: Wednesday, August 15, 2012 5:22 PM
> To: Hao, Xudong
> Cc: Zhang, Xiantao; xen-devel@lists.xen.org; tim@xen.org
> Subject: Re: [Xen-devel] [PATCH 2/3] xen/p2m: Using INVALID_MFN instead of
> mfn_valid
> 
> >>> On 15.08.12 at 08:57, Xudong Hao <xudong.hao@intel.com> wrote:
> > 64 bits big bar's MMIO address may out of the highest gfn, then mfn_valid
> > may return failure, so using INVALID_MFN to measure.
> 
> Hmm, that can be true for 32-bit BARs too (on systems with less
> than 4Gb).
> 

Exactly right, thanks comments.

> > Signed-off-by: Xiantao Zhang <xiantao.zhang@intel.com>
> > Signed-off-by: Xudong Hao <xudong.hao@intel.com>
> >
> > diff -r 663eb766cdde xen/arch/x86/mm/p2m-ept.c
> > --- a/xen/arch/x86/mm/p2m-ept.c	Tue Jul 24 17:02:04 2012 +0200
> > +++ b/xen/arch/x86/mm/p2m-ept.c	Thu Jul 26 15:40:01 2012 +0800
> > @@ -428,7 +428,7 @@ ept_set_entry(struct p2m_domain *p2m, un
> >      }
> >
> >      /* Track the highest gfn for which we have ever had a valid mapping */
> > -    if ( mfn_valid(mfn_x(mfn)) &&
> > +    if ( (mfn_x(mfn) != INVALID_MFN) &&
> >           (gfn + (1UL << order) - 1 > p2m->max_mapped_pfn) )
> >          p2m->max_mapped_pfn = gfn + (1UL << order) - 1;
> 
> Depending on how the above comment gets addressed (i.e.
> whether MMIO MFNs are to be considered here at all), this
> might need changing anyway, as this a huge max_mapped_pfn
> value likely wouldn't be very useful anymore.
> 

Jan,

Your viewpoint is similar with us. Here max_mapped_pfn value is for memory but not for MMIO. I think this is a simple changes, do you have another suggestion?

> Jan
> 

Re: [PATCH 2/3] xen/p2m: Using INVALID_MFN instead of mfn_valid

[Jan Beulich]

>>> On 16.08.12 at 12:05, "Hao, Xudong" <xudong.hao@intel.com> wrote:
>> From: Jan Beulich [mailto:JBeulich@suse.com]
>> >>> On 15.08.12 at 08:57, Xudong Hao <xudong.hao@intel.com> wrote:
>> > --- a/xen/arch/x86/mm/p2m-ept.c	Tue Jul 24 17:02:04 2012 +0200
>> > +++ b/xen/arch/x86/mm/p2m-ept.c	Thu Jul 26 15:40:01 2012 +0800
>> > @@ -428,7 +428,7 @@ ept_set_entry(struct p2m_domain *p2m, un
>> >      }
>> >
>> >      /* Track the highest gfn for which we have ever had a valid mapping */
>> > -    if ( mfn_valid(mfn_x(mfn)) &&
>> > +    if ( (mfn_x(mfn) != INVALID_MFN) &&
>> >           (gfn + (1UL << order) - 1 > p2m->max_mapped_pfn) )
>> >          p2m->max_mapped_pfn = gfn + (1UL << order) - 1;
>> 
>> Depending on how the above comment gets addressed (i.e.
>> whether MMIO MFNs are to be considered here at all), this
>> might need changing anyway, as this a huge max_mapped_pfn
>> value likely wouldn't be very useful anymore.
> 
> Your viewpoint is similar with us. Here max_mapped_pfn value is for memory 
> but not for MMIO. I think this is a simple changes, do you have another 
> suggestion?

The question is why this needs to be changed at all. If this is
only about RAM, then mfn_valid() is the right thing to use. If
this is about MMIO too, then the condition is wrong already
(since, as we appear to agree, even now there can be MMIO
above RAM, provided there's little enough RAM).

Jan

Re: [PATCH 2/3] xen/p2m: Using INVALID_MFN instead of mfn_valid

[Hao, Xudong]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: Thursday, August 16, 2012 6:12 PM
> To: Hao, Xudong
> Cc: Zhang, Xiantao; xen-devel@lists.xen.org; tim@xen.org
> Subject: RE: [Xen-devel] [PATCH 2/3] xen/p2m: Using INVALID_MFN instead of
> mfn_valid
> 
> >>> On 16.08.12 at 12:05, "Hao, Xudong" <xudong.hao@intel.com> wrote:
> >> From: Jan Beulich [mailto:JBeulich@suse.com]
> >> >>> On 15.08.12 at 08:57, Xudong Hao <xudong.hao@intel.com> wrote:
> >> > --- a/xen/arch/x86/mm/p2m-ept.c	Tue Jul 24 17:02:04 2012 +0200
> >> > +++ b/xen/arch/x86/mm/p2m-ept.c	Thu Jul 26 15:40:01 2012 +0800
> >> > @@ -428,7 +428,7 @@ ept_set_entry(struct p2m_domain *p2m, un
> >> >      }
> >> >
> >> >      /* Track the highest gfn for which we have ever had a valid mapping
> */
> >> > -    if ( mfn_valid(mfn_x(mfn)) &&
> >> > +    if ( (mfn_x(mfn) != INVALID_MFN) &&
> >> >           (gfn + (1UL << order) - 1 > p2m->max_mapped_pfn) )
> >> >          p2m->max_mapped_pfn = gfn + (1UL << order) - 1;
> >>
> >> Depending on how the above comment gets addressed (i.e.
> >> whether MMIO MFNs are to be considered here at all), this
> >> might need changing anyway, as this a huge max_mapped_pfn
> >> value likely wouldn't be very useful anymore.
> >
> > Your viewpoint is similar with us. Here max_mapped_pfn value is for memory
> > but not for MMIO. I think this is a simple changes, do you have another
> > suggestion?
> 
> The question is why this needs to be changed at all. If this is
> only about RAM, then mfn_valid() is the right thing to use. If
> this is about MMIO too, then the condition is wrong already
> (since, as we appear to agree, even now there can be MMIO
> above RAM, provided there's little enough RAM).
> 

The original code considered EPT only, now for the device assignment, it need to consider MMIO. So how about remove the mfn_valid() here?

> Jan

Re: [PATCH 2/3] xen/p2m: Using INVALID_MFN instead of mfn_valid

[Jan Beulich]

>>> On 16.08.12 at 12:31, "Hao, Xudong" <xudong.hao@intel.com> wrote:
>>  -----Original Message-----
>> From: Jan Beulich [mailto:JBeulich@suse.com]
>> Sent: Thursday, August 16, 2012 6:12 PM
>> To: Hao, Xudong
>> Cc: Zhang, Xiantao; xen-devel@lists.xen.org; tim@xen.org 
>> Subject: RE: [Xen-devel] [PATCH 2/3] xen/p2m: Using INVALID_MFN instead of
>> mfn_valid
>> 
>> >>> On 16.08.12 at 12:05, "Hao, Xudong" <xudong.hao@intel.com> wrote:
>> >> From: Jan Beulich [mailto:JBeulich@suse.com]
>> >> >>> On 15.08.12 at 08:57, Xudong Hao <xudong.hao@intel.com> wrote:
>> >> > --- a/xen/arch/x86/mm/p2m-ept.c	Tue Jul 24 17:02:04 2012 +0200
>> >> > +++ b/xen/arch/x86/mm/p2m-ept.c	Thu Jul 26 15:40:01 2012 +0800
>> >> > @@ -428,7 +428,7 @@ ept_set_entry(struct p2m_domain *p2m, un
>> >> >      }
>> >> >
>> >> >      /* Track the highest gfn for which we have ever had a valid mapping
>> */
>> >> > -    if ( mfn_valid(mfn_x(mfn)) &&
>> >> > +    if ( (mfn_x(mfn) != INVALID_MFN) &&
>> >> >           (gfn + (1UL << order) - 1 > p2m->max_mapped_pfn) )
>> >> >          p2m->max_mapped_pfn = gfn + (1UL << order) - 1;
>> >>
>> >> Depending on how the above comment gets addressed (i.e.
>> >> whether MMIO MFNs are to be considered here at all), this
>> >> might need changing anyway, as this a huge max_mapped_pfn
>> >> value likely wouldn't be very useful anymore.
>> >
>> > Your viewpoint is similar with us. Here max_mapped_pfn value is for memory
>> > but not for MMIO. I think this is a simple changes, do you have another
>> > suggestion?
>> 
>> The question is why this needs to be changed at all. If this is
>> only about RAM, then mfn_valid() is the right thing to use. If
>> this is about MMIO too, then the condition is wrong already
>> (since, as we appear to agree, even now there can be MMIO
>> above RAM, provided there's little enough RAM).
>> 
> 
> The original code considered EPT only, now for the device assignment, it 
> need to consider MMIO. So how about remove the mfn_valid() here?

I don't think it's there without reason, but I'm not sure. Tim?

Jan

Re: [PATCH 2/3] xen/p2m: Using INVALID_MFN instead of mfn_valid

[Tim Deegan]

At 11:41 +0100 on 16 Aug (1345117281), Jan Beulich wrote:
> >>> On 16.08.12 at 12:31, "Hao, Xudong" <xudong.hao@intel.com> wrote:
> >> >> >>> On 15.08.12 at 08:57, Xudong Hao <xudong.hao@intel.com> wrote:
> >> >> > --- a/xen/arch/x86/mm/p2m-ept.c	Tue Jul 24 17:02:04 2012 +0200
> >> >> > +++ b/xen/arch/x86/mm/p2m-ept.c	Thu Jul 26 15:40:01 2012 +0800
> >> >> > @@ -428,7 +428,7 @@ ept_set_entry(struct p2m_domain *p2m, un
> >> >> >      }
> >> >> >
> >> >> >      /* Track the highest gfn for which we have ever had a valid mapping
> >> */
> >> >> > -    if ( mfn_valid(mfn_x(mfn)) &&
> >> >> > +    if ( (mfn_x(mfn) != INVALID_MFN) &&
> >> >> >           (gfn + (1UL << order) - 1 > p2m->max_mapped_pfn) )
> >> >> >          p2m->max_mapped_pfn = gfn + (1UL << order) - 1;
> >> >>
> >> >> Depending on how the above comment gets addressed (i.e.
> >> >> whether MMIO MFNs are to be considered here at all), this
> >> >> might need changing anyway, as this a huge max_mapped_pfn
> >> >> value likely wouldn't be very useful anymore.
> >> >
> >> > Your viewpoint is similar with us. Here max_mapped_pfn value is for memory
> >> > but not for MMIO. I think this is a simple changes, do you have another
> >> > suggestion?
> >> 
> >> The question is why this needs to be changed at all. If this is
> >> only about RAM, then mfn_valid() is the right thing to use. If
> >> this is about MMIO too, then the condition is wrong already
> >> (since, as we appear to agree, even now there can be MMIO
> >> above RAM, provided there's little enough RAM).
> >> 
> > 
> > The original code considered EPT only, now for the device assignment, it 
> > need to consider MMIO. So how about remove the mfn_valid() here?
> 
> I don't think it's there without reason, but I'm not sure. Tim?

max_mapped_pfn should be the highest entry that's even had a mapping in
the p2m.  Its intent was to provide a fast path exit from p2m lookups in
the (at the time) common case where _emulated_ MMIO addresses were
higher than all the actual p2m mappings, and the cost of a failed lookup
(on 32-bit) was a page fault in the linear map.  Also, at the time, the
p2m wasn't typed and we didn't support direct MMIO, so mfn_valid() was
equivalent to 'entry is present'.

These days, I'm not sure how useful max_mapped_pfn is, since (a) for any
VM with >3GB RAM the emulated MMIO lookups are not avoided, and (b) on
64-bit builds there's not pagefault for a failed lookup.  Also it seems to
have been abused in a few places to do for() loops that touch every PFN
instead of just walking the tries.  So I might get rid of it after 4.2
is out.

In the meantime, the patch at the top of this thread is definitely an
improvement.  However, I think this is a better fix:

diff -r c887c30a0a35 xen/arch/x86/mm/p2m-ept.c
--- a/xen/arch/x86/mm/p2m-ept.c	Thu Aug 16 10:16:19 2012 +0200
+++ b/xen/arch/x86/mm/p2m-ept.c	Thu Aug 16 11:57:44 2012 +0100
@@ -428,7 +428,7 @@ ept_set_entry(struct p2m_domain *p2m, un
     }
 
     /* Track the highest gfn for which we have ever had a valid mapping */
-    if ( mfn_valid(mfn_x(mfn)) &&
+    if ( p2mt != p2m_invalid &&
          (gfn + (1UL << order) - 1 > p2m->max_mapped_pfn) )
         p2m->max_mapped_pfn = gfn + (1UL << order) - 1;
 
diff -r c887c30a0a35 xen/arch/x86/mm/p2m-pt.c
--- a/xen/arch/x86/mm/p2m-pt.c	Thu Aug 16 10:16:19 2012 +0200
+++ b/xen/arch/x86/mm/p2m-pt.c	Thu Aug 16 11:57:44 2012 +0100
@@ -454,7 +454,7 @@ p2m_set_entry(struct p2m_domain *p2m, un
     }
 
     /* Track the highest gfn for which we have ever had a valid mapping */
-    if ( mfn_valid(mfn) 
+    if ( p2mt != p2m_invalid
          && (gfn + (1UL << page_order) - 1 > p2m->max_mapped_pfn) )
         p2m->max_mapped_pfn = gfn + (1UL << page_order) - 1;
 
and I'll commit it this afternoon or tomorrow.

Tim.

Re: [PATCH 2/3] xen/p2m: Using INVALID_MFN instead of mfn_valid

[Mukesh Rathor]

On Thu, 16 Aug 2012 10:31:30 +0000
"Hao, Xudong" <xudong.hao@intel.com> wrote:

> > */
> > >> > -    if ( mfn_valid(mfn_x(mfn)) &&
> > >> > +    if ( (mfn_x(mfn) != INVALID_MFN) &&
> > >> >           (gfn + (1UL << order) - 1 > p2m->max_mapped_pfn) )
> > >> >          p2m->max_mapped_pfn = gfn + (1UL << order) - 1;
> > >>


BTW, here's the change in my PVH/hybrid tree that Tim D had suggested 
couple months ago:

     /* Track the highest gfn for which we have ever had a valid mapping */
     -    if ( mfn_valid(mfn_x(mfn)) &&
     +    if ( p2mt != p2m_invalid && p2mt != p2m_mmio_dm &&
               (gfn + (1UL << order) - 1 > p2m->max_mapped_pfn) )
	                p2m->max_mapped_pfn = gfn + (1UL << order) - 1;

thanks,
Mukesh

[PATCH 2/3] xen/p2m: Using INVALID_MFN instead of mfn_valid

[Xudong Hao]

64 bits big bar's MMIO address may out of the highest gfn, then mfn_valid 
may return failure, so using INVALID_MFN to measure.

Signed-off-by: Xiantao Zhang <xiantao.zhang@intel.com>
Signed-off-by: Xudong Hao <xudong.hao@intel.com>

diff -r 663eb766cdde xen/arch/x86/mm/p2m-ept.c
--- a/xen/arch/x86/mm/p2m-ept.c	Tue Jul 24 17:02:04 2012 +0200
+++ b/xen/arch/x86/mm/p2m-ept.c	Thu Jul 26 15:40:01 2012 +0800
@@ -428,7 +428,7 @@ ept_set_entry(struct p2m_domain *p2m, un
     }
 
     /* Track the highest gfn for which we have ever had a valid mapping */
-    if ( mfn_valid(mfn_x(mfn)) &&
+    if ( (mfn_x(mfn) != INVALID_MFN) &&
          (gfn + (1UL << order) - 1 > p2m->max_mapped_pfn) )
         p2m->max_mapped_pfn = gfn + (1UL << order) - 1;