efi: Fix the size not consistent issue when unmapping memory map

PATCH In review Lee, Chun-Yi 3 participants 3 messages May 2, 2018 – May 4, 2018 EFI

Share & tools

Lee, Chun-Yi

author

2018-05-02 06:17 UTC #0 lore raw

PATCH 1/1 efi: Fix the size not consistent issue when unmapping memory map

When using kdump, SOMETIMES the "size not consistent" warning message shows up when the crash kernel boots with early_ioremap_debug parameter:

WARNING: CPU: 0 PID: 0 at ../mm/early_ioremap.c:182 early_iounmap+0x4f/0x12c()

early_iounmap(ffffffffff200180, 00000118) [0] size not consistent 00000120

The root cause is that the unmapping size of memory map doesn't match with the original size when mapping:

in __efi_memmap_init()

	map.map = early_memremap(phys_map, data->size);

in efi_memmap_unmap()

        size = efi.memmap.desc_size * efi.memmap.nr_map;
        early_memunmap(efi.memmap.map, size);

But the efi.memmap.nr_map is from __efi_memmap_init(). The remainder of size was discarded when calculating the nr_map:

        map.nr_map = data->size / data->desc_size;

When the original size of memory map region does not equal to the result of multiplication. The "size not consistent" warning will be triggered.

This issue sometimes was hit by kdump because kexec set the efi map size to align with 16 when loading crash kernel image:

in bzImage64_load()

        efi_map_sz = efi_get_runtime_map_size();
        efi_map_sz = ALIGN(efi_map_sz, 16);

Dave Young's a841aa83d patch fixed kexec issue. On UEFI side, this patch changes the logic in the unmapping function. Using the end address of map to calcuate original size.

Thank Randy Wright for his report and testing. And also thank Takashi Iwai for his help to trace issue.

Cc Ard Biesheuvel <[email protected]>

Cc Takashi Iwai <[email protected]>

Cc Vivek Goyal <[email protected]>

Cc Ingo Molnar <[email protected]>

Tested-by Randy Wright <[email protected]>

Signed-off-by "Lee, Chun-Yi" <[email protected]>

---
 drivers/firmware/efi/memmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/firmware/efi/memmap.c +1 -1

index 5fc7052..1f592d8 100644
@@ -121,7 +121,7 @@ void __init efi_memmap_unmap(void)
 	if (!efi.memmap.late) {
 		unsigned long size;
 
-		size = efi.memmap.desc_size * efi.memmap.nr_map;
+		size = efi.memmap.map_end - efi.memmap.map;
 		early_memunmap(efi.memmap.map, size);
 	} else {
 		memunmap(efi.memmap.map);

Signature · 1 line

2.10.2

Ard Biesheuvel

2018-05-03 12:05 UTC #1 lore raw

Quoted 27 lines · Chun-Yi

When using kdump, SOMETIMES the "size not consistent" warning message shows up when the crash kernel boots with early_ioremap_debug parameter:
WARNING: CPU: 0 PID: 0 at ../mm/early_ioremap.c:182 early_iounmap+0x4f/0x12c()
early_iounmap(ffffffffff200180, 00000118) [0] size not consistent 00000120
The root cause is that the unmapping size of memory map doesn't match with the original size when mapping:
in __efi_memmap_init()
        map.map = early_memremap(phys_map, data->size);
in efi_memmap_unmap()
        size = efi.memmap.desc_size * efi.memmap.nr_map;
        early_memunmap(efi.memmap.map, size);
But the efi.memmap.nr_map is from __efi_memmap_init(). The remainder of size was discarded when calculating the nr_map:
        map.nr_map = data->size / data->desc_size;
When the original size of memory map region does not equal to the result of multiplication. The "size not consistent" warning will be triggered.
This issue sometimes was hit by kdump because kexec set the efi map size to align with 16 when loading crash kernel image:

Quoted 8 lines

in bzImage64_load()
        efi_map_sz = efi_get_runtime_map_size();
        efi_map_sz = ALIGN(efi_map_sz, 16);
Dave Young's a841aa83d patch fixed kexec issue. On UEFI side, this patch changes the logic in the unmapping function. Using the end address of map to calcuate original size.

Why do we still need this patch? I.e., in which circumstances will efi_memory_map_data::size assume a value that is rounded up or otherwise incorrect?

Quoted 29 lines

Thank Randy Wright for his report and testing. And also thank Takashi Iwai for his help to trace issue.
Cc Ard Biesheuvel <[email protected]>
Cc Takashi Iwai <[email protected]>
Cc Vivek Goyal <[email protected]>
Cc Ingo Molnar <[email protected]>
Tested-by Randy Wright <[email protected]>
Signed-off-by "Lee, Chun-Yi" <[email protected]>
---
 drivers/firmware/efi/memmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/firmware/efi/memmap.c +1 -2
index 5fc7052..1f592d8 100644
@@ -121,7 +121,7 @@ void __init efi_memmap_unmap(void)
        if (!efi.memmap.late) {
                unsigned long size;
 
-               size = efi.memmap.desc_size * efi.memmap.nr_map;
+               size = efi.memmap.map_end - efi.memmap.map;
                early_memunmap(efi.memmap.map, size);
        } else {
                memunmap(efi.memmap.map);
--
2.10.2

joeyli

2018-05-04 07:29 UTC #2 lore raw

Hi Ard,

Quoted 32 lines · Ard Biesheuvel

Quoted 31 lines · Chun-Yi
When using kdump, SOMETIMES the "size not consistent" warning message shows up when the crash kernel boots with early_ioremap_debug parameter:
WARNING: CPU: 0 PID: 0 at ../mm/early_ioremap.c:182 early_iounmap+0x4f/0x12c()
early_iounmap(ffffffffff200180, 00000118) [0] size not consistent 00000120
The root cause is that the unmapping size of memory map doesn't match with the original size when mapping:
in __efi_memmap_init()
        map.map = early_memremap(phys_map, data->size);
in efi_memmap_unmap()
        size = efi.memmap.desc_size * efi.memmap.nr_map;
        early_memunmap(efi.memmap.map, size);
But the efi.memmap.nr_map is from __efi_memmap_init(). The remainder of size was discarded when calculating the nr_map:
        map.nr_map = data->size / data->desc_size;
When the original size of memory map region does not equal to the result of multiplication. The "size not consistent" warning will be triggered.
This issue sometimes was hit by kdump because kexec set the efi map size to align with 16 when loading crash kernel image:
in bzImage64_load()
        efi_map_sz = efi_get_runtime_map_size();
        efi_map_sz = ALIGN(efi_map_sz, 16);

Quoted 9 lines

Quoted 4 lines
Dave Young's a841aa83d patch fixed kexec issue. On UEFI side, this patch changes the logic in the unmapping function. Using the end address of map to calcuate original size.
Why do we still need this patch? I.e., in which circumstances will efi_memory_map_data::size assume a value that is rounded up or otherwise incorrect?

There have no other case except kexec. But I think that it's better to sync mapping/unmapping size between __efi_memmap_init() and efi_memmap_unmap().

Thanks
Joey Lee

Quoted 29 lines

Quoted 29 lines
Thank Randy Wright for his report and testing. And also thank Takashi Iwai for his help to trace issue.
Cc Ard Biesheuvel <[email protected]>
Cc Takashi Iwai <[email protected]>
Cc Vivek Goyal <[email protected]>
Cc Ingo Molnar <[email protected]>
Tested-by Randy Wright <[email protected]>
Signed-off-by "Lee, Chun-Yi" <[email protected]>
---
 drivers/firmware/efi/memmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/firmware/efi/memmap.c +1 -2
index 5fc7052..1f592d8 100644
@@ -121,7 +121,7 @@ void __init efi_memmap_unmap(void)
        if (!efi.memmap.late) {
                unsigned long size;
 
-               size = efi.memmap.desc_size * efi.memmap.nr_map;
+               size = efi.memmap.map_end - efi.memmap.map;
                early_memunmap(efi.memmap.map, size);
        } else {
                memunmap(efi.memmap.map);
--
2.10.2