[PATCH] apci: dump slit

[PATCH] apci: dump slit

[Yinghai Lu]

Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>

---
 drivers/acpi/numa.c |    8 ++++++++
 1 file changed, 8 insertions(+)

Index: linux-2.6/drivers/acpi/numa.c
===================================================================
--- linux-2.6.orig/drivers/acpi/numa.c
+++ linux-2.6/drivers/acpi/numa.c
@@ -150,6 +150,14 @@ static __init int slit_valid(struct acpi
 {
 	int i, j;
 	int d = slit->locality_count;
+	printk(KERN_DEBUG "ACPI: SLIT: nodes = %d\n", d);
+	for (i = 0; i < d; i++) {
+		for (j = 0; j < d; j++)  {
+			u8 val = slit->entry[d*i + j];
+			printk(" %d", val);
+		}
+		printk("\n");
+	}
 	for (i = 0; i < d; i++) {
 		for (j = 0; j < d; j++)  {
 			u8 val = slit->entry[d*i + j];

Re: [PATCH] apci: dump slit

[Andrew Morton]

On Fri, 12 Sep 2008 11:17:48 -0700
Yinghai Lu <yhlu.kernel@gmail.com> wrote:

> Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>
> 
> ---
>  drivers/acpi/numa.c |    8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> Index: linux-2.6/drivers/acpi/numa.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/numa.c
> +++ linux-2.6/drivers/acpi/numa.c
> @@ -150,6 +150,14 @@ static __init int slit_valid(struct acpi
>  {
>  	int i, j;
>  	int d = slit->locality_count;
> +	printk(KERN_DEBUG "ACPI: SLIT: nodes = %d\n", d);
> +	for (i = 0; i < d; i++) {
> +		for (j = 0; j < d; j++)  {
> +			u8 val = slit->entry[d*i + j];
> +			printk(" %d", val);
> +		}
> +		printk("\n");
> +	}
>  	for (i = 0; i < d; i++) {
>  		for (j = 0; j < d; j++)  {
>  			u8 val = slit->entry[d*i + j];

why?

Re: [PATCH] apci: dump slit

[Yinghai Lu]

On Fri, Sep 12, 2008 at 12:20 PM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Fri, 12 Sep 2008 11:17:48 -0700
> Yinghai Lu <yhlu.kernel@gmail.com> wrote:
>
>> Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>
>>
>> ---
>>  drivers/acpi/numa.c |    8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> Index: linux-2.6/drivers/acpi/numa.c
>> ===================================================================
>> --- linux-2.6.orig/drivers/acpi/numa.c
>> +++ linux-2.6/drivers/acpi/numa.c
>> @@ -150,6 +150,14 @@ static __init int slit_valid(struct acpi
>>  {
>>       int i, j;
>>       int d = slit->locality_count;
>> +     printk(KERN_DEBUG "ACPI: SLIT: nodes = %d\n", d);
>> +     for (i = 0; i < d; i++) {
>> +             for (j = 0; j < d; j++)  {
>> +                     u8 val = slit->entry[d*i + j];
>> +                     printk(" %d", val);
>> +             }
>> +             printk("\n");
>> +     }
>>       for (i = 0; i < d; i++) {
>>               for (j = 0; j < d; j++)  {
>>                       u8 val = slit->entry[d*i + j];
>
> why?

to see how wrong could be set by BIOS.

YH

Re: [PATCH] apci: dump slit

[H. Peter Anvin]

Yinghai Lu wrote:
> 
> to see how wrong could be set by BIOS.
> 

I'm starting to think that all this dumping of ACPI information needs to 
be put under a command-line option.

	-hpa

Re: [PATCH] apci: dump slit

[Andi Kleen]

"H. Peter Anvin" <hpa@zytor.com> writes:

> Yinghai Lu wrote:
>> to see how wrong could be set by BIOS.
>>
>
> I'm starting to think that all this dumping of ACPI information needs
> to be put under a command-line option.

The majority of ACPI information at boot is related to interrupt
routing. While interrupt routing isn't that big a problem anymore
it used to be and when something goes wrong with it you often 
have a non booting system. I think it's safer to dump
it by default.

On the other hand there's quite some other information dumped
at boot that could be cut out. e.g. It doesn't really make 
much sense to dump the cache information of each CPU at boot
when that information can easily be gotten from user space
later.

I personally was also always sceptical of some of the
more detailed new memory map dumping, like those "nosave
memory" or the early reservation/early node/add_active_range output.
It's pretty much a direct function of the e820 map dumped earlier.
Also node information has now the dubious distinction to be 
the most redundantly dumped kernel information of all.

-Andi

---
ak@linux.intel.com

Re: [PATCH] apci: dump slit

[Thomas Gleixner]

On Fri, 12 Sep 2008, Andi Kleen wrote:
> On the other hand there's quite some other information dumped
> at boot that could be cut out. e.g. It doesn't really make 
> much sense to dump the cache information of each CPU at boot
> when that information can easily be gotten from user space
> later.

You're assuming that you ever reach user space, which is a pretty dumb
assumption.

I prefer to have a detailed view of the machine when I have to look at
a bug report where the machine does _NOT_ reach user space.
 
> I personally was also always sceptical of some of the
> more detailed new memory map dumping, like those "nosave
> memory" or the early reservation/early node/add_active_range output.
> It's pretty much a direct function of the e820 map dumped earlier.
> Also node information has now the dubious distinction to be 
> the most redundantly dumped kernel information of all.

Bla, bla, bla. 

This code was reworked significantly and every information which is
given about the various decisions made are useful to debug problems. I
prefer a more noisy startup over some silent fuckup when a problem
hits.

We had enough trouble to decode silent "fixups" which failed
miserably.

Thanks,

	tglx

Re: [PATCH] apci: dump slit

[Andi Kleen]

"Yinghai Lu" <yhlu.kernel@gmail.com> writes:
>
> to see how wrong could be set by BIOS.

Just dump it from user space then.  There are plenty of tools
to access ACPI tables.

-Andi

-- 
ak@linux.intel.com

Re: [PATCH] apci: dump slit

[Thomas Gleixner]

On Fri, 12 Sep 2008, Andi Kleen wrote:
> "Yinghai Lu" <yhlu.kernel@gmail.com> writes:
> >
> > to see how wrong could be set by BIOS.
> 
> Just dump it from user space then.  There are plenty of tools
> to access ACPI tables.

1.) what guarantees that we reach user space ?

2) If it is _valuable_ information which we can gather via kernel
   output, then it is much more conveniant than asking the user to type
   whatevercryptictoolcommandline and provide the output.

Nervertheless the changelog of the patch needs some useful information
why we actually need this information.

Thanks,

	tglx

Re: [PATCH] apci: dump slit

[Andi Kleen]

> 1.) what guarantees that we reach user space ?

Broken SLITs don't prevent booting.

> 2) If it is _valuable_ information which we can gather via kernel
>    output, then it is much more conveniant than asking the user to type
>    whatevercryptictoolcommandline and provide the output.

Yes excellent point.  Should we dump the complete ACPI tables?
Do you prefer hexdump or do we need a ASL disassembler?
Also complete SMBIOS? Also the legacy mptables of course too.
And all the EFI methods.  Ideally a disassembly of the complete BIOS.
Did I miss anything?

-Andi

Re: [PATCH] apci: dump slit

[Matt Keenan]

On Fri, 2008-09-12 at 23:30 +0200, Andi Kleen wrote:
> > 1.) what guarantees that we reach user space ?
> 
> Broken SLITs don't prevent booting.
> 
> > 2) If it is _valuable_ information which we can gather via kernel
> >    output, then it is much more conveniant than asking the user to type
> >    whatevercryptictoolcommandline and provide the output.
> 
> Yes excellent point.  Should we dump the complete ACPI tables?
> Do you prefer hexdump or do we need a ASL disassembler?
> Also complete SMBIOS? Also the legacy mptables of course too.
> And all the EFI methods.  Ideally a disassembly of the complete BIOS.
> Did I miss anything?
> 

Harsh... but harsh.

Matt

Re: [PATCH] apci: dump slit

[Thomas Gleixner]

On Fri, 12 Sep 2008, Andi Kleen wrote:
> > 1.) what guarantees that we reach user space ?
> 
> Broken SLITs don't prevent booting.

Of course you can guarantee that.

Thanks,

	tglx

Re: [PATCH] apci: dump slit

[Andi Kleen]

On Fri, Sep 12, 2008 at 11:49:04PM +0200, Thomas Gleixner wrote:
> On Fri, 12 Sep 2008, Andi Kleen wrote:
> > > 1.) what guarantees that we reach user space ?
> > 
> > Broken SLITs don't prevent booting.
> 
> Of course you can guarantee that.

Yes I'm pretty sure:

(a) the kernel does a sanity check on SLITs and replaces them
with a fallback if they're bogus. Even that is not strictly needed
for correctness, just for performance.
(b) SLIT information is only an optimization, but does not really break
anything if it's wrong. In the worst case your scheduler domains
or your zone fallback lists are not quite optimally set up, but
they will still work.

-Andi

-- 
ak@linux.intel.com

Re: [PATCH] apci: dump slit

[Yinghai Lu]

On Fri, Sep 12, 2008 at 2:30 PM, Andi Kleen <andi@firstfloor.org> wrote:
>
> Yes excellent point.  Should we dump the complete ACPI tables?
> Do you prefer hexdump

good point. there is one time BIOS put blank slit there. (only have
acpi table header)
only after dump that earlier in kernel boot log, they shut up and go
back to fix their problem.

YH

Re: [PATCH] apci: dump slit

[Andi Kleen]

On Fri, Sep 12, 2008 at 02:54:12PM -0700, Yinghai Lu wrote:
> On Fri, Sep 12, 2008 at 2:30 PM, Andi Kleen <andi@firstfloor.org> wrote:
> >
> > Yes excellent point.  Should we dump the complete ACPI tables?
> > Do you prefer hexdump
> 
> good point. there is one time BIOS put blank slit there. (only have
> acpi table header)
> only after dump that earlier in kernel boot log, they shut up and go
> back to fix their problem.

You should have asked them to run firmwaretestkit (and if fwtestkit 
doesn't test for a correct SLIT fix it there). That is what
it is for. But adding it 
to all Linux bootlogs ever just for this special case is the wrong way.


-Andi

-- 
ak@linux.intel.com

Re: [PATCH] apci: dump slit

[Pavel Machek]

On Fri 2008-09-12 23:19:21, Thomas Gleixner wrote:
> On Fri, 12 Sep 2008, Andi Kleen wrote:
> > "Yinghai Lu" <yhlu.kernel@gmail.com> writes:
> > >
> > > to see how wrong could be set by BIOS.
> > 
> > Just dump it from user space then.  There are plenty of tools
> > to access ACPI tables.
> 
> 1.) what guarantees that we reach user space ?

We can dump in *any* kernel.

> 2) If it is _valuable_ information which we can gather via kernel
>    output, then it is much more conveniant than asking the user to type
>    whatevercryptictoolcommandline and provide the output.

Ooh, 'lets dump more junk at everyone, typing commands is hard' :-(.

							Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] apci: dump slit

[Ingo Molnar]

* Pavel Machek <pavel@suse.cz> wrote:

> On Fri 2008-09-12 23:19:21, Thomas Gleixner wrote:
> > On Fri, 12 Sep 2008, Andi Kleen wrote:
> > > "Yinghai Lu" <yhlu.kernel@gmail.com> writes:
> > > >
> > > > to see how wrong could be set by BIOS.
> > > 
> > > Just dump it from user space then.  There are plenty of tools
> > > to access ACPI tables.
> > 
> > 1.) what guarantees that we reach user space ?
> 
> We can dump in *any* kernel.
>
> > 2) If it is _valuable_ information which we can gather via kernel
> >    output, then it is much more conveniant than asking the user to type
> >    whatevercryptictoolcommandline and provide the output.
> 
> Ooh, 'lets dump more junk at everyone, typing commands is hard' :-(.

no, the principle is, information like the boot-time CPUID information 
(and even the BIOS environment) can be indicative of _kernel bugs_. It 
is often essential to dump what the booting (and failing) kernel thinks 
is its environment.

Often that environment is corrupted (by the kernel) and that leads to 
problems. This 'environment' can also be affected by things like hard 
boot vs cold boot differences, whether it's in a kexec environment, 
whether it's booted as a virtual guest, etc., etc.

For a long time we had the kernel's x86 bootup pretty much as a mostly 
silent black box and when it broke we tried to figure things out 
afterwards which was difficult and error-prone. Now we've got various 
quite effective debug mechanisms (which includes printouts as well) and 
figuring out x86 problems is visibly easier. We definitely wont go back 
to the 'black box code, can only be debugged by a few experts' method.

So extending on that is a good and obvious idea in general - and i agree 
with Peter that this should be command-line dependent, i.e. not printed 
by default. Only printing it when 'debug' is specified on the command 
line is a good solution.

	Ingo

Re: [PATCH] apci: dump slit

[Pavel Machek]

On Sun 2008-09-14 14:26:18, Ingo Molnar wrote:
> 
> * Pavel Machek <pavel@suse.cz> wrote:
> 
> > On Fri 2008-09-12 23:19:21, Thomas Gleixner wrote:
> > > On Fri, 12 Sep 2008, Andi Kleen wrote:
> > > > "Yinghai Lu" <yhlu.kernel@gmail.com> writes:
> > > > >
> > > > > to see how wrong could be set by BIOS.
> > > > 
> > > > Just dump it from user space then.  There are plenty of tools
> > > > to access ACPI tables.
> > > 
> > > 1.) what guarantees that we reach user space ?
> > 
> > We can dump in *any* kernel.
> >
> > > 2) If it is _valuable_ information which we can gather via kernel
> > >    output, then it is much more conveniant than asking the user to type
> > >    whatevercryptictoolcommandline and provide the output.
> > 
> > Ooh, 'lets dump more junk at everyone, typing commands is hard' :-(.
> 
> no, the principle is, information like the boot-time CPUID information 
> (and even the BIOS environment) can be indicative of _kernel bugs_. It 
> is often essential to dump what the booting (and failing) kernel thinks 
> is its environment.
> 
> Often that environment is corrupted (by the kernel) and that leads to 
> problems. This 'environment' can also be affected by things like hard 
> boot vs cold boot differences, whether it's in a kexec environment, 
> whether it's booted as a virtual guest, etc., etc.
> 
> For a long time we had the kernel's x86 bootup pretty much as a mostly 
> silent black box and when it broke we tried to figure things out 
> afterwards which was difficult and error-prone. Now we've got various 
> quite effective debug mechanisms (which includes printouts as well) and 
> figuring out x86 problems is visibly easier. We definitely wont go back 
> to the 'black box code, can only be debugged by a few experts' method.
> 
> So extending on that is a good and obvious idea in general - and i agree 
> with Peter that this should be command-line dependent, i.e. not printed 
> by default. Only printing it when 'debug' is specified on the command 
> line is a good solution.

Dumping when user requested it on commandline seems like good
compromise to me.
							Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] apci: dump slit

