Re: [XenPPC] Re: patch flow
On Jan 3, 2007, at 12:01 PM, Hollis Blanchard wrote: On Sun, 2006-12-17 at 18:00 +, Xen patchbot-xenppc-unstable wrote: # HG changeset patch # User Jimi Xenidis [EMAIL PROTECTED] # Node ID b04e24db308f2215c6bafaf358d1c10da79f244f # Parent 965d3e42dddaf5971001f7d172d192f925537644 [XEN][POWERPC] get cpu_*_maps correct so physinfo and affinity is accurate Signed-off-by: Jimi Xenidis [EMAIL PROTECTED] Please commit all uncontroversial patches first to xenppc-merge, then pull xenppc-merge into xenppc-unstable. Unfortunately, the above patch was controversial and the rework in on my queue, but thats not the point here. It's OK if we change our minds later and revert changesets in -merge. To recap, xenppc-merge contains patches going upstream; it is pulled directly into xen-unstable. xenppc-unstable is xen-unstable plus any temporary hacks we need to make progress. (When you define xenppc-unstable this way, it obviously should be a child tree, not a parent tree.) I think this model is premature since the only purpose of -merge is to be a consistent pull tree. Unfortunately, -merge is incomplete, may not run or even build, we cannot base work on that. This will ensure that we never have the same difficulty staying in sync with xen-unstable that we've had in the past. I'm not sure how.. Are you asking all developers to send diffs based on the -merge tree? Are you asking the maintainers to go thru the following process? 1) apply diffs to childof-unstable, test 2) apply to childof-merge and then push -merg blind 3) pull -merge into -unstable 4) reset childof-unstable I think we need to get -unstable to a point where patches are acceptable make the tip of -merge be that snapshot and continue working in -unstable. So lets bear down and get whatever change set that is in -unstable into -merge. We can consider what you propose when -merge works completely -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 @@
RE: [XenPPC] systemsim-gpul problems
Whad'ya know...if you wait long enough it works! Thanks Mark. From: Mark F Mergen [mailto:[EMAIL PROTECTED] Sent: Thursday, January 04, 2007 4:30 PM To: Yoder Stuart-B08248; xen-ppc-devel@lists.xensource.com Subject: Fw: [XenPPC] systemsim-gpul problems Sorry, I should have read your note more carefully. Yes, my latest builds do boot all the way into Linux, but the timing is drastically altered from the past. There are places in the boot sequence (and I think what you mention: after protocol family 17 is one of them) where you must wait a VERY long time for something to happen. I can't tell you exactly how long, but it took me a while and accidental inattention to what was happening before I discovered this. I have not had time to work on why this is so. Also, if you do get all the way to a Linux command prompt, you may conclude it is not responding to commands. Just type a command that you think should work (such as pwd), hit Enter, and then WAIT A LONG TIME. If your experience turns out to be like mine, eventually the characters you typed will be echoed and the command will be executed with proper output, however slowly. Obviously, more debugging is needed. Maybe it's something simple, like some simulator option for how real time is reflected to the simulated machine. Maybe you will be rewarded by a little (or a LOT of) patience in your next try. Mark Mergen - Forwarded by Mark F Mergen/Watson/IBM on 01/04/2007 05:15 PM - Mark F Mergen/Watson/IBM 01/04/2007 05:12 PM To [EMAIL PROTECTED] cc Yoder Stuart-B08248 [EMAIL PROTECTED], xen-ppc-devel@lists.xensource.com Subject Re: [XenPPC] systemsim-gpul problemsLink Notes://D01MLC04/852564F1000C2D56/C57E6BC8EC0391E7852571CA007FA25B/9BD9 00E03249E9608525725900705623 I pull only from xenppc-unstable.hg and linux-ppc-2.6.hg. I'm up to date with all latest that was pushed to them, and this combo runs on systemsim-gpul. There was a bug(s?) that caused symptoms like you mention, but they were fixed by Amos Waterland about 2nd week in December, and pushed to aformentioned repositories by the maintainers. I can't quote or vouch for specific changesets. Mark Mergen [EMAIL PROTECTED] Sent by: [EMAIL PROTECTED] 01/04/2007 03:11 PM Please respond to [EMAIL PROTECTED] To Yoder Stuart-B08248 [EMAIL PROTECTED] cc xen-ppc-devel@lists.xensource.com Subject Re: [XenPPC] systemsim-gpul problems On Thu, 2007-01-04 at 10:32 -0700, Yoder Stuart-B08248 wrote: I've been trying to get systemsim-gpul working with the latest Xen-PPC. The last changeset that worked was 7ad4645e7a54 (11/22/06). Changeset ce8c1e26b2ae (Early boot memory avoidance improvemnts) broke the simulator with the 'Could not allocate RTAS tree' / HANG error. With changeset 878ce1f78ad3 (Fix systemsim-gpul failure to boot) and later changesets the RTAS allocation / HANG problem is fixed but the simulator still won't boot into Linux. If I compare logs of the working and non-working Xen/Linux runs in the simulator, with the current Xen Linux hangs near the end of its boot. Last messages from Linux are: i2c /dev entries driver IPv4 over IPv4 tunneling driver TCP bic registered NET: Registered protocol family 1 NET: Registered protocol family 17 ...then nothing. Shortly after this point in the boot is where the RAMDISK is decompressed and accessed. I'm wondering if the boot related memory improvements have affected a RAMDISK built into Linux and Xen. Have you tried attaching GDB to systemsim to figure out what's going on? Has anyone else had recent changesets working on the simulator? I haven't tried simulator in quite some time... -- Hollis Blanchard IBM Linux Technology Center ___ Xen-ppc-devel mailing list Xen-ppc-devel@lists.xensource.com http://lists.xensource.com/xen-ppc-devel ___ Xen-ppc-devel mailing list Xen-ppc-devel@lists.xensource.com http://lists.xensource.com/xen-ppc-devel
RE: [XenPPC] systemsim-gpul problems
On Thu, 2007-01-04 at 16:48 -0700, Yoder Stuart-B08248 wrote: Whad'ya know...if you wait long enough it works! So simulator performance is acceptable until mid-way through dom0 boot? It would be good to figure out the source of the slowdown. -- 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] systemsim-gpul problems
Could be that midway through a Linux boot all code is sequential, but at a later point Linux tries to initialize its timing facilities. This involves linux spinning on the timebase register which forces systemsim to simulate many, many useless cycles. Beyond that Linux initialization sees a lot of cache initialization which requires memory zeroing, which also is not simulator friendly. I see this sort of behavior simply by running linux on the simulator wit or without a hypervisor. On Thu, 2007-01-04 at 18:19 -0600, Hollis Blanchard wrote: On Thu, 2007-01-04 at 16:48 -0700, Yoder Stuart-B08248 wrote: Whad'ya know...if you wait long enough it works! So simulator performance is acceptable until mid-way through dom0 boot? It would be good to figure out the source of the slowdown. -- Michal Ostrowski [EMAIL PROTECTED] ___ Xen-ppc-devel mailing list Xen-ppc-devel@lists.xensource.com http://lists.xensource.com/xen-ppc-devel
RE: [XenPPC] systemsim-gpul problems: reserved memory areas
On Thu, 2007-01-04 at 14:15 -0700, Yoder Stuart-B08248 wrote: I have turned on debug prints in arch/powerpc/boot_of.c. One thing I'm puzzling over is why boot_of_alloc_init() is marking overlapping regions of memory. Does that make sense?? It doesn't exactly make sense, but in this case it's not a real problem... [snip] boot_of_alloc_init: marking 0x0 - 0x0^M boot_of_alloc_init: marking 0x0 - 0x400^M These two come from your Open Firmware device tree. They don't make sense IMHO. systemsim may have a bug in its device tree. We could also have a bug in the way we parse the available property, but I believe our code has been tested on PowerMac and SLOF, and it looks correct to me. boot_of_alloc_init: marking 0x40 - 0x88b000^M This is our code reserving Xen's text, _start to _end. boot_of_alloc_init: marking 0x0 - 0x3000^M This is our code reserving the exception handlers. -- Hollis Blanchard IBM Linux Technology Center ___ Xen-ppc-devel mailing list Xen-ppc-devel@lists.xensource.com http://lists.xensource.com/xen-ppc-devel
[XenPPC] [PATCH]fix xencomm_copy_{from, to}_guest.
fix xencomm_copy_{from, to}_guest. It should not call paddr_to_maddr() with invalid address. Originally this issue was found in xen/ia64 xencomm code and in fact I didn't test this patch because currently ia64 xencomm forked from common code. They should be consolidated somehow. -- yamahata # HG changeset patch # User [EMAIL PROTECTED] # Date 1167966417 -32400 # Node ID 25cdcc5f21f8147371aed5fb8f56d93479df0ca8 # Parent 338ceb7b1f0993bf9735c0c1c5d21e39c381cf2f fix xencomm_copy_{from, to}_guest. It should not call paddr_to_maddr() with invalid address. PATCHNAME: fix_xencomm_copy_tofrom_guest Signed-off-by: Isaku Yamahata [EMAIL PROTECTED] diff -r 338ceb7b1f09 -r 25cdcc5f21f8 xen/common/xencomm.c --- a/xen/common/xencomm.c Thu Jan 04 10:58:01 2007 + +++ b/xen/common/xencomm.c Fri Jan 05 12:06:57 2007 +0900 @@ -119,7 +119,7 @@ xencomm_copy_from_guest(void *to, const chunksz -= chunk_skip; skip -= chunk_skip; -if (skip == 0) { +if (skip == 0 chunksz 0) { unsigned long src_maddr; unsigned long dest = (unsigned long)to + to_pos; unsigned int bytes = min(chunksz, n - to_pos); @@ -225,7 +225,7 @@ xencomm_copy_to_guest(void *to, const vo chunksz -= chunk_skip; skip -= chunk_skip; -if (skip == 0) { +if (skip == 0 chunksz 0) { unsigned long dest_maddr; unsigned long source = (unsigned long)from + from_pos; unsigned int bytes = min(chunksz, n - from_pos); ___ Xen-ppc-devel mailing list Xen-ppc-devel@lists.xensource.com http://lists.xensource.com/xen-ppc-devel