* [PATCH 01/10] x86/efi: Mark can_free_region() as an __init function
2019-02-02 9:41 [GIT PULL 00/10] EFI changes for v5.1 Ard Biesheuvel
@ 2019-02-02 9:41 ` Ard Biesheuvel
2019-02-02 9:41 ` [PATCH 02/10] x86/efi: Return error status if mapping EFI regions fail Ard Biesheuvel
` (8 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Ard Biesheuvel @ 2019-02-02 9:41 UTC (permalink / raw)
To: linux-efi, Ingo Molnar, Thomas Gleixner
Cc: Ard Biesheuvel, linux-kernel, AKASHI Takahiro, Alexander Graf,
Bjorn Andersson, Borislav Petkov, Heinrich Schuchardt,
Jeffrey Hugo, Lee Jones, Leif Lindholm, Linus Torvalds,
Peter Jones, Peter Zijlstra, Sai Praneeth Prakhya
From: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
can_free_region() is called only once during _boot_ by
efi_reserve_boot_services(). Hence, mark it as __init function.
Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/x86/platform/efi/quirks.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index 17456a1d3f04..9ce85e605052 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -304,7 +304,7 @@ void __init efi_arch_mem_reserve(phys_addr_t addr, u64 size)
* - Not within any part of the kernel
* - Not the BIOS reserved area (E820_TYPE_RESERVED, E820_TYPE_NVS, etc)
*/
-static bool can_free_region(u64 start, u64 size)
+static __init bool can_free_region(u64 start, u64 size)
{
if (start + size > __pa_symbol(_text) && start <= __pa_symbol(_end))
return false;
--
2.17.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 02/10] x86/efi: Return error status if mapping EFI regions fail
2019-02-02 9:41 [GIT PULL 00/10] EFI changes for v5.1 Ard Biesheuvel
2019-02-02 9:41 ` [PATCH 01/10] x86/efi: Mark can_free_region() as an __init function Ard Biesheuvel
@ 2019-02-02 9:41 ` Ard Biesheuvel
2019-02-04 7:18 ` Ingo Molnar
2019-02-02 9:41 ` [PATCH 03/10] efi: memattr: don't bail on zero VA if it equals the region's PA Ard Biesheuvel
` (7 subsequent siblings)
9 siblings, 1 reply; 18+ messages in thread
From: Ard Biesheuvel @ 2019-02-02 9:41 UTC (permalink / raw)
To: linux-efi, Ingo Molnar, Thomas Gleixner
Cc: Ard Biesheuvel, linux-kernel, AKASHI Takahiro, Alexander Graf,
Bjorn Andersson, Borislav Petkov, Heinrich Schuchardt,
Jeffrey Hugo, Lee Jones, Leif Lindholm, Linus Torvalds,
Peter Jones, Peter Zijlstra, Sai Praneeth Prakhya
From: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
efi_map_region() creates VA mappings for an given EFI region using any one
of the two helper functions (namely __map_region() and old_map_region()).
These helper functions *could* fail while creating mappings and presently
their return value is not checked. Not checking for the return value of
these functions might create issues because after these functions return
"md->virt_addr" is set to the requested virtual address (so it's assumed
that these functions always succeed which is not quite true). This
assumption leads to "md->virt_addr" having invalid mapping should any of
__map_region() or old_map_region() fail.
Hence, check for the return value of these functions and if indeed they
fail, turn off EFI Runtime Services forever because kernel cannot
prioritize among EFI regions.
This also fixes the comment "FIXME: add error handling" in
kexec_enter_virtual_mode().
Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/x86/include/asm/efi.h | 6 +++---
arch/x86/platform/efi/efi.c | 21 +++++++++++++-----
arch/x86/platform/efi/efi_32.c | 6 +++---
arch/x86/platform/efi/efi_64.c | 39 ++++++++++++++++++++++------------
4 files changed, 48 insertions(+), 24 deletions(-)
diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index 107283b1eb1e..a37378f986ec 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -125,12 +125,12 @@ extern pgd_t * __init efi_call_phys_prolog(void);
extern void __init efi_call_phys_epilog(pgd_t *save_pgd);
extern void __init efi_print_memmap(void);
extern void __init efi_memory_uc(u64 addr, unsigned long size);
-extern void __init efi_map_region(efi_memory_desc_t *md);
-extern void __init efi_map_region_fixed(efi_memory_desc_t *md);
+extern int __init efi_map_region(efi_memory_desc_t *md);
+extern int __init efi_map_region_fixed(efi_memory_desc_t *md);
extern void efi_sync_low_kernel_mappings(void);
extern int __init efi_alloc_page_tables(void);
extern int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages);
-extern void __init old_map_region(efi_memory_desc_t *md);
+extern int __init old_map_region(efi_memory_desc_t *md);
extern void __init runtime_code_page_mkexec(void);
extern void __init efi_runtime_update_mappings(void);
extern void __init efi_dump_pagetable(void);
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index e1cb01a22fa8..3d43ec58775b 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -581,7 +581,7 @@ void __init efi_memory_uc(u64 addr, unsigned long size)
set_memory_uc(addr, npages);
}
-void __init old_map_region(efi_memory_desc_t *md)
+int __init old_map_region(efi_memory_desc_t *md)
{
u64 start_pfn, end_pfn, end;
unsigned long size;
@@ -601,10 +601,14 @@ void __init old_map_region(efi_memory_desc_t *md)
va = efi_ioremap(md->phys_addr, size,
md->type, md->attribute);
- md->virt_addr = (u64) (unsigned long) va;
- if (!va)
+ if (!va) {
pr_err("ioremap of 0x%llX failed!\n",
(unsigned long long)md->phys_addr);
+ return -ENOMEM;
+ }
+
+ md->virt_addr = (u64)(unsigned long)va;
+ return 0;
}
/* Merge contiguous regions of the same type and attribute */
@@ -797,7 +801,9 @@ static void * __init efi_map_regions(int *count, int *pg_shift)
if (!should_map_region(md))
continue;
- efi_map_region(md);
+ if (efi_map_region(md))
+ return NULL;
+
get_systab_virt_addr(md);
if (left < desc_size) {
@@ -849,7 +855,12 @@ static void __init kexec_enter_virtual_mode(void)
* fixed addr which was used in first kernel of a kexec boot.
*/
for_each_efi_memory_desc(md) {
- efi_map_region_fixed(md); /* FIXME: add error handling */
+ if (efi_map_region_fixed(md)) {
+ pr_err("Error mapping EFI regions, EFI runtime non-functional!\n");
+ clear_bit(EFI_RUNTIME_SERVICES, &efi.flags);
+ return;
+ }
+
get_systab_virt_addr(md);
}
diff --git a/arch/x86/platform/efi/efi_32.c b/arch/x86/platform/efi/efi_32.c
index 9959657127f4..4d369118391a 100644
--- a/arch/x86/platform/efi/efi_32.c
+++ b/arch/x86/platform/efi/efi_32.c
@@ -58,12 +58,12 @@ int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages)
return 0;
}
-void __init efi_map_region(efi_memory_desc_t *md)
+int __init efi_map_region(efi_memory_desc_t *md)
{
- old_map_region(md);
+ return old_map_region(md);
}
-void __init efi_map_region_fixed(efi_memory_desc_t *md) {}
+int __init efi_map_region_fixed(efi_memory_desc_t *md) { return 0; }
void __init parse_efi_setup(u64 phys_addr, u32 data_len) {}
pgd_t * __init efi_call_phys_prolog(void)
diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
index cf0347f61b21..ba83e2e2664b 100644
--- a/arch/x86/platform/efi/efi_64.c
+++ b/arch/x86/platform/efi/efi_64.c
@@ -408,7 +408,7 @@ int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages)
return 0;
}
-static void __init __map_region(efi_memory_desc_t *md, u64 va)
+static int __init __map_region(efi_memory_desc_t *md, u64 va)
{
unsigned long flags = _PAGE_RW;
unsigned long pfn;
@@ -421,12 +421,16 @@ static void __init __map_region(efi_memory_desc_t *md, u64 va)
flags |= _PAGE_ENC;
pfn = md->phys_addr >> PAGE_SHIFT;
- if (kernel_map_pages_in_pgd(pgd, pfn, va, md->num_pages, flags))
- pr_warn("Error mapping PA 0x%llx -> VA 0x%llx!\n",
- md->phys_addr, va);
+ if (kernel_map_pages_in_pgd(pgd, pfn, va, md->num_pages, flags)) {
+ pr_err("Error mapping PA 0x%llx -> VA 0x%llx!\n",
+ md->phys_addr, va);
+ return -ENOMEM;
+ }
+
+ return 0;
}
-void __init efi_map_region(efi_memory_desc_t *md)
+int __init efi_map_region(efi_memory_desc_t *md)
{
unsigned long size = md->num_pages << PAGE_SHIFT;
u64 pa = md->phys_addr;
@@ -439,7 +443,8 @@ void __init efi_map_region(efi_memory_desc_t *md)
* firmware which doesn't update all internal pointers after switching
* to virtual mode and would otherwise crap on us.
*/
- __map_region(md, md->phys_addr);
+ if (__map_region(md, md->phys_addr))
+ return -ENOMEM;
/*
* Enforce the 1:1 mapping as the default virtual address when
@@ -448,7 +453,7 @@ void __init efi_map_region(efi_memory_desc_t *md)
*/
if (!efi_is_native () && IS_ENABLED(CONFIG_EFI_MIXED)) {
md->virt_addr = md->phys_addr;
- return;
+ return 0;
}
efi_va -= size;
@@ -468,13 +473,16 @@ void __init efi_map_region(efi_memory_desc_t *md)
}
if (efi_va < EFI_VA_END) {
- pr_warn(FW_WARN "VA address range overflow!\n");
- return;
+ pr_err(FW_WARN "VA address range overflow!\n");
+ return -ENOMEM;
}
/* Do the VA map */
- __map_region(md, efi_va);
+ if (__map_region(md, efi_va))
+ return -ENOMEM;
+
md->virt_addr = efi_va;
+ return 0;
}
/*
@@ -482,10 +490,15 @@ void __init efi_map_region(efi_memory_desc_t *md)
* md->virt_addr is the original virtual address which had been mapped in kexec
* 1st kernel.
*/
-void __init efi_map_region_fixed(efi_memory_desc_t *md)
+int __init efi_map_region_fixed(efi_memory_desc_t *md)
{
- __map_region(md, md->phys_addr);
- __map_region(md, md->virt_addr);
+ if (__map_region(md, md->phys_addr))
+ return -ENOMEM;
+
+ if (__map_region(md, md->virt_addr))
+ return -ENOMEM;
+
+ return 0;
}
void __iomem *__init efi_ioremap(unsigned long phys_addr, unsigned long size,
--
2.17.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH 02/10] x86/efi: Return error status if mapping EFI regions fail
2019-02-02 9:41 ` [PATCH 02/10] x86/efi: Return error status if mapping EFI regions fail Ard Biesheuvel
@ 2019-02-04 7:18 ` Ingo Molnar
2019-02-04 7:25 ` Ingo Molnar
2019-02-04 7:28 ` Ard Biesheuvel
0 siblings, 2 replies; 18+ messages in thread
From: Ingo Molnar @ 2019-02-04 7:18 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-efi, Thomas Gleixner, linux-kernel, AKASHI Takahiro,
Alexander Graf, Bjorn Andersson, Borislav Petkov,
Heinrich Schuchardt, Jeffrey Hugo, Lee Jones, Leif Lindholm,
Linus Torvalds, Peter Jones, Peter Zijlstra,
Sai Praneeth Prakhya
* Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> From: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
>
> efi_map_region() creates VA mappings for an given EFI region using any one
> of the two helper functions (namely __map_region() and old_map_region()).
> These helper functions *could* fail while creating mappings and presently
> their return value is not checked. Not checking for the return value of
> these functions might create issues because after these functions return
> "md->virt_addr" is set to the requested virtual address (so it's assumed
> that these functions always succeed which is not quite true). This
> assumption leads to "md->virt_addr" having invalid mapping should any of
> __map_region() or old_map_region() fail.
>
> Hence, check for the return value of these functions and if indeed they
> fail, turn off EFI Runtime Services forever because kernel cannot
> prioritize among EFI regions.
>
> This also fixes the comment "FIXME: add error handling" in
> kexec_enter_virtual_mode().
>
> Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Ingo Molnar <mingo@kernel.org>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> arch/x86/include/asm/efi.h | 6 +++---
> arch/x86/platform/efi/efi.c | 21 +++++++++++++-----
> arch/x86/platform/efi/efi_32.c | 6 +++---
> arch/x86/platform/efi/efi_64.c | 39 ++++++++++++++++++++++------------
> 4 files changed, 48 insertions(+), 24 deletions(-)
>
> diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
> index 107283b1eb1e..a37378f986ec 100644
> --- a/arch/x86/include/asm/efi.h
> +++ b/arch/x86/include/asm/efi.h
> @@ -125,12 +125,12 @@ extern pgd_t * __init efi_call_phys_prolog(void);
> extern void __init efi_call_phys_epilog(pgd_t *save_pgd);
> extern void __init efi_print_memmap(void);
> extern void __init efi_memory_uc(u64 addr, unsigned long size);
> -extern void __init efi_map_region(efi_memory_desc_t *md);
> -extern void __init efi_map_region_fixed(efi_memory_desc_t *md);
> +extern int __init efi_map_region(efi_memory_desc_t *md);
> +extern int __init efi_map_region_fixed(efi_memory_desc_t *md);
> extern void efi_sync_low_kernel_mappings(void);
> extern int __init efi_alloc_page_tables(void);
> extern int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages);
> -extern void __init old_map_region(efi_memory_desc_t *md);
> +extern int __init old_map_region(efi_memory_desc_t *md);
> extern void __init runtime_code_page_mkexec(void);
> extern void __init efi_runtime_update_mappings(void);
> extern void __init efi_dump_pagetable(void);
> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> index e1cb01a22fa8..3d43ec58775b 100644
> --- a/arch/x86/platform/efi/efi.c
> +++ b/arch/x86/platform/efi/efi.c
> @@ -581,7 +581,7 @@ void __init efi_memory_uc(u64 addr, unsigned long size)
> set_memory_uc(addr, npages);
> }
>
> -void __init old_map_region(efi_memory_desc_t *md)
> +int __init old_map_region(efi_memory_desc_t *md)
> {
> u64 start_pfn, end_pfn, end;
> unsigned long size;
> @@ -601,10 +601,14 @@ void __init old_map_region(efi_memory_desc_t *md)
> va = efi_ioremap(md->phys_addr, size,
> md->type, md->attribute);
>
> - md->virt_addr = (u64) (unsigned long) va;
> - if (!va)
> + if (!va) {
> pr_err("ioremap of 0x%llX failed!\n",
> (unsigned long long)md->phys_addr);
> + return -ENOMEM;
> + }
> +
> + md->virt_addr = (u64)(unsigned long)va;
> + return 0;
Just wondering, shouldn't the failure path set ->virt_addr to something
safe, just in case a caller doesn't check the error and relies on it?
That's because in this commit we've now changed it from 0 to undefined.
> +int __init efi_map_region_fixed(efi_memory_desc_t *md) { return 0; }
Inline functions should be marked inline ...
> if (efi_va < EFI_VA_END) {
> - pr_warn(FW_WARN "VA address range overflow!\n");
> - return;
> + pr_err(FW_WARN "VA address range overflow!\n");
> + return -ENOMEM;
> }
>
> /* Do the VA map */
> - __map_region(md, efi_va);
> + if (__map_region(md, efi_va))
> + return -ENOMEM;
> +
> md->virt_addr = efi_va;
> + return 0;
Same error return problem of leaving ->virt_addr undefined.
Note that I also fixed up the grammar and readability of the changelog -
see the updated version below.
Thanks,
Ingo
=============>
Subject: x86/efi: Return error status if mapping of EFI regions fails
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Date: Sat, 2 Feb 2019 10:41:11 +0100
From: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
efi_map_region() creates VA mappings for a given EFI region using one
of the two helper functions (namely __map_region() and old_map_region()).
These helper functions could fail while creating mappings and presently
their return value is not checked.
Not checking for the return value of these functions might create bugs,
because after these functions return "md->virt_addr" is set to the
requested virtual address (so it's assumed that these functions always
succeed which is not quite true). This assumption leads to
"md->virt_addr" having invalid mapping, should any of __map_region()
or old_map_region() fail.
Hence, check for the return value of these functions and if indeed they
fail, turn off EFI Runtime Services forever because kernel cannot
prioritize among EFI regions.
This also fixes the comment "FIXME: add error handling" in
kexec_enter_virtual_mode().
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 02/10] x86/efi: Return error status if mapping EFI regions fail
2019-02-04 7:18 ` Ingo Molnar
@ 2019-02-04 7:25 ` Ingo Molnar
2019-02-04 7:28 ` Ard Biesheuvel
1 sibling, 0 replies; 18+ messages in thread
From: Ingo Molnar @ 2019-02-04 7:25 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-efi, Thomas Gleixner, linux-kernel, AKASHI Takahiro,
Alexander Graf, Bjorn Andersson, Borislav Petkov,
Heinrich Schuchardt, Jeffrey Hugo, Lee Jones, Leif Lindholm,
Linus Torvalds, Peter Jones, Peter Zijlstra,
Sai Praneeth Prakhya
* Ingo Molnar <mingo@kernel.org> wrote:
> Note that I also fixed up the grammar and readability of the changelog -
> see the updated version below.
>
> Thanks,
>
> Ingo
>
> =============>
> Subject: x86/efi: Return error status if mapping of EFI regions fails
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Date: Sat, 2 Feb 2019 10:41:11 +0100
>
> From: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
Also note that I left out this patch from the series for the time being
under the assumption that it gets a v2 update. Since it has no
dependencies in the remaining patches AFAICS I applied all the other
patches to tip:efi/core.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 02/10] x86/efi: Return error status if mapping EFI regions fail
2019-02-04 7:18 ` Ingo Molnar
2019-02-04 7:25 ` Ingo Molnar
@ 2019-02-04 7:28 ` Ard Biesheuvel
2019-02-04 22:29 ` Prakhya, Sai Praneeth
1 sibling, 1 reply; 18+ messages in thread
From: Ard Biesheuvel @ 2019-02-04 7:28 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-efi, Thomas Gleixner, Linux Kernel Mailing List,
AKASHI Takahiro, Alexander Graf, Bjorn Andersson,
Borislav Petkov, Heinrich Schuchardt, Jeffrey Hugo, Lee Jones,
Leif Lindholm, Linus Torvalds, Peter Jones, Peter Zijlstra,
Sai Praneeth Prakhya
On Mon, 4 Feb 2019 at 08:18, Ingo Molnar <mingo@kernel.org> wrote:
>
>
> * Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> > From: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
> >
> > efi_map_region() creates VA mappings for an given EFI region using any one
> > of the two helper functions (namely __map_region() and old_map_region()).
> > These helper functions *could* fail while creating mappings and presently
> > their return value is not checked. Not checking for the return value of
> > these functions might create issues because after these functions return
> > "md->virt_addr" is set to the requested virtual address (so it's assumed
> > that these functions always succeed which is not quite true). This
> > assumption leads to "md->virt_addr" having invalid mapping should any of
> > __map_region() or old_map_region() fail.
> >
> > Hence, check for the return value of these functions and if indeed they
> > fail, turn off EFI Runtime Services forever because kernel cannot
> > prioritize among EFI regions.
> >
> > This also fixes the comment "FIXME: add error handling" in
> > kexec_enter_virtual_mode().
> >
> > Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
> > Cc: Borislav Petkov <bp@alien8.de>
> > Cc: Ingo Molnar <mingo@kernel.org>
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > ---
> > arch/x86/include/asm/efi.h | 6 +++---
> > arch/x86/platform/efi/efi.c | 21 +++++++++++++-----
> > arch/x86/platform/efi/efi_32.c | 6 +++---
> > arch/x86/platform/efi/efi_64.c | 39 ++++++++++++++++++++++------------
> > 4 files changed, 48 insertions(+), 24 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
> > index 107283b1eb1e..a37378f986ec 100644
> > --- a/arch/x86/include/asm/efi.h
> > +++ b/arch/x86/include/asm/efi.h
> > @@ -125,12 +125,12 @@ extern pgd_t * __init efi_call_phys_prolog(void);
> > extern void __init efi_call_phys_epilog(pgd_t *save_pgd);
> > extern void __init efi_print_memmap(void);
> > extern void __init efi_memory_uc(u64 addr, unsigned long size);
> > -extern void __init efi_map_region(efi_memory_desc_t *md);
> > -extern void __init efi_map_region_fixed(efi_memory_desc_t *md);
> > +extern int __init efi_map_region(efi_memory_desc_t *md);
> > +extern int __init efi_map_region_fixed(efi_memory_desc_t *md);
> > extern void efi_sync_low_kernel_mappings(void);
> > extern int __init efi_alloc_page_tables(void);
> > extern int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages);
> > -extern void __init old_map_region(efi_memory_desc_t *md);
> > +extern int __init old_map_region(efi_memory_desc_t *md);
> > extern void __init runtime_code_page_mkexec(void);
> > extern void __init efi_runtime_update_mappings(void);
> > extern void __init efi_dump_pagetable(void);
> > diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> > index e1cb01a22fa8..3d43ec58775b 100644
> > --- a/arch/x86/platform/efi/efi.c
> > +++ b/arch/x86/platform/efi/efi.c
> > @@ -581,7 +581,7 @@ void __init efi_memory_uc(u64 addr, unsigned long size)
> > set_memory_uc(addr, npages);
> > }
> >
> > -void __init old_map_region(efi_memory_desc_t *md)
> > +int __init old_map_region(efi_memory_desc_t *md)
> > {
> > u64 start_pfn, end_pfn, end;
> > unsigned long size;
> > @@ -601,10 +601,14 @@ void __init old_map_region(efi_memory_desc_t *md)
> > va = efi_ioremap(md->phys_addr, size,
> > md->type, md->attribute);
> >
> > - md->virt_addr = (u64) (unsigned long) va;
> > - if (!va)
> > + if (!va) {
> > pr_err("ioremap of 0x%llX failed!\n",
> > (unsigned long long)md->phys_addr);
> > + return -ENOMEM;
> > + }
> > +
> > + md->virt_addr = (u64)(unsigned long)va;
> > + return 0;
>
> Just wondering, shouldn't the failure path set ->virt_addr to something
> safe, just in case a caller doesn't check the error and relies on it?
>
> That's because in this commit we've now changed it from 0 to undefined.
>
Indeed. We don't usually rely on the value of ->virt_addr when
EFI_RUNTIME_SERVICES is unset, but there is some sysfs code, and
perhaps some other places where we do reference ->virt_addr, and not
assigning it at all is obviously wrong, and potentially hazardous.
> > +int __init efi_map_region_fixed(efi_memory_desc_t *md) { return 0; }
>
> Inline functions should be marked inline ...
>
> > if (efi_va < EFI_VA_END) {
> > - pr_warn(FW_WARN "VA address range overflow!\n");
> > - return;
> > + pr_err(FW_WARN "VA address range overflow!\n");
> > + return -ENOMEM;
> > }
> >
> > /* Do the VA map */
> > - __map_region(md, efi_va);
> > + if (__map_region(md, efi_va))
> > + return -ENOMEM;
> > +
> > md->virt_addr = efi_va;
> > + return 0;
>
> Same error return problem of leaving ->virt_addr undefined.
>
> Note that I also fixed up the grammar and readability of the changelog -
> see the updated version below.
>
> Thanks,
>
> Ingo
>
> =============>
> Subject: x86/efi: Return error status if mapping of EFI regions fails
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Date: Sat, 2 Feb 2019 10:41:11 +0100
>
> From: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
>
> efi_map_region() creates VA mappings for a given EFI region using one
> of the two helper functions (namely __map_region() and old_map_region()).
>
> These helper functions could fail while creating mappings and presently
> their return value is not checked.
>
> Not checking for the return value of these functions might create bugs,
> because after these functions return "md->virt_addr" is set to the
> requested virtual address (so it's assumed that these functions always
> succeed which is not quite true). This assumption leads to
> "md->virt_addr" having invalid mapping, should any of __map_region()
> or old_map_region() fail.
>
> Hence, check for the return value of these functions and if indeed they
> fail, turn off EFI Runtime Services forever because kernel cannot
> prioritize among EFI regions.
>
> This also fixes the comment "FIXME: add error handling" in
> kexec_enter_virtual_mode().
>
Thanks Ingo.
Sai, could you please respin this and use Ingo's updated version of
the commit log?
^ permalink raw reply [flat|nested] 18+ messages in thread* RE: [PATCH 02/10] x86/efi: Return error status if mapping EFI regions fail
2019-02-04 7:28 ` Ard Biesheuvel
@ 2019-02-04 22:29 ` Prakhya, Sai Praneeth
2019-02-08 15:50 ` Ard Biesheuvel
0 siblings, 1 reply; 18+ messages in thread
From: Prakhya, Sai Praneeth @ 2019-02-04 22:29 UTC (permalink / raw)
To: Ard Biesheuvel, Ingo Molnar
Cc: linux-efi, Thomas Gleixner, Linux Kernel Mailing List,
AKASHI Takahiro, Alexander Graf, Bjorn Andersson,
Borislav Petkov, Heinrich Schuchardt, Jeffrey Hugo, Lee Jones,
Leif Lindholm, Linus Torvalds, Peter Jones, Peter Zijlstra
> > > efi_map_region() creates VA mappings for an given EFI region using
> > > any one of the two helper functions (namely __map_region() and
> old_map_region()).
> > > These helper functions *could* fail while creating mappings and
> > > presently their return value is not checked. Not checking for the
> > > return value of these functions might create issues because after
> > > these functions return "md->virt_addr" is set to the requested
> > > virtual address (so it's assumed that these functions always succeed
> > > which is not quite true). This assumption leads to "md->virt_addr"
> > > having invalid mapping should any of
> > > __map_region() or old_map_region() fail.
> > >
> > > Hence, check for the return value of these functions and if indeed
> > > they fail, turn off EFI Runtime Services forever because kernel
> > > cannot prioritize among EFI regions.
> > >
[...................]
> > > -void __init old_map_region(efi_memory_desc_t *md)
> > > +int __init old_map_region(efi_memory_desc_t *md)
> > > {
> > > u64 start_pfn, end_pfn, end;
> > > unsigned long size;
> > > @@ -601,10 +601,14 @@ void __init old_map_region(efi_memory_desc_t
> *md)
> > > va = efi_ioremap(md->phys_addr, size,
> > > md->type, md->attribute);
> > >
> > > - md->virt_addr = (u64) (unsigned long) va;
> > > - if (!va)
> > > + if (!va) {
> > > pr_err("ioremap of 0x%llX failed!\n",
> > > (unsigned long long)md->phys_addr);
> > > + return -ENOMEM;
> > > + }
> > > +
> > > + md->virt_addr = (u64)(unsigned long)va;
> > > + return 0;
> >
> > Just wondering, shouldn't the failure path set ->virt_addr to
> > something safe, just in case a caller doesn't check the error and relies on it?
> >
> > That's because in this commit we've now changed it from 0 to undefined.
> >
>
> Indeed. We don't usually rely on the value of ->virt_addr when
> EFI_RUNTIME_SERVICES is unset, but there is some sysfs code, and perhaps
> some other places where we do reference ->virt_addr, and not assigning it at all
> is obviously wrong, and potentially hazardous.
>
Ok.. makes sense.
Do you think md->virt_addr = 0 for fail case is ok?
> > > +int __init efi_map_region_fixed(efi_memory_desc_t *md) { return 0;
> > > +}
> >
> > Inline functions should be marked inline ...
> >
> > > if (efi_va < EFI_VA_END) {
> > > - pr_warn(FW_WARN "VA address range overflow!\n");
> > > - return;
> > > + pr_err(FW_WARN "VA address range overflow!\n");
> > > + return -ENOMEM;
> > > }
> > >
> > > /* Do the VA map */
> > > - __map_region(md, efi_va);
> > > + if (__map_region(md, efi_va))
> > > + return -ENOMEM;
> > > +
> > > md->virt_addr = efi_va;
> > > + return 0;
> >
> > Same error return problem of leaving ->virt_addr undefined.
> >
Sure! Will fix it in V2.
> > Note that I also fixed up the grammar and readability of the changelog
> > - see the updated version below.
Thanks for fixing :)
> >
> > Thanks,
> >
> > Ingo
> >
> > =============>
> > Subject: x86/efi: Return error status if mapping of EFI regions fails
> > From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Date: Sat, 2 Feb 2019 10:41:11 +0100
> >
> > From: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
> >
> > efi_map_region() creates VA mappings for a given EFI region using one
> > of the two helper functions (namely __map_region() and old_map_region()).
> >
> > These helper functions could fail while creating mappings and
> > presently their return value is not checked.
> >
> > Not checking for the return value of these functions might create
> > bugs, because after these functions return "md->virt_addr" is set to
> > the requested virtual address (so it's assumed that these functions
> > always succeed which is not quite true). This assumption leads to
> > "md->virt_addr" having invalid mapping, should any of __map_region()
> > or old_map_region() fail.
> >
> > Hence, check for the return value of these functions and if indeed
> > they fail, turn off EFI Runtime Services forever because kernel cannot
> > prioritize among EFI regions.
> >
> > This also fixes the comment "FIXME: add error handling" in
> > kexec_enter_virtual_mode().
> >
>
> Thanks Ingo.
>
> Sai, could you please respin this and use Ingo's updated version of the commit
> log?
Sure! I will send a V2 with the mentioned changes.
Regards,
Sai
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 02/10] x86/efi: Return error status if mapping EFI regions fail
2019-02-04 22:29 ` Prakhya, Sai Praneeth
@ 2019-02-08 15:50 ` Ard Biesheuvel
0 siblings, 0 replies; 18+ messages in thread
From: Ard Biesheuvel @ 2019-02-08 15:50 UTC (permalink / raw)
To: Prakhya, Sai Praneeth
Cc: Ingo Molnar, linux-efi, Thomas Gleixner,
Linux Kernel Mailing List, AKASHI Takahiro, Alexander Graf,
Bjorn Andersson, Borislav Petkov, Heinrich Schuchardt,
Jeffrey Hugo, Lee Jones, Leif Lindholm, Linus Torvalds,
Peter Jones, Peter Zijlstra
On Mon, 4 Feb 2019 at 23:29, Prakhya, Sai Praneeth
<sai.praneeth.prakhya@intel.com> wrote:
>
> > > > efi_map_region() creates VA mappings for an given EFI region using
> > > > any one of the two helper functions (namely __map_region() and
> > old_map_region()).
> > > > These helper functions *could* fail while creating mappings and
> > > > presently their return value is not checked. Not checking for the
> > > > return value of these functions might create issues because after
> > > > these functions return "md->virt_addr" is set to the requested
> > > > virtual address (so it's assumed that these functions always succeed
> > > > which is not quite true). This assumption leads to "md->virt_addr"
> > > > having invalid mapping should any of
> > > > __map_region() or old_map_region() fail.
> > > >
> > > > Hence, check for the return value of these functions and if indeed
> > > > they fail, turn off EFI Runtime Services forever because kernel
> > > > cannot prioritize among EFI regions.
> > > >
> [...................]
> > > > -void __init old_map_region(efi_memory_desc_t *md)
> > > > +int __init old_map_region(efi_memory_desc_t *md)
> > > > {
> > > > u64 start_pfn, end_pfn, end;
> > > > unsigned long size;
> > > > @@ -601,10 +601,14 @@ void __init old_map_region(efi_memory_desc_t
> > *md)
> > > > va = efi_ioremap(md->phys_addr, size,
> > > > md->type, md->attribute);
> > > >
> > > > - md->virt_addr = (u64) (unsigned long) va;
> > > > - if (!va)
> > > > + if (!va) {
> > > > pr_err("ioremap of 0x%llX failed!\n",
> > > > (unsigned long long)md->phys_addr);
> > > > + return -ENOMEM;
> > > > + }
> > > > +
> > > > + md->virt_addr = (u64)(unsigned long)va;
> > > > + return 0;
> > >
> > > Just wondering, shouldn't the failure path set ->virt_addr to
> > > something safe, just in case a caller doesn't check the error and relies on it?
> > >
> > > That's because in this commit we've now changed it from 0 to undefined.
> > >
> >
> > Indeed. We don't usually rely on the value of ->virt_addr when
> > EFI_RUNTIME_SERVICES is unset, but there is some sysfs code, and perhaps
> > some other places where we do reference ->virt_addr, and not assigning it at all
> > is obviously wrong, and potentially hazardous.
> >
>
> Ok.. makes sense.
> Do you think md->virt_addr = 0 for fail case is ok?
>
0 should be fine. You shouldn't be able to dereference that anywhere.
> > > > +int __init efi_map_region_fixed(efi_memory_desc_t *md) { return 0;
> > > > +}
> > >
> > > Inline functions should be marked inline ...
> > >
> > > > if (efi_va < EFI_VA_END) {
> > > > - pr_warn(FW_WARN "VA address range overflow!\n");
> > > > - return;
> > > > + pr_err(FW_WARN "VA address range overflow!\n");
> > > > + return -ENOMEM;
> > > > }
> > > >
> > > > /* Do the VA map */
> > > > - __map_region(md, efi_va);
> > > > + if (__map_region(md, efi_va))
> > > > + return -ENOMEM;
> > > > +
> > > > md->virt_addr = efi_va;
> > > > + return 0;
> > >
> > > Same error return problem of leaving ->virt_addr undefined.
> > >
>
> Sure! Will fix it in V2.
>
> > > Note that I also fixed up the grammar and readability of the changelog
> > > - see the updated version below.
>
> Thanks for fixing :)
>
> > >
> > > Thanks,
> > >
> > > Ingo
> > >
> > > =============>
> > > Subject: x86/efi: Return error status if mapping of EFI regions fails
> > > From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > Date: Sat, 2 Feb 2019 10:41:11 +0100
> > >
> > > From: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
> > >
> > > efi_map_region() creates VA mappings for a given EFI region using one
> > > of the two helper functions (namely __map_region() and old_map_region()).
> > >
> > > These helper functions could fail while creating mappings and
> > > presently their return value is not checked.
> > >
> > > Not checking for the return value of these functions might create
> > > bugs, because after these functions return "md->virt_addr" is set to
> > > the requested virtual address (so it's assumed that these functions
> > > always succeed which is not quite true). This assumption leads to
> > > "md->virt_addr" having invalid mapping, should any of __map_region()
> > > or old_map_region() fail.
> > >
> > > Hence, check for the return value of these functions and if indeed
> > > they fail, turn off EFI Runtime Services forever because kernel cannot
> > > prioritize among EFI regions.
> > >
> > > This also fixes the comment "FIXME: add error handling" in
> > > kexec_enter_virtual_mode().
> > >
> >
> > Thanks Ingo.
> >
> > Sai, could you please respin this and use Ingo's updated version of the commit
> > log?
>
> Sure! I will send a V2 with the mentioned changes.
>
> Regards,
> Sai
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 03/10] efi: memattr: don't bail on zero VA if it equals the region's PA
2019-02-02 9:41 [GIT PULL 00/10] EFI changes for v5.1 Ard Biesheuvel
2019-02-02 9:41 ` [PATCH 01/10] x86/efi: Mark can_free_region() as an __init function Ard Biesheuvel
2019-02-02 9:41 ` [PATCH 02/10] x86/efi: Return error status if mapping EFI regions fail Ard Biesheuvel
@ 2019-02-02 9:41 ` Ard Biesheuvel
2019-02-02 9:41 ` [PATCH 04/10] efi: use 32-bit alignment for efi_guid_t Ard Biesheuvel
` (6 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Ard Biesheuvel @ 2019-02-02 9:41 UTC (permalink / raw)
To: linux-efi, Ingo Molnar, Thomas Gleixner
Cc: Ard Biesheuvel, linux-kernel, AKASHI Takahiro, Alexander Graf,
Bjorn Andersson, Borislav Petkov, Heinrich Schuchardt,
Jeffrey Hugo, Lee Jones, Leif Lindholm, Linus Torvalds,
Peter Jones, Peter Zijlstra, Sai Praneeth Prakhya
The EFI memory attributes code cross-references the EFI memory map with
the more granular EFI memory attributes table to ensure that they are in
sync before applying the strict permissions to the regions it describes.
Since we always install virtual mappings for the EFI runtime regions to
which these strict permissions apply, we currently perform a sanity check
on the EFI memory descriptor, and ensure that the EFI_MEMORY_RUNTIME bit
is set, and that the virtual address has been assigned.
However, in cases where a runtime region exists at physical address 0x0,
and the virtual mapping equals the physical mapping, e.g., when running
in mixed mode on x86, we encounter a memory descriptor with the runtime
attribute and virtual address 0x0, and incorrectly draw the conclusion
that a runtime region exists for which no virtual mapping was installed,
and give up altogether. The consequence of this is that firmware mappings
retain their read-write-execute permissions, making the system more
vulnerable to attacks.
So let's only bail if the virtual address of 0x0 has been assigned to a
physical region that does not reside at address 0x0.
Fixes: 10f0d2f577053 ("efi: Implement generic support for the Memory ...")
Acked-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
drivers/firmware/efi/memattr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/firmware/efi/memattr.c b/drivers/firmware/efi/memattr.c
index 8986757eafaf..aac972b056d9 100644
--- a/drivers/firmware/efi/memattr.c
+++ b/drivers/firmware/efi/memattr.c
@@ -94,7 +94,7 @@ static bool entry_is_valid(const efi_memory_desc_t *in, efi_memory_desc_t *out)
if (!(md->attribute & EFI_MEMORY_RUNTIME))
continue;
- if (md->virt_addr == 0) {
+ if (md->virt_addr == 0 && md->phys_addr != 0) {
/* no virtual mapping has been installed by the stub */
break;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 04/10] efi: use 32-bit alignment for efi_guid_t
2019-02-02 9:41 [GIT PULL 00/10] EFI changes for v5.1 Ard Biesheuvel
` (2 preceding siblings ...)
2019-02-02 9:41 ` [PATCH 03/10] efi: memattr: don't bail on zero VA if it equals the region's PA Ard Biesheuvel
@ 2019-02-02 9:41 ` Ard Biesheuvel
2019-02-02 9:41 ` [PATCH 05/10] efi/fdt: More cleanups Ard Biesheuvel
` (5 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Ard Biesheuvel @ 2019-02-02 9:41 UTC (permalink / raw)
To: linux-efi, Ingo Molnar, Thomas Gleixner
Cc: Ard Biesheuvel, linux-kernel, AKASHI Takahiro, Alexander Graf,
Bjorn Andersson, Borislav Petkov, Heinrich Schuchardt,
Jeffrey Hugo, Lee Jones, Leif Lindholm, Linus Torvalds,
Peter Jones, Peter Zijlstra, Sai Praneeth Prakhya
The UEFI spec and EDK2 reference implementation both define EFI_GUID as
struct { u32 a; u16; b; u16 c; u8 d[8]; }; and so the implied alignment
is 32 bits not 8 bits like our guid_t. In some cases (i.e., on 32-bit ARM),
this means that firmware services invoked by the kernel may assume that
efi_guid_t* arguments are 32-bit aligned, and use memory accessors that
do not tolerate misalignment. So let's set the minimum alignment to 32 bits.
Note that the UEFI spec as well as some comments in the EDK2 code base
suggest that EFI_GUID should be 64-bit aligned, but this appears to be
a mistake, given that no code seems to exist that actually enforces that
or relies on it.
Reported-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
include/linux/efi.h | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 45ff763fba76..be08518c2553 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -48,7 +48,20 @@ typedef u16 efi_char16_t; /* UNICODE character */
typedef u64 efi_physical_addr_t;
typedef void *efi_handle_t;
-typedef guid_t efi_guid_t;
+/*
+ * The UEFI spec and EDK2 reference implementation both define EFI_GUID as
+ * struct { u32 a; u16; b; u16 c; u8 d[8]; }; and so the implied alignment
+ * is 32 bits not 8 bits like our guid_t. In some cases (i.e., on 32-bit ARM),
+ * this means that firmware services invoked by the kernel may assume that
+ * efi_guid_t* arguments are 32-bit aligned, and use memory accessors that
+ * do not tolerate misalignment. So let's set the minimum alignment to 32 bits.
+ *
+ * Note that the UEFI spec as well as some comments in the EDK2 code base
+ * suggest that EFI_GUID should be 64-bit aligned, but this appears to be
+ * a mistake, given that no code seems to exist that actually enforces that
+ * or relies on it.
+ */
+typedef guid_t efi_guid_t __aligned(__alignof__(u32));
#define EFI_GUID(a,b,c,d0,d1,d2,d3,d4,d5,d6,d7) \
GUID_INIT(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7)
--
2.17.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 05/10] efi/fdt: More cleanups
2019-02-02 9:41 [GIT PULL 00/10] EFI changes for v5.1 Ard Biesheuvel
` (3 preceding siblings ...)
2019-02-02 9:41 ` [PATCH 04/10] efi: use 32-bit alignment for efi_guid_t Ard Biesheuvel
@ 2019-02-02 9:41 ` Ard Biesheuvel
2019-02-02 9:41 ` [PATCH 06/10] efi: replace GPL license boilerplate with SPDX headers Ard Biesheuvel
` (4 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Ard Biesheuvel @ 2019-02-02 9:41 UTC (permalink / raw)
To: linux-efi, Ingo Molnar, Thomas Gleixner
Cc: Ard Biesheuvel, linux-kernel, AKASHI Takahiro, Alexander Graf,
Bjorn Andersson, Borislav Petkov, Heinrich Schuchardt,
Jeffrey Hugo, Lee Jones, Leif Lindholm, Linus Torvalds,
Peter Jones, Peter Zijlstra, Sai Praneeth Prakhya
From: Ingo Molnar <mingo@kernel.org>
Apply a number of cleanups:
- Introduce fdt_setprop_*var() helper macros to simplify and shorten repetitive
sequences - this also makes it less likely that the wrong variable size is
passed in. This change makes a lot of the property-setting calls single-line
and easier to read.
- Harmonize comment style: capitalization, punctuation, whitespaces, etc.
- Fix some whitespace noise in the libstub Makefile which I happened to notice.
- Use the standard tabular initialization style:
- map.map = &runtime_map;
- map.map_size = &map_size;
- map.desc_size = &desc_size;
- map.desc_ver = &desc_ver;
- map.key_ptr = &mmap_key;
- map.buff_size = &buff_size;
+ map.map = &runtime_map;
+ map.map_size = &map_size;
+ map.desc_size = &desc_size;
+ map.desc_ver = &desc_ver;
+ map.key_ptr = &mmap_key;
+ map.buff_size = &buff_size;
- Use tabular structure definition for better readability.
- Make all pr*() lines single-line, even if they marginally exceed 80 cols - this
makes them visually less intrusive.
- Unbreak line breaks into single lines when the length exceeds 80 cols only
marginally, for better readability.
- Move assignment closer to the actual usage site.
- Plus some other smaller cleanups, spelling fixes, etc.
No change in functionality intended.
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
[ardb: move changes to upstream libfdt into local header]
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
drivers/firmware/efi/libstub/Makefile | 4 +-
drivers/firmware/efi/libstub/efistub.h | 11 +++
drivers/firmware/efi/libstub/fdt.c | 107 ++++++++++++-------------
3 files changed, 64 insertions(+), 58 deletions(-)
diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index d9845099635e..b0103e16fc1b 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -52,7 +52,7 @@ lib-$(CONFIG_EFI_ARMSTUB) += arm-stub.o fdt.o string.o random.o \
lib-$(CONFIG_ARM) += arm32-stub.o
lib-$(CONFIG_ARM64) += arm64-stub.o
-CFLAGS_arm64-stub.o := -DTEXT_OFFSET=$(TEXT_OFFSET)
+CFLAGS_arm64-stub.o := -DTEXT_OFFSET=$(TEXT_OFFSET)
#
# arm64 puts the stub in the kernel proper, which will unnecessarily retain all
@@ -89,7 +89,7 @@ quiet_cmd_stubcopy = STUBCPY $@
cmd_stubcopy = if $(STRIP) --strip-debug $(STUBCOPY_RM-y) -o $@ $<; \
then if $(OBJDUMP) -r $@ | grep $(STUBCOPY_RELOC-y); \
then (echo >&2 "$@: absolute symbol references not allowed in the EFI stub"; \
- rm -f $@; /bin/false); \
+ rm -f $@; /bin/false); \
else $(OBJCOPY) $(STUBCOPY_FLAGS-y) $< $@; fi \
else /bin/false; fi
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index 32799cf039ef..cefcf6ba3150 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -64,4 +64,15 @@ efi_status_t check_platform_features(efi_system_table_t *sys_table_arg);
efi_status_t efi_random_get_seed(efi_system_table_t *sys_table_arg);
+/* Helper macros for the usual case of using simple C variables: */
+#ifndef fdt_setprop_inplace_var
+#define fdt_setprop_inplace_var(fdt, node_offset, name, var) \
+ fdt_setprop_inplace((fdt), (node_offset), (name), &(var), sizeof(var))
+#endif
+
+#ifndef fdt_setprop_var
+#define fdt_setprop_var(fdt, node_offset, name, var) \
+ fdt_setprop((fdt), (node_offset), (name), &(var), sizeof(var))
+#endif
+
#endif
diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
index 0dc7b4987cc2..d474964b873b 100644
--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -26,10 +26,8 @@ static void fdt_update_cell_size(efi_system_table_t *sys_table, void *fdt)
offset = fdt_path_offset(fdt, "/");
/* Set the #address-cells and #size-cells values for an empty tree */
- fdt_setprop_u32(fdt, offset, "#address-cells",
- EFI_DT_ADDR_CELLS_DEFAULT);
-
- fdt_setprop_u32(fdt, offset, "#size-cells", EFI_DT_SIZE_CELLS_DEFAULT);
+ fdt_setprop_u32(fdt, offset, "#address-cells", EFI_DT_ADDR_CELLS_DEFAULT);
+ fdt_setprop_u32(fdt, offset, "#size-cells", EFI_DT_SIZE_CELLS_DEFAULT);
}
static efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
@@ -42,7 +40,7 @@ static efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
u32 fdt_val32;
u64 fdt_val64;
- /* Do some checks on provided FDT, if it exists*/
+ /* Do some checks on provided FDT, if it exists: */
if (orig_fdt) {
if (fdt_check_header(orig_fdt)) {
pr_efi_err(sys_table, "Device Tree header not valid!\n");
@@ -50,7 +48,7 @@ static efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
}
/*
* We don't get the size of the FDT if we get if from a
- * configuration table.
+ * configuration table:
*/
if (orig_fdt_size && fdt_totalsize(orig_fdt) > orig_fdt_size) {
pr_efi_err(sys_table, "Truncated device tree! foo!\n");
@@ -64,8 +62,8 @@ static efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
status = fdt_create_empty_tree(fdt, new_fdt_size);
if (status == 0) {
/*
- * Any failure from the following function is non
- * critical
+ * Any failure from the following function is
+ * non-critical:
*/
fdt_update_cell_size(sys_table, fdt);
}
@@ -86,12 +84,13 @@ static efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
if (node < 0) {
node = fdt_add_subnode(fdt, 0, "chosen");
if (node < 0) {
- status = node; /* node is error code when negative */
+ /* 'node' is an error code when negative: */
+ status = node;
goto fdt_set_fail;
}
}
- if ((cmdline_ptr != NULL) && (strlen(cmdline_ptr) > 0)) {
+ if (cmdline_ptr != NULL && strlen(cmdline_ptr) > 0) {
status = fdt_setprop(fdt, node, "bootargs", cmdline_ptr,
strlen(cmdline_ptr) + 1);
if (status)
@@ -103,13 +102,12 @@ static efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
u64 initrd_image_end;
u64 initrd_image_start = cpu_to_fdt64(initrd_addr);
- status = fdt_setprop(fdt, node, "linux,initrd-start",
- &initrd_image_start, sizeof(u64));
+ status = fdt_setprop_var(fdt, node, "linux,initrd-start", initrd_image_start);
if (status)
goto fdt_set_fail;
+
initrd_image_end = cpu_to_fdt64(initrd_addr + initrd_size);
- status = fdt_setprop(fdt, node, "linux,initrd-end",
- &initrd_image_end, sizeof(u64));
+ status = fdt_setprop_var(fdt, node, "linux,initrd-end", initrd_image_end);
if (status)
goto fdt_set_fail;
}
@@ -117,30 +115,28 @@ static efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
/* Add FDT entries for EFI runtime services in chosen node. */
node = fdt_subnode_offset(fdt, 0, "chosen");
fdt_val64 = cpu_to_fdt64((u64)(unsigned long)sys_table);
- status = fdt_setprop(fdt, node, "linux,uefi-system-table",
- &fdt_val64, sizeof(fdt_val64));
+
+ status = fdt_setprop_var(fdt, node, "linux,uefi-system-table", fdt_val64);
if (status)
goto fdt_set_fail;
fdt_val64 = U64_MAX; /* placeholder */
- status = fdt_setprop(fdt, node, "linux,uefi-mmap-start",
- &fdt_val64, sizeof(fdt_val64));
+
+ status = fdt_setprop_var(fdt, node, "linux,uefi-mmap-start", fdt_val64);
if (status)
goto fdt_set_fail;
fdt_val32 = U32_MAX; /* placeholder */
- status = fdt_setprop(fdt, node, "linux,uefi-mmap-size",
- &fdt_val32, sizeof(fdt_val32));
+
+ status = fdt_setprop_var(fdt, node, "linux,uefi-mmap-size", fdt_val32);
if (status)
goto fdt_set_fail;
- status = fdt_setprop(fdt, node, "linux,uefi-mmap-desc-size",
- &fdt_val32, sizeof(fdt_val32));
+ status = fdt_setprop_var(fdt, node, "linux,uefi-mmap-desc-size", fdt_val32);
if (status)
goto fdt_set_fail;
- status = fdt_setprop(fdt, node, "linux,uefi-mmap-desc-ver",
- &fdt_val32, sizeof(fdt_val32));
+ status = fdt_setprop_var(fdt, node, "linux,uefi-mmap-desc-ver", fdt_val32);
if (status)
goto fdt_set_fail;
@@ -150,8 +146,7 @@ static efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
efi_status = efi_get_random_bytes(sys_table, sizeof(fdt_val64),
(u8 *)&fdt_val64);
if (efi_status == EFI_SUCCESS) {
- status = fdt_setprop(fdt, node, "kaslr-seed",
- &fdt_val64, sizeof(fdt_val64));
+ status = fdt_setprop_var(fdt, node, "kaslr-seed", fdt_val64);
if (status)
goto fdt_set_fail;
} else if (efi_status != EFI_NOT_FOUND) {
@@ -159,7 +154,7 @@ static efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
}
}
- /* shrink the FDT back to its minimum size */
+ /* Shrink the FDT back to its minimum size: */
fdt_pack(fdt);
return EFI_SUCCESS;
@@ -182,26 +177,26 @@ static efi_status_t update_fdt_memmap(void *fdt, struct efi_boot_memmap *map)
return EFI_LOAD_ERROR;
fdt_val64 = cpu_to_fdt64((unsigned long)*map->map);
- err = fdt_setprop_inplace(fdt, node, "linux,uefi-mmap-start",
- &fdt_val64, sizeof(fdt_val64));
+
+ err = fdt_setprop_inplace_var(fdt, node, "linux,uefi-mmap-start", fdt_val64);
if (err)
return EFI_LOAD_ERROR;
fdt_val32 = cpu_to_fdt32(*map->map_size);
- err = fdt_setprop_inplace(fdt, node, "linux,uefi-mmap-size",
- &fdt_val32, sizeof(fdt_val32));
+
+ err = fdt_setprop_inplace_var(fdt, node, "linux,uefi-mmap-size", fdt_val32);
if (err)
return EFI_LOAD_ERROR;
fdt_val32 = cpu_to_fdt32(*map->desc_size);
- err = fdt_setprop_inplace(fdt, node, "linux,uefi-mmap-desc-size",
- &fdt_val32, sizeof(fdt_val32));
+
+ err = fdt_setprop_inplace_var(fdt, node, "linux,uefi-mmap-desc-size", fdt_val32);
if (err)
return EFI_LOAD_ERROR;
fdt_val32 = cpu_to_fdt32(*map->desc_ver);
- err = fdt_setprop_inplace(fdt, node, "linux,uefi-mmap-desc-ver",
- &fdt_val32, sizeof(fdt_val32));
+
+ err = fdt_setprop_inplace_var(fdt, node, "linux,uefi-mmap-desc-ver", fdt_val32);
if (err)
return EFI_LOAD_ERROR;
@@ -209,13 +204,13 @@ static efi_status_t update_fdt_memmap(void *fdt, struct efi_boot_memmap *map)
}
#ifndef EFI_FDT_ALIGN
-#define EFI_FDT_ALIGN EFI_PAGE_SIZE
+# define EFI_FDT_ALIGN EFI_PAGE_SIZE
#endif
struct exit_boot_struct {
- efi_memory_desc_t *runtime_map;
- int *runtime_entry_count;
- void *new_fdt_addr;
+ efi_memory_desc_t *runtime_map;
+ int *runtime_entry_count;
+ void *new_fdt_addr;
};
static efi_status_t exit_boot_func(efi_system_table_t *sys_table_arg,
@@ -235,7 +230,7 @@ static efi_status_t exit_boot_func(efi_system_table_t *sys_table_arg,
}
#ifndef MAX_FDT_SIZE
-#define MAX_FDT_SIZE SZ_2M
+# define MAX_FDT_SIZE SZ_2M
#endif
/*
@@ -266,16 +261,16 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
unsigned long mmap_key;
efi_memory_desc_t *memory_map, *runtime_map;
efi_status_t status;
- int runtime_entry_count = 0;
+ int runtime_entry_count;
struct efi_boot_memmap map;
struct exit_boot_struct priv;
- map.map = &runtime_map;
- map.map_size = &map_size;
- map.desc_size = &desc_size;
- map.desc_ver = &desc_ver;
- map.key_ptr = &mmap_key;
- map.buff_size = &buff_size;
+ map.map = &runtime_map;
+ map.map_size = &map_size;
+ map.desc_size = &desc_size;
+ map.desc_ver = &desc_ver;
+ map.key_ptr = &mmap_key;
+ map.buff_size = &buff_size;
/*
* Get a copy of the current memory map that we will use to prepare
@@ -289,15 +284,13 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
return status;
}
- pr_efi(sys_table,
- "Exiting boot services and installing virtual address map...\n");
+ pr_efi(sys_table, "Exiting boot services and installing virtual address map...\n");
map.map = &memory_map;
status = efi_high_alloc(sys_table, MAX_FDT_SIZE, EFI_FDT_ALIGN,
new_fdt_addr, max_addr);
if (status != EFI_SUCCESS) {
- pr_efi_err(sys_table,
- "Unable to allocate memory for new device tree.\n");
+ pr_efi_err(sys_table, "Unable to allocate memory for new device tree.\n");
goto fail;
}
@@ -318,11 +311,12 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
goto fail_free_new_fdt;
}
- priv.runtime_map = runtime_map;
- priv.runtime_entry_count = &runtime_entry_count;
- priv.new_fdt_addr = (void *)*new_fdt_addr;
- status = efi_exit_boot_services(sys_table, handle, &map, &priv,
- exit_boot_func);
+ runtime_entry_count = 0;
+ priv.runtime_map = runtime_map;
+ priv.runtime_entry_count = &runtime_entry_count;
+ priv.new_fdt_addr = (void *)*new_fdt_addr;
+
+ status = efi_exit_boot_services(sys_table, handle, &map, &priv, exit_boot_func);
if (status == EFI_SUCCESS) {
efi_set_virtual_address_map_t *svam;
@@ -363,6 +357,7 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
fail:
sys_table->boottime->free_pool(runtime_map);
+
return EFI_LOAD_ERROR;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 06/10] efi: replace GPL license boilerplate with SPDX headers
2019-02-02 9:41 [GIT PULL 00/10] EFI changes for v5.1 Ard Biesheuvel
` (4 preceding siblings ...)
2019-02-02 9:41 ` [PATCH 05/10] efi/fdt: More cleanups Ard Biesheuvel
@ 2019-02-02 9:41 ` Ard Biesheuvel
2019-02-02 9:41 ` [PATCH 07/10] efi: arm/arm64: allow SetVirtualAddressMap() to be omitted Ard Biesheuvel
` (3 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Ard Biesheuvel @ 2019-02-02 9:41 UTC (permalink / raw)
To: linux-efi, Ingo Molnar, Thomas Gleixner
Cc: Ard Biesheuvel, linux-kernel, AKASHI Takahiro, Alexander Graf,
Bjorn Andersson, Borislav Petkov, Heinrich Schuchardt,
Jeffrey Hugo, Lee Jones, Leif Lindholm, Linus Torvalds,
Peter Jones, Peter Zijlstra, Sai Praneeth Prakhya
Replace all GPL license blurbs with an equivalent SPDX header (most
files are GPLv2, some are GPLv2+). While at it, drop some outdated
header changelogs as well.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
drivers/firmware/efi/apple-properties.c | 13 +----
drivers/firmware/efi/arm-init.c | 6 +-
drivers/firmware/efi/arm-runtime.c | 6 +-
drivers/firmware/efi/capsule-loader.c | 4 +-
drivers/firmware/efi/capsule.c | 4 +-
drivers/firmware/efi/cper-arm.c | 14 +----
drivers/firmware/efi/cper.c | 14 +----
drivers/firmware/efi/dev-path-parser.c | 9 +--
drivers/firmware/efi/efi-bgrt.c | 5 +-
drivers/firmware/efi/efi-pstore.c | 2 +
drivers/firmware/efi/efibc.c | 10 +---
drivers/firmware/efi/efivars.c | 58 +------------------
drivers/firmware/efi/esrt.c | 1 +
drivers/firmware/efi/fake_mem.c | 16 +----
drivers/firmware/efi/libstub/arm32-stub.c | 6 +-
drivers/firmware/efi/libstub/arm64-stub.c | 6 +-
.../firmware/efi/libstub/efi-stub-helper.c | 5 +-
drivers/firmware/efi/libstub/fdt.c | 5 +-
drivers/firmware/efi/libstub/gop.c | 4 +-
drivers/firmware/efi/libstub/random.c | 6 +-
drivers/firmware/efi/libstub/secureboot.c | 4 +-
drivers/firmware/efi/libstub/tpm.c | 4 +-
drivers/firmware/efi/memattr.c | 5 +-
drivers/firmware/efi/runtime-map.c | 3 +-
drivers/firmware/efi/test/efi_test.c | 1 +
drivers/firmware/efi/test/efi_test.h | 2 +-
drivers/firmware/efi/tpm.c | 5 +-
drivers/firmware/efi/vars.c | 15 +----
28 files changed, 29 insertions(+), 204 deletions(-)
diff --git a/drivers/firmware/efi/apple-properties.c b/drivers/firmware/efi/apple-properties.c
index ac1654f74dc7..0e206c9e0d7a 100644
--- a/drivers/firmware/efi/apple-properties.c
+++ b/drivers/firmware/efi/apple-properties.c
@@ -1,19 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* apple-properties.c - EFI device properties on Macs
* Copyright (C) 2016 Lukas Wunner <lukas@wunner.de>
*
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License (version 2) as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, see <http://www.gnu.org/licenses/>.
- *
* Note, all properties are considered as u8 arrays.
* To get a value of any of them the caller must use device_property_read_u8_array().
*/
diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
index 1a6a77df8a5e..311cd349a862 100644
--- a/drivers/firmware/efi/arm-init.c
+++ b/drivers/firmware/efi/arm-init.c
@@ -1,14 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* Extensible Firmware Interface
*
* Based on Extensible Firmware Interface Specification version 2.4
*
* Copyright (C) 2013 - 2015 Linaro Ltd.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- *
*/
#define pr_fmt(fmt) "efi: " fmt
diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
index 23ea1ed409d1..19ab3ad9cbba 100644
--- a/drivers/firmware/efi/arm-runtime.c
+++ b/drivers/firmware/efi/arm-runtime.c
@@ -1,14 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* Extensible Firmware Interface
*
* Based on Extensible Firmware Interface Specification version 2.4
*
* Copyright (C) 2013, 2014 Linaro Ltd.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- *
*/
#include <linux/dmi.h>
diff --git a/drivers/firmware/efi/capsule-loader.c b/drivers/firmware/efi/capsule-loader.c
index 96688986da56..b1395133389e 100644
--- a/drivers/firmware/efi/capsule-loader.c
+++ b/drivers/firmware/efi/capsule-loader.c
@@ -1,10 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* EFI capsule loader driver.
*
* Copyright 2015 Intel Corporation
- *
- * This file is part of the Linux kernel, and is made available under
- * the terms of the GNU General Public License version 2.
*/
#define pr_fmt(fmt) "efi: " fmt
diff --git a/drivers/firmware/efi/capsule.c b/drivers/firmware/efi/capsule.c
index 4938c29b7c5d..598b7800d14e 100644
--- a/drivers/firmware/efi/capsule.c
+++ b/drivers/firmware/efi/capsule.c
@@ -1,10 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* EFI capsule support.
*
* Copyright 2013 Intel Corporation; author Matt Fleming
- *
- * This file is part of the Linux kernel, and is made available under
- * the terms of the GNU General Public License version 2.
*/
#define pr_fmt(fmt) "efi: " fmt
diff --git a/drivers/firmware/efi/cper-arm.c b/drivers/firmware/efi/cper-arm.c
index 502811344e81..36d3b8b9da47 100644
--- a/drivers/firmware/efi/cper-arm.c
+++ b/drivers/firmware/efi/cper-arm.c
@@ -1,20 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* UEFI Common Platform Error Record (CPER) support
*
* Copyright (C) 2017, The Linux Foundation. All rights reserved.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License version
- * 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/
#include <linux/kernel.h>
diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
index a7902fccdcfa..6a966ecdd80a 100644
--- a/drivers/firmware/efi/cper.c
+++ b/drivers/firmware/efi/cper.c
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* UEFI Common Platform Error Record (CPER) support
*
@@ -9,19 +10,6 @@
*
* For more information about CPER, please refer to Appendix N of UEFI
* Specification version 2.4.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License version
- * 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/
#include <linux/kernel.h>
diff --git a/drivers/firmware/efi/dev-path-parser.c b/drivers/firmware/efi/dev-path-parser.c
index 85d1834ee9b7..85ec99f97841 100644
--- a/drivers/firmware/efi/dev-path-parser.c
+++ b/drivers/firmware/efi/dev-path-parser.c
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* dev-path-parser.c - EFI Device Path parser
* Copyright (C) 2016 Lukas Wunner <lukas@wunner.de>
@@ -5,14 +6,6 @@
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License (version 2) as
* published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, see <http://www.gnu.org/licenses/>.
*/
#include <linux/acpi.h>
diff --git a/drivers/firmware/efi/efi-bgrt.c b/drivers/firmware/efi/efi-bgrt.c
index b22ccfb0c991..a2384184a7de 100644
--- a/drivers/firmware/efi/efi-bgrt.c
+++ b/drivers/firmware/efi/efi-bgrt.c
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* Copyright 2012 Intel Corporation
* Author: Josh Triplett <josh@joshtriplett.org>
@@ -5,10 +6,6 @@
* Based on the bgrt driver:
* Copyright 2012 Red Hat, Inc <mjg@redhat.com>
* Author: Matthew Garrett
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
*/
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
diff --git a/drivers/firmware/efi/efi-pstore.c b/drivers/firmware/efi/efi-pstore.c
index 0f7d97917197..9ea13e8d12ec 100644
--- a/drivers/firmware/efi/efi-pstore.c
+++ b/drivers/firmware/efi/efi-pstore.c
@@ -1,3 +1,5 @@
+// SPDX-License-Identifier: GPL-2.0+
+
#include <linux/efi.h>
#include <linux/module.h>
#include <linux/pstore.h>
diff --git a/drivers/firmware/efi/efibc.c b/drivers/firmware/efi/efibc.c
index 503bbe2a9d49..61e099826cbb 100644
--- a/drivers/firmware/efi/efibc.c
+++ b/drivers/firmware/efi/efibc.c
@@ -1,15 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* efibc: control EFI bootloaders which obey LoaderEntryOneShot var
* Copyright (c) 2013-2016, Intel Corporation.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
- * more details.
*/
#define pr_fmt(fmt) "efibc: " fmt
diff --git a/drivers/firmware/efi/efivars.c b/drivers/firmware/efi/efivars.c
index 8061667a6765..7576450c8254 100644
--- a/drivers/firmware/efi/efivars.c
+++ b/drivers/firmware/efi/efivars.c
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0+
/*
* Originally from efivars.c,
*
@@ -6,63 +7,6 @@
*
* This code takes all variables accessible from EFI runtime and
* exports them via sysfs
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
- *
- * Changelog:
- *
- * 17 May 2004 - Matt Domsch <Matt_Domsch@dell.com>
- * remove check for efi_enabled in exit
- * add MODULE_VERSION
- *
- * 26 Apr 2004 - Matt Domsch <Matt_Domsch@dell.com>
- * minor bug fixes
- *
- * 21 Apr 2004 - Matt Tolentino <matthew.e.tolentino@intel.com)
- * converted driver to export variable information via sysfs
- * and moved to drivers/firmware directory
- * bumped revision number to v0.07 to reflect conversion & move
- *
- * 10 Dec 2002 - Matt Domsch <Matt_Domsch@dell.com>
- * fix locking per Peter Chubb's findings
- *
- * 25 Mar 2002 - Matt Domsch <Matt_Domsch@dell.com>
- * move uuid_unparse() to include/asm-ia64/efi.h:efi_guid_to_str()
- *
- * 12 Feb 2002 - Matt Domsch <Matt_Domsch@dell.com>
- * use list_for_each_safe when deleting vars.
- * remove ifdef CONFIG_SMP around include <linux/smp.h>
- * v0.04 release to linux-ia64@linuxia64.org
- *
- * 20 April 2001 - Matt Domsch <Matt_Domsch@dell.com>
- * Moved vars from /proc/efi to /proc/efi/vars, and made
- * efi.c own the /proc/efi directory.
- * v0.03 release to linux-ia64@linuxia64.org
- *
- * 26 March 2001 - Matt Domsch <Matt_Domsch@dell.com>
- * At the request of Stephane, moved ownership of /proc/efi
- * to efi.c, and now efivars lives under /proc/efi/vars.
- *
- * 12 March 2001 - Matt Domsch <Matt_Domsch@dell.com>
- * Feedback received from Stephane Eranian incorporated.
- * efivar_write() checks copy_from_user() return value.
- * efivar_read/write() returns proper errno.
- * v0.02 release to linux-ia64@linuxia64.org
- *
- * 26 February 2001 - Matt Domsch <Matt_Domsch@dell.com>
- * v0.01 release to linux-ia64@linuxia64.org
*/
#include <linux/efi.h>
diff --git a/drivers/firmware/efi/esrt.c b/drivers/firmware/efi/esrt.c
index 5d06bd247d07..d6dd5f503fa2 100644
--- a/drivers/firmware/efi/esrt.c
+++ b/drivers/firmware/efi/esrt.c
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0+
/*
* esrt.c
*
diff --git a/drivers/firmware/efi/fake_mem.c b/drivers/firmware/efi/fake_mem.c
index 6c7d60c239b5..9501edc0fcfb 100644
--- a/drivers/firmware/efi/fake_mem.c
+++ b/drivers/firmware/efi/fake_mem.c
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* fake_mem.c
*
@@ -8,21 +9,6 @@
* By specifying this parameter, you can add arbitrary attribute to
* specific memory range by updating original (firmware provided) EFI
* memmap.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
- * more details.
- *
- * You should have received a copy of the GNU General Public License along with
- * this program; if not, see <http://www.gnu.org/licenses/>.
- *
- * The full GNU General Public License is included in this distribution in
- * the file called "COPYING".
*/
#include <linux/kernel.h>
diff --git a/drivers/firmware/efi/libstub/arm32-stub.c b/drivers/firmware/efi/libstub/arm32-stub.c
index becbda445913..e8f7aefb6813 100644
--- a/drivers/firmware/efi/libstub/arm32-stub.c
+++ b/drivers/firmware/efi/libstub/arm32-stub.c
@@ -1,10 +1,6 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* Copyright (C) 2013 Linaro Ltd; <roy.franz@linaro.org>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- *
*/
#include <linux/efi.h>
#include <asm/efi.h>
diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
index 1b4d465cc5d9..1550d244e996 100644
--- a/drivers/firmware/efi/libstub/arm64-stub.c
+++ b/drivers/firmware/efi/libstub/arm64-stub.c
@@ -1,13 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* Copyright (C) 2013, 2014 Linaro Ltd; <roy.franz@linaro.org>
*
* This file implements the EFI boot stub for the arm64 kernel.
* Adapted from ARM version by Mark Salter <msalter@redhat.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- *
*/
/*
diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index e94975f4655b..8f2d000bec75 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -1,13 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* Helper functions used by the EFI stub on multiple
* architectures. This should be #included by the EFI stub
* implementation files.
*
* Copyright 2011 Intel Corporation; author Matt Fleming
- *
- * This file is part of the Linux kernel, and is made available
- * under the terms of the GNU General Public License version 2.
- *
*/
#include <linux/efi.h>
diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
index d474964b873b..7cfc51935c4b 100644
--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -1,13 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* FDT related Helper functions used by the EFI stub on multiple
* architectures. This should be #included by the EFI stub
* implementation files.
*
* Copyright 2013 Linaro Limited; author Roy Franz
- *
- * This file is part of the Linux kernel, and is made available
- * under the terms of the GNU General Public License version 2.
- *
*/
#include <linux/efi.h>
diff --git a/drivers/firmware/efi/libstub/gop.c b/drivers/firmware/efi/libstub/gop.c
index 24c461dea7af..0101ca4c13b1 100644
--- a/drivers/firmware/efi/libstub/gop.c
+++ b/drivers/firmware/efi/libstub/gop.c
@@ -1,10 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
/* -----------------------------------------------------------------------
*
* Copyright 2011 Intel Corporation; author Matt Fleming
*
- * This file is part of the Linux kernel, and is made available under
- * the terms of the GNU General Public License version 2.
- *
* ----------------------------------------------------------------------- */
#include <linux/efi.h>
diff --git a/drivers/firmware/efi/libstub/random.c b/drivers/firmware/efi/libstub/random.c
index e0e603a89aa9..b4b1d1dcb5fd 100644
--- a/drivers/firmware/efi/libstub/random.c
+++ b/drivers/firmware/efi/libstub/random.c
@@ -1,10 +1,6 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* Copyright (C) 2016 Linaro Ltd; <ard.biesheuvel@linaro.org>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- *
*/
#include <linux/efi.h>
diff --git a/drivers/firmware/efi/libstub/secureboot.c b/drivers/firmware/efi/libstub/secureboot.c
index 72d9dfbebf08..edba5e7a3743 100644
--- a/drivers/firmware/efi/libstub/secureboot.c
+++ b/drivers/firmware/efi/libstub/secureboot.c
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* Secure boot handling.
*
@@ -5,9 +6,6 @@
* Roy Franz <roy.franz@linaro.org
* Copyright (C) 2013 Red Hat, Inc.
* Mark Salter <msalter@redhat.com>
- *
- * This file is part of the Linux kernel, and is made available under the
- * terms of the GNU General Public License version 2.
*/
#include <linux/efi.h>
#include <asm/efi.h>
diff --git a/drivers/firmware/efi/libstub/tpm.c b/drivers/firmware/efi/libstub/tpm.c
index a90b0b8fc69a..5bd04f75d8d6 100644
--- a/drivers/firmware/efi/libstub/tpm.c
+++ b/drivers/firmware/efi/libstub/tpm.c
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* TPM handling.
*
@@ -5,9 +6,6 @@
* Copyright (C) 2017 Google, Inc.
* Matthew Garrett <mjg59@google.com>
* Thiebaud Weksteen <tweek@google.com>
- *
- * This file is part of the Linux kernel, and is made available under the
- * terms of the GNU General Public License version 2.
*/
#include <linux/efi.h>
#include <linux/tpm_eventlog.h>
diff --git a/drivers/firmware/efi/memattr.c b/drivers/firmware/efi/memattr.c
index aac972b056d9..58452fde92cc 100644
--- a/drivers/firmware/efi/memattr.c
+++ b/drivers/firmware/efi/memattr.c
@@ -1,9 +1,6 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* Copyright (C) 2016 Linaro Ltd. <ard.biesheuvel@linaro.org>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
*/
#define pr_fmt(fmt) "efi: memattr: " fmt
diff --git a/drivers/firmware/efi/runtime-map.c b/drivers/firmware/efi/runtime-map.c
index 84a11d0a8023..ad9ddefc9dcb 100644
--- a/drivers/firmware/efi/runtime-map.c
+++ b/drivers/firmware/efi/runtime-map.c
@@ -1,8 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* linux/drivers/efi/runtime-map.c
* Copyright (C) 2013 Red Hat, Inc., Dave Young <dyoung@redhat.com>
- *
- * This file is released under the GPLv2.
*/
#include <linux/string.h>
diff --git a/drivers/firmware/efi/test/efi_test.c b/drivers/firmware/efi/test/efi_test.c
index 51ecf7d6da48..877745c3aaf2 100644
--- a/drivers/firmware/efi/test/efi_test.c
+++ b/drivers/firmware/efi/test/efi_test.c
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0+
/*
* EFI Test Driver for Runtime Services
*
diff --git a/drivers/firmware/efi/test/efi_test.h b/drivers/firmware/efi/test/efi_test.h
index 5f4818bf112f..f2446aa1c2e3 100644
--- a/drivers/firmware/efi/test/efi_test.h
+++ b/drivers/firmware/efi/test/efi_test.h
@@ -1,4 +1,4 @@
-/* SPDX-License-Identifier: GPL-2.0 */
+/* SPDX-License-Identifier: GPL-2.0+ */
/*
* EFI Test driver Header
*
diff --git a/drivers/firmware/efi/tpm.c b/drivers/firmware/efi/tpm.c
index 0cbeb3d46b18..3a689b40ccc0 100644
--- a/drivers/firmware/efi/tpm.c
+++ b/drivers/firmware/efi/tpm.c
@@ -1,10 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* Copyright (C) 2017 Google, Inc.
* Thiebaud Weksteen <tweek@google.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
*/
#include <linux/efi.h>
diff --git a/drivers/firmware/efi/vars.c b/drivers/firmware/efi/vars.c
index fceaafd67ec6..436d1776bc7b 100644
--- a/drivers/firmware/efi/vars.c
+++ b/drivers/firmware/efi/vars.c
@@ -1,22 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0+
/*
* Originally from efivars.c
*
* Copyright (C) 2001,2003,2004 Dell <Matt_Domsch@dell.com>
* Copyright (C) 2004 Intel Corporation <matthew.e.tolentino@intel.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/
#include <linux/capability.h>
--
2.17.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 07/10] efi: arm/arm64: allow SetVirtualAddressMap() to be omitted
2019-02-02 9:41 [GIT PULL 00/10] EFI changes for v5.1 Ard Biesheuvel
` (5 preceding siblings ...)
2019-02-02 9:41 ` [PATCH 06/10] efi: replace GPL license boilerplate with SPDX headers Ard Biesheuvel
@ 2019-02-02 9:41 ` Ard Biesheuvel
2019-02-02 9:41 ` [PATCH 08/10] x86: make ARCH_USE_MEMREMAP_PROT a generic Kconfig symbol Ard Biesheuvel
` (2 subsequent siblings)
9 siblings, 0 replies; 18+ messages in thread
From: Ard Biesheuvel @ 2019-02-02 9:41 UTC (permalink / raw)
To: linux-efi, Ingo Molnar, Thomas Gleixner
Cc: Ard Biesheuvel, linux-kernel, AKASHI Takahiro, Alexander Graf,
Bjorn Andersson, Borislav Petkov, Heinrich Schuchardt,
Jeffrey Hugo, Lee Jones, Leif Lindholm, Linus Torvalds,
Peter Jones, Peter Zijlstra, Sai Praneeth Prakhya
The UEFI spec revision 2.7 errata A section 8.4 has the following to
say about the virtual memory runtime services:
"This section contains function definitions for the virtual memory
support that may be optionally used by an operating system at runtime.
If an operating system chooses to make EFI runtime service calls in a
virtual addressing mode instead of the flat physical mode, then the
operating system must use the services in this section to switch the
EFI runtime services from flat physical addressing to virtual
addressing."
So it is pretty clear that calling SetVirtualAddressMap() is entirely
optional, and so there is no point in doing so unless it achieves
anything useful for us.
This is not the case for 64-bit ARM. The identity mapping used by the
firmware is arbitrarily converted into another permutation of userland
addresses (i.e., bits [63:48] cleared), and the runtime code could easily
deal with the original layout in exactly the same way as it deals with
the converted layout. However, due to constraints related to page size
differences if the OS is not running with 4k pages, and related to
systems that may expose the individual sections of PE/COFF runtime
modules as different memory regions, creating the virtual layout is a
bit fiddly, and requires us to sort the memory map and reason about
adjacent regions with identical memory types etc etc.
So the obvious fix is to stop calling SetVirtualAddressMap() altogether
on arm64 systems. However, to avoid surprises, which are notoriously
hard to diagnose when it comes to OS<->firmware interactions, let's
start by making it an opt-out feature, and implement support for the
'efi=novamap' kernel command line parameter on ARM and arm64 systems.
(Note that 32-bit ARM generally does require SetVirtualAddressMap() to be
used, given that the physical memory map and the kernel virtual address
map are not guaranteed to be non-overlapping like on arm64. However,
having support for efi=novamap,noruntime on 32-bit ARM, combined with
the recently proposed support for earlycon=efifb, is likely to be useful
to diagnose boot issues on such systems if they have no accessible serial
port)
Cc: Alexander Graf <agraf@suse.de>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: AKASHI Takahiro <takahiro.akashi@linaro.org>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Tested-by: Jeffrey Hugo <jhugo@codeaurora.org>
Tested-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Tested-by: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
drivers/firmware/efi/libstub/arm-stub.c | 5 +++++
drivers/firmware/efi/libstub/efi-stub-helper.c | 10 ++++++++++
drivers/firmware/efi/libstub/efistub.h | 1 +
drivers/firmware/efi/libstub/fdt.c | 3 +++
4 files changed, 19 insertions(+)
diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
index eee42d5e25ee..626ec4b4a664 100644
--- a/drivers/firmware/efi/libstub/arm-stub.c
+++ b/drivers/firmware/efi/libstub/arm-stub.c
@@ -370,6 +370,11 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map, unsigned long map_size,
paddr = in->phys_addr;
size = in->num_pages * EFI_PAGE_SIZE;
+ if (novamap()) {
+ in->virt_addr = in->phys_addr;
+ continue;
+ }
+
/*
* Make the mapping compatible with 64k pages: this allows
* a 4k page size kernel to kexec a 64k page size kernel and
diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index 8f2d000bec75..e4610e72b78f 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -31,6 +31,7 @@ static unsigned long __chunk_size = EFI_READ_CHUNK_SIZE;
static int __section(.data) __nokaslr;
static int __section(.data) __quiet;
+static int __section(.data) __novamap;
int __pure nokaslr(void)
{
@@ -40,6 +41,10 @@ int __pure is_quiet(void)
{
return __quiet;
}
+int __pure novamap(void)
+{
+ return __novamap;
+}
#define EFI_MMAP_NR_SLACK_SLOTS 8
@@ -479,6 +484,11 @@ efi_status_t efi_parse_options(char const *cmdline)
__chunk_size = -1UL;
}
+ if (!strncmp(str, "novamap", 7)) {
+ str += strlen("novamap");
+ __novamap = 1;
+ }
+
/* Group words together, delimited by "," */
while (*str && *str != ' ' && *str != ',')
str++;
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index cefcf6ba3150..1b1dfcaa6fb9 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -27,6 +27,7 @@
extern int __pure nokaslr(void);
extern int __pure is_quiet(void);
+extern int __pure novamap(void);
#define pr_efi(sys_table, msg) do { \
if (!is_quiet()) efi_printk(sys_table, "EFI stub: "msg); \
diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
index 7cfc51935c4b..5440ba17a1c5 100644
--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -318,6 +318,9 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
if (status == EFI_SUCCESS) {
efi_set_virtual_address_map_t *svam;
+ if (novamap())
+ return EFI_SUCCESS;
+
/* Install the new virtual address map */
svam = sys_table->runtime->set_virtual_address_map;
status = svam(runtime_entry_count * desc_size, desc_size,
--
2.17.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 08/10] x86: make ARCH_USE_MEMREMAP_PROT a generic Kconfig symbol
2019-02-02 9:41 [GIT PULL 00/10] EFI changes for v5.1 Ard Biesheuvel
` (6 preceding siblings ...)
2019-02-02 9:41 ` [PATCH 07/10] efi: arm/arm64: allow SetVirtualAddressMap() to be omitted Ard Biesheuvel
@ 2019-02-02 9:41 ` Ard Biesheuvel
2019-02-02 9:41 ` [PATCH 09/10] efi: x86: convert x86 EFI earlyprintk into generic earlycon implementation Ard Biesheuvel
2019-02-02 9:41 ` [PATCH 10/10] acpi: bgrt: parse BGRT to obtain BMP address before it gets clobbered Ard Biesheuvel
9 siblings, 0 replies; 18+ messages in thread
From: Ard Biesheuvel @ 2019-02-02 9:41 UTC (permalink / raw)
To: linux-efi, Ingo Molnar, Thomas Gleixner
Cc: Ard Biesheuvel, linux-kernel, AKASHI Takahiro, Alexander Graf,
Bjorn Andersson, Borislav Petkov, Heinrich Schuchardt,
Jeffrey Hugo, Lee Jones, Leif Lindholm, Linus Torvalds,
Peter Jones, Peter Zijlstra, Sai Praneeth Prakhya
Turn ARCH_USE_MEMREMAP_PROT into a generic Kconfig symbol, and fix the
dependency expression to reflect that AMD_MEM_ENCRYPT depends on it,
instead of the other way around. This will permit ARCH_USE_MEMREMAP_PROT
to be selected by other architectures.
Note that the encryption related early memremap routines in
arch/x86/mm/ioremap.c cannot be built for 32-bit x86 without triggering
the following warning:
arch/x86//mm/ioremap.c: In function 'early_memremap_encrypted':
>> arch/x86/include/asm/pgtable_types.h:193:27: warning: conversion from
'long long unsigned int' to 'long unsigned int' changes
value from '9223372036854776163' to '355' [-Woverflow]
#define __PAGE_KERNEL_ENC (__PAGE_KERNEL | _PAGE_ENC)
^~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/x86//mm/ioremap.c:713:46: note: in expansion of macro '__PAGE_KERNEL_ENC'
return early_memremap_prot(phys_addr, size, __PAGE_KERNEL_ENC);
which essentially means they are 64-bit only anyway. However, we cannot
make them dependent on CONFIG_ARCH_HAS_MEM_ENCRYPT, since that is always
defined, even for i386 (and changing that results in a slew of build errors)
So instead, build those routines only if CONFIG_AMD_MEM_ENCRYPT is
defined.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/Kconfig | 3 +++
arch/x86/Kconfig | 5 +----
arch/x86/mm/ioremap.c | 4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/Kconfig b/arch/Kconfig
index 4cfb6de48f79..9f0213213da8 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -885,6 +885,9 @@ config HAVE_ARCH_PREL32_RELOCATIONS
architectures, and don't require runtime relocation on relocatable
kernels.
+config ARCH_USE_MEMREMAP_PROT
+ bool
+
source "kernel/gcov/Kconfig"
source "scripts/gcc-plugins/Kconfig"
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 15af091611e2..09c2c5600579 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1510,6 +1510,7 @@ config AMD_MEM_ENCRYPT
bool "AMD Secure Memory Encryption (SME) support"
depends on X86_64 && CPU_SUP_AMD
select DYNAMIC_PHYSICAL_MASK
+ select ARCH_USE_MEMREMAP_PROT
---help---
Say yes to enable support for the encryption of system memory.
This requires an AMD processor that supports Secure Memory
@@ -1529,10 +1530,6 @@ config AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT
If set to N, then the encryption of system memory can be
activated with the mem_encrypt=on command line option.
-config ARCH_USE_MEMREMAP_PROT
- def_bool y
- depends on AMD_MEM_ENCRYPT
-
# Common NUMA Features
config NUMA
bool "Numa Memory Allocation and Scheduler Support"
diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index 5378d10f1d31..0029604af8a4 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -705,7 +705,7 @@ bool phys_mem_access_encrypted(unsigned long phys_addr, unsigned long size)
return arch_memremap_can_ram_remap(phys_addr, size, 0);
}
-#ifdef CONFIG_ARCH_USE_MEMREMAP_PROT
+#ifdef CONFIG_AMD_MEM_ENCRYPT
/* Remap memory with encryption */
void __init *early_memremap_encrypted(resource_size_t phys_addr,
unsigned long size)
@@ -747,7 +747,7 @@ void __init *early_memremap_decrypted_wp(resource_size_t phys_addr,
return early_memremap_prot(phys_addr, size, __PAGE_KERNEL_NOENC_WP);
}
-#endif /* CONFIG_ARCH_USE_MEMREMAP_PROT */
+#endif /* CONFIG_AMD_MEM_ENCRYPT */
static pte_t bm_pte[PAGE_SIZE/sizeof(pte_t)] __page_aligned_bss;
--
2.17.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 09/10] efi: x86: convert x86 EFI earlyprintk into generic earlycon implementation
2019-02-02 9:41 [GIT PULL 00/10] EFI changes for v5.1 Ard Biesheuvel
` (7 preceding siblings ...)
2019-02-02 9:41 ` [PATCH 08/10] x86: make ARCH_USE_MEMREMAP_PROT a generic Kconfig symbol Ard Biesheuvel
@ 2019-02-02 9:41 ` Ard Biesheuvel
2019-02-02 9:41 ` [PATCH 10/10] acpi: bgrt: parse BGRT to obtain BMP address before it gets clobbered Ard Biesheuvel
9 siblings, 0 replies; 18+ messages in thread
From: Ard Biesheuvel @ 2019-02-02 9:41 UTC (permalink / raw)
To: linux-efi, Ingo Molnar, Thomas Gleixner
Cc: Ard Biesheuvel, linux-kernel, AKASHI Takahiro, Alexander Graf,
Bjorn Andersson, Borislav Petkov, Heinrich Schuchardt,
Jeffrey Hugo, Lee Jones, Leif Lindholm, Linus Torvalds,
Peter Jones, Peter Zijlstra, Sai Praneeth Prakhya
Move the x86 EFI earlyprintk implementation to a shared location under
drivers/firmware and tweak it slightly so we can expose it as an earlycon
implementation (which is generic) rather than earlyprintk (which is only
implemented for a few architectures)
This also involves switching to write-combine mappings by default (which
is required on ARM since device mappings lack memory semantics, and so
memcpy/memset may not be used on them), and adding support for shared
memory framebuffers on cache coherent non-x86 systems (which do not
tolerate mismatched attributes)
Note that 32-bit ARM does not populate its struct screen_info early
enough for earlycon=efifb to work, so it is disabled there.
Reviewed-by: Alexander Graf <agraf@suse.de>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
.../admin-guide/kernel-parameters.txt | 8 +-
arch/x86/Kconfig.debug | 10 --
arch/x86/include/asm/efi.h | 1 -
arch/x86/kernel/early_printk.c | 4 -
arch/x86/platform/efi/Makefile | 1 -
drivers/firmware/efi/Kconfig | 6 +
drivers/firmware/efi/Makefile | 1 +
.../firmware/efi/earlycon.c | 134 +++++++-----------
8 files changed, 64 insertions(+), 101 deletions(-)
rename arch/x86/platform/efi/early_printk.c => drivers/firmware/efi/earlycon.c (42%)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index b799bcf67d7b..76dd3baa31e0 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1073,9 +1073,15 @@
specified address. The serial port must already be
setup and configured. Options are not yet supported.
+ efifb,[options]
+ Start an early, unaccelerated console on the EFI
+ memory mapped framebuffer (if available). On cache
+ coherent non-x86 systems that use system memory for
+ the framebuffer, pass the 'ram' option so that it is
+ mapped with the correct attributes.
+
earlyprintk= [X86,SH,ARM,M68k,S390]
earlyprintk=vga
- earlyprintk=efi
earlyprintk=sclp
earlyprintk=xen
earlyprintk=serial[,ttySn[,baudrate]]
diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
index 0723dff17e6c..15d0fbe27872 100644
--- a/arch/x86/Kconfig.debug
+++ b/arch/x86/Kconfig.debug
@@ -40,16 +40,6 @@ config EARLY_PRINTK_DBGP
with klogd/syslogd or the X server. You should normally say N here,
unless you want to debug such a crash. You need usb debug device.
-config EARLY_PRINTK_EFI
- bool "Early printk via the EFI framebuffer"
- depends on EFI && EARLY_PRINTK
- select FONT_SUPPORT
- ---help---
- Write kernel log output directly into the EFI framebuffer.
-
- This is useful for kernel debugging when your machine crashes very
- early before the console code is initialized.
-
config EARLY_PRINTK_USB_XDBC
bool "Early printk via the xHCI debug port"
depends on EARLY_PRINTK && PCI
diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index a37378f986ec..08ebb2270d87 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -170,7 +170,6 @@ static inline bool efi_runtime_supported(void)
return false;
}
-extern struct console early_efi_console;
extern void parse_efi_setup(u64 phys_addr, u32 data_len);
extern void efifb_setup_from_dmi(struct screen_info *si, const char *opt);
diff --git a/arch/x86/kernel/early_printk.c b/arch/x86/kernel/early_printk.c
index 374a52fa5296..9b33904251a9 100644
--- a/arch/x86/kernel/early_printk.c
+++ b/arch/x86/kernel/early_printk.c
@@ -388,10 +388,6 @@ static int __init setup_early_printk(char *buf)
if (!strncmp(buf, "xen", 3))
early_console_register(&xenboot_console, keep);
#endif
-#ifdef CONFIG_EARLY_PRINTK_EFI
- if (!strncmp(buf, "efi", 3))
- early_console_register(&early_efi_console, keep);
-#endif
#ifdef CONFIG_EARLY_PRINTK_USB_XDBC
if (!strncmp(buf, "xdbc", 4))
early_xdbc_parse_parameter(buf + 4);
diff --git a/arch/x86/platform/efi/Makefile b/arch/x86/platform/efi/Makefile
index e4dc3862d423..fe29f3f5d384 100644
--- a/arch/x86/platform/efi/Makefile
+++ b/arch/x86/platform/efi/Makefile
@@ -3,5 +3,4 @@ OBJECT_FILES_NON_STANDARD_efi_thunk_$(BITS).o := y
OBJECT_FILES_NON_STANDARD_efi_stub_$(BITS).o := y
obj-$(CONFIG_EFI) += quirks.o efi.o efi_$(BITS).o efi_stub_$(BITS).o
-obj-$(CONFIG_EARLY_PRINTK_EFI) += early_printk.o
obj-$(CONFIG_EFI_MIXED) += efi_thunk_$(BITS).o
diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
index 89110dfc7127..190be0b1d109 100644
--- a/drivers/firmware/efi/Kconfig
+++ b/drivers/firmware/efi/Kconfig
@@ -198,3 +198,9 @@ config EFI_DEV_PATH_PARSER
bool
depends on ACPI
default n
+
+config EFI_EARLYCON
+ def_bool y
+ depends on SERIAL_EARLYCON && !ARM && !IA64
+ select FONT_SUPPORT
+ select ARCH_USE_MEMREMAP_PROT
diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile
index 5f9f5039de50..d2d0d2030620 100644
--- a/drivers/firmware/efi/Makefile
+++ b/drivers/firmware/efi/Makefile
@@ -30,5 +30,6 @@ arm-obj-$(CONFIG_EFI) := arm-init.o arm-runtime.o
obj-$(CONFIG_ARM) += $(arm-obj-y)
obj-$(CONFIG_ARM64) += $(arm-obj-y)
obj-$(CONFIG_EFI_CAPSULE_LOADER) += capsule-loader.o
+obj-$(CONFIG_EFI_EARLYCON) += earlycon.o
obj-$(CONFIG_UEFI_CPER_ARM) += cper-arm.o
obj-$(CONFIG_UEFI_CPER_X86) += cper-x86.o
diff --git a/arch/x86/platform/efi/early_printk.c b/drivers/firmware/efi/earlycon.c
similarity index 42%
rename from arch/x86/platform/efi/early_printk.c
rename to drivers/firmware/efi/earlycon.c
index 7138bc7a265c..dc1ae14ea4cf 100644
--- a/arch/x86/platform/efi/early_printk.c
+++ b/drivers/firmware/efi/earlycon.c
@@ -1,8 +1,6 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* Copyright (C) 2013 Intel Corporation; author Matt Fleming
- *
- * This file is part of the Linux kernel, and is made available under
- * the terms of the GNU General Public License version 2.
*/
#include <linux/console.h>
@@ -10,104 +8,68 @@
#include <linux/font.h>
#include <linux/io.h>
#include <linux/kernel.h>
-#include <asm/setup.h>
+#include <linux/serial_core.h>
+#include <linux/screen_info.h>
+
+#include <asm/early_ioremap.h>
static const struct font_desc *font;
static u32 efi_x, efi_y;
-static void *efi_fb;
-static bool early_efi_keep;
-
-/*
- * efi earlyprintk need use early_ioremap to map the framebuffer.
- * But early_ioremap is not usable for earlyprintk=efi,keep, ioremap should
- * be used instead. ioremap will be available after paging_init() which is
- * earlier than initcall callbacks. Thus adding this early initcall function
- * early_efi_map_fb to map the whole efi framebuffer.
- */
-static __init int early_efi_map_fb(void)
-{
- u64 base, size;
+static u64 fb_base;
+static pgprot_t fb_prot;
- if (!early_efi_keep)
- return 0;
-
- base = boot_params.screen_info.lfb_base;
- if (boot_params.screen_info.capabilities & VIDEO_CAPABILITY_64BIT_BASE)
- base |= (u64)boot_params.screen_info.ext_lfb_base << 32;
- size = boot_params.screen_info.lfb_size;
- efi_fb = ioremap(base, size);
-
- return efi_fb ? 0 : -ENOMEM;
-}
-early_initcall(early_efi_map_fb);
-
-/*
- * early_efi_map maps efi framebuffer region [start, start + len -1]
- * In case earlyprintk=efi,keep we have the whole framebuffer mapped already
- * so just return the offset efi_fb + start.
- */
-static __ref void *early_efi_map(unsigned long start, unsigned long len)
+static __ref void *efi_earlycon_map(unsigned long start, unsigned long len)
{
- u64 base;
-
- base = boot_params.screen_info.lfb_base;
- if (boot_params.screen_info.capabilities & VIDEO_CAPABILITY_64BIT_BASE)
- base |= (u64)boot_params.screen_info.ext_lfb_base << 32;
-
- if (efi_fb)
- return (efi_fb + start);
- else
- return early_ioremap(base + start, len);
+ return early_memremap_prot(fb_base + start, len, pgprot_val(fb_prot));
}
-static __ref void early_efi_unmap(void *addr, unsigned long len)
+static __ref void efi_earlycon_unmap(void *addr, unsigned long len)
{
- if (!efi_fb)
- early_iounmap(addr, len);
+ early_memunmap(addr, len);
}
-static void early_efi_clear_scanline(unsigned int y)
+static void efi_earlycon_clear_scanline(unsigned int y)
{
unsigned long *dst;
u16 len;
- len = boot_params.screen_info.lfb_linelength;
- dst = early_efi_map(y*len, len);
+ len = screen_info.lfb_linelength;
+ dst = efi_earlycon_map(y*len, len);
if (!dst)
return;
memset(dst, 0, len);
- early_efi_unmap(dst, len);
+ efi_earlycon_unmap(dst, len);
}
-static void early_efi_scroll_up(void)
+static void efi_earlycon_scroll_up(void)
{
unsigned long *dst, *src;
u16 len;
u32 i, height;
- len = boot_params.screen_info.lfb_linelength;
- height = boot_params.screen_info.lfb_height;
+ len = screen_info.lfb_linelength;
+ height = screen_info.lfb_height;
for (i = 0; i < height - font->height; i++) {
- dst = early_efi_map(i*len, len);
+ dst = efi_earlycon_map(i*len, len);
if (!dst)
return;
- src = early_efi_map((i + font->height) * len, len);
+ src = efi_earlycon_map((i + font->height) * len, len);
if (!src) {
- early_efi_unmap(dst, len);
+ efi_earlycon_unmap(dst, len);
return;
}
memmove(dst, src, len);
- early_efi_unmap(src, len);
- early_efi_unmap(dst, len);
+ efi_earlycon_unmap(src, len);
+ efi_earlycon_unmap(dst, len);
}
}
-static void early_efi_write_char(u32 *dst, unsigned char c, unsigned int h)
+static void efi_earlycon_write_char(u32 *dst, unsigned char c, unsigned int h)
{
const u32 color_black = 0x00000000;
const u32 color_white = 0x00ffffff;
@@ -128,14 +90,14 @@ static void early_efi_write_char(u32 *dst, unsigned char c, unsigned int h)
}
static void
-early_efi_write(struct console *con, const char *str, unsigned int num)
+efi_earlycon_write(struct console *con, const char *str, unsigned int num)
{
struct screen_info *si;
unsigned int len;
const char *s;
void *dst;
- si = &boot_params.screen_info;
+ si = &screen_info;
len = si->lfb_linelength;
while (num) {
@@ -155,7 +117,7 @@ early_efi_write(struct console *con, const char *str, unsigned int num)
for (h = 0; h < font->height; h++) {
unsigned int n, x;
- dst = early_efi_map((efi_y + h) * len, len);
+ dst = efi_earlycon_map((efi_y + h) * len, len);
if (!dst)
return;
@@ -164,12 +126,12 @@ early_efi_write(struct console *con, const char *str, unsigned int num)
x = efi_x;
while (n-- > 0) {
- early_efi_write_char(dst + x*4, *s, h);
+ efi_earlycon_write_char(dst + x*4, *s, h);
x += font->width;
s++;
}
- early_efi_unmap(dst, len);
+ efi_earlycon_unmap(dst, len);
}
num -= count;
@@ -192,26 +154,39 @@ early_efi_write(struct console *con, const char *str, unsigned int num)
u32 i;
efi_y -= font->height;
- early_efi_scroll_up();
+ efi_earlycon_scroll_up();
for (i = 0; i < font->height; i++)
- early_efi_clear_scanline(efi_y + i);
+ efi_earlycon_clear_scanline(efi_y + i);
}
}
}
-static __init int early_efi_setup(struct console *con, char *options)
+static int __init efi_earlycon_setup(struct earlycon_device *device,
+ const char *opt)
{
struct screen_info *si;
u16 xres, yres;
u32 i;
- si = &boot_params.screen_info;
+ if (screen_info.orig_video_isVGA != VIDEO_TYPE_EFI)
+ return -ENODEV;
+
+ fb_base = screen_info.lfb_base;
+ if (screen_info.capabilities & VIDEO_CAPABILITY_64BIT_BASE)
+ fb_base |= (u64)screen_info.ext_lfb_base << 32;
+
+ if (opt && !strcmp(opt, "ram"))
+ fb_prot = PAGE_KERNEL;
+ else
+ fb_prot = pgprot_writecombine(PAGE_KERNEL);
+
+ si = &screen_info;
xres = si->lfb_width;
yres = si->lfb_height;
/*
- * early_efi_write_char() implicitly assumes a framebuffer with
+ * efi_earlycon_write_char() implicitly assumes a framebuffer with
* 32-bits per pixel.
*/
if (si->lfb_depth != 32)
@@ -223,18 +198,9 @@ static __init int early_efi_setup(struct console *con, char *options)
efi_y = rounddown(yres, font->height) - font->height;
for (i = 0; i < (yres - efi_y) / font->height; i++)
- early_efi_scroll_up();
+ efi_earlycon_scroll_up();
- /* early_console_register will unset CON_BOOT in case ,keep */
- if (!(con->flags & CON_BOOT))
- early_efi_keep = true;
+ device->con->write = efi_earlycon_write;
return 0;
}
-
-struct console early_efi_console = {
- .name = "earlyefi",
- .write = early_efi_write,
- .setup = early_efi_setup,
- .flags = CON_PRINTBUFFER,
- .index = -1,
-};
+EARLYCON_DECLARE(efifb, efi_earlycon_setup);
--
2.17.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 10/10] acpi: bgrt: parse BGRT to obtain BMP address before it gets clobbered
2019-02-02 9:41 [GIT PULL 00/10] EFI changes for v5.1 Ard Biesheuvel
` (8 preceding siblings ...)
2019-02-02 9:41 ` [PATCH 09/10] efi: x86: convert x86 EFI earlyprintk into generic earlycon implementation Ard Biesheuvel
@ 2019-02-02 9:41 ` Ard Biesheuvel
2019-02-05 19:07 ` Ghannam, Yazen
9 siblings, 1 reply; 18+ messages in thread
From: Ard Biesheuvel @ 2019-02-02 9:41 UTC (permalink / raw)
To: linux-efi, Ingo Molnar, Thomas Gleixner
Cc: Ard Biesheuvel, linux-kernel, AKASHI Takahiro, Alexander Graf,
Bjorn Andersson, Borislav Petkov, Heinrich Schuchardt,
Jeffrey Hugo, Lee Jones, Leif Lindholm, Linus Torvalds,
Peter Jones, Peter Zijlstra, Sai Praneeth Prakhya
The bitmap left in the framebuffer by the firmware is described by an
ACPI table called "BGRT", which describes the size, pixel format and
the address of a BMP image in memory. While the BGRT ACPI table is
guaranteed to reside in a "ACPI reclaim" memory region, which is
never touched by Linux. The BMP image, however, typically resides
in EFI Boot Services Memory, which may have been overwritten by the
time the BGRT discovery routine runs.
So instead, drop the handling from the ACPI init code, and call the
BGRT parsing code immediately after going over the EFI configuration
table array, at which time no memory has been touched yet except for
the .data/.bss regions covered by the static kernel image.
Unfortunately, this involves a non-trivial amount of ACPI entry
point and root table parsing, but we cannot rely on the normal
ACPI infrastructure yet this early in the boot.
Also note that we cannot take the 'acpi_disabled' global variable
into account, since it may not have assumed the correct value yet
(on arm64, the default value is '1' which is overridden to '0' if
no DT description has been made available by the firmware)
Cc: Peter Jones <pjones@redhat.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/kernel/acpi.c | 2 -
arch/x86/kernel/acpi/boot.c | 2 -
drivers/acpi/bgrt.c | 6 ---
drivers/firmware/efi/efi-bgrt.c | 84 ++++++++++++++++++++++++++++++---
drivers/firmware/efi/efi.c | 13 +++++
include/linux/efi-bgrt.h | 4 +-
6 files changed, 92 insertions(+), 19 deletions(-)
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index 44e3c351e1ea..7429a811f76d 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -230,8 +230,6 @@ void __init acpi_boot_table_init(void)
early_init_dt_scan_chosen_stdout();
} else {
acpi_parse_spcr(earlycon_acpi_spcr_enable, true);
- if (IS_ENABLED(CONFIG_ACPI_BGRT))
- acpi_table_parse(ACPI_SIG_BGRT, acpi_parse_bgrt);
}
}
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 2624de16cd7a..2d3535b62752 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -1633,8 +1633,6 @@ int __init acpi_boot_init(void)
acpi_process_madt();
acpi_table_parse(ACPI_SIG_HPET, acpi_parse_hpet);
- if (IS_ENABLED(CONFIG_ACPI_BGRT))
- acpi_table_parse(ACPI_SIG_BGRT, acpi_parse_bgrt);
if (!acpi_noirq)
x86_init.pci.init = pci_acpi_init;
diff --git a/drivers/acpi/bgrt.c b/drivers/acpi/bgrt.c
index 75af78361ce5..048413e06898 100644
--- a/drivers/acpi/bgrt.c
+++ b/drivers/acpi/bgrt.c
@@ -81,12 +81,6 @@ static const struct attribute_group bgrt_attribute_group = {
.bin_attrs = bgrt_bin_attributes,
};
-int __init acpi_parse_bgrt(struct acpi_table_header *table)
-{
- efi_bgrt_init(table);
- return 0;
-}
-
static int __init bgrt_init(void)
{
int ret;
diff --git a/drivers/firmware/efi/efi-bgrt.c b/drivers/firmware/efi/efi-bgrt.c
index a2384184a7de..9c50d453b143 100644
--- a/drivers/firmware/efi/efi-bgrt.c
+++ b/drivers/firmware/efi/efi-bgrt.c
@@ -24,24 +24,94 @@ struct bmp_header {
u32 size;
} __packed;
-void __init efi_bgrt_init(struct acpi_table_header *table)
+void __init efi_bgrt_init(unsigned long rsdp_phys)
{
void *image;
struct bmp_header bmp_header;
struct acpi_table_bgrt *bgrt = &bgrt_tab;
+ struct acpi_table_bgrt *table = NULL;
+ struct acpi_table_rsdp *rsdp;
+ struct acpi_table_header *hdr;
+ u64 xsdt_phys = 0;
+ u32 rsdt_phys = 0;
+ size_t len;
- if (acpi_disabled)
+ if (!efi_enabled(EFI_MEMMAP))
return;
- if (!efi_enabled(EFI_MEMMAP))
+ /* map the root pointer table to find the xsdt/rsdt values */
+ rsdp = early_memremap_ro(rsdp_phys, sizeof(*rsdp));
+ if (rsdp) {
+ if (ACPI_VALIDATE_RSDP_SIG(rsdp->signature)) {
+ xsdt_phys = rsdp->xsdt_physical_address;
+ rsdt_phys = rsdp->rsdt_physical_address;
+ }
+ early_memunmap(rsdp, sizeof(*rsdp));
+ }
+
+ if (WARN_ON(!xsdt_phys && !rsdt_phys))
return;
- if (table->length < sizeof(bgrt_tab)) {
- pr_notice("Ignoring BGRT: invalid length %u (expected %zu)\n",
- table->length, sizeof(bgrt_tab));
+ /* obtain the length of whichever table we will be using */
+ hdr = early_memremap_ro(xsdt_phys ?: rsdt_phys, sizeof(*hdr));
+ if (WARN_ON(!hdr))
+ return;
+ len = hdr->length;
+ early_memunmap(hdr, sizeof(*hdr));
+
+ /* remap with the correct length */
+ hdr = early_memremap_ro(xsdt_phys ?: rsdt_phys, len);
+ if (WARN_ON(!hdr))
+ return;
+
+ if (xsdt_phys) {
+ struct acpi_table_xsdt *xsdt = (void *)hdr;
+ int i;
+
+ for (i = 0; i < (len - sizeof(*hdr)) / sizeof(u64); i++) {
+ table = early_memremap_ro(xsdt->table_offset_entry[i],
+ sizeof(*table));
+ if (WARN_ON(!table))
+ break;
+
+ if (ACPI_COMPARE_NAME(table->header.signature,
+ ACPI_SIG_BGRT))
+ break;
+ early_memunmap(table, sizeof(*table));
+ table = NULL;
+ }
+ } else if (rsdt_phys) {
+ struct acpi_table_rsdt *rsdt = (void *)hdr;
+ int i;
+
+ for (i = 0; i < (len - sizeof(*hdr)) / sizeof(u32); i++) {
+ table = early_memremap_ro(rsdt->table_offset_entry[i],
+ sizeof(*table));
+ if (WARN_ON(!table))
+ break;
+
+ if (ACPI_COMPARE_NAME(table->header.signature,
+ ACPI_SIG_BGRT))
+ break;
+ early_memunmap(table, sizeof(*table));
+ table = NULL;
+ }
+ }
+ early_memunmap(hdr, len);
+
+ if (!table)
return;
+
+ len = table->header.length;
+ memcpy(bgrt, table, min(len, sizeof(bgrt_tab)));
+ early_memunmap(table, sizeof(*table));
+
+ if (len < sizeof(bgrt_tab)) {
+ pr_notice("Ignoring BGRT: invalid length %zu (expected %zu)\n",
+ len, sizeof(bgrt_tab));
+ goto out;
}
- *bgrt = *(struct acpi_table_bgrt *)table;
+
if (bgrt->version != 1) {
pr_notice("Ignoring BGRT: invalid version %u (expected 1)\n",
bgrt->version);
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 4c46ff6f2242..e5ef5c0eacc1 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -20,6 +20,7 @@
#include <linux/init.h>
#include <linux/device.h>
#include <linux/efi.h>
+#include <linux/efi-bgrt.h>
#include <linux/of.h>
#include <linux/of_fdt.h>
#include <linux/io.h>
@@ -592,6 +593,18 @@ int __init efi_config_parse_tables(void *config_tables, int count, int sz,
early_memunmap(tbl, sizeof(*tbl));
}
+
+ /*
+ * We need to parse the BGRT table (which is an ACPI table not a UEFI
+ * configuration table) by hand and figure out where the bitmap it
+ * describes lives in memory so we can reserve it early on. Otherwise,
+ * it may be clobbered by the time we get to it during the ordinary ACPI
+ * table init sequence.
+ */
+ if (IS_ENABLED(CONFIG_ACPI_BGRT) &&
+ efi.acpi20 != EFI_INVALID_TABLE_ADDR)
+ efi_bgrt_init(efi.acpi20);
+
return 0;
}
diff --git a/include/linux/efi-bgrt.h b/include/linux/efi-bgrt.h
index e6cd51005633..528ea62d99ec 100644
--- a/include/linux/efi-bgrt.h
+++ b/include/linux/efi-bgrt.h
@@ -6,7 +6,7 @@
#ifdef CONFIG_ACPI_BGRT
-void efi_bgrt_init(struct acpi_table_header *table);
+void efi_bgrt_init(unsigned long rsdp_phys);
int __init acpi_parse_bgrt(struct acpi_table_header *table);
/* The BGRT data itself; only valid if bgrt_image != NULL. */
@@ -15,7 +15,7 @@ extern struct acpi_table_bgrt bgrt_tab;
#else /* !CONFIG_ACPI_BGRT */
-static inline void efi_bgrt_init(struct acpi_table_header *table) {}
+static inline void efi_bgrt_init(unsigned long rsdp_phys) {}
static inline int __init acpi_parse_bgrt(struct acpi_table_header *table)
{
return 0;
--
2.17.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* RE: [PATCH 10/10] acpi: bgrt: parse BGRT to obtain BMP address before it gets clobbered
2019-02-02 9:41 ` [PATCH 10/10] acpi: bgrt: parse BGRT to obtain BMP address before it gets clobbered Ard Biesheuvel
@ 2019-02-05 19:07 ` Ghannam, Yazen
2019-02-05 23:27 ` Ard Biesheuvel
0 siblings, 1 reply; 18+ messages in thread
From: Ghannam, Yazen @ 2019-02-05 19:07 UTC (permalink / raw)
To: Ard Biesheuvel, linux-efi, Ingo Molnar, Thomas Gleixner
Cc: linux-kernel, AKASHI Takahiro, Alexander Graf, Bjorn Andersson,
Borislav Petkov, Heinrich Schuchardt, Jeffrey Hugo, Lee Jones,
Leif Lindholm, Linus Torvalds, Peter Jones, Peter Zijlstra,
Sai Praneeth Prakhya, Lendacky, Thomas
> -----Original Message-----
> From: linux-kernel-owner@vger.kernel.org <linux-kernel-
> owner@vger.kernel.org> On Behalf Of Ard Biesheuvel
> Sent: Saturday, February 2, 2019 3:41 AM
> To: linux-efi@vger.kernel.org; Ingo Molnar <mingo@kernel.org>; Thomas
> Gleixner <tglx@linutronix.de>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; linux-kernel@vger.kernel.org;
> AKASHI Takahiro <takahiro.akashi@linaro.org>; Alexander Graf
> <agraf@suse.de>; Bjorn Andersson <bjorn.andersson@linaro.org>; Borislav
> Petkov <bp@alien8.de>; Heinrich Schuchardt <xypron.glpk@gmx.de>; Jeffrey
> Hugo <jhugo@codeaurora.org>; Lee Jones <lee.jones@linaro.org>; Leif
> Lindholm <leif.lindholm@linaro.org>; Linus Torvalds <torvalds@linux-
> foundation.org>; Peter Jones <pjones@redhat.com>; Peter Zijlstra
> <peterz@infradead.org>; Sai Praneeth Prakhya
> <sai.praneeth.prakhya@intel.com>
> Subject: [PATCH 10/10] acpi: bgrt: parse BGRT to obtain BMP address before it
> gets clobbered
>
> The bitmap left in the framebuffer by the firmware is described by an
> ACPI table called "BGRT", which describes the size, pixel format and
> the address of a BMP image in memory. While the BGRT ACPI table is
> guaranteed to reside in a "ACPI reclaim" memory region, which is
> never touched by Linux. The BMP image, however, typically resides
> in EFI Boot Services Memory, which may have been overwritten by the
> time the BGRT discovery routine runs.
>
> So instead, drop the handling from the ACPI init code, and call the
> BGRT parsing code immediately after going over the EFI configuration
> table array, at which time no memory has been touched yet except for
> the .data/.bss regions covered by the static kernel image.
>
> Unfortunately, this involves a non-trivial amount of ACPI entry
> point and root table parsing, but we cannot rely on the normal
> ACPI infrastructure yet this early in the boot.
>
> Also note that we cannot take the 'acpi_disabled' global variable
> into account, since it may not have assumed the correct value yet
> (on arm64, the default value is '1' which is overridden to '0' if
> no DT description has been made available by the firmware)
>
> Cc: Peter Jones <pjones@redhat.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
Hi Ard, et. al.,
I'm trying out tip/master and I find that my system panics early during boot. Reverting
this patch seems to resolve the issue. Please see the trace below.
I've started debugging, but I'm not familiar with this code. Please let me know if you
have any ideas or if there's anything you'd like me to try.
Thanks,
Yazen
[ 0.000000] Kernel panic - not syncing: ERROR: Failed to allocate 0x0000000000000b40 bytes below 0x0000000000000000.
[ 0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc5-merged-bases+ #101
[ 0.000000] Call Trace:
[ 0.000000] dump_stack+0x63/0x85
[ 0.000000] panic+0xfe/0x2a4
[ 0.000000] memblock_alloc_base+0x33/0x35
[ 0.000000] memblock_phys_alloc+0x10/0x12
[ 0.000000] efi_memmap_alloc+0x62/0x65
[ 0.000000] efi_arch_mem_reserve+0x10e/0x194
[ 0.000000] efi_mem_reserve+0x31/0x36
[ 0.000000] ? efi_mem_reserve+0x31/0x36
[ 0.000000] efi_bgrt_init+0x2c6/0x2e0
[ 0.000000] efi_config_parse_tables+0x1b2/0x1dd
[ 0.000000] efi_config_init+0x7b/0x9f
[ 0.000000] ? efi_config_init+0x7b/0x9f
[ 0.000000] efi_init+0x366/0x465
[ 0.000000] ? 0xffffffff87800000
[ 0.000000] setup_arch+0x42f/0xcc9
[ 0.000000] ? printk+0x52/0x6e
[ 0.000000] start_kernel+0x6c/0x516
[ 0.000000] x86_64_start_reservations+0x24/0x26
[ 0.000000] x86_64_start_kernel+0x74/0x77
[ 0.000000] secondary_startup_64+0xa4/0xb0
[ 0.000000] ---[ end Kernel panic - not syncing: ERROR: Failed to allocate 0x0000000000000b40 bytes below 0x0000000000000000. ]---
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 10/10] acpi: bgrt: parse BGRT to obtain BMP address before it gets clobbered
2019-02-05 19:07 ` Ghannam, Yazen
@ 2019-02-05 23:27 ` Ard Biesheuvel
0 siblings, 0 replies; 18+ messages in thread
From: Ard Biesheuvel @ 2019-02-05 23:27 UTC (permalink / raw)
To: Ghannam, Yazen
Cc: linux-efi, Ingo Molnar, Thomas Gleixner, linux-kernel,
AKASHI Takahiro, Alexander Graf, Bjorn Andersson,
Borislav Petkov, Heinrich Schuchardt, Jeffrey Hugo, Lee Jones,
Leif Lindholm, Linus Torvalds, Peter Jones, Peter Zijlstra,
Sai Praneeth Prakhya, Lendacky, Thomas
On Tue, 5 Feb 2019 at 19:07, Ghannam, Yazen <Yazen.Ghannam@amd.com> wrote:
>
> > -----Original Message-----
> > From: linux-kernel-owner@vger.kernel.org <linux-kernel-
> > owner@vger.kernel.org> On Behalf Of Ard Biesheuvel
> > Sent: Saturday, February 2, 2019 3:41 AM
> > To: linux-efi@vger.kernel.org; Ingo Molnar <mingo@kernel.org>; Thomas
> > Gleixner <tglx@linutronix.de>
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; linux-kernel@vger.kernel.org;
> > AKASHI Takahiro <takahiro.akashi@linaro.org>; Alexander Graf
> > <agraf@suse.de>; Bjorn Andersson <bjorn.andersson@linaro.org>; Borislav
> > Petkov <bp@alien8.de>; Heinrich Schuchardt <xypron.glpk@gmx.de>; Jeffrey
> > Hugo <jhugo@codeaurora.org>; Lee Jones <lee.jones@linaro.org>; Leif
> > Lindholm <leif.lindholm@linaro.org>; Linus Torvalds <torvalds@linux-
> > foundation.org>; Peter Jones <pjones@redhat.com>; Peter Zijlstra
> > <peterz@infradead.org>; Sai Praneeth Prakhya
> > <sai.praneeth.prakhya@intel.com>
> > Subject: [PATCH 10/10] acpi: bgrt: parse BGRT to obtain BMP address before it
> > gets clobbered
> >
> > The bitmap left in the framebuffer by the firmware is described by an
> > ACPI table called "BGRT", which describes the size, pixel format and
> > the address of a BMP image in memory. While the BGRT ACPI table is
> > guaranteed to reside in a "ACPI reclaim" memory region, which is
> > never touched by Linux. The BMP image, however, typically resides
> > in EFI Boot Services Memory, which may have been overwritten by the
> > time the BGRT discovery routine runs.
> >
> > So instead, drop the handling from the ACPI init code, and call the
> > BGRT parsing code immediately after going over the EFI configuration
> > table array, at which time no memory has been touched yet except for
> > the .data/.bss regions covered by the static kernel image.
> >
> > Unfortunately, this involves a non-trivial amount of ACPI entry
> > point and root table parsing, but we cannot rely on the normal
> > ACPI infrastructure yet this early in the boot.
> >
> > Also note that we cannot take the 'acpi_disabled' global variable
> > into account, since it may not have assumed the correct value yet
> > (on arm64, the default value is '1' which is overridden to '0' if
> > no DT description has been made available by the firmware)
> >
> > Cc: Peter Jones <pjones@redhat.com>
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > ---
>
> Hi Ard, et. al.,
>
> I'm trying out tip/master and I find that my system panics early during boot. Reverting
> this patch seems to resolve the issue. Please see the trace below.
>
> I've started debugging, but I'm not familiar with this code. Please let me know if you
> have any ideas or if there's anything you'd like me to try.
>
Hi Yazen,
Thanks for the report, you are the second person to flag this issue,
so in the mean time, I have asked Ingo to drop it from the efi/core
queue, and so the patch will be gone from -next as soon as it
refreshes.
I'll cc you on the updated version of this patch once I get around to
looking into it, which will probably be around early next week.
Thanks,
Ard.
>
> [ 0.000000] Kernel panic - not syncing: ERROR: Failed to allocate 0x0000000000000b40 bytes below 0x0000000000000000.
> [ 0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc5-merged-bases+ #101
> [ 0.000000] Call Trace:
> [ 0.000000] dump_stack+0x63/0x85
> [ 0.000000] panic+0xfe/0x2a4
> [ 0.000000] memblock_alloc_base+0x33/0x35
> [ 0.000000] memblock_phys_alloc+0x10/0x12
> [ 0.000000] efi_memmap_alloc+0x62/0x65
> [ 0.000000] efi_arch_mem_reserve+0x10e/0x194
> [ 0.000000] efi_mem_reserve+0x31/0x36
> [ 0.000000] ? efi_mem_reserve+0x31/0x36
> [ 0.000000] efi_bgrt_init+0x2c6/0x2e0
> [ 0.000000] efi_config_parse_tables+0x1b2/0x1dd
> [ 0.000000] efi_config_init+0x7b/0x9f
> [ 0.000000] ? efi_config_init+0x7b/0x9f
> [ 0.000000] efi_init+0x366/0x465
> [ 0.000000] ? 0xffffffff87800000
> [ 0.000000] setup_arch+0x42f/0xcc9
> [ 0.000000] ? printk+0x52/0x6e
> [ 0.000000] start_kernel+0x6c/0x516
> [ 0.000000] x86_64_start_reservations+0x24/0x26
> [ 0.000000] x86_64_start_kernel+0x74/0x77
> [ 0.000000] secondary_startup_64+0xa4/0xb0
> [ 0.000000] ---[ end Kernel panic - not syncing: ERROR: Failed to allocate 0x0000000000000b40 bytes below 0x0000000000000000. ]---
>
^ permalink raw reply [flat|nested] 18+ messages in thread