[Pavel Machek]

On Sun 2008-09-14 17:59:28, Pavel Machek wrote:
> On Sun 2008-09-14 14:26:18, Ingo Molnar wrote:
> > 
> > * Pavel Machek <pavel@suse.cz> wrote:
> > 
> > > On Fri 2008-09-12 23:19:21, Thomas Gleixner wrote:
> > > > On Fri, 12 Sep 2008, Andi Kleen wrote:
> > > > > "Yinghai Lu" <yhlu.kernel@gmail.com> writes:
> > > > > >
> > > > > > to see how wrong could be set by BIOS.
> > > > > 
> > > > > Just dump it from user space then.  There are plenty of tools
> > > > > to access ACPI tables.
> > > > 
> > > > 1.) what guarantees that we reach user space ?
> > > 
> > > We can dump in *any* kernel.
> > >
> > > > 2) If it is _valuable_ information which we can gather via kernel
> > > >    output, then it is much more conveniant than asking the user to type
> > > >    whatevercryptictoolcommandline and provide the output.
> > > 
> > > Ooh, 'lets dump more junk at everyone, typing commands is hard' :-(.
> > 
> > no, the principle is, information like the boot-time CPUID information 
> > (and even the BIOS environment) can be indicative of _kernel bugs_. It 
> > is often essential to dump what the booting (and failing) kernel thinks 
> > is its environment.
> > 
> > Often that environment is corrupted (by the kernel) and that leads to 
> > problems. This 'environment' can also be affected by things like hard 
> > boot vs cold boot differences, whether it's in a kexec environment, 
> > whether it's booted as a virtual guest, etc., etc.
> > 
> > For a long time we had the kernel's x86 bootup pretty much as a mostly 
> > silent black box and when it broke we tried to figure things out 
> > afterwards which was difficult and error-prone. Now we've got various 
> > quite effective debug mechanisms (which includes printouts as well) and 
> > figuring out x86 problems is visibly easier. We definitely wont go back 
> > to the 'black box code, can only be debugged by a few experts' method.
> > 
> > So extending on that is a good and obvious idea in general - and i agree 
> > with Peter that this should be command-line dependent, i.e. not printed 
> > by default. Only printing it when 'debug' is specified on the command 
> > line is a good solution.
> 
> Dumping when user requested it on commandline seems like good
> compromise to me.

(I meant 'on kernel commandline' == 'in kernel parameters'. I agree with ingo here.)

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] apci: dump slit

