linux-kernel - Re: [PATCH 3/3] kdump: round up the total memory size to 128M for crashkernel reservation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20171024055722.GG27757@x1>
Date:   Tue, 24 Oct 2017 13:57:22 +0800
From:   Baoquan He <bhe@...hat.com>
To:     dyoung@...hat.com
Cc:     kexec@...ts.infradead.org, linux-kernel@...r.kernel.org,
        akpm@...ux-foundation.org, vgoyal@...hat.com, yinghai@...nel.org,
        corbet@....net
Subject: Re: [PATCH 3/3] kdump: round up the total memory size to 128M for
 crashkernel reservation

Hi Dave,

On 10/24/17 at 01:31pm, Dave Young wrote:
> The total memory size we get in kernel is usually slightly less than 2G with a
> 2G memory module machine. The main reason is bios/firmware reserve some area
> it will not export all memory as usable to Linux.
> 
> 2G memory X86 kvm guest test result of the total_mem value:
> UEFI boot with ovmf: 0x7ef10000
> Legacy boot kvm guest: 0x7ff7cc00
> 
> An option is to use dmi/smbios to get physical memory size, but it's not
> reliable as well. According to Prarit hardware vendors sometimes screw this up.
> Thus we choose to round up total size to 128M to workaround this problem.
> This is a best effort workaround, will improve it when we have better way
> in the future. 

Thanks for this posting. While I don't get the point of this patch. So
firmware take piece of memory, then why we need to count it into the
total memory which we want to calculate a crashkernel memory based on.

Not counting that, is there anyting incorrect?

Thanks
Baoquan

> 
> Signed-off-by: Dave Young <dyoung@...hat.com>
> ---
>  kernel/crash_core.c |   17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> --- linux.orig/kernel/crash_core.c
> +++ linux/kernel/crash_core.c
> @@ -42,6 +42,15 @@ static int __init parse_crashkernel_mem(
>  {
>  	char *cur = cmdline, *tmp;
>  	bool infinite_end = false;
> +	unsigned long long total_mem = system_ram;
> +
> +	/*
> +	 * Firmware usually reserves some memory regions for it's own use.
> +	 * so we get less than actual system memory size.
> +	 * We workaround this by round up the total size to 128M which is
> +	 * enough for most test cases.
> +	 */
> +	total_mem = roundup(total_mem, 0x8000000);
>  
>  	/* for each entry of the comma-separated list */
>  	do {
> @@ -86,13 +95,13 @@ static int __init parse_crashkernel_mem(
>  			return -EINVAL;
>  		}
>  		cur = tmp;
> -		if (size >= system_ram) {
> +		if (size >= total_mem) {
>  			pr_warn("crashkernel: invalid size\n");
>  			return -EINVAL;
>  		}
>  
>  		/* match ? */
> -		if (system_ram >= start && system_ram < end) {
> +		if (total_mem >= start && total_mem < end) {
>  			*crash_size = size;
>  			if (end == ULLONG_MAX)
>  				infinite_end = true;
> @@ -126,9 +135,9 @@ static int __init parse_crashkernel_mem(
>  				pr_warn("Memory reservation scale order expected after '^'\n");
>  				return -EINVAL;
>  			}
> -			size = (system_ram - *crash_size) >> shift;
> +			size = (total_mem - *crash_size) >> shift;
>  			size = *crash_size + roundup(size, 1ULL << 20);
> -			if (size < system_ram)
> +			if (size < total_mem)
>  				*crash_size = size;
>  			cur = tmp;
>  		} else
> 
>