Attention is currently required from: Patrick Rudolph.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59917 )
Change subject: [WIP]soc/intel/systemagent.c: Optimize resources for MTRR solution
......................................................................
Patch Set 2:
(1 comment)
File src/soc/intel/common/block/systemagent/systemagent.c:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-135014):
https://review.coreboot.org/c/coreboot/+/59917/comment/74a25ba9_b5988734
PS2, Line 207: reserved_ram_resource(dev, index++, base_k / KiB, size_k / KiB);
code indent should use tabs where possible
--
To view, visit https://review.coreboot.org/c/coreboot/+/59917
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I11c910d1002bcf6e782d89a209d12d12bebb45d4
Gerrit-Change-Number: 59917
Gerrit-PatchSet: 2
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Mon, 06 Dec 2021 14:21:52 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Patrick Rudolph.
Hello Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/59917
to look at the new patch set (#2).
Change subject: [WIP]soc/intel/systemagent.c: Optimize resources for MTRR solution
......................................................................
[WIP]soc/intel/systemagent.c: Optimize resources for MTRR solution
Is this a good idea?
Change-Id: I11c910d1002bcf6e782d89a209d12d12bebb45d4
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M src/soc/intel/common/block/systemagent/systemagent.c
1 file changed, 10 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/59917/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/59917
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I11c910d1002bcf6e782d89a209d12d12bebb45d4
Gerrit-Change-Number: 59917
Gerrit-PatchSet: 2
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Patrick Georgi, Julius Werner, Jan Dabros.
Jakub Czapiga has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59843 )
Change subject: libpayload/Makefile: Improve object files list creation
......................................................................
Patch Set 1:
(2 comments)
File payloads/libpayload/Makefile:
https://review.coreboot.org/c/coreboot/+/59843/comment/16e1b668_c9e0d7fe
PS1, Line 322: # $1 lib name
> the toplevel Makefile uses "stage name" here. […]
Are you sure, we should change it? coreboot has stages after all, and libpayload does have libs. "class" is just an abstract name for "bag of files". I *think* that these names fit well.
https://review.coreboot.org/c/coreboot/+/59843/comment/964386ec_e6022544
PS1, Line 373: cscope -bR -s$(coreboottop)/src/commonlib/bsd
> This seems to be unrelated?
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/59843
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If1280c0a3f7e99aad2ecf8a0379a98af31ccefc3
Gerrit-Change-Number: 59843
Gerrit-PatchSet: 1
Gerrit-Owner: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Reviewer: Jan Dabros <jsd(a)semihalf.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Jan Dabros <jsd(a)semihalf.com>
Gerrit-Comment-Date: Mon, 06 Dec 2021 14:17:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Patrick Georgi <patrick(a)coreboot.org>
Gerrit-MessageType: comment
Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/59917 )
Change subject: [WIP]soc/intel/systemagent.c: Optimize resources for MTRR solution
......................................................................
[WIP]soc/intel/systemagent.c: Optimize resources for MTRR solution
Is this a good idea?
Change-Id: I11c910d1002bcf6e782d89a209d12d12bebb45d4
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M src/soc/intel/common/block/systemagent/systemagent.c
1 file changed, 9 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/59917/1
diff --git a/src/soc/intel/common/block/systemagent/systemagent.c b/src/soc/intel/common/block/systemagent/systemagent.c
index 5868a16..63b20f9 100644
--- a/src/soc/intel/common/block/systemagent/systemagent.c
+++ b/src/soc/intel/common/block/systemagent/systemagent.c
@@ -180,7 +180,11 @@
size_k = sa_map_values[SA_TSEG_REG] - base_k;
if (size_k) {
printk(BIOS_DEBUG, "%s UC memory: base=0x%lx, size=0x%lx\n", __func__, base_k, size_k);
- mmio_resource(dev, index++, base_k / KiB, size_k / KiB);
+ /*
+ * This is likely DRAM, no idea why it's supposed to be UC? Mark it as reserved
+ * DRAM to optimize the MTRR solution.
+ */
+ reserved_dram_resource(dev, index++, base_k / KiB, size_k / KiB);
}
/* TSEG */
@@ -196,6 +200,10 @@
size_k = sa_map_values[SA_TOLUD_REG] - base_k;
if (size_k) {
printk(BIOS_DEBUG, "%s GSM: base=0x%lx, size=0x%lx\n", __func__, base_k, size_k);
+ /*
+ * This dram is taked by the IGD, but is not supposed to be accessed
+ * by the host anyway. Mark it as a reserved ram resource to optimize MTRRs.
+ */
mmio_resource(dev, index++, base_k / KiB, size_k / KiB);
}
--
To view, visit https://review.coreboot.org/c/coreboot/+/59917
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I11c910d1002bcf6e782d89a209d12d12bebb45d4
Gerrit-Change-Number: 59917
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: newchange
Attention is currently required from: Patrick Georgi.
Jakub Czapiga has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59916 )
Change subject: libpayload: Provide includes for payloads
......................................................................
Patch Set 2:
(3 comments)
File payloads/libpayload/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/59916/comment/9addba7f_08175cea
PS1, Line 74: early-headers-install:
Is there any way to "cache" *.h files easily? I do not want to create dep-files by hand.
File payloads/libpayload/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/59916/comment/b9d32db3_1f83ba75
PS2, Line 75: printf " INSTALL $(obj)/libpayload/include\n"
: install -m 755 -d "$(obj)/libpayload/include"
: for file in `find include -name *.h -type f`; do \
: install -m 755 -d "$(obj)/libpayload/`dirname $$file`"; \
: install -m 644 "$$file" "$(obj)/libpayload/$$file"; \
: done
: for file in `find $(coreboottop)/src/commonlib/bsd/include -name *.h -type f`; do \
: dest_file=$$(realpath --relative-to=$(coreboottop)/src/commonlib/bsd/ $$file); \
: install -m 755 -d "$(obj)/libpayload/`dirname $$dest_file`"; \
: install -m 644 "$$file" "$(obj)/libpayload/$$dest_file"; \
: done
I am not sure, if we really need this. Payloads should be using the _install_ target anyway, so copying these files in this target might be sufficient.
Moreover, I do not think, that it is a good idea to merge the build and install trees.
https://review.coreboot.org/c/coreboot/+/59916/comment/88145e5d_9bcb8fa0
PS2, Line 133: dest_file=$$(realpath --relative-to=$(coreboottop)/src/commonlib/bsd/ $$file); \
Required due to the commonlib/bsd includes being higher in the FS.
--
To view, visit https://review.coreboot.org/c/coreboot/+/59916
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Idc7175240f3077ec98280331f9a952310aae4341
Gerrit-Change-Number: 59916
Gerrit-PatchSet: 2
Gerrit-Owner: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org>
Gerrit-Comment-Date: Mon, 06 Dec 2021 14:03:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Patrick Georgi.
Hello Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/59916
to look at the new patch set (#2).
Change subject: libpayload: Provide includes for payloads
......................................................................
libpayload: Provide includes for payloads
Provide one, consistent include directory for payloads to use. It
includes external header files used in libpayload.
Change-Id: Idc7175240f3077ec98280331f9a952310aae4341
Signed-off-by: Jakub Czapiga <jacz(a)semihalf.com>
---
M payloads/libpayload/Makefile.inc
1 file changed, 19 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/59916/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/59916
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Idc7175240f3077ec98280331f9a952310aae4341
Gerrit-Change-Number: 59916
Gerrit-PatchSet: 2
Gerrit-Owner: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org>
Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Patrick Georgi, Julius Werner.
Jakub Czapiga has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59494 )
Change subject: libpayload/libc/fmap: Implement new FlashMap API
......................................................................
Patch Set 4:
(1 comment)
File payloads/libpayload/include/fmap.h:
https://review.coreboot.org/c/coreboot/+/59494/comment/0f8286e4_61a8eea6
PS4, Line 6: #include <commonlib/bsd/cb_err.h>
> Before using commonlib in exported headers, you'll also need to ensure that libpayload-using payload […]
Done in CB:59916. Let's continnue discussion there. I will leave this comment open until CB:59916 is merged or solved in another way.
--
To view, visit https://review.coreboot.org/c/coreboot/+/59494
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Idbf9016ce73aa58e17f3ee19920ab83dc6c25abb
Gerrit-Change-Number: 59494
Gerrit-PatchSet: 4
Gerrit-Owner: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-CC: Patrick Georgi <patrick(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Comment-Date: Mon, 06 Dec 2021 13:56:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Patrick Georgi <patrick(a)coreboot.org>
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Patrick Rudolph.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59844 )
Change subject: soc/intel/common/systemagent: Remove weak functions
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/59844/comment/edd5602d_e81aadc0
PS1, Line 9: an useful
a useful
--
To view, visit https://review.coreboot.org/c/coreboot/+/59844
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I773311ef6f8d98441ce461343066f8b25c741fbc
Gerrit-Change-Number: 59844
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Mon, 06 Dec 2021 13:13:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment