coreboot-gerrit November 2023

coreboot-gerrit@coreboot.org

1 participants
3037 discussions

[M] Change in coreboot[main]: google/*: Clean up Kconfg board selection for Google MTK boards
by Yu-Ping Wu (Code Review) 20 Nov '23

20 Nov '23

Attention is currently required from: Felix Singer, Hung-Te Lin, Julius Werner, Yidi Lin. Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79063?usp=email ) Change subject: google/*: Clean up Kconfg board selection for Google MTK boards ...................................................................... Patch Set 1: (2 comments) File src/mainboard/google/corsola/Kconfig: https://review.coreboot.org/c/coreboot/+/79063/comment/085bd560_8e6847ac : PS1, Line 22: select DRIVER_ANALOGIX_ANX7625 if BOARD_GOOGLE_CHINCHOU > Hmm... I think I find my version a bit clearer to be honest, but it's your board so happy to switch this to your proposal if you prefer. I think the way I wrote it makes it pretty straight-forward to read: > - All the Kinglers have ANX, except for Steelix which also has Parade in addition. > - All the Krabbies have Parade, except Chinchou which has ANX instead. > - All the Staryus have MIPI. That looks clean because right now we only have one exception for Chinchou. As there's even no guarantee that "most Kinglers will have Parade" (that's entirely up to the OEM's choice), I think it's better to not view Chinchou as minority. That's also the reason why I decided to decouple Parade/ANX from Kingler/Krabby in the past. If there are more boards like Chinchou, then your version would look like: ``` config BOARD_GOOGLE_KRABBY_COMMON select DRIVER_ANALOGIX_ANX7625 if BOARD_A || BOARD_B || BOARD_C select DRIVER_PARADE_PS8640 if !(BOARD_A || BOARD_B || BOARD_C) # Or applying De Morgan's laws ``` which doesn't look clean to me. Therefore I think I still prefer ``` select DRIVER_ANALOGIX_ANX7625 if \ BOARD_GOOGLE_KINGLER_COMMON || BOARD_GOOGLE_CHINCHOU select DRIVER_PARADE_PS8640 if \ !(DRIVER_ANALOGIX_ANX7625 || BOARD_GOOGLE_STARYU_COMMON) || \ BOARD_GOOGLE_STEELIX ``` And if in the future things become more complicated (for example a Kingler with Parade), we can easily modify that to specify each board in the condition. Hung-Te/Yidi what's your opinion? File src/mainboard/google/kukui/Kconfig: https://review.coreboot.org/c/coreboot/+/79063/comment/6a988a2a_b444f2bb : PS1, Line 27: BOARD_GOOGLE_WILLOW > I did that by adding JACUZZI_COMMON to the def_bool list for KUKUI_COMMON above. […] Acknowledged -- To view, visit https://review.coreboot.org/c/coreboot/+/79063?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I40880e7609ba703d0053ad01da742871e54d4e7a Gerrit-Change-Number: 79063 Gerrit-PatchSet: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Comment-Date: Mon, 20 Nov 2023 10:15:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: Documentation/mb/asus/p8z77-m: Document latest test results
by Kevin Keijzer (Code Review) 20 Nov '23

20 Nov '23

Attention is currently required from: Angel Pons, Keith Hui, Paul Menzel. Kevin Keijzer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78204?usp=email ) Change subject: Documentation/mb/asus/p8z77-m: Document latest test results ...................................................................... Patch Set 2: (1 comment) File Documentation/mainboard/asus/p8z77-m.md: https://review.coreboot.org/c/coreboot/+/78204/comment/8e38d3ba_a5e30706 : PS2, Line 106: It appears all memory modules rated for DDR3-1600 will fail to boot if : max_mem_clock_mhz is set to 800 in devicetree. > @Kevin do you have a Sandy or Ivy Bridge CPU? It matters - native RAM init skips some steps on Sandy […] @Angel I only have Ivy Bridge cpu's, so that's all I've tested. -- To view, visit https://review.coreboot.org/c/coreboot/+/78204?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I4f4c9268cd272caa83267be3f71d4a2022c26a1c Gerrit-Change-Number: 78204 Gerrit-PatchSet: 2 Gerrit-Owner: Keith Hui <buurin(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Kevin Keijzer <kevin(a)quietlife.nl> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Keith Hui <buurin(a)gmail.com> Gerrit-Comment-Date: Mon, 20 Nov 2023 10:08:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Kevin Keijzer <kevin(a)quietlife.nl> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Keith Hui <buurin(a)gmail.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: Documentation/mb/asus/p8z77-m: Document latest test results
by Angel Pons (Code Review) 20 Nov '23

20 Nov '23

1 0

[S] Change in coreboot[main]: Documentation/mb/asus/p8z77-m: Document latest test results
by Angel Pons (Code Review) 20 Nov '23

20 Nov '23

Attention is currently required from: Keith Hui, Kevin Keijzer, Paul Menzel. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78204?usp=email ) Change subject: Documentation/mb/asus/p8z77-m: Document latest test results ...................................................................... Patch Set 2: (1 comment) File Documentation/mainboard/asus/p8z77-m.md: https://review.coreboot.org/c/coreboot/+/78204/comment/26047048_955ee8ed : PS2, Line 106: It appears all memory modules rated for DDR3-1600 will fail to boot if : max_mem_clock_mhz is set to 800 in devicetree. > A little off topic, do we know enough about Ivy Bridge that we can try automatically downclock the m […] @Kevin do you have a Sandy or Ivy Bridge CPU? It matters - native RAM init skips some steps on Sandy. @Keith I've been thinking about how to implement a "when raminit fails at high clock speed, try again but with a lower clock speed" mechanism. Because you can only set the memory controller frequency once, I would need to use the SSKPD (sticky scratchpad) register to store the high clock speed (which didn't work), do a warm reset (to unset the memory controller frequency) and try again with the lower clock speed. -- To view, visit https://review.coreboot.org/c/coreboot/+/78204?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I4f4c9268cd272caa83267be3f71d4a2022c26a1c Gerrit-Change-Number: 78204 Gerrit-PatchSet: 2 Gerrit-Owner: Keith Hui <buurin(a)gmail.com> Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Kevin Keijzer <kevin(a)quietlife.nl> Gerrit-Attention: Kevin Keijzer <kevin(a)quietlife.nl> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Keith Hui <buurin(a)gmail.com> Gerrit-Comment-Date: Mon, 20 Nov 2023 10:02:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Kevin Keijzer <kevin(a)quietlife.nl> Comment-In-Reply-To: Keith Hui <buurin(a)gmail.com> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: acpi.c: Fix generating pointer to cb_tables located >4G
by Arthur Heymans (Code Review) 20 Nov '23

20 Nov '23

Attention is currently required from: Angel Pons, Benjamin Doron, Eric Lai, Felix Held, Lance Zhao, Maximilian Brune, Tim Wawrzynczak. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76181?usp=email ) Change subject: acpi.c: Fix generating pointer to cb_tables located >4G ...................................................................... Patch Set 4: (1 comment) File src/acpi/acpi.c: https://review.coreboot.org/c/coreboot/+/76181/comment/af3a66af_a751d56b : PS4, Line 301: if (base < UINT32_MAX) > Actually, shouldn't we check `base + size <= UINT32_MAX` instead? Otherwise, there will be problems when cbtables cross the 4 GiB boundary. ACPI only specifies that base and length need to be 32bit value, not that the whole range needs to be in a 4G segment. -- To view, visit https://review.coreboot.org/c/coreboot/+/76181?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I1bc553b18d08cee502b765166227810f8e619631 Gerrit-Change-Number: 76181 Gerrit-PatchSet: 4 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Mon, 20 Nov 2023 09:56:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <inforichland(a)gmail.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: mb/asus/p8z77-m: Fix ACPI S3 suspend
by Angel Pons (Code Review) 20 Nov '23

20 Nov '23

Attention is currently required from: Keith Hui, Kevin Keijzer, Paul Menzel. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78205?usp=email ) Change subject: mb/asus/p8z77-m: Fix ACPI S3 suspend ...................................................................... Patch Set 2: Code-Review+2 (3 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/78205/comment/0c46d27c_efe977ec : PS2, Line 7: Fix ACPI S3 suspend : When 3VSBSW# is disabled, the RAM loses power in S3 suspend, so S3 resume fails. So, what this change does can also be summarised as follows: > mb/asus/p8z77-m: Ensure RAM stays powered in S3 suspend https://review.coreboot.org/c/coreboot/+/78205/comment/c994294c_7c68cfde : PS2, Line 10: S3 suspend totally fails nit: Strictly speaking, it is S3 *resume* that totally fails. The board happily enters S3 suspend, but cannot resume from S3 because the RAM lost power during S3 suspend. https://review.coreboot.org/c/coreboot/+/78205/comment/a9d8aa73_d5bb3932 : PS2, Line 9: Enable 3VSBSW# in NCT6779D super I/O like other variants in the : family. Without it S3 suspend totally fails. > Maybe also mention the long description (Switch 3VSB power to memory when in S3 state.). It would be nice to factor in the root cause: S3 resume fails because the RAM was not powered during S3 suspend. -- To view, visit https://review.coreboot.org/c/coreboot/+/78205?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ia8059b2a263ab5c459e54685f046eeb913776473 Gerrit-Change-Number: 78205 Gerrit-PatchSet: 2 Gerrit-Owner: Keith Hui <buurin(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Kevin Keijzer <kevin(a)quietlife.nl> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Kevin Keijzer <kevin(a)quietlife.nl> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Keith Hui <buurin(a)gmail.com> Gerrit-Comment-Date: Mon, 20 Nov 2023 09:55:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: acpi: Add IO Remapping Table structures
by Naresh Solanki (Code Review) 20 Nov '23

20 Nov '23

Attention is currently required from: Angel Pons, Arthur Heymans, Cliff Huang, Lance Zhao, Martin L Roth, Patrick Rudolph, Paul Menzel, Tim Wawrzynczak. Naresh Solanki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77884?usp=email ) Change subject: acpi: Add IO Remapping Table structures ...................................................................... Patch Set 6: (2 comments) Commit Message: PS6: > Which specification is this code based on? I found `DEN0049E_IO_Remapping_Table. […] As of now, I have used specific revision as mentioned in commit message. I haven't created structure with all latest revision table from the document. File src/include/acpi/acpi.h: https://review.coreboot.org/c/coreboot/+/77884/comment/9d1c6169_f1b607f8 : PS6, Line 92: IORT, /* Input Output Remapping Table */ > > I can't find the specification for this table in the ACPI specifications, only in ARM document `DE […] Acknowledged -- To view, visit https://review.coreboot.org/c/coreboot/+/77884?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I4e8e3323caa714a56882939914cac510bf95d30b Gerrit-Change-Number: 77884 Gerrit-PatchSet: 6 Gerrit-Owner: Naresh Solanki <naresh.solanki(a)9elements.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Mon, 20 Nov 2023 09:53:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: acpi: Add IO Remapping Table structures
by Arthur Heymans (Code Review) 20 Nov '23

