coreboot-gerrit November 2021

coreboot-gerrit@coreboot.org

1 participants
6394 discussions

Change in coreboot[master]: mb/google/brya/variants/primus: enable RTD3 for PCIe-eMMC bridge
by build bot (Jenkins) (Code Review) 11 Nov '21

11 Nov '21

Attention is currently required from: Tim Wawrzynczak. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59135 ) Change subject: mb/google/brya/variants/primus: enable RTD3 for PCIe-eMMC bridge ...................................................................... Patch Set 1: (5 comments) File src/mainboard/google/brya/variants/primus/variant.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-132930): https://review.coreboot.org/c/coreboot/+/59135/comment/073f924c_7abfe6c0 PS1, Line 21: if (child->chip_ops == &soc_intel_common_block_pcie_rtd3_ops) suspect code indent for conditional statements (16, 16) Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-132930): https://review.coreboot.org/c/coreboot/+/59135/comment/8ab38cb5_3a90f1d5 PS1, Line 22: rtd3_dev = child; code indent should use tabs where possible Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-132930): https://review.coreboot.org/c/coreboot/+/59135/comment/cd94bc37_aa083338 PS1, Line 22: rtd3_dev = child; please, no spaces at the start of a line Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-132930): https://review.coreboot.org/c/coreboot/+/59135/comment/c66dcc5f_8f556b70 PS1, Line 28: rtd3_dev->enabled = 0; please, no spaces at the start of a line Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-132930): https://review.coreboot.org/c/coreboot/+/59135/comment/c54481cd_7c89ed15 PS1, Line 34: devtree_update_emmc_rtd3(board_ver); please, no spaces at the start of a line -- To view, visit https://review.coreboot.org/c/coreboot/+/59135 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5836d65cedfe3907af2c4c33de7a396c4bb8b727 Gerrit-Change-Number: 59135 Gerrit-PatchSet: 1 Gerrit-Owner: Malik Hsu <malik_hsu(a)wistron.corp-partner.google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Comment-Date: Thu, 11 Nov 2021 02:21:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/brya/variants/primus: enable RTD3 for PCIe-eMMC bridge
by Malik Hsu (Code Review) 11 Nov '21

11 Nov '21

