* [PATCH] x86, efi: print debug values in Kib not MB
@ 2014-07-29 17:09 Prarit Bhargava
[not found] ` <1406653761-3884-1-git-send-email-prarit-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
0 siblings, 1 reply; 22+ messages in thread
From: Prarit Bhargava @ 2014-07-29 17:09 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Prarit Bhargava, lszubowi-H+wXaHxf7aLQT0dZR+AlfA, Matt Fleming,
Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
x86-DgEjT+Ai2ygdnm+yROfE0A, linux-efi-u79uwXL29TY76Z2rM5mHXA
The current debug print in EFI does
[ 0.000000] efi: mem84: type=3, attr=0xf, range=[0x00000000645b5000-0x00000000645fb000) (0MB)
and rounds off the size to 0MB and isn't very useful. We should print this in
Kib. After applying this patch we get better info with
[ 0.000000] efi: mem84: type=3, attr=0xf, range=[0x00000000645b5000-0x00000000645fb000) (280kiB)
Signed-off-by: Prarit Bhargava <prarit-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: lszubowi-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
Cc: Matt Fleming <matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
Cc: Ingo Molnar <mingo-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: "H. Peter Anvin" <hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>
Cc: x86-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
arch/x86/platform/efi/efi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 87fc96b..3875090 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -384,10 +384,10 @@ static void __init print_efi_memmap(void)
p < memmap.map_end;
p += memmap.desc_size, i++) {
md = p;
- pr_info("mem%02u: type=%u, attr=0x%llx, range=[0x%016llx-0x%016llx) (%lluMB)\n",
+ pr_info("mem%02u: type=%u, attr=0x%llx, range=[0x%016llx-0x%016llx) (%lluKiB)\n",
i, md->type, md->attribute, md->phys_addr,
md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT),
- (md->num_pages >> (20 - EFI_PAGE_SHIFT)));
+ (md->num_pages << 2));
}
#endif /* EFI_DEBUG */
}
--
1.7.9.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] x86, efi: print debug values in Kib not MB
[not found] ` <1406653761-3884-1-git-send-email-prarit-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2014-07-29 20:29 ` David Rientjes
2014-07-29 22:02 ` Joe Perches
[not found] ` <alpine.DEB.2.02.1407291327450.13073-X6Q0R45D7oAcqpCFd4KODRPsWskHk0ljAL8bYrjMMd8@public.gmane.org>
2014-07-29 22:29 ` Borislav Petkov
1 sibling, 2 replies; 22+ messages in thread
From: David Rientjes @ 2014-07-29 20:29 UTC (permalink / raw)
To: Prarit Bhargava
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
lszubowi-H+wXaHxf7aLQT0dZR+AlfA, Matt Fleming, Thomas Gleixner,
Ingo Molnar, H. Peter Anvin, x86-DgEjT+Ai2ygdnm+yROfE0A,
linux-efi-u79uwXL29TY76Z2rM5mHXA
On Tue, 29 Jul 2014, Prarit Bhargava wrote:
> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> index 87fc96b..3875090 100644
> --- a/arch/x86/platform/efi/efi.c
> +++ b/arch/x86/platform/efi/efi.c
> @@ -384,10 +384,10 @@ static void __init print_efi_memmap(void)
> p < memmap.map_end;
> p += memmap.desc_size, i++) {
> md = p;
> - pr_info("mem%02u: type=%u, attr=0x%llx, range=[0x%016llx-0x%016llx) (%lluMB)\n",
> + pr_info("mem%02u: type=%u, attr=0x%llx, range=[0x%016llx-0x%016llx) (%lluKiB)\n",
> i, md->type, md->attribute, md->phys_addr,
> md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT),
> - (md->num_pages >> (20 - EFI_PAGE_SHIFT)));
> + (md->num_pages << 2));
> }
> #endif /* EFI_DEBUG */
> }
If EFI_PAGE_SHIFT were to ever change from its hard-coded 12, then this
would be wrong. Any reason to not simply print EFI_PAGE_SIZE and
md->num_pages?
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86, efi: print debug values in Kib not MB
2014-07-29 20:29 ` David Rientjes
@ 2014-07-29 22:02 ` Joe Perches
2014-07-29 22:05 ` Prarit Bhargava
[not found] ` <alpine.DEB.2.02.1407291327450.13073-X6Q0R45D7oAcqpCFd4KODRPsWskHk0ljAL8bYrjMMd8@public.gmane.org>
1 sibling, 1 reply; 22+ messages in thread
From: Joe Perches @ 2014-07-29 22:02 UTC (permalink / raw)
To: David Rientjes
Cc: Prarit Bhargava, linux-kernel, lszubowi, Matt Fleming,
Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-efi
On Tue, 2014-07-29 at 13:29 -0700, David Rientjes wrote:
> On Tue, 29 Jul 2014, Prarit Bhargava wrote:
> > diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
[]
> > @@ -384,10 +384,10 @@ static void __init print_efi_memmap(void)
> > p < memmap.map_end;
> > p += memmap.desc_size, i++) {
> > md = p;
> > - pr_info("mem%02u: type=%u, attr=0x%llx, range=[0x%016llx-0x%016llx) (%lluMB)\n",
> > + pr_info("mem%02u: type=%u, attr=0x%llx, range=[0x%016llx-0x%016llx) (%lluKiB)\n",
> > i, md->type, md->attribute, md->phys_addr,
> > md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT),
> > - (md->num_pages >> (20 - EFI_PAGE_SHIFT)));
> > + (md->num_pages << 2));
> > }
> > #endif /* EFI_DEBUG */
> > }
>
> If EFI_PAGE_SHIFT were to ever change from its hard-coded 12, then( this
> would be wrong. Any reason to not simply print EFI_PAGE_SIZE and
> md->num_pages?
Or maybe ((u64)EFI_PAGE_SIZE * md->num_pages) >> 10
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86, efi: print debug values in Kib not MB
[not found] ` <alpine.DEB.2.02.1407291327450.13073-X6Q0R45D7oAcqpCFd4KODRPsWskHk0ljAL8bYrjMMd8@public.gmane.org>
@ 2014-07-29 22:05 ` Prarit Bhargava
0 siblings, 0 replies; 22+ messages in thread
From: Prarit Bhargava @ 2014-07-29 22:05 UTC (permalink / raw)
To: David Rientjes
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
lszubowi-H+wXaHxf7aLQT0dZR+AlfA, Matt Fleming, Thomas Gleixner,
Ingo Molnar, H. Peter Anvin, x86-DgEjT+Ai2ygdnm+yROfE0A,
linux-efi-u79uwXL29TY76Z2rM5mHXA
On 07/29/2014 04:29 PM, David Rientjes wrote:
> On Tue, 29 Jul 2014, Prarit Bhargava wrote:
>
>> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
>> index 87fc96b..3875090 100644
>> --- a/arch/x86/platform/efi/efi.c
>> +++ b/arch/x86/platform/efi/efi.c
>> @@ -384,10 +384,10 @@ static void __init print_efi_memmap(void)
>> p < memmap.map_end;
>> p += memmap.desc_size, i++) {
>> md = p;
>> - pr_info("mem%02u: type=%u, attr=0x%llx, range=[0x%016llx-0x%016llx) (%lluMB)\n",
>> + pr_info("mem%02u: type=%u, attr=0x%llx, range=[0x%016llx-0x%016llx) (%lluKiB)\n",
>> i, md->type, md->attribute, md->phys_addr,
>> md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT),
>> - (md->num_pages >> (20 - EFI_PAGE_SHIFT)));
>> + (md->num_pages << 2));
>> }
>> #endif /* EFI_DEBUG */
>> }
>
> If EFI_PAGE_SHIFT were to ever change from its hard-coded 12, then this
> would be wrong. Any reason to not simply print EFI_PAGE_SIZE and
> md->num_pages?
Hmm ... maybe it might be better to print out (md->num_pages * EFI_PAGE_SIZE) >>
10 ? I'll just double check that in a kernel and repost.
Thanks for the suggestion David :)
P.
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86, efi: print debug values in Kib not MB
2014-07-29 22:02 ` Joe Perches
@ 2014-07-29 22:05 ` Prarit Bhargava
0 siblings, 0 replies; 22+ messages in thread
From: Prarit Bhargava @ 2014-07-29 22:05 UTC (permalink / raw)
To: Joe Perches
Cc: David Rientjes, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
lszubowi-H+wXaHxf7aLQT0dZR+AlfA, Matt Fleming, Thomas Gleixner,
Ingo Molnar, H. Peter Anvin, x86-DgEjT+Ai2ygdnm+yROfE0A,
linux-efi-u79uwXL29TY76Z2rM5mHXA
On 07/29/2014 06:02 PM, Joe Perches wrote:
> On Tue, 2014-07-29 at 13:29 -0700, David Rientjes wrote:
>> On Tue, 29 Jul 2014, Prarit Bhargava wrote:
>>> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> []
>>> @@ -384,10 +384,10 @@ static void __init print_efi_memmap(void)
>>> p < memmap.map_end;
>>> p += memmap.desc_size, i++) {
>>> md = p;
>>> - pr_info("mem%02u: type=%u, attr=0x%llx, range=[0x%016llx-0x%016llx) (%lluMB)\n",
>>> + pr_info("mem%02u: type=%u, attr=0x%llx, range=[0x%016llx-0x%016llx) (%lluKiB)\n",
>>> i, md->type, md->attribute, md->phys_addr,
>>> md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT),
>>> - (md->num_pages >> (20 - EFI_PAGE_SHIFT)));
>>> + (md->num_pages << 2));
>>> }
>>> #endif /* EFI_DEBUG */
>>> }
>>
>> If EFI_PAGE_SHIFT were to ever change from its hard-coded 12, then( this
>> would be wrong. Any reason to not simply print EFI_PAGE_SIZE and
>> md->num_pages?
>
> Or maybe ((u64)EFI_PAGE_SIZE * md->num_pages) >> 10
Heh :) See my just sent email. I'll give this a shot :)
P.
>
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86, efi: print debug values in Kib not MB
[not found] ` <1406653761-3884-1-git-send-email-prarit-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2014-07-29 20:29 ` David Rientjes
@ 2014-07-29 22:29 ` Borislav Petkov
[not found] ` <20140729222932.GA17481-fF5Pk5pvG8Y@public.gmane.org>
1 sibling, 1 reply; 22+ messages in thread
From: Borislav Petkov @ 2014-07-29 22:29 UTC (permalink / raw)
To: Prarit Bhargava
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
lszubowi-H+wXaHxf7aLQT0dZR+AlfA, Matt Fleming, Thomas Gleixner,
Ingo Molnar, H. Peter Anvin, x86-DgEjT+Ai2ygdnm+yROfE0A,
linux-efi-u79uwXL29TY76Z2rM5mHXA
On Tue, Jul 29, 2014 at 01:09:21PM -0400, Prarit Bhargava wrote:
> The current debug print in EFI does
>
> [ 0.000000] efi: mem84: type=3, attr=0xf, range=[0x00000000645b5000-0x00000000645fb000) (0MB)
>
> and rounds off the size to 0MB and isn't very useful. We should print this in
> Kib. After applying this patch we get better info with
>
> [ 0.000000] efi: mem84: type=3, attr=0xf, range=[0x00000000645b5000-0x00000000645fb000) (280kiB)
Turning this into kiB unconditionally won't always work ok:
First of all, there might be something which parses that output so I'd
make sure I'm not breaking that. Maybe fwts... Matt will know.
Then, I have an UEFI region which is > 13G:
[ 0.000000] efi: mem42: type=7, attr=0xf, range=[0x0000000100000000-0x0000000450000000) (13568MB)
With your patch it is even worse:
[ 0.000000] efi: mem42: type=7, attr=0xf, range=[0x0000000100000000-0x0000000450000000) (13893632kiB)
I'd guess you'll have to go all out and do this properly. :-)
Something like showing the highest unit which is still > 1. In the above
case, this should be (13GB) or maybe even introduce fractions.
Depends on how involved this output should be.
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86, efi: print debug values in Kib not MB
[not found] ` <20140729222932.GA17481-fF5Pk5pvG8Y@public.gmane.org>
@ 2014-07-29 22:32 ` Prarit Bhargava
[not found] ` <53D82118.6090202-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2014-07-30 14:48 ` Matt Fleming
1 sibling, 1 reply; 22+ messages in thread
From: Prarit Bhargava @ 2014-07-29 22:32 UTC (permalink / raw)
To: Borislav Petkov
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
lszubowi-H+wXaHxf7aLQT0dZR+AlfA, Matt Fleming, Thomas Gleixner,
Ingo Molnar, H. Peter Anvin, x86-DgEjT+Ai2ygdnm+yROfE0A,
linux-efi-u79uwXL29TY76Z2rM5mHXA
On 07/29/2014 06:29 PM, Borislav Petkov wrote:
> On Tue, Jul 29, 2014 at 01:09:21PM -0400, Prarit Bhargava wrote:
>> The current debug print in EFI does
>>
>> [ 0.000000] efi: mem84: type=3, attr=0xf, range=[0x00000000645b5000-0x00000000645fb000) (0MB)
>>
>> and rounds off the size to 0MB and isn't very useful. We should print this in
>> Kib. After applying this patch we get better info with
>>
>> [ 0.000000] efi: mem84: type=3, attr=0xf, range=[0x00000000645b5000-0x00000000645fb000) (280kiB)
>
> Turning this into kiB unconditionally won't always work ok:
>
> First of all, there might be something which parses that output so I'd
> make sure I'm not breaking that. Maybe fwts... Matt will know.
>
> Then, I have an UEFI region which is > 13G:
>
> [ 0.000000] efi: mem42: type=7, attr=0xf, range=[0x0000000100000000-0x0000000450000000) (13568MB)
>
>
> With your patch it is even worse:
>
> [ 0.000000] efi: mem42: type=7, attr=0xf, range=[0x0000000100000000-0x0000000450000000) (13893632kiB)
>
>
> I'd guess you'll have to go all out and do this properly. :-)
>
> Something like showing the highest unit which is still > 1. In the above
> case, this should be (13GB) or maybe even introduce fractions.
>
> Depends on how involved this output should be.
Yeah, I thought about doing something like that but figured if I'm at this level
I can do some math ;), and it was best to keep the code simple with a KiB.
Matt -- will anything be broken here if we change the output?
P.
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86, efi: print debug values in Kib not MB
[not found] ` <53D82118.6090202-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2014-07-29 22:36 ` Borislav Petkov
[not found] ` <20140729223603.GC17241-fF5Pk5pvG8Y@public.gmane.org>
0 siblings, 1 reply; 22+ messages in thread
From: Borislav Petkov @ 2014-07-29 22:36 UTC (permalink / raw)
To: Prarit Bhargava
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
lszubowi-H+wXaHxf7aLQT0dZR+AlfA, Matt Fleming, Thomas Gleixner,
Ingo Molnar, H. Peter Anvin, x86-DgEjT+Ai2ygdnm+yROfE0A,
linux-efi-u79uwXL29TY76Z2rM5mHXA
On Tue, Jul 29, 2014 at 06:32:56PM -0400, Prarit Bhargava wrote:
> and it was best to keep the code simple with a KiB.
You're missing the point - the output doesn't get simple with KiB. Read
the example I just gave you!
(13893632kiB) is actively *not* helping when one looks at it!
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86, efi: print debug values in Kib not MB
[not found] ` <20140729223603.GC17241-fF5Pk5pvG8Y@public.gmane.org>
@ 2014-07-29 22:42 ` David Rientjes
2014-07-29 23:01 ` Borislav Petkov
[not found] ` <alpine.DEB.2.02.1407291541480.20991-X6Q0R45D7oAcqpCFd4KODRPsWskHk0ljAL8bYrjMMd8@public.gmane.org>
2014-07-29 22:54 ` Prarit Bhargava
1 sibling, 2 replies; 22+ messages in thread
From: David Rientjes @ 2014-07-29 22:42 UTC (permalink / raw)
To: Borislav Petkov
Cc: Prarit Bhargava, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
lszubowi-H+wXaHxf7aLQT0dZR+AlfA, Matt Fleming, Thomas Gleixner,
Ingo Molnar, H. Peter Anvin, x86-DgEjT+Ai2ygdnm+yROfE0A,
linux-efi-u79uwXL29TY76Z2rM5mHXA
On Wed, 30 Jul 2014, Borislav Petkov wrote:
> On Tue, Jul 29, 2014 at 06:32:56PM -0400, Prarit Bhargava wrote:
> > and it was best to keep the code simple with a KiB.
>
> You're missing the point - the output doesn't get simple with KiB. Read
> the example I just gave you!
>
> (13893632kiB) is actively *not* helping when one looks at it!
>
/proc/meminfo must be frustrating then, too.
If Prarit is going to implement this suggested reverse memparse() then it
would be nice to also have it as a generic function that others can use.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86, efi: print debug values in Kib not MB
[not found] ` <20140729223603.GC17241-fF5Pk5pvG8Y@public.gmane.org>
2014-07-29 22:42 ` David Rientjes
@ 2014-07-29 22:54 ` Prarit Bhargava
[not found] ` <53D82623.4080903-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
1 sibling, 1 reply; 22+ messages in thread
From: Prarit Bhargava @ 2014-07-29 22:54 UTC (permalink / raw)
To: Borislav Petkov
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
lszubowi-H+wXaHxf7aLQT0dZR+AlfA, Matt Fleming, Thomas Gleixner,
Ingo Molnar, H. Peter Anvin, x86-DgEjT+Ai2ygdnm+yROfE0A,
linux-efi-u79uwXL29TY76Z2rM5mHXA
On 07/29/2014 06:36 PM, Borislav Petkov wrote:
> On Tue, Jul 29, 2014 at 06:32:56PM -0400, Prarit Bhargava wrote:
>> and it was best to keep the code simple with a KiB.
>
> You're missing the point - the output doesn't get simple with KiB. Read
> the example I just gave you!
>
> (13893632kiB) is actively *not* helping when one looks at it!
I did get your point and I'm (politely) disagreeing with it. Your case ONLY
works if the number is _exactly_ 13GB. What if it is 13.5? Then we're still
rounding off and reporting 14GB. Which, if this code is meant for debug, makes
no sense to me. Why round it off?
P.
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86, efi: print debug values in Kib not MB
2014-07-29 22:42 ` David Rientjes
@ 2014-07-29 23:01 ` Borislav Petkov
[not found] ` <20140729230102.GD17241-fF5Pk5pvG8Y@public.gmane.org>
[not found] ` <alpine.DEB.2.02.1407291541480.20991-X6Q0R45D7oAcqpCFd4KODRPsWskHk0ljAL8bYrjMMd8@public.gmane.org>
1 sibling, 1 reply; 22+ messages in thread
From: Borislav Petkov @ 2014-07-29 23:01 UTC (permalink / raw)
To: David Rientjes
Cc: Prarit Bhargava, linux-kernel, lszubowi, Matt Fleming,
Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-efi
On Tue, Jul 29, 2014 at 03:42:55PM -0700, David Rientjes wrote:
> If Prarit is going to implement this suggested reverse memparse() ...
The only meaningful argument about the formatting here IMO is what
people staring at this output are going to understand from it.
And since those people most often tend to be the poor souls dealing with
UEFI, we should ask them whether this needs fixing and if so, how.
That's it.
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86, efi: print debug values in Kib not MB
[not found] ` <20140729230102.GD17241-fF5Pk5pvG8Y@public.gmane.org>
@ 2014-07-29 23:03 ` Prarit Bhargava
0 siblings, 0 replies; 22+ messages in thread
From: Prarit Bhargava @ 2014-07-29 23:03 UTC (permalink / raw)
To: Borislav Petkov
Cc: David Rientjes, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
lszubowi-H+wXaHxf7aLQT0dZR+AlfA, Matt Fleming, Thomas Gleixner,
Ingo Molnar, H. Peter Anvin, x86-DgEjT+Ai2ygdnm+yROfE0A,
linux-efi-u79uwXL29TY76Z2rM5mHXA
On 07/29/2014 07:01 PM, Borislav Petkov wrote:
> On Tue, Jul 29, 2014 at 03:42:55PM -0700, David Rientjes wrote:
>> If Prarit is going to implement this suggested reverse memparse() ...
>
> The only meaningful argument about the formatting here IMO is what
> people staring at this output are going to understand from it.
>
> And since those people most often tend to be the poor souls dealing with
> UEFI, we should ask them whether this needs fixing and if so, how.
>
... I did do a get_maintainers.pl on this before sending it. Did I miss someone
or some list? :(
P.
> That's it.
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86, efi: print debug values in Kib not MB
[not found] ` <alpine.DEB.2.02.1407291541480.20991-X6Q0R45D7oAcqpCFd4KODRPsWskHk0ljAL8bYrjMMd8@public.gmane.org>
@ 2014-07-29 23:07 ` Joe Perches
2014-07-29 23:44 ` David Rientjes
0 siblings, 1 reply; 22+ messages in thread
From: Joe Perches @ 2014-07-29 23:07 UTC (permalink / raw)
To: David Rientjes
Cc: Borislav Petkov, Prarit Bhargava,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
lszubowi-H+wXaHxf7aLQT0dZR+AlfA, Matt Fleming, Thomas Gleixner,
Ingo Molnar, H. Peter Anvin, x86-DgEjT+Ai2ygdnm+yROfE0A,
linux-efi-u79uwXL29TY76Z2rM5mHXA
On Tue, 2014-07-29 at 15:42 -0700, David Rientjes wrote:
> On Wed, 30 Jul 2014, Borislav Petkov wrote:
>
> > On Tue, Jul 29, 2014 at 06:32:56PM -0400, Prarit Bhargava wrote:
> > > and it was best to keep the code simple with a KiB.
> >
> > You're missing the point - the output doesn't get simple with KiB. Read
> > the example I just gave you!
> >
> > (13893632kiB) is actively *not* helping when one looks at it!
> >
>
> /proc/meminfo must be frustrating then, too.
>
> If Prarit is going to implement this suggested reverse memparse() then it
> would be nice to also have it as a generic function that others can use.
Maybe yet another vsprintf extension?
Maybe %pH<vartype> where vartype is one of [hh, h, u, ul, ull]
with something like
u64 t1 = (u64)*(appropriate cast)vartype;
u64 t2 = t1;
int index = 0;
while ((t1 >>= 10)) {
index++;
t2 >>= 10;
}
to output the equivalent of
%llu%s, t2, "bkmgtpezy"[index]
ala dump_pagetables
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86, efi: print debug values in Kib not MB
2014-07-29 23:07 ` Joe Perches
@ 2014-07-29 23:44 ` David Rientjes
0 siblings, 0 replies; 22+ messages in thread
From: David Rientjes @ 2014-07-29 23:44 UTC (permalink / raw)
To: Joe Perches
Cc: Borislav Petkov, Prarit Bhargava,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
lszubowi-H+wXaHxf7aLQT0dZR+AlfA, Matt Fleming, Thomas Gleixner,
Ingo Molnar, H. Peter Anvin, x86-DgEjT+Ai2ygdnm+yROfE0A,
linux-efi-u79uwXL29TY76Z2rM5mHXA
On Tue, 29 Jul 2014, Joe Perches wrote:
> Maybe yet another vsprintf extension?
>
> Maybe %pH<vartype> where vartype is one of [hh, h, u, ul, ull]
> with something like
> u64 t1 = (u64)*(appropriate cast)vartype;
> u64 t2 = t1;
> int index = 0;
> while ((t1 >>= 10)) {
> index++;
> t2 >>= 10;
> }
>
> to output the equivalent of
> %llu%s, t2, "bkmgtpezy"[index]
> ala dump_pagetables
>
Prarit has a good point about unnecessarily rounding the value, which is
exactly what the patch is addressing, and it wouldn't be good to be
changing the units depending on the value if anybody is using it for
anything other than reading it with their own two eyes. Since
EFI_PAGE_SIZE will always be in KB, it makes sense to export it in KB.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86, efi: print debug values in Kib not MB
[not found] ` <53D82623.4080903-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2014-07-30 3:25 ` Steven Noonan
0 siblings, 0 replies; 22+ messages in thread
From: Steven Noonan @ 2014-07-30 3:25 UTC (permalink / raw)
To: Prarit Bhargava
Cc: Borislav Petkov, Linux Kernel mailing List,
lszubowi-H+wXaHxf7aLQT0dZR+AlfA, Matt Fleming, Thomas Gleixner,
Ingo Molnar, H. Peter Anvin, Linux-X86,
linux-efi-u79uwXL29TY76Z2rM5mHXA
On Tue, Jul 29, 2014 at 3:54 PM, Prarit Bhargava <prarit-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>
>
> On 07/29/2014 06:36 PM, Borislav Petkov wrote:
>> On Tue, Jul 29, 2014 at 06:32:56PM -0400, Prarit Bhargava wrote:
>>> and it was best to keep the code simple with a KiB.
>>
>> You're missing the point - the output doesn't get simple with KiB. Read
>> the example I just gave you!
>>
>> (13893632kiB) is actively *not* helping when one looks at it!
>
> I did get your point and I'm (politely) disagreeing with it. Your case ONLY
> works if the number is _exactly_ 13GB. What if it is 13.5? Then we're still
> rounding off and reporting 14GB. Which, if this code is meant for debug, makes
> no sense to me. Why round it off?
>
I think if it was being represented in procfs or sysfs, we'd need to
be extremely specific to make it machine-readable, but for a
human-readable printk, it makes sense to me to print it in the smaller
unit size until the value is in tens of the next higher unit size
(e.g. print in KiB until 10+MiB, print in MiB until 10+GiB).
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86, efi: print debug values in Kib not MB
[not found] ` <20140729222932.GA17481-fF5Pk5pvG8Y@public.gmane.org>
2014-07-29 22:32 ` Prarit Bhargava
@ 2014-07-30 14:48 ` Matt Fleming
[not found] ` <20140730144803.GB15082-HNK1S37rvNbeXh+fF434Mdi2O/JbrIOy@public.gmane.org>
1 sibling, 1 reply; 22+ messages in thread
From: Matt Fleming @ 2014-07-30 14:48 UTC (permalink / raw)
To: Borislav Petkov
Cc: Prarit Bhargava, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
lszubowi-H+wXaHxf7aLQT0dZR+AlfA, Matt Fleming, Thomas Gleixner,
Ingo Molnar, H. Peter Anvin, x86-DgEjT+Ai2ygdnm+yROfE0A,
linux-efi-u79uwXL29TY76Z2rM5mHXA, Colin Ian King
On Wed, 30 Jul, at 12:29:32AM, Borislav Petkov wrote:
> On Tue, Jul 29, 2014 at 01:09:21PM -0400, Prarit Bhargava wrote:
> > The current debug print in EFI does
> >
> > [ 0.000000] efi: mem84: type=3, attr=0xf, range=[0x00000000645b5000-0x00000000645fb000) (0MB)
> >
> > and rounds off the size to 0MB and isn't very useful. We should print this in
> > Kib. After applying this patch we get better info with
> >
> > [ 0.000000] efi: mem84: type=3, attr=0xf, range=[0x00000000645b5000-0x00000000645fb000) (280kiB)
>
> Turning this into kiB unconditionally won't always work ok:
>
> First of all, there might be something which parses that output so I'd
> make sure I'm not breaking that. Maybe fwts... Matt will know.
I'm not aware of anything that parses the dmesg output, but I'm
including Colin in case he has any insight.
--
Matt Fleming, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86, efi: print debug values in Kib not MB
[not found] ` <20140730144803.GB15082-HNK1S37rvNbeXh+fF434Mdi2O/JbrIOy@public.gmane.org>
@ 2014-07-30 15:05 ` Colin Ian King
[not found] ` <53D909C3.9050205-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
0 siblings, 1 reply; 22+ messages in thread
From: Colin Ian King @ 2014-07-30 15:05 UTC (permalink / raw)
To: Matt Fleming, Borislav Petkov
Cc: Prarit Bhargava, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
lszubowi-H+wXaHxf7aLQT0dZR+AlfA, Matt Fleming, Thomas Gleixner,
Ingo Molnar, H. Peter Anvin, x86-DgEjT+Ai2ygdnm+yROfE0A,
linux-efi-u79uwXL29TY76Z2rM5mHXA
On 30/07/14 15:48, Matt Fleming wrote:
> On Wed, 30 Jul, at 12:29:32AM, Borislav Petkov wrote:
>> On Tue, Jul 29, 2014 at 01:09:21PM -0400, Prarit Bhargava wrote:
>>> The current debug print in EFI does
>>>
>>> [ 0.000000] efi: mem84: type=3, attr=0xf, range=[0x00000000645b5000-0x00000000645fb000) (0MB)
>>>
>>> and rounds off the size to 0MB and isn't very useful. We should print this in
>>> Kib. After applying this patch we get better info with
>>>
>>> [ 0.000000] efi: mem84: type=3, attr=0xf, range=[0x00000000645b5000-0x00000000645fb000) (280kiB)
>>
>> Turning this into kiB unconditionally won't always work ok:
>>
>> First of all, there might be something which parses that output so I'd
>> make sure I'm not breaking that. Maybe fwts... Matt will know.
>
> I'm not aware of anything that parses the dmesg output, but I'm
> including Colin in case he has any insight.
>
That won't break fwts.
Colin
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86, efi: print debug values in Kib not MB
[not found] ` <53D909C3.9050205-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
@ 2014-07-30 17:04 ` Prarit Bhargava
2014-07-30 17:10 ` Randy Dunlap
0 siblings, 1 reply; 22+ messages in thread
From: Prarit Bhargava @ 2014-07-30 17:04 UTC (permalink / raw)
To: Matt Fleming
Cc: Colin Ian King, Borislav Petkov,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
lszubowi-H+wXaHxf7aLQT0dZR+AlfA, Matt Fleming, Thomas Gleixner,
Ingo Molnar, H. Peter Anvin, x86-DgEjT+Ai2ygdnm+yROfE0A,
linux-efi-u79uwXL29TY76Z2rM5mHXA
On 07/30/2014 11:05 AM, Colin Ian King wrote:
> On 30/07/14 15:48, Matt Fleming wrote:
>> On Wed, 30 Jul, at 12:29:32AM, Borislav Petkov wrote:
>>> On Tue, Jul 29, 2014 at 01:09:21PM -0400, Prarit Bhargava wrote:
>>>> The current debug print in EFI does
>>>>
>>>> [ 0.000000] efi: mem84: type=3, attr=0xf, range=[0x00000000645b5000-0x00000000645fb000) (0MB)
>>>>
>>>> and rounds off the size to 0MB and isn't very useful. We should print this in
>>>> Kib. After applying this patch we get better info with
>>>>
>>>> [ 0.000000] efi: mem84: type=3, attr=0xf, range=[0x00000000645b5000-0x00000000645fb000) (280kiB)
>>>
>>> Turning this into kiB unconditionally won't always work ok:
>>>
>>> First of all, there might be something which parses that output so I'd
>>> make sure I'm not breaking that. Maybe fwts... Matt will know.
>>
>> I'm not aware of anything that parses the dmesg output, but I'm
>> including Colin in case he has any insight.
>>
> That won't break fwts.
Matt -- could you make a decision on whether or not this should be strictly in
KiB or if it should be a rolling KiB, Mib, and GiB?
My issue is simply that I'd like to know *exactly* how big each range is and not
have it rounded off. Others think that it is easier to read Mib & Gib ...
P.
>
> Colin
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86, efi: print debug values in Kib not MB
2014-07-30 17:04 ` Prarit Bhargava
@ 2014-07-30 17:10 ` Randy Dunlap
[not found] ` <53D92700.3040803-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
0 siblings, 1 reply; 22+ messages in thread
From: Randy Dunlap @ 2014-07-30 17:10 UTC (permalink / raw)
To: Prarit Bhargava, Matt Fleming
Cc: Colin Ian King, Borislav Petkov, linux-kernel, lszubowi,
Matt Fleming, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86,
linux-efi
On 07/30/14 10:04, Prarit Bhargava wrote:
>
>
> On 07/30/2014 11:05 AM, Colin Ian King wrote:
>> On 30/07/14 15:48, Matt Fleming wrote:
>>> On Wed, 30 Jul, at 12:29:32AM, Borislav Petkov wrote:
>>>> On Tue, Jul 29, 2014 at 01:09:21PM -0400, Prarit Bhargava wrote:
>>>>> The current debug print in EFI does
>>>>>
>>>>> [ 0.000000] efi: mem84: type=3, attr=0xf, range=[0x00000000645b5000-0x00000000645fb000) (0MB)
>>>>>
>>>>> and rounds off the size to 0MB and isn't very useful. We should print this in
>>>>> Kib. After applying this patch we get better info with
>>>>>
>>>>> [ 0.000000] efi: mem84: type=3, attr=0xf, range=[0x00000000645b5000-0x00000000645fb000) (280kiB)
>>>>
>>>> Turning this into kiB unconditionally won't always work ok:
>>>>
>>>> First of all, there might be something which parses that output so I'd
>>>> make sure I'm not breaking that. Maybe fwts... Matt will know.
>>>
>>> I'm not aware of anything that parses the dmesg output, but I'm
>>> including Colin in case he has any insight.
>>>
>> That won't break fwts.
>
> Matt -- could you make a decision on whether or not this should be strictly in
> KiB or if it should be a rolling KiB, Mib, and GiB?
>
> My issue is simply that I'd like to know *exactly* how big each range is and not
> have it rounded off. Others think that it is easier to read Mib & Gib ...
It is easier to read in MiB or GiB IMHO, but since it is debug info,
exact is good. It would be even better [more readable] to use commas
(or periods/dots) to separate it into groups of 3 digits.
--
~Randy
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86, efi: print debug values in Kib not MB
[not found] ` <53D92700.3040803-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
@ 2014-07-30 17:21 ` H. Peter Anvin
[not found] ` <53D92996.1050906-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>
0 siblings, 1 reply; 22+ messages in thread
From: H. Peter Anvin @ 2014-07-30 17:21 UTC (permalink / raw)
To: Randy Dunlap, Prarit Bhargava, Matt Fleming
Cc: Colin Ian King, Borislav Petkov,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
lszubowi-H+wXaHxf7aLQT0dZR+AlfA, Matt Fleming, Thomas Gleixner,
Ingo Molnar, x86-DgEjT+Ai2ygdnm+yROfE0A,
linux-efi-u79uwXL29TY76Z2rM5mHXA
On 07/30/2014 10:10 AM, Randy Dunlap wrote:
>>
>> My issue is simply that I'd like to know *exactly* how big each range is and not
>> have it rounded off. Others think that it is easier to read Mib & Gib ...
>
> It is easier to read in MiB or GiB IMHO, but since it is debug info,
> exact is good. It would be even better [more readable] to use commas
> (or periods/dots) to separate it into groups of 3 digits.
>
Arguably the exactness is available in the range...
-hpa
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86, efi: print debug values in Kib not MB
[not found] ` <53D92996.1050906-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>
@ 2014-07-30 19:58 ` Borislav Petkov
2014-07-30 20:43 ` Randy Dunlap
0 siblings, 1 reply; 22+ messages in thread
From: Borislav Petkov @ 2014-07-30 19:58 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Randy Dunlap, Prarit Bhargava, Matt Fleming, Colin Ian King,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
lszubowi-H+wXaHxf7aLQT0dZR+AlfA, Matt Fleming, Thomas Gleixner,
Ingo Molnar, x86-DgEjT+Ai2ygdnm+yROfE0A,
linux-efi-u79uwXL29TY76Z2rM5mHXA
On Wed, Jul 30, 2014 at 10:21:26AM -0700, H. Peter Anvin wrote:
> Arguably the exactness is available in the range...
... and the size too. FWIW, other region dumps don't even print size:
[ 0.000000] e820: BIOS-provided physical RAM map:
[ 0.000000] BIOS-e820: [mem 0x0000000000000000-0x000000000009e7ff] usable
[ 0.000000] BIOS-e820: [mem 0x000000000009e800-0x000000000009ffff] reserved
[ 0.000000] BIOS-e820: [mem 0x00000000000e0000-0x00000000000fffff] reserved
...
If we do that too for UEFI, we can certainly put an end to the
bikeshedding... provided everyone is tired of it by now. :-P
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86, efi: print debug values in Kib not MB
2014-07-30 19:58 ` Borislav Petkov
@ 2014-07-30 20:43 ` Randy Dunlap
0 siblings, 0 replies; 22+ messages in thread
From: Randy Dunlap @ 2014-07-30 20:43 UTC (permalink / raw)
To: Borislav Petkov, H. Peter Anvin
Cc: Prarit Bhargava, Matt Fleming, Colin Ian King, linux-kernel,
lszubowi, Matt Fleming, Thomas Gleixner, Ingo Molnar, x86,
linux-efi
On 07/30/14 12:58, Borislav Petkov wrote:
> On Wed, Jul 30, 2014 at 10:21:26AM -0700, H. Peter Anvin wrote:
>> Arguably the exactness is available in the range...
>
> ... and the size too. FWIW, other region dumps don't even print size:
>
> [ 0.000000] e820: BIOS-provided physical RAM map:
> [ 0.000000] BIOS-e820: [mem 0x0000000000000000-0x000000000009e7ff] usable
> [ 0.000000] BIOS-e820: [mem 0x000000000009e800-0x000000000009ffff] reserved
> [ 0.000000] BIOS-e820: [mem 0x00000000000e0000-0x00000000000fffff] reserved
> ...
>
> If we do that too for UEFI, we can certainly put an end to the
> bikeshedding... provided everyone is tired of it by now. :-P
Yes, please. (I'm tired and I barely got started ;)
--
~Randy
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2014-07-30 20:43 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-29 17:09 [PATCH] x86, efi: print debug values in Kib not MB Prarit Bhargava
[not found] ` <1406653761-3884-1-git-send-email-prarit-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2014-07-29 20:29 ` David Rientjes
2014-07-29 22:02 ` Joe Perches
2014-07-29 22:05 ` Prarit Bhargava
[not found] ` <alpine.DEB.2.02.1407291327450.13073-X6Q0R45D7oAcqpCFd4KODRPsWskHk0ljAL8bYrjMMd8@public.gmane.org>
2014-07-29 22:05 ` Prarit Bhargava
2014-07-29 22:29 ` Borislav Petkov
[not found] ` <20140729222932.GA17481-fF5Pk5pvG8Y@public.gmane.org>
2014-07-29 22:32 ` Prarit Bhargava
[not found] ` <53D82118.6090202-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2014-07-29 22:36 ` Borislav Petkov
[not found] ` <20140729223603.GC17241-fF5Pk5pvG8Y@public.gmane.org>
2014-07-29 22:42 ` David Rientjes
2014-07-29 23:01 ` Borislav Petkov
[not found] ` <20140729230102.GD17241-fF5Pk5pvG8Y@public.gmane.org>
2014-07-29 23:03 ` Prarit Bhargava
[not found] ` <alpine.DEB.2.02.1407291541480.20991-X6Q0R45D7oAcqpCFd4KODRPsWskHk0ljAL8bYrjMMd8@public.gmane.org>
2014-07-29 23:07 ` Joe Perches
2014-07-29 23:44 ` David Rientjes
2014-07-29 22:54 ` Prarit Bhargava
[not found] ` <53D82623.4080903-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2014-07-30 3:25 ` Steven Noonan
2014-07-30 14:48 ` Matt Fleming
[not found] ` <20140730144803.GB15082-HNK1S37rvNbeXh+fF434Mdi2O/JbrIOy@public.gmane.org>
2014-07-30 15:05 ` Colin Ian King
[not found] ` <53D909C3.9050205-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
2014-07-30 17:04 ` Prarit Bhargava
2014-07-30 17:10 ` Randy Dunlap
[not found] ` <53D92700.3040803-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
2014-07-30 17:21 ` H. Peter Anvin
[not found] ` <53D92996.1050906-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>
2014-07-30 19:58 ` Borislav Petkov
2014-07-30 20:43 ` Randy Dunlap
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).