PATCH: Allow over-ride of smp_found_cfg with kernel cmd-line option.

PATCH: Allow over-ride of smp_found_cfg with kernel cmd-line option.

[Ben Greear]

[-- Attachment #1: Type: text/plain, Size: 471 bytes --]

Allow user to keep smp_found_cfg set to 1 even if MADT
cannot be parsed.  This works around funky BIOS on FWA-7304
(VIA CN700 chipset) system, and possibly other systems as well.
Without this override, performance drops by around 15% on
network throughput tests on this system.

This is based on 2.6.29-rc3

Signed-Off-By:  Ben Greear <greearb@candelatech.com>


Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


[-- Attachment #2: patch1.patch --]
[-- Type: text/x-patch, Size: 1395 bytes --]

diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index d37593c..e96bf19 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -87,6 +87,7 @@ u8 acpi_sci_flags __initdata;
 int acpi_sci_override_gsi __initdata;
 int acpi_skip_timer_override __initdata;
 int acpi_use_timer_override __initdata;
+int force_smp_found_cfg __initdata;
 
 #ifdef CONFIG_X86_LOCAL_APIC
 static u64 acpi_lapic_addr __initdata = APIC_DEFAULT_PHYS_BASE;
@@ -1335,6 +1336,14 @@ static void __init early_acpi_process_madt(void)
 #endif
 }
 
+static int __init parse_force_smp_found_cfg(char *arg)
+{
+	force_smp_found_cfg = 1;
+	return 0;
+}
+early_param("force_smp_found_cfg", parse_force_smp_found_cfg);
+
+
 static void __init acpi_process_madt(void)
 {
 #ifdef CONFIG_X86_LOCAL_APIC
@@ -1381,9 +1390,14 @@ static void __init acpi_process_madt(void)
  		 * Boot with "acpi=off" to use MPS on such a system.
  		 */
 		if (smp_found_config) {
-			printk(KERN_WARNING PREFIX
-				"No APIC-table, disabling MPS\n");
-			smp_found_config = 0;
+			if (force_smp_found_cfg)
+				printk(KERN_WARNING PREFIX
+				       "No APIC-table, wanted to disable MPS, but will not\n  due to force_smp_found_cfg=1\n");
+			else {
+				printk(KERN_WARNING PREFIX
+				       "No APIC-table, disabling MPS.  Use force_smp_found_cfg=1 to override\n");
+				smp_found_config = 0;
+			}
 		}
 	}
 

Re: PATCH: Allow over-ride of smp_found_cfg with kernel cmd-line option.

[Randy Dunlap]

Ben Greear wrote:
> Allow user to keep smp_found_cfg set to 1 even if MADT
> cannot be parsed.  This works around funky BIOS on FWA-7304
> (VIA CN700 chipset) system, and possibly other systems as well.
> Without this override, performance drops by around 15% on
> network throughput tests on this system.
> 
> This is based on 2.6.29-rc3
> 
> Signed-Off-By:  Ben Greear <greearb@candelatech.com>


Both patches (smp and tsc) need entries in Documentation/kernel-parameters.txt
(if they are accepted/merged).

And thunderbird can be coerced into sending patches inline instead of
using attachments... please.  Documentation/email-clients.txt might help you.

Thanks,
-- 
~Randy

Re: PATCH: Allow over-ride of smp_found_cfg with kernel cmd-line option.

[Ingo Molnar]

* Ben Greear <greearb@candelatech.com> wrote:

> Allow user to keep smp_found_cfg set to 1 even if MADT
> cannot be parsed.  This works around funky BIOS on FWA-7304
> (VIA CN700 chipset) system, and possibly other systems as well.
> Without this override, performance drops by around 15% on
> network throughput tests on this system.
>
> This is based on 2.6.29-rc3
>
> Signed-Off-By:  Ben Greear <greearb@candelatech.com>
>
>
> Thanks,
> Ben
>
> -- 
> Ben Greear <greearb@candelatech.com>
> Candela Technologies Inc  http://www.candelatech.com
>

> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
> index d37593c..e96bf19 100644
> --- a/arch/x86/kernel/acpi/boot.c
> +++ b/arch/x86/kernel/acpi/boot.c
> @@ -87,6 +87,7 @@ u8 acpi_sci_flags __initdata;
>  int acpi_sci_override_gsi __initdata;
>  int acpi_skip_timer_override __initdata;
>  int acpi_use_timer_override __initdata;
> +int force_smp_found_cfg __initdata;
>  
>  #ifdef CONFIG_X86_LOCAL_APIC
>  static u64 acpi_lapic_addr __initdata = APIC_DEFAULT_PHYS_BASE;
> @@ -1335,6 +1336,14 @@ static void __init early_acpi_process_madt(void)
>  #endif
>  }
>  
> +static int __init parse_force_smp_found_cfg(char *arg)
> +{
> +	force_smp_found_cfg = 1;
> +	return 0;
> +}
> +early_param("force_smp_found_cfg", parse_force_smp_found_cfg);
> +
> +
>  static void __init acpi_process_madt(void)
>  {
>  #ifdef CONFIG_X86_LOCAL_APIC
> @@ -1381,9 +1390,14 @@ static void __init acpi_process_madt(void)
>   		 * Boot with "acpi=off" to use MPS on such a system.
>   		 */
>  		if (smp_found_config) {
> -			printk(KERN_WARNING PREFIX
> -				"No APIC-table, disabling MPS\n");
> -			smp_found_config = 0;
> +			if (force_smp_found_cfg)
> +				printk(KERN_WARNING PREFIX
> +				       "No APIC-table, wanted to disable MPS, but will not\n  due to force_smp_found_cfg=1\n");
> +			else {
> +				printk(KERN_WARNING PREFIX
> +				       "No APIC-table, disabling MPS.  Use force_smp_found_cfg=1 to override\n");
> +				smp_found_config = 0;
> +			}
>  		}
>  	}
>  

