Re: [Qemu-arm] [PATCH for-2.9] hw/misc/imx6_src: Don't crash trying to reset missing CPUs

[Peter Maydell]

On 14 March 2017 at 17:12, Alex Bennée <address@hidden> wrote:
>
> Peter Maydell <address@hidden> writes:
>
>> Commit 4881658a4b introduced a call to arm_get_cpu_by_id(),
>> and Coverity noticed that we weren't checking that it didn't
>> return NULL (CID 1371652).
>>
>> Normally this won't happen (because all 4 CPUs are expected
>> to exist), but it's possible the user requested fewer CPUs
>> on the command line. Handle this possibility by silently
>> doing nothing, which is the same behaviour as before commit
>> 4881658a4b and also how we handle the other CPU operations
>> (since we ignore the INVALID_PARAM returns from arm_set_cpu_on()
>> and friends).
>>
>> There is a slight behavioural difference to the pre-4881658a4b
>> situation: the "reset this core" bit will remain set rather
>> than not being permitted to be set. The imx6 datasheet is
>> unclear about the behaviour in this odd corner case, so we
>> opt for the simpler code rather than complicated logic to
>> maintain identical behaviour.
>>
>> Signed-off-by: Peter Maydell <address@hidden>
>> ---
>> I couldn't actually get this to crash even with -smp 1 with
>> my test image, but we should fix it anyhow.
>>
>>  hw/misc/imx6_src.c | 8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/misc/imx6_src.c b/hw/misc/imx6_src.c
>> index edbb756..cfb0871 100644
>> --- a/hw/misc/imx6_src.c
>> +++ b/hw/misc/imx6_src.c
>> @@ -143,13 +143,17 @@ static void imx6_defer_clear_reset_bit(int cpuid,
>>                                         unsigned long reset_shift)
>>  {
>>      struct SRCSCRResetInfo *ri;
>> +    CPUState *cpu = arm_get_cpu_by_id(cpuid);
>> +
>> +    if (!cpu) {
>> +        return;
>> +    }
>>
>>      ri = g_malloc(sizeof(struct SRCSCRResetInfo));
>>      ri->s = s;
>>      ri->reset_bit = reset_shift;
>>
>> -    async_run_on_cpu(arm_get_cpu_by_id(cpuid), imx6_clear_reset_bit,
>> -                     RUN_ON_CPU_HOST_PTR(ri));
>> +    async_run_on_cpu(cpu, imx6_clear_reset_bit, RUN_ON_CPU_HOST_PTR(ri));
>>  }
>
> Reviewed-by: Alex Bennée <address@hidden>

Thanks; applied to master.

-- PMM