Some fixes to support operating systems running in machine mode

[sebhub]

Which branch should I use for pull requests?

[michaeljclark]

We can't allow setting of any of the M-mode interrupt pending bits as this is illegal in the Privileged ISA. See extracts from the RISC-V ISA Specification, for your convenience, below.

I would be amenable to allowing emulation of a hardware implementation of RDTIME however we would need to make this a processor specific feature (add a RISCV_FEATURE_RDTIME enum value in cpu.h) not applicable to the current processor implementation, and I would like for this new processor model to be based on some actual processor implementation (as per the current cpu implementations).

--

Regarding your first patch, while it might be legal to implement RDTIME in hardware, however, we have followed the behaviour of spike (riscv-isa-sim), the RISC-V ISA Golden reference simulator. There were changes in the Privileged ISA to move the time register from a CSR into a memory mapped register mtime, and the RDTIME instruction in the Base ISA is implemented using trap-and-emulate. Currently spike, QEMU and SiFive's silicon (E20,E21,E31,E51,U54) are all consistent in this respect.

Refer to the RISC-V Privileged ISA Specification: 3.1.15 Machine Timer Registers (mtime and mtimecmp)

Platforms provide a real-time counter, exposed as a memory-mapped
machine-mode register, mtime. mtime must run at constant frequency,
and the platform must provide a mechanism for determining the
timebase of mtime.

---

Regarding your second patch to make MTIP and MSIP writable from M-mode

Refer to the RISC-V Privileged ISA Specification: 3.1.14 Machine Interrupt Registers (mip and mie)

- The MTIP bit is read-only and is cleared by writing to the
memory-mapped machine-mode timer compare register

and

The machine-level MSIP bits are written by accesses to memory-
mapped control registers, which are used by remote harts to provide
machine-mode interprocessor interrupts.

This is what QEMU currently implements. The MTIP and MSIP bits are set and cleared by the CLINT. While the naming of the CLINT is not in the specification, that is the name of logical block implemented in spike and QEMU, responsible for the memory mapped time register mtime, the memory mapped timer comparator registers mtimecmpn for per hart timer interrupts, and the memory mapped msipn registers for software interrupts,

---

Regarding your third patch to make MEIP writable. First, your commit comment is erroneous as it mentions MEIE in the commit message however the patch makes MEIP writable.

Refer to the RISC-V Privileged ISA Specification: 3.1.14 Machine Interrupt Registers (mip and mie)

The MEIP field in mip is a read-only bit that indicates a
machine-mode external interrupt is pending. MEIP is set
and cleared by a platform-specific interrupt controller, such
as the standard platform-level interrupt controller specified
in Chapter 7

---

In summary, if you can point us to a processor that implements RDTIME in hardware, then we would accept a patch that adds a new CPU model with hardware RDTIME conditional on a RISCV_FEATURE_RDTIME flag. You would need to add a new CPU model.

We can't allow setting of MTIP, MEIP or MSIP unless it was deemed legal by the specification, for hardware compatibility reasons.

The branch to make pull requests against is qemu-for-upstream.

Thanks for the patches anyway.

[sebhub]

Thanks for your detailed review. It is very helpful for me. I do currently an operating system port of the real-time operating system RTEMS to RISC-V and Qemu is my first target. I didn't work with a real processor so far.

For the first two patches I opened topics on the RISC-V SW Dev group:

https://groups.google.com/a/groups.riscv.org/forum/#!topic/sw-dev/fpVg4Aopu-U https://groups.google.com/a/groups.riscv.org/forum/#!topic/sw-dev/BDnxG8643Dc

The third patch is valid from my point of view. The all_ints variable is only used here in csr_write_helper():

case CSR_MIE: {
    env->mie = (env->mie & ~all_ints) |
        (val_to_write & all_ints);
    break;
}

[sebhub]

I tried to rebase the third patch to the current qemu-for-upstream branch. It seems a lot of work was done on that branch. Here we have in target/riscv/csr.c:

#define M_MODE_INTERRUPTS (MIP_MSIP | MIP_MTIP | MIP_MEIP)
#define S_MODE_INTERRUPTS (MIP_SSIP | MIP_STIP | MIP_SEIP)

static const target_ulong delegable_ints = S_MODE_INTERRUPTS;
static const target_ulong all_ints = M_MODE_INTERRUPTS | S_MODE_INTERRUPTS;
    ....
static int write_mie(CPURISCVState *env, int csrno, target_ulong val)
{
    env->mie = (env->mie & ~all_ints) | (val & all_ints);
    return 0;
}

So, the patch is no longer necessary. The operating system port of RTEMS works fine on the qemu-for-upstream branch as is.