I'd _much_ rather have a warning message printed and have a default, 
unconditional fallback to mptable parsing in this case.

I.e. same end result just no ugly and inconvenient force_smp_found_cfg.

The mptable works fine in most such cases to enumerate apic details. (as it 
does in your case, evidently)

And if both the ACPI and the MPTABLE is hosed on a box, it cannot be of much 
use on other OSs either, right?

So please do a patch that removes the 'smp_found_config' and which changes 
the warning message to warn about no ACPI table in the MADT table. Ok?

	Ingo

Re: PATCH: Allow over-ride of smp_found_cfg with kernel cmd-line option.

[Ben Greear]

Ingo Molnar wrote:

> I'd _much_ rather have a warning message printed and have a default, 
> unconditional fallback to mptable parsing in this case.
> 
> I.e. same end result just no ugly and inconvenient force_smp_found_cfg.
> 
> The mptable works fine in most such cases to enumerate apic details. (as it 
> does in your case, evidently)
> 
> And if both the ACPI and the MPTABLE is hosed on a box, it cannot be of much 
> use on other OSs either, right?
> 
> So please do a patch that removes the 'smp_found_config' and which changes 
> the warning message to warn about no ACPI table in the MADT table. Ok?

That is fine with me, but I know next to zero about the deep logic
of this code.  I'd be worried that Len put in the change to fix something
more critical than my problem on my particular board.

If no one argues otherwise, I'll get a patch together & tested this evening.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: PATCH: (v2) Allow over-ride of smp_found_cfg with kernel cmd-line option.

[Ben Greear]

