On Thu, 2007-01-04 at 15:56 -0500, Jimi Xenidis wrote:
> On Jan 4, 2007, at 3:02 PM, Hollis Blanchard wrote:
> 
> > Hi Jimi, thanks for the comments.
> >
> > I'm really not interested in reworking all this stuff, which is why I
> > took shortcuts like relocating the modules rather than spending effort
> > on your preferred solution.
> 
> Ok, then it can wait till grub is ready and stable?

GRUB is ready. I'm going to commit the GRUB loader as soon as it can
load Xen (i.e. as soon as it's tested). This patch is the required first
step.

> Otherwise the code does not add anything other than a new data
> structure.
> I thought you intended on pushing this soon?

After the rework we're discussing, but yes.

> > Unfortunately, all this code was intimately
> > linked with the multiboot structure, so I had to change it.
> >
> > On Thu, 2007-01-04 at 12:27 -0500, Jimi Xenidis wrote:
> >> On Jan 3, 2007, at 12:32 PM, Hollis Blanchard wrote:
> >>
> >>> Xen on x86 uses GRUB's "multiboot" capabilities to load an arbitrary
> >>> number of images, including Xen, dom0 kernel, dom0 initrd, and ACM
> >>> policy. On PowerPC, we've had to build Xen, the dom0 kernel and the
> >>> dom0
> >>> initrd all into the same file to get them loaded.
> >>
> >> We also boot from yaboot which allows us to separate dom0 from xen,
> >> as does newer PIBS firmware via r3 & r4 and trivial to teach SLOF and
> >> other OF how as well. Please do not assume that we are fully linked
> >> only and do not remove that logic.  Using your new boot loader case
> >> should be additive.
> >
> > I believe you yourself told me those other methods were unimportant
> > and
> > could be removed. :)
> 
> IIRC I agreed that defining location in the cmdline could certainly
> go, but the r3,r4 pairing should stay.

Will do.

> >>> With this patch, our memory map now looks
> >>> like this:
> >>>
> >>>         exception handlers (0x0 to 0x4000)
> >>>         RTAS
> >>>         Open Firmware device tree
> >>
> >> RTAS and OFD are subject to "avail" properties in from OF and whether
> >> or not they can be claimed.  I don't think you should write code that
> >> assumes they are here.
> >
> > The code does not.
> actually your patch in setup.c assumes the memory after _end is free
> and clear, this is not the case if the OFD was placed after the image
> which is certainly possible, see below.
> 
> > Anyways, regardless of the exact placement (which is
> > subject to "available"), the order remains the same.
> 
> well, not really, RTAS could fit but OFD could easily be after the
> image.  It does now because we I made it smaller becaus SLOF/PIBS
> devtrees are small. Unfortunately apple devtrees are HUGE and the
> original size that we allocated (to accommodate them) cause it to
> exist after the xen image.

OK.

> >>> +static void *alloc_tag(int key, int len)
> >>> +{
> >>> +    struct mb2_tag_header *tag;
> >>> +
> >>
> >> Does len include sizeof(*tag)? it looks like it does, why not make it
> >> implicit?
> >> Since the the largest member of *tag is a uint32_t then it must be 4
> >> byte aligned, please make sure your allocations are rounded up to 4
> >> bytes, unless you _know_ that len is, but I'd do it anyway.
> >
> > It's easy to go back and forth on this issue (I have already). In the
> > end this is best because you can "alloc_tag(FOO, sizeof(foo));"
> 
> I'm sure you have, but please don't forget to round up your allocation.

OK, I missed that point before. It's a good one, since we're allocating
variable-length strings here.

> >> We did a lot of work here so that stuff could be placed anywhere. I
> >> admit it was not pretty but I'd expect this patch to replace/improve
> >> not remove.
> >
> > The memmove below means this logic is unnecessary.
> 
> I'd prefer some logic over blind moves of several megabytes (3-16),
> this will kill simulators.

Yes. I forgot about Stuart. ;)

> >
> >>> +    /* copy modules to a contiguous space immediately after Xen */
> >>
> >> We stopped doing this copy because it was unnecessary, I believe it
> >> still is what the reason for putting it back?
> >
> > The trouble is that interface for adding memory to the heap is start,
> > len.
> >
> > You have a list of an arbitrary number of modules in arbitrary
> > locations
> > of memory. I don't think it's worth it to sort them so you can use
> > that
> > interface.
> 
> IMHO, sorting would be trivial, or even better a bitmap would do
> nicely and be the most efficient.

I looked at the bitmap approach, specifically exporting the
yet-another-bitmap we're creating in early boot for the early early
allocator. I decided I was over-engineering it at the time.

> We'll need boot_of_alloc_init() to remove all the modules from
> allocation consideration before we do any kind of allocation for RTAS
> of OFD.  We need to do something here anyway.
>
> I do not think it is fair to assume that you can have that space.
> 
> As of now, if the modules are linked in we have a large hunk of
> memory (in the xen image) that will not go away.

OK, I'll look into it some more.

-- 
Hollis Blanchard
IBM Linux Technology Center


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

Reply via email to