coreboot-gerrit January 2023

coreboot-gerrit@coreboot.org

1 participants
5879 discussions

[S] Change in coreboot[master]: mb/google/brya/var/marasov: Remove wlan RTD3 driver for meet design c...
by Frank Chu (Code Review) 07 Jan '23

07 Jan '23

Attention is currently required from: Tarun Tuli, Kyle Lin. Frank Chu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/71693 ) Change subject: mb/google/brya/var/marasov: Remove wlan RTD3 driver for meet design change ...................................................................... Patch Set 4: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/71693 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I22448a8cb28ddadb93b114c096e364980feab6fd Gerrit-Change-Number: 71693 Gerrit-PatchSet: 4 Gerrit-Owner: Frank Chu <frank_chu(a)pegatron.corp-partner.google.com> Gerrit-Reviewer: Frank Chu <frank_chu(a)pegatron.corp-partner.google.com> Gerrit-Reviewer: Kyle Lin <kylelinck(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Ken Lu <ken_lu(a)pegatron.corp-partner.google.com> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Kyle Lin <kylelinck(a)google.com> Gerrit-Comment-Date: Sat, 07 Jan 2023 09:03:40 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in coreboot[master]: mb/google/brya/var/omnigul: Update GPIO settings
by Jamie Chen (Code Review) 07 Jan '23

07 Jan '23

Attention is currently required from: Tarun Tuli, Dtrain Hsu, Ian Feng. Jamie Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/71706 ) Change subject: mb/google/brya/var/omnigul: Update GPIO settings ...................................................................... Patch Set 10: (12 comments) File src/mainboard/google/brya/variants/omnigul/gpio.c: https://review.coreboot.org/c/coreboot/+/71706/comment/6aa601cf_eba2fa71 PS9, Line 92: PAD_CFG_GPI_APIC_LOCK(GPP_D5, NONE, LEVEL, NONE, LOCK_CONFIG), > Does it use for interrupt? If no, it might be PAD_CFG_GPI_LOCK. Done https://review.coreboot.org/c/coreboot/+/71706/comment/8618abca_7b0a91ed PS9, Line 96: PAD_CFG_GPI_APIC > Does it use for interrupt? If no, it might be PAD_CFG_GPI. Done https://review.coreboot.org/c/coreboot/+/71706/comment/8aa65d1e_2d4b65a4 PS9, Line 104: PAD_CFG_GPI_APIC > Same as GPP_D8. Done https://review.coreboot.org/c/coreboot/+/71706/comment/36dd5350_873494d8 PS9, Line 108: PAD_CFG_GPI_APIC > Same as GPP_D8. Done https://review.coreboot.org/c/coreboot/+/71706/comment/f57be8b8_9043d3d0 PS9, Line 110: PAD_CFG_GPI_APIC > Same as GPP_D8. Done https://review.coreboot.org/c/coreboot/+/71706/comment/0b593022_7a8bc41a PS9, Line 112: PAD_CFG_GPI_APIC > Same as GPP_D8. Done https://review.coreboot.org/c/coreboot/+/71706/comment/949e1459_b4af5181 PS9, Line 122: PAD_CFG_GPI_APIC > Same as GPP_D8. Done https://review.coreboot.org/c/coreboot/+/71706/comment/2e772b42_c2551ea4 PS9, Line 128: PAD_CFG_GPI_APIC > Same as GPP_D8. Done https://review.coreboot.org/c/coreboot/+/71706/comment/4049a50b_197e61de PS9, Line 132: PAD_CFG_GPI_APIC_LOCK(GPP_E10, NONE, LEVEL, NONE, LOCK_CONFIG), > Same as GPP_D5. Done https://review.coreboot.org/c/coreboot/+/71706/comment/1165b37b_02125c20 PS9, Line 141: PAD_CFG_GPI_APIC_LOCK(GPP_E17, NONE, LEVEL, NONE, LOCK_CONFIG), > Same as GPP_D5. Done https://review.coreboot.org/c/coreboot/+/71706/comment/da6149e7_57e5b4a5 PS9, Line 145: PAD_CFG_GPI_APIC > Same as GPP_D8. Done https://review.coreboot.org/c/coreboot/+/71706/comment/7003598b_a0ff564e PS9, Line 180: PAD_CFG_GPI_APIC_LOCK > Same as GPP_D5. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/71706 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5cfaa8fce6df7f09b744fb3e0b7b1d5b6acdc79b Gerrit-Change-Number: 71706 Gerrit-PatchSet: 10 Gerrit-Owner: Jamie Chen <jamie_chen(a)compal.corp-partner.google.com> Gerrit-Reviewer: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> Gerrit-Reviewer: Ian Feng <ian_feng(a)compal.corp-partner.google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> Gerrit-Attention: Ian Feng <ian_feng(a)compal.corp-partner.google.com> Gerrit-Comment-Date: Sat, 07 Jan 2023 09:00:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: drivers/spi/tpm: Wait for STS_COMMAND_READY
by Yu-Ping Wu (Code Review) 07 Jan '23

