2018-05-02 06:18:02

by Lee, Chun-Yi

[permalink] [raw]
Subject: [PATCH] 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(-)

diff --git a/drivers/firmware/efi/memmap.c b/drivers/firmware/efi/memmap.c
index 5fc7052..1f592d8 100644
--- a/drivers/firmware/efi/memmap.c
+++ b/drivers/firmware/efi/memmap.c
@@ -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



2018-05-03 12:06:54

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH] efi: Fix the size not consistent issue when unmapping memory map

On 2 May 2018 at 08:17, Lee, Chun-Yi <[email protected]> wrote:
> 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.
>

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?

> 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(-)
>
> diff --git a/drivers/firmware/efi/memmap.c b/drivers/firmware/efi/memmap.c
> index 5fc7052..1f592d8 100644
> --- a/drivers/firmware/efi/memmap.c
> +++ b/drivers/firmware/efi/memmap.c
> @@ -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
>

2018-05-04 07:30:09

by joeyli

[permalink] [raw]
Subject: Re: [PATCH] efi: Fix the size not consistent issue when unmapping memory map

Hi Ard,

On Thu, May 03, 2018 at 02:05:51PM +0200, Ard Biesheuvel wrote:
> On 2 May 2018 at 08:17, Lee, Chun-Yi <[email protected]> wrote:
> > 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.
> >
>
> 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

> > 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(-)
> >
> > diff --git a/drivers/firmware/efi/memmap.c b/drivers/firmware/efi/memmap.c
> > index 5fc7052..1f592d8 100644
> > --- a/drivers/firmware/efi/memmap.c
> > +++ b/drivers/firmware/efi/memmap.c
> > @@ -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
> >