Attention is currently required from: Sean Rhodes.
Subrata Banik has posted comments on this change by Sean Rhodes. ( https://review.coreboot.org/c/coreboot/+/84133?usp=email )
Change subject: drivers/usb/acpi: Add DSM for Intel Bluetooth
......................................................................
Patch Set 3:
(4 comments)
File src/drivers/usb/acpi/chip.h:
https://review.coreboot.org/c/coreboot/+/84133/comment/edd28b1d_e883830c?us… :
PS3, Line 51: bool intel_bt;
can you please use a meaningful name ? I don't understand what you mean by `intel_bt` ? is it like `is_bt_vendor_intel`?
File src/drivers/usb/acpi/usb_acpi.c:
https://review.coreboot.org/c/coreboot/+/84133/comment/62a3288e_7c155c2f?us… :
PS3, Line 12:
just one space to start the comment
https://review.coreboot.org/c/coreboot/+/84133/comment/852daf8f_83a617cc?us… :
PS3, Line 27: 0x03
can you please explain the meaning of hard coded value ?
https://review.coreboot.org/c/coreboot/+/84133/comment/cedc4416_5fdf9930?us… :
PS3, Line 36: /* aa10f4e0-81ac-4233-abf6-3b2ac50e28d9 */
unable to follow why you have used the GUID here ?
--
To view, visit https://review.coreboot.org/c/coreboot/+/84133?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Icc18f867604876b27ced2ee4356e47b3aa6b4f74
Gerrit-Change-Number: 84133
Gerrit-PatchSet: 3
Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Comment-Date: Fri, 30 Aug 2024 16:15:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Arthur Heymans, Krystian Hebel, Patrick Rudolph, Sergii Dmytruk.
Martin L Roth has posted comments on this change by Sergii Dmytruk. ( https://review.coreboot.org/c/coreboot/+/83426?usp=email )
Change subject: Documentation/drivers/smmstorev2.md: describe capsule update API
......................................................................
Patch Set 5:
(1 comment)
File Documentation/drivers/smmstorev2.md:
https://review.coreboot.org/c/coreboot/+/83426/comment/fdd1d5aa_25b5b04d?us… :
PS5, Line 221: It is expected that the
: payload won't allow regular OS to boot if the handler is enabled without
: rebooting first.
Sorry about all the questions here:
Why would this be expected?
How is this information passed to the payload?
What payloads would support this?
How is it enforced?
--
To view, visit https://review.coreboot.org/c/coreboot/+/83426?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I94761d18be567e5302d1a836f09f0a7eecb4fb00
Gerrit-Change-Number: 83426
Gerrit-PatchSet: 5
Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: 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-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Comment-Date: Fri, 30 Aug 2024 16:12:38 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Arthur Heymans, Felix Held, Patrick Rudolph, Sergii Dmytruk.
Martin L Roth has posted comments on this change by Sergii Dmytruk. ( https://review.coreboot.org/c/coreboot/+/83425?usp=email )
Change subject: drivers/smmstore: add logic to disable capsule update handling code
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83425?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I3dc175ea313aae1edae304520595b82db7206cbb
Gerrit-Change-Number: 83425
Gerrit-PatchSet: 6
Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
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-CC: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Fri, 30 Aug 2024 16:06:57 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Arthur Heymans, Felix Held, Krystian Hebel, Patrick Rudolph, Sergii Dmytruk.
Martin L Roth has posted comments on this change by Sergii Dmytruk. ( https://review.coreboot.org/c/coreboot/+/83424?usp=email )
Change subject: drivers/smmstore: add ability to write to whole flash
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83424?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I7f3dbfa965b9dcbade8b2f06a5bd2ac1345c7972
Gerrit-Change-Number: 83424
Gerrit-PatchSet: 5
Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com>
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: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Fri, 30 Aug 2024 16:03:30 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Arthur Heymans, Felix Held, Krystian Hebel, Patrick Rudolph, Sergii Dmytruk.
Martin L Roth has posted comments on this change by Sergii Dmytruk. ( https://review.coreboot.org/c/coreboot/+/83424?usp=email )
Change subject: drivers/smmstore: add ability to write to whole flash
......................................................................
Patch Set 5:
(1 comment)
File src/drivers/smmstore/store.c:
https://review.coreboot.org/c/coreboot/+/83424/comment/2e095796_02199a1a?us… :
PS5, Line 49: if (*cmd & SMMSTORE_CMD_USE_FULL_FLASH) {
> Can we add a check to make sure that we're actually doing a capsule update before enabling this inst […]
Oh, I guess I should have looked at the next commit in the patch train.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83424?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I7f3dbfa965b9dcbade8b2f06a5bd2ac1345c7972
Gerrit-Change-Number: 83424
Gerrit-PatchSet: 5
Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: 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-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Fri, 30 Aug 2024 16:03:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Martin L Roth <gaumless(a)gmail.com>
Attention is currently required from: Anil Kumar K, Bora Guvendik, Cliff Huang, Hannah Williams, Jamie Ryu, Kapil Porwal, Paul Menzel, Pranava Y N, Ravishankar Sarawadi, Saurabh Mishra, Wonkyu Kim.
Subrata Banik has posted comments on this change by Ravishankar Sarawadi. ( https://review.coreboot.org/c/coreboot/+/83772?usp=email )
Change subject: soc/intel/ptl: Add SoC ACPI directory for Panther Lake
......................................................................
Patch Set 75:
(5 comments)
File src/soc/intel/pantherlake/acpi/tcss_pcierp.asl:
https://review.coreboot.org/c/coreboot/+/83772/comment/e53ba525_344c2817?us… :
PS23, Line 56: 0xBAC
> Subrata, should we have soc/tcss.h provides the defines needed for the hard-coded offsets in this file and the other two tcss asl files?
if u feel these macros have scope just beyond ASL file then add into some common TCSS.h file or local to this file also works
File src/soc/intel/pantherlake/acpi/tcss_pcierp.asl:
https://review.coreboot.org/c/coreboot/+/83772/comment/7888816c_45df4bf4?us… :
PS75, Line 27:
single tab ?
https://review.coreboot.org/c/coreboot/+/83772/comment/8126ea68_43bb0356?us… :
PS75, Line 28: ,
same
https://review.coreboot.org/c/coreboot/+/83772/comment/d6b57646_bb61c27b?us… :
PS75, Line 29:
same
https://review.coreboot.org/c/coreboot/+/83772/comment/c164ee41_0a2ffa24?us… :
PS75, Line 32: Offset (0x404),
: LSOE, 1,
: LNSE, 1,
same
--
To view, visit https://review.coreboot.org/c/coreboot/+/83772?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ia5cf899b049cb8eb27b4ea30c7f3ce7a14884f15
Gerrit-Change-Number: 83772
Gerrit-PatchSet: 75
Gerrit-Owner: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com>
Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.com>
Gerrit-Reviewer: Saurabh Mishra <mishra.saurabh(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Appukuttan V K <appukuttan.vk(a)intel.com>
Gerrit-CC: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com>
Gerrit-CC: Balaji Manigandan <balaji.manigandan(a)intel.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-CC: Sanju Jose Thottan <sanjujose.thottan(a)intel.com>
Gerrit-CC: Saurabh Mishra <mishra.saurabh(a)intel.corp-partner.google.com>
Gerrit-CC: Vikrant L Jadeja <vikrant.l.jadeja(a)intel.com>
Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Attention: Saurabh Mishra <mishra.saurabh(a)intel.com>
Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Attention: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Attention: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Pranava Y N <pranavayn(a)google.com>
Gerrit-Comment-Date: Fri, 30 Aug 2024 16:00:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Cliff Huang <cliff.huang(a)intel.com>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com>
Attention is currently required from: Arthur Heymans, Felix Held, Krystian Hebel, Patrick Rudolph, Sergii Dmytruk.
Martin L Roth has posted comments on this change by Sergii Dmytruk. ( https://review.coreboot.org/c/coreboot/+/83424?usp=email )
Change subject: drivers/smmstore: add ability to write to whole flash
......................................................................
Patch Set 5:
(2 comments)
File src/drivers/smmstore/store.c:
https://review.coreboot.org/c/coreboot/+/83424/comment/021441be_1bb24d3a?us… :
PS5, Line 44: use_full_flash
Nit: smstore_use_full_flash?
https://review.coreboot.org/c/coreboot/+/83424/comment/8876f273_1770b2ec?us… :
PS5, Line 49: if (*cmd & SMMSTORE_CMD_USE_FULL_FLASH) {
Can we add a check to make sure that we're actually doing a capsule update before enabling this instead of just that the capsule update code is built in?
Can we disable the bit again after the capsule update is completed?
This seems like it could be a security gap otherwise. I could be mistaken about this, but it just seems like we don't need to leave this enabled all the time either way.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83424?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I7f3dbfa965b9dcbade8b2f06a5bd2ac1345c7972
Gerrit-Change-Number: 83424
Gerrit-PatchSet: 5
Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: 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-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Fri, 30 Aug 2024 15:58:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Krystian Hebel, Sergii Dmytruk.
Martin L Roth has posted comments on this change by Sergii Dmytruk. ( https://review.coreboot.org/c/coreboot/+/83423?usp=email )
Change subject: configs/config.msi_ms7d25_ddr4: enable UEFI capsule updates
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83423?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I2b18cb614f5fc38f2a417f7595475fdedfb6d625
Gerrit-Change-Number: 83423
Gerrit-PatchSet: 5
Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Comment-Date: Fri, 30 Aug 2024 15:48:58 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes