From: Baoquan He <bhe@redhat.com>
To: Lianbo Jiang <lijiang@redhat.com>
Cc: linux-kernel@vger.kernel.org, linux-efi@vger.kernel.org,
platform-driver-x86@vger.kernel.org, x86@kernel.org,
ardb@kernel.org, tglx@linutronix.de, mingo@redhat.com,
bp@alien8.de, dvhart@infradead.org, andy@infradead.org,
hpa@zytor.com, kexec@lists.infradead.org, dyoung@redhat.com
Subject: Re: [PATCH] x86/efi: Do not release sub-1MB memory regions when the crashkernel option is specified
Date: Fri, 9 Apr 2021 20:44:43 +0800 [thread overview]
Message-ID: <20210409124443.GA20513@MiWiFi-R3L-srv> (raw)
In-Reply-To: <20210407140316.30210-1-lijiang@redhat.com>
On 04/07/21 at 10:03pm, Lianbo Jiang wrote:
> Some sub-1MB memory regions may be reserved by EFI boot services, and the
> memory regions will be released later in the efi_free_boot_services().
>
> Currently, always reserve all sub-1MB memory regions when the crashkernel
> option is specified, but unfortunately EFI boot services may have already
> reserved some sub-1MB memory regions before the crash_reserve_low_1M() is
> called, which makes that the crash_reserve_low_1M() only own the
> remaining sub-1MB memory regions, not all sub-1MB memory regions, because,
> subsequently EFI boot services will free its own sub-1MB memory regions.
> Eventually, DMA will be able to allocate memory from the sub-1MB area and
> cause the following error:
>
So this patch is fixing a problem found in crash utility. We ever met
the similar issue, later fixed by always reserving low 1M in commit
6f599d84231fd27 ("x86/kdump: Always reserve the low 1M when the crashkernel
option is specified"). Seems the commit is not fixing it completely.
> crash> kmem -s |grep invalid
> kmem: dma-kmalloc-512: slab: ffffd52c40001900 invalid freepointer: ffff9403c0067300
> kmem: dma-kmalloc-512: slab: ffffd52c40001900 invalid freepointer: ffff9403c0067300
> crash> vtop ffff9403c0067300
> VIRTUAL PHYSICAL
> ffff9403c0067300 67300 --->The physical address falls into this range [0x0000000000063000-0x000000000008efff]
>
> kernel debugging log:
> ...
> [ 0.008927] memblock_reserve: [0x0000000000010000-0x0000000000013fff] efi_reserve_boot_services+0x85/0xd0
> [ 0.008930] memblock_reserve: [0x0000000000063000-0x000000000008efff] efi_reserve_boot_services+0x85/0xd0
> ...
> [ 0.009425] memblock_reserve: [0x0000000000000000-0x00000000000fffff] crash_reserve_low_1M+0x2c/0x49
> ...
> [ 0.010586] Zone ranges:
> [ 0.010587] DMA [mem 0x0000000000001000-0x0000000000ffffff]
> [ 0.010589] DMA32 [mem 0x0000000001000000-0x00000000ffffffff]
> [ 0.010591] Normal [mem 0x0000000100000000-0x0000000c7fffffff]
> [ 0.010593] Device empty
> ...
> [ 8.814894] __memblock_free_late: [0x0000000000063000-0x000000000008efff] efi_free_boot_services+0x14b/0x23b
> [ 8.815793] __memblock_free_late: [0x0000000000010000-0x0000000000013fff] efi_free_boot_services+0x14b/0x23b
In commit 6f599d84231fd27, we call crash_reserve_low_1M() to lock the
whole low 1M area if crashkernel is specified in kernel cmdline.
But earlier efi_reserve_boot_services() invokation will break the
intention of the whole low 1M reserving. In efi_reserve_boot_services(),
if any memory under low 1M hasn't been reserved, it will call
memblock_reserve() to reserve it and leave it to
efi_free_boot_services() to free.
Hi Lianbo,
Please correct me if I am wrong or anything is missed. IIUC, can we move
efi_reserve_boot_services() after reserve_real_mode() to fix this bug?
Or move reserve_real_mode() before efi_reserve_boot_services() since
those real mode regions are all under 1M? Assume efi boot code/data
won't rely on low 1M area any more at this moment.
Thanks
Baoquan
>
> Do not release sub-1MB memory regions even though they are reserved by
> EFI boot services, so that always reserve all sub-1MB memory regions when
> the crashkernel option is specified.
>
> Signed-off-by: Lianbo Jiang <lijiang@redhat.com>
> ---
> arch/x86/platform/efi/quirks.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
> index 67d93a243c35..637f932c4fd4 100644
> --- a/arch/x86/platform/efi/quirks.c
> +++ b/arch/x86/platform/efi/quirks.c
> @@ -18,6 +18,7 @@
> #include <asm/cpu_device_id.h>
> #include <asm/realmode.h>
> #include <asm/reboot.h>
> +#include <asm/cmdline.h>
>
> #define EFI_MIN_RESERVE 5120
>
> @@ -303,6 +304,19 @@ void __init efi_arch_mem_reserve(phys_addr_t addr, u64 size)
> */
> static __init bool can_free_region(u64 start, u64 size)
> {
> + /*
> + * Some sub-1MB memory regions may be reserved by EFI boot
> + * services, and these memory regions will be released later
> + * in the efi_free_boot_services().
> + *
> + * Do not release sub-1MB memory regions even though they are
> + * reserved by EFI boot services, because, always reserve all
> + * sub-1MB memory when the crashkernel option is specified.
> + */
> + if (cmdline_find_option(boot_command_line, "crashkernel", NULL, 0) > 0
> + && (start + size < (1<<20)))
> + return false;
> +
> if (start + size > __pa_symbol(_text) && start <= __pa_symbol(_end))
> return false;
>
> --
> 2.17.1
>
next prev parent reply other threads:[~2021-04-09 12:45 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-07 14:03 [PATCH] x86/efi: Do not release sub-1MB memory regions when the crashkernel option is specified Lianbo Jiang
2021-04-09 12:44 ` Baoquan He [this message]
2021-04-10 2:56 ` lijiang
[not found] ` <D7D32C89-4F99-434A-B7AF-7BEBDA494172@zytor.com>
2021-04-12 1:13 ` Baoquan He
2021-04-12 1:49 ` Andy Lutomirski
2021-04-12 9:52 ` Baoquan He
2021-04-12 10:49 ` lijiang
2021-04-12 15:24 ` Andy Lutomirski
2021-04-13 9:45 ` Baoquan He
[not found] ` <CANU+ZydgWTSg+iUix8ggn-cSPpg8qtShaUQ47cOeeMxFmXp_zQ@mail.gmail.com>
[not found] ` <CANU+ZydyKsctuFjPfBz7PuS=FaUtK0gs5Lq06pL5nuRJKe+J0w@mail.gmail.com>
2021-05-24 8:32 ` Baoquan He
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210409124443.GA20513@MiWiFi-R3L-srv \
--to=bhe@redhat.com \
--cc=andy@infradead.org \
--cc=ardb@kernel.org \
--cc=bp@alien8.de \
--cc=dvhart@infradead.org \
--cc=dyoung@redhat.com \
--cc=hpa@zytor.com \
--cc=kexec@lists.infradead.org \
--cc=lijiang@redhat.com \
--cc=linux-efi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=platform-driver-x86@vger.kernel.org \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).