1. Patchew
  2. QEMU
  3. [PATCH v2 0/8] mips: do not list individual devices from configs/
  4. View patch

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

Paolo Bonzini posted 8 patches 2 years ago
Maintainers: "Philippe Mathieu-Daudé" <philmd@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Huacai Chen <chenhuacai@kernel.org>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>, Gerd Hoffmann <kraxel@redhat.com>
There is a newer version of this series
[PATCH v2 3/8] hw/mips/Kconfig: Remove ISA dependencies from MIPSsim board
Posted by Paolo Bonzini 2 years ago 
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
     select MIPSNET
 
 config JAZZ
-- 
2.43.0
Re: [PATCH v2 3/8] hw/mips/Kconfig: Remove ISA dependencies from MIPSsim board
Posted by Philippe Mathieu-Daudé 2 years ago 
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.
---
Re: [PATCH v2 3/8] hw/mips/Kconfig: Remove ISA dependencies from MIPSsim board
Posted by Paolo Bonzini 2 years ago 
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.
> ---
>
Re: [PATCH v2 3/8] hw/mips/Kconfig: Remove ISA dependencies from MIPSsim board
Posted by Philippe Mathieu-Daudé 2 years ago 
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.
>> ---
>>
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.