In xen/arch/x86/mm.c, arch_init_memory() does not follow the same logic
as find_max_pfn(). If a 4k non page aligned E820_RAM entry exists as
the last RAM entry in the e820 map, it will be skipped by
find_max_pfn(), but not by arch_init_memory(), triggering BUG_ON(pfn !max_page).
The logic in arch_init_memory() could be changed to mirror
find_max_pfn():
--- xen/arch/x86/mm.c.orig 2007-06-22 10:10:19.000000000 -0500
+++ xen/arch/x86/mm.c 2007-06-22 10:11:39.000000000 -0500
@@ -220,6 +220,8 @@ void arch_init_memory(void)
/* Every page from cursor to start of next RAM region is I/O.
*/
rstart_pfn = PFN_UP(e820.map[i].addr);
rend_pfn = PFN_DOWN(e820.map[i].addr + e820.map[i].size);
+ if (rstart_pfn >= rend_pfn)
+ continue;
for ( ; pfn < rstart_pfn; pfn++ )
{
BUG_ON(!mfn_valid(pfn));
But I think this breaks the sharing logic. It might be better to just
remove the BUG_ON():
--- xen/arch/x86/mm.c.orig 2007-06-22 10:10:19.000000000 -0500
+++ xen/arch/x86/mm.c 2007-06-22 11:20:42.000000000 -0500
@@ -229,7 +229,6 @@ void arch_init_memory(void)
/* Skip the RAM region. */
pfn = rend_pfn;
}
- BUG_ON(pfn != max_page);
subarch_init_memory();
}
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
The code in arch/x86/setup.c with comment "Ensure all E820 RAM regions are page-aligned and -sized" is supposed to fix this. In this case it should turn the E820 entry into a zero-sized entry that is then discarded when the e820 map is sanitized. Are you running a version of Xen that''s missing this code? -- Keir On 22/6/07 17:28, "Altobelli, David" <david.altobelli@hp.com> wrote:> In xen/arch/x86/mm.c, arch_init_memory() does not follow the same logic > as find_max_pfn(). If a 4k non page aligned E820_RAM entry exists as > the last RAM entry in the e820 map, it will be skipped by > find_max_pfn(), but not by arch_init_memory(), triggering BUG_ON(pfn !> max_page). > > The logic in arch_init_memory() could be changed to mirror > find_max_pfn(): > --- xen/arch/x86/mm.c.orig 2007-06-22 10:10:19.000000000 -0500 > +++ xen/arch/x86/mm.c 2007-06-22 10:11:39.000000000 -0500 > @@ -220,6 +220,8 @@ void arch_init_memory(void) > /* Every page from cursor to start of next RAM region is I/O. > */ > rstart_pfn = PFN_UP(e820.map[i].addr); > rend_pfn = PFN_DOWN(e820.map[i].addr + e820.map[i].size); > + if (rstart_pfn >= rend_pfn) > + continue; > for ( ; pfn < rstart_pfn; pfn++ ) > { > BUG_ON(!mfn_valid(pfn)); > > But I think this breaks the sharing logic. It might be better to just > remove the BUG_ON(): > --- xen/arch/x86/mm.c.orig 2007-06-22 10:10:19.000000000 -0500 > +++ xen/arch/x86/mm.c 2007-06-22 11:20:42.000000000 -0500 > @@ -229,7 +229,6 @@ void arch_init_memory(void) > /* Skip the RAM region. */ > pfn = rend_pfn; > } > - BUG_ON(pfn != max_page); > > subarch_init_memory(); > } > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Actually, whether that fix works or not, I''m inclined to agree that just fixing this particular e820 iterator is the better way to go. -- Keir On 23/6/07 16:18, "Keir Fraser" <Keir.Fraser@cl.cam.ac.uk> wrote:> The code in arch/x86/setup.c with comment "Ensure all E820 RAM regions are > page-aligned and -sized" is supposed to fix this. In this case it should > turn the E820 entry into a zero-sized entry that is then discarded when the > e820 map is sanitized. Are you running a version of Xen that''s missing this > code? > > -- Keir > > On 22/6/07 17:28, "Altobelli, David" <david.altobelli@hp.com> wrote: > >> In xen/arch/x86/mm.c, arch_init_memory() does not follow the same logic >> as find_max_pfn(). If a 4k non page aligned E820_RAM entry exists as >> the last RAM entry in the e820 map, it will be skipped by >> find_max_pfn(), but not by arch_init_memory(), triggering BUG_ON(pfn !>> max_page). >> >> The logic in arch_init_memory() could be changed to mirror >> find_max_pfn(): >> --- xen/arch/x86/mm.c.orig 2007-06-22 10:10:19.000000000 -0500 >> +++ xen/arch/x86/mm.c 2007-06-22 10:11:39.000000000 -0500 >> @@ -220,6 +220,8 @@ void arch_init_memory(void) >> /* Every page from cursor to start of next RAM region is I/O. >> */ >> rstart_pfn = PFN_UP(e820.map[i].addr); >> rend_pfn = PFN_DOWN(e820.map[i].addr + e820.map[i].size); >> + if (rstart_pfn >= rend_pfn) >> + continue; >> for ( ; pfn < rstart_pfn; pfn++ ) >> { >> BUG_ON(!mfn_valid(pfn)); >> >> But I think this breaks the sharing logic. It might be better to just >> remove the BUG_ON(): >> --- xen/arch/x86/mm.c.orig 2007-06-22 10:10:19.000000000 -0500 >> +++ xen/arch/x86/mm.c 2007-06-22 11:20:42.000000000 -0500 >> @@ -229,7 +229,6 @@ void arch_init_memory(void) >> /* Skip the RAM region. */ >> pfn = rend_pfn; >> } >> - BUG_ON(pfn != max_page); >> >> subarch_init_memory(); >> } >> >> _______________________________________________ >> Xen-devel mailing list >> Xen-devel@lists.xensource.com >> http://lists.xensource.com/xen-devel > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
I noticed this in version 3.0.3-rc5-8.el5, which does not have the code you mention in setup.c. Sorry for not fully checking a newer version. By the way, do you want to be sharing the ACPI region? The choice currently seems to depend on how RAM regions are laid out - if there is a RAM region post-ACPI, then you will share it, otherwise not. dave> -----Original Message----- > From: Keir Fraser [mailto:Keir.Fraser@cl.cam.ac.uk] > Sent: Saturday, June 23, 2007 10:51 AM > To: Keir Fraser; Altobelli, David; xen-devel@lists.xensource.com > Subject: Re: [Xen-devel] arch_init_memory() change > > Actually, whether that fix works or not, I''m inclined to > agree that just > fixing this particular e820 iterator is the better way to go. > > -- Keir > > On 23/6/07 16:18, "Keir Fraser" <Keir.Fraser@cl.cam.ac.uk> wrote: > > > The code in arch/x86/setup.c with comment "Ensure all E820 > RAM regions are > > page-aligned and -sized" is supposed to fix this. In this > case it should > > turn the E820 entry into a zero-sized entry that is then > discarded when the > > e820 map is sanitized. Are you running a version of Xen > that''s missing this > > code? > > > > -- Keir > > > > On 22/6/07 17:28, "Altobelli, David" <david.altobelli@hp.com> wrote: > > > >> In xen/arch/x86/mm.c, arch_init_memory() does not follow > the same logic > >> as find_max_pfn(). If a 4k non page aligned E820_RAM > entry exists as > >> the last RAM entry in the e820 map, it will be skipped by > >> find_max_pfn(), but not by arch_init_memory(), triggering > BUG_ON(pfn !> >> max_page). > >> > >> The logic in arch_init_memory() could be changed to mirror > >> find_max_pfn(): > >> --- xen/arch/x86/mm.c.orig 2007-06-22 10:10:19.000000000 -0500 > >> +++ xen/arch/x86/mm.c 2007-06-22 10:11:39.000000000 -0500 > >> @@ -220,6 +220,8 @@ void arch_init_memory(void) > >> /* Every page from cursor to start of next RAM > region is I/O. > >> */ > >> rstart_pfn = PFN_UP(e820.map[i].addr); > >> rend_pfn = PFN_DOWN(e820.map[i].addr + > e820.map[i].size); > >> + if (rstart_pfn >= rend_pfn) > >> + continue; > >> for ( ; pfn < rstart_pfn; pfn++ ) > >> { > >> BUG_ON(!mfn_valid(pfn)); > >> > >> But I think this breaks the sharing logic. It might be > better to just > >> remove the BUG_ON(): > >> --- xen/arch/x86/mm.c.orig 2007-06-22 10:10:19.000000000 -0500 > >> +++ xen/arch/x86/mm.c 2007-06-22 11:20:42.000000000 -0500 > >> @@ -229,7 +229,6 @@ void arch_init_memory(void) > >> /* Skip the RAM region. */ > >> pfn = rend_pfn; > >> } > >> - BUG_ON(pfn != max_page); > >> > >> subarch_init_memory(); > >> } > >> > >> _______________________________________________ > >> Xen-devel mailing list > >> Xen-devel@lists.xensource.com > >> http://lists.xensource.com/xen-devel > > > > > > > > _______________________________________________ > > Xen-devel mailing list > > Xen-devel@lists.xensource.com > > http://lists.xensource.com/xen-devel > > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 24/6/07 15:00, "Altobelli, David" <david.altobelli@hp.com> wrote:> I noticed this in version 3.0.3-rc5-8.el5, which does not have the code > you mention in setup.c. Sorry for not fully checking a newer version. > > By the way, do you want to be sharing the ACPI region? The choice > currently seems to depend on how RAM regions are laid out - if there is > a RAM region post-ACPI, then you will share it, otherwise not.Everything that isn''t in a page-aligned and -sized e820 ram region should be marked as belonging to dom_io. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel