Re: [PATCH v3 08/18] hw/ide: Introduce generic ide_init_ioport()

[Mark Cave-Ayland]

On 02/03/2023 22:40, Philippe Mathieu-Daudé wrote:

> Add ide_init_ioport() which is not restricted to the ISA bus.
> (Next commit will use it for a PCI device).
>
> Inspired-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/ide/ioport.c           | 12 ++++++++++--
>   include/hw/ide/internal.h |  2 ++
>   2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/hw/ide/ioport.c b/hw/ide/ioport.c
> index d869f8018a..ed7957dbae 100644
> --- a/hw/ide/ioport.c
> +++ b/hw/ide/ioport.c
> @@ -46,8 +46,6 @@ int ide_bus_init_ioport_isa(IDEBus *bus, ISADevice *dev,
>   {
>       int ret;
>   
> -    /* ??? Assume only ISA and PCI configurations, and that the PCI-ISA
> -       bridge has been setup properly to always register with ISA.  */
>       ret = isa_register_portio_list(dev, &bus->portio_list,
>                                      iobase, ide_portio_list, bus, "ide");
>   
> @@ -58,3 +56,13 @@ int ide_bus_init_ioport_isa(IDEBus *bus, ISADevice *dev,
>   
>       return ret;
>   }
> +
> +void ide_bus_init_ioport(IDEBus *bus, Object *owner, MemoryRegion *io,
> +                         int iobase, int iobase2)
> +{
> +    portio_list_init(&bus->portio_list, owner, ide_portio_list, bus, "ide");
> +    portio_list_add(&bus->portio_list, io, iobase);
> +
> +    portio_list_init(&bus->portio2_list, owner, ide_portio2_list, bus, "ide");
> +    portio_list_add(&bus->portio_list, io, iobase2);
> +}
> diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
> index d3b7fdc504..6967ca13e0 100644
> --- a/include/hw/ide/internal.h
> +++ b/include/hw/ide/internal.h
> @@ -617,6 +617,8 @@ int ide_init_drive(IDEState *s, BlockBackend *blk, 
> IDEDriveKind kind,
>                      uint32_t cylinders, uint32_t heads, uint32_t secs,
>                      int chs_trans, Error **errp);
>   void ide_exit(IDEState *s);
> +void ide_bus_init_ioport(IDEBus *bus, Object *owner, MemoryRegion *io,
> +                         int iobase, int iobase2);
>   void ide_bus_init_output_irq(IDEBus *bus, qemu_irq irq_out);
>   void ide_bus_set_irq(IDEBus *bus);
>   void ide_bus_register_restart_cb(IDEBus *bus);

Since I took the opposite approach with regard to the ISA IDE ioports, I have a very 
similar patch locally except that as all the remaining users are PCIDevices, I moved 
the logic into a function called pci_ide_register_legacy_ioports() in hw/ide/pci.c 
(see https://github.com/mcayland/qemu/commit/c2aa28fd6306eeeb60b3ec21be48dd9c8841e20b).

Thoughts?


ATB,

Mark.