Malik Hsu has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/59135 ) Change subject: mb/google/brya/variants/primus: enable RTD3 for PCIe-eMMC bridge ...................................................................... mb/google/brya/variants/primus: enable RTD3 for PCIe-eMMC bridge Enable RTD3 driver for PCIe-eMMC bridge, If the board version is less than 1, do not enable RTD3 driver. BUG=b:204469567 TEST=Boot into eMMC storage and perform suspend stress 100 cycle passed Signed-off-by: Malik_Hsu <malik_hsu(a)wistron.corp-partner.google.com> Change-Id: I5836d65cedfe3907af2c4c33de7a396c4bb8b727 --- M src/mainboard/google/brya/variants/primus/Makefile.inc M src/mainboard/google/brya/variants/primus/gpio.c M src/mainboard/google/brya/variants/primus/overridetree.cb A src/mainboard/google/brya/variants/primus/variant.c 4 files changed, 43 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/59135/1 diff --git a/src/mainboard/google/brya/variants/primus/Makefile.inc b/src/mainboard/google/brya/variants/primus/Makefile.inc index 129a293..536bdf3 100644 --- a/src/mainboard/google/brya/variants/primus/Makefile.inc +++ b/src/mainboard/google/brya/variants/primus/Makefile.inc @@ -3,3 +3,4 @@ romstage-y += gpio.c ramstage-y += gpio.c ramstage-$(CONFIG_FW_CONFIG) += fw_config.c +ramstage-y += variant.c \ No newline at end of file diff --git a/src/mainboard/google/brya/variants/primus/gpio.c b/src/mainboard/google/brya/variants/primus/gpio.c index c1dfee2..a324fa3 100644 --- a/src/mainboard/google/brya/variants/primus/gpio.c +++ b/src/mainboard/google/brya/variants/primus/gpio.c @@ -45,8 +45,6 @@ PAD_NC(GPP_E3, NONE), /* E7 : PROC_GP1 ==> NC */ PAD_NC(GPP_E7, NONE), - /* E20 : USB_C1_LSX_SOC_TX ==> EN_PP3300_eMMC */ - PAD_CFG_GPO(GPP_E20, 1, DEEP), /* F19 : SRCCLKREQ6# ==> EMMC_CLKREQ_ODL */ PAD_CFG_NF(GPP_F19, NONE, DEEP, NF1), @@ -107,6 +105,8 @@ PAD_CFG_GPO(GPP_E16, 0, DEEP), /* E15 : RSVD_TP ==> PCH_WP_OD */ PAD_CFG_GPI_GPIO_DRIVER(GPP_E15, NONE, DEEP), + /* E20 : USB_C1_LSX_SOC_TX ==> EN_PP3300_eMMC */ + PAD_CFG_GPO(GPP_E20, 1, DEEP), /* F21 : EXT_PWR_GATE2# ==> WWAN_FCPO_L (updated in romstage) */ PAD_CFG_GPO(GPP_F21, 0, DEEP), /* F18 : THC1_SPI2_INT# ==> EC_IN_RW_OD */ diff --git a/src/mainboard/google/brya/variants/primus/overridetree.cb b/src/mainboard/google/brya/variants/primus/overridetree.cb index c4debb4..0e6f451 100644 --- a/src/mainboard/google/brya/variants/primus/overridetree.cb +++ b/src/mainboard/google/brya/variants/primus/overridetree.cb @@ -119,6 +119,11 @@ end end device ref pcie_rp3 on + chip soc/intel/common/block/pcie/rtd3 + register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_B3)" + register "srcclk_pin" = "6" + device generic 0 on end + end # Enable PCIe-to-eMMC bridge PCIE 3 using clk 6 register "pch_pcie_rp[PCH_RP(3)]" = "{ .clk_src = 6, diff --git a/src/mainboard/google/brya/variants/primus/variant.c b/src/mainboard/google/brya/variants/primus/variant.c new file mode 100644 index 0000000..90f2f1f --- /dev/null +++ b/src/mainboard/google/brya/variants/primus/variant.c @@ -0,0 +1,35 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <console/console.h> +#include <baseboard/variants.h> +#include <boardid.h> +#include <soc/pci_devs.h> +#include <device/device.h> + +extern struct chip_operations soc_intel_common_block_pcie_rtd3_ops; + +static void devtree_update_emmc_rtd3(uint32_t board_ver) +{ + struct device *rtd3_dev = NULL, *child = NULL; + struct device *pcie_rp3_dev = pcidev_path_on_root(PCH_DEVFN_PCIE3); + + while ((child = dev_bus_each_child(pcie_rp3_dev->link_list, child)) != NULL) { + if (child->path.type != DEVICE_PATH_GENERIC) + continue; + if (child->path.generic.id != 0) + continue; + if (child->chip_ops == &soc_intel_common_block_pcie_rtd3_ops) + rtd3_dev = child; + } + + if (board_ver > 1) + return; + + rtd3_dev->enabled = 0; +} + +void variant_devtree_update(void) +{ + uint32_t board_ver = board_id(); + devtree_update_emmc_rtd3(board_ver); +} -- To view, visit https://review.coreboot.org/c/coreboot/+/59135 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5836d65cedfe3907af2c4c33de7a396c4bb8b727 Gerrit-Change-Number: 59135 Gerrit-PatchSet: 1 Gerrit-Owner: Malik Hsu <malik_hsu(a)wistron.corp-partner.google.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: security/tpm/tcg-2.0: Handle TPM_RC_NV_RANGE return code
by Karthik Ramasubramanian (Code Review) 11 Nov '21

11 Nov '21

Attention is currently required from: Christian Walter. Karthik Ramasubramanian has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/59134 ) Change subject: security/tpm/tcg-2.0: Handle TPM_RC_NV_RANGE return code ...................................................................... security/tpm/tcg-2.0: Handle TPM_RC_NV_RANGE return code As per the TPM spec, if offset and the size field of data add to a value that is greater than the dataSize field of the NV Index referenced by nvIndex, the TPM shall return an error (TPM_RC_NV_RANGE). When TPM returns this error, no data is read from the concerned NV Index. Handle the error and map it to an appropriate vboot error. BUG=None TEST=Build and boot to OS in Guybrush. Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com> Change-Id: I8b403e2f33cc1368065cc21f73df1102695f73eb --- M src/security/tpm/tss/tcg-2.0/tss.c M src/security/tpm/tss/tcg-2.0/tss_structures.h 2 files changed, 4 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/59134/1 diff --git a/src/security/tpm/tss/tcg-2.0/tss.c b/src/security/tpm/tss/tcg-2.0/tss.c index cfa533b..8404d25 100644 --- a/src/security/tpm/tss/tcg-2.0/tss.c +++ b/src/security/tpm/tss/tcg-2.0/tss.c @@ -242,6 +242,9 @@ case TPM_RC_CR50_NV_UNDEFINED: return TPM_E_BADINDEX; + case TPM_RC_NV_RANGE: + return TPM_E_READ_EMPTY; + default: return TPM_E_READ_FAILURE; } diff --git a/src/security/tpm/tss/tcg-2.0/tss_structures.h b/src/security/tpm/tss/tcg-2.0/tss_structures.h index cb8b4f9..c0e354d 100644 --- a/src/security/tpm/tss/tcg-2.0/tss_structures.h +++ b/src/security/tpm/tss/tcg-2.0/tss_structures.h @@ -144,6 +144,7 @@ /* Values copied from tpm2/tpm_types.h */ #define RC_VER1 0x100 #define TPM_RC_INITIALIZE ((TPM_RC)(RC_VER1 + 0x000)) +#define TPM_RC_NV_RANGE ((TPM_RC)(RC_VER1 + 0x046)) #define TPM_RC_NV_UNINITIALIZED ((TPM_RC)(RC_VER1 + 0x04A)) /* -- To view, visit https://review.coreboot.org/c/coreboot/+/59134 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8b403e2f33cc1368065cc21f73df1102695f73eb Gerrit-Change-Number: 59134 Gerrit-PatchSet: 1 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: security/vboot: Use default kernel secdata size
by Karthik Ramasubramanian (Code Review) 11 Nov '21

11 Nov '21

Attention is currently required from: Tim Wawrzynczak, Julius Werner, Andrey Pronin, Yu-Ping Wu, Karthik Ramasubramanian. Karthik Ramasubramanian has uploaded a new patch set (#2) to the change originally created by Tim Wawrzynczak. ( https://review.coreboot.org/c/coreboot/+/58669 ) Change subject: security/vboot: Use default kernel secdata size ...................................................................... security/vboot: Use default kernel secdata size When fetching antirollback information for the kernel, it is not always known ahead of time what the current size of the kernel secdata area is. If the incorrect size is passed, the TPM will return back the correct size, but at the cost of an extra transaction; when using cr50 over I2C, this can be as much as 20ms. Currently, the first attempt uses the minimium size (aka version 0 or 0.2), and if another size is used (which is the case for all modern cr50-based boards, version 1 or 1.0), then a transaction is wasted on every boot. Therefore, change the default size sent to the TPM to be the default one used in the VB2 API instead of the minimum one. BUG=b:201304784 TEST=verify TPM initialization time drops by ~20ms. Also the Kernel NV Index is read correctly in the BIOS logs. src/security/tpm/tss/tcg-2.0/tss.c:231 index 0x1007 return code 0 src/security/tpm/tss/tcg-2.0/tss.c:231 index 0x1008 return code 0 504:finished TPM initialization 99,953 (65,606) Change-Id: I22d9c0079bb1175f24ff7317d116e79aa5ba08ed Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com> --- M src/security/vboot/secdata_tpm.c 1 file changed, 15 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/58669/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/58669 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I22d9c0079bb1175f24ff7317d116e79aa5ba08ed Gerrit-Change-Number: 58669 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Andrey Pronin <apronin(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Andrey Pronin <apronin(a)chromium.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Andrey Pronin <apronin(a)chromium.org> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: libpayload: Add support for PCI MMIO CONFIG access.
by Shelley Chen (Code Review) 11 Nov '21

11 Nov '21

Attention is currently required from: Ravi kumar, Furquan Shaikh, Arthur Heymans, Prasad Malisetty. Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57615 ) Change subject: libpayload: Add support for PCI MMIO CONFIG access. ...................................................................... Patch Set 18: (1 comment) File payloads/libpayload/drivers/pci_ops.c: https://review.coreboot.org/c/coreboot/+/57615/comment/75948de3_c7ace065 PS17, Line 34: uintptr_t get_pci_mmio_cfgbase(pcidev_t dev) > ATU configuration needed for every config/mem read/write. […] Hi Prasad. Sorry if I was unclear. I meant that the code should be functionally equivalent to the code implemented in coreboot, specifically the pcicfg() function that I proposed which does the address translation that needs to be run for every read/write: https://review.coreboot.org/c/coreboot/+/58668/4/src/soc/qualcomm/sc7280/pc… My assumption is that in the future, you will need to map > 1 device at a time in the MMIO space. Is that not true? Also, please take a look at CB:57861 for the details on how we are reorganizing everything. -- To view, visit https://review.coreboot.org/c/coreboot/+/57615 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7d1217502cbd7d4d0cdd298919ae82435630d61c Gerrit-Change-Number: 57615 Gerrit-PatchSet: 18 Gerrit-Owner: Ravi kumar <rbokka(a)codeaurora.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Prasad Malisetty <pmaliset(a)qualcomm.corp-partner.google.com> Gerrit-CC: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Ravi kumar <rbokka(a)codeaurora.org> Gerrit-Attention: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Prasad Malisetty <pmaliset(a)qualcomm.corp-partner.google.com> Gerrit-Comment-Date: Thu, 11 Nov 2021 00:53:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Shelley Chen <shchen(a)google.com> Comment-In-Reply-To: Prasad Malisetty <pmaliset(a)qualcomm.corp-partner.google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: security/vboot: Add NVRAM counter for TPM 2.0
by Julius Werner (Code Review) 11 Nov '21

11 Nov '21

Attention is currently required from: Miriam Polzer, Andrey Pronin, Yu-Ping Wu. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59097 ) Change subject: security/vboot: Add NVRAM counter for TPM 2.0 ...................................................................... Patch Set 2: (7 comments) Patchset: PS2: I'm not really convinced this is necessary yet... please see my comment on your design doc. File src/security/vboot/antirollback.h: https://review.coreboot.org/c/coreboot/+/59097/comment/5c13a025_7e9b0683 PS2, Line 31: #define ENT_ROLLBACK_NV_INDEX 0x100e This should probably have COUNTER in the name (e.g. ENT_ROLLBACK_COUNTER_INDEX, analogous to ZTE_RMA_BYTES_COUNTER_INDEX). File src/security/vboot/secdata_tpm.c: https://review.coreboot.org/c/coreboot/+/59097/comment/974c97f6_ee675cb8 PS2, Line 113: const static TPMA_NV rw_counter_attributes = { I would suggest adding PPREAD/PPWRITE here as well even if your design doesn't need it, just in case it comes in handy in a future change. https://review.coreboot.org/c/coreboot/+/59097/comment/d2f1d5f8_bf7c0573 PS2, Line 116: .TPMA_NV_PLATFORMCREATE = 1, Similarly, might as well set WRITE_STCLEAR too. It shouldn't hurt, and it might come in handy if we want to change how we use this in the future. https://review.coreboot.org/c/coreboot/+/59097/comment/1270877f_7fb30699 PS2, Line 189: * Empty policy digest. What's this? AFAIK you should only need a policy if TPMA_NV_POLICY_DELETE is set, which it isn't for your space. (That means the space may be deleted with platform authorization, which I think should be fine in your use case.) For spaces that don't need a policy, just pass NULL, 0 to setup_space(). https://review.coreboot.org/c/coreboot/+/59097/comment/ff9c113b_069d4aae PS2, Line 374: This is where you should set up this space, guarded by if (CONFIG(CHROMEOS)) File src/security/vboot/vboot_logic.c: https://review.coreboot.org/c/coreboot/+/59097/comment/b0651228_1a7d008a PS2, Line 281: enterprise_rollback_create_counter(); Spaces should be created from factory_initialize_tpm(), not here. If you want to backport this to shipped devices you can make a change in the respective firmware branches to call it from somewhere else, but ToT should be written from the perspective of clean, new devices without hacks to migrate from an older state. -- To view, visit https://review.coreboot.org/c/coreboot/+/59097 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I511dba3b3461713ce20fb2bda9fced0fee6517e1 Gerrit-Change-Number: 59097 Gerrit-PatchSet: 2 Gerrit-Owner: Miriam Polzer <mpolzer(a)google.com> Gerrit-Reviewer: Andrey Pronin <apronin(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Miriam Polzer <mpolzer(a)google.com> Gerrit-Attention: Andrey Pronin <apronin(a)chromium.org> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Thu, 11 Nov 2021 00:42:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/guybrush: Make GPIO_69 default for SD_AUX_RESET_L
by Kevin Chiu (Code Review) 11 Nov '21

