| Message ID | 20240207111411.115040-4-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mips: do not list individual devices from configs/ | expand |
On 7/2/24 12:14, Paolo Bonzini wrote: > From: Bernhard Beschow <shentey@gmail.com> > > The board doesn't seem to have an ISA bus at all. > > Signed-off-by: Bernhard Beschow <shentey@gmail.com> > Message-ID: <20230109204124.102592-3-shentey@gmail.com> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > hw/mips/mipssim.c | 1 - > hw/mips/Kconfig | 3 +-- > 2 files changed, 1 insertion(+), 3 deletions(-) > > diff --git a/hw/mips/mipssim.c b/hw/mips/mipssim.c > index 01e323904d9..abbeb6390e1 100644 > --- a/hw/mips/mipssim.c > +++ b/hw/mips/mipssim.c > @@ -31,7 +31,6 @@ > #include "hw/clock.h" > #include "hw/mips/mips.h" > #include "hw/char/serial.h" > -#include "hw/isa/isa.h" > #include "net/net.h" > #include "sysemu/sysemu.h" > #include "hw/boards.h" > diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig > index ab61af209a0..afcfb2b8eca 100644 > --- a/hw/mips/Kconfig > +++ b/hw/mips/Kconfig > @@ -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); /* * A single 16450 sits at offset 0x3f8. It is attached to * MIPS CPU INT2, which is interrupt 4. ---
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. > --- >
On 8/2/24 09:11, Paolo Bonzini wrote: > 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. > + */ Yes, thanks! Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > 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. >> --- >> >
diff --git a/hw/mips/mipssim.c b/hw/mips/mipssim.c index 01e323904d9..abbeb6390e1 100644 --- a/hw/mips/mipssim.c +++ b/hw/mips/mipssim.c @@ -31,7 +31,6 @@ #include "hw/clock.h" #include "hw/mips/mips.h" #include "hw/char/serial.h" -#include "hw/isa/isa.h" #include "net/net.h" #include "sysemu/sysemu.h" #include "hw/boards.h" diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig index ab61af209a0..afcfb2b8eca 100644 --- a/hw/mips/Kconfig +++ b/hw/mips/Kconfig @@ -6,8 +6,7 @@ config MALTA config MIPSSIM bool - select ISA_BUS - select SERIAL_ISA + select SERIAL select MIPSNET config JAZZ