[next,v2,37/74] arm_boot: Pass ARMCPU to do_cpu_reset()

Allows us to use cpu_reset() in place of cpu_state_reset().

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/arm_boot.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

On 10 May 2012 01:14, Andreas Färber <afaerber@suse.de> wrote:
> Allows us to use cpu_reset() in place of cpu_state_reset().
>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  hw/arm_boot.c |    9 ++++++---
>  1 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/hw/arm_boot.c b/hw/arm_boot.c
> index 7447f5c..eb2d176 100644
> --- a/hw/arm_boot.c
> +++ b/hw/arm_boot.c
> @@ -274,10 +274,11 @@ static int load_dtb(target_phys_addr_t addr, const struct arm_boot_info *binfo)
>
>  static void do_cpu_reset(void *opaque)
>  {
> -    CPUARMState *env = opaque;
> +    ARMCPU *cpu = opaque;
> +    CPUARMState *env = &cpu->env;
>     const struct arm_boot_info *info = env->boot_info;
>
> -    cpu_state_reset(env);
> +    cpu_reset(CPU(cpu));

(Sadface for QOM method invocation requiring the explicit cast to
base class...)

>     if (info) {
>         if (!info->is_linux) {
>             /* Jump to the entry point.  */
> @@ -302,6 +303,7 @@ static void do_cpu_reset(void *opaque)
>
>  void arm_load_kernel(CPUARMState *env, struct arm_boot_info *info)
>  {
> +    ARMCPU *cpu;
>     int kernel_size;
>     int initrd_size;
>     int n;

This variable could be kept in the more limited scope inside
the for loop below, right?

> @@ -406,7 +408,8 @@ void arm_load_kernel(CPUARMState *env, struct arm_boot_info *info)
>     info->is_linux = is_linux;
>
>     for (; env; env = env->next_cpu) {
> +        cpu = arm_env_get_cpu(env);
>         env->boot_info = info;
> -        qemu_register_reset(do_cpu_reset, env);
> +        qemu_register_reset(do_cpu_reset, cpu);
>     }
>  }
> --
> 1.7.7
>
>


-- PMM

Am 10.05.2012 23:41, schrieb Peter Maydell:
> On 10 May 2012 01:14, Andreas Färber <afaerber@suse.de> wrote:
>> Allows us to use cpu_reset() in place of cpu_state_reset().
>>
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>> ---
>>  hw/arm_boot.c |    9 ++++++---
>>  1 files changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/arm_boot.c b/hw/arm_boot.c
>> index 7447f5c..eb2d176 100644
>> --- a/hw/arm_boot.c
>> +++ b/hw/arm_boot.c
[...]
>> @@ -302,6 +303,7 @@ static void do_cpu_reset(void *opaque)
>>
>>  void arm_load_kernel(CPUARMState *env, struct arm_boot_info *info)
>>  {
>> +    ARMCPU *cpu;
>>     int kernel_size;
>>     int initrd_size;
>>     int n;
> 
> This variable could be kept in the more limited scope inside
> the for loop below, right?

It could. The function currently has a CPUARMState *env argument. That
is supposed to change to ARMCPU *cpu in one of the followup series, in
which case the for loop can reuse that parameter. I generally try to
keep cpu and env close together so that it's visible in diff/grep
context, as explained for your cpu_reset_model_id() series.

Would you like it better if I unneededly assigned ARMCPU *cpu =
arm_env_get_cpu(env); to show its relation to env? The next step would
be to pass cpu through to the write_secondary_boot callback.

I have not included the signature changes for arm_load_kernel() or the
callback in this series because it requires comparable changes to more
actively maintained machines such as exynos and highbank and would've
made it even longer. I'd be happy to reprioritize such a series though
if you're willing to review it and handle any arising conflicts. :)

/-F

>> @@ -406,7 +408,8 @@ void arm_load_kernel(CPUARMState *env, struct arm_boot_info *info)
>>     info->is_linux = is_linux;
>>
>>     for (; env; env = env->next_cpu) {
>> +        cpu = arm_env_get_cpu(env);
>>         env->boot_info = info;
>> -        qemu_register_reset(do_cpu_reset, env);
>> +        qemu_register_reset(do_cpu_reset, cpu);
>>     }
>>  }
>> --
>> 1.7.7

On 10 May 2012 23:40, Andreas Färber <afaerber@suse.de> wrote:
> Am 10.05.2012 23:41, schrieb Peter Maydell:
>> On 10 May 2012 01:14, Andreas Färber <afaerber@suse.de> wrote:
>>> Allows us to use cpu_reset() in place of cpu_state_reset().
>>>
>>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>>> ---
>>>  hw/arm_boot.c |    9 ++++++---
>>>  1 files changed, 6 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/hw/arm_boot.c b/hw/arm_boot.c
>>> index 7447f5c..eb2d176 100644
>>> --- a/hw/arm_boot.c
>>> +++ b/hw/arm_boot.c
> [...]
>>> @@ -302,6 +303,7 @@ static void do_cpu_reset(void *opaque)
>>>
>>>  void arm_load_kernel(CPUARMState *env, struct arm_boot_info *info)
>>>  {
>>> +    ARMCPU *cpu;
>>>     int kernel_size;
>>>     int initrd_size;
>>>     int n;
>>
>> This variable could be kept in the more limited scope inside
>> the for loop below, right?
>
> It could. The function currently has a CPUARMState *env argument. That
> is supposed to change to ARMCPU *cpu in one of the followup series, in
> which case the for loop can reuse that parameter. I generally try to
> keep cpu and env close together so that it's visible in diff/grep
> context, as explained for your cpu_reset_model_id() series.

Ah, I see.

Acked-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM

Am 11.05.2012 09:17, schrieb Peter Maydell:
> On 10 May 2012 23:40, Andreas Färber <afaerber@suse.de> wrote:
>> Am 10.05.2012 23:41, schrieb Peter Maydell:
>>> On 10 May 2012 01:14, Andreas Färber <afaerber@suse.de> wrote:
>>>> Allows us to use cpu_reset() in place of cpu_state_reset().
>>>>
>>>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>>>> ---
>>>>  hw/arm_boot.c |    9 ++++++---
>>>>  1 files changed, 6 insertions(+), 3 deletions(-)
> 
> Acked-by: Peter Maydell <peter.maydell@linaro.org>

Thanks, applied to qom-next:
http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/qom-next

/-F