[PATCH 0/4] x86/efi: Minor cleanup patches

[PATCH 0/4] x86/efi: Minor cleanup patches

[Mathias Krause]

Hi Matt,

this series is a collection of rather random cleanup patches. The init code
patch, though, should result in a lower runtime memory usage as more memory
can be released after initialization, now.

Please apply!


Mathias Krause (4):
  x86/efi: Remove unused efi_call* macros
  x86/efi: Unexport add_efi_memmap variable
  x86/efi: Update comment regarding required phys mapped EFI services
  x86/efi: Mark initialization code as such

 arch/x86/include/asm/efi.h          |   31 +++++++++------------------
 arch/x86/platform/efi/efi.c         |   20 ++++++++---------
 arch/x86/platform/efi/efi_32.c      |   12 ++++++-----
 arch/x86/platform/efi/efi_64.c      |    6 +++---
 arch/x86/platform/efi/efi_stub_32.S |    4 ++--
 5 files changed, 31 insertions(+), 42 deletions(-)

-- 
1.7.10.4

[PATCH 1/4] x86/efi: Remove unused efi_call* macros

[Mathias Krause]

Complement commit 62fa6e69a436 ("x86/efi: Delete most of the efi_call*
macros") and delete the stub macros for the !CONFIG_EFI case, too. In
fact, there are no EFI calls in this case so we don't need a dummy for
efi_call() even.

Signed-off-by: Mathias Krause <minipli-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 arch/x86/include/asm/efi.h |   10 ----------
 1 file changed, 10 deletions(-)

diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index 044a2fd3c5fe..8aee01536c6c 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -186,16 +186,6 @@ extern struct efi_config *efi_early;
 extern bool efi_reboot_required(void);
 
 #else
-/*
- * IF EFI is not configured, have the EFI calls return -ENOSYS.
- */
-#define efi_call0(_f)					(-ENOSYS)
-#define efi_call1(_f, _a1)				(-ENOSYS)
-#define efi_call2(_f, _a1, _a2)				(-ENOSYS)
-#define efi_call3(_f, _a1, _a2, _a3)			(-ENOSYS)
-#define efi_call4(_f, _a1, _a2, _a3, _a4)		(-ENOSYS)
-#define efi_call5(_f, _a1, _a2, _a3, _a4, _a5)		(-ENOSYS)
-#define efi_call6(_f, _a1, _a2, _a3, _a4, _a5, _a6)	(-ENOSYS)
 static inline void parse_efi_setup(u64 phys_addr, u32 data_len) {}
 static inline bool efi_reboot_required(void)
 {
-- 
1.7.10.4

[PATCH 2/4] x86/efi: Unexport add_efi_memmap variable

[Mathias Krause]

This variable was accidentally exported, even though it's only used in
this compilation unit and only during initialization.

Remove the bogus export, make the variable static instead and mark it
as __initdata.

Fixes: 200001eb140e ("x86 boot: only pick up additional EFI memmap...")
Cc: Paul Jackson <pj-sJ/iWh9BUns@public.gmane.org>
Signed-off-by: Mathias Krause <minipli-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 arch/x86/include/asm/efi.h  |    1 -
 arch/x86/platform/efi/efi.c |    4 +---
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index 8aee01536c6c..b07ecf332bdf 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -86,7 +86,6 @@ extern void __iomem *efi_ioremap(unsigned long addr, unsigned long size,
 
 #endif /* CONFIG_X86_32 */
 
-extern int add_efi_memmap;
 extern struct efi_scratch efi_scratch;
 extern void efi_set_executable(efi_memory_desc_t *md, bool executable);
 extern int efi_memblock_x86_reserve_range(void);
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 850da94fef30..9e43b0f3d891 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -78,9 +78,7 @@ static int __init setup_noefi(char *arg)
 }
 early_param("noefi", setup_noefi);
 
-int add_efi_memmap;
-EXPORT_SYMBOL(add_efi_memmap);
-
+static int add_efi_memmap __initdata;
 static int __init setup_add_efi_memmap(char *arg)
 {
 	add_efi_memmap = 1;
-- 
1.7.10.4

[PATCH 3/4] x86/efi: Update comment regarding required phys mapped EFI services

[Mathias Krause]

Commit 3f4a7836e331 ("x86/efi: Rip out phys_efi_get_time()") left
set_virtual_address_map as the only runtime service needed with a
phys mapping but missed to update the preceding comment. Fix that.

Signed-off-by: Mathias Krause <minipli-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 arch/x86/platform/efi/efi.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 9e43b0f3d891..504174c36a73 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -342,9 +342,9 @@ static int __init efi_runtime_init32(void)
 	}
 
 	/*
-	 * We will only need *early* access to the following two
-	 * EFI runtime services before set_virtual_address_map
-	 * is invoked.
+	 * We will only need *early* access to the SetVirtualAddressMap
+	 * EFI runtime service. All other runtime services will be called
+	 * via the virtual mapping.
 	 */
 	efi_phys.set_virtual_address_map =
 			(efi_set_virtual_address_map_t *)
@@ -366,9 +366,9 @@ static int __init efi_runtime_init64(void)
 	}
 
 	/*
-	 * We will only need *early* access to the following two
-	 * EFI runtime services before set_virtual_address_map
-	 * is invoked.
+	 * We will only need *early* access to the SetVirtualAddressMap
+	 * EFI runtime service. All other runtime services will be called
+	 * via the virtual mapping.
 	 */
 	efi_phys.set_virtual_address_map =
 			(efi_set_virtual_address_map_t *)
-- 
1.7.10.4

[PATCH 4/4] x86/efi: Mark initialization code as such

[Mathias Krause]

The 32 bit and 64 bit implementations differ in their __init annotations
for some functions referenced from the common EFI code. Namely, the 32
bit variant is missing some of the __init annotations the 64 bit variant
has.

To solve the colliding annotations, mark the corresponding functions in
efi_32.c as initialization code, too -- as it is such.

Actually, quite a few more functions are only used during initialization
and therefore can be marked __init. They are therefore annotated, too.
Also add the __init annotation to the prototypes in the efi.h header so
users of those functions will see it's meant as initialization code
only.

This patch also fixes the "prelog" typo. ("prologue" / "epilogue" might
be more appropriate but this is C code after all, not an opera! :D)

Signed-off-by: Mathias Krause <minipli-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

---
The function efi_cleanup_page_tables() is not referenced at all so might
also be removed. But I didn't do this, only marked the code as __init
code. So, at least, at runtime it won't be occupying memory any more.
---
 arch/x86/include/asm/efi.h          |   20 ++++++++++----------
 arch/x86/platform/efi/efi.c         |    4 ++--
 arch/x86/platform/efi/efi_32.c      |   12 +++++++-----
 arch/x86/platform/efi/efi_64.c      |    6 +++---
 arch/x86/platform/efi/efi_stub_32.S |    4 ++--
 5 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index b07ecf332bdf..408ad6d3222e 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -81,23 +81,23 @@ extern u64 asmlinkage efi_call(void *fp, ...);
  */
 #define __efi_call_virt(f, args...) efi_call_virt(f, args)
 
-extern void __iomem *efi_ioremap(unsigned long addr, unsigned long size,
-				 u32 type, u64 attribute);
+extern void __iomem *__init efi_ioremap(unsigned long addr, unsigned long size,
+					u32 type, u64 attribute);
 
 #endif /* CONFIG_X86_32 */
 
 extern struct efi_scratch efi_scratch;
-extern void efi_set_executable(efi_memory_desc_t *md, bool executable);
-extern int efi_memblock_x86_reserve_range(void);
-extern void efi_call_phys_prelog(void);
-extern void efi_call_phys_epilog(void);
-extern void efi_unmap_memmap(void);
-extern void efi_memory_uc(u64 addr, unsigned long size);
+extern void __init efi_set_executable(efi_memory_desc_t *md, bool executable);
+extern int __init efi_memblock_x86_reserve_range(void);
+extern void __init efi_call_phys_prolog(void);
+extern void __init efi_call_phys_epilog(void);
+extern void __init efi_unmap_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 void efi_sync_low_kernel_mappings(void);
-extern int efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages);
-extern void efi_cleanup_page_tables(unsigned long pa_memmap, unsigned num_pages);
+extern int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages);
+extern void __init efi_cleanup_page_tables(unsigned long pa_memmap, unsigned num_pages);
 extern void __init old_map_region(efi_memory_desc_t *md);
 extern void __init runtime_code_page_mkexec(void);
 extern void __init efi_runtime_mkexec(void);
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 504174c36a73..0e439a34034d 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -94,7 +94,7 @@ static efi_status_t __init phys_efi_set_virtual_address_map(
 {
 	efi_status_t status;
 
-	efi_call_phys_prelog();
+	efi_call_phys_prolog();
 	status = efi_call_phys(efi_phys.set_virtual_address_map,
 			       memory_map_size, descriptor_size,
 			       descriptor_version, virtual_map);
@@ -535,7 +535,7 @@ void __init runtime_code_page_mkexec(void)
 	}
 }
 
