* [PATCH] efi_loader: Handle GD_FLG_SKIP_RELOC
@ 2021-10-10 21:48 marek.vasut
2021-10-11 11:36 ` Heinrich Schuchardt
0 siblings, 1 reply; 5+ messages in thread
From: marek.vasut @ 2021-10-10 21:48 UTC (permalink / raw)
To: u-boot; +Cc: Marek Vasut, Heinrich Schuchardt, Simon Glass, Tom Rini
From: Marek Vasut <marek.vasut+renesas@gmail.com>
In case U-Boot starts with GD_FLG_SKIP_RELOC, the efi loader
relocation code breaks down because it assumes gd->relocaddr
points to relocated U-Boot code, which is not the case. Add
special case for handling GD_FLG_SKIP_RELOC, which uses the
__image_copy_start instead of gd->relocaddr for efi loader
code relocation source address.
Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Simon Glass <sjg@chromium.org>
Cc: Tom Rini <trini@konsulko.com>
---
common/board_r.c | 7 ++++++-
lib/efi_loader/efi_runtime.c | 19 ++++++++++++++++---
2 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/common/board_r.c b/common/board_r.c
index 31a59c585a..3e95123f95 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -178,7 +178,12 @@ static int initr_reloc_global_data(void)
*/
efi_save_gd();
- efi_runtime_relocate(gd->relocaddr, NULL);
+#ifdef CONFIG_ARM
+ if (gd->flags & GD_FLG_SKIP_RELOC)
+ efi_runtime_relocate((ulong)__image_copy_start, NULL);
+ else
+#endif
+ efi_runtime_relocate(gd->relocaddr, NULL);
#endif
return 0;
diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
index 93a695fc27..876ca09106 100644
--- a/lib/efi_loader/efi_runtime.c
+++ b/lib/efi_loader/efi_runtime.c
@@ -17,6 +17,8 @@
#include <asm/global_data.h>
#include <u-boot/crc.h>
+#include <asm/sections.h>
+
/* For manual relocation support */
DECLARE_GLOBAL_DATA_PTR;
@@ -629,13 +631,23 @@ out:
return ret;
}
+static unsigned long efi_get_reloc_start(void)
+{
+#ifdef CONFIG_ARM
+ if (gd->flags & GD_FLG_SKIP_RELOC)
+ return (unsigned long)__image_copy_start;
+ else
+#endif
+ return gd->relocaddr;
+}
+
static __efi_runtime void efi_relocate_runtime_table(ulong offset)
{
ulong patchoff;
void **pos;
/* Relocate the runtime services pointers */
- patchoff = offset - gd->relocaddr;
+ patchoff = offset - efi_get_reloc_start();
for (pos = (void **)&efi_runtime_services.get_time;
pos <= (void **)&efi_runtime_services.query_variable_info; ++pos) {
if (*pos)
@@ -681,7 +693,7 @@ void efi_runtime_relocate(ulong offset, struct efi_mem_desc *map)
ulong *p;
ulong newaddr;
- p = (void*)((ulong)rel->offset - base) + gd->relocaddr;
+ p = (void*)((ulong)rel->offset - base) + efi_get_reloc_start();
/*
* The runtime services table is updated in
@@ -852,7 +864,8 @@ static efi_status_t EFIAPI efi_set_virtual_address_map(
map = (void*)virtmap + (descriptor_size * i);
if (map->type == EFI_RUNTIME_SERVICES_CODE) {
ulong new_offset = map->virtual_start -
- map->physical_start + gd->relocaddr;
+ map->physical_start +
+ efi_get_reloc_start();
efi_relocate_runtime_table(new_offset);
efi_runtime_relocate(new_offset, map);
--
2.33.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] efi_loader: Handle GD_FLG_SKIP_RELOC
2021-10-10 21:48 [PATCH] efi_loader: Handle GD_FLG_SKIP_RELOC marek.vasut
@ 2021-10-11 11:36 ` Heinrich Schuchardt
2021-10-23 23:03 ` Marek Vasut
0 siblings, 1 reply; 5+ messages in thread
From: Heinrich Schuchardt @ 2021-10-11 11:36 UTC (permalink / raw)
To: marek.vasut; +Cc: Marek Vasut, Simon Glass, Tom Rini, u-boot
On 10/10/21 23:48, marek.vasut@gmail.com wrote:
> From: Marek Vasut <marek.vasut+renesas@gmail.com>
>
> In case U-Boot starts with GD_FLG_SKIP_RELOC, the efi loader
> relocation code breaks down because it assumes gd->relocaddr
> points to relocated U-Boot code, which is not the case. Add
> special case for handling GD_FLG_SKIP_RELOC, which uses the
> __image_copy_start instead of gd->relocaddr for efi loader
> code relocation source address.
GD_FLG_SKIP_RELOC is only to be used by the x86 EFI app which is
incompatible with CONFIG_EFI_LOADER=y.
lib/efi/efi_app.c:131: board_init_f(GD_FLG_SKIP_RELOC);
Why do we need this patch?
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Tom Rini <trini@konsulko.com>
> ---
> common/board_r.c | 7 ++++++-
> lib/efi_loader/efi_runtime.c | 19 ++++++++++++++++---
> 2 files changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/common/board_r.c b/common/board_r.c
> index 31a59c585a..3e95123f95 100644
> --- a/common/board_r.c
> +++ b/common/board_r.c
> @@ -178,7 +178,12 @@ static int initr_reloc_global_data(void)
> */
> efi_save_gd();
>
> - efi_runtime_relocate(gd->relocaddr, NULL);
> +#ifdef CONFIG_ARM
Why should this be ARM specific?
Best regards
Heinrich
> + if (gd->flags & GD_FLG_SKIP_RELOC)
> + efi_runtime_relocate((ulong)__image_copy_start, NULL);
> + else
> +#endif
> + efi_runtime_relocate(gd->relocaddr, NULL);
> #endif
>
> return 0;
> diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
> index 93a695fc27..876ca09106 100644
> --- a/lib/efi_loader/efi_runtime.c
> +++ b/lib/efi_loader/efi_runtime.c
> @@ -17,6 +17,8 @@
> #include <asm/global_data.h>
> #include <u-boot/crc.h>
>
> +#include <asm/sections.h>
> +
> /* For manual relocation support */
> DECLARE_GLOBAL_DATA_PTR;
>
> @@ -629,13 +631,23 @@ out:
> return ret;
> }
>
> +static unsigned long efi_get_reloc_start(void)
> +{
> +#ifdef CONFIG_ARM
> + if (gd->flags & GD_FLG_SKIP_RELOC)
> + return (unsigned long)__image_copy_start;
> + else
> +#endif
> + return gd->relocaddr;
> +}
> +
> static __efi_runtime void efi_relocate_runtime_table(ulong offset)
> {
> ulong patchoff;
> void **pos;
>
> /* Relocate the runtime services pointers */
> - patchoff = offset - gd->relocaddr;
> + patchoff = offset - efi_get_reloc_start();
> for (pos = (void **)&efi_runtime_services.get_time;
> pos <= (void **)&efi_runtime_services.query_variable_info; ++pos) {
> if (*pos)
> @@ -681,7 +693,7 @@ void efi_runtime_relocate(ulong offset, struct efi_mem_desc *map)
> ulong *p;
> ulong newaddr;
>
> - p = (void*)((ulong)rel->offset - base) + gd->relocaddr;
> + p = (void*)((ulong)rel->offset - base) + efi_get_reloc_start();
>
> /*
> * The runtime services table is updated in
> @@ -852,7 +864,8 @@ static efi_status_t EFIAPI efi_set_virtual_address_map(
> map = (void*)virtmap + (descriptor_size * i);
> if (map->type == EFI_RUNTIME_SERVICES_CODE) {
> ulong new_offset = map->virtual_start -
> - map->physical_start + gd->relocaddr;
> + map->physical_start +
> + efi_get_reloc_start();
>
> efi_relocate_runtime_table(new_offset);
> efi_runtime_relocate(new_offset, map);
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] efi_loader: Handle GD_FLG_SKIP_RELOC
2021-10-11 11:36 ` Heinrich Schuchardt
@ 2021-10-23 23:03 ` Marek Vasut
2021-10-25 18:46 ` Heinrich Schuchardt
0 siblings, 1 reply; 5+ messages in thread
From: Marek Vasut @ 2021-10-23 23:03 UTC (permalink / raw)
To: Heinrich Schuchardt; +Cc: Simon Glass, Tom Rini, u-boot
On 10/11/21 1:36 PM, Heinrich Schuchardt wrote:
Hi,
>> In case U-Boot starts with GD_FLG_SKIP_RELOC, the efi loader
>> relocation code breaks down because it assumes gd->relocaddr
>> points to relocated U-Boot code, which is not the case. Add
>> special case for handling GD_FLG_SKIP_RELOC, which uses the
>> __image_copy_start instead of gd->relocaddr for efi loader
>> code relocation source address.
>
> GD_FLG_SKIP_RELOC is only to be used by the x86 EFI app which is
> incompatible with CONFIG_EFI_LOADER=y.
> lib/efi/efi_app.c:131: board_init_f(GD_FLG_SKIP_RELOC);
>
> Why do we need this patch?
GD_FLG_SKIP_RELOC is set when U-Boot doesn't perform relocation, it has
nothing to do with EFI. That case is currently broken, try setting
GD_FLG_SKIP_RELOC early on in U-Boot (somewhere early, e.g. in
mach_cpu_init()) and the system crashes on board_init_r when
initializing EFI.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] efi_loader: Handle GD_FLG_SKIP_RELOC
2021-10-23 23:03 ` Marek Vasut
@ 2021-10-25 18:46 ` Heinrich Schuchardt
2021-10-25 18:52 ` Marek Vasut
0 siblings, 1 reply; 5+ messages in thread
From: Heinrich Schuchardt @ 2021-10-25 18:46 UTC (permalink / raw)
To: Marek Vasut; +Cc: Simon Glass, Tom Rini, u-boot
On 10/24/21 01:03, Marek Vasut wrote:
> On 10/11/21 1:36 PM, Heinrich Schuchardt wrote:
> Hi,
>
>>> In case U-Boot starts with GD_FLG_SKIP_RELOC, the efi loader
>>> relocation code breaks down because it assumes gd->relocaddr
>>> points to relocated U-Boot code, which is not the case. Add
>>> special case for handling GD_FLG_SKIP_RELOC, which uses the
>>> __image_copy_start instead of gd->relocaddr for efi loader
>>> code relocation source address.
>>
>> GD_FLG_SKIP_RELOC is only to be used by the x86 EFI app which is
>> incompatible with CONFIG_EFI_LOADER=y.
>> lib/efi/efi_app.c:131: board_init_f(GD_FLG_SKIP_RELOC);
>>
>> Why do we need this patch?
>
> GD_FLG_SKIP_RELOC is set when U-Boot doesn't perform relocation, it has
> nothing to do with EFI. That case is currently broken, try setting
> GD_FLG_SKIP_RELOC early on in U-Boot (somewhere early, e.g. in
> mach_cpu_init()) and the system crashes on board_init_r when
> initializing EFI.
You missed the second question:
Why should the change be ARM specific?
Best regards
Heinrich
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] efi_loader: Handle GD_FLG_SKIP_RELOC
2021-10-25 18:46 ` Heinrich Schuchardt
@ 2021-10-25 18:52 ` Marek Vasut
0 siblings, 0 replies; 5+ messages in thread
From: Marek Vasut @ 2021-10-25 18:52 UTC (permalink / raw)
To: Heinrich Schuchardt; +Cc: Simon Glass, Tom Rini, u-boot
On 10/25/21 8:46 PM, Heinrich Schuchardt wrote:
>
>
> On 10/24/21 01:03, Marek Vasut wrote:
>> On 10/11/21 1:36 PM, Heinrich Schuchardt wrote:
>> Hi,
>>
>>>> In case U-Boot starts with GD_FLG_SKIP_RELOC, the efi loader
>>>> relocation code breaks down because it assumes gd->relocaddr
>>>> points to relocated U-Boot code, which is not the case. Add
>>>> special case for handling GD_FLG_SKIP_RELOC, which uses the
>>>> __image_copy_start instead of gd->relocaddr for efi loader
>>>> code relocation source address.
>>>
>>> GD_FLG_SKIP_RELOC is only to be used by the x86 EFI app which is
>>> incompatible with CONFIG_EFI_LOADER=y.
>>> lib/efi/efi_app.c:131: board_init_f(GD_FLG_SKIP_RELOC);
>>>
>>> Why do we need this patch?
>>
>> GD_FLG_SKIP_RELOC is set when U-Boot doesn't perform relocation, it has
>> nothing to do with EFI. That case is currently broken, try setting
>> GD_FLG_SKIP_RELOC early on in U-Boot (somewhere early, e.g. in
>> mach_cpu_init()) and the system crashes on board_init_r when
>> initializing EFI.
>
> You missed the second question:
>
> Why should the change be ARM specific?
It shouldn't, do you have a more generic idea ?
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-10-25 18:52 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-10 21:48 [PATCH] efi_loader: Handle GD_FLG_SKIP_RELOC marek.vasut
2021-10-11 11:36 ` Heinrich Schuchardt
2021-10-23 23:03 ` Marek Vasut
2021-10-25 18:46 ` Heinrich Schuchardt
2021-10-25 18:52 ` Marek Vasut
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.