coreboot-gerrit November 2017

coreboot-gerrit@coreboot.org

1 participants
1623 discussions

Change in coreboot[master]: drivers/i2c/tpm/cr50: Simplify and increase init delay to 30 seconds
by Vadim Bendebury (Code Review) 21 Nov '17

21 Nov '17

Vadim Bendebury has uploaded this change for review. ( https://review.coreboot.org/22554 Change subject: drivers/i2c/tpm/cr50: Simplify and increase init delay to 30 seconds ...................................................................... drivers/i2c/tpm/cr50: Simplify and increase init delay to 30 seconds The Cr50 i2c driver provides separate entry points for probing and initialization, but probing function does not really do much. It also claims and releases locality on every coreboot stage, but there is no need for this - locality should be claimed in the beginning and retained through the boot process. On top of that the driver does not properly account for long time it could take the Cr50 chip to come around to reset processing if TPM reset request was posted during a lengthy TPM operation. This patch addresses the issues as follows: - tpm_vendor_probe() and tpm_vendor_cleanup() become noops, kept around to conform to the expected driver API. - tpm_vendor_init() invokes request_locality() only when initializing the TPM after reset (typically during verstage) - request_locality() checks if locality is taken, and if so - waits for it to be released, which is an indication of TPM finishing reset processing. BRANCH=none BUG=b:65867313, b:68729265 TEST=Verified that reef no longer hangs during EC reboot and firmware_Cr50ClearTPMOwner (not yet merged) tests. Change-Id: Iba8445caf1342e3a5fefcb2664b0759a1a8e84e3 Signed-off-by: Vadim Bendebury <vbendeb(a)chromium.org> --- M src/drivers/i2c/tpm/cr50.c 1 file changed, 60 insertions(+), 82 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/22554/1 diff --git a/src/drivers/i2c/tpm/cr50.c b/src/drivers/i2c/tpm/cr50.c index 9cc9e95..38d9b73 100644 --- a/src/drivers/i2c/tpm/cr50.c +++ b/src/drivers/i2c/tpm/cr50.c @@ -170,57 +170,68 @@ return cr50_i2c_wait_tpm_ready(chip); } -static int check_locality(struct tpm_chip *chip, int loc) +static int request_locality(struct tpm_chip *chip) { - uint8_t mask = TPM_ACCESS_VALID | TPM_ACCESS_ACTIVE_LOCALITY; - uint8_t buf; + struct stopwatch sw; + uint8_t access; - if (cr50_i2c_read(chip, TPM_ACCESS(loc), &buf, 1) < 0) - return -1; + /* + * Locality is released by TPM reset. + * + * If locality is taken at this point, this could be due to the fact + * that the TPM is performing a long operation and has not processed + * reset request yet. We'll wait up to CR50_TIMEOUT_INIT_MS and see if + * it releases locality when reset is processed. + */ + stopwatch_init_msecs_expire(&sw, CR50_TIMEOUT_INIT_MS); + do { + int rv; + uint8_t mask = TPM_ACCESS_VALID | TPM_ACCESS_ACTIVE_LOCALITY; - if ((buf & mask) == mask) { - chip->vendor.locality = loc; - return loc; - } + rv = cr50_i2c_read(chip, TPM_ACCESS(0), + &access, sizeof(access)); + if (rv || ((access & mask) == mask)) { + /* + * Don't bombard the chip with traffic, let it keep + * processing the command. + */ + mdelay(2); + continue; + } + + /* + * Ok, the locality is free, TPM must be reset, let's claim + * it. + */ + access = TPM_ACCESS_REQUEST_USE; + if (cr50_i2c_write(chip, TPM_ACCESS(0), + &access, sizeof(access)) < 0) { + printk(BIOS_ERR, "%s:%d failed requesting locality\n", + __func__, __LINE__); + return -1; + } + + rv = cr50_i2c_read(chip, TPM_ACCESS(0), + &access, sizeof(access)); + if (rv || ((access & mask) != mask)) { + printk(BIOS_ERR, + "%s:%d failed requesting locality, " + "rv=%d, access %#x\n", + __func__, __LINE__, rv, access); + return -1; + } + + printk(BIOS_INFO, "TPM ready after %ld ms\n", + stopwatch_duration_msecs(&sw)); + + return 0; + } while (!stopwatch_expired(&sw)); + + printk(BIOS_ERR, + "Failed to claim locality 0 after %ld ms, status: %#x\n", + stopwatch_duration_msecs(&sw), access); return -1; -} - -static void release_locality(struct tpm_chip *chip, int force) -{ - uint8_t mask = TPM_ACCESS_VALID | TPM_ACCESS_REQUEST_PENDING; - uint8_t addr = TPM_ACCESS(chip->vendor.locality); - uint8_t buf; - - if (cr50_i2c_read(chip, addr, &buf, 1) < 0) - return; - - if (force || (buf & mask) == mask) { - buf = TPM_ACCESS_ACTIVE_LOCALITY; - cr50_i2c_write(chip, addr, &buf, 1); - } - - chip->vendor.locality = 0; -} - -static int request_locality(struct tpm_chip *chip, int loc) -{ - uint8_t buf = TPM_ACCESS_REQUEST_USE; - struct stopwatch sw; - - if (check_locality(chip, loc) >= 0) - return loc; - - if (cr50_i2c_write(chip, TPM_ACCESS(loc), &buf, 1) < 0) - return -1; - - stopwatch_init_msecs_expire(&sw, CR50_TIMEOUT_LONG_MS); - while (check_locality(chip, loc) < 0) { - if (stopwatch_expired(&sw)) - return -1; - mdelay(CR50_TIMEOUT_SHORT_MS); - } - return loc; } /* cr50 requires all 4 bytes of status register to be read */ @@ -419,38 +430,6 @@ int tpm_vendor_probe(unsigned int bus, uint32_t addr) { - struct tpm_inf_dev *tpm_dev = car_get_var_ptr(&g_tpm_dev); - struct tpm_chip probe_chip; - struct stopwatch sw; - uint8_t buf = 0; - int ret; - long sw_run_duration = CR50_TIMEOUT_INIT_MS; - - tpm_dev->bus = bus; - tpm_dev->addr = addr; - - cr50_vendor_init(&probe_chip); - - /* Wait for TPM_ACCESS register ValidSts bit to be set */ - stopwatch_init_msecs_expire(&sw, sw_run_duration); - do { - ret = cr50_i2c_read(&probe_chip, TPM_ACCESS(0), &buf, 1); - if (!ret && (buf & TPM_STS_VALID)) { - sw_run_duration = stopwatch_duration_msecs(&sw); - break; - } - mdelay(CR50_TIMEOUT_SHORT_MS); - } while (!stopwatch_expired(&sw)); - - printk(BIOS_INFO, - "%s: ValidSts bit %s(%d) in TPM_ACCESS register after %ld ms\n", - __func__, (buf & TPM_STS_VALID) ? "set" : "clear", - (buf & TPM_STS_VALID) >> 7, sw_run_duration); - - /* Claim failure if the ValidSts (bit 7) is clear */ - if (!(buf & TPM_STS_VALID)) - return -1; - return 0; } @@ -469,8 +448,9 @@ cr50_vendor_init(chip); - if (request_locality(chip, 0) != 0) - return -1; + if (ENV_VERSTAGE || ENV_BOOTBLOCK) + if (request_locality(chip)) + return -1; /* Read four bytes from DID_VID register */ if (cr50_i2c_read(chip, TPM_DID_VID(0), (uint8_t *)&vendor, 4) < 0) @@ -488,11 +468,9 @@ return 0; out_err: - release_locality(chip, 1); return -1; } void tpm_vendor_cleanup(struct tpm_chip *chip) { - release_locality(chip, 1); } -- To view, visit https://review.coreboot.org/22554 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Iba8445caf1342e3a5fefcb2664b0759a1a8e84e3 Gerrit-Change-Number: 22554 Gerrit-PatchSet: 1 Gerrit-Owner: Vadim Bendebury <vbendeb(a)chromium.org> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org>

1 0

Change in coreboot[master]: src/soc/intel/apollolake: move TCO1 disable into bootblock
by build bot (Jenkins) (Code Review) 21 Nov '17

21 Nov '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/22553 ) Change subject: src/soc/intel/apollolake: move TCO1 disable into bootblock ...................................................................... Patch Set 2: Verified+1 Build Successful https://qa.coreboot.org/job/coreboot-checkpatch/18524/ : SUCCESS https://qa.coreboot.org/job/coreboot-gerrit/63624/ : SUCCESS -- To view, visit https://review.coreboot.org/22553 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaf1f97924590163e45bcac667b6c607503cc8b87 Gerrit-Change-Number: 22553 Gerrit-PatchSet: 2 Gerrit-Owner: Vadim Bendebury <vbendeb(a)chromium.org> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Vadim Bendebury <vbendeb(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 21 Nov 2017 17:36:34 +0000 Gerrit-HasComments: No

1 0

Change in coreboot[master]: apollolake: move TCO1 disable into bootblock
by build bot (Jenkins) (Code Review) 21 Nov '17

21 Nov '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/22553 ) Change subject: apollolake: move TCO1 disable into bootblock ...................................................................... Patch Set 1: Verified-1 Build Unstable https://qa.coreboot.org/job/coreboot-gerrit/63623/ : UNSTABLE https://qa.coreboot.org/job/coreboot-checkpatch/18523/ : SUCCESS -- To view, visit https://review.coreboot.org/22553 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaf1f97924590163e45bcac667b6c607503cc8b87 Gerrit-Change-Number: 22553 Gerrit-PatchSet: 1 Gerrit-Owner: Vadim Bendebury <vbendeb(a)chromium.org> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Vadim Bendebury <vbendeb(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 21 Nov 2017 17:17:01 +0000 Gerrit-HasComments: No

1 0

Change in coreboot[master]: apollolake: move TCO1 disable into bootblock
by Vadim Bendebury (Code Review) 21 Nov '17

21 Nov '17

Vadim Bendebury has uploaded this change for review. ( https://review.coreboot.org/22553 Change subject: apollolake: move TCO1 disable into bootblock ...................................................................... apollolake: move TCO1 disable into bootblock Cr50 reset processing could take long time, up to 30 s in the worst case. The TCO watchdog needs to be disabled before Cr50 driver starts, let's disable it in bootblock. BRANCH=none BUG=b:65867313, b:68729265 TEST=verified that resetting the device while keys are being generated by the TPM does not cause falling into recovery. Change-Id: Iaf1f97924590163e45bcac667b6c607503cc8b87 Signed-off-by: Vadim Bendebury <vbendeb(a)chromium.org> --- M src/soc/intel/apollolake/bootblock/bootblock.c M src/soc/intel/apollolake/romstage.c 2 files changed, 8 insertions(+), 11 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/53/22553/1 diff --git a/src/soc/intel/apollolake/bootblock/bootblock.c b/src/soc/intel/apollolake/bootblock/bootblock.c index f02d8cf..214d4fd 100644 --- a/src/soc/intel/apollolake/bootblock/bootblock.c +++ b/src/soc/intel/apollolake/bootblock/bootblock.c @@ -89,6 +89,8 @@ void bootblock_soc_early_init(void) { + uint32_t reg; + enable_pmcbar(); /* Clear global reset promotion bit */ @@ -109,4 +111,10 @@ /* Initialize GPE for use as interrupt status */ pmc_gpe_init(); + + /* Stop TCO timer */ + reg = inl(ACPI_PMIO_BASE + TCO1_CNT); + reg |= TCO_TMR_HLT; + outl(reg, ACPI_PMIO_BASE + TCO1_CNT); + } diff --git a/src/soc/intel/apollolake/romstage.c b/src/soc/intel/apollolake/romstage.c index f315e61..a003ea0 100644 --- a/src/soc/intel/apollolake/romstage.c +++ b/src/soc/intel/apollolake/romstage.c @@ -99,16 +99,6 @@ lpc_io_setup_comm_a_b(); } -static void disable_watchdog(void) -{ - uint32_t reg; - - /* Stop TCO timer */ - reg = inl(ACPI_BASE_ADDRESS + TCO1_CNT); - reg |= TCO_TMR_HLT; - outl(reg, ACPI_BASE_ADDRESS + TCO1_CNT); -} - /* * Punit Initialization code. This all isn't documented, but * this is the recipe. @@ -202,7 +192,6 @@ timestamp_add_now(TS_START_ROMSTAGE); soc_early_romstage_init(); - disable_watchdog(); console_init(); -- To view, visit https://review.coreboot.org/22553 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Iaf1f97924590163e45bcac667b6c607503cc8b87 Gerrit-Change-Number: 22553 Gerrit-PatchSet: 1 Gerrit-Owner: Vadim Bendebury <vbendeb(a)chromium.org>

1 0

Change in coreboot[master]: soc/amd/common: Include appropriate headers in dimm_spd.h
by build bot (Jenkins) (Code Review) 21 Nov '17

21 Nov '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/22547 ) Change subject: soc/amd/common: Include appropriate headers in dimm_spd.h ...................................................................... Patch Set 2: Verified+1 Build Successful https://qa.coreboot.org/job/coreboot-checkpatch/18520/ : SUCCESS https://qa.coreboot.org/job/coreboot-gerrit/63620/ : SUCCESS -- To view, visit https://review.coreboot.org/22547 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I69e8eaffefbda4fdfb89264a55762558950aa5e2 Gerrit-Change-Number: 22547 Gerrit-PatchSet: 2 Gerrit-Owner: Marc Jones <marc(a)marcjonesconsulting.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 21 Nov 2017 15:30:10 +0000 Gerrit-HasComments: No

1 0

Change in coreboot[master]: google/kahlee/variants/grunt: Add SPD function
by build bot (Jenkins) (Code Review) 21 Nov '17

21 Nov '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/22486 ) Change subject: google/kahlee/variants/grunt: Add SPD function ...................................................................... Patch Set 4: Verified+1 Build Successful https://qa.coreboot.org/job/coreboot-checkpatch/18522/ : SUCCESS https://qa.coreboot.org/job/coreboot-gerrit/63622/ : SUCCESS -- To view, visit https://review.coreboot.org/22486 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I789002bfadc1a2b24f9046708986d29c0e2daf33 Gerrit-Change-Number: 22486 Gerrit-PatchSet: 4 Gerrit-Owner: Marc Jones <marc(a)marcjonesconsulting.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Marc Jones <marc(a)marcjonesconsulting.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 21 Nov 2017 15:26:14 +0000 Gerrit-HasComments: No

1 0

Change in coreboot[master]: soc/amd/stoneyridge: Get entire DDR4 SPD
by build bot (Jenkins) (Code Review) 21 Nov '17

21 Nov '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/22484 ) Change subject: soc/amd/stoneyridge: Get entire DDR4 SPD ...................................................................... Patch Set 4: Verified+1 Build Successful https://qa.coreboot.org/job/coreboot-checkpatch/18519/ : SUCCESS https://qa.coreboot.org/job/coreboot-gerrit/63619/ : SUCCESS -- To view, visit https://review.coreboot.org/22484 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0bdf8101de22533b2f4337d3c9e4423d62e6c66d Gerrit-Change-Number: 22484 Gerrit-PatchSet: 4 Gerrit-Owner: Marc Jones <marc(a)marcjonesconsulting.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Marc Jones <marc(a)marcjonesconsulting.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 21 Nov 2017 15:25:48 +0000 Gerrit-HasComments: No

1 0

Change in coreboot[master]: mainboard/google/fizz: override SMBIOS type01 MANUFACTURER
by build bot (Jenkins) (Code Review) 21 Nov '17

21 Nov '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/22462 ) Change subject: mainboard/google/fizz: override SMBIOS type01 MANUFACTURER ...................................................................... Patch Set 6: Verified-1 Build Unstable https://qa.coreboot.org/job/coreboot-gerrit/63618/ : UNSTABLE https://qa.coreboot.org/job/coreboot-checkpatch/18518/ : SUCCESS -- To view, visit https://review.coreboot.org/22462 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic71ef09959f01ba97600ae3fbda1e22a606759fd Gerrit-Change-Number: 22462 Gerrit-PatchSet: 6 Gerrit-Owner: Kevin Chiu <Kevin.Chiu(a)quantatw.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Keith Tzeng <keith.tzeng(a)quantatw.com> Gerrit-Reviewer: Kevin Chiu <Kevin.Chiu(a)quantatw.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Shawn Nematbakhsh <shawnn(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 21 Nov 2017 15:21:45 +0000 Gerrit-HasComments: No

1 0

Change in coreboot[master]: soc/amd/stoneyridge: Add mainboard call for SPD values
by build bot (Jenkins) (Code Review) 21 Nov '17

21 Nov '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/22485 ) Change subject: soc/amd/stoneyridge: Add mainboard call for SPD values ...................................................................... Patch Set 4: Verified+1 Build Successful https://qa.coreboot.org/job/coreboot-checkpatch/18521/ : SUCCESS https://qa.coreboot.org/job/coreboot-gerrit/63621/ : SUCCESS -- To view, visit https://review.coreboot.org/22485 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id42622008b49b4559e648a7fa1bfd9f26e1f56a4 Gerrit-Change-Number: 22485 Gerrit-PatchSet: 4 Gerrit-Owner: Marc Jones <marc(a)marcjonesconsulting.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Marc Jones <marc(a)marcjonesconsulting.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 21 Nov 2017 15:20:25 +0000 Gerrit-HasComments: No

1 0

Change in coreboot[master]: Create SOC description file soc.asl
by build bot (Jenkins) (Code Review) 21 Nov '17

21 Nov '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/22541 ) Change subject: Create SOC description file soc.asl ...................................................................... Patch Set 3: Verified+1 Build Successful https://qa.coreboot.org/job/coreboot-checkpatch/18517/ : SUCCESS https://qa.coreboot.org/job/coreboot-gerrit/63617/ : SUCCESS -- To view, visit https://review.coreboot.org/22541 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I83d850cf9457f7c2c787336823d993ae2e9d28ce Gerrit-Change-Number: 22541 Gerrit-PatchSet: 3 Gerrit-Owner: Richard Spiegel <richard.spiegel(a)silverbackltd.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Richard Spiegel <richard.spiegel(a)silverbackltd.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 21 Nov 2017 14:46:44 +0000 Gerrit-HasComments: No

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2017