Attention is currently required from: Patrick Rudolph, Richard Hughes, Maximilian Brune, Nicholas Chin.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68995 )
Change subject: Documentation/sbom: Add SBOM Documentation
......................................................................
Patch Set 6:
(10 comments)
File Documentation/sbom/sbom.md:
https://review.coreboot.org/c/coreboot/+/68995/comment/b1be7670_abb02d22
PS6, Line 2: simply
nit: omit "simply"
https://review.coreboot.org/c/coreboot/+/68995/comment/0276c4ce_51c9cf8d
PS6, Line 4: much
"many", because "software parts" is countable
[one would use "much" for uncountable things, like "water" and "time"]
https://review.coreboot.org/c/coreboot/+/68995/comment/9229f343_efd7242a
PS6, Line 12: Systems
Why is "Systems" capitalized?
https://review.coreboot.org/c/coreboot/+/68995/comment/6c6dec5d_e55cd49e
PS6, Line 18: Image
Why is "Image" capitalized?
https://review.coreboot.org/c/coreboot/+/68995/comment/9377252f_0420010d
PS6, Line 92: CONFIG_SBOM_[software-name]\_PATH
You may want to wrap this in backticks ` so that Markdown doesn't try to do anything weird. It also highlights that it's a Kconfig option
https://review.coreboot.org/c/coreboot/+/68995/comment/bb6c74df_bbb43b54
PS6, Line 103: CONFIG_SBOM_[software-name]\_PATH=/path/to/me.bin
You may want to wrap this in backticks ` so that Markdown doesn't try to do anything weird. It also highlights that it's a Kconfig option
https://review.coreboot.org/c/coreboot/+/68995/comment/ee1eddfd_0ac9fab6
PS6, Line 116: The second solution should in general be preferred
Why not place the preferred option first?
https://review.coreboot.org/c/coreboot/+/68995/comment/2a7a1ef2_6d9a9132
PS6, Line 116: The second solution should in general be preferred
Why not place the preferred option first?
File Documentation/sbom/sbom_generation.plantuml:
PS6:
What does this file do?
File Documentation/sbom/sbom_generation.png:
PS3:
> > Images should be kept small. […]
@Max: Given that this is a diagram, can you please make it a SVG?
@Nicolas: The file size is reasonable, although converting it to JPEG should make it smaller. SVG should be even better: it's essentially XML, so it works nicely with Git should the diagram need to be updated.
--
To view, visit https://review.coreboot.org/c/coreboot/+/68995
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I39fbcba60a0fbdbed9f662119ed7692c0a0fd30e
Gerrit-Change-Number: 68995
Gerrit-PatchSet: 6
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: Richard Hughes <richard(a)hughsie.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Richard Hughes <richard(a)hughsie.com>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Tue, 01 Nov 2022 10:30:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Maximilian Brune <maximilian.brune(a)9elements.com>
Comment-In-Reply-To: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Shelley Chen, Taniya Das, Venkat Thogaru, Julius Werner, Sudheer Amrabadi.
Taniya Das has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68384 )
Change subject: soc/qualcomm/sc7280: Add socinfo_pro_part() function in coreboot
......................................................................
Patch Set 8:
(1 comment)
File src/soc/qualcomm/sc7280/socinfo.c:
https://review.coreboot.org/c/coreboot/+/68384/comment/5f62e26d_77cecb89
PS4, Line 12: static struct chipinfo chipinfolut[] = {
> This is to keep it more aligned to our internal code and easy code readability.
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/68384
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Id9f23696384a6c1a89000292eafebd8a16c273ca
Gerrit-Change-Number: 68384
Gerrit-PatchSet: 8
Gerrit-Owner: Sudheer Amrabadi <samrabad(a)codeaurora.org>
Gerrit-Reviewer: Shelley Chen <shchen(a)google.com>
Gerrit-Reviewer: Taniya Das <quic_tdas(a)quicinc.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-CC: Taniya Das <tdas(a)qualcomm.corp-partner.google.com>
Gerrit-CC: Venkat Thogaru <thogaru(a)qualcomm.corp-partner.google.com>
Gerrit-Attention: Shelley Chen <shchen(a)google.com>
Gerrit-Attention: Taniya Das <quic_tdas(a)quicinc.com>
Gerrit-Attention: Venkat Thogaru <thogaru(a)qualcomm.corp-partner.google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Sudheer Amrabadi <samrabad(a)codeaurora.org>
Gerrit-Comment-Date: Tue, 01 Nov 2022 10:03:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Taniya Das <tdas(a)qualcomm.corp-partner.google.com>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: YH Lin, Zhuohao Lee.
Raihow Shi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69094 )
Change subject: mb/google/brask/variants/moli: remove fan setting
......................................................................
Patch Set 2: Code-Review+1
This change is ready for review.
--
To view, visit https://review.coreboot.org/c/coreboot/+/69094
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie8851800d30ebf4d948d6eaadda2387c8afe52d8
Gerrit-Change-Number: 69094
Gerrit-PatchSet: 2
Gerrit-Owner: Raihow Shi <raihow_shi(a)wistron.corp-partner.google.com>
Gerrit-Reviewer: Raihow Shi <raihow_shi(a)wistron.corp-partner.google.com>
Gerrit-Reviewer: YH Lin <yueherngl(a)google.com>
Gerrit-Reviewer: Zhuohao Lee <zhuohao(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Ariel Fang <ariel_fang(a)wistron.corp-partner.google.com>
Gerrit-CC: Casper Chang <casper_chang(a)wistron.corp-partner.google.com>
Gerrit-CC: Malik Hsu <malik_hsu(a)wistron.corp-partner.google.com>
Gerrit-Attention: YH Lin <yueherngl(a)google.com>
Gerrit-Attention: Zhuohao Lee <zhuohao(a)google.com>
Gerrit-Comment-Date: Tue, 01 Nov 2022 09:58:47 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Shelley Chen, Taniya Das, Venkat Thogaru, Julius Werner, Sudheer Amrabadi.
Taniya Das has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68384 )
Change subject: soc/qualcomm/sc7280: Add socinfo_pro_part() function in coreboot
......................................................................
Patch Set 8:
(3 comments)
File src/soc/qualcomm/sc7280/include/soc/socinfo.h:
https://review.coreboot.org/c/coreboot/+/68384/comment/fd98a3a9_a2760464
PS6, Line 53: uint16_t jtagid;
> This could still be […]
Done
File src/soc/qualcomm/sc7280/socinfo.c:
https://review.coreboot.org/c/coreboot/+/68384/comment/afc861e3_86053dc5
PS4, Line 12: static struct chipinfo chipinfolut[] = {
> Can you clarify what future purpose? Where are we going to need these?
This is to keep it more aligned to our internal code and easy code readability.
File src/soc/qualcomm/sc7280/socinfo.c:
https://review.coreboot.org/c/coreboot/+/68384/comment/56f7bf62_2ad7aac5
PS6, Line 53: for (i = 0; i < ARRAY_SIZE(chipinfolut); i++)
> The lookup part for both of these could be factored out into a helper function.
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/68384
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Id9f23696384a6c1a89000292eafebd8a16c273ca
Gerrit-Change-Number: 68384
Gerrit-PatchSet: 8
Gerrit-Owner: Sudheer Amrabadi <samrabad(a)codeaurora.org>
Gerrit-Reviewer: Shelley Chen <shchen(a)google.com>
Gerrit-Reviewer: Taniya Das <quic_tdas(a)quicinc.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-CC: Taniya Das <tdas(a)qualcomm.corp-partner.google.com>
Gerrit-CC: Venkat Thogaru <thogaru(a)qualcomm.corp-partner.google.com>
Gerrit-Attention: Shelley Chen <shchen(a)google.com>
Gerrit-Attention: Taniya Das <quic_tdas(a)quicinc.com>
Gerrit-Attention: Venkat Thogaru <thogaru(a)qualcomm.corp-partner.google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Sudheer Amrabadi <samrabad(a)codeaurora.org>
Gerrit-Comment-Date: Tue, 01 Nov 2022 09:55:04 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Comment-In-Reply-To: Taniya Das <tdas(a)qualcomm.corp-partner.google.com>
Gerrit-MessageType: comment
Attention is currently required from: Jonathan Zhang.
Hello Jonathan Zhang,
I'd like you to do a code review.
Please visit
https://review.coreboot.org/c/coreboot/+/69093
to review the following change.
Change subject: inc/dev/pci_def.h: add definitions for RCEC EA Ext. Capbility
......................................................................
inc/dev/pci_def.h: add definitions for RCEC EA Ext. Capbility
Root Complex Event Collector Endpoint Association Extended
Capability is defined in section 7.9.10 of PCIe 5.0 spec.
Add its Extended Capability ID, association bitmap for RCiEPs
register, and RCEC associated bus numbers register.
Signed-off-by: Jonathan Zhang <jonzhang(a)fb.com>
Change-Id: I7bede8ed88304a2925e6e1e4128bcdd625ee0e53
---
M src/include/device/pci_def.h
1 file changed, 23 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/69093/1
diff --git a/src/include/device/pci_def.h b/src/include/device/pci_def.h
index 64c1ac2..54bccc8 100644
--- a/src/include/device/pci_def.h
+++ b/src/include/device/pci_def.h
@@ -474,6 +474,9 @@
#define PCI_EXP_SEC_LNK_CTL3 4 /* Link Control 3 */
#define PCI_EXP_SEC_LANE_ERR_STATUS 8 /* Lane Error Status */
+/* Root Complex Event Collector Endpoint Association Extended Capability */
+#define PCIE_EXT_CAP_RCECEA_ID 0x0007
+
/* Advanced Error Reporting */
#define PCI_ERR_UNCOR_STATUS 4 /* Uncorrectable Error Status */
#define PCI_ERR_UNC_TRAIN 0x00000001 /* Training */
@@ -546,6 +549,10 @@
#define PCI_REBAR_CTRL_SIZE_MASK 0xffff0000
#define PCI_REBAR_CTRL_SIZE_SHIFT 16
+/* Root Complex Event Collector Endpoint Association */
+#define PCI_RCECEA_BITMAP 4
+#define PCI_RCECEA_BUSNUM 8
+
/*
* The PCI interface treats multi-function devices as independent
* devices. The slot/function address of each device is encoded
--
To view, visit https://review.coreboot.org/c/coreboot/+/69093
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7bede8ed88304a2925e6e1e4128bcdd625ee0e53
Gerrit-Change-Number: 69093
Gerrit-PatchSet: 1
Gerrit-Owner: Shuming Chu (Shuming) <s1218944(a)gmail.com>
Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-CC: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-MessageType: newchange
Attention is currently required from: Jason Glenesk, Raul Rangel, Matt DeVillier, Fred Reitberger, Felix Held.
Elyes Haouas has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69033 )
Change subject: soc/amd: Include <cpu/cpu.h> instead of <arch/cpu.h>
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
NB: https://review.coreboot.org/c/coreboot/+/69060 is needed before current patch.
--
To view, visit https://review.coreboot.org/c/coreboot/+/69033
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iea29938623fe1b2bcdd7f869b0accbc1f8758e7a
Gerrit-Change-Number: 69033
Gerrit-PatchSet: 2
Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Tue, 01 Nov 2022 09:38:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment