Re: [XenPPC] [PATCH] Move lots of memory logic earlier
On Jan 10, 2007, at 12:43 PM, Hollis Blanchard wrote: On Tue, 2007-01-09 at 12:24 -0500, Jimi Xenidis wrote: 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. You say we can't trust "claim", hmm, "trust" implies a lot, I guess... but a) we trust "available" properties, We trust "available" to have correct information but not to be complete, or up to date. and b) we trust the return code from "claim". Excellent point, we trust the "claim" is implemented and that it _may_ object if the address conflicts. If "claim" is not implemented _at_all_ then the current code would interpret it as rejecting all addresses and we would be screwed. So the only thing you could mean is that we don't trust that "claims" will be reflected in the "available" properties. Is that what you mean? Where have we seen that? IIRC: PIBS: - does not implement "available", but may in the next release (so I'm told) - "claim" will tell you only about PIBS conflicts - will not tell you about conflicts with other claims or loaded images SLOF: - does implement, but does not update "available", though recent resions might - "claim" will only tell you about conflicts its self - will not tell you about conflicts with other claims or loaded images GFW: does everything as spec'ed Apple: does everything as spec'ed -JX ___ Xen-ppc-devel mailing list Xen-ppc-devel@lists.xensource.com http://lists.xensource.com/xen-ppc-devel
Re: [XenPPC] [PATCH] Move lots of memory logic earlier
On Tue, 2007-01-09 at 12:24 -0500, Jimi Xenidis wrote: > > > 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. You say we can't trust "claim", but a) we trust "available" properties, and b) we trust the return code from "claim". So the only thing you could mean is that we don't trust that "claims" will be reflected in the "available" properties. Is that what you mean? Where have we seen that? -- Hollis Blanchard IBM Linux Technology Center ___ Xen-ppc-devel mailing list Xen-ppc-devel@lists.xensource.com http://lists.xensource.com/xen-ppc-devel
Re: [XenPPC] [PATCH] Move lots of memory logic earlier
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@lists.xensource.com
http://lists.xensource.com/xen-ppc-devel
Re: [XenPPC] [PATCH] Move lots of memory logic earlier
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@lists.xensource.com
http://lists.xensource.com/xen-ppc-devel
Re: [XenPPC] [PATCH] Move lots of memory logic earlier
I disagree with what this patch does. On Jan 8, 2007, at 4:03 PM, Hollis Blanchard wrote: # HG changeset patch # User Hollis Blanchard <[EMAIL PROTECTED]> # Date 1168293789 21600 # Node ID e1ee8b26c15de7afd7dec2d604b00d607e1307f4 # Parent dbc74db14a4b39d359365fcf8257216d968fa269 Move lots of memory logic earlier. - We now require the common boot allocator has been initialized before __start_xen(), and we use that in boot_of.c instead of managing our own. It is far more important that we do as little as possible in boot_of, just enough to know where we are and instantiate any parts of memory that need it. When we are booted with a flattened devtree (via kexec, or some other loader), we will never call into boot_of.c. The custom boot_of allocator is trivial and allows for simple tracking of available memory. Removing our custom allocator is important to simplify the upcoming multiboot2 conversion. how? - 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. - 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. -JX ___ Xen-ppc-devel mailing list Xen-ppc-devel@lists.xensource.com http://lists.xensource.com/xen-ppc-devel
[XenPPC] [PATCH] Move lots of memory logic earlier
# HG changeset patch
# User Hollis Blanchard <[EMAIL PROTECTED]>
# Date 1168293789 21600
# Node ID e1ee8b26c15de7afd7dec2d604b00d607e1307f4
# Parent dbc74db14a4b39d359365fcf8257216d968fa269
Move lots of memory logic earlier.
- We now require the common boot allocator has been initialized before
__start_xen(), and we use that in boot_of.c instead of managing our own.
Removing our custom allocator is important to simplify the upcoming
multiboot2 conversion.
- We also handle arbitrary-sized properties now, instead of
probably-large-enough fixed-sized buffers.
- This will also be needed to support non-Open Firmware systems (though the
firmware-specific interface was not the focus of this patch).
Signed-off-by: Hollis Blanchard <[EMAIL PROTECTED]>
diff -r dbc74db14a4b -r e1ee8b26c15d xen/arch/powerpc/boot_of.c
--- a/xen/arch/powerpc/boot_of.cTue Dec 12 14:35:07 2006 -0600
+++ b/xen/arch/powerpc/boot_of.cMon Jan 08 16:03:09 2007 -0600
@@ -13,13 +13,14 @@
* along with this program; if not, write to the Free Software
* Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*
- * Copyright (C) IBM Corp. 2005, 2006
+ * Copyright IBM Corp. 2005, 2006, 2007
*
* Authors: Jimi Xenidis <[EMAIL PROTECTED]>
* Hollis Blanchard <[EMAIL PROTECTED]>
*/
#include
+#include
#include
#include
#include
@@ -35,19 +36,32 @@
#include "oftree.h"
#include "rtas.h"
+typedef ulong (*of_walk_fn)(int node, void *arg);
+
/* Secondary processors use this for handshaking with main processor. */
volatile unsigned int __spin_ack;
static ulong of_vec;
static ulong of_msr;
static int of_out;
-static ulong eomem;
-
-#define MEM_AVAILABLE_PAGES ((32 << 20) >> PAGE_SHIFT)
-static DECLARE_BITMAP(mem_available_pages, MEM_AVAILABLE_PAGES);
+static uint addr_cells;
+static uint size_cells;
+
+ulong xenheap_size;
+ulong xenheap_phys_end;
extern char builtin_cmdline[];
extern struct ns16550_defaults ns16550;
+
+extern ulong nr_pages;
+extern ulong bitmap_addr;
+
+/*
+ * opt_xenheap_megabytes: Size of Xen heap in megabytes, excluding the
+ * page_info table and allocation bitmap.
+ */
+static unsigned int opt_xenheap_megabytes = XENHEAP_DEFAULT_MB;
+integer_param("xenheap_megabytes", opt_xenheap_megabytes);
#undef OF_DEBUG
#undef OF_DEBUG_LOW
@@ -72,6 +86,11 @@ struct of_service {
u32 ofs_nargs;
u32 ofs_nrets;
u32 ofs_args[10];
+};
+
+struct membuf {
+ulong start;
+ulong size;
};
static int bof_chosen;
@@ -377,234 +396,204 @@ static int __init of_open(const char *de
return rets[0];
}
-static void boot_of_alloc_init(int m, uint addr_cells, uint size_cells)
-{
-int rc;
-uint pg;
-uint a[64];
-int tst;
-u64 start;
-u64 size;
-
-rc = of_getprop(m, "available", a, sizeof (a));
-if (rc > 0) {
-int l = rc / sizeof(a[0]);
-int r = 0;
-
-#ifdef OF_DEBUG
-{
-int i;
-of_printf("avail:\n");
-for (i = 0; i < l; i += 4)
-of_printf(" 0x%x%x, 0x%x%x\n",
- a[i], a[i + 1],
- a[i + 2] ,a[i + 3]);
-}
-#endif
-
-pg = 0;
-while (pg < MEM_AVAILABLE_PAGES && r < l) {
-ulong end;
-
-start = a[r++];
-if (addr_cells == 2 && (r < l) )
-start = (start << 32) | a[r++];
-
-size = a[r++];
-if (size_cells == 2 && (r < l) )
-size = (size << 32) | a[r++];
-
-end = ALIGN_DOWN(start + size, PAGE_SIZE);
-
-start = ALIGN_UP(start, PAGE_SIZE);
-
-DBG("%s: marking 0x%x - 0x%lx\n", __func__,
-pg << PAGE_SHIFT, start);
-
-start >>= PAGE_SHIFT;
-while (pg < MEM_AVAILABLE_PAGES && pg < start) {
-set_bit(pg, mem_available_pages);
-pg++;
+
+static ulong boot_of_alloc(ulong size)
+{
+return alloc_boot_pages(size >> PAGE_SHIFT, 1) << PAGE_SHIFT;
+}
+
+static ulong of_walk_type(int root, const char *type, of_walk_fn fn, void *arg)
+{
+int child;
+ulong rc;
+
+child = of_getchild(root);
+while (child > 0) {
+int proplen;
+char *propval;
+
+proplen = of_getprop(child, "device_type", NULL, 0);
+if (proplen > 0) {
+propval = alloca(proplen);
+proplen = of_getprop(child, "device_type", propval, proplen);
+if (!strcmp(propval, type)) {
+/* if the type matches, call the callback */
+rc = fn(child, arg);
+if (rc)
+return rc;
}
-
-pg = end >> PAGE_SHIFT;
-}
-}
-
-/* Now make sure we mark our own memory */
-pg = (ulong)_start >> PAGE_SHIFT;
-start = (ulong)_end >> PAGE_SHIFT;
-
-DBG("%s: marking 0x%x - 0x%lx\n", __func__,
-pg << PAGE_SHIFT, start <<
