Hello Patrick Rudolph, Subrata Banik, Lean Sheng Tan, Aamir Bohra, Maulik V Vaghela, Rizwan Qureshi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37434
to look at the new patch set (#3).
Change subject: soc/intel/common: Add Jasper Lake Device IDs
......................................................................
soc/intel/common: Add Jasper Lake Device IDs
Jasper Lake CPU, SA and PCH IDs
Change-Id: I2c9ec1ee4236184b986d99250f263172c80f7117
Signed-off-by: Ronak Kanabar <ronak.kanabar(a)intel.com>
---
M src/include/device/pci_ids.h
M src/soc/intel/common/block/cpu/mp_init.c
M src/soc/intel/common/block/cse/cse.c
M src/soc/intel/common/block/dsp/dsp.c
M src/soc/intel/common/block/graphics/graphics.c
M src/soc/intel/common/block/i2c/i2c.c
M src/soc/intel/common/block/include/intelblocks/mp_init.h
M src/soc/intel/common/block/lpc/lpc.c
M src/soc/intel/common/block/p2sb/p2sb.c
M src/soc/intel/common/block/pcie/pcie.c
M src/soc/intel/common/block/pmc/pmc.c
M src/soc/intel/common/block/sata/sata.c
M src/soc/intel/common/block/scs/sd.c
M src/soc/intel/common/block/smbus/smbus.c
M src/soc/intel/common/block/spi/spi.c
M src/soc/intel/common/block/sram/sram.c
M src/soc/intel/common/block/systemagent/systemagent.c
M src/soc/intel/common/block/uart/uart.c
M src/soc/intel/common/block/xhci/xhci.c
M src/soc/intel/tigerlake/bootblock/report_platform.c
20 files changed, 79 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/37434/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/37434
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I2c9ec1ee4236184b986d99250f263172c80f7117
Gerrit-Change-Number: 37434
Gerrit-PatchSet: 3
Gerrit-Owner: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com>
Gerrit-Reviewer: Lean Sheng Tan <lean.sheng.tan(a)intel.com>
Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-MessageType: newpatchset
Hello Patrick Rudolph, Subrata Banik, Lean Sheng Tan, Aamir Bohra, Maulik V Vaghela, Rizwan Qureshi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37434
to look at the new patch set (#2).
Change subject: soc/intel/common: Add Jasper Lake Device IDs
......................................................................
soc/intel/common: Add Jasper Lake Device IDs
Jasper Lake CPU, SA and PCH IDs
Change-Id: I2c9ec1ee4236184b986d99250f263172c80f7117
Signed-off-by: rkanabar <ronak.kanabar(a)intel.com>
---
M src/include/device/pci_ids.h
M src/soc/intel/common/block/cpu/mp_init.c
M src/soc/intel/common/block/cse/cse.c
M src/soc/intel/common/block/dsp/dsp.c
M src/soc/intel/common/block/graphics/graphics.c
M src/soc/intel/common/block/i2c/i2c.c
M src/soc/intel/common/block/include/intelblocks/mp_init.h
M src/soc/intel/common/block/lpc/lpc.c
M src/soc/intel/common/block/p2sb/p2sb.c
M src/soc/intel/common/block/pcie/pcie.c
M src/soc/intel/common/block/pmc/pmc.c
M src/soc/intel/common/block/sata/sata.c
M src/soc/intel/common/block/scs/sd.c
M src/soc/intel/common/block/smbus/smbus.c
M src/soc/intel/common/block/spi/spi.c
M src/soc/intel/common/block/sram/sram.c
M src/soc/intel/common/block/systemagent/systemagent.c
M src/soc/intel/common/block/uart/uart.c
M src/soc/intel/common/block/xhci/xhci.c
M src/soc/intel/tigerlake/bootblock/report_platform.c
20 files changed, 79 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/37434/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/37434
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I2c9ec1ee4236184b986d99250f263172c80f7117
Gerrit-Change-Number: 37434
Gerrit-PatchSet: 2
Gerrit-Owner: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com>
Gerrit-Reviewer: Lean Sheng Tan <lean.sheng.tan(a)intel.com>
Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-MessageType: newpatchset
Kangheui Won has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37460 )
Change subject: mainboard/google : Add initial Puff GPIO table
......................................................................
Patch Set 1:
This change is ready for review.
--
To view, visit https://review.coreboot.org/c/coreboot/+/37460
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I91ebd6bd3ba712ee8b48472c0d3b6a4f982fbb28
Gerrit-Change-Number: 37460
Gerrit-PatchSet: 1
Gerrit-Owner: Kangheui Won <khwon(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Kangheui Won <khwon(a)chromium.org>
Gerrit-Reviewer: Shelley Chen <shchen(a)google.com>
Gerrit-Comment-Date: Wed, 04 Dec 2019 03:21:44 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Johnny Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37444 )
Change subject: drivers/ipmi: Add IPMI Read FRU function
......................................................................
Patch Set 4:
This change is planned to be cherry-picked to master after reviewed and merged.
--
To view, visit https://review.coreboot.org/c/coreboot/+/37444
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: 4.11_branch
Gerrit-Change-Id: Id6353f5ce3f7ddd3bb161b91364b3cf276d020b8
Gerrit-Change-Number: 37444
Gerrit-PatchSet: 4
Gerrit-Owner: Johnny Lin
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Jingle Hsu
Gerrit-Reviewer: Johnny Lin
Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Morgan Jang
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Comment-Date: Wed, 04 Dec 2019 01:48:52 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36622 )
Change subject: drivers/fsp2_0: drop support for FSP-T
......................................................................
Patch Set 4:
So I just saw this change.
I am fully aware that emotions are hard to control, and that it's too easy for them to run out of control, and end up taking matters personally. Because attacking each other is futile, I would encourage everyone to be constructive: work towards finding a solution that satisfies everyone's needs as much as possible.
After reading the conversations, I understood the following:
Reasons to drop FSP-T:
- An open-source alternative already exists in coreboot.
- FSP-T is undocumented, and its integration is rather fragile.
- Largely untested on upstream. Sometimes, changes that were build-tested by Jenkins did not build when selecting FSP-T.
Reasons against dropping FSP-T:
- The native CAR init code does not work with all possible configurations. For example, Bootguard.
I may have missed something from either side, or understood something the wrong way. Any comments, clarifications and suggestions are very welcome :)
One of the points that were brought up is the microcode update problem Nico explained. As I understand it, an invalid configuration, which could be detected at build-time, instead results in a silent boot failure. Most people can only debug coreboot using console logs and a postcode card, usually by placing debug statements around to narrow down where things go wrong. Since FSP's source code is not public, adding debug prints to FSP is not an option for most people. Moreover, there are next to no known-working configurations with FSP-T to compare against. As a result, trying to make FSP-T work without privileged information is extremely frustrating.
Another thing to consider: Bootguard. As I understand it, coreboot's CAR init code does not take Bootguard into account, and would not work correctly if Bootguard is enabled. It doesn't seem to be very complicated: basically, the Bootguard ACM sets up CAR already, so coreboot code should not do it again. However, getting a Bootguard-enabled board on which to test this on is pretty much impossible. Since Bootguard requires firmware to be signed, and the signing key is kept secret, running a random coreboot image on a commercially available board with Bootguard is just not possible. From another perspective, the current situation means that that dropping FSP-T support also implies dropping Bootguard support.
Personally, I would prefer the open-source approach. I would not mind having the option to use FSP-T, as long as it doesn't rot away. This means that it should be build-tested automatically. In addition, there should be known-good configurations to use as a reference: regularly-updated successful boot records for several boards of each platform with FSP-T support, for example board status reports. This is especially useful when doing major overhauls to coreboot's structure, like C_ENVIRONMENT_BOOTBLOCK. Otherwise, the technical debt quickly builds up, and then rewriting everything from scratch is easier than fixing the resulting mess, which nobody wants to do.
So, I think that getting CB:36682 boot-tested and merged in will be good for everyone. If so, what are we waiting for?
--
To view, visit https://review.coreboot.org/c/coreboot/+/36622
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib791b30b621730f4b7c0a5f668a3b6559245daf5
Gerrit-Change-Number: 36622
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: David Guckian
Gerrit-Reviewer: David Guckian <david.guckian(a)intel.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Lean Sheng Tan <lean.sheng.tan(a)intel.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Michael Niewöhner
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: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Tue, 03 Dec 2019 17:55:47 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment