Re: [XenPPC] [patch] multiboot2 support
On Thu, 2007-01-04 at 15:56 -0500, Jimi Xenidis wrote: 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. Wait a minute, doesn't systemsim has a passthrough call for memmove? If we should wire that up then this won't impact performance at all. -- 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] multiboot2 support
On Jan 9, 2007, at 12:09 PM, Hollis Blanchard wrote: On Thu, 2007-01-04 at 15:56 -0500, Jimi Xenidis wrote: 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. Wait a minute, doesn't systemsim has a passthrough call for memmove? If we should wire that up then this won't impact performance at all. We were/are trying to eliminate all simulator specific passthrus in the Xen core code. -JX ___ Xen-ppc-devel mailing list Xen-ppc-devel@lists.xensource.com http://lists.xensource.com/xen-ppc-devel
Re: [XenPPC] [patch] multiboot2 support
On Jan 9, 2007, at 12:34 PM, Segher Boessenkool wrote: Wait a minute, doesn't systemsim has a passthrough call for memmove? If we should wire that up then this won't impact performance at all. We were/are trying to eliminate all simulator specific passthrus in the Xen core code. That sounds rather counterproductive. What's wrong with having some minimal passthroughs? They help performance a *lot*, it's quite painful to run without. Of course, a command line option to disable it would be nice. There are currently only 3 large copies in xen, dom0-kernel, dom0- initrd (optional), and dom0-devtree after that the passthru gains us very little since the passthru is unavailable to the domain since the passthru requires machine addresses. Thing is in fast mode systemsim is good enuff, right now the biggest simulation problem is uncompressing kernels, and even that is not so bad. We have only one required on_sim() call and that is because systemsim does not provide a DART simulation (which means that any IO is unusable). Further more, I'm hoping to see more simulators each having more accelerators further complicating the passthru logic. I'd rather simply push back to the simulators. -JX ___ Xen-ppc-devel mailing list Xen-ppc-devel@lists.xensource.com http://lists.xensource.com/xen-ppc-devel
Re: [XenPPC] [patch] multiboot2 support
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.
Since we currently
boot directly from Open Firmware, early PPC Xen code then fakes up a
multiboot info structure which later PPC Xen code extracts data from.
GRUB2, which supports PowerPC, is defining a new multiboot format
because the original was far too x86-specific. That loader is not yet
committed but is fairly complete, so to test it out I'm adapting
PPC Xen
to it.
This is the first step: I've replaced our early multiboot code to fake
up a multiboot2 structure instead. I haven't yet tried to boot this
from
GRUB2, but we will likely want to continue supporting directly booting
from firmware so this code needs to work.
Of interest is that I've changed the memory map to simplify some of
the
early memory allocation code (which unlike x86 tried to handle
unordered
discontiguous allocations). 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.
boot allocator bitmap (common Xen code)
Xen heap
...
_start (0x40; 64MB)
0x40 is 4MiB
I had a paragraph on how 64MiB was bad and then realized it is still
4MiB :)
[Xen text/data]
_end
modules
[dom0, dom0 initrd, etc]
Xen heap
...
[fixed size]
domain heap
...
[consumes all remaining memory]
Comments and testing are welcome, or I'll probably check this in in a
day or so.
Signed-off-by: Hollis Blanchard [EMAIL PROTECTED]
diff -r dbc74db14a4b 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.cWed Jan 03 10:50:07 2007 -0600
@@ -22,7 +22,7 @@
#include xen/config.h
#include xen/init.h
#include xen/lib.h
-#include xen/multiboot.h
+#include xen/multiboot2.h
#include xen/version.h
#include xen/spinlock.h
#include xen/serial.h
@@ -30,6 +30,7 @@
#include xen/sched.h
#include asm/page.h
#include asm/io.h
+#include asm/boot.h
#include exceptions.h
#include of-devtree.h
#include oftree.h
@@ -77,6 +78,28 @@ static int bof_chosen;
static int bof_chosen;
static struct of_service s;
+
+static unsigned char xentags[512];
+static int xentags_pos;
+
+static int of_printf(const char *fmt, ...)
+__attribute__ ((format (printf, 1, 2)));
+
+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.
+if (xentags_pos + len sizeof(xentags))
+of_panic(Couldn't allocate multiboot tag.);
+
+tag = (struct mb2_tag_header *)(xentags + xentags_pos);
+tag-key = key;
+tag-len = len;
+
+xentags_pos += len;
+
+return tag;
+}
static int __init of_call(
const char *service, u32 nargs, u32 nrets, s32 rets[], ...)
@@ -160,8 +183,6 @@ static int __init of_write(int ih, const
return sum;
}
-static int of_printf(const char *fmt, ...)
-__attribute__ ((format (printf, 1, 2)));
static int __init of_printf(const char *fmt, ...)
{
static char buf[1024];
@@ -609,8 +630,11 @@ static ulong boot_of_mem_init(void)
return 0;
}
-static void boot_of_bootargs(multiboot_info_t *mbi)
-{
+static char *boot_of_bootargs(char **dom0_cmdline)
+{
+static const char *sepr[] = { -- , || };
+char *p;
+int sepr_index;
int rc;
if (builtin_cmdline[0] == '\0') {
@@ -620,10 +644,22 @@ static void boot_of_bootargs(multiboot_i
of_panic(bootargs[] not big enough for /chosen/
bootargs\n);
}
-mbi-flags |= MBI_CMDLINE;
-mbi-cmdline = (ulong)builtin_cmdline;
-
of_printf(bootargs = %s\n, builtin_cmdline);
+
+/* look for delimiter: -- or || */
+for (sepr_index = 0; sepr_index ARRAY_SIZE(sepr); sepr_index+
+){
+p = strstr((char *)builtin_cmdline, sepr[sepr_index]);
Is the cast necessary?
+if (p != NULL) {
+/* Xen proper
Re: [XenPPC] [patch] multiboot2 support
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. 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. :)
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. Anyways, regardless of the exact placement (which is
subject to available), the order remains the same.
+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));
+static char *boot_of_bootargs(char **dom0_cmdline)
+{
+static const char *sepr[] = { -- , || };
+char *p;
+int sepr_index;
int rc;
if (builtin_cmdline[0] == '\0') {
@@ -620,10 +644,22 @@ static void boot_of_bootargs(multiboot_i
of_panic(bootargs[] not big enough for /chosen/
bootargs\n);
}
-mbi-flags |= MBI_CMDLINE;
-mbi-cmdline = (ulong)builtin_cmdline;
-
of_printf(bootargs = %s\n, builtin_cmdline);
+
+/* look for delimiter: -- or || */
+for (sepr_index = 0; sepr_index ARRAY_SIZE(sepr); sepr_index+
+){
+p = strstr((char *)builtin_cmdline, sepr[sepr_index]);
Is the cast necessary?
Doesn't look like it.
+if (p != NULL) {
+/* Xen proper should never know about the dom0 args. */
+*(char *)p = '\0';
Why casting here?
This code was moved from another location where p was const. I'll fix.
+static int __init boot_of_rtas(void)
{
int rtas_node;
int rtas_instance;
@@ -1026,12 +1060,10 @@ static int __init boot_of_rtas(module_t
rtas_end = mem + size;
rtas_msr = of_msr;
-mod-mod_start = rtas_base;
-mod-mod_end = rtas_end;
return 1;
hmm, aren't you going to create a tag so you know where RTAS has been
placed and how big it is?
No. RTAS is not a module GRUB passes to kernels.
+/* Create a structure as if we were loaded by a multiboot
bootloader. */
+struct mb2_tag_header __init *boot_of_multiboot(void)
+{
+struct mb2_tag_start *start;
+struct mb2_tag_name *name;
+struct mb2_tag_module *xenmod;
+struct mb2_tag_end *end;
+char *xen_cmdline;
+char *dom0_cmdline = NULL;
+static char *namestr = xen;
+
+/* create start tag. start-size is filled in later. */
+start = alloc_tag(MB2_TAG_START, sizeof(*start));
+
+/* create name tag. */
+name = alloc_tag(MB2_TAG_NAME, sizeof(*name) + strlen(namestr));
Are you intentionally skipping the '\0' that in the alloc
Nope, good catch!
@@ -141,43 +143,9 @@ static void ofd_walk_mem(void *m, walk_m
}
}
-static void setup_xenheap(module_t *mod, int mcount)
-{
-int i;
-ulong freemem;
-
-freemem = ALIGN_UP((ulong)_end, PAGE_SIZE);
-
-for (i = 0; i mcount; i++) {
-u32 s;
-
-if (mod[i].mod_end == mod[i].mod_start)
-continue;
-
-s = ALIGN_DOWN(mod[i].mod_start, PAGE_SIZE);
-
-if (mod[i].mod_start (ulong)_start
-mod[i].mod_start (ulong)_end) {
-/* mod was linked in */
-continue;
-}
-
-if (s freemem)
-panic(module addresses must assend\n);
-
-free_xenheap(freemem, s);
-freemem = ALIGN_UP(mod[i].mod_end, PAGE_SIZE);
-
-}
-
-/* the rest of the xenheap, starting at the end of
Re: [XenPPC] [patch] multiboot2 support
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?
Otherwise the code does not add anything other than a new data
structure.
I thought you intended on pushing this soon?
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.
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.
+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.
+static char *boot_of_bootargs(char **dom0_cmdline)
+{
+static const char *sepr[] = { -- , || };
+char *p;
+int sepr_index;
int rc;
if (builtin_cmdline[0] == '\0') {
@@ -620,10 +644,22 @@ static void boot_of_bootargs(multiboot_i
of_panic(bootargs[] not big enough for /chosen/
bootargs\n);
}
-mbi-flags |= MBI_CMDLINE;
-mbi-cmdline = (ulong)builtin_cmdline;
-
of_printf(bootargs = %s\n, builtin_cmdline);
+
+/* look for delimiter: -- or || */
+for (sepr_index = 0; sepr_index ARRAY_SIZE(sepr); sepr_index+
+){
+p = strstr((char *)builtin_cmdline, sepr[sepr_index]);
Is the cast necessary?
Doesn't look like it.
+if (p != NULL) {
+/* Xen proper should never know about the dom0
args. */
+*(char *)p = '\0';
Why casting here?
This code was moved from another location where p was const. I'll fix.
+static int __init boot_of_rtas(void)
{
int rtas_node;
int rtas_instance;
@@ -1026,12 +1060,10 @@ static int __init boot_of_rtas(module_t
rtas_end = mem + size;
rtas_msr = of_msr;
-mod-mod_start = rtas_base;
-mod-mod_end = rtas_end;
return 1;
hmm, aren't you going to create a tag so you know where RTAS has been
placed and how big it is?
No. RTAS is not a module GRUB passes to kernels.
right, no tag is necessary since you have rtas_{base,end}.
+/* Create a structure as if we were loaded by a multiboot
bootloader. */
+struct mb2_tag_header __init *boot_of_multiboot(void)
+{
+struct mb2_tag_start *start;
+struct mb2_tag_name *name;
+struct mb2_tag_module *xenmod;
+struct mb2_tag_end *end;
+char *xen_cmdline;
+char *dom0_cmdline = NULL;
+static char *namestr = xen;
+
+/* create start tag. start-size is filled in later. */
+start = alloc_tag(MB2_TAG_START, sizeof(*start));
+
+/* create name tag. */
+name = alloc_tag(MB2_TAG_NAME, sizeof(*name) + strlen
(namestr));
Are you intentionally skipping the '\0' that in the alloc
Nope, good catch!
@@ -141,43 +143,9 @@
[XenPPC] [patch] multiboot2 support
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. Since we currently
boot directly from Open Firmware, early PPC Xen code then fakes up a
multiboot info structure which later PPC Xen code extracts data from.
GRUB2, which supports PowerPC, is defining a new multiboot format
because the original was far too x86-specific. That loader is not yet
committed but is fairly complete, so to test it out I'm adapting PPC Xen
to it.
This is the first step: I've replaced our early multiboot code to fake
up a multiboot2 structure instead. I haven't yet tried to boot this from
GRUB2, but we will likely want to continue supporting directly booting
from firmware so this code needs to work.
Of interest is that I've changed the memory map to simplify some of the
early memory allocation code (which unlike x86 tried to handle unordered
discontiguous allocations). With this patch, our memory map now looks
like this:
exception handlers (0x0 to 0x4000)
RTAS
Open Firmware device tree
boot allocator bitmap (common Xen code)
Xen heap
...
_start (0x40; 64MB)
[Xen text/data]
_end
modules
[dom0, dom0 initrd, etc]
Xen heap
...
[fixed size]
domain heap
...
[consumes all remaining memory]
Comments and testing are welcome, or I'll probably check this in in a
day or so.
Signed-off-by: Hollis Blanchard [EMAIL PROTECTED]
diff -r dbc74db14a4b 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.cWed Jan 03 10:50:07 2007 -0600
@@ -22,7 +22,7 @@
#include xen/config.h
#include xen/init.h
#include xen/lib.h
-#include xen/multiboot.h
+#include xen/multiboot2.h
#include xen/version.h
#include xen/spinlock.h
#include xen/serial.h
@@ -30,6 +30,7 @@
#include xen/sched.h
#include asm/page.h
#include asm/io.h
+#include asm/boot.h
#include exceptions.h
#include of-devtree.h
#include oftree.h
@@ -77,6 +78,28 @@ static int bof_chosen;
static int bof_chosen;
static struct of_service s;
+
+static unsigned char xentags[512];
+static int xentags_pos;
+
+static int of_printf(const char *fmt, ...)
+__attribute__ ((format (printf, 1, 2)));
+
+static void *alloc_tag(int key, int len)
+{
+struct mb2_tag_header *tag;
+
+if (xentags_pos + len sizeof(xentags))
+of_panic(Couldn't allocate multiboot tag.);
+
+tag = (struct mb2_tag_header *)(xentags + xentags_pos);
+tag-key = key;
+tag-len = len;
+
+xentags_pos += len;
+
+return tag;
+}
static int __init of_call(
const char *service, u32 nargs, u32 nrets, s32 rets[], ...)
@@ -160,8 +183,6 @@ static int __init of_write(int ih, const
return sum;
}
-static int of_printf(const char *fmt, ...)
-__attribute__ ((format (printf, 1, 2)));
static int __init of_printf(const char *fmt, ...)
{
static char buf[1024];
@@ -609,8 +630,11 @@ static ulong boot_of_mem_init(void)
return 0;
}
-static void boot_of_bootargs(multiboot_info_t *mbi)
-{
+static char *boot_of_bootargs(char **dom0_cmdline)
+{
+static const char *sepr[] = { -- , || };
+char *p;
+int sepr_index;
int rc;
if (builtin_cmdline[0] == '\0') {
@@ -620,10 +644,22 @@ static void boot_of_bootargs(multiboot_i
of_panic(bootargs[] not big enough for /chosen/bootargs\n);
}
-mbi-flags |= MBI_CMDLINE;
-mbi-cmdline = (ulong)builtin_cmdline;
-
of_printf(bootargs = %s\n, builtin_cmdline);
+
+/* look for delimiter: -- or || */
+for (sepr_index = 0; sepr_index ARRAY_SIZE(sepr); sepr_index++){
+p = strstr((char *)builtin_cmdline, sepr[sepr_index]);
+if (p != NULL) {
+/* Xen proper should never know about the dom0 args. */
+*(char *)p = '\0';
+p += strlen(sepr[sepr_index]);
+*dom0_cmdline = p;
+of_printf(%s: dom0 cmdline: %s\n, __func__, *dom0_cmdline);
+break;
+}
+}
+
+return builtin_cmdline;
}
static int save_props(void *m, ofdn_t n, int pkg)
@@ -894,8 +930,8 @@ static void __init boot_of_fix_maple(voi
}
}
}
-
-static int __init boot_of_serial(void *oft)
+
+void __init boot_of_serial(void *oft)
{
int n;
int p;
@@ -975,11 +1011,9 @@ static int __init boot_of_serial(void *o
__func__, ns16550.irq);
ns16550.irq = 0;
}
-
-return 1;
-}
-
-static int __init boot_of_rtas(module_t *mod, multiboot_info_t *mbi)
+}
+
+static int __init boot_of_rtas(void)
{
int rtas_node;
int rtas_instance;
@@ -1026,12 +1060,10 @@ static int __init boot_of_rtas(module_t
