Re: [PATCH v1 2/2] x86/efi: print size and base in binary units in efi_print_memmap

[Andy Shevchenko]

On Tue, Jan 12, 2016 at 3:13 PM, Andy Shevchenko
<andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> From: Robert Elliott <elliott@xxxxxxx>
>
> 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@xxxxxxx>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>

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