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

[Alex Williamson <alex.williamson@xxxxxx>]

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