diff mbox series

s390x: express dependencies with Kconfig

Message ID 1548689366-31916-1-git-send-email-thuth@redhat.com
State New
Headers show
Series s390x: express dependencies with Kconfig | expand

Commit Message

Thomas Huth Jan. 28, 2019, 3:29 p.m. UTC
Instead of hard-coding all config switches in the config file
default-configs/s390x-softmmu.mak, let's use the new Kconfig files
to express the necessary dependencies: The S390_CCW_VIRTIO config switch
for the "s390-ccw-virtio" machine now selects all non-optional devices.

And since we already have the VIRTIO_PCI and VIRTIO_MMIO config switches
for the other two virtio transports, this patch also introduces a new
config switch VIRTIO_CCW for the third, s390x-specific virtio transport,
so that all three virtio transports are now handled in the same way.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 This goes on top of the curren Kconfig series:

 Based-on: 1548410831-19553-1-git-send-email-pbonzini@redhat.com

 default-configs/s390x-softmmu.mak | 12 ++++++++----
 hw/s390x/Kconfig                  |  4 ++++
 hw/s390x/Makefile.objs            |  2 +-
 hw/virtio/Kconfig                 |  4 ++++
 4 files changed, 17 insertions(+), 5 deletions(-)

Comments

Cornelia Huck Jan. 28, 2019, 4:08 p.m. UTC | #1
On Mon, 28 Jan 2019 16:29:26 +0100
Thomas Huth <thuth@redhat.com> wrote:

> Instead of hard-coding all config switches in the config file
> default-configs/s390x-softmmu.mak, let's use the new Kconfig files
> to express the necessary dependencies: The S390_CCW_VIRTIO config switch
> for the "s390-ccw-virtio" machine now selects all non-optional devices.
> 
> And since we already have the VIRTIO_PCI and VIRTIO_MMIO config switches
> for the other two virtio transports, this patch also introduces a new
> config switch VIRTIO_CCW for the third, s390x-specific virtio transport,
> so that all three virtio transports are now handled in the same way.

I haven't found time to look at the Kconfig patches, so take the
following with a generous amount of salt.

> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  This goes on top of the curren Kconfig series:
> 
>  Based-on: 1548410831-19553-1-git-send-email-pbonzini@redhat.com
> 
>  default-configs/s390x-softmmu.mak | 12 ++++++++----
>  hw/s390x/Kconfig                  |  4 ++++
>  hw/s390x/Makefile.objs            |  2 +-
>  hw/virtio/Kconfig                 |  4 ++++
>  4 files changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/default-configs/s390x-softmmu.mak b/default-configs/s390x-softmmu.mak
> index 2be5059..08b1683 100644
> --- a/default-configs/s390x-softmmu.mak
> +++ b/default-configs/s390x-softmmu.mak
> @@ -1,9 +1,13 @@
> -CONFIG_PCI=y
> +# Default configuration for s390x-softmmu
> +
> +# Optional devices:
> +#
>  CONFIG_VIRTIO_PCI=y
> -CONFIG_SCLPCONSOLE=y
>  CONFIG_TERMINAL3270=y
> -CONFIG_S390_FLIC=y
>  CONFIG_WDT_DIAG288=y
> -CONFIG_S390_CCW_VIRTIO=y
>  CONFIG_VFIO_CCW=y
>  CONFIG_VFIO_AP=y

This looks sensible.

> +
> +# Boards:
> +#
> +CONFIG_S390_CCW_VIRTIO=y
> diff --git a/hw/s390x/Kconfig b/hw/s390x/Kconfig
> index 303db7f..9a36c39 100644
> --- a/hw/s390x/Kconfig
> +++ b/hw/s390x/Kconfig
> @@ -1,2 +1,6 @@
>  config S390_CCW_VIRTIO
>      bool
> +    select PCI
> +    select S390_FLIC
> +    select SCLPCONSOLE
> +    select VIRTIO_CCW

This also makes sense. (I assume CONFIG_PCI, CONFIG_S390_FLIC, and
CONFIG_SCLPCONSOLE are already defined in the base patch series.)

> diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
> index a884aae..0dc798a 100644
> --- a/hw/s390x/Makefile.objs
> +++ b/hw/s390x/Makefile.objs
> @@ -8,7 +8,7 @@ obj-y += ipl.o
>  obj-y += css.o
>  obj-y += s390-virtio-ccw.o
>  obj-y += 3270-ccw.o

