Comments
Patch
===================================================================
@@ -423,14 +423,13 @@
{
struct segment *ptr;
- unsigned long required_bounce_size = lb_end - lb_start;
+ unsigned long bounce_high = lb_end;
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 (ptr->s_dstaddr + ptr->s_memsz > bounce_high)
+ bounce_high = ptr->s_dstaddr + ptr->s_memsz;
}
- get_bounce_buffer(mem, required_bounce_size);
+ get_bounce_buffer(mem, bounce_high - lb_start);
if (!bounce_buffer) {
printk_err("Could not find a bounce buffer...\n");
return 0;
@@ -502,6 +501,24 @@
/* Zero the extra bytes */
memset(middle, 0, end - middle);
}
+ /* Copy the data that's outside the area that shadows coreboot_ram */
+ printk_debug("dest %lx, end %lx, bouncebuffer %lx\n", dest, end, bounce_buffer);
+ if ((unsigned long)end > bounce_buffer) {
+ if ((unsigned long)dest < bounce_buffer) {
+ unsigned long from = dest;
+ unsigned long to = lb_start-(bounce_buffer-(unsigned long)dest);
+ unsigned long amount = bounce_buffer-(unsigned long)dest;
+ printk_debug("move prefix around: from %lx, to %lx, amount: %lx\n", from, to, amount);
+ memcpy(to, from, amount);
+ }
+ if ((unsigned long)end > bounce_buffer + (lb_end - lb_start)) {
+ unsigned long from = bounce_buffer + bounce_size;
+ unsigned long to = lb_end;
+ unsigned long amount = end - (bounce_buffer + bounce_size);
+ printk_debug("move suffix around: from %lx, to %lx, amount: %lx\n", from, to, amount);
+ memcpy(to, from, amount);
+ }
+ }
}
}
return 1;
Hi, attached patch reworks the payload handling with CBFS (ie. selfboot). We use a bounce buffer (both in CBFS and older) to keep coreboot running while allowing the payload to use the same memory region, and this is a tricky part of our code. Assumptions in the code are: The bounce buffer is at least 2x coreboot_ram size (lb_end - lb_start), with the first half being the shadow region that the payload's data lives in (exactly the bytes that cover the area of coreboot_ram), while the second half exists to host a copy of coreboot while copying. The change leads to the following behaviour: At least 2x coreboot_ram size is looked for. The area before the bounce buffer is assumed to be reasonably free (but this isn't validated) Payload segments that conflict with coreboot_ram are loaded to the bounce buffer, probably using the memory before it, and filling up more than coreboot_ram bytes there Right after copying, the "out of area" bytes are copied to their final location (around coreboot_ram). I want to revisit the prefix/suffix calculation, as I'm not 100% sure that it's doing the right thing (those copy operations should copy _exactly_ the necessary set of bytes, nothing more), review is welcome (look for "prefix" and "suffix" in selfboot.c) The code is runtime tested on qemu-x86 (after cranking up the STACK_SIZE to >16kb, so lzma could run), with the following scenarios: RAMBASE = 0x4000, payload starting at 0x00100000 (no collision between coreboot_ram and payload) RAMBASE = 0x00100000, payload starting at 0x00100000 (RAMBASE below or at payload starting address, this worked before) RAMBASE = 0x00101000, payload starting at 0x00100000 (RAMBASE above payload starting address, this fails without this patch. AMD boards were notorious with that one) Pending the review of the prefix/suffix calculations (by me and hopefully others), this is Signed-off-by: Patrick Georgi <patrick.georgi@coresystems.de> Once this is in, there are no CBFS issues that I know of, and hence I propose to eliminate non-CBFS support in the tree soonish after that patch is in. I have some older patch for this, but it will likely not apply anymore, but I can update it. Opinions? Regards, Patrick