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