coreboot-gerrit February 2019

coreboot-gerrit@coreboot.org

1 participants
1249 discussions

Change in ...coreboot[master]: drivers/intel/fsp2_0: Update dependency of USE_FSP_REPO
by Aamir Bohra (Code Review) 22 Feb '19

22 Feb '19

Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31530 ) Change subject: drivers/intel/fsp2_0: Update dependency of USE_FSP_REPO ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/31530 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5e5d5a9fdf3f5d3e79922e97719e8491aa514cef Gerrit-Change-Number: 31530 Gerrit-PatchSet: 2 Gerrit-Owner: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Comment-Date: Fri, 22 Feb 2019 05:39:58 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: vendorcode/intel/fsp/fsp2_0/cml: Add FSP header files for Cometlake
by Aamir Bohra (Code Review) 22 Feb '19

22 Feb '19

Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31529 ) Change subject: vendorcode/intel/fsp/fsp2_0/cml: Add FSP header files for Cometlake ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/31529 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I734316445dda5b1feb4098ce3c58b6dd8ce2d272 Gerrit-Change-Number: 31529 Gerrit-PatchSet: 2 Gerrit-Owner: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Comment-Date: Fri, 22 Feb 2019 05:38:43 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: libpayload: Fix CONFIG_LP_DEBUG_MALLOC for 64-bit archs
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/+/14921 ) Change subject: libpayload: Fix CONFIG_LP_DEBUG_MALLOC for 64-bit archs ...................................................................... Patch Set 3: (25 comments) https://review.coreboot.org/#/c/14921/3/payloads/libpayload/include/stdlib.h File payloads/libpayload/include/stdlib.h: https://review.coreboot.org/#/c/14921/3/payloads/libpayload/include/stdlib.… PS3, Line 51: void free(void *); function definition argument 'void *' should also have an identifier name https://review.coreboot.org/#/c/14921/3/payloads/libpayload/include/stdlib.… PS3, Line 52: void *malloc(size_t); function definition argument 'size_t' should also have an identifier name https://review.coreboot.org/#/c/14921/3/payloads/libpayload/include/stdlib.… PS3, Line 53: void *calloc(size_t,size_t); space required after that ',' (ctx:VxV) https://review.coreboot.org/#/c/14921/3/payloads/libpayload/include/stdlib.… PS3, Line 53: void *calloc(size_t,size_t); function definition argument 'size_t' should also have an identifier name https://review.coreboot.org/#/c/14921/3/payloads/libpayload/include/stdlib.… PS3, Line 53: void *calloc(size_t,size_t); function definition argument 'size_t' should also have an identifier name https://review.coreboot.org/#/c/14921/3/payloads/libpayload/include/stdlib.… PS3, Line 54: void *realloc(void*,size_t); space required after that ',' (ctx:OxV) https://review.coreboot.org/#/c/14921/3/payloads/libpayload/include/stdlib.… PS3, Line 54: void *realloc(void*,size_t); function definition argument 'void*' should also have an identifier name https://review.coreboot.org/#/c/14921/3/payloads/libpayload/include/stdlib.… PS3, Line 54: void *realloc(void*,size_t); function definition argument 'size_t' should also have an identifier name https://review.coreboot.org/#/c/14921/3/payloads/libpayload/include/stdlib.… PS3, Line 55: void *memalign(size_t, size_t); function definition argument 'size_t' should also have an identifier name https://review.coreboot.org/#/c/14921/3/payloads/libpayload/include/stdlib.… PS3, Line 55: void *memalign(size_t, size_t); function definition argument 'size_t' should also have an identifier name https://review.coreboot.org/#/c/14921/3/payloads/libpayload/include/stdlib.… PS3, Line 56: void *dma_malloc(size_t); function definition argument 'size_t' should also have an identifier name https://review.coreboot.org/#/c/14921/3/payloads/libpayload/include/stdlib.… PS3, Line 57: void *dma_memalign(size_t, size_t); function definition argument 'size_t' should also have an identifier name https://review.coreboot.org/#/c/14921/3/payloads/libpayload/include/stdlib.… PS3, Line 57: void *dma_memalign(size_t, size_t); function definition argument 'size_t' should also have an identifier name https://review.coreboot.org/#/c/14921/3/payloads/libpayload/include/stdlib.… PS3, Line 60: void * __p = p; \ Statements should start on a tabstop https://review.coreboot.org/#/c/14921/3/payloads/libpayload/include/stdlib.… PS3, Line 60: void * __p = p; \ "foo * bar" should be "foo *bar" https://review.coreboot.org/#/c/14921/3/payloads/libpayload/include/stdlib.… PS3, Line 71: size_t __s = s; \ Statements should start on a tabstop https://review.coreboot.org/#/c/14921/3/payloads/libpayload/include/stdlib.… PS3, Line 84: size_t __n = n, __s = s; \ Statements should start on a tabstop https://review.coreboot.org/#/c/14921/3/payloads/libpayload/include/stdlib.… PS3, Line 97: void *__p = p; \ Statements should start on a tabstop https://review.coreboot.org/#/c/14921/3/payloads/libpayload/include/stdlib.… PS3, Line 98: size_t __s = s; \ Statements should start on a tabstop https://review.coreboot.org/#/c/14921/3/payloads/libpayload/include/stdlib.… PS3, Line 111: size_t __a = a, __s = s; \ Statements should start on a tabstop https://review.coreboot.org/#/c/14921/3/payloads/libpayload/include/stdlib.… PS3, Line 114: __FILE__, __func__, __LINE__);\ code indent should use tabs where possible https://review.coreboot.org/#/c/14921/3/payloads/libpayload/include/stdlib.… PS3, Line 114: __FILE__, __func__, __LINE__);\ please, no space before tabs https://review.coreboot.org/#/c/14921/3/payloads/libpayload/include/stdlib.… PS3, Line 124: size_t __s = s; \ Statements should start on a tabstop https://review.coreboot.org/#/c/14921/3/payloads/libpayload/include/stdlib.… PS3, Line 137: size_t __a = a, __s = s; \ Statements should start on a tabstop https://review.coreboot.org/#/c/14921/3/payloads/libpayload/libc/malloc.c File payloads/libpayload/libc/malloc.c: https://review.coreboot.org/#/c/14921/3/payloads/libpayload/libc/malloc.c@4… PS3, Line 483: printf(" found memalign region. %u free, %zu required\n", reg->free, (size + align - 1)/align); line over 80 characters -- To view, visit https://review.coreboot.org/c/coreboot/+/14921 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib54ebc3cfba99f372690365b78c7ceb372c0bd45 Gerrit-Change-Number: 14921 Gerrit-PatchSet: 3 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philippe Mathieu-Daudé <f4bug(a)amsat.org> Gerrit-Reviewer: Uwe Hermann <uwe(a)hermann-uwe.de> Gerrit-Reviewer: You-Cheng Syu <youcheng(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 22 Feb 2019 05:31:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: vendorcode/intel/fsp/fsp2_0/cml: Add FSP header files for Cometlake
by Subrata Banik (Code Review) 22 Feb '19

