Re: [RFC PATCH] arm64/efi: efistub: reuse EFI mapping for Image if it is lower

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 16 July 2014 16:10, Mark Salter <msalter@xxxxxxxxxx> wrote:
> On Tue, 2014-07-15 at 18:50 +0200, 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).
>>
>> To work around this, we check in the stub whether the relocated offset is higher
>> than the original offset. If this is the case, the original and relocated Images
>> switch roles, and we move the original Image inside its original lower mapping
>> while executing from the relocated Image.
>>
>> To ensure the original mapping has sufficient size, we add 2 megs + TEXT_OFFSET
>> of padding to the PE/COFF .text section, this should be sufficient to cover
>> rounding and adding TEXT_OFFSET.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
>> ---
>>
>> This was tested on a Foundation Model using the patch below, which forces the
>> Image to be loaded at 0x80000000 (base of DRAM).
>>
>> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
>> index 2b7bd5eff776..eeadc073bad3 100644
>> --- a/arch/arm64/kernel/head.S
>> +++ b/arch/arm64/kernel/head.S
>> @@ -138,12 +138,12 @@ pe_header:
>>       .short  0
>>  coff_header:
>>       .short  0xaa64                          // AArch64
>> -     .short  2                               // nr_sections
>> +     .short  1                               // nr_sections
>>       .long   0                               // TimeDateStamp
>>       .long   0                               // PointerToSymbolTable
>>       .long   1                               // NumberOfSymbols
>>       .short  section_table - optional_header // SizeOfOptionalHeader
>> -     .short  0x206                           // Characteristics.
>> +     .short  0x207                           // Characteristics.
>>                                               // IMAGE_FILE_DEBUG_STRIPPED |
>>                                               // IMAGE_FILE_EXECUTABLE_IMAGE |
>>                                               // IMAGE_FILE_LINE_NUMS_STRIPPED
>> @@ -158,7 +158,7 @@ optional_header:
>>       .long   stext_offset                    // BaseOfCode
>>
>>  extra_header_fields:
>> -     .quad   0                               // ImageBase
>> +     .quad   0x80000000                      // ImageBase
>>       .long   0x20                            // SectionAlignment
>>       .long   0x8                             // FileAlignment
>>       .short  0                               // MajorOperatingSystemVersion
>> @@ -193,6 +193,7 @@ extra_header_fields:
>>       // Section table
>>  section_table:
>>
>> +#if 0
>>       /*
>>        * The EFI application loader requires a relocation section
>>        * because EFI applications must be relocatable.  This is a
>> @@ -212,6 +213,7 @@ section_table:
>>       .long   0x42100040              // Characteristics (section flags)
>>
>>
>> +#endif
>>       .ascii  ".text"
>>       .byte   0
>>       .byte   0
>>
>>
>>  arch/arm64/kernel/Makefile    |  1 +
>>  arch/arm64/kernel/efi-entry.S | 48 +++++++++++++++++++++++++++++++++----------
>>  arch/arm64/kernel/head.S      |  7 ++++---
>>  3 files changed, 42 insertions(+), 14 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
>> index cdaedad3afe5..d55da3cd8a97 100644
>> --- a/arch/arm64/kernel/Makefile
>> +++ b/arch/arm64/kernel/Makefile
>> @@ -6,6 +6,7 @@ CPPFLAGS_vmlinux.lds  := -DTEXT_OFFSET=$(TEXT_OFFSET)
>>  AFLAGS_head.o                := -DTEXT_OFFSET=$(TEXT_OFFSET)
>>  CFLAGS_efi-stub.o    := -DTEXT_OFFSET=$(TEXT_OFFSET) \
>>                          -I$(src)/../../../scripts/dtc/libfdt
>> +AFLAGS_efi-entry.o   := -DTEXT_OFFSET=$(TEXT_OFFSET)
>>
>>  CFLAGS_REMOVE_ftrace.o = -pg
>>  CFLAGS_REMOVE_insn.o = -pg
>> diff --git a/arch/arm64/kernel/efi-entry.S b/arch/arm64/kernel/efi-entry.S
>> index a0016d3a17da..d154f3c73937 100644
>> --- a/arch/arm64/kernel/efi-entry.S
>> +++ b/arch/arm64/kernel/efi-entry.S
>> @@ -11,6 +11,7 @@
>>   */
>>  #include <linux/linkage.h>
>>  #include <linux/init.h>
>> +#include <linux/sizes.h>
>>
>>  #include <asm/assembler.h>
>>
>> @@ -45,10 +46,13 @@ ENTRY(efi_stub_entry)
>>        *                         efi_system_table_t *sys_table,
>>        *                         unsigned long *image_addr) ;
>>        */
>> -     adrp    x8, _text
>> -     add     x8, x8, #:lo12:_text
>> +     adrp    x22, _text
>> +     add     x22, x22, #:lo12:_text          // x22: base of Image
>
> Why change x8 to x22? Seems unnecessary. Not only that, it is
> callee-saved, but you don't save it. This could cause a problem
> if boot fails and we try to return to firmware.
>
>> +     adrp    x24, _edata
>> +     add     x24, x24, #:lo12:_edata
>> +     sub     x24, x24, x22                   // x24: size of Image
>
> Should use a caller-saved register instead of x24.
>