11 Nov '21

Attention is currently required from: Raul Rangel, Rob Barnes. Kevin Chiu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59053 ) Change subject: mb/google/guybrush: Make GPIO_69 default for SD_AUX_RESET_L ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/59053 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6546ad9961f6f7146aa3aefc35d39a2eb282a252 Gerrit-Change-Number: 59053 Gerrit-PatchSet: 3 Gerrit-Owner: Rob Barnes <robbarnes(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Kevin Chiu <kevin.chiu.17802(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Rob Barnes <robbarnes(a)google.com> Gerrit-Comment-Date: Thu, 11 Nov 2021 00:36:12 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: amdfwtool: Add ISH header support for A/B recovery layout
by Felix Held (Code Review) 11 Nov '21

11 Nov '21

Attention is currently required from: Bao Zheng, Jason Glenesk, Martin Roth, Marshall Dawson, Zheng Bao. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57747 ) Change subject: amdfwtool: Add ISH header support for A/B recovery layout ...................................................................... Patch Set 24: (8 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/57747/comment/521bccd0_5c9dda69 PS24, Line 11: EFS -> PSP L1 0x48 -> PSP L2 A -> ISH A -> BIOS L2 A shouldn't this be: EFS -> PSP L1 0x48 -> ISH A -> PSP L2 A -> BIOS L2 A ? same for the line below File util/amdfwtool/amdfwtool.h: https://review.coreboot.org/c/coreboot/+/57747/comment/9fa0d2c9_afb01629 PS24, Line 217: uint32_t glitch_retry_count; document #55758 Rev. 1.13 has one byte for this and 3 reserved bytes after that. since it's little endian, this shouldn't be much of a difference though. when splitting this into an uint8_t and a 3 element uint8_t array for the reserved bytes, it's probably a good idea to mark the struct as packed. also not sure if that document is up to date File util/amdfwtool/amdfwtool.c: https://review.coreboot.org/c/coreboot/+/57747/comment/bf00e831_2895c73e PS24, Line 394: 0x1000 would be good to have a define for this https://review.coreboot.org/c/coreboot/+/57747/comment/6d8c0c42_42d2b684 PS24, Line 396: 0x1000 would be good to have a define for this https://review.coreboot.org/c/coreboot/+/57747/comment/85e657d7_3504b369 PS24, Line 662: 0xf the specification i'm looking says that this should be 1 https://review.coreboot.org/c/coreboot/+/57747/comment/26be47f7_e1020627 PS24, Line 670: 0x2 the specification i'm looking says that this should be 1 https://review.coreboot.org/c/coreboot/+/57747/comment/f03be735_1d1bce62 PS24, Line 671: 0xf the specification i'm looking says that this should be 1 https://review.coreboot.org/c/coreboot/+/57747/comment/62d242fd_f732ed3d PS24, Line 686: 0x1000 would be good to have a define for this -- To view, visit https://review.coreboot.org/c/coreboot/+/57747 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib0690cde1dce949514c7aacebe13096b7814ceff Gerrit-Change-Number: 57747 Gerrit-PatchSet: 24 Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Zheng Bao Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Bao Zheng <zheng.bao(a)amd.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Zheng Bao Gerrit-Comment-Date: Thu, 11 Nov 2021 00:33:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: [NOTFORMERGE]link Agesa into bootblock
by build bot (Jenkins) (Code Review) 11 Nov '21

11 Nov '21

Attention is currently required from: Arthur Heymans, Kyösti Mälkki. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55070 ) Change subject: [NOTFORMERGE]link Agesa into bootblock ...................................................................... Patch Set 6: (1 comment) File src/drivers/amd/agesa/bootblock.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-132927): https://review.coreboot.org/c/coreboot/+/55070/comment/a14cbc1f_695afbe9 PS6, Line 47: void (*ap_romstage_entry)(void) = get_ap_entry_ptr(); function definition argument 'void' should also have an identifier name -- To view, visit https://review.coreboot.org/c/coreboot/+/55070 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic4c71b9c9a245e07d713839fb3628cbfc0dc3457 Gerrit-Change-Number: 55070 Gerrit-PatchSet: 6 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Comment-Date: Thu, 11 Nov 2021 00:22:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: [NOTFORMERGE]link Agesa into bootblock
by Arthur Heymans (Code Review) 11 Nov '21