07 Jan '23

Attention is currently required from: Christian Walter, Andrey Pronin. Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/71724 ) Change subject: drivers/spi/tpm: Wait for STS_COMMAND_READY ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/71724/comment/df690105_daabb473 PS1, Line 8: Andrey, I need your opinion about the following questions: 1. Should we apply same the logic for Cr50? How about non-Google TPMs? 2. Do we need to keep calling `write_tpm_sts`? Or just need to write once and waiting for `read_tpm_sts` to return the expected status? 3. What's the suggested timeout (currently 1ms)? 4. What's the suggested delay in the loop (currently 10us)? -- To view, visit https://review.coreboot.org/c/coreboot/+/71724 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I06fd8f400622f6b9afea520cec19ea962ebe8568 Gerrit-Change-Number: 71724 Gerrit-PatchSet: 1 Gerrit-Owner: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: Andrey Pronin <apronin(a)chromium.org> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Andrey Pronin <apronin(a)chromium.org> Gerrit-Comment-Date: Sat, 07 Jan 2023 08:42:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: drivers/spi/tpm: Wait for STS_COMMAND_READY
by Yu-Ping Wu (Code Review) 07 Jan '23

07 Jan '23

Yu-Ping Wu has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/71724 ) Change subject: drivers/spi/tpm: Wait for STS_COMMAND_READY ...................................................................... drivers/spi/tpm: Wait for STS_COMMAND_READY If we reset Ti50 in the middle of a command, it will not become ready for the 1st command after reboot until the TPM app returns the buffer from the previously ongoing command. Therefore, add a loop to retry writing TPM_STS_COMMAND_READY to STS register. BUG=b:263531882 TEST=emerge-corsola coreboot BRANCH=none Change-Id: I06fd8f400622f6b9afea520cec19ea962ebe8568 Signed-off-by: Yu-Ping Wu <yupingso(a)chromium.org> --- M src/drivers/spi/tpm/tpm.c 1 file changed, 42 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/71724/1 diff --git a/src/drivers/spi/tpm/tpm.c b/src/drivers/spi/tpm/tpm.c index 976a8d8..050edd3 100644 --- a/src/drivers/spi/tpm/tpm.c +++ b/src/drivers/spi/tpm/tpm.c @@ -604,6 +604,7 @@ uint8_t *rsp_body = tpm2_response; union fifo_transfer_buffer fifo_buffer; const int HEADER_SIZE = 6; + struct stopwatch sw; /* Do not try using an uninitialized TPM. */ if (!tpm_info.vendor_id) @@ -622,11 +623,28 @@ return 0; } - /* Let the TPM know that the command is coming. */ - if (write_tpm_sts(TPM_STS_COMMAND_READY) != CB_SUCCESS) { - printk(BIOS_ERR, "TPM_STS_COMMAND_READY failed\n"); - return 0; - } + /* + * Let the TPM know that the command is coming. + * + * If we reset Ti50 in the middle of a command, it will not become ready for the 1st + * command after reboot until the TPM app returns the buffer from the previously ongoing + * command. + */ + expected_status_bits = TPM_STS_VALID | TPM_STS_COMMAND_READY; + stopwatch_init_usecs_expire(&sw, 1000); + status = 0; + do { + enum cb_err ret = write_tpm_sts(TPM_STS_COMMAND_READY); + udelay(10); + if (ret == CB_SUCCESS) + read_tpm_sts(&status); + else + printk(BIOS_WARNING, "failed to write TPM_STS_COMMAND_READY\n"); + if (stopwatch_expired(&sw)) { + printk(BIOS_ERR, "timeout waiting for TPM_STS_COMMAND_READY\n"); + return 0; + } + } while (!(status & expected_status_bits)); /* * TPM commands and responses written to and read from the FIFO -- To view, visit https://review.coreboot.org/c/coreboot/+/71724 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I06fd8f400622f6b9afea520cec19ea962ebe8568 Gerrit-Change-Number: 71724 Gerrit-PatchSet: 1 Gerrit-Owner: Yu-Ping Wu <yupingso(a)google.com> Gerrit-MessageType: newchange

1 0

[L] Change in coreboot[master]: mb/google/brya/var/omnigul: Update GPIO settings
by Dtrain Hsu (Code Review) 07 Jan '23

07 Jan '23

Attention is currently required from: Jamie Chen, Tarun Tuli, Ian Feng. Dtrain Hsu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/71706 ) Change subject: mb/google/brya/var/omnigul: Update GPIO settings ...................................................................... Patch Set 9: Code-Review+1 (12 comments) File src/mainboard/google/brya/variants/omnigul/gpio.c: https://review.coreboot.org/c/coreboot/+/71706/comment/3e1dbf5c_84d05b95 PS9, Line 92: PAD_CFG_GPI_APIC_LOCK(GPP_D5, NONE, LEVEL, NONE, LOCK_CONFIG), Does it use for interrupt? If no, it might be PAD_CFG_GPI_LOCK. https://review.coreboot.org/c/coreboot/+/71706/comment/a53e1d16_6202d3c1 PS9, Line 96: PAD_CFG_GPI_APIC Does it use for interrupt? If no, it might be PAD_CFG_GPI. https://review.coreboot.org/c/coreboot/+/71706/comment/a654caa4_d027cc0f PS9, Line 104: PAD_CFG_GPI_APIC Same as GPP_D8. https://review.coreboot.org/c/coreboot/+/71706/comment/108aa692_4d323620 PS9, Line 108: PAD_CFG_GPI_APIC Same as GPP_D8. https://review.coreboot.org/c/coreboot/+/71706/comment/2b2634d2_39a00721 PS9, Line 110: PAD_CFG_GPI_APIC Same as GPP_D8. https://review.coreboot.org/c/coreboot/+/71706/comment/011f07c2_bfca600b PS9, Line 112: PAD_CFG_GPI_APIC Same as GPP_D8. https://review.coreboot.org/c/coreboot/+/71706/comment/74074aca_6b5b8240 PS9, Line 122: PAD_CFG_GPI_APIC Same as GPP_D8. https://review.coreboot.org/c/coreboot/+/71706/comment/93283b1d_6ddcd7a9 PS9, Line 128: PAD_CFG_GPI_APIC Same as GPP_D8. https://review.coreboot.org/c/coreboot/+/71706/comment/e0bf2828_c60e2c30 PS9, Line 132: PAD_CFG_GPI_APIC_LOCK(GPP_E10, NONE, LEVEL, NONE, LOCK_CONFIG), Same as GPP_D5. https://review.coreboot.org/c/coreboot/+/71706/comment/b75c59c7_a96c294c PS9, Line 141: PAD_CFG_GPI_APIC_LOCK(GPP_E17, NONE, LEVEL, NONE, LOCK_CONFIG), Same as GPP_D5. https://review.coreboot.org/c/coreboot/+/71706/comment/6f6e4b32_430bddf5 PS9, Line 145: PAD_CFG_GPI_APIC Same as GPP_D8. https://review.coreboot.org/c/coreboot/+/71706/comment/1e98c83a_d75a4eb9 PS9, Line 180: PAD_CFG_GPI_APIC_LOCK Same as GPP_D5. -- To view, visit https://review.coreboot.org/c/coreboot/+/71706 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5cfaa8fce6df7f09b744fb3e0b7b1d5b6acdc79b Gerrit-Change-Number: 71706 Gerrit-PatchSet: 9 Gerrit-Owner: Jamie Chen <jamie_chen(a)compal.corp-partner.google.com> Gerrit-Reviewer: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> Gerrit-Reviewer: Ian Feng <ian_feng(a)compal.corp-partner.google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jamie Chen <jamie_chen(a)compal.corp-partner.google.com> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Ian Feng <ian_feng(a)compal.corp-partner.google.com> Gerrit-Comment-Date: Sat, 07 Jan 2023 08:27:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: mb/intel/adlrvp: Enable ISH and UFS for reworked ADL-P RVP
by Cliff Huang (Code Review) 07 Jan '23

07 Jan '23

Cliff Huang has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/71740 ) Change subject: mb/intel/adlrvp: Enable ISH and UFS for reworked ADL-P RVP ...................................................................... mb/intel/adlrvp: Enable ISH and UFS for reworked ADL-P RVP UFS support is added to ADL-P RVP. Rework is required for UFS to work. Secondary CPU and PCH SSD slots will be disabled after rework. ISH is also enabled as part of UFS enablement. ISH UART0 is enabled. The SW5C1 on MICA3 needs to be set properly to route ISH UART signals instead of CR50. To enable PCH SSD slot, comment out the following in the overrides.cb: device ref pcie_rp9 off end device ref pcie_rp11 off end device ref ufs on end BUG=b:252785850 TEST=Check that UFS device is detected from lspci command. Signed-off-by: Cliff Huang <cliff.huang(a)intel.com> Change-Id: I812786b9a25c31bc008f055af90467d4edf6158e --- M src/mainboard/intel/adlrvp/Kconfig M src/mainboard/intel/adlrvp/gpio.c M src/mainboard/intel/adlrvp/variants/adlrvp_rpl_ext_ec/overridetree.cb 3 files changed, 49 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/71740/1 diff --git a/src/mainboard/intel/adlrvp/Kconfig b/src/mainboard/intel/adlrvp/Kconfig index 7a5f1ec..1a10304 100644 --- a/src/mainboard/intel/adlrvp/Kconfig +++ b/src/mainboard/intel/adlrvp/Kconfig @@ -43,6 +43,7 @@ select MAINBOARD_HAS_TPM2 select SPI_TPM select TPM_GOOGLE_CR50 + select DRIVERS_INTEL_ISH config BOARD_INTEL_ADLRVP_P_MCHP select BOARD_INTEL_ADLRVP_COMMON diff --git a/src/mainboard/intel/adlrvp/gpio.c b/src/mainboard/intel/adlrvp/gpio.c index 3c43a74..d9b11cc 100644 --- a/src/mainboard/intel/adlrvp/gpio.c +++ b/src/mainboard/intel/adlrvp/gpio.c @@ -11,16 +11,16 @@ /* GPIO A0-A6, A9-A10 default function is NF1 for eSPI interface when eSPI is enabled */ - /* SSD1_PWREN CPU SSD1 */ - PAD_CFG_GPO(GPP_D14, 1, PLTRST), + /* ISH UART0 TXD */ + PAD_CFG_NF(GPP_D14, NONE, DEEP, NF1), /* SSD1_RESET CPU SSD1 */ PAD_CFG_GPO(GPP_F20, 1, PLTRST), /* BT_RF_KILL_N */ PAD_CFG_GPO(GPP_A13, 1, PLTRST), /* WLAN RST# */ PAD_CFG_GPO(GPP_H2, 1, PLTRST), - /* WIFI_WAKE_N */ - PAD_CFG_GPI_IRQ_WAKE(GPP_D13, NONE, DEEP, LEVEL, INVERT), + /* ISH UART0 RXD */ + PAD_CFG_NF(GPP_D13, NONE, DEEP, NF1), /* x4 PCIE slot1 PWREN */ PAD_CFG_GPO(GPP_H17, 0, PLTRST), /* x4 PCIE slot 1 RESET */ diff --git a/src/mainboard/intel/adlrvp/variants/adlrvp_rpl_ext_ec/overridetree.cb b/src/mainboard/intel/adlrvp/variants/adlrvp_rpl_ext_ec/overridetree.cb index c78189f..7c16b2f 100644 --- a/src/mainboard/intel/adlrvp/variants/adlrvp_rpl_ext_ec/overridetree.cb +++ b/src/mainboard/intel/adlrvp/variants/adlrvp_rpl_ext_ec/overridetree.cb @@ -103,5 +103,26 @@ end end end + # NOTE: The following changes are for ADL-P LP5 RVP with rework for UFS2.1 or UFS3.1. + # RVP UFS support changed to default, whereas rework is required. + # a. Rework to route clk_src 4 that connected to the 2nd CPU SSD slot for UFS. + # b. Rework to change rp9 and rp10, which is used to be PCH SSD lanes, for UFS1. + # c. Rework for UFS reset signal. + # d. Additional rework is needed to change 3.3v to 2.5v and 1.8v to 1.2v for UFS3.1. + # d. When UFS is enabled, rp9 and rp10 function as UFS link. + # For RVP without UFS rework, NVMe/SSD device can only be connected to the first CPU + # SSD slot with the device config below. + # + device ref pcie4_1 off end + device ref pcie_rp9 off end + device ref pcie_rp11 off end + device ref ish on + chip drivers/intel/ish + register "add_acpi_dma_property" = "true" + device generic 0 on + end + end + end + device ref ufs on end end end -- To view, visit https://review.coreboot.org/c/coreboot/+/71740 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I812786b9a25c31bc008f055af90467d4edf6158e Gerrit-Change-Number: 71740 Gerrit-PatchSet: 1 Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com> Gerrit-CC: Cliff Huang <cliff.huang(a)intel.corp-partner.google.com> Gerrit-MessageType: newchange

1 0

[S] Change in coreboot[master]: soc/intel/alderlake: Disable Intel TXT
by Paul Menzel (Code Review) 07 Jan '23

07 Jan '23

Attention is currently required from: Subrata Banik, Angel Pons, Lean Sheng Tan, Werner Zeh. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/71575 ) Change subject: soc/intel/alderlake: Disable Intel TXT ...................................................................... Patch Set 6: (3 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/71575/comment/c2bcfaa5_a5c2f67f PS6, Line 8: … for framebuffer access https://review.coreboot.org/c/coreboot/+/71575/comment/fdb263c2_040badcc PS6, Line 10: Please add a problem description, why disabling TXT might be useful. File src/soc/intel/alderlake/romstage/romstage.c: https://review.coreboot.org/c/coreboot/+/71575/comment/dca89924_0658d4f8 PS6, Line 142: * It would help to access VGA framebuffer prior calling into FSP-M Please add a dot/period at the end of sentences. -- To view, visit https://review.coreboot.org/c/coreboot/+/71575 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9dd7c5492a5f45eef0dd9e836cc2da1844c78919 Gerrit-Change-Number: 71575 Gerrit-PatchSet: 6 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.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: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Comment-Date: Sat, 07 Jan 2023 08:21:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: security/intel/txt: Helper function to disable TXT
by Paul Menzel (Code Review) 07 Jan '23

07 Jan '23

Attention is currently required from: Subrata Banik, Jérémy Compostella, Sridhar Siricilla, Angel Pons, Lean Sheng Tan, Werner Zeh. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/71574 ) Change subject: security/intel/txt: Helper function to disable TXT ...................................................................... Patch Set 5: (9 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/71574/comment/2318d419_9e53bd1c PS5, Line 6: > Possible long commit subject (prefer a maximum 65 characters) This is a false warning as UTF-8 string markers are added for some reason. ``` ### checkpatch.out ### WARNING: Possible long commit subject (prefer a maximum 65 characters) #4: Subject: [PATCH] =?UTF-8?q?security/intel/txt:=20Helper=20function=20to=20?= ``` https://review.coreboot.org/c/coreboot/+/71574/comment/8194ef6f_f5b2f4e6 PS5, Line 7: security/intel/txt: Helper function to disable TXT Please make it a statement: Add helper function to disable TXT https://review.coreboot.org/c/coreboot/+/71574/comment/023304b0_a78a3b20 PS5, Line 9: This patch Add a function to disable TXT as per … https://review.coreboot.org/c/coreboot/+/71574/comment/a4c1e73c_6bdc18d2 PS5, Line 12: On platform with TXT disabled, the memory can be unlocked using platforms File src/security/intel/txt/txtlib.c: https://review.coreboot.org/c/coreboot/+/71574/comment/bd904b2b_b410796a PS5, Line 57: /* Don't disable if INTEL_TXT config is selected */ I’d say the comment is redundant, as the code is clear. https://review.coreboot.org/c/coreboot/+/71574/comment/026e391c_cfcf65cb PS5, Line 59: return; I’d check that on the caller side. If I run `disable_intel_txt()` I’d expect it to disable TXT independent of the configuration option. https://review.coreboot.org/c/coreboot/+/71574/comment/3d691387_acca28e8 PS5, Line 63: printk(BIOS_INFO, "CPU is not TXT capable.\n"); No idea how the function is supposed to be called, but I’d say something is wrong, if it’s called on a non-TXT capable CPU? (Maybe print the CPU model?) https://review.coreboot.org/c/coreboot/+/71574/comment/5c6d271b_116f43c8 PS5, Line 70: security(TPM) Add a space before the (? https://review.coreboot.org/c/coreboot/+/71574/comment/e6c88340_4e84453e PS5, Line 74: printk(BIOS_INFO, "TXT disabled successfully- Unlock Memory\n"); 1. Add a space before the -? 2. Maybe: Unlock*ed* *m*emory -- To view, visit https://review.coreboot.org/c/coreboot/+/71574 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I27f613428e82a1dd924172eab853d2ce9c32b473 Gerrit-Change-Number: 71574 Gerrit-PatchSet: 5 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.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: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Comment-Date: Sat, 07 Jan 2023 08:19:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XL] Change in coreboot[master]: Add minimal GNAT run time system (RTS)
by Elyes Haouas (Code Review) 07 Jan '23

07 Jan '23

Attention is currently required from: Nico Huber. Elyes Haouas has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/11836 ) Change subject: Add minimal GNAT run time system (RTS) ...................................................................... Patch Set 15: (1 comment) Patchset: PS15: > https://review.coreboot.org/c/coreboot/+/67828 […] #67828 restored -- To view, visit https://review.coreboot.org/c/coreboot/+/11836 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie6522abf093f0a516b9ae18ddc69131bd721dc0c Gerrit-Change-Number: 11836 Gerrit-PatchSet: 15 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Adrian-Ken Rueegsegger <ken(a)codelabs.ch> Gerrit-Reviewer: Edward O'Callaghan <edward.ocallaghan(a)koparo.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Reto Buerki <reet(a)codelabs.ch> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Sat, 07 Jan 2023 08:15:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Reto Buerki <reet(a)codelabs.ch> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: lib/gnat: Remove Compiler_Unit_Warning pragmas
by Elyes Haouas (Code Review) 07 Jan '23

