From b4691fe734f0525bdfcb8d64f305954f42b88351 Mon Sep 17 00:00:00 2001 From: Randall Spangler Date: Wed, 26 Jul 2017 13:00:37 -0700 Subject: [PATCH] cr50: CCD V1 controls SPI access SPI access now depends on CCD_CAP_AP_FLASH and CCD_CAP_EC_FLASH. usb_spi_state.enabled_host and .enabled_device are now bitfields which depend on which SPI interface is enabled. This was implied before by a single & comparing enabled_host to enabled_device, but is now explicit so that the device can decide to enable just a subset of buses. BUG=b:62537474 BRANCH=cr50 BRANCH=cr50 TEST=manual with CR50_DEV=1 Connect host PC to dev board USB port On host PC: sudo servod -c ccd_cr50.xml -c reef_r1_inas.xml In test protocol below, (test EC) means this command: sudo flashrom -p raiden_debug_spi:target=EC --wp-status And (test AP) means this command: sudo flashrom -p raiden_debug_spi:target=AP --wp-status "pass" means no console warning about "SPI access denied" "fail" means console warnings about "SPI access denied" To get even more confirmation, in chip/g/usb_spi.c temporarily put this debug statement at the end of usb_spi_deferred(): CPRINTS("SPI res=%d", (int)res); Pass is res=0, fail is res=5. ccdoops (test AP) --> pass (test EC) --> pass ccdunlock (test AP) --> fail (test EC) --> fail ccdoops ccdset flashap unlesslocked ccdunlock (test AP) --> pass (test EC) --> fail ccdoops ccdset flashec unlesslocked ccdunlock (test AP) --> fail (test EC) --> pass Change-Id: I3d37d088b748832f164f2ca0ff29a93d6532ebed Signed-off-by: Randall Spangler Reviewed-on: https://chromium-review.googlesource.com/590858 Reviewed-by: Aseda Aboagye --- board/cr50/usb_spi.c | 22 +++++++++++++--------- chip/g/usb_spi.c | 17 ++++++++++++++--- chip/g/usb_spi.h | 9 +++++---- 3 files changed, 32 insertions(+), 16 deletions(-) diff --git a/board/cr50/usb_spi.c b/board/cr50/usb_spi.c index 5ae47d7dbe..29452104dc 100644 --- a/board/cr50/usb_spi.c +++ b/board/cr50/usb_spi.c @@ -3,6 +3,7 @@ * found in the LICENSE file. */ +#include "case_closed_debug.h" #include "console.h" #include "gpio.h" #include "hooks.h" @@ -50,19 +51,21 @@ static void enable_ap_spi(void) int usb_spi_board_enable(struct usb_spi_config const *config) { - /* Prevent SPI access if the console is currently locked. */ - if (console_is_restricted()) { - CPRINTS("usb_spi access denied (console is restricted."); - return EC_ERROR_ACCESS_DENIED; - } - disable_ec_ap_spi(); - if (config->state->enabled_host == USB_SPI_EC) + if (config->state->enabled_host == USB_SPI_EC) { + if (!ccd_is_cap_enabled(CCD_CAP_EC_FLASH)) { + CPRINTS("EC SPI access denied"); + return EC_ERROR_ACCESS_DENIED; + } enable_ec_spi(); - else if (config->state->enabled_host == USB_SPI_AP) + } else if (config->state->enabled_host == USB_SPI_AP) { + if (!ccd_is_cap_enabled(CCD_CAP_AP_FLASH)) { + CPRINTS("AP SPI access denied"); + return EC_ERROR_ACCESS_DENIED; + } enable_ap_spi(); - else { + } else { CPRINTS("DEVICE NOT SUPPORTED"); return EC_ERROR_INVAL; } @@ -128,6 +131,7 @@ int usb_spi_interface(struct usb_spi_config const *config, break; case USB_SPI_REQ_ENABLE: CPRINTS("ERROR: Must specify target"); + /* Fall through... */ case USB_SPI_REQ_DISABLE: config->state->enabled_host = USB_SPI_DISABLE; break; diff --git a/chip/g/usb_spi.c b/chip/g/usb_spi.c index 55259b2fc8..140f3c8a28 100644 --- a/chip/g/usb_spi.c +++ b/chip/g/usb_spi.c @@ -3,6 +3,7 @@ * found in the LICENSE file. */ +#include "case_closed_debug.h" #include "common.h" #include "link_defs.h" #include "gpio.h" @@ -55,8 +56,8 @@ void usb_spi_deferred(struct usb_spi_config const *config) * If our overall enabled state has changed we call the board specific * enable or disable routines and save our new state. */ - int enabled = (config->state->enabled_host & - config->state->enabled_device); + int enabled = !!(config->state->enabled_host & + config->state->enabled_device); if (enabled ^ config->state->enabled) { if (enabled) @@ -130,7 +131,17 @@ struct consumer_ops const usb_spi_consumer_ops = { void usb_spi_enable(struct usb_spi_config const *config, int enabled) { - config->state->enabled_device = enabled ? 0xf : 0; + config->state->enabled_device = 0; + if (enabled) { +#ifdef CONFIG_CASE_CLOSED_DEBUG_V1 + if (ccd_is_cap_enabled(CCD_CAP_AP_FLASH)) + config->state->enabled_device |= USB_SPI_AP; + if (ccd_is_cap_enabled(CCD_CAP_EC_FLASH)) + config->state->enabled_device |= USB_SPI_EC; +#else + config->state->enabled_device = USB_SPI_ALL; +#endif + } hook_call_deferred(config->deferred, 0); } diff --git a/chip/g/usb_spi.h b/chip/g/usb_spi.h index 2240999950..b852310f1e 100644 --- a/chip/g/usb_spi.h +++ b/chip/g/usb_spi.h @@ -72,12 +72,13 @@ enum usb_spi_request { USB_SPI_REQ_SOCKET = 0x0007, }; -/* USB SPI device indexes */ +/* USB SPI device bitmasks */ enum usb_spi { USB_SPI_DISABLE = 0, - USB_SPI_AP, - USB_SPI_EC, - USB_SPI_H1, + USB_SPI_AP = (1 << 0), + USB_SPI_EC = (1 << 1), + USB_SPI_H1 = (1 << 2), + USB_SPI_ALL = USB_SPI_AP | USB_SPI_EC | USB_SPI_H1 };