[Qemu-devel] [PATCH 41/52] isa: express dependencies with kconfig

Paolo Bonzini posted 52 patches 7 years ago
Maintainers: Eduardo Habkost <ehabkost@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Palmer Dabbelt <palmer@sifive.com>, Max Filippov <jcmvbkbc@gmail.com>, Alistair Francis <alistair@alistair23.me>, Guan Xuetao <gxt@mprc.pku.edu.cn>, Max Reitz <mreitz@redhat.com>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Igor Mammedov <imammedo@redhat.com>, Aleksandar Markovic <amarkovic@wavecomp.com>, Fam Zheng <fam@euphon.net>, Cornelia Huck <cohuck@redhat.com>, Peter Crosthwaite <crosthwaite.peter@gmail.com>, Gerd Hoffmann <kraxel@redhat.com>, Aleksandar Rikalo <arikalo@wavecomp.com>, Corey Minyard <minyard@acm.org>, Sagar Karandikar <sagark@eecs.berkeley.edu>, Alex Williamson <alex.williamson@redhat.com>, Alberto Garcia <berto@igalia.com>, Anthony Green <green@moxielogic.com>, Jason Wang <jasowang@redhat.com>, Aurelien Jarno <aurelien@aurel32.net>, Richard Henderson <rth@twiddle.net>, Greg Kurz <groug@kaod.org>, "Daniel P. Berrangé" <berrange@redhat.com>, Alistair Francis <Alistair.Francis@wdc.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Michael Clark <mjc@sifive.com>, Kevin Wolf <kwolf@redhat.com>, Chris Wulff <crwulff@gmail.com>, "Michael S. Tsirkin" <mst@redhat.com>, Stafford Horne <shorne@gmail.com>, Michael Walle <michael@walle.cc>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, David Hildenbrand <david@redhat.com>, Marek Vasut <marex@denx.de>, Paolo Bonzini <pbonzini@redhat.com>, David Gibson <david@gibson.dropbear.id.au>, Peter Maydell <peter.maydell@linaro.org>, Halil Pasic <pasic@linux.ibm.com>, Stefan Berger <stefanb@linux.ibm.com>, John Snow <jsnow@redhat.com>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Christian Borntraeger <borntraeger@de.ibm.com>, Artyom Tarasenko <atar4qemu@gmail.com>, Cleber Rosa <crosa@redhat.com>
There is a newer version of this series
[Qemu-devel] [PATCH 41/52] isa: express dependencies with kconfig
Posted by Paolo Bonzini 7 years ago
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Yang Zhong <yang.zhong@intel.com>
Message-Id: <20190123065618.3520-36-yang.zhong@intel.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 default-configs/i386-softmmu.mak | 9 ---------
 hw/audio/Kconfig                 | 2 ++
 hw/block/Kconfig                 | 2 ++
 hw/char/Kconfig                  | 6 ++++++
 hw/display/Kconfig               | 3 +++
 hw/dma/Kconfig                   | 1 +
 hw/i386/Kconfig                  | 1 +
 hw/ide/Kconfig                   | 1 +
 hw/input/Kconfig                 | 2 ++
 hw/isa/Kconfig                   | 7 +++++++
 hw/misc/Kconfig                  | 4 ++++
 hw/net/Kconfig                   | 3 +++
 hw/sparc64/Kconfig               | 1 +
 hw/watchdog/Kconfig              | 2 ++
 14 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index 4073c62..8e6a810 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -8,19 +8,12 @@ CONFIG_VGA_ISA=y
 CONFIG_VMWARE_VGA=y
 CONFIG_VMXNET3_PCI=y
 CONFIG_VIRTIO_VGA=y
-CONFIG_VMMOUSE=y
 CONFIG_IPMI=y
 CONFIG_IPMI_LOCAL=y
 CONFIG_IPMI_EXTERN=y
 CONFIG_ISA_IPMI_KCS=y
 CONFIG_ISA_IPMI_BT=y
-CONFIG_SERIAL=y
-CONFIG_SERIAL_ISA=y
-CONFIG_PARALLEL=y
 CONFIG_I8254=y
-CONFIG_PCSPK=y
-CONFIG_PCKBD=y
-CONFIG_FDC=y
 CONFIG_ACPI=y
 CONFIG_ACPI_X86=y
 CONFIG_ACPI_X86_ICH=y
@@ -30,14 +23,12 @@ CONFIG_APM=y
 CONFIG_I8257=y
 CONFIG_IDE_ISA=y
 CONFIG_IDE_PIIX=y
-CONFIG_NE2000_ISA=y
 CONFIG_HPET=y
 CONFIG_APPLESMC=y
 CONFIG_I8259=y
 CONFIG_PFLASH_CFI01=y
 CONFIG_MC146818RTC=y
 CONFIG_PCI_PIIX=y
-CONFIG_WDT_IB700=y
 CONFIG_ISA_DEBUG=y
 CONFIG_ISA_TESTDEV=y
 CONFIG_VMPORT=y
diff --git a/hw/audio/Kconfig b/hw/audio/Kconfig
index dedb513..01aea55 100644
--- a/hw/audio/Kconfig
+++ b/hw/audio/Kconfig
@@ -35,6 +35,8 @@ config HDA
 
 config PCSPK
     bool
