riscv-isa-sim icon indicating copy to clipboard operation
riscv-isa-sim copied to clipboard

UART peripheral (NS16550) broke SYS_READ syscall

Open PhilippvK opened this issue 11 months ago • 1 comments

I regularly use spike + riscv-pk to run baremetal programs communicating with a host application via STDIN/STDOUT. The code looks roughly as follows and worked fine until October 2022 when #889 got merged.

#include <stdio.h>
#include <unistd.h>

void process(char c) {
    …
}

int main() {
    char c;

    while (1) {
        int ret = read(STDIN_FILENO, &c, 1);
        process(c);
    }
    return 0;
}

To demonstrate the issue I made a minimal test program which should echo the entered characters back to the terminal processing one byte at a time:

#include <stdio.h>
#include <unistd.h>

int main() {
    char c;

    while (1) {
        int ret = read(STDIN_FILENO, &c, 1);
        printf("-> %c\n", c);
    }
    return 0;
}

The expected output for some inputs looks as follows:

$ spike --isa=rv32imc pk program.elf
bbl loader
foo
-> f
-> o
-> o
-> 

foobar
-> f
-> o
-> o
-> b
-> a
-> r
-> 

f
-> f
-> 

o
-> o
-> 

o
-> o
->

However Spike now produces the following:

$ spike --isa=rv32imc pk program.elf
bbl loader
foo
-> f
foobar
-> f
-> r
f
-> f
o
-> o
o
-> o

My code is relying on the syscall functionality implemented in fesvr/syscall.cc. The first entered character is correctly captured by the syscall_t::sys_read function while the remaining characters stay in the STDIN buffer. However until the next invocation of read(…), the tick() method of UART peripheral is being called several times, each time reading a single character from the terminal and sending it to the rx_fifo which is never used since my program is not interfacing with the UART. Therefore depending of the number of instructions in the loop body, 4 or more characters are being dropped and never captured by the target sw.

My workaround for the problem is either reverting commit 191634d2854dfed448fc323195f9b65c305e2d77 or commenting out the call to ns16550->tick(); in riscv/sim.cc

How could this be solved on the long term? I had some ideas but would be happy about any other recommendations.

  • Allow disabling the UART completely via the cmdline either at runtime or during compilation
  • Expose the UART fifo size to the cmdline (size=0 also fixes the problem)

PhilippvK avatar Jul 05 '23 22:07 PhilippvK

I think disabling UART via a custom DTB is the easiest way right now, with the following approach:

  1. Generate spike's default dts with --dump-dts
  2. Remove the node corresponding to the ns16550
  3. Compile a dtb from the dts with dtc
  4. Pass in the dtb to spike with --dtb=

Spike should configure itself based on the contents of the dtb, which should omit the UART if not present.

Allow disabling the UART completely via the cmdline either at runtime or during compilation

We could add a flag to disable UART completely, but moving towards DTS/DTB-based configuration seems to be the more sustainable way forwards. To that end, perhaps we should add a --dts= flag to simplify DTS-driven configuration.

jerryz123 avatar Jul 05 '23 22:07 jerryz123