Attention is currently required from: Arthur Heymans.
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/56044
to look at the new patch set (#5).
Change subject: [TESTONLY]do we use cbfs-relative base addresses as input??
......................................................................
[TESTONLY]do we use cbfs-relative base addresses as input??
If no-one is using this, we might as well get rid of it?
Change-Id: I87349e98d6aeac5d3f79c8a067982cd136a0b863
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M util/cbfstool/cbfstool.c
1 file changed, 7 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/56044/5
--
To view, visit https://review.coreboot.org/c/coreboot/+/56044
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I87349e98d6aeac5d3f79c8a067982cd136a0b863
Gerrit-Change-Number: 56044
Gerrit-PatchSet: 5
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: newpatchset
Attention is currently required from: Patrick Rudolph.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56096 )
Change subject: TESTONLY: Link pagetables into programs instead of using a cbfs file
......................................................................
Patch Set 1:
(1 comment)
File src/cpu/x86/64bit/pt.S:
https://review.coreboot.org/c/coreboot/+/56096/comment/1138c642_9ec526a8
PS1, Line 14: .align 32
> This is the alignment relative to _program. Shouldn't all CBFS files using this assembly code then be aligned to 4096 as well?
>
> Does it work for bootblock where _eprogram is aligned to 4GiB instead of _program?
So bootblock is a special program. It is linked at the address it gets executed, which means this just works. Other programs should be aligned to 4096, which would be cbfstool's job.
Relocatable stages already have 4K runtime alignment afaict: https://review.coreboot.org/plugins/gitiles/coreboot/+/refs/heads/master/sr….
So the only thing left to do is 4K alignment for early programs, especially those that are XIP.
Anyway RO/part of program PT are not always needed. We might need to differentiate:
- Some platforms have enough CAR that you could generate it in place
- On Intel GLK, PT are copied from cbfs into CAR -> this could be probably be improved too
- RAM stages could have the PT in DRAM: care needs to be taken in postcar, e.g. if PT are in CAR. I think with stages in DRAM you want the PT in .bss or .data an just put an identity table in there, either generated or static.
- SMM is weird. You can't link a PT inside the stub, but the stub needs to know where it is: preferably inside the permanent handler or inside ramstage during relocation. That means a stub relocatable param pointing to it.
- Sipi Vector in ramstage: you probably also want a pointer to the ramstage PT inside the relocatable params.
--
To view, visit https://review.coreboot.org/c/coreboot/+/56096
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ice4fd15d131b8c91bc98afad7f459ce3485b786d
Gerrit-Change-Number: 56096
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Comment-Date: Tue, 06 Jul 2021 08:12:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans.
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56096 )
Change subject: TESTONLY: Link pagetables into programs instead of using a cbfs file
......................................................................
Patch Set 1:
(1 comment)
File src/cpu/x86/64bit/pt.S:
https://review.coreboot.org/c/coreboot/+/56096/comment/1f697318_f3e876de
PS1, Line 14: .align 32
This is the alignment relative to _program. Shouldn't all CBFS files using this assembly code then be aligned to 4096 as well?
Does it work for bootblock where _eprogram is aligned to 4GiB instead of _program?
--
To view, visit https://review.coreboot.org/c/coreboot/+/56096
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ice4fd15d131b8c91bc98afad7f459ce3485b786d
Gerrit-Change-Number: 56096
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Tue, 06 Jul 2021 07:48:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Sean Rhodes.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56088 )
Change subject: mainboard: Add Star Labs labtop series
......................................................................
Patch Set 8:
(2 comments)
File src/mainboard/starlabs/labtop/variants/cml/romstage.c:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-123481):
https://review.coreboot.org/c/coreboot/+/56088/comment/4805e1a9_2b4e535c
PS8, Line 106: const uint8_t ht = get_uint_option("hyper_threading", memupd->FspmConfig.HyperThreading);
line over 96 characters
File src/mainboard/starlabs/labtop/variants/tgl/devtree.c:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-123481):
https://review.coreboot.org/c/coreboot/+/56088/comment/71b51e66_35ba1a40
PS8, Line 13: // TODO: TGL needs power_limits_config[POWER_LIMITS_U_2_CORE] and power_limits_config[POWER_LIMITS_U_4_CORE]
line over 96 characters
--
To view, visit https://review.coreboot.org/c/coreboot/+/56088
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I090971a9e8d2be5b08be886d00d304607304b645
Gerrit-Change-Number: 56088
Gerrit-PatchSet: 8
Gerrit-Owner: Sean Rhodes <admin(a)starlabs.systems>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Sean Rhodes <admin(a)starlabs.systems>
Gerrit-Comment-Date: Tue, 06 Jul 2021 07:33:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Arthur Heymans has submitted this change. ( https://review.coreboot.org/c/coreboot/+/56042 )
Change subject: configs/config.foxconn_g41m: Build test with X86_64
......................................................................
configs/config.foxconn_g41m: Build test with X86_64
Change-Id: I755f2037bc9368e610eb97a2633aa66da7f626b0
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/56042
Reviewed-by: Patrick Rudolph <siro(a)das-labor.org>
Reviewed-by: Paul Menzel <paulepanter(a)mailbox.org>
Reviewed-by: Angel Pons <th3fanbus(a)gmail.com>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
---
A configs/config.foxconn_g41m
1 file changed, 3 insertions(+), 0 deletions(-)
Approvals:
build bot (Jenkins): Verified
Paul Menzel: Looks good to me, but someone else must approve
Patrick Rudolph: Looks good to me, approved
Angel Pons: Looks good to me, approved
diff --git a/configs/config.foxconn_g41m b/configs/config.foxconn_g41m
new file mode 100644
index 0000000..f5deea5
--- /dev/null
+++ b/configs/config.foxconn_g41m
@@ -0,0 +1,3 @@
+CONFIG_VENDOR_FOXCONN=y
+CONFIG_BOARD_FOXCONN_G41M=y
+CONFIG_USE_EXP_X86_64_SUPPORT=y
--
To view, visit https://review.coreboot.org/c/coreboot/+/56042
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I755f2037bc9368e610eb97a2633aa66da7f626b0
Gerrit-Change-Number: 56042
Gerrit-PatchSet: 4
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Attention is currently required from: Paul Menzel.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56042 )
Change subject: configs/config.foxconn_g41m: Build test with X86_64
......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/56042/comment/e559d25d_cf706425
PS1, Line 7: Buildtest
> Nit: Build test
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/56042
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I755f2037bc9368e610eb97a2633aa66da7f626b0
Gerrit-Change-Number: 56042
Gerrit-PatchSet: 2
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Tue, 06 Jul 2021 06:11:35 +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: Arthur Heymans.
Hello build bot (Jenkins), Paul Menzel, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/56042
to look at the new patch set (#3).
Change subject: configs/config.foxconn_g41m: Build test with X86_64
......................................................................
configs/config.foxconn_g41m: Build test with X86_64
Change-Id: I755f2037bc9368e610eb97a2633aa66da7f626b0
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
A configs/config.foxconn_g41m
1 file changed, 3 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/56042/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/56042
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I755f2037bc9368e610eb97a2633aa66da7f626b0
Gerrit-Change-Number: 56042
Gerrit-PatchSet: 3
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
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-MessageType: newpatchset