When debug logging is enabled, a message such as '* AP 02 timed out:02010501' is sometimes logged. The reason is that the AP first sets a completion value such as 0x13, which is what function wait_cpu_state() is waiting for. Then a short time later, the AP calls function init_fidvid_ap(). This function sets a completion value of 01. When logging is off, wait_cpu_state is fast enough to see the initial completion value for each of the APs. But with logging enabled, one or more APs may go on to complete function init_fidvid_ap, which sets the completion value to 01. While mostly harmless, the timeout does increase boot time. The following patch eliminates the timeout by making function wait_cpu_state recognize 01 as an additional valid AP completion value.
Signed-off-by: Scott Duplichan scott@notabs.org
Index: src/cpu/amd/model_10xxx/init_cpus.c =================================================================== --- src/cpu/amd/model_10xxx/init_cpus.c (revision 5965) +++ src/cpu/amd/model_10xxx/init_cpus.c (working copy) @@ -189,8 +189,10 @@ int loop = 4000000; while (--loop > 0) { if (lapic_remote_read(apicid, LAPIC_MSG_REG, &readback) != 0) - continue; - if ((readback & 0x3f) == state) { + continue; + // stop polling if expected state is reached, or if + // init_fidvid_ap() sets 'ready for warm reset' state + if ((readback & 0x3f) == state || (readback & 0x3f) == 1) { timeout = 0; break; //target cpu is in stage started } @@ -273,7 +275,7 @@
/* that is from initial apicid, we need nodeid and coreid later */ - id = get_node_core_id_x(); + id = get_node_core_id_x();
/* NB_CFG MSR is shared between cores, so we need make sure core0 is done at first --- use wait_all_core0_started */
Scott Duplichan wrote:
patch eliminates the timeout by making function wait_cpu_state recognize 01 as an additional valid AP completion value.
Do we want some names for those values? (rather than magical 1?)
Signed-off-by: Scott Duplichan scott@notabs.org
Acked-by: Peter Stuge peter@stuge.se
]> patch eliminates the timeout by making function wait_cpu_state ]> recognize 01 as an additional valid AP completion value. ] ]Do we want some names for those values? (rather than magical 1?) ] ]> Signed-off-by: Scott Duplichan scott@notabs.org ] ]Acked-by: Peter Stuge peter@stuge.se
Hello Peter,
How about this:
Signed-off-by: Scott Duplichan scott@notabs.org
Index: src/cpu/amd/model_10xxx/fidvid.c =================================================================== --- src/cpu/amd/model_10xxx/fidvid.c (revision 5965) +++ src/cpu/amd/model_10xxx/fidvid.c (working copy) @@ -530,7 +530,7 @@
// Send signal to BSP about this AP max fid // This also indicates this AP is ready for warm reset (if required). - lapic_write(LAPIC_MSG_REG, send | 1); + lapic_write(LAPIC_MSG_REG, send | F10_APSTATE_RESET); }
static u32 calc_common_fid(u32 fid_packed, u32 fid_packed_new) Index: src/cpu/amd/model_10xxx/init_cpus.c =================================================================== --- src/cpu/amd/model_10xxx/init_cpus.c (revision 5965) +++ src/cpu/amd/model_10xxx/init_cpus.c (working copy) @@ -166,9 +166,6 @@ return result; }
-/* Use the LAPIC timer count register to hold each cores init status */ -#define LAPIC_MSG_REG 0x380 - #if SET_FIDVID == 1 static void init_fidvid_ap(u32 bsp_apicid, u32 apicid, u32 nodeid, u32 coreid); #endif @@ -190,7 +187,7 @@ while (--loop > 0) { if (lapic_remote_read(apicid, LAPIC_MSG_REG, &readback) != 0) continue; - if ((readback & 0x3f) == state) { + if ((readback & 0x3f) == state || (readback & 0x3f) == F10_APSTATE_RESET) { timeout = 0; break; //target cpu is in stage started } @@ -207,7 +204,7 @@ static void wait_ap_started(u32 ap_apicid, void *gp) { u32 timeout; - timeout = wait_cpu_state(ap_apicid, 0x13); // started + timeout = wait_cpu_state(ap_apicid, F10_APSTATE_STARTED); printk(BIOS_DEBUG, "* AP %02x", ap_apicid); if (timeout) { printk(BIOS_DEBUG, " timed out:%08x\n", timeout); @@ -231,7 +228,7 @@ /* FIXME Do APs use this? */
// allow aps to stop use 6 bits for state - lapic_write(LAPIC_MSG_REG, (bsp_apicid << 24) | 0x14); + lapic_write(LAPIC_MSG_REG, (bsp_apicid << 24) | F10_APSTATE_STOPPED); }
static void enable_apic_ext_id(u32 node) @@ -331,7 +328,7 @@ distinguish_cpu_resets(id.nodeid); // Also indicates we are started } // Mark the core as started. - lapic_write(LAPIC_MSG_REG, (apicid << 24) | 0x13); + lapic_write(LAPIC_MSG_REG, (apicid << 24) | F10_APSTATE_STARTED);
if (apicid != bsp_apicid) { /* Setup each AP's cores MSRs. Index: src/northbridge/amd/amdfam10/amdfam10.h =================================================================== --- src/northbridge/amd/amdfam10/amdfam10.h (revision 5965) +++ src/northbridge/amd/amdfam10/amdfam10.h (working copy) @@ -953,6 +953,16 @@ #define NonCoherent (1 << 2) #define ConnectionPending (1 << 4)
+// Use the LAPIC timer count register to hold each core's init status +// Format: byte 0 - state +// byte 1 - fid_max +// byte 2 - nb_cof_vid_update +// byte 3 - apic id + +#define LAPIC_MSG_REG 0x380 +#define F10_APSTATE_STARTED 0x13 // start of AP execution +#define F10_APSTATE_STOPPED 0x14 // allow AP to stop +#define F10_APSTATE_RESET 0x01 // waiting for warm reset
#include "amdfam10_nums.h"
Scott Duplichan wrote:
]> patch eliminates the timeout by making function wait_cpu_state ]> recognize 01 as an additional valid AP completion value. ] ]Do we want some names for those values? (rather than magical 1?) ] ]> Signed-off-by: Scott Duplichan scott@notabs.org ] ]Acked-by: Peter Stuge peter@stuge.se
Hello Peter,
How about this:
Sure thing! Double ack. :)
Signed-off-by: Scott Duplichan scott@notabs.org
Acked-by: Peter Stuge peter@stuge.se
-
Peter Stuge -
Scott Duplichan