From d1c1d7d7ee943aca2731b0b9bd344d8e1727b4d2 Mon Sep 17 00:00:00 2001 From: Kuan-Wei Chiu Date: Tue, 9 Jan 2024 05:24:51 +0800 Subject: [PATCH 01/14] Fix signed integer overflow in RV32M The current implementation of the mul instruction does not guard against integer overflow, potentially leading to undefined behavior. Cast the operands to int64_t before performing the multiplication to ensure that the result can be accommodated without overflow. The lower 32 bits of the product are then extracted, preserving the correct uint32_t type. --- riscv.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/riscv.c b/riscv.c index 36ccfa4..3c65a1b 100644 --- a/riscv.c +++ b/riscv.c @@ -594,8 +594,11 @@ static uint32_t op_mul(uint32_t insn, uint32_t a, uint32_t b) { /* TODO: Test ifunc7 zeros */ switch (decode_func3(insn)) { - case 0b000: /* MUL */ - return a * b; + case 0b000: { /* MUL */ + const int64_t _a = (int32_t) a; + const int64_t _b = (int32_t) b; + return ((uint64_t) (_a * _b)) & ((1ULL << 32) - 1); + } case 0b001: { /* MULH */ const int64_t _a = (int32_t) a; const int64_t _b = (int32_t) b; From b790a3376929a5efca78a704cd318c64aec93a4d Mon Sep 17 00:00:00 2001 From: Kuan-Wei Chiu Date: Tue, 9 Jan 2024 05:41:55 +0800 Subject: [PATCH 02/14] Fix undefined behavior in RV_MARCHID definition The RV_MARCHID macro is defined using a left shift operation without explicitly specifying the type of the constant. This can lead to undefined behavior, specifically a shift too many bits issue. Modify the RV_MARCHID definition to use the ULL suffix for the constant, ensuring it is treated as an unsigned long long. This eliminates the undefined behavior. --- main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/main.c b/main.c index f9bc178..b6238bf 100644 --- a/main.c +++ b/main.c @@ -185,7 +185,7 @@ static inline sbi_ret_t handle_sbi_ecall_RST(vm_t *vm, int32_t fid) } #define RV_MVENDORID 0x12345678 -#define RV_MARCHID ((1 << 31) | 1) +#define RV_MARCHID ((1ULL << 31) | 1) #define RV_MIMPID 1 static inline sbi_ret_t handle_sbi_ecall_BASE(vm_t *vm, int32_t fid) From fb25cb18a154a75a7e2c62427683a3fc3620f763 Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Tue, 9 Jan 2024 14:54:56 +0800 Subject: [PATCH 03/14] Bump copyright year --- LICENSE | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/LICENSE b/LICENSE index 6ad766b..b05666a 100644 --- a/LICENSE +++ b/LICENSE @@ -1,6 +1,6 @@ MIT License -Copyright (C) 2022-2023 National Cheng Kung University, Taiwan. +Copyright (C) 2022-2024 National Cheng Kung University, Taiwan. All rights reserved. Permission is hereby granted, free of charge, to any person obtaining a copy From 0252892e8f7a2a966ca02d403e3e2319573b8cc4 Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Tue, 9 Jan 2024 14:58:47 +0800 Subject: [PATCH 04/14] Proofread --- CONTRIBUTING.md | 87 +++++++++++++++++++++++++++++++++++++------------ 1 file changed, 66 insertions(+), 21 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 25e99b3..d15633d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -2,42 +2,57 @@ :+1::tada: First off, thanks for taking the time to contribute! :tada::+1: -The following is a set of guidelines for contributing to [semu](https://github.com/jserv/semu) -hosted on GitHub. These are mostly guidelines, not rules. Use your best -judgment, and feel free to propose changes to this document in a pull request. +The following is a set of guidelines for contributing to [semu](https://github.com/sysprog21/semu) +hosted on GitHub. These are mostly guidelines, not rules. +Use your best judgment, and feel free to propose changes to this document in a pull request. ## Issues -This project uses GitHub Issues to track ongoing development, discuss project plans, and keep track of bugs. Be sure to search for existing issues before you create another one. +This project uses GitHub Issues to track ongoing development, discuss project plans, and keep track of bugs. +Be sure to search for existing issues before you create another one. -Visit our [Issues page on GitHub](https://github.com/jserv/semu/issues) to search and submit. +Visit our [Issues page on GitHub](https://github.com/sysprog21/semu/issues) to search and submit. ## Coding Convention -We welcome all contributions from corporate, academic and individual developers. However, there are a number of fundamental ground rules that you must adhere to in order to participate. These rules are outlined as follows: -* All code must adhere to the existing C coding style (see below). While we are somewhat flexible in basic style, you will adhere to what is currently in place. Uncommented, complicated algorithmic constructs will be rejected. -* All external pull requests must contain sufficient documentation in the pull request comments in order to be accepted. +Contributions from developers across corporations, academia, and individuals are welcome. +However, participation requires adherence to fundamental ground rules: +* Code must strictly adhere to the established C coding style (refer to the guidelines below). + While there is some flexibility in basic style, it is crucial to stick to the current coding standards. + Complex algorithmic constructs without proper comments will not be accepted. +* External pull requests should include thorough documentation in the pull request comments for consideration. Software requirement: [clang-format](https://clang.llvm.org/docs/ClangFormat.html) version 12 or later. -Use the command `$ clang-format -i *.[ch]` to enforce a consistent coding style. +This repository consistently contains an up-to-date `.clang-format` file with rules that match the explained ones. +For maintaining a uniform coding style, execute the command `clang-format -i *.[ch]`. ## Coding Style for Modern C -This coding style is a variation of the K&R style. Some general principles: honor tradition, but accept progress; be consistent; -embrace the latest C standards; embrace modern compilers, their static analysis -capabilities and sanitizers. +This coding style is a variant of the K&R style. +Adhere to established practices while being open to innovation. +Maintain consistency, adopt the latest C standards, +and embrace modern compilers along with their advanced static analysis capabilities and sanitizers. ### Indentation -Use 4 spaces rather than tabs. +In this coding style guide, the use of 4 spaces for indentation instead of tabs is strongly enforced to ensure consistency. +Consistently apply a single space before and after comparison and assignment operators to maintain readable code. +Additionally, it is crucial to include a single space after every comma. +e.g., +```c +for (int i = 0; i < 10; i++) { + printf("%d\n", i); + /* some operations */ +} +``` ### Line length -All lines should generally be within 80 characters. Wrap long lines. -There are some good reasons behind this: -* It forces the developer to write more succinct code; -* Humans are better at processing information in smaller quantity portions; +All lines should typically stay within 80 characters, and longer lines should be wrapped. +There are valid rationales for this practice: +* It encourages concise code writing by developers. +* Smaller portions of information are easier for humans to process. * It helps users of vi/vim (and potentially other editors) who use vertical splits. ### Comments @@ -66,7 +81,12 @@ Leave two spaces between the statement and the inline comment. ### Spacing and brackets Use one space after the conditional or loop keyword, no spaces around -their brackets, and one space before the opening curly bracket. +their brackets, and one space before the opening curly bracket. e.g., +```c +do { + /* some operations */ +} while (condition); +``` Functions (their declarations or calls), `sizeof` operator or similar macros shall not have a space after their name/keyword or around the @@ -81,6 +101,8 @@ but otherwise avoid redundant or excessive brackets. ### Variable names and declarations +- Ensure that functions, variables, and comments are consistently named using English names/text. + - Use descriptive names for global variables and short names for locals. Find the right balance between descriptive and succinct. @@ -97,6 +119,29 @@ const uint8_t * const charmap; /* const pointer and const data */ const void * restrict key; /* const pointer which does not alias */ ``` +- Local variables of the same type should be declared in the same line. +```c +void func(void) +{ + char a, b; /* OK */ + + char a; + char b; /* Incorrect: A variable with char type already exists. */ +} +``` + +- Always include a trailing comma in the last element of structure initialization, including its children, to assist clang-format in correctly formatting structures. However, this can be omitted in very simple and short structures. +```c +typedef struct { + int width, height; +} screen_t; + +screen_t s = { + .width = 640, + .height = 480, /* comma here */ +} +``` + ### Type definitions Declarations shall be on the same line, e.g., @@ -418,9 +463,9 @@ printf("val %lld\n", SOME_CONSTANT); #### Avoid unaligned access -Do not assume unaligned access is safe. It is not safe on Arm, POWER, -and various other architectures. Moreover, even on x86 unaligned access -is slower. +Avoid assuming that unaligned access is safe. +It is not secure on architectures like Arm, POWER, and others. +Additionally, even on x86, unaligned access can be slower. #### Avoid extreme portability From 2600bf8b929e72d38df1ea30652e8c6ac7f19b94 Mon Sep 17 00:00:00 2001 From: ChinYikMing Date: Mon, 15 Jan 2024 01:27:12 +0800 Subject: [PATCH 05/14] Suppress DTC warnings The DTC of version 1.6.1 generates the following warnings: 1. (unit_address_vs_reg) /soc: node has a reg or ranges property, but no unit name 2. (interrupt_provider): /cpus/cpu@0/interrupt-controller: Missing #address-cells in interrupt provider To suppress them, add unit-name with parent-bus-address to the SOC node and #address-cells to the CPU interrupt controller. --- minimal.dts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/minimal.dts b/minimal.dts index f6126f1..0e631d3 100644 --- a/minimal.dts +++ b/minimal.dts @@ -6,7 +6,7 @@ model = "semu"; aliases { - serial0 = "/soc/serial@4000000"; + serial0 = "/soc@F0000000/serial@4000000"; }; chosen { @@ -28,6 +28,7 @@ mmu-type = "riscv,rv32"; cpu0_intc: interrupt-controller { #interrupt-cells = <1>; + #address-cells = <0>; interrupt-controller; compatible = "riscv,cpu-intc"; }; @@ -40,7 +41,7 @@ reg-names = "sram0"; }; - soc { + soc: soc@F0000000 { #address-cells = <1>; #size-cells = <1>; compatible = "simple-bus"; From ac75287e6774b87e0592dbae9351cf8c82a46bb5 Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Wed, 17 Jan 2024 23:59:19 +0800 Subject: [PATCH 06/14] Cache MMU fetch access The emulator's environment interface has been updated to facilitate page lookup instead of specific fetches. Additionally, a preliminary cache for fetches has been implemented, which encompasses both MMU and physical page lookups. This enhancement boosts performance. This approach, which includes bypassing the PTE bit set operation in the cache, should be compliant with RISC-V standards. It allows MMU lookups to be carried out speculatively, adhering to the architecture's guidelines. --- main.c | 7 ++++--- riscv.c | 36 +++++++++++++++++++++++++++++------- riscv.h | 19 ++++++++++++++----- 3 files changed, 47 insertions(+), 15 deletions(-) diff --git a/main.c b/main.c index b6238bf..ced5eb8 100644 --- a/main.c +++ b/main.c @@ -15,15 +15,15 @@ /* Define fetch separately since it is simpler (fixed width, already checked * alignment, only main RAM is executable). */ -static void mem_fetch(vm_t *vm, uint32_t addr, uint32_t *value) +static void mem_fetch(vm_t *vm, uint32_t n_pages, uint32_t **page_addr) { emu_state_t *data = (emu_state_t *) vm->priv; - if (unlikely(addr >= RAM_SIZE)) { + if (unlikely(n_pages >= RAM_SIZE / RV_PAGE_SIZE)) { /* TODO: check for other regions */ vm_set_exception(vm, RV_EXC_FETCH_FAULT, vm->exc_val); return; } - *value = data->ram[addr >> 2]; + *page_addr = &data->ram[n_pages << (RV_PAGE_SHIFT - 2)]; } /* Similarly, only main memory pages can be used as page tables. */ @@ -369,6 +369,7 @@ static int semu_start(int argc, char **argv) .mem_store = mem_store, .mem_page_table = mem_page_table, }; + vm_init(&vm); /* Set up RAM */ emu.ram = mmap(NULL, RAM_SIZE, PROT_READ | PROT_WRITE, diff --git a/riscv.c b/riscv.c index 3c65a1b..785b2fc 100644 --- a/riscv.c +++ b/riscv.c @@ -167,11 +167,17 @@ static inline uint32_t read_rs2(const vm_t *vm, uint32_t insn) /* virtual addressing */ +static void mmu_invalidate(vm_t *vm) +{ + vm->cache_fetch.n_pages = 0xFFFFFFFF; +} + /* Pre-verify the root page table to minimize page table access during * translation time. */ static void mmu_set(vm_t *vm, uint32_t satp) { + mmu_invalidate(vm); if (satp >> 31) { uint32_t *page_table = vm->mem_page_table(vm, satp & MASK(22)); if (!page_table) @@ -267,18 +273,27 @@ static void mmu_translate(vm_t *vm, *addr = ((*addr) & MASK(RV_PAGE_SHIFT)) | (ppn << RV_PAGE_SHIFT); } -static void mmu_fence(vm_t *vm UNUSED, uint32_t insn UNUSED) +static void mmu_fence(vm_t *vm, uint32_t insn UNUSED) { - /* no-op for now */ + mmu_invalidate(vm); } static void mmu_fetch(vm_t *vm, uint32_t addr, uint32_t *value) { - mmu_translate(vm, &addr, (1 << 3), (1 << 6), false, RV_EXC_FETCH_FAULT, - RV_EXC_FETCH_PFAULT); - if (vm->error) - return; - vm->mem_fetch(vm, addr, value); + uint32_t vpn = addr >> RV_PAGE_SHIFT; + if (unlikely(vpn != vm->cache_fetch.n_pages)) { + mmu_translate(vm, &addr, (1 << 3), (1 << 6), false, RV_EXC_FETCH_FAULT, + RV_EXC_FETCH_PFAULT); + if (vm->error) + return; + uint32_t *page_addr; + vm->mem_fetch(vm, addr >> RV_PAGE_SHIFT, &page_addr); + if (vm->error) + return; + vm->cache_fetch.n_pages = vpn; + vm->cache_fetch.page_addr = page_addr; + } + *value = vm->cache_fetch.page_addr[(addr >> 2) & MASK(RV_PAGE_SHIFT - 2)]; } static void mmu_load(vm_t *vm, @@ -347,6 +362,7 @@ void vm_trap(vm_t *vm) /* Set */ vm->sstatus_sie = false; + mmu_invalidate(vm); vm->s_mode = true; vm->pc = vm->stvec_addr; if (vm->stvec_vectored) @@ -359,6 +375,7 @@ static void op_sret(vm_t *vm) { /* Restore from stack */ vm->pc = vm->sepc; + mmu_invalidate(vm); vm->s_mode = vm->sstatus_spp; vm->sstatus_sie = vm->sstatus_spie; @@ -763,6 +780,11 @@ static void op_amo(vm_t *vm, uint32_t insn) } } +void vm_init(vm_t *vm) +{ + mmu_invalidate(vm); +} + void vm_step(vm_t *vm) { if (unlikely(vm->error)) diff --git a/riscv.h b/riscv.h index c416384..04b60b8 100644 --- a/riscv.h +++ b/riscv.h @@ -29,10 +29,15 @@ typedef enum { ERR_USER, /**< user-specific error */ } vm_error_t; -/* To use the emulator, start by initializing a "vm_t" struct with zero values - * and set the required environment-supplied callbacks. You may also set other - * necessary fields such as argument registers and s_mode, ensuring that all - * field restrictions are met to avoid undefined behavior. +typedef struct { + uint32_t n_pages; + uint32_t *page_addr; +} mmu_cache_t; + +/* To use the emulator, start by initializing a vm_t object with zero values, + * invoke vm_init(), and set the required environment-supplied callbacks. You + * may also set other necessary fields such as argument registers and s_mode, + * ensuring that all field restrictions are met to avoid undefined behavior. * * Once the emulator is set up, execute the emulation loop by calling * "vm_step()" repeatedly. Each call attempts to execute a single instruction. @@ -77,6 +82,8 @@ struct __vm_internal { */ uint32_t exc_cause, exc_val; + mmu_cache_t cache_fetch; + /* Supervisor state */ bool s_mode; bool sstatus_spp; /**< state saved at trap */ @@ -101,7 +108,7 @@ struct __vm_internal { /* Memory access sets the vm->error to indicate failure. On successful * access, it reads or writes the specified "value". */ - void (*mem_fetch)(vm_t *vm, uint32_t addr, uint32_t *value); + void (*mem_fetch)(vm_t *vm, uint32_t n_pages, uint32_t **page_addr); void (*mem_load)(vm_t *vm, uint32_t addr, uint8_t width, uint32_t *value); void (*mem_store)(vm_t *vm, uint32_t addr, uint8_t width, uint32_t value); @@ -112,6 +119,8 @@ struct __vm_internal { uint32_t *(*mem_page_table)(const vm_t *vm, uint32_t ppn); }; +void vm_init(vm_t *vm); + /* Emulate the next instruction. This is a no-op if the error is already set. */ void vm_step(vm_t *vm); From c27d0de2a0a7b6e7d18fbbed4274c459bf5b68f2 Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Thu, 18 Jan 2024 10:15:30 +0800 Subject: [PATCH 07/14] Enforce 64-bit timer manipulation The initial code was overly complex and also exhibited slightly slower performance. --- device.h | 2 +- main.c | 9 ++++----- riscv.c | 5 ++--- riscv.h | 2 +- 4 files changed, 8 insertions(+), 10 deletions(-) diff --git a/device.h b/device.h index fd7f07b..754065f 100644 --- a/device.h +++ b/device.h @@ -185,5 +185,5 @@ typedef struct { #if SEMU_HAS(VIRTIOBLK) virtio_blk_state_t vblk; #endif - uint32_t timer_lo, timer_hi; + uint64_t timer; } emu_state_t; diff --git a/main.c b/main.c index ced5eb8..c6e2fc0 100644 --- a/main.c +++ b/main.c @@ -162,8 +162,8 @@ static inline sbi_ret_t handle_sbi_ecall_TIMER(vm_t *vm, int32_t fid) emu_state_t *data = (emu_state_t *) vm->priv; switch (fid) { case SBI_TIMER__SET_TIMER: - data->timer_lo = vm->x_regs[RV_R_A0]; - data->timer_hi = vm->x_regs[RV_R_A1]; + data->timer = (((uint64_t) vm->x_regs[RV_R_A1]) << 32) | + (uint64_t) (vm->x_regs[RV_R_A0]); return (sbi_ret_t){SBI_SUCCESS, 0}; default: return (sbi_ret_t){SBI_ERR_NOT_SUPPORTED, 0}; @@ -406,7 +406,7 @@ static int semu_start(int argc, char **argv) atexit(unmap_files); /* Set up RISC-V hart */ - emu.timer_hi = emu.timer_lo = 0xFFFFFFFF; + emu.timer = 0xFFFFFFFFFFFFFFFF; vm.s_mode = true; vm.x_regs[RV_R_A0] = 0; /* hart ID. i.e., cpuid */ vm.x_regs[RV_R_A1] = dtb_addr; @@ -446,8 +446,7 @@ static int semu_start(int argc, char **argv) #endif } - if (vm.insn_count_hi > emu.timer_hi || - (vm.insn_count_hi == emu.timer_hi && vm.insn_count > emu.timer_lo)) + if (vm.insn_count > emu.timer) vm.sip |= RV_INT_STI_BIT; else vm.sip &= ~RV_INT_STI_BIT; diff --git a/riscv.c b/riscv.c index 785b2fc..d36fb22 100644 --- a/riscv.c +++ b/riscv.c @@ -439,7 +439,7 @@ static void csr_read(vm_t *vm, uint16_t addr, uint32_t *value) * and writes should set the value after the increment. However, * we do not expose any way to write the counters. */ - *value = (addr & (1 << 7)) ? vm->insn_count_hi : vm->insn_count; + *value = vm->insn_count >> ((addr & (1 << 7)) ? 32 : 0); } return; } @@ -806,9 +806,8 @@ void vm_step(vm_t *vm) return; vm->pc += 4; + /* Assume no integer overflow */ vm->insn_count++; - if (!vm->insn_count) - vm->insn_count_hi++; uint32_t insn_opcode = insn & MASK(7), value; switch (insn_opcode) { diff --git a/riscv.h b/riscv.h index 04b60b8..d318c31 100644 --- a/riscv.h +++ b/riscv.h @@ -68,7 +68,7 @@ struct __vm_internal { * utilized in these capacities and should not be modified between logical * resets. */ - uint32_t insn_count, insn_count_hi; + uint64_t insn_count; /* Instruction execution state must be set to "NONE" for instruction * execution to continue. If the state is not "NONE," the vm_step() From 71c37463eb03866dde860a53add0b981d15f9c45 Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Thu, 18 Jan 2024 10:57:16 +0800 Subject: [PATCH 08/14] Unify naming scheme --- plic.c | 4 ++-- ram.c | 4 ++-- riscv.c | 28 ++++++++++++++-------------- riscv_private.h | 2 +- uart.c | 4 ++-- virtio-blk.c | 4 ++-- virtio-net.c | 4 ++-- 7 files changed, 25 insertions(+), 25 deletions(-) diff --git a/plic.c b/plic.c index 0f72ae4..0801125 100644 --- a/plic.c +++ b/plic.c @@ -89,7 +89,7 @@ void plic_read(vm_t *vm, vm_set_exception(vm, RV_EXC_LOAD_MISALIGN, vm->exc_val); return; default: - vm_set_exception(vm, RV_EXC_ILLEGAL_INSTR, 0); + vm_set_exception(vm, RV_EXC_ILLEGAL_INSN, 0); return; } } @@ -110,7 +110,7 @@ void plic_write(vm_t *vm, vm_set_exception(vm, RV_EXC_STORE_MISALIGN, vm->exc_val); return; default: - vm_set_exception(vm, RV_EXC_ILLEGAL_INSTR, 0); + vm_set_exception(vm, RV_EXC_ILLEGAL_INSN, 0); return; } } diff --git a/ram.c b/ram.c index d14ffc5..49c7c7d 100644 --- a/ram.c +++ b/ram.c @@ -38,7 +38,7 @@ void ram_read(vm_t *vm, RAM_FUNC(1, *value = (uint32_t) (int32_t) (int8_t) ((*cell) >> offset)); break; default: - vm_set_exception(vm, RV_EXC_ILLEGAL_INSTR, 0); + vm_set_exception(vm, RV_EXC_ILLEGAL_INSN, 0); return; } } @@ -63,7 +63,7 @@ void ram_write(vm_t *vm, << offset); break; default: - vm_set_exception(vm, RV_EXC_ILLEGAL_INSTR, 0); + vm_set_exception(vm, RV_EXC_ILLEGAL_INSN, 0); return; } } diff --git a/riscv.c b/riscv.c index d36fb22..699846f 100644 --- a/riscv.c +++ b/riscv.c @@ -391,7 +391,7 @@ static void op_privileged(vm_t *vm, uint32_t insn) return; } if (insn & ((MASK(5) << 7) | (MASK(5) << 15))) { - vm_set_exception(vm, RV_EXC_ILLEGAL_INSTR, 0); + vm_set_exception(vm, RV_EXC_ILLEGAL_INSN, 0); return; } switch (decode_i_unsigned(insn)) { @@ -408,7 +408,7 @@ static void op_privileged(vm_t *vm, uint32_t insn) /* TODO: Implement this */ break; default: - vm_set_exception(vm, RV_EXC_ILLEGAL_INSTR, 0); + vm_set_exception(vm, RV_EXC_ILLEGAL_INSN, 0); break; } } @@ -432,7 +432,7 @@ static void csr_read(vm_t *vm, uint16_t addr, uint32_t *value) if ((addr >> 8) == 0xC) { uint16_t idx = addr & MASK(7); if (idx >= 0x20 || !(vm->s_mode || ((vm->scounteren >> idx) & 1))) - vm_set_exception(vm, RV_EXC_ILLEGAL_INSTR, 0); + vm_set_exception(vm, RV_EXC_ILLEGAL_INSN, 0); else { /* Use the instruction counter for all of the counters. * Ideally, reads should return the value before the increment, @@ -445,7 +445,7 @@ static void csr_read(vm_t *vm, uint16_t addr, uint32_t *value) } if (!vm->s_mode) { - vm_set_exception(vm, RV_EXC_ILLEGAL_INSTR, 0); + vm_set_exception(vm, RV_EXC_ILLEGAL_INSN, 0); return; } @@ -488,14 +488,14 @@ static void csr_read(vm_t *vm, uint16_t addr, uint32_t *value) *value = vm->stval; break; default: - vm_set_exception(vm, RV_EXC_ILLEGAL_INSTR, 0); + vm_set_exception(vm, RV_EXC_ILLEGAL_INSN, 0); } } static void csr_write(vm_t *vm, uint16_t addr, uint32_t value) { if (!vm->s_mode) { - vm_set_exception(vm, RV_EXC_ILLEGAL_INSTR, 0); + vm_set_exception(vm, RV_EXC_ILLEGAL_INSN, 0); return; } @@ -540,7 +540,7 @@ static void csr_write(vm_t *vm, uint16_t addr, uint32_t value) vm->stval = value; break; default: - vm_set_exception(vm, RV_EXC_ILLEGAL_INSTR, 0); + vm_set_exception(vm, RV_EXC_ILLEGAL_INSN, 0); } } @@ -600,7 +600,7 @@ static void op_system(vm_t *vm, uint32_t insn) break; default: - vm_set_exception(vm, RV_EXC_ILLEGAL_INSTR, 0); + vm_set_exception(vm, RV_EXC_ILLEGAL_INSN, 0); return; } } @@ -689,7 +689,7 @@ static bool op_jmp(vm_t *vm, uint32_t insn, uint32_t a, uint32_t b) case 0b101: /* BFUNC_BGE */ return ((int32_t) a) >= ((int32_t) b); } - vm_set_exception(vm, RV_EXC_ILLEGAL_INSTR, 0); + vm_set_exception(vm, RV_EXC_ILLEGAL_INSN, 0); return false; } @@ -724,7 +724,7 @@ static void op_jump_link(vm_t *vm, uint32_t insn, uint32_t addr) static void op_amo(vm_t *vm, uint32_t insn) { if (unlikely(decode_func3(insn) != 0b010 /* amo.w */)) - return vm_set_exception(vm, RV_EXC_ILLEGAL_INSTR, 0); + return vm_set_exception(vm, RV_EXC_ILLEGAL_INSN, 0); uint32_t addr = read_rs1(vm, insn); uint32_t value, value2; switch (decode_func5(insn)) { @@ -732,7 +732,7 @@ static void op_amo(vm_t *vm, uint32_t insn) if (addr & 0b11) return vm_set_exception(vm, RV_EXC_LOAD_MISALIGN, addr); if (decode_rs2(insn)) - return vm_set_exception(vm, RV_EXC_ILLEGAL_INSTR, 0); + return vm_set_exception(vm, RV_EXC_ILLEGAL_INSN, 0); mmu_load(vm, addr, RV_MEM_LW, &value, true); if (vm->error) return; @@ -775,7 +775,7 @@ static void op_amo(vm_t *vm, uint32_t insn) AMO_OP(value > value2 ? value : value2); break; default: - vm_set_exception(vm, RV_EXC_ILLEGAL_INSTR, 0); + vm_set_exception(vm, RV_EXC_ILLEGAL_INSN, 0); return; } } @@ -860,7 +860,7 @@ void vm_step(vm_t *vm) /* TODO: implement for multi-threading */ break; default: - vm_set_exception(vm, RV_EXC_ILLEGAL_INSTR, 0); + vm_set_exception(vm, RV_EXC_ILLEGAL_INSN, 0); break; } break; @@ -871,7 +871,7 @@ void vm_step(vm_t *vm) op_system(vm, insn); break; default: - vm_set_exception(vm, RV_EXC_ILLEGAL_INSTR, 0); + vm_set_exception(vm, RV_EXC_ILLEGAL_INSN, 0); break; } } diff --git a/riscv_private.h b/riscv_private.h index c4d5e55..94a3007 100644 --- a/riscv_private.h +++ b/riscv_private.h @@ -62,7 +62,7 @@ enum { enum { RV_EXC_PC_MISALIGN = 0, /**< Instruction address misaligned */ RV_EXC_FETCH_FAULT = 1, /**< Instruction access fault */ - RV_EXC_ILLEGAL_INSTR = 2, /**< Illegal instruction */ + RV_EXC_ILLEGAL_INSN = 2, /**< Illegal instruction */ RV_EXC_BREAKPOINT = 3, /**< Breakpoint */ RV_EXC_LOAD_MISALIGN = 4, /**< Load address misaligned */ RV_EXC_LOAD_FAULT = 5, /**< Load access fault */ diff --git a/uart.c b/uart.c index e9f0b44..bb0e0a7 100644 --- a/uart.c +++ b/uart.c @@ -182,7 +182,7 @@ void u8250_read(vm_t *vm, vm_set_exception(vm, RV_EXC_LOAD_MISALIGN, vm->exc_val); return; default: - vm_set_exception(vm, RV_EXC_ILLEGAL_INSTR, 0); + vm_set_exception(vm, RV_EXC_ILLEGAL_INSN, 0); return; } } @@ -202,7 +202,7 @@ void u8250_write(vm_t *vm, vm_set_exception(vm, RV_EXC_STORE_MISALIGN, vm->exc_val); return; default: - vm_set_exception(vm, RV_EXC_ILLEGAL_INSTR, 0); + vm_set_exception(vm, RV_EXC_ILLEGAL_INSN, 0); return; } } diff --git a/virtio-blk.c b/virtio-blk.c index 0b1988e..f16a531 100644 --- a/virtio-blk.c +++ b/virtio-blk.c @@ -403,7 +403,7 @@ void virtio_blk_read(vm_t *vm, vm_set_exception(vm, RV_EXC_LOAD_MISALIGN, vm->exc_val); return; default: - vm_set_exception(vm, RV_EXC_ILLEGAL_INSTR, 0); + vm_set_exception(vm, RV_EXC_ILLEGAL_INSN, 0); return; } } @@ -424,7 +424,7 @@ void virtio_blk_write(vm_t *vm, vm_set_exception(vm, RV_EXC_STORE_MISALIGN, vm->exc_val); return; default: - vm_set_exception(vm, RV_EXC_ILLEGAL_INSTR, 0); + vm_set_exception(vm, RV_EXC_ILLEGAL_INSN, 0); return; } } diff --git a/virtio-net.c b/virtio-net.c index ddb7e47..18fa4b4 100644 --- a/virtio-net.c +++ b/virtio-net.c @@ -407,7 +407,7 @@ void virtio_net_read(vm_t *vm, vm_set_exception(vm, RV_EXC_LOAD_MISALIGN, vm->exc_val); return; default: - vm_set_exception(vm, RV_EXC_ILLEGAL_INSTR, 0); + vm_set_exception(vm, RV_EXC_ILLEGAL_INSN, 0); return; } } @@ -428,7 +428,7 @@ void virtio_net_write(vm_t *vm, vm_set_exception(vm, RV_EXC_STORE_MISALIGN, vm->exc_val); return; default: - vm_set_exception(vm, RV_EXC_ILLEGAL_INSTR, 0); + vm_set_exception(vm, RV_EXC_ILLEGAL_INSN, 0); return; } } From 55bf643e9c2d9bd5541c4e1d784cdf85e40dc8f2 Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Thu, 18 Jan 2024 16:24:45 +0800 Subject: [PATCH 09/14] Facilitate PRIV macro --- main.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/main.c b/main.c index c6e2fc0..ba1b6d0 100644 --- a/main.c +++ b/main.c @@ -12,12 +12,14 @@ #include "riscv.h" #include "riscv_private.h" +#define PRIV(x) ((emu_state_t *) x->priv) + /* Define fetch separately since it is simpler (fixed width, already checked * alignment, only main RAM is executable). */ static void mem_fetch(vm_t *vm, uint32_t n_pages, uint32_t **page_addr) { - emu_state_t *data = (emu_state_t *) vm->priv; + emu_state_t *data = PRIV(vm); if (unlikely(n_pages >= RAM_SIZE / RV_PAGE_SIZE)) { /* TODO: check for other regions */ vm_set_exception(vm, RV_EXC_FETCH_FAULT, vm->exc_val); @@ -29,7 +31,7 @@ static void mem_fetch(vm_t *vm, uint32_t n_pages, uint32_t **page_addr) /* Similarly, only main memory pages can be used as page tables. */ static uint32_t *mem_page_table(const vm_t *vm, uint32_t ppn) { - emu_state_t *data = (emu_state_t *) vm->priv; + emu_state_t *data = PRIV(vm); if (ppn < (RAM_SIZE / RV_PAGE_SIZE)) return &data->ram[ppn << (RV_PAGE_SHIFT - 2)]; return NULL; @@ -37,7 +39,7 @@ static uint32_t *mem_page_table(const vm_t *vm, uint32_t ppn) static void emu_update_uart_interrupts(vm_t *vm) { - emu_state_t *data = (emu_state_t *) vm->priv; + emu_state_t *data = PRIV(vm); u8250_update_interrupts(&data->uart); if (data->uart.pending_ints) data->plic.active |= IRQ_UART_BIT; @@ -49,7 +51,7 @@ static void emu_update_uart_interrupts(vm_t *vm) #if SEMU_HAS(VIRTIONET) static void emu_update_vnet_interrupts(vm_t *vm) { - emu_state_t *data = (emu_state_t *) vm->priv; + emu_state_t *data = PRIV(vm); if (data->vnet.InterruptStatus) data->plic.active |= IRQ_VNET_BIT; else @@ -61,7 +63,7 @@ static void emu_update_vnet_interrupts(vm_t *vm) #if SEMU_HAS(VIRTIOBLK) static void emu_update_vblk_interrupts(vm_t *vm) { - emu_state_t *data = (emu_state_t *) vm->priv; + emu_state_t *data = PRIV(vm); if (data->vblk.InterruptStatus) data->plic.active |= IRQ_VBLK_BIT; else @@ -72,8 +74,7 @@ static void emu_update_vblk_interrupts(vm_t *vm) static void mem_load(vm_t *vm, uint32_t addr, uint8_t width, uint32_t *value) { - emu_state_t *data = (emu_state_t *) vm->priv; - + emu_state_t *data = PRIV(vm); /* RAM at 0x00000000 + RAM_SIZE */ if (addr < RAM_SIZE) { ram_read(vm, data->ram, addr, width, value); @@ -111,8 +112,7 @@ static void mem_load(vm_t *vm, uint32_t addr, uint8_t width, uint32_t *value) static void mem_store(vm_t *vm, uint32_t addr, uint8_t width, uint32_t value) { - emu_state_t *data = (emu_state_t *) vm->priv; - + emu_state_t *data = PRIV(vm); /* RAM at 0x00000000 + RAM_SIZE */ if (addr < RAM_SIZE) { ram_write(vm, data->ram, addr, width, value); @@ -159,7 +159,7 @@ typedef struct { static inline sbi_ret_t handle_sbi_ecall_TIMER(vm_t *vm, int32_t fid) { - emu_state_t *data = (emu_state_t *) vm->priv; + emu_state_t *data = PRIV(vm); switch (fid) { case SBI_TIMER__SET_TIMER: data->timer = (((uint64_t) vm->x_regs[RV_R_A1]) << 32) | @@ -172,7 +172,7 @@ static inline sbi_ret_t handle_sbi_ecall_TIMER(vm_t *vm, int32_t fid) static inline sbi_ret_t handle_sbi_ecall_RST(vm_t *vm, int32_t fid) { - emu_state_t *data = (emu_state_t *) vm->priv; + emu_state_t *data = PRIV(vm); switch (fid) { case SBI_RST__SYSTEM_RESET: fprintf(stderr, "system reset: type=%u, reason=%u\n", From af0b37a7b460614504e9627ff41d452772dcee62 Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Sat, 20 Jan 2024 15:16:12 +0800 Subject: [PATCH 10/14] CI: Bump dependency version --- .github/workflows/main.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 1255cda..27b9152 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -6,7 +6,7 @@ jobs: rv32emu: runs-on: ubuntu-22.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: install-dependencies run: | sudo apt-get install build-essential device-tree-compiler @@ -17,7 +17,7 @@ jobs: coding_style: runs-on: ubuntu-22.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: coding convention run: | sudo apt-get install -q -y clang-format-12 From ace0e7e8b712f28c28d75a374baa5642ddc46216 Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Sat, 20 Jan 2024 16:54:01 +0800 Subject: [PATCH 11/14] CI: Automate the runtime checks This commit aims to automate the validation process by utilizing the 'expect' tool to verify if semu can successfully boot the Linux kernel and run BusyBox as anticipated. Currently, only the "uname" command is executed and its output is checked. --- .ci/autorun.sh | 24 ++++++++++++++++++++++++ .github/workflows/main.yml | 6 +++++- 2 files changed, 29 insertions(+), 1 deletion(-) create mode 100755 .ci/autorun.sh diff --git a/.ci/autorun.sh b/.ci/autorun.sh new file mode 100755 index 0000000..3c58bb2 --- /dev/null +++ b/.ci/autorun.sh @@ -0,0 +1,24 @@ +#!/usr/bin/env bash + +pkill -9 semu + +expect < Date: Sat, 20 Jan 2024 17:55:53 +0800 Subject: [PATCH 12/14] Update checksum for prebuilt Linux image --- mk/external.mk | 4 ++-- scripts/build-image.sh | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/mk/external.mk b/mk/external.mk index 84f8e45..17667bc 100644 --- a/mk/external.mk +++ b/mk/external.mk @@ -8,12 +8,12 @@ COMMON_URL = https://github.com/sysprog21/semu/raw/blob # kernel KERNEL_DATA_URL = $(COMMON_URL)/Image.bz2 KERNEL_DATA = Image -KERNEL_DATA_SHA1 = 36d770efe97beac85204f1f50f8de81e3e529d84 +KERNEL_DATA_SHA1 = b0af9fa4c736dbde5568181c1442182700584495 # initrd INITRD_DATA_URL = $(COMMON_URL)/rootfs.cpio.bz2 INITRD_DATA = rootfs.cpio -INITRD_DATA_SHA1 = fad749d0a9eb33178525f961d6b82e7c0ce917a7 +INITRD_DATA_SHA1 = 4f0c2e646eb99af21ed2c25d48e1c44f6bd58f91 define download $($(T)_DATA): diff --git a/scripts/build-image.sh b/scripts/build-image.sh index 3d0cdd5..cb8f3c1 100755 --- a/scripts/build-image.sh +++ b/scripts/build-image.sh @@ -21,7 +21,7 @@ PARALLEL="-j$(nproc)" function do_buildroot { - ASSERT git clone https://github.com/buildroot/buildroot -b 2023.05.1 --depth=1 + ASSERT git clone https://github.com/buildroot/buildroot -b 2023.11.x --depth=1 cp -f configs/buildroot.config buildroot/.config cp -f configs/busybox.config buildroot/busybox.config # Otherwise, the error below raises: From 78ec6f77a61ebad67c45424ebed7a94b5b955bf0 Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Sat, 20 Jan 2024 20:51:45 +0800 Subject: [PATCH 13/14] CI: Improve error-prone support The timeout for 'expect' script is reduced to 90 seconds, which should be reasonable for testing purpose. --- .ci/autorun.sh | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/.ci/autorun.sh b/.ci/autorun.sh index 3c58bb2..d07494b 100755 --- a/.ci/autorun.sh +++ b/.ci/autorun.sh @@ -1,9 +1,23 @@ #!/usr/bin/env bash -pkill -9 semu +function cleanup { + sleep 1 + pkill -9 semu +} -expect < Date: Tue, 23 Jan 2024 19:48:20 +0800 Subject: [PATCH 14/14] Declare read-only pointers as const --- virtio-blk.c | 9 +++++---- virtio-net.c | 18 +++++++++--------- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/virtio-blk.c b/virtio-blk.c index f16a531..4fb6715 100644 --- a/virtio-blk.c +++ b/virtio-blk.c @@ -106,7 +106,7 @@ static void virtio_blk_write_handler(virtio_blk_state_t *vblk, uint32_t len) { void *dest = (void *) ((uintptr_t) vblk->disk + sector * DISK_BLK_SIZE); - void *src = (void *) ((uintptr_t) vblk->ram + desc_addr); + const void *src = (void *) ((uintptr_t) vblk->ram + desc_addr); memcpy(dest, src, len); } @@ -116,7 +116,8 @@ static void virtio_blk_read_handler(virtio_blk_state_t *vblk, uint32_t len) { void *dest = (void *) ((uintptr_t) vblk->ram + desc_addr); - void *src = (void *) ((uintptr_t) vblk->disk + sector * DISK_BLK_SIZE); + const void *src = + (void *) ((uintptr_t) vblk->disk + sector * DISK_BLK_SIZE); memcpy(dest, src, len); } @@ -140,7 +141,7 @@ static int virtio_blk_desc_handler(virtio_blk_state_t *vblk, /* Collect the descriptors */ for (int i = 0; i < 3; i++) { /* The size of the `struct virtq_desc` is 4 words */ - uint32_t *desc = &vblk->ram[queue->QueueDesc + desc_idx * 4]; + const uint32_t *desc = &vblk->ram[queue->QueueDesc + desc_idx * 4]; /* Retrieve the fields of current descriptor */ vq_desc[i].addr = desc[0]; @@ -162,7 +163,7 @@ static int virtio_blk_desc_handler(virtio_blk_state_t *vblk, } /* Process the header */ - struct vblk_req_header *header = + const struct vblk_req_header *header = (struct vblk_req_header *) ((uintptr_t) vblk->ram + vq_desc[0].addr); uint32_t type = header->type; uint64_t sector = header->sector; diff --git a/virtio-net.c b/virtio-net.c index 18fa4b4..da1d7df 100644 --- a/virtio-net.c +++ b/virtio-net.c @@ -116,15 +116,15 @@ static bool vnet_iovec_read(struct iovec **vecs, /* Require existing 'desc_idx' to use as iteration variable, and input * 'buffer_idx'. */ -#define VNET_ITERATE_BUFFER(checked, body) \ - desc_idx = buffer_idx; \ - while (1) { \ - if (checked && desc_idx >= queue->QueueNum) \ - return virtio_net_set_fail(vnet); \ - uint32_t *desc = &ram[queue->QueueDesc + desc_idx * 4]; \ - uint16_t desc_flags = desc[3]; \ - body if (!(desc_flags & VIRTIO_DESC_F_NEXT)) break; \ - desc_idx = desc[3] >> 16; \ +#define VNET_ITERATE_BUFFER(checked, body) \ + desc_idx = buffer_idx; \ + while (1) { \ + if (checked && desc_idx >= queue->QueueNum) \ + return virtio_net_set_fail(vnet); \ + const uint32_t *desc = &ram[queue->QueueDesc + desc_idx * 4]; \ + uint16_t desc_flags = desc[3]; \ + body if (!(desc_flags & VIRTIO_DESC_F_NEXT)) break; \ + desc_idx = desc[3] >> 16; \ } /* Input: 'buffer_idx'.