Attention is currently required from: Patrick Rudolph, Arthur Heymans.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52969 )
Change subject: security/intel/txt: Set up TPM in bootblock if using measured boot
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
Patchset:
PS1:
I haven't tested this patch on Asrock B85M Pro4. However, it's a no-op by default since TPM_MEASURED_BOOT is disabled by default, so I'm not worried.
--
To view, visit https://review.coreboot.org/c/coreboot/+/52969
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1225757dbc4c6fb5a30d1aa12987661a0a6eb538
Gerrit-Change-Number: 52969
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Fri, 07 May 2021 12:31:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Alexander Couzens, Patrick Rudolph.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52942 )
Change subject: cpu/intel/socket_p: Increase DCACHE_RAM_SIZE
......................................................................
Patch Set 1:
(1 comment)
File src/cpu/intel/socket_p/Kconfig:
https://review.coreboot.org/c/coreboot/+/52942/comment/7c190640_85b79808
PS1, Line 16: default 0x10000
CPU_INTEL_SOCKET_BGA956 and CPU_INTEL_SOCKET_M use 0x8000 here, and are never used with NO_CBFS_MCACHE. I'd simply drop `select NO_CBFS_MCACHE` from t400.
--
To view, visit https://review.coreboot.org/c/coreboot/+/52942
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I0d6f7f9151ecd4c9fbbba4ed033dfda8724b6772
Gerrit-Change-Number: 52942
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Fri, 07 May 2021 12:27:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Damien Zammit, Arthur Heymans, Patrick Rudolph.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52941 )
Change subject: nb/intel/pineview: Use cbfs mcache
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/52941/comment/793f8d6b_f3db17df
PS1, Line 11:
I've just tested this change. I'd suggest mentioning so in the commit message as follows:
Tested on Gigabyte GA-D510UD, still boots and resumes.
Tested-by: Angel Pons <th3fanbus(a)gmail.com>
--
To view, visit https://review.coreboot.org/c/coreboot/+/52941
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1487ba9decd3aa22424a3ac111de7fbdb867d38d
Gerrit-Change-Number: 52941
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Damien Zammit
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Damien Zammit
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Fri, 07 May 2021 12:23:42 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Paul Fagerburg, Jan Dabros.
Jakub Czapiga has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52938 )
Change subject: tests: Add lib/spd_cache-test test case
......................................................................
Patch Set 3:
(2 comments)
File tests/include/tests/lib/spd_cache_data.h:
https://review.coreboot.org/c/coreboot/+/52938/comment/ba8966f9_d9da15a5
PS2, Line 5: unsigned char spd_data_ddr4_1[] = {
> For the record: […]
Done
File tests/lib/spd_cache-test.c:
https://review.coreboot.org/c/coreboot/+/52938/comment/eac2edcf_65cd8b93
PS2, Line 50: static void test_load_spd_cache(void **state)
> Yes, this requires some explanation. I will add it in next version.
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/52938
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic9a1420e49e1e80d180117c931e630e54c90cd75
Gerrit-Change-Number: 52938
Gerrit-PatchSet: 3
Gerrit-Owner: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Reviewer: Jan Dabros <jsd(a)semihalf.com>
Gerrit-Reviewer: Paul Fagerburg <pfagerburg(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Paul Fagerburg <pfagerburg(a)chromium.org>
Gerrit-Attention: Jan Dabros <jsd(a)semihalf.com>
Gerrit-Comment-Date: Fri, 07 May 2021 11:55:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Jakub Czapiga <jacz(a)semihalf.com>
Comment-In-Reply-To: Paul Fagerburg <pfagerburg(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Paul Fagerburg, Jan Dabros.
Hello build bot (Jenkins), Paul Fagerburg, Jan Dabros,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/52938
to look at the new patch set (#3).
Change subject: tests: Add lib/spd_cache-test test case
......................................................................
tests: Add lib/spd_cache-test test case
Signed-off-by: Jakub Czapiga <jacz(a)semihalf.com>
Change-Id: Ic9a1420e49e1e80d180117c931e630e54c90cd75
---
A tests/include/tests/lib/spd_cache_data.h
M tests/lib/Makefile.inc
A tests/lib/spd_cache-test.c
3 files changed, 385 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/52938/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/52938
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic9a1420e49e1e80d180117c931e630e54c90cd75
Gerrit-Change-Number: 52938
Gerrit-PatchSet: 3
Gerrit-Owner: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Reviewer: Jan Dabros <jsd(a)semihalf.com>
Gerrit-Reviewer: Paul Fagerburg <pfagerburg(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Paul Fagerburg <pfagerburg(a)chromium.org>
Gerrit-Attention: Jan Dabros <jsd(a)semihalf.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Felix Singer, Raul Rangel, Furquan Shaikh, Matt DeVillier, Angel Pons, Michael Niewöhner, Patrick Rudolph, Felix Held.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52493 )
Change subject: [RFC] device: Introduce new method for setting device states
......................................................................
Patch Set 20:
(1 comment)
File src/soc/intel/skylake/chip.c:
https://review.coreboot.org/c/coreboot/+/52493/comment/5206a8fe_746b3501
PS13, Line 461: if (!xdci_can_enable())
: params->XdciEnable = 0;
> I was about commenting that I don't find this useful since AFAICS there would be only two devices wh […]
I would say the name depends on the exact implementation. If you decide for
a function that is always called to decide the enable state, I'd simply call
the pointer `enable`. The caller would look like this:
*devmap[i].option = devmap[i].enable(dev);
(or maybe `should_enable`, something that gives some meaning to what
return value it provides)
--
To view, visit https://review.coreboot.org/c/coreboot/+/52493
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I70fde306c65a8881f565c5f923be20f380ea64d3
Gerrit-Change-Number: 52493
Gerrit-PatchSet: 20
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Fri, 07 May 2021 11:45:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net>
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49345 )
Change subject: mb/google/octopus/Kconfig: Remove space saving options
......................................................................
Patch Set 2: Code-Review+1
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/49345/comment/bdbeca56_b745570f
PS2, Line 9: 28e61f "device: Use __pci_0_00_0_config in config_of_soc()"
The commit hash is too short and does not resolve properly. I'd suggest:
Commit 28e61f1634 "device: Use __pci_0_00_0_config in config_of_soc()"
https://review.coreboot.org/c/coreboot/+/49345/comment/a4aa13c8_a28b3593
PS2, Line 14: google_octopus_spi_flash_console
Did you try a vboot-enabled config? I'm not sure how much bootblock space this config uses.
--
To view, visit https://review.coreboot.org/c/coreboot/+/49345
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I208211d30cc2805113a16a02cdab957b8c584c92
Gerrit-Change-Number: 49345
Gerrit-PatchSet: 2
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Fri, 07 May 2021 11:44:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Ravi kumar, Shelley Chen, Paul Menzel, mturney mturney, Julius Werner.
Ravi Kumar Bokka has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50587 )
Change subject: sc7280: Reserve wlan & wpss dram memory regions
......................................................................
Patch Set 31:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/50587/comment/3209d8d1_aaa19318
PS7, Line 7: sc7280: reserved wlan & wpss dram memory regions
> https://chris.beams. […]
Ack
--
To view, visit https://review.coreboot.org/c/coreboot/+/50587
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic98b5d08a0a7b3f772582bf85d94f901a7c53010
Gerrit-Change-Number: 50587
Gerrit-PatchSet: 31
Gerrit-Owner: Ravi kumar <rbokka(a)codeaurora.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Shelley Chen <shchen(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: mturney mturney <mturney(a)codeaurora.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Ravi Kumar Bokka <c_rbokka(a)qualcomm.corp-partner.google.com>
Gerrit-Attention: Ravi kumar <rbokka(a)codeaurora.org>
Gerrit-Attention: Shelley Chen <shchen(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: mturney mturney <mturney(a)codeaurora.org>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Comment-Date: Fri, 07 May 2021 11:26:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: comment
Attention is currently required from: Vinod Polimera, Douglas Anderson, Julius Werner.
Xuxin Xiong has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52959 )
Change subject: drivers/sn65dsi86: Switch EDID reading to use "indirect mode"
......................................................................
Patch Set 4:
(1 comment)
Patchset:
PS4:
Should we need add these fields as follows?
BUG=b:186098520
BRANCH=trogdor
TEST=emerge-strongbad and test with power on.
--
To view, visit https://review.coreboot.org/c/coreboot/+/52959
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I65f80193380d3c3841f9f5c26897ed672f45e15a
Gerrit-Change-Number: 52959
Gerrit-PatchSet: 4
Gerrit-Owner: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Douglas Anderson <dianders(a)chromium.org>
Gerrit-Reviewer: Vinod Polimera <vpolimer(a)qualcomm.corp-partner.google.com>
Gerrit-Reviewer: Xuxin Xiong <xuxinxiong(a)huaqin.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Ravi Kumar Bokka <c_rbokka(a)qualcomm.corp-partner.google.com>
Gerrit-CC: mturney mturney <mturney(a)codeaurora.org>
Gerrit-Attention: Vinod Polimera <vpolimer(a)qualcomm.corp-partner.google.com>
Gerrit-Attention: Douglas Anderson <dianders(a)chromium.org>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Comment-Date: Fri, 07 May 2021 11:17:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Martin Roth, Stefan Reinauer, Christian Walter.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49461 )
Change subject: payloads/external/seabios: Scan all PCI domains on Xeon-SP platforms
......................................................................
Patch Set 2:
(2 comments)
File payloads/external/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/49461/comment/5d7e6837_f9457ebc
PS2, Line 117: domains
> Does this scan all PCI domains or all PCI buses? Does it scan bus 255 or everything up to 255?
I tried to explain this below. It controls the maximum number of
trees hanging on host bridges. So we could limit this by the number
of host bridges on a board.
Without any change, SeaBIOS would already scan bus 255 _if_ it is
found in the hierarchy below the first host bridge. With this
change, it still only looks at "domain" 0, but it would continue
scanning buses even if they are not part of a single hierarchy.
That 255 is used here is confusing because it's also the maximum
bus number. 255 extra PCI roots would mean we'd have exactly one
bus per root _and_ no space for any PCI bridge in the whole system
(every bus would hang directly below a host bridge). Doesn't make
much sense, maybe not worth to try to understand it ;)
https://review.coreboot.org/c/coreboot/+/49461/comment/1c109d67_de75e0a6
PS2, Line 118: 255
> On my Xeon-SP there are devices on bus 0xff.
> Please note that
> for SeaBIOS this is not the maximum bus number but the maximum number
> of individual trees of buses.
Let me provide an example, this is from the Thinkpad X201 (Ironlake)
I'm typing on (lsusb -t):
-+-[0000:ff]-+-00.0
| +-00.1
| +-02.0
| +-02.1
| +-02.2
| \-02.3
\-[0000:00]-+-00.0
+-02.0
+-16.0
+-19.0
+-1a.0
+-1b.0
+-1c.0-[01]--
+-1c.1-[02]--
+-1c.3-[03]--
+-1c.4-[04]----00.0
+-1d.0
+-1e.0-[05]--
+-1f.0
+-1f.2
+-1f.3
\-1f.6
What we can see here is that bus 255 is not part of the hierarchy
below bus 0. Hence, this is an extra root (in SeaBIOS' words).
Without any change, SeaBIOS would start scanning bus 0, at some
point encounter 1e.0 and see the highest secondary bus number 5.
So SeaBIOS would continue to scan buses up to 5. Then stop,
because it doesn't consider extra roots by default.
With a setting of `extra-pci-roots = 1`, it would continue
scanning until it finds another device, in this case `ff:00.0`.
Why not do so by default? it has to scan 250 * 32 additional
device addresses before it finds `ff:00.0`.
In case of my Thinkpad, this doesn't matter because there are
no boot devices below the second root (only internal stuff that
Intel doesn't want us to know about anyway). Maybe for Xeon-SP
it's similar (no boot devices below the last root) and the opti-
mum is the number of host bridges - 1 - 1 (another -1 because
it already scans one host bridge by default).
--
To view, visit https://review.coreboot.org/c/coreboot/+/49461
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1a99e1b9b6ea77fede6ed8239016bd92c5291c63
Gerrit-Change-Number: 49461
Gerrit-PatchSet: 2
Gerrit-Owner: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
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: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Comment-Date: Fri, 07 May 2021 11:09:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-MessageType: comment