+    default y
+    depends on I8254
 
 config WM8750
     bool
diff --git a/hw/block/Kconfig b/hw/block/Kconfig
index f7b9d3a..dc91e67 100644
--- a/hw/block/Kconfig
+++ b/hw/block/Kconfig
@@ -1,5 +1,7 @@
 config FDC
     bool
+    default y
+    depends on ISA_BUS
 
 config SSI_M25P80
     bool
diff --git a/hw/char/Kconfig b/hw/char/Kconfig
index 6eba69a..fc18481 100644
--- a/hw/char/Kconfig
+++ b/hw/char/Kconfig
@@ -3,6 +3,8 @@ config ESCC
 
 config PARALLEL
     bool
+    default y
+    depends on ISA_BUS
 
 config PL011
     bool
@@ -12,11 +14,15 @@ config SERIAL
 
 config SERIAL_ISA
     bool
+    default y
+    depends on ISA_BUS
+    select SERIAL
 
 config SERIAL_PCI
     bool
     default y if PCI_DEVICES
     depends on PCI
+    select SERIAL
 
 config VIRTIO_SERIAL
     bool
diff --git a/hw/display/Kconfig b/hw/display/Kconfig
index f8d63c6..64a5764 100644
--- a/hw/display/Kconfig
+++ b/hw/display/Kconfig
@@ -39,9 +39,12 @@ config VGA_PCI
 
 config VGA_ISA
     bool
+    depends on ISA_BUS
+    select VGA
 
 config VGA_ISA_MM
     bool
+    select VGA
 
 config VMWARE_VGA
     bool
diff --git a/hw/dma/Kconfig b/hw/dma/Kconfig
index b9ce1c5..751dec5 100644
--- a/hw/dma/Kconfig
+++ b/hw/dma/Kconfig
@@ -9,6 +9,7 @@ config PL330
 
 config I82374
     bool
+    select I8257
 
 config I8257
     bool
diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index 9a0e559..ff41be3 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -10,6 +10,7 @@ config I440FX
 
 config ISAPC
     bool
+    select ISA_BUS
 
 config Q35
     bool
diff --git a/hw/ide/Kconfig b/hw/ide/Kconfig
index 246e27b..ab47b6a 100644
--- a/hw/ide/Kconfig
+++ b/hw/ide/Kconfig
@@ -12,6 +12,7 @@ config IDE_PCI
 
 config IDE_ISA
     bool
+    depends on ISA_BUS
     select IDE_QDEV
 
 config IDE_PIIX
diff --git a/hw/input/Kconfig b/hw/input/Kconfig
index 98a18a1..bdb4237 100644
--- a/hw/input/Kconfig
+++ b/hw/input/Kconfig
@@ -6,6 +6,8 @@ config LM832X
 
 config PCKBD
     bool
+    default y
+    depends on ISA_BUS
 
 config PL050
     bool
diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
index b59d074..af68af9 100644
--- a/hw/isa/Kconfig
+++ b/hw/isa/Kconfig
@@ -6,18 +6,25 @@ config APM
 
 config I82378
     bool
+    select ISA_BUS
 
 config PC87312
     bool
+    select ISA_BUS
 
 config PIIX4
     bool
+    select ISA_BUS
 
 config VT82C686
     bool
+    select ISA_BUS
 
 config SMC37C669
     bool
+    select ISA_BUS
 
 config LPC_ICH9
     bool
+    select ISA_BUS
+    select ACPI_X86_ICH
diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
index c85c085..ca051fb 100644
--- a/hw/misc/Kconfig
+++ b/hw/misc/Kconfig
@@ -1,5 +1,6 @@
 config APPLESMC
     bool
+    depends on ISA_BUS
 
 config MAX111X
     bool
@@ -12,9 +13,11 @@ config TMP421
 
 config ISA_DEBUG
     bool
+    depends on ISA_BUS
 
 config SGA
     bool
+    depends on ISA_BUS
 
 config ISA_TESTDEV
     bool
@@ -93,6 +96,7 @@ config IOTKIT_SYSINFO
 
 config PVPANIC
     bool
+    depends on ISA_BUS
 
 config AUX
     bool
diff --git a/hw/net/Kconfig b/hw/net/Kconfig
index d50e301..6d15720 100644
--- a/hw/net/Kconfig
+++ b/hw/net/Kconfig
@@ -48,6 +48,9 @@ config LAN9118
 
 config NE2000_ISA
     bool
+    default y
+    depends on ISA_BUS
+    depends on PCI # for NE2000State
 
 config OPENCORES_ETH
     bool
diff --git a/hw/sparc64/Kconfig b/hw/sparc64/Kconfig
index 8c13345..41f7295 100644
--- a/hw/sparc64/Kconfig
+++ b/hw/sparc64/Kconfig
@@ -1,5 +1,6 @@
 config SUN4U
     bool
+    select ISA_BUS
 
 config NIAGARA
     bool
diff --git a/hw/watchdog/Kconfig b/hw/watchdog/Kconfig
index edb3d42..35ccb72 100644
--- a/hw/watchdog/Kconfig
+++ b/hw/watchdog/Kconfig
@@ -8,6 +8,8 @@ config WDT_IB6300ESB
 
 config WDT_IB700
     bool
+    default y
+    depends on ISA_BUS
 
 config WDT_DIAG288
     bool
-- 
1.8.3.1



Re: [Qemu-devel] [PATCH 41/52] isa: express dependencies with kconfig
Posted by Thomas Huth 7 years ago
On 2019-01-25 11:07, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Yang Zhong <yang.zhong@intel.com>
> Message-Id: <20190123065618.3520-36-yang.zhong@intel.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  default-configs/i386-softmmu.mak | 9 ---------
>  hw/audio/Kconfig                 | 2 ++
>  hw/block/Kconfig                 | 2 ++
>  hw/char/Kconfig                  | 6 ++++++
>  hw/display/Kconfig               | 3 +++
>  hw/dma/Kconfig                   | 1 +
>  hw/i386/Kconfig                  | 1 +
>  hw/ide/Kconfig                   | 1 +
>  hw/input/Kconfig                 | 2 ++
>  hw/isa/Kconfig                   | 7 +++++++
>  hw/misc/Kconfig                  | 4 ++++
>  hw/net/Kconfig                   | 3 +++
>  hw/sparc64/Kconfig               | 1 +
>  hw/watchdog/Kconfig              | 2 ++
>  14 files changed, 35 insertions(+), 9 deletions(-)
> 
> diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
> index 4073c62..8e6a810 100644
> --- a/default-configs/i386-softmmu.mak
> +++ b/default-configs/i386-softmmu.mak
> @@ -8,19 +8,12 @@ CONFIG_VGA_ISA=y
>  CONFIG_VMWARE_VGA=y
>  CONFIG_VMXNET3_PCI=y
>  CONFIG_VIRTIO_VGA=y
> -CONFIG_VMMOUSE=y
>  CONFIG_IPMI=y
>  CONFIG_IPMI_LOCAL=y
>  CONFIG_IPMI_EXTERN=y
>  CONFIG_ISA_IPMI_KCS=y
>  CONFIG_ISA_IPMI_BT=y
> -CONFIG_SERIAL=y
> -CONFIG_SERIAL_ISA=y
> -CONFIG_PARALLEL=y
>  CONFIG_I8254=y
> -CONFIG_PCSPK=y
> -CONFIG_PCKBD=y
> -CONFIG_FDC=y
>  CONFIG_ACPI=y
>  CONFIG_ACPI_X86=y
>  CONFIG_ACPI_X86_ICH=y
> @@ -30,14 +23,12 @@ CONFIG_APM=y
>  CONFIG_I8257=y
>  CONFIG_IDE_ISA=y
>  CONFIG_IDE_PIIX=y
> -CONFIG_NE2000_ISA=y
>  CONFIG_HPET=y
>  CONFIG_APPLESMC=y
>  CONFIG_I8259=y
>  CONFIG_PFLASH_CFI01=y
>  CONFIG_MC146818RTC=y
>  CONFIG_PCI_PIIX=y
> -CONFIG_WDT_IB700=y
>  CONFIG_ISA_DEBUG=y
>  CONFIG_ISA_TESTDEV=y
>  CONFIG_VMPORT=y
> diff --git a/hw/audio/Kconfig b/hw/audio/Kconfig
> index dedb513..01aea55 100644
> --- a/hw/audio/Kconfig
> +++ b/hw/audio/Kconfig
> @@ -35,6 +35,8 @@ config HDA
>  
>  config PCSPK
>      bool
> +    default y
> +    depends on I8254
>  
>  config WM8750
>      bool
> diff --git a/hw/block/Kconfig b/hw/block/Kconfig
> index f7b9d3a..dc91e67 100644
> --- a/hw/block/Kconfig
> +++ b/hw/block/Kconfig
> @@ -1,5 +1,7 @@
>  config FDC
>      bool
> +    default y
> +    depends on ISA_BUS
>  
>  config SSI_M25P80
>      bool
> diff --git a/hw/char/Kconfig b/hw/char/Kconfig
> index 6eba69a..fc18481 100644
> --- a/hw/char/Kconfig
> +++ b/hw/char/Kconfig
> @@ -3,6 +3,8 @@ config ESCC
>  
>  config PARALLEL
>      bool
> +    default y
> +    depends on ISA_BUS
>  
>  config PL011
>      bool
> @@ -12,11 +14,15 @@ config SERIAL
>  
>  config SERIAL_ISA
>      bool
> +    default y
> +    depends on ISA_BUS
> +    select SERIAL
>  
>  config SERIAL_PCI
>      bool
>      default y if PCI_DEVICES
>      depends on PCI
> +    select SERIAL
>  
>  config VIRTIO_SERIAL
>      bool
> diff --git a/hw/display/Kconfig b/hw/display/Kconfig
> index f8d63c6..64a5764 100644
> --- a/hw/display/Kconfig
> +++ b/hw/display/Kconfig
> @@ -39,9 +39,12 @@ config VGA_PCI
>  
>  config VGA_ISA
>      bool
> +    depends on ISA_BUS
> +    select VGA
>  
>  config VGA_ISA_MM
>      bool
> +    select VGA
>  
>  config VMWARE_VGA
>      bool
> diff --git a/hw/dma/Kconfig b/hw/dma/Kconfig
> index b9ce1c5..751dec5 100644
> --- a/hw/dma/Kconfig
> +++ b/hw/dma/Kconfig
> @@ -9,6 +9,7 @@ config PL330
>  
>  config I82374
>      bool
> +    select I8257
>  
>  config I8257
>      bool
> diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
> index 9a0e559..ff41be3 100644
> --- a/hw/i386/Kconfig
> +++ b/hw/i386/Kconfig
> @@ -10,6 +10,7 @@ config I440FX
>  
>  config ISAPC
>      bool
> +    select ISA_BUS
>  
>  config Q35
>      bool
> diff --git a/hw/ide/Kconfig b/hw/ide/Kconfig
> index 246e27b..ab47b6a 100644
> --- a/hw/ide/Kconfig
> +++ b/hw/ide/Kconfig
> @@ -12,6 +12,7 @@ config IDE_PCI
>  
>  config IDE_ISA
>      bool
> +    depends on ISA_BUS
>      select IDE_QDEV
>  
>  config IDE_PIIX
> diff --git a/hw/input/Kconfig b/hw/input/Kconfig
> index 98a18a1..bdb4237 100644
> --- a/hw/input/Kconfig
> +++ b/hw/input/Kconfig
> @@ -6,6 +6,8 @@ config LM832X
>  
>  config PCKBD
>      bool
> +    default y
> +    depends on ISA_BUS
>  
>  config PL050
>      bool
> diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
> index b59d074..af68af9 100644
> --- a/hw/isa/Kconfig
> +++ b/hw/isa/Kconfig
> @@ -6,18 +6,25 @@ config APM
>  
>  config I82378
>      bool
> +    select ISA_BUS
>  
>  config PC87312
>      bool
> +    select ISA_BUS
>  
>  config PIIX4
>      bool
> +    select ISA_BUS
>  
>  config VT82C686
>      bool
> +    select ISA_BUS
>  
>  config SMC37C669
>      bool
> +    select ISA_BUS
>  
>  config LPC_ICH9
>      bool
> +    select ISA_BUS
> +    select ACPI_X86_ICH
> diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
> index c85c085..ca051fb 100644
> --- a/hw/misc/Kconfig
> +++ b/hw/misc/Kconfig
> @@ -1,5 +1,6 @@
>  config APPLESMC
>      bool
> +    depends on ISA_BUS
>  
>  config MAX111X
>      bool
> @@ -12,9 +13,11 @@ config TMP421
>  
>  config ISA_DEBUG
>      bool
> +    depends on ISA_BUS
>  
>  config SGA
>      bool
> +    depends on ISA_BUS
>  
>  config ISA_TESTDEV
>      bool
> @@ -93,6 +96,7 @@ config IOTKIT_SYSINFO
>  
>  config PVPANIC
>      bool
> +    depends on ISA_BUS
>  
>  config AUX
>      bool
> diff --git a/hw/net/Kconfig b/hw/net/Kconfig
> index d50e301..6d15720 100644
> --- a/hw/net/Kconfig
> +++ b/hw/net/Kconfig
> @@ -48,6 +48,9 @@ config LAN9118
>  
>  config NE2000_ISA
>      bool
> +    default y
> +    depends on ISA_BUS
> +    depends on PCI # for NE2000State
>  
>  config OPENCORES_ETH
>      bool
> diff --git a/hw/sparc64/Kconfig b/hw/sparc64/Kconfig
> index 8c13345..41f7295 100644
> --- a/hw/sparc64/Kconfig
> +++ b/hw/sparc64/Kconfig
> @@ -1,5 +1,6 @@
>  config SUN4U
>      bool
> +    select ISA_BUS
>  
>  config NIAGARA
>      bool
> diff --git a/hw/watchdog/Kconfig b/hw/watchdog/Kconfig
> index edb3d42..35ccb72 100644
> --- a/hw/watchdog/Kconfig
> +++ b/hw/watchdog/Kconfig
> @@ -8,6 +8,8 @@ config WDT_IB6300ESB
>  
>  config WDT_IB700
>      bool
> +    default y
> +    depends on ISA_BUS