22 Feb '19

Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31529 ) Change subject: vendorcode/intel/fsp/fsp2_0/cml: Add FSP header files for Cometlake ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/31529 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I734316445dda5b1feb4098ce3c58b6dd8ce2d272 Gerrit-Change-Number: 31529 Gerrit-PatchSet: 2 Gerrit-Owner: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Comment-Date: Fri, 22 Feb 2019 04:35:03 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: mb/google/poppy/variants/atlas: move WiFi wake to GPP_B7
by Furquan Shaikh (Code Review) 22 Feb '19

22 Feb '19

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31211 ) Change subject: mb/google/poppy/variants/atlas: move WiFi wake to GPP_B7 ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/31211 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1816500dd0ab6186fd51aa6945faf73d00c152fe Gerrit-Change-Number: 31211 Gerrit-PatchSet: 2 Gerrit-Owner: Caveh Jalali <caveh(a)google.com> Gerrit-Reviewer: Caveh Jalali <caveh(a)google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: caveh jalali <caveh(a)chromium.org> Gerrit-Comment-Date: Fri, 22 Feb 2019 03:55:58 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: mb/google/octopus: Check for invalid pin termination settings
by Furquan Shaikh (Code Review) 22 Feb '19

22 Feb '19

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29110 ) Change subject: mb/google/octopus: Check for invalid pin termination settings ...................................................................... Patch Set 3: (4 comments) https://review.coreboot.org/#/c/29110/3//COMMIT_MSG Commit Message: https://review.coreboot.org/#/c/29110/3//COMMIT_MSG@7 PS3, Line 7: mb/google/octopus This is not correct. https://review.coreboot.org/#/c/29110/3/src/soc/intel/common/block/include/… File src/soc/intel/common/block/include/intelblocks/gpio_defs.h: https://review.coreboot.org/#/c/29110/3/src/soc/intel/common/block/include/… PS3, Line 140: : /* Active (S0) and Standby state terminations should not conflict. When : * a Pull Up is engaged in S0, a Pull down shouldn't be used in Standby. : * Similarly, when a Pull Down is engaged in S0, a Pull Up shouldn't be : * used in Standby. : */ : #define PAD_CFG_CHECK_ENPD(value) \ : !(((((value) & PAD_CFG1_IOSTERM_MASK) == PAD_CFG1_IOSTERM_ENPD) \ : && (((value) & PAD_CFG1_PULL_MASK) > PAD_CFG1_PULL_DN_20K)) || \ : ((((value) & PAD_CFG1_IOSTERM_MASK) == PAD_CFG1_IOSTERM_ENPD) && \ : (((value) & PAD_CFG1_PULL_MASK) == PAD_CFG1_PULL_NONE))) : : #define PAD_CFG_CHECK_ENPU(value) \ : !(((((value) & PAD_CFG1_IOSTERM_MASK) == PAD_CFG1_IOSTERM_ENPU) \ : && (((value) & PAD_CFG1_PULL_MASK) < PAD_CFG1_PULL_UP_1K)) || \ : ((((value) & PAD_CFG1_IOSTERM_MASK) == PAD_CFG1_IOSTERM_ENPU) && \ : (((value) & PAD_CFG1_PULL_MASK) == PAD_CFG1_PULL_NATIVE))) : : #if IS_ENABLED(CONFIG_SOC_INTEL_GLK) : /* For dual-voltage pins, only allow 20K PU or None termination setting */ : #define PAD_CFG_CHECK_VCCIO(pad, value) \ : !(((((pad) >= GPIO_98) && ((pad) <= GPIO_155)) \ : || (((pad) >= GPIO_176) && ((pad) <= GPIO_178))) && \ : ((((value) & PAD_CFG1_PULL_MASK) != PAD_CFG1_PULL_NONE) && \ : (((value) & PAD_CFG1_PULL_MASK) != PAD_CFG1_PULL_UP_20K))) : #else : #define PAD_CFG_CHECK_VCCIO(pad, value) 1 : #endif I think a better way to handle this would be to put all the comparison logic within some gpio*.h file in specific SoC(that needs it) and have a config that says this SoC needs dw1 pad_config valid check and then PAD_CFG_IS_VALID(...) would default to __config1 in this file if that config is not set, otherwise the SoC definition will take effect. https://review.coreboot.org/#/c/29110/3/src/soc/intel/common/block/include/… PS3, Line 170: 572688 I don't think this doc applies to all Intel SoCs. Can you instead add meaningful comment indicating what the constraints are? Also, do these constraints apply to all SoCs? If not, then it would be better to just guard this appropriately. https://review.coreboot.org/#/c/29110/3/src/soc/intel/common/block/include/… PS3, Line 183: 0xFFFFFFFFFF Can we make use of Static_assert in _PAD_CFG_IS_VALID instead of assigning this value to make the error messages more relevant? -- To view, visit https://review.coreboot.org/c/coreboot/+/29110 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I64317e75f27e4cc2faf88b640b1bf9401db59dec Gerrit-Change-Number: 29110 Gerrit-PatchSet: 3 Gerrit-Owner: Shamile Khan <shamile.khan(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Reviewer: John Zhao <john.zhao(a)intel.com> Gerrit-Reviewer: Justin TerAvest <teravest(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Shamile Khan <shamile.khan(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Fri, 22 Feb 2019 03:25:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: Revert "src/drivers/intel/wifi: Add a W/A for Intel ThP2 9260"
by Duncan Laurie (Code Review) 22 Feb '19