11 Nov '21

Attention is currently required from: Arthur Heymans, Kyösti Mälkki. Arthur Heymans has uploaded a new patch set (#6) to the change originally created by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/55070 ) Change subject: [NOTFORMERGE]link Agesa into bootblock ...................................................................... [NOTFORMERGE]link Agesa into bootblock Untested but it does seem to build for AGESA targets. For AGESA targets the APs are started by the BSP in romstage and need to jump past the bootblock into the romstage. When there is a separate romstage this is complicated as the APs in boot need to know where to jump in romstage. This is done by setting a pointer in BIOSRAM. When the romstage is linked inside the bootblock there is no need for such tricks as the symbol for the AP jump address can directly be referenced. Change-Id: Ic4c71b9c9a245e07d713839fb3628cbfc0dc3457 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/arch/x86/Kconfig M src/arch/x86/bootblock.ld M src/arch/x86/car.ld M src/cpu/amd/agesa/Kconfig M src/drivers/amd/agesa/bootblock.c M src/drivers/amd/agesa/romstage.c M src/northbridge/amd/agesa/agesa_helper.h M src/vendorcode/amd/agesa/Makefile.inc 8 files changed, 28 insertions(+), 13 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/55070/6 -- To view, visit https://review.coreboot.org/c/coreboot/+/55070 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic4c71b9c9a245e07d713839fb3628cbfc0dc3457 Gerrit-Change-Number: 55070 Gerrit-PatchSet: 6 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2021