By the way, for pluggable ISA devices (like NE2000_ISA or WDT_IB700), it
would be great to have a ISA_DEVICES config switch, just like the
"PCI_DEVICES" switch, which the ISA devices should then depend on
instead of "ISA_BUS". Then it would be easier for the users to disable
optional devices in their builds.

 Thomas

Re: [Qemu-devel] [PATCH 41/52] isa: express dependencies with kconfig
Posted by Paolo Bonzini 7 years ago
On 30/01/19 11:53, Thomas Huth wrote:
>> diff --git a/hw/watchdog/Kconfig b/hw/watchdog/Kconfig
>> index edb3d42..35ccb72 100644
>> --- a/hw/watchdog/Kconfig
>> +++ b/hw/watchdog/Kconfig
>> @@ -8,6 +8,8 @@ config WDT_IB6300ESB
>>  
>>  config WDT_IB700
>>      bool
>> +    default y
>> +    depends on ISA_BUS
> By the way, for pluggable ISA devices (like NE2000_ISA or WDT_IB700), it
> would be great to have a ISA_DEVICES config switch, just like the
> "PCI_DEVICES" switch, which the ISA devices should then depend on
> instead of "ISA_BUS". Then it would be easier for the users to disable
> optional devices in their builds.

Note that the only reason for PCI_DEVICES's existence is that s390x does
not want _all_ PCI devices.  Is there a use case for disabling
CONFIG_ISA_DEVICES but not wanting to base your configuration on
"allnoconfig"?