[-- Attachment #1: Type: text/plain, Size: 2227 bytes --]

Ingo Molnar wrote:
> * Ben Greear <greearb@candelatech.com> wrote:
> 
>> Allow user to keep smp_found_cfg set to 1 even if MADT
>> cannot be parsed.  This works around funky BIOS on FWA-7304
>> (VIA CN700 chipset) system, and possibly other systems as well.
>> Without this override, performance drops by around 15% on
>> network throughput tests on this system.

> I'd _much_ rather have a warning message printed and have a default, 
> unconditional fallback to mptable parsing in this case.
> 
> I.e. same end result just no ugly and inconvenient force_smp_found_cfg.
> 
> The mptable works fine in most such cases to enumerate apic details. (as it 
> does in your case, evidently)
> 
> And if both the ACPI and the MPTABLE is hosed on a box, it cannot be of much 
> use on other OSs either, right?
> 
> So please do a patch that removes the 'smp_found_config' and which changes 
> the warning message to warn about no ACPI table in the MADT table. Ok?

An updated patch is attached.  It's also inline below, but I'm sure the
inline version is white-space corrupted.

Do not disable mptable parsing just because there is no ACPI table
in the MADT table.  This effectively backs out changeset
7b37b5fd9ba32c0c5afc3537eed7e7466f2173e2

Signed-Off-By:  Ben Greear<greearb@candelatech.com>

Thanks,
Ben


diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index d37593c..74a50b6 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -1379,12 +1379,14 @@ static void __init acpi_process_madt(void)
   		 * ACPI found no MADT, and so ACPI wants UP PIC mode.
   		 * In the event an MPS table was found, forget it.
   		 * Boot with "acpi=off" to use MPS on such a system.
+		 *
+		 * NOTE:  This assumption seems wrong in at least some cases, perhaps
+		 *  when ACPI is off in the BIOS.  Changing to a warning without
+		 *  setting smp_found_config to zero.
   		 */
-		if (smp_found_config) {
+		if (smp_found_config)
  			printk(KERN_WARNING PREFIX
-				"No APIC-table, disabling MPS\n");
-			smp_found_config = 0;
-		}
+				"WARNING: No ACPI-table found in the MADT\n");
  	}

  	/*



-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


[-- Attachment #2: madt_patch.txt --]
[-- Type: text/plain, Size: 819 bytes --]

diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index d37593c..74a50b6 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -1379,12 +1379,14 @@ static void __init acpi_process_madt(void)
  		 * ACPI found no MADT, and so ACPI wants UP PIC mode.
  		 * In the event an MPS table was found, forget it.
  		 * Boot with "acpi=off" to use MPS on such a system.
+		 *
+		 * NOTE:  This assumption seems wrong in at least some cases, perhaps
+		 *  when ACPI is off in the BIOS.  Changing to a warning without
+		 *  setting smp_found_config to zero.
  		 */
-		if (smp_found_config) {
+		if (smp_found_config)
 			printk(KERN_WARNING PREFIX
-				"No APIC-table, disabling MPS\n");
-			smp_found_config = 0;
-		}
+				"WARNING: No ACPI-table found in the MADT\n");
 	}
 
 	/*

Re: PATCH: (v2) Allow over-ride of smp_found_cfg with kernel cmd-line option.

[Randy Dunlap]

Ben Greear wrote:
> Ingo Molnar wrote:
>> * Ben Greear <greearb@candelatech.com> wrote:
>>
>>> Allow user to keep smp_found_cfg set to 1 even if MADT
>>> cannot be parsed.  This works around funky BIOS on FWA-7304
>>> (VIA CN700 chipset) system, and possibly other systems as well.
>>> Without this override, performance drops by around 15% on
>>> network throughput tests on this system.
> 
>> I'd _much_ rather have a warning message printed and have a default,
>> unconditional fallback to mptable parsing in this case.
>>
>> I.e. same end result just no ugly and inconvenient force_smp_found_cfg.
>>
>> The mptable works fine in most such cases to enumerate apic details.
>> (as it does in your case, evidently)
>>
>> And if both the ACPI and the MPTABLE is hosed on a box, it cannot be
>> of much use on other OSs either, right?
>>
>> So please do a patch that removes the 'smp_found_config' and which
>> changes the warning message to warn about no ACPI table in the MADT
>> table. Ok?
> 
> An updated patch is attached.  It's also inline below, but I'm sure the
> inline version is white-space corrupted.
> 
> Do not disable mptable parsing just because there is no ACPI table
> in the MADT table.  This effectively backs out changeset
> 7b37b5fd9ba32c0c5afc3537eed7e7466f2173e2
> 
> Signed-Off-By:  Ben Greear<greearb@candelatech.com>
> 
> Thanks,
> Ben
> 
> 
> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
> index d37593c..74a50b6 100644
> --- a/arch/x86/kernel/acpi/boot.c
> +++ b/arch/x86/kernel/acpi/boot.c
> @@ -1379,12 +1379,14 @@ static void __init acpi_process_madt(void)
>            * ACPI found no MADT, and so ACPI wants UP PIC mode.
>            * In the event an MPS table was found, forget it.
>            * Boot with "acpi=off" to use MPS on such a system.
> +         *
> +         * NOTE:  This assumption seems wrong in at least some cases,
> perhaps
> +         *  when ACPI is off in the BIOS.  Changing to a warning without
> +         *  setting smp_found_config to zero.
>            */
> -        if (smp_found_config) {
> +        if (smp_found_config)
>              printk(KERN_WARNING PREFIX
> -                "No APIC-table, disabling MPS\n");
> -            smp_found_config = 0;
> -        }
> +                "WARNING: No ACPI-table found in the MADT\n");

		                APIC table

