Re: [PATCH v2 3/8] hw/mips/Kconfig: Remove ISA dependencies from MIPSsim board

[Paolo Bonzini <pbonzini@redhat.com>]

On Wed, Feb 7, 2024 at 7:58 PM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> > @@ -6,8 +6,7 @@ config MALTA
> >
> >   config MIPSSIM
> >       bool
> > -    select ISA_BUS
> > -    select SERIAL_ISA
> > +    select SERIAL
>
> Hmm there is an ISA bus which can be exposed with:
>
> -- >8 --
> diff --git a/hw/mips/mipssim.c b/hw/mips/mipssim.c
> index 16af31648e..a1a4688861 100644
> --- a/hw/mips/mipssim.c
> +++ b/hw/mips/mipssim.c
> @@ -209,8 +209,9 @@ mips_mipssim_init(MachineState *machine)
>       /* Register 64 KB of ISA IO space at 0x1fd00000. */
>       memory_region_init_alias(isa, NULL, "isa_mmio",
>                                get_system_io(), 0, 0x00010000);
>       memory_region_add_subregion(get_system_memory(), 0x1fd00000, isa);
> +    isa_bus_new(NULL, get_system_memory(), get_system_io(), &error_abort);

Quoting you from
https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg08752.html,
"there is an ISA MMIO space mapped at 0x1fd00000, however this is not
a real ISA bus (no ISA IRQ)".

If mipssim cannot support "-device isa-serial" as a replacement for
"-serial", there's no reason for it to expose the bus. In the end,
-device support is the main thing that an ISA bus provides over
sysbus, and if it cannot work due to the missing interrupts, I think
this patch is correct.

I can add a comment:

diff --git a/hw/mips/mipssim.c b/hw/mips/mipssim.c
index 01e323904d9..47b37b454e7 100644
--- a/hw/mips/mipssim.c
+++ b/hw/mips/mipssim.c
@@ -204,7 +204,11 @@ mips_mipssim_init(MachineState *machine)
     cpu_mips_irq_init_cpu(cpu);
     cpu_mips_clock_init(cpu);

-    /* Register 64 KB of ISA IO space at 0x1fd00000. */
+    /*
+     * Register 64 KB of ISA IO space at 0x1fd00000.  But without interrupts
+     * (except for the hardcoded serial port interrupt) -device cannot work,
+     * so do not expose the ISA bus to the user.
+     */
     memory_region_init_alias(isa, NULL, "isa_mmio",
                              get_system_io(), 0, 0x00010000);
     memory_region_add_subregion(get_system_memory(), 0x1fd00000, isa);

Paolo

>       /*
>        * A single 16450 sits at offset 0x3f8. It is attached to
>        * MIPS CPU INT2, which is interrupt 4.
> ---
>