| 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
| ||
|
Message-ID: <aCXFdvWiNW94F24R@gmail.com>
Date: Thu, 15 May 2025 12:44:06 +0200
From: Ingo Molnar <mingo@...nel.org>
To: Andy Shevchenko <andy.shevchenko@...il.com>
Cc: linux-kernel@...r.kernel.org, Andy Shevchenko <andy@...nel.org>,
Arnd Bergmann <arnd@...nel.org>, Borislav Petkov <bp@...en8.de>,
Juergen Gross <jgross@...e.com>, "H . Peter Anvin" <hpa@...or.com>,
Kees Cook <keescook@...omium.org>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Mike Rapoport <rppt@...nel.org>,
Paul Menzel <pmenzel@...gen.mpg.de>,
Peter Zijlstra <peterz@...radead.org>,
Thomas Gleixner <tglx@...utronix.de>,
David Woodhouse <dwmw@...zon.co.uk>
Subject: Re: [PATCH 07/29] x86/boot/e820: Print out sizes of E820 memory
ranges
* Andy Shevchenko <andy.shevchenko@...il.com> wrote:
> Mon, Apr 21, 2025 at 08:51:47PM +0200, Ingo Molnar kirjoitti:
> > 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.
>
> ...
>
> > +/*
> > + * 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);
>
> I would add some spaces here and there for the sake of readability.
I think it's perfectly readable, skipping the whitespace for numeric
literals is standard style. Linus himself does that occasionally, see:
94a2bc0f611c ("arm64: add 'runtime constant' support")
static inline void __runtime_fixup_ptr(void *where, unsigned long val)
{
__le32 *p = lm_alias(where);
__runtime_fixup_16(p, val);
__runtime_fixup_16(p+1, val >> 16);
__runtime_fixup_16(p+2, val >> 32);
__runtime_fixup_16(p+3, val >> 48);
__runtime_fixup_caches(where, 4);
}
Or:
938df695e98d ("vsprintf: associate the format state with the format pointer")
+ unsigned int shift = 32 - size*8;
which uses visual grouping to make arithmethic expressions more
readable.
>
> > + return;
> > + }
> > + if (size < SZ_1G) {
>
> Can be written in one line as
>
> } else if (...) {
Done. (See delta patch below.)
>
> Ditto for the rest.
>
> > + if (size & (SZ_1M-1))
> > + pr_cont(" %4llu.%01llu MB", size/SZ_1M, 10*(size & (SZ_1M-1))/SZ_1M);
> > + else
> > + pr_cont(" %4llu MB", size/SZ_1M);
> > + return;
> > + }
> > + if (size < SZ_1T) {
> > + if (size & (SZ_1G-1))
> > + pr_cont(" %4llu.%01llu GB", size/SZ_1G, 10*(size & (SZ_1G-1))/SZ_1G);
> > + else
> > + pr_cont(" %4llu GB", size/SZ_1G);
> > + return;
> > + }
> > + if (size & (SZ_1T-1))
> > + pr_cont(" %4llu.%01llu TB", size/SZ_1T, 10*(size & (SZ_1T-1))/SZ_1T);
> > + else
> > + pr_cont(" %4llu TB", size/SZ_1T);
> > +}
>
> Don't you want to use string_helpers.h provided API?
> string_get_size().
I don't think string_get_size() knows the fine distinction between:
BIOS-e820: [mem 0x00000000fffc0000-0x00000000ffffffff] 256 KB device reserved
and:
BIOS-e820: [mem 0x00000000fffc0000-0x00000000ffffffff] 256.0 KB device reserved
"256 KB" is exactly 256 KB, while "256.0 KB" denotes a value that is a
bit larger than 256 KB but rounds down to 256 KB at 1 KB granularity.
When reading platform boot logs it's useful to know when such values
are exact, at a glance.
Thanks,
Ingo
====================>
arch/x86/kernel/e820.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 7f600d32a999..67a477203c97 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -213,22 +213,19 @@ static void e820_print_size(u64 size)
else
pr_cont(" %4llu KB", size/SZ_1K);
return;
- }
- if (size < SZ_1G) {
+ } else if (size < SZ_1G) {
if (size & (SZ_1M-1))
pr_cont(" %4llu.%01llu MB", size/SZ_1M, 10*(size & (SZ_1M-1))/SZ_1M);
else
pr_cont(" %4llu MB", size/SZ_1M);
return;
- }
- if (size < SZ_1T) {
+ } else if (size < SZ_1T) {
if (size & (SZ_1G-1))
pr_cont(" %4llu.%01llu GB", size/SZ_1G, 10*(size & (SZ_1G-1))/SZ_1G);
else
pr_cont(" %4llu GB", size/SZ_1G);
return;
- }
- if (size & (SZ_1T-1))
+ } else if (size & (SZ_1T-1))
pr_cont(" %4llu.%01llu TB", size/SZ_1T, 10*(size & (SZ_1T-1))/SZ_1T);
else
pr_cont(" %4llu TB", size/SZ_1T);
Powered by blists - more mailing lists