coreboot-gerrit January 2020

coreboot-gerrit@coreboot.org

1 participants
1869 discussions

Change in coreboot[master]: soc/intel/tigerlake: Update GPIO config
by Shaunak Saha (Code Review) 09 Jan '20

09 Jan '20

Shaunak Saha has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37427 ) Change subject: soc/intel/tigerlake: Update GPIO config ...................................................................... Patch Set 16: (17 comments) https://review.coreboot.org/c/coreboot/+/37427/16//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/37427/16//COMMIT_MSG@9 PS16, Line 9: tigerlake > Tiger Lake Ack https://review.coreboot.org/c/coreboot/+/37427/16//COMMIT_MSG@13 PS16, Line 13: > Once you have verified with the kernel driver, I would recommend adding all the details about what k […] Ack https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/a… File src/soc/intel/tigerlake/acpi/gpio.asl: https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/a… PS16, Line 19: > nit: extra blank line not required. Ack https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/a… PS16, Line 33: GPIO_IRQ14 > Along with this, you also need to set FSP param GpioIrqRoute. Ack https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/a… PS16, Line 107: Local2 = PCRB (Local0) : Local2 += PAD_CFG_BASE > This and the next match can be done in a single line? […] Will check. https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/a… PS16, Line 109: /* Return (Add (Local2, Multiply (Local1, 16))) */ > Commented code can be removed. Ack https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/a… PS16, Line 124: >>= > I don't think this is correct. […] Ack https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/a… PS16, Line 129: > 1 extra blank line not required. Ack https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/a… PS16, Line 160: > 2 extra blank lines not required. Ack https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/g… File src/soc/intel/tigerlake/gpio.c: https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/g… PS16, Line 94: .gpi_int_sts_reg_0 = GPI_INT_STS_0, : .gpi_int_en_reg_0 = GPI_INT_EN_0, > Why are these removed? Ack. As mentioned in gpio_def.h i will bring this part back. https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/g… PS16, Line 197: { PMC_GPP_G, GPP_G }, > Why was this removed? Is it not possible to map that to GPE? I dont see these groups in EDS.SO i would guess for this version of silicon its not possible. If in other version of silicon these groups come back we can put this back here with proper indexing. https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/i… File src/soc/intel/tigerlake/include/soc/gpio_defs.h: https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/i… PS16, Line 268: #define GPI_INT_STS_0 0x100 : #define GPI_INT_EN_0 0x110 > I am confused. […] This is removed in between version 9-10, not sure why.I see we would need these to clear the pms registers if needed. Will bring this one back. https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/i… PS16, Line 119: * > space before * Ack https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/i… File src/soc/intel/tigerlake/include/soc/gpio_soc_defs.h: https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/i… PS16, Line 35: 26 > Why is this set to 26? This should be the max number of pins in any group we have. but i see VGPIO have 27. so this should be 27 actually. https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/i… PS16, Line 232: 171 > I am still skeptical about the GPD group numbering here. […] With the present kernel code i can still see only 67(0-66) pins.I will try with the 5.4 kernel pinctrl driver.But i guess that kernel problem is different from the GPD here as this is com2, if some mismatch hapens atleast kernel should have read the com 0 and 1. but agreed with you regarding this can break the kernel mapping. Will debug more. https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/i… PS16, Line 376: TOTAL_GPIO_COMM 5 > If we are going with this, I would recommend at least having macros for the communities: […] Ack https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/i… PS16, Line 379: /* > I don't think we need the pinmux settings. […] ACK.Will remove this part. -- To view, visit https://review.coreboot.org/c/coreboot/+/37427 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I585100375feee39b5a9105bdf6d9f5ca3a5bb2fa Gerrit-Change-Number: 37427 Gerrit-PatchSet: 16 Gerrit-Owner: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org> Gerrit-Reviewer: Francois Toguo Fotso <francois.toguo.fotso(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com> Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Shaunak Saha <shaunak.saha(a)intel.com> Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Venkata Krishna Nimmagadda Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Thu, 09 Jan 2020 06:37:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)users.sourceforge.net> Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/tigerlake: Update GPIO config
by Furquan Shaikh (Code Review) 09 Jan '20

09 Jan '20

1 0

Change in coreboot[master]: cpu/intel/microcode: Apply more strict guard for assembly files
by Kyösti Mälkki (Code Review) 08 Jan '20

08 Jan '20

Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37891 ) Change subject: cpu/intel/microcode: Apply more strict guard for assembly files ...................................................................... cpu/intel/microcode: Apply more strict guard for assembly files Change-Id: I8243be7c9a57402b2ac1cfa1c0552990d4a4ba74 Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- M src/arch/x86/Makefile.inc M src/cpu/intel/microcode/Makefile.inc M src/drivers/intel/fsp1_1/Kconfig 3 files changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/37891/1 diff --git a/src/arch/x86/Makefile.inc b/src/arch/x86/Makefile.inc index 9486bc0..6ed93e5 100644 --- a/src/arch/x86/Makefile.inc +++ b/src/arch/x86/Makefile.inc @@ -124,7 +124,7 @@ $(eval $(call early_x86_stage,bootblock,elf64-x86-64)) endif -bootblock-$(CONFIG_ARCH_BOOTBLOCK_X86_32) += walkcbfs.S +bootblock-$(CONFIG_MICROCODE_UPDATE_PRE_RAM) += walkcbfs.S endif # CONFIG_ARCH_BOOTBLOCK_X86_32 / CONFIG_ARCH_BOOTBLOCK_X86_64 diff --git a/src/cpu/intel/microcode/Makefile.inc b/src/cpu/intel/microcode/Makefile.inc index b13172e..119c38f 100644 --- a/src/cpu/intel/microcode/Makefile.inc +++ b/src/cpu/intel/microcode/Makefile.inc @@ -1,5 +1,4 @@ bootblock-$(CONFIG_MICROCODE_UPDATE_PRE_RAM) += microcode_asm.S -romstage-$(CONFIG_MICROCODE_UPDATE_PRE_RAM) += microcode_asm.S bootblock-$(CONFIG_SUPPORT_CPU_UCODE_IN_CBFS) += microcode.c ramstage-$(CONFIG_SUPPORT_CPU_UCODE_IN_CBFS) += microcode.c diff --git a/src/drivers/intel/fsp1_1/Kconfig b/src/drivers/intel/fsp1_1/Kconfig index 93af4f7..7c69888 100644 --- a/src/drivers/intel/fsp1_1/Kconfig +++ b/src/drivers/intel/fsp1_1/Kconfig @@ -15,6 +15,7 @@ bool select UEFI_2_4_BINDING select INTEL_GMA_ADD_VBT if RUN_FSP_GOP + select MICROCODE_UPDATE_PRE_RAM help Does the code require the Intel Firmware Support Package? -- To view, visit https://review.coreboot.org/c/coreboot/+/37891 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8243be7c9a57402b2ac1cfa1c0552990d4a4ba74 Gerrit-Change-Number: 37891 Gerrit-PatchSet: 1 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-MessageType: newchange

3 3

Change in coreboot[master]: nb/intel/nehalem: Fix typo
by HAOUAS Elyes (Code Review) 08 Jan '20

08 Jan '20

HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38246 ) Change subject: nb/intel/nehalem: Fix typo ...................................................................... nb/intel/nehalem: Fix typo Change-Id: If4b47ccef2f0aa5396b6add5ab2f768b36b2ab5c Signed-off-by: Elyes HAOUAS <ehaouas(a)noos.fr> --- M src/northbridge/intel/nehalem/raminit.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/38246/1 diff --git a/src/northbridge/intel/nehalem/raminit.c b/src/northbridge/intel/nehalem/raminit.c index 215e9b8..ae51b4a 100644 --- a/src/northbridge/intel/nehalem/raminit.c +++ b/src/northbridge/intel/nehalem/raminit.c @@ -1509,7 +1509,7 @@ else gav(info->uma_enabled = pci_read_config8(NORTHBRIDGE, D0F0_DEVEN) & 8); - /* Unrecognised: [0000:fffd3d2d] 37f81.37f82 ! CPUID: eax: 00000001; ecx: 00000e00 => 00020655.00010800.029ae3ff.bfebfbff */ + /* Unrecognized: [0000:fffd3d2d] 37f81.37f82 ! CPUID: eax: 00000001; ecx: 00000e00 => 00020655.00010800.029ae3ff.bfebfbff */ info->silicon_revision = 0; if (capid0[2] & 2) { -- To view, visit https://review.coreboot.org/c/coreboot/+/38246 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If4b47ccef2f0aa5396b6add5ab2f768b36b2ab5c Gerrit-Change-Number: 38246 Gerrit-PatchSet: 1 Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-MessageType: newchange

5 6

Change in coreboot[master]: soc/intel/tigerlake: Update Kconfig
by Furquan Shaikh (Code Review) 08 Jan '20

08 Jan '20

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37426 ) Change subject: soc/intel/tigerlake: Update Kconfig ...................................................................... Patch Set 16: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/37426 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I04020d55f1063d521b15f8d0dabbd6f1dabf577c Gerrit-Change-Number: 37426 Gerrit-PatchSet: 16 Gerrit-Owner: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org> Gerrit-Reviewer: Francois Toguo Fotso <francois.toguo.fotso(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com> Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Shaunak Saha <shaunak.saha(a)intel.com> Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Venkata Krishna Nimmagadda Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.corp-partner.google.com> Gerrit-Comment-Date: Wed, 08 Jan 2020 20:17:34 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/tigerlake: Update GPIO config
by Furquan Shaikh (Code Review) 08 Jan '20

08 Jan '20

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37427 ) Change subject: soc/intel/tigerlake: Update GPIO config ...................................................................... Patch Set 16: (17 comments) https://review.coreboot.org/c/coreboot/+/37427/16//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/37427/16//COMMIT_MSG@13 PS16, Line 13: Once you have verified with the kernel driver, I would recommend adding all the details about what kernel driver version was used for testing, what the expectations are, etc. https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/a… File src/soc/intel/tigerlake/acpi/gpio.asl: https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/a… PS16, Line 19: nit: extra blank line not required. https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/a… PS16, Line 33: GPIO_IRQ14 Along with this, you also need to set FSP param GpioIrqRoute. https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/a… PS16, Line 107: Local2 = PCRB (Local0) : Local2 += PAD_CFG_BASE This and the next match can be done in a single line? Local2 = PCRB(Local0) + PAD_CFG_BASE + Local1 * 16 Return Local2 https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/a… PS16, Line 109: /* Return (Add (Local2, Multiply (Local1, 16))) */ Commented code can be removed. https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/a… PS16, Line 124: >>= I don't think this is correct. This should just be >> https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/a… PS16, Line 124: ( Are the braces around the and ops really required? https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/a… PS16, Line 129: 1 extra blank line not required. https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/a… PS16, Line 160: 2 extra blank lines not required. https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/g… File src/soc/intel/tigerlake/gpio.c: https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/g… PS16, Line 94: .gpi_int_sts_reg_0 = GPI_INT_STS_0, : .gpi_int_en_reg_0 = GPI_INT_EN_0, Why are these removed? https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/g… PS16, Line 197: { PMC_GPP_G, GPP_G }, Why was this removed? Is it not possible to map that to GPE? https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/i… File src/soc/intel/tigerlake/include/soc/gpio_defs.h: https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/i… PS16, Line 268: #define GPI_INT_STS_0 0x100 : #define GPI_INT_EN_0 0x110 I am confused. Why are these removed? https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/i… PS16, Line 119: * space before * https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/i… File src/soc/intel/tigerlake/include/soc/gpio_soc_defs.h: https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/i… PS16, Line 35: 26 Why is this set to 26? https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/i… PS16, Line 376: TOTAL_GPIO_COMM 5 If we are going with this, I would recommend at least having macros for the communities: #define COMM_0 0 #define COMM_1 1 #define COMM_2 2 /* GPIO community 3 is not exposed to be used and hence is skipped. */ #define COMM_4 3 #define COMM_5 4 #define TOTAL_GPIO_COMM (COMM_5 + 1) And then use these COMM_* macros in gpio.c as index for the various communities. This will ensure that if we switch to using COMM_3 in the future, mainboard and SoC code wouldn't break unexpectedly. https://review.coreboot.org/c/coreboot/+/37427/16/src/soc/intel/tigerlake/i… PS16, Line 379: /* I don't think we need the pinmux settings. At best, this should be added as a separate CL if they are really required. https://review.coreboot.org/c/coreboot/+/37427/12/src/soc/intel/tigerlake/i… File src/soc/intel/tigerlake/include/soc/gpio_soc_defs.h: https://review.coreboot.org/c/coreboot/+/37427/12/src/soc/intel/tigerlake/i… PS12, Line 374: #define GPP_MLK 277 > I had asked kernel team. Waiting for their reply. Any update here? -- To view, visit https://review.coreboot.org/c/coreboot/+/37427 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I585100375feee39b5a9105bdf6d9f5ca3a5bb2fa Gerrit-Change-Number: 37427 Gerrit-PatchSet: 16 Gerrit-Owner: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org> Gerrit-Reviewer: Francois Toguo Fotso <francois.toguo.fotso(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com> Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Shaunak Saha <shaunak.saha(a)intel.com> Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Venkata Krishna Nimmagadda Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Wed, 08 Jan 2020 19:53:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Shaunak Saha <shaunak.saha(a)intel.com> Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common/block/cse: Make me_hfsts1 structure SoC specific
by Sridhar Siricilla (Code Review) 08 Jan '20

