Peter Stuge wrote:
On Tue, Jun 10, 2008 at 12:47:38PM -0400, Warren Togami wrote:
The attached patch by Peter Jones <[EMAIL PROTECTED]> implements
intelligent placement of the ramdisk in memory during boot of an
ELF image created by mkelfimage.
I appreciate this patch!
But can you (or Peter) please describe the algorithm in english?
I don't like a commit message that claims code to be intelligent.. :p
I personally don't understand the algorithm beyond your interpretation.
Peter explained only a few bits below. Lots of this algorithm is simply
how other bootloaders have worked for years. This was just a simple way
to implement something known to work.
In any case it is certainly better than the previous hard coded value.
I hope others test this patch so we can be certain it doesn't break
anyone where it previously worked. It shouldn't.
diff -up mkelfImage-2.7/linux-i386/convert_params.c.ramdisk_base
mkelfImage-2.7/linux-i386/convert_params.c
--- mkelfImage-2.7/linux-i386/convert_params.c.ramdisk_base 2006-03-27
18:44:59.000000000 -0500
+++ mkelfImage-2.7/linux-i386/convert_params.c 2008-05-21 12:55:44.000000000
-0400
@@ -1301,6 +1301,44 @@ static void query_firmware_values(struct
}
+static void relocate_ramdisk(struct param_info *info)
+{
+ struct e820entry *e820_map;
+ struct e820entry *highest = 0;
+ unsigned long load_addr;
+ int i;
+
+ e820_map = info->real_mode->e820_map;
+#if 0
+ printf("initrd_start = 0x%lx\n", info->real_mode->initrd_start);
+ printf("real_mode->e820_map_nr: %d\n", info->real_mode->e820_map_nr);
+#endif
I think these debugging messages should either be hooked to some
verbose option or simply removed.
For merging with upstream I would just remove these blocks unless you
already have an established way of building with runtime debug messages.
Just go ahead and change it as you see fit for merging.
+ if (e820_map[i].addr + info->real_mode->initrd_size >=
0x38000000)
+ continue;
..what is this 3.5MB limit about? Also might this condition be
sensitive to an overflow error?
Peter isn't sure why 3.5MB specifically. He said it is known to work
for many years in other boot loaders. There may have been a historic
reason for this particular value but he doesn't know.
diff -up mkelfImage-2.7/linux-i386/mkelf-linux-i386.c.ramdisk_base
mkelfImage-2.7/linux-i386/mkelf-linux-i386.c
--- mkelfImage-2.7/linux-i386/mkelf-linux-i386.c.ramdisk_base 2006-03-17
09:08:22.000000000 -0500
+++ mkelfImage-2.7/linux-i386/mkelf-linux-i386.c 2008-05-21
10:47:42.000000000 -0400
@@ -352,6 +352,9 @@ int linux_i386_mkelf(int argc, char **ar
*/
params->initrd_start = params->initrd_size = 0;
if (ramdisk_size) {
+ while (ramdisk_base <= kernel_size)
+ ramdisk_base <<= 1;
+
Does this start with 1MB, then try 2MB, 4MB and so on?
According to Peter there this is only a simple way that avoids alignment
earlier. While not expressly necessary (you could add), this requires
less code, and more importantly, it is known to work.
phdr[index].p_paddr = ramdisk_base;
phdr[index].p_vaddr = ramdisk_base;
phdr[index].p_filesz = ramdisk_size;
Please keep in mind to also include Signed-off-by: lines from all who
have touched the patch. Please see
http://www.coreboot.org/Development_Guidelines#Sign-off_Procedure
OK, he agreed. Attached is the same patch with his signing off (but not
me since I didn't change it.)
Warren Togami
[EMAIL PROTECTED]
diff -up mkelfImage-2.7/linux-i386/convert_params.c.ramdisk_base mkelfImage-2.7/linux-i386/convert_params.c
--- mkelfImage-2.7/linux-i386/convert_params.c.ramdisk_base 2006-03-27 18:44:59.000000000 -0500
+++ mkelfImage-2.7/linux-i386/convert_params.c 2008-05-21 12:55:44.000000000 -0400
@@ -1301,6 +1301,44 @@ static void query_firmware_values(struct
}
+static void relocate_ramdisk(struct param_info *info)
+{
+ struct e820entry *e820_map;
+ struct e820entry *highest = 0;
+ unsigned long load_addr;
+ int i;
+
+ e820_map = info->real_mode->e820_map;
+#if 0
+ printf("initrd_start = 0x%lx\n", info->real_mode->initrd_start);
+ printf("real_mode->e820_map_nr: %d\n", info->real_mode->e820_map_nr);
+#endif
+ for (i = 0; i < info->real_mode->e820_map_nr; i++) {
+ if (e820_map[i].type != E820_RAM)
+ continue;
+#if 0
+ printf("addr: 0x%lx len: %x\n", e820_map[i].addr, e820_map[i].size);
+#endif
+ if (highest && e820_map[i].addr < highest->addr)
+ continue;
+ if (e820_map[i].size < info->real_mode->initrd_size)
+ continue;
+ if (e820_map[i].addr + info->real_mode->initrd_size >= 0x38000000)
+ continue;
+ highest = &e820_map[i];
+ }
+
+ if (highest == 0)
+ return;
+
+ load_addr = highest->addr + highest->size;
+ load_addr -= info->real_mode->initrd_size;
+ load_addr &= ~0xfffUL;
+
+ memcpy((void *)load_addr, (void *)info->real_mode->initrd_start, info->real_mode->initrd_size);
+ printf("relocating ramdisk to 0x%lx\n", load_addr);
+ info->real_mode->initrd_start = load_addr;
+}
/*
* Debug
* =============================================================================
@@ -1533,6 +1571,10 @@ void *convert_params(unsigned type, void
query_firmware_class(&info);
query_firmware_values(&info);
query_bootloader_values(&info);
+ if (info.real_mode->initrd_size) {
+ /* Make sure the initrd is in a relatively safe place. */
+ relocate_ramdisk(&info);
+ }
/* Do the hardware setup that linux might forget... */
hardware_setup(&info);
diff -up mkelfImage-2.7/linux-i386/mkelf-linux-i386.c.ramdisk_base mkelfImage-2.7/linux-i386/mkelf-linux-i386.c
--- mkelfImage-2.7/linux-i386/mkelf-linux-i386.c.ramdisk_base 2006-03-17 09:08:22.000000000 -0500
+++ mkelfImage-2.7/linux-i386/mkelf-linux-i386.c 2008-05-21 10:47:42.000000000 -0400
@@ -352,6 +352,9 @@ int linux_i386_mkelf(int argc, char **ar
*/
params->initrd_start = params->initrd_size = 0;
if (ramdisk_size) {
+ while (ramdisk_base <= kernel_size)
+ ramdisk_base <<= 1;
+
phdr[index].p_paddr = ramdisk_base;
phdr[index].p_vaddr = ramdisk_base;
phdr[index].p_filesz = ramdisk_size;
Signed-off-by: Peter Jones <[EMAIL PROTECTED]>
--
coreboot mailing list
[email protected]
http://www.coreboot.org/mailman/listinfo/coreboot
