<p>Bill XIE has uploaded this change for <strong>review</strong>.</p><p><a href="https://review.coreboot.org/21416">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">ifdtool: refactor region-permission-related feature<br><br>combine lock_descriptor() and unlock_descriptor() into one<br>function fmba_toggle_permission(), and port the feature to<br>"remove the ME/TXE Read/Write permissions to the other regions<br>(-d)" of me_cleaner (https://github.com/corna/me_cleaner/) here.<br><br>Change-Id: I4d34606545db4d970ce3ec202da4bf4c5cbad18c<br>Signed-off-by: Bill XIE <persmule@gmail.com><br>---<br>M util/ifdtool/ifdtool.c<br>M util/ifdtool/ifdtool.h<br>2 files changed, 97 insertions(+), 69 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/21416/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/util/ifdtool/ifdtool.c b/util/ifdtool/ifdtool.c<br>index b213929..a133efb 100644<br>--- a/util/ifdtool/ifdtool.c<br>+++ b/util/ifdtool/ifdtool.c<br>@@ -758,83 +758,94 @@<br> write_image(filename, image, size);<br> }<br> <br>-static void lock_descriptor(const char *filename, char *image, int size)<br>+static void fmba_toggle_permission(fmba_t *fmba, enum toggle_mode lock_fd,<br>+ enum toggle_mode jail_me)<br> {<br> int wr_shift, rd_shift;<br>- fmba_t *fmba = find_fmba(image, size);<br>- if (!fmba)<br>- exit(EXIT_FAILURE);<br> /* TODO: Dynamically take Platform Data Region and GbE Region<br> * into regard.<br> */<br>-<br> if (ifd_version >= IFD_VERSION_2) {<br> wr_shift = FLMSTR_WR_SHIFT_V2;<br> rd_shift = FLMSTR_RD_SHIFT_V2;<br>-<br>- /* Clear non-reserved bits */<br>- fmba->flmstr1 &= 0xff;<br>- fmba->flmstr2 &= 0xff;<br>- fmba->flmstr3 &= 0xff;<br> } else {<br> wr_shift = FLMSTR_WR_SHIFT_V1;<br> rd_shift = FLMSTR_RD_SHIFT_V1;<br>-<br>- fmba->flmstr1 = 0;<br>- fmba->flmstr2 = 0;<br>- /* Requestor ID */<br>- fmba->flmstr3 = 0x118;<br> }<br>+ if (lock_fd == TM_set) {<br>+ if (ifd_version >= IFD_VERSION_2) {<br>+ /* Clear non-reserved bits */<br>+ fmba->flmstr1 &= 0xff;<br>+ fmba->flmstr2 &= 0xff;<br>+ fmba->flmstr3 &= 0xff;<br>+ } else {<br>+ fmba->flmstr1 = 0;<br>+ fmba->flmstr2 = 0;<br>+ /* Requestor ID */<br>+ fmba->flmstr3 = 0x118;<br>+ }<br> <br>- switch (platform) {<br>- case PLATFORM_APOLLOLAKE:<br>- /* CPU/BIOS can read descriptor and BIOS */<br>- fmba->flmstr1 |= 0x3 << rd_shift;<br>- /* CPU/BIOS can write BIOS */<br>- fmba->flmstr1 |= 0x2 << wr_shift;<br>- /* TXE can read descriptor, BIOS and Device Expansion */<br>- fmba->flmstr2 |= 0x23 << rd_shift;<br>- /* TXE can only write Device Expansion */<br>- fmba->flmstr2 |= 0x20 << wr_shift;<br>- break;<br>- default:<br>+ switch (platform) {<br>+ case PLATFORM_APOLLOLAKE:<br>+ /* CPU/BIOS can read descriptor and BIOS */<br>+ fmba->flmstr1 |= 0x3 << rd_shift;<br>+ /* CPU/BIOS can write BIOS */<br>+<br>+ /* TXE can read descriptor,<br>+ * BIOS and Device Expansion<br>+ */<br>+ fmba->flmstr2 |= 0x23 << rd_shift;<br>+ /* TXE can only write Device<br>+ * Expansion<br>+ */<br>+ fmba->flmstr2 |= 0x20 << wr_shift;<br>+ break;<br>+ default:<br> /* CPU/BIOS can read descriptor, BIOS, and GbE. */<br>- fmba->flmstr1 |= 0xb << rd_shift;<br>- /* CPU/BIOS can write BIOS and GbE. */<br>- fmba->flmstr1 |= 0xa << wr_shift;<br>- /* ME can read descriptor, ME, and GbE. */<br>- fmba->flmstr2 |= 0xd << rd_shift;<br>- /* ME can write ME and GbE. */<br>- fmba->flmstr2 |= 0xc << wr_shift;<br>- /* GbE can write only GbE. */<br>- fmba->flmstr3 |= 0x8 << rd_shift;<br>- /* GbE can read only GbE. */<br>- fmba->flmstr3 |= 0x8 << wr_shift;<br>- break;<br>- }<br>-<br>- write_image(filename, image, size);<br>-}<br>-<br>-static void unlock_descriptor(const char *filename, char *image, int size)<br>-{<br>- fmba_t *fmba = find_fmba(image, size);<br>- if (!fmba)<br>- exit(EXIT_FAILURE);<br>-<br>- if (ifd_version >= IFD_VERSION_2) {<br>- /* Access bits for each region are read: 19:8 write: 31:20 */<br>- fmba->flmstr1 = 0xffffff00 | (fmba->flmstr1 & 0xff);<br>- fmba->flmstr2 = 0xffffff00 | (fmba->flmstr2 & 0xff);<br>- fmba->flmstr3 = 0xffffff00 | (fmba->flmstr3 & 0xff);<br>+ fmba->flmstr1 |= 0xb << rd_shift;<br>+ /* CPU/BIOS can write BIOS and GbE. */<br>+ fmba->flmstr1 |= 0xa << wr_shift;<br>+ /* ME can read descriptor, ME, and GbE. */<br>+ fmba->flmstr2 |= 0xd << rd_shift;<br>+ /* ME can write ME and GbE. */<br>+ fmba->flmstr2 |= 0xc << wr_shift;<br>+ /* GbE can write only GbE. */<br>+ fmba->flmstr3 |= 0x8 << rd_shift;<br>+ /* GbE can read only GbE. */<br>+ fmba->flmstr3 |= 0x8 << wr_shift;<br>+ break;<br>+ }<br> } else {<br>- fmba->flmstr1 = 0xffff0000;<br>- fmba->flmstr2 = 0xffff0000;<br>- /* Keep chipset specific Requester ID */<br>- fmba->flmstr3 = 0x08080000 | (fmba->flmstr3 & 0xffff);<br>+ if (ifd_version >= IFD_VERSION_2) {<br>+ /* Access bits for each region are read:<br>+ * 19:8 write: 31:20<br>+ */<br>+ fmba->flmstr1 = 0xffffff00 |<br>+ (fmba->flmstr1 & 0xff);<br>+ if (jail_me) {<br>+ fmba->flmstr2 &= 0xff;<br>+ /* ME can read ME. */<br>+ fmba->flmstr2 |= 0x4 << rd_shift;<br>+ /* ME can write ME. */<br>+ fmba->flmstr2 |= 0x4 << wr_shift;<br>+ } else {<br>+ fmba->flmstr2 = 0xffffff00 |<br>+ (fmba->flmstr2 & 0xff);<br>+ }<br>+ fmba->flmstr3 = 0xffffff00 |<br>+ (fmba->flmstr3 & 0xff);<br>+ } else {<br>+ fmba->flmstr1 = 0xffff0000;<br>+ if (jail_me) {<br>+ fmba->flmstr2 = 0x04040000;<br>+ } else {<br>+ fmba->flmstr2 = 0xffff0000;<br>+ }<br>+ /* Keep chipset specific Requester ID */<br>+ fmba->flmstr3 = 0x08080000 |<br>+ (fmba->flmstr3 & 0xffff);<br>+ }<br> }<br>-<br>- write_image(filename, image, size);<br> }<br> <br> void inject_region(const char *filename, char *image, int size, unsigned int region_type,<br>@@ -1152,6 +1163,9 @@<br> " Dual Output Fast Read Support\n"<br> " -l | --lock Lock firmware descriptor and ME region\n"<br> " -u | --unlock Unlock firmware descriptor and ME region\n"<br>+ " -j | --jail Unlock firmware descriptor and ME region,\n"<br>+ " and remove the ME/TXE's Read/Write permissions\n"<br>+ " to the other regions"<br> " -p | --platform Add platform-specific quirks\n"<br> " aplk - Apollo Lake\n"<br> " -v | --version: print the version\n"<br>@@ -1164,7 +1178,7 @@<br> {<br> int opt, option_index = 0;<br> int mode_dump = 0, mode_extract = 0, mode_inject = 0, mode_spifreq = 0;<br>- int mode_em100 = 0, mode_locked = 0, mode_unlocked = 0;<br>+ int mode_em100 = 0, mode_locked = 0, mode_unlocked = 0, mode_jail = 0;<br> int mode_layout = 0, mode_newlayout = 0, mode_density = 0;<br> char *region_type_string = NULL, *region_fname = NULL;<br> const char *layout_fname = NULL;<br>@@ -1184,13 +1198,14 @@<br> {"em100", 0, NULL, 'e'},<br> {"lock", 0, NULL, 'l'},<br> {"unlock", 0, NULL, 'u'},<br>+ {"jail", 0, NULL, 'j'},<br> {"version", 0, NULL, 'v'},<br> {"help", 0, NULL, 'h'},<br> {"platform", 0, NULL, 'p'},<br> {0, 0, 0, 0}<br> };<br> <br>- while ((opt = getopt_long(argc, argv, "df:D:C:xi:n:s:p:eluvh?",<br>+ while ((opt = getopt_long(argc, argv, "df:D:C:xi:n:s:p:elujvh?",<br> long_options, &option_index)) != EOF) {<br> switch (opt) {<br> case 'd':<br>@@ -1341,6 +1356,14 @@<br> exit(EXIT_FAILURE);<br> }<br> break;<br>+ case 'j':<br>+ mode_unlocked = 1;<br>+ mode_jail = 1;<br>+ if (mode_locked == 1) {<br>+ fprintf(stderr, "Locking/Unlocking FD and ME are mutually exclusive\n");<br>+ exit(EXIT_FAILURE);<br>+ }<br>+ break;<br> case 'p':<br> if (!strcmp(optarg, "aplk")) {<br> platform = PLATFORM_APOLLOLAKE;<br>@@ -1439,11 +1462,11 @@<br> if (mode_em100)<br> set_em100_mode(filename, image, size);<br> <br>- if (mode_locked)<br>- lock_descriptor(filename, image, size);<br>-<br>- if (mode_unlocked)<br>- unlock_descriptor(filename, image, size);<br>+ if (mode_locked || mode_unlocked) {<br>+ fmba_t *fmba = find_fmba(image, size);<br>+ fmba_toggle_permission(fmba, mode_locked, mode_jail);<br>+ write_image(filename, image, size);<br>+ }<br> <br> free(image);<br> <br>diff --git a/util/ifdtool/ifdtool.h b/util/ifdtool/ifdtool.h<br>index a8be820..e0e1886 100644<br>--- a/util/ifdtool/ifdtool.h<br>+++ b/util/ifdtool/ifdtool.h<br>@@ -1,4 +1,4 @@<br>-/*<br>+\/*<br> * ifdtool - dump Intel Firmware Descriptor information<br> *<br> * Copyright (C) 2011 The ChromiumOS Authors. All rights reserved.<br>@@ -81,6 +81,11 @@<br> ALD_right<br> };<br> <br>+enum toggle_mode {<br>+ TM_unset = 0,<br>+ TM_set = 1<br>+};<br>+<br> typedef struct {<br> uint32_t flreg[MAX_REGIONS];<br> } __attribute__((packed)) frba_t;<br></pre><p>To view, visit <a href="https://review.coreboot.org/21416">change 21416</a>. To unsubscribe, visit <a href="https://review.coreboot.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://review.coreboot.org/21416"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: coreboot </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: I4d34606545db4d970ce3ec202da4bf4c5cbad18c </div>
<div style="display:none"> Gerrit-Change-Number: 21416 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Bill XIE <persmule@gmail.com> </div>