coreboot-gerrit November 2018

coreboot-gerrit@coreboot.org

1 participants
1699 discussions

Change in coreboot[master]: lenovo/h8, thinkpads: Re-do USB Always On
by build bot (Jenkins) (Code Review) 10 Nov '18

10 Nov '18

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/29565 ) Change subject: lenovo/h8,thinkpads: Re-do USB Always On ...................................................................... Patch Set 1: (2 comments) https://review.coreboot.org/#/c/29565/1/src/ec/lenovo/h8/h8.c File src/ec/lenovo/h8/h8.c: https://review.coreboot.org/#/c/29565/1/src/ec/lenovo/h8/h8.c@138 PS1, Line 138: switch(on) { switch and case should be at the same indent https://review.coreboot.org/#/c/29565/1/src/ec/lenovo/h8/h8.c@138 PS1, Line 138: switch(on) { space required before the open parenthesis '(' -- To view, visit https://review.coreboot.org/29565 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If812cd1ef8fb1a24d7fadbe834f574b40cbcd56a Gerrit-Change-Number: 29565 Gerrit-PatchSet: 1 Gerrit-Owner: Nathaniel Roach <nroach44(a)gmail.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sat, 10 Nov 2018 00:48:23 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in coreboot[master]: lenovo/h8, thinkpads: Re-do USB Always On
by build bot (Jenkins) (Code Review) 10 Nov '18

10 Nov '18

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/29565 ) Change subject: lenovo/h8,thinkpads: Re-do USB Always On ...................................................................... Patch Set 2: (2 comments) https://review.coreboot.org/#/c/29565/2/src/ec/lenovo/h8/h8.c File src/ec/lenovo/h8/h8.c: https://review.coreboot.org/#/c/29565/2/src/ec/lenovo/h8/h8.c@138 PS2, Line 138: switch(on) { switch and case should be at the same indent https://review.coreboot.org/#/c/29565/2/src/ec/lenovo/h8/h8.c@138 PS2, Line 138: switch(on) { space required before the open parenthesis '(' -- To view, visit https://review.coreboot.org/29565 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If812cd1ef8fb1a24d7fadbe834f574b40cbcd56a Gerrit-Change-Number: 29565 Gerrit-PatchSet: 2 Gerrit-Owner: Nathaniel Roach <nroach44(a)gmail.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sat, 10 Nov 2018 00:47:45 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in coreboot[master]: lenovo/h8, thinkpads: Re-do USB Always On
by Nathaniel Roach (Code Review) 10 Nov '18

10 Nov '18

