| Message ID | 20220531210544.181322-1-abrestic@rivosinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/riscv: Wake on VS-level external interrupts | expand |
On Wed, Jun 1, 2022 at 7:07 AM Andrew Bresticker <abrestic@rivosinc.com> wrote: > > Whether or not VSEIP is pending isn't reflected in env->mip and must > instead be determined from hstatus.vgein and hgeip. As a result a > CPU in WFI won't wake on a VSEIP, which violates the WFI behavior as > specified in the privileged ISA. Just use riscv_cpu_all_pending() > instead, which already accounts for VSEIP. > > Signed-off-by: Andrew Bresticker <abrestic@rivosinc.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > target/riscv/cpu.c | 2 +- > target/riscv/cpu.h | 1 + > target/riscv/cpu_helper.c | 2 +- > 3 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index a91253d4bd..c6cc08c355 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -391,7 +391,7 @@ static bool riscv_cpu_has_work(CPUState *cs) > * Definition of the WFI instruction requires it to ignore the privilege > * mode and delegation registers, but respect individual enables > */ > - return (env->mip & env->mie) != 0; > + return riscv_cpu_all_pending(env) != 0; > #else > return true; > #endif > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h > index f08c3e8813..758ab6c90b 100644 > --- a/target/riscv/cpu.h > +++ b/target/riscv/cpu.h > @@ -488,6 +488,7 @@ int riscv_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg); > int riscv_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg); > int riscv_cpu_hviprio_index2irq(int index, int *out_irq, int *out_rdzero); > uint8_t riscv_cpu_default_priority(int irq); > +uint64_t riscv_cpu_all_pending(CPURISCVState *env); > int riscv_cpu_mirq_pending(CPURISCVState *env); > int riscv_cpu_sirq_pending(CPURISCVState *env); > int riscv_cpu_vsirq_pending(CPURISCVState *env); > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c > index d99fac9d2d..16c6045459 100644 > --- a/target/riscv/cpu_helper.c > +++ b/target/riscv/cpu_helper.c > @@ -340,7 +340,7 @@ static int riscv_cpu_pending_to_irq(CPURISCVState *env, > return best_irq; > } > > -static uint64_t riscv_cpu_all_pending(CPURISCVState *env) > +uint64_t riscv_cpu_all_pending(CPURISCVState *env) > { > uint32_t gein = get_field(env->hstatus, HSTATUS_VGEIN); > uint64_t vsgein = (env->hgeip & (1ULL << gein)) ? MIP_VSEIP : 0; > -- > 2.25.1 > >
On Wed, Jun 1, 2022 at 7:07 AM Andrew Bresticker <abrestic@rivosinc.com> wrote: > > Whether or not VSEIP is pending isn't reflected in env->mip and must > instead be determined from hstatus.vgein and hgeip. As a result a > CPU in WFI won't wake on a VSEIP, which violates the WFI behavior as > specified in the privileged ISA. Just use riscv_cpu_all_pending() > instead, which already accounts for VSEIP. > > Signed-off-by: Andrew Bresticker <abrestic@rivosinc.com> Thanks! Applied to riscv-to-apply.next Alistair > --- > target/riscv/cpu.c | 2 +- > target/riscv/cpu.h | 1 + > target/riscv/cpu_helper.c | 2 +- > 3 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index a91253d4bd..c6cc08c355 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -391,7 +391,7 @@ static bool riscv_cpu_has_work(CPUState *cs) > * Definition of the WFI instruction requires it to ignore the privilege > * mode and delegation registers, but respect individual enables > */ > - return (env->mip & env->mie) != 0; > + return riscv_cpu_all_pending(env) != 0; > #else > return true; > #endif > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h > index f08c3e8813..758ab6c90b 100644 > --- a/target/riscv/cpu.h > +++ b/target/riscv/cpu.h > @@ -488,6 +488,7 @@ int riscv_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg); > int riscv_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg); > int riscv_cpu_hviprio_index2irq(int index, int *out_irq, int *out_rdzero); > uint8_t riscv_cpu_default_priority(int irq); > +uint64_t riscv_cpu_all_pending(CPURISCVState *env); > int riscv_cpu_mirq_pending(CPURISCVState *env); > int riscv_cpu_sirq_pending(CPURISCVState *env); > int riscv_cpu_vsirq_pending(CPURISCVState *env); > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c > index d99fac9d2d..16c6045459 100644 > --- a/target/riscv/cpu_helper.c > +++ b/target/riscv/cpu_helper.c > @@ -340,7 +340,7 @@ static int riscv_cpu_pending_to_irq(CPURISCVState *env, > return best_irq; > } > > -static uint64_t riscv_cpu_all_pending(CPURISCVState *env) > +uint64_t riscv_cpu_all_pending(CPURISCVState *env) > { > uint32_t gein = get_field(env->hstatus, HSTATUS_VGEIN); > uint64_t vsgein = (env->hgeip & (1ULL << gein)) ? MIP_VSEIP : 0; > -- > 2.25.1 > >
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index a91253d4bd..c6cc08c355 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -391,7 +391,7 @@ static bool riscv_cpu_has_work(CPUState *cs) * Definition of the WFI instruction requires it to ignore the privilege * mode and delegation registers, but respect individual enables */ - return (env->mip & env->mie) != 0; + return riscv_cpu_all_pending(env) != 0; #else return true; #endif diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index f08c3e8813..758ab6c90b 100644 --- a/target/riscv/cpu.h +++ b/target/riscv/cpu.h @@ -488,6 +488,7 @@ int riscv_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg); int riscv_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg); int riscv_cpu_hviprio_index2irq(int index, int *out_irq, int *out_rdzero); uint8_t riscv_cpu_default_priority(int irq); +uint64_t riscv_cpu_all_pending(CPURISCVState *env); int riscv_cpu_mirq_pending(CPURISCVState *env); int riscv_cpu_sirq_pending(CPURISCVState *env); int riscv_cpu_vsirq_pending(CPURISCVState *env); diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c index d99fac9d2d..16c6045459 100644 --- a/target/riscv/cpu_helper.c +++ b/target/riscv/cpu_helper.c @@ -340,7 +340,7 @@ static int riscv_cpu_pending_to_irq(CPURISCVState *env, return best_irq; } -static uint64_t riscv_cpu_all_pending(CPURISCVState *env) +uint64_t riscv_cpu_all_pending(CPURISCVState *env) { uint32_t gein = get_field(env->hstatus, HSTATUS_VGEIN); uint64_t vsgein = (env->hgeip & (1ULL << gein)) ? MIP_VSEIP : 0;
Whether or not VSEIP is pending isn't reflected in env->mip and must instead be determined from hstatus.vgein and hgeip. As a result a CPU in WFI won't wake on a VSEIP, which violates the WFI behavior as specified in the privileged ISA. Just use riscv_cpu_all_pending() instead, which already accounts for VSEIP. Signed-off-by: Andrew Bresticker <abrestic@rivosinc.com> --- target/riscv/cpu.c | 2 +- target/riscv/cpu.h | 1 + target/riscv/cpu_helper.c | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-)