Re: [PATCH] arm64/efi: fix variable 'si' set but not used
On Tue, Jul 30, 2019 at 05:23:48PM -0400, Qian Cai wrote: > GCC throws out this warning on arm64. > > drivers/firmware/efi/libstub/arm-stub.c: In function 'efi_entry': > drivers/firmware/efi/libstub/arm-stub.c:132:22: warning: variable 'si' > set but not used [-Wunused-but-set-variable] > > Fix it by making free_screen_info() a static inline function. > > Signed-off-by: Qian Cai Queued for 5.3-rc3. Thanks. -- Catalin
Re: [PATCH -next] efi/arm64: return zero from ptdump_init()
On Mon, Feb 04, 2019 at 04:01:01PM -0500, Qian Cai wrote:
> The commit e2a2e56e4082 ("arm64: dump: no need to check return value of
> debugfs_create functions") converted ptdump_debugfs_register() from
> void, but forgot to fix the efi version of ptdump_init().
>
> drivers/firmware/efi/arm-runtime.c: In function 'ptdump_init':
> drivers/firmware/efi/arm-runtime.c:52:9: error: void value not ignored as it
> ought to be
>52 | return ptdump_debugfs_register(&efi_ptdump_info, "efi_page_tables");
> | ^~~~
> drivers/firmware/efi/arm-runtime.c:53:1: warning: control reaches end of
> non-void function [-Wreturn-type]
>
> Fixes: e2a2e56e4082 ("arm64: dump: no need to check return value of
> debugfs_create functions")
> Signed-off-by: Qian Cai
A fix is already in -next (commit 67f52a9540e0).
--
Catalin
Re: [RESEND PATCH] efi: let kmemleak ignore false positives
On Thu, Dec 06, 2018 at 11:16:33AM -0500, Qian Cai wrote: > unreferenced object 0x8096c1acf580 (size 128): > comm "swapper/63", pid 0, jiffies 4294937418 (age 1201.230s) > hex dump (first 32 bytes): > 80 87 b5 c1 96 00 00 00 00 00 cc c2 16 00 00 00 > 00 00 01 00 00 00 00 00 6b 6b 6b 6b 6b 6b 6b 6b > backtrace: > [<1d2549ba>] kmem_cache_alloc_trace+0x430/0x500 > [<93a6dfab>] efi_mem_reserve_persistent+0x50/0xf8 > [<0a730828>] its_cpu_init_lpis+0x394/0x4b8 > [<edf04e07>] its_cpu_init+0x104/0x150 > [<4d0342c5>] gic_starting_cpu+0x34/0x40 > [<5d9da772>] cpuhp_invoke_callback+0x228/0x1d68 > [<61eace9b>] notify_cpu_starting+0xc0/0x118 > [<48bc2dc5>] secondary_start_kernel+0x23c/0x3b0 > [<15137d6a>] 0x > > efi_mem_reserve_persistent+0x50/0xf8: > kmalloc at include/linux/slab.h:546 > (inlined by) efi_mem_reserve_persistent at drivers/firmware/efi/efi.c:979 > > This line, > > rsv = kmalloc(sizeof(*rsv), GFP_ATOMIC); > > Kmemleak has a known limitation that can only track pointers in the kernel > virtual space. Hence, it will report false positives due to "rsv" will only > reference to other physical addresses, > > rsv->next = efi_memreserve_root->next; > efi_memreserve_root->next = __pa(rsv); > > Signed-off-by: Qian Cai Acked-by: Catalin Marinas
Re: [PATCH v2 1/6] arm64: memblock: don't permit memblock resizing until linear mapping is up
On Wed, Nov 07, 2018 at 03:16:06PM +0100, Ard Biesheuvel wrote: > Bhupesh reports that having numerous memblock reservations at early > boot may result in the following crash: > > Unable to handle kernel paging request at virtual address 80003ffe > ... > Call trace: >__memcpy+0x110/0x180 >memblock_add_range+0x134/0x2e8 >memblock_reserve+0x70/0xb8 >memblock_alloc_base_nid+0x6c/0x88 >__memblock_alloc_base+0x3c/0x4c >memblock_alloc_base+0x28/0x4c >memblock_alloc+0x2c/0x38 >early_pgtable_alloc+0x20/0xb0 >paging_init+0x28/0x7f8 > > This is caused by the fact that we permit memblock resizing before the > linear mapping is up, and so the memblock_reserved() array is moved > into memory that is not mapped yet. > > So let's ensure that this crash can no longer occur, by deferring to > call to memblock_allow_resize() to after the linear mapping has been > created. > > Reported-by: Bhupesh Sharma > Acked-by: Will Deacon > Tested-by: Marc Zyngier > Signed-off-by: Ard Biesheuvel I missed this patch (wasn't cc'ed) but Will pinged me on IRC, so queued for 4.20. Thanks. -- Catalin
Re: [PATCH] efi/libstub: arm/arm64: don't use TASK_SIZE when randomising the RT space
On Mon, Apr 10, 2017 at 11:30:38AM +0100, Ard Biesheuvel wrote:
> As reported by James, Catalin and Mark, commit e69176d68d26
> ("ef/libstub/arm/arm64: Randomize the base of the UEFI rt services
> region") results in a crash in the firmware regardless of whether KASLR
> is in effect or not, and whether the firmware implements EFI_RNG_PROTOCOL
> or not.
>
> Mark has identified the root cause to be the inappropriate use of
> TASK_SIZE in the stub, which arm64 defines as
>
> #define TASK_SIZE (test_thread_flag(TIF_32BIT) ? \
> TASK_SIZE_32 : TASK_SIZE_64)
>
> and testing thread flags at this point results in the dereference of
> pointers in uninitialized structures.
>
> So instead, introduce a preprocessor symbol EFI_RT_VIRTUAL_LIMIT and
> define it to TASK_SIZE_64 on arm64 and TASK_SIZE on ARM, both of which
> are compile time constants. Also, change the 'headroom' variable to
> static const to force an error if this changes in the future.
>
> Cc: James Morse
> Cc: Mark Rutland
> Cc: Catalin Marinas
> Cc: Matt Fleming
> Cc: Ingo Molnar
> Signed-off-by: Ard Biesheuvel
It works for me as well:
Tested-by: Catalin Marinas
Thanks Ard and Mark.
--
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 12/12] ef/libstub: arm/arm64: Randomize the base of the UEFI rt services region
Hi Ard, On 4 April 2017 at 17:09, Ard Biesheuvel wrote: > Update the allocation logic for the virtual mapping of the UEFI runtime > services to start from a randomized base address if KASLR is in effect, > and if the UEFI firmware exposes an implementation of EFI_RNG_PROTOCOL. > > This makes it more difficult to predict the location of exploitable > data structures in the runtime UEFI firmware, which increases robustness > against attacks. Note that these regions are only mapped during the > time a runtime service call is in progress, and only on a single CPU > at a time, bit give the lack of a downside, let's enable it nonetheless. > > Cc: Ingo Molnar > Cc: Borislav Petkov > Cc: Matt Fleming > Signed-off-by: Ard Biesheuvel > --- > drivers/firmware/efi/libstub/arm-stub.c | 49 > - > 1 file changed, 36 insertions(+), 13 deletions(-) This patch causes a boot regression on Juno and Seattle (reported by James Morse) with defconfig. On Juno I get a Synchronous Exception (translation fault, second level). If I build the kernel with 64K pages, it boots ok. See below, though likely wrapped by gmail: EFI stub: Booting Linux Kernel... EFI stub: Using DTB from configuration table Synchronous Exception at 0xF82A045C X0 0x X1 0xFDF01614 X2 0x7BF580F1AD8AE581 X3 0x X4 0x X5 0xFDFBFC60 X6 0x42F27CEB1CE1E5BC X7 0x7BF580F1AD8AE581 X8 0x X9 0x0007 X10 0xFB5C X11 0xFB5C5FFF X12 0x X13 0x000E X14 0x8001 X15 0x X16 0xFEFFF5E0 X17 0x X18 0x X19 0xFDFB0018 X20 0xF839 X21 0xFB5BDA18 X22 0xFEFFF5D0 X23 0x0009 X24 0xFEFFF598 X25 0x X26 0x X27 0x X28 0x FP 0xFEFFF510 LR 0xF82A0454 V0 0xA4775AF0716632D7 DB7509796C96DACB V1 0xE67211106C932C4D 4F7141B24C78074F V2 0x550C7DC3243185BE 12835B01D807AA98 V3 0xC19BF1749BDC06A7 80DEB1FE72BE5D74 V4 0x240CA1CC0FC19DC6 EFBE4786E49B69C1 V5 0x76F988DA5CB0A9DC 4A7484AA2DE92C6F V6 0xBF597FC7B00327C8 A831C66D983E5152 V7 0x1429296706CA6351 D5A79147C6E00BF3 V8 0x 2E1B213827B70A85 V9 0x 766A0ABB650A7354 V10 0x A81A664BA2BFE8A1 V11 0x D6990624D192E819 V12 0x 1E376C0819A4C116 V13 0x 4ED8AA4A391C0CB3 V14 0x 78A5636F748F82EE V15 0x A4506CEB90BEFFFA V16 0x3C83709547545153 BC990E9F897D4FA8 V17 0xBEBEF297C5EC0578 E1D99AE8C2B92607 V18 0x13242833C5F05BAB 101C24C521387481 V19 0x0A50ABCAD0BDDA12 996865483E880969 V20 0x6C4ABAA53A9BE1CB 4416D9F479B08221 V21 0x60952147C3A68574 55B8AE51435C1A1A V22 0x9FEB2A3B4AB8D3BF 88C1883495C7F76F V23 0xD0C224BC8FB77E09 3DB8D233CF470963 V24 0x0E72963E02D21C93 C95B757DA62B3A12 V25 0x2A77DBA1E4EE5D5C E08479A1B557DFA8 V26 0xB593F86668CA8129 927A0019CDEC1A76 V27 0xFF03D7BC13D5FFF6 BBA1EFEF4B817FFA V28 0xE3FEE1F77F5FF733 5FBC3CE8E54BBB53 V29 0x4E8582114F72CE7E EED3F6BF4B0F5BDC V30 0xFFFA7FFBF72FF9F7 F8F06FDB9FF5EDEA V31 0xDE3785D1AEB3DAB7 12E08FF9AD0F87FF SP 0xFEFFF500 ELR 0xF82A045C SPSR 0x6209 FPSR 0x ESR 0x9606 FAR 0x ESR : EC 0x25 IL 0x1 ISS 0x0006 Data abort: Translation fault, second level Stack dump: 0FEFFF400: 996865483E880969 0A50ABCAD0BDDA12 4416D9F479B08221 6C4ABAA53A9BE1CB 0FEFFF420: 55B8AE51435C1A1A 60952147C3A68574 88C1883495C7F76F 9FEB2A3B4AB8D3BF 0FEFFF440: 3DB8D233CF470963 D0C224BC8FB77E09 C95B757DA62B3A12 0E72963E02D21C93 0FEFFF460: E08479A1B557DFA8 2A77DBA1E4EE5D5C 927A0019CDEC1A76 B593F86668CA8129 0FEFFF480: BBA1EFEF4B817FFA FF03D7BC13D5FFF6 5FBC3CE8E54BBB53 E3FEE1F77F5FF733 0FEFFF4A0: EED3F6BF4B0F5BDC 4E8582114F72CE7E F8F06FDB9FF5EDEA FFFA7FFBF72FF9F7 0FEFFF4C0: 12E08FF9AD0F87FF DE3785D1AEB3DAB7 F82A045C 6209 0FEFFF4E0: 9606 7BF580F1AD8AE581 > 0FEFFF500: FEFFF520 FDFE1658 FEFFF5C0 > F829D4CC 0FEFFF520: FB5C6040 FCA74698 0FEFFF540: 00B0 00F2F865A8B0 0FEFFF560: FB5C6040 F86C 0FEFFF580: 503F 8008 00F2 0FEFFF5A0: 11D295625B1B31A1 3B7269C9A0003F8E 4A3823DC9042A9DE 6A5180D0DE7AFB96 0FEFFF5C0: FEFFF5E0 FDFD3AA8 8008 FB5D6C18 0FEFFF5E0: FEFFF660 F859C830 F865A8B0 Thanks, Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info
Re: [PATCH V14 00/10] Add UEFI 2.6 and ACPI 6.1 updates for RAS on ARM64
Hi Rafael, On Tue, Mar 28, 2017 at 01:30:30PM -0600, Tyler Baicar wrote: > Tyler Baicar (9): > acpi: apei: read ack upon ghes record consumption > ras: acpi/apei: cper: generic error data entry v3 per ACPI 6.1 > efi: parse ARM processor error > arm64: exception: handle Synchronous External Abort > acpi: apei: handle SEA notification type for ARMv8 > efi: print unrecognized CPER section > ras: acpi / apei: generate trace event for unrecognized CPER section > trace, ras: add ARM processor error trace event > arm/arm64: KVM: add guest SEA support It looks like the KVM parts are getting acked and the arm64 and efi bits have been acked as well. Are you ok to take this series through the ACPI tree? Thanks. -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V13 10/10] arm/arm64: KVM: add guest SEA support
On Tue, Mar 21, 2017 at 04:47:05PM -0600, Tyler Baicar wrote: > Currently external aborts are unsupported by the guest abort > handling. Add handling for SEAs so that the host kernel reports > SEAs which occur in the guest kernel. > > Signed-off-by: Tyler Baicar > --- > arch/arm/include/asm/kvm_arm.h | 10 + > arch/arm/include/asm/system_misc.h | 5 + > arch/arm/kvm/mmu.c | 41 > ++-- > arch/arm64/include/asm/kvm_arm.h | 10 + > arch/arm64/include/asm/system_misc.h | 2 ++ > arch/arm64/mm/fault.c| 19 +++-- > drivers/acpi/apei/ghes.c | 13 ++-- > include/acpi/ghes.h | 2 +- > 8 files changed, 86 insertions(+), 16 deletions(-) For arm64: Acked-by: Catalin Marinas Marc, Christoffer, could you please check the kvm bits in here? Thanks. -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V13 05/10] acpi: apei: handle SEA notification type for ARMv8
On Tue, Mar 21, 2017 at 04:47:00PM -0600, Tyler Baicar wrote: > ARM APEI extension proposal added SEA (Synchronous External Abort) > notification type for ARMv8. > Add a new GHES error source handling function for SEA. If an error > source's notification type is SEA, then this function can be registered > into the SEA exception handler. That way GHES will parse and report > SEA exceptions when they occur. > An SEA can interrupt code that had interrupts masked and is treated as > an NMI. To aid this the page of address space for mapping APEI buffers > while in_nmi() is always reserved, and ghes_ioremap_pfn_nmi() is > changed to use the helper methods to find the prot_t to map with in > the same way as ghes_ioremap_pfn_irq(). > > Signed-off-by: Tyler Baicar > CC: Jonathan (Zhixiong) Zhang > Reviewed-by: James Morse > --- > arch/arm64/Kconfig| 2 ++ > arch/arm64/mm/fault.c | 13 + > drivers/acpi/apei/Kconfig | 15 ++ > drivers/acpi/apei/ghes.c | 70 > +++ > include/acpi/ghes.h | 7 + > 5 files changed, 101 insertions(+), 6 deletions(-) For the arch/arm64 part in here: Acked-by: Catalin Marinas -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V13 04/10] arm64: exception: handle Synchronous External Abort
On Tue, Mar 21, 2017 at 04:46:59PM -0600, Tyler Baicar wrote: > SEA exceptions are often caused by an uncorrected hardware > error, and are handled when data abort and instruction abort > exception classes have specific values for their Fault Status > Code. > When SEA occurs, before killing the process, report the error > in the kernel logs. > Update fault_info[] with specific SEA faults so that the > new SEA handler is used. > > Signed-off-by: Tyler Baicar > CC: Jonathan (Zhixiong) Zhang > Reviewed-by: James Morse > --- > arch/arm64/include/asm/esr.h | 1 + > arch/arm64/mm/fault.c| 43 +++++++++-- Acked-by: Catalin Marinas -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/6] arm64 UEFI early FDT handling
On Mon, Nov 16, 2015 at 10:43:52AM +, Will Deacon wrote: > On Tue, Sep 29, 2015 at 08:38:57AM +0100, Ard Biesheuvel wrote: > > On 22 September 2015 at 02:21, Ard Biesheuvel > > wrote: > > > This is a followup to the "arm64: update/clarify/relax Image and FDT > > > placement > > > rules" series I sent a while ago: > > > (http://article.gmane.org/gmane.linux.ports.arm.kernel/407148) > > > > > > This has now been split in two series: this first series deals with the > > > early FDT handling, primarily in the context of UEFI, but not exclusively. > > > > > > A number of minor issues exist in the early UEFI/FDT handling path, such > > > as: > > > - when booting via UEFI, memreserve entries are removed from the device > > > tree but > > > the /reserved-memory node is not > > > > After reading > > Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt > > again, I think simply ignoring the reserved-memory node is not the way > > to go. The reason is that it may contain dynamic allocations that are > > referenced by other nodes in the DT, and there is no good technical > > reason IMO to disallow those. OTOH, static allocations may conflict > > with the UEFI memory map, so those need to be dropped or at least > > checked against the memory map. The problem here is that static nodes > > may also be referenced by phandle, so we need to handle the referring > > node in some way as well. > > > > So I think we have a number of options: > > - ignore /memreserve/s and reject static allocations in > > /reserved-memory (*) but honor dynamic ones > > - ignore /memreserve/s and honor all of /reserved-memory after > > checking that static allocations don't conflict > > - honor all /memreserve/s and /reserved-memory nodes and check all for > > conflicts > > - ... > > > > (*) static allocations for regions that the UEFI memory map does not > > describe should be OK, though > > > > I personally prefer the first one, since a dynamic allocation > > implicitly conveys that the region does not contain anything special > > when coming out of boot, and there is very little we need to do other > > than perform the actual reservation. Static allocations are ambiguous > > in the sense that there is no annotation that explains the choice of > > address. > > > > Thoughts, please? > > What's the status of this series? It was on my "list of patches to watch" > that I'm just refreshing for 4.5, but I can't see any comments on-list > about it. I thought it's being taken over by this series: http://lkml.kernel.org/r/1446126059-25336-1-git-send-email-ard.biesheu...@linaro.org -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 3/3] arm64/efi: isolate EFI stub from the kernel proper
On Fri, Oct 30, 2015 at 01:17:24PM +0100, Ard Biesheuvel wrote: > On 27 October 2015 at 23:44, Jeremy Linton wrote: > > On 10/26/2015 09:20 PM, Ard Biesheuvel wrote: > >> > >> Thanks for the report. The following patch should fix it > > > > > > Ard, > > > > Thanks, it does fix the build problem... > > > > Now to get the darn thing to boot (not related to this AFAIK). > > Would you like me to resend this as a separate patch? No need to but it would be good to get an Ack from Matt since it touches drivers/firmware/efi/libstub/Makefile (the patch introducing STUBCOPY_FLAGS was already acked). -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm64/efi: move arm64 specific stub C code to libstub
On Fri, Oct 30, 2015 at 12:57:48PM +0100, Ard Biesheuvel wrote: > On 29 October 2015 at 19:39, Catalin Marinas wrote: > > On Fri, Oct 23, 2015 at 04:48:14PM +0200, Ard Biesheuvel wrote: > >> Now that we added special handling to the C files in libstub, move > >> the one remaining arm64 specific EFI stub C file to libstub as > >> well, so that it gets the same treatment. This should prevent future > >> changes from resulting in binaries that may execute incorrectly in > >> UEFI context. > >> > >> With efi-entry.S the only remaining EFI stub source file under > >> arch/arm64, we can also simplify the Makefile logic somewhat. > >> > >> Signed-off-by: Ard Biesheuvel > >> --- > >> > >> I would like to suggest that this be taken on top of the stuff that is > >> queued for 4.4 at the moment (if it is not too late already). > > > > Applied. Thanks. > > Please note (before you push it out), that Will just pulled a 4.3-rc > fix that changes arch/arm64/kernel/efi-stub.c Thanks for the heads up. Apparently git is smart enough to move the patched file if I merge my upstream branch into Will's without even complaining. -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm64/efi: move arm64 specific stub C code to libstub
On Fri, Oct 23, 2015 at 04:48:14PM +0200, Ard Biesheuvel wrote: > Now that we added special handling to the C files in libstub, move > the one remaining arm64 specific EFI stub C file to libstub as > well, so that it gets the same treatment. This should prevent future > changes from resulting in binaries that may execute incorrectly in > UEFI context. > > With efi-entry.S the only remaining EFI stub source file under > arch/arm64, we can also simplify the Makefile logic somewhat. > > Signed-off-by: Ard Biesheuvel > --- > > I would like to suggest that this be taken on top of the stuff that is > queued for 4.4 at the moment (if it is not too late already). Applied. Thanks. -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 0/3] arm64: EFI stub isolation
On Sat, Oct 10, 2015 at 11:40:51PM +0100, Matt Fleming wrote: > On Thu, 08 Oct, at 08:02:01PM, Ard Biesheuvel wrote: > > We need to ensure that the EFI stub only uses parts of the kernel proper > > that are safe to use when the kernel virtual mapping is not active yet. > > > > So move all C code dependencies to the libstub, which is built with all > > instrumentation (gcov, kasan) disabled, and do a verification pass to > > ensure that no absolute relocations are used. > > > > On the arm64 side, annotate all the stub's dependencies as safe for PI > > (position independent > > > > @Matt: if you are OK with these, may we please have your ack on patches #1 > > and > > #2 so that Catalin can pick up the series? Thanks. > > I assumed you meant PATCH 1 and PATCH 3. If so, yeah, these look fine > to be taken through Catalin's tree. Thanks for the review. Patches queued for 4.4. -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/3] arm64: EFI stub isolation
On Tue, Oct 06, 2015 at 12:03:41PM +0100, Ard Biesheuvel wrote: > We need to ensure that the EFI stub only uses parts of the kernel proper > that are safe to use when the kernel virtual mapping is not active yet. > > So move all C code dependencies to the libstub, and do a verification pass > to ensure that no absolute relocations are used. > > On the arm64 side, annotate all the stub's dependencies as safe for PI > (position independent [...] > Ard Biesheuvel (3): > arm64/efi: remove /chosen/linux,uefi-stub-kern-ver DT property > arm64: use ENDPIPROC() to annotate position independent assembler > routines > arm64/efi: isolate EFI stub from the kernel proper The patches look fine to me. I haven't tested them yet since they don't apply to the arm64 for-next/core cleanly and you said you are going to repost the series anyway. I'll wait. -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/6] KASAN for arm64
On Thu, Oct 08, 2015 at 02:09:26PM +0200, Ard Biesheuvel wrote:
> On 8 October 2015 at 13:23, Andrey Ryabinin wrote:
> > On 10/08/2015 02:11 PM, Mark Rutland wrote:
> >> On Thu, Oct 08, 2015 at 01:36:09PM +0300, Andrey Ryabinin wrote:
> >>> 2015-10-07 13:04 GMT+03:00 Catalin Marinas :
> >>>> On Thu, Sep 17, 2015 at 12:38:06PM +0300, Andrey Ryabinin wrote:
> >>>>> As usual patches available in git
> >>>>> git://github.com/aryabinin/linux.git kasan/arm64v6
> >>>>>
> >>>>> Changes since v5:
> >>>>> - Rebase on top of 4.3-rc1
> >>>>> - Fixed EFI boot.
> >>>>> - Updated Doc/features/KASAN.
> >>>>
> >>>> I tried to merge these patches (apart from the x86 one which is already
> >>>> merged) but it still doesn't boot on Juno as an EFI application.
> >>>>
> >>>
> >>> 4.3-rc1 was ok and 4.3-rc4 is not. Break caused by 0ce3cc008ec04
> >>> ("arm64/efi: Fix boot crash by not padding between EFI_MEMORY_RUNTIME
> >>> regions")
> >>> It introduced sort() call in efi_get_virtmap().
> >>> sort() is generic kernel function and it's instrumented, so we crash
> >>> when KASAN tries to access shadow in sort().
> >>
> >> I believe this is solved by Ard's stub isolation series [1,2], which
> >> will build a stub-specific copy of sort() and various other functions
> >> (see the arm-deps in [2]).
> >>
> >> So long as the stub is not built with ASAN, that should work.
> >
> > Thanks, this should help, as we already build the stub without ASAN
> > instrumentation.
>
> Indeed. I did not mention instrumentation in the commit log for those
> patches, but obviously, something like KASAN instrumentation cannot be
> tolerated in the stub since it makes assumptions about the memory
> layout
I'll review your latest EFI stub isolation patches and try Kasan again
on top (most likely tomorrow).
Thanks.
--
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm64/efi: map the entire UEFI vendor string before reading it
On Sun, Jul 26, 2015 at 02:59:00PM +0200, Ard Biesheuvel wrote: > At boot, the UTF-16 UEFI vendor string is copied from the system > table into a char array with a size of 100 bytes. However, this > size of 100 bytes is also used for memremapping() the source, > which may not be sufficient if the vendor string exceeds 50 > UTF-16 characters, and the placement of the vendor string inside > a 4 KB page happens to leave the end unmapped. > > So use the correct '100 * sizeof(efi_char16_t)' for the size of > the mapping. > > Signed-off-by: Ard Biesheuvel > --- > Hello Catalin, > > I think this should go into v4.2 with a cc: stable. If it's cc stable, do you have a Fixes: tag? (it saves me some searching). -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/8] of/fdt: split off FDT self reservation from memreserve processing
On Mon, May 11, 2015 at 08:41:53AM +0200, Ard Biesheuvel wrote: > This splits off the reservation of the memory occupied by the FDT > binary itself from the processing of the memory reservations it > contains. This is necessary because the physical address of the FDT, > which is needed to perform the reservation, may not be known to the > FDT driver core, i.e., it may be mapped outside the linear direct > mapping, in which case __pa() returns a bogus value. > > Cc: Russell King > Cc: Benjamin Herrenschmidt > Cc: Paul Mackerras > Acked-by: Rob Herring > Acked-by: Mark Rutland > Signed-off-by: Ard Biesheuvel For the arm64 part: Acked-by: Catalin Marinas -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] efi/arm64: use UEFI for system reset and poweroff
On Tue, Mar 10, 2015 at 02:48:57PM +, Matt Fleming wrote: > On Fri, 06 Mar, at 03:49:24PM, Ard Biesheuvel wrote: > > If UEFI Runtime Services are available, they are preferred over direct > > PSCI calls or other methods to reset the system. > > > > For the reset case, we need to hook into machine_restart(), as the > > arm_pm_restart function pointer may be overwritten by modules. > > > > Tested-by: Mark Rutland > > Reviewed-by: Mark Rutland > > Signed-off-by: Ard Biesheuvel > > --- > > arch/arm64/kernel/efi.c | 9 + > > arch/arm64/kernel/process.c | 8 > > 2 files changed, 17 insertions(+) > > Looks good to me. > > Reviewed-by: Matt Fleming > > Catalin, are you picking this up? Yes, I will. Thanks. -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm64/efi: move virtmap init to early initcall
On Thu, Jan 22, 2015 at 05:37:58PM +, Ard Biesheuvel wrote: > On 22 January 2015 at 15:59, Catalin Marinas wrote: > > On Thu, Jan 22, 2015 at 10:01:40AM +, Ard Biesheuvel wrote: > >> Now that the create_mapping() code in mm/mmu.c is able to support > >> setting up kernel page tables at initcall time, we can move the whole > >> virtmap creation to arm64_enable_runtime_services() instead of having > >> a distinct stage during early boot. This also allows us to drop the > >> arm64-specific EFI_VIRTMAP flag. > >> > >> Signed-off-by: Ard Biesheuvel > > > > Applied. Thanks. > > Wow that was quick. I noticed that you mangled my S-o-b line though: > not something I care deeply about, just a FYI Interesting. I have a script to use the message-id and download the raw message from gmane.org (as our email system doesn't like patches). For some strange reason, it converts your email address but not Laura's (I used exactly the same script with her patches). -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm64/efi: move virtmap init to early initcall
On Thu, Jan 22, 2015 at 10:01:40AM +, Ard Biesheuvel wrote: > Now that the create_mapping() code in mm/mmu.c is able to support > setting up kernel page tables at initcall time, we can move the whole > virtmap creation to arm64_enable_runtime_services() instead of having > a distinct stage during early boot. This also allows us to drop the > arm64-specific EFI_VIRTMAP flag. > > Signed-off-by: Ard Biesheuvel Applied. Thanks. -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm64/efi: handle potential failure to remap memory map
On Thu, Jan 15, 2015 at 04:40:11PM +, Mark Rutland wrote: > On Thu, Jan 15, 2015 at 12:01:06PM +, Ard Biesheuvel wrote: > > When remapping the UEFI memory map using ioremap_cache(), we > > have to deal with potential failure. Note that, even if the > > common case is for ioremap_cache() to return the existing linear > > mapping of the memory map, we cannot rely on that to be always the > > case, e.g., in the presence of a mem= kernel parameter. > > > > At the same time, remove a stale comment and move the memmap code > > together. > > > > Signed-off-by: Ard Biesheuvel > > Acked-by: Mark Rutland > > This should probably be picked up before my mem= patch, which I will be > resending shortly. I'll mention that when posting. > > I guess Catalin should pick this up given the other portions are in the > arm64 tree, and this falls under arch/arm64. I picked both up. Thanks. -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm64/efi: set PE/COFF file alignment to 512 bytes
On Fri, Oct 10, 2014 at 05:58:55PM +0100, Ard Biesheuvel wrote: > Change our PE/COFF header to use the minimum file alignment of > 512 bytes (0x200), as mandated by the PE/COFF spec v8.3 > > Also update the linker script so that the Image file itself is also a > round multiple of FileAlignment. > > Signed-off-by: Ard Biesheuvel > --- > > Another one from the PE/COFF rabbit hole. I will follow up next week and > repost > as a coherent series, once I have collected all the feedback. If Matt is taking more patches in this series, here's my ack: Acked-by: Catalin Marinas -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 32/44] arm64: psci: Register with kernel poweroff handler
On Tue, Oct 07, 2014 at 06:28:34AM +0100, Guenter Roeck wrote: > Register with kernel poweroff handler instead of setting pm_power_off > directly. > > Cc: Catalin Marinas > Cc: Will Deacon > Signed-off-by: Guenter Roeck > --- > arch/arm64/kernel/psci.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c > index 5539547..c1f3d09 100644 > --- a/arch/arm64/kernel/psci.c > +++ b/arch/arm64/kernel/psci.c > @@ -286,7 +286,7 @@ static int __init psci_0_2_init(struct device_node *np) > > arm_pm_restart = psci_sys_reset; > > - pm_power_off = psci_sys_poweroff; > + register_poweroff_handler_simple(psci_sys_poweroff, 128); > > out_put_node: > of_node_put(np); Acked-by: Catalin Marinas -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 12/44] mfd: ab8500-sysctrl: Register with kernel poweroff handler
On Tue, Oct 07, 2014 at 09:00:48AM +0100, Lee Jones wrote:
> On Mon, 06 Oct 2014, Guenter Roeck wrote:
> > --- a/drivers/mfd/ab8500-sysctrl.c
> > +++ b/drivers/mfd/ab8500-sysctrl.c
> > @@ -6,6 +6,7 @@
>
> [...]
>
> > +static int ab8500_power_off(struct notifier_block *this, unsigned long
> > unused1,
> > + void *unused2)
> > {
> > sigset_t old;
> > sigset_t all;
> > @@ -34,11 +36,6 @@ static void ab8500_power_off(void)
> > struct power_supply *psy;
> > int ret;
> >
> > - if (sysctrl_dev == NULL) {
> > - pr_err("%s: sysctrl not initialized\n", __func__);
> > - return;
> > - }
>
> Can you explain the purpose of this change please?
I guess it's because the sysctrl_dev is already initialised when
registering the power_off handler, so there isn't a way to call the
above function with a NULL sysctrl_dev. Probably even with the original
code you didn't need this check (after some race fix in
ab8500_sysctrl_remove but races is one of the things Guenter's patches
try to address).
--
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm64: ignore DT memreserve entries when booting in UEFI mode
On Wed, Aug 13, 2014 at 01:22:39PM +0100, Leif Lindholm wrote:
> On Mon, Jul 28, 2014 at 07:14:49PM +0100, Mark Rutland wrote:
> > On Mon, Jul 28, 2014 at 07:03:03PM +0100, Leif Lindholm wrote:
> > > UEFI provides its own method for marking regions to reserve, via the
> > > memory map which is also used to initialise memblock. So when using the
> > > UEFI memory map, ignore any memreserve entries present in the DT.
> >
> > It's worth noting that no-one is relying on this at present, and were
> > they it would imply that their UEFI implementation is broken (providing
> > a dodgy memory map).
> >
> > So before people start doing dodgy things, let's, close that hole. If we
> > need a way of modifying the memory map we should come up with a more
> > general plan.
>
> Catalin - any comments?
> As it is a bugfix, can it go into 3.17?
It looks fine to me:
Acked-by: Catalin Marinas
(cc'ing Will who handles the 3.17 merging)
> > > Reported-by: Mark Rutland
> > > Signed-off-by: Leif Lindholm
> > > ---
> > > arch/arm64/kernel/efi.c |2 ++
> > > arch/arm64/mm/init.c|4 +++-
> > > 2 files changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> > > index 14db1f6..7ad17b2 100644
> > > --- a/arch/arm64/kernel/efi.c
> > > +++ b/arch/arm64/kernel/efi.c
> > > @@ -188,6 +188,8 @@ static __init void reserve_regions(void)
> > > if (uefi_debug)
> > > pr_cont("\n");
> > > }
> > > +
> > > + set_bit(EFI_MEMMAP, &efi.flags);
> > > }
> > >
> > >
> > > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> > > index e90c542..58dbf2e 100644
> > > --- a/arch/arm64/mm/init.c
> > > +++ b/arch/arm64/mm/init.c
> > > @@ -32,6 +32,7 @@
> > > #include
> > > #include
> > > #include
> > > +#include
> > >
> > > #include
> > > #include
> > > @@ -151,7 +152,8 @@ void __init arm64_memblock_init(void)
> > > memblock_reserve(__pa(swapper_pg_dir), SWAPPER_DIR_SIZE);
> > > memblock_reserve(__pa(idmap_pg_dir), IDMAP_DIR_SIZE);
> > >
> > > - early_init_fdt_scan_reserved_mem();
> > > + if (!efi_enabled(EFI_MEMMAP))
> > > + early_init_fdt_scan_reserved_mem();
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] efi: fdt: Do not report an error during boot if UEFI is not available
Currently, fdt_find_uefi_params() reports an error if no EFI parameters
are found in the DT. This is however a valid case for non-UEFI kernel
booting. This patch checks changes the error reporting to a
pr_info("UEFI not found") when no EFI parameters are found in the DT.
Signed-off-by: Catalin Marinas
Acked-by: Leif Lindholm
Tested-by: Leif Lindholm
---
drivers/firmware/efi/efi.c | 22 +++---
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index eff1a2f22f09..dc79346689e6 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -346,6 +346,7 @@ static __initdata struct {
struct param_info {
int verbose;
+ int found;
void *params;
};
@@ -362,16 +363,12 @@ static int __init fdt_find_uefi_params(unsigned long
node, const char *uname,
(strcmp(uname, "chosen") != 0 && strcmp(uname, "chosen@0") != 0))
return 0;
- pr_info("Getting parameters from FDT:\n");
-
for (i = 0; i < ARRAY_SIZE(dt_params); i++) {
prop = of_get_flat_dt_prop(node, dt_params[i].propname, &len);
- if (!prop) {
- pr_err("Can't find %s in device tree!\n",
- dt_params[i].name);
+ if (!prop)
return 0;
- }
dest = info->params + dt_params[i].offset;
+ info->found++;
val = of_read_number(prop, len / sizeof(u32));
@@ -390,10 +387,21 @@ static int __init fdt_find_uefi_params(unsigned long
node, const char *uname,
int __init efi_get_fdt_params(struct efi_fdt_params *params, int verbose)
{
struct param_info info;
+ int ret;
+
+ pr_info("Getting EFI parameters from FDT:\n");
info.verbose = verbose;
+ info.found = 0;
info.params = params;
- return of_scan_flat_dt(fdt_find_uefi_params, &info);
+ ret = of_scan_flat_dt(fdt_find_uefi_params, &info);
+ if (!info.found)
+ pr_info("UEFI not found.\n");
+ else if (!ret)
+ pr_err("Can't find '%s' in device tree!\n",
+ dt_params[info.found].name);
+
+ return ret;
}
#endif /* CONFIG_EFI_PARAMS_FROM_FDT */
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: EFI_STUB fails to boot non-EFI on arm64
I forgot about this thread. I think we need it sorted in some way.
On Fri, May 23, 2014 at 04:03:31PM +0100, Leif Lindholm wrote:
> On Fri, May 23, 2014 at 02:47:20PM +0100, Catalin Marinas wrote:
> > Can we add another of detecting whether it's an EFI application and
> > avoid calling efi_init()? I can see x86 sets some efi_loader_signature
> > string in exit_boot() and checks against it later when calling
> > efi_init().
So, to be in line with 32-bit arm, the only way to tell the uncompressed
kernel image that it was started as an EFI application is via FDT.
> My view is that this should be fixed in fdt_find_uefi_params(). A
> single info message that we can't find evidence of UEFI should be
> printed in the non-error case.
[...]
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index cd36deb..4bb42e1e 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -366,11 +366,8 @@ static int __init fdt_find_uefi_params(unsigned long
> node, const char *uname,
>
> for (i = 0; i < ARRAY_SIZE(dt_params); i++) {
> prop = of_get_flat_dt_prop(node, dt_params[i].propname, &len);
> - if (!prop) {
> - pr_err("Can't find %s in device tree!\n",
> -dt_params[i].name);
> - return 0;
> - }
> + if (!prop)
> + goto fail;
> dest = info->params + dt_params[i].offset;
>
> val = of_read_number(prop, len / sizeof(u32));
> @@ -385,6 +382,14 @@ static int __init fdt_find_uefi_params(unsigned long
> node, const char *uname,
> dt_params[i].size * 2, val);
> }
> return 1;
> +
> + fail:
> + if (i == 0)
> + pr_info(" UEFI not found.\n");
> + else
> + pr_err("Can't find %s in device tree!\n", dt_params[i].name);
> +
> + return 0;
I'm ok with the idea but I don't particularly like the implementation.
Does this look any better (functionally the same as yours)?
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index eff1a2f22f09..dc79346689e6 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -346,6 +346,7 @@ static __initdata struct {
struct param_info {
int verbose;
+ int found;
void *params;
};
@@ -362,16 +363,12 @@ static int __init fdt_find_uefi_params(unsigned long
node, const char *uname,
(strcmp(uname, "chosen") != 0 && strcmp(uname, "chosen@0") != 0))
return 0;
- pr_info("Getting parameters from FDT:\n");
-
for (i = 0; i < ARRAY_SIZE(dt_params); i++) {
prop = of_get_flat_dt_prop(node, dt_params[i].propname, &len);
- if (!prop) {
- pr_err("Can't find %s in device tree!\n",
- dt_params[i].name);
+ if (!prop)
return 0;
- }
dest = info->params + dt_params[i].offset;
+ info->found++;
val = of_read_number(prop, len / sizeof(u32));
@@ -390,10 +387,21 @@ static int __init fdt_find_uefi_params(unsigned long
node, const char *uname,
int __init efi_get_fdt_params(struct efi_fdt_params *params, int verbose)
{
struct param_info info;
+ int ret;
+
+ pr_info("Getting EFI parameters from FDT:\n");
info.verbose = verbose;
+ info.found = 0;
info.params = params;
- return of_scan_flat_dt(fdt_find_uefi_params, &info);
+ ret = of_scan_flat_dt(fdt_find_uefi_params, &info);
+ if (!info.found)
+ pr_info("UEFI not found.\n");
+ else if (!ret)
+ pr_err("Can't find '%s' in device tree!\n",
+ dt_params[info.found].name);
+
+ return ret;
}
#endif /* CONFIG_EFI_PARAMS_FROM_FDT */
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] efi/arm64: handle missing virtual mapping for UEFI System Table
On 4 Jul 2014, at 18:19, Ard Biesheuvel wrote: > On 4 July 2014 19:01, Catalin Marinas wrote: >> On Fri, Jul 04, 2014 at 05:52:36PM +0100, Mark Salter wrote: >>> On Fri, 2014-07-04 at 17:25 +0200, Ard Biesheuvel wrote: >>>> If we cannot resolve the virtual address of the UEFI System Table, its >>>> physical >>>> offset must be missing from the virtual memory map, and there is really no >>>> point >>>> in proceeding with installing the virtual memory map and the runtime >>>> services >>>> dispatch table. So back out gracefully. >>>> >>>> Signed-off-by: Ard Biesheuvel >>>> --- >>> >>> Acked-by: Mark Salter >> >> Thanks. Do I take this patch or Matt already has other EFI patches to >> push? If the latter: >> >> Acked-by: Catalin Marinas > > Thanks. You can take it for 3.16 if you like, otherwise it is probably > better if we ask Matt to take it through his tree with some other > stuff we have in flight at the moment. OK, I’ll leave it to Matt then together with the other EFI patches he’s already queuing. Thanks, Catalin-- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] efi/arm64: handle missing virtual mapping for UEFI System Table
On Fri, Jul 04, 2014 at 05:52:36PM +0100, Mark Salter wrote: > On Fri, 2014-07-04 at 17:25 +0200, Ard Biesheuvel wrote: > > If we cannot resolve the virtual address of the UEFI System Table, its > > physical > > offset must be missing from the virtual memory map, and there is really no > > point > > in proceeding with installing the virtual memory map and the runtime > > services > > dispatch table. So back out gracefully. > > > > Signed-off-by: Ard Biesheuvel > > --- > > Acked-by: Mark Salter Thanks. Do I take this patch or Matt already has other EFI patches to push? If the latter: Acked-by: Catalin Marinas -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] efi/arm64: preserve FP/SIMD registers on UEFI runtime services calls
On Fri, Jul 04, 2014 at 04:51:31PM +0100, Ard Biesheuvel wrote: > On 4 July 2014 17:45, Catalin Marinas wrote: > > On Thu, Jun 26, 2014 at 11:09:06AM +0100, Ard Biesheuvel wrote: > >> According to the UEFI spec section 2.3.6.4, the use of FP/SIMD > >> instructions is > >> allowed, and should adhere to the AAPCS64 calling convention, which states > >> that > >> 'only the bottom 64 bits of each value stored in registers v8-v15 need to > >> be > >> preserved' (section 5.1.2). > >> > >> This applies equally to UEFI Runtime Services called by the kernel, so > >> make sure > >> the FP/SIMD register file is preserved in this case. > >> > >> Signed-off-by: Ard Biesheuvel > > > > While the code looks fine, I think there is a mismatch between what the > > subject says and what the patch does (enabling EFI_RUNTIME_WRAPPERS). > > > > Not entirely. In order to be able to insert calls to > kernel_neon_begin()/end() into the runtime services calls, we need > a) to supply definitions for efi_call_virt() and __efi_call_virt() > that contain those calls to kernel_neon_begin()/end() > b) to enable runtime wrappers (which is what uses those definitions) > > Would you prefer those to be split in 2 patches? No, that's fine. You could just add the above explanation to the commit log. Otherwise: Acked-by: Catalin Marinas -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] efi/arm64: preserve FP/SIMD registers on UEFI runtime services calls
On Thu, Jun 26, 2014 at 11:09:06AM +0100, Ard Biesheuvel wrote: > According to the UEFI spec section 2.3.6.4, the use of FP/SIMD instructions is > allowed, and should adhere to the AAPCS64 calling convention, which states > that > 'only the bottom 64 bits of each value stored in registers v8-v15 need to be > preserved' (section 5.1.2). > > This applies equally to UEFI Runtime Services called by the kernel, so make > sure > the FP/SIMD register file is preserved in this case. > > Signed-off-by: Ard Biesheuvel While the code looks fine, I think there is a mismatch between what the subject says and what the patch does (enabling EFI_RUNTIME_WRAPPERS). -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] efi/arm64: fix potential NULL dereference of efi.systab
On Fri, Jul 04, 2014 at 02:54:19PM +0100, Ard Biesheuvel wrote: > On 4 July 2014 15:38, Catalin Marinas wrote: > > On Wed, Jul 02, 2014 at 11:10:01AM +0100, Ard Biesheuvel wrote: > >> We assign efi.systab using efi_lookup_mapped_addr(), and test for !NULL, > >> but > >> then go on an dereference it anyway. > >> > >> Signed-off-by: Ard Biesheuvel > > > > Thanks. I applied this one (the other patch needs to go in via a > > different route). > > > > Eh, actually, I proposed a v2 based on Mark's feedback, but I now see > that I accidentally removed you from the To list. > > http://marc.info/?l=linux-arm-kernel&m=140446911506688&w=2 OK. I'll wait for acks on the other version then. -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] efi/arm64: fix potential NULL dereference of efi.systab
On Wed, Jul 02, 2014 at 11:10:01AM +0100, Ard Biesheuvel wrote: > We assign efi.systab using efi_lookup_mapped_addr(), and test for !NULL, but > then go on an dereference it anyway. > > Signed-off-by: Ard Biesheuvel Thanks. I applied this one (the other patch needs to go in via a different route). -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL] EFI urgent fix
On Fri, Jun 20, 2014 at 10:00:08AM +0100, Matt Fleming wrote: > Please pull the following compiler warning fix. Sorry, I've been pretty > slow in getting this pull request sent. Multiple people have reported > hitting it and I've now received 4 patches for the same warning, BTW, one of them is not just a simple warning but a bug. Pointer to "long" variable on the stack passed to a function that only takes a pointer to "int". We are probably lucky that we don't hit the bug on arm64. Anyway, thanks for pushing the fix. (and lesson learnt not to trust the ARM EFI_STUB developers, won't name them, with properly testing their code ;)) -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] efi: Fix compiler warnings (unused, const, type)
This patch fixes a few compiler warning in the efi code for unused
variable, discarding const qualifier and wrong pointer type:
drivers/firmware/efi/fdt.c|66 col 22| warning: unused variable ‘name’
[-Wunused-variable]
drivers/firmware/efi/efi.c|368 col 3| warning: passing argument 3 of
‘of_get_flat_dt_prop’ from incompatible pointer type [enabled by default]
drivers/firmware/efi/efi.c|368 col 8| warning: assignment discards ‘const’
qualifier from pointer target type [enabled by default]
Signed-off-by: Catalin Marinas
---
Matt,
Unless you have fixed these already, please consider applying (warnings in
latest next/master branch).
Thanks,
Catalin
drivers/firmware/efi/efi.c | 6 +++---
drivers/firmware/efi/fdt.c | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index cd36deb619fa..eff1a2f22f09 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -353,10 +353,10 @@ static int __init fdt_find_uefi_params(unsigned long
node, const char *uname,
int depth, void *data)
{
struct param_info *info = data;
- void *prop, *dest;
- unsigned long len;
+ const void *prop;
+ void *dest;
u64 val;
- int i;
+ int i, len;
if (depth != 1 ||
(strcmp(uname, "chosen") != 0 && strcmp(uname, "chosen@0") != 0))
diff --git a/drivers/firmware/efi/fdt.c b/drivers/firmware/efi/fdt.c
index 5c6a8e8a9580..82d774161cc9 100644
--- a/drivers/firmware/efi/fdt.c
+++ b/drivers/firmware/efi/fdt.c
@@ -63,7 +63,7 @@ static efi_status_t update_fdt(efi_system_table_t *sys_table,
void *orig_fdt,
*/
prev = 0;
for (;;) {
- const char *type, *name;
+ const char *type;
int len;
node = fdt_next_node(fdt, prev, NULL);
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: EFI_STUB fails to boot non-EFI on arm64
On Fri, May 23, 2014 at 02:16:56PM +0100, Leif Lindholm wrote:
> Subject: [PATCH] arm64: efi: only attempt efi map setup if booting via EFI
>
> Booting a kernel with CONFIG_EFI enabled on a non-EFI system caused
> an oops with the current UEFI support code.
> Add the required test to prevent this.
>
> Signed-off-by: Leif Lindholm
> ---
> arch/arm64/kernel/efi.c |3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index 7bfd650..14db1f6 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -333,6 +333,9 @@ void __init efi_init(void)
>
> void __init efi_idmap_init(void)
> {
> + if (!efi_enabled(EFI_BOOT))
> + return;
> +
That's a first (possibly temporary) step and I think it's fine:
Acked-by: Catalin Marinas
But we need some further tweaking to the way we call efi_init().
Currently it doesn't matter whether Linux booted as an EFI application
or not and efi_init() is always called, causing some pr_err() in
fdt_find_uefi_params(). It's not really an error as we support the same
image booting non-EFI as well.
Can we add another of detecting whether it's an EFI application and
avoid calling efi_init()? I can see x86 sets some efi_loader_signature
string in exit_boot() and checks against it later when calling
efi_init().
--
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
EFI_STUB fails to boot non-EFI on arm64
Hi, As the EFI_STUB for arm64 got into tip and soon into -next, I thought about giving it a try (tip/arm64/efi). Using my boot-wrapper (non-EFI) is still supposed to work but I get the trace below. It looks like memmap.map is NULL but the code still assumes the kernel was started as an EFI app. Have you guys tested these patches properly? Linux version 3.15.0-rc1+ (cmarinas@e102109-lin) (gcc version 4.8.3 20140401 (prerelease) (0.12.310) ) #451 SMP PREEMPT Fri May 23 10:40:10 BST 2014 CPU: AArch64 Processor [410fd0f0] revision 0 bootconsole [earlycon0] enabled efi: Getting parameters from FDT: efi: Can't find System Table in device tree! cma: CMA: reserved 16 MiB at 8ff00 Trying ::1... Trying 127.0.0.1... Connected to localhost. Escape character is '^]'. Unable to handle kernel NULL pointer dereference at virtual address 0020 pgd = ffc7d000 [0020] *pgd= Internal error: Oops: 9605 [#1] PREEMPT SMP Modules linked in: CPU: 0 PID: 0 Comm: swapper Not tainted 3.15.0-rc1+ #451 task: ffc0005f4890 ti: ffc0005e8000 task.ti: ffc0005e8000 PC is at efi_idmap_init+0x74/0xc8 LR is at efi_idmap_init+0x4c/0xc8 pc : [] lr : [] pstate: 62c5 sp : ffc0005ebee0 x29: ffc0005ebee0 x28: 00408000 x27: ffc0005f60c8 x26: ffc0005f6000 x25: ffc00053f618 x24: ffc0005f8fa8 x23: ffc00060 x22: ffc000629000 x21: ffc00060 x20: ffc000629338 x19: x18: x17: ffc000635d48 x16: 0001 x15: 000a x14: 0009 x13: 0018 x12: 00088000 x11: ffc0 x10: 0040 x9 : 02c00713 x8 : 4000 x7 : 0008 x6 : ffc000629390 x5 : ffc000629000 x4 : x3 : 0008ffe00711 x2 : x1 : x0 : Process swapper (pid: 0, stack limit = 0xffc0005e8058) Stack: (0xffc0005ebee0 to 0xffc0005ec000) bee0: 005ebf10 ffc0 005b667c ffc0 7effdf80 ffc8 00635d78 ffc0 bf00: 005f6108 ffc0 005b65ec ffc0 005ebfa0 ffc0 005b4528 ffc0 bf20: 00629000 ffc0 0001 005d7fd8 ffc0 410fd0f0 bf40: 805f6000 8000 8007b000 8007d000 bf60: 00080590 ffc0 0044d088 ffc0 0001 8800 bf80: 0062f278 ffc0 0002 0062ee32 ffc0 bfa0: 80080330 0e12 bfc0: 8800 410fd0f0 805f6000 bfe0: 005d7fd8 ffc0 Call trace: [] efi_idmap_init+0x74/0xc8 [] setup_arch+0x3d4/0x57c [] start_kernel+0x88/0x364 Code: f9401a80 cb20 eb00027f 54000248 (f9401260) ---[ end trace f3ac502fe4422ad7 ]--- Kernel panic - not syncing: Attempted to kill the idle task! ---[ end Kernel panic - not syncing: Attempted to kill the idle task! -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL] EFI changes for arm64
On Mon, May 19, 2014 at 11:50:42AM +0100, Matt Fleming wrote: > On Wed, 30 Apr, at 09:03:32PM, Matt Fleming wrote: > > I pulled the arm64 EFI changes into the following topic branch. Catalin > > was happy for this to go through tip, which definitely makes things > > easier for the next merge window because of the dependency these patches > > have on tip/x86/efi. > > > > The following changes since commit e33655a386ed3b26ad36fb97a47ebb1c2ca1e928: > > > > efivars: Add compatibility code for compat tasks (2014-04-17 13:53:53 > > +0100) > > > > are available in the git repository at: > > > > git://git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi.git arm64-efi > > > > for you to fetch changes up to 345c736edd07b657a8c48190baed2719b85d0938: > > > > efi/arm64: ignore dtb= when UEFI SecureBoot is enabled (2014-04-30 > > 19:57:06 +0100) > > Ping? Alternatively, I can merge the arm64-efi tree and wait for tip x86/efi to go in before sending my pull request. Peter, Ingo, do you have any preference? Either way, I'd like to see this branch in -next to make sure there are no (significant) conflicts before the merging window. Thanks. -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/10] arm64: UEFI support
On Tue, Apr 29, 2014 at 03:47:13PM +0100, Matt Fleming wrote: > On Tue, 29 Apr, at 02:47:28PM, Catalin Marinas wrote: > > Given that Leif's series contains both generic efi and arm64 patches, > > what's your preference for merging them? I'm happy to add my ack and > > they go via your tree (or the other way around). > > I'm happy either way, though if I take them through my tree (and > subsequently through tip) you won't have to worry about the merge window > rigmarole, which is a plus. > > So, eveyone happy for me to take these with Catalin's Acked-by? Fine by me. Just in case I haven't stated it explicitly for this series: Acked-by: Catalin Marinas Thanks. -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/10] arm64: UEFI support
On Tue, Apr 29, 2014 at 12:43:56PM +0100, Matt Fleming wrote: > (Pulling in Peter and Stephen) > > On Tue, 29 Apr, at 11:28:17AM, Catalin Marinas wrote: > > > > The patches look fine to me, they've been through several rounds of > > review already. How do we propose these get merged as the series > > contains both generic and arm64 patches? And there are dependencies > > already in linux-next. > > > > Are the EFI patches in -next pulled from some non-rebaseable branch? > > Peter suggsted a plan when he took the generic EFI stuff that's in tip > (and hence currently in linux-next), > > It doesn't hurt to inform Stephen, although I think it will simply fall > out automatically since he uses git to merge and git will recognize the > graph. > > During the merge window, it means they should not push their patches > until Linus has accepted the precondition patches from the tip tree. > Since Ingo and I try to push most of the tip tree as early as possible > in the merge window, this is usually not a problem. > > So we currently have the prerequisites in tip/x86/efi, and assuming that > this 10-patch series gets merged into a single branch somewhere, things > should work automatically for linux-next. > > It may be prudent to negotiate a plan now for when the merge window > opens because, as Peter mentions above, the stuff in tip/x86/efi needs > to be merged by Linus first to avoid build breakage with the arm64 > stuff. Waiting for the tip/x86/efi to be merged first is not a problem. We also need a stable base for testing the arm64 UEFI series, so I assume this series can be based onto tip/x86/efi (would such branch be rebased before hitting mainline?). Given that Leif's series contains both generic efi and arm64 patches, what's your preference for merging them? I'm happy to add my ack and they go via your tree (or the other way around). Thanks. -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/10] arm64: UEFI support
On Fri, Apr 25, 2014 at 05:09:04PM +0100, Leif Lindholm wrote: > This set adds support for UEFI to the arm64 port - a stub loader, as > well as runtime services support for efivars. > > It depends on some core EFI patches currently in linux-next. The patches look fine to me, they've been through several rounds of review already. How do we propose these get merged as the series contains both generic and arm64 patches? And there are dependencies already in linux-next. Are the EFI patches in -next pulled from some non-rebaseable branch? -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 11/15] arm64: add EFI stub
On Wed, Mar 19, 2014 at 03:13:18PM +, Mark Salter wrote: > On Wed, 2014-03-19 at 10:57 +0000, Catalin Marinas wrote: > > On Tue, Mar 18, 2014 at 09:40:31PM +, Mark Salter wrote: > > > On Tue, 2014-03-18 at 18:28 +0000, Catalin Marinas wrote: > > > > If UEFI doesn't handle the caches, the only thing left to EFI_STUB is to > > > > flush by MVA. We don't need to flush the whole DRAM (and I would even > > > > recommend it) but at least the relevant kernel code/data touched with > > > > the MMU disabled. > > > > > > So, it goes like this: > > > > > > 1) UEFI calls stub with MMU/Caches on. Stub/kernel can be anywhere. > > > 2) Stub runs and relocates kernel to the desired runtime location > > > but continues to execute from wherever UEFI loaded it until just > > > after ExitBootServices(). > > > 3) After ExitBootServices, efi_entry() returns relocated entry point > > > for kernel to efi_stub_entry() in efi-entry.S where the Dcache and > > > MMU are turned off, the __flush_dcache_all is called, then the > > > code jumps to the kernel proper entry point. > > > > > > It isn't clear to me if UEFI does cache flushing at ExitBootServices > > > time, but even so, at least stack use will get cached between then and > > > the kernel entry point. The stub could conceivably get its hands on the > > > EFI memmap and invalidate dcache using address ranges from UEFI memory > > > descriptors so maybe that is the way we should do it. > > > > I think the stub just needs to flush the relocated kernel image, ensure > > it is sync with the memory. Additional flushing can be done by the > > kernel for bits it writes (like page tables, code patching etc). We can > > enter the kernel with the SCTLR.I bit set, so it can allocate in an > > unified cache already and D-cache maintenance would be needed anyway. > > How about this? > > diff --git a/arch/arm64/kernel/efi-entry.S b/arch/arm64/kernel/efi-entry.S > index 83bfb72..ed480b2 100644 > --- a/arch/arm64/kernel/efi-entry.S > +++ b/arch/arm64/kernel/efi-entry.S > @@ -52,11 +52,19 @@ ENTRY(efi_stub_entry) >* efi_entry() will have relocated the kernel image if necessary >* and we return here with device tree address in x0 and the kernel >* entry point stored at *image_addr. Save those values in registers > - * which are preserved by __flush_dcache_all. > + * which are callee preserved. >*/ > - ldr x1, [sp, #16] > mov x20, x0 > - mov x21, x1 > + ldr x0, [sp, #16] > + mov x21, x0 BTW, some comments to make it easier to review the code later about what's at [sp, #16]. > + > + adrpx1, _text > + add x1, x1, #:lo12:_text > + adrpx2, _edata > + add x2, x2, #:lo12:_edata > + sub x1, x2, x1 > + > + bl __flush_dcache_area Looks fine. We also need a "ic ialluis", is any other part of EFI_STUB invalidating the I-cache? We need a patch to invalidate the D-cache for areas of memory that the kernel touches but I'll do this as a separate patch. -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 11/15] arm64: add EFI stub
On Tue, Mar 18, 2014 at 09:40:31PM +, Mark Salter wrote: > On Tue, 2014-03-18 at 18:28 +0000, Catalin Marinas wrote: > > If UEFI doesn't handle the caches, the only thing left to EFI_STUB is to > > flush by MVA. We don't need to flush the whole DRAM (and I would even > > recommend it) but at least the relevant kernel code/data touched with > > the MMU disabled. > > So, it goes like this: > > 1) UEFI calls stub with MMU/Caches on. Stub/kernel can be anywhere. > 2) Stub runs and relocates kernel to the desired runtime location > but continues to execute from wherever UEFI loaded it until just > after ExitBootServices(). > 3) After ExitBootServices, efi_entry() returns relocated entry point > for kernel to efi_stub_entry() in efi-entry.S where the Dcache and > MMU are turned off, the __flush_dcache_all is called, then the > code jumps to the kernel proper entry point. > > It isn't clear to me if UEFI does cache flushing at ExitBootServices > time, but even so, at least stack use will get cached between then and > the kernel entry point. The stub could conceivably get its hands on the > EFI memmap and invalidate dcache using address ranges from UEFI memory > descriptors so maybe that is the way we should do it. I think the stub just needs to flush the relocated kernel image, ensure it is sync with the memory. Additional flushing can be done by the kernel for bits it writes (like page tables, code patching etc). We can enter the kernel with the SCTLR.I bit set, so it can allocate in an unified cache already and D-cache maintenance would be needed anyway. Another aspect in the booting document is that we require any external cache to be disabled. I think on ARMv8 with a more transparent external cache we can relax this (Applied's SoC already does this). Otherwise, since external cache enabling is usually a secure operation, we would need some SoC specific code just for this early during boot and I'd like to avoid it. -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 11/15] arm64: add EFI stub
On Tue, Mar 18, 2014 at 10:21:05PM +, Jason Gunthorpe wrote: > On Tue, Mar 18, 2014 at 02:48:30PM -0700, Roy Franz wrote: > > > > It isn't clear to me if UEFI does cache flushing at ExitBootServices > > > time, but even so, at least stack use will get cached between then and > > > the kernel entry point. The stub could conceivably get its hands on the > > > EFI memmap and invalidate dcache using address ranges from UEFI memory > > > descriptors so maybe that is the way we should do it. > > > I looked at the UEFI spec and there is no mention of cache flushing > > in ExitBootServices(), so it seems it is up to the OS to do any > > cache management. > > Something to think about: On mvebu we recently confirmed a situation > where turning off the L1 cache is not sufficient for booting. The L2 > cache must also be completely off and disabled prior to jumping in to > the kernel. > > The issue is the decompressor turns the L1 cache back on, and if the > L2 is also enabled at this point then it gets filled with > decompression data. Things go wrong from here because after > decompression the L1 dcache is switched off, but the L2 isn't > flush-invalidated. > > So now the L2 holds writeback data and uncached reads return garbage, > and/or the L2 misses the uncached writes (eg relocation fixup) and > becomes inconsistent with memory. Either case gives a black screen > crash at boot. > > Fundementally if the L2 doesn't monitor uncached read/writes then it > will cause a big problem. Yes, that's a problem on (ARMv7) SoCs booting in non-secure mode with external L2 already enabled. For ARMv8 at least, the SoCs I've seen with external caches detect the data cache by MVA ops. > Thus, if the UEFI calls the sub with the caches on, and the stub > doesn't know enough to turn off the L2 then you might not be able to > turn the dcache off at all. :( > > On ARM64 at least the L1 cache ops are standardized so maybe ARM64 > could keep the mmu+caches enabled during boot and do the L1 > d-flush/i-inval required for instruction coherency? We thought about keeping the MMU on from EFI_STUB into the kernel but it gets messier since UEFI has different MMU settings. So with some sane external cache, we could get away with flushing a range. -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 11/15] arm64: add EFI stub
On Tue, Mar 18, 2014 at 02:40:29PM +, Mark Salter wrote: > On Tue, 2014-03-18 at 12:09 +0000, Catalin Marinas wrote: > > On Thu, Mar 13, 2014 at 10:47:04PM +, Leif Lindholm wrote: > > > --- /dev/null > > > +++ b/arch/arm64/kernel/efi-entry.S > > > @@ -0,0 +1,93 @@ > > > +/* > > > + * EFI entry point. > > > + * > > > + * Copyright (C) 2013 Red Hat, Inc. > > > + * Author: Mark Salter > > > + * > > > + * This program is free software; you can redistribute it and/or modify > > > + * it under the terms of the GNU General Public License version 2 as > > > + * published by the Free Software Foundation. > > > + * > > > + */ > > > +#include > > > +#include > > > + > > > +#include > > > + > > > +#define EFI_LOAD_ERROR 0x8001 > > > + > > > + __INIT > > > + > > > + /* > > > +* We arrive here from the EFI boot manager with: > > > +* > > > +** MMU on with identity-mapped RAM. > > > +** Icache and Dcache on > > > +* > > > +* We will most likely be running from some place other than where > > > +* we want to be. The kernel image wants to be placed at > > > TEXT_OFFSET > > > +* from start of RAM. > > > +*/ > > > +ENTRY(efi_stub_entry) > > > + stp x29, x30, [sp, #-32]! > > > + > > > + /* > > > +* Call efi_entry to do the real work. > > > +* x0 and x1 are already set up by firmware. Current runtime > > > +* address of image is calculated and passed via *image_addr. > > > +* > > > +* unsigned long efi_entry(void *handle, > > > +* efi_system_table_t *sys_table, > > > +* unsigned long *image_addr) ; > > > +*/ > > > + adrpx8, _text > > > + add x8, x8, #:lo12:_text > > > + add x2, sp, 16 > > > + str x8, [x2] > > > + bl efi_entry > > > + cmn x0, #1 > > > + b.eqefi_load_fail > > > + > > > + /* > > > +* efi_entry() will have relocated the kernel image if necessary > > > +* and we return here with device tree address in x0 and the > > > kernel > > > +* entry point stored at *image_addr. Save those values in > > > registers > > > +* which are preserved by __flush_dcache_all. > > > +*/ > > > + ldr x1, [sp, #16] > > > + mov x20, x0 > > > + mov x21, x1 > > > + > > > + /* Turn off Dcache and MMU */ > > > + mrs x0, CurrentEL > > > + cmp x0, #PSR_MODE_EL2t > > > + ccmpx0, #PSR_MODE_EL2h, #0x4, ne > > > + b.ne1f > > > + mrs x0, sctlr_el2 > > > + bic x0, x0, #1 << 0 // clear SCTLR.M > > > + bic x0, x0, #1 << 2 // clear SCTLR.C > > > + msr sctlr_el2, x0 > > > + isb > > > + b 2f > > > +1: > > > + mrs x0, sctlr_el1 > > > + bic x0, x0, #1 << 0 // clear SCTLR.M > > > + bic x0, x0, #1 << 2 // clear SCTLR.C > > > + msr sctlr_el1, x0 > > > + isb > > > +2: > > > + bl __flush_dcache_all > > > > In linux-next I'm pushing a patch which no longer exports the > > __flush_dcache_all function. The reason is that it doesn't really work > > if you have a (not fully transparent) external cache like on the Applied > > Micro boards. There other issues when running as a guest as well. > > > > If you know exactly what needs to be flushed here, can you use a range > > (MVA) operation? > > This is just before the EFI stub jumps to kernel proper. The only things > in the dcache would be from identity mapped references to RAM used by > UEFI. The booting.txt doc says dcache should be off and invalidated. I > am just wanting to comply with that. The code here doesn't really know > the extent of DRAM to flush by address. Does UEFI do anything with the caches before invoking the EFI_STUB code? I guess it doesn't since that's just another application for it. Can UEFI flush the caches via exit boot? When is this called? As I said, we have a real problem here since the EFI_STUB call does not have information about the SoC to be able to flush all the caches. But UEFI should know more about the hardware. If UEFI doesn't handle the caches, the only thing left to EFI_STUB is to flush by MVA. We don't need to flush the whole DRAM (and I would even recommend it) but at least the relevant kernel code/data touched with the MMU disabled. -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 13/15] arm64: add EFI runtime services
On Tue, Mar 18, 2014 at 02:16:49PM +, Mark Salter wrote: > On Tue, 2014-03-18 at 12:34 +0000, Catalin Marinas wrote: > > On Thu, Mar 13, 2014 at 10:47:06PM +, Leif Lindholm wrote: > > > --- /dev/null > > > +++ b/arch/arm64/kernel/efi.c > > [...] > > > +/* > > > + * Called from setup_arch with interrupts disabled. > > > + */ > > > +void __init efi_enter_virtual_mode(void) > > [...] > > > --- a/init/main.c > > > +++ b/init/main.c > > > @@ -902,6 +902,10 @@ static noinline void __init > > > kernel_init_freeable(void) > > > smp_prepare_cpus(setup_max_cpus); > > > > > > do_pre_smp_initcalls(); > > > + > > > + if (IS_ENABLED(CONFIG_ARM64) && efi_enabled(EFI_BOOT)) > > > + efi_enter_virtual_mode(); > > > > The comment for the efi_enter_virtual_mode() function says "called from > > setup_arch with interrupts disabled". None of these are true for the > > call above (and I would really prefer an arch call than this arm64 > > conditional call in init/main.c. > > > > Right, the call changed to later in boot but the comment didn't. Calling > from setup_arch is too early. But an early_initcall would work and would > get rid of the ugly CONFIG_ARM64 test. I agree, early_initcall() is better. -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 13/15] arm64: add EFI runtime services
On Thu, Mar 13, 2014 at 10:47:06PM +, Leif Lindholm wrote: > --- /dev/null > +++ b/arch/arm64/kernel/efi.c [...] > +/* > + * Called from setup_arch with interrupts disabled. > + */ > +void __init efi_enter_virtual_mode(void) [...] > --- a/init/main.c > +++ b/init/main.c > @@ -902,6 +902,10 @@ static noinline void __init kernel_init_freeable(void) > smp_prepare_cpus(setup_max_cpus); > > do_pre_smp_initcalls(); > + > + if (IS_ENABLED(CONFIG_ARM64) && efi_enabled(EFI_BOOT)) > + efi_enter_virtual_mode(); The comment for the efi_enter_virtual_mode() function says "called from setup_arch with interrupts disabled". None of these are true for the call above (and I would really prefer an arch call than this arm64 conditional call in init/main.c. -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 11/15] arm64: add EFI stub
On Thu, Mar 13, 2014 at 10:47:04PM +, Leif Lindholm wrote: > --- /dev/null > +++ b/arch/arm64/kernel/efi-entry.S > @@ -0,0 +1,93 @@ > +/* > + * EFI entry point. > + * > + * Copyright (C) 2013 Red Hat, Inc. > + * Author: Mark Salter > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + */ > +#include > +#include > + > +#include > + > +#define EFI_LOAD_ERROR 0x8001 > + > + __INIT > + > + /* > +* We arrive here from the EFI boot manager with: > +* > +** MMU on with identity-mapped RAM. > +** Icache and Dcache on > +* > +* We will most likely be running from some place other than where > +* we want to be. The kernel image wants to be placed at TEXT_OFFSET > +* from start of RAM. > +*/ > +ENTRY(efi_stub_entry) > + stp x29, x30, [sp, #-32]! > + > + /* > +* Call efi_entry to do the real work. > +* x0 and x1 are already set up by firmware. Current runtime > +* address of image is calculated and passed via *image_addr. > +* > +* unsigned long efi_entry(void *handle, > +* efi_system_table_t *sys_table, > +* unsigned long *image_addr) ; > +*/ > + adrpx8, _text > + add x8, x8, #:lo12:_text > + add x2, sp, 16 > + str x8, [x2] > + bl efi_entry > + cmn x0, #1 > + b.eqefi_load_fail > + > + /* > +* efi_entry() will have relocated the kernel image if necessary > +* and we return here with device tree address in x0 and the kernel > +* entry point stored at *image_addr. Save those values in registers > +* which are preserved by __flush_dcache_all. > +*/ > + ldr x1, [sp, #16] > + mov x20, x0 > + mov x21, x1 > + > + /* Turn off Dcache and MMU */ > + mrs x0, CurrentEL > + cmp x0, #PSR_MODE_EL2t > + ccmpx0, #PSR_MODE_EL2h, #0x4, ne > + b.ne1f > + mrs x0, sctlr_el2 > + bic x0, x0, #1 << 0 // clear SCTLR.M > + bic x0, x0, #1 << 2 // clear SCTLR.C > + msr sctlr_el2, x0 > + isb > + b 2f > +1: > + mrs x0, sctlr_el1 > + bic x0, x0, #1 << 0 // clear SCTLR.M > + bic x0, x0, #1 << 2 // clear SCTLR.C > + msr sctlr_el1, x0 > + isb > +2: > + bl __flush_dcache_all In linux-next I'm pushing a patch which no longer exports the __flush_dcache_all function. The reason is that it doesn't really work if you have a (not fully transparent) external cache like on the Applied Micro boards. There other issues when running as a guest as well. If you know exactly what needs to be flushed here, can you use a range (MVA) operation? > diff --git a/arch/arm64/kernel/efi-stub.c b/arch/arm64/kernel/efi-stub.c > new file mode 100644 > index 000..bf30913 > --- /dev/null > +++ b/arch/arm64/kernel/efi-stub.c > @@ -0,0 +1,83 @@ > +/* > + * linux/arch/arm/boot/compressed/efi-stub.c Nitpick: arch/arm64/... But we don't really need to write the file name here, I use a smart editor that tells me which file I'm viewing ;). Better write a one-line summary of what this file is about. > + * > + * Copyright (C) 2013, 2014 Linaro Ltd; > + * > + * This file implements the EFI boot stub for the arm64 kernel. > + * Adapted from ARM version by Mark Salter > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + */ > +#include > +#include > +#include > +#include > +#include > + > +/* > + * EFI function call wrappers. These are not required for arm/arm64, but > + * wrappers are required for X86 to convert between ABIs. These wrappers are > + * provided to allow code sharing between X86 and other architectures. Since > + * these wrappers directly invoke the EFI function pointer, the function > + * pointer type must be properly defined, which is not the case for X86. One > + * advantage of this is it allows for type checking of arguments, which is > not > + * possible with the X86 wrappers. > + */ > +#define efi_call_phys0(f) f() > +#define efi_call_phys1(f, a1) f(a1) > +#define efi_call_phys2(f, a1, a2) f(a1, a2) > +#define efi_call_phys3(f, a1, a2, a3) f(a1, a2, a3) > +#define efi_call_phys4(f, a1, a2, a3, a4) f(a1, a2, a3, a4) > +#define efi_call_phys5(f, a1, a2, a3, a4, a5) f(a1, a2, a3, a4, a5) > + > +/* > + * AArch64 requires the DTB to be 8-byte aligned in the first 512MiB from > + * start of kernel and may not cross a 2MiB boundary. We set alignment
Re: [PATCH 16/22] arm64: Add function to create identity mappings
On Wed, Feb 05, 2014 at 05:04:07PM +, Leif Lindholm wrote:
> +void __init create_id_mapping(phys_addr_t addr, phys_addr_t size, int map_io)
> +{
> + pgd_t *pgd = &idmap_pg_dir[pgd_index(addr)];
> +
> + if (pgd >= &idmap_pg_dir[ARRAY_SIZE(idmap_pg_dir)]) {
> + pr_warn("BUG: not creating id mapping for 0x%016llx\n", addr);
> + return;
> + }
> + __create_mapping(pgd, addr, addr, size, map_io);
> +}
I'm a bit worried because I gave some feedback in the past but it didn't
make to this patch.
What I said on 22nd of January:
The condition above is always false since pgd_index() already ands the
index with (PTRS_PER_PGD - 1). Better check addr against something like
(PTRS_PER_PGD * PGDIR_SIZE) (for clarity, you could do other shifts,
doesn't really matter).
--
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] arm64: add EFI runtime services
On Fri, Jan 10, 2014 at 10:29:10PM +, Mark Salter wrote: > diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h > new file mode 100644 > index 000..a835b2c > --- /dev/null > +++ b/arch/arm64/include/asm/efi.h > @@ -0,0 +1,12 @@ > +#ifndef _ASM_ARM64_EFI_H > +#define _ASM_ARM64_EFI_H __ASM_EFI_H please for consistency with the other files. > diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c > new file mode 100644 > index 000..a42dc38 > --- /dev/null > +++ b/arch/arm64/kernel/efi.c > @@ -0,0 +1,353 @@ > +/* > + * Extensible Firmware Interface > + * > + * Based on Extensible Firmware Interface Specification version 2.4 > + * > + * Copyright (C) 2013 Linaro Ltd. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > + > +struct efi_memory_map memmap; Is this variable used outside this file or from common code? It has a too generic name, may clash with other things. > +static unsigned long arm_efi_facility; You could drop the "arm_" prefix here, that's just local to this file. > diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c > index 5516f54..7a45095 100644 > --- a/arch/arm64/kernel/setup.c > +++ b/arch/arm64/kernel/setup.c > @@ -41,6 +41,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -55,6 +56,7 @@ > #include > #include > #include > +#include > > unsigned int processor_id; > EXPORT_SYMBOL(processor_id); > @@ -229,9 +231,13 @@ void __init setup_arch(char **cmdline_p) > > arm64_memblock_init(); > > + efi_init(); > paging_init(); > request_standard_resources(); > > + if (efi_enabled(EFI_BOOT)) > + efi_enter_virtual_mode(); The common start_kernel() function already has this call: #ifdef CONFIG_X86 if (efi_enabled(EFI_RUNTIME_SERVICES)) efi_enter_virtual_mode(); #endif Could we not use it for arm64 as well? Ideally with an empty efi_enter_virtual_mode() in include/linux/efi.h if !CONFIG_EFI but I noticed that ia64 does a separate call and they don't seem to have efi_enabled(), so probably not feasible for them. -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/6] arm64: add EFI stub
On Fri, Jan 10, 2014 at 10:29:08PM +, Mark Salter wrote: > +ENTRY(efi_stub_entry) > + stp x29, x30, [sp, #-32]! > + > + /* > +* Call efi_entry to do the real work. > +* x0 and x1 are already set up by firmware. Current runtime > +* address of image is calculated and passed via *image_addr. > +* > +* unsigned long efi_entry(void *handle, > +* efi_system_table_t *sys_table, > +* unsigned long *image_addr) ; > +*/ > + adrpx8, _text > +addx8, x8, #:lo12:_text Possibly some minor whitespace corruption (or it's our email server). -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/6] arm64: Add function to create identity mappings
On Fri, Jan 10, 2014 at 10:29:06PM +, Mark Salter wrote:
> +void __init create_id_mapping(phys_addr_t addr, phys_addr_t size)
> +{
> + pgd_t *pgd = &idmap_pg_dir[pgd_index(addr)];
> +
> + if (pgd >= &idmap_pg_dir[ARRAY_SIZE(idmap_pg_dir)]) {
> + pr_warn("BUG: not creating id mapping for 0x%016llx\n", addr);
> + return;
> + }
The condition above is always false since pgd_index() already ands the
index with (PTRS_PER_PGD - 1). Better check addr against something like
(PTRS_PER_PGD * PGDIR_SIZE) (for clarity, you could do other shifts,
doesn't really matter).
--
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] arm64: add EFI stub
On Fri, Dec 06, 2013 at 02:55:54PM +, Mark Salter wrote: > On Thu, 2013-12-05 at 14:18 +0000, Catalin Marinas wrote: > > On Fri, Nov 29, 2013 at 10:05:10PM +, Mark Salter wrote: > > > + add x2, sp, 16 > > > + str x8, [x2] > > > + bl efi_entry > > > + cmn x0, #1 > > > + b.eqefi_load_fail > > > + > > > + /* > > > +* efi_entry() will have relocated the kernel image if necessary > > > +* and we return here with device tree address in x0 and the > > > kernel > > > +* entry point stored at *image_addr. Save those values in > > > registers > > > +* which are preserved by __flush_dcache_all. > > > +*/ > > > + ldr x1, [sp, #16] > > > + mov x20, x0 > > > + mov x21, x1 > > > + > > > + bl __flush_dcache_all > > > > Regarding __flush_dcache_all, I plan to remove it for all cases apart > > from power management with the MMU disabled. With MMU enabled, there is > > no guarantee that this function does the right thing. It's even worse in > > the guest context. > > According to booting.txt, the dcache needs to be invalidated. Is there > something existing I can use or do I need to write it? The function will stay for a few cases where needed. But here the D-cache and MMU are still on at this point and there is a slight chance of speculative loads after the flush (though only clean lines). You could move this after he MMU disabling (but still keep the I-cache on for performance). -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] arm64: add EFI runtime services
On Thu, Dec 05, 2013 at 03:52:41PM +, Mark Salter wrote: > On Thu, 2013-12-05 at 15:25 +0000, Catalin Marinas wrote: > > I lost track of the early_ioremap status for arm/arm64? Was there more > > progress since October (I think)? > > See the two patch series: > > https://lkml.org/lkml/2013/11/25/474 > > and > > https://lkml.org/lkml/2013/11/27/621 > > The latter early_ioremap depends on the first although there are no > arm64 bits in the first. All of the arm64 early_ioremap stuff is in > the latter. Thanks. I was even cc'ed but too many emails in my inbox ;). I'll have a look. -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] arm64: add EFI stub
On Thu, Dec 05, 2013 at 02:43:23PM +, Mark Salter wrote: > On Thu, 2013-12-05 at 14:18 +0000, Catalin Marinas wrote: > > On Fri, Nov 29, 2013 at 10:05:10PM +, Mark Salter wrote: > > > This patch adds PE/COFF header fields to the start of the Image > > > so that it appears as an EFI application to EFI firmware. An EFI > > > stub is included to allow direct booting of the kernel Image. Due > > > to EFI firmware limitations, only little endian kernels with 4K > > > page sizes are supported at this time. > > > > I don't fully understand the EFI firmware limitations but for big endian > > we could have the EFI_STUB wrapper in little endian and get the kernel > > to switch to big endian once booted. The image header should always be > > little endian. > > That would be fun. :) You'd also have to switch back and forth to make > EFI runtime services calls. OK, we'll have to live with this restriction. > > And I have to dig further into the 4K limitation (or you could give a > > quick summary ;)). > > Just that the current UEFI spec mandates 4K pages for UEFI. So if UEFI > maps two 4k pages with different attributes and those two pages are > within the same 64k kernel page, there'd be a problem. I'd be better if > UEFI used 64k pages, then the kernel could easily use 4k or 64k. For server space, we may see some people asking for 64K pages. But I don't know whether there are any UEFI plans here. Thanks. -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] arm64: add EFI runtime services
On Fri, Nov 29, 2013 at 10:05:12PM +, Mark Salter wrote:
> diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
> new file mode 100644
> index 000..7384048
> --- /dev/null
> +++ b/arch/arm64/include/asm/efi.h
> @@ -0,0 +1,18 @@
> +#ifndef _ASM_ARM64_EFI_H
> +#define _ASM_ARM64_EFI_H
> +
> +#include
> +
> +#ifdef CONFIG_EFI
> +extern void efi_init(void);
> +#else
> +#define efi_init()
> +#endif
> +
> +#define efi_remap(cookie, size) __ioremap((cookie), (size), PAGE_KERNEL_EXEC)
> +#define efi_ioremap(cookie, size) __ioremap((cookie), (size), \
> + __pgprot(PROT_DEVICE_nGnRE))
> +#define efi_unmap(cookie) __iounmap((cookie))
> +#define efi_iounmap(cookie) __iounmap((cookie))
I prefer to use the ioremap_*() functions rather than the lower-level
__ioremap(). The ioremap_cache() should give us executable memory.
Looking at the code, I think we have a bug with ioremap_cache() using
MT_NORMAL since it doesn't have the shareability bit (Device memory does
not require this on AArch64). PROT_DEFAULT should change to
pgprot_default | PTE_DIRTY.
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> new file mode 100644
> index 000..1bad8a7
> --- /dev/null
> +++ b/arch/arm64/kernel/efi.c
> @@ -0,0 +1,507 @@
> +/*
> + * Extensible Firmware Interface
> + *
> + * Based on Extensible Firmware Interface Specification version 2.3.1
> + *
> + * Copyright (C) 2013 Linaro Ltd.
> + *
> + * Adapted for arm64 from arch/arm/kernel/efi.c code
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +
> +#define efi_early_remap(a, b) \
> + ((__force void *)early_ioremap((a), (b)))
> +#define efi_early_unmap(a, b) \
> + early_iounmap((void __iomem *)(a), (b))
I lost track of the early_ioremap status for arm/arm64? Was there more
progress since October (I think)?
> +static int __init fdt_find_efi_params(unsigned long node, const char *uname,
> + int depth, void *data)
> +{
> + unsigned long len, size;
> + __be32 *prop;
> +
> + if (depth != 1 ||
> + (strcmp(uname, "chosen") != 0 && strcmp(uname, "chosen@0") != 0))
> + return 0;
> +
> + pr_info("Getting EFI parameters from FDT.\n");
> +
> + prop = of_get_flat_dt_prop(node, "linux,uefi-system-table", &len);
> + if (!prop) {
> + pr_err("No EFI system table in FDT\n");
> + return 0;
> + }
> + efi_system_table = of_read_ulong(prop, len/4);
> +
> + prop = of_get_flat_dt_prop(node, "linux,uefi-mmap-start", &len);
> + if (!prop) {
> + pr_err("No EFI memmap in FDT\n");
> + return 0;
> + }
> + memmap.phys_map = (void *)of_read_ulong(prop, len/4);
> +
> + prop = of_get_flat_dt_prop(node, "linux,uefi-mmap-size", &len);
> + if (!prop) {
> + pr_err("No EFI memmap size in FDT\n");
> + return 0;
> + }
> + size = of_read_ulong(prop, len/4);
> + memmap.map_end = memmap.phys_map + size;
> +
> + /* reserve memmap */
> + memblock_reserve((u64)memmap.phys_map, size);
> +
> + prop = of_get_flat_dt_prop(node, "linux,uefi-mmap-desc-size", &len);
> + if (!prop) {
> + pr_err("No EFI memmap descriptor size in FDT\n");
> + return 0;
> + }
> + memmap.desc_size = of_read_ulong(prop, len/4);
> +
> + memmap.nr_map = size / memmap.desc_size;
> +
> + prop = of_get_flat_dt_prop(node, "linux,uefi-mmap-desc-ver", &len);
> + if (!prop) {
> + pr_err("No EFI memmap descriptor version in FDT\n");
> + return 0;
> + }
> + memmap.desc_version = of_read_ulong(prop, len/4);
> +
> + if (uefi_debug) {
> + pr_info(" EFI system table @ %p\n", (void
> *)efi_system_table);
> + pr_info(" EFI mmap @ %p-%p\n", memmap.phys_map,
> + memmap.map_end);
> + pr_info(" EFI mmap descriptor size = 0x%lx\n",
> + memmap.desc_size);
> + pr_info(" EFI mmap descriptor version = 0x%lx\n",
> + memmap.desc_version);
> + }
> +
> + return 1;
> +}
Are these checks generic to other architectures? We may do with some
helpers to avoid duplication.
> +
> +#define PGD_END (&idmap_pg_dir[sizeof(idmap_pg_dir)/sizeof(pgd_t)])
Just &idmap_pg_dir[PTRS_PER_PGD] would do (or idmap_pg_dir +
ARRAY_SIZE(idmap_pg_dir)).
> +#ifndef CONFIG_SMP
> +#define PMD_FLAGS (PMD_ATTRINDX(MT_NORMAL) | PMD_TYPE_SECT | PMD_SECT_AF)
> +#else
> +#define PMD_FLAGS (PMD_ATTRINDX(MT_NORMAL) | PMD_TYPE_SECT | PMD_SECT_AF | \
> + PMD_SECT_S)
> +#endif
> +
> +static void __init create_idmap(unsigned long addr, unsigned long len)
I would change this to use
Re: [PATCH 1/3] arm64: add EFI stub
Hi Mark, On Fri, Nov 29, 2013 at 10:05:10PM +, Mark Salter wrote: > This patch adds PE/COFF header fields to the start of the Image > so that it appears as an EFI application to EFI firmware. An EFI > stub is included to allow direct booting of the kernel Image. Due > to EFI firmware limitations, only little endian kernels with 4K > page sizes are supported at this time. I don't fully understand the EFI firmware limitations but for big endian we could have the EFI_STUB wrapper in little endian and get the kernel to switch to big endian once booted. The image header should always be little endian. And I have to dig further into the 4K limitation (or you could give a quick summary ;)). > diff --git a/arch/arm64/kernel/efi-entry.S b/arch/arm64/kernel/efi-entry.S > new file mode 100644 > index 000..5f6d179 > --- /dev/null > +++ b/arch/arm64/kernel/efi-entry.S > @@ -0,0 +1,81 @@ > +/* > + * EFI entry point. > + * > + * Copyright (C) 2013 Red Hat, Inc. > + * Author: Mark Salter > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + */ > +#include > +#include > + > +#include > + > +#define EFI_LOAD_ERROR 0x8001 It's defined already but I see why you can't include efi.h here. Maybe a comment. > + > + __INIT > + > + /* > +* We arrive here from the EFI boot manager with: > +* > +** MMU on with identity-mapped RAM. > +** Icache and Dcache on > +* > +* We will most likely be running from some place other than where > +* we want to be. The kernel image wants to be placed at TEXT_OFFSET > +* from start of RAM. > +*/ > +ENTRY(efi_stub_entry) > + stp x29, x30, [sp, #-32]! > + > + /* > +* Call efi_entry to do the real work. > +* x0 and x1 are already set up by firmware. Current runtime > +* address of image is calculated and passed via *image_addr. > +* > +* unsigned long efi_entry(void *handle, > +* efi_system_table_t *sys_table, > +* unsigned long *image_addr) ; > +*/ > + adrpx8, _text > +addx8, x8, #:lo12:_text Minor: some wrong whitespace (but I don't trust our incoming mail server either, it corrupts patches usually). > + add x2, sp, 16 > + str x8, [x2] > + bl efi_entry > + cmn x0, #1 > + b.eqefi_load_fail > + > + /* > +* efi_entry() will have relocated the kernel image if necessary > +* and we return here with device tree address in x0 and the kernel > +* entry point stored at *image_addr. Save those values in registers > +* which are preserved by __flush_dcache_all. > +*/ > + ldr x1, [sp, #16] > + mov x20, x0 > + mov x21, x1 > + > + bl __flush_dcache_all Regarding __flush_dcache_all, I plan to remove it for all cases apart from power management with the MMU disabled. With MMU enabled, there is no guarantee that this function does the right thing. It's even worse in the guest context. > + /* Turn off Dcache and MMU */ > + mrs x0, sctlr_el1 > + bic x0, x0, #1 << 0 // clear SCTLR.M > + bic x0, x0, #1 << 2 // clear SCTLR.C > + msr sctlr_el1, x0 > + isb I assume an EFI app is running with the MMU enabled (and UP only). Do we always run it in EL1? What about EL2 mode (needed by KVM and Xen)? > + > + /* Jump to real entry point */ > + mov x0, x20 > + mov x1, xzr > + mov x2, xzr > + mov x3, xzr > + br x21 > + > +efi_load_fail: > + mov x0, EFI_LOAD_ERROR Needs #EFI_LOAD_ERROR (strange that gas doesn't complain). > + ldp x29, x30, [sp], #32 > + ret > + > +ENDPROC(efi_stub_entry) > diff --git a/arch/arm64/kernel/efi-stub.c b/arch/arm64/kernel/efi-stub.c > new file mode 100644 > index 000..f000b04 > --- /dev/null > +++ b/arch/arm64/kernel/efi-stub.c > @@ -0,0 +1,280 @@ > +/* > + * linux/arch/arm/boot/compressed/efi-stub.c > + * > + * Copyright (C) 2013 Linaro Ltd; > + * > + * This file implements the EFI boot stub for the arm64 kernel. > + * Adapted from ARM version by Mark Salter > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + */ > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +/* error code which can't be mistaken for valid address */ > +#define EFI_ERROR (~0UL) > + > +/* > + * EFI function call wrappers. These are not required for arm64, but wrappers > + * are required for X86 to convert between
