From 31708a39d3e76cbd6a190a6fc852a00dbbf3d161 Mon Sep 17 00:00:00 2001 From: Daniel Jobson Date: Thu, 7 Nov 2024 13:10:10 +0100 Subject: [PATCH 1/7] Add API for syscall MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a register to store an address to a syscall function defined in firmware. Set the reset value to an illegal address, to make sure a call to an unset address will halt the CPU. Co-authored-by: Mikael Ågren --- hw/application_fpga/core/tk1/rtl/tk1.v | 24 ++++++++++++++++++++++++ hw/application_fpga/fw/tk1_mem.h | 1 + 2 files changed, 25 insertions(+) diff --git a/hw/application_fpga/core/tk1/rtl/tk1.v b/hw/application_fpga/core/tk1/rtl/tk1.v index 7e39ce04..fbd71bdc 100644 --- a/hw/application_fpga/core/tk1/rtl/tk1.v +++ b/hw/application_fpga/core/tk1/rtl/tk1.v @@ -80,6 +80,7 @@ module tk1 #( localparam ADDR_APP_SIZE = 8'h0d; localparam ADDR_BLAKE2S = 8'h10; + localparam ADDR_SYSCALL = 8'h12; localparam ADDR_CDI_FIRST = 8'h20; localparam ADDR_CDI_LAST = 8'h27; @@ -108,6 +109,10 @@ module tk1 #( localparam FW_RAM_LAST = 32'hd00007ff; + // ILLEGAL_ADDR is outside of the physical memory, and will trap in + // the security monitor if accessed. + localparam ILLEGAL_ADDR = 32'h4f000000; + //---------------------------------------------------------------- // Registers including update variables and write enable. //---------------------------------------------------------------- @@ -136,6 +141,9 @@ module tk1 #( reg [31 : 0] blake2s_addr_reg; reg blake2s_addr_we; + reg [31 : 0] syscall_addr_reg; + reg syscall_addr_we; + reg [23 : 0] cpu_trap_ctr_reg; reg [23 : 0] cpu_trap_ctr_new; reg [ 2 : 0] cpu_trap_led_reg; @@ -259,6 +267,7 @@ module tk1 #( app_start_reg <= 32'h0; app_size_reg <= APP_SIZE; blake2s_addr_reg <= 32'h0; + syscall_addr_reg <= ILLEGAL_ADDR; cdi_mem[0] <= 32'h0; cdi_mem[1] <= 32'h0; cdi_mem[2] <= 32'h0; @@ -317,6 +326,10 @@ module tk1 #( blake2s_addr_reg <= write_data; end + if (syscall_addr_we) begin + syscall_addr_reg <= write_data; + end + if (cdi_mem_we) begin cdi_mem[address[2 : 0]] <= write_data; end @@ -423,6 +436,7 @@ module tk1 #( app_start_we = 1'h0; app_size_we = 1'h0; blake2s_addr_we = 1'h0; + syscall_addr_we = 1'h0; cdi_mem_we = 1'h0; ram_addr_rand_we = 1'h0; ram_data_rand_we = 1'h0; @@ -478,6 +492,12 @@ module tk1 #( end end + if (address == ADDR_SYSCALL) begin + if (!system_mode_reg) begin + syscall_addr_we = 1'h1; + end + end + if ((address >= ADDR_CDI_FIRST) && (address <= ADDR_CDI_LAST)) begin if (!system_mode_reg) begin cdi_mem_we = 1'h1; @@ -562,6 +582,10 @@ module tk1 #( tmp_read_data = blake2s_addr_reg; end + if (address == ADDR_SYSCALL) begin + tmp_read_data = syscall_addr_reg; + end + if ((address >= ADDR_CDI_FIRST) && (address <= ADDR_CDI_LAST)) begin tmp_read_data = cdi_mem[address[2 : 0]]; end diff --git a/hw/application_fpga/fw/tk1_mem.h b/hw/application_fpga/fw/tk1_mem.h index 246097b7..945dd196 100644 --- a/hw/application_fpga/fw/tk1_mem.h +++ b/hw/application_fpga/fw/tk1_mem.h @@ -125,6 +125,7 @@ #define TK1_MMIO_TK1_APP_SIZE 0xff000034 #define TK1_MMIO_TK1_BLAKE2S 0xff000040 +#define TK1_MMIO_TK1_SYSCALL 0xff000048 #define TK1_MMIO_TK1_CDI_FIRST 0xff000080 #define TK1_MMIO_TK1_CDI_LAST 0xff00009c From 8c6ab6902db471c52ec77abce4fc6e9cfb570227 Mon Sep 17 00:00:00 2001 From: Daniel Jobson Date: Thu, 14 Nov 2024 14:02:53 +0100 Subject: [PATCH 2/7] Automatically control system_mode in hardware MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Raise privilege (go to firmware mode) when a function call occurs to the function set in syscall_addr_reg. Automatically revoke privilege when executing above ROM (go to app mode). Remove the option of writing to system_mode through the API. Co-authored-by: Mikael Ågren --- hw/application_fpga/core/tk1/rtl/tk1.v | 31 +++++++++++++++++++++----- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/hw/application_fpga/core/tk1/rtl/tk1.v b/hw/application_fpga/core/tk1/rtl/tk1.v index fbd71bdc..71afd7d6 100644 --- a/hw/application_fpga/core/tk1/rtl/tk1.v +++ b/hw/application_fpga/core/tk1/rtl/tk1.v @@ -108,6 +108,7 @@ module tk1 #( localparam FW_RAM_FIRST = 32'hd0000000; localparam FW_RAM_LAST = 32'hd00007ff; + localparam FW_ROM_LAST = 32'h00001fff; // ILLEGAL_ADDR is outside of the physical memory, and will trap in // the security monitor if accessed. @@ -120,6 +121,7 @@ module tk1 #( reg cdi_mem_we; reg system_mode_reg; + reg system_mode_new; reg system_mode_we; reg [ 2 : 0] led_reg; @@ -299,7 +301,7 @@ module tk1 #( gpio2_reg[1] <= gpio2_reg[0]; if (system_mode_we) begin - system_mode_reg <= 1'h1; + system_mode_reg <= system_mode_new; end if (led_we) begin @@ -424,12 +426,33 @@ module tk1 #( end end + //---------------------------------------------------------------- + // system_mode_ctrl will raise the privilege when the function in + // `syscall_addr_reg` is called. + // + // Automatically lowers the privilege when executing above ROM + // ---------------------------------------------------------------- + always @* begin : system_mode_ctrl + system_mode_new = 1'h0; + system_mode_we = 1'h0; + + if (cpu_valid & cpu_instr) begin + if (cpu_addr == syscall_addr_reg) begin + system_mode_new = 1'h0; + system_mode_we = 1'h1; + end + + if (cpu_addr > FW_ROM_LAST) begin + system_mode_new = 1'h1; + system_mode_we = 1'h1; + end + end + end //---------------------------------------------------------------- // api //---------------------------------------------------------------- always @* begin : api - system_mode_we = 1'h0; led_we = 1'h0; gpio3_we = 1'h0; gpio4_we = 1'h0; @@ -457,10 +480,6 @@ module tk1 #( if (cs) begin tmp_ready = 1'h1; if (we) begin - if (address == ADDR_SYSTEM_MODE_CTRL) begin - system_mode_we = 1'h1; - end - if (address == ADDR_LED) begin led_we = 1'h1; end From 9062b4980401b38cbe35838fd7d1e7bd9712f9d5 Mon Sep 17 00:00:00 2001 From: Daniel Jobson Date: Wed, 13 Nov 2024 16:13:16 +0100 Subject: [PATCH 3/7] Deny access to the SPI master in app mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Mikael Ågren --- hw/application_fpga/core/tk1/rtl/tk1.v | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/hw/application_fpga/core/tk1/rtl/tk1.v b/hw/application_fpga/core/tk1/rtl/tk1.v index 71afd7d6..97ef15d5 100644 --- a/hw/application_fpga/core/tk1/rtl/tk1.v +++ b/hw/application_fpga/core/tk1/rtl/tk1.v @@ -474,8 +474,8 @@ module tk1 #( spi_start = 1'h0; spi_tx_data_vld = 1'h0; - spi_enable = write_data[0]; - spi_tx_data = write_data[7 : 0]; + spi_enable = write_data[0] & ~system_mode_reg; + spi_tx_data = write_data[7 : 0] & ~{8{system_mode_reg}}; if (cs) begin tmp_ready = 1'h1; @@ -552,15 +552,21 @@ module tk1 #( end if (address == ADDR_SPI_EN) begin - spi_enable_vld = 1'h1; + if (!system_mode_reg) begin + spi_enable_vld = 1'h1; + end end if (address == ADDR_SPI_XFER) begin - spi_start = 1'h1; + if (!system_mode_reg) begin + spi_start = 1'h1; + end end if (address == ADDR_SPI_DATA) begin - spi_tx_data_vld = 1'h1; + if (!system_mode_reg) begin + spi_tx_data_vld = 1'h1; + end end end @@ -616,11 +622,15 @@ module tk1 #( end if (address == ADDR_SPI_XFER) begin - tmp_read_data[0] = spi_ready; + if (!system_mode_reg) begin + tmp_read_data[0] = spi_ready; + end end if (address == ADDR_SPI_DATA) begin - tmp_read_data[7 : 0] = spi_rx_data; + if (!system_mode_reg) begin + tmp_read_data[7 : 0] = spi_rx_data; + end end end From 690bb53267ef10ac83d46eb02867f2060016878f Mon Sep 17 00:00:00 2001 From: Daniel Jobson Date: Fri, 15 Nov 2024 09:28:01 +0100 Subject: [PATCH 4/7] Introduce new bit to mark ROM as non-executable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is dynamically set by hw in system_mode_ctrl. ROM will reset to executable, but will be marked as non-executable as soon as we are no longer executing in ROM, like system_mode. ROM will be marked as executable again, if function calls are made to either `syscall_addr_reg` or `blake2s_addr_reg`. Set reset value of `blake2s_addr_reg` to an illegal address, halting the CPU if it is called unset. The blake2s function is 4-byte aligned, to ensure the cpu_addr is is aligned with the address in the register. Co-authored-by: Mikael Ågren --- hw/application_fpga/core/tk1/rtl/tk1.v | 117 +++++++++++++------ hw/application_fpga/fw/tk1/blake2s/blake2s.c | 2 +- 2 files changed, 81 insertions(+), 38 deletions(-) diff --git a/hw/application_fpga/core/tk1/rtl/tk1.v b/hw/application_fpga/core/tk1/rtl/tk1.v index 97ef15d5..d9cd96e2 100644 --- a/hw/application_fpga/core/tk1/rtl/tk1.v +++ b/hw/application_fpga/core/tk1/rtl/tk1.v @@ -117,13 +117,17 @@ module tk1 #( //---------------------------------------------------------------- // Registers including update variables and write enable. //---------------------------------------------------------------- - reg [31 : 0] cdi_mem [0 : 7]; + reg [31 : 0] cdi_mem [0 : 7]; reg cdi_mem_we; reg system_mode_reg; reg system_mode_new; reg system_mode_we; + reg rom_executable_reg; + reg rom_executable_new; + reg rom_executable_we; + reg [ 2 : 0] led_reg; reg led_we; @@ -260,33 +264,34 @@ module tk1 #( //---------------------------------------------------------------- always @(posedge clk) begin : reg_update if (!reset_n) begin - system_mode_reg <= 1'h0; - led_reg <= 3'h6; - gpio1_reg <= 2'h0; - gpio2_reg <= 2'h0; - gpio3_reg <= 1'h0; - gpio4_reg <= 1'h0; - app_start_reg <= 32'h0; - app_size_reg <= APP_SIZE; - blake2s_addr_reg <= 32'h0; - syscall_addr_reg <= ILLEGAL_ADDR; - cdi_mem[0] <= 32'h0; - cdi_mem[1] <= 32'h0; - cdi_mem[2] <= 32'h0; - cdi_mem[3] <= 32'h0; - cdi_mem[4] <= 32'h0; - cdi_mem[5] <= 32'h0; - cdi_mem[6] <= 32'h0; - cdi_mem[7] <= 32'h0; - cpu_trap_ctr_reg <= 24'h0; - cpu_trap_led_reg <= 3'h0; - cpu_mon_en_reg <= 1'h0; - cpu_mon_first_reg <= 32'h0; - cpu_mon_last_reg <= 32'h0; - ram_addr_rand_reg <= 15'h0; - ram_data_rand_reg <= 32'h0; - force_trap_reg <= 1'h0; - system_reset_reg <= 1'h0; + system_mode_reg <= 1'h0; + rom_executable_reg <= 1'h1; + led_reg <= 3'h6; + gpio1_reg <= 2'h0; + gpio2_reg <= 2'h0; + gpio3_reg <= 1'h0; + gpio4_reg <= 1'h0; + app_start_reg <= 32'h0; + app_size_reg <= APP_SIZE; + blake2s_addr_reg <= ILLEGAL_ADDR; + syscall_addr_reg <= ILLEGAL_ADDR; + cdi_mem[0] <= 32'h0; + cdi_mem[1] <= 32'h0; + cdi_mem[2] <= 32'h0; + cdi_mem[3] <= 32'h0; + cdi_mem[4] <= 32'h0; + cdi_mem[5] <= 32'h0; + cdi_mem[6] <= 32'h0; + cdi_mem[7] <= 32'h0; + cpu_trap_ctr_reg <= 24'h0; + cpu_trap_led_reg <= 3'h0; + cpu_mon_en_reg <= 1'h0; + cpu_mon_first_reg <= 32'h0; + cpu_mon_last_reg <= 32'h0; + ram_addr_rand_reg <= 15'h0; + ram_data_rand_reg <= 32'h0; + force_trap_reg <= 1'h0; + system_reset_reg <= 1'h0; end else begin @@ -304,6 +309,10 @@ module tk1 #( system_mode_reg <= system_mode_new; end + if (rom_executable_we) begin + rom_executable_reg <= rom_executable_new; + end + if (led_we) begin led_reg <= write_data[2 : 0]; end @@ -400,6 +409,13 @@ module tk1 #( // // Trying to execute instructions in FW-RAM. // + // Executing instructions in ROM, while ROM is marked as not + // executable. Note that there is an exception, if the cpu_addr is + // the first instruction in either functions set in syscall_addr_reg + // or blake2s_addr_reg, it is allowed. For the next instruction ROM + // will be marked as executable. + // Note that the address to the functions needs to be 4-bytes aligned. + // // Trying to execute code in mem area set to be data access only. // This requires execution monitor to have been setup and // enabled. @@ -417,6 +433,14 @@ module tk1 #( force_trap_set = 1'h1; end + if (!rom_executable_reg) begin + if ((cpu_addr != syscall_addr_reg) && (cpu_addr != blake2s_addr_reg)) begin + if (cpu_addr <= FW_ROM_LAST) begin // Only valid as long as ROM starts at address 0x00. + force_trap_set = 1'h1; + end + end + end + if (cpu_mon_en_reg) begin if ((cpu_addr >= cpu_mon_first_reg) && (cpu_addr <= cpu_mon_last_reg)) begin force_trap_set = 1'h1; @@ -427,24 +451,43 @@ module tk1 #( end //---------------------------------------------------------------- - // system_mode_ctrl will raise the privilege when the function in - // `syscall_addr_reg` is called. + // system_mode_ctrl dynamically sets the system mode and if ROM is + // executable depending on where we are currently executing. + // + // If the function in `syscall_addr_reg` is called system_mode will + // be set to firmware mode, and ROM will be marked as executable. + // + // If the function in `blake2s_addr_reg` is called only ROM will be + // marked as executable. // - // Automatically lowers the privilege when executing above ROM + // Automatically lowers the privilege and removes ROM execution when + // executing above ROM. + // + // Note that the address to the functions needs to be 4-bytes aligned. // ---------------------------------------------------------------- always @* begin : system_mode_ctrl - system_mode_new = 1'h0; - system_mode_we = 1'h0; + system_mode_new = 1'h0; + system_mode_we = 1'h0; + rom_executable_new = 1'h0; + rom_executable_we = 1'h0; if (cpu_valid & cpu_instr) begin if (cpu_addr == syscall_addr_reg) begin - system_mode_new = 1'h0; - system_mode_we = 1'h1; + system_mode_new = 1'h0; + system_mode_we = 1'h1; + rom_executable_new = 1'h1; + rom_executable_we = 1'h1; + end + if (cpu_addr == blake2s_addr_reg) begin + rom_executable_new = 1'h1; + rom_executable_we = 1'h1; end if (cpu_addr > FW_ROM_LAST) begin - system_mode_new = 1'h1; - system_mode_we = 1'h1; + system_mode_new = 1'h1; + system_mode_we = 1'h1; + rom_executable_new = 1'h0; + rom_executable_we = 1'h1; end end end diff --git a/hw/application_fpga/fw/tk1/blake2s/blake2s.c b/hw/application_fpga/fw/tk1/blake2s/blake2s.c index b46ad8af..eacd753d 100644 --- a/hw/application_fpga/fw/tk1/blake2s/blake2s.c +++ b/hw/application_fpga/fw/tk1/blake2s/blake2s.c @@ -332,7 +332,7 @@ void blake2s_final(blake2s_ctx *ctx, void *out) //------------------------------------------------------------------ // Convenience function for all-in-one computation. //------------------------------------------------------------------ -int blake2s(void *out, size_t outlen, +int __attribute__((aligned(4))) blake2s(void *out, size_t outlen, const void *key, size_t keylen, const void *in, size_t inlen, blake2s_ctx *ctx) From 2abe93cf06aad551d0d254a8219a2037a861971b Mon Sep 17 00:00:00 2001 From: Daniel Jobson Date: Fri, 15 Nov 2024 11:19:40 +0100 Subject: [PATCH 5/7] Make sensitive assets only readable/writable before system_mode is set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After the first time system_mode is set to one, the assets will no longer be read- or writeable, even if system_mode is set to zero at a later syscall. This is to make sure syscalls does not have the same privilege as the firmware has at first boot. We need to monitor when system_mode is set to one, otherwise we might accedentially lock the assets before actually leaving firmware, for example if firmware would use a function set in any of the registers used in system_mode_ctrl. Co-authored-by: Mikael Ågren --- hw/application_fpga/core/tk1/rtl/tk1.v | 25 +++++++++++++------ hw/application_fpga/core/uds/rtl/uds.v | 4 +-- hw/application_fpga/rtl/application_fpga.v | 4 ++- hw/application_fpga/tb/application_fpga_sim.v | 4 ++- 4 files changed, 25 insertions(+), 12 deletions(-) diff --git a/hw/application_fpga/core/tk1/rtl/tk1.v b/hw/application_fpga/core/tk1/rtl/tk1.v index d9cd96e2..b228eee9 100644 --- a/hw/application_fpga/core/tk1/rtl/tk1.v +++ b/hw/application_fpga/core/tk1/rtl/tk1.v @@ -21,6 +21,7 @@ module tk1 #( input wire cpu_trap, output wire system_mode, + output wire rw_locked, input wire [31 : 0] cpu_addr, input wire cpu_instr, @@ -128,6 +129,8 @@ module tk1 #( reg rom_executable_new; reg rom_executable_we; + reg rw_locked_reg; + reg [ 2 : 0] led_reg; reg led_we; @@ -202,6 +205,7 @@ module tk1 #( assign ready = tmp_ready; assign system_mode = system_mode_reg; + assign rw_locked = rw_locked_reg; assign force_trap = force_trap_reg; @@ -266,6 +270,7 @@ module tk1 #( if (!reset_n) begin system_mode_reg <= 1'h0; rom_executable_reg <= 1'h1; + rw_locked_reg <= 1'h0; led_reg <= 3'h6; gpio1_reg <= 2'h0; gpio2_reg <= 2'h0; @@ -307,6 +312,10 @@ module tk1 #( if (system_mode_we) begin system_mode_reg <= system_mode_new; + + if (system_mode_new) begin + rw_locked_reg <= 1'h1; + end end if (rom_executable_we) begin @@ -533,13 +542,13 @@ module tk1 #( end if (address == ADDR_APP_START) begin - if (!system_mode_reg) begin + if (!rw_locked_reg) begin app_start_we = 1'h1; end end if (address == ADDR_APP_SIZE) begin - if (!system_mode_reg) begin + if (!rw_locked_reg) begin app_size_we = 1'h1; end end @@ -549,31 +558,31 @@ module tk1 #( end if (address == ADDR_BLAKE2S) begin - if (!system_mode_reg) begin + if (!rw_locked_reg) begin blake2s_addr_we = 1'h1; end end if (address == ADDR_SYSCALL) begin - if (!system_mode_reg) begin + if (!rw_locked_reg) begin syscall_addr_we = 1'h1; end end if ((address >= ADDR_CDI_FIRST) && (address <= ADDR_CDI_LAST)) begin - if (!system_mode_reg) begin + if (!rw_locked_reg) begin cdi_mem_we = 1'h1; end end if (address == ADDR_RAM_ADDR_RAND) begin - if (!system_mode_reg) begin + if (!rw_locked_reg) begin ram_addr_rand_we = 1'h1; end end if (address == ADDR_RAM_DATA_RAND) begin - if (!system_mode_reg) begin + if (!rw_locked_reg) begin ram_data_rand_we = 1'h1; end end @@ -659,7 +668,7 @@ module tk1 #( end if ((address >= ADDR_UDI_FIRST) && (address <= ADDR_UDI_LAST)) begin - if (!system_mode_reg) begin + if (!rw_locked_reg) begin tmp_read_data = udi_rdata; end end diff --git a/hw/application_fpga/core/uds/rtl/uds.v b/hw/application_fpga/core/uds/rtl/uds.v index 2aaa112d..d0b3c516 100644 --- a/hw/application_fpga/core/uds/rtl/uds.v +++ b/hw/application_fpga/core/uds/rtl/uds.v @@ -17,7 +17,7 @@ module uds ( input wire clk, input wire reset_n, - input wire system_mode, + input wire rw_locked, input wire cs, input wire [ 2 : 0] address, @@ -89,7 +89,7 @@ module uds ( if (cs) begin tmp_ready = 1'h1; - if (!system_mode) begin + if (!rw_locked) begin if (uds_rd_reg[address[2 : 0]] == 1'h0) begin uds_rd_we = 1'h1; end diff --git a/hw/application_fpga/rtl/application_fpga.v b/hw/application_fpga/rtl/application_fpga.v index 39650d88..a373b360 100644 --- a/hw/application_fpga/rtl/application_fpga.v +++ b/hw/application_fpga/rtl/application_fpga.v @@ -143,6 +143,7 @@ module application_fpga ( wire [31 : 0] tk1_read_data; wire tk1_ready; wire system_mode; + wire rw_locked; wire force_trap; wire [14 : 0] ram_addr_rand; wire [31 : 0] ram_data_rand; @@ -277,7 +278,7 @@ module application_fpga ( .clk(clk), .reset_n(reset_n), - .system_mode(system_mode), + .rw_locked(rw_locked), .cs(uds_cs), .address(uds_address), @@ -321,6 +322,7 @@ module application_fpga ( .reset_n(reset_n), .system_mode(system_mode), + .rw_locked (rw_locked), .cpu_addr (cpu_addr), .cpu_instr (cpu_instr), diff --git a/hw/application_fpga/tb/application_fpga_sim.v b/hw/application_fpga/tb/application_fpga_sim.v index 00da22d0..3ddd1276 100644 --- a/hw/application_fpga/tb/application_fpga_sim.v +++ b/hw/application_fpga/tb/application_fpga_sim.v @@ -155,6 +155,7 @@ module application_fpga_sim ( wire [31 : 0] tk1_read_data; wire tk1_ready; wire system_mode; + wire rw_locked; wire force_trap; wire [14 : 0] ram_addr_rand; wire [31 : 0] ram_data_rand; @@ -288,7 +289,7 @@ module application_fpga_sim ( .clk(clk), .reset_n(reset_n), - .system_mode(system_mode), + .rw_locked(rw_locked), .cs(uds_cs), .address(uds_address), @@ -334,6 +335,7 @@ module application_fpga_sim ( .reset_n(reset_n), .system_mode(system_mode), + .rw_locked (rw_locked), .cpu_addr (cpu_addr), .cpu_instr (cpu_instr), From 04eefe01fa7f6f425ddd166b4cc803d4ede8566c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=85gren?= Date: Thu, 21 Nov 2024 08:46:49 +0100 Subject: [PATCH 6/7] Fix tb_tk1.v tests broken when implementing hw controlled system mode --- hw/application_fpga/core/tk1/tb/tb_tk1.v | 63 ++++++++++++++++++++---- 1 file changed, 53 insertions(+), 10 deletions(-) diff --git a/hw/application_fpga/core/tk1/tb/tb_tk1.v b/hw/application_fpga/core/tk1/tb/tb_tk1.v index 640a7144..e24d4f43 100644 --- a/hw/application_fpga/core/tk1/tb/tb_tk1.v +++ b/hw/application_fpga/core/tk1/tb/tb_tk1.v @@ -27,8 +27,6 @@ module tb_tk1 (); localparam ADDR_NAME1 = 8'h01; localparam ADDR_VERSION = 8'h02; - localparam ADDR_SYSTEM_MODE_CTRL = 8'h08; - localparam ADDR_LED = 8'h09; localparam LED_R_BIT = 2; localparam LED_G_BIT = 1; @@ -62,6 +60,7 @@ module tb_tk1 (); localparam ADDR_SPI_XFER = 8'h81; localparam ADDR_SPI_DATA = 8'h82; + localparam APP_RAM_START = 32'h40000000; //---------------------------------------------------------------- // Register and Wire declarations. @@ -369,6 +368,38 @@ module tb_tk1 (); endtask // read_check_word + //---------------------------------------------------------------- + // `check_equal() + // + // Check that two values are equal + //---------------------------------------------------------------- + `define check_equal(_value, _expected) \ + begin \ + if ((_value) != (_expected)) begin \ + $display("--- Error: (%s) != (%s)\n 0x%x != 0x%x", `"_value`", `"_expected`", (_value), (_expected)); \ + error_ctr = error_ctr + 1; \ + end \ + end // `check_equal + + + //---------------------------------------------------------------- + // fetch_instruction() + // + // Simulate fetch of an instruction at specified address. + //---------------------------------------------------------------- + task fetch_instruction(input [31:0] address); + begin : fetch_instruction + tb_cpu_addr = address; + tb_cpu_instr = 1'h1; + tb_cpu_valid = 1'h1; + #(CLK_PERIOD); + tb_cpu_addr = 32'h0; + tb_cpu_instr = 1'h0; + tb_cpu_valid = 1'h0; + end + endtask // fetch_instruction + + //---------------------------------------------------------------- // test1() // Read out name and version. @@ -440,8 +471,8 @@ module tb_tk1 (); read_check_word(ADDR_CDI_FIRST + 6, 32'h80818283); read_check_word(ADDR_CDI_LAST + 0, 32'h70717273); - $display("--- test3: Switch to app mode."); - write_word(ADDR_SYSTEM_MODE_CTRL, 32'hdeadbeef); + $display("--- test3: Fetch instruction from app RAM."); + fetch_instruction(APP_RAM_START); $display("--- test3: Try to write CDI again."); write_word(ADDR_CDI_FIRST + 0, 32'hfffefdfc); @@ -488,8 +519,8 @@ module tb_tk1 (); $display("--- test4: Read Blake2s entry point."); read_check_word(ADDR_BLAKE2S, 32'hcafebabe); - $display("--- test4: Switch to app mode."); - write_word(ADDR_SYSTEM_MODE_CTRL, 32'hf00ff00f); + $display("--- test4: Fetch instruction from app RAM."); + fetch_instruction(APP_RAM_START); $display("--- test4: Write Blake2s entry point again."); write_word(ADDR_BLAKE2S, 32'hdeadbeef); @@ -524,8 +555,8 @@ module tb_tk1 (); read_check_word(ADDR_APP_START, 32'h13371337); read_check_word(ADDR_APP_SIZE, 32'h47114711); - $display("--- test5: Switch to app mode."); - write_word(ADDR_SYSTEM_MODE_CTRL, 32'hf000000); + $display("--- test5: Fetch instruction from app RAM."); + fetch_instruction(APP_RAM_START); $display("--- test5: Write app start address and size again."); write_word(ADDR_APP_START, 32'hdeadbeef); @@ -562,9 +593,11 @@ module tb_tk1 (); "--- test6: Check value in dut ADDR_RAM_ADDR_RAND and ADDR_RAM_DATA_RAND registers."); $display("--- test6: ram_addr_rand_reg: 0x%04x, ram_data_rand_reg: 0x%08x", dut.ram_addr_rand, dut.ram_data_rand); + `check_equal(dut.ram_addr_rand, 32'h13371337 & {15{1'b1}}); + `check_equal(dut.ram_data_rand, 32'h47114711); - $display("--- test6: Switch to app mode."); - write_word(ADDR_SYSTEM_MODE_CTRL, 32'hf000000); + $display("--- test6: Fetch instruction from app RAM."); + fetch_instruction(APP_RAM_START); $display("--- test6: Write to ADDR_RAM_ADDR_RAND and ADDR_RAM_DATA_RAND again."); write_word(ADDR_RAM_ADDR_RAND, 32'hdeadbeef); @@ -574,6 +607,8 @@ module tb_tk1 (); "--- test6: Check value in dut ADDR_RAM_ADDR_RAND and ADDR_RAM_DATA_RAND registers."); $display("--- test6: ram_addr_rand_reg: 0x%04x, ram_data_rand_reg: 0x%08x", dut.ram_addr_rand, dut.ram_data_rand); + `check_equal(dut.ram_addr_rand, 32'h13371337 & {15{1'b1}}); + `check_equal(dut.ram_data_rand, 32'h47114711); $display("--- test6: completed."); $display(""); @@ -666,6 +701,12 @@ module tb_tk1 (); $display("--- test9: cpu_addr: 0x%08x, cpu_instr: 0x%1x, cpu_valid: 0x%1x", tb_cpu_addr, tb_cpu_instr, tb_cpu_valid); $display("--- test9: force_trap: 0x%1x", tb_force_trap); + `check_equal(tb_force_trap, 1); + + $display("--- test9: restore CPU mem interface."); + tb_cpu_addr = 32'h0; + tb_cpu_instr = 1'h0; + tb_cpu_valid = 1'h0; $display("--- test9: completed."); $display(""); @@ -685,6 +726,8 @@ module tb_tk1 (); $display(""); $display("--- test10: Loopback in SPI Master started."); + $display("--- test10: Reset DUT to switch to fw mode."); + reset_dut(); #(CLK_PERIOD); From c7e44d35756c7169c55e6f4d16f7751d609a2a18 Mon Sep 17 00:00:00 2001 From: Daniel Jobson Date: Thu, 21 Nov 2024 13:24:04 +0100 Subject: [PATCH 7/7] Update tk1/README and fpga README regarding system mode Updates Readme with: - Dynamic execution mode control in hardware - ROM execution - Syscall API - Sensitive assets only read-/writable before first switch to app mode - SPI master only accessible in firmware mode --- hw/application_fpga/README.md | 9 ++ hw/application_fpga/core/tk1/README.md | 148 +++++++++++++++++++------ 2 files changed, 122 insertions(+), 35 deletions(-) diff --git a/hw/application_fpga/README.md b/hw/application_fpga/README.md index 1f3ab788..2883be3f 100644 --- a/hw/application_fpga/README.md +++ b/hw/application_fpga/README.md @@ -108,6 +108,7 @@ Contains: - General purpose input/output (GPIO) pin control. - Application introspection: start address and size of binary. - BLAKE2s function access. +- Syscall function access. - Compound Device Identity (CDI). - Unique Device Identity (UDI). - RAM memory protection. @@ -115,6 +116,14 @@ Contains: - SPI main. - System reset. +### Execution mode control + +The execution mode consists of two modes, firmware mode and +application mode. These modes have certain privileges, and in general +protects sensitive assets against access when in application mode. The +execution mode is dynamically controlled in the hardware, depending on +current location of execution. + ### Illegal instruction monitor Execution of illegal instructions will cause the CPU to enter its trap diff --git a/hw/application_fpga/core/tk1/README.md b/hw/application_fpga/core/tk1/README.md index cd80f145..1c0cb966 100644 --- a/hw/application_fpga/core/tk1/README.md +++ b/hw/application_fpga/core/tk1/README.md @@ -25,13 +25,69 @@ applications. ### Control of execution mode +The execution mode consists of two modes, firmware mode and +application mode. These modes have certain privileges. The execution +mode is dynamically controlled in the hardware, depending on current +location of execution. After a reset the device starts in firmware +mode, as soon as the executions moves outside of ROM the mode is +changed to application mode. + +There also exists an API to access two different function pointers to +functions located in ROM, i.e., in firmware. These are the syscall +function and the Blake2s function. Since these operates in ROM, they +need a higher privilege compared to application mode. The only way +back into a raised privilege, is through the blake2s or syscall API. + +The syscall function operates in firmware mode, except for not being +able to write to the sensitive assets, see the list of sensitive +assets further down. + +The blake2s function operates in application mode, but ROM is +executable during the function call. + +For a complete overview, see the modes and privileges depicted in the +table below: + +| *name* | *ROM* | *FW RAM* | *SPI* | *Sensitive assets* | +|-----------------|--------|-----------|--------|--------------------| +| Firmware mode | r/x | r/w | r/w | r/w* | +| app mode | r | i | i | r | +| syscall | r/x | r/w | r/w | r | +| blake2s | r/x | i | i | r | + +Legend: +r = readable +w = writeable +x = executable +i = invisible +* = only writeable during first time in firmware mode + + +These sensitive assets are only readable and/or writeable in firmware +mode, before the first switch to app mode: +- ADDR_APP_START +- ADDR_APP_SIZE +- ADDR_BLAKE2S +- ADDR_SYSCALL +- ADDR_CDI_FIRST +- ADDR_CDI_LAST +- ADDR_RAM_ADDR_RAND +- ADDR_RAM_DATA_RAND +- ADDR_UDI_FIRST +- ADDR_UDI_LAST + +Note that these assets have different properties, some are read-only +and some are write-only. The list above only shows if they are +restricted in app mode. See each individual API further down to find +out more about their properties. + + ``` ADDR_SYSTEM_MODE_CTRL: 0x08 ``` -This register controls if the device is executing in FW mode or in App -mode. The register can be written once between power cycles, and only -by FW. If set the device is in app mode. +This register is read-only and shows if the device is executing in FW +mode or in App mode. If set the device is in app mode. ### Control of RGB LED @@ -72,10 +128,10 @@ ADDR_APP_START: 0x0c ADDR_APP_SIZE: 0x0d ``` -These registers provide read only information to the loaded app to +These registers provide read-only information to the loaded app to itself - where it was loaded and its size. The values are written by FW as part of the loading of the app. The registers can't be written -when the `ADDR_SYSTEM_MODE_CTRL` has been set. +after the `ADDR_SYSTEM_MODE_CTRL` has been set for the first time. ### Access to Blake2s @@ -86,8 +142,31 @@ ADDR_BLAKE2S: 0x10 This register provides the 32-bit function pointer address to the Blake2s hash function in the FW. It is written by FW during boot. The -register can't be written to when the `ADDR_SYSTEM_MODE_CTRL` has been -set. +register can't be written after the `ADDR_SYSTEM_MODE_CTRL` has been +set for the first time. + +This register will default to an illegal address, so if it is left +unset by firmware and an application tries to call it the CPU will +halt. + +### Syscall access + +``` +ADDR_SYSCALL: 0x12 +``` + +This register provides the 32-bit function pointer address to the +syscall function in the FW. The syscall function provides access to +high privilege tasks in a secure manner, such as access to the SPI +flash. + +The register is written by FW during boot. The register can't be +written after the `ADDR_SYSTEM_MODE_CTRL` has been set for the first +time. + +This register will default to an illegal address, so if it is left +unset by firmware and an application tries to call it the CPU will +halt. ### Access to CDI @@ -99,10 +178,10 @@ ADDR_CDI_LAST: 0x27 These registers provide access to the 256-bit compound device secret calculated by the FW as part of loading an application. The registers -are written by the FW. The register can't be written to when the -`ADDR_SYSTEM_MODE_CTRL` has been set. The CDI is readable by apps, -which can then use it as a base secret for any other secrets required -to carry out their intended use case. +are written by the FW. The register can't be written to after the +`ADDR_SYSTEM_MODE_CTRL` has been set for the first time. The CDI is +readable by apps, which can then use it as a base secret to generate +any other secrets required to carry out their intended use case. ### Access to UDI @@ -113,7 +192,8 @@ ADDR_UDI_LAST: 0x31 ``` These read-only registers provide access to the 64-bit Unique Device -Identity (UDI). +Identity (UDI). The register can only be read in firmware mode before +the first switch to app mode. The two UDI words are stored using 32 named SB\_LUT4 FPGA multiplexer (MUX) instances, identified in the source code as "udi\_rom\_idx". One @@ -164,38 +244,34 @@ ADDR_CPU_MON_LAST: 0x62 Monitors events and state changes in the SoC and handles security violations. Currently checks for: -1. Trying to execute instructions in FW\_RAM. *Always enabled.* +1. Trying to execute instructions in FW\_RAM. *Always enabled* 2. Trying to access RAM outside of the physical memory. *Always enabled* -3. Trying to execute instructions from a memory area in RAM defined by +3. Trying to execute instructions in ROM, while ROM being marked as + non-executable. Except for the first instruction set in + `ADDR_SYSCALL` and `ADDR_BLAKE2S`. *Always enabled* +4. Trying to execute instructions from a memory area in RAM defined by the application. -Number 1 and 2 are always enabled. Number 3 is set and enabled by the -device application. Once enabled, by writing to `ADDR_CPU_MON_CTRL`, -the memory defined by `ADDR_CPU_MON_FIRST` and `ADDR_CPU_MON_LAST` -will be protected against execution. Typically the application -developer will set this protection to cover the application stack -and/or heap. +Number 1, 2 and 3 are always enabled. Number 4 is set and enabled by +the device application. -An application can write to these registers to define the area and -then enable the monitor. Once enabled the monitor can't be disabled, -and the `ADDR_CPU_MON_FIRST` and `ADDR_CPU_MON_LAST` registers can't be -changed. This means that an application that wants to use the monitor -must define the area first before enabling the monitor. +An application must write to the `ADDR_CPU_MON_FIRST` and +`ADDR_CPU_MON_LAST` first, before enabling the monitor by writing to +`ADDR_CPU_MON_CTRL`. This effectively marks the region defined as +data-only, and will protect against execution. Typically a application +developer will set this protection to cover the application stack +and/or heap. Once enabled the monitor can't be disabled, and the +registers can't be written. Once enabled, if the CPU tries to read an instruction from the defined -area, the core will force the CPU to instead read an all zero, which -is an illegal instruction. This illegal instruction will trigger the +area, the core will force the CPU to instead read an all zero +instruction, which is an illegal instruction. This will trigger the CPU to enter its TRAP state, from which it can't return unless the TKey is power cycled. -The firmware will not write to these registers as part of loading an -app. The app developer must define the area and enable the monitor to -get the protection. - -One feature not obvious from the API is that when the CPU traps the -core will detect that and start flashing the status LED with a red -light indicating that the CPU is in a trapped state and no further -execution is possible. +Another feature is that when the CPU traps the core will detect it and +start flashing the status LED with a red light, indicating that the +CPU is in a trapped state and no further execution is possible. ## SPI-master @@ -203,6 +279,8 @@ The TK1 includes a minimal SPI-master that provides access to the Winbond Flash memory mounted on the board. The SPI-master is byte oriented and very minimalistic. +The SPI master can only be used in firmware mode. + In order to transfer more than a single byte, SW must read status and write commands needed to send a sequence of bytes. In order to read out a sequence of bytes from the memory, SW must send as many dummy