Re: [XenPPC] [PATCH] Move lots of memory logic earlier

[Hollis Blanchard <hollisb@xxxxxxxxxx>]

On Mon, 2007-01-08 at 18:38 -0500, Jimi Xenidis wrote:
> >   Removing our custom allocator is important to simplify the upcoming
> >   multiboot2 conversion.
> 
> how?

We have currently have three page allocators. The first is PPC-specific,
and it includes the Xen image, RTAS, and our copy of the Open Firmware
device tree. It's also limited to 32 MB, and that makes some of the code
rather hackful (particularly boot_of_alloc_init() and
boot_of_mem_init()).

Then we need to populate the common boot allocator. Right now this is
being done with ad-hoc communication between boot_of.c and memory.c via
a variety of global variables. We make lots of assumptions about the
location of those reserved areas. Since those areas can be placed
arbitrarily (like by a multiboot loader, for example), those assumptions
need to go. So instead we should use the PPC allocator bitmap to
populate the common allocator bitmap, and avoid all these globals.

Except we can't just copy it, because the second bitmap itself is not
present in the first bitmap. We also need to invent an interface to
access the early bitmap, or make it global, and don't you think we
already have too many globals in this area? So now the copy needs to
look something like this:

        i = 0;
        while (1) {
                // here's the accessor we've invented:
                i = boot_of_get_next_available(i, &base, &len);
                if (i == -1)
                        break;
                if (base, len) overlaps with (bitmap_addr, bitmap_len)
                        mangle (base, len) somehow
                init_boot_pages(base, len);
        }
        
If you take a step back, you might ask yourself why we have a bitmap
that's just being copied into another bitmap, when we could have used
the second bitmap all along? So that's what this patch does. In fact,
despite being more flexible than the current code (e.g. it does not
require the init_boot_allocator() to be below _start), this patch
removes more code than it adds.

> > - We also handle arbitrary-sized properties now, instead of
> >   probably-large-enough fixed-sized buffers.
> 
> this is fine by me, I'm a big fan of alloca()!
> However, you use:
>           proplen = of_getprop(child, string, NULL, 0);
> when
>           proplen = of_getproplen(child, string);
> 
> Is sufficient and defined and used already in this file.

Great, will use that.

> > - This will also be needed to support non-Open Firmware systems
> > (though the
> >   firmware-specific interface was not the focus of this patch).
> 
> But what is there is designed with non-OF in mind.
> IMHO this is a step backwards.

"rtas_base" has no place in a firmware-agnostic memory.c, so I think
you'd agree there are at least some rough edges remaining?

I'll think about how to adapt this code to take into account firmware
that passes a flattened tree. In this patch, most of the new code would
need to be duplicated to call "ofd_" routines instead of "of_" routines
(a very poorly-named distinction IMHO).

-- 
Hollis Blanchard
IBM Linux Technology Center


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