coreboot-gerrit November 2022

coreboot-gerrit@coreboot.org

1 participants
7286 discussions

[M] Change in coreboot[master]: src/soc/intel: Remove unnecessary space after casts
by Sean Rhodes (Code Review) 21 Nov '22

21 Nov '22

Attention is currently required from: Jeff Daly, Jonathan Zhang, Arthur Heymans, Tarun Tuli, Subrata Banik, Johnny Lin, Kapil Porwal, Christian Walter, Vanessa Eusebio, Lean Sheng Tan, Werner Zeh, Elyes Haouas, Tim Chu. Sean Rhodes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69806 ) Change subject: src/soc/intel: Remove unnecessary space after casts ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/69806 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I098104f32dd7c66d7bb79588ef315a242c3889ba Gerrit-Change-Number: 69806 Gerrit-PatchSet: 2 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Jeff Daly <jeffd(a)silicom-usa.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jeff Daly <jeffd(a)silicom-usa.com> Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Mon, 21 Nov 2022 16:33:47 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: Revert "src/arch/x86: Use core apic id to get cpu_index()"
by Subrata Banik (Code Review) 21 Nov '22

21 Nov '22

Attention is currently required from: Nico Huber, Johnny Lin, Christian Walter, Arthur Heymans, Andrey Petrov, Tim Chu. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69509 ) Change subject: Revert "src/arch/x86: Use core apic id to get cpu_index()" ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/69509 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3a40156ba275b572d7d1913d8c17c24b4c8f6d78 Gerrit-Change-Number: 69509 Gerrit-PatchSet: 4 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> 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: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Mon, 21 Nov 2022 16:29:03 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: Revert "src/arch/x86: Use core apic id to get cpu_index()"
by Arthur Heymans (Code Review) 21 Nov '22

21 Nov '22

Attention is currently required from: Nico Huber, Subrata Banik, Johnny Lin, Christian Walter, Arthur Heymans, Andrey Petrov, Tim Chu. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69509 ) Change subject: Revert "src/arch/x86: Use core apic id to get cpu_index()" ...................................................................... Patch Set 4: (1 comment) Patchset: PS3: > > > I ran into CPU exception after cherry-picking this CL > > > > > > [EMERG] CPU Index 1 - APIC 14 Unexpected Exception:6 @ 10:75f2f5e3 - Halting > > > [EMERG] Code: 0 eflags: 00010046 cr2: 00000000 > > > [EMERG] eax: 13121110 ebx: 75f2a1e0 ecx: 00000014 edx: 17161514 > > > [EMERG] edi: 75bf3ff8 esi: 75f2a1c8 ebp: 75be5fa8 esp: 75be5f50 > > > > > > [EMERG] 0x75f2f5a0: 00 00 00 f0 0f c1 01 40 > > > [EMERG] 0x75f2f5a8: 43 83 c1 04 eb df 8b 45 CPU Index 2 - APIC 12 Unexpected Exception:6 @ 10:75f2f5e3 - Halting > > > [EMERG] Code: 0 eflags: 00010046 cr2: 00000000 > > > [EMERG] eax: 13121110 ebx: 75f2a1e0 ecx: 00000014 edx: 17161514 > > > [EMERG] edi: 75bf4098 esi: 75f2a1c8 ebp: 75be3fa8 esp: 75be3f50 > > > > > > [EMERG] 0x75f2f5a0: 00 00 00 f0 0f c1 01 40 > > > [EMERG] 0x75f2f5a8: 43 83 c1 04 > > > > > > > Ok I fixed the code and it work on Intel aldrvp board. Can you try again? > > Tested on MTL(rex), LGTM Do you mind to +2 this? -- To view, visit https://review.coreboot.org/c/coreboot/+/69509 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3a40156ba275b572d7d1913d8c17c24b4c8f6d78 Gerrit-Change-Number: 69509 Gerrit-PatchSet: 4 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Subrata Banik <subratabanik(a)google.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: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Mon, 21 Nov 2022 16:22:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[L] Change in coreboot[master]: util/amdfwtool: Deal with psp position in flash offset directly
by Arthur Heymans (Code Review) 21 Nov '22

21 Nov '22

