* [PATCH v2] efi/libstub: arm*: Pass latest memory map to the kernel
@ 2016-12-19 14:24 ` James Morse
0 siblings, 0 replies; 8+ messages in thread
From: James Morse @ 2016-12-19 14:24 UTC (permalink / raw)
To: linux-efi; +Cc: matt, jhugo, linux-arm-kernel, Ard Biesheuvel
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
As reported by James, the current libstub code involving the annotated
memory map only works somewhat correctly by accident, due to the fact
that a pool allocation happens to be reused immediately, retaining its
former contents.
Instead of juggling memory maps, which makes the code more complex than
it needs to be, simply put a placholder value into the FDT, and only
write the actual value after ExitBootServices() has been called.
Reported-by: James Morse <james.morse@arm.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
[Update mmap-size too, remove updated_fdt()s unused params and header entry]
Signed-off-by: James Morse <james.morse@arm.com>
---
Hi Ard,
This is a v2 of your patch that updates the mmap-size too. This solves the
truncated memmap problem I saw with v1 on Seattle.
The original patch was CC-stable, so I think this should also have:
Cc: <stable@vger.kernel.org>
Fixes: ed9cc156c42f ("efi/libstub: Use efi_exit_boot_services() in FDT")
Thanks,
James
drivers/firmware/efi/libstub/efistub.h | 8 ----
drivers/firmware/efi/libstub/fdt.c | 75 +++++++++++++++++++++-------------
2 files changed, 47 insertions(+), 36 deletions(-)
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index ee49cd23ee63..fac67992bede 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -30,14 +30,6 @@ efi_status_t efi_file_close(void *handle);
unsigned long get_dram_base(efi_system_table_t *sys_table_arg);
-efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
- unsigned long orig_fdt_size,
- void *fdt, int new_fdt_size, char *cmdline_ptr,
- u64 initrd_addr, u64 initrd_size,
- efi_memory_desc_t *memory_map,
- unsigned long map_size, unsigned long desc_size,
- u32 desc_ver);
-
efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
void *handle,
unsigned long *new_fdt_addr,
diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
index a6a93116a8f0..6d33d709f6b3 100644
--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -16,13 +16,11 @@
#include "efistub.h"
-efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
- unsigned long orig_fdt_size,
- void *fdt, int new_fdt_size, char *cmdline_ptr,
- u64 initrd_addr, u64 initrd_size,
- efi_memory_desc_t *memory_map,
- unsigned long map_size, unsigned long desc_size,
- u32 desc_ver)
+static efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
+ unsigned long orig_fdt_size,
+ void *fdt, int new_fdt_size, char *cmdline_ptr,
+ u64 initrd_addr, u64 initrd_size,
+ unsigned long desc_size, u32 desc_ver)
{
int node, num_rsv;
int status;
@@ -101,13 +99,13 @@ efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
if (status)
goto fdt_set_fail;
- fdt_val64 = cpu_to_fdt64((u64)(unsigned long)memory_map);
+ fdt_val64 = U64_MAX; /* placeholder */
status = fdt_setprop(fdt, node, "linux,uefi-mmap-start",
&fdt_val64, sizeof(fdt_val64));
if (status)
goto fdt_set_fail;
- fdt_val32 = cpu_to_fdt32(map_size);
+ fdt_val32 = U32_MAX; /* placeholder */
status = fdt_setprop(fdt, node, "linux,uefi-mmap-size",
&fdt_val32, sizeof(fdt_val32));
if (status)
@@ -148,6 +146,32 @@ efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
return EFI_LOAD_ERROR;
}
+static efi_status_t update_fdt_memmap(void *fdt, u64 memmap, u32 map_size)
+{
+ int node = fdt_path_offset(fdt, "/chosen");
+ efi_status_t status;
+
+ if (node < 0)
+ return EFI_LOAD_ERROR;
+
+ memmap = cpu_to_fdt64(memmap);
+ status = fdt_setprop_inplace(fdt, node, "linux,uefi-mmap-start",
+ &memmap, sizeof(memmap));
+
+ if (status)
+ return EFI_LOAD_ERROR;
+
+ map_size = cpu_to_fdt32(map_size);
+ status = fdt_setprop_inplace(fdt, node, "linux,uefi-mmap-size",
+ &map_size, sizeof(map_size));
+
+ if (status)
+ return EFI_LOAD_ERROR;
+
+
+ return EFI_SUCCESS;
+}
+
#ifndef EFI_FDT_ALIGN
#define EFI_FDT_ALIGN EFI_PAGE_SIZE
#endif
@@ -243,20 +267,11 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
goto fail;
}
- /*
- * Now that we have done our final memory allocation (and free)
- * we can get the memory map key needed for
- * exit_boot_services().
- */
- status = efi_get_memory_map(sys_table, &map);
- if (status != EFI_SUCCESS)
- goto fail_free_new_fdt;
-
status = update_fdt(sys_table,
(void *)fdt_addr, fdt_size,
(void *)*new_fdt_addr, new_fdt_size,
cmdline_ptr, initrd_addr, initrd_size,
- memory_map, map_size, desc_size, desc_ver);
+ desc_size, desc_ver);
/* Succeeding the first time is the expected case. */
if (status == EFI_SUCCESS)
@@ -266,20 +281,16 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
/*
* We need to allocate more space for the new
* device tree, so free existing buffer that is
- * too small. Also free memory map, as we will need
- * to get new one that reflects the free/alloc we do
- * on the device tree buffer.
+ * too small.
*/
efi_free(sys_table, new_fdt_size, *new_fdt_addr);
- sys_table->boottime->free_pool(memory_map);
new_fdt_size += EFI_PAGE_SIZE;
} else {
pr_efi_err(sys_table, "Unable to construct new device tree.\n");
- goto fail_free_mmap;
+ goto fail_free_new_fdt;
}
}
- sys_table->boottime->free_pool(memory_map);
priv.runtime_map = runtime_map;
priv.runtime_entry_count = &runtime_entry_count;
status = efi_exit_boot_services(sys_table, handle, &map, &priv,
@@ -288,6 +299,17 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
if (status == EFI_SUCCESS) {
efi_set_virtual_address_map_t *svam;
+ status = update_fdt_memmap((void *)*new_fdt_addr,
+ (u64)memory_map, (u32)map_size);
+ 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,
@@ -319,9 +341,6 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
pr_efi_err(sys_table, "Exit boot services failed.\n");
-fail_free_mmap:
- sys_table->boottime->free_pool(memory_map);
-
fail_free_new_fdt:
efi_free(sys_table, new_fdt_size, *new_fdt_addr);
--
2.10.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2] efi/libstub: arm*: Pass latest memory map to the kernel
@ 2016-12-19 14:24 ` James Morse
0 siblings, 0 replies; 8+ messages in thread
From: James Morse @ 2016-12-19 14:24 UTC (permalink / raw)
To: linux-arm-kernel
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
As reported by James, the current libstub code involving the annotated
memory map only works somewhat correctly by accident, due to the fact
that a pool allocation happens to be reused immediately, retaining its
former contents.
Instead of juggling memory maps, which makes the code more complex than
it needs to be, simply put a placholder value into the FDT, and only
write the actual value after ExitBootServices() has been called.
Reported-by: James Morse <james.morse@arm.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
[Update mmap-size too, remove updated_fdt()s unused params and header entry]
Signed-off-by: James Morse <james.morse@arm.com>
---
Hi Ard,
This is a v2 of your patch that updates the mmap-size too. This solves the
truncated memmap problem I saw with v1 on Seattle.
The original patch was CC-stable, so I think this should also have:
Cc: <stable@vger.kernel.org>
Fixes: ed9cc156c42f ("efi/libstub: Use efi_exit_boot_services() in FDT")
Thanks,
James
drivers/firmware/efi/libstub/efistub.h | 8 ----
drivers/firmware/efi/libstub/fdt.c | 75 +++++++++++++++++++++-------------
2 files changed, 47 insertions(+), 36 deletions(-)
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index ee49cd23ee63..fac67992bede 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -30,14 +30,6 @@ efi_status_t efi_file_close(void *handle);
unsigned long get_dram_base(efi_system_table_t *sys_table_arg);
-efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
- unsigned long orig_fdt_size,
- void *fdt, int new_fdt_size, char *cmdline_ptr,
- u64 initrd_addr, u64 initrd_size,
- efi_memory_desc_t *memory_map,
- unsigned long map_size, unsigned long desc_size,
- u32 desc_ver);
-
efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
void *handle,
unsigned long *new_fdt_addr,
diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
index a6a93116a8f0..6d33d709f6b3 100644
--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -16,13 +16,11 @@
#include "efistub.h"
-efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
- unsigned long orig_fdt_size,
- void *fdt, int new_fdt_size, char *cmdline_ptr,
- u64 initrd_addr, u64 initrd_size,
- efi_memory_desc_t *memory_map,
- unsigned long map_size, unsigned long desc_size,
- u32 desc_ver)
+static efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
+ unsigned long orig_fdt_size,
+ void *fdt, int new_fdt_size, char *cmdline_ptr,
+ u64 initrd_addr, u64 initrd_size,
+ unsigned long desc_size, u32 desc_ver)
{
int node, num_rsv;
int status;
@@ -101,13 +99,13 @@ efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
if (status)
goto fdt_set_fail;
- fdt_val64 = cpu_to_fdt64((u64)(unsigned long)memory_map);
+ fdt_val64 = U64_MAX; /* placeholder */
status = fdt_setprop(fdt, node, "linux,uefi-mmap-start",
&fdt_val64, sizeof(fdt_val64));
if (status)
goto fdt_set_fail;
- fdt_val32 = cpu_to_fdt32(map_size);
+ fdt_val32 = U32_MAX; /* placeholder */
status = fdt_setprop(fdt, node, "linux,uefi-mmap-size",
&fdt_val32, sizeof(fdt_val32));
if (status)
@@ -148,6 +146,32 @@ efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
return EFI_LOAD_ERROR;
}
+static efi_status_t update_fdt_memmap(void *fdt, u64 memmap, u32 map_size)
+{
+ int node = fdt_path_offset(fdt, "/chosen");
+ efi_status_t status;
+
+ if (node < 0)
+ return EFI_LOAD_ERROR;
+
+ memmap = cpu_to_fdt64(memmap);
+ status = fdt_setprop_inplace(fdt, node, "linux,uefi-mmap-start",
+ &memmap, sizeof(memmap));
+
+ if (status)
+ return EFI_LOAD_ERROR;
+
+ map_size = cpu_to_fdt32(map_size);
+ status = fdt_setprop_inplace(fdt, node, "linux,uefi-mmap-size",
+ &map_size, sizeof(map_size));
+
+ if (status)
+ return EFI_LOAD_ERROR;
+
+
+ return EFI_SUCCESS;
+}
+
#ifndef EFI_FDT_ALIGN
#define EFI_FDT_ALIGN EFI_PAGE_SIZE
#endif
@@ -243,20 +267,11 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
goto fail;
}
- /*
- * Now that we have done our final memory allocation (and free)
- * we can get the memory map key needed for
- * exit_boot_services().
- */
- status = efi_get_memory_map(sys_table, &map);
- if (status != EFI_SUCCESS)
- goto fail_free_new_fdt;
-
status = update_fdt(sys_table,
(void *)fdt_addr, fdt_size,
(void *)*new_fdt_addr, new_fdt_size,
cmdline_ptr, initrd_addr, initrd_size,
- memory_map, map_size, desc_size, desc_ver);
+ desc_size, desc_ver);
/* Succeeding the first time is the expected case. */
if (status == EFI_SUCCESS)
@@ -266,20 +281,16 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
/*
* We need to allocate more space for the new
* device tree, so free existing buffer that is
- * too small. Also free memory map, as we will need
- * to get new one that reflects the free/alloc we do
- * on the device tree buffer.
+ * too small.
*/
efi_free(sys_table, new_fdt_size, *new_fdt_addr);
- sys_table->boottime->free_pool(memory_map);
new_fdt_size += EFI_PAGE_SIZE;
} else {
pr_efi_err(sys_table, "Unable to construct new device tree.\n");
- goto fail_free_mmap;
+ goto fail_free_new_fdt;
}
}
- sys_table->boottime->free_pool(memory_map);
priv.runtime_map = runtime_map;
priv.runtime_entry_count = &runtime_entry_count;
status = efi_exit_boot_services(sys_table, handle, &map, &priv,
@@ -288,6 +299,17 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
if (status == EFI_SUCCESS) {
efi_set_virtual_address_map_t *svam;
+ status = update_fdt_memmap((void *)*new_fdt_addr,
+ (u64)memory_map, (u32)map_size);
+ 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,
@@ -319,9 +341,6 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
pr_efi_err(sys_table, "Exit boot services failed.\n");
-fail_free_mmap:
- sys_table->boottime->free_pool(memory_map);
-
fail_free_new_fdt:
efi_free(sys_table, new_fdt_size, *new_fdt_addr);
--
2.10.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] efi/libstub: arm*: Pass latest memory map to the kernel
2016-12-19 14:24 ` James Morse
@ 2016-12-19 21:38 ` Matt Fleming
-1 siblings, 0 replies; 8+ messages in thread
From: Matt Fleming @ 2016-12-19 21:38 UTC (permalink / raw)
To: James Morse
Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Ard Biesheuvel, jhugo-sgV2jX0FEOL9JmXXK+q4OQ
On Mon, 19 Dec, at 02:24:19PM, James Morse wrote:
> From: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>
> As reported by James, the current libstub code involving the annotated
> memory map only works somewhat correctly by accident, due to the fact
> that a pool allocation happens to be reused immediately, retaining its
> former contents.
>
> Instead of juggling memory maps, which makes the code more complex than
> it needs to be, simply put a placholder value into the FDT, and only
> write the actual value after ExitBootServices() has been called.
>
> Reported-by: James Morse <james.morse-5wv7dgnIgG8@public.gmane.org>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> [Update mmap-size too, remove updated_fdt()s unused params and header entry]
> Signed-off-by: James Morse <james.morse-5wv7dgnIgG8@public.gmane.org>
> ---
> Hi Ard,
>
> This is a v2 of your patch that updates the mmap-size too. This solves the
> truncated memmap problem I saw with v1 on Seattle.
>
> The original patch was CC-stable, so I think this should also have:
> Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> Fixes: ed9cc156c42f ("efi/libstub: Use efi_exit_boot_services() in FDT")
>
>
> Thanks,
>
> James
>
> drivers/firmware/efi/libstub/efistub.h | 8 ----
> drivers/firmware/efi/libstub/fdt.c | 75 +++++++++++++++++++++-------------
> 2 files changed, 47 insertions(+), 36 deletions(-)
Thanks James. I've queued this one up in the 'urgent' queue and tagged
it for stable. I'll send it to tip before the end of the week.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] efi/libstub: arm*: Pass latest memory map to the kernel
@ 2016-12-19 21:38 ` Matt Fleming
0 siblings, 0 replies; 8+ messages in thread
From: Matt Fleming @ 2016-12-19 21:38 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, 19 Dec, at 02:24:19PM, James Morse wrote:
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> As reported by James, the current libstub code involving the annotated
> memory map only works somewhat correctly by accident, due to the fact
> that a pool allocation happens to be reused immediately, retaining its
> former contents.
>
> Instead of juggling memory maps, which makes the code more complex than
> it needs to be, simply put a placholder value into the FDT, and only
> write the actual value after ExitBootServices() has been called.
>
> Reported-by: James Morse <james.morse@arm.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> [Update mmap-size too, remove updated_fdt()s unused params and header entry]
> Signed-off-by: James Morse <james.morse@arm.com>
> ---
> Hi Ard,
>
> This is a v2 of your patch that updates the mmap-size too. This solves the
> truncated memmap problem I saw with v1 on Seattle.
>
> The original patch was CC-stable, so I think this should also have:
> Cc: <stable@vger.kernel.org>
> Fixes: ed9cc156c42f ("efi/libstub: Use efi_exit_boot_services() in FDT")
>
>
> Thanks,
>
> James
>
> drivers/firmware/efi/libstub/efistub.h | 8 ----
> drivers/firmware/efi/libstub/fdt.c | 75 +++++++++++++++++++++-------------
> 2 files changed, 47 insertions(+), 36 deletions(-)
Thanks James. I've queued this one up in the 'urgent' queue and tagged
it for stable. I'll send it to tip before the end of the week.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] efi/libstub: arm*: Pass latest memory map to the kernel
2016-12-19 21:38 ` Matt Fleming
@ 2016-12-20 22:32 ` Ard Biesheuvel
-1 siblings, 0 replies; 8+ messages in thread
From: Ard Biesheuvel @ 2016-12-20 22:32 UTC (permalink / raw)
To: Matt Fleming
Cc: James Morse, linux-efi-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jeff Hugo
On 19 December 2016 at 21:38, Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org> wrote:
> On Mon, 19 Dec, at 02:24:19PM, James Morse wrote:
>> From: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>
>> As reported by James, the current libstub code involving the annotated
>> memory map only works somewhat correctly by accident, due to the fact
>> that a pool allocation happens to be reused immediately, retaining its
>> former contents.
>>
>> Instead of juggling memory maps, which makes the code more complex than
>> it needs to be, simply put a placholder value into the FDT, and only
>> write the actual value after ExitBootServices() has been called.
>>
>> Reported-by: James Morse <james.morse-5wv7dgnIgG8@public.gmane.org>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> [Update mmap-size too, remove updated_fdt()s unused params and header entry]
>> Signed-off-by: James Morse <james.morse-5wv7dgnIgG8@public.gmane.org>
>> ---
>> Hi Ard,
>>
>> This is a v2 of your patch that updates the mmap-size too. This solves the
>> truncated memmap problem I saw with v1 on Seattle.
>>
>> The original patch was CC-stable, so I think this should also have:
>> Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
>> Fixes: ed9cc156c42f ("efi/libstub: Use efi_exit_boot_services() in FDT")
>>
>>
>> Thanks,
>>
>> James
>>
>> drivers/firmware/efi/libstub/efistub.h | 8 ----
>> drivers/firmware/efi/libstub/fdt.c | 75 +++++++++++++++++++++-------------
>> 2 files changed, 47 insertions(+), 36 deletions(-)
>
> Thanks James. I've queued this one up in the 'urgent' queue and tagged
> it for stable. I'll send it to tip before the end of the week.
Could we fold the hunk below, please?
diff --git a/drivers/firmware/efi/libstub/fdt.c
b/drivers/firmware/efi/libstub/fdt.c
index 9b11b0559a23..90ab96845937 100644
--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -149,26 +149,25 @@ static efi_status_t
update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
static efi_status_t update_fdt_memmap(void *fdt, u64 memmap, u32 map_size)
{
int node = fdt_path_offset(fdt, "/chosen");
- efi_status_t status;
+ int err;
if (node < 0)
return EFI_LOAD_ERROR;
memmap = cpu_to_fdt64(memmap);
- status = fdt_setprop_inplace(fdt, node, "linux,uefi-mmap-start",
- &memmap, sizeof(memmap));
+ err = fdt_setprop_inplace(fdt, node, "linux,uefi-mmap-start",
+ &memmap, sizeof(memmap));
if (status)
return EFI_LOAD_ERROR;
map_size = cpu_to_fdt32(map_size);
- status = fdt_setprop_inplace(fdt, node, "linux,uefi-mmap-size",
- &map_size, sizeof(map_size));
+ err = fdt_setprop_inplace(fdt, node, "linux,uefi-mmap-size",
+ &map_size, sizeof(map_size));
if (status)
return EFI_LOAD_ERROR;
-
return EFI_SUCCESS;
}
My mistake, and harmless in practice, but sloppy nonetheless
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2] efi/libstub: arm*: Pass latest memory map to the kernel
@ 2016-12-20 22:32 ` Ard Biesheuvel
0 siblings, 0 replies; 8+ messages in thread
From: Ard Biesheuvel @ 2016-12-20 22:32 UTC (permalink / raw)
To: linux-arm-kernel
On 19 December 2016 at 21:38, Matt Fleming <matt@codeblueprint.co.uk> wrote:
> On Mon, 19 Dec, at 02:24:19PM, James Morse wrote:
>> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>
>> As reported by James, the current libstub code involving the annotated
>> memory map only works somewhat correctly by accident, due to the fact
>> that a pool allocation happens to be reused immediately, retaining its
>> former contents.
>>
>> Instead of juggling memory maps, which makes the code more complex than
>> it needs to be, simply put a placholder value into the FDT, and only
>> write the actual value after ExitBootServices() has been called.
>>
>> Reported-by: James Morse <james.morse@arm.com>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> [Update mmap-size too, remove updated_fdt()s unused params and header entry]
>> Signed-off-by: James Morse <james.morse@arm.com>
>> ---
>> Hi Ard,
>>
>> This is a v2 of your patch that updates the mmap-size too. This solves the
>> truncated memmap problem I saw with v1 on Seattle.
>>
>> The original patch was CC-stable, so I think this should also have:
>> Cc: <stable@vger.kernel.org>
>> Fixes: ed9cc156c42f ("efi/libstub: Use efi_exit_boot_services() in FDT")
>>
>>
>> Thanks,
>>
>> James
>>
>> drivers/firmware/efi/libstub/efistub.h | 8 ----
>> drivers/firmware/efi/libstub/fdt.c | 75 +++++++++++++++++++++-------------
>> 2 files changed, 47 insertions(+), 36 deletions(-)
>
> Thanks James. I've queued this one up in the 'urgent' queue and tagged
> it for stable. I'll send it to tip before the end of the week.
Could we fold the hunk below, please?
diff --git a/drivers/firmware/efi/libstub/fdt.c
b/drivers/firmware/efi/libstub/fdt.c
index 9b11b0559a23..90ab96845937 100644
--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -149,26 +149,25 @@ static efi_status_t
update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
static efi_status_t update_fdt_memmap(void *fdt, u64 memmap, u32 map_size)
{
int node = fdt_path_offset(fdt, "/chosen");
- efi_status_t status;
+ int err;
if (node < 0)
return EFI_LOAD_ERROR;
memmap = cpu_to_fdt64(memmap);
- status = fdt_setprop_inplace(fdt, node, "linux,uefi-mmap-start",
- &memmap, sizeof(memmap));
+ err = fdt_setprop_inplace(fdt, node, "linux,uefi-mmap-start",
+ &memmap, sizeof(memmap));
if (status)
return EFI_LOAD_ERROR;
map_size = cpu_to_fdt32(map_size);
- status = fdt_setprop_inplace(fdt, node, "linux,uefi-mmap-size",
- &map_size, sizeof(map_size));
+ err = fdt_setprop_inplace(fdt, node, "linux,uefi-mmap-size",
+ &map_size, sizeof(map_size));
if (status)
return EFI_LOAD_ERROR;
-
return EFI_SUCCESS;
}
My mistake, and harmless in practice, but sloppy nonetheless
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] efi/libstub: arm*: Pass latest memory map to the kernel
2016-12-20 22:32 ` Ard Biesheuvel
@ 2016-12-20 22:33 ` Ard Biesheuvel
-1 siblings, 0 replies; 8+ messages in thread
From: Ard Biesheuvel @ 2016-12-20 22:33 UTC (permalink / raw)
To: Matt Fleming
Cc: James Morse, linux-efi-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jeff Hugo
On 20 December 2016 at 22:32, Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On 19 December 2016 at 21:38, Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org> wrote:
>> On Mon, 19 Dec, at 02:24:19PM, James Morse wrote:
>>> From: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>>
>>> As reported by James, the current libstub code involving the annotated
>>> memory map only works somewhat correctly by accident, due to the fact
>>> that a pool allocation happens to be reused immediately, retaining its
>>> former contents.
>>>
>>> Instead of juggling memory maps, which makes the code more complex than
>>> it needs to be, simply put a placholder value into the FDT, and only
>>> write the actual value after ExitBootServices() has been called.
>>>
>>> Reported-by: James Morse <james.morse-5wv7dgnIgG8@public.gmane.org>
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>> [Update mmap-size too, remove updated_fdt()s unused params and header entry]
>>> Signed-off-by: James Morse <james.morse-5wv7dgnIgG8@public.gmane.org>
>>> ---
>>> Hi Ard,
>>>
>>> This is a v2 of your patch that updates the mmap-size too. This solves the
>>> truncated memmap problem I saw with v1 on Seattle.
>>>
>>> The original patch was CC-stable, so I think this should also have:
>>> Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
>>> Fixes: ed9cc156c42f ("efi/libstub: Use efi_exit_boot_services() in FDT")
>>>
>>>
>>> Thanks,
>>>
>>> James
>>>
>>> drivers/firmware/efi/libstub/efistub.h | 8 ----
>>> drivers/firmware/efi/libstub/fdt.c | 75 +++++++++++++++++++++-------------
>>> 2 files changed, 47 insertions(+), 36 deletions(-)
>>
>> Thanks James. I've queued this one up in the 'urgent' queue and tagged
>> it for stable. I'll send it to tip before the end of the week.
>
> Could we fold the hunk below, please?
>
> diff --git a/drivers/firmware/efi/libstub/fdt.c
> b/drivers/firmware/efi/libstub/fdt.c
> index 9b11b0559a23..90ab96845937 100644
> --- a/drivers/firmware/efi/libstub/fdt.c
> +++ b/drivers/firmware/efi/libstub/fdt.c
> @@ -149,26 +149,25 @@ static efi_status_t
> update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
> static efi_status_t update_fdt_memmap(void *fdt, u64 memmap, u32 map_size)
> {
> int node = fdt_path_offset(fdt, "/chosen");
> - efi_status_t status;
> + int err;
>
> if (node < 0)
> return EFI_LOAD_ERROR;
>
> memmap = cpu_to_fdt64(memmap);
> - status = fdt_setprop_inplace(fdt, node, "linux,uefi-mmap-start",
> - &memmap, sizeof(memmap));
> + err = fdt_setprop_inplace(fdt, node, "linux,uefi-mmap-start",
> + &memmap, sizeof(memmap));
>
> if (status)
> return EFI_LOAD_ERROR;
>
> map_size = cpu_to_fdt32(map_size);
> - status = fdt_setprop_inplace(fdt, node, "linux,uefi-mmap-size",
> - &map_size, sizeof(map_size));
> + err = fdt_setprop_inplace(fdt, node, "linux,uefi-mmap-size",
> + &map_size, sizeof(map_size));
>
> if (status)
> return EFI_LOAD_ERROR;
>
> -
> return EFI_SUCCESS;
> }
>
> My mistake, and harmless in practice, but sloppy nonetheless
... but with the 'if (status)' replaced as well, of course (2x)
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] efi/libstub: arm*: Pass latest memory map to the kernel
@ 2016-12-20 22:33 ` Ard Biesheuvel
0 siblings, 0 replies; 8+ messages in thread
From: Ard Biesheuvel @ 2016-12-20 22:33 UTC (permalink / raw)
To: linux-arm-kernel
On 20 December 2016 at 22:32, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 19 December 2016 at 21:38, Matt Fleming <matt@codeblueprint.co.uk> wrote:
>> On Mon, 19 Dec, at 02:24:19PM, James Morse wrote:
>>> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>
>>> As reported by James, the current libstub code involving the annotated
>>> memory map only works somewhat correctly by accident, due to the fact
>>> that a pool allocation happens to be reused immediately, retaining its
>>> former contents.
>>>
>>> Instead of juggling memory maps, which makes the code more complex than
>>> it needs to be, simply put a placholder value into the FDT, and only
>>> write the actual value after ExitBootServices() has been called.
>>>
>>> Reported-by: James Morse <james.morse@arm.com>
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> [Update mmap-size too, remove updated_fdt()s unused params and header entry]
>>> Signed-off-by: James Morse <james.morse@arm.com>
>>> ---
>>> Hi Ard,
>>>
>>> This is a v2 of your patch that updates the mmap-size too. This solves the
>>> truncated memmap problem I saw with v1 on Seattle.
>>>
>>> The original patch was CC-stable, so I think this should also have:
>>> Cc: <stable@vger.kernel.org>
>>> Fixes: ed9cc156c42f ("efi/libstub: Use efi_exit_boot_services() in FDT")
>>>
>>>
>>> Thanks,
>>>
>>> James
>>>
>>> drivers/firmware/efi/libstub/efistub.h | 8 ----
>>> drivers/firmware/efi/libstub/fdt.c | 75 +++++++++++++++++++++-------------
>>> 2 files changed, 47 insertions(+), 36 deletions(-)
>>
>> Thanks James. I've queued this one up in the 'urgent' queue and tagged
>> it for stable. I'll send it to tip before the end of the week.
>
> Could we fold the hunk below, please?
>
> diff --git a/drivers/firmware/efi/libstub/fdt.c
> b/drivers/firmware/efi/libstub/fdt.c
> index 9b11b0559a23..90ab96845937 100644
> --- a/drivers/firmware/efi/libstub/fdt.c
> +++ b/drivers/firmware/efi/libstub/fdt.c
> @@ -149,26 +149,25 @@ static efi_status_t
> update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
> static efi_status_t update_fdt_memmap(void *fdt, u64 memmap, u32 map_size)
> {
> int node = fdt_path_offset(fdt, "/chosen");
> - efi_status_t status;
> + int err;
>
> if (node < 0)
> return EFI_LOAD_ERROR;
>
> memmap = cpu_to_fdt64(memmap);
> - status = fdt_setprop_inplace(fdt, node, "linux,uefi-mmap-start",
> - &memmap, sizeof(memmap));
> + err = fdt_setprop_inplace(fdt, node, "linux,uefi-mmap-start",
> + &memmap, sizeof(memmap));
>
> if (status)
> return EFI_LOAD_ERROR;
>
> map_size = cpu_to_fdt32(map_size);
> - status = fdt_setprop_inplace(fdt, node, "linux,uefi-mmap-size",
> - &map_size, sizeof(map_size));
> + err = fdt_setprop_inplace(fdt, node, "linux,uefi-mmap-size",
> + &map_size, sizeof(map_size));
>
> if (status)
> return EFI_LOAD_ERROR;
>
> -
> return EFI_SUCCESS;
> }
>
> My mistake, and harmless in practice, but sloppy nonetheless
... but with the 'if (status)' replaced as well, of course (2x)
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-12-20 22:33 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-19 14:24 [PATCH v2] efi/libstub: arm*: Pass latest memory map to the kernel James Morse
2016-12-19 14:24 ` James Morse
[not found] ` <20161219142419.16780-1-james.morse-5wv7dgnIgG8@public.gmane.org>
2016-12-19 21:38 ` Matt Fleming
2016-12-19 21:38 ` Matt Fleming
[not found] ` <20161219213859.GA2225-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
2016-12-20 22:32 ` Ard Biesheuvel
2016-12-20 22:32 ` Ard Biesheuvel
[not found] ` <CAKv+Gu8_8+GdaV86_ctf=PzKxPvT3tbUOcuz5OkyBya5OC+ekw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-12-20 22:33 ` Ard Biesheuvel
2016-12-20 22:33 ` Ard Biesheuvel
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.