Received-SPF: pass (google.com: domain of linux-kernel+bounces-91495-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Tue, 5 Mar 2024 09:32:26 +0800
From: Dave Young <dyoung@redhat.com>
To: x86@kernel.org
Cc: Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>,
	Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	"H. Peter Anvin" <hpa@zytor.com>, linux-kernel@vger.kernel.org,
	kexec@lists.infradead.org, Baoquan He <bhe@redhat.com>,
	Eric Biederman <ebiederm@xmission.com>
Subject: [PATCH] x86/kexec: do not update E820 kexec table for setup_data
Message-ID: <ZeZ2Kos-OOZNSrmO@darkstar.users.ipa.redhat.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

crashkernel reservation failed on a Thinkpad t440s laptop recently,
Actually the memblock reservation succeeded, but later insert_resource()
failed.

Test step:
kexec load ->
	kexec reboot -> 
		check the crashkernel memory
		dmesg|grep "crashkernel reserved"; saw reserved suceeeded:
		0x00000000d0000000 - 0x00000000da000000
		grep Crash /proc/iomem: got nothing 

The background story is like below:
Currently E820 code reserves setup_data regions for both the current kernel
and the kexec kernel, and it will also insert them into resources list.
Before the kexec kernel reboot nobody passes the old setup_data, kexec only
passes SETUP_EFI and SETUP_IMA if needed.  Thus the old setup data memory
are not used at all. But due to old kernel updated the kexec e820 table
as well so kexec kernel see them as E820_TYPE_RESERVED_KERN regions, later
the old setup_data regions will be inserted into resources list in kexec
kernel by e820__reserve_resources().

Note, due to no setup_data passed in for those old regions they are not
early reserved (by function early_reserve_memory), crashkernel memblock
reservation will just regard them as usable memory and it could reserve
reserve crashkernel region overlaps with the old setup_data regions.

Just like the bug I noticed here, kdump insert_resource failed because
e820__reserve_resources added the overlapped chunks in /proc/iomem already.

Finally, looking at the code, the old setup_data regions are not used
at all as no setup_data passed in by the kexec boot loader. Although
something like SETUP_PCI etc could be needed, kexec should pass
the info as setup_data so that kexec kernel can take care of them.
This should be taken care of in other separate patches if needed.

Thus drop the useless buggy code here.

Signed-off-by: Dave Young <dyoung@redhat.com>
---
 arch/x86/kernel/e820.c |   16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

Index: linux/arch/x86/kernel/e820.c
===================================================================
--- linux.orig/arch/x86/kernel/e820.c
+++ linux/arch/x86/kernel/e820.c
@@ -1015,16 +1015,6 @@ void __init e820__reserve_setup_data(voi
 		pa_next = data->next;
 
 		e820__range_update(pa_data, sizeof(*data)+data->len, E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
-
-		/*
-		 * SETUP_EFI and SETUP_IMA are supplied by kexec and do not need
-		 * to be reserved.
-		 */
-		if (data->type != SETUP_EFI && data->type != SETUP_IMA)
-			e820__range_update_kexec(pa_data,
-						 sizeof(*data) + data->len,
-						 E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
-
 		if (data->type == SETUP_INDIRECT) {
 			len += data->len;
 			early_memunmap(data, sizeof(*data));
@@ -1036,12 +1026,9 @@ void __init e820__reserve_setup_data(voi
 
 			indirect = (struct setup_indirect *)data->data;
 
-			if (indirect->type != SETUP_INDIRECT) {
+			if (indirect->type != SETUP_INDIRECT)
 				e820__range_update(indirect->addr, indirect->len,
 						   E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
-				e820__range_update_kexec(indirect->addr, indirect->len,
-							 E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
-			}
 		}
 
 		pa_data = pa_next;
@@ -1049,7 +1036,6 @@ void __init e820__reserve_setup_data(voi
 	}
 
 	e820__update_table(e820_table);
-	e820__update_table(e820_table_kexec);
 
 	pr_info("extended physical RAM map:\n");
 	e820__print_table("reserve setup_data");