x86: make 64bit efi to use ioremap_cache for efi_ioremap

RFC PATCH In review Yinghai Lu 5 participants 9 messages Oct 4, 2008 – Oct 6, 2008

Subsystems (2)

x86 EFI

lore

Share & tools

lore

Yinghai Lu

author

2008-10-04 06:24 UTC #0 lore raw

RFC PATCH 1/1 x86: make 64bit efi to use ioremap_cache for efi_ioremap

Signed-off-by Yinghai Lu <[email protected]>

---
 arch/x86/kernel/efi.c       |    5 +----
 arch/x86/kernel/efi_64.c    |   27 ++-------------------------
 include/asm-x86/efi.h       |    8 ++------
 include/asm-x86/fixmap_64.h |    3 ---
 4 files changed, 5 insertions(+), 38 deletions(-)

arch/x86/kernel/efi.c +1 -4

Index: linux-2.6/arch/x86/kernel/efi.c
===================================================================
@@ -475,10 +475,7 @@ void __init efi_enter_virtual_mode(void)
 		size = md->num_pages << EFI_PAGE_SHIFT;
 		end = md->phys_addr + size;
 
-		if (PFN_UP(end) <= max_low_pfn_mapped)
-			va = __va(md->phys_addr);
-		else
-			va = efi_ioremap(md->phys_addr, size);
+		va = efi_ioremap(md->phys_addr, size);
 
 		md->virt_addr = (u64) (unsigned long) va;

arch/x86/kernel/efi_64.c +2 ~~-25~~

Index: linux-2.6/arch/x86/kernel/efi_64.c
===================================================================
@@ -46,8 +46,8 @@ static void __init early_mapping_set_exe
 {
 	unsigned long num_pages;
 
-	start &= PMD_MASK;
-	end = (end + PMD_SIZE - 1) & PMD_MASK;
+	start &= PAGE_MASK;
+	end = (end + PAGE_SIZE - 1) & PAGE_MASK;
 	num_pages = (end - start) >> PAGE_SHIFT;
 	if (executable)
 		set_memory_x((unsigned long)__va(start), num_pages);

@@ -97,26 +97,3 @@ void __init efi_call_phys_epilog(void)
 	early_runtime_code_mapping_set_exec(0);
 }
 
-void __iomem *__init efi_ioremap(unsigned long phys_addr, unsigned long size)
-{
-	static unsigned pages_mapped __initdata;
-	unsigned i, pages;
-	unsigned long offset;
-
-	pages = PFN_UP(phys_addr + size) - PFN_DOWN(phys_addr);
-	offset = phys_addr & ~PAGE_MASK;
-	phys_addr &= PAGE_MASK;
-
-	if (pages_mapped + pages > MAX_EFI_IO_PAGES)
-		return NULL;
-
-	for (i = 0; i < pages; i++) {
-		__set_fixmap(FIX_EFI_IO_MAP_FIRST_PAGE - pages_mapped,
-			     phys_addr, PAGE_KERNEL);
-		phys_addr += PAGE_SIZE;
-		pages_mapped++;
-	}
-
-	return (void __iomem *)__fix_to_virt(FIX_EFI_IO_MAP_FIRST_PAGE - \
-					     (pages_mapped - pages)) + offset;
-}

include/asm-x86/efi.h +2 -6

Index: linux-2.6/include/asm-x86/efi.h
===================================================================
@@ -33,12 +33,8 @@ extern unsigned long asmlinkage efi_call
 #define efi_call_virt6(f, a1, a2, a3, a4, a5, a6)	\
 	efi_call_virt(f, a1, a2, a3, a4, a5, a6)
 
-#define efi_ioremap(addr, size)			ioremap_cache(addr, size)
-
 #else /* !CONFIG_X86_32 */
 
-#define MAX_EFI_IO_PAGES	100
-
 extern u64 efi_call0(void *fp);
 extern u64 efi_call1(void *fp, u64 arg1);
 extern u64 efi_call2(void *fp, u64 arg1, u64 arg2);

@@ -86,10 +82,10 @@ extern u64 efi_call6(void *fp, u64 arg1,
 	efi_call6((void *)(efi.systab->runtime->f), (u64)(a1), (u64)(a2), \
 		  (u64)(a3), (u64)(a4), (u64)(a5), (u64)(a6))
 
-extern void __iomem *efi_ioremap(unsigned long addr, unsigned long size);
-
 #endif /* CONFIG_X86_32 */
 
+#define efi_ioremap(addr, size)			ioremap_cache(addr, size)
+
 extern void efi_reserve_early(void);
 extern void efi_call_phys_prelog(void);
 extern void efi_call_phys_epilog(void);

include/asm-x86/fixmap_64.h +0 -3

Index: linux-2.6/include/asm-x86/fixmap_64.h
===================================================================
@@ -43,9 +43,6 @@ enum fixed_addresses {
 	FIX_APIC_BASE,	/* local (CPU) APIC) -- required for SMP or not */
 	FIX_IO_APIC_BASE_0,
 	FIX_IO_APIC_BASE_END = FIX_IO_APIC_BASE_0 + MAX_IO_APICS - 1,
-	FIX_EFI_IO_MAP_LAST_PAGE,
-	FIX_EFI_IO_MAP_FIRST_PAGE = FIX_EFI_IO_MAP_LAST_PAGE
-				  + MAX_EFI_IO_PAGES - 1,
 #ifdef CONFIG_PARAVIRT
 	FIX_PARAVIRT_BOOTMAP,
 #endif

Ingo Molnar

2008-10-04 08:47 UTC #1 lore raw

Quoted 8 lines · Yinghai Lu

Signed-off-by Yinghai Lu <[email protected]>

---
 arch/x86/kernel/efi.c       |    5 +----
 arch/x86/kernel/efi_64.c    |   27 ++-------------------------
 include/asm-x86/efi.h       |    8 ++------
 include/asm-x86/fixmap_64.h |    3 ---
 4 files changed, 5 insertions(+), 38 deletions(-)

ok, that looks like a very nice cleanup and should make the EFI mapping code more generic and more robust as well.

Huang, any chance you could test this against latest tip/master on a real EFI system?

Ingo

huang ying

2008-10-04 09:36 UTC #2 lore raw

In reply to Yinghai Lu

Hi, Yinghai,

On Sat, Oct 4, 2008 at 2:23 PM, Yinghai Lu <[email protected]> wrote:
[...]

arch/x86/kernel/efi.c void __init efi_enter_virtual_mode(void) · line 478

Quoted 11 lines

arch/x86/kernel/efi.c +1 -4

@@ -475,10 +475,7 @@ void __init efi_enter_virtual_mode(void)
               size = md->num_pages << EFI_PAGE_SHIFT;
               end = md->phys_addr + size;
 
-               if (PFN_UP(end) <= max_low_pfn_mapped)
-                       va = __va(md->phys_addr);
-               else
-                       va = efi_ioremap(md->phys_addr, size);
+               va = efi_ioremap(md->phys_addr, size);

Using __va and efi_ioremap() here is to make EFI support compatible with kexec. Because EFI provide only efi_enter_virtual_mode(), no efi_leave_virtual_mode(), we should make EFI runtime memory area mapped to same virtual memory area in original kernel and kexeced kernel, so that the EFI runtime services can be used in kexeced kernel.

[...]

arch/x86/kernel/efi_64.c static void __init early_mapping_set_exe · line 53

Quoted 13 lines

arch/x86/kernel/efi_64.c +2 -2

@@ -46,8 +46,8 @@ static void __init early_mapping_set_exe
 {
       unsigned long num_pages;
 
-       start &= PMD_MASK;
-       end = (end + PMD_SIZE - 1) & PMD_MASK;
+       start &= PAGE_MASK;
+       end = (end + PAGE_SIZE - 1) & PAGE_MASK;
       num_pages = (end - start) >> PAGE_SHIFT;
       if (executable)
               set_memory_x((unsigned long)__va(start), num_pages);

early_mapping_set_exe() may be called before page allocator is available. Using PAGE_MASK may make set_memory_x() allocate new page, while using PMD_MASK will not.

Best Regards,
Huang Ying

Yinghai Lu

2008-10-04 17:44 UTC #3 lore raw

Quoted 10 lines · huang ying

Hi, Yinghai,
On Sat, Oct 4, 2008 at 2:23 PM, Yinghai Lu <[email protected]> wrote:
[...]
Quoted 6 lines
arch/x86/kernel/efi.c +0 -0
@@ -475,10 +475,7 @@ void __init efi_enter_virtual_mode(void)
               size = md->num_pages << EFI_PAGE_SHIFT;
               end = md->phys_addr + size;
 

Quoted 12 lines

arch/x86/kernel/efi.c void __init efi_enter_virtual_mode(void) · line 478
Quoted 5 lines
arch/x86/kernel/efi.c +1 -4
-               if (PFN_UP(end) <= max_low_pfn_mapped)
-                       va = __va(md->phys_addr);
-               else
-                       va = efi_ioremap(md->phys_addr, size);
+               va = efi_ioremap(md->phys_addr, size);
Using __va and efi_ioremap() here is to make EFI support compatible with kexec. Because EFI provide only efi_enter_virtual_mode(), no efi_leave_virtual_mode(), we should make EFI runtime memory area mapped to same virtual memory area in original kernel and kexeced kernel, so that the EFI runtime services can be used in kexeced kernel.

so need to make efi range all under direct-mapping like E820-RAM?

if so we need to update
e820_end_of_low_ram_pfn to return correct max_low_pfn, and then max_low_pfn_mapped...

huang ying

2008-10-05 08:56 UTC #4 lore raw

Quoted 1 line · Yinghai Lu

On Sat, Oct 4, 2008 at 2:35 AM, huang ying <[email protected]> wrote:

[...]

Quoted 8 lines

Quoted 6 lines
Using __va and efi_ioremap() here is to make EFI support compatible with kexec. Because EFI provide only efi_enter_virtual_mode(), no efi_leave_virtual_mode(), we should make EFI runtime memory area mapped to same virtual memory area in original kernel and kexeced kernel, so that the EFI runtime services can be used in kexeced kernel.
so need to make efi range all under direct-mapping like E820-RAM?

Some EFI runtime range is just some RAM area used by EFI runtime services, they can be direct-mapped. Some EFI runtime range may be IO MEM range used by EFI runtime services, it is possible that these IO MEM range can not be direct-mapped. So I implement efi_ioremap() to deal with them.

Best Regards,
Huang Ying

Ingo Molnar

2008-10-05 10:05 UTC #5 lore raw

Quoted 10 lines · huang ying

Quoted 1 line · Yinghai Lu
On Sat, Oct 4, 2008 at 2:35 AM, huang ying <[email protected]> wrote:
[...]
Quoted 7 lines
Quoted 6 lines
Using __va and efi_ioremap() here is to make EFI support compatible with kexec. Because EFI provide only efi_enter_virtual_mode(), no efi_leave_virtual_mode(), we should make EFI runtime memory area mapped to same virtual memory area in original kernel and kexeced kernel, so that the EFI runtime services can be used in kexeced kernel.

Quoted 7 lines

Quoted 1 line
so need to make efi range all under direct-mapping like E820-RAM?
Some EFI runtime range is just some RAM area used by EFI runtime services, they can be direct-mapped. Some EFI runtime range may be IO MEM range used by EFI runtime services, it is possible that these IO MEM range can not be direct-mapped. So I implement efi_ioremap() to deal with them.

hm, but in the "some RAM area" case, that area should not be listed in the e820 map (or any EFI memory map), and hence it should never be mapped directly. I.e. you should be able to just standardize on ioremap() and have no parallel facility for this.

Ingo

Huang Ying

2008-10-06 01:48 UTC #6 lore raw

Quoted 1 line · Ingo Molnar

* huang ying <[email protected]> wrote:

[...]

Quoted 10 lines

Quoted 5 lines
Some EFI runtime range is just some RAM area used by EFI runtime services, they can be direct-mapped. Some EFI runtime range may be IO MEM range used by EFI runtime services, it is possible that these IO MEM range can not be direct-mapped. So I implement efi_ioremap() to deal with them.
hm, but in the "some RAM area" case, that area should not be listed in the e820 map (or any EFI memory map), and hence it should never be mapped directly. I.e. you should be able to just standardize on ioremap() and have no parallel facility for this.

EFI runtime memory area will be E820_RESERVED in e820 map and EFI_RUNTIME_SERVICES_CODE/DATA in EFI memory map. In general they will be mapped directly.

ioremap() should not used here, because for kexec to work, EFI runtime memory area should be mapped to same virtual address across reboot.

Best Regards,
Huang Ying

Yinghai Lu

2008-10-05 18:04 UTC #7 lore raw

In reply to huang ying

Quoted 10 lines · huang ying

Quoted 1 line · Yinghai Lu
On Sat, Oct 4, 2008 at 2:35 AM, huang ying <[email protected]> wrote:
[...]
Quoted 7 lines
Quoted 6 lines
Using __va and efi_ioremap() here is to make EFI support compatible with kexec. Because EFI provide only efi_enter_virtual_mode(), no efi_leave_virtual_mode(), we should make EFI runtime memory area mapped to same virtual memory area in original kernel and kexeced kernel, so that the EFI runtime services can be used in kexeced kernel.

Quoted 8 lines

Quoted 1 line
so need to make efi range all under direct-mapping like E820-RAM?
Some EFI runtime range is just some RAM area used by EFI runtime services, they can be direct-mapped. Some EFI runtime range may be IO MEM range used by EFI runtime services, it is possible that these IO MEM range can not be direct-mapped. So I implement efi_ioremap() to deal with them.

i'm confused.

arch/x86/kernel/efi.c +1 -4

@@ -475,10 +475,7 @@ void __init efi_enter_virtual_mode(void)
               size = md->num_pages << EFI_PAGE_SHIFT;
               end = md->phys_addr + size;
 
-               if (PFN_UP(end) <= max_low_pfn_mapped)
-                       va = __va(md->phys_addr);
-               else
-                       va = efi_ioremap(md->phys_addr, size);
+               va = efi_ioremap(md->phys_addr, size);
 
               md->virt_addr = (u64) (unsigned long) va;

should be ok.

then how about use ioremap directly to replace fixed mapping in 64bit with efi_ioremap?

Huang Ying

2008-10-06 01:51 UTC #8 lore raw

Quoted 36 lines · Yinghai Lu

Quoted 10 lines · huang ying
Quoted 1 line · Yinghai Lu
On Sat, Oct 4, 2008 at 2:35 AM, huang ying <[email protected]> wrote:
[...]
Quoted 7 lines
Quoted 6 lines
Using __va and efi_ioremap() here is to make EFI support compatible with kexec. Because EFI provide only efi_enter_virtual_mode(), no efi_leave_virtual_mode(), we should make EFI runtime memory area mapped to same virtual memory area in original kernel and kexeced kernel, so that the EFI runtime services can be used in kexeced kernel.
Quoted 8 lines
Quoted 1 line
so need to make efi range all under direct-mapping like E820-RAM?
Some EFI runtime range is just some RAM area used by EFI runtime services, they can be direct-mapped. Some EFI runtime range may be IO MEM range used by EFI runtime services, it is possible that these IO MEM range can not be direct-mapped. So I implement efi_ioremap() to deal with them.
i'm confused.
so
arch/x86/kernel/efi.c +1 -4
@@ -475,10 +475,7 @@ void __init efi_enter_virtual_mode(void)
               size = md->num_pages << EFI_PAGE_SHIFT;
               end = md->phys_addr + size;
 
-               if (PFN_UP(end) <= max_low_pfn_mapped)
-                       va = __va(md->phys_addr);
-               else
-                       va = efi_ioremap(md->phys_addr, size);
+               va = efi_ioremap(md->phys_addr, size);
 
               md->virt_addr = (u64) (unsigned long) va;

Quoted 1 line

should be ok.

I think __va() and efi_ioremap() should be used.

Quoted 2 lines

then how about use ioremap directly to replace fixed mapping in 64bit with efi_ioremap?

Because for kexec to work, EFI runtime memory area should be mapped to same virtual address across reboot.

Best Regards,
Huang Ying