[Ingo Molnar]

* Pavel Machek <pavel@suse.cz> wrote:

> On Sun 2008-09-14 14:26:18, Ingo Molnar wrote:
> > 
> > * Pavel Machek <pavel@suse.cz> wrote:
> > 
> > > On Fri 2008-09-12 23:19:21, Thomas Gleixner wrote:
> > > > On Fri, 12 Sep 2008, Andi Kleen wrote:
> > > > > "Yinghai Lu" <yhlu.kernel@gmail.com> writes:
> > > > > >
> > > > > > to see how wrong could be set by BIOS.
> > > > > 
> > > > > Just dump it from user space then.  There are plenty of tools
> > > > > to access ACPI tables.
> > > > 
> > > > 1.) what guarantees that we reach user space ?
> > > 
> > > We can dump in *any* kernel.
> > >
> > > > 2) If it is _valuable_ information which we can gather via kernel
> > > >    output, then it is much more conveniant than asking the user to type
> > > >    whatevercryptictoolcommandline and provide the output.
> > > 
> > > Ooh, 'lets dump more junk at everyone, typing commands is hard' :-(.
> > 
> > no, the principle is, information like the boot-time CPUID information 
> > (and even the BIOS environment) can be indicative of _kernel bugs_. It 
> > is often essential to dump what the booting (and failing) kernel thinks 
> > is its environment.
> > 
> > Often that environment is corrupted (by the kernel) and that leads to 
> > problems. This 'environment' can also be affected by things like hard 
> > boot vs cold boot differences, whether it's in a kexec environment, 
> > whether it's booted as a virtual guest, etc., etc.
> > 
> > For a long time we had the kernel's x86 bootup pretty much as a mostly 
> > silent black box and when it broke we tried to figure things out 
> > afterwards which was difficult and error-prone. Now we've got various 
> > quite effective debug mechanisms (which includes printouts as well) and 
> > figuring out x86 problems is visibly easier. We definitely wont go back 
> > to the 'black box code, can only be debugged by a few experts' method.
> > 
> > So extending on that is a good and obvious idea in general - and i agree 
> > with Peter that this should be command-line dependent, i.e. not printed 
> > by default. Only printing it when 'debug' is specified on the command 
> > line is a good solution.
> 
> Dumping when user requested it on commandline seems like good
> compromise to me.

great! Yinghai, mind reworking the patch in that direction please? I'd 
suggest to enable the optional dumping of all things BIOS environment 
related, not just the ACPI/SLIT.

there's also CONFIG_X86_VERBOSE_BOOTUP=y [default disabled] - which 
could be coupled with an x86=verbose boot parameter or so.

	Ingo

Re: [PATCH] apci: dump slit

[Yinghai Lu]

On Sun, Sep 14, 2008 at 9:04 AM, Ingo Molnar <mingo@elte.hu> wrote:
>>
>> Dumping when user requested it on commandline seems like good
>> compromise to me.
>
> great! Yinghai, mind reworking the patch in that direction please? I'd
> suggest to enable the optional dumping of all things BIOS environment
> related, not just the ACPI/SLIT.
>
> there's also CONFIG_X86_VERBOSE_BOOTUP=y [default disabled] - which
> could be coupled with an x86=verbose boot parameter or so.

will work on it.

YH

Re: [PATCH] apci: dump slit

[Andi Kleen]

> great! Yinghai, mind reworking the patch in that direction please? I'd 
> suggest to enable the optional dumping of all things BIOS environment 
> related, not just the ACPI/SLIT.

Great. I'm preparing the "kernel iasl -d" patch now @)

-Andi