Nathaniel Roach has uploaded a new patch set (#2). ( https://review.coreboot.org/29565 ) Change subject: lenovo/h8,thinkpads: Re-do USB Always On ...................................................................... lenovo/h8,thinkpads: Re-do USB Always On Re-write the UAO handling code as it had stopped working (#171) (the flag was not getting read from the RTC properly in SMM) Remove the SMM code as it's not needed (but EC flag won't be set upon entering S3 now) Set the EC flags on boot the same way other flags are set Document bitwise operators for clarity Propagate changes to other Thinkpads (updated X201 to have 2 bits for the flag as it only had 1) Per Nicola Corna's previous commits, 0x0d is set for "AC only" "AC only" does exhibit different behaviour - the USB port is turned on a few seconds after entering S3, rather than < 1 sec, regardless of AC status Tested on X220 Change-Id: If812cd1ef8fb1a24d7fadbe834f574b40cbcd56a Signed-off-by: Nathaniel Roach <nroach44(a)gmail.com> --- M src/ec/lenovo/h8/Makefile.inc M src/ec/lenovo/h8/h8.c M src/ec/lenovo/h8/h8.h D src/ec/lenovo/h8/smm.c M src/mainboard/lenovo/l520/smihandler.c M src/mainboard/lenovo/t420/smihandler.c M src/mainboard/lenovo/t420s/smihandler.c M src/mainboard/lenovo/t430/smihandler.c M src/mainboard/lenovo/t430s/smihandler.c M src/mainboard/lenovo/t520/smihandler.c M src/mainboard/lenovo/t530/smihandler.c M src/mainboard/lenovo/x201/cmos.layout M src/mainboard/lenovo/x201/smihandler.c M src/mainboard/lenovo/x220/cmos.layout M src/mainboard/lenovo/x220/smihandler.c M src/mainboard/lenovo/x230/smihandler.c 16 files changed, 43 insertions(+), 73 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/29565/2 -- To view, visit https://review.coreboot.org/29565 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If812cd1ef8fb1a24d7fadbe834f574b40cbcd56a Gerrit-Change-Number: 29565 Gerrit-PatchSet: 2 Gerrit-Owner: Nathaniel Roach <nroach44(a)gmail.com>

1 0

Change in coreboot[master]: lenovo/h8, thinkpads: Re-do USB Always On
by Nathaniel Roach (Code Review) 10 Nov '18

10 Nov '18

Nathaniel Roach has uploaded this change for review. ( https://review.coreboot.org/29565 Change subject: lenovo/h8,thinkpads: Re-do USB Always On ...................................................................... lenovo/h8,thinkpads: Re-do USB Always On Re-write the UAO handling code as it had stopped working (#171) (the flag was not getting read from the RTC properly in SMM) Remove the SMM code as it's not needed (but EC flag won't be set upon entering S3 now) Set the EC flags on boot the same way other flags are set Document bitwise operators for clarity Propagate changes to other Thinkpads (updated X201 to have 2 bits for the flag as it only had 1) Per Nicola Corna's previous commits, 0x0d is set for "AC only" "AC only" does exhibit different behaviour - the USB port is turned on a few seconds after entering S3, rather than < 1 sec, regardless of AC status Tested on X220 Change-Id: If812cd1ef8fb1a24d7fadbe834f574b40cbcd56a Signed-off-by: Nathaniel Roach <nroach44(a)gmail.com> --- M src/ec/lenovo/h8/Makefile.inc M src/ec/lenovo/h8/h8.c M src/ec/lenovo/h8/h8.h M src/mainboard/lenovo/l520/smihandler.c M src/mainboard/lenovo/t420/smihandler.c M src/mainboard/lenovo/t420s/smihandler.c M src/mainboard/lenovo/t430/smihandler.c M src/mainboard/lenovo/t430s/smihandler.c M src/mainboard/lenovo/t520/smihandler.c M src/mainboard/lenovo/t530/smihandler.c M src/mainboard/lenovo/x201/cmos.layout M src/mainboard/lenovo/x201/smihandler.c M src/mainboard/lenovo/x220/cmos.layout M src/mainboard/lenovo/x220/smihandler.c M src/mainboard/lenovo/x230/smihandler.c 15 files changed, 43 insertions(+), 26 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/29565/1 diff --git a/src/ec/lenovo/h8/Makefile.inc b/src/ec/lenovo/h8/Makefile.inc index e4408f1..ebf6d7d 100644 --- a/src/ec/lenovo/h8/Makefile.inc +++ b/src/ec/lenovo/h8/Makefile.inc @@ -9,6 +9,5 @@ ramstage-y += bluetooth.c ramstage-y += wwan.c ramstage-$(CONFIG_HAVE_ACPI_TABLES) += ssdt.c -smm-y += smm.c endif diff --git a/src/ec/lenovo/h8/h8.c b/src/ec/lenovo/h8/h8.c index 1f1655d..d70de78 100644 --- a/src/ec/lenovo/h8/h8.c +++ b/src/ec/lenovo/h8/h8.c @@ -128,6 +128,35 @@ } +void h8_usb_always_on_enable(enum usb_always_on on) +{ + u8 val; + + // bit 0 enables USB Always On + // bits 2&3 equal, when they are 0 AC_AND_BATTERY, when 1 AC_ONLY + + switch(on) { + case UAO_OFF: + val = ec_read(H8_USB_ALWAYS_ON); + val &= 0xF2; // Clear bits 0,2,3 + ec_write(H8_USB_ALWAYS_ON, val); + break; + + case UAO_AC_AND_BATTERY: + val = ec_read(H8_USB_ALWAYS_ON); + val |= 0x01; // Set bit 0 + val &= 0xF3; // Clear bits 2 and 3 + ec_write(H8_USB_ALWAYS_ON, val); + break; + + case UAO_AC_ONLY: + val = ec_read(H8_USB_ALWAYS_ON); + val |= 0x0D; // Set bits 0,2,3 + ec_write(H8_USB_ALWAYS_ON, val); + break; + } +} + void h8_usb_power_enable(int onoff) { if (onoff) @@ -270,8 +299,10 @@ ec_write(0x1f, conf->eventf_enable); ec_write(H8_FAN_CONTROL, H8_FAN_CONTROL_AUTO); - ec_write(H8_USB_ALWAYS_ON, ec_read(H8_USB_ALWAYS_ON) & - ~H8_USB_ALWAYS_ON_ENABLE); + + if (get_option(&val, "usb_always_on") != CB_SUCCESS) + val = 0; + h8_usb_always_on_enable(val); if (get_option(&val, "wlan") != CB_SUCCESS) val = 1; diff --git a/src/ec/lenovo/h8/h8.h b/src/ec/lenovo/h8/h8.h index 4ac395a..7deb0b8 100644 --- a/src/ec/lenovo/h8/h8.h +++ b/src/ec/lenovo/h8/h8.h @@ -19,9 +19,16 @@ #include <stdint.h> #include <device/device.h> +enum usb_always_on { + UAO_OFF = 0, + UAO_AC_AND_BATTERY = 1, + UAO_AC_ONLY = 2 +}; + void h8_trackpoint_enable(int on); void h8_wlan_enable(int on); void h8_set_audio_mute(int on); +void h8_usb_always_on_enable(enum usb_always_on on); void h8_usb_power_enable(int on); void h8_enable_event(int event); void h8_disable_event(int event); @@ -87,7 +94,7 @@ #define H8_USB_ALWAYS_ON 0x0d #define H8_USB_ALWAYS_ON_ENABLE 0x01 -#define H8_USB_ALWAYS_ON_AC_ONLY 0x0c +#define H8_USB_ALWAYS_ON_AC_ONLY 0x0d #define H8_FAN_CONTROL 0x2f #define H8_FAN_CONTROL_AUTO 0x80 diff --git a/src/mainboard/lenovo/l520/smihandler.c b/src/mainboard/lenovo/l520/smihandler.c index fa038ed..982233d 100644 --- a/src/mainboard/lenovo/l520/smihandler.c +++ b/src/mainboard/lenovo/l520/smihandler.c @@ -73,8 +73,6 @@ void mainboard_smi_sleep(u8 slp_typ) { - h8_usb_always_on(); - if (slp_typ == 3) { u8 ec_wake = ec_read(0x32); /* If EC wake events are enabled, enable wake on EC WAKE GPE. */ diff --git a/src/mainboard/lenovo/t420/smihandler.c b/src/mainboard/lenovo/t420/smihandler.c index dd29232..bc92cf1 100644 --- a/src/mainboard/lenovo/t420/smihandler.c +++ b/src/mainboard/lenovo/t420/smihandler.c @@ -72,8 +72,6 @@ void mainboard_smi_sleep(u8 slp_typ) { - h8_usb_always_on(); - if (slp_typ == 3) { u8 ec_wake = ec_read(0x32); /* If EC wake events are enabled, enable wake on EC WAKE GPE. */ diff --git a/src/mainboard/lenovo/t420s/smihandler.c b/src/mainboard/lenovo/t420s/smihandler.c index c34cb5b..e361ebb 100644 --- a/src/mainboard/lenovo/t420s/smihandler.c +++ b/src/mainboard/lenovo/t420s/smihandler.c @@ -107,8 +107,6 @@ void mainboard_smi_sleep(u8 slp_typ) { - h8_usb_always_on(); - if (slp_typ == 3) { u8 ec_wake = ec_read(0x32); /* If EC wake events are enabled, enable wake on EC WAKE GPE. */ diff --git a/src/mainboard/lenovo/t430/smihandler.c b/src/mainboard/lenovo/t430/smihandler.c index 120d9a0..f9d2373 100644 --- a/src/mainboard/lenovo/t430/smihandler.c +++ b/src/mainboard/lenovo/t430/smihandler.c @@ -74,8 +74,6 @@ void mainboard_smi_sleep(u8 slp_typ) { if (slp_typ == 3) { - h8_usb_always_on(); - u8 ec_wake = ec_read(0x32); /* If EC wake events are enabled, enable wake on EC WAKE GPE. */ if (ec_wake & 0x14) { diff --git a/src/mainboard/lenovo/t430s/smihandler.c b/src/mainboard/lenovo/t430s/smihandler.c index 1313eac..3259b9d 100644 --- a/src/mainboard/lenovo/t430s/smihandler.c +++ b/src/mainboard/lenovo/t430s/smihandler.c @@ -106,8 +106,6 @@ void mainboard_smi_sleep(u8 slp_typ) { - h8_usb_always_on(); - if (slp_typ == 3) { u8 ec_wake = ec_read(0x32); /* If EC wake events are enabled, enable wake on EC WAKE GPE. */ diff --git a/src/mainboard/lenovo/t520/smihandler.c b/src/mainboard/lenovo/t520/smihandler.c index 4e61a98..9a154a1 100644 --- a/src/mainboard/lenovo/t520/smihandler.c +++ b/src/mainboard/lenovo/t520/smihandler.c @@ -107,8 +107,6 @@ void mainboard_smi_sleep(u8 slp_typ) { - h8_usb_always_on(); - if (slp_typ == 3) { u8 ec_wake = ec_read(0x32); /* If EC wake events are enabled, enable wake on EC WAKE GPE. */ diff --git a/src/mainboard/lenovo/t530/smihandler.c b/src/mainboard/lenovo/t530/smihandler.c index 0491392..20137f8 100644 --- a/src/mainboard/lenovo/t530/smihandler.c +++ b/src/mainboard/lenovo/t530/smihandler.c @@ -107,8 +107,6 @@ void mainboard_smi_sleep(u8 slp_typ) { - h8_usb_always_on(); - if (slp_typ == 3) { u8 ec_wake = ec_read(0x32); /* If EC wake events are enabled, enable wake on EC WAKE GPE. */ diff --git a/src/mainboard/lenovo/x201/cmos.layout b/src/mainboard/lenovo/x201/cmos.layout index d8d8794..0cda679 100644 --- a/src/mainboard/lenovo/x201/cmos.layout +++ b/src/mainboard/lenovo/x201/cmos.layout @@ -70,7 +70,7 @@ 419 1 e 1 power_management_beeps 420 1 e 1 low_battery_beep 421 1 e 9 sata_mode -422 1 e 11 usb_always_on +422 2 e 11 usb_always_on #423 1 r 1 unused # coreboot config options: northbridge diff --git a/src/mainboard/lenovo/x201/smihandler.c b/src/mainboard/lenovo/x201/smihandler.c index fbbec09..2106c57 100644 --- a/src/mainboard/lenovo/x201/smihandler.c +++ b/src/mainboard/lenovo/x201/smihandler.c @@ -177,8 +177,6 @@ void mainboard_smi_sleep(u8 slp_typ) { - h8_usb_always_on(); - if (slp_typ == 3) { u8 ec_wake = ec_read(0x32); /* If EC wake events are enabled, enable wake on EC WAKE GPE. */ diff --git a/src/mainboard/lenovo/x220/cmos.layout b/src/mainboard/lenovo/x220/cmos.layout index c6e270a..d4a4ed3 100644 --- a/src/mainboard/lenovo/x220/cmos.layout +++ b/src/mainboard/lenovo/x220/cmos.layout @@ -69,7 +69,7 @@ 418 1 e 1 sticky_fn 419 1 e 1 power_management_beeps 421 1 e 9 sata_mode -422 1 e 12 usb_always_on +422 2 e 12 usb_always_on #423 1 r 1 unused # coreboot config options: cpu diff --git a/src/mainboard/lenovo/x220/smihandler.c b/src/mainboard/lenovo/x220/smihandler.c index 0491392..20137f8 100644 --- a/src/mainboard/lenovo/x220/smihandler.c +++ b/src/mainboard/lenovo/x220/smihandler.c @@ -107,8 +107,6 @@ void mainboard_smi_sleep(u8 slp_typ) { - h8_usb_always_on(); - if (slp_typ == 3) { u8 ec_wake = ec_read(0x32); /* If EC wake events are enabled, enable wake on EC WAKE GPE. */ diff --git a/src/mainboard/lenovo/x230/smihandler.c b/src/mainboard/lenovo/x230/smihandler.c index a69b78f..2425927 100644 --- a/src/mainboard/lenovo/x230/smihandler.c +++ b/src/mainboard/lenovo/x230/smihandler.c @@ -72,8 +72,6 @@ void mainboard_smi_sleep(u8 slp_typ) { - h8_usb_always_on(); - if (slp_typ == 3) { u8 ec_wake = ec_read(0x32); /* If EC wake events are enabled, enable wake on EC WAKE GPE. */ -- To view, visit https://review.coreboot.org/29565 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: If812cd1ef8fb1a24d7fadbe834f574b40cbcd56a Gerrit-Change-Number: 29565 Gerrit-PatchSet: 1 Gerrit-Owner: Nathaniel Roach <nroach44(a)gmail.com>

1 0

Change in coreboot[master]: Kalista: Disable EC-EFS
by Daisuke Nojiri (Code Review) 10 Nov '18

10 Nov '18

Daisuke Nojiri has uploaded this change for review. ( https://review.coreboot.org/29564 Change subject: Kalista: Disable EC-EFS ...................................................................... Kalista: Disable EC-EFS This patch disables EC-EFS. Signed-off-by: Daisuke Nojiri <dnojiri(a)chromium.org> CQ-DEPEND=CL:1330171 BUG=b:118386511 BRANCH=none TEST=Boot Fizz. Verify normal boot, soft sync, recovery mode work. Change-Id: Icd18662ae1e76f35eb9bcd521b1951aacc713252 Signed-off-by: Daisuke Nojiri <dnojiri(a)chromium.org> --- M src/mainboard/google/fizz/Kconfig 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/64/29564/1 diff --git a/src/mainboard/google/fizz/Kconfig b/src/mainboard/google/fizz/Kconfig index 4d08eda..e1fc38d 100644 --- a/src/mainboard/google/fizz/Kconfig +++ b/src/mainboard/google/fizz/Kconfig @@ -37,7 +37,7 @@ config VBOOT select EC_GOOGLE_CHROMEEC_SWITCHES - select VBOOT_EC_EFS + select VBOOT_EC_EFS if BOARD_GOOGLE_FIZZ select VBOOT_PHYSICAL_REC_SWITCH select HAS_RECOVERY_MRC_CACHE select MRC_CLEAR_NORMAL_CACHE_ON_RECOVERY_RETRAIN -- To view, visit https://review.coreboot.org/29564 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Icd18662ae1e76f35eb9bcd521b1951aacc713252 Gerrit-Change-Number: 29564 Gerrit-PatchSet: 1 Gerrit-Owner: Daisuke Nojiri <dnojiri(a)chromium.org>

1 0

Change in coreboot[master]: security/tpm: Fix TCPA log feature
by build bot (Jenkins) (Code Review) 09 Nov '18

09 Nov '18

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/29563 ) Change subject: security/tpm: Fix TCPA log feature ...................................................................... Patch Set 1: (9 comments) https://review.coreboot.org/#/c/29563/1/src/arch/x86/car.ld File src/arch/x86/car.ld: https://review.coreboot.org/#/c/29563/1/src/arch/x86/car.ld@35 PS1, Line 35: * Needs to be transfered until CBMEM is available 'transfered' may be misspelled - perhaps 'transferred'? https://review.coreboot.org/#/c/29563/1/src/include/memlayout.h File src/include/memlayout.h: https://review.coreboot.org/#/c/29563/1/src/include/memlayout.h@168 PS1, Line 168: #define VBOOT2_TPM_LOG(addr, size) \ Macros with multiple statements should be enclosed in a do - while loop https://review.coreboot.org/#/c/29563/1/src/include/memlayout.h@168 PS1, Line 168: #define VBOOT2_TPM_LOG(addr, size) \ macros should not use a trailing semicolon https://review.coreboot.org/#/c/29563/1/src/security/tpm/tspi/log.c File src/security/tpm/tspi/log.c: https://review.coreboot.org/#/c/29563/1/src/security/tpm/tspi/log.c@31 PS1, Line 31: tclt = (struct tcpa_table*)_vboot2_tpm_log; "(foo*)" should be "(foo *)" https://review.coreboot.org/#/c/29563/1/src/security/tpm/tspi/log.c@70 PS1, Line 70: snprintf((char *)&(hash[j * 2]), tce->digest_length, "%02x", tce->digest[j]); line over 80 characters https://review.coreboot.org/#/c/29563/1/src/security/tpm/tspi/log.c@72 PS1, Line 72: snprintf(log_string, TCPA_LOG_STRING_LENGTH, " %u %s 00 [%s]\n", line over 80 characters https://review.coreboot.org/#/c/29563/1/src/security/vboot/vboot_crtm.c File src/security/vboot/vboot_crtm.c: https://review.coreboot.org/#/c/29563/1/src/security/vboot/vboot_crtm.c@162 PS1, Line 162: const struct tcpa_table *tclt = (struct tcpa_table*)_vboot2_tpm_log; "(foo*)" should be "(foo *)" https://review.coreboot.org/#/c/29563/1/src/security/vboot/vboot_crtm.c@165 PS1, Line 165: for (i = 0; i < tclt->num_entries; i++) { braces {} are not necessary for single statement blocks https://review.coreboot.org/#/c/29563/1/src/security/vboot/vboot_crtm.c@166 PS1, Line 166: tcpa_log_add_table_entry(tclt->entries[i].name, tclt->entries[i].pcr, tclt->entries[i].digest, tclt->entries[i].digest_length); line over 80 characters -- To view, visit https://review.coreboot.org/29563 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic93133531b84318f48940d34bded48cbae739c44 Gerrit-Change-Number: 29563 Gerrit-PatchSet: 1 Gerrit-Owner: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 09 Nov 2018 23:39:32 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in coreboot[master]: security/tpm: Fix TCPA log feature
by Philipp Deppenwiese (Code Review) 09 Nov '18

09 Nov '18

Philipp Deppenwiese has uploaded this change for review. ( https://review.coreboot.org/29563 Change subject: security/tpm: Fix TCPA log feature ...................................................................... security/tpm: Fix TCPA log feature * Refactor TCPA log code * Add TCPA log dump fucntion * Make TCPA log available in bootblock * Fix TCPA log formatting Change-Id: Ic93133531b84318f48940d34bded48cbae739c44 Signed-off-by: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> --- M src/arch/x86/car.ld M src/commonlib/include/commonlib/tcpa_log_serialized.h M src/include/memlayout.h M src/security/tpm/tspi.h M src/security/tpm/tspi/log.c M src/security/vboot/Kconfig M src/security/vboot/symbols.h M src/security/vboot/vboot_crtm.c M src/soc/cavium/cn81xx/include/soc/memlayout.ld M util/cbmem/cbmem.c 10 files changed, 111 insertions(+), 42 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/29563/1 diff --git a/src/arch/x86/car.ld b/src/arch/x86/car.ld index 33fb5c9..ea1c5f1 100644 --- a/src/arch/x86/car.ld +++ b/src/arch/x86/car.ld @@ -31,6 +31,12 @@ #if IS_ENABLED(CONFIG_VBOOT_SEPARATE_VERSTAGE) VBOOT2_WORK(., 16K) #endif + /* Vboot measured boot TCPA log measurements + * Needs to be transfered until CBMEM is available + */ +#if IS_ENABLED(CONFIG_VBOOT_TCPA_LOG_RECOVERY) + VBOOT2_TPM_LOG(., 2K) +#endif /* Stack for CAR stages. Since it persists across all stages that * use CAR it can be reused. The chipset/SoC is expected to provide * the stack size. */ diff --git a/src/commonlib/include/commonlib/tcpa_log_serialized.h b/src/commonlib/include/commonlib/tcpa_log_serialized.h index 1e9ffaa..f5ebfb1 100644 --- a/src/commonlib/include/commonlib/tcpa_log_serialized.h +++ b/src/commonlib/include/commonlib/tcpa_log_serialized.h @@ -18,11 +18,12 @@ #include <stdint.h> +#define MAX_PREMEM_TCPA_LOG_ENTRIES 14 #define MAX_TCPA_LOG_ENTRIES 50 -#define TCPA_LOG_STRING_LENGTH 512 +#define TCPA_LOG_STRING_LENGTH 256 #define TCPA_FORMAT_HASH_LENGTH 128 #define TCPA_DIGEST_MAX_LENGTH 64 -#define TCPA_PCR_HASH_NAME 256 +#define TCPA_PCR_HASH_NAME 64 struct tcpa_entry { uint32_t pcr; diff --git a/src/include/memlayout.h b/src/include/memlayout.h index 5de2370..ba6644f 100644 --- a/src/include/memlayout.h +++ b/src/include/memlayout.h @@ -165,6 +165,10 @@ REGION(vboot2_work, addr, size, 16) \ _ = ASSERT(size >= 12K, "vboot2 work buffer must be at least 12K!"); +#define VBOOT2_TPM_LOG(addr, size) \ + REGION(vboot2_tpm_log, addr, size, 2) \ + _ = ASSERT(size >= 2K, "vboot2 tpm log buffer must be at least 2K!"); + #if ENV_VERSTAGE #define VERSTAGE(addr, sz) \ SYMBOL(verstage, addr) \ diff --git a/src/security/tpm/tspi.h b/src/security/tpm/tspi.h index d69b976..99a7e1d 100644 --- a/src/security/tpm/tspi.h +++ b/src/security/tpm/tspi.h @@ -33,6 +33,11 @@ const size_t digest_length); /** + * Dump TCPA log entries on console + */ +void tcpa_log_dump(void); + +/** * Ask vboot for a digest and extend a TPM PCR with it. * @param pcr sets the pcr index * @param digest sets the hash to extend into the tpm diff --git a/src/security/tpm/tspi/log.c b/src/security/tpm/tspi/log.c index 17f6f1f..33486a5 100644 --- a/src/security/tpm/tspi/log.c +++ b/src/security/tpm/tspi/log.c @@ -14,67 +14,92 @@ */ #include <string.h> -#include <cbmem.h> #include <console/console.h> #include <security/tpm/tspi.h> +#include <arch/early_variables.h> +#include <fmap.h> +#include <region_file.h> +#include <string.h> +#include <cbmem.h> +#include <security/vboot/symbols.h> static struct tcpa_table *tcpa_log_init(void) { - MAYBE_STATIC struct tcpa_table *tclt = NULL; + struct tcpa_table *tclt; - if (!cbmem_possibly_online()) - return NULL; + if (!cbmem_possibly_online()) { + tclt = (struct tcpa_table*)_vboot2_tpm_log; + tclt->max_entries = MAX_PREMEM_TCPA_LOG_ENTRIES; + tclt->num_entries = 0; + } else { + tclt = cbmem_find(CBMEM_ID_TCPA_LOG); + if (!tclt) { + tclt = cbmem_add(CBMEM_ID_TCPA_LOG, + sizeof(struct tcpa_table) + + MAX_TCPA_LOG_ENTRIES * + sizeof(struct tcpa_entry)); - if (tclt != NULL) - return tclt; - - tclt = (struct tcpa_table *) cbmem_entry_find(CBMEM_ID_TCPA_LOG); - if (tclt) - return tclt; - - tclt = cbmem_add(CBMEM_ID_TCPA_LOG, - sizeof(struct tcpa_table) + - MAX_TCPA_LOG_ENTRIES * - sizeof(struct tcpa_entry)); - - if (!tclt) { - printk(BIOS_ERR, "ERROR: Could not create TCPA log table\n"); - return NULL; + tclt->max_entries = MAX_TCPA_LOG_ENTRIES; + tclt->num_entries = 0; + } } - tclt->max_entries = MAX_TCPA_LOG_ENTRIES; - tclt->num_entries = 0; - - printk(BIOS_DEBUG, "TCPA log created at %p\n", tclt); - + printk(BIOS_DEBUG, "TPM: TCPA log created at %p\n", tclt); return tclt; } +void tcpa_log_dump(void) +{ + int i, j; + char log_string[TCPA_LOG_STRING_LENGTH]; + char hash[TCPA_FORMAT_HASH_LENGTH]; + struct tcpa_table *tclt; + + tclt = tcpa_log_init(); + if (!tclt) + return; + + printk(BIOS_INFO, "coreboot TCPA log:\n"); + for (i = 0; i < tclt->num_entries; i++) { + struct tcpa_entry *tce = &tclt->entries[i]; + if (tce) { + memset(log_string, 0, TCPA_LOG_STRING_LENGTH); + memset(hash, 0, TCPA_LOG_STRING_LENGTH); + + for (j = 0; j < tce->digest_length; j++) + snprintf((char *)&(hash[j * 2]), tce->digest_length, "%02x", tce->digest[j]); + + snprintf(log_string, TCPA_LOG_STRING_LENGTH, " %u %s 00 [%s]\n", + tce->pcr, hash, tce->name); + printk(BIOS_INFO, "%s", log_string); + } + } +} + void tcpa_log_add_table_entry(const char *name, const uint32_t pcr, const uint8_t *digest, const size_t digest_length) { - struct tcpa_table *tclt; - struct tcpa_entry *tce; - - tclt = tcpa_log_init(); - + struct tcpa_table *tclt = tcpa_log_init(); if (!tclt) return; - if (tclt->num_entries == tclt->max_entries) { - printk(BIOS_WARNING, "ERROR: TCPA log table is full\n"); + if (tclt->num_entries >= tclt->max_entries) { + printk(BIOS_WARNING, "TPM: TCPA log table is full\n"); return; } - tce = &tclt->entries[tclt->num_entries++]; + struct tcpa_entry *tce = &tclt->entries[tclt->num_entries++]; + if (tce) { + strncpy(tce->name, name, TCPA_PCR_HASH_NAME - 1); + tce->pcr = pcr; - strncpy(tce->name, name, TCPA_PCR_HASH_NAME - 1); - tce->pcr = pcr; + if (digest_length > TCPA_DIGEST_MAX_LENGTH) { + printk(BIOS_WARNING, "TPM: PCR digest too long for TCPA log entry\n"); + return; + } - if (digest_length > TCPA_DIGEST_MAX_LENGTH) { - printk(BIOS_WARNING, "ERROR: PCR digest too long for TCPA log entry\n"); + memcpy(tce->digest, digest, digest_length); + tce->digest_length = digest_length; return; } - memcpy(tce->digest, digest, digest_length); - tce->digest_length = digest_length; } diff --git a/src/security/vboot/Kconfig b/src/security/vboot/Kconfig index e7cb43a..fd69114 100644 --- a/src/security/vboot/Kconfig +++ b/src/security/vboot/Kconfig @@ -241,6 +241,13 @@ Add a space delimited list of filenames that should only be in the RO section. +config VBOOT_TCPA_LOG_RECOVERY + bool + default y + help + Enables TCPA log measurements recovery from bootblock to + romstage. + menu "GBB configuration" config GBB_HWID diff --git a/src/security/vboot/symbols.h b/src/security/vboot/symbols.h index ddf9d95..92cbeea 100644 --- a/src/security/vboot/symbols.h +++ b/src/security/vboot/symbols.h @@ -20,4 +20,6 @@ extern u8 _evboot2_work[]; #define _vboot2_work_size (_evboot2_work - _vboot2_work) +extern u8 _vboot2_tpm_log[]; + #endif /* __VBOOT_SYMBOLS_H__ */ diff --git a/src/security/vboot/vboot_crtm.c b/src/security/vboot/vboot_crtm.c index 5b76aa8..b71fce8 100644 --- a/src/security/vboot/vboot_crtm.c +++ b/src/security/vboot/vboot_crtm.c @@ -18,7 +18,9 @@ #include <fmap.h> #include <security/tpm/tspi.h> #include <security/vboot/vboot_crtm.h> +#include <security/vboot/symbols.h> #include <program_loading.h> +#include <cbmem.h> uint32_t vboot_init_crtm(void) { @@ -132,6 +134,8 @@ case PROG_PAYLOAD: tpm_measure_region(prog_rdev(prog), TPM_PAYLOAD_PCR, prog->name); + /* Last measurement, afterwards dump completed log */ + tcpa_log_dump(); break; case PROG_REFCODE: tpm_measure_region(prog_rdev(prog), TPM_INTEL_FSP_PCR, @@ -152,3 +156,15 @@ break; } } + +static void recover_tcpa_log(int is_recovery) +{ + const struct tcpa_table *tclt = (struct tcpa_table*)_vboot2_tpm_log; + if (tclt) { + int i; + for (i = 0; i < tclt->num_entries; i++) { + tcpa_log_add_table_entry(tclt->entries[i].name, tclt->entries[i].pcr, tclt->entries[i].digest, tclt->entries[i].digest_length); + } + } +} +ROMSTAGE_CBMEM_INIT_HOOK(recover_tcpa_log); diff --git a/src/soc/cavium/cn81xx/include/soc/memlayout.ld b/src/soc/cavium/cn81xx/include/soc/memlayout.ld index e3bf61f..0562ed6 100644 --- a/src/soc/cavium/cn81xx/include/soc/memlayout.ld +++ b/src/soc/cavium/cn81xx/include/soc/memlayout.ld @@ -35,7 +35,10 @@ PRERAM_CBMEM_CONSOLE(BOOTROM_OFFSET + 0x8000, 8K) BOOTBLOCK(BOOTROM_OFFSET + 0x20000, 64K) VBOOT2_WORK(BOOTROM_OFFSET + 0x30000, 12K) - VERSTAGE(BOOTROM_OFFSET + 0x33000, 52K) +#if IS_ENABLED(CONFIG_VBOOT_TCPA_LOG_RECOVERY) + VBOOT2_TPM_LOG(BOOTROM_OFFSET + 0x33000, 2K) +#endif + VERSTAGE(BOOTROM_OFFSET + 0x34000, 50K) ROMSTAGE(BOOTROM_OFFSET + 0x40000, 256K) SRAM_END(BOOTROM_OFFSET + 0x80000) diff --git a/util/cbmem/cbmem.c b/util/cbmem/cbmem.c index 8ab80b0..79852da 100644 --- a/util/cbmem/cbmem.c +++ b/util/cbmem/cbmem.c @@ -720,7 +720,7 @@ for (j = 0; j < tce->digest_length; j++) sprintf((char *)&(hash[j * 2]), "%02x", tce->digest[j]); - snprintf(log_string, TCPA_LOG_STRING_LENGTH, "%u %s 00 [%s]\n", + snprintf(log_string, TCPA_LOG_STRING_LENGTH, " %u %s 00 [%s]\n", tce->pcr, hash, tce->name); printf("%s", log_string); } -- To view, visit https://review.coreboot.org/29563 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ic93133531b84318f48940d34bded48cbae739c44 Gerrit-Change-Number: 29563 Gerrit-PatchSet: 1 Gerrit-Owner: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>

1 0

Change in coreboot[master]: mb/google/poppy/variant/atlas: I2C: run trackpad at 1MHz
by Caveh Jalali (Code Review) 09 Nov '18

09 Nov '18

1 0

Change in coreboot[master]: mb/google/poppy/variant/atlas: I2C: run trackpad at 1MHz
by Caveh Jalali (Code Review) 09 Nov '18

09 Nov '18

Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/29553 ) Change subject: mb/google/poppy/variant/atlas: I2C: run trackpad at 1MHz ...................................................................... Patch Set 3: (2 comments) https://review.coreboot.org/#/c/29553/1//COMMIT_MSG Commit Message: https://review.coreboot.org/#/c/29553/1//COMMIT_MSG@15 PS1, Line 15: Data Hold_time larger than 0ns. > It shouldn't be 260ns. According to Grant, it should be 0ns for fast plus mode, please double check. […] Done https://review.coreboot.org/#/c/29553/1//COMMIT_MSG@34 PS1, Line 34: Verified by gaggery.tsai(a)intel.corp-partner.google.com. : Scope shots posted here: : https://b.corp.google.com/issues/78601949#comment177 : : BUG=b:78601949 > Do we need this in comment? Done -- To view, visit https://review.coreboot.org/29553 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaf42ba7b8818b7cd9c8dcc657823dac705659d38 Gerrit-Change-Number: 29553 Gerrit-PatchSet: 3 Gerrit-Owner: Caveh Jalali <caveh(a)google.com> Gerrit-Reviewer: Caveh Jalali <caveh(a)google.com> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Gaggery Tsai <gaggery.tsai(a)intel.com> Gerrit-Reviewer: Gaggery Tsai <gaggery.tsai(a)intel.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: caveh jalali <caveh(a)chromium.org> Gerrit-Comment-Date: Fri, 09 Nov 2018 23:22:47 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in coreboot[master]: mb/google/poppy/variant/atlas: I2C: run trackpad at 1MHz
by Caveh Jalali (Code Review) 09 Nov '18

09 Nov '18

Hello Gaggery Tsai, caveh jalali, Duncan Laurie, Gaggery Tsai, build bot (Jenkins), Furquan Shaikh, I'd like you to reexamine a change. Please visit https://review.coreboot.org/29553 to look at the new patch set (#3). Change subject: mb/google/poppy/variant/atlas: I2C: run trackpad at 1MHz ...................................................................... mb/google/poppy/variant/atlas: I2C: run trackpad at 1MHz With this change, coreboot thinks we're running at 1MHz: DW I2C bus 2 at 0xd1133000 (1000 KHz) Elan eKT3644 IC Specification (trackpad) requires: Low Time larger than 500ns (61 * 8.3ns = 506ns). High Time larger than 260ns (32 * 8.3ns = 265ns), Data Hold_time larger than 0ns. Start Condition Hold time larger than 250ns. Rise/Fall time of less than 120ns. HCNT controls both High Time and Start Condition Hold time. LCNT controls Low Time. SDA_HOLD controls Data Hold Time. P2 Atlas "Rise time" is 90ns and "Fall time" is 32ns and tuned using resistors on the board and must be considered when adjusting any of the parameters since these times are all measured at 30 or 70% of base and peak voltages (0v/1.8v). The eKT3644 requirements are met with LCNT=69, HCNT=33, SDA_HOLD=20 which yields the SCL at around 950KHz - suboptimal but compliant. Lower LCNT or HCNT results in "lost arbitration" errors or not complying with eKT3644 requirements. Verified by gaggery.tsai(a)intel.corp-partner.google.com. Scope shots posted here: https://b.corp.google.com/issues/78601949#comment177 BUG=b:78601949 BRANCH=none TEST=Farzam provided test points on track pad for SCL/SDA/GND. Waveforms measured with oscilloscope and screen shots attached to bug (comment #177, #155, #100). Operate trackpad/touchscreen Review dmesg (kernel) output for correct speed, parameters, and no errors (e.g. "lost arbitration" or "host controller timeout") Change-Id: Iaf42ba7b8818b7cd9c8dcc657823dac705659d38 Signed-off-by: Caveh Jalali <caveh(a)chromium.org> Signed-off-by: Grant Grundler <grundler(a)chromium.org> Tested-by: gaggery.tsai(a)intel.corp-partner.google.com Tested-by: grundler(a)chromium.org --- M src/mainboard/google/poppy/variants/atlas/devicetree.cb 1 file changed, 8 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/53/29553/3 -- To view, visit https://review.coreboot.org/29553 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iaf42ba7b8818b7cd9c8dcc657823dac705659d38 Gerrit-Change-Number: 29553 Gerrit-PatchSet: 3 Gerrit-Owner: Caveh Jalali <caveh(a)google.com> Gerrit-Reviewer: Caveh Jalali <caveh(a)google.com> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Gaggery Tsai <gaggery.tsai(a)intel.com> Gerrit-Reviewer: Gaggery Tsai <gaggery.tsai(a)intel.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: caveh jalali <caveh(a)chromium.org>

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2018