Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48282 )
Change subject: drivers/intel/fsp2_0: FSP-T requires NO_CBFS_MCACHE ......................................................................
drivers/intel/fsp2_0: FSP-T requires NO_CBFS_MCACHE
When FSP-T is used, the first thing done in postcar is to call FSP-M to tear down CAR. This is done before cbmem is initialized, which means CBFS_MCACHE is not accessible, which results in FSP-M not being found, failing the boot.
TESTED: ocp/deltalake boots again.
Change-Id: Icb41b802c636d42b0ebeb3e3850551813accda91 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/drivers/intel/fsp2_0/Kconfig 1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/48282/1
diff --git a/src/drivers/intel/fsp2_0/Kconfig b/src/drivers/intel/fsp2_0/Kconfig index 03b9c2b..96ae282 100644 --- a/src/drivers/intel/fsp2_0/Kconfig +++ b/src/drivers/intel/fsp2_0/Kconfig @@ -113,6 +113,7 @@ config FSP_CAR bool default n + select NO_CBFS_MCACHE help Use FSP APIs to initialize & Tear Down the Cache-As-Ram
Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48282 )
Change subject: drivers/intel/fsp2_0: FSP-T requires NO_CBFS_MCACHE ......................................................................
Patch Set 1: Code-Review+2
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48282 )
Change subject: drivers/intel/fsp2_0: FSP-T requires NO_CBFS_MCACHE ......................................................................
Patch Set 1: Code-Review+2
Hmm... okay let's land this for now, will have to think more about how to address this correctly. Is there a good reason the CAR teardown must happen before CBMEM reinit? Could we just flip the order of those?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48282 )
Change subject: drivers/intel/fsp2_0: FSP-T requires NO_CBFS_MCACHE ......................................................................
Patch Set 1: Code-Review+2
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48282 )
Change subject: drivers/intel/fsp2_0: FSP-T requires NO_CBFS_MCACHE ......................................................................
Patch Set 1:
Patch Set 1: Code-Review+2
Hmm... okay let's land this for now, will have to think more about how to address this correctly. Is there a good reason the CAR teardown must happen before CBMEM reinit? Could we just flip the order of those?
Hmm I would suspect that this works but just for performance reasons alone it's a bad idea because cbmem is UC so CAR should be torn down ASAP. Other than that I think it's bad to assume cacheability of cbmem in postcar in general. I have some patches that set it up as WB, which would break if cbmem is set up before teardown.
FSP-T just has to go, at least on FSP2.0 where it should be optional by design (WIP).
Arthur Heymans has submitted this change. ( https://review.coreboot.org/c/coreboot/+/48282 )
Change subject: drivers/intel/fsp2_0: FSP-T requires NO_CBFS_MCACHE ......................................................................
drivers/intel/fsp2_0: FSP-T requires NO_CBFS_MCACHE
When FSP-T is used, the first thing done in postcar is to call FSP-M to tear down CAR. This is done before cbmem is initialized, which means CBFS_MCACHE is not accessible, which results in FSP-M not being found, failing the boot.
TESTED: ocp/deltalake boots again.
Change-Id: Icb41b802c636d42b0ebeb3e3850551813accda91 Signed-off-by: Arthur Heymans arthur@aheymans.xyz Reviewed-on: https://review.coreboot.org/c/coreboot/+/48282 Reviewed-by: Christian Walter christian.walter@9elements.com Reviewed-by: Julius Werner jwerner@chromium.org Reviewed-by: Angel Pons th3fanbus@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/drivers/intel/fsp2_0/Kconfig 1 file changed, 1 insertion(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Julius Werner: Looks good to me, approved Angel Pons: Looks good to me, approved Christian Walter: Looks good to me, approved
diff --git a/src/drivers/intel/fsp2_0/Kconfig b/src/drivers/intel/fsp2_0/Kconfig index 03b9c2b..96ae282 100644 --- a/src/drivers/intel/fsp2_0/Kconfig +++ b/src/drivers/intel/fsp2_0/Kconfig @@ -113,6 +113,7 @@ config FSP_CAR bool default n + select NO_CBFS_MCACHE help Use FSP APIs to initialize & Tear Down the Cache-As-Ram
-
Angel Pons (Code Review) -
Arthur Heymans (Code Review)
-
Christian Walter (Code Review)
-
Julius Werner (Code Review)