Felix Held submitted this change.

View Change

Approvals: build bot (Jenkins): Verified Reka Norman: Looks good to me, approved
util/ifdtool: Add support for checking GPR0 status

This patch adds support for the new command-line option `-c` to
the ifdtool, which is able to check GPR0 (Global Protected Range)
status.

This patch also add helper function get_enabled_gprd() to get enabled
GPR0 settings. It used in enable_gpr0() and is_gpr0_protected().

Developers can use ifdtool with '-c' option to check whether GPR0 is
set to enabled or disabled in the binary file.

BUG=none
TEST=(1) > ifdtool -p mtl -E image-unlocked.bin -O image-lock.bin
...
Value at GPRD offset (64) is 0x83220004
--------- GPR0 Protected Range --------------
Start address = 0x00004000
End address = 0x00322fff
...
GPR0 protection is now enabled

(2) > ifdtool -p mtl -c image-unlocked.bin
GPR0 status: Disabled

Value at GPRD offset (64) is 0x00000000
--------- GPR0 Protected Range --------------
Start address = 0x00000000
End address = 0x00000fff

(3) > ifdtool -p mtl -c image-lock.bin
GPR0 status: Enabled

Value at GPRD offset (64) is 0x83220004
--------- GPR0 Protected Range --------------
Start address = 0x00004000
End address = 0x00322fff

Change-Id: I6b3af973be784200b965a68e5f6b7737cba03ed7
Signed-off-by: Tyler Wang <tyler.wang@quanta.corp-partner.google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/81928
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Reka Norman <rekanorman@chromium.org>
---
M util/ifdtool/ifdtool.c
1 file changed, 58 insertions(+), 16 deletions(-)

diff --git a/util/ifdtool/ifdtool.c b/util/ifdtool/ifdtool.c
index 516570e..c04d504 100644
--- a/util/ifdtool/ifdtool.c
+++ b/util/ifdtool/ifdtool.c
@@ -1746,6 +1746,22 @@
return 0;
}

+static union gprd get_enabled_gprd(char *image, int size)
+{
+ union gprd enabled_gprd_reg;
+ uint32_t gpr0_range_start, gpr0_range_end;
+ enabled_gprd_reg.value = 0;
+ if (calculate_gpr0_range(image, size, &gpr0_range_start, &gpr0_range_end))
+ exit(EXIT_FAILURE);
+
+ enabled_gprd_reg.data.start = (gpr0_range_start >> 12) & 0x7fff;
+ enabled_gprd_reg.data.end = (gpr0_range_end >> 12) & 0x7fff;
+ enabled_gprd_reg.data.read_protect_en = 0;
+ enabled_gprd_reg.data.write_protect_en = 1;
+
+ return enabled_gprd_reg;
+}
+
static void enable_gpr0(const char *filename, char *image, int size)
{
struct fpsba *fpsba = find_fpsba(image, size);
@@ -1767,23 +1783,41 @@
return;
}

- uint32_t gpr0_range_start, gpr0_range_end;
+ union gprd enabled_gprd = get_enabled_gprd(image, size);

- if (calculate_gpr0_range(image, size, &gpr0_range_start, &gpr0_range_end))
- exit(EXIT_FAILURE);
-
- reg.data.start = (gpr0_range_start >> 12) & 0x7fff;
- reg.data.end = (gpr0_range_end >> 12) & 0x7fff;
- reg.data.read_protect_en = 0;
- reg.data.write_protect_en = 1;
-
- fpsba->pchstrp[gpr0_offset] = reg.value;
- printf("Value at GPRD offset (%d) is 0x%08x\n", gpr0_offset, reg.value);
- print_gpr0_range(reg);
+ fpsba->pchstrp[gpr0_offset] = enabled_gprd.value;
+ printf("Value at GPRD offset (%d) is 0x%08x\n", gpr0_offset, enabled_gprd.value);
+ print_gpr0_range(enabled_gprd);
write_image(filename, image, size);
printf("GPR0 protection is now enabled\n");
}

