Re: [Xen-ia64-devel] [PATCH 14/14] memmap: allow huge size efi memory map of real machine

[Isaku Yamahata <yamahata@xxxxxxxxxxxxx>]

On Wed, May 30, 2007 at 05:26:50PM -0600, Alex Williamson wrote:

>    Thanks for the debug patch.  It helps, but I didn't have enough time
> today to find the problem.  I'm back to the case where my main test
> system boots fine, but another one hangs.  I'll attach the boot log with
> the debug output.  However, I do have a few comments and found another
> issue or two.  See below.  Thanks,

Thank you very much for review and test. It will greatly helps.

I will fix unsigned long cast, size_t and PAGE_MASK.
For do {} while () loop, see below.


> > +
> >                         if (!(md->attribute & EFI_MEMORY_WB))
> >                                 break;
> >  
> > -                       start = max(FW_END_PADDR, start);
> > -                       end = min(start + dom_mem, end);
> > -                       if (end <= start)
> > -                               break;
> > -
> > -                       dom_md->type = EFI_CONVENTIONAL_MEMORY;
> > -                       dom_md->phys_addr = start;
> > -                       dom_md->virt_addr = 0;
> > -                       dom_md->num_pages = (end - start) >>
> > EFI_PAGE_SHIFT;
> > -                       dom_md->attribute = EFI_MEMORY_WB;
> > -                       num_mds++;
> > -
> > -                       dom_mem -= dom_md->num_pages <<
> > EFI_PAGE_SHIFT;
> > -                       break;
> > +                       dom_md_start = max(tables->fw_end_paddr,
> > start);
> > +                       dom_md_end = dom_md_start;
> > +                       do {
> > +                               dom_md_end =
> > +                                       min(dom_md_end + left_mem,
> > end);
> > +                               if (dom_md_end < dom_md_start +
> > PAGE_SIZE)
> > +                                       break;
> > +
> > +                               dom_md->type =
> > EFI_CONVENTIONAL_MEMORY;
> > +                               dom_md->phys_addr = dom_md_start;
> > +                               dom_md->virt_addr = 0;
> > +                               dom_md->num_pages =
> > +                                       (dom_md_end - dom_md_start) >>
> > +                                       EFI_PAGE_SHIFT;
> > +                               dom_md->attribute = EFI_MEMORY_WB;
> > +
> > +                               assign_new_domain0_range(d, dom_md);
> > +                               /*
> > +                                * recalculate left_mem.
> > +                                * we might already allocated memory
> > in
> > +                                * this region because of kernel
> > loader.
> > +                                * So we might consumed less than
> > +                                * (dom_md_end - dom_md_start) above.
> > +                                */
> > +                               left_mem = (d->max_pages -
> > d->tot_pages) <<
> > +                                       PAGE_SHIFT;
> > +                       } while (left_mem > 0 && dom_md_end < end);
> 
> I don't see why this do {} while () is here.  In what case can it
> repeat?

loaddomainelfimage() or other functions might have already allocated
pages in the region.


                                                  dom_md_end
     |<------------------left_mem--------------------->|
   start=dom_md_start                                  V         end
     |----------------------------------------------------------->|
             |<---------image_size---------->|           
               kernel image/initrd image                   
               This page already allocated by              
               construct_dom0()/loaddomainelfimage()


Suppose left_mem << (end - start).
At first loop, dom_md_end = dom_md_start + left_mem.
However assign_new_domain0_page() consumes only
left_mem - image_size which is less than left_mem.
At second loop
dom_md_end = dom_md_start + (left_mem of first loop) + image_size.


-- 
yamahata

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