Attention is currently required from: Patrick Georgi, Martin L Roth, Maxim Polyakov, ron minnich.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59958 )
Change subject: util/docker/coreboot.org-status: Rewrite parser
......................................................................
Patch Set 16: Code-Review+1
(1 comment)
Patchset:
PS16:
Neat!
--
To view, visit https://review.coreboot.org/c/coreboot/+/59958
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I4fab86d24088e4f9eff434c21ce9caa077f3f9e2
Gerrit-Change-Number: 59958
Gerrit-PatchSet: 16
Gerrit-Owner: Patrick Georgi <patrick(a)coreboot.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Maxim Polyakov <max.senia.poliak(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: ron minnich <rminnich(a)gmail.com>
Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Maxim Polyakov <max.senia.poliak(a)gmail.com>
Gerrit-Attention: ron minnich <rminnich(a)gmail.com>
Gerrit-Comment-Date: Fri, 28 Oct 2022 14:54:52 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Paul Menzel.
Solomon Alan-Dei has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68941 )
Change subject: arch/x86: fix copying beyond eos in smbios struct
......................................................................
Patch Set 4:
(4 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/68941/comment/d887a2da_508188bf
PS2, Line 7: src/
> Please remove.
Done
https://review.coreboot.org/c/coreboot/+/68941/comment/9105b773_70376aa7
PS2, Line 9: Coverity scan indicated there may be a copy beyond eos in
: smbios struct declared in smbios.h but the code is intentional.
: This type of issue is fixed by assigning the source for the text string
: to a const pointer, and passing the pointer to smbios_add_string calls
: in smbios_defaults.c
> Please reflow for 72 characters per line, and add exactly one blank line between paragraphs. […]
Done
https://review.coreboot.org/c/coreboot/+/68941/comment/1a7428e8_ac14c575
PS2, Line 14: CID: 1487449
> Found-by: Coverity (CID 1487449: …)
Done
Patchset:
PS4:
Thank you for pointing out the issues. Kindly check if they’re resolved.
--
To view, visit https://review.coreboot.org/c/coreboot/+/68941
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I48c5903a2a3b59c9f0e89cd96ef1580b3a229771
Gerrit-Change-Number: 68941
Gerrit-PatchSet: 4
Gerrit-Owner: Solomon Alan-Dei <alandei.solomon(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Fri, 28 Oct 2022 14:52:18 +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: Tarun Tuli, Michał Żygowski.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68945 )
Change subject: soc/intel/alderlake: Hook up the OC watchdog
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
I don't quite understand the usage of WDT in so early in the boot flow?
I would avoid adding more stuffs in bootblock as we would like to keep this stage as simple as possible to getting into any problem due to RO freeze
--
To view, visit https://review.coreboot.org/c/coreboot/+/68945
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1c2c640d48b7e03ad8cd8d6cdf6aac447e93cd86
Gerrit-Change-Number: 68945
Gerrit-PatchSet: 2
Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Comment-Date: Fri, 28 Oct 2022 14:05:12 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Michał Żygowski.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68944 )
Change subject: soc/intel/common/block/oc_wdt: Add OC watchdog common block
......................................................................
Patch Set 2:
(5 comments)
File src/soc/intel/common/block/oc_wdt/oc_wdt.c:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-161580):
https://review.coreboot.org/c/coreboot/+/68944/comment/cf723aa0_a65e8809
PS2, Line 65: readback = inl(PCH_OC_WDT_CTL);
code indent should use tabs where possible
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-161580):
https://review.coreboot.org/c/coreboot/+/68944/comment/64b44fc3_390a0cbd
PS2, Line 65: readback = inl(PCH_OC_WDT_CTL);
please, no space before tabs
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-161580):
https://review.coreboot.org/c/coreboot/+/68944/comment/17e26c32_777c51db
PS2, Line 65: readback = inl(PCH_OC_WDT_CTL);
please, no spaces at the start of a line
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-161580):
https://review.coreboot.org/c/coreboot/+/68944/comment/fd38a583_a85824e4
PS2, Line 80: printk (BIOS_ERR, "Watchdog: Failure detected\n");
space prohibited between function name and open parenthesis '('
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-161580):
https://review.coreboot.org/c/coreboot/+/68944/comment/5a6b6bf2_eb232522
PS2, Line 188: printk (BIOS_ERR, "Watchdog: timer expiration detected.\n");
space prohibited between function name and open parenthesis '('
--
To view, visit https://review.coreboot.org/c/coreboot/+/68944
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib494aa0c7581351abca8b496fc5895b2c7cbc5bc
Gerrit-Change-Number: 68944
Gerrit-PatchSet: 2
Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Comment-Date: Fri, 28 Oct 2022 13:59:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Michał Żygowski.
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/68944
to look at the new patch set (#2).
Change subject: soc/intel/common/block/oc_wdt: Add OC watchdog common block
......................................................................
soc/intel/common/block/oc_wdt: Add OC watchdog common block
Add new block for handling overcloking watchdog. The watchdog is
present since Skylake or maybe even earlier so it is safe to use with
most of the micrarchitectures utilizing intelblocks.
Some FSPs are also utilizing OC watchdog so care must be taken when
initializing it. Example integration provided in subsequent patch.
The goal is to provide coreboot with additional reliability settings.
The patch adds the common block for initializing and feeding the
watchdog. Timeout is configurable via Kconfig and cannot be set to
less than 60 seconds to avoid reset loops when full memory training
is needed.
The patch also adds support for feeding watchdog in driveless mode,
i.e. it utilizies periodic SMI to reload the timeout value and restart
the watchdog timer. THis is optional and selectabel by Kconfig option
as well. If the option is not enabled, payload and/or software must
ensure to keep feeding the watchdog, otherwise the platform will
reset.
TEST=Enable watchdog on MSI PRO Z690-A and see the platform resets
after some time. Enable the watchdog in driverless mode and see the
platform no longer resets and periodic SMI keeps feeding the watchdog.
Signed-off-by: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Change-Id: Ib494aa0c7581351abca8b496fc5895b2c7cbc5bc
---
A src/soc/intel/common/block/include/intelblocks/oc_wdt.h
A src/soc/intel/common/block/oc_wdt/Kconfig
A src/soc/intel/common/block/oc_wdt/Makefile.inc
A src/soc/intel/common/block/oc_wdt/oc_wdt.c
M src/soc/intel/common/block/smm/smihandler.c
M src/soc/intel/common/block/smm/smm.c
6 files changed, 336 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/68944/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/68944
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib494aa0c7581351abca8b496fc5895b2c7cbc5bc
Gerrit-Change-Number: 68944
Gerrit-PatchSet: 2
Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Hung-Te Lin, Jason Glenesk, Raul Rangel, Matt DeVillier, Julius Werner, Fred Reitberger, Felix Held.
Jakub Czapiga has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/66909 )
Change subject: vboot: Add VBOOT_CBFS_INTEGRATION support
......................................................................
Patch Set 18:
(1 comment)
File src/security/vboot/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/66909/comment/967d6773_5fc959cf
PS15, Line 104: bootblock-y += secdata_mock.c
> > Won't the change to `vboot_save_data()` with `die()` make it always just die instead of calling `v […]
I changed the code as You suggested. However I still do not understand one thing: Why will it work correctly, when `vboot_fail_and_reboot() -> vboot_save_and_reboot() -> vboot_save_data()`, which is called by `cbfs_file_hash_mismatch()` in case of incorrect CBFS file data hash, which can happen in any stage? What am I missing?
--
To view, visit https://review.coreboot.org/c/coreboot/+/66909
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I40ae01c477c4e4f7a1c90e4026a8a868ae64b5ca
Gerrit-Change-Number: 66909
Gerrit-PatchSet: 18
Gerrit-Owner: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Fri, 28 Oct 2022 13:55:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Jakub Czapiga <jacz(a)semihalf.com>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment