Messages in this thread Patch in this message | | | Date | Fri, 2 Jun 2023 16:06:41 +0200 | From | Borislav Petkov <> | Subject | Re: [PATCHv13 4/9] x86/boot/compressed: Handle unaccepted memory |
| |
On Thu, Jun 01, 2023 at 09:25:38PM +0300, Kirill A. Shutemov wrote: > diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c > index 454757fbdfe5..749f0fe7e446 100644 > --- a/arch/x86/boot/compressed/kaslr.c > +++ b/arch/x86/boot/compressed/kaslr.c > @@ -672,6 +672,28 @@ static bool process_mem_region(struct mem_vector *region, > } > > #ifdef CONFIG_EFI > + > +/* > + * Only EFI_CONVENTIONAL_MEMORY and EFI_UNACCEPTED_MEMORY (if supported) are > + * guaranteed to be free. > + * > + * It is more conservative in picking free memory than the EFI spec allows:
"Pick free memory more conservatively than the EFI spec allows: EFI_BOOT_SERVICES_ ..."
> + * > + * According to the spec, EFI_BOOT_SERVICES_{CODE|DATA} are also free memory > + * and thus available to place the kernel image into, but in practice there's > + * firmware where using that memory leads to crashes.
... because that firmware still scratches into that memory or why?
> + */ > +static inline bool memory_type_is_free(efi_memory_desc_t *md) > +{ > + if (md->type == EFI_CONVENTIONAL_MEMORY) > + return true; > + > + if (md->type == EFI_UNACCEPTED_MEMORY) > + return IS_ENABLED(CONFIG_UNACCEPTED_MEMORY);
Make it plan and simple:
if (IS_ENABLED(CONFIG_UNACCEPTED_MEMORY) && md->type == EFI_UNACCEPTED_MEMORY) return true;
> + > + return false; > +} > + > /* > * Returns true if we processed the EFI memmap, which we prefer over the E820 > * table if it is available. > @@ -716,18 +738,7 @@ process_efi_entries(unsigned long minimum, unsigned long image_size) > for (i = 0; i < nr_desc; i++) { > md = efi_early_memdesc_ptr(pmap, e->efi_memdesc_size, i); > > - /* > - * Here we are more conservative in picking free memory than > - * the EFI spec allows: > - * > - * According to the spec, EFI_BOOT_SERVICES_{CODE|DATA} are also > - * free memory and thus available to place the kernel image into, > - * but in practice there's firmware where using that memory leads > - * to crashes. > - * > - * Only EFI_CONVENTIONAL_MEMORY is guaranteed to be free. > - */ > - if (md->type != EFI_CONVENTIONAL_MEMORY) > + if (!memory_type_is_free(md)) > continue; > > if (efi_soft_reserve_enabled() && > diff --git a/arch/x86/boot/compressed/mem.c b/arch/x86/boot/compressed/mem.c > index 67594fcb11d9..4ecf26576a77 100644 > --- a/arch/x86/boot/compressed/mem.c > +++ b/arch/x86/boot/compressed/mem.c > @@ -1,9 +1,40 @@ > // SPDX-License-Identifier: GPL-2.0-only > > #include "error.h" > +#include "misc.h" > > void arch_accept_memory(phys_addr_t start, phys_addr_t end) > { > /* Platform-specific memory-acceptance call goes here */ > error("Cannot accept memory"); > } > + > +void init_unaccepted_memory(void) > +{ > + guid_t guid = LINUX_EFI_UNACCEPTED_MEM_TABLE_GUID;
An additional space after the "=".
> + struct efi_unaccepted_memory *unaccepted_table; > + unsigned long cfg_table_pa; > + unsigned int cfg_table_len; > + enum efi_type et; > + int ret; > + > + et = efi_get_type(boot_params); > + if (et == EFI_TYPE_NONE) > + return; > + > + ret = efi_get_conf_table(boot_params, &cfg_table_pa, &cfg_table_len); > + if (ret) > + error("EFI config table not found.");
What's the point in erroring out here?
> + unaccepted_table = (void *)efi_find_vendor_table(boot_params, > + cfg_table_pa, > + cfg_table_len, > + guid); > + if (!unaccepted_table) > + return; > + > + if (unaccepted_table->version != 1) > + error("Unknown version of unaccepted memory table\n"); > + > + set_unaccepted_table(unaccepted_table);
Why is this a function at all and not a simple assignment?
> diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c > index 014ff222bf4b..36535a3753f5 100644 > --- a/arch/x86/boot/compressed/misc.c > +++ b/arch/x86/boot/compressed/misc.c > @@ -455,6 +455,13 @@ asmlinkage __visible void *extract_kernel(void *rmode, memptr heap, > #endif > > debug_putstr("\nDecompressing Linux... "); > + > + if (IS_ENABLED(CONFIG_UNACCEPTED_MEMORY)) { > + debug_putstr("Accepting memory... ");
This needs to happen...
> + init_unaccepted_memory();
... after the init, after the init has parsed the config table and has found unaccepted memory.
If not, you don't need to issue anything as that would be wrong.
> + accept_memory(__pa(output), __pa(output) + needed_size); > + } > + > __decompress(input_data, input_len, NULL, NULL, output, output_len, > NULL, error); > entry_offset = parse_elf(output);
-- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
| |