RE: [Xen-devel] arch_init_memory() change

["Altobelli, David" <david.altobelli@xxxxxx>]

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@xxxxxxxxxxxx] 
> Sent: Saturday, June 23, 2007 10:51 AM
> To: Keir Fraser; Altobelli, David; xen-devel@xxxxxxxxxxxxxxxxxxx
> 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@xxxxxxxxxxxx> 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@xxxxxx> 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@xxxxxxxxxxxxxxxxxxx
> >> http://lists.xensource.com/xen-devel
> > 
> > 
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@xxxxxxxxxxxxxxxxxxx
> > http://lists.xensource.com/xen-devel
> 
> 
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel