Change in coreboot[master]: drivers/intel/fsp2_0: Do AP re-init after FSP-S if USE_INTEL_FSP_MP_I...

[Subrata Banik (Code Review)]

Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44077 )

Change subject: drivers/intel/fsp2_0: Do AP re-init after FSP-S if USE_INTEL_FSP_MP_INIT enable
......................................................................

drivers/intel/fsp2_0: Do AP re-init after FSP-S if USE_INTEL_FSP_MP_INIT enable

This patch ensures to take control of APs back by doing the full AP
re-initialization after FSP-S is done and control has been transferred
back to coreboot.

TEST=Able to see all cores available after booting to OS using below command
when coreboot is built with CONFIG_USE_INTEL_FSP_MP_INIT enable.
> cat /proc/cpuinfo

Without this CL :

shows only 1 core (only BSP)

With this CL :

shows all possible cores available (BSP + APs)

Change-Id: I247d8d1166c77bd01922323b6a0f14ec6640a666
Signed-off-by: Subrata Banik 
---
M src/drivers/intel/fsp2_0/silicon_init.c
1 file changed, 9 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/44077/1

diff --git a/src/drivers/intel/fsp2_0/silicon_init.c b/src/drivers/intel/fsp2_0/silicon_init.c
index 663b1d7..7b77590 100644
--- a/src/drivers/intel/fsp2_0/silicon_init.c
+++ b/src/drivers/intel/fsp2_0/silicon_init.c
@@ -7,6 +7,7 @@
 #include <console/console.h>
 #include <fsp/api.h>
 #include <fsp/util.h>
+#include <intelblocks/mp_init.h>
 #include <program_loading.h>
 #include <soc/intel/common/vbt.h>
 #include <stage_cache.h>
@@ -127,6 +128,14 @@
    fsp_debug_after_silicon_init(status);
    fsps_return_value_handler(FSP_SILICON_INIT_API, status);

+	/*
+	 * As per FSP integration guide:
+	 * If bootloader needs to take control of APs back the full AP re-initialization
+	 * required after FSP-S completed and control has been transferred back to bootloader
+	 */
+	if (CONFIG(USE_INTEL_FSP_MP_INIT))
+		init_cpus();
+
    if (!CONFIG(PLATFORM_USES_FSP2_2))
    	return;

[Subrata Banik (Code Review)]

Hello build bot (Jenkins), Andrey Petrov, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/44077

