diff mbox series

[v8,24/40] x86/compressed/acpi: move EFI system table lookup to helper

Message ID 20211210154332.11526-25-brijesh.singh@amd.com (mailing list archive)
State New, archived
Headers show
Series Add AMD Secure Nested Paging (SEV-SNP) Guest Support | expand

Commit Message

Brijesh Singh Dec. 10, 2021, 3:43 p.m. UTC
From: Michael Roth <michael.roth@amd.com>

Future patches for SEV-SNP-validated CPUID will also require early
parsing of the EFI configuration. Incrementally 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   | 60 ++++++++++----------------
 arch/x86/boot/compressed/efi.c    | 72 +++++++++++++++++++++++++++++++
 arch/x86/boot/compressed/misc.h   | 14 ++++++
 4 files changed, 109 insertions(+), 38 deletions(-)
 create mode 100644 arch/x86/boot/compressed/efi.c

Comments

Dave Hansen Dec. 10, 2021, 6:54 p.m. UTC | #1
On 12/10/21 7:43 AM, Brijesh Singh wrote:
> +/*
> + * Helpers for early access to EFI configuration table
> + *
> + * Copyright (C) 2021 Advanced Micro Devices, Inc.
> + *
> + * Author: Michael Roth <michael.roth@amd.com>
> + */

It doesn't seem quite right to slap this copyright on a file that's full
of content that came from other files.  It would be one thing if
arch/x86/boot/compressed/acpi.c had this banner in it already.  Also, a
bunch of the lines in this file seem to come from:

	commit 33f0df8d843deb9ec24116dcd79a40ca0ea8e8a9
	Author: Chao Fan <fanc.fnst@cn.fujitsu.com>
	Date:   Wed Jan 23 19:08:46 2019 +0800
Michael Roth Dec. 13, 2021, 3:47 p.m. UTC | #2
On Fri, Dec 10, 2021 at 10:54:35AM -0800, Dave Hansen wrote:
> On 12/10/21 7:43 AM, Brijesh Singh wrote:
> > +/*
> > + * Helpers for early access to EFI configuration table
> > + *
> > + * Copyright (C) 2021 Advanced Micro Devices, Inc.
> > + *
> > + * Author: Michael Roth <michael.roth@amd.com>
> > + */
> 
> It doesn't seem quite right to slap this copyright on a file that's full
> of content that came from other files.  It would be one thing if
> arch/x86/boot/compressed/acpi.c had this banner in it already.  Also, a

Yah, acpi.c didn't have any copyright banner so I used my 'default'
template for new files here to cover any additions, but that does give
a misleading impression.

I'm not sure how this is normally addressed, but I'm planning on just
continuing the acpi.c tradition of *not* adding copyright notices for new
code, and simply document that the contents of the file are mostly movement
from acpi.c

> arch/x86/boot/compressed/acpi.c had this banner in it already.  Also, a
> bunch of the lines in this file seem to come from:
> 
> 	commit 33f0df8d843deb9ec24116dcd79a40ca0ea8e8a9
> 	Author: Chao Fan <fanc.fnst@cn.fujitsu.com>
> 	Date:   Wed Jan 23 19:08:46 2019 +0800

AFAICT the full author list for the changes in question are, in
alphabetical order:

  Chao Fan <fanc.fnst@cn.fujitsu.com>
  Junichi Nomura <j-nomura@ce.jp.nec.com>
  Borislav Petkov <bp@suse.de>

Chao, Junichi, Borislav,

If you would like to be listed as an author in efi.c (which is mainly just a
movement of EFI config table parsing code from acpi.c into re-usable helper
functions in efi.c), please let me know and I'll add you.

