[coreboot] [patch] mkelfimage intelligent placement of ramdisk

[Peter Stuge]

On Tue, Jun 10, 2008 at 12:47:38PM -0400, Warren Togami wrote:
> The attached patch by Peter Jones <pjones at redhat.com> 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


> This patch allows a wrapped Fedora i586 kernel to boot on both KVM
> and Artec Group's Thincan DBE61C (coreboot).  It probably needs
> more testing in other circumstances.

Thanks again for the patch. See below for some comments.


> 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 (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

..as should this.


> +		if (highest && e820_map[i].addr < highest->addr)
> +			continue;
> +		if (e820_map[i].size < info->real_mode->initrd_size)
> +			continue;

Find the highest free RAM block in the e820 map which is big enough to
hold the initrd..


> +		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?


> +		highest = &e820_map[i];
> +	}
> +
> +	if (highest == 0)
> +		return;
> +
> +	load_addr = highest->addr + highest->size;
> +	load_addr -= info->real_mode->initrd_size;
> +	load_addr &= ~0xfffUL;

Load the initrd to the top of this RAM block on a 4k boundary.


> +
> +	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;
> +

Does this start with 1MB, then try 2MB, 4MB and so on?


>  		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

Thanks again for your contribution!


//Peter