to look at the new patch set (#2).

Change subject: drivers/intel/fsp2_0: Do AP re-init after FSP-S if USE_INTEL_FSP_MP_INIT enable
......................................................................

drivers/intel/fsp2_0: Do AP re-init after FSP-S if USE_INTEL_FSP_MP_INIT enable

This patch ensures to take control of APs back by doing the full AP
re-initialization after FSP-S is done and control has been transferred
back to coreboot.

TEST=Able to see all cores available after booting to OS using below command
when coreboot is built with USE_INTEL_FSP_MP_INIT enable.
> cat /proc/cpuinfo

Without this CL :

shows only 1 core (only BSP)

With this CL :

shows all possible cores available (BSP + APs)

Change-Id: I247d8d1166c77bd01922323b6a0f14ec6640a666
Signed-off-by: Subrata Banik 
---
M src/drivers/intel/fsp2_0/silicon_init.c
1 file changed, 9 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/44077/2

[Subrata Banik (Code Review)]

Hello build bot (Jenkins), Aaron Durbin, Furquan Shaikh, Wonkyu Kim, Aamir Bohra, Andrey Petrov, Aaron Durbin, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/44077

to look at the new patch set (#3).

Change subject: drivers/intel/fsp2_0: Do AP re-init after FSP-S if USE_INTEL_FSP_MP_INIT enable
......................................................................

drivers/intel/fsp2_0: Do AP re-init after FSP-S if USE_INTEL_FSP_MP_INIT enable

This patch ensures to take control of APs back by doing the full AP
re-initialization after FSP-S is done and control has been transferred
back to coreboot.

TEST=Able to see all cores available after booting to OS using below command
when coreboot is built with USE_INTEL_FSP_MP_INIT enable.
> cat /proc/cpuinfo

Without this CL :

shows only 1 core (only BSP)

With this CL :

shows all possible cores available (BSP + APs)

Change-Id: I247d8d1166c77bd01922323b6a0f14ec6640a666
Signed-off-by: Subrata Banik 
---
M src/drivers/intel/fsp2_0/silicon_init.c
1 file changed, 11 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/44077/3

[Subrata Banik (Code Review)]

Hello build bot (Jenkins), Aaron Durbin, Furquan Shaikh, Wonkyu Kim, Aamir Bohra, Andrey Petrov, Aaron Durbin, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/44077

to look at the new patch set (#4).

Change subject: drivers/intel/fsp2_0: Do AP re-init after FSP-S if USE_INTEL_FSP_MP_INIT enable
......................................................................

drivers/intel/fsp2_0: Do AP re-init after FSP-S if USE_INTEL_FSP_MP_INIT enable

This patch ensures to take control of APs back by doing the full AP
re-initialization after FSP-S is done and control has been transferred
back to coreboot.

TEST=Able to see all cores available after booting to OS using below command
when coreboot is built with USE_INTEL_FSP_MP_INIT enable.
> cat /proc/cpuinfo

Without this CL :

shows only 1 core (only BSP)

With this CL :

shows all possible cores available (BSP + APs)

Change-Id: I247d8d1166c77bd01922323b6a0f14ec6640a666
Signed-off-by: Subrata Banik 
---
M src/drivers/intel/fsp2_0/silicon_init.c
1 file changed, 11 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/44077/4

[Subrata Banik (Code Review)]

Hello build bot (Jenkins), Aaron Durbin, Furquan Shaikh, Wonkyu Kim, Aamir Bohra, Andrey Petrov, Aaron Durbin, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/44077

to look at the new patch set (#5).

Change subject: drivers/intel/fsp2_0: Do AP re-init after FSP-S if USE_INTEL_FSP_MP_INIT enable
......................................................................

drivers/intel/fsp2_0: Do AP re-init after FSP-S if USE_INTEL_FSP_MP_INIT enable

This patch ensures to take control of APs back by doing the full AP
re-initialization after FSP-S is done and control has been transferred
back to coreboot.

TEST=Able to see all cores available after booting to OS using below command
when coreboot is built with USE_INTEL_FSP_MP_INIT enable.
> cat /proc/cpuinfo

Without this CL :

shows only 1 core (only BSP)

With this CL :

shows all possible cores available (BSP + APs)

Change-Id: I247d8d1166c77bd01922323b6a0f14ec6640a666
Signed-off-by: Subrata Banik 
---
M src/drivers/intel/fsp2_0/Makefile.inc
A src/drivers/intel/fsp2_0/fsp_mpinit.c
M src/drivers/intel/fsp2_0/include/fsp/api.h
M src/drivers/intel/fsp2_0/silicon_init.c
4 files changed, 26 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/44077/5

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44077 )

Change subject: drivers/intel/fsp2_0: Do AP re-init after FSP-S if USE_INTEL_FSP_MP_INIT enable
......................................................................

Patch Set 5: Code-Review+1

[Aamir Bohra (Code Review)]

Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44077 )

Change subject: drivers/intel/fsp2_0: Do AP re-init after FSP-S if USE_INTEL_FSP_MP_INIT enable
......................................................................

Patch Set 5: Code-Review+1

(2 comments)

https://review.coreboot.org/c/coreboot/+/44077/5/src/drivers/intel/fsp2_0/fs... 
File src/drivers/intel/fsp2_0/fsp_mpinit.c:

https://review.coreboot.org/c/coreboot/+/44077/5/src/drivers/intel/fsp2_0/fs... 
PS5, Line 8: Ps back the full AP re-initialization
           :  * required
back, A full AP re-initialization is required

https://review.coreboot.org/c/coreboot/+/44077/5/src/drivers/intel/fsp2_0/in... 
File src/drivers/intel/fsp2_0/include/fsp/api.h:

https://review.coreboot.org/c/coreboot/+/44077/5/src/drivers/intel/fsp2_0/in... 
PS5, Line 80: required
nit: back, A full AP re-initialization is required after FSP is completed

[Subrata Banik (Code Review)]

Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44077 )

Change subject: drivers/intel/fsp2_0: Do AP re-init after FSP-S if USE_INTEL_FSP_MP_INIT enable
......................................................................

Patch Set 5:

(2 comments)

https://review.coreboot.org/c/coreboot/+/44077/5/src/drivers/intel/fsp2_0/fs... 
File src/drivers/intel/fsp2_0/fsp_mpinit.c:

https://review.coreboot.org/c/coreboot/+/44077/5/src/drivers/intel/fsp2_0/fs... 
PS5, Line 8: Ps back the full AP re-initialization
           :  * required
> back, A full AP re-initialization is required

Ack

https://review.coreboot.org/c/coreboot/+/44077/5/src/drivers/intel/fsp2_0/in... 
File src/drivers/intel/fsp2_0/include/fsp/api.h:

https://review.coreboot.org/c/coreboot/+/44077/5/src/drivers/intel/fsp2_0/in... 
PS5, Line 80: required
> nit: back, A full AP re-initialization is required after FSP is completed

Ack

[Subrata Banik (Code Review)]

Hello build bot (Jenkins), Aaron Durbin, Patrick Georgi, Martin Roth, Furquan Shaikh, Wonkyu Kim, Angel Pons, Aamir Bohra, Andrey Petrov, Aaron Durbin, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/44077

to look at the new patch set (#6).

Change subject: drivers/intel/fsp2_0: Do AP re-init after FSP-S if USE_INTEL_FSP_MP_INIT enable
......................................................................

drivers/intel/fsp2_0: Do AP re-init after FSP-S if USE_INTEL_FSP_MP_INIT enable

This patch ensures that coreboot is able to take control of APs back
by doing a full AP re-initialization after FSP-S is done.

TEST=Able to see all cores available after booting to OS using below command
when coreboot is built with USE_INTEL_FSP_MP_INIT enable.
> cat /proc/cpuinfo

Without this CL :

shows only 1 core (only BSP)

With this CL :

shows all possible cores available (BSP + APs)

Change-Id: I247d8d1166c77bd01922323b6a0f14ec6640a666
Signed-off-by: Subrata Banik 
---
M src/drivers/intel/fsp2_0/Makefile.inc
A src/drivers/intel/fsp2_0/fsp_mpinit.c
M src/drivers/intel/fsp2_0/include/fsp/api.h
M src/drivers/intel/fsp2_0/silicon_init.c
4 files changed, 26 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/44077/6

[Subrata Banik (Code Review)]

Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44077 )

Change subject: drivers/intel/fsp2_0: Do AP re-init after FSP-S if USE_INTEL_FSP_MP_INIT enable
......................................................................

Patch Set 6:

Can we get some review here ?

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44077 )