08 Jan '20

Sridhar Siricilla has uploaded a new patch set (#18) to the change originally created by Rizwan Qureshi. ( https://review.coreboot.org/c/coreboot/+/35546 ) Change subject: soc/intel/common/block/cse: Make me_hfsts1 structure SoC specific ...................................................................... soc/intel/common/block/cse: Make me_hfsts1 structure SoC specific Move me_hfsts1 structure from common code to SoC specific. 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/cannonlake/include/soc/me.h M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h M src/soc/intel/skylake/include/soc/me.h 4 files changed, 49 insertions(+), 24 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/35546/18 -- 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: 18 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: Nico Huber <nico.h(a)gmx.de> 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: Sridhar Siricilla <sridhar.siricilla(a)intel.corp-partner.google.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-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/intel/common/basecode: Implement CSE update flow
by Sridhar Siricilla (Code Review) 08 Jan '20

08 Jan '20

Hello Patrick Rudolph, Subrata Banik, Balaji Manigandan, Aamir Bohra, Sridhar Siricilla, Rizwan Qureshi, build bot (Jenkins), Patrick Georgi, Furquan Shaikh, V Sowmya, Andrey Petrov, Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/35403 to look at the new patch set (#41). Change subject: soc/intel/common/basecode: Implement CSE update flow ...................................................................... soc/intel/common/basecode: Implement CSE update flow This is the core patch that implement CSE FW update flow. To enable the FW update flow the following are required: * Descriptor change to accommodate a larger CSME region The CSME size is 4.1MB. * FMAP changes to accommodate ME update binary in RW CBFSes & larger CSE binary. Due to the increased CSME binary size and to accommodate the extra CSME RW binaries (which are ~2.5 MB) in RW CBFSes, the board FMAP has to be modified. * The new CSE binary with new partitions and respective RW area binaries. The following changes have been done in this patch: * Implement Update flow Get the partition info containing version of ME RW using GET_BOOT_PARTITION_INFO HECI command Get the me_rw.version from the currently selected RW slot. If the version from the above 2 locations don't match start the update - Set the CSE's next boot partition to RO using SET_BOOT_PARTITION HECI command. - Send global reset command to reset only the CSME - Wait for CSME to enter SOFT_TEMP_DISABLE operation mode (indicated by HFSTS1 register bit 19:16) - Enable HMRFPO (Host ME Region Flash Protection Override) using the HMRFPO_ENABLE HECI command - Erase and Copy the CBFS ME RW to ME RW partition - Set the CSE's next boot partition to RW using SET_BOOT_PARTITION HECI command - Trigger global reset The system should boot with the Updated ME Verified basic update flows on Cometlake RVP and hatch. BUG=b:111330995 Change-Id: I12f6bba3324069d65edabaccd234006b0840e700 Signed-off-by: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Signed-off-by: V Sowmya <v.sowmya(a)intel.com> Signed-off-by: Sridhar Siricilla <sridhar.siricilla(a)intel.com> --- A Documentation/soc/intel/cse_fw_update/Layout_after.svg A Documentation/soc/intel/cse_fw_update/Layout_before.svg A Documentation/soc/intel/cse_fw_update/cse_fw_update.md M Documentation/soc/intel/index.md A src/soc/intel/common/basecode/fw_update/Kconfig A src/soc/intel/common/basecode/fw_update/Makefile.inc A src/soc/intel/common/basecode/fw_update/cse_update.c A src/soc/intel/common/basecode/include/intelbasecode/cse_update.h 8 files changed, 857 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/35403/41 -- To view, visit https://review.coreboot.org/c/coreboot/+/35403 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I12f6bba3324069d65edabaccd234006b0840e700 Gerrit-Change-Number: 35403 Gerrit-PatchSet: 41 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Andrey Petrov <anpetrov(a)fb.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: 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: Sridhar Siricilla <sridhar.siricilla(a)intel.corp-partner.google.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: Maxim Polyakov <max.senia.poliak(a)gmail.com> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: newpatchset

1 0

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

08 Jan '20

Hello Patrick Rudolph, Subrata Banik, Balaji Manigandan, Aamir Bohra, Sridhar Siricilla, Rizwan Qureshi, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, V Sowmya, Nico Huber, Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/35402 to look at the new patch set (#47). 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 platforms, CSE can boot from either BP1 or BP2. The CSE image layout appears as below.. ------------- ------------------ -------------------------- |CSE REGION | => | RO | RW | => | 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 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. BUG=b:145809764 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, 489 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/35402/47 -- 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: 47 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: Sridhar Siricilla <sridhar.siricilla(a)intel.corp-partner.google.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-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/intel/common/block/cse: Make me_hfsts1 structure SoC specific
by Sridhar Siricilla (Code Review) 08 Jan '20

08 Jan '20

Sridhar Siricilla has uploaded a new patch set (#17) to the change originally created by Rizwan Qureshi. ( https://review.coreboot.org/c/coreboot/+/35546 ) Change subject: soc/intel/common/block/cse: Make me_hfsts1 structure SoC specific ...................................................................... soc/intel/common/block/cse: Make me_hfsts1 structure SoC specific Move me_hfsts1 structure from common code to SoC specific. 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/cannonlake/include/soc/me.h M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h M src/soc/intel/skylake/include/soc/me.h 4 files changed, 49 insertions(+), 24 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/35546/17 -- 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: 17 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: Nico Huber <nico.h(a)gmx.de> 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: Sridhar Siricilla <sridhar.siricilla(a)intel.corp-partner.google.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-MessageType: newpatchset

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit January 2020