Yes, you are right, I moved it to x22 so I can reuse the value after
the call to efi_entry().
I will correct it when/if I do a v2, i.e., if there is any demand for this.

Thanks,
-- 
Ard.


>>       add     x2, sp, 16
>> -     str     x8, [x2]
>> +     str     x22, [x2]
>>       bl      efi_entry
>>       cmn     x0, #1
>>       b.eq    efi_load_fail
>> @@ -61,19 +65,41 @@ ENTRY(efi_stub_entry)
>>        */
>>       mov     x20, x0         // DTB address
>>       ldr     x0, [sp, #16]   // relocated _text address
>> -     ldr     x21, =stext_offset
>> -     add     x21, x0, x21
>> +     ldr     x23, =stext_offset
>> +     add     x21, x0, x23
>>
>>       /*
>> +      * 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
>> +
>> +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
>>
>> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
>> index 5cd1f3491df5..2b7bd5eff776 100644
>> --- a/arch/arm64/kernel/head.S
>> +++ b/arch/arm64/kernel/head.S
>> @@ -129,6 +129,7 @@ efi_head:
>>  #endif
>>
>>  #ifdef CONFIG_EFI
>> +#define PE_TEXT_PAD          SZ_2M + TEXT_OFFSET
>>       .globl  stext_offset
>>       .set    stext_offset, stext - efi_head
>>       .align 3
>> @@ -150,7 +151,7 @@ optional_header:
>>       .short  0x20b                           // PE32+ format
>>       .byte   0x02                            // MajorLinkerVersion
>>       .byte   0x14                            // MinorLinkerVersion
>> -     .long   _edata - stext                  // SizeOfCode
>> +     .long   _edata - stext + PE_TEXT_PAD    // SizeOfCode
>>       .long   0                               // SizeOfInitializedData
>>       .long   0                               // SizeOfUninitializedData
>>       .long   efi_stub_entry - efi_head       // AddressOfEntryPoint
>> @@ -168,7 +169,7 @@ extra_header_fields:
>>       .short  0                               // MinorSubsystemVersion
>>       .long   0                               // Win32VersionValue
>>
>> -     .long   _edata - efi_head               // SizeOfImage
>> +     .long   _edata - efi_head + PE_TEXT_PAD // SizeOfImage
>>
>>       // Everything before the kernel image is considered part of the header
>>       .long   stext_offset                    // SizeOfHeaders
>> @@ -215,7 +216,7 @@ section_table:
>>       .byte   0
>>       .byte   0
>>       .byte   0                       // end of 0 padding of section name
>> -     .long   _edata - stext          // VirtualSize
>> +     .long   _edata - stext + PE_TEXT_PAD    // VirtualSize
>>       .long   stext_offset            // VirtualAddress
>>       .long   _edata - stext          // SizeOfRawData
>>       .long   stext_offset            // PointerToRawData
>
>
--
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




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux