Attention is currently required from: Nico Huber, Martin L Roth, Angel Pons, Arthur Heymans.
Maximilian Brune has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63639 )
Change subject: Add SBOM (Software Bill of Materials) Generation
......................................................................
Patch Set 14:
(1 comment)
Patchset:
PS12:
> Given that this change is WIP, I imagine that SBOM entries for other firmware ingredients will be im […]
Yes,
the goal is that the software owners of these various blobs, supply a SBOM file along with it. So we only have to include it in the build (with a given path).
for microcode I am currently extracting the information myself (which is not the intention/goal), but should be good enough for a proof of concept.
I am planning to add an option to the goswid tool. It will probably look like this:
goswid -o sbom-new.uswid -a my-custom-payload.json sbom-old.uswid
the '-a' would be for 'append'
Then you only have to use the cbfstool to add sbom-new.uswid into the CBFS.
--
To view, visit https://review.coreboot.org/c/coreboot/+/63639
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Icb7481d4903f95d200eddbfed7728fbec51819d0
Gerrit-Change-Number: 63639
Gerrit-PatchSet: 14
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Martin L Roth <martinroth(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Christian Walter <christian.walter(a)9elements.com>
Gerrit-CC: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Martin L Roth <martinroth(a)google.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Tue, 03 May 2022 09:03:44 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Mario Scheithauer.
Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63888 )
Change subject: soc/intel/ehl: Provide function to change PHY-to-MAC IRQ polarity
......................................................................
Patch Set 2:
(2 comments)
File src/soc/intel/elkhartlake/tsn_gbe.c:
https://review.coreboot.org/c/coreboot/+/63888/comment/63edd610_0e553fbe
PS2, Line 38: udelay(TSN_GMII_DELAY_US);
Do you need this udelay() here at all? Are there restrictions in how often you can read the busy bit? If not we could get rind of it completely, stopwatch will handle the rest.
https://review.coreboot.org/c/coreboot/+/63888/comment/cb4b20ba_ba71dc9b
PS2, Line 44: Timeout at %ld
Maybe:
"Timeout after %ld msec"
--
To view, visit https://review.coreboot.org/c/coreboot/+/63888
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ia314014c7cacbeb72629c773c8c0bb5f002a3f54
Gerrit-Change-Number: 63888
Gerrit-PatchSet: 2
Gerrit-Owner: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Comment-Date: Tue, 03 May 2022 08:59:13 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Mario Scheithauer, Werner Zeh.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63888 )
Change subject: soc/intel/ehl: Provide function to change PHY-to-MAC IRQ polarity
......................................................................
Patch Set 2:
(1 comment)
File src/soc/intel/elkhartlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/63888/comment/8f40231c_62f12dc1
PS2, Line 246: config EHL_TSN_PHY2MAC_IRQ_ACTIVE_HIGH
Instead of Kconfig could that be made a devicetree option?
--
To view, visit https://review.coreboot.org/c/coreboot/+/63888
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ia314014c7cacbeb72629c773c8c0bb5f002a3f54
Gerrit-Change-Number: 63888
Gerrit-PatchSet: 2
Gerrit-Owner: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Comment-Date: Tue, 03 May 2022 08:56:57 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Mario Scheithauer.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63863 )
Change subject: soc/intel/elkhartlake: Provide ability to update TSN GbE MAC addresses
......................................................................
Patch Set 3:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/63863/comment/c1ee940b_566e4d55
PS3, Line 13: following patch
next patch in the series
--
To view, visit https://review.coreboot.org/c/coreboot/+/63863
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I2303a64cfd09fa02734ca9452d26591af2a76221
Gerrit-Change-Number: 63863
Gerrit-PatchSet: 3
Gerrit-Owner: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Comment-Date: Tue, 03 May 2022 08:55:53 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Mario Scheithauer.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63861 )
Change subject: soc/intel/elkhartlake: Implement TSN GbE driver
......................................................................
Patch Set 3:
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/63861/comment/14a48615_5b8293ec
PS3, Line 7: TSN
Sorry, what is TSN?
File src/soc/intel/elkhartlake/tsn_gbe.c:
https://review.coreboot.org/c/coreboot/+/63861/comment/a1b8a241_de4f7c8f
PS3, Line 19: 0x4b32
Add first to sort?
https://review.coreboot.org/c/coreboot/+/63861/comment/e74b82d3_86b76beb
PS3, Line 24: .devices = gbe_tsn_device_ids,
Either align or do not align the =.
--
To view, visit https://review.coreboot.org/c/coreboot/+/63861
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7522914c56b74486bb088280d2686acf7027d1d3
Gerrit-Change-Number: 63861
Gerrit-PatchSet: 3
Gerrit-Owner: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Comment-Date: Tue, 03 May 2022 08:54:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Mario Scheithauer.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63862 )
Change subject: mb/siemens/mc_ehl2: Adjust PSE TSN settings in devicetree
......................................................................
Patch Set 3:
(4 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/63862/comment/7dca5204_b84cabaa
PS3, Line 8:
What is the current problem? nNot all controllers showing up?
https://review.coreboot.org/c/coreboot/+/63862/comment/ad897281_5dcb523d
PS3, Line 12: This patch enables the Serial Gigabit Media Independent Interface
Please add a blank line above to separate paragraphs.
https://review.coreboot.org/c/coreboot/+/63862/comment/a84b6601_c14ee9ad
PS3, Line 15: And furthermore, the PCH TSN Link speed is set to 1
: Gbps.
Please do that in a separate commit.
https://review.coreboot.org/c/coreboot/+/63862/comment/31924fa9_2ed0c560
PS3, Line 19: ifconfig
New tools are `ip`. ;-) For example: `ip l` and `ip a`.
--
To view, visit https://review.coreboot.org/c/coreboot/+/63862
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I74e660548b2c44d5dbdb6023d5a36cfdd7e96f43
Gerrit-Change-Number: 63862
Gerrit-PatchSet: 3
Gerrit-Owner: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Comment-Date: Tue, 03 May 2022 08:53:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Mario Scheithauer.
Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63863 )
Change subject: soc/intel/elkhartlake: Provide ability to update TSN GbE MAC addresses
......................................................................
Patch Set 3: Code-Review+1
(1 comment)
File src/soc/intel/elkhartlake/tsn_gbe.c:
https://review.coreboot.org/c/coreboot/+/63863/comment/ca903e58_59dd13b8
PS2, Line 25: *mac_p = (*mac_p & 0xFFFF0000) | (adr_to_set[5] << 8) | adr_to_set[4];
> Great idea. […]
Looks more readble IMHO now, thanks.
--
To view, visit https://review.coreboot.org/c/coreboot/+/63863
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I2303a64cfd09fa02734ca9452d26591af2a76221
Gerrit-Change-Number: 63863
Gerrit-PatchSet: 3
Gerrit-Owner: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Comment-Date: Tue, 03 May 2022 08:50:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Comment-In-Reply-To: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-MessageType: comment
Attention is currently required from: Mario Scheithauer.
Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63862 )
Change subject: mb/siemens/mc_ehl2: Adjust PSE TSN settings in devicetree
......................................................................
Patch Set 3:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/63862/comment/9e1ddebe_d994c0c9
PS3, Line 9: Two
: of them reside in PCH including Intel Programmable Services Engine (PSE)
: and are controlled by these.
Maybe:
Two of them are initialized by the Programmable Service Engine (PSE).
--
To view, visit https://review.coreboot.org/c/coreboot/+/63862
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I74e660548b2c44d5dbdb6023d5a36cfdd7e96f43
Gerrit-Change-Number: 63862
Gerrit-PatchSet: 3
Gerrit-Owner: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Comment-Date: Tue, 03 May 2022 08:46:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Martin L Roth, Maximilian Brune, Arthur Heymans.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63639 )
Change subject: Add SBOM (Software Bill of Materials) Generation
......................................................................
Patch Set 14:
(4 comments)
File util/goswid/cmd/main.go:
https://review.coreboot.org/c/coreboot/+/63639/comment/db51514d_dae4b73c
PS12, Line 54: //if (output_format != uswid) && (len(input_file_paths) > 1) {
: // ErrorOut("multiple input files are only supported in conjunction with the .uswid output file extension\n")
: //}
> since this corresponds to a different project/repo, I will make the changes there and pull them at a […]
Ack
https://review.coreboot.org/c/coreboot/+/63639/comment/84f5ab8b_58e6ca5e
PS12, Line 68: if if_len >= 5 && input_file_path[if_len-5:if_len] == ".json" {
: err = input_tag.FromJSON(input_file)
: } else if if_len >= 4 && input_file_path[if_len-4:if_len] == ".xml" {
: err = input_tag.FromXML(input_file)
: } else if if_len >= 5 && input_file_path[if_len-5:if_len] == ".cbor" {
: err = input_tag.FromCBOR(input_file)
: } else if if_len >= 6 && input_file_path[if_len-6:if_len] == ".uswid" {
: _, err = uswid_input_tag.FromUSWID(input_file)
: isUSWID = true
: } else {
: fmt.Printf("input file extension not recognized, assuming USWID: %s\n", input_file_path)
: _, err = uswid_input_tag.FromUSWID(input_file)
: isUSWID = true
: }
> I guess yours is a bit more explicit. […]
Ack. My approach avoids having the same information in three different places on each line:
if if_len >= 5 && input_file_path[if_len-5:if_len] == ".json" {
Here, the length of the string to compare appears twice in the form of `5` and is also implicit in the string literal.
File util/goswid/pkg/uswid/uswid.go:
https://review.coreboot.org/c/coreboot/+/63639/comment/c740532f_8d4845f3
PS12, Line 14: // can't be const...
> to be honest I was kind of confused with this one. […]
Huh, weird.
https://review.coreboot.org/c/coreboot/+/63639/comment/1774aa64_a03343d6
PS12, Line 32: 16
> in my opinion this is the most explicit and readable version of doing protocol/encoding stuff. An abstraction would only make it worse (at least in my opinion)
Hmmm, my main complaint is that having magic offsets makes it hard to know what's being accessed (one would need to check some spec/description of the binary data), that's why I'd try to define constants for the numbers (e.g. something like `offset_header_version := 16`).
By abstraction I was thinking of something that can convert the binary data into a structure or an object and viceversa. Does something like this still exist? https://www.jonathan-petitcolas.com/2014/09/25/parsing-binary-files-in-go.h…
In any case, this is not too critical, as the tool is a separate project.
--
To view, visit https://review.coreboot.org/c/coreboot/+/63639
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Icb7481d4903f95d200eddbfed7728fbec51819d0
Gerrit-Change-Number: 63639
Gerrit-PatchSet: 14
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Martin L Roth <martinroth(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Christian Walter <christian.walter(a)9elements.com>
Gerrit-CC: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Martin L Roth <martinroth(a)google.com>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Tue, 03 May 2022 08:44:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Maximilian Brune <maximilian.brune(a)9elements.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment