Messages in this thread | | | Subject | Re: [PATCH v7 2/8] efi: Add embedded peripheral firmware support | From | Hans de Goede <> | Date | Thu, 14 Nov 2019 21:13:22 +0100 |
| |
Hi,
On 14-11-2019 20:42, Luis Chamberlain wrote: > On Thu, Nov 14, 2019 at 12:27:01PM +0100, Hans de Goede wrote: >> Hi Luis, >> >> Thank you for the reviews and sorry for being a bit slow to respind. >> >> On 11-10-2019 16:48, Luis Chamberlain wrote: >>> On Fri, Oct 04, 2019 at 04:50:50PM +0200, Hans de Goede wrote: >>>> +static int __init efi_check_md_for_embedded_firmware( >>>> + efi_memory_desc_t *md, const struct efi_embedded_fw_desc *desc) >>>> +{ >>>> + const u64 prefix = *((u64 *)desc->prefix); >>>> + struct sha256_state sctx; >>>> + struct embedded_fw *fw; >>>> + u8 sha256[32]; >>>> + u64 i, size; >>>> + void *map; >>>> + >>>> + size = md->num_pages << EFI_PAGE_SHIFT; >>>> + map = memremap(md->phys_addr, size, MEMREMAP_WB); >>> >>> Since our limitaiton is the init process must have mostly finished, >>> it implies early x86 boot code cannot use this, what measures can we >>> take to prevent / check for such conditions to be detected and >>> gracefully errored out? >> >> As with all (EFI) early boot code, there simply is a certain order >> in which things need to be done. This needs to happen after the basic >> mm is setup, but before efi_free_boot_services() gets called, there >> isn't really a way to check for all these conditions. As with all >> early boot code, people making changes need to be careful to not >> break stuff. > > I rather we take a proactive measure here and add whatever it is we need > to ensure the API works only when its supposed to, rather than try and > fail, and then expect the user to know these things. > > I'd prefer if we at least try to address this.
This is purely internal x86/EFI API it is not intended for drivers or anything like that. It has only one caller under arch/x86 and it is not supposed to get any other callers outside of arch/* ever.
Note that this all runs before even core_initcall-s get run, none if the code which runs before then has any sort of ordering checks and I don't see how this bit is special and thus does need ordering checks; and there really is no mechanism for such checks so early during boot.
The drivers/firmware/efi/embedded-firmware.c file does add some API which can be used normally, specifically the efi_get_embedded_fw() but that has no special ordering constrains and it does not directly use the function we are discussing now. It reads back data stored by the earlier functions; and if somehow called before those functions run (*), then it will simply return -ENOENT.
Regards,
Hans
*) which would mean before core_initcalls run so really really early
| |