Re: [PATCH 1/1] target/riscv: fix VS interrupts forwarding to HS

[Jose Martins <josemartins90@gmail.com>]

> If the Hypervisor sets the V* interrupts why does it then want to
> receive the interrupt itself?

I don't think this is a question of whether there is a use case for it
or not (I agree with you, of the top of my head I don't see why would
you forward v* interrupts to the hypervisor). However,  from what I
can understand,  the spec allows for it. If you don't set the
corresponding bits in hideleg, v* interrupts should be forwarded to HS
(as I said, they are guaranteed not to be forwarded to m mode because
these bits must be hardwired in mideleg). Otherwise, there would be no
purpose for the hideleg register, as v* interrupts bits are the only
ones that can be written in it (am I missing something?).

> Isn't hs_sie only ever accessed if riscv_cpu_virt_enabled(env)?
> Doesn't this just set hs_sie to always be 1?

I don't understand if you don't agree that hs_sie should be always set
when riscv_cpu_virt_enabled(env), or if you agree with it and don't
see the need for the hs_sie variable at all. If it is the latter, I
agree with you. So the patch would become:

Signed-off-by: Jose Martins <josemartins90@gmail.com>
---
 target/riscv/cpu_helper.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index d3ba9efb02..a85eadb4fb 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -41,10 +41,8 @@ static int riscv_cpu_local_irq_pending(CPURISCVState *env)

     target_ulong mstatus_mie = get_field(env->mstatus, MSTATUS_MIE);
     target_ulong mstatus_sie = get_field(env->mstatus, MSTATUS_SIE);
-    target_ulong hs_mstatus_sie = get_field(env->mstatus_hs, MSTATUS_SIE);

-    target_ulong pending = env->mip & env->mie &
-                               ~(MIP_VSSIP | MIP_VSTIP | MIP_VSEIP);
+    target_ulong pending = env->mip & env->mie;
     target_ulong vspending = (env->mip & env->mie &
                               (MIP_VSSIP | MIP_VSTIP | MIP_VSEIP));

@@ -52,11 +50,9 @@ static int riscv_cpu_local_irq_pending(CPURISCVState *env)
                           (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;
+        target_ulong pending_hs_irq = pending & ~env->hideleg;

         if (pending_hs_irq) {
             riscv_cpu_set_force_hs_excep(env, FORCE_HS_EXCEP);
-- 
2.17.1

Jose