-void efi_memory_uc(u64 addr, unsigned long size)
+void __init efi_memory_uc(u64 addr, unsigned long size)
 {
 	unsigned long page_shift = 1UL << EFI_PAGE_SHIFT;
 	u64 npages;
diff --git a/arch/x86/platform/efi/efi_32.c b/arch/x86/platform/efi/efi_32.c
index 9ee3491e31fb..40e7cda52936 100644
--- a/arch/x86/platform/efi/efi_32.c
+++ b/arch/x86/platform/efi/efi_32.c
@@ -33,7 +33,7 @@
 
 /*
  * To make EFI call EFI runtime service in physical addressing mode we need
- * prelog/epilog before/after the invocation to disable interrupt, to
+ * prolog/epilog before/after the invocation to disable interrupt, to
  * claim EFI runtime service handler exclusively and to duplicate a memory in
  * low memory space say 0 - 3G.
  */
@@ -41,11 +41,13 @@ static unsigned long efi_rt_eflags;
 
 void efi_sync_low_kernel_mappings(void) {}
 void __init efi_dump_pagetable(void) {}
-int efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages)
+int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages)
 {
 	return 0;
 }
-void efi_cleanup_page_tables(unsigned long pa_memmap, unsigned num_pages) {}
+void __init efi_cleanup_page_tables(unsigned long pa_memmap, unsigned num_pages)
+{
+}
 
 void __init efi_map_region(efi_memory_desc_t *md)
 {
@@ -55,7 +57,7 @@ void __init efi_map_region(efi_memory_desc_t *md)
 void __init efi_map_region_fixed(efi_memory_desc_t *md) {}
 void __init parse_efi_setup(u64 phys_addr, u32 data_len) {}
 
-void efi_call_phys_prelog(void)
+void __init efi_call_phys_prolog(void)
 {
 	struct desc_ptr gdt_descr;
 
@@ -69,7 +71,7 @@ void efi_call_phys_prelog(void)
 	load_gdt(&gdt_descr);
 }
 
-void efi_call_phys_epilog(void)
+void __init efi_call_phys_epilog(void)
 {
 	struct desc_ptr gdt_descr;
 
diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
index 290d397e1dd9..35aecb6042fb 100644
--- a/arch/x86/platform/efi/efi_64.c
+++ b/arch/x86/platform/efi/efi_64.c
@@ -79,7 +79,7 @@ static void __init early_code_mapping_set_exec(int executable)
 	}
 }
 