07 Jan '23

Hello build bot (Jenkins), Nico Huber, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/67828 to look at the new patch set (#11). Change subject: lib/gnat: Remove Compiler_Unit_Warning pragmas ...................................................................... lib/gnat: Remove Compiler_Unit_Warning pragmas 'pragma Compiler_Unit_Warning' is removed upstream: https://gcc.gnu.org/git/?p=gcc.git&a=search&h=HEAD&st=commit&s=pragma+Compi… Fix: GCC libgnat-x86_32/lib/gnat/interfac.o interfac.ads:36:08: warning: unrecognized pragma "Compiler_Unit_Warning" [-gnatwg] Change-Id: Ia55304da1e3b2db1532300c4f1a2dd958f1975c4 Signed-off-by: Elyes Haouas <ehaouas(a)noos.fr> --- M src/lib/gnat/interfac.ads M src/lib/gnat/s-imenne.adb M src/lib/gnat/s-imenne.ads M src/lib/gnat/s-parame.ads M src/lib/gnat/s-stoele.adb M src/lib/gnat/s-stoele.ads M src/lib/gnat/s-unstyp.ads 7 files changed, 17 insertions(+), 13 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/28/67828/11 -- To view, visit https://review.coreboot.org/c/coreboot/+/67828 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia55304da1e3b2db1532300c4f1a2dd958f1975c4 Gerrit-Change-Number: 67828 Gerrit-PatchSet: 11 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit January 2023