[PATCH v3 0/2] target/riscv: Allow software access to MIP SEIP

qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed

* [PATCH v3 0/2] target/riscv: Allow software access to MIP SEIP
@ 2022-03-17  6:18 Alistair Francis
  2022-03-17  6:18 ` [PATCH v3 1/2] target/riscv: cpu: Fixup indentation Alistair Francis
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Alistair Francis @ 2022-03-17  6:18 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv
  Cc: bmeng.cn, Alistair Francis, Bin Meng, palmer, alistair23

From: Alistair Francis <alistair.francis@wdc.com>

The RISC-V specification states that:
  "Supervisor-level external interrupts are made pending based on the
  logical-OR of the software-writable SEIP bit and the signal from the
  external interrupt controller."

We currently only allow either the interrupt controller or software to
set the bit, which is incorrect.

This patch removes the miclaim mask when writing MIP to allow M-mode
software to inject interrupts, even with an interrupt controller.

We then also need to keep track of which source is setting MIP_SEIP. The
final value is a OR of both, so we add two bools and use that to keep
track of the current state. This way either source can change without
losing the correct value.

This fixes: https://gitlab.com/qemu-project/qemu/-/issues/904

Alistair Francis (2):
  target/riscv: cpu: Fixup indentation
  target/riscv: Allow software access to MIP SEIP

 target/riscv/cpu.h |  8 ++++++++
 target/riscv/cpu.c | 30 +++++++++++++++++++-----------
 target/riscv/csr.c |  8 ++++++--
 3 files changed, 33 insertions(+), 13 deletions(-)

-- 
2.35.1



^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v3 1/2] target/riscv: cpu: Fixup indentation
  2022-03-17  6:18 [PATCH v3 0/2] target/riscv: Allow software access to MIP SEIP Alistair Francis
@ 2022-03-17  6:18 ` Alistair Francis
  2022-03-17  6:18 ` [PATCH v3 2/2] target/riscv: Allow software access to MIP SEIP Alistair Francis
  2022-03-17 23:13 ` [PATCH v3 0/2] " Alistair Francis
  2 siblings, 0 replies; 4+ messages in thread
From: Alistair Francis @ 2022-03-17  6:18 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv
  Cc: bmeng.cn, Alistair Francis, Bin Meng, palmer, alistair23,
	Richard Henderson

From: Alistair Francis <alistair.francis@wdc.com>

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/riscv/cpu.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index ddda4906ff..41b757995d 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -567,18 +567,18 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
         if (cpu->cfg.ext_i && cpu->cfg.ext_e) {
             error_setg(errp,
                        "I and E extensions are incompatible");
-                       return;
-       }
+            return;
+        }
 
         if (!cpu->cfg.ext_i && !cpu->cfg.ext_e) {
             error_setg(errp,
                        "Either I or E extension must be set");
-                       return;
-       }
+            return;
+        }
 