Should this depend on TERMINAL3270 (at least in the future)?

> -obj-y += virtio-ccw.o
> +obj-$(CONFIG_VIRTIO_CCW) += virtio-ccw.o
>  obj-$(CONFIG_VIRTIO_SERIAL) += virtio-ccw-serial.o

Hm. Should the individual device types depend both on VIRTIO_<device>
and on VIRTIO_CCW? Making VIRTIO_<device> depend on VIRTIO_PCI ||
VIRTIO_CCW || VIRTIO_MMIO is probably not enough, as we could be trying
to build virtio-ccw devices if only VIRTIO_PCI is set (or virtio-pci
devices if only VIRTIO_CCW is set, for that matter.)

Or maybe I simply need to read the patches and find the answer there :)

>  obj-$(CONFIG_VIRTIO_BALLOON) += virtio-ccw-balloon.o
>  obj-$(CONFIG_VIRTIO_SCSI) += virtio-ccw-scsi.o
> diff --git a/hw/virtio/Kconfig b/hw/virtio/Kconfig
> index 74f4573..e0452de 100644
> --- a/hw/virtio/Kconfig
> +++ b/hw/virtio/Kconfig
> @@ -16,6 +16,10 @@ config VIRTIO_MMIO
>      bool
>      select VIRTIO
>  
> +config VIRTIO_CCW
> +    bool
> +    select VIRTIO
> +
>  config VIRTIO_BALLOON
>      bool
>      default y
Thomas Huth Jan. 29, 2019, 9:08 a.m. UTC | #2
On 2019-01-28 17:08, Cornelia Huck wrote:
> On Mon, 28 Jan 2019 16:29:26 +0100
> Thomas Huth <thuth@redhat.com> wrote:
> 
>> Instead of hard-coding all config switches in the config file
>> default-configs/s390x-softmmu.mak, let's use the new Kconfig files
>> to express the necessary dependencies: The S390_CCW_VIRTIO config switch
>> for the "s390-ccw-virtio" machine now selects all non-optional devices.
>>
>> And since we already have the VIRTIO_PCI and VIRTIO_MMIO config switches
>> for the other two virtio transports, this patch also introduces a new
>> config switch VIRTIO_CCW for the third, s390x-specific virtio transport,
>> so that all three virtio transports are now handled in the same way.
> 
> I haven't found time to look at the Kconfig patches, so take the
> following with a generous amount of salt.
> 
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  This goes on top of the curren Kconfig series:
>>
>>  Based-on: 1548410831-19553-1-git-send-email-pbonzini@redhat.com
>>
>>  default-configs/s390x-softmmu.mak | 12 ++++++++----
>>  hw/s390x/Kconfig                  |  4 ++++
>>  hw/s390x/Makefile.objs            |  2 +-
>>  hw/virtio/Kconfig                 |  4 ++++
>>  4 files changed, 17 insertions(+), 5 deletions(-)
>>
>> diff --git a/default-configs/s390x-softmmu.mak b/default-configs/s390x-softmmu.mak
>> index 2be5059..08b1683 100644
>> --- a/default-configs/s390x-softmmu.mak
>> +++ b/default-configs/s390x-softmmu.mak
>> @@ -1,9 +1,13 @@
>> -CONFIG_PCI=y
>> +# Default configuration for s390x-softmmu
>> +
>> +# Optional devices:
>> +#
>>  CONFIG_VIRTIO_PCI=y
>> -CONFIG_SCLPCONSOLE=y
>>  CONFIG_TERMINAL3270=y
>> -CONFIG_S390_FLIC=y
>>  CONFIG_WDT_DIAG288=y
>> -CONFIG_S390_CCW_VIRTIO=y
>>  CONFIG_VFIO_CCW=y
>>  CONFIG_VFIO_AP=y
> 
> This looks sensible.
> 
>> +
>> +# Boards:
>> +#
>> +CONFIG_S390_CCW_VIRTIO=y
>> diff --git a/hw/s390x/Kconfig b/hw/s390x/Kconfig
>> index 303db7f..9a36c39 100644
>> --- a/hw/s390x/Kconfig
>> +++ b/hw/s390x/Kconfig
>> @@ -1,2 +1,6 @@
>>  config S390_CCW_VIRTIO
>>      bool
>> +    select PCI
>> +    select S390_FLIC
>> +    select SCLPCONSOLE
>> +    select VIRTIO_CCW
> 
> This also makes sense. (I assume CONFIG_PCI, CONFIG_S390_FLIC, and
> CONFIG_SCLPCONSOLE are already defined in the base patch series.)

