sifive_plic mismatch between pending and priority base address

[vsiles]

While trying to toy with the uart RX interrupt on the sifive_u machine, I spotted an issue where I was correctly notified of interrupts, but the "pending" part of the PLIC was always set to 0. I found this bug:

diff --git a/hw/riscv/sifive_plic.c b/hw/riscv/sifive_plic.c
index a4ac910..f2a8b0b 100644
--- a/hw/riscv/sifive_plic.c
+++ b/hw/riscv/sifive_plic.c
@@ -214,7 +214,7 @@ static uint64_t sifive_plic_read(void *opaque, hwaddr addr, unsigned size)
     } else if (addr >= plic->pending_base && /* 1 bit per source */
                addr < plic->pending_base + (plic->num_sources >> 3))
     {
-        uint32_t word = (addr - plic->priority_base) >> 2;
+        uint32_t word = (addr - plic->pending_base) >> 2;
         if (RISCV_DEBUG_PLIC) {
             qemu_log("plic: read pending: word=%d value=%d\n",
                 word, plic->pending[word]);

I didn't check yet for other occurrences of this mistake, it might be duplicated at other locations.

I'm currently using:

 git log -1
commit 4b9099b943ab274b1dca1cb5202f77e9a1a651f6
Author: Michael Clark <mjc@sifive.com>
Date:   Mon Apr 30 12:29:34 2018 +1200

    RISC-V: Add missing free for plic_hart_config

    Cc: Palmer Dabbelt <palmer@sifive.com>
    Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
    Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
    Cc: Alistair Francis <Alistair.Francis@wdc.com>
    Signed-off-by: Michael Clark <mjc@sifive.com>

but the issue seems present in all branches.

[michaeljclark]

On 11/05/2018, at 9:02 PM, Vincent notifications@github.com wrote:

While trying to toy with the uart RX interrupt on the sifive_u machine, I spotted an issue where I was correctly notified of interrupts, but the "pending" part of the PLIC was always set to 0. I found this bug:

Thanks a lot for looking into this.

It probably went unnoticed because I think the Linux PLIC driver just uses the interrupt number from the claim reg and then writes the same interrupt number back to signal acknowledgement. I don’t think the Linux PLIC driver digs into the pending bitfield.

We also have more work to do on the SiFive UART as we don’t yet support the FIFOs, high watermarks and TX interrupts. However it currently supports enough for bbl/Linux as we get Linux boot logs over the SiFive UART on the ‘sifive_u’ machine.

diff --git a/hw/riscv/sifive_plic.c b/hw/riscv/sifive_plic.c index a4ac910..f2a8b0b 100644 --- a/hw/riscv/sifive_plic.c +++ b/hw/riscv/sifive_plic.c @@ -214,7 +214,7 @@ static uint64_t sifive_plic_read(void opaque, hwaddr addr, unsigned size) } else if (addr >= plic->pending_base && / 1 bit per source */ addr < plic->pending_base + (plic->num_sources >> 3)) {

• uint32_t word = (addr - plic->priority_base) >> 2;
• uint32_t word = (addr - plic->pending_base) >> 2; if (RISCV_DEBUG_PLIC) { qemu_log("plic: read pending: word=%d value=%d\n", word, plic->pending[word]); I didn't check yet for other occurrences of this mistake, it might be duplicated at other locations.

I'm currently using:

git log -1 commit 4b9099b943ab274b1dca1cb5202f77e9a1a651f6 Author: Michael Clark mjc@sifive.com Date: Mon Apr 30 12:29:34 2018 +1200

RISC-V: Add missing free for plic_hart_config

Cc: Palmer Dabbelt <palmer@sifive.com>
Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Cc: Alistair Francis <Alistair.Francis@wdc.com>
Signed-off-by: Michael Clark <mjc@sifive.com>

but the issue seems present in all branches.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[michaeljclark]

Here is the fix: https://github.com/riscv/riscv-qemu/commits/qemu-2.13-for-upstream (https://github.com/riscv/riscv-qemu/commit/0cfdb117e5e4d63f0e9be958198c2b70f2c5ff96)

[vsiles]

That was quick, thanks a lot !