[RFC,2/2] ARM: arm_cpu_reset: make possible to use high vectors for reset_exc

Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
---
 target-arm/cpu.c | 4 ++++
 1 file changed, 4 insertions(+)

On Sat, Dec 7, 2013 at 10:55 AM, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> ---
>  target-arm/cpu.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index f838499..6f548c7 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -126,6 +126,10 @@ static void arm_cpu_reset(CPUState *s)
>              env->cp15.c1_sys = c1_sys | (1 << 13);
>      }
>
> +    if (!IS_M(env) && env->cp15.c1_sys & (1 << 13)) {
> +            env->regs[15] = 0xFFFF0000;

Dont see the need for the second if. This should only be reachable if
reset_hivecs == true. Can you just put this line after the above
"env->cp15.c1_sys = c1_sys | (1 << 13);" ?

With the change I suggested in P1, the reset_hivecs variable wont be a
property of M CPUs so it will be impossible for an M to have
rest_hivecs regardless of board code (mis)behaviour. So I think you
are safe to drop the IS_M here.

Regards,
Peter

> +    }
> +
>      env->vfp.xregs[ARM_VFP_FPEXC] = 0;
>  #endif
>      set_flush_to_zero(1, &env->vfp.standard_fp_status);
> --
> 1.8.5
>
>

On Sat, 7 Dec 2013 11:08:40 +1000
Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:

> On Sat, Dec 7, 2013 at 10:55 AM, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> > Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> > ---
> >  target-arm/cpu.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> > index f838499..6f548c7 100644
> > --- a/target-arm/cpu.c
> > +++ b/target-arm/cpu.c
> > @@ -126,6 +126,10 @@ static void arm_cpu_reset(CPUState *s)
> >              env->cp15.c1_sys = c1_sys | (1 << 13);
> >      }
> >
> > +    if (!IS_M(env) && env->cp15.c1_sys & (1 << 13)) {
> > +            env->regs[15] = 0xFFFF0000;
> 
> Dont see the need for the second if. This should only be reachable if
> reset_hivecs == true. Can you just put this line after the above
> "env->cp15.c1_sys = c1_sys | (1 << 13);" ?

Here we have a small problem.
On the one hand we have 'reset_hivecs' property, on the other hand we have c1_sys[13].

What we have to do if during reset c1_sys[13] == 1 and 'reset_hivecs' is not set?

Also if c1_sys[13] == 0, but 'reset_hivecs' is set IMHO we have to set c1_sys[13] = 1.


> With the change I suggested in P1, the reset_hivecs variable wont be a
> property of M CPUs so it will be impossible for an M to have
> rest_hivecs regardless of board code (mis)behaviour. So I think you
> are safe to drop the IS_M here.
> 
> Regards,
> Peter
> 
> > +    }
> > +
> >      env->vfp.xregs[ARM_VFP_FPEXC] = 0;
> >  #endif
> >      set_flush_to_zero(1, &env->vfp.standard_fp_status);
> > --
> > 1.8.5
> >
> >

On 7 December 2013 20:49, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> On Sat, 7 Dec 2013 11:08:40 +1000
> Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
>
>> On Sat, Dec 7, 2013 at 10:55 AM, Antony Pavlov <antonynpavlov@gmail.com> wrote:
>> > Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
>> > ---
>> >  target-arm/cpu.c | 4 ++++
>> >  1 file changed, 4 insertions(+)
>> >
>> > diff --git a/target-arm/cpu.c b/target-arm/cpu.c
>> > index f838499..6f548c7 100644
>> > --- a/target-arm/cpu.c
>> > +++ b/target-arm/cpu.c
>> > @@ -126,6 +126,10 @@ static void arm_cpu_reset(CPUState *s)
>> >              env->cp15.c1_sys = c1_sys | (1 << 13);
>> >      }
>> >
>> > +    if (!IS_M(env) && env->cp15.c1_sys & (1 << 13)) {
>> > +            env->regs[15] = 0xFFFF0000;
>>
>> Dont see the need for the second if. This should only be reachable if
>> reset_hivecs == true. Can you just put this line after the above
>> "env->cp15.c1_sys = c1_sys | (1 << 13);" ?
>
> Here we have a small problem.
> On the one hand we have 'reset_hivecs' property, on the other hand we have c1_sys[13].
>
> What we have to do if during reset c1_sys[13] == 1 and 'reset_hivecs' is not set?

See my other mail: on reset you should not care about the value
of reset_hivecs. At realize (ie in arm_cpu_realizefn(), at the top
(so it takes effect before that function calls
register_cp_regs_for_features()) you should do:

    if (cpu->reset_hivecs) {
        cpu->reset_sctlr |= (1 << 13);
    }

This will mean that c1_sys (the SCTLR) will always reset with
the hivecs bit set, which is what we want.

Then the only thing you need to do at reset is:
    if (env->cp15.c1_sys & (1 << 13)) {
        /* hivecs setting also affects the CPU reset address */
        env->regs[15] = 0xffff0000;
    }

(as Peter says we can rely on this never being true for M profile
so no specific check for that is required).

thanks
-- PMM