Attention is currently required from: Jason Glenesk, Raul Rangel, Jason Nien, Matt DeVillier, Martin Roth, Fred Reitberger, Felix Held. Hello Jason Glenesk, Raul Rangel, Jason Nien, Matt DeVillier, Martin Roth, Fred Reitberger, Felix Held, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/69852 to look at the new patch set (#4). Change subject: util/amdfwtool: Deal with psp position in flash offset directly ...................................................................... util/amdfwtool: Deal with psp position in flash offset directly Using a memory mapped offset is simply wrong to specify the PSP base as it depends on the flash size. Setting the flash offset directly in Kconfig allows to remove a lot of unnecessary aritmetics in both amdfwtool and Makefile. It also gets rid of the weird index semantics. This does not result in identical files as amdfwutil images differ depending on stack usage. TESTED: google/vilboz still boots. Change-Id: I89c9e73e7db748379c97e3c0ad69af3faedc8d66 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/mainboard/amd/birman/Kconfig M src/mainboard/amd/chausie/Kconfig M src/mainboard/amd/majolica/Kconfig M src/mainboard/amd/mandolin/Kconfig M src/mainboard/google/guybrush/Kconfig M src/mainboard/google/kahlee/Kconfig M src/mainboard/google/skyrim/Kconfig M src/mainboard/google/zork/Kconfig M src/soc/amd/cezanne/Kconfig M src/soc/amd/cezanne/Makefile.inc M src/soc/amd/common/Makefile.inc M src/soc/amd/common/block/include/amdblocks/psp_efs.h M src/soc/amd/common/block/psp/Kconfig M src/soc/amd/glinda/Kconfig M src/soc/amd/glinda/Makefile.inc M src/soc/amd/mendocino/Kconfig M src/soc/amd/mendocino/Makefile.inc M src/soc/amd/morgana/Kconfig M src/soc/amd/morgana/Makefile.inc M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/Makefile.inc M src/soc/amd/stoneyridge/Kconfig M src/soc/amd/stoneyridge/Makefile.inc M src/soc/amd/stoneyridge/fch.c M util/amdfwtool/amdfwtool.c 25 files changed, 109 insertions(+), 324 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/69852/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/69852 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I89c9e73e7db748379c97e3c0ad69af3faedc8d66 Gerrit-Change-Number: 69852 Gerrit-PatchSet: 4 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> 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: Jason Nien <jason.nien(a)amd.corp-partner.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-CC: 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: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[master]: security/tpm: support compiling in multiple TPM drivers
by Michał Żygowski (Code Review) 21 Nov '22

21 Nov '22

Attention is currently required from: Jason Glenesk, Raul Rangel, Matt DeVillier, Christian Walter, Julius Werner, Krystian Hebel, Fred Reitberger, Sergii Dmytruk, Felix Held. Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69162 ) Change subject: security/tpm: support compiling in multiple TPM drivers ...................................................................... Patch Set 7: (3 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/69162/comment/8afd70a0_7c020971 PS7, Line 9: Starting from here CONFIG_TPM1 and CONFIG_TPM2 are no longer mutually : exclusive. : We would also want to have MEMORY_MAPPED_TPM and CRB_TPM to stop being mutually exclusive. Is it possible with current patchset? https://review.coreboot.org/c/coreboot/+/69162/comment/6c337ed9_d364ad8d PS7, Line 12: Making probe functions static and always using them uncovered that : bootblock stage included TPM driver which it didn't use. This is why : Makefile.inc files were updated to replace `all-*` with : romstage, ramstage and verstage. In some cases TPM driver is used in bootblock, e.g. when Intel TXT or BootGuard is enabled. Why can't we include TPM drivers in bootblock? File src/lib/program.ld: https://review.coreboot.org/c/coreboot/+/69162/comment/4f51f6f9_0a315c1e PS7, Line 42: . = ALIGN(ARCH_POINTER_ALIGN_SIZE); : _tis_drivers = .; : KEEP(*(.rodata.tis_driver)); : _etis_drivers = .; : RECORD_SIZE(tis_drivers) Not sure why do we need it. Could you elaborate? -- To view, visit https://review.coreboot.org/c/coreboot/+/69162 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I44c5a1d825afe414c2f5c2c90f4cfe41ba9bef5f Gerrit-Change-Number: 69162 Gerrit-PatchSet: 7 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.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: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.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: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Mon, 21 Nov 2022 15:32:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: drivers/net/phy/m88e1512: Add downshift enable
by Mario Scheithauer (Code Review) 21 Nov '22

21 Nov '22

Mario Scheithauer has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/69853 ) Change subject: drivers/net/phy/m88e1512: Add downshift enable ...................................................................... drivers/net/phy/m88e1512: Add downshift enable This patch provides the functionality to enable downshift on Marvell PHY. By setting a downshift counter, the PHY is correspondingly often attempted to establish Gigabit link before the PHY downshifts to the next highest speed. The range is limited to 8 trials. To activate downshift, a software reset must follow to take effect. Change-Id: I4224eab6c1fc13824d53556c80435bc130a13bdb Signed-off-by: Mario Scheithauer <mario.scheithauer(a)siemens.com> --- M src/drivers/net/phy/m88e1512/chip.h M src/drivers/net/phy/m88e1512/m88e1512.c M src/drivers/net/phy/m88e1512/m88e1512.h 3 files changed, 50 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/53/69853/1 diff --git a/src/drivers/net/phy/m88e1512/chip.h b/src/drivers/net/phy/m88e1512/chip.h index 9a3523d..915001a 100644 --- a/src/drivers/net/phy/m88e1512/chip.h +++ b/src/drivers/net/phy/m88e1512/chip.h @@ -8,4 +8,7 @@ unsigned char led_1_ctrl; /* LED[1] Control */ unsigned char led_2_ctrl; /* LED[2] Control */ bool enable_int; /* INTn can be routed to LED[2] pin */ + /* 1x, 2x,...8x is the number of times the PHY attempts to establish Gigabit link + before the PHY downshifts to the next highest speed. */ + unsigned char downshift_cnt; }; diff --git a/src/drivers/net/phy/m88e1512/m88e1512.c b/src/drivers/net/phy/m88e1512/m88e1512.c index a8d1c73..f32991f 100644 --- a/src/drivers/net/phy/m88e1512/m88e1512.c +++ b/src/drivers/net/phy/m88e1512/m88e1512.c @@ -17,6 +17,28 @@ struct drivers_net_phy_m88e1512_config *config = dev->chip_info; uint16_t reg; + /* Enable downshift. */ + if (config->downshift_cnt) { + if (config->downshift_cnt > DOWNSHIFT_CNT_MAX) { + printk(BIOS_INFO, "%s: Downshift counter for %s is too large.\n", + dev_path(dev->bus->dev), dev->chip_ops->name); + } else { + printk(BIOS_DEBUG, "%s: Enable downshift after %d attempts for %s.\n", + dev_path(dev->bus->dev), config->downshift_cnt, + dev->chip_ops->name); + + reg = mdio_read(dev, COPPER_SPEC_CTRL_REG_1); + clrsetbits16(&reg, DOWNSHIFT_CNT_MASK, + DOWNSHIFT_CNT(config->downshift_cnt) | DOWNSHIFT_EN); + mdio_write(dev, COPPER_SPEC_CTRL_REG_1, reg); + + /* Downshift enable requires a software reset to take effect. */ + reg = mdio_read(dev, COPPER_CTRL_REG); + setbits16(&reg, SOFTWARE_RESET); + mdio_write(dev, COPPER_CTRL_REG, reg); + } + } + /* Configure LEDs if requested. */ if (config->configure_leds) { printk(BIOS_DEBUG, "%s: Set a customized LED mode for %s.\n", diff --git a/src/drivers/net/phy/m88e1512/m88e1512.h b/src/drivers/net/phy/m88e1512/m88e1512.h index ca0f756..449cc57 100644 --- a/src/drivers/net/phy/m88e1512/m88e1512.h +++ b/src/drivers/net/phy/m88e1512/m88e1512.h @@ -5,6 +5,15 @@ /* Register layout */ #define PAGE_REG 0x16 +/* Page 0 registers */ +#define COPPER_CTRL_REG 0 +#define SOFTWARE_RESET (1 << 15) +#define COPPER_SPEC_CTRL_REG_1 0x10 +#define DOWNSHIFT_CNT_MASK 0x7000 +#define DOWNSHIFT_CNT_MAX 8 +#define DOWNSHIFT_CNT(cnt) ((cnt - 1) << 12) +#define DOWNSHIFT_EN (1 << 11) +/* Page 3 registers */ #define LED_FUNC_CTRL_REG 0x10 #define LED_FUNC_CTRL_MASK 0x0FFF #define LED_TIMER_CTRL_REG 0x12 -- To view, visit https://review.coreboot.org/c/coreboot/+/69853 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4224eab6c1fc13824d53556c80435bc130a13bdb Gerrit-Change-Number: 69853 Gerrit-PatchSet: 1 Gerrit-Owner: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-MessageType: newchange

1 0

[M] Change in coreboot[master]: security/tpm: make tis_probe() return tpm_family
by Michał Żygowski (Code Review) 21 Nov '22

21 Nov '22

Attention is currently required from: Christian Walter, Julius Werner, Krystian Hebel, Sergii Dmytruk. Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69159 ) Change subject: security/tpm: make tis_probe() return tpm_family ...................................................................... Patch Set 4: (1 comment) File src/drivers/i2c/tpm/tpm.c: https://review.coreboot.org/c/coreboot/+/69159/comment/b06cc68d_83dd5945 PS1, Line 461: *tpm_family = 1; > Judging by the differences in addresses they might have implemented their own protocol derived from […] if (TPM_INTF_CAPABILITY.InterfaceVersion == 0) tpm_family = 1 // (TIS <= 1.21, not used by TPM 2.0 devices) else // (TIS 1.3, used both by TPM 1.2 and 2.0) if (TPM_STS.tpmFamily == 0) tpm_family = 1 else tpm_family = 2 This procedure is wrong, I added an appropriate procedure in parent change as a comment. -- To view, visit https://review.coreboot.org/c/coreboot/+/69159 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5006e0cdfef76ff79ce9e1cf280fcd5515ae01b0 Gerrit-Change-Number: 69159 Gerrit-PatchSet: 4 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Mon, 21 Nov 2022 15:17:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Comment-In-Reply-To: Krystian Hebel <krystian.hebel(a)3mdeb.com> Comment-In-Reply-To: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: drivers/pc80/tpm: probe for TPM family of a device
by Michał Żygowski (Code Review) 21 Nov '22

21 Nov '22

Attention is currently required from: Christian Walter, Julius Werner, Krystian Hebel, Sergii Dmytruk. Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69023 ) Change subject: drivers/pc80/tpm: probe for TPM family of a device ...................................................................... Patch Set 4: (3 comments) File src/drivers/pc80/tpm/tis.c: https://review.coreboot.org/c/coreboot/+/69023/comment/8c4d5204_5240586e PS4, Line 407: sts = tpm_read_status(locality); We cannot rely on TPM_STS register as older TPMs may not implement TPM family field. See https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClientTPMInterfa… Table 15. https://review.coreboot.org/c/coreboot/+/69023/comment/9e12c984_1e2395c7 PS4, Line 408: if ((intf_id & 0xf) == 0xf || (sts & TPM_STS_FAMILY_MASK) != TPM_STS_FAMILY_TPM_2_0) If intf_id is equal 0xf that means – FIFO interface as defined in TIS1.3 is active That doesn't mean the TPM is 2.0 or 1.2. We have to consult TIS_REG_INTF_CAPABILITY in such case. See Table 23 https://trustedcomputinggroup.org/wp-content/uploads/PC-Client-Specific-Pla… and check for the field InterfaceVersion, it should be supported by older TPMs: value 0 or 2 should mean TPM 1.2, and value of 3 should mean TPM 2.0 https://review.coreboot.org/c/coreboot/+/69023/comment/22fa3254_1b9b4092 PS4, Line 436: if (vendor_name == NULL) { : printk(BIOS_INFO, "Found TPM 0x%04x by 0x%04x\n", did, vid); : } else if (device_name == NULL) { : printk(BIOS_INFO, "Found TPM 0x%04x by %s (0x%04x)\n", did, vendor_name, vid); : } else { : const char *tpm_family = (family == TPM_1 ? "TPM1" : "TPM2"); : printk(BIOS_INFO, "Found %s %s (0x%04x) by %s (0x%04x)\n", tpm_family, : device_name, did, vendor_name, vid); : } : Let's report TPM family in each branch of execution. -- To view, visit https://review.coreboot.org/c/coreboot/+/69023 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5464771836c66bcc441efb7189ded416b8f53827 Gerrit-Change-Number: 69023 Gerrit-PatchSet: 4 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Mon, 21 Nov 2022 15:15:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: src/superio: Remove unnecessary space after casts
by Felix Held (Code Review) 21 Nov '22

