Nick Vaccaro has submitted this change. ( https://review.coreboot.org/c/coreboot/+/58351 )
Change subject: soc/intel/common: add generic gpio lock mechanism ......................................................................
soc/intel/common: add generic gpio lock mechanism
For added security, there are some gpios that an SoC will want to lock once initially configured, such as gpios attached to non-host (x86) controllers, so that they can't be recofigured at a later point in time by rogue code.
Likewise, a mainboard may have some gpios connected to secure busses and/or devices that they want to protect from being changed post initial configuration.
This change adds a generic gpio locking mechanism that allows the SoC to export a list of GPIOs to be locked down and allows the mainboard to export a list of GPIOs that it wants locked down once initialization is complete.
Use the SOC_INTEL_COMMON_BLOCK_SMM_LOCK_GPIO_PADS Kconfig option to enable this feature.
BUG=b:201430600 TEST='emerge-brya coreboot chromeos-bootimage', flash and verify brya0 boots successfully to kernel.
Signed-off-by: Nick Vaccaro nvaccaro@google.com Change-Id: I42979fb89567d8bcd9392da4fb8c4113ef427b14 Reviewed-on: https://review.coreboot.org/c/coreboot/+/58351 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/soc/intel/common/block/gpio/gpio.c M src/soc/intel/common/block/include/intelblocks/gpio.h M src/soc/intel/common/block/smm/Kconfig M src/soc/intel/common/block/smm/smihandler.c 4 files changed, 154 insertions(+), 40 deletions(-)
Approvals: build bot (Jenkins): Verified Tim Wawrzynczak: Looks good to me, approved
diff --git a/src/soc/intel/common/block/gpio/gpio.c b/src/soc/intel/common/block/gpio/gpio.c index d4a312b..ce23a2e 100644 --- a/src/soc/intel/common/block/gpio/gpio.c +++ b/src/soc/intel/common/block/gpio/gpio.c @@ -447,14 +447,27 @@ return !!(reg & PAD_CFG0_RX_STATE); }
-int gpio_lock_pad(const gpio_t pad, enum gpio_lock_action action) +static int sideband_msg_err(int status, int response) { - const struct pad_community *comm = gpio_get_community(pad); - size_t rel_pad; - uint16_t offset; - uint32_t data; + if (status || response) { + printk(BIOS_ERR, "%s: error status=%x response=%x\n", + __func__, status, response); + return (status == -1) ? -1 : response; + } + + return 0; +} + +int gpio_lock_pads(const struct gpio_lock_config *pad_list, const size_t count) +{ + const struct pad_community *comm; + enum gpio_lock_action action; + int status, err_response = 0; uint8_t response; - int status; + uint16_t offset; + size_t rel_pad; + uint32_t data; + gpio_t pad;
/* * FSP-S will unlock all the GPIO pads and hide the P2SB device. With @@ -468,23 +481,13 @@ return -1; }
- if (!(action & GPIO_LOCK_FULL)) { - printk(BIOS_ERR, "%s: Error: no action specified!\n", __func__); + if ((pad_list == NULL) || (count == 0)) { + printk(BIOS_ERR, "%s: Error: pad_list null or count = 0!\n", __func__); return -1; }
- rel_pad = relative_pad_in_comm(comm, pad); - offset = comm->pad_cfg_lock_offset; - if (!offset) { - printk(BIOS_ERR, "%s: Error: offset is not defined!\n", __func__); - return -1; - } - offset += gpio_group_index_scaled(comm, rel_pad, 2 * sizeof(uint32_t)); - /* We must use the sideband interface in order to lock the pad. */ struct pcr_sbi_msg msg = { - .pid = comm->port, - .offset = offset, .opcode = GPIO_LOCK_UNLOCK, .is_posted = false, .fast_byte_enable = 0xF, @@ -494,35 +497,69 @@
p2sb_unhide();
- data = gpio_bitmask_within_group(comm, rel_pad); + for (int x = 0; x < count; x++) { + int err;
- if (action & GPIO_LOCK_CONFIG) { - printk(BIOS_INFO, "%s: Locking pad %d configuration\n", - __func__, pad); - status = pcr_execute_sideband_msg(&msg, &data, &response); - if (status || response) { - printk(BIOS_ERR, "%s: error status=%x response=%x\n", __func__, status, - response); - p2sb_hide(); - return status == -1 ? -1 : response; + pad = pad_list[x].gpio; + action = pad_list[x].action; + + if (!(action & GPIO_LOCK_FULL)) { + printk(BIOS_ERR, "%s: Error: no action specified for pad %d!\n", + __func__, pad); + continue; } - }
- if (action & GPIO_LOCK_TX) { - printk(BIOS_INFO, "%s: Locking pad %d TX state\n", __func__, - pad); - msg.offset = msg.offset + 4; - status = pcr_execute_sideband_msg(&msg, &data, &response); - if (status || response) { - printk(BIOS_ERR, "%s: error status=%x response=%x\n", __func__, status, - response); - p2sb_hide(); - return status == -1 ? -1 : response; + comm = gpio_get_community(pad); + rel_pad = relative_pad_in_comm(comm, pad); + offset = comm->pad_cfg_lock_offset; + if (!offset) { + printk(BIOS_ERR, "%s: Error: offset not defined for pad %d!\n", + __func__, pad); + continue; + } + offset += gpio_group_index_scaled(comm, rel_pad, 2 * sizeof(uint32_t)); + + data = gpio_bitmask_within_group(comm, rel_pad); + msg.pid = comm->port; + msg.offset = offset; + + if (action & GPIO_LOCK_CONFIG) { + if (CONFIG(DEBUG_GPIO)) + printk(BIOS_INFO, "%s: Locking pad %d configuration\n", + __func__, pad); + status = pcr_execute_sideband_msg(&msg, &data, &response); + if ((err = sideband_msg_err(status, response)) != 0) { + err_response = err; + continue; + } + } + + if (action & GPIO_LOCK_TX) { + if (CONFIG(DEBUG_GPIO)) + printk(BIOS_INFO, "%s: Locking pad %d TX state\n", + __func__, pad); + msg.offset += 4; + status = pcr_execute_sideband_msg(&msg, &data, &response); + if ((err = sideband_msg_err(status, response)) != 0) { + err_response = err; + continue; + } } }
p2sb_hide(); - return 0; + + return err_response; +} + +int gpio_lock_pad(const gpio_t pad, enum gpio_lock_action action) +{ + const struct gpio_lock_config pads = { + .gpio = pad, + .action = action + }; + + return gpio_lock_pads(&pads, 1); }
void gpio_set(gpio_t gpio_num, int value) diff --git a/src/soc/intel/common/block/include/intelblocks/gpio.h b/src/soc/intel/common/block/include/intelblocks/gpio.h index c47f908..91f9abe 100644 --- a/src/soc/intel/common/block/include/intelblocks/gpio.h +++ b/src/soc/intel/common/block/include/intelblocks/gpio.h @@ -205,6 +205,11 @@ GPIO_LOCK_FULL = GPIO_LOCK_CONFIG | GPIO_LOCK_TX, };
+struct gpio_lock_config { + gpio_t gpio; + enum gpio_lock_action action; +}; + /* * Lock a GPIO's configuration. * @@ -232,6 +237,33 @@ int gpio_lock_pad(const gpio_t pad, enum gpio_lock_action action);
/* + * gpio_lock_pads() can be used to lock an array of gpio pads, avoiding + * the p2sb_unhide() and p2sb_hide() calls between each gpio lock that would + * occur if gpio_lock_pad() were used to lock each pad in the list. + * + * @param pad_list: array of gpio_lock_config structures, one for each gpio to lock + * @param count: number of gpio_lock_config structs in the pad_list array + * @return 0 if successful, + * 1 - unsuccessful + * 2 - powered down + * 3 - multi-cast mixed + * -1 - sideband message failed or other error + */ +int gpio_lock_pads(const struct gpio_lock_config *pad_list, const size_t count); + +/* + * Returns an array of gpio_lock_config entries that the SoC + * deems security risks that should be locked down. + */ +const struct gpio_lock_config *soc_gpio_lock_config(size_t *num); + +/* + * Returns an array of gpio_lock_config entries that the mainboard + * deems security risks that should be locked down. + */ +const struct gpio_lock_config *mb_gpio_lock_config(size_t *num); + +/* * Returns the pmc_gpe to gpio_gpe mapping table * */ diff --git a/src/soc/intel/common/block/smm/Kconfig b/src/soc/intel/common/block/smm/Kconfig index 77ba00c..2d960d6 100644 --- a/src/soc/intel/common/block/smm/Kconfig +++ b/src/soc/intel/common/block/smm/Kconfig @@ -8,6 +8,13 @@ help Intel Processor trap flag if it is supported
+config SOC_INTEL_COMMON_BLOCK_SMM_LOCK_GPIO_PADS + bool + help + Enable locking of security-sensitive SoC and mainboard GPIOs. + An SoC may provide a list of gpios to lock, and the mainboard + may also provide a list of gpios to lock. + config SOC_INTEL_COMMON_BLOCK_SMM_ESPI_DISABLE bool default n diff --git a/src/soc/intel/common/block/smm/smihandler.c b/src/soc/intel/common/block/smm/smihandler.c index e1eadb6..32de2f6 100644 --- a/src/soc/intel/common/block/smm/smihandler.c +++ b/src/soc/intel/common/block/smm/smihandler.c @@ -311,6 +311,40 @@ } }
+__weak const struct gpio_lock_config *soc_gpio_lock_config(size_t *num) +{ + *num = 0; + return NULL; +} + +__weak const struct gpio_lock_config *mb_gpio_lock_config(size_t *num) +{ + *num = 0; + return NULL; +} + +static void soc_lock_gpios(void) +{ + const struct gpio_lock_config *soc_gpios; + const struct gpio_lock_config *mb_gpios; + size_t soc_gpio_num; + size_t mb_gpio_num; + + /* get list of gpios from SoC */ + soc_gpios = soc_gpio_lock_config(&soc_gpio_num); + + /* get list of gpios from mainboard */ + mb_gpios = mb_gpio_lock_config(&mb_gpio_num); + + /* Lock any soc requested gpios */ + if (soc_gpio_num) + gpio_lock_pads(soc_gpios, soc_gpio_num); + + /* Lock any mainboard requested gpios */ + if (mb_gpio_num) + gpio_lock_pads(mb_gpios, mb_gpio_num); +} + static void finalize(void) { static int finalize_done; @@ -337,6 +371,10 @@ */ mainboard_smi_finalize();
+ /* Lock down all GPIOs that may have been requested by the SoC and/or the mainboard. */ + if (CONFIG(SOC_INTEL_COMMON_BLOCK_SMM_LOCK_GPIO_PADS)) + soc_lock_gpios(); + /* Specific SOC SMI handler during ramstage finalize phase */ smihandler_soc_at_finalize(); }
-
Nick Vaccaro (Code Review)