coreboot-gerrit January 2019

coreboot-gerrit@coreboot.org

1 participants
1296 discussions

Change in ...coreboot[master]: nb/intel/i945: Use parallel MP init
by HAOUAS Elyes (Code Review) 16 Jan '19

16 Jan '19

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/25600 ) Change subject: nb/intel/i945: Use parallel MP init ...................................................................... Patch Set 40: Code-Review+1 (1 comment) https://review.coreboot.org/#/c/25600/40/src/cpu/intel/model_f4x/model_f4x_… File src/cpu/intel/model_f4x/model_f4x_init.c: https://review.coreboot.org/#/c/25600/40/src/cpu/intel/model_f4x/model_f4x_… PS40, Line 27: if (!IS_ENABLED(CONFIG_PARALLEL_MP) && !intel_ht_sibling()) { : /* MTRRs are shared between threads */ : x86_setup_mtrrs(); : x86_mtrr_check(); : : /* Update the microcode */ : intel_update_microcode_from_cbfs(); : } please, why don't you drop this and "intel_sibling_init(cpu)" ? -- To view, visit https://review.coreboot.org/c/coreboot/+/25600 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I955551b99e9cbc397f99c2a6bd355c6070390bcb Gerrit-Change-Number: 25600 Gerrit-PatchSet: 40 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> 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: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 16 Jan 2019 15:59:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: nb/intel/pineview: Use parallel MP init
by Arthur Heymans (Code Review) 16 Jan '19

16 Jan '19

Hello Patrick Rudolph, Stefan Reinauer, Paul Menzel, build bot (Jenkins), Patrick Georgi, Damien Zammit, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/25602 to look at the new patch set (#39). Change subject: nb/intel/pineview: Use parallel MP init ...................................................................... nb/intel/pineview: Use parallel MP init Remove guards around CPU code on which all platforms use parallel MP init code. This removes the option to disable HT siblings. Tested on Foxconn D41S. Change-Id: I89f7d514d75fe933c3a8858da37004419189674b Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/cpu/intel/model_106cx/model_106cx_init.c M src/mainboard/foxconn/d41s/cmos.layout M src/mainboard/intel/d510mo/cmos.layout M src/northbridge/intel/pineview/Kconfig M src/northbridge/intel/pineview/northbridge.c M src/southbridge/intel/i82801gx/lpc.c 6 files changed, 2 insertions(+), 45 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/25602/39 -- To view, visit https://review.coreboot.org/c/coreboot/+/25602 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I89f7d514d75fe933c3a8858da37004419189674b Gerrit-Change-Number: 25602 Gerrit-PatchSet: 39 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Damien Zammit <damien(a)zamaudio.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in ...coreboot[master]: nb/intel/x4x: Use parallel MP init
by Arthur Heymans (Code Review) 16 Jan '19

16 Jan '19

Hello Patrick Rudolph, Angel Pons, Stefan Reinauer, Paul Menzel, build bot (Jenkins), Patrick Georgi, Damien Zammit, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/25601 to look at the new patch set (#39). Change subject: nb/intel/x4x: Use parallel MP init ...................................................................... nb/intel/x4x: Use parallel MP init Use parallel MP init code to initialize all AP's. Also remove guards around CPU code where all platforms now use parallel MP init. Tested on Intel DG41WV, shaves off about 90ms on a quad core. Change-Id: Id5a2729f5bf6b525abad577e63d7953ae6640921 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/cpu/intel/model_1067x/model_1067x_init.c M src/cpu/intel/model_6fx/model_6fx_init.c M src/cpu/intel/model_f4x/model_f4x_init.c M src/mainboard/asrock/g41c-gs/cmos.layout M src/mainboard/foxconn/g41s-k/cmos.layout M src/mainboard/gigabyte/ga-g41m-es2l/cmos.layout M src/northbridge/intel/x4x/Kconfig M src/northbridge/intel/x4x/northbridge.c M src/southbridge/intel/i82801jx/lpc.c 9 files changed, 2 insertions(+), 75 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/25601/39 -- To view, visit https://review.coreboot.org/c/coreboot/+/25601 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id5a2729f5bf6b525abad577e63d7953ae6640921 Gerrit-Change-Number: 25601 Gerrit-PatchSet: 39 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Damien Zammit <damien(a)zamaudio.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-MessageType: newpatchset

1 0

Change in ...coreboot[master]: nb/intel/i945: Use parallel MP init
by Arthur Heymans (Code Review) 16 Jan '19

16 Jan '19

Hello Patrick Rudolph, HAOUAS Elyes, Stefan Reinauer, Paul Menzel, build bot (Jenkins), Patrick Georgi, Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/25600 to look at the new patch set (#40). Change subject: nb/intel/i945: Use parallel MP init ...................................................................... nb/intel/i945: Use parallel MP init Use the parallel mp init path to initialize AP's. This should result in a moderate speedup. Tested on Intel D945GCLF (1 core 2 threads), still boots fine and is 26ms faster compared to lapic_cpu_init. This removes the option to disable HT siblings. Change-Id: I955551b99e9cbc397f99c2a6bd355c6070390bcb Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/cpu/intel/model_106cx/Makefile.inc M src/cpu/intel/model_106cx/model_106cx_init.c M src/cpu/intel/model_6ex/Makefile.inc M src/cpu/intel/model_6ex/model_6ex_init.c M src/cpu/intel/model_f3x/Makefile.inc M src/cpu/intel/model_f3x/model_f3x_init.c M src/cpu/intel/model_f4x/Makefile.inc M src/cpu/intel/model_f4x/model_f4x_init.c M src/mainboard/apple/macbook21/cmos.default M src/mainboard/apple/macbook21/cmos.layout M src/mainboard/asus/p5gc-mx/cmos.default M src/mainboard/asus/p5gc-mx/cmos.layout M src/mainboard/getac/p470/cmos.layout M src/mainboard/gigabyte/ga-945gcm-s2l/cmos.default M src/mainboard/gigabyte/ga-945gcm-s2l/cmos.layout M src/mainboard/ibase/mb899/cmos.layout M src/mainboard/intel/d945gclf/cmos.default M src/mainboard/intel/d945gclf/cmos.layout M src/mainboard/kontron/986lcd-m/cmos.layout M src/mainboard/lenovo/t60/cmos.default M src/mainboard/lenovo/t60/cmos.layout M src/mainboard/lenovo/x60/cmos.default M src/mainboard/lenovo/x60/cmos.layout M src/mainboard/lenovo/z61t/cmos.default M src/mainboard/lenovo/z61t/cmos.layout M src/mainboard/roda/rk886ex/cmos.layout M src/northbridge/intel/i945/Kconfig M src/northbridge/intel/i945/northbridge.c M src/southbridge/intel/i82801gx/lpc.c 29 files changed, 22 insertions(+), 61 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/00/25600/40 -- To view, visit https://review.coreboot.org/c/coreboot/+/25600 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I955551b99e9cbc397f99c2a6bd355c6070390bcb Gerrit-Change-Number: 25600 Gerrit-PatchSet: 40 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> 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: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in ...coreboot[master]: soc/intel/denverton_ns: Select CPU_INTEL_FIRMWARE_INTERFACE_TABLE
by Nico Huber (Code Review) 16 Jan '19

16 Jan '19

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/26928 ) Change subject: soc/intel/denverton_ns: Select CPU_INTEL_FIRMWARE_INTERFACE_TABLE ...................................................................... Patch Set 10: Code-Review+1 (1 comment) https://review.coreboot.org/#/c/26928/10//COMMIT_MSG Commit Message: https://review.coreboot.org/#/c/26928/10//COMMIT_MSG@10 PS10, Line 10: > What problem does this fix? We currently only use the FIT to provide microcode updates to be applied before the CPU comes out of reset. As it is the mostly likely thing it fixes, I don't think it has to be mentioned. I wouldn't mind, though. -- To view, visit https://review.coreboot.org/c/coreboot/+/26928 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9d4901ea56d5bf5225a8f3a6015d2ea80a9e46b5 Gerrit-Change-Number: 26928 Gerrit-PatchSet: 10 Gerrit-Owner: Vanny E <vanessa.f.eusebio(a)intel.com> Gerrit-Reviewer: David Guckian Gerrit-Reviewer: David Guckian <david.guckian(a)intel.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: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Vanny E <vanessa.f.eusebio(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Wed, 16 Jan 2019 14:19:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: soc/intel/basecode: Add support for updating ucode loaded via FIT
by Nico Huber (Code Review) 16 Jan '19

16 Jan '19

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/27369 ) Change subject: soc/intel/basecode: Add support for updating ucode loaded via FIT ...................................................................... Patch Set 38: >> Patch Set 36: >> I do understand the process in this change. Let me elaborate >> my theory more accurately. You can have a RO FIT with entries >> to both RO MCU and RW MCU (at least that is what I understood >> earlier). Booting with the process of this change and top-swap >> disabled should have the same effect as booting with such a >> two-entry FIT and erased RW MCU. Right? > > It's a design goal in Chromebooks that stuff like recovery mode > remains predictable (ie. rely on read-only data _only_), while > still allowing features like dev mode. (There's also the option of > disabling write protect, but that's a whole different story.) > > The top-swap solution provides this feature under these constraints > even for FIT-based ucode updates. Your proposal requires erasing a > block in RW even though it might not be "ours" anymore, but managed > by the device owner (eg. in dev mode). Well, in this change, the stage ucode might not be "yours" anymore and you rely on the TS bit that might not be "yours" anymore either. I don't want to debate about this (whose nvram is whose), just wanted to know the reasoning. Thank you :) Erasing the stage ucode everytime to go into recovery wouldn't be wise, so I prefer the TS solution, too. -- To view, visit https://review.coreboot.org/c/coreboot/+/27369 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iab6ba36a2eb587f331fe522c778e2c430c8eb655 Gerrit-Change-Number: 27369 Gerrit-PatchSet: 38 Gerrit-Owner: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> 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 <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: dhaval v sharma <dhaval.v.sharma(a)intel.com> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Dhaval Sharma <dhaval.v.sharma(a)intel.corp-partner.google.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Wed, 16 Jan 2019 14:04:13 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: soc/intel/basecode: Add support for updating ucode loaded via FIT
by Nico Huber (Code Review) 16 Jan '19

