Paul Menzel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/30830
Change subject: Revert "cpu/amd: Use `get_option()`" ......................................................................
Revert "cpu/amd: Use `get_option()`"
This reverts commit 6fffd70435084fb1d3237fcb1a11f11849721e8f.
Doing more tests on the Asus KGPE-D16, it seems to cause a reboot loop quite often. Therefore, revert the commit.
The problem might be caused by the spinlocks used by `get_option()`, and which are not used by `read_option()`.
Change-Id: Ic25129aa71c8e8e40a65bb2658de78005766fea8 Signed-off-by: Paul Menzel pmenzel@molgen.mpg.de --- M src/cpu/amd/family_10h-family_15h/init_cpus.c 1 file changed, 6 insertions(+), 16 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/30830/1
diff --git a/src/cpu/amd/family_10h-family_15h/init_cpus.c b/src/cpu/amd/family_10h-family_15h/init_cpus.c index b8e11e4..2ddbc43 100644 --- a/src/cpu/amd/family_10h-family_15h/init_cpus.c +++ b/src/cpu/amd/family_10h-family_15h/init_cpus.c @@ -147,9 +147,6 @@ // here assume the OS don't change our apicid u32 ap_apicid;
- u8 nvram; - bool multicore; - u32 nodes; u32 disable_siblings; u32 cores_found; @@ -158,13 +155,12 @@ /* get_nodes define in ht_wrapper.c */ nodes = get_nodes();
- multicore = true; - if (get_option(&nvram, "multi_core") == CB_SUCCESS) - multicore = !!nvram; - - disable_siblings = 0; - if (!IS_ENABLED(CONFIG_LOGICAL_CPUS) || !multicore) + if (!IS_ENABLED(CONFIG_LOGICAL_CPUS) || + read_option(multi_core, 0) != 0) { // 0 means multi core disable_siblings = 1; + } else { + disable_siblings = 0; + }
for (i = 0; i < nodes; i++) { if ((node >= 0) && (i != node)) @@ -639,17 +635,11 @@ //it is running on core0 of node0 void start_other_cores(uint32_t bsp_apicid) { - u8 nvram; u32 nodes; u32 nodeid; - bool multicore;
// disable multi_core - multicore = true; - if (get_option(&nvram, "multi_core") == CB_SUCCESS) - multicore = !!nvram; - - if (!multicore) { + if (read_option(multi_core, 0) != 0) { printk(BIOS_DEBUG, "Skip additional core init\n"); return; }
Kyösti Mälkki has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/30830 )
Change subject: Revert "cpu/amd: Use `get_option()`" ......................................................................
Revert "cpu/amd: Use `get_option()`"
This reverts commit 6fffd70435084fb1d3237fcb1a11f11849721e8f.
Doing more tests on the Asus KGPE-D16, it seems to cause a reboot loop quite often. Therefore, revert the commit.
The problem might be caused by the spinlocks used by `get_option()`, and which are not used by `read_option()`.
Change-Id: Ic25129aa71c8e8e40a65bb2658de78005766fea8 Signed-off-by: Paul Menzel pmenzel@molgen.mpg.de Reviewed-on: https://review.coreboot.org/c/30830 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/cpu/amd/family_10h-family_15h/init_cpus.c 1 file changed, 6 insertions(+), 16 deletions(-)
Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, approved Angel Pons: Looks good to me, but someone else must approve
diff --git a/src/cpu/amd/family_10h-family_15h/init_cpus.c b/src/cpu/amd/family_10h-family_15h/init_cpus.c index b8e11e4..2ddbc43 100644 --- a/src/cpu/amd/family_10h-family_15h/init_cpus.c +++ b/src/cpu/amd/family_10h-family_15h/init_cpus.c @@ -147,9 +147,6 @@ // here assume the OS don't change our apicid u32 ap_apicid;
- u8 nvram; - bool multicore; - u32 nodes; u32 disable_siblings; u32 cores_found; @@ -158,13 +155,12 @@ /* get_nodes define in ht_wrapper.c */ nodes = get_nodes();
- multicore = true; - if (get_option(&nvram, "multi_core") == CB_SUCCESS) - multicore = !!nvram; - - disable_siblings = 0; - if (!IS_ENABLED(CONFIG_LOGICAL_CPUS) || !multicore) + if (!IS_ENABLED(CONFIG_LOGICAL_CPUS) || + read_option(multi_core, 0) != 0) { // 0 means multi core disable_siblings = 1; + } else { + disable_siblings = 0; + }
for (i = 0; i < nodes; i++) { if ((node >= 0) && (i != node)) @@ -639,17 +635,11 @@ //it is running on core0 of node0 void start_other_cores(uint32_t bsp_apicid) { - u8 nvram; u32 nodes; u32 nodeid; - bool multicore;
// disable multi_core - multicore = true; - if (get_option(&nvram, "multi_core") == CB_SUCCESS) - multicore = !!nvram; - - if (!multicore) { + if (read_option(multi_core, 0) != 0) { printk(BIOS_DEBUG, "Skip additional core init\n"); return; }
-
Kyösti Mälkki (Code Review) -
Paul Menzel (Code Review)