Re: [PATCH] check memory descriptor over lap in dom_fw_init() (was Re: [Xen-ia64-devel] [PATCH]Fix domain0 no VGA console bug.)

[Isaku Yamahata <yamahata@xxxxxxxxxxxxx>]

Hi Alex.

You're right. It could be simplified. It had also a bug.
The new patch is attached. please find it.

Thanks.

On Thu, Jun 01, 2006 at 01:19:02PM -0600, Alex Williamson wrote:
> Hi Isaku,
> 
>    I like this idea, this should really help creating dom0s with large
> memory (the swiotlb doesn't like it when we don't put any memory below
> 4GB).  I'm having trouble with the loop below though, it seems overly
> complicated, but maybe I'm missing something important.  We're just
> looking for gaps in the MDT above the hypercall areas, right?  Could the
> original be simplified as something like this (untested):
> 
>     for (j = 0; j < num_mds; j++) {
>             efi_memory_desc_t* next_md;
>             unsigned long end;
> 
>             md = &efi_memmap[j];
>             end = md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT);
>             if (maxmem < start)
>                     break;
> 
>             // Avoid "legacy" low memory addresses and the
>             // HYPERCALL patch area.
>             if (md->phys_addr < HYPERCALL_END) {
>                     BUG_ON(end > HYPERCALL_START);
>                     continue;
>             }
> 
>             if (j + 1 == num_mds && end < maxmem) {
>                     // End of list, map memory
>                     MAKE_MD(EFI_CONVENTIONAL_MEMORY, EFI_MEMORY_WB,
>                             end, maxmem, 0);
>                     continue;
>             }
> 
>             next_md = &efi_memmap[j + 1];
> 
>             if (next_md->phys_addr > end) {
>                     // Found a gap, insert memory.
>                     // Should we look for some minimum sized gap?
>                     MAKE_MD(EFI_CONVENTIONAL_MEMORY, EFI_MEMORY_WB,
>                             end, min(next_md->phys_addr, maxmem), 0);
>                     continue;
>             } else if (next_md->phys_addr < end) {
>                     // Bad, overlapping MDT ranges
>                     BUG_ON(next_md->phys_addr < end);
>             }
>     }
> 
> The MDT we're parsing is already sorted, so there's no reason to look
> further than j + 1 for the next gap.  I'm not sure I follow all of the
> tests at the end of the original loop, but I think I've captured them
> above.  Does this look right?  Thanks,
> 
>       Alex
> 
> On Thu, 2006-06-01 at 11:46 +0900, Isaku Yamahata wrote:
> > +#ifdef CONFIG_XEN_IA64_DOM0_VP
> > +        // simple
> > +        // MAKE_MD(EFI_CONVENTIONAL_MEMORY, EFI_MEMORY_WB,
> > +        //         HYPERCALL_END, maxmem, 0);
> > +        // is not good. Check overlap.
> > +        sort(efi_memmap, i, sizeof(efi_memory_desc_t), efi_mdt_cmp,
> > NULL);
> > +
> > +        num_mds = i;
> > +        for (j = 0; j < num_mds; j++) {
> > +            unsigned long start;
> > +            unsigned long end;
> > +            unsigned long next_start;
> > +            int k;
> > +            
> > +            md = &efi_memmap[j];
> > +            start = md->phys_addr;
> > +            end = md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT);
> > +
> > +            if (maxmem < start)
> > +                break;
> > +
> > +            // find gap
> > +            next_start = maxmem;
> > +            for (k = j + 1; k < num_mds; k++) {
> > +                efi_memory_desc_t* next_md = &efi_memmap[k];
> > +
> > +                if (end != next_md->phys_addr) {
> > +                    next_start = next_md->phys_addr;
> > +                    break;
> > +                }
> > +                end = next_md->phys_addr +
> > +                    (next_md->num_pages << EFI_PAGE_SHIFT);
> > +            }
> > +
> > +            if (next_start < HYPERCALL_END)
> > +                continue;
> > +
> > +            if (end < HYPERCALL_END)
> > +                end = HYPERCALL_END;
> > +            if (next_start > maxmem)
> > +                next_start = maxmem;
> > +            MAKE_MD(EFI_CONVENTIONAL_MEMORY, EFI_MEMORY_WB,
> > +                    end, next_start, 0);
> > +        }
> > +#endif   
> -- 
> Alex Williamson                             HP Open Source & Linux Org.
> 
> 
> _______________________________________________
> Xen-ia64-devel mailing list
> Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-ia64-devel
> 

-- 
yamahata

Attachment: 10236:8036118ac726_assign_page_to_dom0_precisely.patch
Description: Text document

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