Re: [PATCH v3 15/17] hw/microblaze: Support various endianness for s3adsp1800 machines

[Thomas Huth <thuth@redhat.com>]

On 08/11/2024 16.43, Philippe Mathieu-Daudé wrote:
> Introduce an abstract machine parent class which defines
> the 'little_endian' property. Duplicate the current machine,
> which endian is tied to the binary endianness, to one big
> endian and a little endian machine; updating the machine
> description. Keep the current default machine for each binary.
> 
> 'petalogix-s3adsp1800' machine is aliased as:
> - 'petalogix-s3adsp1800-be' on big-endian binary,
> - 'petalogix-s3adsp1800-le' on little-endian one.
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/microblaze/petalogix_s3adsp1800_mmu.c | 62 +++++++++++++++++++-----
>   1 file changed, 51 insertions(+), 11 deletions(-)
...
>   static const TypeInfo petalogix_s3adsp1800_machine_types[] = {
>       {
>           .name           = TYPE_PETALOGIX_S3ADSP1800_MACHINE,
>           .parent         = TYPE_MACHINE,
> -        .class_init     = petalogix_s3adsp1800_machine_class_init,
> +        .abstract       = true,
> +        .class_size     = sizeof(PetalogixS3adsp1800MachineClass),
> +    },
> +    {
> +        .name           = MACHINE_TYPE_NAME("petalogix-s3adsp1800-be"),
> +        .parent         = TYPE_PETALOGIX_S3ADSP1800_MACHINE,
> +        .class_init     = petalogix_s3adsp1800_machine_class_init_be,
> +    },
> +    {
> +        .name           = MACHINE_TYPE_NAME("petalogix-s3adsp1800-le"),
> +        .parent         = TYPE_PETALOGIX_S3ADSP1800_MACHINE,
> +        .class_init     = petalogix_s3adsp1800_machine_class_init_le,
>       },
>   };
>   

Do we really want additional machine types for this? Can't we simply let the 
user set the machine property instead? (otherwise, all tests that run for 
each machine types (see qtest_cb_for_every_machine) will now be executed 
three times instead of only once). IMHO it should be sufficient to have a 
machine property for this and add proper documentation for the machine.

  Thomas