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

[Jimi Xenidis <jimix@xxxxxxxxxxxxxx>]

On Jan 9, 2007, at 11:24 AM, Hollis Blanchard wrote:

> 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.
More precisely, it is OF-specific and exists because we cannot trust  
the "claim" OF-method, so really it is more of a workaround.  This is  
analogous to the custom allocator in prom.c in Linux.
The ultimate use of boot_of.c is to get the minimum information  
necessary need from OF, put everything in a canonical form and then  
go to common code.  So if we are ever booted without OF, we would run  
some other boot_XXX.c and then go to common code.
> 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()).
Not hacky at all it is a practical limit for OF that we empirically  
know is sufficient.
> 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.
I agree, but formalizing the canonical form and having minimal  
"translators" to get us to that point is the correct way to go.  I  
believe that right now we are using multiboot2 as our canonical form.
Once that is designed we can happily leave the globals behind.

> 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?
agreed

> 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.
the bitmap need not exist beyond boot_of.c but we do need an ordered  
way to describe memory that is occupied once OF is terminated and we  
enter common code.
I am beginning to think that multiboot2 is insufficient and is simply  
yet another temporary boot-time form that should not be accessed from  
common boot code and whose information could be expressed in a  
flatened-devtree or the current OFD that is used by common code  
beyond setup.c.
> > > - 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,
I would argue that neither does bultiboot2.

> so I think
> you'd agree there are at least some rough edges remaining?
Oh I do indeed! :) Just looking for improvement rather then another  
shuffle which we all seem to do quite a bit.
> 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).
I know your sentiments well.  "of_"  prefixes for OpenFirmware calls  
and "ofd_" OpenFirmwareDevtree calls. AFAICT it is a definate  
improvement over the prefixes in Linux.
-JX



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