I just realized that I missed a few questions on my first reply. Please see my responses below.
On 10/7/20 5:52 PM, Russ Weight wrote: > > On 10/6/20 9:34 AM, Tom Rix wrote: >> On 10/2/20 6:24 PM, Russ Weight wrote: >>> Add macros and definitions required by the MAX10 BMC >>> Security Engine driver. >>> >>> Signed-off-by: Russ Weight <[email protected]> >>> --- >>> v2: >>> - These functions and macros were previously distributed among >>> the patches that needed them. They are now grouped together >>> in a single patch containing changes to the Intel MAX10 BMC >>> driver. >>> - Added DRBL_ prefix to some definitions >>> - Some address definitions were moved here from the .c files that >>> use them. >>> --- >>> include/linux/mfd/intel-m10-bmc.h | 134 ++++++++++++++++++++++++++++++ >>> 1 file changed, 134 insertions(+) >>> >>> diff --git a/include/linux/mfd/intel-m10-bmc.h >>> b/include/linux/mfd/intel-m10-bmc.h >>> index c8ef2f1654a4..880f907302eb 100644 >>> --- a/include/linux/mfd/intel-m10-bmc.h >>> +++ b/include/linux/mfd/intel-m10-bmc.h >>> @@ -13,6 +13,9 @@ >>> #define M10BMC_SYS_BASE 0x300800 >>> #define M10BMC_MEM_END 0x200000fc >>> >>> +#define M10BMC_STAGING_BASE 0x18000000 >>> +#define M10BMC_STAGING_SIZE 0x3800000 >> The staging size is not used, please use it in m10bmc_sec_write_blk to >> >> check the input parameter 'size' > It is used to check the input size in the prepare function: > m10bmc_sec_prepare() > > if (smgr->remaining_size > M10BMC_STAGING_SIZE) > return FPGA_SEC_ERR_INVALID_SIZE; > > - Russ > >>> + >>> /* Register offset of system registers */ >>> #define NIOS2_FW_VERSION 0x0 >>> #define M10BMC_TEST_REG 0x3c >>> @@ -21,6 +24,88 @@ >>> #define M10BMC_VER_PCB_INFO_MSK GENMASK(31, 24) >>> #define M10BMC_VER_LEGACY_INVALID 0xffffffff >>> >>> +/* Secure update doorbell register, in system register region */ >>> +#define M10BMC_DOORBELL 0x400 >>> + >>> +/* Authorization Result register, in system register region */ >>> +#define M10BMC_AUTH_RESULT 0x404 >>> + >>> +/* Doorbell register fields */ >>> +#define DRBL_RSU_REQUEST BIT(0) >>> +#define DRBL_RSU_PROGRESS GENMASK(7, 4) >>> +#define DRBL_HOST_STATUS GENMASK(11, 8) >>> +#define DRBL_RSU_STATUS GENMASK(23, 16) >>> +#define DRBL_PKVL_EEPROM_LOAD_SEC BIT(24) >>> +#define DRBL_PKVL1_POLL_EN BIT(25) >>> +#define DRBL_PKVL2_POLL_EN BIT(26) >> PKVL seems like it would be n3000 specific. >> >> For this and similar it may be good to add a _N3000_ in the name. I'm hoping Yilun and Hao might chime in here. We have substituted PKVL for Retimer in some places in the code. >> >>> +#define DRBL_CONFIG_SEL BIT(28) >>> +#define DRBL_REBOOT_REQ BIT(29) >>> +#define DRBL_REBOOT_DISABLED BIT(30) >>> + >>> +/* Progress states */ >>> +#define RSU_PROG_IDLE 0x0 >>> +#define RSU_PROG_PREPARE 0x1 >>> +#define RSU_PROG_READY 0x3 >>> +#define RSU_PROG_AUTHENTICATING 0x4 >>> +#define RSU_PROG_COPYING 0x5 >>> +#define RSU_PROG_UPDATE_CANCEL 0x6 >>> +#define RSU_PROG_PROGRAM_KEY_HASH 0x7 >>> +#define RSU_PROG_RSU_DONE 0x8 >>> +#define RSU_PROG_PKVL_PROM_DONE 0x9 >>> + >>> +/* Device and error states */ >>> +#define RSU_STAT_NORMAL 0x0 >>> +#define RSU_STAT_TIMEOUT 0x1 >>> +#define RSU_STAT_AUTH_FAIL 0x2 >>> +#define RSU_STAT_COPY_FAIL 0x3 >>> +#define RSU_STAT_FATAL 0x4 >>> +#define RSU_STAT_PKVL_REJECT 0x5 >>> +#define RSU_STAT_NON_INC 0x6 >>> +#define RSU_STAT_ERASE_FAIL 0x7 >>> +#define RSU_STAT_WEAROUT 0x8 >>> +#define RSU_STAT_NIOS_OK 0x80 >>> +#define RSU_STAT_USER_OK 0x81 >>> +#define RSU_STAT_FACTORY_OK 0x82 >>> +#define RSU_STAT_USER_FAIL 0x83 >>> +#define RSU_STAT_FACTORY_FAIL 0x84 >>> +#define RSU_STAT_NIOS_FLASH_ERR 0x85 >>> +#define RSU_STAT_FPGA_FLASH_ERR 0x86 >>> + >>> +#define HOST_STATUS_IDLE 0x0 >>> +#define HOST_STATUS_WRITE_DONE 0x1 >>> +#define HOST_STATUS_ABORT_RSU 0x2 >>> + >>> +#define rsu_prog(doorbell) FIELD_GET(DRBL_RSU_PROGRESS, doorbell) >>> +#define rsu_stat(doorbell) FIELD_GET(DRBL_RSU_STATUS, doorbell) >>> + >>> +/* interval 100ms and timeout 5s */ >>> +#define NIOS_HANDSHAKE_INTERVAL_US (100 * 1000) >>> +#define NIOS_HANDSHAKE_TIMEOUT_US (5 * 1000 * 1000) >>> + >>> +/* RSU PREP Timeout (2 minutes) to erase flash staging area */ >>> +#define RSU_PREP_INTERVAL_MS 100 >>> +#define RSU_PREP_TIMEOUT_MS (2 * 60 * 1000) >>> + >>> +/* RSU Complete Timeout (40 minutes) for full flash update */ >>> +#define RSU_COMPLETE_INTERVAL_MS 1000 >>> +#define RSU_COMPLETE_TIMEOUT_MS (40 * 60 * 1000) >> minutes is an unusual timeout unit. >> >> It may be worthwhile to spell out MINUTES to avoid confusing with micro >> seconds. The _MS at the end of the constants means milliseconds. That is pretty standard, isn't it? Although the wait is 40 minutes, the constant is a millisecond count that corresponds to 40 minutes. I expanded the definition to make things more clear: 40 (minutes) * 60 (seconds) * 1000 milliseconds = (40 minutes expressed in milliseconds). >> Tom >> >>> + >>> +/* Addresses for security related data in FLASH */ >>> +#define BMC_REH_ADDR 0x17ffc004 >>> +#define BMC_PROG_ADDR 0x17ffc000 >>> +#define BMC_PROG_MAGIC 0x5746 >>> + >>> +#define SR_REH_ADDR 0x17ffd004 >>> +#define SR_PROG_ADDR 0x17ffd000 >>> +#define SR_PROG_MAGIC 0x5253 >>> + >>> +#define PR_REH_ADDR 0x17ffe004 >>> +#define PR_PROG_ADDR 0x17ffe000 >>> +#define PR_PROG_MAGIC 0x5250 >>> + >>> +/* Address of inverted bit vector containing user the image FLASH count */ >>> +#define USER_FLASH_COUNT 0x17ffb000 >>> + >>> /** >>> * struct intel_m10bmc - Intel MAX 10 BMC parent driver data structure >>> * @dev: this device >>> @@ -35,7 +120,11 @@ struct intel_m10bmc { >>> * register access helper functions. >>> * >>> * m10bmc_raw_read - read m10bmc register per addr >>> + * m10bmc_raw_bulk_read - bulk read max10 registers per addr >>> + * m10bmc_raw_bulk_write - bulk write max10 registers per addr >>> + * m10bmc_raw_update_bits - update max10 register per addr >>> * m10bmc_sys_read - read m10bmc system register per offset >>> + * m10bmc_sys_update_bits - update max10 system register per offset >>> */ >>> static inline int >>> m10bmc_raw_read(struct intel_m10bmc *m10bmc, unsigned int addr, >>> @@ -51,6 +140,48 @@ m10bmc_raw_read(struct intel_m10bmc *m10bmc, unsigned >>> int addr, >>> return ret; >>> } >>> >>> +static inline int >>> +m10bmc_raw_bulk_read(struct intel_m10bmc *m10bmc, unsigned int addr, >>> + void *val, size_t cnt) >>> +{ >>> + int ret; >>> + >>> + ret = regmap_bulk_read(m10bmc->regmap, addr, val, cnt); >>> + if (ret) >>> + dev_err(m10bmc->dev, "fail to read raw reg %x cnt %zx: %d\n", >>> + addr, cnt, ret); >>> + >>> + return ret; >>> +} >>> + >>> +static inline int >>> +m10bmc_raw_bulk_write(struct intel_m10bmc *m10bmc, unsigned int addr, >>> + void *val, size_t cnt) >>> +{ >>> + int ret; >>> + >>> + ret = regmap_bulk_write(m10bmc->regmap, addr, val, cnt); >>> + if (ret) >>> + dev_err(m10bmc->dev, "fail to write raw reg %x cnt %zx: %d\n", >>> + addr, cnt, ret); >>> + >>> + return ret; >>> +} >>> + >>> +static inline int >>> +m10bmc_raw_update_bits(struct intel_m10bmc *m10bmc, unsigned int addr, >>> + unsigned int msk, unsigned int val) >>> +{ >>> + int ret; >>> + >>> + ret = regmap_update_bits(m10bmc->regmap, addr, msk, val); >>> + if (ret) >>> + dev_err(m10bmc->dev, "fail to update raw reg %x: %d\n", >>> + addr, ret); >>> + >>> + return ret; >>> +} >>> + >>> /* >>> * The base of the system registers could be configured by HW developers, >>> and >>> * in HW SPEC, the base is not added to the addresses of the system >>> registers. >>> @@ -62,4 +193,7 @@ m10bmc_raw_read(struct intel_m10bmc *m10bmc, unsigned >>> int addr, >>> #define m10bmc_sys_read(m10bmc, offset, val) \ >>> m10bmc_raw_read(m10bmc, M10BMC_SYS_BASE + (offset), val) >>> >>> +#define m10bmc_sys_update_bits(m10bmc, offset, msk, val) \ >>> + m10bmc_raw_update_bits(m10bmc, M10BMC_SYS_BASE + (offset), msk, val) >>> + >>> #endif /* __MFD_INTEL_M10_BMC_H */
