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

Aaron Lindsay posted 22 patches 7 years, 7 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v3 10/22] target/arm: Allow EL change hooks to do IO
Posted by Aaron Lindsay 7 years, 7 months ago
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();
         /* 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.


Re: [Qemu-devel] [PATCH v3 10/22] target/arm: Allow EL change hooks to do IO
Posted by Peter Maydell 7 years, 6 months ago
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.)

>          /* 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

Re: [Qemu-devel] [PATCH v3 10/22] target/arm: Allow EL change hooks to do IO
Posted by Aaron Lindsay 7 years, 6 months ago
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.

Re: [Qemu-devel] [PATCH v3 10/22] target/arm: Allow EL change hooks to do IO
Posted by Peter Maydell 7 years, 6 months ago
On 12 April 2018 at 18:08, Aaron Lindsay <alindsay@codeaurora.org> wrote:
> 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?

Yes, that's sufficient. (The other thing that needs to happen
for a gen_io_start/end is that the insn has to end the TB --
but in all these cases that's already true as they set
s->base.is_jmp = DISAS_EXIT.)

thanks
-- PMM