From 046343e525050d525dcf840ca1152067db819748 Mon Sep 17 00:00:00 2001 From: Michael Cardell Widerkrantz Date: Mon, 4 Mar 2024 15:42:11 +0100 Subject: [PATCH 1/3] Change memory constants to defines Instead of putting memory constant into an enum we use defines. Use the direct memory address instead of ORing constants together to compute the address. An enum in ISO C is a signed int. Some of are memory addresses are to large to fit in a signed int. This is not a problem since we're not using ISO C (-std=gnu99) but it doesn't look very nice if you turn on pedantic warnings. Also, if someone would use another compiler which at least supports the inline assembly we use, but possible not other GNU extensions, things would probably break. --- hw/application_fpga/fw/tk1_mem.h | 211 ++++++++++++++++++------------- 1 file changed, 120 insertions(+), 91 deletions(-) diff --git a/hw/application_fpga/fw/tk1_mem.h b/hw/application_fpga/fw/tk1_mem.h index f75af526..d4d99bc1 100644 --- a/hw/application_fpga/fw/tk1_mem.h +++ b/hw/application_fpga/fw/tk1_mem.h @@ -7,96 +7,125 @@ // clang-format off -#ifndef TK1_MEM_H -#define TK1_MEM_H - -// The canonical location of this file is: -// repo: https://github.com/tillitis/tillitis-key1 -// path: /hw/application_fpga/fw/tk1_mem.h - -// The contents are derived from the Verilog code. For use by QEMU model, -// firmware, and apps. - -enum { - TK1_ROM_BASE = 0x00000000, // 0b00000000... - TK1_RAM_BASE = 0x40000000, // 0b01000000... - TK1_RAM_SIZE = 0x20000, // 128 KB - TK1_RESERVED_BASE = 0x80000000, // 0b10000000... - TK1_MMIO_BASE = 0xc0000000, // 0b11000000... - TK1_MMIO_SIZE = 0xffffffff - TK1_MMIO_BASE, - - TK1_APP_MAX_SIZE = TK1_RAM_SIZE, - - TK1_MMIO_TRNG_BASE = TK1_MMIO_BASE | 0x00000000, - TK1_MMIO_TIMER_BASE = TK1_MMIO_BASE | 0x01000000, - TK1_MMIO_UDS_BASE = TK1_MMIO_BASE | 0x02000000, - TK1_MMIO_UART_BASE = TK1_MMIO_BASE | 0x03000000, - TK1_MMIO_TOUCH_BASE = TK1_MMIO_BASE | 0x04000000, - TK1_MMIO_FW_RAM_BASE = TK1_MMIO_BASE | 0x10000000, - TK1_MMIO_FW_RAM_SIZE = 2048, - // This "core" only exists in QEMU - TK1_MMIO_QEMU_BASE = TK1_MMIO_BASE | 0x3e000000, - TK1_MMIO_TK1_BASE = TK1_MMIO_BASE | 0x3f000000, // 0xff000000 - - TK1_NAME0_SUFFIX = 0x00, - TK1_NAME1_SUFFIX = 0x04, - TK1_VERSION_SUFFIX = 0x08, - - TK1_MMIO_TRNG_STATUS = TK1_MMIO_TRNG_BASE | 0x24, - TK1_MMIO_TRNG_STATUS_READY_BIT = 0, - TK1_MMIO_TRNG_ENTROPY = TK1_MMIO_TRNG_BASE | 0x80, - - TK1_MMIO_TIMER_CTRL = TK1_MMIO_TIMER_BASE | 0x20, - TK1_MMIO_TIMER_CTRL_START_BIT = 0, - TK1_MMIO_TIMER_CTRL_STOP_BIT = 1, - TK1_MMIO_TIMER_STATUS = TK1_MMIO_TIMER_BASE | 0x24, - TK1_MMIO_TIMER_STATUS_RUNNING_BIT = 0, - TK1_MMIO_TIMER_PRESCALER = TK1_MMIO_TIMER_BASE | 0x28, - TK1_MMIO_TIMER_TIMER = TK1_MMIO_TIMER_BASE | 0x2c, - - TK1_MMIO_UDS_FIRST = TK1_MMIO_UDS_BASE | 0x40, - TK1_MMIO_UDS_LAST = TK1_MMIO_UDS_BASE | 0x5c, // Address of last 32-bit word of UDS - - TK1_MMIO_UART_BIT_RATE = TK1_MMIO_UART_BASE | 0x40, - TK1_MMIO_UART_DATA_BITS = TK1_MMIO_UART_BASE | 0x44, - TK1_MMIO_UART_STOP_BITS = TK1_MMIO_UART_BASE | 0x48, - TK1_MMIO_UART_RX_STATUS = TK1_MMIO_UART_BASE | 0x80, - TK1_MMIO_UART_RX_DATA = TK1_MMIO_UART_BASE | 0x84, - TK1_MMIO_UART_RX_BYTES = TK1_MMIO_UART_BASE | 0x88, - TK1_MMIO_UART_TX_STATUS = TK1_MMIO_UART_BASE | 0x100, - TK1_MMIO_UART_TX_DATA = TK1_MMIO_UART_BASE | 0x104, - - TK1_MMIO_TOUCH_STATUS = TK1_MMIO_TOUCH_BASE | 0x24, - TK1_MMIO_TOUCH_STATUS_EVENT_BIT = 0, - - // This will only ever exist in QEMU: - TK1_MMIO_QEMU_DEBUG = TK1_MMIO_QEMU_BASE | 0x1000, - - TK1_MMIO_TK1_NAME0 = TK1_MMIO_TK1_BASE | TK1_NAME0_SUFFIX, - TK1_MMIO_TK1_NAME1 = TK1_MMIO_TK1_BASE | TK1_NAME1_SUFFIX, - TK1_MMIO_TK1_VERSION = TK1_MMIO_TK1_BASE | TK1_VERSION_SUFFIX, - TK1_MMIO_TK1_SWITCH_APP = TK1_MMIO_TK1_BASE | 0x20, - TK1_MMIO_TK1_LED = TK1_MMIO_TK1_BASE | 0x24, - TK1_MMIO_TK1_LED_R_BIT = 2, - TK1_MMIO_TK1_LED_G_BIT = 1, - TK1_MMIO_TK1_LED_B_BIT = 0, - TK1_MMIO_TK1_GPIO = TK1_MMIO_TK1_BASE | 0x28, - TK1_MMIO_TK1_GPIO1_BIT = 0, - TK1_MMIO_TK1_GPIO2_BIT = 1, - TK1_MMIO_TK1_GPIO3_BIT = 2, - TK1_MMIO_TK1_GPIO4_BIT = 3, - TK1_MMIO_TK1_APP_ADDR = TK1_MMIO_TK1_BASE | 0x30, - TK1_MMIO_TK1_APP_SIZE = TK1_MMIO_TK1_BASE | 0x34, - TK1_MMIO_TK1_BLAKE2S = TK1_MMIO_TK1_BASE | 0x40, - TK1_MMIO_TK1_CDI_FIRST = TK1_MMIO_TK1_BASE | 0x80, - TK1_MMIO_TK1_CDI_LAST = TK1_MMIO_TK1_BASE | 0x9c, // Address of last 32-bit word of CDI. - TK1_MMIO_TK1_UDI_FIRST = TK1_MMIO_TK1_BASE | 0xc0, - TK1_MMIO_TK1_UDI_LAST = TK1_MMIO_TK1_BASE | 0xc4, // Address of last 32-bit word of UDI. - TK1_MMIO_TK1_RAM_ASLR = TK1_MMIO_TK1_BASE | 0x100, - TK1_MMIO_TK1_RAM_SCRAMBLE = TK1_MMIO_TK1_BASE | 0x104, - TK1_MMIO_TK1_CPU_MON_CTRL = TK1_MMIO_TK1_BASE | 0x180, - TK1_MMIO_TK1_CPU_MON_FIRST = TK1_MMIO_TK1_BASE | 0x184, - TK1_MMIO_TK1_CPU_MON_LAST = TK1_MMIO_TK1_BASE | 0x188, -}; +#ifndef TKEY_TK1_MEM_H +#define TKEY_TK1_MEM_H +/* + + The canonical location of this file is in: + + https://github.com/tillitis/tillitis-key1 + + /hw/application_fpga/fw/tk1_mem.h + + The contents are derived from the Verilog code. For use by QEMU model, + firmware, and apps. + + Memory map + + Top level prefix, the first 2 bits in a 32-bit address: + + name prefix address length + -------------------------------------------------------- + ROM 0b00 30 bit address + RAM 0b01 30 bit address + Reserved 0b10 + MMIO 0b11 6 bits for core select, 24 bits rest + + Address Prefix, the first 8 bits in a 32-bit address: + + name prefix + -------------------- + ROM 0x00 + RAM 0x40 + MMIO 0xc0 + MMIO TRNG 0xc0 + MMIO TIMER 0xc1 + MMIO UDS 0xc2 + MMIO UART 0xc3 + MMIO TOUCH 0xc4 + MMIO FW_RAM 0xd0 + MMIO TK1 0xff + */ + +#define TK1_ROM_BASE 0x00000000 +#define TK1_RAM_BASE 0x40000000 +#define TK1_RAM_SIZE 0x20000 + +#define TK1_APP_MAX_SIZE 0x20000 + +#define TK1_MMIO_FW_RAM_BASE 0xd0000000 +// FW_RAM is 2048 bytes +#define TK1_MMIO_FW_RAM_SIZE 0x800 + +#define TK1_MMIO_TRNG_BASE 0xc0000000 +#define TK1_MMIO_TRNG_STATUS 0xc0000024 +#define TK1_MMIO_TRNG_STATUS_READY_BIT 0 +#define TK1_MMIO_TRNG_ENTROPY 0xc0000080 + +#define TK1_MMIO_TIMER_CTRL 0xc1000020 +#define TK1_MMIO_TIMER_CTRL_START_BIT 0 +#define TK1_MMIO_TIMER_CTRL_STOP_BIT 1 + +#define TK1_MMIO_TIMER_STATUS 0xc1000024 +#define TK1_MMIO_TIMER_STATUS_RUNNING_BIT 0 +#define TK1_MMIO_TIMER_PRESCALER 0xc1000028 +#define TK1_MMIO_TIMER_TIMER 0xc100002c + +#define TK1_MMIO_UDS_BASE 0xc2000000 +#define TK1_MMIO_UDS_FIRST 0xc2000040 +#define TK1_MMIO_UDS_LAST 0xc200005c + +#define TK1_MMIO_UART_BASE 0xc3000000 +#define TK1_MMIO_UART_BIT_RATE 0xc3000040 +#define TK1_MMIO_UART_DATA_BITS 0xc3000044 +#define TK1_MMIO_UART_STOP_BITS 0xc3000048 +#define TK1_MMIO_UART_RX_STATUS 0xc3000080 +#define TK1_MMIO_UART_RX_DATA 0xc3000084 +#define TK1_MMIO_UART_RX_BYTES 0xc3000088 +#define TK1_MMIO_UART_TX_STATUS 0xc3000100 +#define TK1_MMIO_UART_TX_DATA 0xc3000104 + +#define TK1_MMIO_TOUCH_BASE 0xc4000000 +#define TK1_MMIO_TOUCH_STATUS 0xc4000024 +#define TK1_MMIO_TOUCH_STATUS_EVENT_BIT 0 + +// This only exists in QEMU +#define TK1_MMIO_QEMU_DEBUG 0xfe001000 + +#define TK1_MMIO_TK1_NAME0 0xff000000 +#define TK1_MMIO_TK1_NAME1 0xff000004 +#define TK1_MMIO_TK1_VERSION 0xff000008 + +#define TK1_MMIO_TK1_SWITCH_APP 0xff000020 + +#define TK1_MMIO_TK1_LED 0xff000024 +#define TK1_MMIO_TK1_LED_R_BIT 2 +#define TK1_MMIO_TK1_LED_G_BIT 1 +#define TK1_MMIO_TK1_LED_B_BIT 0 + +#define TK1_MMIO_TK1_GPIO 0xff000028 +#define TK1_MMIO_TK1_GPIO1_BIT 0 +#define TK1_MMIO_TK1_GPIO2_BIT 1 +#define TK1_MMIO_TK1_GPIO3_BIT 2 +#define TK1_MMIO_TK1_GPIO4_BIT 3 + +#define TK1_MMIO_TK1_APP_ADDR 0xff000030 +#define TK1_MMIO_TK1_APP_SIZE 0xff000034 + +#define TK1_MMIO_TK1_BLAKE2S 0xff000040 + +#define TK1_MMIO_TK1_CDI_FIRST 0xff000080 +#define TK1_MMIO_TK1_CDI_LAST 0xff00009c + +#define TK1_MMIO_TK1_UDI_FIRST 0xff0000c0 +#define TK1_MMIO_TK1_UDI_LAST 0xff0000c4 + +#define TK1_MMIO_TK1_RAM_ASLR 0xff000100 +#define TK1_MMIO_TK1_RAM_SCRAMBLE 0xff000104 + +#define TK1_MMIO_TK1_CPU_MON_CTRL 0xff000180 +#define TK1_MMIO_TK1_CPU_MON_FIRST 0xff000184 +#define TK1_MMIO_TK1_CPU_MON_LAST 0xff000188 #endif + From e085d0ebd03a4e8eef79fb5bb020172d03c25c3e Mon Sep 17 00:00:00 2001 From: Michael Cardell Widerkrantz Date: Mon, 4 Mar 2024 15:53:12 +0100 Subject: [PATCH 2/3] Add void to function signatures meant to be used without args --- hw/application_fpga/fw/testfw/main.c | 4 ++-- hw/application_fpga/fw/tk1/main.c | 14 +++++++------- hw/application_fpga/fw/tk1/proto.c | 2 +- hw/application_fpga/fw/tk1/proto.h | 2 +- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/hw/application_fpga/fw/testfw/main.c b/hw/application_fpga/fw/testfw/main.c index 835d3705..4302097e 100644 --- a/hw/application_fpga/fw/testfw/main.c +++ b/hw/application_fpga/fw/testfw/main.c @@ -106,7 +106,7 @@ uint32_t wait_timer_tick(uint32_t last_timer) } } -void zero_fwram() +void zero_fwram(void) { for (int i = 0; i < TK1_MMIO_FW_RAM_SIZE; i++) { fw_ram[i] = 0x00; @@ -149,7 +149,7 @@ void failmsg(char *s) puts("\r\n"); } -int main() +int main(void) { // Function pointer to blake2s() volatile int (*fw_blake2s)(void *, unsigned long, const void *, diff --git a/hw/application_fpga/fw/tk1/main.c b/hw/application_fpga/fw/tk1/main.c index 2aa6240d..60c1e719 100644 --- a/hw/application_fpga/fw/tk1/main.c +++ b/hw/application_fpga/fw/tk1/main.c @@ -42,9 +42,9 @@ struct context { uint8_t uss[32]; // User Supplied Secret, if any }; -static void print_hw_version(); +static void print_hw_version(void); static void print_digest(uint8_t *md); -static uint32_t rnd_word(); +static uint32_t rnd_word(void); static void compute_cdi(const uint8_t *digest, const uint8_t use_uss, const uint8_t *uss); static void copy_name(uint8_t *buf, const size_t bufsiz, const uint32_t word); @@ -55,9 +55,9 @@ static enum state loading_commands(const struct frame_header *hdr, const uint8_t *cmd, enum state state, struct context *ctx); static void run(const struct context *ctx); -static void scramble_ram(); +static void scramble_ram(void); -static void print_hw_version() +static void print_hw_version(void) { htif_puts("Hello, I'm firmware with"); htif_puts(" tk1_name0:"); @@ -81,7 +81,7 @@ static void print_digest(uint8_t *md) htif_lf(); } -static uint32_t rnd_word() +static uint32_t rnd_word(void) { while ((*trng_status & (1 << TK1_MMIO_TRNG_STATUS_READY_BIT)) == 0) { } @@ -362,7 +362,7 @@ static void run(const struct context *ctx) __builtin_unreachable(); } -static void scramble_ram() +static void scramble_ram(void) { uint32_t *ram = (uint32_t *)(TK1_RAM_BASE); uint32_t rnd = rnd_word(); @@ -384,7 +384,7 @@ static void scramble_ram() *ram_scramble = rnd_word(); } -int main() +int main(void) { struct context ctx = {0}; struct frame_header hdr = {0}; diff --git a/hw/application_fpga/fw/tk1/proto.c b/hw/application_fpga/fw/tk1/proto.c index a78c5391..b01a3f79 100644 --- a/hw/application_fpga/fw/tk1/proto.c +++ b/hw/application_fpga/fw/tk1/proto.c @@ -142,7 +142,7 @@ static void write(uint8_t *buf, size_t nbytes) } } -uint8_t readbyte() +uint8_t readbyte(void) { for (;;) { if (*can_rx) { diff --git a/hw/application_fpga/fw/tk1/proto.h b/hw/application_fpga/fw/tk1/proto.h index 105d5ad0..4bf225f0 100644 --- a/hw/application_fpga/fw/tk1/proto.h +++ b/hw/application_fpga/fw/tk1/proto.h @@ -51,7 +51,7 @@ struct frame_header { }; void writebyte(uint8_t b); -uint8_t readbyte(); +uint8_t readbyte(void); void fwreply(struct frame_header hdr, enum fwcmd rspcode, uint8_t *buf); int readcommand(struct frame_header *hdr, uint8_t *cmd, int state); #endif From 746d7f0e0dc83066bb7bf538346a1faaf835aa86 Mon Sep 17 00:00:00 2001 From: Michael Cardell Widerkrantz Date: Mon, 4 Mar 2024 15:57:29 +0100 Subject: [PATCH 3/3] Use pedantic warnings Use pedantic warnings but still allow inline assembly, so turn off language-extension-token warnings. --- hw/application_fpga/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/application_fpga/Makefile b/hw/application_fpga/Makefile index 38efc6c2..350e66d9 100644 --- a/hw/application_fpga/Makefile +++ b/hw/application_fpga/Makefile @@ -36,7 +36,7 @@ CC = clang CFLAGS = -target riscv32-unknown-none-elf -march=rv32iczmmul -mabi=ilp32 \ -static -std=gnu99 -O2 -ffast-math -fno-common -fno-builtin-printf \ -fno-builtin-putchar -fno-builtin-memcpy -nostdlib -mno-relax -Wall \ - -flto -g -DNOCONSOLE + -Wpedantic -Wno-language-extension-token -flto -g -DNOCONSOLE AS = clang ASFLAGS = -target riscv32-unknown-none-elf -march=rv32iczmmul -mabi=ilp32 -mno-relax