coreboot-gerrit February 2019

coreboot-gerrit@coreboot.org

1 participants
1249 discussions

Change in ...coreboot[master]: soc/intel/common: Include cometlake CPU IDs
by Paul Menzel (Code Review) 22 Feb '19

22 Feb '19

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31143 ) Change subject: soc/intel/common: Include cometlake CPU IDs ...................................................................... Patch Set 6: (1 comment) https://review.coreboot.org/#/c/31143/6//COMMIT_MSG Commit Message: https://review.coreboot.org/#/c/31143/6//COMMIT_MSG@7 PS6, Line 7: cometlake Official spelling seems to be Comet Lake. -- To view, visit https://review.coreboot.org/c/coreboot/+/31143 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I75d5b82524c9df1402abf6659d62dbc716c28c30 Gerrit-Change-Number: 31143 Gerrit-PatchSet: 6 Gerrit-Owner: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Fri, 22 Feb 2019 15:02:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: src/arch/x86/acpi.c: Update ACPI table's revision numbers
by Paul Menzel (Code Review) 22 Feb '19

22 Feb '19

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29579 ) Change subject: src/arch/x86/acpi.c: Update ACPI table's revision numbers ...................................................................... Patch Set 5: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/29579 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I22020bd156536ee8f23a267d7c7b2d7af6c7cfeb Gerrit-Change-Number: 29579 Gerrit-PatchSet: 5 Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 22 Feb 2019 14:56:47 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: arch/arm64: Add PCI config support in romstage
by Paul Menzel (Code Review) 22 Feb '19

22 Feb '19

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31307 ) Change subject: arch/arm64: Add PCI config support in romstage ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/31307 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9cc3dc51764f24b986434080f480932dceb8d133 Gerrit-Change-Number: 31307 Gerrit-PatchSet: 2 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 22 Feb 2019 14:55:49 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: security/vboot: Add measured boot mode
by Werner Zeh (Code Review) 22 Feb '19

22 Feb '19

Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29547 ) Change subject: security/vboot: Add measured boot mode ...................................................................... Patch Set 70: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/29547 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I339a2f1051e44f36aba9f99828f130592a09355e Gerrit-Change-Number: 29547 Gerrit-PatchSet: 70 Gerrit-Owner: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Huang Jin <huang.jin(a)intel.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: York Yang <york.yang(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 22 Feb 2019 13:35:42 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: security/tpm: Fix TCPA log feature
by Werner Zeh (Code Review) 22 Feb '19

22 Feb '19

Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29563 ) Change subject: security/tpm: Fix TCPA log feature ...................................................................... Patch Set 43: (10 comments) https://review.coreboot.org/#/c/29563/43/src/arch/x86/car.ld File src/arch/x86/car.ld: https://review.coreboot.org/#/c/29563/43/src/arch/x86/car.ld@35 PS43, Line 35: N No need for capital letter here. https://review.coreboot.org/#/c/29563/43/src/commonlib/include/commonlib/tc… File src/commonlib/include/commonlib/tcpa_log_serialized.h: https://review.coreboot.org/#/c/29563/43/src/commonlib/include/commonlib/tc… PS43, Line 24: TCPA_PCR_HASH_LEN 9 So far what I have seen you need to store either "SHA1", "SHA256" or "SHA512". 7 characters would be enough for that. Any extensions in mind where 9 chars will be needed? https://review.coreboot.org/#/c/29563/43/src/commonlib/include/commonlib/tc… PS43, Line 25: // Use a propper comment style. Or is it something clang-format has introduced and I am just no aware of it? https://review.coreboot.org/#/c/29563/43/src/security/tpm/tspi.h File src/security/tpm/tspi.h: https://review.coreboot.org/#/c/29563/43/src/security/tpm/tspi.h@50 PS43, Line 50: const char *digest_type Mind to update the doxygen comment with this new argument? https://review.coreboot.org/#/c/29563/43/src/security/tpm/tspi/log.c File src/security/tpm/tspi/log.c: https://review.coreboot.org/#/c/29563/43/src/security/tpm/tspi/log.c@32 PS43, Line 32: /* We are dealing here with pre CBMEM environment Add a "." at the end as your sentence is finished. https://review.coreboot.org/#/c/29563/43/src/security/tpm/tspi/log.c@50 PS43, Line 50: if (!cbmem_possibly_online() && IS_ENABLED(CONFIG_VBOOT_TCPA_LOG_RECOVERY)) { Maybe just "else if (IS_ENABLED(CONFIG_VBOOT_TCPA_LOG_RECOVERY))" here as cbmem_possibly_online() has been processed already and you are checking for !cbmem_possibly_online() here again. https://review.coreboot.org/#/c/29563/43/src/security/tpm/tspi/log.c@68 PS43, Line 68: tcpa_log_init(); Do you really want to call this init_code when the tables are dumped? Aren't you just need to get the pointer to the table without modifying the contents? Or do I oversee something here? https://review.coreboot.org/#/c/29563/43/src/security/tpm/tspi/tspi.c File src/security/tpm/tspi/tspi.c: https://review.coreboot.org/#/c/29563/43/src/security/tpm/tspi/tspi.c@243 PS43, Line 243: strncpy(hash_name, "SHA1", TCPA_PCR_HASH_LEN - 1); Maybe provide a const array of strings and then index into this array with hash_alg to get the right string? https://review.coreboot.org/#/c/29563/43/util/cbmem/cbmem.c File util/cbmem/cbmem.c: https://review.coreboot.org/#/c/29563/43/util/cbmem/cbmem.c@687 PS43, Line 687: static void dump_tcpa_log(void) Ther is already a function which is very similar: tcpa_log_dump() in tspi/log.c Can't you use it to dump the log? https://review.coreboot.org/#/c/29563/43/util/cbmem/cbmem.c@694 PS43, Line 694: tcpa_log Make sure tcpa_log is not NULL? -- To view, visit https://review.coreboot.org/c/coreboot/+/29563 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic93133531b84318f48940d34bded48cbae739c44 Gerrit-Change-Number: 29563 Gerrit-PatchSet: 43 Gerrit-Owner: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Rudolph Gerrit-CC: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Fri, 22 Feb 2019 13:03:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: [WIP]drivers/cavium: Add UART PCI driver
by Patrick Rudolph (Code Review) 22 Feb '19

