| Message ID | 20250212112413.37553-3-philmd@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | hw/microblaze: Allow running cross-endian vCPUs | expand |
On 12/02/2025 12.24, 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. > Set the proper endianness for each machine using the device. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > hw/intc/xilinx_intc.c | 60 ++++++++++++++++++------ > 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, 52 insertions(+), 14 deletions(-) > > diff --git a/hw/intc/xilinx_intc.c b/hw/intc/xilinx_intc.c > index 6930f83907a..523402b688c 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,29 @@ 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] = { Hmm, ok, so here we have now an assumption that ENDIAN_MODE_BIG and ENDIAN_MODE_LITTLE match 0 and 1, which would not work anymore when using 0 as unspecified... a little bit ugly ... so maybe instead of changing pic_ops into an array .... > + .read = pic_read, > + .write = pic_write, > + .endianness = DEVICE_BIG_ENDIAN, > + .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 > - } > + [ENDIAN_MODE_BIG].endianness = DEVICE_BIG_ENDIAN, > + [ENDIAN_MODE_LITTLE].endianness = DEVICE_LITTLE_ENDIAN, > }; > > static void irq_handler(void *opaque, int irq, int level) > @@ -174,13 +191,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; > + } ... would it be possible to patch in the right value for pic_ops.endianness here instead? Thomas > + memory_region_init_io(&p->mmio, OBJECT(dev), > + &pic_ops[p->model_endianness], > + 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 +219,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);
On 12/2/25 12:42, Thomas Huth wrote: > On 12/02/2025 12.24, 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. >> Set the proper endianness for each machine using the device. >> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> --- >> hw/intc/xilinx_intc.c | 60 ++++++++++++++++++------ >> 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, 52 insertions(+), 14 deletions(-) >> >> diff --git a/hw/intc/xilinx_intc.c b/hw/intc/xilinx_intc.c >> index 6930f83907a..523402b688c 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,29 @@ 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] = { > > Hmm, ok, so here we have now an assumption that ENDIAN_MODE_BIG and > ENDIAN_MODE_LITTLE match 0 and 1, which would not work anymore when > using 0 as unspecified... a little bit ugly ... so maybe instead of > changing pic_ops into an array .... > >> + .read = pic_read, >> + .write = pic_write, >> + .endianness = DEVICE_BIG_ENDIAN, >> + .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 >> - } >> + [ENDIAN_MODE_BIG].endianness = DEVICE_BIG_ENDIAN, >> + [ENDIAN_MODE_LITTLE].endianness = DEVICE_LITTLE_ENDIAN, >> }; >> static void irq_handler(void *opaque, int irq, int level) >> @@ -174,13 +191,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; >> + } > > ... would it be possible to patch in the right value for pic_ops.endianness > here instead? Ah, clever than my reply on the previous patch. I'll give it a try, thanks! > > Thomas
diff --git a/hw/intc/xilinx_intc.c b/hw/intc/xilinx_intc.c index 6930f83907a..523402b688c 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,29 @@ 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, + .endianness = DEVICE_BIG_ENDIAN, + .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 - } + [ENDIAN_MODE_BIG].endianness = DEVICE_BIG_ENDIAN, + [ENDIAN_MODE_LITTLE].endianness = DEVICE_LITTLE_ENDIAN, }; static void irq_handler(void *opaque, int irq, int level) @@ -174,13 +191,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], + 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 +219,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);
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. Set the proper endianness for each machine using the device. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- hw/intc/xilinx_intc.c | 60 ++++++++++++++++++------ 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, 52 insertions(+), 14 deletions(-)