Yes, they are.

>> diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
>> index a884aae..0dc798a 100644
>> --- a/hw/s390x/Makefile.objs
>> +++ b/hw/s390x/Makefile.objs
>> @@ -8,7 +8,7 @@ obj-y += ipl.o
>>  obj-y += css.o
>>  obj-y += s390-virtio-ccw.o
>>  obj-y += 3270-ccw.o
> 
> Should this depend on TERMINAL3270 (at least in the future)?

Yes, this is related to TERMINAL3270 which is already used for
hw/char/terminal3270. So I think we could use $(CONFIG_TERMINAL3270)
here, too.

>> -obj-y += virtio-ccw.o
>> +obj-$(CONFIG_VIRTIO_CCW) += virtio-ccw.o
>>  obj-$(CONFIG_VIRTIO_SERIAL) += virtio-ccw-serial.o
> 
> Hm. Should the individual device types depend both on VIRTIO_<device>
> and on VIRTIO_CCW? Making VIRTIO_<device> depend on VIRTIO_PCI ||
> VIRTIO_CCW || VIRTIO_MMIO is probably not enough, as we could be trying
> to build virtio-ccw devices if only VIRTIO_PCI is set (or virtio-pci
> devices if only VIRTIO_CCW is set, for that matter.)

You're right, the whole section should be fenced via

ifeq ($(CONFIG_VIRTIO_CCW),y)
...
endif

I'll update my patch accordingly and send a v2.

 Thanks,
  Thomas
diff mbox series

Patch

diff --git a/default-configs/s390x-softmmu.mak b/default-configs/s390x-softmmu.mak
index 2be5059..08b1683 100644
--- a/default-configs/s390x-softmmu.mak
+++ b/default-configs/s390x-softmmu.mak
@@ -1,9 +1,13 @@ 
-CONFIG_PCI=y
+# Default configuration for s390x-softmmu
+
+# Optional devices:
+#
 CONFIG_VIRTIO_PCI=y
-CONFIG_SCLPCONSOLE=y
 CONFIG_TERMINAL3270=y
-CONFIG_S390_FLIC=y
 CONFIG_WDT_DIAG288=y
-CONFIG_S390_CCW_VIRTIO=y
 CONFIG_VFIO_CCW=y
 CONFIG_VFIO_AP=y
+
+# Boards:
+#
+CONFIG_S390_CCW_VIRTIO=y
diff --git a/hw/s390x/Kconfig b/hw/s390x/Kconfig
index 303db7f..9a36c39 100644
--- a/hw/s390x/Kconfig
+++ b/hw/s390x/Kconfig
@@ -1,2 +1,6 @@ 
 config S390_CCW_VIRTIO
     bool
+    select PCI
+    select S390_FLIC
+    select SCLPCONSOLE
+    select VIRTIO_CCW
diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
index a884aae..0dc798a 100644
--- a/hw/s390x/Makefile.objs
+++ b/hw/s390x/Makefile.objs
@@ -8,7 +8,7 @@  obj-y += ipl.o
 obj-y += css.o
 obj-y += s390-virtio-ccw.o
 obj-y += 3270-ccw.o
-obj-y += virtio-ccw.o
+obj-$(CONFIG_VIRTIO_CCW) += virtio-ccw.o
 obj-$(CONFIG_VIRTIO_SERIAL) += virtio-ccw-serial.o
 obj-$(CONFIG_VIRTIO_BALLOON) += virtio-ccw-balloon.o
 obj-$(CONFIG_VIRTIO_SCSI) += virtio-ccw-scsi.o
diff --git a/hw/virtio/Kconfig b/hw/virtio/Kconfig
index 74f4573..e0452de 100644
--- a/hw/virtio/Kconfig
+++ b/hw/virtio/Kconfig
@@ -16,6 +16,10 @@  config VIRTIO_MMIO
     bool
     select VIRTIO
 
+config VIRTIO_CCW
+    bool
+    select VIRTIO
+
 config VIRTIO_BALLOON
     bool
     default y