+static void is_gpr0_protected(char *image, int size)
+{
+ struct fpsba *fpsba = find_fpsba(image, size);
+ if (!fpsba)
+ exit(EXIT_FAILURE);
+
+ uint32_t gpr0_offset = get_gpr0_offset();
+ if (gpr0_offset == 0xffffffff) {
+ fprintf(stderr, "Checking GPR0 not supported on this platform\n");
+ exit(EXIT_FAILURE);
+ }
+ union gprd reg;
+ union gprd enabled_gprd = get_enabled_gprd(image, size);
+ reg.value = fpsba->pchstrp[gpr0_offset];
+
+ if (fpsba->pchstrp[gpr0_offset] == enabled_gprd.value)
+ printf("GPR0 status: Enabled\n\n");
+ else if (fpsba->pchstrp[gpr0_offset] == 0)
+ printf("GPR0 status: Disabled\n\n");
+ else
+ printf("ERROR: GPR0 setting is not expected\n\n");
+
+ printf("Value at GPRD offset (%d) is 0x%08x\n", gpr0_offset, fpsba->pchstrp[gpr0_offset]);
+ print_gpr0_range(reg);
+}
+
static void set_pchstrap(struct fpsba *fpsba, const struct fdbar *fdb, const int strap,
const unsigned int value)
{
@@ -2125,6 +2159,7 @@
" -u | --unlock Unlock firmware descriptor and ME region\n"
" -g | --gpr0-disable Disable GPR0 (Global Protected Range) register\n"
" -E | --gpr0-enable Enable GPR0 (Global Protected Range) register\n"
+ " -c | --gpr0-status Checking GPR0 (Global Protected Range) register status\n"
" -M | --altmedisable <0|1> Set the MeDisable and AltMeDisable (or HAP for skylake or newer platform)\n"
" bits to disable ME\n"
" -p | --platform Add platform-specific quirks\n"
@@ -2158,7 +2193,7 @@
int mode_em100 = 0, mode_locked = 0, mode_unlocked = 0, mode_validate = 0;
int mode_layout = 0, mode_newlayout = 0, mode_density = 0, mode_setstrap = 0;
int mode_read = 0, mode_altmedisable = 0, altmedisable = 0, mode_fmap_template = 0;
- int mode_gpr0_disable = 0, mode_gpr0_enable = 0;
+ int mode_gpr0_disable = 0, mode_gpr0_enable = 0, mode_gpr0_status = 0;
char *region_type_string = NULL, *region_fname = NULL;
const char *layout_fname = NULL;
char *new_filename = NULL;
@@ -2186,6 +2221,7 @@
{"unlock", 0, NULL, 'u'},
{"gpr0-disable", 0, NULL, 'g'},
{"gpr0-enable", 0, NULL, 'E'},
+ {"gpr0-status", 0, NULL, 'c'},
{"version", 0, NULL, 'v'},
{"help", 0, NULL, 'h'},
{"platform", 1, NULL, 'p'},
@@ -2195,7 +2231,7 @@
{0, 0, 0, 0}
};

- while ((opt = getopt_long(argc, argv, "S:V:df:F:D:C:M:xi:n:O:s:p:elrugEvth?",
+ while ((opt = getopt_long(argc, argv, "S:V:df:F:D:C:M:xi:n:O:s:p:elrugEcvth?",
long_options, &option_index)) != EOF) {
switch (opt) {
case 'd':
@@ -2404,6 +2440,9 @@
case 'E':
mode_gpr0_enable = 1;
break;
+ case 'c':
+ mode_gpr0_status = 1;
+ break;
case 'p':
if (!strcmp(optarg, "aplk")) {
platform = PLATFORM_APL;
@@ -2457,7 +2496,7 @@
if ((mode_dump + mode_layout + mode_fmap_template + mode_extract + mode_inject +
mode_setstrap + mode_newlayout + (mode_spifreq | mode_em100 |
mode_unlocked | mode_locked) + mode_altmedisable + mode_validate +
- (mode_gpr0_disable | mode_gpr0_enable)) > 1) {
+ (mode_gpr0_disable | mode_gpr0_enable) + mode_gpr0_status) > 1) {
fprintf(stderr, "You may not specify more than one mode.\n\n");
fprintf(stderr, "run '%s -h' for usage\n", argv[0]);
exit(EXIT_FAILURE);
@@ -2466,7 +2505,7 @@
if ((mode_dump + mode_layout + mode_fmap_template + mode_extract + mode_inject +
mode_setstrap + mode_newlayout + mode_spifreq + mode_em100 +
mode_locked + mode_unlocked + mode_density + mode_altmedisable +
- mode_validate + (mode_gpr0_disable | mode_gpr0_enable)) == 0) {
+ mode_validate + (mode_gpr0_disable | mode_gpr0_enable) + mode_gpr0_status) == 0) {
fprintf(stderr, "You need to specify a mode.\n\n");
fprintf(stderr, "run '%s -h' for usage\n", argv[0]);
exit(EXIT_FAILURE);
@@ -2569,6 +2608,9 @@
if (mode_gpr0_enable)
enable_gpr0(new_filename, image, size);

+ if (mode_gpr0_status)
+ is_gpr0_protected(image, size);
+
if (mode_setstrap) {
struct fpsba *fpsba = find_fpsba(image, size);
const struct fdbar *fdb = find_fd(image, size);

To view, visit change 81928. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I6b3af973be784200b965a68e5f6b7737cba03ed7
Gerrit-Change-Number: 81928
Gerrit-PatchSet: 13
Gerrit-Owner: Tyler Wang <tyler.wang@quanta.corp-partner.google.com>
Gerrit-Reviewer: David Wu <david_wu@quanta.corp-partner.google.com>
Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de>
Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d@intel.com>
Gerrit-Reviewer: Reka Norman <rekanorman@chromium.org>
Gerrit-Reviewer: Reka Norman <rekanorman@google.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer@coreboot.org>
Gerrit-Reviewer: Subrata Banik <subratabanik@google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-MessageType: merged