-void __init efi_call_phys_prelog(void)
+void __init efi_call_phys_prolog(void)
 {
 	unsigned long vaddress;
 	int pgd;
@@ -139,7 +139,7 @@ void efi_sync_low_kernel_mappings(void)
 		sizeof(pgd_t) * num_pgds);
 }
 
-int efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages)
+int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages)
 {
 	unsigned long text;
 	struct page *page;
@@ -192,7 +192,7 @@ int efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages)
 	return 0;
 }
 
-void efi_cleanup_page_tables(unsigned long pa_memmap, unsigned num_pages)
+void __init efi_cleanup_page_tables(unsigned long pa_memmap, unsigned num_pages)
 {
 	pgd_t *pgd = (pgd_t *)__va(real_mode_header->trampoline_pgd);
 
diff --git a/arch/x86/platform/efi/efi_stub_32.S b/arch/x86/platform/efi/efi_stub_32.S
index fbe66e626c09..040192b50d02 100644
--- a/arch/x86/platform/efi/efi_stub_32.S
+++ b/arch/x86/platform/efi/efi_stub_32.S
@@ -27,13 +27,13 @@ ENTRY(efi_call_phys)
 	 * set to 0x0010, DS and SS have been set to 0x0018. In EFI, I found
 	 * the values of these registers are the same. And, the corresponding
 	 * GDT entries are identical. So I will do nothing about segment reg
-	 * and GDT, but change GDT base register in prelog and epilog.
+	 * and GDT, but change GDT base register in prolog and epilog.
 	 */
 
 	/*
 	 * 1. Now I am running with EIP = <physical address> + PAGE_OFFSET.
 	 * But to make it smoothly switch from virtual mode to flat mode.
-	 * The mapping of lower virtual memory has been created in prelog and
+	 * The mapping of lower virtual memory has been created in prolog and
 	 * epilog.
 	 */
 	movl	$1f, %edx
-- 
1.7.10.4

Re: [PATCH 0/4] x86/efi: Minor cleanup patches

[Matt Fleming]

On Sun, 07 Sep, at 07:42:13PM, Mathias Krause wrote:
> Hi Matt,
> 
> this series is a collection of rather random cleanup patches. The init code
> patch, though, should result in a lower runtime memory usage as more memory
> can be released after initialization, now.
> 
> Please apply!

Thanks Mathias, these look good. Applied!

Since they don't apply to the 'next' branch (queue for v3.18) and since
they're cleanups with minimal risk, I've applied them to the 'urgent'
branch.

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH 0/4] x86/efi: Minor cleanup patches