21 Nov '22

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/69814 ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: src/superio: Remove unnecessary space after casts ...................................................................... src/superio: Remove unnecessary space after casts Change-Id: Iab76316f345fb0cf2ca2a3eaf40f64a1f1b7fc13 Signed-off-by: Elyes Haouas <ehaouas(a)noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/69814 Reviewed-by: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/superio/aspeed/common/aspeed.h M src/superio/fintek/f81803a/fan_control.c 2 files changed, 15 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Eric Lai: Looks good to me, approved diff --git a/src/superio/aspeed/common/aspeed.h b/src/superio/aspeed/common/aspeed.h index 3673a71..f41ca0f 100644 --- a/src/superio/aspeed/common/aspeed.h +++ b/src/superio/aspeed/common/aspeed.h @@ -408,7 +408,7 @@ #define ASPEED_EXIT_KEY 0xAA #define TO_BE_UPDATE 0 -#define AndMask32(HighBit, LowBit) ~((((uint32_t) 1 << (HighBit - LowBit + 1)) - 1) << LowBit) +#define AndMask32(HighBit, LowBit) ~((((uint32_t)1 << (HighBit - LowBit + 1)) - 1) << LowBit) typedef enum { GPIOA = 0, diff --git a/src/superio/fintek/f81803a/fan_control.c b/src/superio/fintek/f81803a/fan_control.c index 06a8338..8b5ddac 100644 --- a/src/superio/fintek/f81803a/fan_control.c +++ b/src/superio/fintek/f81803a/fan_control.c @@ -281,7 +281,7 @@ if (value > 100) return message_invalid_2(HWM_STATUS_INVALID_SECTION_VALUE, fan); temp = (255 * value) / 100; - value = (u8) (temp & 0x00ff); + value = (u8)(temp & 0x00ff); write_hwm_reg(base_address, index, value); index++; } -- To view, visit https://review.coreboot.org/c/coreboot/+/69814 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iab76316f345fb0cf2ca2a3eaf40f64a1f1b7fc13 Gerrit-Change-Number: 69814 Gerrit-PatchSet: 3 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in coreboot[master]: Add basic support for Rapto Lake S CPUs on Alder Lake PCH (#640555) T...
by Paul Menzel (Code Review) 21 Nov '22

21 Nov '22

Attention is currently required from: Tarun Tuli, Subrata Banik, Name of user not set #1004406, Michal Zygowski. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69798 ) Change subject: Add basic support for Rapto Lake S CPUs on Alder Lake PCH (#640555) This was tested on a MSI PRO Z690-A (ms7d25) with Ubuntu 22.10 and LinuxBoot. TODOS are Intel VBT. ...................................................................... Patch Set 3: (3 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/69798/comment/2ad0c1a6_7658860d PS3, Line 7: Rapto Rapto*r* https://review.coreboot.org/c/coreboot/+/69798/comment/44358ee1_83f58def PS3, Line 7: Add basic support for Rapto Lake S CPUs on Alder Lake PCH (#640555) Please add a blank line between the summary and the commit message body, cf [*How to Write a Git Commit Message*][1]. [1]: https://cbea.ms/git-commit/ Patchset: PS3: Thank you for the update. You can mark the comments, you addressed, as done by replying to them. -- To view, visit https://review.coreboot.org/c/coreboot/+/69798 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I767dd08a169a6af59188d9ecd73520b916f69155 Gerrit-Change-Number: 69798 Gerrit-PatchSet: 3 Gerrit-Owner: Name of user not set #1004406 Gerrit-Reviewer: Michal Zygowski <miczyg94(a)gmail.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Name of user not set #1004406 Gerrit-Attention: Michal Zygowski <miczyg94(a)gmail.com> Gerrit-Comment-Date: Mon, 21 Nov 2022 14:59:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2022