linux-kernel - Re: [PATCH v1 2/2] x86/efi: print size and base in binary units in efi_print

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: <CAHp75VfNttKVfVMdNO6EGcBiQmL+uY6QUEU1u4FV9Yi=3o=OGQ@mail.gmail.com>
Date:	Tue, 19 Jan 2016 09:32:26 +0200
From:	Andy Shevchenko <andy.shevchenko@...il.com>
To:	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:	Robert Elliott <elliott@....com>,
	Matt Fleming <matt@...eblueprint.co.uk>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-efi@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1 2/2] x86/efi: print size and base in binary units in efi_print_memmap

On Tue, Jan 12, 2016 at 3:13 PM, Andy Shevchenko
<andriy.shevchenko@...ux.intel.com> wrote:
> From: Robert Elliott <elliott@....com>
>
> Print the base address for each range in decimal alongside the size.
> Use a "(size @ base)" format similar to the fake_memmap kernel parameter.
>
> Print the range and base in the best-fit B, KiB, MiB, etc. units rather than
> always MiB. This avoids rounding, which can be misleading.
>
> Use proper IEC binary units (KiB, MiB, etc.) rather than misuse SI
> decimal units (KB, MB, etc.).
>
> old:
>     efi: mem61: [Persistent Memory  |   |  |  |  |  |  |   |WB|WT|WC|UC] range=[0x0000000880000000-0x0000000c7fffffff) (16384MB)
>
> new:
>     efi: mem61: [Persistent Memory  |   |  |  |  |  |  |   |WB|WT|WC|UC] range=[0x0000000880000000-0x0000000c7fffffff] (16 GiB @ 34 GiB)
>
> Signed-off-by: Robert Elliott <elliott@....com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>

Matt, seems another approach is not in favour, which means we rather
stick with this one. But see one comment below.

> ---
>  arch/x86/platform/efi/efi.c | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> index e0846b5..e762969 100644
> --- a/arch/x86/platform/efi/efi.c
> +++ b/arch/x86/platform/efi/efi.c
> @@ -35,10 +35,12 @@
>  #include <linux/efi.h>
>  #include <linux/efi-bgrt.h>
>  #include <linux/export.h>
> +#include <linux/bitops.h>
>  #include <linux/bootmem.h>
>  #include <linux/slab.h>
>  #include <linux/memblock.h>
>  #include <linux/spinlock.h>
> +#include <linux/string_helpers.h>
>  #include <linux/uaccess.h>
>  #include <linux/time.h>
>  #include <linux/io.h>
> @@ -222,6 +224,19 @@ int __init efi_memblock_x86_reserve_range(void)
>         return 0;
>  }
>
> +static char * __init efi_size_format(char *buf, size_t size, u64 bytes)
> +{
> +       unsigned long i;
> +
> +       if (bytes)
> +               i = __ffs64(bytes) / 10;
> +       else
> +               i = 0;
> +
> +       snprintf(buf, size, "%llu%s", bytes >> (i * 10), string_units_2[i]);

Here is missed space, should be: "%llu %s"

> +       return buf;
> +}
> +
>  void __init efi_print_memmap(void)
>  {
>  #ifdef EFI_DEBUG
> @@ -232,14 +247,16 @@ void __init efi_print_memmap(void)
>         for (p = memmap.map, i = 0;
>              p < memmap.map_end;
>              p += memmap.desc_size, i++) {
> -               char buf[64];
> +               char buf[64], buf2[32], buf3[32];
>
>                 md = p;
> -               pr_info("mem%02u: %s range=[0x%016llx-0x%016llx] (%lluMB)\n",
> +               pr_info("mem%02u: %s range=[0x%016llx-0x%016llx] (%s @ %s)\n",
>                         i, efi_md_typeattr_format(buf, sizeof(buf), md),
>                         md->phys_addr,
>                         md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT) - 1,
> -                       (md->num_pages >> (20 - EFI_PAGE_SHIFT)));
> +                       efi_size_format(buf3, sizeof(buf3),
> +                               md->num_pages << EFI_PAGE_SHIFT),
> +                       efi_size_format(buf2, sizeof(buf2), md->phys_addr));
>         }
>  #endif  /*  EFI_DEBUG  */
>  }
> --
> 2.6.4
>



-- 
With Best Regards,
Andy Shevchenko