Attention is currently required from: Caveh Jalali, Forest Mittelberg.
Hello Caveh Jalali, Forest Mittelberg,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/81965?usp=email
to look at the new patch set (#2).
Change subject: ec/google/chromeec: Update ec_cmd_api.h and ec_commands.h
......................................................................
ec/google/chromeec: Update ec_cmd_api.h and ec_commands.h
Generated using update_ec_headers.sh [EC-DIR].
The original include/ec_commands.h version in the EC repo is:
5d59b97480 features: Add UCSI PPM feature flag
The original include/ec_cmd_api.h version in the EC repo is:
562316a71e include: Add fingerprint host commands to ec_cmd_api.h
BUG=b:333078787
TEST=cros build-packages --board brox \
chromeos-bootimage depthcharge coreboot
TEST=cros build-packages --board brya \
chromeos-bootimage depthcharge coreboot
BRANCH=none
Change-Id: I94b509cd6ad8f24bfc3b44ef02633d06320f1e22
Signed-off-by: Pavan Holla <pholla(a)google.com>
---
M src/ec/google/chromeec/ec_commands.h
1 file changed, 5 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/81965/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/81965?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I94b509cd6ad8f24bfc3b44ef02633d06320f1e22
Gerrit-Change-Number: 81965
Gerrit-PatchSet: 2
Gerrit-Owner: Pavan Holla <pholla(a)google.com>
Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Reviewer: Forest Mittelberg <bmbm(a)google.com>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Attention: Forest Mittelberg <bmbm(a)google.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Arthur Heymans, Julius Werner, Jérémy Compostella, Kapil Porwal, Nico Huber.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81960?usp=email )
Change subject: arch/x86: Enable long mode entry into payload for x86_64 support
......................................................................
Patch Set 2:
(1 comment)
File src/arch/x86/boot.c:
https://review.coreboot.org/c/coreboot/+/81960/comment/1a5229d0_2fe6659a :
PS1, Line 25: if (CONFIG(PAYLOAD_X86_64_SUPPORT)) {
> > > > Would it be possible to make it an offset? Then we could have a 32-bit entry
> > > > point followed by a 64-bit one, and a coreboot that knows the new ABI could skip
> > > > the first and jump directly to the new entry point. This would allow the most
> > > > compatibility between coreboot and payloads, as we could say the 64-bit entry
> > > > point is optional on both ends (except for X86S, ofc.).
> > > >
> > > > I guess the complexity of such a 32-bit entry point, that switches to long mode,
> > > > would depend a lot on the expectations on page tables in the new ABI.
> > >
> >
> > I guess a minimal expectation of at least 4G identity mapped is reasonable. Iirc UPL does that too. I like the idea of multiple entry points over a CBFS attribute as it's indeed the best way to be remain compatible. Linux already has exactly this btw. Question remains on how to achieve this? Have an expectation on the namespace in the ELF file which cbfstool can pick up? .text._entry64?
>
> can we review CB:81964 that adds the payload cbfs attribute for 64-bit. Then this CL has now updated to fetch the cbfs attribute rather reading any static Kconfig.
following kernel concurrent 32-bit and 64-bit entry implementation may not be desired in AP FW as out configuration are very much static. We have to anyway select Kconfig in SoC to know if we are building 32-bit coreboot vs 64-bit coreboot hence, rather than complicating the flow, not sure what else we will bring by adding kernel like implementation while transferring control from coreboot to payload.
Anyway, lets review this CL to ensure we have atleast some ways to launch 64-bit payload from a 64-bit coreboot w/o lowering into 32-bit mode. Later we can desire more scalable approach for handoff. btw, i will bring x64 support cls next for libpayload.
--
To view, visit https://review.coreboot.org/c/coreboot/+/81960?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ic5e6f0af11c05e8b075b8c20880c012747a1df9b
Gerrit-Change-Number: 81960
Gerrit-PatchSet: 2
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Thu, 18 Apr 2024 14:06:00 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Julius Werner, Jérémy Compostella, Kapil Porwal, Nico Huber.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81960?usp=email )
Change subject: arch/x86: Enable long mode entry into payload for x86_64 support
......................................................................
Patch Set 2:
(1 comment)
File src/arch/x86/boot.c:
https://review.coreboot.org/c/coreboot/+/81960/comment/5e384d9e_15591bd4 :
PS1, Line 25: if (CONFIG(PAYLOAD_X86_64_SUPPORT)) {
> > > Would it be possible to make it an offset? Then we could have a 32-bit entry
> > > point followed by a 64-bit one, and a coreboot that knows the new ABI could skip
> > > the first and jump directly to the new entry point. This would allow the most
> > > compatibility between coreboot and payloads, as we could say the 64-bit entry
> > > point is optional on both ends (except for X86S, ofc.).
> > >
> > > I guess the complexity of such a 32-bit entry point, that switches to long mode,
> > > would depend a lot on the expectations on page tables in the new ABI.
> >
>
> I guess a minimal expectation of at least 4G identity mapped is reasonable. Iirc UPL does that too. I like the idea of multiple entry points over a CBFS attribute as it's indeed the best way to be remain compatible. Linux already has exactly this btw. Question remains on how to achieve this? Have an expectation on the namespace in the ELF file which cbfstool can pick up? .text._entry64?
can we review CB:81964 that adds the payload cbfs attribute for 64-bit. Then this CL has now updated to fetch the cbfs attribute rather reading any static Kconfig.
--
To view, visit https://review.coreboot.org/c/coreboot/+/81960?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ic5e6f0af11c05e8b075b8c20880c012747a1df9b
Gerrit-Change-Number: 81960
Gerrit-PatchSet: 2
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Thu, 18 Apr 2024 13:58:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Attention is currently required from: Julius Werner, Jérémy Compostella, Kapil Porwal, Nico Huber, Subrata Banik.
Hello Arthur Heymans, Julius Werner, Jérémy Compostella, Kapil Porwal, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/81960?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: arch/x86: Enable long mode entry into payload for x86_64 support
......................................................................
arch/x86: Enable long mode entry into payload for x86_64 support
This patch allows coreboot to enter libpayload in long mode (64-bit)
instead of protected mode (32-bit), enabling support for upcoming Intel
platforms and payloads requiring access to memory beyond 4GB.
This change ensures compatibility between libpayload and depthcharge,
preventing compilation issues and stack misalignment during the
transition.
BUG=b:242829490
TEST=Entered libpayload in long mode, successfully parsed coreboot
table.
Change-Id: Ic5e6f0af11c05e8b075b8c20880c012747a1df9b
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
---
M src/arch/x86/boot.c
1 file changed, 33 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/81960/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/81960?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ic5e6f0af11c05e8b075b8c20880c012747a1df9b
Gerrit-Change-Number: 81960
Gerrit-PatchSet: 2
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Krystian Hebel, Michał Kopeć, Michał Żygowski, Piotr Król, Piotr Kubaj.
Maciej Pijanowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81963?usp=email )
Change subject: mb/protectli/vault_cml: use combo v1/v2 FSP
......................................................................
Patch Set 2:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/81963/comment/e3a0931a_019828e4 :
PS2, Line 8:
It is rather unlikely to submit patches with no description at all. We would like to learn what is the reason behind this change, and what it brings. Some information what has been tested would be nice, too. Even here you had an example: https://github.com/Dasharo/coreboot/pull/423
Patchset:
PS2:
Does it make sense not to squash these patches into one? It seems both are needed to provide this feature, being part of one logical change.
--
To view, visit https://review.coreboot.org/c/coreboot/+/81963?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I1a6f6e873e4ec35b9777dc17c0495151348d1d88
Gerrit-Change-Number: 81963
Gerrit-PatchSet: 2
Gerrit-Owner: Piotr Kubaj <pkubaj(a)anongoth.pl>
Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Reviewer: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com>
Gerrit-Attention: Piotr Kubaj <pkubaj(a)anongoth.pl>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Attention: Piotr Król <piotr.krol(a)3mdeb.com>
Gerrit-Comment-Date: Thu, 18 Apr 2024 13:48:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment