Keir Fraser wrote:
> On 10/11/2009 08:19, "Xiao Guangrong" <xiaoguangrong@xxxxxxxxxxxxxx> wrote:
>
>>> Firstly, your 'break' was not inside that if-else block; it was right at the
>>> end of the for loop. Secondly, just because we found one RAM region entirely
>>> beyond the end of the clip boundary, does not mean there isn't another. We
>>> can't just bail -- we have to iterate all the way to the end of the e820
>>> map.
>>>
>> I think that sanitize_e820_map() can sort e820 items from low address
>> to high address, so, if we meet one e820 item beyond the end of the clip
>> boundary, subsequent items also beyond it.
>>
>> Maybe I misunderstand sanitize_e820_map()? I'll reread it :-)
>
> No, you understand it. And if we meet one e820 item beyond the end of the
> clip boundary, all subsequent items are also beyond it. But that doesn't
> mean we shouldn't handle them -- in fact we must handle them, as one of them
> could be E820_RAM. Right?
>
Yeah, It's my mistake, Thanks very much, Keir!
And I think find_max_pfn() can be optimized. like this:
--- ../a/xen/arch/x86/e820.c 2009-08-06 21:57:27.000000000 +0800
+++ ../b/xen/arch/x86/e820.c 2009-10-25 17:31:42.762997342 +0800
@@ -312,8 +312,9 @@ static unsigned long __init find_max_pfn
}
#endif
- for (i = 0; i < e820.nr_map; i++) {
+ for (i = e820.nr_map -1; i >= 0; i--) {
unsigned long start, end;
+
/* RAM? */
if (e820.map[i].type != E820_RAM)
continue;
@@ -321,8 +322,8 @@ static unsigned long __init find_max_pfn
end = PFN_DOWN(e820.map[i].addr + e820.map[i].size);
if (start >= end)
continue;
- if (end > max_pfn)
- max_pfn = end;
+ max_pfn = end;
+ break;
}
return max_pfn;
Thanks,
Xiao
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home