Re: [PATCH v7 02/10] hw/intc/xilinx_intc: Make device endianness configurable

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 12/2/25 13:36, Philippe Mathieu-Daudé wrote:
> Replace the DEVICE_NATIVE_ENDIAN MemoryRegionOps by a pair
> of DEVICE_LITTLE_ENDIAN / DEVICE_BIG_ENDIAN.
> Add the "little-endian" property to select the device
> endianness, defaulting to little endian.

Grr, please read:

   Add the "endianness" property to select the device endianness.
   This property is unspecified by default, and machines need to
   make it explicit.

(same in following patches).

> Set the proper endianness for each machine using the device.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/intc/xilinx_intc.c                    | 59 ++++++++++++++++++------
>   hw/microblaze/petalogix_ml605_mmu.c      |  1 +
>   hw/microblaze/petalogix_s3adsp1800_mmu.c |  3 ++
>   hw/ppc/virtex_ml507.c                    |  1 +
>   hw/riscv/microblaze-v-generic.c          |  1 +
>   5 files changed, 51 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/intc/xilinx_intc.c b/hw/intc/xilinx_intc.c
> index 6930f83907a..ab1c4a32221 100644
> --- a/hw/intc/xilinx_intc.c
> +++ b/hw/intc/xilinx_intc.c
> @@ -3,6 +3,9 @@
>    *
>    * Copyright (c) 2009 Edgar E. Iglesias.
>    *
> + * https://docs.amd.com/v/u/en-US/xps_intc
> + * DS572: LogiCORE IP XPS Interrupt Controller (v2.01a)
> + *
>    * Permission is hereby granted, free of charge, to any person obtaining a copy
>    * of this software and associated documentation files (the "Software"), to deal
>    * in the Software without restriction, including without limitation the rights
> @@ -23,10 +26,12 @@
>    */
>   
>   #include "qemu/osdep.h"
> +#include "qapi/error.h"
>   #include "hw/sysbus.h"
>   #include "qemu/module.h"
>   #include "hw/irq.h"
>   #include "hw/qdev-properties.h"
> +#include "hw/qdev-properties-system.h"
>   #include "qom/object.h"
>   
>   #define D(x)
> @@ -49,6 +54,7 @@ struct XpsIntc
>   {
>       SysBusDevice parent_obj;
>   
> +    EndianMode model_endianness;
>       MemoryRegion mmio;
>       qemu_irq parent_irq;
>   
> @@ -140,18 +146,28 @@ static void pic_write(void *opaque, hwaddr addr,
>       update_irq(p);
>   }
>   
> -static const MemoryRegionOps pic_ops = {
> -    .read = pic_read,
> -    .write = pic_write,
> -    .endianness = DEVICE_NATIVE_ENDIAN,
> -    .impl = {
> -        .min_access_size = 4,
> -        .max_access_size = 4,
> +static const MemoryRegionOps pic_ops[2] = {
> +    [0 ... 1] = {
> +        .read = pic_read,
> +        .write = pic_write,
> +        .impl = {
> +            .min_access_size = 4,
> +            .max_access_size = 4,
> +        },
> +        .valid = {
> +            /*
> +             * All XPS INTC registers are accessed through the PLB interface.
> +             * The base address for these registers is provided by the
> +             * configuration parameter, C_BASEADDR. Each register is 32 bits
> +             * although some bits may be unused and is accessed on a 4-byte
> +             * boundary offset from the base address.
> +             */
> +            .min_access_size = 4,
> +            .max_access_size = 4,
> +        },
>       },
> -    .valid = {
> -        .min_access_size = 4,
> -        .max_access_size = 4
> -    }
> +    [0].endianness = DEVICE_LITTLE_ENDIAN,
> +    [1].endianness = DEVICE_BIG_ENDIAN,
>   };
>   
>   static void irq_handler(void *opaque, int irq, int level)
> @@ -174,13 +190,27 @@ static void xilinx_intc_init(Object *obj)
>   
>       qdev_init_gpio_in(DEVICE(obj), irq_handler, 32);
>       sysbus_init_irq(SYS_BUS_DEVICE(obj), &p->parent_irq);
> -
> -    memory_region_init_io(&p->mmio, obj, &pic_ops, p, "xlnx.xps-intc",
> -                          R_MAX * 4);
>       sysbus_init_mmio(SYS_BUS_DEVICE(obj), &p->mmio);
>   }
>   
> +static void xilinx_intc_realize(DeviceState *dev, Error **errp)
> +{
> +    XpsIntc *p = XILINX_INTC(dev);
> +
> +    if (p->model_endianness == ENDIAN_MODE_UNSPECIFIED) {
> +        error_setg(errp, TYPE_XILINX_INTC " property 'endianness'"
> +                         " must be set to 'big' or 'little'");
> +        return;
> +    }
> +
> +    memory_region_init_io(&p->mmio, OBJECT(dev),
> +                          &pic_ops[p->model_endianness == ENDIAN_MODE_BIG],
> +                          p, "xlnx.xps-intc",
> +                          R_MAX * 4);
> +}
> +
>   static const Property xilinx_intc_properties[] = {
> +    DEFINE_PROP_ENDIAN_NODEFAULT("endianness", XpsIntc, model_endianness),
>       DEFINE_PROP_UINT32("kind-of-intr", XpsIntc, c_kind_of_intr, 0),
>   };
>   
> @@ -188,6 +218,7 @@ static void xilinx_intc_class_init(ObjectClass *klass, void *data)
>   {
>       DeviceClass *dc = DEVICE_CLASS(klass);
>   
> +    dc->realize = xilinx_intc_realize;
>       device_class_set_props(dc, xilinx_intc_properties);
>   }
>   
> diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c
> index 8b44be75a22..55398cc67d1 100644
> --- a/hw/microblaze/petalogix_ml605_mmu.c
> +++ b/hw/microblaze/petalogix_ml605_mmu.c
> @@ -111,6 +111,7 @@ petalogix_ml605_init(MachineState *machine)
>   
>   
>       dev = qdev_new("xlnx.xps-intc");
> +    qdev_prop_set_enum(dev, "endianness", ENDIAN_MODE_LITTLE);
>       qdev_prop_set_uint32(dev, "kind-of-intr", 1 << TIMER_IRQ);
>       sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>       sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, INTC_BASEADDR);
> diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
> index 2c0d8c34cd2..15cabe11777 100644
> --- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
> +++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
> @@ -71,6 +71,8 @@ petalogix_s3adsp1800_init(MachineState *machine)
>       MemoryRegion *phys_ram = g_new(MemoryRegion, 1);
>       qemu_irq irq[32];
>       MemoryRegion *sysmem = get_system_memory();
> +    EndianMode endianness = TARGET_BIG_ENDIAN ? ENDIAN_MODE_BIG
> +                                              : ENDIAN_MODE_LITTLE;
>   
>       cpu = MICROBLAZE_CPU(object_new(TYPE_MICROBLAZE_CPU));
>       object_property_set_str(OBJECT(cpu), "version", "7.10.d", &error_abort);
> @@ -95,6 +97,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
>                             64 * KiB, 1, 0x89, 0x18, 0x0000, 0x0, 1);
>   
>       dev = qdev_new("xlnx.xps-intc");
> +    qdev_prop_set_enum(dev, "endianness", endianness);
>       qdev_prop_set_uint32(dev, "kind-of-intr",
>                            1 << ETHLITE_IRQ | 1 << UARTLITE_IRQ);
>       sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
> diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
> index 23238119273..df8f9644829 100644
> --- a/hw/ppc/virtex_ml507.c
> +++ b/hw/ppc/virtex_ml507.c
> @@ -217,6 +217,7 @@ static void virtex_init(MachineState *machine)
>   
>       cpu_irq = qdev_get_gpio_in(DEVICE(cpu), PPC40x_INPUT_INT);
>       dev = qdev_new("xlnx.xps-intc");
> +    qdev_prop_set_enum(dev, "endianness", ENDIAN_MODE_BIG);
>       qdev_prop_set_uint32(dev, "kind-of-intr", 0);
>       sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>       sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, INTC_BASEADDR);
> diff --git a/hw/riscv/microblaze-v-generic.c b/hw/riscv/microblaze-v-generic.c
> index 26788a1824a..ebdd461ae98 100644
> --- a/hw/riscv/microblaze-v-generic.c
> +++ b/hw/riscv/microblaze-v-generic.c
> @@ -79,6 +79,7 @@ static void mb_v_generic_init(MachineState *machine)
>       memory_region_add_subregion(sysmem, ddr_base, phys_ram);
>   
>       dev = qdev_new("xlnx.xps-intc");
> +    qdev_prop_set_enum(dev, "endianness", ENDIAN_MODE_LITTLE);
>       qdev_prop_set_uint32(dev, "kind-of-intr",
>                            1 << UARTLITE_IRQ);
>       sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);