Change subject: drivers/intel/fsp2_0: Do AP re-init after FSP-S if USE_INTEL_FSP_MP_INIT enable
......................................................................

Patch Set 6: Code-Review+2

[Subrata Banik (Code Review)]

Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44077 )

Change subject: drivers/intel/fsp2_0: Do AP re-init after FSP-S if USE_INTEL_FSP_MP_INIT enable
......................................................................

Patch Set 6:
> Patch Set 6:
>
> Can we get some review here ?

Thanks Patrick.

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44077 )

Change subject: drivers/intel/fsp2_0: Do AP re-init after FSP-S if USE_INTEL_FSP_MP_INIT enable
......................................................................

Patch Set 6: Code-Review+2

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44077 )

Change subject: drivers/intel/fsp2_0: Do AP re-init after FSP-S if USE_INTEL_FSP_MP_INIT enable
......................................................................

Patch Set 6: Code-Review+2

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44077 )

Change subject: drivers/intel/fsp2_0: Do AP re-init after FSP-S if USE_INTEL_FSP_MP_INIT enable
......................................................................

Patch Set 6: Code-Review+2

[Subrata Banik (Code Review)]

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44077 )

Change subject: drivers/intel/fsp2_0: Do AP re-init after FSP-S if USE_INTEL_FSP_MP_INIT enable
......................................................................

drivers/intel/fsp2_0: Do AP re-init after FSP-S if USE_INTEL_FSP_MP_INIT enable

