Krishna P Bhat D has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32111
Change subject: mb/google/hatch: Re-configure GPP_A12 as GPO before entering sleep
......................................................................
mb/google/hatch: Re-configure GPP_A12 as GPO before entering sleep
GPP_A12 has a Native3 (SX_EXIT_HOLDOFF#) mode, which allows to delay
resuming to S0. If this pad is not locked and platform was not initially
designed for this functionality, malware could reconfigure this pads
setting under OS (switch to Native3), which would make platform not able
to resume until G3 is applied. To prevent misuse of this pad,
re-configure this pad before entering S3 and S5 to guarantee that the
pad configuration is correct.
BUG=b:128686027
Change-Id: I1e7979baa491acf2c56d223afb4618f0f6429e37
Signed-off-by: Krishna Prasad Bhat <krishna.p.bhat.d(a)intel.com>
---
M src/mainboard/google/hatch/variants/baseboard/gpio.c
1 file changed, 6 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/32111/1
diff --git a/src/mainboard/google/hatch/variants/baseboard/gpio.c b/src/mainboard/google/hatch/variants/baseboard/gpio.c
index b974e49..8529d5916 100644
--- a/src/mainboard/google/hatch/variants/baseboard/gpio.c
+++ b/src/mainboard/google/hatch/variants/baseboard/gpio.c
@@ -411,8 +411,12 @@
return gpio_table;
}
-/* Default GPIO settings before entering sleep. */
+/*
+ * Default GPIO settings before entering sleep. Configure A12: FPMCU_RST_ODL
+ * as GPO before entering sleep.
+ */
static const struct pad_config default_sleep_gpio_table[] = {
+ PAD_CFG_GPO(GPP_A12, 1, DEEP), /* FPMCU_RST_ODL */
};
/*
@@ -421,6 +425,7 @@
* turn off EN_PP3300_WWAN.
*/
static const struct pad_config s5_sleep_gpio_table[] = {
+ PAD_CFG_GPO(GPP_A12, 1, DEEP), /* FPMCU_RST_ODL */
PAD_CFG_GPO(GPP_A18, 0, DEEP), /* EN_PP3300_WWAN */
};
--
To view, visit https://review.coreboot.org/c/coreboot/+/32111
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1e7979baa491acf2c56d223afb4618f0f6429e37
Gerrit-Change-Number: 32111
Gerrit-PatchSet: 1
Gerrit-Owner: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com>
Gerrit-MessageType: newchange
Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31385
Change subject: util/spdtool: Add tool to extract SPD from BLOBs
......................................................................
util/spdtool: Add tool to extract SPD from BLOBs
Opens a binary file to extract DDR SPDs using known bits.
At the moment only DDR4 SPDs are supported.
Dumps the found SPDs into the current folder, as either
binary or hex encoded file.
Works with python2 and python3.
Change-Id: I26dd73d43b724ea6891bb5b6e96856c42db8577c
Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com>
---
A util/spdtool/description.md
A util/spdtool/spdtool.py
2 files changed, 221 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/31385/1
diff --git a/util/spdtool/description.md b/util/spdtool/description.md
new file mode 100644
index 0000000..39d4a5c
--- /dev/null
+++ b/util/spdtool/description.md
@@ -0,0 +1,3 @@
+Dumps SPD ROMs from a given blob to seperate files using known patterns
+and reserved bits. Useful for analysing firmware that holds SPDs on boards
+that have soldered down DRAM. `python`
diff --git a/util/spdtool/spdtool.py b/util/spdtool/spdtool.py
new file mode 100644
index 0000000..c20ee57
--- /dev/null
+++ b/util/spdtool/spdtool.py
@@ -0,0 +1,218 @@
+#!/usr/bin/env python
+
+# spdtool - Tool for partial deblobbing of Intel ME/TXE firmware images
+# Copyright (C) 2019 9elements Agency GmbH <patrick.rudolph(a)9elements.com>
+#
+# 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; either version 3 of the License, or
+# (at your option) any later version.
+#
+# 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.
+#
+#
+# Parse a BLOB and search for SPD files.
+# First it is searched for a possible SPD header.
+#
+# For each candidate the function verify_match is invoked to check
+# additional fields (known bits, reserved bits, CRC, ...)
+#
+# Dumps the found SPDs into the current folder.
+#
+# Implemented:
+# DDR4 SPDs
+#
+
+import argparse
+import crc16
+import struct
+
+class Parser(object):
+ def __init__(self, blob, verbose=False, ignorecrc=False):
+ self.blob = blob
+ self.ignorecrc = ignorecrc
+ self.verbose = verbose
+ def get_matches(self):
+ """Return the first byte to look for"""
+ raise Exception("Function not implemented")
+ def verify_match(self, header, offset):
+ """Return true if it looks like a SPD"""
+ raise Exception("Function not implemented")
+ def get_len(self, header, offset):
+ """Return the length of the SPD"""
+ raise Exception("Function not implemented")
+ def get_part_number(self, offset):
+ """Return the part number in SPD"""
+ return ""
+ def get_manufacturer_id(self, offset):
+ """Return the manufacturer ID in SPD"""
+ return 0xffff
+ def get_mtransfers(self, offset):
+ """Return the number of MT/s"""
+ return 0
+
+ def get_manufacturer(self, offset):
+ """Return manufacturer as string"""
+ id = self.get_manufacturer_id(offset)
+ if id == 0xffff:
+ return "Unknown"
+ ids = {
+ 0x2c80: "Crucial/Micron",
+ 0x4304: "Ramaxel",
+ 0x4f01: "Transcend",
+ 0x9801: "Kingston",
+ 0x987f: "Hynix",
+ 0x9e02: "Corsair",
+ 0xb004: "OCZ",
+ 0xad80: "Hynix/Hyundai",
+ 0xb502: "SuperTalent",
+ 0xcd04: "GSkill",
+ 0xce80: "Samsung",
+ 0xfe02: "Elpida",
+ 0xff2c: "Micron",
+ }
+ if id in ids:
+ return ids[id]
+ return "Unknown"
+
+ def blob_as_ord(self, offset):
+ """Helper for python2/python3 compability"""
+ return self.blob[offset] if type(self.blob[offset]) is int else ord(self.blob[offset])
+
+ def search(self, start):
+ """Search for SPD at start. Returns -1 on error or offset
+ if found.
+ """
+ for i in self.get_matches():
+ for offset in range(start, len(self.blob)):
+ if self.blob_as_ord(offset) != i:
+ continue
+ if not self.verify_match(i, offset):
+ continue
+ return offset, self.get_len(i, offset)
+ return -1, 0
+
+class SPD4Parser(Parser):
+ def get_matches(self):
+ """Return DDR4 possible header candidates"""
+ ret = []
+ for i in [1, 2, 3, 4]:
+ for j in [1, 2]:
+ ret.append(i + j * 16)
+ return ret
+
+ def verify_match(self, header, offset):
+ """Verify DDR4 specific bit fields."""
+ # offset 0 is a candidate, no need to validate
+ if self.blob_as_ord(offset + 1) == 0xff:
+ return False
+ if self.blob_as_ord(offset + 2) != 0x0c:
+ return False
+ if self.blob_as_ord(offset + 5) & 0xc0 > 0:
+ return False
+ if self.blob_as_ord(offset + 6) & 0xc > 0:
+ return False
+ if self.blob_as_ord(offset + 7) & 0xc0 > 0:
+ return False
+ if self.blob_as_ord(offset + 8) != 0:
+ return False
+ if self.blob_as_ord(offset + 9) & 0xf > 0:
+ return False
+ if self.verbose:
+ print("%x: Looks like DDR4 SPD" % offset)
+
+ crc = crc16.crc16xmodem(self.blob[offset:offset + 0x7d + 1])
+ # Vendors ignore the endianness...
+ crc_spd1 = self.blob_as_ord(offset + 0x7f) | (self.blob_as_ord(offset + 0x7e) << 8)
+ crc_spd2 = self.blob_as_ord(offset + 0x7e) | (self.blob_as_ord(offset + 0x7f) << 8)
+ if crc != crc_spd1 and crc != crc_spd2:
+ if self.verbose:
+ print("%x: CRC16 doesn't match" % offset)
+ if not self.ignorecrc:
+ return False
+
+ return True
+
+ def get_len(self, header, offset):
+ """Return the length of the SPD found."""
+ if (header >> 4) & 7 == 1:
+ return 256
+ if (header >> 4) & 7 == 2:
+ return 512
+ return 0
+
+ def get_part_number(self, offset):
+ """Return part number as string"""
+ if offset + 0x15c > len(self.blob):
+ return ""
+ return self.blob[offset + 0x149:offset + 0x15c + 1].decode('utf-8').rstrip()
+
+ def get_manufacturer_id(self, offset):
+ """Return manufacturer ID"""
+ if offset + 0x141 > len(self.blob):
+ return 0xffff
+ return struct.unpack('H', self.blob[offset + 0x140:offset + 0x141 + 1])[0]
+
+ def get_mtransfers(self, offset):
+ """Return MT/s as specified by MTB and FTB"""
+ if offset + 0x7d > len(self.blob):
+ return 0
+
+ if self.blob_as_ord(offset + 0x11) != 0:
+ return 0
+ mtb = 8.0
+ ftb = 1000.0
+ tckm = struct.unpack('B', self.blob[offset + 0x12:offset + 0x12 + 1])[0]
+ tckf = struct.unpack('b', self.blob[offset + 0x7d:offset + 0x7d + 1])[0]
+ return int(2000 / (tckm / mtb + tckf / ftb))
+
+if __name__ == "__main__":
+ parser = argparse.ArgumentParser(description='SPD rom dumper')
+ parser.add_argument('--blob', required=True,
+ help='The ROM to search SPDs in.')
+ parser.add_argument('--spd4', action='store_true', default=False,
+ help='Search for DDR4 SPDs.')
+ parser.add_argument('--hex', action='store_true', default=False,
+ help='Store SPD in hex format otherwise binary.')
+ parser.add_argument('-v', '--verbose', help='increase output verbosity',
+ action='store_true')
+ parser.add_argument('--ignorecrc', help='Ignore CRC missmatch',
+ action='store_true', default=False)
+ args = parser.parse_args()
+
+ blob = open(args.blob, "rb").read()
+
+ if args.spd4:
+ p = SPD4Parser(blob, args.verbose, args.ignorecrc)
+ else:
+ raise Exception("Must specify one of the following arguments:\n--spd4")
+
+ offset = 0
+ length = 0
+ cnt = 0
+ while True:
+ offset, length = p.search(offset)
+ if length == 0:
+ break
+ print("Found SPD at 0x%x" % offset)
+ print(" '%s', size %d, manufacturer %s (0x%04x) %d MT/s\n" %
+ (p.get_part_number(offset), length, p.get_manufacturer(offset),
+ p.get_manufacturer_id(offset), p.get_mtransfers(offset)))
+ filename = "spd-%d-%s-%s.bin" % (cnt, p.get_part_number(offset),
+ p.get_manufacturer(offset))
+ filename = filename.replace("/", "_")
+ filename = "".join([c for c in filename if c.isalpha() or c.isdigit() or c=='-' or c=='.' or c=='_']).rstrip()
+ if not args.hex:
+ open(filename, "wb").write(blob[offset:offset + length])
+ else:
+ filename += ".hex"
+ fn = open(filename, "w")
+ j = 0
+ for i in blob[offset:offset + length]:
+ fn.write("%02X%s" % (struct.unpack('B',i)[0], " " if j < 15 else "\n"))
+ j = (j + 1) % 16
+ offset += 1
+ cnt += 1
--
To view, visit https://review.coreboot.org/c/coreboot/+/31385
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I26dd73d43b724ea6891bb5b6e96856c42db8577c
Gerrit-Change-Number: 31385
Gerrit-PatchSet: 1
Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-MessageType: newchange
Julius Werner has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32113
Change subject: assert: Don't stringify 'message' in dead_code()
......................................................................
assert: Don't stringify 'message' in dead_code()
dead_code() is already supposed to be called with a string message, we
don't need to stringify the argument again (and doing so makes the
output look a bit weird).
Change-Id: I63399dc484e2150d8c027bc0256d9285e471f7cc
Signed-off-by: Julius Werner <jwerner(a)chromium.org>
---
M src/include/assert.h
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/32113/1
diff --git a/src/include/assert.h b/src/include/assert.h
index afbed03..6a442d6 100644
--- a/src/include/assert.h
+++ b/src/include/assert.h
@@ -45,7 +45,7 @@
* valid if a certain Kconfig option is set.
*/
#define __dead_code(message, line) do { \
- __attribute__((error(#message " in " __FILE__ ":" #line))) \
+ __attribute__((error(message " in " __FILE__ ":" #line))) \
extern void dead_code_assertion_failed_##line(void); \
dead_code_assertion_failed_##line(); \
} while (0)
--
To view, visit https://review.coreboot.org/c/coreboot/+/32113
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I63399dc484e2150d8c027bc0256d9285e471f7cc
Gerrit-Change-Number: 32113
Gerrit-PatchSet: 1
Gerrit-Owner: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: newchange
Hello Furquan Shaikh,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/32114
to review the following change.
Change subject: vboot: Select CONFIG_VBOOT_OPROM_MATTERS in more cases
......................................................................
vboot: Select CONFIG_VBOOT_OPROM_MATTERS in more cases
This patch enables CONFIG_VBOOT_OPROM_MATTERS in a few more cases where
I think(?) it should be. Haswell, Broadwell and Baytrail Chromebooks
have this enabled in their old depthcharge firmware branches -- we
presumably just forgot to move it over when vboot2 migrated the option
to coreboot. Braswell didn't, but it seems like this requirement was
added when it was migrated to FSP 1.1...? (Not very sure about that one,
but it does call load_vbt() right now which executes things based on
display_init_required().) Additionally, it seems to make sense to enable
it whenever the user explicitly selects VGA_ROM_RUN in menuconfig (like
one of the Intel defconfigs does).
Once we have all this, one could take a step back and ask whether this
option still makes sense at all anymore. It's enabled for almost all
devices (that work with vboot at all), it will presumably be enabled for
all future devices, and it seems that most devices that don't enable it
use libgfxinit, which as far as I can tell isn't gated on
display_init_required() but probably should be. Realistically, whatever
kind of display init a board needs to do (native or option ROM), it's
probably expensive enough that it's worth skipping on a normal mode
vboot boot, and we'd want to have this enabled by default on everything
except boards that actually don't have a display. So maybe we should
flip it around to CONFIG_VBOOT_OPROM_DOESNT_MATTER, but doing that would
probably lead to nobody ever selecting it at all.
Not sure what the best solution there is yet, but I think this patch
at least moves things in the more correct direction.
Change-Id: Id96a88296ddb9cfbb58ea67d93e1638d95570e2c
Signed-off-by: Julius Werner <jwerner(a)chromium.org>
---
M src/northbridge/intel/haswell/Kconfig
M src/security/vboot/Kconfig
M src/soc/intel/baytrail/Kconfig
M src/soc/intel/braswell/Kconfig
M src/soc/intel/broadwell/Kconfig
5 files changed, 5 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/32114/1
diff --git a/src/northbridge/intel/haswell/Kconfig b/src/northbridge/intel/haswell/Kconfig
index 0362ffe..082f2d6 100644
--- a/src/northbridge/intel/haswell/Kconfig
+++ b/src/northbridge/intel/haswell/Kconfig
@@ -26,6 +26,7 @@
if NORTHBRIDGE_INTEL_HASWELL
config VBOOT
+ select VBOOT_OPROM_MATTERS
select VBOOT_STARTS_IN_ROMSTAGE
config BOOTBLOCK_NORTHBRIDGE_INIT
diff --git a/src/security/vboot/Kconfig b/src/security/vboot/Kconfig
index ca25423..a0cfca5 100644
--- a/src/security/vboot/Kconfig
+++ b/src/security/vboot/Kconfig
@@ -171,6 +171,7 @@
config VBOOT_OPROM_MATTERS
bool
+ default y if VGA_ROM_RUN
default n
help
Set this option to indicate to vboot that this platform will skip its
diff --git a/src/soc/intel/baytrail/Kconfig b/src/soc/intel/baytrail/Kconfig
index e824ee4..a83a7e9 100644
--- a/src/soc/intel/baytrail/Kconfig
+++ b/src/soc/intel/baytrail/Kconfig
@@ -43,6 +43,7 @@
select CPU_HAS_L2_ENABLE_MSR
config VBOOT
+ select VBOOT_OPROM_MATTERS
select VBOOT_STARTS_IN_ROMSTAGE
config BOOTBLOCK_CPU_INIT
diff --git a/src/soc/intel/braswell/Kconfig b/src/soc/intel/braswell/Kconfig
index fda5a6d..8db4795 100644
--- a/src/soc/intel/braswell/Kconfig
+++ b/src/soc/intel/braswell/Kconfig
@@ -53,6 +53,7 @@
select SOUTHBRIDGE_INTEL_COMMON_SMBUS
config VBOOT
+ select VBOOT_OPROM_MATTERS
select VBOOT_STARTS_IN_ROMSTAGE
config BOOTBLOCK_CPU_INIT
diff --git a/src/soc/intel/broadwell/Kconfig b/src/soc/intel/broadwell/Kconfig
index 1a8349d0..5f503da 100644
--- a/src/soc/intel/broadwell/Kconfig
+++ b/src/soc/intel/broadwell/Kconfig
@@ -65,6 +65,7 @@
default y
config VBOOT
+ select VBOOT_OPROM_MATTERS
select VBOOT_STARTS_IN_ROMSTAGE
config BOOTBLOCK_CPU_INIT
--
To view, visit https://review.coreboot.org/c/coreboot/+/32114
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Id96a88296ddb9cfbb58ea67d93e1638d95570e2c
Gerrit-Change-Number: 32114
Gerrit-PatchSet: 1
Gerrit-Owner: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-MessageType: newchange
Lijian Zhao has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32124
Change subject: mb/google/sarien: Enable Bluetooth RF kill
......................................................................
mb/google/sarien: Enable Bluetooth RF kill
Add bluetooth Rfkill function to recover the Bluetooth controller in
cases where itself has entered a bad state and needs to be recovered.
Bug=b:123342945
TEST=Boot up into OS and dump SSDT table, check there's _DSD entry under
Bluetooth devices with GPIO in. Also confirm bluetooth itself is
functional.
Signed-off-by: Lijian Zhao <lijian.zhao(a)intel.com>
Change-Id: I79a310a55d94d7d20d1705afc11fe47cbb81abc1
---
M src/mainboard/google/sarien/variants/sarien/devicetree.cb
1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/32124/1
diff --git a/src/mainboard/google/sarien/variants/sarien/devicetree.cb b/src/mainboard/google/sarien/variants/sarien/devicetree.cb
index 2714b60..72dea1e 100644
--- a/src/mainboard/google/sarien/variants/sarien/devicetree.cb
+++ b/src/mainboard/google/sarien/variants/sarien/devicetree.cb
@@ -264,6 +264,7 @@
chip drivers/usb/acpi
register "desc" = ""Bluetooth""
register "type" = "UPC_TYPE_INTERNAL"
+ register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_H15)"
device usb 2.9 on end
end
chip drivers/usb/acpi
--
To view, visit https://review.coreboot.org/c/coreboot/+/32124
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I79a310a55d94d7d20d1705afc11fe47cbb81abc1
Gerrit-Change-Number: 32124
Gerrit-PatchSet: 1
Gerrit-Owner: Lijian Zhao <lijian.zhao(a)intel.com>
Gerrit-MessageType: newchange
Krishna P Bhat D has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32126
Change subject: mb/google/hatch: Unlock GPIO pads
......................................................................
mb/google/hatch: Unlock GPIO pads
GPP_A12 is being used as FPMCU_RST in hatch. This GPIO is being padlocked in
FSP and cannot used in kernel. Hence unlock the GPIO pads to export this pin
in kernel to be used as FPMCU_RST.
BUG=b:128686027
Change-Id: Ie0439956e6c8e386435e535665ccaf2ab82adeb0
Signed-off-by: Krishna Prasad Bhat <krishna.p.bhat.d(a)intel.com>
---
M src/mainboard/google/hatch/variants/baseboard/devicetree.cb
1 file changed, 2 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/32126/1
diff --git a/src/mainboard/google/hatch/variants/baseboard/devicetree.cb b/src/mainboard/google/hatch/variants/baseboard/devicetree.cb
index dc7cc24..984852f 100644
--- a/src/mainboard/google/hatch/variants/baseboard/devicetree.cb
+++ b/src/mainboard/google/hatch/variants/baseboard/devicetree.cb
@@ -73,6 +73,8 @@
register "DdiPortBHpd" = "1"
register "DdiPortCHpd" = "1"
register "tcc_offset" = "10" # TCC of 90C
+ # Unlock GPIO pads
+ register "PchUnlockGpioPads" = "1"
register "usb2_ports[0]" = "USB2_PORT_TYPE_C(OC2)" # Type-C Port 0
register "usb2_ports[1]" = "USB2_PORT_TYPE_C(OC2)" # Type-C Port 1
--
To view, visit https://review.coreboot.org/c/coreboot/+/32126
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie0439956e6c8e386435e535665ccaf2ab82adeb0
Gerrit-Change-Number: 32126
Gerrit-PatchSet: 1
Gerrit-Owner: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com>
Gerrit-MessageType: newchange