Otherwise, I'll plan on adopting the acpi.c precedent for this as well, which
is to not list individual authors, since it doesn't seem right to add Author
fields retroactively without their permission.
Dave Hansen Dec. 13, 2021, 4:21 p.m. UTC | #3
On 12/13/21 7:47 AM, Michael Roth wrote:
> Otherwise, I'll plan on adopting the acpi.c precedent for this as well, which
> is to not list individual authors, since it doesn't seem right to add Author
> fields retroactively without their permission.

That's fine with me, especially if it follows precedent in the subsystem.

Could you also please take a quick scan over the rest of the series to
make sure there are no more of these?
Michael Roth Dec. 13, 2021, 6 p.m. UTC | #4
On Mon, Dec 13, 2021 at 08:21:44AM -0800, Dave Hansen wrote:
> On 12/13/21 7:47 AM, Michael Roth wrote:
> > Otherwise, I'll plan on adopting the acpi.c precedent for this as well, which
> > is to not list individual authors, since it doesn't seem right to add Author
> > fields retroactively without their permission.
> 
> That's fine with me, especially if it follows precedent in the subsystem.
> 
> Could you also please take a quick scan over the rest of the series to
> make sure there are no more of these?

Outside of the guest driver there's only one other new file addition in
the series:

  arch/x86/include/asm/cpuid.h

where I moved some code out of arch/x86/kvm/cpuid.c in similar fashion, so
cpuid.h should probably inherit cpuid.c's copyright banner. I'll make that
change for the next spin as well. Thanks for the catches.
Borislav Petkov Jan. 5, 2022, 11:50 p.m. UTC | #5
On Fri, Dec 10, 2021 at 09:43:16AM -0600, Brijesh Singh wrote:
> +int efi_get_system_table(struct boot_params *boot_params, unsigned long *sys_tbl_pa,
> +			 bool *is_efi_64)

Nah, that's doing two things in one function.

The signature should be a lot simpler:

unsigned long efi_get_system_table(struct boot_params *bp)

returns 0 on failure, non-null on success and then it returns the
physical address of the system table.

> +{
> +	unsigned long sys_tbl;
> +	struct efi_info *ei;
> +	bool efi_64;
> +	char *sig;
> +
> +	if (!sys_tbl_pa || !is_efi_64)
> +		return -EINVAL;
> +

This...

> +	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 -EOPNOTSUPP;
> +	}

... up to here needs to be another function:

enum get_efi_type(sig)

where enum is { EFI64, EFI32, INVALID } or so.

And you call this function at the call sites:

	if (efi_get_type(sig) == INVALID)
		error...

	sys_tbl_pa = efi_get_system_table(bp);
	if (!sys_tbl_pa)
		error...


Completely pseudo but you get the idea.

Thx.
Venu Busireddy Jan. 6, 2022, 7:59 p.m. UTC | #6
On 2021-12-10 09:43:16 -0600, 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. Incrementally 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>

Reviewed-by: Venu Busireddy <venu.busireddy@oracle.com>