Paolo

Re: [Qemu-devel] [PATCH 41/52] isa: express dependencies with kconfig
Posted by Thomas Huth 7 years ago
On 2019-01-30 12:13, Paolo Bonzini wrote:
> On 30/01/19 11:53, Thomas Huth wrote:
>>> diff --git a/hw/watchdog/Kconfig b/hw/watchdog/Kconfig
>>> index edb3d42..35ccb72 100644
>>> --- a/hw/watchdog/Kconfig
>>> +++ b/hw/watchdog/Kconfig
>>> @@ -8,6 +8,8 @@ config WDT_IB6300ESB
>>>  
>>>  config WDT_IB700
>>>      bool
>>> +    default y
>>> +    depends on ISA_BUS
>> By the way, for pluggable ISA devices (like NE2000_ISA or WDT_IB700), it
>> would be great to have a ISA_DEVICES config switch, just like the
>> "PCI_DEVICES" switch, which the ISA devices should then depend on
>> instead of "ISA_BUS". Then it would be easier for the users to disable
>> optional devices in their builds.
> 
> Note that the only reason for PCI_DEVICES's existence is that s390x does
> not want _all_ PCI devices.

Oh, I thought this was for convenience. So for s390x, the main problem is
AFAIK that the architecture does not support LSI, so all devices that
require LSI can not work here. Maybe we should introduce a "LSI" switch
instead, so that the boards which provide LSI can select it, and all normal
PCI devices (at least the ones that do not work with MSI only) would
"depend on LSI", too?

> Is there a use case for disabling
> CONFIG_ISA_DEVICES but not wanting to base your configuration on
> "allnoconfig"?

"make allnoconfig" is not working for me yet:

make MINIKCONF="python -B /home/thuth/devel/qemu/scripts/minikconf.py  --" config-all-devices.mak
make[1]: Entering directory `/tmp/qemu-kconfig'
  GEN     x86_64-softmmu/config-devices.mak.tmp
/home/thuth/devel/qemu/scripts/minikconf.py: invalid option --

So I disabled PCI_DEVICES here intead for my tests for the hard
requirements, and then I ran into this error:

hw/net/ne2000-isa.c:65: undefined reference to `ne2000_setup_io'
hw/net/ne2000-isa.c:71: undefined reference to `ne2000_reset'
../hw/net/ne2000-isa.o:(.data.rel+0x1d0): undefined reference to `vmstate_ne2000'
../hw/net/ne2000-isa.o:(.data.rel+0x270): undefined reference to `ne2000_receive'

That's why I thought a CONFIG_ISA_DEVICES switch would be good, too.
But we likely should simply fix the dependency of the NE2000_ISA switch
instead.

 Thomas

Re: [Qemu-devel] [PATCH 41/52] isa: express dependencies with kconfig
Posted by Paolo Bonzini 7 years ago
On 30/01/19 12:32, Thomas Huth wrote:
> Oh, I thought this was for convenience. So for s390x, the main problem is
> AFAIK that the architecture does not support LSI, so all devices that
> require LSI can not work here. Maybe we should introduce a "LSI" switch
> instead, so that the boards which provide LSI can select it, and all normal
> PCI devices (at least the ones that do not work with MSI only) would
> "depend on LSI", too?

Makes sense (I would call it "depends on PCI_INTX", or even make it a
negative "depends on !NO_PCI_INTX" that would only be selected by s390).

Another idea is to provide an alternative way

    config X
        imply Y

to write

    config Y
        default y if X

("imply" was added a year or two ago to Kconfig).  This would simplify
writing the configuration for variables such as CONFIG_PCI_DEVICES, and
it would also be useful to express soft dependencies such as

    config PC
        imply PCI_DEVICES
        imply QXL

    config Q35
        imply VTD

etc.  This can be left for future work though.

>> Is there a use case for disabling
>> CONFIG_ISA_DEVICES but not wanting to base your configuration on
>> "allnoconfig"?
> 
> "make allnoconfig" is not working for me yet:
> 
> make MINIKCONF="python -B /home/thuth/devel/qemu/scripts/minikconf.py  --" config-all-devices.mak
> make[1]: Entering directory `/tmp/qemu-kconfig'
>   GEN     x86_64-softmmu/config-devices.mak.tmp
> /home/thuth/devel/qemu/scripts/minikconf.py: invalid option --
> 
> So I disabled PCI_DEVICES here intead for my tests for the hard
> requirements, and then I ran into this error:
> 
> hw/net/ne2000-isa.c:65: undefined reference to `ne2000_setup_io'
> hw/net/ne2000-isa.c:71: undefined reference to `ne2000_reset'
> ../hw/net/ne2000-isa.o:(.data.rel+0x1d0): undefined reference to `vmstate_ne2000'
> ../hw/net/ne2000-isa.o:(.data.rel+0x270): undefined reference to `ne2000_receive'
> 
> That's why I thought a CONFIG_ISA_DEVICES switch would be good, too.
> But we likely should simply fix the dependency of the NE2000_ISA switch
> instead.

Ok!

Paolo

