[v1,20/36] target/riscv: Add support for virtual interrupt setting
diff mbox series

Message ID 2389483485d4642a6e5670e2546c62e493e91fd6.1575914822.git.alistair.francis@wdc.com
State New
Headers show
Series
  • Add RISC-V Hypervisor Extension v0.5
Related show

Commit Message

Alistair Francis Dec. 9, 2019, 6:11 p.m. UTC
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/cpu_helper.c | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

Comments

Palmer Dabbelt Jan. 9, 2020, 12:49 a.m. UTC | #1
On Mon, 09 Dec 2019 10:11:32 PST (-0800), Alistair Francis wrote:
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/cpu_helper.c | 33 ++++++++++++++++++++++++++++-----
>  1 file changed, 28 insertions(+), 5 deletions(-)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 63439c9370..85eed5d885 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -37,13 +37,36 @@ int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch)
>  #ifndef CONFIG_USER_ONLY
>  static int riscv_cpu_local_irq_pending(CPURISCVState *env)
>  {
> +    target_ulong irqs;
> +
>      target_ulong mstatus_mie = get_field(*env->mstatus, MSTATUS_MIE);
>      target_ulong mstatus_sie = get_field(*env->mstatus, MSTATUS_SIE);
> -    target_ulong pending = env->mip & env->mie;
> -    target_ulong mie = env->priv < PRV_M || (env->priv == PRV_M && mstatus_mie);
> -    target_ulong sie = env->priv < PRV_S || (env->priv == PRV_S && mstatus_sie);
> -    target_ulong irqs = (pending & ~env->mideleg & -mie) |
> -                        (pending &  env->mideleg & -sie);
> +    target_ulong hs_mstatus_sie = get_field(env->mstatus_novirt, MSTATUS_SIE);
> +
> +    target_ulong pending = env->mip & env->mie &
> +                               ~(MIP_VSSIP | MIP_VSTIP | MIP_VSEIP);
> +    target_ulong vspending = (env->mip & env->mie &
> +                              (MIP_VSSIP | MIP_VSTIP | MIP_VSEIP)) >> 1;
> +
> +    target_ulong mie    = env->priv < PRV_M ||
> +                          (env->priv == PRV_M && mstatus_mie);
> +    target_ulong sie    = env->priv < PRV_S ||
> +                          (env->priv == PRV_S && mstatus_sie);
> +    target_ulong hs_sie = env->priv < PRV_S ||
> +                          (env->priv == PRV_S && hs_mstatus_sie);
> +
> +    if (riscv_cpu_virt_enabled(env)) {
> +        target_ulong pending_hs_irq = pending & -hs_sie;
> +
> +        if (pending_hs_irq) {
> +            riscv_cpu_set_force_hs_excep(env, FORCE_HS_EXCEP);
> +            return ctz64(pending_hs_irq);
> +        }
> +
> +        pending = vspending;
> +    }
> +
> +    irqs = (pending & ~env->mideleg & -mie) | (pending &  env->mideleg & -sie);

Isn't "-unsigned" implementation defined?  I can't get GCC to throw a warning
and it was already there, so maybe I'm just wrong?

>
>      if (irqs) {
>          return ctz64(irqs); /* since non-zero */

Reviewed-by: Palmer Dabbelt <palmerdabbelt@google.com>
Richard Henderson Jan. 9, 2020, 2:33 a.m. UTC | #2
On 1/9/20 11:49 AM, Palmer Dabbelt wrote:
>> +    irqs = (pending & ~env->mideleg & -mie) | (pending &  env->mideleg & -sie);
> 
> Isn't "-unsigned" implementation defined?  I can't get GCC to throw a warning
> and it was already there, so maybe I'm just wrong?

(1) You're confusing implementation defined with undefined, and unsigned
arithmetic is the former not the latter.

(2) There is no such thing as ones-compliment or sign-magnitude integer
hardware anymore, so for this case "implementation defined" is in fact universal.

(3) We build with -fwrapv, so we're explicitly asking for sane behaviour from
our signed types as well.


r~
Palmer Dabbelt Jan. 10, 2020, 11:21 p.m. UTC | #3
On Wed, 08 Jan 2020 18:33:40 PST (-0800), richard.henderson@linaro.org wrote:
> On 1/9/20 11:49 AM, Palmer Dabbelt wrote:
>>> +    irqs = (pending & ~env->mideleg & -mie) | (pending &  env->mideleg & -sie);
>>
>> Isn't "-unsigned" implementation defined?  I can't get GCC to throw a warning
>> and it was already there, so maybe I'm just wrong?
>
> (1) You're confusing implementation defined with undefined, and unsigned
> arithmetic is the former not the latter.
>
> (2) There is no such thing as ones-compliment or sign-magnitude integer
> hardware anymore, so for this case "implementation defined" is in fact universal.
>
> (3) We build with -fwrapv, so we're explicitly asking for sane behaviour from
> our signed types as well.

Ah, thanks -- I didn't know we had -fwrapv.  In that case we're safe!

>
>
> r~

Patch
diff mbox series

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 63439c9370..85eed5d885 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -37,13 +37,36 @@  int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch)
 #ifndef CONFIG_USER_ONLY
 static int riscv_cpu_local_irq_pending(CPURISCVState *env)
 {
+    target_ulong irqs;
+
     target_ulong mstatus_mie = get_field(*env->mstatus, MSTATUS_MIE);
     target_ulong mstatus_sie = get_field(*env->mstatus, MSTATUS_SIE);
-    target_ulong pending = env->mip & env->mie;
-    target_ulong mie = env->priv < PRV_M || (env->priv == PRV_M && mstatus_mie);
-    target_ulong sie = env->priv < PRV_S || (env->priv == PRV_S && mstatus_sie);
-    target_ulong irqs = (pending & ~env->mideleg & -mie) |
-                        (pending &  env->mideleg & -sie);
+    target_ulong hs_mstatus_sie = get_field(env->mstatus_novirt, MSTATUS_SIE);
+
+    target_ulong pending = env->mip & env->mie &
+                               ~(MIP_VSSIP | MIP_VSTIP | MIP_VSEIP);
+    target_ulong vspending = (env->mip & env->mie &
+                              (MIP_VSSIP | MIP_VSTIP | MIP_VSEIP)) >> 1;
+
+    target_ulong mie    = env->priv < PRV_M ||
+                          (env->priv == PRV_M && mstatus_mie);
+    target_ulong sie    = env->priv < PRV_S ||
+                          (env->priv == PRV_S && mstatus_sie);
+    target_ulong hs_sie = env->priv < PRV_S ||
+                          (env->priv == PRV_S && hs_mstatus_sie);
+
+    if (riscv_cpu_virt_enabled(env)) {
+        target_ulong pending_hs_irq = pending & -hs_sie;
+
+        if (pending_hs_irq) {
+            riscv_cpu_set_force_hs_excep(env, FORCE_HS_EXCEP);
+            return ctz64(pending_hs_irq);
+        }
+
+        pending = vspending;
+    }
+
+    irqs = (pending & ~env->mideleg & -mie) | (pending &  env->mideleg & -sie);
 
     if (irqs) {
         return ctz64(irqs); /* since non-zero */