Some changes

- remove comment that related to MMU test suite in syscall_handler and mention them in Makefile instead - rename int and ints to intr and intrs respectively - use enum for PLIC's regs - Disable termios keyboard echoing and capture CTRL+a+x to exit VM
sysprog21 · Nov 4, 2024 · 5b64af8 · 5b64af8
1 parent 5ed8482
commit 5b64af8
Show file tree

Hide file tree

Showing 9 changed files with 49 additions and 49 deletions.
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -67,11 +67,11 @@ jobs:
     - name: misalignment test in block emulation
       run: |
             make -C tests/system/alignment/
-            make distclean && make ENABLE_USE_ELF=1 ENABLE_EXT_C=0 ENABLE_SYSTEM=1 misalign-in-blk-emu -j$(nproc)
+            make distclean && make ENABLE_ELF_LOADER=1 ENABLE_EXT_C=0 ENABLE_SYSTEM=1 misalign-in-blk-emu -j$(nproc)
     - name: MMU test
       run: |
             make -C tests/system/mmu/
-            make distclean && make ENABLE_ON_TEST=1 ENABLE_USE_ELF=1 ENABLE_SYSTEM=1 mmu-test -j$(nproc)
+            make distclean && make ENABLE_ON_TEST=1 ENABLE_ELF_LOADER=1 ENABLE_SYSTEM=1 mmu-test -j$(nproc)
     - name: gdbstub test
       run: |
             make distclean && make ENABLE_GDBSTUB=1 gdbstub-test -j$(nproc)

diff --git a/Makefile b/Makefile
@@ -20,8 +20,8 @@ $(call set-feature, ELF_LOADER)
 
 # The MMU test suite relies on rv32emu's syscall function instead of
 # its syscall table, unlike the Linux kernel. Therefore, use the
-# ON_TEST macro to distinguish between the Linux kernel and MMU test
-# suite as the emulation target.
+# ON_TEST feature macro to distinguish between the Linux kernel and MMU
+# test suite as the emulation target.
 ENABLE_ON_TEST ?= 0
 $(call set-feature, ON_TEST)
 

diff --git a/src/devices/plic.c b/src/devices/plic.c
@@ -33,17 +33,17 @@ uint32_t plic_read(plic_t *plic, const uint32_t addr)
     uint32_t plic_read_val = 0;
 
     switch (addr) {
-    case 0x400:
+    case INTR_PENDING:
         plic_read_val = plic->ip;
         break;
-    case 0x800:
+    case INTR_ENABLE:
         plic_read_val = plic->ie;
         break;
-    case 0x80000:
+    case INTR_PRIORITY:
         /* no priority support: target priority threshold hardwired to 0 */
         plic_read_val = 0;
         break;
-    case 0x80001:
+    case INTR_CLAIM_OR_COMPLETE:
         /* claim */
         {
             uint32_t intr_candidate = plic->ip & plic->ie;
@@ -67,13 +67,13 @@ void plic_write(plic_t *plic, const uint32_t addr, uint32_t value)
         return;
 
     switch (addr) {
-    case 0x800:
+    case INTR_ENABLE:
         plic->ie = (value & ~1);
         break;
-    case 0x80000:
+    case INTR_PRIORITY:
         /* no priority support: target priority threshold hardwired to 0 */
         break;
-    case 0x80001:
+    case INTR_CLAIM_OR_COMPLETE:
         /* completion */
         if (plic->ie & (1U << value))
             plic->masked &= ~(1U << value);

diff --git a/src/devices/plic.h b/src/devices/plic.h
@@ -7,6 +7,13 @@
 
 #include <stdint.h>
 
+enum PLIC_REG {
+    INTR_PENDING = 0x400,
+    INTR_ENABLE = 0x800,
+    INTR_PRIORITY = 0x80000,
+    INTR_CLAIM_OR_COMPLETE = 0x80001,
+};
+
 /* PLIC */
 typedef struct {
     uint32_t masked;

diff --git a/src/devices/uart.c b/src/devices/uart.c
@@ -22,16 +22,16 @@ void u8250_update_interrupts(u8250_state_t *uart)
     /* Some interrupts are level-generated. */
     /* TODO: does it also generate an LSR change interrupt? */
     if (uart->in_ready)
-        uart->pending_ints |= 1;
+        uart->pending_intrs |= 1;
     else
-        uart->pending_ints &= ~1;
+        uart->pending_intrs &= ~1;
 
     /* Prevent generating any disabled interrupts in the first place */
-    uart->pending_ints &= uart->ier;
+    uart->pending_intrs &= uart->ier;
 
     /* Update current interrupt (higher bits -> more priority) */
-    if (uart->pending_ints)
-        uart->current_int = ilog2(uart->pending_ints);
+    if (uart->pending_intrs)
+        uart->current_intr = ilog2(uart->pending_intrs);
 }
 
 void u8250_check_ready(u8250_state_t *uart)
@@ -60,16 +60,15 @@ static uint8_t u8250_handle_in(u8250_state_t *uart)
 
     if (read(uart->in_fd, &value, 1) < 0)
         fprintf(stderr, "failed to read UART input: %s\n", strerror(errno));
-    // printf("success\n");
     uart->in_ready = false;
     u8250_check_ready(uart);
 
-    // if (value == 1) {           /* start of heading (Ctrl-a) */
-    //     if (getchar() == 120) { /* keyboard x */
-    //         printf("\n");       /* end emulator with newline */
-    //         exit(0);
-    //     }
-    // }
+    if (value == 1) {           /* start of heading (Ctrl-a) */
+        if (getchar() == 120) { /* keyboard x */
+            printf("\n");       /* end emulator with newline */
+            exit(0);
+        }
+    }
 
     return value;
 }
@@ -79,19 +78,17 @@ static uint8_t u8250_reg_read(u8250_state_t *uart, uint32_t addr)
     uint8_t ret;
     switch (addr) {
     case 0:
-        if (uart->lcr & (1 << 7)) { /* DLAB */
+        if (uart->lcr & (1 << 7)) /* DLAB */
             return uart->dll;
-        }
         return u8250_handle_in(uart);
     case 1:
-        if (uart->lcr & (1 << 7)) { /* DLAB */
+        if (uart->lcr & (1 << 7)) /* DLAB */
             return uart->dlh;
-        }
         return uart->ier;
     case 2:
-        ret = (uart->current_int << 1) | (uart->pending_ints ? 0 : 1);
-        if (uart->current_int == U8250_INT_THRE)
-            uart->pending_ints &= ~(1 << uart->current_int);
+        ret = (uart->current_intr << 1) | (uart->pending_intrs ? 0 : 1);
+        if (uart->current_intr == U8250_INT_THRE)
+            uart->pending_intrs &= ~(1 << uart->current_intr);
         return ret;
     case 3:
         return uart->lcr;
@@ -121,7 +118,7 @@ static void u8250_reg_write(u8250_state_t *uart, uint32_t addr, uint8_t value)
             break;
         }
         u8250_handle_out(uart, value);
-        uart->pending_ints |= 1 << U8250_INT_THRE;
+        uart->pending_intrs |= 1 << U8250_INT_THRE;
         break;
     case 1:
         if (uart->lcr & (1 << 7)) { /* DLAB */

diff --git a/src/devices/uart.h b/src/devices/uart.h
@@ -8,14 +8,14 @@
 #include <stdbool.h>
 #include <stdint.h>
 
-#define IRQ_UART 1
-#define IRQ_UART_BIT (1 << IRQ_UART)
+#define IRQ_UART_SHIFT 1
+#define IRQ_UART_BIT (1 << IRQ_UART_SHIFT)
 
 typedef struct {
-    uint8_t dll, dlh;                  /* divisor (ignored) */
-    uint8_t lcr;                       /* UART config */
-    uint8_t ier;                       /* interrupt config */
-    uint8_t current_int, pending_ints; /* interrupt status */
+    uint8_t dll, dlh;                    /* divisor (ignored) */
+    uint8_t lcr;                         /* UART config */
+    uint8_t ier;                         /* interrupt config */
+    uint8_t current_intr, pending_intrs; /* interrupt status */
     uint8_t mcr;       /* other output signals, loopback mode (ignored) */
     int in_fd, out_fd; /* I/O handling */
     bool in_ready;

diff --git a/src/emulate.c b/src/emulate.c
@@ -88,7 +88,7 @@ static inline void update_time(riscv_t *rv)
 }
 
 #if RV32_HAS(SYSTEM)
-static inline void get_time_now(struct timeval *tv)
+static void get_time_now(struct timeval *tv)
 {
     rv_gettimeofday(tv);
 }
@@ -936,7 +936,7 @@ void rv_step(void *arg)
 
     /* loop until hitting the cycle target */
     while (rv->csr_cycle < cycles_target && !rv->halt) {
-#if RV32_HAS(SYSTEM) && !defined(USE_ELF)
+#if RV32_HAS(SYSTEM) && !RV32_HAS(ELF_LOADER)
         /* check for any interrupt after every block emulation */
 
         /* now time */
@@ -976,7 +976,7 @@ void rv_step(void *arg)
                 break;
             }
         }
-#endif /* RV32_HAS(SYSTEM) && !defined(USE_ELF) */
+#endif /* RV32_HAS(SYSTEM) && !RV32_HAS(ELF_LOADER) */
 
         if (prev && prev->pc_start != last_pc) {
             /* update previous block */
@@ -1206,11 +1206,7 @@ void ecall_handler(riscv_t *rv)
 {
     assert(rv);
 
-/*
- * FIXME: MMU test suite rely on rv32emu syscall instead of its own syscall
- * table
- */
-#if defined(ON_TEST)
+#if RV32_HAS(ON_TEST)
     rv->PC += 4;
     syscall_handler(rv);
 #elif RV32_HAS(SYSTEM)

diff --git a/src/riscv.c b/src/riscv.c
@@ -259,6 +259,7 @@ static void reset_keyboard_input()
     term.c_lflag |= ICANON | ECHO;
     tcsetattr(0, TCSANOW, &term);
 }
+
 /* Asynchronous communication to capture all keyboard input for the VM. */
 static void capture_keyboard_input()
 {
@@ -289,9 +290,6 @@ riscv_t *rv_create(riscv_user_t rv_attr)
     /* reset */
     rv_reset(rv, 0U);
 
-    /* FIXME */
-    // capture_keyboard_input();
-
 #if !RV32_HAS(SYSTEM) || (RV32_HAS(SYSTEM) && RV32_HAS(ELF_LOADER))
     elf_t *elf = elf_new();
     assert(elf && elf_open(elf, attr->data.user.elf_program));
@@ -383,6 +381,8 @@ riscv_t *rv_create(riscv_user_t rv_attr)
     assert(attr->uart);
     attr->uart->in_fd = 0;
     attr->uart->out_fd = 1;
+
+    capture_keyboard_input();
 #endif /* !RV32_HAS(SYSTEM) || (RV32_HAS(SYSTEM) && RV32_HAS(ELF_LOADER)) */
 
     /* default standard stream.

diff --git a/src/system.c b/src/system.c
@@ -15,7 +15,7 @@ void emu_update_uart_interrupts(riscv_t *rv)
 {
     vm_attr_t *attr = PRIV(rv);
     u8250_update_interrupts(attr->uart);
-    if (attr->uart->pending_ints) {
+    if (attr->uart->pending_intrs) {
         attr->plic->active |= IRQ_UART_BIT;
     } else
         attr->plic->active &= ~IRQ_UART_BIT;