Re: [Qemu-devel] [PATCH 41/52] isa: express dependencies with kconfig
Posted by Thomas Huth 7 years ago
On 2019-01-25 11:07, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Yang Zhong <yang.zhong@intel.com>
> Message-Id: <20190123065618.3520-36-yang.zhong@intel.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
[...]
> diff --git a/hw/net/Kconfig b/hw/net/Kconfig
> index d50e301..6d15720 100644
> --- a/hw/net/Kconfig
> +++ b/hw/net/Kconfig
> @@ -48,6 +48,9 @@ config LAN9118
>  
>  config NE2000_ISA
>      bool
> +    default y
> +    depends on ISA_BUS
> +    depends on PCI # for NE2000State

 Yang,

for the time being, could you please change the last line into

 depends on NE2000_PCI

instead? Otherwise, it's still possible to enable NE2000_ISA while
NE2000_PCI is disabled, and this will currently result in linking errors.

In the long run, we should likely put the common functions into a
separate file, but that's future work, after your series has been
merged, I think.

 Thanks,
  Thomas

Re: [Qemu-devel] [PATCH 41/52] isa: express dependencies with kconfig
Posted by Yang Zhong 7 years ago
On Wed, Jan 30, 2019 at 12:58:23PM +0100, Thomas Huth wrote:
> On 2019-01-25 11:07, Paolo Bonzini wrote:
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > Signed-off-by: Yang Zhong <yang.zhong@intel.com>
> > Message-Id: <20190123065618.3520-36-yang.zhong@intel.com>
> > Reviewed-by: Thomas Huth <thuth@redhat.com>
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> [...]
> > diff --git a/hw/net/Kconfig b/hw/net/Kconfig
> > index d50e301..6d15720 100644
> > --- a/hw/net/Kconfig
> > +++ b/hw/net/Kconfig
> > @@ -48,6 +48,9 @@ config LAN9118
> >  
> >  config NE2000_ISA
> >      bool
> > +    default y
> > +    depends on ISA_BUS
> > +    depends on PCI # for NE2000State
> 
>  Yang,
> 
> for the time being, could you please change the last line into
> 
>  depends on NE2000_PCI
> 
> instead? Otherwise, it's still possible to enable NE2000_ISA while
> NE2000_PCI is disabled, and this will currently result in linking errors.
>
  Yes, i will use "depends on NE2000_PCI" to replace "depends on PCI # for NE2000State". thanks!

  Yang
 
> In the long run, we should likely put the common functions into a
> separate file, but that's future work, after your series has been
> merged, I think.
> 
>  Thanks,
>   Thomas

Re: [Qemu-devel] [PATCH 41/52] isa: express dependencies with kconfig
Posted by Philippe Mathieu-Daudé 7 years ago
Hi Paolo,

