Re: [Qemu-devel] [PATCH v3 10/22] target/arm: Allow EL change hooks to do IO

[Aaron Lindsay <alindsay@codeaurora.org>]

On Apr 12 17:53, Peter Maydell wrote:
> On 16 March 2018 at 20:31, Aaron Lindsay <alindsay@codeaurora.org> wrote:
> > During code generation, surround CPSR writes and exception returns which
> > call the EL change hooks with gen_io_start/end. The immediate need is
> > for the PMU to access the clock and icount during EL change to support
> > mode filtering.
> >
> > Signed-off-by: Aaron Lindsay <alindsay@codeaurora.org>
> > ---
> >  target/arm/translate-a64.c | 2 ++
> >  target/arm/translate.c     | 4 ++++
> >  2 files changed, 6 insertions(+)
> >
> > diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> > index 31ff047..e1ae676 100644
> > --- a/target/arm/translate-a64.c
> > +++ b/target/arm/translate-a64.c
> > @@ -1919,7 +1919,9 @@ static void disas_uncond_b_reg(DisasContext *s, uint32_t insn)
> >              unallocated_encoding(s);
> >              return;
> >          }
> > +        gen_io_start();
> >          gen_helper_exception_return(cpu_env);
> > +        gen_io_end();
> 
> You don't want to call gen_io_start() or gen_io_end() unless
> tb_cflags(s->base.tb) & CF_USE_ICOUNT) is true.
> 
> (Ditto in the other cases below.)

I assume there's nothing tricky about this and updating this as follows
is sufficient?

> > +        if (tb_cflags(s->base.tb) & CF_USE_ICOUNT) {
> > +            gen_io_start();
> > +        }
> >          gen_helper_exception_return(cpu_env);
> > +        if (tb_cflags(s->base.tb) & CF_USE_ICOUNT) {
> > +            gen_io_end();
> > +        }

-Aaron

> 
> >          /* Must exit loop to check un-masked IRQs */
> >          s->base.is_jmp = DISAS_EXIT;
> >          return;
> > diff --git a/target/arm/translate.c b/target/arm/translate.c
> > index ba6ab7d..fd5871e 100644
> > --- a/target/arm/translate.c
> > +++ b/target/arm/translate.c
> > @@ -4536,7 +4536,9 @@ static void gen_rfe(DisasContext *s, TCGv_i32 pc, TCGv_i32 cpsr)
> >       * appropriately depending on the new Thumb bit, so it must
> >       * be called after storing the new PC.
> >       */
> > +    gen_io_start();
> >      gen_helper_cpsr_write_eret(cpu_env, cpsr);
> > +    gen_io_end();
> >      tcg_temp_free_i32(cpsr);
> >      /* Must exit loop to check un-masked IRQs */
> >      s->base.is_jmp = DISAS_EXIT;
> > @@ -9828,7 +9830,9 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn)
> >                  if (exc_return) {
> >                      /* Restore CPSR from SPSR.  */
> >                      tmp = load_cpu_field(spsr);
> > +                    gen_io_start();
> >                      gen_helper_cpsr_write_eret(cpu_env, tmp);
> > +                    gen_io_end();
> >                      tcg_temp_free_i32(tmp);
> >                      /* Must exit loop to check un-masked IRQs */
> >                      s->base.is_jmp = DISAS_EXIT;
> > --
> > Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
> > Qualcomm Technologies, Inc. is a member of the
> > Code Aurora Forum, a Linux Foundation Collaborative Project.
> >
> 
> thanks
> -- PMM

-- 
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.