22 Feb '19

Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/30608 Change subject: [WIP]drivers/cavium: Add UART PCI driver ...................................................................... [WIP]drivers/cavium: Add UART PCI driver Move PCI handling from cn81xx/soc to its own pci driver. Change-Id: I0fa2f086aba9b4f9c6dba7a35a84ea61c5fa64e4 Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> --- A src/drivers/cavium/pci-cn8xxx/Kconfig A src/drivers/cavium/pci-cn8xxx/Makefile.inc A src/drivers/cavium/pci-cn8xxx/uart.c M src/include/device/pci_ids.h M src/soc/cavium/cn81xx/soc.c 5 files changed, 57 insertions(+), 12 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/30608/1 diff --git a/src/drivers/cavium/pci-cn8xxx/Kconfig b/src/drivers/cavium/pci-cn8xxx/Kconfig new file mode 100644 index 0000000..1de774f --- /dev/null +++ b/src/drivers/cavium/pci-cn8xxx/Kconfig @@ -0,0 +1,7 @@ +config DRIVERS_CAVIUM + bool + depends on SOC_CAVIUM_COMMON + default y if SOC_CAVIUM_COMMON + default n + help + When enabled, adds PCI drivers for Cavium SoCs. diff --git a/src/drivers/cavium/pci-cn8xxx/Makefile.inc b/src/drivers/cavium/pci-cn8xxx/Makefile.inc new file mode 100644 index 0000000..d85fc3e --- /dev/null +++ b/src/drivers/cavium/pci-cn8xxx/Makefile.inc @@ -0,0 +1 @@ +ramstage-$(CONFIG_DRIVERS_CAVIUM) += uart.c diff --git a/src/drivers/cavium/pci-cn8xxx/uart.c b/src/drivers/cavium/pci-cn8xxx/uart.c new file mode 100644 index 0000000..2230d5d --- /dev/null +++ b/src/drivers/cavium/pci-cn8xxx/uart.c @@ -0,0 +1,46 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2019 Facebook, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#include <device/device.h> +#include <console/console.h> +#include <soc/uart.h> +#include <device/pci.h> +#include <device/pci_ids.h> + +static void cavium_uart_init(struct device *dev) +{ + printk(BIOS_ERR, "UART: debug\n"); + + /* using device enable state from devicetree.cb */ + if (dev->enabled) { + const u8 fn = PCI_FUNC(dev->path.pci.devfn); + + /* Calling uart_setup with no baudrate will do minimal HW + * enough for the kernel to not panic */ + if (!uart_is_enabled(fn)) + uart_setup(fn, 0); + } +} + +static struct device_operations device_ops = { + .init = cavium_uart_init, +}; + +static const struct pci_driver soc_cavium_uart __pci_driver = { + .ops = &device_ops, + .vendor = PCI_VENDOR_CAVIUM, + .device = PCI_DEVICE_ID_CAVIUM_THUNDERX_UART, +}; diff --git a/src/include/device/pci_ids.h b/src/include/device/pci_ids.h index 751cca0..ba55f1d 100644 --- a/src/include/device/pci_ids.h +++ b/src/include/device/pci_ids.h @@ -132,6 +132,9 @@ #define PCI_DEVICE_ID_BERKOM_A4T 0xffa4 #define PCI_DEVICE_ID_BERKOM_SCITEL_QUADRO 0xffa8 +#define PCI_VENDOR_CAVIUM 0x177d +#define PCI_DEVICE_ID_CAVIUM_THUNDERX_UART 0xa00f + #define PCI_VENDOR_ID_COMPAQ 0x0e11 #define PCI_DEVICE_ID_COMPAQ_TOKENRING 0x0508 #define PCI_DEVICE_ID_COMPAQ_1280 0x3033 diff --git a/src/soc/cavium/cn81xx/soc.c b/src/soc/cavium/cn81xx/soc.c index 2046d21..0748096 100644 --- a/src/soc/cavium/cn81xx/soc.c +++ b/src/soc/cavium/cn81xx/soc.c @@ -388,18 +388,6 @@ } } - /* Init UARTs */ - size_t i; - struct device *uart_dev; - for (i = 0; i <= 3; i++) { - uart_dev = dev_find_slot(1, PCI_DEVFN(8, i)); - /* using device enable state from devicetree.cb */ - if (uart_dev && uart_dev->enabled) { - if (!uart_is_enabled(i)) - uart_setup(i, 0); - } - } - if (IS_ENABLED(CONFIG_ARM64_USE_ARM_TRUSTED_FIRMWARE)) soc_init_atf(); } -- To view, visit https://review.coreboot.org/c/coreboot/+/30608 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0fa2f086aba9b4f9c6dba7a35a84ea61c5fa64e4 Gerrit-Change-Number: 30608 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: newchange

3 8

Change in ...coreboot[master]: security/vboot: Add measured boot mode
by Werner Zeh (Code Review) 22 Feb '19

22 Feb '19

Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29547 ) Change subject: security/vboot: Add measured boot mode ...................................................................... Patch Set 70: (2 comments) > Patch Set 70: > > (2 comments) > > LGTM from a code perspective now, but the whitespace and line length issues still need to be fixed. If you want to fixup whitespace that's fine, but please do it in a separate CL. Do not mix feature changes with so many unrelated cleanups. The problem Philipp faces here is the decision of using clang-format. We made this decision some time ago and he is just following. With switching to clang-format you will one day hit exactly this case: One touches some code and have clang-format active. Now clang-format steps in and touches some formatting. One can either separate this now into a "code-only" change and a "this is what clang-format has done" change but is it something we really want? What we see here should be visible more and more in the future and is just the result of the decision enabling clang-format. Back to the patch itself: We have tested the latest patch set (70) on mc_bdx1 and mc_apl5 and it behaves as intended. I vote for bring this patch in. We all know that this will not be the final state of this experimental feature, it will be improved. And this can and will be done in follow-up patches. Having this patch for further several month in this state does only increase the effort of maintain it. https://review.coreboot.org/#/c/29547/70/src/lib/prog_loaders.c File src/lib/prog_loaders.c: https://review.coreboot.org/#/c/29547/70/src/lib/prog_loaders.c@77 PS70, Line 77: Is this really what clang-format gives you? Looks weird as there is the same space left in the upper line. https://review.coreboot.org/#/c/29547/70/src/security/vboot/vboot_crtm.c File src/security/vboot/vboot_crtm.c: https://review.coreboot.org/#/c/29547/70/src/security/vboot/vboot_crtm.c@135 PS70, Line 135: pcr_index = TPM_CRTM_PCR; > Actually... […] I think the intention was to provide a way to separate execution code (stages, payload, fsp, microcode_blob, ...) and "other" data measurements into different PCRs. For example, Siemens boards use a dataset in cbfs which contains logistic data of the mainboard. One can think of a case where a part of this dataset needs to be updated in the field. If we would have this dataset measured into PCR2, then this update would lead to an inconsistent TPM state or would require pre-attestation in order to have a trustworthy state after the update. While this would make sense for code updates it does not make sense for this mentioned dataset. -- To view, visit https://review.coreboot.org/c/coreboot/+/29547 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I339a2f1051e44f36aba9f99828f130592a09355e Gerrit-Change-Number: 29547 Gerrit-PatchSet: 70 Gerrit-Owner: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Huang Jin <huang.jin(a)intel.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: York Yang <york.yang(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 22 Feb 2019 12:09:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: security/tpm: Fix TCPA log feature
by build bot (Jenkins) (Code Review) 22 Feb '19

