Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1571844 > unrolled thread
| Started by | Ard Biesheuvel <ard.biesheuvel@linaro.org> |
|---|---|
| First post | 2017-02-01 18:50 +0100 |
| Last post | 2017-02-02 02:40 +0100 |
| Articles | 8 — 4 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH] efi: fdt: avoid FDT manipulation after ExitBootServices() Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2017-02-01 18:50 +0100
Re: [PATCH] efi: fdt: avoid FDT manipulation after ExitBootServices() Ingo Molnar <mingo@kernel.org> - 2017-02-01 20:10 +0100
Re: [PATCH] efi: fdt: avoid FDT manipulation after ExitBootServices() Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2017-02-01 20:30 +0100
Re: [PATCH] efi: fdt: avoid FDT manipulation after ExitBootServices() Ingo Molnar <mingo@kernel.org> - 2017-02-01 21:20 +0100
Re: [PATCH] efi: fdt: avoid FDT manipulation after ExitBootServices() Jeffrey Hugo <jhugo@codeaurora.org> - 2017-02-01 22:10 +0100
Re: [PATCH] efi: fdt: avoid FDT manipulation after ExitBootServices() Jeffrey Hugo <jhugo@codeaurora.org> - 2017-02-02 00:00 +0100
Re: [PATCH] efi: fdt: avoid FDT manipulation after ExitBootServices() Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2017-02-02 17:20 +0100
[tip:efi/urgent] efi/fdt: Avoid FDT manipulation after ExitBootServices() tip-bot for Ard Biesheuvel <tipbot@zytor.com> - 2017-02-02 02:40 +0100
| From | Ard Biesheuvel <ard.biesheuvel@linaro.org> |
|---|---|
| Date | 2017-02-01 18:50 +0100 |
| Subject | [PATCH] efi: fdt: avoid FDT manipulation after ExitBootServices() |
| Message-ID | <t67cC-tm-27@gated-at.bofh.it> |
Some AArch64 UEFI implementations disable the MMU in ExitBootServices(),
after which unaligned accesses to RAM are no longer supported.
Commit abfb7b686a3e ("efi/libstub/arm*: Pass latest memory map to the
kernel") fixed an issue in the memory map handling of the stub FDT code,
but inadvertently created an issue with such firmwares, by moving some
of the FDT manipulation to after the invocation of ExitBootServices().
Given that the stub's libfdt implementation uses the ordinary, accelerated
string functions, which rely on hardware handling of unaligned accesses,
manipulating the FDT with the MMU off may result in alignment faults.
So fix the situation by moving the update_fdt_memmap() call into the
callback function invoked by efi_exit_boot_services() right before it
calls the ExitBootServices() UEFI service (which is arguably a better
place for it anyway)
Note that disabling the MMU in ExitBootServices() is not compliant with
the UEFI spec, and carries great risk due to the fact that switching from
cached to uncached memory accesses halfway through compiler generated code
(i.e., involving a stack) can never be done in a way that is architecturally
safe.
Cc: <stable@vger.kernel.org>
Fixes: abfb7b686a3e ("efi/libstub/arm*: Pass latest memory map to the kernel")
Tested-by: Riku Voipio <riku.voipio@linaro.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
drivers/firmware/efi/libstub/fdt.c | 14 +++-----------
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
index 921dfa047202..260c4b4b492e 100644
--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -187,6 +187,7 @@ static efi_status_t update_fdt_memmap(void *fdt, struct efi_boot_memmap *map)
struct exit_boot_struct {
efi_memory_desc_t *runtime_map;
int *runtime_entry_count;
+ void *new_fdt_addr;
};
static efi_status_t exit_boot_func(efi_system_table_t *sys_table_arg,
@@ -202,7 +203,7 @@ static efi_status_t exit_boot_func(efi_system_table_t *sys_table_arg,
efi_get_virtmap(*map->map, *map->map_size, *map->desc_size,
p->runtime_map, p->runtime_entry_count);
- return EFI_SUCCESS;
+ return update_fdt_memmap(p->new_fdt_addr, map);
}
/*
@@ -300,22 +301,13 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
priv.runtime_map = runtime_map;
priv.runtime_entry_count = &runtime_entry_count;
+ priv.new_fdt_addr = (void *)*new_fdt_addr;
status = efi_exit_boot_services(sys_table, handle, &map, &priv,
exit_boot_func);
if (status == EFI_SUCCESS) {
efi_set_virtual_address_map_t *svam;
- status = update_fdt_memmap((void *)*new_fdt_addr, &map);
- if (status != EFI_SUCCESS) {
- /*
- * The kernel won't get far without the memory map, but
- * may still be able to print something meaningful so
- * return success here.
- */
- return EFI_SUCCESS;
- }
-
/* Install the new virtual address map */
svam = sys_table->runtime->set_virtual_address_map;
status = svam(runtime_entry_count * desc_size, desc_size,
--
2.7.4
[toc] | [next] | [standalone]
| From | Ingo Molnar <mingo@kernel.org> |
|---|---|
| Date | 2017-02-01 20:10 +0100 |
| Message-ID | <t68s2-1q4-17@gated-at.bofh.it> |
| In reply to | #1571844 |
* Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> Some AArch64 UEFI implementations disable the MMU in ExitBootServices(),
> after which unaligned accesses to RAM are no longer supported.
>
> Commit abfb7b686a3e ("efi/libstub/arm*: Pass latest memory map to the
> kernel") fixed an issue in the memory map handling of the stub FDT code,
> but inadvertently created an issue with such firmwares, by moving some
> of the FDT manipulation to after the invocation of ExitBootServices().
> Given that the stub's libfdt implementation uses the ordinary, accelerated
> string functions, which rely on hardware handling of unaligned accesses,
> manipulating the FDT with the MMU off may result in alignment faults.
>
> So fix the situation by moving the update_fdt_memmap() call into the
> callback function invoked by efi_exit_boot_services() right before it
> calls the ExitBootServices() UEFI service (which is arguably a better
> place for it anyway)
>
> Note that disabling the MMU in ExitBootServices() is not compliant with
> the UEFI spec, and carries great risk due to the fact that switching from
> cached to uncached memory accesses halfway through compiler generated code
> (i.e., involving a stack) can never be done in a way that is architecturally
> safe.
>
> Cc: <stable@vger.kernel.org>
> Fixes: abfb7b686a3e ("efi/libstub/arm*: Pass latest memory map to the kernel")
> Tested-by: Riku Voipio <riku.voipio@linaro.org>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> drivers/firmware/efi/libstub/fdt.c | 14 +++-----------
> 1 file changed, 3 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
> index 921dfa047202..260c4b4b492e 100644
> --- a/drivers/firmware/efi/libstub/fdt.c
> +++ b/drivers/firmware/efi/libstub/fdt.c
> @@ -187,6 +187,7 @@ static efi_status_t update_fdt_memmap(void *fdt, struct efi_boot_memmap *map)
> struct exit_boot_struct {
> efi_memory_desc_t *runtime_map;
> int *runtime_entry_count;
> + void *new_fdt_addr;
> };
>
> static efi_status_t exit_boot_func(efi_system_table_t *sys_table_arg,
> @@ -202,7 +203,7 @@ static efi_status_t exit_boot_func(efi_system_table_t *sys_table_arg,
> efi_get_virtmap(*map->map, *map->map_size, *map->desc_size,
> p->runtime_map, p->runtime_entry_count);
>
> - return EFI_SUCCESS;
> + return update_fdt_memmap(p->new_fdt_addr, map);
> }
>
> /*
> @@ -300,22 +301,13 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
>
> priv.runtime_map = runtime_map;
> priv.runtime_entry_count = &runtime_entry_count;
> + priv.new_fdt_addr = (void *)*new_fdt_addr;
Nit: could the type cast by dropped?
Thanks,
Ingo
[toc] | [prev] | [next] | [standalone]
| From | Ard Biesheuvel <ard.biesheuvel@linaro.org> |
|---|---|
| Date | 2017-02-01 20:30 +0100 |
| Message-ID | <t68Ln-1wJ-1@gated-at.bofh.it> |
| In reply to | #1571907 |
On 1 February 2017 at 19:03, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
>> Some AArch64 UEFI implementations disable the MMU in ExitBootServices(),
>> after which unaligned accesses to RAM are no longer supported.
>>
>> Commit abfb7b686a3e ("efi/libstub/arm*: Pass latest memory map to the
>> kernel") fixed an issue in the memory map handling of the stub FDT code,
>> but inadvertently created an issue with such firmwares, by moving some
>> of the FDT manipulation to after the invocation of ExitBootServices().
>> Given that the stub's libfdt implementation uses the ordinary, accelerated
>> string functions, which rely on hardware handling of unaligned accesses,
>> manipulating the FDT with the MMU off may result in alignment faults.
>>
>> So fix the situation by moving the update_fdt_memmap() call into the
>> callback function invoked by efi_exit_boot_services() right before it
>> calls the ExitBootServices() UEFI service (which is arguably a better
>> place for it anyway)
>>
>> Note that disabling the MMU in ExitBootServices() is not compliant with
>> the UEFI spec, and carries great risk due to the fact that switching from
>> cached to uncached memory accesses halfway through compiler generated code
>> (i.e., involving a stack) can never be done in a way that is architecturally
>> safe.
>>
>> Cc: <stable@vger.kernel.org>
>> Fixes: abfb7b686a3e ("efi/libstub/arm*: Pass latest memory map to the kernel")
>> Tested-by: Riku Voipio <riku.voipio@linaro.org>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>> drivers/firmware/efi/libstub/fdt.c | 14 +++-----------
>> 1 file changed, 3 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
>> index 921dfa047202..260c4b4b492e 100644
>> --- a/drivers/firmware/efi/libstub/fdt.c
>> +++ b/drivers/firmware/efi/libstub/fdt.c
>> @@ -187,6 +187,7 @@ static efi_status_t update_fdt_memmap(void *fdt, struct efi_boot_memmap *map)
>> struct exit_boot_struct {
>> efi_memory_desc_t *runtime_map;
>> int *runtime_entry_count;
>> + void *new_fdt_addr;
>> };
>>
>> static efi_status_t exit_boot_func(efi_system_table_t *sys_table_arg,
>> @@ -202,7 +203,7 @@ static efi_status_t exit_boot_func(efi_system_table_t *sys_table_arg,
>> efi_get_virtmap(*map->map, *map->map_size, *map->desc_size,
>> p->runtime_map, p->runtime_entry_count);
>>
>> - return EFI_SUCCESS;
>> + return update_fdt_memmap(p->new_fdt_addr, map);
>> }
>>
>> /*
>> @@ -300,22 +301,13 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
>>
>> priv.runtime_map = runtime_map;
>> priv.runtime_entry_count = &runtime_entry_count;
>> + priv.new_fdt_addr = (void *)*new_fdt_addr;
>
> Nit: could the type cast by dropped?
>
new_fdt_addr is a pointer to unsigned long, so we can't drop the cast
here without introducing it somewhere else.
[toc] | [prev] | [next] | [standalone]
| From | Ingo Molnar <mingo@kernel.org> |
|---|---|
| Date | 2017-02-01 21:20 +0100 |
| Message-ID | <t69xR-2ab-9@gated-at.bofh.it> |
| In reply to | #1571909 |
* Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 1 February 2017 at 19:03, Ingo Molnar <mingo@kernel.org> wrote:
> >
> > * Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >
> >> Some AArch64 UEFI implementations disable the MMU in ExitBootServices(),
> >> after which unaligned accesses to RAM are no longer supported.
> >>
> >> Commit abfb7b686a3e ("efi/libstub/arm*: Pass latest memory map to the
> >> kernel") fixed an issue in the memory map handling of the stub FDT code,
> >> but inadvertently created an issue with such firmwares, by moving some
> >> of the FDT manipulation to after the invocation of ExitBootServices().
> >> Given that the stub's libfdt implementation uses the ordinary, accelerated
> >> string functions, which rely on hardware handling of unaligned accesses,
> >> manipulating the FDT with the MMU off may result in alignment faults.
> >>
> >> So fix the situation by moving the update_fdt_memmap() call into the
> >> callback function invoked by efi_exit_boot_services() right before it
> >> calls the ExitBootServices() UEFI service (which is arguably a better
> >> place for it anyway)
> >>
> >> Note that disabling the MMU in ExitBootServices() is not compliant with
> >> the UEFI spec, and carries great risk due to the fact that switching from
> >> cached to uncached memory accesses halfway through compiler generated code
> >> (i.e., involving a stack) can never be done in a way that is architecturally
> >> safe.
> >>
> >> Cc: <stable@vger.kernel.org>
> >> Fixes: abfb7b686a3e ("efi/libstub/arm*: Pass latest memory map to the kernel")
> >> Tested-by: Riku Voipio <riku.voipio@linaro.org>
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> ---
> >> drivers/firmware/efi/libstub/fdt.c | 14 +++-----------
> >> 1 file changed, 3 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
> >> index 921dfa047202..260c4b4b492e 100644
> >> --- a/drivers/firmware/efi/libstub/fdt.c
> >> +++ b/drivers/firmware/efi/libstub/fdt.c
> >> @@ -187,6 +187,7 @@ static efi_status_t update_fdt_memmap(void *fdt, struct efi_boot_memmap *map)
> >> struct exit_boot_struct {
> >> efi_memory_desc_t *runtime_map;
> >> int *runtime_entry_count;
> >> + void *new_fdt_addr;
> >> };
> >>
> >> static efi_status_t exit_boot_func(efi_system_table_t *sys_table_arg,
> >> @@ -202,7 +203,7 @@ static efi_status_t exit_boot_func(efi_system_table_t *sys_table_arg,
> >> efi_get_virtmap(*map->map, *map->map_size, *map->desc_size,
> >> p->runtime_map, p->runtime_entry_count);
> >>
> >> - return EFI_SUCCESS;
> >> + return update_fdt_memmap(p->new_fdt_addr, map);
> >> }
> >>
> >> /*
> >> @@ -300,22 +301,13 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
> >>
> >> priv.runtime_map = runtime_map;
> >> priv.runtime_entry_count = &runtime_entry_count;
> >> + priv.new_fdt_addr = (void *)*new_fdt_addr;
> >
> > Nit: could the type cast by dropped?
>
> new_fdt_addr is a pointer to unsigned long, so we can't drop the cast
> here without introducing it somewhere else.
Fair enough, I missed the dereference of new_fdt_addr.
Thanks,
Ingo
[toc] | [prev] | [next] | [standalone]
| From | Jeffrey Hugo <jhugo@codeaurora.org> |
|---|---|
| Date | 2017-02-01 22:10 +0100 |
| Message-ID | <t6ak9-2Pd-7@gated-at.bofh.it> |
| In reply to | #1571844 |
On 2/1/2017 10:45 AM, Ard Biesheuvel wrote:
> Some AArch64 UEFI implementations disable the MMU in ExitBootServices(),
> after which unaligned accesses to RAM are no longer supported.
>
> Commit abfb7b686a3e ("efi/libstub/arm*: Pass latest memory map to the
> kernel") fixed an issue in the memory map handling of the stub FDT code,
> but inadvertently created an issue with such firmwares, by moving some
> of the FDT manipulation to after the invocation of ExitBootServices().
> Given that the stub's libfdt implementation uses the ordinary, accelerated
> string functions, which rely on hardware handling of unaligned accesses,
> manipulating the FDT with the MMU off may result in alignment faults.
>
> So fix the situation by moving the update_fdt_memmap() call into the
> callback function invoked by efi_exit_boot_services() right before it
> calls the ExitBootServices() UEFI service (which is arguably a better
> place for it anyway)
>
> Note that disabling the MMU in ExitBootServices() is not compliant with
> the UEFI spec, and carries great risk due to the fact that switching from
> cached to uncached memory accesses halfway through compiler generated code
> (i.e., involving a stack) can never be done in a way that is architecturally
> safe.
>
> Cc: <stable@vger.kernel.org>
> Fixes: abfb7b686a3e ("efi/libstub/arm*: Pass latest memory map to the kernel")
> Tested-by: Riku Voipio <riku.voipio@linaro.org>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
NACK, please. This causes a regression on my platform, in the form of
an assert in UEFI once ExitBootServices() is called, per initial
testing. I'll do more testing to determine why.
--
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
[toc] | [prev] | [next] | [standalone]
| From | Jeffrey Hugo <jhugo@codeaurora.org> |
|---|---|
| Date | 2017-02-02 00:00 +0100 |
| Message-ID | <t6c2B-3RH-7@gated-at.bofh.it> |
| In reply to | #1571977 |
On 2/1/2017 2:08 PM, Jeffrey Hugo wrote:
> On 2/1/2017 10:45 AM, Ard Biesheuvel wrote:
>> Some AArch64 UEFI implementations disable the MMU in ExitBootServices(),
>> after which unaligned accesses to RAM are no longer supported.
>>
>> Commit abfb7b686a3e ("efi/libstub/arm*: Pass latest memory map to the
>> kernel") fixed an issue in the memory map handling of the stub FDT code,
>> but inadvertently created an issue with such firmwares, by moving some
>> of the FDT manipulation to after the invocation of ExitBootServices().
>> Given that the stub's libfdt implementation uses the ordinary,
>> accelerated
>> string functions, which rely on hardware handling of unaligned accesses,
>> manipulating the FDT with the MMU off may result in alignment faults.
>>
>> So fix the situation by moving the update_fdt_memmap() call into the
>> callback function invoked by efi_exit_boot_services() right before it
>> calls the ExitBootServices() UEFI service (which is arguably a better
>> place for it anyway)
>>
>> Note that disabling the MMU in ExitBootServices() is not compliant with
>> the UEFI spec, and carries great risk due to the fact that switching from
>> cached to uncached memory accesses halfway through compiler generated
>> code
>> (i.e., involving a stack) can never be done in a way that is
>> architecturally
>> safe.
>>
>> Cc: <stable@vger.kernel.org>
>> Fixes: abfb7b686a3e ("efi/libstub/arm*: Pass latest memory map to the
>> kernel")
>> Tested-by: Riku Voipio <riku.voipio@linaro.org>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> NACK, please. This causes a regression on my platform, in the form of
> an assert in UEFI once ExitBootServices() is called, per initial
> testing. I'll do more testing to determine why.
>
Sorry, false alarm. The assert appears to have been the result of a bad
tree and a bad target configuration, and was not reproduced on another
setup, nor did the assert make sense in context with this change.
I withdraw my NACK.
--
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
[toc] | [prev] | [next] | [standalone]
| From | Ard Biesheuvel <ard.biesheuvel@linaro.org> |
|---|---|
| Date | 2017-02-02 17:20 +0100 |
| Message-ID | <t6sh4-6ho-27@gated-at.bofh.it> |
| In reply to | #1572037 |
On 1 February 2017 at 22:59, Jeffrey Hugo <jhugo@codeaurora.org> wrote:
> On 2/1/2017 2:08 PM, Jeffrey Hugo wrote:
>>
>> On 2/1/2017 10:45 AM, Ard Biesheuvel wrote:
>>>
>>> Some AArch64 UEFI implementations disable the MMU in ExitBootServices(),
>>> after which unaligned accesses to RAM are no longer supported.
>>>
>>> Commit abfb7b686a3e ("efi/libstub/arm*: Pass latest memory map to the
>>> kernel") fixed an issue in the memory map handling of the stub FDT code,
>>> but inadvertently created an issue with such firmwares, by moving some
>>> of the FDT manipulation to after the invocation of ExitBootServices().
>>> Given that the stub's libfdt implementation uses the ordinary,
>>> accelerated
>>> string functions, which rely on hardware handling of unaligned accesses,
>>> manipulating the FDT with the MMU off may result in alignment faults.
>>>
>>> So fix the situation by moving the update_fdt_memmap() call into the
>>> callback function invoked by efi_exit_boot_services() right before it
>>> calls the ExitBootServices() UEFI service (which is arguably a better
>>> place for it anyway)
>>>
>>> Note that disabling the MMU in ExitBootServices() is not compliant with
>>> the UEFI spec, and carries great risk due to the fact that switching from
>>> cached to uncached memory accesses halfway through compiler generated
>>> code
>>> (i.e., involving a stack) can never be done in a way that is
>>> architecturally
>>> safe.
>>>
>>> Cc: <stable@vger.kernel.org>
>>> Fixes: abfb7b686a3e ("efi/libstub/arm*: Pass latest memory map to the
>>> kernel")
>>> Tested-by: Riku Voipio <riku.voipio@linaro.org>
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>
>>
>> NACK, please. This causes a regression on my platform, in the form of
>> an assert in UEFI once ExitBootServices() is called, per initial
>> testing. I'll do more testing to determine why.
>>
>
> Sorry, false alarm. The assert appears to have been the result of a bad
> tree and a bad target configuration, and was not reproduced on another
> setup, nor did the assert make sense in context with this change.
>
No worries. Thanks for taking the time to test the patch.
--
Ard.
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for Ard Biesheuvel <tipbot@zytor.com> |
|---|---|
| Date | 2017-02-02 02:40 +0100 |
| Subject | [tip:efi/urgent] efi/fdt: Avoid FDT manipulation after ExitBootServices() |
| Message-ID | <t6exr-5vr-1@gated-at.bofh.it> |
| In reply to | #1571844 |
Commit-ID: c8f325a59cfc718d13a50fbc746ed9b415c25e92
Gitweb: http://git.kernel.org/tip/c8f325a59cfc718d13a50fbc746ed9b415c25e92
Author: Ard Biesheuvel <ard.biesheuvel@linaro.org>
AuthorDate: Wed, 1 Feb 2017 17:45:02 +0000
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 1 Feb 2017 21:17:49 +0100
efi/fdt: Avoid FDT manipulation after ExitBootServices()
Some AArch64 UEFI implementations disable the MMU in ExitBootServices(),
after which unaligned accesses to RAM are no longer supported.
Commit:
abfb7b686a3e ("efi/libstub/arm*: Pass latest memory map to the kernel")
fixed an issue in the memory map handling of the stub FDT code, but
inadvertently created an issue with such firmware, by moving some
of the FDT manipulation to after the invocation of ExitBootServices().
Given that the stub's libfdt implementation uses the ordinary, accelerated
string functions, which rely on hardware handling of unaligned accesses,
manipulating the FDT with the MMU off may result in alignment faults.
So fix the situation by moving the update_fdt_memmap() call into the
callback function invoked by efi_exit_boot_services() right before it
calls the ExitBootServices() UEFI service (which is arguably a better
place for it anyway)
Note that disabling the MMU in ExitBootServices() is not compliant with
the UEFI spec, and carries great risk due to the fact that switching from
cached to uncached memory accesses halfway through compiler generated code
(i.e., involving a stack) can never be done in a way that is architecturally
safe.
Fixes: abfb7b686a3e ("efi/libstub/arm*: Pass latest memory map to the kernel")
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Tested-by: Riku Voipio <riku.voipio@linaro.org>
Cc: <stable@vger.kernel.org>
Cc: mark.rutland@arm.com
Cc: linux-efi@vger.kernel.org
Cc: matt@codeblueprint.co.uk
Cc: leif.lindholm@linaro.org
Cc: linux-arm-kernel@lists.infradead.org
Link: http://lkml.kernel.org/r/1485971102-23330-2-git-send-email-ard.biesheuvel@linaro.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
drivers/firmware/efi/libstub/fdt.c | 14 +++-----------
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
index 921dfa0..260c4b4 100644
--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -187,6 +187,7 @@ static efi_status_t update_fdt_memmap(void *fdt, struct efi_boot_memmap *map)
struct exit_boot_struct {
efi_memory_desc_t *runtime_map;
int *runtime_entry_count;
+ void *new_fdt_addr;
};
static efi_status_t exit_boot_func(efi_system_table_t *sys_table_arg,
@@ -202,7 +203,7 @@ static efi_status_t exit_boot_func(efi_system_table_t *sys_table_arg,
efi_get_virtmap(*map->map, *map->map_size, *map->desc_size,
p->runtime_map, p->runtime_entry_count);
- return EFI_SUCCESS;
+ return update_fdt_memmap(p->new_fdt_addr, map);
}
/*
@@ -300,22 +301,13 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
priv.runtime_map = runtime_map;
priv.runtime_entry_count = &runtime_entry_count;
+ priv.new_fdt_addr = (void *)*new_fdt_addr;
status = efi_exit_boot_services(sys_table, handle, &map, &priv,
exit_boot_func);
if (status == EFI_SUCCESS) {
efi_set_virtual_address_map_t *svam;
- status = update_fdt_memmap((void *)*new_fdt_addr, &map);
- if (status != EFI_SUCCESS) {
- /*
- * The kernel won't get far without the memory map, but
- * may still be able to print something meaningful so
- * return success here.
- */
- return EFI_SUCCESS;
- }
-
/* Install the new virtual address map */
svam = sys_table->runtime->set_virtual_address_map;
status = svam(runtime_entry_count * desc_size, desc_size,
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web