On 11/14/2019 11:05 PM, Zhang, Tianfei wrote:
>
>> -----Original Message-----
>> From: Xu, Rosen
>> Sent: Thursday, November 14, 2019 5:03 PM
>> To: dev@dpdk.org
>> Cc: Xu, Rosen <rosen...@intel.com>; Zhang, Tianfei <tianfei.zh...@intel.com>;
>> Pei, Andy <andy....@intel.com>; Ye, Xiaolong <xiaolong...@intel.com>; Yigit,
>> Ferruh <ferruh.yi...@intel.com>
>> Subject: [PATCH v18 13/19] raw/ifpga/base: add secure support
>>
>> From: Tianfei zhang <tianfei.zh...@intel.com>
>>
>> Add secure max10 device support.
>
> In PAC N3000 Card, it implements the secure functionality on the MAX10 Board
> Management Controller (BMC) as Root of Trust (RoT). It changes to MAX10 (RTL
> and Nios FW) to enable secure RSU (Remote System Update) authentication and
> integrity checks for FPGA Flat image, and FW updates to the card. The card's
> BMC continues to support features such as power sequence management, board
> monitoring via sensors, JTAG management and in-band SPI interface access. The
> external Flash on the card shall be programmed with the Intel root public key
> hash (for BMC images and FW) and Customer/User key (for FPGA Flat image)
> during manufacturing, so the image updates (RSU) shall be verified using
> ECDSA-256 P-256 and SHA2-256 before being written to the MAX10 Image Flash or
> FPGA Image Flash.
>
> This patch add RoT support for MAX10 because some registers and the content
> of Device Tree have changed between RoT solution and Non-RoT solution.
Thanks Tianfei for the additional information, I will update commit as
following, please let me know if you have any objection:
raw/ifpga/base: support max10 security feature
In PAC N3000 Card, MAX10 Board Management Controller (BMC) implements
the security functionality.
Security functionality adds secure Remote System Update (RSU)
authentication and integrity checks for FPGA flat image,
and FW updates to the card.
This patch adds security feature support for MAX10, in secure solution
some registers and the content of the Device Tree changes.
>
>
>>
>> Signed-off-by: Tianfei zhang <tianfei.zh...@intel.com>
>> Signed-off-by: Andy Pei <andy....@intel.com>
>> ---
>> drivers/raw/ifpga/base/ifpga_defines.h | 2 +
>> drivers/raw/ifpga/base/ifpga_fme.c | 26 ++++--
>> drivers/raw/ifpga/base/opae_intel_max10.c | 137
>> +++++++++++++++++++++++++-----
>> drivers/raw/ifpga/base/opae_intel_max10.h | 80 ++++++++++++-----
>> 4 files changed, 198 insertions(+), 47 deletions(-)
>>
>> diff --git a/drivers/raw/ifpga/base/ifpga_defines.h
>> b/drivers/raw/ifpga/base/ifpga_defines.h
>> index 8993cc6..1e84b15 100644
>> --- a/drivers/raw/ifpga/base/ifpga_defines.h
>> +++ b/drivers/raw/ifpga/base/ifpga_defines.h
>> @@ -1698,6 +1698,8 @@ struct ifpga_fme_board_info {
>> u32 patch_version;
>> u32 minor_version;
>> u32 major_version;
>> + u32 max10_version;
>> + u32 nios_fw_version;
>> u32 nums_of_retimer;
>> u32 ports_per_retimer;
>> u32 nums_of_fvl;
>> diff --git a/drivers/raw/ifpga/base/ifpga_fme.c
>> b/drivers/raw/ifpga/base/ifpga_fme.c
>> index 794ca09..87fa596 100644
>> --- a/drivers/raw/ifpga/base/ifpga_fme.c
>> +++ b/drivers/raw/ifpga/base/ifpga_fme.c
>> @@ -825,6 +825,7 @@ static int board_type_to_info(u32 type, static int
>> fme_get_board_interface(struct ifpga_fme_hw *fme) {
>> struct fme_bitstream_id id;
>> + u32 val;
>>
>> if (fme_hdr_get_bitstream_id(fme, &id.id))
>> return -EINVAL;
>> @@ -850,6 +851,18 @@ static int fme_get_board_interface(struct
>> ifpga_fme_hw *fme)
>> fme->board_info.nums_of_fvl,
>> fme->board_info.ports_per_fvl);
>>
>> + if (max10_sys_read(MAX10_BUILD_VER, &val))
>> + return -EINVAL;
>> + fme->board_info.max10_version = val & 0xffffff;
>> +
>> + if (max10_sys_read(NIOS2_FW_VERSION, &val))
>> + return -EINVAL;
>> + fme->board_info.nios_fw_version = val & 0xffffff;
>> +
>> + dev_info(fme, "max10 version 0x%x, nios fw version 0x%x\n",
>> + fme->board_info.max10_version,
>> + fme->board_info.nios_fw_version);
>> +
>> return 0;
>> }
>>
>> @@ -858,16 +871,11 @@ static int spi_self_checking(void)
>> u32 val;
>> int ret;
>>
>> - ret = max10_reg_read(0x30043c, &val);
>> + ret = max10_sys_read(MAX10_TEST_REG, &val);
>> if (ret)
>> return -EIO;
>>
>> - if (val != 0x87654321) {
>> - dev_err(NULL, "Read MAX10 test register fail: 0x%x\n", val);
>> - return -EIO;
>> - }
>> -
>> - dev_info(NULL, "Read MAX10 test register success, SPI self-test
>> done\n");
>> + dev_info(NULL, "Read MAX10 test register 0x%x\n", val);
>>
>> return 0;
>> }
>> @@ -1283,7 +1291,7 @@ int fme_mgr_get_retimer_status(struct
>> ifpga_fme_hw *fme,
>> if (!dev)
>> return -ENODEV;
>>
>> - if (max10_reg_read(PKVL_LINK_STATUS, &val)) {
>> + if (max10_sys_read(PKVL_LINK_STATUS, &val)) {
>> dev_err(dev, "%s: read pkvl status fail\n", __func__);
>> return -EINVAL;
>> }
>> @@ -1311,7 +1319,7 @@ int fme_mgr_get_sensor_value(struct ifpga_fme_hw
>> *fme,
>> if (!dev)
>> return -ENODEV;
>>
>> - if (max10_reg_read(sensor->value_reg, value)) {
>> + if (max10_sys_read(sensor->value_reg, value)) {
>> dev_err(dev, "%s: read sensor value register 0x%x fail\n",
>> __func__, sensor->value_reg);
>> return -EINVAL;
>> diff --git a/drivers/raw/ifpga/base/opae_intel_max10.c
>> b/drivers/raw/ifpga/base/opae_intel_max10.c
>> index ae7a8df..748ab56 100644
>> --- a/drivers/raw/ifpga/base/opae_intel_max10.c
>> +++ b/drivers/raw/ifpga/base/opae_intel_max10.c
>> @@ -30,6 +30,22 @@ int max10_reg_write(unsigned int reg, unsigned int val)
>> reg, 4, (unsigned char *)&tmp);
>> }
>>
>> +int max10_sys_read(unsigned int offset, unsigned int *val) {
>> + if (!g_max10)
>> + return -ENODEV;
>> +
>> + return max10_reg_read(g_max10->base + offset, val); }
>> +
>> +int max10_sys_write(unsigned int offset, unsigned int val) {
>> + if (!g_max10)
>> + return -ENODEV;
>> +
>> + return max10_reg_write(g_max10->base + offset, val); }
>> +
>> static struct max10_compatible_id max10_id_table[] = {
>> {.compatible = MAX10_PAC,},
>> {.compatible = MAX10_PAC_N3000,},
>> @@ -66,7 +82,8 @@ static void max10_check_capability(struct
>> intel_max10_device *max10)
>> max10->flags |= MAX10_FLAGS_NO_I2C2 |
>> MAX10_FLAGS_NO_BMCIMG_FLASH;
>> dev_info(max10, "found %s card\n", max10->id->compatible);
>> - }
>> + } else
>> + max10->flags |= MAX10_FLAGS_MAC_CACHE;
>> }
>>
>> static int altera_nor_flash_read(u32 offset, @@ -100,7 +117,7 @@ static int
>> enable_nor_flash(bool on)
>> unsigned int val = 0;
>> int ret;
>>
>> - ret = max10_reg_read(RSU_REG_OFF, &val);
>> + ret = max10_sys_read(RSU_REG, &val);
>> if (ret) {
>> dev_err(NULL "enabling flash error\n");
>> return ret;
>> @@ -111,7 +128,7 @@ static int enable_nor_flash(bool on)
>> else
>> val &= ~RSU_ENABLE;
>>
>> - return max10_reg_write(RSU_REG_OFF, val);
>> + return max10_sys_write(RSU_REG, val);
>> }
>>
>> static int init_max10_device_table(struct intel_max10_device *max10) @@
>> -123,7 +140,7 @@ static int init_max10_device_table(struct
>> intel_max10_device *max10)
>> u32 dt_size, dt_addr, val;
>> int ret;
>>
>> - ret = max10_reg_read(DT_AVAIL_REG_OFF, &val);
>> + ret = max10_sys_read(DT_AVAIL_REG, &val);
>> if (ret) {
>> dev_err(max10 "cannot read DT_AVAIL_REG\n");
>> return ret;
>> @@ -134,7 +151,7 @@ static int init_max10_device_table(struct
>> intel_max10_device *max10)
>> return -EINVAL;
>> }
>>
>> - ret = max10_reg_read(DT_BASE_ADDR_REG_OFF, &dt_addr);
>> + ret = max10_sys_read(DT_BASE_ADDR_REG, &dt_addr);
>> if (ret) {
>> dev_info(max10 "cannot get base addr of device table\n");
>> return ret;
>> @@ -315,7 +332,7 @@ static int max10_add_sensor(struct raw_sensor_info
>> *info,
>> if (!sensor_reg_valid(&info->regs[i]))
>> continue;
>>
>> - ret = max10_reg_read(info->regs[i].regoff, &val);
>> + ret = max10_sys_read(info->regs[i].regoff, &val);
>> if (ret)
>> break;
>>
>> @@ -355,7 +372,8 @@ static int max10_add_sensor(struct raw_sensor_info
>> *info,
>> return ret;
>> }
>>
>> -static int max10_sensor_init(struct intel_max10_device *dev)
>> +static int
>> +max10_sensor_init(struct intel_max10_device *dev, int parent)
>> {
>> int i, ret = 0, offset = 0;
>> const fdt32_t *num;
>> @@ -370,7 +388,7 @@ static int max10_sensor_init(struct intel_max10_device
>> *dev)
>> return 0;
>> }
>>
>> - fdt_for_each_subnode(offset, fdt_root, 0) {
>> + fdt_for_each_subnode(offset, fdt_root, parent) {
>> ptr = fdt_get_name(fdt_root, offset, NULL);
>> if (!ptr) {
>> dev_err(dev, "failed to fdt get name\n"); @@ -417,7
>> +435,16
>> @@ static int max10_sensor_init(struct intel_max10_device *dev)
>> continue;
>> }
>>
>> - raw->regs[i].regoff = start;
>> + /* This is a hack to compatible with non-secure
>> + * solution. If sensors are included in root node,
>> + * then it's non-secure dtb, which use absolute addr
>> + * of non-secure solution.
>> + */
>> + if (parent)
>> + raw->regs[i].regoff = start;
>> + else
>> + raw->regs[i].regoff = start -
>> + MAX10_BASE_ADDR;
>> raw->regs[i].size = size;
>> }
>>
>> @@ -469,6 +496,63 @@ static int max10_sensor_init(struct
>> intel_max10_device *dev)
>> return ret;
>> }
>>
>> +static int check_max10_version(struct intel_max10_device *dev) {
>> + unsigned int v;
>> +
>> + if (!max10_reg_read(MAX10_SEC_BASE_ADDR + MAX10_BUILD_VER,
>> + &v)) {
>> + if (v != 0xffffffff) {
>> + dev_info(dev, "secure MAX10 detected\n");
>> + dev->base = MAX10_SEC_BASE_ADDR;
>> + dev->flags |= MAX10_FLAGS_SECURE;
>> + } else {
>> + dev_info(dev, "non-secure MAX10 detected\n");
>> + dev->base = MAX10_BASE_ADDR;
>> + }
>> + return 0;
>> + }
>> +
>> + return -ENODEV;
>> +}
>> +
>> +static int
>> +max10_secure_hw_init(struct intel_max10_device *dev) {
>> + int offset, sysmgr_offset = 0;
>> + char *fdt_root;
>> +
>> + fdt_root = dev->fdt_root;
>> + if (!fdt_root) {
>> + dev_debug(dev, "skip init as not find Device Tree\n");
>> + return 0;
>> + }
>> +
>> + fdt_for_each_subnode(offset, fdt_root, 0) {
>> + if (!fdt_node_check_compatible(fdt_root, offset,
>> + "intel-max10,system-manager")) {
>> + sysmgr_offset = offset;
>> + break;
>> + }
>> + }
>> +
>> + max10_check_capability(dev);
>> +
>> + max10_sensor_init(dev, sysmgr_offset);
>> +
>> + return 0;
>> +}
>> +
>> +static int
>> +max10_non_secure_hw_init(struct intel_max10_device *dev) {
>> + max10_check_capability(dev);
>> +
>> + max10_sensor_init(dev, 0);
>> +
>> + return 0;
>> +}
>> +
>> struct intel_max10_device *
>> intel_max10_device_probe(struct altera_spi_device *spi,
>> int chipselect)
>> @@ -492,32 +576,47 @@ struct intel_max10_device *
>> /* set the max10 device firstly */
>> g_max10 = dev;
>>
>> - /* init the MAX10 device table */
>> + /* check the max10 version */
>> + ret = check_max10_version(dev);
>> + if (ret) {
>> + dev_err(dev, "Failed to find max10 hardware!\n");
>> + goto free_dev;
>> + }
>> +
>> + /* load the MAX10 device table */
>> ret = init_max10_device_table(dev);
>> if (ret) {
>> - dev_err(dev, "init max10 device table fail\n");
>> + dev_err(dev, "Init max10 device table fail\n");
>> goto free_dev;
>> }
>>
>> - max10_check_capability(dev);
>> + /* init max10 devices, like sensor*/
>> + if (dev->flags & MAX10_FLAGS_SECURE)
>> + ret = max10_secure_hw_init(dev);
>> + else
>> + ret = max10_non_secure_hw_init(dev);
>> + if (ret) {
>> + dev_err(dev, "Failed to init max10 hardware!\n");
>> + goto free_dtb;
>> + }
>>
>> /* read FPGA loading information */
>> - ret = max10_reg_read(FPGA_PAGE_INFO_OFF, &val);
>> + ret = max10_sys_read(FPGA_PAGE_INFO, &val);
>> if (ret) {
>> dev_err(dev, "fail to get FPGA loading info\n");
>> - goto spi_tran_fail;
>> + goto release_max10_hw;
>> }
>> dev_info(dev, "FPGA loaded from %s Image\n", val ? "User" : "Factory");
>>
>> -
>> - max10_sensor_init(dev);
>> -
>> return dev;
>>
>> -spi_tran_fail:
>> +release_max10_hw:
>> + max10_sensor_uinit();
>> +free_dtb:
>> if (dev->fdt_root)
>> opae_free(dev->fdt_root);
>> - spi_transaction_remove(dev->spi_tran_dev);
>> + if (dev->spi_tran_dev)
>> + spi_transaction_remove(dev->spi_tran_dev);
>> free_dev:
>> g_max10 = NULL;
>> opae_free(dev);
>> diff --git a/drivers/raw/ifpga/base/opae_intel_max10.h
>> b/drivers/raw/ifpga/base/opae_intel_max10.h
>> index 90bf098..e632941 100644
>> --- a/drivers/raw/ifpga/base/opae_intel_max10.h
>> +++ b/drivers/raw/ifpga/base/opae_intel_max10.h
>> @@ -23,6 +23,8 @@ struct max10_compatible_id {
>> #define MAX10_FLAGS_SPI BIT(3)
>> #define MAX10_FLGAS_NIOS_SPI BIT(4)
>> #define MAX10_FLAGS_PKVL BIT(5)
>> +#define MAX10_FLAGS_SECURE BIT(6)
>> +#define MAX10_FLAGS_MAC_CACHE BIT(7)
>>
>> struct intel_max10_device {
>> unsigned int flags; /*max10 hardware capability*/ @@ -30,6 +32,7 @@
>> struct intel_max10_device {
>> struct spi_transaction_dev *spi_tran_dev;
>> struct max10_compatible_id *id; /*max10 compatible*/
>> char *fdt_root;
>> + unsigned int base; /* max10 base address */
>> };
>>
>> /* retimer speed */
>> @@ -74,30 +77,69 @@ struct opae_retimer_status { #define FLASH_BASE
>> 0x10000000 #define FLASH_OPTION_BITS 0x10000
>>
>> -#define NIOS2_FW_VERSION_OFF 0x300400
>> -#define RSU_REG_OFF 0x30042c
>> -#define FPGA_RP_LOAD BIT(3)
>> -#define NIOS2_PRERESET BIT(4)
>> -#define NIOS2_HANG BIT(5)
>> -#define RSU_ENABLE BIT(6)
>> -#define NIOS2_RESET BIT(7)
>> -#define NIOS2_I2C2_POLL_STOP BIT(13)
>> -#define FPGA_RECONF_REG_OFF 0x300430
>> -#define COUNTDOWN_START BIT(18)
>> -#define MAX10_BUILD_VER_OFF 0x300468
>> -#define PCB_INFO GENMASK(31, 24)
>> -#define MAX10_BUILD_VERION GENMASK(23, 0)
>> -#define FPGA_PAGE_INFO_OFF 0x30046c
>> -#define DT_AVAIL_REG_OFF 0x300490
>> -#define DT_AVAIL BIT(0)
>> -#define DT_BASE_ADDR_REG_OFF 0x300494
>> -#define PKVL_POLLING_CTRL 0x300480
>> -#define PKVL_LINK_STATUS 0x300564
>> +/* System Registers */
>> +#define MAX10_BASE_ADDR 0x300400
>> +#define MAX10_SEC_BASE_ADDR 0x300800
>> +/* Register offset of system registers */
>> +#define NIOS2_FW_VERSION 0x0
>> +#define MAX10_MACADDR1 0x10
>> +#define MAX10_MAC_BYTE4 GENMASK(7, 0)
>> +#define MAX10_MAC_BYTE3 GENMASK(15, 8)
>> +#define MAX10_MAC_BYTE2 GENMASK(23, 16)
>> +#define MAX10_MAC_BYTE1 GENMASK(31, 24)
>> +#define MAX10_MACADDR2 0x14
>> +#define MAX10_MAC_BYTE6 GENMASK(7, 0)
>> +#define MAX10_MAC_BYTE5 GENMASK(15, 8)
>> +#define MAX10_MAC_COUNT GENMASK(23, 16)
>> +#define RSU_REG 0x2c
>> +#define FPGA_RECONF_PAGE GENMASK(2, 0)
>> +#define FPGA_RP_LOAD BIT(3)
>> +#define NIOS2_PRERESET BIT(4)
>> +#define NIOS2_HANG BIT(5)
>> +#define RSU_ENABLE BIT(6)
>> +#define NIOS2_RESET BIT(7)
>> +#define NIOS2_I2C2_POLL_STOP BIT(13)
>> +#define PKVL_EEPROM_LOAD BIT(31)
>> +#define FPGA_RECONF_REG 0x30
>> +#define MAX10_TEST_REG 0x3c
>> +#define COUNTDOWN_START BIT(18)
>> +#define MAX10_BUILD_VER 0x68
>> +#define MAX10_VERSION_MAJOR GENMASK(23, 16)
>> +#define PCB_INFO GENMASK(31, 24)
>> +#define FPGA_PAGE_INFO 0x6c
>> +#define DT_AVAIL_REG 0x90
>> +#define DT_AVAIL BIT(0)
>> +#define DT_BASE_ADDR_REG 0x94
>> +#define MAX10_DOORBELL 0x400
>> +#define RSU_REQUEST BIT(0)
>> +#define SEC_PROGRESS GENMASK(7, 4)
>> +#define HOST_STATUS GENMASK(11, 8)
>> +#define SEC_STATUS GENMASK(23, 16)
>> +
>> +/* PKVL related registers, in system register region */
>> +#define PKVL_POLLING_CTRL 0x80
>> +#define POLLING_MODE GENMASK(15, 0)
>> +#define PKVL_A_PRELOAD BIT(16)
>> +#define PKVL_A_PRELOAD_TIMEOUT BIT(17)
>> +#define PKVL_A_DATA_TOO_BIG BIT(18)
>> +#define PKVL_A_HDR_CHECKSUM BIT(20)
>> +#define PKVL_B_PRELOAD BIT(24)
>> +#define PKVL_B_PRELOAD_TIMEOUT BIT(25)
>> +#define PKVL_B_DATA_TOO_BIG BIT(26)
>> +#define PKVL_B_HDR_CHECKSUM BIT(28)
>> +#define PKVL_EEPROM_UPG_STATUS GENMASK(31, 16)
>> +#define PKVL_LINK_STATUS 0x164
>> +#define PKVL_A_VERSION 0x254
>> +#define PKVL_B_VERSION 0x258
>> +#define SERDES_VERSION GENMASK(15, 0)
>> +#define SBUS_VERSION GENMASK(31, 16)
>>
>> #define DFT_MAX_SIZE 0x7e0000
>>
>> int max10_reg_read(unsigned int reg, unsigned int *val); int
>> max10_reg_write(unsigned int reg, unsigned int val);
>> +int max10_sys_read(unsigned int offset, unsigned int *val); int
>> +max10_sys_write(unsigned int offset, unsigned int val);
>> struct intel_max10_device *
>> intel_max10_device_probe(struct altera_spi_device *spi,
>> int chipselect);
>> --
>> 1.8.3.1
>
