[PATCH v2 0/8] mips: do not list individual devices from configs/

[PATCH v2 0/8] mips: do not list individual devices from configs/

[Paolo Bonzini]

Back when Kconfig was introduced, the individual dependencies for MIPS
boards were never added to hw/mips/Kconfig.  Do it now.

To simplify the task, include a couple cleanups to the SuperIO chip
configuration symbols, as well as a change that makes USB device
creation available even when building without default devices.

Tested by comparing old and new kconfigs; and also by building each of
the boards one by one, with default devices disabled, and checking that
the board can be started.

Paolo

Based-on: <20240129115809.1039924-1-pbonzini@redhat.com>

v1->v2:
- more cleanup to hw/isa
- include "mips: allow compiling out CONFIG_MIPS_ITU"
- split out patch 3, previously submitted by Bernard
- new patches 4-5 further cleaning up ISA_SUPERIO
- clean up new fdc37m81x-superio.c file
- do not select IDE_ISA for FDC37M81X

Bernhard Beschow (1):
  hw/mips/Kconfig: Remove ISA dependencies from MIPSsim board

Paolo Bonzini (8):
  usb: inline device creation functions
  isa: clean up Kconfig selections for ISA_SUPERIO
  isa: fix ISA_SUPERIO dependencies
  isa: specify instance_size in isa_superio_type_info
  isa: extract FDC37M81X to a separate file
  mips: allow compiling out CONFIG_MIPS_ITU
  mips: do not list individual devices from configs/

 configs/devices/mips-softmmu/common.mak      | 26 ++---------------
 configs/devices/mips64el-softmmu/default.mak |  3 --
 include/hw/usb.h                             | 27 +++++++++++++++--
 hw/isa/fdc37m81x-superio.c                   | 32 ++++++++++++++++++++
 hw/isa/isa-superio.c                         | 18 -----------
 hw/isa/smc37c669-superio.c                   |  1 -
 hw/mips/loongson3_virt.c                     |  5 +--
 hw/mips/mips_itu-stub.c                      | 26 ++++++++++++++++
 hw/mips/mipssim.c                            |  1 -
 hw/usb/bus.c                                 | 23 --------------
 hw/display/Kconfig                           |  2 +-
 hw/isa/Kconfig                               | 20 ++++++------
 hw/isa/meson.build                           |  1 +
 hw/mips/Kconfig                              | 25 ++++++++++++---
 hw/mips/meson.build                          |  1 +
 15 files changed, 122 insertions(+), 89 deletions(-)
 create mode 100644 hw/isa/fdc37m81x-superio.c
 create mode 100644 hw/mips/mips_itu-stub.c

-- 
2.43.0

[PATCH v2 1/8] usb: inline device creation functions

[Paolo Bonzini]

Allow boards to use the device creation functions even if USB itself
is not available; of course the functions will fail inexorably, but
this can be okay if the calls are conditional on the existence of
some USB host controller device.  This is for example the case for
hw/mips/loongson3_virt.c.

Acked-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/hw/usb.h | 27 ++++++++++++++++++++++++---
 hw/usb/bus.c     | 23 -----------------------
 2 files changed, 24 insertions(+), 26 deletions(-)

diff --git a/include/hw/usb.h b/include/hw/usb.h
index 32c23a5ca2a..cfeead28403 100644
--- a/include/hw/usb.h
+++ b/include/hw/usb.h
@@ -30,6 +30,7 @@
 #include "qemu/iov.h"
 #include "qemu/queue.h"
 #include "qom/object.h"
+#include "qapi/error.h"
 
 /* Constants related to the USB / PCI interaction */
 #define USB_SBRN    0x60 /* Serial Bus Release Number Register */
@@ -500,9 +501,6 @@ void usb_bus_release(USBBus *bus);
 USBBus *usb_bus_find(int busnr);
 void usb_legacy_register(const char *typename, const char *usbdevice_name,
                          USBDevice *(*usbdevice_init)(void));
-USBDevice *usb_new(const char *name);
-bool usb_realize_and_unref(USBDevice *dev, USBBus *bus, Error **errp);
-USBDevice *usb_create_simple(USBBus *bus, const char *name);
 USBDevice *usbdevice_create(const char *cmdline);
 void usb_register_port(USBBus *bus, USBPort *port, void *opaque, int index,
                        USBPortOps *ops, int speedmask);
@@ -582,4 +580,27 @@ void usb_pcap_init(FILE *fp);
 void usb_pcap_ctrl(USBPacket *p, bool setup);
 void usb_pcap_data(USBPacket *p, bool setup);
 
+static inline USBDevice *usb_new(const char *name)
+{
+    return USB_DEVICE(qdev_new(name));
+}
+
+static inline USBDevice *usb_try_new(const char *name)
+{
+    return USB_DEVICE(qdev_try_new(name));
+}
+
+static inline bool usb_realize_and_unref(USBDevice *dev, USBBus *bus, Error **errp)
+{
+    return qdev_realize_and_unref(&dev->qdev, &bus->qbus, errp);
+}
+
+static inline USBDevice *usb_create_simple(USBBus *bus, const char *name)
+{
+    USBDevice *dev = usb_new(name);
+
+    usb_realize_and_unref(dev, bus, &error_abort);
+    return dev;
+}
+
 #endif
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index 59c39945ddd..76fda41b7ec 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -329,29 +329,6 @@ void usb_legacy_register(const char *typename, const char *usbdevice_name,
     }
 }
 
-USBDevice *usb_new(const char *name)
-{
-    return USB_DEVICE(qdev_new(name));
-}
-
-static USBDevice *usb_try_new(const char *name)
-{
-    return USB_DEVICE(qdev_try_new(name));
-}
-
-bool usb_realize_and_unref(USBDevice *dev, USBBus *bus, Error **errp)
-{
-    return qdev_realize_and_unref(&dev->qdev, &bus->qbus, errp);
-}
-
-USBDevice *usb_create_simple(USBBus *bus, const char *name)
-{
-    USBDevice *dev = usb_new(name);
-
-    usb_realize_and_unref(dev, bus, &error_abort);
-    return dev;
-}
-
 static void usb_fill_port(USBPort *port, void *opaque, int index,
                           USBPortOps *ops, int speedmask)
 {
-- 
2.43.0

[PATCH v2 2/8] isa: clean up Kconfig selections for ISA_SUPERIO

[Paolo Bonzini]

All users of ISA_SUPERIO include a floppy disk controller, serial port
and parallel port via the automatic creation mechanism of isa-superio.c.

Select the symbol and remove it from the dependents.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/isa/Kconfig | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
index 040a18c0709..7884179d08b 100644
--- a/hw/isa/Kconfig
+++ b/hw/isa/Kconfig
@@ -17,7 +17,11 @@ config ISA_SUPERIO
     bool
     select ISA_BUS
     select PCKBD
+    select PARALLEL
+    select SERIAL_ISA
     select FDC_ISA
+    # Some users of ISA_SUPERIO do not use it
+    #select IDE_ISA
 
 config PC87312
     bool
@@ -26,9 +30,6 @@ config PC87312
     select I8254
     select I8257
     select MC146818RTC
-    select SERIAL_ISA
-    select PARALLEL
-    select FDC_ISA
     select IDE_ISA
 
 config PIIX
@@ -49,8 +50,6 @@ config VT82C686
     select ISA_SUPERIO
     select ACPI
     select ACPI_SMBUS
-    select SERIAL_ISA
-    select FDC_ISA
     select USB_UHCI
     select APM
     select I8254
@@ -58,14 +57,10 @@ config VT82C686
     select I8259
     select IDE_VIA
     select MC146818RTC
-    select PARALLEL
 
 config SMC37C669
     bool
     select ISA_SUPERIO
-    select SERIAL_ISA
-    select PARALLEL
-    select FDC_ISA
 
 config LPC_ICH9
     bool
-- 
2.43.0

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

[Paolo Bonzini]

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

[Philippe Mathieu-Daudé]

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

[Paolo Bonzini]

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

[Philippe Mathieu-Daudé]

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.
>> ---
>>
> 

[PATCH v2 4/8] isa: fix ISA_SUPERIO dependencies

[Paolo Bonzini]

ISA_SUPERIO does not provide an ISA bus, so it should not select the symbol:
instead it requires one.  Among its users, VT82C686 is the only one that
is a PCI-ISA bridge and does not already select ISA_BUS.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/isa/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
index 7884179d08b..5df3c09cd51 100644
--- a/hw/isa/Kconfig
+++ b/hw/isa/Kconfig
@@ -15,7 +15,7 @@ config I82378
 
 config ISA_SUPERIO
     bool
-    select ISA_BUS
+    depends on ISA_BUS
     select PCKBD
     select PARALLEL
     select SERIAL_ISA
@@ -47,6 +47,7 @@ config PIIX
 
 config VT82C686
     bool
+    select ISA_BUS
     select ISA_SUPERIO
     select ACPI
     select ACPI_SMBUS
-- 
2.43.0

Re: [PATCH v2 4/8] isa: fix ISA_SUPERIO dependencies

[BALATON Zoltan]

On Wed, 7 Feb 2024, Paolo Bonzini wrote:
> ISA_SUPERIO does not provide an ISA bus, so it should not select the symbol:
> instead it requires one.  Among its users, VT82C686 is the only one that
> is a PCI-ISA bridge and does not already select ISA_BUS.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>

> ---
> hw/isa/Kconfig | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
> index 7884179d08b..5df3c09cd51 100644
> --- a/hw/isa/Kconfig
> +++ b/hw/isa/Kconfig
> @@ -15,7 +15,7 @@ config I82378
>
> config ISA_SUPERIO
>     bool
> -    select ISA_BUS
> +    depends on ISA_BUS
>     select PCKBD
>     select PARALLEL
>     select SERIAL_ISA
> @@ -47,6 +47,7 @@ config PIIX
>
> config VT82C686
>     bool
> +    select ISA_BUS
>     select ISA_SUPERIO
>     select ACPI
>     select ACPI_SMBUS
>

Re: [PATCH v2 4/8] isa: fix ISA_SUPERIO dependencies

[Philippe Mathieu-Daudé]

On 7/2/24 12:14, Paolo Bonzini wrote:
> ISA_SUPERIO does not provide an ISA bus, so it should not select the symbol:
> instead it requires one.  Among its users, VT82C686 is the only one that
> is a PCI-ISA bridge and does not already select ISA_BUS.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   hw/isa/Kconfig | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v2 4/8] isa: fix ISA_SUPERIO dependencies