>      }
> 
>      /*
> 
> 
> 


-- 
~Randy

Re: PATCH: Allow over-ride of smp_found_cfg with kernel cmd-line option.

[Ingo Molnar]

* Ben Greear <greearb@candelatech.com> wrote:

> Allow user to keep smp_found_cfg set to 1 even if MADT
> cannot be parsed.  This works around funky BIOS on FWA-7304
> (VIA CN700 chipset) system, and possibly other systems as well.
> Without this override, performance drops by around 15% on
> network throughput tests on this system.

The question is, _why_ does network performance drop?

Do you have a before/after pair of /proc/interrupts output
perhaps? Does the layout of IRQs change?

The system is not SMP - so only something about UP-IOAPIC can
be helpful to performance here - or some unknown effect.

	Ingo

Re: PATCH: (v2) Allow over-ride of smp_found_cfg with kernel cmd-line option.

[Yinghai Lu]

Randy Dunlap wrote:
> Ben Greear wrote:
>> Ingo Molnar wrote:
>>> * Ben Greear <greearb@candelatech.com> wrote:
>>>
>>>> Allow user to keep smp_found_cfg set to 1 even if MADT
>>>> cannot be parsed.  This works around funky BIOS on FWA-7304
>>>> (VIA CN700 chipset) system, and possibly other systems as well.
>>>> Without this override, performance drops by around 15% on
>>>> network throughput tests on this system.
>>> I'd _much_ rather have a warning message printed and have a default,
>>> unconditional fallback to mptable parsing in this case.
>>>
>>> I.e. same end result just no ugly and inconvenient force_smp_found_cfg.
>>>
>>> The mptable works fine in most such cases to enumerate apic details.
>>> (as it does in your case, evidently)
>>>
>>> And if both the ACPI and the MPTABLE is hosed on a box, it cannot be
>>> of much use on other OSs either, right?
>>>
>>> So please do a patch that removes the 'smp_found_config' and which
>>> changes the warning message to warn about no ACPI table in the MADT
>>> table. Ok?
>> An updated patch is attached.  It's also inline below, but I'm sure the
>> inline version is white-space corrupted.
>>
>> Do not disable mptable parsing just because there is no ACPI table
>> in the MADT table.  This effectively backs out changeset
>> 7b37b5fd9ba32c0c5afc3537eed7e7466f2173e2
>>
>> Signed-Off-By:  Ben Greear<greearb@candelatech.com>
>>
>> Thanks,
>> Ben
>>
>>
>> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
>> index d37593c..74a50b6 100644
>> --- a/arch/x86/kernel/acpi/boot.c
>> +++ b/arch/x86/kernel/acpi/boot.c
>> @@ -1379,12 +1379,14 @@ static void __init acpi_process_madt(void)
>>            * ACPI found no MADT, and so ACPI wants UP PIC mode.
>>            * In the event an MPS table was found, forget it.
>>            * Boot with "acpi=off" to use MPS on such a system.
>> +         *
>> +         * NOTE:  This assumption seems wrong in at least some cases,
>> perhaps
>> +         *  when ACPI is off in the BIOS.  Changing to a warning without
>> +         *  setting smp_found_config to zero.
>>            */
>> -        if (smp_found_config) {
>> +        if (smp_found_config)
>>              printk(KERN_WARNING PREFIX
>> -                "No APIC-table, disabling MPS\n");
>> -            smp_found_config = 0;
>> -        }
>> +                "WARNING: No ACPI-table found in the MADT\n");
> 
> 		                APIC table
> 