22 Feb '19

Hello Patrick Rudolph, Aaron Durbin, Subrata Banik, Balaji Manigandan, Caveh Jalali, Rizwan Qureshi, Gaggery Tsai, build bot (Jenkins), Furquan Shaikh, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/31559 to review the following change. Change subject: Revert "src/drivers/intel/wifi: Add a W/A for Intel ThP2 9260" ...................................................................... Revert "src/drivers/intel/wifi: Add a W/A for Intel ThP2 9260" This reverts commit 3afb84a24583f5dee9fb407f11b32253d59392bf. Reason for revert: This is causing issues with the PCIe link and the system is unable to enter S0ix. Until it can be fixed in coreboot revert the change here that is not working properly. BUG=b:124264120 Change-Id: Ia20da9ab560ca35950b4a916667f51e0f541b382 Signed-off-by: Duncan Laurie <dlaurie(a)google.com> --- M src/drivers/intel/wifi/wifi.c 1 file changed, 0 insertions(+), 25 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/31559/1 diff --git a/src/drivers/intel/wifi/wifi.c b/src/drivers/intel/wifi/wifi.c index 01a338d..e197114 100644 --- a/src/drivers/intel/wifi/wifi.c +++ b/src/drivers/intel/wifi/wifi.c @@ -255,34 +255,9 @@ } #endif -static void pci_dev_apply_quirks(struct device *dev) -{ - unsigned int cap; - uint16_t val; - struct device *root = dev->bus->dev; - - switch (dev->device) { - case PCI_DEVICE_ID_TP_9260_SERIES_WIFI: - cap = pci_find_capability(root, PCI_CAP_ID_PCIE); - /* Check the LTR for root port and enable it */ - if (cap) { - val = pci_read_config16(root, cap + - PCI_EXP_DEV_CAP2_OFFSET); - if (val & LTR_MECHANISM_SUPPORT) { - val = pci_read_config16(root, cap + - PCI_EXP_DEV_CTL_STS2_CAP_OFFSET); - val |= LTR_MECHANISM_EN; - pci_write_config16(root, cap + - PCI_EXP_DEV_CTL_STS2_CAP_OFFSET, val); - } - } - } -} - static void wifi_pci_dev_init(struct device *dev) { pci_dev_init(dev); - pci_dev_apply_quirks(dev); if (IS_ENABLED(CONFIG_ELOG)) { uint32_t val; -- To view, visit https://review.coreboot.org/c/coreboot/+/31559 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia20da9ab560ca35950b4a916667f51e0f541b382 Gerrit-Change-Number: 31559 Gerrit-PatchSet: 1 Gerrit-Owner: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-Reviewer: Caveh Jalali <caveh(a)google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Gaggery Tsai <gaggery.tsai(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newchange

