Re: [PATCH v6 0/8] Unified CPU type check

[Gavin Shan <gshan@redhat.com>]

On 11/21/23 05:42, Marcin Juszkiewicz wrote:
> W dniu 20.11.2023 o 01:27, Gavin Shan pisze:
>> Testing
>> =======
>>
>> With the following command lines, the output messages are varied before
>> and after the series is applied.
>>
>>    ./build/qemu-system-aarch64            \
>>    -accel tcg -machine virt,gic-version=3 \
>>    -cpu cortex-a8 -smp maxcpus=2,cpus=1
>>
>> Before the series is applied:
>>
>>    qemu-system-aarch64: mach-virt: CPU type cortex-a8-arm-cpu not supported
>>
>> After the series is applied:
>>
>>    qemu-system-aarch64: Invalid CPU type: cortex-a8-arm-cpu
>>    The valid models are: cortex-a7, cortex-a15, cortex-a35, cortex-a55,
>>                          cortex-a72, cortex-a76, a64fx, neoverse-n1,
>>                          neoverse-v1, cortex-a53, cortex-a57, max
> 
> 
> $ ./build/qemu-system-aarch64 -M sbsa-ref -cpu cortex-a53
> qemu-system-aarch64: Invalid CPU type: cortex-a53
> The valid types are: cortex-a57, cortex-a72, neoverse-n1, neoverse-v1, neoverse-n2, max
> 
> $ ./build/qemu-system-aarch64 -M sbsa-ref -cpu sa1100
> Unexpected error in object_property_find_err() at ../qom/object.c:1329:
> qemu-system-aarch64: Property 'sa1100-arm-cpu.secure-memory' not found
> Aborted (core dumped)
> 
> 
> Similar with 'host' or 'pxa250' while QEMU/master does:
> 
> $ qemu-system-aarch64 -M sbsa-ref -cpu sa1100
> qemu-system-aarch64: sbsa-ref: CPU type sa1100-arm-cpu not supported
> 

Good catch! I didn't realize that the syntax of error_propagate() has been changed
due to ERRP_GUARD() since ae7c80a7bd7 ("error: New macro ERRP_GUARD()"). Prior to
the commit, QEMU process is terminated immediately by error_propagate(). After the
commit, the termination is delayed until machine_run_board_init() returns. For the
later case, mc->init() is called and causing coredump on 'sbsa-ref' board.

A 'return' is needed after error_propagate() in machine_run_board_init() so that
mc->init() won't be called.

void machine_run_board_init(MachineState *machine, const char *mem_path, Error **errp)
{
     ERRP_GUARD();
     MachineClass *machine_class = MACHINE_GET_CLASS(machine);
     Error *local_err = NULL;
     :
     /* Check if the CPU type is supported */
     is_cpu_type_supported(machine, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         return;                               <<<<< A 'return' is needed here >>>>>
     }
     :
     accel_init_interfaces(ACCEL_GET_CLASS(machine->accelerator));
     machine_class->init(machine);
     phase_advance(PHASE_MACHINE_INITIALIZED);
}

With the 'return' added, everything looks good. I would let v6 floating for another 2-3
days before posting v7 to fix the issue.

[gshan@gshan q]$ ./build/qemu-system-aarch64 -M virt -cpu cortex-a8
qemu-system-aarch64: Invalid CPU type: cortex-a8
The valid types are: cortex-a7, cortex-a15, cortex-a35, cortex-a55, cortex-a72, cortex-a76, cortex-a710, a64fx, neoverse-n1, neoverse-v1, neoverse-n2, cortex-a53, cortex-a57, max

[gshan@gshan q]$ ./build/qemu-system-aarch64 -M sbsa-ref -cpu sa1100
qemu-system-aarch64: Invalid CPU type: sa1100
The valid types are: cortex-a57, cortex-a72, neoverse-n1, neoverse-v1, neoverse-n2, max

[gshan@gshan q]$ ./build/qemu-system-aarch64 -M sbsa-ref -cpu cortex-m0
qemu-system-aarch64: Invalid CPU type: cortex-m0
The valid types are: cortex-a57, cortex-a72, neoverse-n1, neoverse-v1, neoverse-n2, max

[gshan@gshan q]$ ./build/qemu-system-aarch64 -M sbsa-ref -cpu pxa250
qemu-system-aarch64: Invalid CPU type: pxa250
The valid types are: cortex-a57, cortex-a72, neoverse-n1, neoverse-v1, neoverse-n2, max

[gshan@gshan q]$ ./build/qemu-system-aarch64 -M sbsa-ref -cpu host
qemu-system-aarch64: unable to find CPU model 'host'

Thanks,
Gavin