Attention is currently required from: Julius Werner, Patrick Rudolph.
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61960 )
Change subject: bsd/cb_err: Add error code for UEFI variable store
......................................................................
Patch Set 2:
(1 comment)
File src/commonlib/bsd/include/commonlib/bsd/cb_err.h:
https://review.coreboot.org/c/coreboot/+/61960/comment/2a1ca1dd_8b43def6
PS1, Line 27: CB_EFI_FVH_INVALID = -105, /**< UEFI FVH is corrupted */
> I think this should just go by prefix. If you call them CB_EFI_..., they're a new category. […]
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/61960
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I6baea9fb138d1a2755d22a3d587105793adb9c90
Gerrit-Change-Number: 61960
Gerrit-PatchSet: 2
Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Joey Madafferi <joeymadafferi(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-CC: Patrick Rudolph
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Patrick Rudolph
Gerrit-Comment-Date: Thu, 03 Mar 2022 09:31:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Comment-In-Reply-To: Patrick Rudolph
Gerrit-MessageType: comment
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/62015
to look at the new patch set (#5).
Change subject: Documentation: Add RFC how to handle UEFI variables
......................................................................
Documentation: Add RFC how to handle UEFI variables
Describe how the UEFI variable store should be used.
Change-Id: Iddf43a9ff6bf25232fbe2aa8aae2e466e5514492
Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com>
---
A Documentation/RFC/efivars.md
1 file changed, 88 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/62015/5
--
To view, visit https://review.coreboot.org/c/coreboot/+/62015
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iddf43a9ff6bf25232fbe2aa8aae2e466e5514492
Gerrit-Change-Number: 62015
Gerrit-PatchSet: 5
Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Sean Rhodes, Michał Żygowski, Martin Roth, Jakub Czapiga, Patrick Rudolph.
Hello build bot (Jenkins), Sean Rhodes, Martin Roth, Jakub Czapiga, Matt DeVillier, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/52564
to look at the new patch set (#10).
Change subject: drivers/efi: Add EFI variable store option support
......................................................................
drivers/efi: Add EFI variable store option support
Add a driver to read and write EFI variables stored in a region device.
This is particullary useful for EDK2 as payload and allows to reuse
existing EFI tools to set/get options used by the firmware.
The write implementation is fault tolerant and doesn't corrupt the
variable store. A faulting write might result in using the old value
even though a 'newer' had been completely written.
Implemented basic unit tests for header corruption, writing existing data
and append new data into the store.
Initial firmware region state:
Initially the variable store region isn't formatted. Usually this is done
in the EDK2 payload when no valid firmware volume could be found.
It might be useful to do this offline or in coreboot to have a working
option store on the first boot or when it was corrupted.
Performance improvements:
Right now the code always checks if the firmware volume header is valid.
This could be optimised by caching the test result in heap. For write
operations it would be good to cache the end of the variable store in
the heap as well, instead of walking the whole store.
Reclaiming memory:
The EFI variable store is append write only. To update an existing
variable, first a new is written to the end of the store and then the
previous is marked invalid. This only works on PNOR flash that allow to
clear set bits, but keep cleared bits state.
This mechanisms allows a fault tolerant write, but it also requires to
"clean" the variable store for time to time. This cleaning would remove
variables that have been marked "deleted".
Such cleaning mechanism in turn must be fault tolerant and thus must use
a second parition in the SPI flash as backup/working region.
For now to cleaning is done in coreboot.
Fault checking:
The driver should check if a previous write was successfull and if not mark
variables as deleted on the next operation.
Tested and working:
- Could enumerate all existing variables
- Could read variables
- Could write variables
Change-Id: I8079f71d29da5dc2db956fc68bef1486fe3906bb
Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com>
---
A src/drivers/efi/Kconfig
A src/drivers/efi/Makefile.inc
A src/drivers/efi/efivars.c
A src/drivers/efi/efivars.h
M tests/Makefile.inc
A tests/drivers/Makefile.inc
A tests/drivers/efivars.c
7 files changed, 866 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/64/52564/10
--
To view, visit https://review.coreboot.org/c/coreboot/+/52564
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I8079f71d29da5dc2db956fc68bef1486fe3906bb
Gerrit-Change-Number: 52564
Gerrit-PatchSet: 10
Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-CC: Patrick Rudolph
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Patrick Rudolph.
Hello build bot (Jenkins), Joey Madafferi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/61960
to look at the new patch set (#2).
Change subject: bsd/cb_err: Add error code for UEFI variable store
......................................................................
bsd/cb_err: Add error code for UEFI variable store
Add a new set of errors that will be used by the introduced EFI
non-volatile variable store in flash.
Change-Id: I6baea9fb138d1a2755d22a3d587105793adb9c90
Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com>
---
M src/commonlib/bsd/include/commonlib/bsd/cb_err.h
1 file changed, 13 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/61960/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/61960
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I6baea9fb138d1a2755d22a3d587105793adb9c90
Gerrit-Change-Number: 61960
Gerrit-PatchSet: 2
Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Joey Madafferi <joeymadafferi(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-CC: Patrick Rudolph
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Patrick Rudolph
Gerrit-MessageType: newpatchset
Attention is currently required from: Hung-Te Lin, Arthur Heymans, Paul Menzel, Yu-Ping Wu.
Jianjun Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56793 )
Change subject: soc/mediatek: Enable PCIe support for mt8195
......................................................................
Patch Set 13:
(1 comment)
File src/soc/mediatek/mt8195/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/56793/comment/1c3b5b89_fe0b43d6
PS12, Line 73: +=
> can we add them only if CONFIG_PCI is set?
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/56793
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I314572955f1021abe9f2f0f4635670135ed08fff
Gerrit-Change-Number: 56793
Gerrit-PatchSet: 13
Gerrit-Owner: Jianjun Wang <jianjun.wang(a)mediatek.corp-partner.google.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Comment-Date: Thu, 03 Mar 2022 09:24:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Paul Menzel, Yu-Ping Wu.
Jianjun Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56793 )
Change subject: soc/mediatek: Enable PCIe support for mt8195
......................................................................
Patch Set 13:
(1 comment)
File src/soc/mediatek/mt8195/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/56793/comment/3efff245_e960da5f
PS11, Line 73: pcie.c
> Move to previous CL. It was not clear in which stage the code ought to be used.
Done, thanks for your review.
--
To view, visit https://review.coreboot.org/c/coreboot/+/56793
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I314572955f1021abe9f2f0f4635670135ed08fff
Gerrit-Change-Number: 56793
Gerrit-PatchSet: 13
Gerrit-Owner: Jianjun Wang <jianjun.wang(a)mediatek.corp-partner.google.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Comment-Date: Thu, 03 Mar 2022 09:24:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-MessageType: comment
Attention is currently required from: Hung-Te Lin, Shelley Chen, Paul Menzel, Yu-Ping Wu.
Jianjun Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62359 )
Change subject: soc/mediatek: PCI: Assert PERST# at bootblock stage
......................................................................
Patch Set 2:
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/62359/comment/d76c6016_a707746c
PS1, Line 10: clock to become stable.
> Maybe reference the specification?
Done
File src/soc/mediatek/mt8195/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/62359/comment/75eefafb_7da12a6a
PS1, Line 16: +=
> add only if CONFIG_PCI ?
Done
File src/soc/mediatek/mt8195/bootblock.c:
https://review.coreboot.org/c/coreboot/+/62359/comment/a0f0b4f1_b2573d1d
PS1, Line 18: mtk_pcie_pre_init
> What about […]
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/62359
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Id5b9369e6f8599f93415588ea585c952a41c5e7d
Gerrit-Change-Number: 62359
Gerrit-PatchSet: 2
Gerrit-Owner: Jianjun Wang <jianjun.wang(a)mediatek.corp-partner.google.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Shelley Chen <shchen(a)google.com>
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: Shelley Chen <shchen(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Comment-Date: Thu, 03 Mar 2022 09:23:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Hung-Te Lin <hungte(a)chromium.org>
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: comment
Attention is currently required from: Hung-Te Lin, Arthur Heymans, Shelley Chen, Nico Huber, Furquan Shaikh, Paul Menzel, Angel Pons, Yu-Ping Wu.
Jianjun Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56791 )
Change subject: soc/mediatek: Add PCIe support
......................................................................
Patch Set 11:
(8 comments)
File src/soc/mediatek/common/pcie.c:
https://review.coreboot.org/c/coreboot/+/56791/comment/ff07555b_43001940
PS9, Line 108: pcie_ctrl->base + PCIE_CFG_OFFSET_ADDR;
: }
> I think it's totally fine to move the read32p/write32p to device/mmio. […]
Done, moved to device/mmio.h:
https://review.coreboot.org/c/coreboot/+/62561/1
File src/soc/mediatek/common/pcie.c:
https://review.coreboot.org/c/coreboot/+/56791/comment/eee44677_3c56e088
PS10, Line 120: val = read32(ctrl->base + PCIE_SETTING_REG);
: val |= PCIE_RC_MODE;
: write32(ctrl->base + PCIE_SETTING_REG, val);
> `setbits32()` expects a pointer, so this would need to be: […]
I use write32p() instead, is that OK?
https://review.coreboot.org/c/coreboot/+/56791/comment/582366ac_a05dfdef
PS10, Line 141: ctrl->base + PCIE_RST_CTRL_REG
> After retyping `base` to `uintptr_t`, this needs a cast (or the parameter type of the function point […]
Done
https://review.coreboot.org/c/coreboot/+/56791/comment/867bd48c_7c96a03d
PS10, Line 147: >
> >=
Done
https://review.coreboot.org/c/coreboot/+/56791/comment/e84bb85b_536b4bce
PS10, Line 154: ms
> This is incorrect. 'tries' is the number of tries, not elapsed time.
Done
https://review.coreboot.org/c/coreboot/+/56791/comment/e6cab86d_357c2928
PS10, Line 193: table + PCIE_ATR_SRC_ADDR_MSB_OFFSET
> +1
Done
https://review.coreboot.org/c/coreboot/+/56791/comment/86a9fa50_2a500192
PS10, Line 204: }
> Is it necessary to report the resource and configure the hardware […]
Done
https://review.coreboot.org/c/coreboot/+/56791/comment/4f5fdc89_9fbdb577
PS10, Line 231: return;
> Why is this done here and not in `chip_ops->enable_dev()` or […]
Moved to ops->enable(), thanks for your review.
--
To view, visit https://review.coreboot.org/c/coreboot/+/56791
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib9b6adaafa20aeee136372ec9564273f86776da0
Gerrit-Change-Number: 56791
Gerrit-PatchSet: 11
Gerrit-Owner: Jianjun Wang <jianjun.wang(a)mediatek.corp-partner.google.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Shelley Chen <shchen(a)google.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-CC: Furquan Shaikh <furquan.m.shaikh(a)gmail.com>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-Attention: Shelley Chen <shchen(a)google.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Furquan Shaikh <furquan.m.shaikh(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Comment-Date: Thu, 03 Mar 2022 09:21:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Hung-Te Lin <hungte(a)chromium.org>
Comment-In-Reply-To: Arthur Heymans <arthur.heymans(a)9elements.com>
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com>
Comment-In-Reply-To: Jianjun Wang <jianjun.wang(a)mediatek.corp-partner.google.com>
Gerrit-MessageType: comment