Patchwork CBFS bounce buffer fix

login
register
about
Submitter Myles Watson
Date 2009-09-23 20:23:05
Message ID <2831fecf0909231323o7f7d5045hf7375730908936e4@mail.gmail.com>
Download mbox | patch
Permalink /patch/285/
State Accepted
Commit r4663
Headers show

Comments

Myles Watson - 2009-09-23 20:23:05
src/boot/selfboot.c: In function 'get_bounce_buffer':
src/boot/ssrc/boot/selfboot.c: In function 'get_bounce_buffer':
src/boot/selfboot.c:113: warning: declaration of 'bounce_size' shadows a
global elfboot.c:113: warning: declaration of 'bounce_size' shadows a global



The global never got initialized, so I couldn't boot a kernel.  Now I can,
but I'm not sure this is the most elegant fix, or even that it is 100%
correct.

While I was there I changed another bounce_size to size to avoid confusion
with the global, and made selfboot static to get rid of this warning:

src/boot/selfboot.c:508: warning: no previous prototype for 'selfboot'

Signed-off-by: Myles Watson <mylesgw@gmail.com>

Thanks,
Myles
ron minnich - 2009-09-23 20:28:27
Acked-by: Ronald G. Minnich <rminnich@gmail.com>
Patrick Georgi - 2009-09-23 20:30:34
Am Mittwoch, den 23.09.2009, 14:23 -0600 schrieb Myles Watson:
> The global never got initialized, so I couldn't boot a kernel.  Now I
> can, but I'm not sure this is the most elegant fix, or even that it is
> 100% correct.
Looks good to me. I had similar changes here already, but messed around
with That Other Selfboot Bug[tm] and thus have nothing finished yet.

> src/boot/selfboot.c:508: warning: no previous prototype for 'selfboot'
> 
> Signed-off-by: Myles Watson <mylesgw@gmail.com>
Acked-by: Patrick Georgi <patrick.georgi@coresystems.de>


Regards,
Patrick
Myles Watson - 2009-09-23 20:33:58
On Wed, Sep 23, 2009 at 2:28 PM, ron minnich <rminnich@gmail.com> wrote:

> Acked-by: Ronald G. Minnich <rminnich@gmail.com>
>
Rev 4663.

Thanks,
Myles
Myles Watson - 2009-09-23 20:35:02
On Wed, Sep 23, 2009 at 2:30 PM, Patrick Georgi <patrick@georgi-clan.de>wrote:

> Am Mittwoch, den 23.09.2009, 14:23 -0600 schrieb Myles Watson:
> > The global never got initialized, so I couldn't boot a kernel.  Now I
> > can, but I'm not sure this is the most elegant fix, or even that it is
> > 100% correct.
> Looks good to me.

Thanks for the review.


> I had similar changes here already, but messed around
> with That Other Selfboot Bug[tm] and thus have nothing finished yet
>
I hope you get the fix committed before I find it the hard way :)

Thanks,
Myles
Patrick Georgi - 2009-09-23 20:42:43
Am Mittwoch, den 23.09.2009, 14:35 -0600 schrieb Myles Watson:
>         I had similar changes here already, but messed around
>         with That Other Selfboot Bug[tm] and thus have nothing
>         finished yet
> I hope you get the fix committed before I find it the hard way :)
The issue is that the loader doesn't properly cope with the case that
the payload overlaps with coreboot and starts at a lower address.

That happens on some AMD boards (with RAMBASE at 2MB, for whatever
reason) and FILO (which has a huge .bss section that moves well into
coreboot's memory).

Any other configuration should not run into this issue, and for AMD, it
should be enough to move RAMBASE to somewhere else (both 1MB and 4MB
worked for me) as a workaround.


Regards,
Patrick
Myles Watson - 2009-09-23 21:00:11
On Wed, Sep 23, 2009 at 2:42 PM, Patrick Georgi <patrick@georgi-clan.de>wrote:

> Am Mittwoch, den 23.09.2009, 14:35 -0600 schrieb Myles Watson:
> >         I had similar changes here already, but messed around
> >         with That Other Selfboot Bug[tm] and thus have nothing
> >         finished yet
> > I hope you get the fix committed before I find it the hard way :)
>


>  Any other configuration should not run into this issue, and for AMD, it
> should be enough to move RAMBASE to somewhere else (both 1MB and 4MB
> worked for me) as a workaround.
>
Great.  I'd forgotten that one, probably because it won't affect me.

Thanks,
Myles

Patch

