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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.08.12 at 12:31, "Hao, Xudong" <xudong.hao@xxxxxxxxx> wrote:
>>  -----Original Message-----
>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> Sent: Thursday, August 16, 2012 6:12 PM
>> To: Hao, Xudong
>> Cc: Zhang, Xiantao; xen-devel@xxxxxxxxxxxxx; tim@xxxxxxx 
>> 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@xxxxxxxxx> wrote:
>> >> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> >> >>> On 15.08.12 at 08:57, Xudong Hao <xudong.hao@xxxxxxxxx> 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


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel