From mboxrd@z Thu Jan 1 00:00:00 1970 From: joeyli Subject: Re: [PATCH] efi: Fix the size not consistent issue when unmapping memory map Date: Fri, 4 May 2018 15:29:16 +0800 Message-ID: <20180504072916.GR4235@linux-l9pv.suse> References: <20180502061724.25833-1-jlee@suse.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Ard Biesheuvel Cc: "Lee, Chun-Yi" , linux-efi@vger.kernel.org, Linux Kernel Mailing List , Takashi Iwai , Vivek Goyal , Ingo Molnar List-Id: linux-efi@vger.kernel.org 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 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 > > Cc: Takashi Iwai > > Cc: Vivek Goyal > > Cc: Ingo Molnar > > Tested-by: Randy Wright > > Signed-off-by: "Lee, Chun-Yi" > > --- > > 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 > >