20 Nov '23

Attention is currently required from: Angel Pons, Cliff Huang, Lance Zhao, Martin L Roth, Naresh Solanki, Patrick Rudolph, Paul Menzel, Tim Wawrzynczak. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77884?usp=email ) Change subject: acpi: Add IO Remapping Table structures ...................................................................... Patch Set 6: (1 comment) File src/include/acpi/acpi.h: https://review.coreboot.org/c/coreboot/+/77884/comment/4b3ee520_3f789ddb : PS6, Line 92: IORT, /* Input Output Remapping Table */ > I can't find the specification for this table in the ACPI specifications, only in ARM document `DEN0049E_IO_Remapping_Table.pdf`. I think it should be added to the second group ("Additional proprietary tables used by coreboot") instead. Quite a few tables are mentioned in the ACPI spec which are links to other documents. I don't think proprietary is right. Propriatary is stuff that has no links in ACPI spec, although SPMI should be moved in that case. -- To view, visit https://review.coreboot.org/c/coreboot/+/77884?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I4e8e3323caa714a56882939914cac510bf95d30b Gerrit-Change-Number: 77884 Gerrit-PatchSet: 6 Gerrit-Owner: Naresh Solanki <naresh.solanki(a)9elements.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Naresh Solanki <naresh.solanki(a)9elements.com> Gerrit-Comment-Date: Mon, 20 Nov 2023 09:44:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: acpi: Optimize enum acpi_tables layout
by Naresh Solanki (Code Review) 20 Nov '23