5 6

Change in ...coreboot[master]: hatch: Make EC software sync enabled by default
by Scott Collyer (Code Review) 22 Feb '19

22 Feb '19

Scott Collyer has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31537 Change subject: hatch: Make EC software sync enabled by default ...................................................................... hatch: Make EC software sync enabled by default EC software sync had been disabled because BIOS was not bundling a useful EC image. This is no longer required. This CL removes that change so EC software sync is enabled by default. BUG=b:124208414 BRANCH=None TEST=Tested with a system that have a different RW image and verified that this image was overwritten to the one bundled in the BIOS and that the EC was running its RW image. Signed-off-by: Scott Collyer <scollyer(a)chromium.org> Change-Id: Ic1ffdb62e9fa2cacb3296cb3807082f23e171ab5 --- M src/mainboard/google/hatch/Kconfig 1 file changed, 0 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/31537/1 diff --git a/src/mainboard/google/hatch/Kconfig b/src/mainboard/google/hatch/Kconfig index 65a3562..b97977c 100644 --- a/src/mainboard/google/hatch/Kconfig +++ b/src/mainboard/google/hatch/Kconfig @@ -27,7 +27,6 @@ select GBB_FLAG_FORCE_DEV_BOOT_USB select GBB_FLAG_FORCE_DEV_BOOT_LEGACY select GBB_FLAG_FORCE_MANUAL_RECOVERY - select GBB_FLAG_DISABLE_EC_SOFTWARE_SYNC select HAS_RECOVERY_MRC_CACHE select MRC_CLEAR_NORMAL_CACHE_ON_RECOVERY_RETRAIN select VBOOT_LID_SWITCH -- To view, visit https://review.coreboot.org/c/coreboot/+/31537 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic1ffdb62e9fa2cacb3296cb3807082f23e171ab5 Gerrit-Change-Number: 31537 Gerrit-PatchSet: 1 Gerrit-Owner: Scott Collyer <scollyer(a)chromium.org> Gerrit-MessageType: newchange

2 3

Change in ...coreboot[master]: src/soc/intel/common/block/pcie: Add a workaround for ThP2 9260
by Gaggery Tsai (Code Review) 22 Feb '19

22 Feb '19