22 Feb '19

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29563 ) Change subject: security/tpm: Fix TCPA log feature ...................................................................... Patch Set 43: (10 comments) https://review.coreboot.org/#/c/29563/43/src/include/memlayout.h File src/include/memlayout.h: https://review.coreboot.org/#/c/29563/43/src/include/memlayout.h@167 PS43, Line 167: #define VBOOT2_TPM_LOG(addr, size) \ Macros with multiple statements should be enclosed in a do - while loop https://review.coreboot.org/#/c/29563/43/src/include/memlayout.h@167 PS43, Line 167: #define VBOOT2_TPM_LOG(addr, size) \ macros should not use a trailing semicolon https://review.coreboot.org/#/c/29563/43/src/security/tpm/tspi.h File src/security/tpm/tspi.h: https://review.coreboot.org/#/c/29563/43/src/security/tpm/tspi.h@50 PS43, Line 50: uint32_t tpm_extend_pcr(int pcr, const char *digest_type, uint8_t *digest, size_t digest_len, line over 80 characters https://review.coreboot.org/#/c/29563/43/src/security/tpm/tspi/log.c File src/security/tpm/tspi/log.c: https://review.coreboot.org/#/c/29563/43/src/security/tpm/tspi/log.c@37 PS43, Line 37: size_t tcpa_log_len = sizeof(struct tcpa_table) + MAX_TCPA_LOG_ENTRIES * sizeof(struct tcpa_entry); line over 80 characters https://review.coreboot.org/#/c/29563/43/src/security/tpm/tspi/log.c@44 PS43, Line 44: strncpy((char *)tclt->header, TCPA_LOG_HEADER, sizeof(tclt->header)); line over 80 characters https://review.coreboot.org/#/c/29563/43/src/security/tpm/tspi/log.c@50 PS43, Line 50: if (!cbmem_possibly_online() && IS_ENABLED(CONFIG_VBOOT_TCPA_LOG_RECOVERY)) { line over 80 characters https://review.coreboot.org/#/c/29563/43/src/security/tpm/tspi/log.c@53 PS43, Line 53: if (strncmp(TCPA_LOG_HEADER, (const char *)tclt->header, sizeof(tclt->header))) { line over 80 characters https://review.coreboot.org/#/c/29563/43/src/security/tpm/tspi/log.c@56 PS43, Line 56: strncpy((char *)tclt->header, TCPA_LOG_HEADER, sizeof(tclt->header)); line over 80 characters https://review.coreboot.org/#/c/29563/43/src/security/tpm/tspi/log.c@91 PS43, Line 91: void tcpa_log_add_table_entry(const char *name, const uint32_t pcr, const char *digest_type, line over 80 characters https://review.coreboot.org/#/c/29563/43/src/security/vboot/secdata_tpm.c File src/security/vboot/secdata_tpm.c: https://review.coreboot.org/#/c/29563/43/src/security/vboot/secdata_tpm.c@86 PS43, Line 86: return tpm_extend_pcr(pcr, "SHA256", buffer, size, TPM_PCR_GBB_HWID_NAME); line over 80 characters -- To view, visit https://review.coreboot.org/c/coreboot/+/29563 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic93133531b84318f48940d34bded48cbae739c44 Gerrit-Change-Number: 29563 Gerrit-PatchSet: 43 Gerrit-Owner: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Rudolph Gerrit-CC: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Fri, 22 Feb 2019 11:59:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: security/tpm: Fix TCPA log feature
by Philipp Deppenwiese (Code Review) 22 Feb '19

22 Feb '19

