From 3097cfe8a2bce61b507f84d59c2513441498f3be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=85gren?= Date: Wed, 11 Dec 2024 14:29:04 +0100 Subject: [PATCH] PoC: Control access to FW RAM Allow FW RAM access only in the following execution contexts: - Firmware mode - IRQ_SYSCALL_HI Input port `system_mode` of the `fw_ram` module is replaced with an enable port. Since access to FW RAM not longer depend only on system_mode --- hw/application_fpga/README.md | 14 +++-- hw/application_fpga/core/fw_ram/rtl/fw_ram.v | 39 ++++++------ hw/application_fpga/core/tk1/rtl/tk1.v | 3 + .../fw/irqpoc_with_app/start.S | 60 +++++++++++++++++-- hw/application_fpga/rtl/application_fpga.v | 5 +- hw/application_fpga/tb/application_fpga_sim.v | 6 +- .../tb/tb_application_fpga_sim.v | 2 +- 7 files changed, 92 insertions(+), 37 deletions(-) diff --git a/hw/application_fpga/README.md b/hw/application_fpga/README.md index e094fb45..a8497810 100644 --- a/hw/application_fpga/README.md +++ b/hw/application_fpga/README.md @@ -138,16 +138,18 @@ Interrupts can be enabled/disabled using the PicoRV32 specific The following table shows resource availablility for each execution context: -| *Execution Context* | *ROM* | -|---------------------|--------| -| Firmware mode | r/x | -| App mode | r | -| IRQ_SYSCALL_LO | r/x | -| IRQ_SYSCALL_HI | r/x | +| *Execution Context* | *ROM* | *FW RAM* | +|---------------------|--------|----------| +| Firmware mode | r/x | r/w | +| App mode | r | i | +| IRQ_SYSCALL_LO | r/x | i | +| IRQ_SYSCALL_HI | r/x | r/w | Legend: r = readable +w = writeable x = executable +i = invisible ## `tk1` diff --git a/hw/application_fpga/core/fw_ram/rtl/fw_ram.v b/hw/application_fpga/core/fw_ram/rtl/fw_ram.v index 150f6b56..f649914f 100644 --- a/hw/application_fpga/core/fw_ram/rtl/fw_ram.v +++ b/hw/application_fpga/core/fw_ram/rtl/fw_ram.v @@ -17,8 +17,7 @@ module fw_ram ( input wire clk, input wire reset_n, - input wire system_mode, - + input wire en, input wire cs, input wire [ 3 : 0] we, input wire [ 8 : 0] address, @@ -31,21 +30,19 @@ module fw_ram ( //---------------------------------------------------------------- // Registers and wires. //---------------------------------------------------------------- - reg [31 : 0] tmp_read_data; - reg [31 : 0] mem_read_data0; - reg [31 : 0] mem_read_data1; - reg ready_reg; - wire system_mode_cs; - reg bank0; - reg bank1; + reg [31 : 0] tmp_read_data; + reg [31 : 0] mem_read_data0; + reg [31 : 0] mem_read_data1; + reg ready_reg; + reg bank0; + reg bank1; //---------------------------------------------------------------- // Concurrent assignment of ports. //---------------------------------------------------------------- - assign read_data = tmp_read_data; - assign ready = ready_reg; - assign system_mode_cs = cs && ~system_mode; + assign read_data = tmp_read_data; + assign ready = ready_reg; //---------------------------------------------------------------- @@ -56,12 +53,12 @@ module fw_ram ( .RADDR({3'h0, address[7 : 0]}), .RCLK(clk), .RCLKE(1'h1), - .RE(system_mode_cs & bank0), + .RE(en & cs & bank0), .WADDR({3'h0, address[7 : 0]}), .WCLK(clk), .WCLKE(1'h1), .WDATA(write_data[15 : 0]), - .WE((|we & system_mode_cs & bank0)), + .WE((|we & en & cs & bank0)), .MASK({{8{~we[1]}}, {8{~we[0]}}}) ); @@ -70,12 +67,12 @@ module fw_ram ( .RADDR({3'h0, address[7 : 0]}), .RCLK(clk), .RCLKE(1'h1), - .RE(system_mode_cs & bank0), + .RE(en & cs & bank0), .WADDR({3'h0, address[7 : 0]}), .WCLK(clk), .WCLKE(1'h1), .WDATA(write_data[31 : 16]), - .WE((|we & system_mode_cs & bank0)), + .WE((|we & en & cs & bank0)), .MASK({{8{~we[3]}}, {8{~we[2]}}}) ); @@ -85,12 +82,12 @@ module fw_ram ( .RADDR({3'h0, address[7 : 0]}), .RCLK(clk), .RCLKE(1'h1), - .RE(system_mode_cs & bank1), + .RE(en & cs & bank1), .WADDR({3'h0, address[7 : 0]}), .WCLK(clk), .WCLKE(1'h1), .WDATA(write_data[15 : 0]), - .WE((|we & system_mode_cs & bank1)), + .WE((|we & en & cs & bank1)), .MASK({{8{~we[1]}}, {8{~we[0]}}}) ); @@ -99,12 +96,12 @@ module fw_ram ( .RADDR({3'h0, address[7 : 0]}), .RCLK(clk), .RCLKE(1'h1), - .RE(system_mode_cs & bank1), + .RE(en & cs & bank1), .WADDR({3'h0, address[7 : 0]}), .WCLK(clk), .WCLKE(1'h1), .WDATA(write_data[31 : 16]), - .WE((|we & system_mode_cs & bank1)), + .WE((|we & en & cs & bank1)), .MASK({{8{~we[3]}}, {8{~we[2]}}}) ); @@ -129,7 +126,7 @@ module fw_ram ( bank1 = 1'h0; tmp_read_data = 32'h0; - if (system_mode_cs) begin + if (en & cs) begin if (address[8]) begin bank1 = 1'h1; tmp_read_data = mem_read_data1; diff --git a/hw/application_fpga/core/tk1/rtl/tk1.v b/hw/application_fpga/core/tk1/rtl/tk1.v index 658b3847..67acf517 100644 --- a/hw/application_fpga/core/tk1/rtl/tk1.v +++ b/hw/application_fpga/core/tk1/rtl/tk1.v @@ -48,6 +48,8 @@ module tk1 #( input wire access_level_hi, input wire access_level_med, + output wire fw_ram_en, + input wire cs, input wire we, input wire [ 7 : 0] address, @@ -202,6 +204,7 @@ module tk1 #( assign system_reset = system_reset_reg; assign rom_exec_en = !system_mode | access_level_med | access_level_hi; + assign fw_ram_en = !system_mode | access_level_hi; //---------------------------------------------------------------- // Module instance. diff --git a/hw/application_fpga/fw/irqpoc_with_app/start.S b/hw/application_fpga/fw/irqpoc_with_app/start.S index 5fc15fab..5e7d77bf 100644 --- a/hw/application_fpga/fw/irqpoc_with_app/start.S +++ b/hw/application_fpga/fw/irqpoc_with_app/start.S @@ -11,6 +11,8 @@ #include "custom_ops.S" // PicoRV32 custom instructions +#define illegal_insn() .word 0 + .section ".text.init" .globl _start _start: @@ -24,18 +26,38 @@ irq_handler: // PicoRV32 stores the IRQ bitmask in x4. // If bit 31 is 1: IRQ31 was triggered. // If bit 30 is 1: IRQ30 was triggered. - - nop // NOPs are not necessary. Only added to make it easier to find - nop // when simulating. - nop +irq_syscall_lo_check: + li t4, (1 << 30) + bne x4, t4, irq_syscall_hi_check + // Firmware RAM should not be readable from IRQ_SYSCALL_LO + call check_cannot_read_test_val_from_fw_ram + j irq_source_check_done +irq_syscall_hi_check: + li t4, (1 << 31) + bne x4, t4, unexpected_irq + // Firmware RAM should be readable from IRQ_SYSCALL_HI + call check_can_read_test_val_from_fw_ram + j irq_source_check_done +unexpected_irq: + illegal_insn() +irq_source_check_done: picorv32_retirq_insn() // Return from interrupt // // Init // - .=0x20 // Setting location of init to 0x20. Makes it easier to find when - // simulating. + .=0x100 init: + // Save test value in firmware RAM + li t0, 0xd0000000 + li t1, 0x5555aaaa + sw t1, 0(t0) + + // Firmware RAM should be readable from firmware mode + call check_can_read_test_val_from_fw_ram + + + // Enable IRQs li t0, 0x3fffffff // IRQ31 & IRQ30 mask picorv32_maskirq_insn(zero, t0) // Enable IRQs @@ -59,10 +81,15 @@ copy_app: // .align 4 app_start: + // Firmware RAM should not be readable from app mode + call check_cannot_read_test_val_from_fw_ram + + // Raise IRQ_SYSCALL_HI li t0, 0xe1000000 // IRQ_SYSCALL_HI (IRQ31) trigger address sw zero, 0(t0) // Raise IRQ by writing to interrupt trigger address. // Writing any data triggers an interrupt. + // Raise IRQ_SYSCALL_LO li t0, 0xe0000000 // IRQ_SYSCALL_LO (IRQ30) trigger address sw zero, 0(t0) // Raise IRQ by writing to interrupt trigger address. // Writing any data triggers an interrupt. @@ -70,6 +97,27 @@ app_start: jalr zero, 0(zero) // Jumping to firmware. Expecting trap app_loop: j app_loop + + +check_cannot_read_test_val_from_fw_ram: + li t0, 0xd0000000 + lw t1, 0(t0) + li t2, 0 + bne t1, t2, cannot_read_test_val_from_fw_ram_fail + ret +cannot_read_test_val_from_fw_ram_fail: + illegal_insn() + +check_can_read_test_val_from_fw_ram: + // Check that saved test value can not be read while in app mode + li t0, 0xd0000000 + lw t1, 0(t0) + li t2, 0x5555aaaa + bne t1, t2, can_read_test_val_from_fw_ram_fail + ret +can_read_test_val_from_fw_ram_fail: + illegal_insn() + .align 4 app_end: diff --git a/hw/application_fpga/rtl/application_fpga.v b/hw/application_fpga/rtl/application_fpga.v index d937ec75..113a362d 100644 --- a/hw/application_fpga/rtl/application_fpga.v +++ b/hw/application_fpga/rtl/application_fpga.v @@ -135,6 +135,7 @@ module application_fpga ( reg [31 : 0] fw_ram_write_data; wire [31 : 0] fw_ram_read_data; wire fw_ram_ready; + wire fw_ram_en; reg touch_sense_cs; reg touch_sense_we; @@ -257,7 +258,7 @@ module application_fpga ( .clk(clk), .reset_n(reset_n), - .system_mode(system_mode), + .en(fw_ram_en), .cs(fw_ram_cs), .we(fw_ram_we), @@ -370,6 +371,8 @@ module application_fpga ( .access_level_hi (irq31_eoi), .access_level_med(irq30_eoi), + .fw_ram_en(fw_ram_en), + .cs(tk1_cs), .we(tk1_we), .address(tk1_address), diff --git a/hw/application_fpga/tb/application_fpga_sim.v b/hw/application_fpga/tb/application_fpga_sim.v index 6a7cbca3..f1e75d57 100644 --- a/hw/application_fpga/tb/application_fpga_sim.v +++ b/hw/application_fpga/tb/application_fpga_sim.v @@ -147,6 +147,7 @@ module application_fpga_sim ( reg [31 : 0] fw_ram_write_data; wire [31 : 0] fw_ram_read_data; wire fw_ram_ready; + wire fw_ram_en; reg touch_sense_cs; reg touch_sense_we; @@ -268,8 +269,7 @@ module application_fpga_sim ( .clk(clk), .reset_n(reset_n), - .system_mode(system_mode), - + .en(fw_ram_en), .cs(fw_ram_cs), .we(fw_ram_we), .address(fw_ram_address), @@ -383,6 +383,8 @@ module application_fpga_sim ( .access_level_hi (irq31_eoi), .access_level_med(irq30_eoi), + .fw_ram_en(fw_ram_en), + .cs(tk1_cs), .we(tk1_we), .address(tk1_address), diff --git a/hw/application_fpga/tb/tb_application_fpga_sim.v b/hw/application_fpga/tb/tb_application_fpga_sim.v index a3d5d6d6..78c4ebdb 100644 --- a/hw/application_fpga/tb/tb_application_fpga_sim.v +++ b/hw/application_fpga/tb/tb_application_fpga_sim.v @@ -80,7 +80,7 @@ module tb_application_fpga_sim (); //---------------------------------------------------------------- initial begin // End simulation after XXX time units (set by timescale) - #1600; + #3000; $display("TIMEOUT"); $finish; end