[Bernhard Beschow]

Am 7. Februar 2024 11:14:06 UTC schrieb Paolo Bonzini <pbonzini@redhat.com>:
>ISA_SUPERIO does not provide an ISA bus, so it should not select the symbol:
>instead it requires one.  Among its users, VT82C686 is the only one that
>is a PCI-ISA bridge and does not already select ISA_BUS.
>
>Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Bernhard Beschow <shentey@gmail.com>

>---
> hw/isa/Kconfig | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
>index 7884179d08b..5df3c09cd51 100644
>--- a/hw/isa/Kconfig
>+++ b/hw/isa/Kconfig
>@@ -15,7 +15,7 @@ config I82378
> 
> config ISA_SUPERIO
>     bool
>-    select ISA_BUS
>+    depends on ISA_BUS
>     select PCKBD
>     select PARALLEL
>     select SERIAL_ISA
>@@ -47,6 +47,7 @@ config PIIX
> 
> config VT82C686
>     bool
>+    select ISA_BUS
>     select ISA_SUPERIO
>     select ACPI
>     select ACPI_SMBUS

[PATCH v2 5/8] isa: specify instance_size in isa_superio_type_info

[Paolo Bonzini]

Right now all subclasses of TYPE_ISA_SUPERIO have to specify an instance_size,
because the ISASuperIODevice struct adds fields to ISADevice but the type does
not include the increased instance size.  Failure to do so results in an access
past the bounds of struct ISADevice as soon as isa_superio_realize is called.
Fix this by specifying the instance_size already in the superclass.

Cc: Bernhard Beschow <shentey@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/isa/isa-superio.c       | 2 +-
 hw/isa/smc37c669-superio.c | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/isa/isa-superio.c b/hw/isa/isa-superio.c
index e06a548c682..98d50844f71 100644
--- a/hw/isa/isa-superio.c
+++ b/hw/isa/isa-superio.c
@@ -187,6 +187,7 @@ static const TypeInfo isa_superio_type_info = {
     .abstract = true,
     .class_size = sizeof(ISASuperIOClass),
     .class_init = isa_superio_class_init,
+    .instance_size = sizeof(ISASuperIODevice),
 };
 
 /* SMS FDC37M817 Super I/O */
@@ -203,7 +204,6 @@ static void fdc37m81x_class_init(ObjectClass *klass, void *data)
 static const TypeInfo fdc37m81x_type_info = {
     .name          = TYPE_FDC37M81X_SUPERIO,
     .parent        = TYPE_ISA_SUPERIO,
-    .instance_size = sizeof(ISASuperIODevice),
     .class_init    = fdc37m81x_class_init,
 };
 
diff --git a/hw/isa/smc37c669-superio.c b/hw/isa/smc37c669-superio.c
index 388e2ed9371..d2e58c9a895 100644
--- a/hw/isa/smc37c669-superio.c
+++ b/hw/isa/smc37c669-superio.c
@@ -85,7 +85,6 @@ static void smc37c669_class_init(ObjectClass *klass, void *data)
 static const TypeInfo smc37c669_type_info = {
     .name          = TYPE_SMC37C669_SUPERIO,
     .parent        = TYPE_ISA_SUPERIO,
-    .instance_size = sizeof(ISASuperIODevice),
     .class_size    = sizeof(ISASuperIOClass),
     .class_init    = smc37c669_class_init,
 };
-- 
2.43.0

Re: [PATCH v2 5/8] isa: specify instance_size in isa_superio_type_info

[Philippe Mathieu-Daudé]

On 7/2/24 12:14, Paolo Bonzini wrote:
> Right now all subclasses of TYPE_ISA_SUPERIO have to specify an instance_size,
> because the ISASuperIODevice struct adds fields to ISADevice but the type does
> not include the increased instance size.  Failure to do so results in an access
> past the bounds of struct ISADevice as soon as isa_superio_realize is called.
> Fix this by specifying the instance_size already in the superclass.
> 
> Cc: Bernhard Beschow <shentey@gmail.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   hw/isa/isa-superio.c       | 2 +-
>   hw/isa/smc37c669-superio.c | 1 -
>   2 files changed, 1 insertion(+), 2 deletions(-)

Oops, thanks!

Fixes: 4c3119a6e3 ("hw/isa/superio: Factor out the parallel code from 
pc87312.c")
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v2 5/8] isa: specify instance_size in isa_superio_type_info

[Bernhard Beschow]

Am 7. Februar 2024 19:03:30 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>On 7/2/24 12:14, Paolo Bonzini wrote:
>> Right now all subclasses of TYPE_ISA_SUPERIO have to specify an instance_size,
>> because the ISASuperIODevice struct adds fields to ISADevice but the type does
>> not include the increased instance size.  Failure to do so results in an access
>> past the bounds of struct ISADevice as soon as isa_superio_realize is called.
>> Fix this by specifying the instance_size already in the superclass.
>> 
>> Cc: Bernhard Beschow <shentey@gmail.com>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>   hw/isa/isa-superio.c       | 2 +-
>>   hw/isa/smc37c669-superio.c | 1 -
>>   2 files changed, 1 insertion(+), 2 deletions(-)
>
>Oops, thanks!
>
>Fixes: 4c3119a6e3 ("hw/isa/superio: Factor out the parallel code from pc87312.c")
>Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Fixes a latent bug which thankfully isn't materialized at the moment.

Reviewed-by: Bernhard Beschow <shentey@gmail.com>

[PATCH v2 6/8] isa: extract FDC37M81X to a separate file

[Paolo Bonzini]

isa-superio.c currently defines a SuperIO chip that is not used
by any other user of the faile.  Extract the chip to a separate file.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/isa/fdc37m81x-superio.c | 32 ++++++++++++++++++++++++++++++++
 hw/isa/isa-superio.c       | 18 ------------------
 hw/isa/Kconfig             |  4 ++++
 hw/isa/meson.build         |  1 +
 hw/mips/Kconfig            |  2 +-
 5 files changed, 38 insertions(+), 19 deletions(-)
 create mode 100644 hw/isa/fdc37m81x-superio.c

diff --git a/hw/isa/fdc37m81x-superio.c b/hw/isa/fdc37m81x-superio.c
new file mode 100644
index 00000000000..55e91fbca17
--- /dev/null
+++ b/hw/isa/fdc37m81x-superio.c
@@ -0,0 +1,32 @@
+/*
+ * SMS FDC37M817 Super I/O
+ *
+ * Copyright (c) 2018 Philippe Mathieu-Daudé
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "hw/isa/superio.h"
+
+static void fdc37m81x_class_init(ObjectClass *klass, void *data)
+{
+    ISASuperIOClass *sc = ISA_SUPERIO_CLASS(klass);
+
+    sc->serial.count = 2; /* NS16C550A */
+    sc->parallel.count = 1;
+    sc->floppy.count = 1; /* SMSC 82077AA Compatible */
+    sc->ide.count = 0;
+}
+
+static const TypeInfo types[] = {
+    {
+        .name          = TYPE_FDC37M81X_SUPERIO,
+        .parent        = TYPE_ISA_SUPERIO,
+        .class_init    = fdc37m81x_class_init,
+    },
+};
+
+DEFINE_TYPES(types)
diff --git a/hw/isa/isa-superio.c b/hw/isa/isa-superio.c
index 98d50844f71..a8c8c58ef7f 100644
--- a/hw/isa/isa-superio.c
+++ b/hw/isa/isa-superio.c
@@ -190,27 +190,9 @@ static const TypeInfo isa_superio_type_info = {
     .instance_size = sizeof(ISASuperIODevice),
 };
 
-/* SMS FDC37M817 Super I/O */
-static void fdc37m81x_class_init(ObjectClass *klass, void *data)
-{
-    ISASuperIOClass *sc = ISA_SUPERIO_CLASS(klass);
-
-    sc->serial.count = 2; /* NS16C550A */
-    sc->parallel.count = 1;
-    sc->floppy.count = 1; /* SMSC 82077AA Compatible */
-    sc->ide.count = 0;
-}
-
-static const TypeInfo fdc37m81x_type_info = {
-    .name          = TYPE_FDC37M81X_SUPERIO,
-    .parent        = TYPE_ISA_SUPERIO,
-    .class_init    = fdc37m81x_class_init,
-};
-
 static void isa_superio_register_types(void)
 {
     type_register_static(&isa_superio_type_info);
-    type_register_static(&fdc37m81x_type_info);
 }
 
 type_init(isa_superio_register_types)
diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
index 5df3c09cd51..73c6470805c 100644
--- a/hw/isa/Kconfig
+++ b/hw/isa/Kconfig
@@ -23,6 +23,10 @@ config ISA_SUPERIO
     # Some users of ISA_SUPERIO do not use it
     #select IDE_ISA
 
+config FDC37M81X
+    bool
+    select ISA_SUPERIO
+
 config PC87312
     bool
     select ISA_SUPERIO
diff --git a/hw/isa/meson.build b/hw/isa/meson.build
index 2ab99ce0c6b..f650b395071 100644
--- a/hw/isa/meson.build
+++ b/hw/isa/meson.build
@@ -4,6 +4,7 @@ system_ss.add(when: 'CONFIG_ISA_BUS', if_true: files('isa-bus.c'))
 system_ss.add(when: 'CONFIG_ISA_SUPERIO', if_true: files('isa-superio.c'))
 system_ss.add(when: 'CONFIG_PC87312', if_true: files('pc87312.c'))
 system_ss.add(when: 'CONFIG_PIIX', if_true: files('piix.c'))
+system_ss.add(when: 'CONFIG_FDC37M81X', if_true: files('fdc37m81x-superio.c'))
 system_ss.add(when: 'CONFIG_SMC37C669', if_true: files('smc37c669-superio.c'))
 system_ss.add(when: 'CONFIG_VT82C686', if_true: files('vt82c686.c'))
 
diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
index afcfb2b8eca..e57db4f6412 100644
--- a/hw/mips/Kconfig
+++ b/hw/mips/Kconfig
@@ -1,7 +1,7 @@
 config MALTA
     bool
+    select FDC37M81X
     select GT64120
-    select ISA_SUPERIO
     select PIIX
 
 config MIPSSIM
-- 
2.43.0

Re: [PATCH v2 6/8] isa: extract FDC37M81X to a separate file

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 4053 bytes --]

On Wed, 7 Feb 2024, Paolo Bonzini wrote:
> isa-superio.c currently defines a SuperIO chip that is not used
> by any other user of the faile.  Extract the chip to a separate file.

Typo: faile -> file?

Regards,
BALATON Zoltan

> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> hw/isa/fdc37m81x-superio.c | 32 ++++++++++++++++++++++++++++++++
> hw/isa/isa-superio.c       | 18 ------------------
> hw/isa/Kconfig             |  4 ++++
> hw/isa/meson.build         |  1 +
> hw/mips/Kconfig            |  2 +-
> 5 files changed, 38 insertions(+), 19 deletions(-)
> create mode 100644 hw/isa/fdc37m81x-superio.c
>
> diff --git a/hw/isa/fdc37m81x-superio.c b/hw/isa/fdc37m81x-superio.c
> new file mode 100644
> index 00000000000..55e91fbca17
> --- /dev/null
> +++ b/hw/isa/fdc37m81x-superio.c
> @@ -0,0 +1,32 @@
> +/*
> + * SMS FDC37M817 Super I/O
> + *
> + * Copyright (c) 2018 Philippe Mathieu-Daudé
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/isa/superio.h"
> +
> +static void fdc37m81x_class_init(ObjectClass *klass, void *data)
> +{
> +    ISASuperIOClass *sc = ISA_SUPERIO_CLASS(klass);
> +
> +    sc->serial.count = 2; /* NS16C550A */
> +    sc->parallel.count = 1;
> +    sc->floppy.count = 1; /* SMSC 82077AA Compatible */
> +    sc->ide.count = 0;
> +}
> +
> +static const TypeInfo types[] = {
> +    {
> +        .name          = TYPE_FDC37M81X_SUPERIO,
> +        .parent        = TYPE_ISA_SUPERIO,
> +        .class_init    = fdc37m81x_class_init,
> +    },
> +};
> +
> +DEFINE_TYPES(types)
> diff --git a/hw/isa/isa-superio.c b/hw/isa/isa-superio.c
> index 98d50844f71..a8c8c58ef7f 100644
> --- a/hw/isa/isa-superio.c
> +++ b/hw/isa/isa-superio.c
> @@ -190,27 +190,9 @@ static const TypeInfo isa_superio_type_info = {
>     .instance_size = sizeof(ISASuperIODevice),
> };
>
> -/* SMS FDC37M817 Super I/O */
> -static void fdc37m81x_class_init(ObjectClass *klass, void *data)
> -{
> -    ISASuperIOClass *sc = ISA_SUPERIO_CLASS(klass);
> -
> -    sc->serial.count = 2; /* NS16C550A */
> -    sc->parallel.count = 1;
> -    sc->floppy.count = 1; /* SMSC 82077AA Compatible */
> -    sc->ide.count = 0;
> -}
> -
> -static const TypeInfo fdc37m81x_type_info = {
> -    .name          = TYPE_FDC37M81X_SUPERIO,
> -    .parent        = TYPE_ISA_SUPERIO,
> -    .class_init    = fdc37m81x_class_init,
> -};
> -
> static void isa_superio_register_types(void)
> {
>     type_register_static(&isa_superio_type_info);
> -    type_register_static(&fdc37m81x_type_info);
> }
>
> type_init(isa_superio_register_types)
> diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
> index 5df3c09cd51..73c6470805c 100644
> --- a/hw/isa/Kconfig
> +++ b/hw/isa/Kconfig
> @@ -23,6 +23,10 @@ config ISA_SUPERIO
>     # Some users of ISA_SUPERIO do not use it
>     #select IDE_ISA
>
> +config FDC37M81X
> +    bool
> +    select ISA_SUPERIO
> +
> config PC87312
>     bool
>     select ISA_SUPERIO
> diff --git a/hw/isa/meson.build b/hw/isa/meson.build
> index 2ab99ce0c6b..f650b395071 100644
> --- a/hw/isa/meson.build
> +++ b/hw/isa/meson.build
> @@ -4,6 +4,7 @@ system_ss.add(when: 'CONFIG_ISA_BUS', if_true: files('isa-bus.c'))
> system_ss.add(when: 'CONFIG_ISA_SUPERIO', if_true: files('isa-superio.c'))
> system_ss.add(when: 'CONFIG_PC87312', if_true: files('pc87312.c'))
> system_ss.add(when: 'CONFIG_PIIX', if_true: files('piix.c'))
> +system_ss.add(when: 'CONFIG_FDC37M81X', if_true: files('fdc37m81x-superio.c'))
> system_ss.add(when: 'CONFIG_SMC37C669', if_true: files('smc37c669-superio.c'))
> system_ss.add(when: 'CONFIG_VT82C686', if_true: files('vt82c686.c'))
>
> diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
> index afcfb2b8eca..e57db4f6412 100644
> --- a/hw/mips/Kconfig
> +++ b/hw/mips/Kconfig
> @@ -1,7 +1,7 @@
> config MALTA
>     bool
> +    select FDC37M81X
>     select GT64120
> -    select ISA_SUPERIO
>     select PIIX
>
> config MIPSSIM
>

Re: [PATCH v2 6/8] isa: extract FDC37M81X to a separate file

[Bernhard Beschow]

Am 7. Februar 2024 11:14:08 UTC schrieb Paolo Bonzini <pbonzini@redhat.com>:
>isa-superio.c currently defines a SuperIO chip that is not used
>by any other user of the faile.  Extract the chip to a separate file.
>
>Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Bernhard Beschow <shentey@gmail.com>

[PATCH v2 7/8] mips: allow compiling out CONFIG_MIPS_ITU

[Paolo Bonzini]

itc_reconfigure() is referenced from TCG, provide a stub if needed.
This makes it possible to build a QEMU binary that only includes
boards without a CPS device (only Malta and Boston create one).

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/mips/mips_itu-stub.c | 26 ++++++++++++++++++++++++++
 hw/mips/meson.build     |  1 +
 2 files changed, 27 insertions(+)
 create mode 100644 hw/mips/mips_itu-stub.c

diff --git a/hw/mips/mips_itu-stub.c b/hw/mips/mips_itu-stub.c
new file mode 100644
index 00000000000..4cc82b8461f
--- /dev/null
+++ b/hw/mips/mips_itu-stub.c
@@ -0,0 +1,26 @@
+/*
+ * Inter-Thread Communication Unit emulation.
+ *
+ * Copyright (c) 2016 Imagination Technologies
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/misc/mips_itu.h"
+
+void itc_reconfigure(MIPSITUState *tag)
+{
+    abort();
+}
diff --git a/hw/mips/meson.build b/hw/mips/meson.build
index f06d88f3430..2b1b96147a6 100644
--- a/hw/mips/meson.build
+++ b/hw/mips/meson.build
@@ -4,6 +4,7 @@ mips_ss.add(when: 'CONFIG_FW_CFG_MIPS', if_true: files('fw_cfg.c'))
 mips_ss.add(when: 'CONFIG_LOONGSON3V', if_true: files('loongson3_bootp.c', 'loongson3_virt.c'))
 mips_ss.add(when: 'CONFIG_MALTA', if_true: files('malta.c'))
 mips_ss.add(when: 'CONFIG_MIPS_CPS', if_true: files('cps.c'))
+mips_ss.add(when: 'CONFIG_MIPS_ITU', if_false: files('mips_itu-stub.c'))
 
 if 'CONFIG_TCG' in config_all_accel
 mips_ss.add(when: 'CONFIG_JAZZ', if_true: files('jazz.c'))
-- 
2.43.0

Re: [PATCH v2 7/8] mips: allow compiling out CONFIG_MIPS_ITU

[BALATON Zoltan]

On Wed, 7 Feb 2024, Paolo Bonzini wrote:
> itc_reconfigure() is referenced from TCG, provide a stub if needed.
> This makes it possible to build a QEMU binary that only includes
> boards without a CPS device (only Malta and Boston create one).
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> hw/mips/mips_itu-stub.c | 26 ++++++++++++++++++++++++++
> hw/mips/meson.build     |  1 +
> 2 files changed, 27 insertions(+)
> create mode 100644 hw/mips/mips_itu-stub.c
>
> diff --git a/hw/mips/mips_itu-stub.c b/hw/mips/mips_itu-stub.c
> new file mode 100644
> index 00000000000..4cc82b8461f
> --- /dev/null
> +++ b/hw/mips/mips_itu-stub.c
> @@ -0,0 +1,26 @@
> +/*
> + * Inter-Thread Communication Unit emulation.
> + *
> + * Copyright (c) 2016 Imagination Technologies
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/misc/mips_itu.h"
> +
> +void itc_reconfigure(MIPSITUState *tag)
> +{
> +    abort();

Isn't g_assert_not_reached() preferred now for it giving better 
dignostics? But I don't know, just asking.

Regards,
BALATON Zoltan

> +}
> diff --git a/hw/mips/meson.build b/hw/mips/meson.build
> index f06d88f3430..2b1b96147a6 100644
> --- a/hw/mips/meson.build
> +++ b/hw/mips/meson.build
> @@ -4,6 +4,7 @@ mips_ss.add(when: 'CONFIG_FW_CFG_MIPS', if_true: files('fw_cfg.c'))
> mips_ss.add(when: 'CONFIG_LOONGSON3V', if_true: files('loongson3_bootp.c', 'loongson3_virt.c'))
> mips_ss.add(when: 'CONFIG_MALTA', if_true: files('malta.c'))
> mips_ss.add(when: 'CONFIG_MIPS_CPS', if_true: files('cps.c'))
> +mips_ss.add(when: 'CONFIG_MIPS_ITU', if_false: files('mips_itu-stub.c'))
>
> if 'CONFIG_TCG' in config_all_accel
> mips_ss.add(when: 'CONFIG_JAZZ', if_true: files('jazz.c'))
>

Re: [PATCH v2 7/8] mips: allow compiling out CONFIG_MIPS_ITU

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 2644 bytes --]

Il mer 7 feb 2024, 14:16 BALATON Zoltan <balaton@eik.bme.hu> ha scritto:

> On Wed, 7 Feb 2024, Paolo Bonzini wrote:
> > itc_reconfigure() is referenced from TCG, provide a stub if needed.
> > This makes it possible to build a QEMU binary that only includes
> > boards without a CPS device (only Malta and Boston create one).
> >
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> > hw/mips/mips_itu-stub.c | 26 ++++++++++++++++++++++++++
> > hw/mips/meson.build     |  1 +
> > 2 files changed, 27 insertions(+)
> > create mode 100644 hw/mips/mips_itu-stub.c
> >
> > diff --git a/hw/mips/mips_itu-stub.c b/hw/mips/mips_itu-stub.c
> > new file mode 100644
> > index 00000000000..4cc82b8461f
> > --- /dev/null
> > +++ b/hw/mips/mips_itu-stub.c
> > @@ -0,0 +1,26 @@
> > +/*
> > + * Inter-Thread Communication Unit emulation.
> > + *
> > + * Copyright (c) 2016 Imagination Technologies
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, see <
> http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "hw/misc/mips_itu.h"
> > +
> > +void itc_reconfigure(MIPSITUState *tag)
> > +{
> > +    abort();
>
> Isn't g_assert_not_reached() preferred now for it giving better
> dignostics? But I don't know, just asking.
>

Sure, changed.

Paolo


> Regards,
> BALATON Zoltan
>
> > +}
> > diff --git a/hw/mips/meson.build b/hw/mips/meson.build
> > index f06d88f3430..2b1b96147a6 100644
> > --- a/hw/mips/meson.build
> > +++ b/hw/mips/meson.build
> > @@ -4,6 +4,7 @@ mips_ss.add(when: 'CONFIG_FW_CFG_MIPS', if_true:
> files('fw_cfg.c'))
> > mips_ss.add(when: 'CONFIG_LOONGSON3V', if_true:
> files('loongson3_bootp.c', 'loongson3_virt.c'))
> > mips_ss.add(when: 'CONFIG_MALTA', if_true: files('malta.c'))
> > mips_ss.add(when: 'CONFIG_MIPS_CPS', if_true: files('cps.c'))
> > +mips_ss.add(when: 'CONFIG_MIPS_ITU', if_false: files('mips_itu-stub.c'))
> >
> > if 'CONFIG_TCG' in config_all_accel
> > mips_ss.add(when: 'CONFIG_JAZZ', if_true: files('jazz.c'))
> >
>
>

[-- Attachment #2: Type: text/html, Size: 3884 bytes --]

Re: [PATCH v2 7/8] mips: allow compiling out CONFIG_MIPS_ITU

[Philippe Mathieu-Daudé]

Hi Paolo,

On 7/2/24 12:14, Paolo Bonzini wrote:
> itc_reconfigure() is referenced from TCG, provide a stub if needed.
> This makes it possible to build a QEMU binary that only includes
> boards without a CPS device (only Malta and Boston create one).
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   hw/mips/mips_itu-stub.c | 26 ++++++++++++++++++++++++++
>   hw/mips/meson.build     |  1 +
>   2 files changed, 27 insertions(+)
>   create mode 100644 hw/mips/mips_itu-stub.c
> 
> diff --git a/hw/mips/mips_itu-stub.c b/hw/mips/mips_itu-stub.c
> new file mode 100644
> index 00000000000..4cc82b8461f
> --- /dev/null
> +++ b/hw/mips/mips_itu-stub.c
> @@ -0,0 +1,26 @@
> +/*
> + * Inter-Thread Communication Unit emulation.
> + *
> + * Copyright (c) 2016 Imagination Technologies
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.

This is your code addition, so "Inter-Thread Communication Unit stubs" /
RH / SPDX GPLv2-or-later.

> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/misc/mips_itu.h"
> +
> +void itc_reconfigure(MIPSITUState *tag)
> +{
> +    abort();

As Zoltan suggested, g_assert_not_reached(). Indeed this path
can't be reached without ITU, TCG won't emit anything and will
call instead:

     qemu_log_mask(LOG_UNIMP, "mthc0 %s (reg %d sel %d)\n",
                   register_name, reg, sel);

I'm reluctant to add stubs, but since it helps you (hoping we
can figure a clean way to split architectural access to hw/ from
tcg/ one day):

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Regards,

Phil.

Re: [PATCH v2 7/8] mips: allow compiling out CONFIG_MIPS_ITU

[Paolo Bonzini]

On Wed, Feb 7, 2024 at 8:12 PM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Hi Paolo,
>
> On 7/2/24 12:14, Paolo Bonzini wrote:
> > itc_reconfigure() is referenced from TCG, provide a stub if needed.
> > This makes it possible to build a QEMU binary that only includes
> > boards without a CPS device (only Malta and Boston create one).
> >
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> >   hw/mips/mips_itu-stub.c | 26 ++++++++++++++++++++++++++
> >   hw/mips/meson.build     |  1 +
> >   2 files changed, 27 insertions(+)
> >   create mode 100644 hw/mips/mips_itu-stub.c
> >
> > diff --git a/hw/mips/mips_itu-stub.c b/hw/mips/mips_itu-stub.c
> > new file mode 100644
> > index 00000000000..4cc82b8461f
> > --- /dev/null
> > +++ b/hw/mips/mips_itu-stub.c
> > @@ -0,0 +1,26 @@
> > +/*
> > + * Inter-Thread Communication Unit emulation.
> > + *
> > + * Copyright (c) 2016 Imagination Technologies
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
>
> This is your code addition, so "Inter-Thread Communication Unit stubs" /
> RH / SPDX GPLv2-or-later.
>
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "hw/misc/mips_itu.h"
> > +
> > +void itc_reconfigure(MIPSITUState *tag)
> > +{
> > +    abort();
>
> As Zoltan suggested, g_assert_not_reached(). Indeed this path
> can't be reached without ITU, TCG won't emit anything and will
> call instead:
>
>      qemu_log_mask(LOG_UNIMP, "mthc0 %s (reg %d sel %d)\n",
>                    register_name, reg, sel);
>
> I'm reluctant to add stubs, but since it helps you (hoping we
> can figure a clean way to split architectural access to hw/ from
> tcg/ one day):

Not sure how it's reached anyway, because the saar field of
DisasContext is never written...

Paolo

Re: [PATCH v2 7/8] mips: allow compiling out CONFIG_MIPS_ITU

[Philippe Mathieu-Daudé]

On 8/2/24 09:26, Paolo Bonzini wrote:
> On Wed, Feb 7, 2024 at 8:12 PM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> Hi Paolo,
>>
>> On 7/2/24 12:14, Paolo Bonzini wrote:
>>> itc_reconfigure() is referenced from TCG, provide a stub if needed.
>>> This makes it possible to build a QEMU binary that only includes
>>> boards without a CPS device (only Malta and Boston create one).
>>>
>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>> ---
>>>    hw/mips/mips_itu-stub.c | 26 ++++++++++++++++++++++++++
>>>    hw/mips/meson.build     |  1 +
>>>    2 files changed, 27 insertions(+)
>>>    create mode 100644 hw/mips/mips_itu-stub.c
>>>
>>> diff --git a/hw/mips/mips_itu-stub.c b/hw/mips/mips_itu-stub.c
>>> new file mode 100644
>>> index 00000000000..4cc82b8461f
>>> --- /dev/null
>>> +++ b/hw/mips/mips_itu-stub.c
>>> @@ -0,0 +1,26 @@
>>> +/*
>>> + * Inter-Thread Communication Unit emulation.
>>> + *
>>> + * Copyright (c) 2016 Imagination Technologies
>>> + *
>>> + * This library is free software; you can redistribute it and/or
>>> + * modify it under the terms of the GNU Lesser General Public
>>> + * License as published by the Free Software Foundation; either
>>> + * version 2.1 of the License, or (at your option) any later version.
>>> + *
>>> + * This library is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>> + * Lesser General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU Lesser General Public
>>> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
>>
>> This is your code addition, so "Inter-Thread Communication Unit stubs" /
>> RH / SPDX GPLv2-or-later.
>>
>>> + */
>>> +
>>> +#include "qemu/osdep.h"
>>> +#include "hw/misc/mips_itu.h"
>>> +
>>> +void itc_reconfigure(MIPSITUState *tag)
>>> +{
>>> +    abort();
>>
>> As Zoltan suggested, g_assert_not_reached(). Indeed this path
>> can't be reached without ITU, TCG won't emit anything and will
>> call instead:
>>
>>       qemu_log_mask(LOG_UNIMP, "mthc0 %s (reg %d sel %d)\n",
>>                     register_name, reg, sel);
>>
>> I'm reluctant to add stubs, but since it helps you (hoping we
>> can figure a clean way to split architectural access to hw/ from
>> tcg/ one day):
> 
> Not sure how it's reached anyway, because the saar field of
> DisasContext is never written...