-       if (cpu->cfg.ext_g && !(cpu->cfg.ext_i & cpu->cfg.ext_m &
-                               cpu->cfg.ext_a & cpu->cfg.ext_f &
-                               cpu->cfg.ext_d)) {
+        if (cpu->cfg.ext_g && !(cpu->cfg.ext_i & cpu->cfg.ext_m &
+                                cpu->cfg.ext_a & cpu->cfg.ext_f &
+                                cpu->cfg.ext_d)) {
             warn_report("Setting G will also set IMAFD");
             cpu->cfg.ext_i = true;
             cpu->cfg.ext_m = true;
@@ -709,11 +709,11 @@ static void riscv_cpu_set_irq(void *opaque, int irq, int level)
         case IRQ_S_EXT:
         case IRQ_VS_EXT:
         case IRQ_M_EXT:
-             if (kvm_enabled()) {
+            if (kvm_enabled()) {
                 kvm_riscv_set_irq(cpu, irq, level);
-             } else {
+            } else {
                 riscv_cpu_update_mip(cpu, 1 << irq, BOOL_TO_MASK(level));
-             }
+            }
              break;
         default:
             g_assert_not_reached();
-- 
2.35.1



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v3 2/2] target/riscv: Allow software access to MIP SEIP
  2022-03-17  6:18 [PATCH v3 0/2] target/riscv: Allow software access to MIP SEIP Alistair Francis
  2022-03-17  6:18 ` [PATCH v3 1/2] target/riscv: cpu: Fixup indentation Alistair Francis
@ 2022-03-17  6:18 ` Alistair Francis
  2022-03-17 23:13 ` [PATCH v3 0/2] " Alistair Francis
  2 siblings, 0 replies; 4+ messages in thread
From: Alistair Francis @ 2022-03-17  6:18 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv
  Cc: bmeng.cn, Alistair Francis, Bin Meng, palmer, alistair23,
	Richard Henderson

From: Alistair Francis <alistair.francis@wdc.com>

The RISC-V specification states that:
  "Supervisor-level external interrupts are made pending based on the
  logical-OR of the software-writable SEIP bit and the signal from the
  external interrupt controller."

We currently only allow either the interrupt controller or software to
set the bit, which is incorrect.

This patch removes the miclaim mask when writing MIP to allow M-mode
software to inject interrupts, even with an interrupt controller.

We then also need to keep track of which source is setting MIP_SEIP. The
final value is a OR of both, so we add two bools and use that to keep
track of the current state. This way either source can change without
losing the correct value.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/904
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/riscv/cpu.h |  8 ++++++++
 target/riscv/cpu.c | 10 +++++++++-
 target/riscv/csr.c |  8 ++++++--
 3 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index c069fe85fa..05d40f8dbd 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -173,6 +173,14 @@ struct CPUArchState {
     uint64_t mstatus;
 
     uint64_t mip;
+    /*
+     * MIP contains the software writable version of SEIP ORed with the
+     * external interrupt value. The MIP register is always up-to-date.
+     * To keep track of the current source, we also save booleans of the values
+     * here.
+     */
+    bool external_seip;
+    bool software_seip;
 
     uint64_t miclaim;
 
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 41b757995d..68373b769c 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -706,7 +706,6 @@ static void riscv_cpu_set_irq(void *opaque, int irq, int level)
         case IRQ_VS_TIMER:
         case IRQ_M_TIMER:
         case IRQ_U_EXT:
-        case IRQ_S_EXT:
         case IRQ_VS_EXT:
         case IRQ_M_EXT:
             if (kvm_enabled()) {
@@ -715,6 +714,15 @@ static void riscv_cpu_set_irq(void *opaque, int irq, int level)
                 riscv_cpu_update_mip(cpu, 1 << irq, BOOL_TO_MASK(level));
             }
              break;
+        case IRQ_S_EXT:
+            if (kvm_enabled()) {
+                kvm_riscv_set_irq(cpu, irq, level);
+            } else {
+                env->external_seip = level;
+                riscv_cpu_update_mip(cpu, 1 << irq,
+                                     BOOL_TO_MASK(level | env->software_seip));
+            }
+            break;
         default:
             g_assert_not_reached();
         }
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 0606cd0ea8..77726ccefb 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1403,10 +1403,14 @@ static RISCVException rmw_mip64(CPURISCVState *env, int csrno,
                                 uint64_t new_val, uint64_t wr_mask)
 {
     RISCVCPU *cpu = env_archcpu(env);
-    /* Allow software control of delegable interrupts not claimed by hardware */
-    uint64_t old_mip, mask = wr_mask & delegable_ints & ~env->miclaim;
+    uint64_t old_mip, mask = wr_mask & delegable_ints;
     uint32_t gin;
 
+    if (mask & MIP_SEIP) {
+        env->software_seip = new_val & MIP_SEIP;
+        new_val |= env->external_seip * MIP_SEIP;
+    }
+
     if (mask) {
         old_mip = riscv_cpu_update_mip(cpu, mask, (new_val & mask));
     } else {
-- 
2.35.1



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v3 0/2] target/riscv: Allow software access to MIP SEIP
  2022-03-17  6:18 [PATCH v3 0/2] target/riscv: Allow software access to MIP SEIP Alistair Francis
  2022-03-17  6:18 ` [PATCH v3 1/2] target/riscv: cpu: Fixup indentation Alistair Francis
  2022-03-17  6:18 ` [PATCH v3 2/2] target/riscv: Allow software access to MIP SEIP Alistair Francis
@ 2022-03-17 23:13 ` Alistair Francis
  2 siblings, 0 replies; 4+ messages in thread
From: Alistair Francis @ 2022-03-17 23:13 UTC (permalink / raw)
  To: Alistair Francis
  Cc: open list:RISC-V, Bin Meng, qemu-devel@nongnu.org Developers,
	Alistair Francis, Palmer Dabbelt, Bin Meng

On Thu, Mar 17, 2022 at 4:18 PM Alistair Francis
<alistair.francis@opensource.wdc.com> wrote:
>
> From: Alistair Francis <alistair.francis@wdc.com>
>
> The RISC-V specification states that:
>   "Supervisor-level external interrupts are made pending based on the
>   logical-OR of the software-writable SEIP bit and the signal from the
>   external interrupt controller."
>
> We currently only allow either the interrupt controller or software to
> set the bit, which is incorrect.
>
> This patch removes the miclaim mask when writing MIP to allow M-mode
> software to inject interrupts, even with an interrupt controller.
>
> We then also need to keep track of which source is setting MIP_SEIP. The
> final value is a OR of both, so we add two bools and use that to keep
> track of the current state. This way either source can change without
> losing the correct value.
>
> This fixes: https://gitlab.com/qemu-project/qemu/-/issues/904
>
> Alistair Francis (2):
>   target/riscv: cpu: Fixup indentation
>   target/riscv: Allow software access to MIP SEIP

Thanks!

Applied to riscv-to-apply.next

Alistair

>
>  target/riscv/cpu.h |  8 ++++++++
>  target/riscv/cpu.c | 30 +++++++++++++++++++-----------
>  target/riscv/csr.c |  8 ++++++--
>  3 files changed, 33 insertions(+), 13 deletions(-)
>
> --
> 2.35.1
>


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-03-17 23:15 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-03-17  6:18 [PATCH v3 0/2] target/riscv: Allow software access to MIP SEIP Alistair Francis
2022-03-17  6:18 ` [PATCH v3 1/2] target/riscv: cpu: Fixup indentation Alistair Francis
2022-03-17  6:18 ` [PATCH v3 2/2] target/riscv: Allow software access to MIP SEIP Alistair Francis
2022-03-17 23:13 ` [PATCH v3 0/2] " Alistair Francis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).