On Tue, May 4, 2021 at 6:45 AM Jose Martins <josemartin...@gmail.com> wrote: > > VS interrupts (2, 6, 10) were not correctly forwarded to hs-mode when > not delegated in hideleg (which was not being taken into account). This > was mainly because hs level sie was not always considered enabled when > it should. The spec states that "Interrupts for higher-privilege modes, > y>x, are always globally enabled regardless of the setting of the global > yIE bit for the higher-privilege mode." and also "For purposes of > interrupt global enables, HS-mode is considered more privileged than > VS-mode, and VS-mode is considered more privileged than VU-mode". > > These interrupts should be treated the same as any other kind of > exception. Therefore, there is no need to "force an hs exception" as the > current privilege level, the state of the global ie and of the > delegation registers should be enough to route the interrupt to the > appropriate privilege level in riscv_cpu_do_interrupt. Also, these > interrupts never target m-mode, which is guaranteed by the hardwiring > of the corresponding bits in mideleg. > > Signed-off-by: Jose Martins <josemartin...@gmail.com> > --- > Alistair, I sent the previous version of this patch a year ago (!). > After our brief exchange on wether we should considered HS a higher > privilege mode than VS (the spec was updated to clarify this in the > meantime) you asked me for a small last tweak before accepting > the patch. This completely slipped my mind, and I only noticed I hadn't > submitted the final version of the patch a few weeks ago. For this, my > apologies. When I took a deeper look at it again, I found some other issues > with the patch so I decided to go for a deeper refactoring. > > target/riscv/cpu_helper.c | 28 ++++++++-------------------- > 1 file changed, 8 insertions(+), 20 deletions(-) > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c > index 21c54ef561..592d4642be 100644 > --- a/target/riscv/cpu_helper.c > +++ b/target/riscv/cpu_helper.c > @@ -38,36 +38,24 @@ 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 virt_enabled = riscv_cpu_virt_enabled(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 vspending = (env->mip & env->mie & > - (MIP_VSSIP | MIP_VSTIP | MIP_VSEIP)); > + 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 hs_sie = env->priv < PRV_S || > - (env->priv == PRV_S && hs_mstatus_sie); > + target_ulong hsie = virt_enabled || sie; > + target_ulong vsie = virt_enabled && 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);
If we no longer need this function then it and related functions should be remove. Alistair > - return ctz64(pending_hs_irq); > - } > - > - pending = vspending; > - } > - > - irqs = (pending & ~env->mideleg & -mie) | (pending & env->mideleg & > -sie); > + target_ulong irqs = > + (pending & ~env->mideleg & -mie) | > + (pending & env->mideleg & ~env->hideleg & -hsie) | > + (pending & env->mideleg & env->hideleg & -vsie); > > if (irqs) { > return ctz64(irqs); /* since non-zero */ > -- > 2.25.1 > >