Attention is currently required from: Patrick Rudolph, Christian Walter, ping1.yang(a)intel.com, Shuo Liu, Nill Ge, Arthur Heymans, Srinidhi N Kaushik, TangYiwei, Tim Chu.
Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/73390 )
Change subject: soc/intel/xeon_sp: Fix PCH IOAPIC ID
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
Patchset:
PS1:
Interesting, so this could be a bug in SPR-SP FSP?
+Ping/Shuo to check the FSP for next generation.
--
To view, visit https://review.coreboot.org/c/coreboot/+/73390
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ia91cb4aef9d15520b8b3402ec10e7b0a4355caeb
Gerrit-Change-Number: 73390
Gerrit-PatchSet: 1
Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Reviewer: Nill Ge <geshijian(a)bytedance.com>
Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com>
Gerrit-Reviewer: TangYiwei
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: ping1.yang(a)intel.com
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: ping1.yang(a)intel.com
Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Attention: Nill Ge <geshijian(a)bytedance.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com>
Gerrit-Attention: TangYiwei
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Comment-Date: Thu, 02 Mar 2023 16:12:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Jason Glenesk, Raul Rangel, Matt DeVillier, Angel Pons, Martin Roth, Fred Reitberger, Karthik Ramasubramanian, Felix Held.
Robert Zieba has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/67936 )
Change subject: soc/amd/common/xhci: Add support for logging XHCI wake events
......................................................................
Patch Set 21:
(2 comments)
File src/soc/amd/common/block/acpi/elog.c:
https://review.coreboot.org/c/coreboot/+/67936/comment/eb639db8_2d9e8cec
PS19, Line 29:
> Is it being called outside of SMM. […]
Done
https://review.coreboot.org/c/coreboot/+/67936/comment/b9bfa0f7_29ec7e35
PS19, Line 34: if (ENV_SMM && CONFIG(SOC_AMD_COMMON_BLOCK_XHCI_ELOG) && i == XHCI_GEVENT)
> Should this be part of the `if (valid_gpe & (1U << i))` block? Otherwise xhci events might be logged […]
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/67936
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic0489e1df55c4e63cb8a306099e3f31c82eebd58
Gerrit-Change-Number: 67936
Gerrit-PatchSet: 21
Gerrit-Owner: Robert Zieba <robertzieba(a)google.com>
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: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.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-CC: Angel Pons <th3fanbus(a)gmail.com>
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: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Thu, 02 Mar 2023 16:10:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Raul Rangel, Martin Roth, Karthik Ramasubramanian.
Robert Zieba has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69914 )
Change subject: device/xhci: Add functions to work with resource pointers
......................................................................
Patch Set 13:
(3 comments)
File src/device/xhci_resource.c:
https://review.coreboot.org/c/coreboot/+/69914/comment/04689673_9c8229d3
PS11, Line 17:
> Anything that should be accessed cautiously in SMM - eg. input parameters. […]
When these functions are used in SMM for XHCI wake-event logging they take in the resources that were stored in SMRAM so there shouldn't be any issues.
https://review.coreboot.org/c/coreboot/+/69914/comment/8d60a9ca_10f56632
PS11, Line 18: enum cb_err xhci_resource_for_each_ext_cap(const struct resource *res, void *context,
> > that open brace { should be on the previous line […]
This seems to be a false positive. Since this is a function definition the brace should be on the next line. Uploading a CL with the brace on the same line results in a warning that it should be on the next line.
https://review.coreboot.org/c/coreboot/+/69914/comment/f9eaf3cf_8110a603
PS11, Line 95: enum cb_err xhci_resource_for_each_supported_usb_cap(
> > that open brace { should be on the previous line […]
Same as other warning.
--
To view, visit https://review.coreboot.org/c/coreboot/+/69914
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If5a74f9529d5dc6031ec968ef5f40a9cad5ffbc4
Gerrit-Change-Number: 69914
Gerrit-PatchSet: 13
Gerrit-Owner: Robert Zieba <robertzieba(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Martin Roth <martin.roth(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: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Thu, 02 Mar 2023 16:09:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Patrick Rudolph, Johnny Lin, Christian Walter, Arthur Heymans, Srinidhi N Kaushik, Tim Chu.
Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/73391 )
Change subject: soc/intel/xeon_sp: Report platform cpu info
......................................................................
Patch Set 2: Code-Review+1
(1 comment)
Patchset:
PS2:
Hi Johnny, does the stepping info look right to you? Do we need to distinguish between MCC and XCC?
+Srinidhi who has similar idea
--
To view, visit https://review.coreboot.org/c/coreboot/+/73391
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9c08fb924aad81608f554523432ab6a549b1b75f
Gerrit-Change-Number: 73391
Gerrit-PatchSet: 2
Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Comment-Date: Thu, 02 Mar 2023 16:09:11 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Raul Rangel, Nico Huber, Angel Pons, Martin Roth, Karthik Ramasubramanian, Felix Held.
Robert Zieba has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/67931 )
Change subject: cpu/x86/smm: Add PCI resource store functionality
......................................................................
Patch Set 20:
(2 comments)
File src/cpu/x86/smm/pci_resource_store.c:
https://review.coreboot.org/c/coreboot/+/67931/comment/25bb2510_d0943fab
PS17, Line 35: 8
> Is there a macro for this Bit Shift magic. Same comment for 0xff mask in the next line.
Done
https://review.coreboot.org/c/coreboot/+/67931/comment/5fba437d_855df5cb
PS17, Line 54: slots[i_slot].resources[i_res - 1].next = (struct resource *)&slots[i_slot].resources[i_res];
> Just curious to know if the last resource is not dangling.
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/67931
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I23fb1e935dd1b89f1cc5c834cc2025f0fe5fda37
Gerrit-Change-Number: 67931
Gerrit-PatchSet: 20
Gerrit-Owner: Robert Zieba <robertzieba(a)google.com>
Gerrit-Reviewer: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
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: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Thu, 02 Mar 2023 16:05:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Patrick Rudolph.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/73392 )
Change subject: [WIP]mb/ibm: Add IBM SBP1
......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/73392/comment/68d0a73d_ab0f4404
PS2, Line 27: The board boots to Linux with all 384 cores available.
: All PCI devices are working and no errors in ACPI.
> Congratulations. […]
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/73392
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie21c744224e8d9e5232d63b8366d2981c9575d70
Gerrit-Change-Number: 73392
Gerrit-PatchSet: 2
Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Comment-Date: Thu, 02 Mar 2023 15:43:30 +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: Patrick Rudolph.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/73392 )
Change subject: [WIP]mb/ibm: Add IBM SBP1
......................................................................
Patch Set 2:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/73392/comment/e8078c91_2924b655
PS2, Line 17:
> 'managment' may be misspelled - perhaps 'management'?
Please fix.
https://review.coreboot.org/c/coreboot/+/73392/comment/4bdf7957_c769c204
PS2, Line 27: The board boots to Linux with all 384 cores available.
: All PCI devices are working and no errors in ACPI.
Congratulations. Nice work!
--
To view, visit https://review.coreboot.org/c/coreboot/+/73392
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie21c744224e8d9e5232d63b8366d2981c9575d70
Gerrit-Change-Number: 73392
Gerrit-PatchSet: 2
Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Comment-Date: Thu, 02 Mar 2023 15:43:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Patrick Rudolph, Jonathan Zhang, Johnny Lin, Christian Walter, Arthur Heymans, Tim Chu.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/73391 )
Change subject: soc/intel/xeon_sp: Report platform cpu info
......................................................................
Patch Set 2:
(4 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/73391/comment/5d42eec8_63e5722a
PS2, Line 9: know
known
https://review.coreboot.org/c/coreboot/+/73391/comment/5c5db510_cad17dd3
PS2, Line 11:
Maybe add example output?
File src/include/cpu/intel/cpu_ids.h:
https://review.coreboot.org/c/coreboot/+/73391/comment/404378b6_59ea5795
PS2, Line 50: 0x806F1
: #define CPUID_SPR_S2 0x806F2
: #define CPUID_SPR_S3 0x806F3
: #define CPUID_SPR_S4 0x806F4
: #define CPUID_SPR_S6 0x806F6
Please use lowercase letters.
File src/soc/intel/xeon_sp/report_platform.c:
https://review.coreboot.org/c/coreboot/+/73391/comment/301af7ef_a6e2136c
PS2, Line 24: i
No need to fix the length?
https://notabs.org/coding/smallIntsBigPenalty.htm
--
To view, visit https://review.coreboot.org/c/coreboot/+/73391
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9c08fb924aad81608f554523432ab6a549b1b75f
Gerrit-Change-Number: 73391
Gerrit-PatchSet: 2
Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Comment-Date: Thu, 02 Mar 2023 15:39:47 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Tarun Tuli has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/73055 )
Change subject: mb/google/brya: Add new baseboard hades with variants hades
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS3:
> Hi Eric. Just noticed something, but shouldn't we rename the hades board variant to hades0 (e.g. […]
discussed offline. not relevant.
--
To view, visit https://review.coreboot.org/c/coreboot/+/73055
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib7fbdf997df8225cc7814a34f8b4e4e04884dbf9
Gerrit-Change-Number: 73055
Gerrit-PatchSet: 3
Gerrit-Owner: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Ivy Jian <ivy.jian(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-Comment-Date: Thu, 02 Mar 2023 15:02:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tarun Tuli <taruntuli(a)google.com>
Gerrit-MessageType: comment