> ---
>  arch/x86/boot/compressed/Makefile |  1 +
>  arch/x86/boot/compressed/acpi.c   | 60 ++++++++++----------------
>  arch/x86/boot/compressed/efi.c    | 72 +++++++++++++++++++++++++++++++
>  arch/x86/boot/compressed/misc.h   | 14 ++++++
>  4 files changed, 109 insertions(+), 38 deletions(-)
>  create mode 100644 arch/x86/boot/compressed/efi.c
> 
> diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
> index 431bf7f846c3..d364192c2367 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.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..9e784bd7b2e6 100644
> --- a/arch/x86/boot/compressed/acpi.c
> +++ b/arch/x86/boot/compressed/acpi.c
> @@ -86,8 +86,8 @@ static acpi_physical_address kexec_get_rsdp_addr(void)
>  {
>  	efi_system_table_64_t *systab;
>  	struct efi_setup_data *esd;
> -	struct efi_info *ei;
> -	char *sig;
> +	bool efi_64;
> +	int ret;
>  
>  	esd = (struct efi_setup_data *)get_kexec_setup_data_addr();
>  	if (!esd)
> @@ -98,18 +98,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_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,45 +117,31 @@ 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 long systab_tbl_pa, config_tables;
>  	unsigned int nr_tables;
> -	struct efi_info *ei;
>  	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;
> -	}
> +	int ret;
>  
> -	/* 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");
> +	/*
> +	 * This function is called even for non-EFI BIOSes, and callers expect
> +	 * failure to locate the EFI system table to result in 0 being returned
> +	 * as indication that EFI is not available, rather than outright
> +	 * failure/abort.
> +	 */
> +	ret = efi_get_system_table(boot_params, &systab_tbl_pa, &efi_64);
> +	if (ret == -EOPNOTSUPP)
>  		return 0;
> -	}
> -	systab = ei->efi_systab;
> -#endif
> -	if (!systab)
> -		error("EFI system table not found.");
> +	if (ret)
> +		error("EFI support advertised, but unable to locate system table.");
>  
>  	/* Handle EFI bitness properly */
>  	if (efi_64) {
> -		efi_system_table_64_t *stbl = (efi_system_table_64_t *)systab;
> +		efi_system_table_64_t *stbl = (efi_system_table_64_t *)systab_tbl_pa;
>  
>  		config_tables	= stbl->tables;
>  		nr_tables	= stbl->nr_tables;
>  	} else {
> -		efi_system_table_32_t *stbl = (efi_system_table_32_t *)systab;
> +		efi_system_table_32_t *stbl = (efi_system_table_32_t *)systab_tbl_pa;
>  
>  		config_tables	= stbl->tables;
>  		nr_tables	= stbl->nr_tables;
> diff --git a/arch/x86/boot/compressed/efi.c b/arch/x86/boot/compressed/efi.c
> new file mode 100644
> index 000000000000..1c626d28f07e
> --- /dev/null
> +++ b/arch/x86/boot/compressed/efi.c
> @@ -0,0 +1,72 @@
> +// 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>
> +
> +/**
> + * efi_get_system_table - Given boot_params, retrieve the physical address of
> + *                        EFI system table.
> + *
> + * @boot_params:        pointer to boot_params
> + * @sys_tbl_pa:         location to store physical address of system table
> + * @is_efi_64:          location to store whether using 64-bit EFI or not
> + *
> + * Return: 0 on success. On error, return params are left unchanged.
> + *
> + * Note: Existing callers like ACPI will call this unconditionally even for
> + * non-EFI BIOSes. In such cases, those callers may treat cases where
> + * bootparams doesn't indicate that a valid EFI system table is available as
> + * non-fatal errors to allow fall-through to non-EFI alternatives. This
> + * class of errors are reported as EOPNOTSUPP and should be kept in sync with
> + * callers who check for that specific error.
> + */
> +int efi_get_system_table(struct boot_params *boot_params, unsigned long *sys_tbl_pa,
> +			 bool *is_efi_64)
> +{
> +	unsigned long sys_tbl;
> +	struct efi_info *ei;
> +	bool efi_64;
> +	char *sig;
> +
> +	if (!sys_tbl_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 -EOPNOTSUPP;
> +	}
> +
> +	/* Get systab from boot params. */
> +#ifdef CONFIG_X86_64
> +	sys_tbl = 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 -EOPNOTSUPP;
> +	}
> +	sys_tbl = ei->efi_systab;
> +#endif
> +	if (!sys_tbl) {
> +		debug_putstr("EFI system table not found.");
> +		return -ENOENT;
> +	}
> +
> +	*sys_tbl_pa = sys_tbl;
> +	*is_efi_64 = efi_64;
> +	return 0;
> +}
> diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
> index 01cc13c12059..165640f64b71 100644
> --- a/arch/x86/boot/compressed/misc.h
> +++ b/arch/x86/boot/compressed/misc.h
> @@ -23,6 +23,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>
> @@ -176,4 +177,17 @@ 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_get_system_table(struct boot_params *boot_params,
> +			 unsigned long *sys_tbl_pa, bool *is_efi_64);
> +#else
> +static inline int
> +efi_get_system_table(struct boot_params *boot_params,
> +		     unsigned long *sys_tbl_pa, bool *is_efi_64)
> +{
> +	return -ENOENT;
> +}
> +#endif /* CONFIG_EFI */
> +
>  #endif /* BOOT_COMPRESSED_MISC_H */
> -- 
> 2.25.1
>
Chao Fan Jan. 11, 2022, 8:59 a.m. UTC | #7
Hi, I am this Chao Fan, and <fanc.fnst@cn.fujitsu.com> won't be used again.
Please add me with fanchao.njupt@gmail.com
Many thanks.

Thanks,
Chao Fan

Michael Roth <michael.roth@amd.com> 于2021年12月14日周二 11:46写道:
>
> On Fri, Dec 10, 2021 at 10:54:35AM -0800, Dave Hansen wrote:
> > On 12/10/21 7:43 AM, Brijesh Singh wrote:
> > > +/*
> > > + * Helpers for early access to EFI configuration table
> > > + *
> > > + * Copyright (C) 2021 Advanced Micro Devices, Inc.
> > > + *
> > > + * Author: Michael Roth <michael.roth@amd.com>
> > > + */
> >
> > It doesn't seem quite right to slap this copyright on a file that's full
> > of content that came from other files.  It would be one thing if
> > arch/x86/boot/compressed/acpi.c had this banner in it already.  Also, a
>
> Yah, acpi.c didn't have any copyright banner so I used my 'default'
> template for new files here to cover any additions, but that does give
> a misleading impression.
>
> I'm not sure how this is normally addressed, but I'm planning on just
> continuing the acpi.c tradition of *not* adding copyright notices for new
> code, and simply document that the contents of the file are mostly movement
> from acpi.c
>
> > arch/x86/boot/compressed/acpi.c had this banner in it already.  Also, a
> > bunch of the lines in this file seem to come from:
> >
> >       commit 33f0df8d843deb9ec24116dcd79a40ca0ea8e8a9
> >       Author: Chao Fan <fanc.fnst@cn.fujitsu.com>
> >       Date:   Wed Jan 23 19:08:46 2019 +0800
>
> AFAICT the full author list for the changes in question are, in
> alphabetical order:
>
>   Chao Fan <fanc.fnst@cn.fujitsu.com>
>   Junichi Nomura <j-nomura@ce.jp.nec.com>
>   Borislav Petkov <bp@suse.de>
>
> Chao, Junichi, Borislav,
>
> If you would like to be listed as an author in efi.c (which is mainly just a
> movement of EFI config table parsing code from acpi.c into re-usable helper
> functions in efi.c), please let me know and I'll add you.
>
> Otherwise, I'll plan on adopting the acpi.c precedent for this as well, which
> is to not list individual authors, since it doesn't seem right to add Author
> fields retroactively without their permission.
diff mbox series

Patch

diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index 431bf7f846c3..d364192c2367 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.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..9e784bd7b2e6 100644
--- a/arch/x86/boot/compressed/acpi.c
+++ b/arch/x86/boot/compressed/acpi.c
@@ -86,8 +86,8 @@  static acpi_physical_address kexec_get_rsdp_addr(void)
 {
 	efi_system_table_64_t *systab;
 	struct efi_setup_data *esd;
-	struct efi_info *ei;
-	char *sig;
+	bool efi_64;
+	int ret;
 
 	esd = (struct efi_setup_data *)get_kexec_setup_data_addr();
 	if (!esd)
@@ -98,18 +98,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_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,45 +117,31 @@  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 long systab_tbl_pa, config_tables;
 	unsigned int nr_tables;
-	struct efi_info *ei;
 	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;
-	}
+	int ret;
 
-	/* 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");
+	/*
+	 * This function is called even for non-EFI BIOSes, and callers expect
+	 * failure to locate the EFI system table to result in 0 being returned
+	 * as indication that EFI is not available, rather than outright
+	 * failure/abort.
+	 */
+	ret = efi_get_system_table(boot_params, &systab_tbl_pa, &efi_64);
+	if (ret == -EOPNOTSUPP)
 		return 0;
-	}
-	systab = ei->efi_systab;
-#endif
-	if (!systab)
-		error("EFI system table not found.");
+	if (ret)
+		error("EFI support advertised, but unable to locate system table.");
 
 	/* Handle EFI bitness properly */
 	if (efi_64) {
-		efi_system_table_64_t *stbl = (efi_system_table_64_t *)systab;
+		efi_system_table_64_t *stbl = (efi_system_table_64_t *)systab_tbl_pa;
 
 		config_tables	= stbl->tables;
 		nr_tables	= stbl->nr_tables;
 	} else {
-		efi_system_table_32_t *stbl = (efi_system_table_32_t *)systab;
+		efi_system_table_32_t *stbl = (efi_system_table_32_t *)systab_tbl_pa;
 
 		config_tables	= stbl->tables;
 		nr_tables	= stbl->nr_tables;
diff --git a/arch/x86/boot/compressed/efi.c b/arch/x86/boot/compressed/efi.c
new file mode 100644
index 000000000000..1c626d28f07e
--- /dev/null
+++ b/arch/x86/boot/compressed/efi.c
@@ -0,0 +1,72 @@ 
+// 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>
+
+/**
+ * efi_get_system_table - Given boot_params, retrieve the physical address of
+ *                        EFI system table.
+ *
+ * @boot_params:        pointer to boot_params
+ * @sys_tbl_pa:         location to store physical address of system table
+ * @is_efi_64:          location to store whether using 64-bit EFI or not
+ *
+ * Return: 0 on success. On error, return params are left unchanged.
+ *
+ * Note: Existing callers like ACPI will call this unconditionally even for
+ * non-EFI BIOSes. In such cases, those callers may treat cases where
+ * bootparams doesn't indicate that a valid EFI system table is available as
+ * non-fatal errors to allow fall-through to non-EFI alternatives. This
+ * class of errors are reported as EOPNOTSUPP and should be kept in sync with
+ * callers who check for that specific error.
+ */
+int efi_get_system_table(struct boot_params *boot_params, unsigned long *sys_tbl_pa,
+			 bool *is_efi_64)
+{
+	unsigned long sys_tbl;
+	struct efi_info *ei;
+	bool efi_64;
+	char *sig;
+
+	if (!sys_tbl_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 -EOPNOTSUPP;
+	}
+
+	/* Get systab from boot params. */
+#ifdef CONFIG_X86_64
+	sys_tbl = 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 -EOPNOTSUPP;
+	}
+	sys_tbl = ei->efi_systab;
+#endif
+	if (!sys_tbl) {
+		debug_putstr("EFI system table not found.");
+		return -ENOENT;
+	}
+
+	*sys_tbl_pa = sys_tbl;
+	*is_efi_64 = efi_64;
+	return 0;
+}
diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
index 01cc13c12059..165640f64b71 100644
--- a/arch/x86/boot/compressed/misc.h
+++ b/arch/x86/boot/compressed/misc.h
@@ -23,6 +23,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>
@@ -176,4 +177,17 @@  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_get_system_table(struct boot_params *boot_params,
+			 unsigned long *sys_tbl_pa, bool *is_efi_64);
+#else
+static inline int
+efi_get_system_table(struct boot_params *boot_params,
+		     unsigned long *sys_tbl_pa, bool *is_efi_64)
+{
+	return -ENOENT;
+}
+#endif /* CONFIG_EFI */
+
 #endif /* BOOT_COMPRESSED_MISC_H */