coreboot-gerrit November 2019

coreboot-gerrit@coreboot.org

1 participants
2418 discussions

Change in coreboot[master]: soc/intel/common/block/cse: Add boot partition related APIs
by Sridhar Siricilla (Code Review) 26 Nov '19

26 Nov '19

Hello Patrick Rudolph, Subrata Banik, Balaji Manigandan, Aamir Bohra, Rizwan Qureshi, V Sowmya, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth, Furquan Shaikh, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/35402 to look at the new patch set (#32). Change subject: soc/intel/common/block/cse: Add boot partition related APIs ...................................................................... soc/intel/common/block/cse: Add boot partition related APIs The CSE region is logically divided into 3 boot partitions when redundancy is enabled. These boot partitions are represented by BP1, BP2 and BP3. In chrome, CSE can boot from either BP1 or BP2. The CSE image layout appears as below.. ------------- ------------------ -------------------------- |CSE REGION | => | RO | RW | DATA | => | BP1 | BP2 + BP3 | DATA | ------------- ------------------ -------------------------- In order to support CSE FW update to RW region, below APIs help coreboot to get info about the boot partitions, and allows the coreboot to set CSE to boot from required boot partition(either BP1(RO) or BP2). GET_BOOT_PARTITION_INFO - provides info on available partitions in the CSE region. The API provides info on boot partitions like start/end offsets of a partition within CSE region, and their version and partition status. SET_BOOT_PARTITION_INFO - Sets the next boot partition to boot for CSE. With the HECI API, firmware can notify CSE to boot from BP1 or BP2 on next boot. Change-Id: Iaa62409c0616d5913d21374a8a6804f82258eb4f Signed-off-by: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Signed-off-by: Sridhar Siricilla <sridhar.siricilla(a)intel.com> --- M src/soc/intel/common/block/cse/Makefile.inc A src/soc/intel/common/block/cse/cse_bp.c M src/soc/intel/common/block/include/intelblocks/cse.h 3 files changed, 490 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/35402/32 -- To view, visit https://review.coreboot.org/c/coreboot/+/35402 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iaa62409c0616d5913d21374a8a6804f82258eb4f Gerrit-Change-Number: 35402 Gerrit-PatchSet: 32 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Andrey Petrov <anpetrov(a)fb.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Sridhar Siricilla <sridhar.siricilla(a)intel.corp-partner.google.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/intel/common/block/cse: Minor cleanup for some late comments
by Paul Menzel (Code Review) 26 Nov '19

26 Nov '19

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35546 ) Change subject: soc/intel/common/block/cse: Minor cleanup for some late comments ...................................................................... Patch Set 10: Code-Review-1 (3 comments) A list of changes in a commit often indicates the commit should be split up into smaller commits. https://review.coreboot.org/c/coreboot/+/35546/9//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/35546/9//COMMIT_MSG@7 PS9, Line 7: soc/intel/common/block/cse: Minor cleanup for some late comments Statement: > Clean up some late comments https://review.coreboot.org/c/coreboot/+/35546/9//COMMIT_MSG@15 PS9, Line 15: * Log some messages. Separate commit please. https://review.coreboot.org/c/coreboot/+/35546/9//COMMIT_MSG@16 PS9, Line 16: Fixup Fix up -- To view, visit https://review.coreboot.org/c/coreboot/+/35546 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If7ea6043d7b5473d0c16e83d7b2d4b620c125652 Gerrit-Change-Number: 35546 Gerrit-PatchSet: 10 Gerrit-Owner: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Rizwan Qureshi <riz.pro(a)gmail.com> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 26 Nov 2019 14:55:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common/block/cse: Add boot partition related APIs
by Sridhar Siricilla (Code Review) 26 Nov '19

26 Nov '19

Hello Patrick Rudolph, Subrata Banik, Balaji Manigandan, Aamir Bohra, Rizwan Qureshi, V Sowmya, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth, Furquan Shaikh, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/35402 to look at the new patch set (#31). Change subject: soc/intel/common/block/cse: Add boot partition related APIs ...................................................................... soc/intel/common/block/cse: Add boot partition related APIs The CSE region is logically divided into 3 boot partitions when redundancy is enabled. These boot partitions are represented by BP1, BP2 and BP3. In chrome, CSE can boot from either BP1 or BP2. The CSE image layout appears as below.. ------------- ------------------ -------------------------- |CSE REGION | => | RO | RW | DATA | => | BP1 | BP2 + BP3 | DATA | ------------- ------------------ -------------------------- In order to support CSE FW update to RW region, below APIs help coreboot to get info about the boot partitions, and allows the coreboot to set CSE to boot from required boot partition(either BP1(RO) or BP2). GET_BOOT_PARTITION_INFO - provides info on available partitions in the CSE region. The API provides info on boot partitions like start/end offsets of a partition within CSE region, and their version and partition status. SET_BOOT_PARTITION_INFO - Sets the next boot partition to boot for CSE. With the HECI API, firmware can notify CSE to boot from BP1 or BP2 on next boot. Change-Id: Iaa62409c0616d5913d21374a8a6804f82258eb4f Signed-off-by: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Signed-off-by: Sridhar Siricilla <sridhar.siricilla(a)intel.com> --- M src/soc/intel/common/block/cse/Makefile.inc A src/soc/intel/common/block/cse/cse_bp.c M src/soc/intel/common/block/include/intelblocks/cse.h 3 files changed, 491 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/35402/31 -- To view, visit https://review.coreboot.org/c/coreboot/+/35402 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iaa62409c0616d5913d21374a8a6804f82258eb4f Gerrit-Change-Number: 35402 Gerrit-PatchSet: 31 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Andrey Petrov <anpetrov(a)fb.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Sridhar Siricilla <sridhar.siricilla(a)intel.corp-partner.google.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/intel/common/block/cse: Minor cleanup for some late comments
by Sridhar Siricilla (Code Review) 26 Nov '19

26 Nov '19

Sridhar Siricilla has uploaded a new patch set (#10) to the change originally created by Rizwan Qureshi. ( https://review.coreboot.org/c/coreboot/+/35546 ) Change subject: soc/intel/common/block/cse: Minor cleanup for some late comments ...................................................................... soc/intel/common/block/cse: Minor cleanup for some late comments Made some changes based on comments received on merged patches. * Consistent HECI command/group ID naming * Allow sending HMRFPO_ENABLE Command only when CSE is in Software Temporay Disable Mode. * Add description for some macros, functions, structure members. * Rename set_host_ready() to cse_set_host_ready() * Log some messages. * Fixup HFSTS1 inconsistency between APL/GKL and rest of the platforms. * Correct comment mentioned for wrapper #ifndef TEST=Build and Boot hatch board. Change-Id: If7ea6043d7b5473d0c16e83d7b2d4b620c125652 Signed-off-by: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Signed-off-by: Sridhar Siricilla <sridhar.siricilla(a)intel.com> --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 56 insertions(+), 29 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/35546/10 -- To view, visit https://review.coreboot.org/c/coreboot/+/35546 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If7ea6043d7b5473d0c16e83d7b2d4b620c125652 Gerrit-Change-Number: 35546 Gerrit-PatchSet: 10 Gerrit-Owner: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Rizwan Qureshi <riz.pro(a)gmail.com> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/intel/common/block/cse: Minor cleanup for some late comments
by build bot (Jenkins) (Code Review) 26 Nov '19

26 Nov '19

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35546 ) Change subject: soc/intel/common/block/cse: Minor cleanup for some late comments ...................................................................... Patch Set 9: (2 comments) https://review.coreboot.org/c/coreboot/+/35546/9/src/soc/intel/common/block… File src/soc/intel/common/block/cse/cse.c: https://review.coreboot.org/c/coreboot/+/35546/9/src/soc/intel/common/block… PS9, Line 663: /* Allow HMRFPO_ENABLE command only when ME's operation mode is ME_HFS_TEMP_DISABLE mode */ line over 96 characters https://review.coreboot.org/c/coreboot/+/35546/9/src/soc/intel/common/block… File src/soc/intel/common/block/include/intelblocks/cse.h: https://review.coreboot.org/c/coreboot/+/35546/9/src/soc/intel/common/block… PS9, Line 135: * - Send HMRFPO_ENABLE command to CSE please, no space before tabs -- To view, visit https://review.coreboot.org/c/coreboot/+/35546 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If7ea6043d7b5473d0c16e83d7b2d4b620c125652 Gerrit-Change-Number: 35546 Gerrit-PatchSet: 9 Gerrit-Owner: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Rizwan Qureshi <riz.pro(a)gmail.com> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 26 Nov 2019 14:36:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common/block/cse: Minor cleanup for some late comments
by Sridhar Siricilla (Code Review) 26 Nov '19

26 Nov '19

Sridhar Siricilla has uploaded a new patch set (#9) to the change originally created by Rizwan Qureshi. ( https://review.coreboot.org/c/coreboot/+/35546 ) Change subject: soc/intel/common/block/cse: Minor cleanup for some late comments ...................................................................... soc/intel/common/block/cse: Minor cleanup for some late comments Made some changes based on comments received on merged patches. * Consistent HECI command/group ID naming * Allow sending HMRFPO_ENABLE Command only when CSE is in Software Temporay Disable Mode. * Add description for some macros, functions, structure members. * Rename set_host_ready() to cse_set_host_ready() * Log some messages. * Fixup HFSTS1 inconsistency between APL/GKL and rest of the platforms. * Correct comment mentioned for wrapper #ifndef TEST=Build and Boot hatch board. Change-Id: If7ea6043d7b5473d0c16e83d7b2d4b620c125652 Signed-off-by: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Signed-off-by: Sridhar Siricilla <sridhar.siricilla(a)intel.com> --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 56 insertions(+), 29 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/35546/9 -- To view, visit https://review.coreboot.org/c/coreboot/+/35546 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If7ea6043d7b5473d0c16e83d7b2d4b620c125652 Gerrit-Change-Number: 35546 Gerrit-PatchSet: 9 Gerrit-Owner: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Rizwan Qureshi <riz.pro(a)gmail.com> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-MessageType: newpatchset

1 0

Change in vboot[master]: build error solve KLOCWORK
by Ronak Kanabar (Code Review) 26 Nov '19

26 Nov '19

Ronak Kanabar has uploaded this change for review. ( https://review.coreboot.org/c/vboot/+/35485 ) Change subject: build error solve KLOCWORK ...................................................................... build error solve KLOCWORK Change-Id: I45bbf1250bff08435b6d7f75b8d84d960a1fe4e4 Signed-off-by: Ronak Kanabar <ronak.kanabar(a)intel.com> --- M Makefile 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/vboot refs/changes/85/35485/1 diff --git a/Makefile b/Makefile index fa63773..407dbd9 100644 --- a/Makefile +++ b/Makefile @@ -142,7 +142,7 @@ -Wundef -Wmissing-prototypes -Wno-trigraphs -Wredundant-decls -Wshadow \ -Wwrite-strings -Wstrict-aliasing -Wdate-time -Wno-unknown-warning \ -Wno-address-of-packed-member -ffunction-sections -fdata-sections \ - -Wimplicit-fallthrough ${DEBUG_FLAGS} + ${DEBUG_FLAGS} # Note: FIRMWARE_ARCH is defined by the Chromium OS ebuild. ifeq (${FIRMWARE_ARCH},arm) -- To view, visit https://review.coreboot.org/c/vboot/+/35485 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: vboot Gerrit-Branch: master Gerrit-Change-Id: I45bbf1250bff08435b6d7f75b8d84d960a1fe4e4 Gerrit-Change-Number: 35485 Gerrit-PatchSet: 1 Gerrit-Owner: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-MessageType: newchange

1 4

Change in coreboot[master]: binaryPI: Drop CAR teardown without POSTCAR_STAGE
by Arthur Heymans (Code Review) 26 Nov '19

26 Nov '19

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/18526 ) Change subject: binaryPI: Drop CAR teardown without POSTCAR_STAGE ...................................................................... Patch Set 11: (1 comment) https://review.coreboot.org/c/coreboot/+/18526/8/src/drivers/amd/agesa/exit… File src/drivers/amd/agesa/exit_car.S: https://review.coreboot.org/c/coreboot/+/18526/8/src/drivers/amd/agesa/exit… PS8, Line 32: /* enable cache */ : movl %cr0, %eax : andl $(~(CR0_CD | CR0_NW)), %eax : movl %eax, %cr0 > > It will be followup if we find this is redunant now. > > FWIW the code in cache_as_ram.S did not do this either on the postcar path. CB:37179 removed it in soc/amd/common. -- To view, visit https://review.coreboot.org/c/coreboot/+/18526 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I693c104c3aab3be537c00695cbd764a48bd603b0 Gerrit-Change-Number: 18526 Gerrit-PatchSet: 11 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Marc Jones <marc(a)marcjonesconsulting.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 26 Nov 2019 12:38:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: binaryPI: Drop CAR teardown without POSTCAR_STAGE
by Michał Żygowski (Code Review) 26 Nov '19

26 Nov '19

Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/18526 ) Change subject: binaryPI: Drop CAR teardown without POSTCAR_STAGE ...................................................................... Patch Set 11: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/18526 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I693c104c3aab3be537c00695cbd764a48bd603b0 Gerrit-Change-Number: 18526 Gerrit-PatchSet: 11 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Marc Jones <marc(a)marcjonesconsulting.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 26 Nov 2019 12:36:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: [WIP] amd/stoneyridge: Fix CAR stack location
by Kyösti Mälkki (Code Review) 26 Nov '19

26 Nov '19

Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34883 ) Change subject: [WIP] amd/stoneyridge: Fix CAR stack location ...................................................................... [WIP] amd/stoneyridge: Fix CAR stack location As far as I can see, for BSP, gcccar.inc exits with %esp at _car_region_end, not _car_stack_end. On exit from ENABLE_AMD_STACK, %esp will have value of DCACHE_RAM_BASE + DCACHE_RAM_SIZE. This was correct for the stack placement with C_ENVIRONMENT_BOOTBLOCK=n. It would be nice if AMD code had guards so we knew how much stack it needs. Setting of DCACHE_BSP_STACK_SIZE works with inverse logic now, the higher you set it, the less space is where the stack is located. This raises the question how its size was determined. 00030000 B _car_region_start 00030000 B _car_stack_start 00034000 B _car_stack_end 00034000 B _preram_cbmem_console 00035600 B _car_relocatable_data_start 00035850 B _car_global_start 00035860 B _car_global_end 00035860 B _car_relocatable_data_end 00040000 B _car_region_end Use the symbol _car_stack_end to have early stage use the location assigned by the linker script car.ld. Change-Id: Id91393f1e7faf86b01fdc113e7940893673a27a7 Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- M src/drivers/amd/agesa/cache_as_ram.S M src/soc/amd/common/block/cpu/car/cache_as_ram.S 2 files changed, 12 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/34883/1 diff --git a/src/drivers/amd/agesa/cache_as_ram.S b/src/drivers/amd/agesa/cache_as_ram.S index 4f0bb3f..c23939c 100644 --- a/src/drivers/amd/agesa/cache_as_ram.S +++ b/src/drivers/amd/agesa/cache_as_ram.S @@ -31,6 +31,8 @@ .globl _cache_as_ram_setup, _cache_as_ram_setup_end .globl chipset_teardown_car +#if ENV_ROMSTAGE + _cache_as_ram_setup: /* Preserve BIST. */ @@ -49,6 +51,10 @@ AMD_ENABLE_STACK + /* Let arch/x86/car.ld determine our stack location. + * FIXME: Only do this for BSP. !! */ + movl $_car_stack_end, %esp + /* Align the stack. */ and $0xFFFFFFF0, %esp @@ -107,6 +113,8 @@ pushl %eax call romstage_main +#endif /* ENV_ROMSTAGE */ + #if CONFIG(POSTCAR_STAGE) /* We do not return. Execution continues with run_postcar_phase() diff --git a/src/soc/amd/common/block/cpu/car/cache_as_ram.S b/src/soc/amd/common/block/cpu/car/cache_as_ram.S index 402da3a..ad0ea53 100644 --- a/src/soc/amd/common/block/cpu/car/cache_as_ram.S +++ b/src/soc/amd/common/block/cpu/car/cache_as_ram.S @@ -38,6 +38,10 @@ AMD_ENABLE_STACK + /* Let arch/x86/car.ld determine our stack location. + * FIXME: Only do this for BSP. !! */ + movl $_car_stack_end, %esp + /* Align the stack and keep aligned for call to bootblock_c_entry() */ and $0xfffffff0, %esp sub $8, %esp -- To view, visit https://review.coreboot.org/c/coreboot/+/34883 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id91393f1e7faf86b01fdc113e7940893673a27a7 Gerrit-Change-Number: 34883 Gerrit-PatchSet: 1 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-MessageType: newchange

4 11

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2019