| Message ID | 20210707181506.30489-25-brijesh.singh@amd.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | Add AMD Secure Nested Paging (SEV-SNP) Guest Support | expand |
On Wed, Jul 07, 2021 at 01:14:54PM -0500, Brijesh Singh wrote: > From: Michael Roth <michael.roth@amd.com> > > Future patches for SEV-SNP-validated CPUID will also require early > parsing of the EFI configuration. Move the related code into a set of > helpers that can be re-used for that purpose. > > Signed-off-by: Michael Roth <michael.roth@amd.com> > Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> > --- > arch/x86/boot/compressed/Makefile | 1 + > arch/x86/boot/compressed/acpi.c | 124 +++++--------- > arch/x86/boot/compressed/efi-config-table.c | 180 ++++++++++++++++++++ > arch/x86/boot/compressed/misc.h | 50 ++++++ > 4 files changed, 272 insertions(+), 83 deletions(-) > create mode 100644 arch/x86/boot/compressed/efi-config-table.c arch/x86/boot/compressed/efi.c should be good enough. And in general, this patch is hard to review because it does a bunch of things at the same time. You should split it: - the first patch sould carve out only the functionality into helpers without adding or changing the existing functionality. - later ones should add the new functionality, in single logical steps. Some preliminary comments below as far as I can: > diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile > index 431bf7f846c3..b41aecfda49c 100644 > --- a/arch/x86/boot/compressed/Makefile > +++ b/arch/x86/boot/compressed/Makefile > @@ -100,6 +100,7 @@ endif > vmlinux-objs-$(CONFIG_ACPI) += $(obj)/acpi.o > > vmlinux-objs-$(CONFIG_EFI_MIXED) += $(obj)/efi_thunk_$(BITS).o > +vmlinux-objs-$(CONFIG_EFI) += $(obj)/efi-config-table.o > efi-obj-$(CONFIG_EFI_STUB) = $(objtree)/drivers/firmware/efi/libstub/lib.a > > $(obj)/vmlinux: $(vmlinux-objs-y) $(efi-obj-y) FORCE > diff --git a/arch/x86/boot/compressed/acpi.c b/arch/x86/boot/compressed/acpi.c > index 8bcbcee54aa1..e087dcaf43b3 100644 > --- a/arch/x86/boot/compressed/acpi.c > +++ b/arch/x86/boot/compressed/acpi.c > @@ -24,42 +24,36 @@ struct mem_vector immovable_mem[MAX_NUMNODES*2]; > * Search EFI system tables for RSDP. If both ACPI_20_TABLE_GUID and > * ACPI_TABLE_GUID are found, take the former, which has more features. > */ > +#ifdef CONFIG_EFI > +static bool > +rsdp_find_fn(efi_guid_t guid, unsigned long vendor_table, bool efi_64, > + void *opaque) > +{ > + acpi_physical_address *rsdp_addr = opaque; > + > + if (!(efi_guidcmp(guid, ACPI_TABLE_GUID))) { > + *rsdp_addr = vendor_table; > + } else if (!(efi_guidcmp(guid, ACPI_20_TABLE_GUID))) { > + *rsdp_addr = vendor_table; > + return false; No "return false" in the ACPI_TABLE_GUID branch above? Maybe this has to do with the preference to ACPI_20_TABLE_GUID. In any case, this looks silly. Please do the iteration simple and stupid without the function pointer and get rid of that efi_foreach_conf_entry() thing - this is not firmware. > diff --git a/arch/x86/boot/compressed/efi-config-table.c b/arch/x86/boot/compressed/efi-config-table.c > new file mode 100644 > index 000000000000..d1a34aa7cefd > --- /dev/null > +++ b/arch/x86/boot/compressed/efi-config-table.c ... > +/* If you're going to add proper comments, make them kernel-doc. I.e., it should start with /** and then use ./scripts/kernel-doc -none arch/x86/boot/compressed/efi-config-table.c to check them all they're proper. > + * Given boot_params, retrieve the physical address of EFI system table. > + * > + * @boot_params: pointer to boot_params > + * @sys_table_pa: location to store physical address of system table > + * @is_efi_64: location to store whether using 64-bit EFI or not > + * > + * Returns 0 on success. On error, return params are left unchanged. > + */ > +int > +efi_bp_get_system_table(struct boot_params *boot_params, There's no need for the "_bp_" - just efi_get_system_table(). Ditto for the other naming. I'll review the rest properly after you've split it. Thx.
On Thu, Aug 19, 2021 at 12:47:59PM +0200, Borislav Petkov wrote: > On Wed, Jul 07, 2021 at 01:14:54PM -0500, Brijesh Singh wrote: > > From: Michael Roth <michael.roth@amd.com> > > > > Future patches for SEV-SNP-validated CPUID will also require early > > parsing of the EFI configuration. Move the related code into a set of > > helpers that can be re-used for that purpose. > > > > Signed-off-by: Michael Roth <michael.roth@amd.com> > > Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> > > --- > > arch/x86/boot/compressed/Makefile | 1 + > > arch/x86/boot/compressed/acpi.c | 124 +++++--------- > > arch/x86/boot/compressed/efi-config-table.c | 180 ++++++++++++++++++++ > > arch/x86/boot/compressed/misc.h | 50 ++++++ > > 4 files changed, 272 insertions(+), 83 deletions(-) > > create mode 100644 arch/x86/boot/compressed/efi-config-table.c > Hi Boris, Thanks for reviewing. Just FYI, Brijesh is prepping v5 to post soon, and I will work to get all your comments addressed as part of that, but there has also been a change to the CPUID handling in the #VC handlers in case you wanted to wait for that to land. > arch/x86/boot/compressed/efi.c > > should be good enough. > > And in general, this patch is hard to review because it does a bunch of > things at the same time. You should split it: > > - the first patch sould carve out only the functionality into helpers > without adding or changing the existing functionality. > > - later ones should add the new functionality, in single logical steps. Not sure what you mean here. All the interfaces introduced here are used by acpi.c. There is another helper added later (efi_bp_find_vendor_table()) in "enable SEV-SNP-validated CPUID in #VC handler", since it's not used here by acpi.c. > > Some preliminary comments below as far as I can: > > > diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile > > index 431bf7f846c3..b41aecfda49c 100644 > > --- a/arch/x86/boot/compressed/Makefile > > +++ b/arch/x86/boot/compressed/Makefile > > @@ -100,6 +100,7 @@ endif > > vmlinux-objs-$(CONFIG_ACPI) += $(obj)/acpi.o > > > > vmlinux-objs-$(CONFIG_EFI_MIXED) += $(obj)/efi_thunk_$(BITS).o > > +vmlinux-objs-$(CONFIG_EFI) += $(obj)/efi-config-table.o > > efi-obj-$(CONFIG_EFI_STUB) = $(objtree)/drivers/firmware/efi/libstub/lib.a > > > > $(obj)/vmlinux: $(vmlinux-objs-y) $(efi-obj-y) FORCE > > diff --git a/arch/x86/boot/compressed/acpi.c b/arch/x86/boot/compressed/acpi.c > > index 8bcbcee54aa1..e087dcaf43b3 100644 > > --- a/arch/x86/boot/compressed/acpi.c > > +++ b/arch/x86/boot/compressed/acpi.c > > @@ -24,42 +24,36 @@ struct mem_vector immovable_mem[MAX_NUMNODES*2]; > > * Search EFI system tables for RSDP. If both ACPI_20_TABLE_GUID and > > * ACPI_TABLE_GUID are found, take the former, which has more features. > > */ > > +#ifdef CONFIG_EFI > > +static bool > > +rsdp_find_fn(efi_guid_t guid, unsigned long vendor_table, bool efi_64, > > + void *opaque) > > +{ > > + acpi_physical_address *rsdp_addr = opaque; > > + > > + if (!(efi_guidcmp(guid, ACPI_TABLE_GUID))) { > > + *rsdp_addr = vendor_table; > > + } else if (!(efi_guidcmp(guid, ACPI_20_TABLE_GUID))) { > > + *rsdp_addr = vendor_table; > > + return false; > > No "return false" in the ACPI_TABLE_GUID branch above? Maybe this has to > do with the preference to ACPI_20_TABLE_GUID. Right, current acpi.c keeps searching in case the preferred ACPI_20_TABLE_GUID is found. > > In any case, this looks silly. Please do the iteration simple > and stupid without the function pointer and get rid of that > efi_foreach_conf_entry() thing - this is not firmware. There is the aforementioned efi_bp_find_vendor_table() that does the simple iteration, but I wasn't sure how to build the "find one of these, but this one is preferred" logic into it in a reasonable way. I could just call it once for each of these GUIDs though. I was hesitant to do so since it's less efficient than existing code, but if it's worth it for the simplification then I'm all for it. So I'll pull efi_bp_find_vendor_table() into this patch, rename to efi_find_vendor_table(), and drop efi_foreach_conf_entry() in favor of it. > > > diff --git a/arch/x86/boot/compressed/efi-config-table.c b/arch/x86/boot/compressed/efi-config-table.c > > new file mode 100644 > > index 000000000000..d1a34aa7cefd > > --- /dev/null > > +++ b/arch/x86/boot/compressed/efi-config-table.c > > ... > > > +/* > > If you're going to add proper comments, make them kernel-doc. I.e., it > should start with > > /** > > and then use > > ./scripts/kernel-doc -none arch/x86/boot/compressed/efi-config-table.c > > to check them all they're proper. Nice, thanks for the tip. > > > > + * Given boot_params, retrieve the physical address of EFI system table. > > + * > > + * @boot_params: pointer to boot_params > > + * @sys_table_pa: location to store physical address of system table > > + * @is_efi_64: location to store whether using 64-bit EFI or not > > + * > > + * Returns 0 on success. On error, return params are left unchanged. > > + */ > > +int > > +efi_bp_get_system_table(struct boot_params *boot_params, > > There's no need for the "_bp_" - just efi_get_system_table(). Ditto for > the other naming. There used to also be an efi_get_conf_table() that did the lookup given a direct pointer to systab rather than going through bootparams, so I needed some way to differentiate, but looks like I dropped that at some point, so I'll rename these as suggested. > > I'll review the rest properly after you've split it. > > Thx. > > -- > Regards/Gruss, > Boris. > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpeople.kernel.org%2Ftglx%2Fnotes-about-netiquette&data=04%7C01%7Cmichael.roth%40amd.com%7C5a35d1d920024a99451608d962febc38%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637649668538296788%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=ED6tez8A1ktSULe%2FhJTxeqPlp4LVb0Yt4i44P9gytAw%3D&reserved=0
On Thu, Aug 19, 2021 at 09:58:31AM -0500, Michael Roth wrote: > Not sure what you mean here. All the interfaces introduced here are used > by acpi.c. There is another helper added later (efi_bp_find_vendor_table()) > in "enable SEV-SNP-validated CPUID in #VC handler", since it's not used > here by acpi.c. Maybe I got confused by the amount of changes in a single patch. I'll try harder with your v5. :) > There is the aforementioned efi_bp_find_vendor_table() that does the > simple iteration, but I wasn't sure how to build the "find one of these, > but this one is preferred" logic into it in a reasonable way. Instead of efi_foreach_conf_entry() you simply do a bog-down simple loop and each time you stop at a table, you examine it and overwrite pointers, if you've found something better. With "overwrite pointers" I mean you cache the pointers to those conf tables you iterate over and dig out so that you don't have to do it a second time. That is, *if* you need them a second time. I believe you call at least efi_bp_get_conf_table() twice... you get the idea. > I could just call it once for each of these GUIDs though. I was > hesitant to do so since it's less efficient than existing code, but if > it's worth it for the simplification then I'm all for it. Yeah, this is executed once during boot so I don't think you can make it more efficient than a single iteration over the config table blobs. I hope that makes more sense.
On Thu, Aug 19, 2021 at 07:09:42PM +0200, Borislav Petkov wrote: > On Thu, Aug 19, 2021 at 09:58:31AM -0500, Michael Roth wrote: > > Not sure what you mean here. All the interfaces introduced here are used > > by acpi.c. There is another helper added later (efi_bp_find_vendor_table()) > > in "enable SEV-SNP-validated CPUID in #VC handler", since it's not used > > here by acpi.c. > > Maybe I got confused by the amount of changes in a single patch. I'll > try harder with your v5. :) > > > There is the aforementioned efi_bp_find_vendor_table() that does the > > simple iteration, but I wasn't sure how to build the "find one of these, > > but this one is preferred" logic into it in a reasonable way. > > Instead of efi_foreach_conf_entry() you simply do a bog-down simple > loop and each time you stop at a table, you examine it and overwrite > pointers, if you've found something better. > > With "overwrite pointers" I mean you cache the pointers to those conf > tables you iterate over and dig out so that you don't have to do it a > second time. That is, *if* you need them a second time. I believe you > call at least efi_bp_get_conf_table() twice... you get the idea. Sorry I'm still a little confused on how to determine "something better", since it's acpi.c that decides which GUID is preferred, whereas efi_find_vendor_table() is a library function with no outside knowledge other than its arguments, so to return the preferred pointer it would need both/multiple GUIDs passed in as arguments, wouldn't it? (or a callback) Another alternative is something like what drivers/firmware/efi/efi.c:common_tables does, where interesting table GUIDs are each associated with a pointer, and all the pointers can then be initialized with to the corresponding table address with a single pass. But would need to be careful to re-initialize those pointers when BSS gets cleared, or declare them in __section(".data"). Is that closer to what you were thinking? > > > I could just call it once for each of these GUIDs though. I was > > hesitant to do so since it's less efficient than existing code, but if > > it's worth it for the simplification then I'm all for it. > > Yeah, this is executed once during boot so I don't think you can make it > more efficient than a single iteration over the config table blobs. In v5, I've simplified things to just call efi_find_vendor_table() once for ACPI_20_TABLE_GUID, then once for ACPI_TABLE_GUID if that's not available. So definitely doesn't sound like what you are suggesting here, but does at least simplify code and gets rid of the efi_foreach* stuff. But happy to rework things if you had something else in mind. > > I hope that makes more sense. > > -- > Regards/Gruss, > Boris. > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpeople.kernel.org%2Ftglx%2Fnotes-about-netiquette&data=04%7C01%7Cmichael.roth%40amd.com%7C2a4304e70b5b4f2137f808d963340eec%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637649897546039774%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=GKDogD%2BOCN0swhmT4RZ2%2B3JmURF3e4qq%2FzgrxxFqJt0%3D&reserved=0
On Thu, Aug 19, 2021 at 06:42:58PM -0500, Michael Roth wrote: > In v5, I've simplified things to just call efi_find_vendor_table() once > for ACPI_20_TABLE_GUID, then once for ACPI_TABLE_GUID if that's not > available. So definitely doesn't sound like what you are suggesting here, > but does at least simplify code and gets rid of the efi_foreach* stuff. But > happy to rework things if you had something else in mind. Ok, thanks. Lemme get to that version and I'll holler if something's still bothering me. Thx.
diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile index 431bf7f846c3..b41aecfda49c 100644 --- a/arch/x86/boot/compressed/Makefile +++ b/arch/x86/boot/compressed/Makefile @@ -100,6 +100,7 @@ endif vmlinux-objs-$(CONFIG_ACPI) += $(obj)/acpi.o vmlinux-objs-$(CONFIG_EFI_MIXED) += $(obj)/efi_thunk_$(BITS).o +vmlinux-objs-$(CONFIG_EFI) += $(obj)/efi-config-table.o efi-obj-$(CONFIG_EFI_STUB) = $(objtree)/drivers/firmware/efi/libstub/lib.a $(obj)/vmlinux: $(vmlinux-objs-y) $(efi-obj-y) FORCE diff --git a/arch/x86/boot/compressed/acpi.c b/arch/x86/boot/compressed/acpi.c index 8bcbcee54aa1..e087dcaf43b3 100644 --- a/arch/x86/boot/compressed/acpi.c +++ b/arch/x86/boot/compressed/acpi.c @@ -24,42 +24,36 @@ struct mem_vector immovable_mem[MAX_NUMNODES*2]; * Search EFI system tables for RSDP. If both ACPI_20_TABLE_GUID and * ACPI_TABLE_GUID are found, take the former, which has more features. */ +#ifdef CONFIG_EFI +static bool +rsdp_find_fn(efi_guid_t guid, unsigned long vendor_table, bool efi_64, + void *opaque) +{ + acpi_physical_address *rsdp_addr = opaque; + + if (!(efi_guidcmp(guid, ACPI_TABLE_GUID))) { + *rsdp_addr = vendor_table; + } else if (!(efi_guidcmp(guid, ACPI_20_TABLE_GUID))) { + *rsdp_addr = vendor_table; + return false; + } + + return true; +} +#endif + static acpi_physical_address -__efi_get_rsdp_addr(unsigned long config_tables, unsigned int nr_tables, +__efi_get_rsdp_addr(unsigned long config_table_pa, unsigned int config_table_len, bool efi_64) { acpi_physical_address rsdp_addr = 0; - #ifdef CONFIG_EFI - int i; - - /* Get EFI tables from systab. */ - for (i = 0; i < nr_tables; i++) { - acpi_physical_address table; - efi_guid_t guid; - - if (efi_64) { - efi_config_table_64_t *tbl = (efi_config_table_64_t *)config_tables + i; - - guid = tbl->guid; - table = tbl->table; - - if (!IS_ENABLED(CONFIG_X86_64) && table >> 32) { - debug_putstr("Error getting RSDP address: EFI config table located above 4GB.\n"); - return 0; - } - } else { - efi_config_table_32_t *tbl = (efi_config_table_32_t *)config_tables + i; - - guid = tbl->guid; - table = tbl->table; - } + int ret; - if (!(efi_guidcmp(guid, ACPI_TABLE_GUID))) - rsdp_addr = table; - else if (!(efi_guidcmp(guid, ACPI_20_TABLE_GUID))) - return table; - } + ret = efi_foreach_conf_entry((void *)config_table_pa, config_table_len, + efi_64, rsdp_find_fn, &rsdp_addr); + if (ret) + debug_putstr("Error getting RSDP address.\n"); #endif return rsdp_addr; } @@ -87,7 +81,9 @@ static acpi_physical_address kexec_get_rsdp_addr(void) efi_system_table_64_t *systab; struct efi_setup_data *esd; struct efi_info *ei; + bool efi_64; char *sig; + int ret; esd = (struct efi_setup_data *)get_kexec_setup_data_addr(); if (!esd) @@ -98,18 +94,16 @@ static acpi_physical_address kexec_get_rsdp_addr(void) return 0; } - ei = &boot_params->efi_info; - sig = (char *)&ei->efi_loader_signature; - if (strncmp(sig, EFI64_LOADER_SIGNATURE, 4)) { + /* Get systab from boot params. */ + ret = efi_bp_get_system_table(boot_params, (unsigned long *)&systab, &efi_64); + if (ret) + error("EFI system table not found in kexec boot_params."); + + if (!efi_64) { debug_putstr("Wrong kexec EFI loader signature.\n"); return 0; } - /* Get systab from boot params. */ - systab = (efi_system_table_64_t *) (ei->efi_systab | ((__u64)ei->efi_systab_hi << 32)); - if (!systab) - error("EFI system table not found in kexec boot_params."); - return __efi_get_rsdp_addr((unsigned long)esd->tables, systab->nr_tables, true); } #else @@ -119,54 +113,18 @@ static acpi_physical_address kexec_get_rsdp_addr(void) { return 0; } static acpi_physical_address efi_get_rsdp_addr(void) { #ifdef CONFIG_EFI - unsigned long systab, config_tables; - unsigned int nr_tables; - struct efi_info *ei; + unsigned long config_table_pa = 0; + unsigned int config_table_len; bool efi_64; - char *sig; - - ei = &boot_params->efi_info; - sig = (char *)&ei->efi_loader_signature; - - if (!strncmp(sig, EFI64_LOADER_SIGNATURE, 4)) { - efi_64 = true; - } else if (!strncmp(sig, EFI32_LOADER_SIGNATURE, 4)) { - efi_64 = false; - } else { - debug_putstr("Wrong EFI loader signature.\n"); - return 0; - } - - /* Get systab from boot params. */ -#ifdef CONFIG_X86_64 - systab = ei->efi_systab | ((__u64)ei->efi_systab_hi << 32); -#else - if (ei->efi_systab_hi || ei->efi_memmap_hi) { - debug_putstr("Error getting RSDP address: EFI system table located above 4GB.\n"); - return 0; - } - systab = ei->efi_systab; -#endif - if (!systab) - error("EFI system table not found."); - - /* Handle EFI bitness properly */ - if (efi_64) { - efi_system_table_64_t *stbl = (efi_system_table_64_t *)systab; - - config_tables = stbl->tables; - nr_tables = stbl->nr_tables; - } else { - efi_system_table_32_t *stbl = (efi_system_table_32_t *)systab; - - config_tables = stbl->tables; - nr_tables = stbl->nr_tables; - } + int ret; - if (!config_tables) - error("EFI config tables not found."); + ret = efi_bp_get_conf_table(boot_params, &config_table_pa, + &config_table_len, &efi_64); + if (ret || !config_table_pa) + error("EFI config table not found."); - return __efi_get_rsdp_addr(config_tables, nr_tables, efi_64); + return __efi_get_rsdp_addr(config_table_pa, config_table_len, + efi_64); #else return 0; #endif diff --git a/arch/x86/boot/compressed/efi-config-table.c b/arch/x86/boot/compressed/efi-config-table.c new file mode 100644 index 000000000000..d1a34aa7cefd --- /dev/null +++ b/arch/x86/boot/compressed/efi-config-table.c @@ -0,0 +1,180 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Helpers for early access to EFI configuration table + * + * Copyright (C) 2021 Advanced Micro Devices, Inc. + * + * Author: Michael Roth <michael.roth@amd.com> + */ + +#include "misc.h" +#include <linux/efi.h> +#include <asm/efi.h> + +/* Get vendor table address/guid from EFI config table at the given index */ +static int get_vendor_table(void *conf_table, unsigned int idx, + unsigned long *vendor_table_pa, + efi_guid_t *vendor_table_guid, + bool efi_64) +{ + if (efi_64) { + efi_config_table_64_t *table_entry = + (efi_config_table_64_t *)conf_table + idx; + + if (!IS_ENABLED(CONFIG_X86_64) && + table_entry->table >> 32) { + debug_putstr("Error: EFI config table entry located above 4GB.\n"); + return -EINVAL; + } + + *vendor_table_pa = table_entry->table; + *vendor_table_guid = table_entry->guid; + + } else { + efi_config_table_32_t *table_entry = + (efi_config_table_32_t *)conf_table + idx; + + *vendor_table_pa = table_entry->table; + *vendor_table_guid = table_entry->guid; + } + + return 0; +} + +/* + * Iterate through the entries in the EFI configuration table and pass the + * associated GUID/physical address of each entry on the provided callback + * function. + * + * @conf_table: pointer to EFI configuration table + * @conf_table_len: number of entries in EFI configuration table + * @efi_64: true if using 64-bit EFI + * @fn: callback function that returns true if iteration + * should continue + * @opaque: optional caller-provided data structure to pass to + * callback function on each iteration + * + * Returns 0 on success. + */ +int +efi_foreach_conf_entry(void *conf_table, unsigned int conf_table_len, + bool efi_64, bool (*fn)(efi_guid_t vendor_table_guid, + unsigned long vendor_table_pa, + bool efi_64, + void *opaque), + void *opaque) +{ + unsigned int i; + + for (i = 0; i < conf_table_len; i++) { + unsigned long vendor_table_pa; + efi_guid_t vendor_table_guid; + + if (get_vendor_table(conf_table, i, &vendor_table_pa, + &vendor_table_guid, efi_64)) + return -EINVAL; + + if (!fn(vendor_table_guid, vendor_table_pa, efi_64, opaque)) + break; + } + + return 0; +} + +/* + * Given boot_params, retrieve the physical address of EFI system table. + * + * @boot_params: pointer to boot_params + * @sys_table_pa: location to store physical address of system table + * @is_efi_64: location to store whether using 64-bit EFI or not + * + * Returns 0 on success. On error, return params are left unchanged. + */ +int +efi_bp_get_system_table(struct boot_params *boot_params, + unsigned long *sys_table_pa, bool *is_efi_64) +{ + unsigned long sys_table; + struct efi_info *ei; + bool efi_64; + char *sig; + + if (!sys_table_pa || !is_efi_64) + return -EINVAL; + + ei = &boot_params->efi_info; + sig = (char *)&ei->efi_loader_signature; + + if (!strncmp(sig, EFI64_LOADER_SIGNATURE, 4)) { + efi_64 = true; + } else if (!strncmp(sig, EFI32_LOADER_SIGNATURE, 4)) { + efi_64 = false; + } else { + debug_putstr("Wrong EFI loader signature.\n"); + return -ENOENT; + } + + /* Get systab from boot params. */ +#ifdef CONFIG_X86_64 + sys_table = ei->efi_systab | ((__u64)ei->efi_systab_hi << 32); +#else + if (ei->efi_systab_hi || ei->efi_memmap_hi) { + debug_putstr("Error: EFI system table located above 4GB.\n"); + return -EINVAL; + } + sys_table = ei->efi_systab; +#endif + if (!sys_table) { + debug_putstr("EFI system table not found."); + return -ENOENT; + } + + *sys_table_pa = sys_table; + *is_efi_64 = efi_64; + return 0; +} + +/* + * Given boot_params, locate EFI system table from it and return the physical + * address EFI configuration table. + * + * @boot_params: pointer to boot_params + * @conf_table_pa: location to store physical address of config table + * @conf_table_len: location to store number of config table entries + * @is_efi_64: location to store whether using 64-bit EFI or not + * + * Returns 0 on success. On error, return params are left unchanged. + */ +int +efi_bp_get_conf_table(struct boot_params *boot_params, + unsigned long *conf_table_pa, + unsigned int *conf_table_len, + bool *is_efi_64) +{ + unsigned long sys_table_pa = 0; + int ret; + + if (!conf_table_pa || !conf_table_len || !is_efi_64) + return -EINVAL; + + ret = efi_bp_get_system_table(boot_params, &sys_table_pa, is_efi_64); + if (ret) + return ret; + + /* Handle EFI bitness properly */ + if (*is_efi_64) { + efi_system_table_64_t *stbl = + (efi_system_table_64_t *)sys_table_pa; + + *conf_table_pa = stbl->tables; + *conf_table_len = stbl->nr_tables; + } else { + efi_system_table_32_t *stbl = + (efi_system_table_32_t *)sys_table_pa; + + *conf_table_pa = stbl->tables; + *conf_table_len = stbl->nr_tables; + } + + return 0; +} diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h index 822e0c254b9a..522baf8ff04a 100644 --- a/arch/x86/boot/compressed/misc.h +++ b/arch/x86/boot/compressed/misc.h @@ -21,6 +21,7 @@ #include <linux/screen_info.h> #include <linux/elf.h> #include <linux/io.h> +#include <linux/efi.h> #include <asm/page.h> #include <asm/boot.h> #include <asm/bootparam.h> @@ -174,4 +175,53 @@ void boot_stage2_vc(void); unsigned long sev_verify_cbit(unsigned long cr3); +#ifdef CONFIG_EFI +/* helpers for early EFI config table access */ +int efi_foreach_conf_entry(void *conf_table, unsigned int conf_table_len, + bool efi_64, + bool (*fn)(efi_guid_t guid, + unsigned long vendor_table_pa, + bool efi_64, + void *opaque), + void *opaque); + +int efi_bp_get_system_table(struct boot_params *boot_params, + unsigned long *sys_table_pa, + bool *is_efi_64); + +int efi_bp_get_conf_table(struct boot_params *boot_params, + unsigned long *conf_table_pa, + unsigned int *conf_table_len, + bool *is_efi_64); +#else +static inline int +efi_foreach_conf_entry(void *conf_table, unsigned int conf_table_len, + bool efi_64, + bool (*fn)(efi_guid_t guid, + unsigned long vendor_table_pa, + bool efi_64, + void *opaque), + void *opaque); +{ + return -ENOENT; +} + +static inline int +efi_bp_get_system_table(struct boot_params *boot_params, + unsigned long *sys_table_pa, + bool *is_efi_64) +{ + return -ENOENT; +} + +static inline int +efi_bp_get_conf_table(struct boot_params *boot_params, + unsigned long *conf_table_pa, + unsigned int *conf_table_len, + bool *is_efi_64) +{ + return -ENOENT; +} +#endif /* CONFIG_EFI */ + #endif /* BOOT_COMPRESSED_MISC_H */