Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/83124?usp=email )
(
1 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
)Change subject: Makefile: update clean-symlink target
......................................................................
Makefile: update clean-symlink target
This almost completely replaces the original clean-symlink target to
remove links from site-local into the coreboot tree. Changes include:
- Symbolic links removed are based on the EXTERNAL_SYMLINKS value of
symlink.txt files under site-local.
- Verify that there are site-local symlink.txt files to work on before
doing anything.
- Verify that the symlink.txt files reference links inside the coreboot
directory.
- Print out whether or not there are remaining symbolic links in the
tree.
Signed-off-by: Martin Roth <gaumless(a)gmail.com>
Change-Id: Ife0e7cf1b856b7394cd5e1de9b35856bd984663c
Reviewed-on: https://review.coreboot.org/c/coreboot/+/83124
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Eric Lai <ericllai(a)google.com>
Reviewed-by: Elyes Haouas <ehaouas(a)noos.fr>
---
M Makefile
1 file changed, 29 insertions(+), 5 deletions(-)
Approvals:
Elyes Haouas: Looks good to me, approved
build bot (Jenkins): Verified
Eric Lai: Looks good to me, but someone else must approve
diff --git a/Makefile b/Makefile
index 60570a0..2dddaab 100644
--- a/Makefile
+++ b/Makefile
@@ -519,11 +519,35 @@
done
clean-symlink:
- @echo "Deleting symbolic link";\
- EXISTING_SYMLINKS=`find -L ./src -xtype l | grep -v 3rdparty`; \
- for link in $$EXISTING_SYMLINKS; do \
- echo -e "\tUNLINK $$link"; \
- rm "$$link"; \
+ if [ -z "$(SYMLINK_LIST)" ]; then \
+ echo "No site-local symbolic links to clean."; \
+ exit 0; \
+ fi; \
+ echo "Removing site-local symbolic links from tree.."; \
+ for link in $(SYMLINK_LIST); do \
+ SYMLINK="$(top)/$$(head -n 1 "$${link}")"; \
+ if [ "$${SYMLINK}" = "$$(echo "$${SYMLINK}" | sed "s|^$(top)||")" ]; then \
+ echo " FAILED: $${SYMLINK} is outside of current directory." >&2; \
+ continue; \
+ elif [ ! -L "$${SYMLINK}" ]; then \
+ echo " $${SYMLINK} does not exist - skipping"; \
+ continue; \
+ fi; \
+ if [ -L "$${SYMLINK}" ]; then \
+ REALDIR="$$(realpath "$${link}")"; \
+ echo " UNLINK $${link} (linked from $${REALDIR})"; \
+ rm "$${SYMLINK}"; \
+ fi; \
+ done; \
+ EXISTING_SYMLINKS="$$(find $(top) -type l | grep -v "3rdparty\|crossgcc" )"; \
+ if [ -z "$${EXISTING_SYMLINKS}" ]; then \
+ echo " No remaining symbolic links found in tree."; \
+ else \
+ echo " Remaining symbolic links found:"; \
+ for link in $${EXISTING_SYMLINKS}; do \
+ echo " $${link}"; \
+ done; \
+ fi
cleanall-symlink:
echo "Deleting all symbolic links in the coreboot tree (excluding 3rdparty & crossgcc)"; \
--
To view, visit https://review.coreboot.org/c/coreboot/+/83124?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ife0e7cf1b856b7394cd5e1de9b35856bd984663c
Gerrit-Change-Number: 83124
Gerrit-PatchSet: 3
Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Attention is currently required from: Felix Singer, Martin L Roth, Maximilian Brune.
Martin Roth has posted comments on this change by Martin L Roth. ( https://review.coreboot.org/c/coreboot/+/83124?usp=email )
Change subject: Makefile: update clean-symlink target
......................................................................
Patch Set 2:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/83124/comment/9a6c3f2c_4301bcc6?us… :
PS1, Line 9: ued
> used?
Done
https://review.coreboot.org/c/coreboot/+/83124/comment/0ade2fb2_cfccada6?us… :
PS1, Line 14: fies
> files
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/83124?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ife0e7cf1b856b7394cd5e1de9b35856bd984663c
Gerrit-Change-Number: 83124
Gerrit-PatchSet: 2
Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Comment-Date: Mon, 01 Jul 2024 16:48:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Attention is currently required from: Martin L Roth, Maximilian Brune.
Martin Roth has uploaded a new patch set (#2) to the change originally created by Martin L Roth. ( https://review.coreboot.org/c/coreboot/+/83124?usp=email )
Change subject: Makefile: update clean-symlink target
......................................................................
Makefile: update clean-symlink target
This almost completely replaces the original clean-symlink target to
remove links from site-local into the coreboot tree. Changes include:
- Symbolic links removed are based on the EXTERNAL_SYMLINKS value of
symlink.txt files under site-local.
- Verify that there are site-local symlink.txt files to work on before
doing anything.
- Verify that the symlink.txt files reference links inside the coreboot
directory.
- Print out whether or not there are remaining symbolic links in the
tree.
Signed-off-by: Martin Roth <gaumless(a)gmail.com>
Change-Id: Ife0e7cf1b856b7394cd5e1de9b35856bd984663c
---
M Makefile
1 file changed, 29 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/83124/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83124?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ife0e7cf1b856b7394cd5e1de9b35856bd984663c
Gerrit-Change-Number: 83124
Gerrit-PatchSet: 2
Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Attention is currently required from: Felix Singer, Martin L Roth, Martin Roth, Maximilian Brune.
Elyes Haouas has posted comments on this change by Maximilian Brune. ( https://review.coreboot.org/c/coreboot/+/83296?usp=email )
Change subject: Revert "util/crossgcc: Update ACPICA from 20230628 to 20240321"
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS3:
> Should we just update crossgcc to use our own mirror of the tarballs by default? Currently it's an o […]
is there a way to compile from git as we do for 3rdparty?
--
To view, visit https://review.coreboot.org/c/coreboot/+/83296?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I3ce0c5798f14162eaa063a9a64e16e6dbbb9e468
Gerrit-Change-Number: 83296
Gerrit-PatchSet: 3
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-CC: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Comment-Date: Mon, 01 Jul 2024 16:30:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Attention is currently required from: Arthur Heymans, Felix Held, Philipp Hug, Ron Minnich.
Maximilian Brune has posted comments on this change by Maximilian Brune. ( https://review.coreboot.org/c/coreboot/+/83098?usp=email )
Change subject: arch/riscv: Factor out common romstage code
......................................................................
Patch Set 5:
(1 comment)
File src/mainboard/sifive/hifive-unmatched/romstage.c:
https://review.coreboot.org/c/coreboot/+/83098/comment/3e7d5253_30313be6?us… :
PS5, Line 19: console_init();
> console_init is already called in the common code before calling this function. […]
UART subsystem depends on `clock_init()`. I meant to remove it and do it like on the unleashed board. I have a few more patches and tests to do on the unmatched board anyway. If you don't mind I will take care of that in another patch.
Acknowledged
--
To view, visit https://review.coreboot.org/c/coreboot/+/83098?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ieb11d2644bf42dacf89ef15b2ec51286fe729d64
Gerrit-Change-Number: 83098
Gerrit-PatchSet: 5
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx>
Gerrit-Reviewer: Ron Minnich <rminnich(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Attention: Philipp Hug <philipp(a)hug.cx>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Ron Minnich <rminnich(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Mon, 01 Jul 2024 16:21:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>
Attention is currently required from: Felix Singer, Martin L Roth, Maximilian Brune.
Martin Roth has posted comments on this change by Maximilian Brune. ( https://review.coreboot.org/c/coreboot/+/83296?usp=email )
Change subject: Revert "util/crossgcc: Update ACPICA from 20230628 to 20240321"
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS3:
Should we just update crossgcc to use our own mirror of the tarballs by default? Currently it's an option.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83296?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I3ce0c5798f14162eaa063a9a64e16e6dbbb9e468
Gerrit-Change-Number: 83296
Gerrit-PatchSet: 3
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-CC: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Comment-Date: Mon, 01 Jul 2024 16:02:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Simon Glass.
Martin Roth has posted comments on this change by Simon Glass. ( https://review.coreboot.org/c/coreboot/+/83293?usp=email )
Change subject: Support CCB at a fixed SPI-flash offset
......................................................................
Patch Set 2:
(7 comments)
File Documentation/technotes/ccb.md:
https://review.coreboot.org/c/coreboot/+/83293/comment/c2aacbf3_e58c6ca1?us… :
PS2, Line 88: Care should be taken that
: this region is in read-only flash, if preventing users from changing it is
: important.
:
This is true for chromeos, but not every platform.
File Makefile.mk:
https://review.coreboot.org/c/coreboot/+/83293/comment/620f75e3_acae9ffd?us… :
PS2, Line 1124: ifeq ($(CONFIG_CCB_FMAP),y)
Maybe print a warning or just error out if the CCB FMAP alignment is different than what's in the fmap file?
https://review.coreboot.org/c/coreboot/+/83293/comment/af163b22_f16cbe24?us… :
PS2, Line 1259: ifneq ($(CONFIG_ARCH_X86),)
Make this the `else` of the above? Reverse the logic?
https://review.coreboot.org/c/coreboot/+/83293/comment/0708f996_0cac1ad4?us… :
PS2, Line 1274: /usr/bin/printf
Maybe use `env printf` instead of the path?
File src/lib/bootblock.c:
https://review.coreboot.org/c/coreboot/+/83293/comment/cac741f2_b93c8b2c?us… :
PS2, Line 63: or FMAP
Why does it need to be initialized late if it's at a known location in the SPI rom? Should this be architecture dependent and different for SoCs that have memory mapped SPI ROMs vs any that don't?
File src/lib/ccb.c:
https://review.coreboot.org/c/coreboot/+/83293/comment/e8acd97d_8375d0d0?us… :
PS2, Line 102: if (fmap_locate_area_as_rdev(CCB_REGION, rdev)) {
It seems to me that the advantage of using FMAP vs CBFS is that we can parse the fmap at build time and just get a direct address for the table so that we don't have to deal with finding the FMAP and then finding the CCB. This implementation seems no better than CBFS to me.
File util/cbfstool/cbfstool.c:
https://review.coreboot.org/c/coreboot/+/83293/comment/2755e319_d3807051?us… :
PS2, Line 731: /* Now try FMAP */
Based on the Kconfig, we can't have CCB in more than one location. Why check more than the one that's specified in Kconfig? Doesn't that create a security hole?
--
To view, visit https://review.coreboot.org/c/coreboot/+/83293?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I8abc5ba55d75a3defdea548fffcedba74d4737c2
Gerrit-Change-Number: 83293
Gerrit-PatchSet: 2
Gerrit-Owner: Simon Glass <sjg(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: Simon Glass <sjg(a)chromium.org>
Gerrit-Comment-Date: Mon, 01 Jul 2024 15:57:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Lean Sheng Tan, Mario Scheithauer, Werner Zeh.
Felix Singer has posted comments on this change by Felix Singer. ( https://review.coreboot.org/c/coreboot/+/83294?usp=email )
Change subject: 3rdparty/fsp: Update submodule to upstream master
......................................................................
Patch Set 3:
(1 comment)
File src/soc/intel/elkhartlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/83294/comment/797f5744_38f8b8fc?us… :
PS3, Line 195: FSPRel.bin"
> We just recently changed it in CB:81344. […]
Yeah, this is what they have on their [master branch](https://github.com/intel/FSP/tree/master/ElkhartLakeFspBinPkg/FspBi….
I think there is a bit chaos in the FSP repository.. Look at the [commit history](Here is the commit history for the FspBin directory. https://github.com/intel/FSP/commits/master/ElkhartLakeFspBinPkg/FspBin) for the FspBin directory.
When I updated the submodule pointer in CB:81344, which is still in use, commit [4707bc7ab7](https://github.com/intel/FSP/commit/4707bc7ab793a1c5f179743f74d… gets introduced, which adds FSP.fd for a newer FSP version, but FSPRel.bin from an older version still remains. @mario.scheithauer@siemens.com pointed out that FSP.fd is the correct name.
In the follow-up commit [aada6a55e1](https://github.com/intel/FSP/commit/aada6a55e1b4c0d9eeb04b7bbe6…, which is pulled in with this change, the commit message suggests that FSPRel.bin is removed, but actually FSP.fd is removed and FSPRel.bin is updated to the version of FSP.fd.
Well, and no filename change with the most [recent commit](https://github.com/intel/FSP/commit/8134dbd3dcfadee8a75300c0a6ef2b7….
--
To view, visit https://review.coreboot.org/c/coreboot/+/83294?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I47013bce65054f2c496c9aa7c16e55b51d65e5fe
Gerrit-Change-Number: 83294
Gerrit-PatchSet: 3
Gerrit-Owner: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Comment-Date: Mon, 01 Jul 2024 15:44:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Werner Zeh <werner.zeh(a)siemens.com>
Hello Martin L Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83297?usp=email
to look at the new patch set (#2).
Change subject: test build IASL
......................................................................
test build IASL
Change-Id: I05a25cefe042411c5ea94b8175c109574ee8ee00
Signed-off-by: Elyes Haouas <ehaouas(a)noos.fr>
---
M util/crossgcc/buildgcc
R util/crossgcc/patches/acpica-unix2-20240321_iasl.patch
D util/crossgcc/sum/acpica-unix-20240321.tar.gz.cksum
A util/crossgcc/sum/acpica-unix2-20240321.tar.gz.cksum
4 files changed, 17 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/97/83297/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83297?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I05a25cefe042411c5ea94b8175c109574ee8ee00
Gerrit-Change-Number: 83297
Gerrit-PatchSet: 2
Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Elyes Haouas has restored this change. ( https://review.coreboot.org/c/coreboot/+/83297?usp=email )
Change subject: test build IASL
......................................................................
Restored
--
To view, visit https://review.coreboot.org/c/coreboot/+/83297?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: restore
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I05a25cefe042411c5ea94b8175c109574ee8ee00
Gerrit-Change-Number: 83297
Gerrit-PatchSet: 1
Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>