On 16 July 2014 11:20, Mark Rutland <mark.rutland@xxxxxxx> wrote: > Hi Ard, > > On Tue, Jul 15, 2014 at 05:50:30PM +0100, Ard Biesheuvel wrote: >> The EFI loader may load Image at any available offset. This means Image may >> reside very close to or at the base of DRAM, in which case the relocation >> done by efi_entry() results in Image being moved up in memory, which is >> undesirable (memory below the kernel is currently not usable). > > Rather than complicating this path I would prefer that we fixed not > being able to address memory below stext - TEXT_OFFSET. That would > simplify the bootloader's job regardless of UEFI, as any loader can then > simply place the kernel at a 2MB-aligned address + TEXT_OFFSET (the > kernel would have to not span a 512MB boundary to keep the initial page > tables simple, but that's still a weaker requirement than what we > currently need). > > The approach taken in this patch seems sane for the current way the VA > space is laid out, but if we're not currently wasting an absurd amount > of memory I'd rather we got the VA layout fixed such that we can address > memory below the kernel image. > > How much memory are we seeing wasted as a result of the existing > relocation code, and how often? > To be honest, I don't have any data that suggest that this is an actual problem. The potential penalty is the size of Image rounded up to 2 megs. But I agree it would be much more useful if the requirements regarding where to locate Image can be relaxed somewhat. >> /* >> + * Check whether the original allocation done by the EFI loader is more >> + * favorable (i.e., closer to the base of DRAM) than the new one created >> + * by efi_entry(). If so, reuse the original one. >> + */ >> + adr x3, 0f >> + cmp x3, x0 >> + bgt 1f // this image is loaded higher, so just proceed normally >> + >> + /* >> + * Jump into the relocated image so we can move the original Image to >> + * a 2 MB boundary + TEXT_OFFSET inside the original mapping without >> + * overwriting ourselves. >> + */ >> + sub x3, x3, x22 // subtract current base offset >> + add x3, x3, x0 // add base offset of relocated image >> + br x3 // jump to next line, but in relocated image > > At this point I don't believe the necessary icache maintenance + isb > have occurred to make the new image visible to the I-side and to ensure > we don't have any stale prefetched instructions in the pipeline. That > still seems to happen later. Have I missed something? > No you haven't. The cache maintenance needs to occur here for the relocated Image if we intend to execute from it. That's what I get for using Foundation model, I guess :-) Anyway, I don't think this is a big deal, it's just one of the puzzle pieces in the wider discussion about PE/COFF loading, relocation, APM Mustang etc. -- Ard. >> + >> +0: mov x1, x0 // copy from relocated Image >> + sub x0, x22, #1 // copy to base of original Image >> + orr x0, x0, #(SZ_2M - 1) // .. rounded up to 2 MB >> + ldr x3, =(TEXT_OFFSET + 1) // .. plus TEXT_OFFSET >> + add x0, x0, x3 >> + mov x2, x24 // copy 'size of Image' bytes >> + bl memcpy >> + add x21, x0, x23 // add stext_offset to entry point >> +1: >> + /* >> * Flush dcache covering current runtime addresses >> * of kernel text/data. Then flush all of icache. >> */ >> - adrp x1, _text >> - add x1, x1, #:lo12:_text >> - adrp x2, _edata >> - add x2, x2, #:lo12:_edata >> - sub x1, x2, x1 >> - >> + mov x1, x24 // size of Image >> bl __flush_dcache_area >> ic ialluis > > Cheers, > Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
