On Thu, 21 Jul, at 02:28:11PM, Jeffrey Hugo wrote:
> efi_get_memory_map() allocates a buffer to store the memory map that it
> retrieves. This buffer may need to be reused by the client after
> ExitBootServices() is called, at which point allocations are not longer
> permitted. To support this usecase, provide the allocated buffer size back
> to the client, and allocate some additional headroom to account for any
> reasonable growth in the map that is likely to happen between the call to
> efi_get_memory_map() and the client reusing the buffer.
>
> Change-Id: Ib0686811581c59eee2eb60b4b62e1628e649d6f0
Please don't include these tags in your patch submission - they don't
mean anything in the upstream kernel and there's always the chance
I'll forget to strip it before applying your patch.
> Signed-off-by: Jeffrey Hugo <jhugo@xxxxxxxxxxxxxx>
> ---
> arch/x86/boot/compressed/eboot.c | 4 +--
> drivers/firmware/efi/libstub/efi-stub-helper.c | 36 +++++++++++++++++++-------
> drivers/firmware/efi/libstub/fdt.c | 8 +++---
> drivers/firmware/efi/libstub/random.c | 3 ++-
> include/linux/efi.h | 3 ++-
> 5 files changed, 37 insertions(+), 17 deletions(-)
[...]
> index 3bd127f9..3071269 100644
> --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
> +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
> @@ -41,6 +41,8 @@ static unsigned long __chunk_size = EFI_READ_CHUNK_SIZE;
> #define EFI_ALLOC_ALIGN EFI_PAGE_SIZE
> #endif
>
> +#define EFI_MMAP_NR_SLACK_SLOTS 8
> +
> struct file_info {
> efi_file_handle_t *handle;
> u64 size;
> @@ -68,20 +70,24 @@ efi_status_t efi_get_memory_map(efi_system_table_t *sys_table_arg,
> unsigned long *map_size,
> unsigned long *desc_size,
> u32 *desc_ver,
> - unsigned long *key_ptr)
> + unsigned long *key_ptr,
> + unsigned long *buff_size)
> {
> efi_memory_desc_t *m = NULL;
> efi_status_t status;
> unsigned long key;
> u32 desc_version;
>
> - *map_size = sizeof(*m) * 32;
> + *desc_size = sizeof(*m);
> + *map_size = *desc_size * 32;
> + *buff_size = *map_size;
> again:
> /*
> * Add an additional efi_memory_desc_t because we're doing an
> * allocation which may be in a new descriptor region.
> */
> - *map_size += sizeof(*m);
> + *map_size += *desc_size;
> + *buff_size = *map_size;
> status = efi_call_early(allocate_pool, EFI_LOADER_DATA,
> *map_size, (void **)&m);
> if (status != EFI_SUCCESS)
Isn't this chunk of code unnecessary now? If we think 8 entries is
enough headroom for all scenarios then there's no need to allocate 9.
> @@ -91,8 +97,17 @@ again:
> key = 0;
> status = efi_call_early(get_memory_map, map_size, m,
> &key, desc_size, &desc_version);
> - if (status == EFI_BUFFER_TOO_SMALL) {
> + if (status == EFI_BUFFER_TOO_SMALL ||
> + (*buff_size - *map_size) / *desc_size < 8) {
Please pull this expression into a static inline wrapper, e.g.
static inline bool mmap_has_headroom(unsigned long buff_size,
unsigned long map_size,
unsigned long desc_size)
{
unsigned long slack = buff_size - map_size;
return slack / desc_size >= EFI_MMAP_NR_SLACK_SLOTS;
}
...
if (status == EFI_BUFFER_TOO_SMALL ||
!mmap_has_headroom(*buff_size, *map_size, *desc_size)) {
--
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
