On Thu, Sep 18, 2008 at 11:59:13PM +0200, Carl-Daniel Hailfinger wrote:
I thought Ron and Stefan asked for unconditional lbtable
They did, and I changed that in my last patch. The rationale being:
- lbtable is useful in some situations (Stefan)
- size is not that important (Ron)
and conditional multiboot.
They didn't. You must be confused.
This patch lacks all the documentation contained in the earlier versions. Please fix.
Can you be more specific? I'm not sure which documentation are you referring to.
- /* FIXME: is this alignment needed for PIRQ table? */
- rom_table_end = (rom_table_end + 1023) & ~1023;
Please explain the two lines above. They are not mentioned in the changelog.
Before my patch, PIRQ tables were implicitly 1k-aligned due to their hardcoded location being 0xf0000. Inmediately after writing them, this 1k-alignment is enforced because it's apparantly needed for ACPI tables. However, existing code doesn't give any indication on whether PIRQ tables require the same alignment or not.
Therefore, since my code pushes them, I thought it's better to be safe than sorry and add the same alignment.
But if someone can answer the question on whether this alignment is needed or not, then we can remove either one or both of these lines.
Index: arch/x86/stage1.c
--- arch/x86/stage1.c (revision 867) +++ arch/x86/stage1.c (working copy) @@ -28,6 +28,7 @@ #include <lib.h> #include <mc146818rtc.h> #include <cpu.h> +#include <multiboot.h>
#ifdef CONFIG_PAYLOAD_ELF_LOADER /* ah, well, what a mess! This is a hard code. FIX ME but how? @@ -139,6 +140,14 @@ } #endif /* CONFIG_PAYLOAD_ELF_LOADER */
+int run_address_multiboot(void *f) +{
- int ret;
- __asm__ __volatile__ ("call *%3" : "=a" (ret) : "a" (MB_MAGIC2), "b" (0xf0000), "c" (f));
- return ret;
+}
The function above belongs in multiboot.c
Note the stage1/stage2 separation. It would require either:
- multiboot.c to be linked in twice, once for stage1 and once for stage2
- multiboot.c to be split in two files, one for stage1 and one for stage2
this is why I initially used multiboot.h and made the function inline (which surprisingly results in zero size increase).
Which way do we go?