20 Nov '23

Attention is currently required from: Arthur Heymans, Martin L Roth, Patrick Rudolph. Naresh Solanki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79099?usp=email ) Change subject: acpi: Optimize enum acpi_tables layout ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/79099/comment/a58a9b2f_4f0b2ad4 : PS1, Line 7: Optimize > I feel that "optimize" isn't the most precise verb to describe this change, but I can't come up with […] Maybe rewrite as: 'acpi: Arrange ACPI table enum' ? -- To view, visit https://review.coreboot.org/c/coreboot/+/79099?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I192339df771d6a3ae67358fe46334fe2b216b974 Gerrit-Change-Number: 79099 Gerrit-PatchSet: 2 Gerrit-Owner: Naresh Solanki <naresh.solanki(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Mon, 20 Nov 2023 09:42:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: acpi.c: Fix generating pointer to cb_tables located >4G
by Angel Pons (Code Review) 20 Nov '23

20 Nov '23

Attention is currently required from: Arthur Heymans, Benjamin Doron, Eric Lai, Felix Held, Lance Zhao, Maximilian Brune, Tim Wawrzynczak. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76181?usp=email ) Change subject: acpi.c: Fix generating pointer to cb_tables located >4G ...................................................................... Patch Set 4: (1 comment) File src/acpi/acpi.c: https://review.coreboot.org/c/coreboot/+/76181/comment/ccff3adf_a990d0bc : PS4, Line 301: if (base < UINT32_MAX) > nit: `<=` ? Actually, shouldn't we check `base + size <= UINT32_MAX` instead? Otherwise, there will be problems when cbtables cross the 4 GiB boundary. -- To view, visit https://review.coreboot.org/c/coreboot/+/76181?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I1bc553b18d08cee502b765166227810f8e619631 Gerrit-Change-Number: 76181 Gerrit-PatchSet: 4 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Mon, 20 Nov 2023 09:40:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2023