Re: [PATCH v2 14/14] hw/ppc/prep: Fix non-contiguous IO control bit

[Akihiko Odaki <akihiko.odaki@gmail.com>]

On 2025/07/03 7:09, BALATON Zoltan wrote:
> The bit that is supposed to control if ISA IO ports are accessed with
> discontiguous addresses was not connected so it did nothing. We can
> now directly enable or disable the discontiguous region so allow the
> bit to function. This did not cause a problem so far as nothing seems
> to use this bit or discontiguous IO addresses.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>   hw/pci-host/raven.c    |  9 ---------
>   hw/ppc/prep.c          |  3 +++
>   hw/ppc/prep_systemio.c | 14 ++++++++------
>   3 files changed, 11 insertions(+), 15 deletions(-)
> 
> diff --git a/hw/pci-host/raven.c b/hw/pci-host/raven.c
> index 0c4eca04bb..fd45acb7eb 100644
> --- a/hw/pci-host/raven.c
> +++ b/hw/pci-host/raven.c
> @@ -161,13 +161,6 @@ static const PCIIOMMUOps raven_iommu_ops = {
>       .get_address_space = raven_pcihost_set_iommu,
>   };
>   
> -static void raven_change_gpio(void *opaque, int n, int level)
> -{
> -    PREPPCIState *s = opaque;
> -
> -    memory_region_set_enabled(&s->pci_discontiguous_io, !!level);
> -}
> -
>   static void raven_pcihost_realize(DeviceState *d, Error **errp)
>   {
>       SysBusDevice *dev = SYS_BUS_DEVICE(d);
> @@ -176,8 +169,6 @@ static void raven_pcihost_realize(DeviceState *d, Error **errp)
>       Object *o = OBJECT(d);
>       MemoryRegion *mr, *bm, *address_space_mem = get_system_memory();
>   
> -    qdev_init_gpio_in(d, raven_change_gpio, 1);
> -
>       memory_region_init(&s->pci_io, o, "pci-io", 0x3f800000);
>       memory_region_init_io(&s->pci_discontiguous_io, o,
>                             &raven_io_ops, &s->pci_io,
> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
> index 23d0e1eeaa..678682fdd2 100644
> --- a/hw/ppc/prep.c
> +++ b/hw/ppc/prep.c
> @@ -358,6 +358,9 @@ static void ibm_40p_init(MachineState *machine)
>           dev = DEVICE(isa_dev);
>           qdev_prop_set_uint32(dev, "ibm-planar-id", 0xfc);
>           qdev_prop_set_uint32(dev, "equipment", 0xc0);
> +        object_property_set_link(OBJECT(dev), "discontiguous-io",
> +                                 OBJECT(sysbus_mmio_get_region(pcihost, 1)),
> +                                 &error_fatal);
>           isa_realize_and_unref(isa_dev, isa_bus, &error_fatal);
>   
>           dev = DEVICE(pci_create_simple(pci_bus, PCI_DEVFN(1, 0),
> diff --git a/hw/ppc/prep_systemio.c b/hw/ppc/prep_systemio.c
> index 41cd923b94..fe767cc4ac 100644
> --- a/hw/ppc/prep_systemio.c
> +++ b/hw/ppc/prep_systemio.c
> @@ -44,9 +44,10 @@ OBJECT_DECLARE_SIMPLE_TYPE(PrepSystemIoState, PREP_SYSTEMIO)
>   
>   struct PrepSystemIoState {
>       ISADevice parent_obj;
> +
>       MemoryRegion ppc_parity_mem;
> +    MemoryRegion *discontiguous_io;
>   
> -    qemu_irq non_contiguous_io_map_irq;
>       uint8_t sreset; /* 0x0092 */
>       uint8_t equipment; /* 0x080c */
>       uint8_t system_control; /* 0x081c */
> @@ -206,8 +207,8 @@ static void prep_port0850_write(void *opaque, uint32_t addr, uint32_t val)
>       PrepSystemIoState *s = opaque;
>   
>       trace_prep_systemio_write(addr, val);
> -    qemu_set_irq(s->non_contiguous_io_map_irq,
> -                 val & PORT0850_IOMAP_NONCONTIGUOUS);
> +    memory_region_set_enabled(s->discontiguous_io,
> +                              !(val & PORT0850_IOMAP_NONCONTIGUOUS));
>       s->iomap_type = val & PORT0850_IOMAP_NONCONTIGUOUS;
>   }
>   
> @@ -257,10 +258,9 @@ static void prep_systemio_realize(DeviceState *dev, Error **errp)
>       PrepSystemIoState *s = PREP_SYSTEMIO(dev);
>       PowerPCCPU *cpu;
>   
> -    qdev_init_gpio_out(dev, &s->non_contiguous_io_map_irq, 1);
>       s->iomap_type = PORT0850_IOMAP_NONCONTIGUOUS;
> -    qemu_set_irq(s->non_contiguous_io_map_irq,
> -                 s->iomap_type & PORT0850_IOMAP_NONCONTIGUOUS);
> +    memory_region_set_enabled(s->discontiguous_io,
> +                              !(s->iomap_type & PORT0850_IOMAP_NONCONTIGUOUS));

I got an error when running device-crash-test:
https://gitlab.com/akihiko.odaki/qemu/-/jobs/11385415195

Apparently it is necessary to check if s->discontiguous_io is set.

>       cpu = POWERPC_CPU(first_cpu);
>       s->softreset_irq = qdev_get_gpio_in(DEVICE(cpu), PPC6xx_INPUT_HRESET);
>   
> @@ -288,6 +288,8 @@ static const VMStateDescription vmstate_prep_systemio = {
>   static const Property prep_systemio_properties[] = {
>       DEFINE_PROP_UINT8("ibm-planar-id", PrepSystemIoState, ibm_planar_id, 0),
>       DEFINE_PROP_UINT8("equipment", PrepSystemIoState, equipment, 0),
> +    DEFINE_PROP_LINK("discontiguous-io", PrepSystemIoState, discontiguous_io,
> +                     TYPE_MEMORY_REGION, MemoryRegion *),
>   };
>   
>   static void prep_systemio_class_initfn(ObjectClass *klass, const void *data)