Op 03-09-14 om 08:06 schreef Ard Biesheuvel:
> On 2 September 2014 21:29, Matt Fleming <matt@xxxxxxxxxxxxxxxxx> wrote:
>> On Tue, 02 Sep, at 05:25:58PM, Maarten Lankhorst wrote:
>>> Hey,
>>>
>>> My macbook pro 8.2 fails to do a efi stub boot with these patches.
>>>
>>> Commit f23cf8bd5c1f49 "efi/x86: efistub: Move shared dependencies to <asm/efi.h>"
>>> causes the first break, but this can be averted by changing
>>>
>>> struct efi_config *efi_early;
>>>
>>> to
>>>
>>> struct efi_config *efi_early __attribute__((visibility("hidden")));
>> Weird. That sounds like a bug in the Apple EFI PE loader. Does any other
>> visibility result in a working kernel?
>>
>>> I also need to revert commit f4f75ad5741fe "efi: efistub: Convert into static library"
>>> to get boot working.
>> I'll take a look at the symbol changes between these commits and try and
>> guess what's going on.
>>
>>> I'm not an early boot expert, so I have no idea what's going on here.
>>> Only console output I see when the boot fails is "setup_efi_pci() failed!" after
>>> the commit that adds this message.
>> Yeah, that should be unrelated.
>>
>> Thanks for the report.
>>
> If x86 is anything like ARM in this respect, this is likely caused by
> the way PIC references to globals are emitted.
> When using default visibility, a reference to a global is emitted by a
> reference to the GOT, and an offset into the GOT, and the two are
> added and dereferenced at runtime. For this to work, the GOT needs to
> contain the correct absolute address for the global in question. This,
> in turn, relies on absolute relocations to be resolved at load time,
> which we don't do with the EFI stub. Could it be that the link address
> and load address are usually the same on x86 EFI but not on the Mac?
>
> With hidden visibility, the PIC reference is emitted using a relative
> relocation, to which the value of the program counter is added at
> runtime, and no GOT is involved, and all relocations can be resolved
> at build time.
>
> I think it makes sense to add a #pragma GCC visibility push(hidden) to
> efistub.h (if pragmas are in fashion these days), making sure that no
> global references (not even externs) will generate GOT entries.
>
Something like that probably.
Following patch FAILS:
diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index f277184e2ac1..f9738d92c9ce 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -15,6 +15,8 @@
#undef memcpy /* Use memcpy from misc.c */
+#pragma GCC visibility push(hidden)
+
#include "eboot.h"
static efi_system_table_t *sys_table;
diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index 32d5cca30f49..7ef10f36cc1b 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -13,6 +13,8 @@
#include <linux/efi.h>
#include <asm/efi.h>
+#pragma GCC visibility push(hidden)
+
#include "efistub.h"
#define EFI_READ_CHUNK_SIZE (1024 * 1024)
-----
But this works:
diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index 7a801a310e37..eebd13ee301b 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -30,11 +30,10 @@ VMLINUX_OBJS = $(obj)/vmlinux.lds $(obj)/head_$(BITS).o $(obj)/misc.o \
$(obj)/string.o $(obj)/cmdline.o $(obj)/early_serial_console.o \
$(obj)/piggy.o $(obj)/cpuflags.o $(obj)/aslr.o
-$(obj)/eboot.o: KBUILD_CFLAGS += -fshort-wchar -mno-red-zone
+$(obj)/eboot.o: KBUILD_CFLAGS += -fshort-wchar -mno-red-zone -fvisibility=hidden
ifeq ($(CONFIG_EFI_STUB), y)
- VMLINUX_OBJS += $(obj)/eboot.o $(obj)/efi_stub_$(BITS).o \
- $(objtree)/drivers/firmware/efi/libstub/lib.a
+ VMLINUX_OBJS += $(obj)/eboot.o $(obj)/efi_stub_$(BITS).o
endif
$(obj)/vmlinux: $(VMLINUX_OBJS) FORCE
diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index f277184e2ac1..e18a265460dd 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -17,6 +17,8 @@
#include "eboot.h"
+#include "../../../drivers/firmware/efi/libstub/efi-stub-helper.c"
+
static efi_system_table_t *sys_table;
struct efi_config *efi_early;
In case it was caused by lacking -fshort-wchar or order of building, I tried the following, also fails:
---
diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index 7a801a310e37..8f62c273badd 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -30,11 +30,10 @@ VMLINUX_OBJS = $(obj)/vmlinux.lds $(obj)/head_$(BITS).o $(obj)/misc.o \
$(obj)/string.o $(obj)/cmdline.o $(obj)/early_serial_console.o \
$(obj)/piggy.o $(obj)/cpuflags.o $(obj)/aslr.o
-$(obj)/eboot.o: KBUILD_CFLAGS += -fshort-wchar -mno-red-zone
+$(obj)/eboot.o: KBUILD_CFLAGS += -fshort-wchar -mno-red-zone -fvisibility=hidden
ifeq ($(CONFIG_EFI_STUB), y)
- VMLINUX_OBJS += $(obj)/eboot.o $(obj)/efi_stub_$(BITS).o \
- $(objtree)/drivers/firmware/efi/libstub/lib.a
+ VMLINUX_OBJS += $(objtree)/drivers/firmware/efi/libstub/efi-stub-helper.o $(obj)/eboot.o $(obj)/efi_stub_$(BITS).o
endif
$(obj)/vmlinux: $(VMLINUX_OBJS) FORCE
diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index b14bc2b9fb4d..66c5536cff4d 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -8,7 +8,8 @@ cflags-$(CONFIG_X86_32) := -march=i386
cflags-$(CONFIG_X86_64) := -mcmodel=small
cflags-$(CONFIG_X86) += -m$(BITS) -D__KERNEL__ $(LINUX_INCLUDE) -O2 \
-fPIC -fno-strict-aliasing -mno-red-zone \
- -mno-mmx -mno-sse -DDISABLE_BRANCH_PROFILING
+ -mno-mmx -mno-sse -DDISABLE_BRANCH_PROFILING \
+ -fvisibility=hidden -fshort-wchar
cflags-$(CONFIG_ARM64) := $(subst -pg,,$(KBUILD_CFLAGS))
cflags-$(CONFIG_ARM) := $(subst -pg,,$(KBUILD_CFLAGS)) \
---
I have no idea why only directly referencing the file in eboot.c works.
~Maarten
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