16 Jan '19

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/27369 ) Change subject: soc/intel/basecode: Add support for updating ucode loaded via FIT ...................................................................... Patch Set 38: (1 comment) The implementation looks good. Only the update condition that Patrick R. already noticed seems concerning. https://review.coreboot.org/#/c/27369/30/src/soc/intel/common/basecode/fw_u… File src/soc/intel/common/basecode/fw_update/ucode_update.c: https://review.coreboot.org/#/c/27369/30/src/soc/intel/common/basecode/fw_u… PS30, Line 163: if (staging_rev != current_ucode || current_ucode != slot_rev || > in a scenario where the header (which has ucode revision) is written completely but the ucode data i […] If you want to handle a corrupted RW ucode this way, you'll need more safety guards. I mean check for exactly that condition `TS_ENABLE && staging_rev == slot_rev && current_ucode == slot_rev` Otherwise, you may rewrite the staging ucode for spurious reasons. For instance, think about what would happen if TS doesn't stick for some reasons, e.g. battery failure and somebody implemented ucode_update_reboot() as a cold boot. You could be running into a reboot loop constantly erasing and rewriting the staging ucode. You could avoid this by explicitly checking before writing if slot_microcode and staging microcoding really differ. Beside that one of the checks is really superfluous, the current `if` is the equivalent of: if (staging_rev != current_ucode) version_mismatch = 1; else /* staging_rev == current_ucode */ if (current_ucode != slot_rev) version_mismatch = 1; else /* staging_rev == current_ucode && current_ucode == slot_rev */ if (staging_rev != slot_rev) /* impossible */ version_mismatch = 1; I guess the control flow could be if (staging_rev != slot_rev || current_ucode != slot_rev) { ... if (!memcmp(..., slot_microcode, get_microcode_size(...))) perform_update(slot_microcode); } Or slightly optimized usual case: if (staging_rev != slot_rev) { perform_update(slot_microcode); } else if (current_ucode != slot_rev) { ... if (!memcmp(..., slot_microcode, get_microcode_size(...))) perform_update(slot_microcode); } -- To view, visit https://review.coreboot.org/c/coreboot/+/27369 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iab6ba36a2eb587f331fe522c778e2c430c8eb655 Gerrit-Change-Number: 27369 Gerrit-PatchSet: 38 Gerrit-Owner: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> 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 <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: dhaval v sharma <dhaval.v.sharma(a)intel.com> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Dhaval Sharma <dhaval.v.sharma(a)intel.corp-partner.google.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Wed, 16 Jan 2019 13:54:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com> Comment-In-Reply-To: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: mb/google/sarien: Enable Camarillo Device
by Lijian Zhao (Code Review) 16 Jan '19

16 Jan '19

Lijian Zhao has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/30858 Change subject: mb/google/sarien: Enable Camarillo Device ...................................................................... mb/google/sarien: Enable Camarillo Device Whiskeylake processor have an internal devices called Camarillo dedicated for thermal management support, turn it on so processor thermal driver can be loaded. BUG=N/A TEST=Boot up and run lspci on Sarien board, Bus 0 Device 4 Funcion 0 can be seen. Signed-off-by: Lijian Zhao <lijian.zhao(a)intel.com> Change-Id: I937960fde2704cddb1fe0058ab622f4b5de401d7 --- M src/mainboard/google/sarien/variants/arcada/devicetree.cb M src/mainboard/google/sarien/variants/sarien/devicetree.cb 2 files changed, 2 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/58/30858/1 diff --git a/src/mainboard/google/sarien/variants/arcada/devicetree.cb b/src/mainboard/google/sarien/variants/arcada/devicetree.cb index cccddce..a532cae 100644 --- a/src/mainboard/google/sarien/variants/arcada/devicetree.cb +++ b/src/mainboard/google/sarien/variants/arcada/devicetree.cb @@ -33,6 +33,7 @@ register "dptf_enable" = "1" register "dmipwroptimize" = "1" register "satapwroptimize" = "1" + register "Device4Enable" = "1" # Intel Common SoC Config register "usb2_ports[0]" = "USB2_PORT_TYPE_C(OC_SKIP)" # Left Type-C Port diff --git a/src/mainboard/google/sarien/variants/sarien/devicetree.cb b/src/mainboard/google/sarien/variants/sarien/devicetree.cb index b590bac..dd6f251 100644 --- a/src/mainboard/google/sarien/variants/sarien/devicetree.cb +++ b/src/mainboard/google/sarien/variants/sarien/devicetree.cb @@ -42,6 +42,7 @@ register "SlowSlewRateForGt" = "2" register "SlowSlewRateForSa" = "2" register "SlowSlewRateForFivr" = "2" + register "Device4Enable" = "1" # Intel Common SoC Config register "usb2_ports[0]" = "USB2_PORT_TYPE_C(OC_SKIP)" # Left Type-C Port -- To view, visit https://review.coreboot.org/c/coreboot/+/30858 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I937960fde2704cddb1fe0058ab622f4b5de401d7 Gerrit-Change-Number: 30858 Gerrit-PatchSet: 1 Gerrit-Owner: Lijian Zhao <lijian.zhao(a)intel.com> Gerrit-MessageType: newchange

6 10

Change in ...coreboot[master]: soc/intel/basecode: Add support for updating ucode loaded via FIT
by Patrick Georgi (Code Review) 16 Jan '19

16 Jan '19

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/27369 ) Change subject: soc/intel/basecode: Add support for updating ucode loaded via FIT ...................................................................... Patch Set 38: > Patch Set 36: > I do understand the process in this change. Let me elaborate > my theory more accurately. You can have a RO FIT with entries > to both RO MCU and RW MCU (at least that is what I understood > earlier). Booting with the process of this change and top-swap > disabled should have the same effect as booting with such a > two-entry FIT and erased RW MCU. Right? It's a design goal in Chromebooks that stuff like recovery mode remains predictable (ie. rely on read-only data _only_), while still allowing features like dev mode. (There's also the option of disabling write protect, but that's a whole different story.) The top-swap solution provides this feature under these constraints even for FIT-based ucode updates. Your proposal requires erasing a block in RW even though it might not be "ours" anymore, but managed by the device owner (eg. in dev mode). -- To view, visit https://review.coreboot.org/c/coreboot/+/27369 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iab6ba36a2eb587f331fe522c778e2c430c8eb655 Gerrit-Change-Number: 27369 Gerrit-PatchSet: 38 Gerrit-Owner: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> 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 <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: dhaval v sharma <dhaval.v.sharma(a)intel.com> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Dhaval Sharma <dhaval.v.sharma(a)intel.corp-partner.google.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Wed, 16 Jan 2019 13:36:41 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: lib/boot_device: Add API for write protect a region
by Rizwan Qureshi (Code Review) 16 Jan '19

16 Jan '19

Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/28724 ) Change subject: lib/boot_device: Add API for write protect a region ...................................................................... Patch Set 18: (3 comments) https://review.coreboot.org/#/c/28724/16/src/drivers/spi/boot_device_rw_nom… File src/drivers/spi/boot_device_rw_nommap.c: https://review.coreboot.org/#/c/28724/16/src/drivers/spi/boot_device_rw_nom… PS16, Line 111: > alignment Done https://review.coreboot.org/#/c/28724/16/src/drivers/spi/boot_device_rw_nom… PS16, Line 129: region_device_region(rd), 1, > replace 1 with true Done https://review.coreboot.org/#/c/28724/16/src/include/boot_device.h File src/include/boot_device.h: https://review.coreboot.org/#/c/28724/16/src/include/boot_device.h@69 PS16, Line 69: enum bootdev_prot_type type); > alignment, […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/28724 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4c9376e2c2c7a4852f13c65824c6cd64a1c6ac0a Gerrit-Change-Number: 28724 Gerrit-PatchSet: 18 Gerrit-Owner: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Wed, 16 Jan 2019 13:30:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit January 2019