APIC-table == MADT

could just use

WARNING: No APIC-table found

YH

Re: PATCH: (v3) Allow over-ride of smp_found_cfg with kernel cmd-line option.

[Ben Greear]

[-- Attachment #1: Type: text/plain, Size: 1631 bytes --]

Yinghai Lu wrote:

>>> +                "WARNING: No ACPI-table found in the MADT\n");
>> 		                APIC table
>>
> 
> APIC-table == MADT
> 
> could just use
> 
> WARNING: No APIC-table found

Ok, here's another attempt.  NOTE:  With another patch that Len sent me,
this work-around seems no longer required for *my* system.  This patch may
still help others, however...


An updated patch is attached.  It's also inline below, but I'm sure the
inline version is white-space corrupted.

Do not disable mptable parsing just because there is no ACPI table
in the MADT table.  This effectively backs out changeset
7b37b5fd9ba32c0c5afc3537eed7e7466f2173e2

Signed-Off-By:  Ben Greear<greearb@candelatech.com>

diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index d37593c..6940fb2 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -1379,12 +1379,14 @@ static void __init acpi_process_madt(void)
   		 * ACPI found no MADT, and so ACPI wants UP PIC mode.
   		 * In the event an MPS table was found, forget it.
   		 * Boot with "acpi=off" to use MPS on such a system.
+		 *
+		 * NOTE:  This assumption seems wrong in at least some cases, perhaps
+		 *  when ACPI is off in the BIOS.  Changing to a warning without
+		 *  setting smp_found_config to zero.
   		 */
-		if (smp_found_config) {
+		if (smp_found_config)
  			printk(KERN_WARNING PREFIX
-				"No APIC-table, disabling MPS\n");
-			smp_found_config = 0;
-		}
+			       "WARNING: No APIC-table found.\n");
  	}

  	/*


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


[-- Attachment #2: madt_patch2.txt --]
[-- Type: text/plain, Size: 814 bytes --]

diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index d37593c..6940fb2 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -1379,12 +1379,14 @@ static void __init acpi_process_madt(void)
  		 * ACPI found no MADT, and so ACPI wants UP PIC mode.
  		 * In the event an MPS table was found, forget it.
  		 * Boot with "acpi=off" to use MPS on such a system.
+		 *
+		 * NOTE:  This assumption seems wrong in at least some cases, perhaps
+		 *  when ACPI is off in the BIOS.  Changing to a warning without
+		 *  setting smp_found_config to zero.
  		 */
-		if (smp_found_config) {
+		if (smp_found_config)
 			printk(KERN_WARNING PREFIX
-				"No APIC-table, disabling MPS\n");
-			smp_found_config = 0;
-		}
+			       "WARNING: No APIC-table found.\n");
 	}
 
 	/*

Re: PATCH: (v3) Allow over-ride of smp_found_cfg with kernel cmd-line option.

[Len Brown]

> Ok, here's another attempt.  NOTE:  With another patch that Len sent me,
> this work-around seems no longer required for *my* system.  This patch may
> still help others, however...

Ben, your system is fixed by my patch, which makes this code not run.
http://lkml.org/lkml/2009/2/6/354

I do not want to apply the patch in this thread, because
I know of one system it will hurt, and zero systems it will help.

thanks,
-Len
Len Brown, Intel Open Source Technology Center

Re: PATCH: Allow over-ride of smp_found_cfg with kernel cmd-line option.

[Len Brown]

> > Without this override, performance drops by around 15% on
> > network throughput tests on this system.
> 
> The question is, _why_ does network performance drop?
>
> Do you have a before/after pair of /proc/interrupts output
> perhaps? Does the layout of IRQs change?
>
> The system is not SMP - so only something about UP-IOAPIC can
> be helpful to performance here - or some unknown effect.

I presume it is IOAPIC (good) vs PIC (bad), for the 
patch that provoked this regression was one that caused
the MPS table to be disabled.  (the problem with that patch
was that it was effective not just in ACPI mode, but in acpi=off mode)

The other un-answered question is why the system under test
has ACPI disabled in the BIOS when it did not ship that
way from the manufacturer.

cheers,
-Len
Intel Open Source Technology Center