This patch ensures that coreboot is able to take control of APs back
by doing a full AP re-initialization after FSP-S is done.

TEST=Able to see all cores available after booting to OS using below command
when coreboot is built with USE_INTEL_FSP_MP_INIT enable.
> cat /proc/cpuinfo

Without this CL :

shows only 1 core (only BSP)

With this CL :

shows all possible cores available (BSP + APs)

Change-Id: I247d8d1166c77bd01922323b6a0f14ec6640a666
Signed-off-by: Subrata Banik 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/44077
Tested-by: build bot (Jenkins) 
Reviewed-by: Patrick Rudolph 
Reviewed-by: Angel Pons 
Reviewed-by: Aaron Durbin 
Reviewed-by: Furquan Shaikh 
---
M src/drivers/intel/fsp2_0/Makefile.inc
A src/drivers/intel/fsp2_0/fsp_mpinit.c
M src/drivers/intel/fsp2_0/include/fsp/api.h
M src/drivers/intel/fsp2_0/silicon_init.c
4 files changed, 26 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Aaron Durbin: Looks good to me, approved
  Furquan Shaikh: Looks good to me, approved
  Patrick Rudolph: Looks good to me, approved
  Angel Pons: Looks good to me, approved

diff --git a/src/drivers/intel/fsp2_0/Makefile.inc b/src/drivers/intel/fsp2_0/Makefile.inc
index e954a46..32140f4 100644
--- a/src/drivers/intel/fsp2_0/Makefile.inc
+++ b/src/drivers/intel/fsp2_0/Makefile.inc
@@ -14,6 +14,7 @@
 romstage-y += cbmem.c

ramstage-y += debug.c
+ramstage-$(CONFIG_USE_INTEL_FSP_MP_INIT) += fsp_mpinit.c
 ramstage-$(CONFIG_RUN_FSP_GOP) += graphics.c
 ramstage-y += hand_off_block.c
 ramstage-$(CONFIG_DISPLAY_FSP_HEADER) += header_display.c
diff --git a/src/drivers/intel/fsp2_0/fsp_mpinit.c b/src/drivers/intel/fsp2_0/fsp_mpinit.c
new file mode 100644
index 0000000..cda9269
--- /dev/null
+++ b/src/drivers/intel/fsp2_0/fsp_mpinit.c
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <fsp/api.h>
+#include <intelblocks/mp_init.h>
+
+/*
+ * As per FSP integration guide:
+ * If bootloader needs to take control of APs back, a full AP re-initialization is
+ * required after FSP-S is completed and control has been transferred back to bootloader
+ */
+void do_mpinit_after_fsp(void)
+{
+	init_cpus();
+}
diff --git a/src/drivers/intel/fsp2_0/include/fsp/api.h b/src/drivers/intel/fsp2_0/include/fsp/api.h
index d2c556f..e0cd96d 100644
--- a/src/drivers/intel/fsp2_0/include/fsp/api.h
+++ b/src/drivers/intel/fsp2_0/include/fsp/api.h
@@ -75,6 +75,13 @@
    struct mma_config_param *mma_cfg);

/*
+ * As per FSP integration guide:
+ * If bootloader needs to take control of APs back, a full AP re-initialization is
+ * required after FSP-S is completed and control has been transferred back to bootloader
+ */
+void do_mpinit_after_fsp(void);
+
+/*
  * # DOCUMENTATION:
  *
  * This file defines the interface between coreboot and the FSP 2.0 wrapper
diff --git a/src/drivers/intel/fsp2_0/silicon_init.c b/src/drivers/intel/fsp2_0/silicon_init.c
index 663b1d7..0b6540e 100644
--- a/src/drivers/intel/fsp2_0/silicon_init.c
+++ b/src/drivers/intel/fsp2_0/silicon_init.c
@@ -127,6 +127,10 @@
    fsp_debug_after_silicon_init(status);
    fsps_return_value_handler(FSP_SILICON_INIT_API, status);

+	/* Reinitialize CPUs if FSP-S has done MP Init */
+	if (CONFIG(USE_INTEL_FSP_MP_INIT))
+		do_mpinit_after_fsp();
+
    if (!CONFIG(PLATFORM_USES_FSP2_2))
    	return;