Gaggery Tsai has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/30486 Change subject: src/soc/intel/common/block/pcie: Add a workaround for ThP2 9260 ...................................................................... src/soc/intel/common/block/pcie: Add a workaround for ThP2 9260 This patch adds a workaround for ThP2. The PCIe root port LCTL2.TLS is by default GEN1 and ThP has bad synchronization on polarity inversion. When the root port request for speed change, ThP doesn’t confirm the request, and both sides are moving to polling after timeout, hot reset is issued, and then most of the CFG space is initialized. From the observation, CCC/ECPM/LTR would be reset to default but CCC/ECPM of root port and end devices have been reconfigured in pci_scan. The LTR configuration for root port is still missing. BUG=B:117618636 BRANCH=None TEST=Add THP2_9260_WORKAROUND in Atlas configuration & emerge-atlas coreboot chromeos-bootimage & Warm/cold reset for 10 times and didn't see unsupported request related AER error messages & $lspci -vvs 00:1c.0|grep LTR and ensure LTR+ is present. Change-Id: Id5d2814488fbc9db927edb2ead972b73ebc336ce Signed-off-by: Gaggery Tsai <gaggery.tsai(a)intel.com> --- M src/soc/intel/common/block/pcie/Kconfig M src/soc/intel/common/block/pcie/pcie.c 2 files changed, 29 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/30486/1 diff --git a/src/soc/intel/common/block/pcie/Kconfig b/src/soc/intel/common/block/pcie/Kconfig index aa32324..36915fa 100644 --- a/src/soc/intel/common/block/pcie/Kconfig +++ b/src/soc/intel/common/block/pcie/Kconfig @@ -13,3 +13,8 @@ Enable debug logs in PCIe module. Allows debug information on memory base and limit, prefetchable memory base and limit, prefetchable memory base upper 32 bits and prefetchable memory limit upper 32 bits. + +config THP2_9260_WORKAROUND + bool + help + THP2 workaround diff --git a/src/soc/intel/common/block/pcie/pcie.c b/src/soc/intel/common/block/pcie/pcie.c index 3ebb4f6..07a2321 100644 --- a/src/soc/intel/common/block/pcie/pcie.c +++ b/src/soc/intel/common/block/pcie/pcie.c @@ -28,12 +28,36 @@ /* PCI-E Sub-System ID */ #define PCIE_SUBSYSTEM_VENDOR_ID 0x94 +/* + * Check the LTR for root port and enable it + */ +static void pciexp_enable_root_port_ltr(struct device *root, unsigned root_cap) +{ + u16 root_ltr; + unsigned int val; + + val = pci_read_config16(root, root_cap + PCI_EXP_DEV_CAP2_OFFSET); + + if (val & LTR_MECHANISM_SUPPORT) { + root_ltr = pci_read_config16(root, root_cap + PCI_EXP_DEV_CTL_STS2_CAP_OFFSET); + root_ltr |= LTR_MECHANISM_EN; + pci_write_config16(root, root_cap + PCI_EXP_DEV_CTL_STS2_CAP_OFFSET, root_ltr); + } +} + static void pch_pcie_init(struct device *dev) { u16 reg16; + unsigned int dev_cap; printk(BIOS_DEBUG, "Initializing PCH PCIe bridge.\n"); + if (IS_ENABLED(CONFIG_THP2_9260_WORKAROUND)) { + dev_cap = pci_find_capability(dev, PCI_CAP_ID_PCIE); + if (dev_cap) + pciexp_enable_root_port_ltr(dev, dev_cap); + } + /* Enable SERR */ pci_or_config32(dev, PCI_COMMAND, PCI_COMMAND_SERR); -- To view, visit https://review.coreboot.org/c/coreboot/+/30486 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id5d2814488fbc9db927edb2ead972b73ebc336ce Gerrit-Change-Number: 30486 Gerrit-PatchSet: 1 Gerrit-Owner: Gaggery Tsai <gaggery.tsai(a)intel.com> Gerrit-MessageType: newchange

2 3

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

21 Feb '19

Julius Werner 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) 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. https://review.coreboot.org/#/c/29547/70/Documentation/security/vboot/measu… File Documentation/security/vboot/measured_boot.md: https://review.coreboot.org/#/c/29547/70/Documentation/security/vboot/measu… PS70, Line 15: will be added later to the implementation. nit: I mean, it *is* possible if the code loading those FMAP partitions just calls the measuring function manually. There's just no automatic integration. But I'm not sure if it would ever make sense to add that because not all our FMAP partitions are really used in a way where that would make sense. 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... is this what you want? I thought the point of the whitelist was to say "this is the runtime data I want to measure (and other runtime data just shouldn't be measured at all)". But with this code, you're throwing the non-whitelisted runtime data into the code PCR instead, which seems wrong? -- 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: Thu, 21 Feb 2019 23:56:04 +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