Commit 5fb2dcd179 ("target/mips: Provide R/W access to SAARI
and SAAR CP0 registers") is an incomplete frontport of
https://github.com/MIPS/qemu/commit/c9340491cc.

Commit 043715d1e0 ("target/mips: Update ITU to utilize SAARI
and SAAR CP0 registers") is an incomplete frontport of
https://github.com/MIPS/qemu/commit/e03079c699.

So this feature is working in MIPS fork since 2017, and during
2019 mainstream only got part of it merged but never completed.

5 years passed.

Re: [PATCH v2 7/8] mips: allow compiling out CONFIG_MIPS_ITU

[Philippe Mathieu-Daudé]

On 9/2/24 08:22, Philippe Mathieu-Daudé wrote:
> On 8/2/24 09:26, Paolo Bonzini wrote:
>> On Wed, Feb 7, 2024 at 8:12 PM Philippe Mathieu-Daudé 
>> <philmd@linaro.org> wrote:
>>>
>>> Hi Paolo,
>>>
>>> On 7/2/24 12:14, Paolo Bonzini wrote:
>>>> itc_reconfigure() is referenced from TCG, provide a stub if needed.
>>>> This makes it possible to build a QEMU binary that only includes
>>>> boards without a CPS device (only Malta and Boston create one).
>>>>
>>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>>> ---
>>>>    hw/mips/mips_itu-stub.c | 26 ++++++++++++++++++++++++++
>>>>    hw/mips/meson.build     |  1 +
>>>>    2 files changed, 27 insertions(+)
>>>>    create mode 100644 hw/mips/mips_itu-stub.c
>>>>
>>>> diff --git a/hw/mips/mips_itu-stub.c b/hw/mips/mips_itu-stub.c
>>>> new file mode 100644
>>>> index 00000000000..4cc82b8461f
>>>> --- /dev/null
>>>> +++ b/hw/mips/mips_itu-stub.c
>>>> @@ -0,0 +1,26 @@
>>>> +/*
>>>> + * Inter-Thread Communication Unit emulation.
>>>> + *
>>>> + * Copyright (c) 2016 Imagination Technologies
>>>> + *
>>>> + * This library is free software; you can redistribute it and/or
>>>> + * modify it under the terms of the GNU Lesser General Public
>>>> + * License as published by the Free Software Foundation; either
>>>> + * version 2.1 of the License, or (at your option) any later version.
>>>> + *
>>>> + * This library is distributed in the hope that it will be useful,
>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>>> + * Lesser General Public License for more details.
>>>> + *
>>>> + * You should have received a copy of the GNU Lesser General Public
>>>> + * License along with this library; if not, see 
>>>> <http://www.gnu.org/licenses/>.
>>>
>>> This is your code addition, so "Inter-Thread Communication Unit stubs" /
>>> RH / SPDX GPLv2-or-later.
>>>
>>>> + */
>>>> +
>>>> +#include "qemu/osdep.h"
>>>> +#include "hw/misc/mips_itu.h"
>>>> +
>>>> +void itc_reconfigure(MIPSITUState *tag)
>>>> +{
>>>> +    abort();
>>>
>>> As Zoltan suggested, g_assert_not_reached(). Indeed this path
>>> can't be reached without ITU, TCG won't emit anything and will
>>> call instead:
>>>
>>>       qemu_log_mask(LOG_UNIMP, "mthc0 %s (reg %d sel %d)\n",
>>>                     register_name, reg, sel);
>>>
>>> I'm reluctant to add stubs, but since it helps you (hoping we
>>> can figure a clean way to split architectural access to hw/ from
>>> tcg/ one day):
>>
>> Not sure how it's reached anyway, because the saar field of
>> DisasContext is never written...

Alternatively:
https://lore.kernel.org/qemu-devel/20240209090513.9401-3-philmd@linaro.org/

> Commit 5fb2dcd179 ("target/mips: Provide R/W access to SAARI
> and SAAR CP0 registers") is an incomplete frontport of
> https://github.com/MIPS/qemu/commit/c9340491cc.
> 
> Commit 043715d1e0 ("target/mips: Update ITU to utilize SAARI
> and SAAR CP0 registers") is an incomplete frontport of
> https://github.com/MIPS/qemu/commit/e03079c699.
> 
> So this feature is working in MIPS fork since 2017, and during
> 2019 mainstream only got part of it merged but never completed.
> 
> 5 years passed.

[PATCH v2 8/8] mips: do not list individual devices from configs/

[Paolo Bonzini]

Add new "select" and "imply" directives if needed.  The resulting
config-devices.mak files are the same as before.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 configs/devices/mips-softmmu/common.mak      | 28 +++-----------------
 configs/devices/mips64el-softmmu/default.mak |  3 ---
 hw/mips/loongson3_virt.c                     |  5 ++--
 hw/display/Kconfig                           |  2 +-
 hw/mips/Kconfig                              | 20 +++++++++++++-
 5 files changed, 27 insertions(+), 31 deletions(-)

diff --git a/configs/devices/mips-softmmu/common.mak b/configs/devices/mips-softmmu/common.mak
index 1a853841b27..416a5d353e8 100644
--- a/configs/devices/mips-softmmu/common.mak
+++ b/configs/devices/mips-softmmu/common.mak
@@ -1,28 +1,8 @@
 # Common mips*-softmmu CONFIG defines
 
-CONFIG_ISA_BUS=y
-CONFIG_PCI=y
-CONFIG_PCI_DEVICES=y
-CONFIG_VGA_ISA=y
-CONFIG_VGA_MMIO=y
-CONFIG_VGA_CIRRUS=y
-CONFIG_VMWARE_VGA=y
-CONFIG_SERIAL=y
-CONFIG_SERIAL_ISA=y
-CONFIG_PARALLEL=y
-CONFIG_I8254=y
-CONFIG_PCSPK=y
-CONFIG_PCKBD=y
-CONFIG_FDC=y
-CONFIG_I8257=y
-CONFIG_IDE_ISA=y
-CONFIG_PFLASH_CFI01=y
-CONFIG_I8259=y
-CONFIG_MC146818RTC=y
-CONFIG_MIPS_CPS=y
-CONFIG_MIPS_ITU=y
+# Uncomment the following lines to disable these optional devices:
+# CONFIG_PCI_DEVICES=n
+# CONFIG_TEST_DEVICES=n
+
 CONFIG_MALTA=y
-CONFIG_PCNET_PCI=y
 CONFIG_MIPSSIM=y
-CONFIG_SMBUS_EEPROM=y
-CONFIG_TEST_DEVICES=y
diff --git a/configs/devices/mips64el-softmmu/default.mak b/configs/devices/mips64el-softmmu/default.mak
index d5188f7ea58..88a37cf27f1 100644
--- a/configs/devices/mips64el-softmmu/default.mak
+++ b/configs/devices/mips64el-softmmu/default.mak
@@ -3,8 +3,5 @@
 include ../mips-softmmu/common.mak
 CONFIG_FULOONG=y
 CONFIG_LOONGSON3V=y
-CONFIG_ATI_VGA=y
-CONFIG_RTL8139_PCI=y
 CONFIG_JAZZ=y
-CONFIG_VT82C686=y
 CONFIG_MIPS_BOSTON=y
diff --git a/hw/mips/loongson3_virt.c b/hw/mips/loongson3_virt.c
index 33eae01eca2..da47af2fa71 100644
--- a/hw/mips/loongson3_virt.c
+++ b/hw/mips/loongson3_virt.c
@@ -447,8 +447,9 @@ static inline void loongson3_virt_devices_init(MachineState *machine,
 
     if (defaults_enabled() && object_class_by_name("pci-ohci")) {
         pci_create_simple(pci_bus, -1, "pci-ohci");
-        usb_create_simple(usb_bus_find(-1), "usb-kbd");
-        usb_create_simple(usb_bus_find(-1), "usb-tablet");
+        Object *usb_bus = object_resolve_path_type("", TYPE_USB_BUS, NULL);
+        usb_create_simple(USB_BUS(usb_bus), "usb-kbd");
+        usb_create_simple(USB_BUS(usb_bus), "usb-tablet");
     }
 
     for (i = 0; i < nb_nics; i++) {
diff --git a/hw/display/Kconfig b/hw/display/Kconfig
index 1aafe1923d2..5b2b3840f7a 100644
--- a/hw/display/Kconfig
+++ b/hw/display/Kconfig
@@ -55,7 +55,7 @@ config VGA_MMIO
 
 config VMWARE_VGA
     bool
-    default y if PCI_DEVICES && PC_PCI
+    default y if PCI_DEVICES && (PC_PCI || MIPS)
     depends on PCI
     select VGA
 
diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
index e57db4f6412..5c83ef49cf6 100644
--- a/hw/mips/Kconfig
+++ b/hw/mips/Kconfig
@@ -1,8 +1,15 @@
 config MALTA
     bool
+    imply PCNET_PCI
+    imply PCI_DEVICES
+    imply TEST_DEVICES
     select FDC37M81X
     select GT64120
+    select MIPS_CPS
     select PIIX
+    select PFLASH_CFI01
+    select SERIAL
+    select SMBUS_EEPROM
 
 config MIPSSIM
     bool
@@ -31,17 +38,26 @@ config JAZZ
 
 config FULOONG
     bool
+    imply PCI_DEVICES
+    imply TEST_DEVICES
+    imply ATI_VGA
+    imply RTL8139_PCI
     select PCI_BONITO
+    select SMBUS_EEPROM
     select VT82C686
 
 config LOONGSON3V
     bool
+    imply PCI_DEVICES
+    imply TEST_DEVICES
+    imply VIRTIO_PCI
+    imply VIRTIO_NET
     imply VIRTIO_VGA
     imply QXL if SPICE
+    imply USB_OHCI_PCI
     select SERIAL
     select GOLDFISH_RTC
     select LOONGSON_LIOINTC
-    select PCI_DEVICES
     select PCI_EXPRESS_GENERIC_BRIDGE
     select MSI_NONBROKEN
     select FW_CFG_MIPS
@@ -53,6 +69,8 @@ config MIPS_CPS
 
 config MIPS_BOSTON
     bool
+    imply PCI_DEVICES
+    imply TEST_DEVICES
     select FITLOADER
     select MIPS_CPS
     select PCI_EXPRESS_XILINX
-- 
2.43.0

Re: [PATCH v2 8/8] mips: do not list individual devices from configs/

[BALATON Zoltan]

On Wed, 7 Feb 2024, Paolo Bonzini wrote:
> Add new "select" and "imply" directives if needed.  The resulting
> config-devices.mak files are the same as before.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> configs/devices/mips-softmmu/common.mak      | 28 +++-----------------
> configs/devices/mips64el-softmmu/default.mak |  3 ---
> hw/mips/loongson3_virt.c                     |  5 ++--
> hw/display/Kconfig                           |  2 +-
> hw/mips/Kconfig                              | 20 +++++++++++++-
> 5 files changed, 27 insertions(+), 31 deletions(-)
>
> diff --git a/configs/devices/mips-softmmu/common.mak b/configs/devices/mips-softmmu/common.mak
> index 1a853841b27..416a5d353e8 100644
> --- a/configs/devices/mips-softmmu/common.mak
> +++ b/configs/devices/mips-softmmu/common.mak
> @@ -1,28 +1,8 @@
> # Common mips*-softmmu CONFIG defines
>
> -CONFIG_ISA_BUS=y
> -CONFIG_PCI=y
> -CONFIG_PCI_DEVICES=y
> -CONFIG_VGA_ISA=y
> -CONFIG_VGA_MMIO=y
> -CONFIG_VGA_CIRRUS=y
> -CONFIG_VMWARE_VGA=y
> -CONFIG_SERIAL=y
> -CONFIG_SERIAL_ISA=y
> -CONFIG_PARALLEL=y
> -CONFIG_I8254=y
> -CONFIG_PCSPK=y
> -CONFIG_PCKBD=y
> -CONFIG_FDC=y
> -CONFIG_I8257=y
> -CONFIG_IDE_ISA=y
> -CONFIG_PFLASH_CFI01=y
> -CONFIG_I8259=y
> -CONFIG_MC146818RTC=y
> -CONFIG_MIPS_CPS=y
> -CONFIG_MIPS_ITU=y
> +# Uncomment the following lines to disable these optional devices:
> +# CONFIG_PCI_DEVICES=n
> +# CONFIG_TEST_DEVICES=n
> +
> CONFIG_MALTA=y
> -CONFIG_PCNET_PCI=y
> CONFIG_MIPSSIM=y
> -CONFIG_SMBUS_EEPROM=y
> -CONFIG_TEST_DEVICES=y
> diff --git a/configs/devices/mips64el-softmmu/default.mak b/configs/devices/mips64el-softmmu/default.mak
> index d5188f7ea58..88a37cf27f1 100644
> --- a/configs/devices/mips64el-softmmu/default.mak
> +++ b/configs/devices/mips64el-softmmu/default.mak
> @@ -3,8 +3,5 @@
> include ../mips-softmmu/common.mak
> CONFIG_FULOONG=y
> CONFIG_LOONGSON3V=y
> -CONFIG_ATI_VGA=y
> -CONFIG_RTL8139_PCI=y
> CONFIG_JAZZ=y
> -CONFIG_VT82C686=y
> CONFIG_MIPS_BOSTON=y
> diff --git a/hw/mips/loongson3_virt.c b/hw/mips/loongson3_virt.c
> index 33eae01eca2..da47af2fa71 100644
> --- a/hw/mips/loongson3_virt.c
> +++ b/hw/mips/loongson3_virt.c
> @@ -447,8 +447,9 @@ static inline void loongson3_virt_devices_init(MachineState *machine,
>
>     if (defaults_enabled() && object_class_by_name("pci-ohci")) {
>         pci_create_simple(pci_bus, -1, "pci-ohci");
> -        usb_create_simple(usb_bus_find(-1), "usb-kbd");
> -        usb_create_simple(usb_bus_find(-1), "usb-tablet");
> +        Object *usb_bus = object_resolve_path_type("", TYPE_USB_BUS, NULL);
> +        usb_create_simple(USB_BUS(usb_bus), "usb-kbd");
> +        usb_create_simple(USB_BUS(usb_bus), "usb-tablet");
>     }
>
>     for (i = 0; i < nb_nics; i++) {

Is this hunk supposed to be in this patch?

Regards,
BALATON Zoltan

> diff --git a/hw/display/Kconfig b/hw/display/Kconfig
> index 1aafe1923d2..5b2b3840f7a 100644
> --- a/hw/display/Kconfig
> +++ b/hw/display/Kconfig
> @@ -55,7 +55,7 @@ config VGA_MMIO
>
> config VMWARE_VGA
>     bool
> -    default y if PCI_DEVICES && PC_PCI
> +    default y if PCI_DEVICES && (PC_PCI || MIPS)
>     depends on PCI
>     select VGA
>
> diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
> index e57db4f6412..5c83ef49cf6 100644
> --- a/hw/mips/Kconfig
> +++ b/hw/mips/Kconfig
> @@ -1,8 +1,15 @@
> config MALTA
>     bool
> +    imply PCNET_PCI
> +    imply PCI_DEVICES
> +    imply TEST_DEVICES
>     select FDC37M81X
>     select GT64120
> +    select MIPS_CPS
>     select PIIX
> +    select PFLASH_CFI01
> +    select SERIAL
> +    select SMBUS_EEPROM
>
> config MIPSSIM
>     bool
> @@ -31,17 +38,26 @@ config JAZZ
>
> config FULOONG
>     bool
> +    imply PCI_DEVICES
> +    imply TEST_DEVICES
> +    imply ATI_VGA
> +    imply RTL8139_PCI
>     select PCI_BONITO
> +    select SMBUS_EEPROM
>     select VT82C686
>
> config LOONGSON3V
>     bool
> +    imply PCI_DEVICES
> +    imply TEST_DEVICES
> +    imply VIRTIO_PCI
> +    imply VIRTIO_NET
>     imply VIRTIO_VGA
>     imply QXL if SPICE
> +    imply USB_OHCI_PCI
>     select SERIAL
>     select GOLDFISH_RTC
>     select LOONGSON_LIOINTC
> -    select PCI_DEVICES
>     select PCI_EXPRESS_GENERIC_BRIDGE
>     select MSI_NONBROKEN
>     select FW_CFG_MIPS
> @@ -53,6 +69,8 @@ config MIPS_CPS
>
> config MIPS_BOSTON
>     bool
> +    imply PCI_DEVICES
> +    imply TEST_DEVICES
>     select FITLOADER
>     select MIPS_CPS
>     select PCI_EXPRESS_XILINX
>

Re: [PATCH v2 8/8] mips: do not list individual devices from configs/

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 2622 bytes --]

Il mer 7 feb 2024, 14:18 BALATON Zoltan <balaton@eik.bme.hu> ha scritto:

> >     if (defaults_enabled() && object_class_by_name("pci-ohci")) {
> >         pci_create_simple(pci_bus, -1, "pci-ohci");
> > -        usb_create_simple(usb_bus_find(-1), "usb-kbd");
> > -        usb_create_simple(usb_bus_find(-1), "usb-tablet");
> > +        Object *usb_bus = object_resolve_path_type("", TYPE_USB_BUS,
> NULL);
> > +        usb_create_simple(USB_BUS(usb_bus), "usb-kbd");
> > +        usb_create_simple(USB_BUS(usb_bus), "usb-tablet");
> >     }
> >
> >     for (i = 0; i < nb_nics; i++) {
>
> Is this hunk supposed to be in this patch?
>

Yes, it fixes compilation --without-default-devices (where usb_bus_find
does not exist). Before this patch CONFIG_USB is always selected, even
without default devices.

Paolo


> Regards,
> BALATON Zoltan
>
> > diff --git a/hw/display/Kconfig b/hw/display/Kconfig
> > index 1aafe1923d2..5b2b3840f7a 100644
> > --- a/hw/display/Kconfig
> > +++ b/hw/display/Kconfig
> > @@ -55,7 +55,7 @@ config VGA_MMIO
> >
> > config VMWARE_VGA
> >     bool
> > -    default y if PCI_DEVICES && PC_PCI
> > +    default y if PCI_DEVICES && (PC_PCI || MIPS)
> >     depends on PCI
> >     select VGA
> >
> > diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
> > index e57db4f6412..5c83ef49cf6 100644
> > --- a/hw/mips/Kconfig
> > +++ b/hw/mips/Kconfig
> > @@ -1,8 +1,15 @@
> > config MALTA
> >     bool
> > +    imply PCNET_PCI
> > +    imply PCI_DEVICES
> > +    imply TEST_DEVICES
> >     select FDC37M81X
> >     select GT64120
> > +    select MIPS_CPS
> >     select PIIX
> > +    select PFLASH_CFI01
> > +    select SERIAL
> > +    select SMBUS_EEPROM
> >
> > config MIPSSIM
> >     bool
> > @@ -31,17 +38,26 @@ config JAZZ
> >
> > config FULOONG
> >     bool
> > +    imply PCI_DEVICES
> > +    imply TEST_DEVICES
> > +    imply ATI_VGA
> > +    imply RTL8139_PCI
> >     select PCI_BONITO
> > +    select SMBUS_EEPROM
> >     select VT82C686
> >
> > config LOONGSON3V
> >     bool
> > +    imply PCI_DEVICES
> > +    imply TEST_DEVICES
> > +    imply VIRTIO_PCI
> > +    imply VIRTIO_NET
> >     imply VIRTIO_VGA
> >     imply QXL if SPICE
> > +    imply USB_OHCI_PCI
> >     select SERIAL
> >     select GOLDFISH_RTC
> >     select LOONGSON_LIOINTC
> > -    select PCI_DEVICES
> >     select PCI_EXPRESS_GENERIC_BRIDGE
> >     select MSI_NONBROKEN
> >     select FW_CFG_MIPS
> > @@ -53,6 +69,8 @@ config MIPS_CPS
> >
> > config MIPS_BOSTON
> >     bool
> > +    imply PCI_DEVICES
> > +    imply TEST_DEVICES
> >     select FITLOADER
> >     select MIPS_CPS
> >     select PCI_EXPRESS_XILINX
> >
>
>

[-- Attachment #2: Type: text/html, Size: 3937 bytes --]

Re: [PATCH v2 8/8] mips: do not list individual devices from configs/

[Bernhard Beschow]

Am 7. Februar 2024 13:55:51 UTC schrieb Paolo Bonzini <pbonzini@redhat.com>:
>Il mer 7 feb 2024, 14:18 BALATON Zoltan <balaton@eik.bme.hu> ha scritto:
>
>> >     if (defaults_enabled() && object_class_by_name("pci-ohci")) {
>> >         pci_create_simple(pci_bus, -1, "pci-ohci");
>> > -        usb_create_simple(usb_bus_find(-1), "usb-kbd");
>> > -        usb_create_simple(usb_bus_find(-1), "usb-tablet");
>> > +        Object *usb_bus = object_resolve_path_type("", TYPE_USB_BUS,
>> NULL);
>> > +        usb_create_simple(USB_BUS(usb_bus), "usb-kbd");
>> > +        usb_create_simple(USB_BUS(usb_bus), "usb-tablet");
>> >     }
>> >
>> >     for (i = 0; i < nb_nics; i++) {
>>
>> Is this hunk supposed to be in this patch?
>>
>
>Yes, it fixes compilation --without-default-devices (where usb_bus_find
>does not exist). Before this patch CONFIG_USB is always selected, even
>without default devices.

Might be worth pointing out in the commit message. I wondered about this, too.

Bernhard

>
>Paolo
>
>
>> Regards,
>> BALATON Zoltan
>>
>> > diff --git a/hw/display/Kconfig b/hw/display/Kconfig
>> > index 1aafe1923d2..5b2b3840f7a 100644
>> > --- a/hw/display/Kconfig
>> > +++ b/hw/display/Kconfig
>> > @@ -55,7 +55,7 @@ config VGA_MMIO
>> >
>> > config VMWARE_VGA
>> >     bool
>> > -    default y if PCI_DEVICES && PC_PCI
>> > +    default y if PCI_DEVICES && (PC_PCI || MIPS)
>> >     depends on PCI
>> >     select VGA
>> >
>> > diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
>> > index e57db4f6412..5c83ef49cf6 100644
>> > --- a/hw/mips/Kconfig
>> > +++ b/hw/mips/Kconfig
>> > @@ -1,8 +1,15 @@
>> > config MALTA
>> >     bool
>> > +    imply PCNET_PCI
>> > +    imply PCI_DEVICES
>> > +    imply TEST_DEVICES
>> >     select FDC37M81X
>> >     select GT64120
>> > +    select MIPS_CPS
>> >     select PIIX
>> > +    select PFLASH_CFI01
>> > +    select SERIAL
>> > +    select SMBUS_EEPROM
>> >
>> > config MIPSSIM
>> >     bool
>> > @@ -31,17 +38,26 @@ config JAZZ
>> >
>> > config FULOONG
>> >     bool
>> > +    imply PCI_DEVICES
>> > +    imply TEST_DEVICES
>> > +    imply ATI_VGA
>> > +    imply RTL8139_PCI
>> >     select PCI_BONITO
>> > +    select SMBUS_EEPROM
>> >     select VT82C686
>> >
>> > config LOONGSON3V
>> >     bool
>> > +    imply PCI_DEVICES
>> > +    imply TEST_DEVICES
>> > +    imply VIRTIO_PCI
>> > +    imply VIRTIO_NET
>> >     imply VIRTIO_VGA
>> >     imply QXL if SPICE
>> > +    imply USB_OHCI_PCI
>> >     select SERIAL
>> >     select GOLDFISH_RTC
>> >     select LOONGSON_LIOINTC
>> > -    select PCI_DEVICES
>> >     select PCI_EXPRESS_GENERIC_BRIDGE
>> >     select MSI_NONBROKEN
>> >     select FW_CFG_MIPS
>> > @@ -53,6 +69,8 @@ config MIPS_CPS
>> >
>> > config MIPS_BOSTON
>> >     bool
>> > +    imply PCI_DEVICES
>> > +    imply TEST_DEVICES
>> >     select FITLOADER
>> >     select MIPS_CPS
>> >     select PCI_EXPRESS_XILINX
>> >
>>
>>