Index: svn/src/boot/selfboot.c
===================================================================
--- svn.orig/src/boot/selfboot.c
+++ svn/src/boot/selfboot.c
@@ -71,9 +71,10 @@  struct ip_checksum_vcb {
 	unsigned short ip_checksum;
 };
 
+static int selfboot(struct lb_memory *mem, struct cbfs_payload *payload);
+
 void * cbfs_load_payload(struct lb_memory *lb_mem, const char *name)
 {
-	int selfboot(struct lb_memory *mem, struct cbfs_payload *payload);
 	struct cbfs_payload *payload;
 
 	payload = (struct cbfs_payload *)cbfs_find_file(name, CBFS_TYPE_PAYLOAD);
@@ -110,7 +111,7 @@  void * cbfs_load_payload(struct lb_memor
 
 static unsigned long bounce_size, bounce_buffer;
 
-static void get_bounce_buffer(struct lb_memory *mem, unsigned long bounce_size)
+static void get_bounce_buffer(struct lb_memory *mem, unsigned long req_size)
 {
 	unsigned long lb_size;
 	unsigned long mem_entries;
@@ -118,7 +119,7 @@  static void get_bounce_buffer(struct lb_
 	int i;
 	lb_size = (unsigned long)(&_eram_seg - &_ram_seg);
 	/* Double coreboot size so I have somewhere to place a copy to return to */
-	lb_size = bounce_size + lb_size;
+	lb_size = req_size + lb_size;
 	mem_entries = (mem->size - sizeof(*mem))/sizeof(mem->map[0]);
 	buffer = 0;
 	for(i = 0; i < mem_entries; i++) {
@@ -142,6 +143,7 @@  static void get_bounce_buffer(struct lb_
 		buffer = tbuffer;
 	}
 	bounce_buffer = buffer;
+	bounce_size = req_size;
 }
 
 static int valid_area(struct lb_memory *mem, unsigned long buffer,
@@ -424,7 +426,8 @@  static int load_self_segments(
 	for(ptr = head->next; ptr != head; ptr = ptr->next) {
 		if (!overlaps_coreboot(ptr)) continue;
 		unsigned long bounce = ptr->s_dstaddr + ptr->s_memsz - lb_start;
-		if (bounce > required_bounce_size) required_bounce_size = bounce;
+		if (bounce > required_bounce_size)
+			required_bounce_size = bounce;
 	}
 	get_bounce_buffer(mem, required_bounce_size);
 	if (!bounce_buffer) {
@@ -458,14 +461,12 @@  static int load_self_segments(
 			size_t len;
 			len = ptr->s_filesz;
 			switch(ptr->compression) {
-#if CONFIG_COMPRESSED_PAYLOAD_LZMA==1
 				case CBFS_COMPRESS_LZMA: {
 					printk_debug("using LZMA\n");
 					unsigned long ulzma(unsigned char *src, unsigned char *dst);		
 					len = ulzma(src, dest);
 					break;
 				}
-#endif
 #if CONFIG_COMPRESSED_PAYLOAD_NRV2B==1
 				case CBFS_COMPRESS_NRV2B: {
 					printk_debug("using NRV2B\n");
@@ -505,7 +506,7 @@  static int load_self_segments(
 	return 1;
 }
 
-int selfboot(struct lb_memory *mem, struct cbfs_payload *payload)
+static int selfboot(struct lb_memory *mem, struct cbfs_payload *payload)
 {
 	u32 entry=0;
 	struct segment head;
Index: svn/src/arch/i386/boot/boot.c
===================================================================
--- svn.orig/src/arch/i386/boot/boot.c
+++ svn/src/arch/i386/boot/boot.c
@@ -68,7 +68,7 @@  int elf_check_arch(Elf_ehdr *ehdr)
 	
 }
 
-void jmp_to_elf_entry(void *entry, unsigned long buffer, unsigned long bounce_size)
+void jmp_to_elf_entry(void *entry, unsigned long buffer, unsigned long size)
 {
 	extern unsigned char _ram_seg, _eram_seg;
 	unsigned long lb_start, lb_size;
@@ -79,7 +79,7 @@  void jmp_to_elf_entry(void *entry, unsig
 
 	lb_start = (unsigned long)&_ram_seg;
 	lb_size = (unsigned long)(&_eram_seg - &_ram_seg);
-	adjust = buffer + bounce_size - lb_start;
+	adjust = buffer +  size - lb_start;
 
 	adjusted_boot_notes = (unsigned long)&elf_boot_notes;
 	adjusted_boot_notes += adjust;