futility: update: Use ifdtool to unlock ME
Currently the unlock_csme_nissa quirk only applies to nissa.
Specifically the GPR0 offset is hardcoded to the ADL value.
Change it to use ifdtool for unlocking FLMSTRs and disabling GPR0
instead. Support for disabling GPR0 was added to ifdtool in CB:79788.
It can now be used on all recent Intel platforms, specifically CML
onwards since older platforms did not have GPR0. Since it's no longer
nissa-specific, rename the quirk to unlock_csme.
ifdtool requires the platform to be passed as an argument. This is
determined by reading CONFIG_IFD_CHIPSET from the config file in CBFS.
However, listing all Kconfigs in the file was only added in CB:69710,
which is not currently included on the nissa firmware branch. So for
nissa use a fallback of checking for 'nissa' in CONFIG_IFD_BIN_PATH.
BRANCH=None
BUG=b:270275115
TEST=Run `futility update --mode=output --unlock_me -i image.bin` on
nissa images from both ToT (with CONFIG_IFD_CHIPSET) and the FW branch
(without CONFIG_IFD_CHIPSET).
Change-Id: Ib3dc4dda693f3709c5649671dcb0c739e8828f20
Signed-off-by: Reka Norman <rekanorman@google.com>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/5191650
Commit-Queue: Tzu-Min Sun <jimmysun@chromium.org>
Tested-by: Kenny Pan <kennypan@google.com>
Auto-Submit: Kenny Pan <kennypan@google.com>
Reviewed-by: Tzu-Min Sun <jimmysun@chromium.org>
diff --git a/futility/cmd_update.c b/futility/cmd_update.c
index c58b37a..77df9e1 100644
--- a/futility/cmd_update.c
+++ b/futility/cmd_update.c
@@ -203,7 +203,7 @@
args.unpack = optarg;
break;
case OPT_UNLOCK_ME:
- WARN("--unlock_me will be deprecated by --quirks unlock_csme_nissa.\n");
+ WARN("--unlock_me will be deprecated by --quirks unlock_csme.\n");
args.unlock_me = true;
break;
case OPT_QUIRKS:
diff --git a/futility/platform_csme.c b/futility/platform_csme.c
index 2215343..33a98ba 100644
--- a/futility/platform_csme.c
+++ b/futility/platform_csme.c
@@ -7,6 +7,8 @@
*/
#include <string.h>
+
+#include "cbfstool.h"
#include "platform_csme.h"
#include "updater.h"
@@ -131,48 +133,110 @@
}
/*
- * Disable GPR0 (Global Protected Range). When enabled, it provides
- * write-protection to part of the SI_ME region, specifically CSE_RO and
- * part of CSE_DATA, so it must be disabled to allow updating SI_ME.
- * Returns 0 on success, otherwise failure.
+ * Determine the platform to pass to ifdtool (e.g. 'adl') by extracting
+ * CONFIG_IFD_CHIPSET from the config file in CBFS. However, old nissa firmware
+ * may not have all config fields in the CBFS file, so fall back to a hack of
+ * checking for 'nissa' in the descriptor file path.
*
- * TODO(b/270275115): Replace with a call to ifdtool, or a generic way.
+ * On success, returns the platform, which must be freed by the caller.
+ * On failure, returns NULL.
*/
-static int disable_gpr0_nissa(struct firmware_image *image)
+static char *determine_ifd_platform(const char *image_path)
{
- /* This offset varies and the constant below is only for Nissa. DON'T USE IT for MTL. */
- const int gpr0_offset = 0x154;
- const uint8_t gpr0_value_disabled[] = { 0x00, 0x00, 0x00, 0x00 };
+ char *platform;
+ char *ifd_path;
- if (overwrite_section(image, FMAP_SI_DESC, gpr0_offset,
- ARRAY_SIZE(gpr0_value_disabled),
- gpr0_value_disabled)) {
- ERROR("Failed disabling GPR0.\n");
- return -1;
+ cbfstool_get_config_value(image_path, NULL, "CONFIG_IFD_CHIPSET", &platform);
+ if (platform)
+ return platform;
+
+ /* Fall back to checking for nissa in the descriptor file path */
+ cbfstool_get_config_value(image_path, NULL, "CONFIG_IFD_BIN_PATH", &ifd_path);
+ if (ifd_path && strstr(ifd_path, "nissa")) {
+ VB2_DEBUG("Use platform 'adl' since descriptor path contains 'nissa'\n");
+ ASPRINTF(&platform, "adl");
}
- INFO("Disabled GPR0.\n");
- return 0;
+ if (ifd_path)
+ free(ifd_path);
+
+ return platform;
}
/*
- * Unlock the CSME for Nissa platforms.
- *
- * This allows the SI_DESC and SI_ME regions to be updated.
- * TODO(b/270275115): Replace with a call to ifdtool.
+ * Run ifdtool with the given options.
*
* Returns 0 on success, otherwise failure.
*/
-int unlock_csme_nissa(struct firmware_image *image)
+static int run_ifdtool(const char *image_path, char *platform, const char *extra_options)
{
- /* Unlock the FLMSTR values (applies to JSL/TGL and newer platforms). */
- if (unlock_flmstrs(image, 0xffffffff, 0xffffffff, 0xffffffff))
- return -1;
+ char *command;
+ int ret = 0;
- /* Disable the GPR0 in the descriptor for CSE lite. */
- if (disable_gpr0_nissa(image))
- return -1;
+ ASPRINTF(&command, "ifdtool -p %s -O \"%s\" \"%s\" %s 2>&1",
+ platform, image_path, image_path, extra_options);
+ if (system(command)) {
+ ERROR("Failed to run: %s\n", command);
+ ret = -1;
+ }
- INFO("Unlocked Intel ME for Nissa platforms.\n");
- return 0;
+ free(command);
+ return ret;
+}
+
+/*
+ * Unlock the CSME for recent Intel platforms (CML onwards).
+ *
+ * This allows the SI_DESC and SI_ME regions to be updated.
+ *
+ * Returns 0 on success, otherwise failure.
+ */
+int unlock_csme(struct updater_config *cfg)
+{
+ const char *temp_path;
+ char *platform;
+ int ret = -1;
+
+ temp_path = get_firmware_image_temp_file(&cfg->image, &cfg->tempfiles);
+ if (!temp_path) {
+ ERROR("Failed to get image temp file\n");
+ return ret;
+ }
+
+ platform = determine_ifd_platform(temp_path);
+ if (!platform) {
+ ERROR("Failed to determine IFD platform\n");
+ return ret;
+ }
+
+ VB2_DEBUG("Using platform '%s'\n", platform);
+
+ /* Unlock FMLSTRs */
+ if (run_ifdtool(temp_path, platform, "-u")) {
+ ERROR("Failed to unlock FLMSTRs\n");
+ goto cleanup;
+ }
+
+ /*
+ * Disable GPR0 (Global Protected Range). When enabled, it provides
+ * write-protection to part of the SI_ME region, specifically CSE_RO and
+ * part of CSE_DATA, so it must be disabled to allow updating SI_ME.
+ */
+ if (run_ifdtool(temp_path, platform, "-g")) {
+ ERROR("Failed to disable GPR0\n");
+ goto cleanup;
+ }
+
+ if (reload_firmware_image(temp_path, &cfg->image)) {
+ ERROR("Failed to reload firmware image\n");
+ goto cleanup;
+ }
+
+ INFO("Unlocked Intel ME\n");
+ ret = 0;
+
+cleanup:
+ free(platform);
+
+ return ret;
}
diff --git a/futility/platform_csme.h b/futility/platform_csme.h
index 88c36bc..8453950 100644
--- a/futility/platform_csme.h
+++ b/futility/platform_csme.h
@@ -16,7 +16,7 @@
/* Unlock the flash descriptor for Skylake and Kabylake platforms. */
int unlock_csme_eve(struct firmware_image *image);
-/* Unlock the CSME for the nissa platform. */
-int unlock_csme_nissa(struct firmware_image *image);
+/* Unlock the CSME for recent Intel platforms (CML onwards). */
+int unlock_csme(struct updater_config *cfg);
#endif /* VBOOT_REFERENCE_FUTILITY_PLATFORM_CSME_H_ */
diff --git a/futility/updater.c b/futility/updater.c
index e09adf5..ac2daa3 100644
--- a/futility/updater.c
+++ b/futility/updater.c
@@ -554,7 +554,7 @@
/* Returns true if the UNLOCK_CSME_* quirks were requested, otherwise false. */
static bool is_unlock_csme_requested(struct updater_config *cfg)
{
- if (get_config_quirk(QUIRK_UNLOCK_CSME_NISSA, cfg) ||
+ if (get_config_quirk(QUIRK_UNLOCK_CSME, cfg) ||
get_config_quirk(QUIRK_UNLOCK_CSME_EVE, cfg))
return true;
return false;
@@ -1721,9 +1721,9 @@
if (cfg->image.data) {
/* Apply any quirks to modify the image before updating. */
if (arg->unlock_me)
- cfg->quirks[QUIRK_UNLOCK_CSME_NISSA].value = 1;
+ cfg->quirks[QUIRK_UNLOCK_CSME].value = 1;
errorcnt += try_apply_quirk(QUIRK_UNLOCK_CSME_EVE, cfg);
- errorcnt += try_apply_quirk(QUIRK_UNLOCK_CSME_NISSA, cfg);
+ errorcnt += try_apply_quirk(QUIRK_UNLOCK_CSME, cfg);
}
/* The images are ready for updating. Output if needed. */
diff --git a/futility/updater.h b/futility/updater.h
index ef4071d..31052a3 100644
--- a/futility/updater.h
+++ b/futility/updater.h
@@ -57,7 +57,7 @@
QUIRK_OVERRIDE_SIGNATURE_ID,
QUIRK_EVE_SMM_STORE,
QUIRK_UNLOCK_CSME_EVE,
- QUIRK_UNLOCK_CSME_NISSA,
+ QUIRK_UNLOCK_CSME,
/* End of quirks */
QUIRK_MAX,
};
diff --git a/futility/updater_quirks.c b/futility/updater_quirks.c
index 47abf1a..7ae6173 100644
--- a/futility/updater_quirks.c
+++ b/futility/updater_quirks.c
@@ -46,14 +46,6 @@
{ .match = "Google_Phaser.", .quirks = "override_signature_id" },
};
-/* Preserves meta data and reload image contents from given file path. */
-static int reload_firmware_image(const char *file_path,
- struct firmware_image *image)
-{
- free_firmware_image(image);
- return load_firmware_image(image, file_path, NULL);
-}
-
/*
* Returns True if the system has EC software sync enabled.
*/
@@ -195,9 +187,9 @@
return unlock_csme_eve(&cfg->image);
}
-static int quirk_unlock_csme_nissa(struct updater_config *cfg)
+static int quirk_unlock_csme(struct updater_config *cfg)
{
- return unlock_csme_nissa(&cfg->image);
+ return unlock_csme(cfg);
}
/*
@@ -445,10 +437,11 @@
quirks->help = "b/35568719; (skl, kbl) only lock management engine in board-postinst.";
quirks->apply = quirk_unlock_csme_eve;
- quirks = &cfg->quirks[QUIRK_UNLOCK_CSME_NISSA];
- quirks->name = "unlock_csme_nissa";
- quirks->help = "b/273168873; (nissa only) unlock the management engine and CSE lite.";
- quirks->apply = quirk_unlock_csme_nissa;
+ quirks = &cfg->quirks[QUIRK_UNLOCK_CSME];
+ quirks->name = "unlock_csme";
+ quirks->help = "b/273168873; unlock the Intel management engine. "
+ "Applies to all recent Intel platforms (CML onwards)";
+ quirks->apply = quirk_unlock_csme;
quirks = &cfg->quirks[QUIRK_EVE_SMM_STORE];
quirks->name = "eve_smm_store";
diff --git a/futility/updater_utils.c b/futility/updater_utils.c
index fa3e8fc..35c6f8a 100644
--- a/futility/updater_utils.c
+++ b/futility/updater_utils.c
@@ -274,6 +274,13 @@
image->programmer = programmer;
}
+/* Preserves meta data and reloads image contents from given file path. */
+int reload_firmware_image(const char *file_path, struct firmware_image *image)
+{
+ free_firmware_image(image);
+ return load_firmware_image(image, file_path, NULL);
+}
+
/*
* Finds a firmware section by given name in the firmware image.
* If successful, return zero and *section argument contains the address and
diff --git a/futility/updater_utils.h b/futility/updater_utils.h
index 2661381..bb2fef1 100644
--- a/futility/updater_utils.h
+++ b/futility/updater_utils.h
@@ -83,6 +83,9 @@
/* Frees the allocated resource from a firmware image object. */
void free_firmware_image(struct firmware_image *image);
+/* Preserves meta data and reloads image contents from given file path. */
+int reload_firmware_image(const char *file_path, struct firmware_image *image);
+
/*
* Generates a temporary file for snapshot of firmware image contents.
*
diff --git a/tests/futility/test_update.sh b/tests/futility/test_update.sh
index 7e08efd..4189759 100755
--- a/tests/futility/test_update.sh
+++ b/tests/futility/test_update.sh
@@ -160,11 +160,6 @@
cp -f "${TMP}.expected.full" "${TMP}.expected.me_unlocked_eve"
patch_file "${TMP}.expected.me_unlocked_eve" SI_DESC 0x60 \
"\x00\xff\xff\xff\x00\xff\xff\xff\x00\xff\xff\xff"
-cp -f "${TMP}.expected.full" "${TMP}.expected.me_unlocked_nissa"
-patch_file "${TMP}.expected.me_unlocked_nissa" SI_DESC 0x60 \
- "\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff"
-patch_file "${TMP}.expected.me_unlocked_nissa" SI_DESC 0x154 \
- "\x00\x00\x00\x00"
cp -f "${TMP}.expected.full" "${TMP}.expected.me_preserved"
"${FUTILITY}" load_fmap "${TMP}.expected.me_preserved" \
"SI_ME:${TMP}.from/SI_ME"
@@ -181,6 +176,58 @@
RW_VPD:"${TMP}.to/RW_VPD"
patch_file "${TMP}.expected.full.empty_rw_vpd" FMAP 0x3fc "$(printf '\010')"
+# Generate images for testing --unlock_me.
+# There are two ways to detect the platform:
+# - Read CONFIG_IFD_CHIPSET from config file in CBFS
+# - Fallback for nissa: check if CONFIG_IFD_BIN_PATH contains 'nissa'
+
+# Rename BOOT_STUB to COREBOOT, which is the default region used by cbfstool.
+rename_boot_stub() {
+ local image="$1"
+
+ "${FUTILITY}" dump_fmap "${image}" -x "FMAP:${TMP}.fmap"
+ sed -i 's/BOOT_STUB/COREBOOT\x00/g' "${TMP}.fmap"
+ "${FUTILITY}" load_fmap "${image}" "FMAP:${TMP}.fmap"
+}
+
+# Add the given line to the config file in CBFS.
+add_config() {
+ local image="$1"
+ local config_line="$2"
+
+ rename_boot_stub "${image}"
+
+ cbfstool "${image}" extract -n config -f "${TMP}.config"
+ echo "${config_line}" >> "${TMP}.config"
+ cbfstool "${image}" remove -n config
+ cbfstool "${image}" add -n config -f "${TMP}.config" -t raw
+}
+
+unlock_me() {
+ local image="$1"
+
+ patch_file "${image}" SI_DESC 0x60 \
+ "\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff"
+ patch_file "${image}" SI_DESC 0x154 \
+ "\x00\x00\x00\x00"
+}
+
+IFD_CHIPSET="CONFIG_IFD_CHIPSET=\"adl\""
+IFD_PATH="CONFIG_IFD_BIN_PATH=\"3rdparty/blobs/mainboard/google/nissa/descriptor-craask.bin\""
+cp -f "${TO_IMAGE}" "${TO_IMAGE}.ifd_chipset"
+cp -f "${TO_IMAGE}" "${TO_IMAGE}.ifd_path"
+cp -f "${TMP}.expected.full" "${TMP}.expected.ifd_chipset"
+cp -f "${TMP}.expected.full" "${TMP}.expected.ifd_path"
+add_config "${TO_IMAGE}.ifd_chipset" "${IFD_CHIPSET}"
+add_config "${TO_IMAGE}.ifd_path" "${IFD_PATH}"
+add_config "${TMP}.expected.ifd_chipset" "${IFD_CHIPSET}"
+add_config "${TMP}.expected.ifd_path" "${IFD_PATH}"
+
+cp -f "${TMP}.expected.ifd_chipset" "${TMP}.expected.me_unlocked.ifd_chipset"
+cp -f "${TMP}.expected.ifd_path" "${TMP}.expected.me_unlocked.ifd_path"
+unlock_me "${TMP}.expected.me_unlocked.ifd_chipset"
+unlock_me "${TMP}.expected.me_unlocked.ifd_path"
+
# Has 3 modes:
# 1. $3 = "!something", run command, expect failure,
# grep for something in log, fail if it is not present
@@ -377,13 +424,19 @@
--quirks unlock_csme_eve \
-i "${TO_IMAGE}" --wp=0 --sys_props 0,0x10001
-test_update "Full update (--quirks unlock_csme_nissa)" \
- "${FROM_IMAGE}" "${TMP}.expected.me_unlocked_nissa" \
- --quirks unlock_csme_nissa -i "${TO_IMAGE}" --wp=0 --sys_props 0,0x10001
+test_update "Full update (--quirks unlock_csme)" \
+ "${FROM_IMAGE}" "${TMP}.expected.me_unlocked.ifd_chipset" \
+ --quirks unlock_csme -i "${TO_IMAGE}.ifd_chipset" \
+ --wp=0 --sys_props 0,0x10001
+
+test_update "Full update (--quirks unlock_csme)" \
+ "${FROM_IMAGE}" "${TMP}.expected.me_unlocked.ifd_path" \
+ --quirks unlock_csme -i "${TO_IMAGE}.ifd_path" \
+ --wp=0 --sys_props 0,0x10001
test_update "Full update (--unlock_me)" \
- "${FROM_IMAGE}" "${TMP}.expected.me_unlocked_nissa" \
- --unlock_me -i "${TO_IMAGE}" --wp=0 --sys_props 0,0x10001
+ "${FROM_IMAGE}" "${TMP}.expected.me_unlocked.ifd_chipset" \
+ --unlock_me -i "${TO_IMAGE}.ifd_chipset" --wp=0 --sys_props 0,0x10001
test_update "Full update (failure by --quirks min_platform_version)" \
"${FROM_IMAGE}" "!Need platform version >= 3 (current is 2)" \
@@ -462,10 +515,10 @@
--output_dir="${TMP}.output"
cmp "${LINK_BIOS}" "${TMP}.output/image.bin"
-echo "TEST: Output (--mode=output, --quirks unlock_csme_nissa)"
-"${FUTILITY}" update -i "${TMP}.expected.full" --mode=output \
- --output_dir="${TMP}.output" --quirks unlock_csme_nissa
-cmp "${TMP}.expected.me_unlocked_nissa" "${TMP}.output/image.bin"
+echo "TEST: Output (--mode=output, --quirks unlock_csme)"
+"${FUTILITY}" update -i "${TMP}.expected.ifd_chipset" --mode=output \
+ --output_dir="${TMP}.output" --quirks unlock_csme
+cmp "${TMP}.expected.me_unlocked.ifd_chipset" "${TMP}.output/image.bin"
mkdir -p "${A}/keyset"
cp -f "${LINK_BIOS}" "${A}/image.bin"