[Mathias Krause]

On 8 September 2014 21:41, Matt Fleming <matt-HNK1S37rvNbeXh+fF434Mdi2O/JbrIOy@public.gmane.org> wrote:
> On Sun, 07 Sep, at 07:42:13PM, Mathias Krause wrote:
>> Hi Matt,
>>
>> this series is a collection of rather random cleanup patches. The init code
>> patch, though, should result in a lower runtime memory usage as more memory
>> can be released after initialization, now.
>>
>> Please apply!
>
> Thanks Mathias, these look good. Applied!
>
> Since they don't apply to the 'next' branch (queue for v3.18) and since
> they're cleanups with minimal risk, I've applied them to the 'urgent'
> branch.

Sorry, I should have checked that! I can send a rebased version if you
want me to.

Mathias

Re: [PATCH 0/4] x86/efi: Minor cleanup patches

[Matt Fleming]

On Tue, 09 Sep, at 09:03:31AM, Mathias Krause wrote:
> On 8 September 2014 21:41, Matt Fleming <matt-HNK1S37rvNbeXh+fF434Mdi2O/JbrIOy@public.gmane.org> wrote:
> >
> > Since they don't apply to the 'next' branch (queue for v3.18) and since
> > they're cleanups with minimal risk, I've applied them to the 'urgent'
> > branch.
> 
> Sorry, I should have checked that! I can send a rebased version if you
> want me to.

Nah, don't worry. I usually prefer that people base their patches on
Linus' tree and an -rc or final v3.x tag.

I got some pushback from the tip folks on including these patches in an
urgent pull request because they're not regression fixes. What I've now
done is stuck them on the queue for v3.18 in 'next'.

Everything applied cleanly apart from "x86/efi: Unexport add_efi_memmap
variable", because of recent changes to move the "noefi" handling code
out of arch/x86.

I fixed up the conflict, but please take a look at the 'next' branch and
see whether your rebased patches look correct.

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH 0/4] x86/efi: Minor cleanup patches

[Mathias Krause]

On 9 September 2014 09:27, Matt Fleming <matt-HNK1S37rvNbeXh+fF434Mdi2O/JbrIOy@public.gmane.org> wrote:
> On Tue, 09 Sep, at 09:03:31AM, Mathias Krause wrote:
>> On 8 September 2014 21:41, Matt Fleming <matt-HNK1S37rvNbeXh+fF434Mdi2O/JbrIOy@public.gmane.org> wrote:
>> >
>> > Since they don't apply to the 'next' branch (queue for v3.18) and since
>> > they're cleanups with minimal risk, I've applied them to the 'urgent'
>> > branch.
>>
>> Sorry, I should have checked that! I can send a rebased version if you
>> want me to.
>
> Nah, don't worry. I usually prefer that people base their patches on
> Linus' tree and an -rc or final v3.x tag.
>
> I got some pushback from the tip folks on including these patches in an
> urgent pull request because they're not regression fixes. What I've now
> done is stuck them on the queue for v3.18 in 'next'.

That's okay, too. They're no urgent fixes, after all.

> Everything applied cleanly apart from "x86/efi: Unexport add_efi_memmap
> variable", because of recent changes to move the "noefi" handling code
> out of arch/x86.

Yeah. I tried a rebase and got the same conflict. It's easy to fix, though.

>
> I fixed up the conflict, but please take a look at the 'next' branch and
> see whether your rebased patches look correct.

Looks good. Thanks, Matt!

Cheers,
Mathias