On 1/25/19 11:07 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Yang Zhong <yang.zhong@intel.com>
> Message-Id: <20190123065618.3520-36-yang.zhong@intel.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  default-configs/i386-softmmu.mak | 9 ---------
>  hw/audio/Kconfig                 | 2 ++
>  hw/block/Kconfig                 | 2 ++
>  hw/char/Kconfig                  | 6 ++++++
>  hw/display/Kconfig               | 3 +++
>  hw/dma/Kconfig                   | 1 +
>  hw/i386/Kconfig                  | 1 +
>  hw/ide/Kconfig                   | 1 +
>  hw/input/Kconfig                 | 2 ++
>  hw/isa/Kconfig                   | 7 +++++++
>  hw/misc/Kconfig                  | 4 ++++
>  hw/net/Kconfig                   | 3 +++
>  hw/sparc64/Kconfig               | 1 +
>  hw/watchdog/Kconfig              | 2 ++
>  14 files changed, 35 insertions(+), 9 deletions(-)
> 
> diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
> index 4073c62..8e6a810 100644
> --- a/default-configs/i386-softmmu.mak
> +++ b/default-configs/i386-softmmu.mak
> @@ -8,19 +8,12 @@ CONFIG_VGA_ISA=y
>  CONFIG_VMWARE_VGA=y
>  CONFIG_VMXNET3_PCI=y
>  CONFIG_VIRTIO_VGA=y
> -CONFIG_VMMOUSE=y
>  CONFIG_IPMI=y
>  CONFIG_IPMI_LOCAL=y
>  CONFIG_IPMI_EXTERN=y
>  CONFIG_ISA_IPMI_KCS=y
>  CONFIG_ISA_IPMI_BT=y
> -CONFIG_SERIAL=y
> -CONFIG_SERIAL_ISA=y
> -CONFIG_PARALLEL=y
>  CONFIG_I8254=y
> -CONFIG_PCSPK=y
> -CONFIG_PCKBD=y
> -CONFIG_FDC=y
>  CONFIG_ACPI=y
>  CONFIG_ACPI_X86=y
>  CONFIG_ACPI_X86_ICH=y
> @@ -30,14 +23,12 @@ CONFIG_APM=y
>  CONFIG_I8257=y
>  CONFIG_IDE_ISA=y
>  CONFIG_IDE_PIIX=y
> -CONFIG_NE2000_ISA=y
>  CONFIG_HPET=y
>  CONFIG_APPLESMC=y
>  CONFIG_I8259=y
>  CONFIG_PFLASH_CFI01=y
>  CONFIG_MC146818RTC=y
>  CONFIG_PCI_PIIX=y
> -CONFIG_WDT_IB700=y
>  CONFIG_ISA_DEBUG=y
>  CONFIG_ISA_TESTDEV=y
>  CONFIG_VMPORT=y
> diff --git a/hw/audio/Kconfig b/hw/audio/Kconfig
> index dedb513..01aea55 100644
> --- a/hw/audio/Kconfig
> +++ b/hw/audio/Kconfig
> @@ -35,6 +35,8 @@ config HDA
>  
>  config PCSPK
>      bool
> +    default y
> +    depends on I8254
>  
>  config WM8750
>      bool
> diff --git a/hw/block/Kconfig b/hw/block/Kconfig
> index f7b9d3a..dc91e67 100644
> --- a/hw/block/Kconfig
> +++ b/hw/block/Kconfig
> @@ -1,5 +1,7 @@
>  config FDC
>      bool
> +    default y
> +    depends on ISA_BUS
>  
>  config SSI_M25P80
>      bool
> diff --git a/hw/char/Kconfig b/hw/char/Kconfig
> index 6eba69a..fc18481 100644
> --- a/hw/char/Kconfig
> +++ b/hw/char/Kconfig
> @@ -3,6 +3,8 @@ config ESCC
>  
>  config PARALLEL
>      bool
> +    default y
> +    depends on ISA_BUS
>  
>  config PL011
>      bool
> @@ -12,11 +14,15 @@ config SERIAL
>  
>  config SERIAL_ISA
>      bool
> +    default y
> +    depends on ISA_BUS
> +    select SERIAL
>  
>  config SERIAL_PCI
>      bool
>      default y if PCI_DEVICES
>      depends on PCI
> +    select SERIAL
>  
>  config VIRTIO_SERIAL
>      bool
> diff --git a/hw/display/Kconfig b/hw/display/Kconfig
> index f8d63c6..64a5764 100644
> --- a/hw/display/Kconfig
> +++ b/hw/display/Kconfig
> @@ -39,9 +39,12 @@ config VGA_PCI
>  
>  config VGA_ISA
>      bool
> +    depends on ISA_BUS
> +    select VGA
>  
>  config VGA_ISA_MM
>      bool
> +    select VGA
>  
>  config VMWARE_VGA
>      bool
> diff --git a/hw/dma/Kconfig b/hw/dma/Kconfig
> index b9ce1c5..751dec5 100644
> --- a/hw/dma/Kconfig
> +++ b/hw/dma/Kconfig
> @@ -9,6 +9,7 @@ config PL330
>  
>  config I82374
>      bool
> +    select I8257
>  
>  config I8257
>      bool
> diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
> index 9a0e559..ff41be3 100644
> --- a/hw/i386/Kconfig
> +++ b/hw/i386/Kconfig
> @@ -10,6 +10,7 @@ config I440FX
>  
>  config ISAPC
>      bool
> +    select ISA_BUS
>  
>  config Q35
>      bool
> diff --git a/hw/ide/Kconfig b/hw/ide/Kconfig
> index 246e27b..ab47b6a 100644
> --- a/hw/ide/Kconfig
> +++ b/hw/ide/Kconfig
> @@ -12,6 +12,7 @@ config IDE_PCI
>  
>  config IDE_ISA
>      bool
> +    depends on ISA_BUS
>      select IDE_QDEV
>  
>  config IDE_PIIX
> diff --git a/hw/input/Kconfig b/hw/input/Kconfig
> index 98a18a1..bdb4237 100644
> --- a/hw/input/Kconfig
> +++ b/hw/input/Kconfig
> @@ -6,6 +6,8 @@ config LM832X
>  
>  config PCKBD
>      bool
> +    default y
> +    depends on ISA_BUS
>  
>  config PL050
>      bool
> diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
> index b59d074..af68af9 100644
> --- a/hw/isa/Kconfig
> +++ b/hw/isa/Kconfig
> @@ -6,18 +6,25 @@ config APM
>  
>  config I82378
>      bool
> +    select ISA_BUS
>  
>  config PC87312
>      bool
> +    select ISA_BUS
>  
>  config PIIX4
>      bool
> +    select ISA_BUS
>  
>  config VT82C686
>      bool
> +    select ISA_BUS
>  
>  config SMC37C669
>      bool
> +    select ISA_BUS

I kinda disagree with the SuperIO generated configs here, but partly my
fault because the previous Makefile.objs missed the CONFIG_ISA_SUPERIO
(I missed to review eae2e2e96bf from Thomas where is introduced
CONFIG_SMC37C669).
So introducing ISA_SUPERIO simplifies this files and SouthBridge
devices. I'm not sure how to provide this patch:

 # generic SuperIO
 config ISA_SUPERIO
     bool
     select ISA_BUS
     select SERIAL_ISA
     select PARALLEL
     select FDC
     select IDE_ISA

 config PC87312
     bool
     select ISA_SUPERIO
     select I8259
     select I8254
     select I8257
     select MC146818RTC

 config SMC37C669
     bool
     select ISA_SUPERIO

I'll see, if the changes are trivial you can cherry-pick at the correct
place in your branch.

