Re: [PATCH 07/29] x86/boot/e820: Print out sizes of E820 memory ranges

[Ingo Molnar <mingo@kernel.org>]

* Mike Rapoport <rppt@kernel.org> wrote:

> On Mon, Apr 21, 2025 at 08:51:47PM +0200, Ingo Molnar wrote:
> > Before:
> > 
> >         BIOS-provided physical RAM map:
> >         BIOS-e820: [mem 0x0000000000000000-0x000000000009fbff] usable
> >         BIOS-e820: [mem 0x000000000009fc00-0x000000000009ffff] reserved
> >         BIOS-e820: [mem 0x00000000000f0000-0x00000000000fffff] reserved
> >         BIOS-e820: [mem 0x0000000000100000-0x000000007ffdbfff] usable
> >         BIOS-e820: [mem 0x000000007ffdc000-0x000000007fffffff] reserved
> >         BIOS-e820: [mem 0x00000000b0000000-0x00000000bfffffff] reserved
> >         BIOS-e820: [mem 0x00000000fed1c000-0x00000000fed1ffff] reserved
> >         BIOS-e820: [mem 0x00000000feffc000-0x00000000feffffff] reserved
> >         BIOS-e820: [mem 0x00000000fffc0000-0x00000000ffffffff] reserved
> >         BIOS-e820: [mem 0x000000fd00000000-0x000000ffffffffff] reserved
> > 
> > After:
> > 
> > 	BIOS-provided physical RAM map:
> > 	BIOS-e820: [mem 0x0000000000000000-0x000000000009fbff]  639   KB kernel usable RAM
> > 	BIOS-e820: [mem 0x000000000009fc00-0x000000000009ffff]    1   KB reserved
> > 	BIOS-e820: [gap 0x00000000000a0000-0x00000000000effff]  320   KB ...
> > 	BIOS-e820: [mem 0x00000000000f0000-0x00000000000fffff]   64   KB reserved
> > 	BIOS-e820: [mem 0x0000000000100000-0x000000007ffdbfff]    1.9 GB kernel usable RAM
> > 	BIOS-e820: [mem 0x000000007ffdc000-0x000000007fffffff]  144   KB reserved
> > 	BIOS-e820: [gap 0x0000000080000000-0x00000000afffffff]  768   MB ...
> > 	BIOS-e820: [mem 0x00000000b0000000-0x00000000bfffffff]  256   MB reserved
> > 	BIOS-e820: [gap 0x00000000c0000000-0x00000000fed1bfff] 1005.1 MB ...
> > 	BIOS-e820: [mem 0x00000000fed1c000-0x00000000fed1ffff]   16   KB reserved
> > 	BIOS-e820: [gap 0x00000000fed20000-0x00000000feffbfff]    2.8 MB ...
> > 	BIOS-e820: [mem 0x00000000feffc000-0x00000000feffffff]   16   KB reserved
> > 	BIOS-e820: [gap 0x00000000ff000000-0x00000000fffbffff]   15.7 MB ...
> > 	BIOS-e820: [mem 0x00000000fffc0000-0x00000000ffffffff]  256   KB reserved
> > 	BIOS-e820: [gap 0x0000000100000000-0x000000fcffffffff] 1008   GB ...
> > 	BIOS-e820: [mem 0x000000fd00000000-0x000000ffffffffff]   12   GB reserved
> > 
> > Note how a 1-digit precision field is printed out if a range is
> > fractional in its largest-enclosing natural size unit.
> > 
> > So the "256 MB" and "12 GB" fields above denote exactly 256 MB and
> > 12 GB regions, while "1.9 GB" signals the region's fractional nature
> > and it being just below 2GB.
> > 
> > Printing E820 maps with such details visualizes 'weird' ranges
> > at a glance, and gives users a better understanding of how
> > large the various ranges are, without having to perform hexadecimal
> > subtraction in their minds.
> > 
> > Signed-off-by: Ingo Molnar <mingo@kernel.org>
> > Cc: Andy Shevchenko <andy@kernel.org>
> > Cc: Arnd Bergmann <arnd@kernel.org>
> > Cc: David Woodhouse <dwmw@amazon.co.uk>
> > Cc: H. Peter Anvin <hpa@zytor.com>
> > Cc: Kees Cook <keescook@chromium.org>
> > Cc: Linus Torvalds <torvalds@linux-foundation.org>
> > Cc: Mike Rapoport (Microsoft) <rppt@kernel.org>
> > (cherry picked from commit d1ac6b8718575a7ea2f0a1ff347835a8879df673)
> > ---
> >  arch/x86/kernel/e820.c | 47 +++++++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 45 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> > index 10bd10bd5672..8ee89962fcbf 100644
> > --- a/arch/x86/kernel/e820.c
> > +++ b/arch/x86/kernel/e820.c
> > @@ -199,6 +199,41 @@ static void __init e820_print_type(enum e820_type type)
> >  	}
> >  }
> >  
> > +/*
> > + * Print out the size of a E820 region, in human-readable
> > + * fashion, going from KB, MB, GB to TB units.
> > + *
> > + * Print out fractional sizes with a single digit of precision.
> > + */
> > +static void e820_print_size(u64 size)
> > +{
> > +	if (size < SZ_1M) {
> > +		if (size & (SZ_1K-1))
> > +			pr_cont(" %4llu.%01llu KB", size/SZ_1K, 10*(size & (SZ_1K-1))/SZ_1K);
> > +		else
> > +			pr_cont(" %4llu   KB", size/SZ_1K);
> > +		return;
> > +	}
> 
> I'd make this a helper, e.g
> 
> static void __e820_print_size(u64 size, u64 unit, const char *unit_name)
> {
> 	if (size & (unit - 1)) {
> 		u64 fraction = 10 * (size & (unit - 1))/unit;
> 
> 		pr_cont(" %4llu.%01llu %s", size/unit, fraction, unit_name);
> 	} else {
> 		pr_cont(" %4llu   %s", size/unit, unit_name);
> 	}
> }

While I like the helper in principle, it doesn't work on 32-bit: it's 
an u64 with u64 division, while with the open coded literals the 
compiler figures it out.

With div64_u64(), or a macro, it's not nearly as obvious a cleanup 
IMHO.

Thanks,

	Ingo