* [PATCH] efi/arm64: efistub: remove local copy of linux_banner
@ 2014-06-13 11:11 Ard Biesheuvel
2014-06-18 8:50 ` Matt Fleming
2014-06-25 21:04 ` Matt Fleming
0 siblings, 2 replies; 5+ messages in thread
From: Ard Biesheuvel @ 2014-06-13 11:11 UTC (permalink / raw)
To: linux-arm-kernel
The shared efistub code for ARM and arm64 contains a local copy of linux_banner,
allowing it to be referenced from separate executables such as the ARM
decompressor. However, this introduces a dependency on generated header files,
causing unnecessary rebuilds of the stub itself and, in case of arm64, vmlinux
which contains it.
On arm64, the copy is not actually needed since we can reference the original
symbol directly, and as it turns out, there may be better ways to deal with this
for ARM as well, so let's remove it from the shared code. If it still needs to
be reintroduced for ARM later, it should live under arch/arm anyway and not in
shared code.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/kernel/efi-stub.c | 2 --
drivers/firmware/efi/fdt.c | 10 ----------
2 files changed, 12 deletions(-)
diff --git a/arch/arm64/kernel/efi-stub.c b/arch/arm64/kernel/efi-stub.c
index 60e98a639ac5..e786e6cdc400 100644
--- a/arch/arm64/kernel/efi-stub.c
+++ b/arch/arm64/kernel/efi-stub.c
@@ -12,8 +12,6 @@
#include <linux/efi.h>
#include <linux/libfdt.h>
#include <asm/sections.h>
-#include <generated/compile.h>
-#include <generated/utsrelease.h>
/*
* AArch64 requires the DTB to be 8-byte aligned in the first 512MiB from
diff --git a/drivers/firmware/efi/fdt.c b/drivers/firmware/efi/fdt.c
index 5c6a8e8a9580..3aec36d7aae9 100644
--- a/drivers/firmware/efi/fdt.c
+++ b/drivers/firmware/efi/fdt.c
@@ -23,16 +23,6 @@ static efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
u32 fdt_val32;
u64 fdt_val64;
- /*
- * Copy definition of linux_banner here. Since this code is
- * built as part of the decompressor for ARM v7, pulling
- * in version.c where linux_banner is defined for the
- * kernel brings other kernel dependencies with it.
- */
- const char linux_banner[] =
- "Linux version " UTS_RELEASE " (" LINUX_COMPILE_BY "@"
- LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION "\n";
-
/* Do some checks on provided FDT, if it exists*/
if (orig_fdt) {
if (fdt_check_header(orig_fdt)) {
--
1.8.3.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH] efi/arm64: efistub: remove local copy of linux_banner
2014-06-13 11:11 [PATCH] efi/arm64: efistub: remove local copy of linux_banner Ard Biesheuvel
@ 2014-06-18 8:50 ` Matt Fleming
2014-06-18 9:12 ` Ard Biesheuvel
2014-06-25 21:04 ` Matt Fleming
1 sibling, 1 reply; 5+ messages in thread
From: Matt Fleming @ 2014-06-18 8:50 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 13 Jun, at 01:11:51PM, Ard Biesheuvel wrote:
> The shared efistub code for ARM and arm64 contains a local copy of linux_banner,
> allowing it to be referenced from separate executables such as the ARM
> decompressor. However, this introduces a dependency on generated header files,
> causing unnecessary rebuilds of the stub itself and, in case of arm64, vmlinux
> which contains it.
>
> On arm64, the copy is not actually needed since we can reference the original
> symbol directly, and as it turns out, there may be better ways to deal with this
> for ARM as well, so let's remove it from the shared code. If it still needs to
> be reintroduced for ARM later, it should live under arch/arm anyway and not in
> shared code.
I remember making some similar arguments when this patch was first
introduced. From looking at my notes, the patch rationale was based on
some DT binding discussion where people wanted an explicit way to
identify the kernel they were booting and everyone agreed upon this
string - I don't know which discussion, I don't have that data.
I'm more than happy to delete this from the shared code, I never wanted
it to go in in the first place. But could someone please give me some
details as to why this change is safe and isn't going to break
ARM/ARM64? How does this affect DT bindings?
--
Matt Fleming, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] efi/arm64: efistub: remove local copy of linux_banner
2014-06-18 8:50 ` Matt Fleming
@ 2014-06-18 9:12 ` Ard Biesheuvel
2014-06-18 9:25 ` Matt Fleming
0 siblings, 1 reply; 5+ messages in thread
From: Ard Biesheuvel @ 2014-06-18 9:12 UTC (permalink / raw)
To: linux-arm-kernel
On 18 June 2014 10:50, Matt Fleming <matt@console-pimps.org> wrote:
> On Fri, 13 Jun, at 01:11:51PM, Ard Biesheuvel wrote:
>> The shared efistub code for ARM and arm64 contains a local copy of linux_banner,
>> allowing it to be referenced from separate executables such as the ARM
>> decompressor. However, this introduces a dependency on generated header files,
>> causing unnecessary rebuilds of the stub itself and, in case of arm64, vmlinux
>> which contains it.
>>
>> On arm64, the copy is not actually needed since we can reference the original
>> symbol directly, and as it turns out, there may be better ways to deal with this
>> for ARM as well, so let's remove it from the shared code. If it still needs to
>> be reintroduced for ARM later, it should live under arch/arm anyway and not in
>> shared code.
>
> I remember making some similar arguments when this patch was first
> introduced. From looking at my notes, the patch rationale was based on
> some DT binding discussion where people wanted an explicit way to
> identify the kernel they were booting and everyone agreed upon this
> string - I don't know which discussion, I don't have that data.
>
> I'm more than happy to delete this from the shared code, I never wanted
> it to go in in the first place. But could someone please give me some
> details as to why this change is safe and isn't going to break
> ARM/ARM64? How does this affect DT bindings?
>
This just removes the duplicate definition of linux_banner, *not* the
reference to it a couple of lines down. IOW, the DT bindings and
everything behind it are not affected at all by this patch.
The reason it works fine for arm64 is that its stub is part of the
kernel proper, so the reference will bind to the global definition
instead.
What will be affected is 32-bit ARM, as its stub is not part of the
kernel proper. In this case, we will propose an alternative [and
better] way of allowing the stub to reference linux_banner, i.e.,
without having to redefine it at the C level. We will take this into
account when we propose the next round of efistub patches for ARM.
Regards,
Ard.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] efi/arm64: efistub: remove local copy of linux_banner
2014-06-18 9:12 ` Ard Biesheuvel
@ 2014-06-18 9:25 ` Matt Fleming
0 siblings, 0 replies; 5+ messages in thread
From: Matt Fleming @ 2014-06-18 9:25 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, 18 Jun, at 11:12:28AM, Ard Biesheuvel wrote:
>
> This just removes the duplicate definition of linux_banner, *not* the
> reference to it a couple of lines down. IOW, the DT bindings and
> everything behind it are not affected at all by this patch.
> The reason it works fine for arm64 is that its stub is part of the
> kernel proper, so the reference will bind to the global definition
> instead.
Aha, thanks.
> What will be affected is 32-bit ARM, as its stub is not part of the
> kernel proper. In this case, we will propose an alternative [and
> better] way of allowing the stub to reference linux_banner, i.e.,
> without having to redefine it at the C level. We will take this into
> account when we propose the next round of efistub patches for ARM.
No complaints from me.
--
Matt Fleming, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] efi/arm64: efistub: remove local copy of linux_banner
2014-06-13 11:11 [PATCH] efi/arm64: efistub: remove local copy of linux_banner Ard Biesheuvel
2014-06-18 8:50 ` Matt Fleming
@ 2014-06-25 21:04 ` Matt Fleming
1 sibling, 0 replies; 5+ messages in thread
From: Matt Fleming @ 2014-06-25 21:04 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 13 Jun, at 01:11:51PM, Ard Biesheuvel wrote:
> The shared efistub code for ARM and arm64 contains a local copy of linux_banner,
> allowing it to be referenced from separate executables such as the ARM
> decompressor. However, this introduces a dependency on generated header files,
> causing unnecessary rebuilds of the stub itself and, in case of arm64, vmlinux
> which contains it.
>
> On arm64, the copy is not actually needed since we can reference the original
> symbol directly, and as it turns out, there may be better ways to deal with this
> for ARM as well, so let's remove it from the shared code. If it still needs to
> be reintroduced for ARM later, it should live under arch/arm anyway and not in
> shared code.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> arch/arm64/kernel/efi-stub.c | 2 --
> drivers/firmware/efi/fdt.c | 10 ----------
> 2 files changed, 12 deletions(-)
Applied, thanks.
--
Matt Fleming, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-06-25 21:04 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-06-13 11:11 [PATCH] efi/arm64: efistub: remove local copy of linux_banner Ard Biesheuvel
2014-06-18 8:50 ` Matt Fleming
2014-06-18 9:12 ` Ard Biesheuvel
2014-06-18 9:25 ` Matt Fleming
2014-06-25 21:04 ` Matt Fleming
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).