>  
>  config LPC_ICH9
>      bool
> +    select ISA_BUS
> +    select ACPI_X86_ICH
> diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
> index c85c085..ca051fb 100644
> --- a/hw/misc/Kconfig
> +++ b/hw/misc/Kconfig
> @@ -1,5 +1,6 @@
>  config APPLESMC
>      bool
> +    depends on ISA_BUS
>  
>  config MAX111X
>      bool
> @@ -12,9 +13,11 @@ config TMP421
>  
>  config ISA_DEBUG
>      bool
> +    depends on ISA_BUS
>  
>  config SGA
>      bool
> +    depends on ISA_BUS
>  
>  config ISA_TESTDEV
>      bool
> @@ -93,6 +96,7 @@ config IOTKIT_SYSINFO
>  
>  config PVPANIC
>      bool
> +    depends on ISA_BUS
>  
>  config AUX
>      bool
> diff --git a/hw/net/Kconfig b/hw/net/Kconfig
> index d50e301..6d15720 100644
> --- a/hw/net/Kconfig
> +++ b/hw/net/Kconfig
> @@ -48,6 +48,9 @@ config LAN9118
>  
>  config NE2000_ISA
>      bool
> +    default y
> +    depends on ISA_BUS
> +    depends on PCI # for NE2000State
>  
>  config OPENCORES_ETH
>      bool
> diff --git a/hw/sparc64/Kconfig b/hw/sparc64/Kconfig
> index 8c13345..41f7295 100644
> --- a/hw/sparc64/Kconfig
> +++ b/hw/sparc64/Kconfig
> @@ -1,5 +1,6 @@
>  config SUN4U
>      bool
> +    select ISA_BUS
>  
>  config NIAGARA
>      bool
> diff --git a/hw/watchdog/Kconfig b/hw/watchdog/Kconfig
> index edb3d42..35ccb72 100644
> --- a/hw/watchdog/Kconfig
> +++ b/hw/watchdog/Kconfig
> @@ -8,6 +8,8 @@ config WDT_IB6300ESB
>  
>  config WDT_IB700
>      bool
> +    default y
> +    depends on ISA_BUS
>  
>  config WDT_DIAG288
>      bool
> 

Re: [Qemu-devel] [PATCH 41/52] isa: express dependencies with kconfig
Posted by Paolo Bonzini 7 years ago
On 31/01/19 22:22, Philippe Mathieu-Daudé wrote:
> I kinda disagree with the SuperIO generated configs here, but partly my
> fault because the previous Makefile.objs missed the CONFIG_ISA_SUPERIO
> (I missed to review eae2e2e96bf from Thomas where is introduced
> CONFIG_SMC37C669).
> So introducing ISA_SUPERIO simplifies this files and SouthBridge
> devices. I'm not sure how to provide this patch:

The problem is different SuperIO chips can have or lack
floppy/serial/parallel, and so they end up having different dependencies.

Config symbols are a tool to generate working QEMUs (where working =
build and pass device-introspect-test more or less), they needn't
reflect precisely the topology of the machine.

Paolo

Re: [Qemu-devel] [PATCH 41/52] isa: express dependencies with kconfig
Posted by Philippe Mathieu-Daudé 7 years ago
On 1/31/19 11:14 PM, Paolo Bonzini wrote:
> On 31/01/19 22:22, Philippe Mathieu-Daudé wrote:
>> I kinda disagree with the SuperIO generated configs here, but partly my
>> fault because the previous Makefile.objs missed the CONFIG_ISA_SUPERIO
>> (I missed to review eae2e2e96bf from Thomas where is introduced
>> CONFIG_SMC37C669).
>> So introducing ISA_SUPERIO simplifies this files and SouthBridge
>> devices. I'm not sure how to provide this patch:
> 
> The problem is different SuperIO chips can have or lack
> floppy/serial/parallel, and so they end up having different dependencies.
> 
> Config symbols are a tool to generate working QEMUs (where working =
> build and pass device-introspect-test more or less), they needn't
> reflect precisely the topology of the machine.

The model implementation is:
- abstract SuperIO parent which can instantiate all configs,
- child implementation.

Childs require their parent, and even if the parent will instantiate
them without all properties, the parent needs to link with them.

In short, if a SuperIO child requires ISA_SUPERIO, it also requires to
link to serial/parallel/floppy/ide.

That's why I prefer the explicit dependencies.

Oh, actually if we correctly use the TYPE_XXX names, it might link.
The only problem would be trying to instantiate a device via QMP then?

Re: [Qemu-devel] [PATCH 41/52] isa: express dependencies with kconfig
Posted by Paolo Bonzini 6 years, 11 months ago
On 31/01/19 23:24, Philippe Mathieu-Daudé wrote:
> The model implementation is:
> - abstract SuperIO parent which can instantiate all configs,
> - child implementation.
> 
> Childs require their parent, and even if the parent will instantiate
> them without all properties, the parent needs to link with them.
> 
> In short, if a SuperIO child requires ISA_SUPERIO, it also requires to
> link to serial/parallel/floppy/ide.

It depends... Some SuperIO chips (children) do not have full
functionality, and will never have one or more of
serial/parallel/floppy/ide.  This is why I left the choice of
"select"ing the components in the children.  The code already has to set
up ISASuperIOClass, so there is knowledge of what to select.

Paolo