Hello Werner Zeh, Aaron Durbin, Julius Werner, Patrick Rudolph, Paul Menzel, David Hendricks, build bot (Jenkins), Patrick Georgi, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/29563 to look at the new patch set (#43). Change subject: security/tpm: Fix TCPA log feature ...................................................................... security/tpm: Fix TCPA log feature Until now the TCPA log wasn't working correctly. * Refactor TCPA log code. * Add TCPA log dump fucntion. * Make TCPA log available in bootblock. * Fix TCPA log formatting. * Add x86 and Cavium memory for early log. 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/tpm/tspi/tspi.c M src/security/vboot/Kconfig M src/security/vboot/secdata_tpm.c M src/security/vboot/symbols.h M src/soc/cavium/cn81xx/include/soc/memlayout.ld M src/soc/imgtec/pistachio/include/soc/memlayout.ld M src/soc/mediatek/mt8173/include/soc/memlayout.ld M src/soc/mediatek/mt8183/include/soc/memlayout.ld M src/soc/nvidia/tegra124/include/soc/memlayout.ld M src/soc/nvidia/tegra210/include/soc/memlayout.ld M src/soc/samsung/exynos5250/include/soc/memlayout.ld M util/cbmem/cbmem.c 17 files changed, 175 insertions(+), 65 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/29563/43 -- To view, visit https://review.coreboot.org/c/coreboot/+/29563 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic93133531b84318f48940d34bded48cbae739c44 Gerrit-Change-Number: 29563 Gerrit-PatchSet: 43 Gerrit-Owner: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Rudolph Gerrit-CC: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newpatchset

1 0

Change in ...coreboot[master]: security/tpm: Fix TCPA log feature
by build bot (Jenkins) (Code Review) 22 Feb '19

22 Feb '19

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29563 ) Change subject: security/tpm: Fix TCPA log feature ...................................................................... Patch Set 42: (10 comments) https://review.coreboot.org/#/c/29563/42/src/include/memlayout.h File src/include/memlayout.h: https://review.coreboot.org/#/c/29563/42/src/include/memlayout.h@167 PS42, Line 167: #define VBOOT2_TPM_LOG(addr, size) \ Macros with multiple statements should be enclosed in a do - while loop https://review.coreboot.org/#/c/29563/42/src/include/memlayout.h@167 PS42, Line 167: #define VBOOT2_TPM_LOG(addr, size) \ macros should not use a trailing semicolon https://review.coreboot.org/#/c/29563/42/src/security/tpm/tspi.h File src/security/tpm/tspi.h: https://review.coreboot.org/#/c/29563/42/src/security/tpm/tspi.h@50 PS42, Line 50: uint32_t tpm_extend_pcr(int pcr, const char *digest_type, uint8_t *digest, size_t digest_len, line over 80 characters https://review.coreboot.org/#/c/29563/42/src/security/tpm/tspi/log.c File src/security/tpm/tspi/log.c: https://review.coreboot.org/#/c/29563/42/src/security/tpm/tspi/log.c@37 PS42, Line 37: size_t tcpa_log_len = sizeof(struct tcpa_table) + MAX_TCPA_LOG_ENTRIES * sizeof(struct tcpa_entry); line over 80 characters https://review.coreboot.org/#/c/29563/42/src/security/tpm/tspi/log.c@44 PS42, Line 44: strncpy((char *)tclt->header, TCPA_LOG_HEADER, sizeof(tclt->header)); line over 80 characters https://review.coreboot.org/#/c/29563/42/src/security/tpm/tspi/log.c@50 PS42, Line 50: if (!cbmem_possibly_online() && IS_ENABLED(CONFIG_VBOOT_TCPA_LOG_RECOVERY)) { line over 80 characters https://review.coreboot.org/#/c/29563/42/src/security/tpm/tspi/log.c@53 PS42, Line 53: if (strncmp(TCPA_LOG_HEADER, (const char *)tclt->header, sizeof(tclt->header))) { line over 80 characters https://review.coreboot.org/#/c/29563/42/src/security/tpm/tspi/log.c@56 PS42, Line 56: strncpy((char *)tclt->header, TCPA_LOG_HEADER, sizeof(tclt->header)); line over 80 characters https://review.coreboot.org/#/c/29563/42/src/security/tpm/tspi/log.c@91 PS42, Line 91: void tcpa_log_add_table_entry(const char *name, const uint32_t pcr, const char *digest_type, line over 80 characters https://review.coreboot.org/#/c/29563/42/src/security/vboot/secdata_tpm.c File src/security/vboot/secdata_tpm.c: https://review.coreboot.org/#/c/29563/42/src/security/vboot/secdata_tpm.c@86 PS42, Line 86: return tpm_extend_pcr(pcr, "SHA256", buffer, size, TPM_PCR_GBB_HWID_NAME); line over 80 characters -- To view, visit https://review.coreboot.org/c/coreboot/+/29563 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic93133531b84318f48940d34bded48cbae739c44 Gerrit-Change-Number: 29563 Gerrit-PatchSet: 42 Gerrit-Owner: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Rudolph Gerrit-CC: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Fri, 22 Feb 2019 11:47:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit February 2019