Re: [PATCH v5 11/16] hw/microblaze: Support various endianness for s3adsp1800 machines

["Daniel P. Berrangé" <berrange@redhat.com>]

On Thu, Feb 06, 2025 at 02:10:47PM +0100, 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.

Does it makes sense to expose these as different machine types ?

If all the HW is identical in both cases, it feels like the
endianness could just be a bool property of the machine type,
rather than a new machine type.

> 
> 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(-)
> 
> diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
> index 96aed4ed1a3..aea727eb7ee 100644
> --- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
> +++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
> @@ -55,8 +55,17 @@
>  #define ETHLITE_IRQ         1
>  #define UARTLITE_IRQ        3
>  
> +typedef struct PetalogixS3adsp1800MachineClass {
> +    MachineClass parent_obj;
> +
> +    bool little_endian;
> +} PetalogixS3adsp1800MachineClass;
> +
>  #define TYPE_PETALOGIX_S3ADSP1800_MACHINE \
> -            MACHINE_TYPE_NAME("petalogix-s3adsp1800")
> +            MACHINE_TYPE_NAME("petalogix-s3adsp1800-common")
> +DECLARE_CLASS_CHECKERS(PetalogixS3adsp1800MachineClass,
> +                       PETALOGIX_S3ADSP1800_MACHINE,
> +                       TYPE_PETALOGIX_S3ADSP1800_MACHINE)
>  
>  static void
>  petalogix_s3adsp1800_init(MachineState *machine)
> @@ -71,11 +80,13 @@ petalogix_s3adsp1800_init(MachineState *machine)
>      MemoryRegion *phys_ram = g_new(MemoryRegion, 1);
>      qemu_irq irq[32];
>      MemoryRegion *sysmem = get_system_memory();
> +    PetalogixS3adsp1800MachineClass *pmc;
>  
> +    pmc = PETALOGIX_S3ADSP1800_MACHINE_GET_CLASS(machine);
>      cpu = MICROBLAZE_CPU(object_new(TYPE_MICROBLAZE_CPU));
>      object_property_set_str(OBJECT(cpu), "version", "7.10.d", &error_abort);
>      object_property_set_bool(OBJECT(cpu), "little-endian",
> -                             !TARGET_BIG_ENDIAN, &error_abort);
> +                             pmc->little_endian, &error_abort);
>      qdev_realize(DEVICE(cpu), NULL, &error_abort);
>  
>      /* Attach emulated BRAM through the LMB.  */
> @@ -95,7 +106,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
>                            64 * KiB, 1, 0x89, 0x18, 0x0000, 0x0, 1);
>  
>      dev = qdev_new("xlnx.xps-intc");
> -    qdev_prop_set_bit(dev, "little-endian", !TARGET_BIG_ENDIAN);
> +    qdev_prop_set_bit(dev, "little-endian", pmc->little_endian);
>      qdev_prop_set_uint32(dev, "kind-of-intr",
>                           1 << ETHLITE_IRQ | 1 << UARTLITE_IRQ);
>      sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
> @@ -107,7 +118,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
>      }
>  
>      dev = qdev_new(TYPE_XILINX_UARTLITE);
> -    qdev_prop_set_bit(dev, "little-endian", !TARGET_BIG_ENDIAN);
> +    qdev_prop_set_bit(dev, "little-endian", pmc->little_endian);
>      qdev_prop_set_chr(dev, "chardev", serial_hd(0));
>      sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>      sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, UARTLITE_BASEADDR);
> @@ -115,7 +126,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
>  
>      /* 2 timers at irq 2 @ 62 Mhz.  */
>      dev = qdev_new("xlnx.xps-timer");
> -    qdev_prop_set_bit(dev, "little-endian", !TARGET_BIG_ENDIAN);
> +    qdev_prop_set_bit(dev, "little-endian", pmc->little_endian);
>      qdev_prop_set_uint32(dev, "one-timer-only", 0);
>      qdev_prop_set_uint32(dev, "clock-frequency", 62 * 1000000);
>      sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
> @@ -123,7 +134,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
>      sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq[TIMER_IRQ]);
>  
>      dev = qdev_new("xlnx.xps-ethernetlite");
> -    qdev_prop_set_bit(dev, "little-endian", !TARGET_BIG_ENDIAN);
> +    qdev_prop_set_bit(dev, "little-endian", pmc->little_endian);
>      qemu_configure_nic_device(dev, true, NULL);
>      qdev_prop_set_uint32(dev, "tx-ping-pong", 0);
>      qdev_prop_set_uint32(dev, "rx-ping-pong", 0);
> @@ -133,26 +144,55 @@ petalogix_s3adsp1800_init(MachineState *machine)
>  
>      create_unimplemented_device("xps_gpio", GPIO_BASEADDR, 0x10000);
>  
> -    microblaze_load_kernel(cpu, !TARGET_BIG_ENDIAN, ddr_base, ram_size,
> +    microblaze_load_kernel(cpu, pmc->little_endian, ddr_base, ram_size,
>                             machine->initrd_filename,
>                             BINARY_DEVICE_TREE_FILE,
>                             NULL);
>  }
>  
> -static void petalogix_s3adsp1800_machine_class_init(ObjectClass *oc, void *data)
> +static void petalogix_s3adsp1800_machine_class_init(ObjectClass *oc,
> +                                                    bool little_endian)
>  {
>      MachineClass *mc = MACHINE_CLASS(oc);
> +    PetalogixS3adsp1800MachineClass *pmc = PETALOGIX_S3ADSP1800_MACHINE_CLASS(oc);
>  
> -    mc->desc = "PetaLogix linux refdesign for xilinx Spartan 3ADSP1800";
>      mc->init = petalogix_s3adsp1800_init;
> -    mc->is_default = true;
> +    pmc->little_endian = little_endian;
> +    mc->desc = little_endian
> +        ? "PetaLogix linux refdesign for xilinx Spartan 3ADSP1800 (little endian)"
> +        : "PetaLogix linux refdesign for xilinx Spartan 3ADSP1800 (big endian)";
> +    if (little_endian == !TARGET_BIG_ENDIAN) {
> +        mc->is_default = true;
> +        mc->alias = "petalogix-s3adsp1800";
> +    }
> +}
> +
> +static void petalogix_s3adsp1800_machine_class_init_be(ObjectClass *oc, void *data)
> +{
> +    petalogix_s3adsp1800_machine_class_init(oc, false);
> +}
> +
> +static void petalogix_s3adsp1800_machine_class_init_le(ObjectClass *oc, void *data)
> +{
> +    petalogix_s3adsp1800_machine_class_init(oc, true);
>  }
>  
>  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,
>      },
>  };
>  
> -- 
> 2.47.1
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|