[PATCH] faster AP launch for recent AMD processors

[Scott Duplichan]

To reduce boot time, removing the double startup IPI and 10 ms
delay from lapic_cpu_init.c should be considered. Because I can
test this code only on recent AMD cpu models, it is only safe
to apply the change in those cases. The following patch uses
a compile time check to identify CPUs where the change is known
to work. Would a runtime check be better?

Thanks,
Scott

Signed-off-by: Scott Duplichan

Index: src/cpu/x86/lapic/lapic_cpu_init.c
===================================================================
--- src/cpu/x86/lapic/lapic_cpu_init.c  (revision 5965)
+++ src/cpu/x86/lapic/lapic_cpu_init.c  (working copy)
@@ -113,7 +113,9 @@
                }
                return 0;
        }
+   #if !defined (CONFIG_CPU_AMD_MODEL_10XXX) && !defined (CONFIG_CPU_AMD_MODEL_14XXX)
        mdelay(10);
+   #endif

printk(BIOS_SPEW, "Deasserting INIT.\n");

@@ -144,6 +146,9 @@
 #endif

num_starts = 2;
+   #if defined (CONFIG_CPU_AMD_MODEL_10XXX) || defined (CONFIG_CPU_AMD_MODEL_14XXX)
+       num_starts = 1;
+       #endif

/*
         * Run STARTUP IPI loop.

[Myles Watson]

> To reduce boot time, removing the double startup IPI and 10 ms
> delay from lapic_cpu_init.c should be considered. Because I can
> test this code only on recent AMD cpu models, it is only safe
> to apply the change in those cases. The following patch uses
> a compile time check to identify CPUs where the change is known
> to work. Would a runtime check be better?

Until Fam10 and K8 can use the same image, compile time is probably the
correct way to do it.  If there ends up being a long list of processors that
don't need the wait, we could add a Kconfig option like
CONFIG_NO_LAPIC_DELAY.
> Signed-off-by: Scott Duplichan

Acked-by: Myles Watson
> #if !defined (CONFIG_CPU_AMD_MODEL_10XXX) && !defined
>
> (CONFIG_CPU_AMD_MODEL_14XXX)
>         mdelay(10);
>
> #endif
> printk(BIOS_SPEW, "Deasserting INIT.\n");
>
> @@ -144,6 +146,9 @@
>  #endif
>     num_starts = 2;
> #if defined (CONFIG_CPU_AMD_MODEL_10XXX) || defined
>
> (CONFIG_CPU_AMD_MODEL_14XXX)
>   num_starts = 1;
>   #endif

I'd prefer it if they both used the same logic.  I think it will make it
easier to update in the future.  Maybe something like this:

#if !defined (CONFIG_CPU_AMD_MODEL_10XXX) && !defined
(CONFIG_CPU_AMD_MODEL_14XXX)
    num_starts = 2;
#else
    num_starts = 1;
#endif

Thanks,
Myles

[Scott Duplichan]

]
]> To reduce boot time, removing the double startup IPI and 10 ms
]> delay from lapic_cpu_init.c should be considered. Because I can
]> test this code only on recent AMD cpu models, it is only safe
]> to apply the change in those cases. The following patch uses
]> a compile time check to identify CPUs where the change is known
]> to work. Would a runtime check be better?
]Until Fam10 and K8 can use the same image, compile time is probably the
]correct way to do it.  If there ends up being a long list of processors that
]don't need the wait, we could add a Kconfig option like
]CONFIG_NO_LAPIC_DELAY.
]
]> Signed-off-by: Scott Duplichan 
]Acked-by: Myles Watson 
]
]> +   #if !defined (CONFIG_CPU_AMD_MODEL_10XXX) && !defined
]> (CONFIG_CPU_AMD_MODEL_14XXX)
]>         mdelay(10);
]> +   #endif
]> 
]>         printk(BIOS_SPEW, "Deasserting INIT.\n");
]> 
]> @@ -144,6 +146,9 @@
]>  #endif
]> 
]>         num_starts = 2;
]> +   #if defined (CONFIG_CPU_AMD_MODEL_10XXX) || defined
]> (CONFIG_CPU_AMD_MODEL_14XXX)
]> +       num_starts = 1;
]> +       #endif
]
]I'd prefer it if they both used the same logic.  I think it will make it
]easier to update in the future.  Maybe something like this:
]
]#if !defined (CONFIG_CPU_AMD_MODEL_10XXX) && !defined
](CONFIG_CPU_AMD_MODEL_14XXX)
]	num_starts = 2;
]#else
]	num_starts = 1;
]#endif
]
]Thanks,
]Myles
]

Hello Myles,

Thanks. Of course you are right, consistent logic is best...
I also tried to make the indentation match the surrounding code.

Signed-off-by: Scott Duplichan

Index: src/cpu/x86/lapic/lapic_cpu_init.c
===================================================================
--- src/cpu/x86/lapic/lapic_cpu_init.c  (revision 5965)
+++ src/cpu/x86/lapic/lapic_cpu_init.c  (working copy)
@@ -113,7 +113,9 @@
                }
                return 0;
        }
+#if !defined (CONFIG_CPU_AMD_MODEL_10XXX) && !defined (CONFIG_CPU_AMD_MODEL_14XXX)
        mdelay(10);
+#endif

printk(BIOS_SPEW, "Deasserting INIT.\n");

@@ -143,7 +145,11 @@
        start_eip = (unsigned long)_secondary_start;
 #endif

+#if !defined (CONFIG_CPU_AMD_MODEL_10XXX) && !defined (CONFIG_CPU_AMD_MODEL_14XXX)
        num_starts = 2;
+#else
+       num_starts = 1;
+#endif

/*
         * Run STARTUP IPI loop.

[Myles Watson]

On Mon, Oct 18, 2010 at 2:24 PM, Scott Duplichan  wrote:
> ]
> ]> To reduce boot time, removing the double startup IPI and 10 ms
> ]> delay from lapic_cpu_init.c should be considered. Because I can
> ]> test this code only on recent AMD cpu models, it is only safe
> ]> to apply the change in those cases. The following patch uses
> ]> a compile time check to identify CPUs where the change is known
> ]> to work.
> Signed-off-by: Scott Duplichan

Acked-by: Myles Watson

Thanks,
Myles