2021-02-23 06:27:58

by Jason Wang

[permalink] [raw]
Subject: [PATCH V4 1/3] virtio: don't prompt CONFIG_VIRTIO_PCI_MODERN

We used to prompt CONFIG_VIRTIO_PCI_MODERN to user which may bring a
lot of confusion. E.g it may break various default configs which want
virtio devices.

So this patch fixes this by hiding the prompot and documenting the
dependency. While at it, rename the module to VIRTIO_PCI_LIB.

Cc: Arnd Bergmann <[email protected]>
Cc: Anders Roxell <[email protected]>
Cc: Guenter Roeck <[email protected]>
Reported-by: Naresh Kamboju <[email protected]>
Fixes: 86b87c9d858b6 ("virtio-pci: introduce modern device module")
Signed-off-by: Jason Wang <[email protected]>
---
drivers/virtio/Kconfig | 11 ++++++-----
drivers/virtio/Makefile | 2 +-
2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
index 6b9b81f4b8c2..ce1b3f6ec325 100644
--- a/drivers/virtio/Kconfig
+++ b/drivers/virtio/Kconfig
@@ -12,13 +12,13 @@ config ARCH_HAS_RESTRICTED_VIRTIO_MEMORY_ACCESS
This option is selected if the architecture may need to enforce
VIRTIO_F_ACCESS_PLATFORM

-config VIRTIO_PCI_MODERN
- tristate "Modern Virtio PCI Device"
- depends on PCI
+config VIRTIO_PCI_LIB
+ tristate
help
Modern PCI device implementation. This module implements the
basic probe and control for devices which are based on modern
- PCI device with possible vendor specific extensions.
+ PCI device with possible vendor specific extensions. Any
+ module that selects this module must depend on PCI.

menuconfig VIRTIO_MENU
bool "Virtio drivers"
@@ -28,7 +28,8 @@ if VIRTIO_MENU

config VIRTIO_PCI
tristate "PCI driver for virtio devices"
- depends on VIRTIO_PCI_MODERN
+ depends on PCI
+ select VIRTIO_PCI_LIB
select VIRTIO
help
This driver provides support for virtio based paravirtual device
diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile
index f097578aaa8f..699bbea0465f 100644
--- a/drivers/virtio/Makefile
+++ b/drivers/virtio/Makefile
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_VIRTIO) += virtio.o virtio_ring.o
-obj-$(CONFIG_VIRTIO_PCI_MODERN) += virtio_pci_modern_dev.o
+obj-$(CONFIG_VIRTIO_PCI_LIB) += virtio_pci_modern_dev.o
obj-$(CONFIG_VIRTIO_MMIO) += virtio_mmio.o
obj-$(CONFIG_VIRTIO_PCI) += virtio_pci.o
virtio_pci-y := virtio_pci_modern.o virtio_pci_common.o
--
2.25.1


2021-02-23 08:11:23

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH V4 1/3] virtio: don't prompt CONFIG_VIRTIO_PCI_MODERN

On Tue, Feb 23, 2021 at 7:19 AM Jason Wang <[email protected]> wrote:
>
> We used to prompt CONFIG_VIRTIO_PCI_MODERN to user which may bring a
> lot of confusion. E.g it may break various default configs which want
> virtio devices.
>
> So this patch fixes this by hiding the prompot and documenting the
> dependency. While at it, rename the module to VIRTIO_PCI_LIB.
>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Anders Roxell <[email protected]>
> Cc: Guenter Roeck <[email protected]>

Acked-by: Arnd Bergmann <[email protected]>

2021-02-23 15:25:35

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH V4 1/3] virtio: don't prompt CONFIG_VIRTIO_PCI_MODERN

On 2/22/21 10:19 PM, Jason Wang wrote:
> We used to prompt CONFIG_VIRTIO_PCI_MODERN to user which may bring a
> lot of confusion. E.g it may break various default configs which want
> virtio devices.
>
> So this patch fixes this by hiding the prompot and documenting the
> dependency. While at it, rename the module to VIRTIO_PCI_LIB.
>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Anders Roxell <[email protected]>
> Cc: Guenter Roeck <[email protected]>
> Reported-by: Naresh Kamboju <[email protected]>
> Fixes: 86b87c9d858b6 ("virtio-pci: introduce modern device module")
> Signed-off-by: Jason Wang <[email protected]>

Acked-by: Guenter Roeck <[email protected]>

> ---
> drivers/virtio/Kconfig | 11 ++++++-----
> drivers/virtio/Makefile | 2 +-
> 2 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
> index 6b9b81f4b8c2..ce1b3f6ec325 100644
> --- a/drivers/virtio/Kconfig
> +++ b/drivers/virtio/Kconfig
> @@ -12,13 +12,13 @@ config ARCH_HAS_RESTRICTED_VIRTIO_MEMORY_ACCESS
> This option is selected if the architecture may need to enforce
> VIRTIO_F_ACCESS_PLATFORM
>
> -config VIRTIO_PCI_MODERN
> - tristate "Modern Virtio PCI Device"
> - depends on PCI
> +config VIRTIO_PCI_LIB
> + tristate
> help
> Modern PCI device implementation. This module implements the
> basic probe and control for devices which are based on modern
> - PCI device with possible vendor specific extensions.
> + PCI device with possible vendor specific extensions. Any
> + module that selects this module must depend on PCI.

Nit: "depends on PCI" might have been useful here, since the config
system would then complain about any violations.

Thanks,
Guenter

>
> menuconfig VIRTIO_MENU
> bool "Virtio drivers"
> @@ -28,7 +28,8 @@ if VIRTIO_MENU
>
> config VIRTIO_PCI
> tristate "PCI driver for virtio devices"
> - depends on VIRTIO_PCI_MODERN
> + depends on PCI
> + select VIRTIO_PCI_LIB
> select VIRTIO
> help
> This driver provides support for virtio based paravirtual device
> diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile
> index f097578aaa8f..699bbea0465f 100644
> --- a/drivers/virtio/Makefile
> +++ b/drivers/virtio/Makefile
> @@ -1,6 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0
> obj-$(CONFIG_VIRTIO) += virtio.o virtio_ring.o
> -obj-$(CONFIG_VIRTIO_PCI_MODERN) += virtio_pci_modern_dev.o
> +obj-$(CONFIG_VIRTIO_PCI_LIB) += virtio_pci_modern_dev.o
> obj-$(CONFIG_VIRTIO_MMIO) += virtio_mmio.o
> obj-$(CONFIG_VIRTIO_PCI) += virtio_pci.o
> virtio_pci-y := virtio_pci_modern.o virtio_pci_common.o
>

2021-02-23 15:54:55

by Daniel Díaz

[permalink] [raw]
Subject: Re: [PATCH V4 1/3] virtio: don't prompt CONFIG_VIRTIO_PCI_MODERN

Hello!

If I may add to the nit-picking...

On Tue, 23 Feb 2021 at 09:23, Guenter Roeck <[email protected]> wrote:
> On 2/22/21 10:19 PM, Jason Wang wrote:
> > We used to prompt CONFIG_VIRTIO_PCI_MODERN to user which may bring a
> > lot of confusion. E.g it may break various default configs which want
> > virtio devices.
> >
> > So this patch fixes this by hiding the prompot and documenting the

*prompt

Greetings!

Daniel Díaz
[email protected]

2021-02-23 15:57:42

by Michael S. Tsirkin

[permalink] [raw]
Subject: Re: [PATCH V4 1/3] virtio: don't prompt CONFIG_VIRTIO_PCI_MODERN

On Tue, Feb 23, 2021 at 09:50:35AM -0600, Daniel D?az wrote:
> Hello!
>
> If I may add to the nit-picking...
>
> On Tue, 23 Feb 2021 at 09:23, Guenter Roeck <[email protected]> wrote:
> > On 2/22/21 10:19 PM, Jason Wang wrote:
> > > We used to prompt CONFIG_VIRTIO_PCI_MODERN to user which may bring a
> > > lot of confusion. E.g it may break various default configs which want
> > > virtio devices.
> > >
> > > So this patch fixes this by hiding the prompot and documenting the
>
> *prompt
>
> Greetings!
>
> Daniel D?az
> [email protected]

I have just folded this patch into the one it's fixing,
so the commit log doesn't matter. Thanks everyone!

--
MST

2021-02-24 13:18:34

by Christian Borntraeger

[permalink] [raw]
Subject: Re: [PATCH V4 1/3] virtio: don't prompt CONFIG_VIRTIO_PCI_MODERN



On 23.02.21 07:19, Jason Wang wrote:
> We used to prompt CONFIG_VIRTIO_PCI_MODERN to user which may bring a
> lot of confusion. E.g it may break various default configs which want
> virtio devices.
>
> So this patch fixes this by hiding the prompot and documenting the
> dependency. While at it, rename the module to VIRTIO_PCI_LIB.
>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Anders Roxell <[email protected]>
> Cc: Guenter Roeck <[email protected]>
> Reported-by: Naresh Kamboju <[email protected]>
> Fixes: 86b87c9d858b6 ("virtio-pci: introduce modern device module")
> Signed-off-by: Jason Wang <[email protected]>

Michael,

Can we please add this NOW to next? virtio-pci is broken without it on
many defconfigs including s390. So linux-next is broken for more than
2 weeks now and it actively breaks several parts of our CI.
I guess there are other CIs that will not run several testcases because
of this. And Naresh reported that on Feb 9th.
There IS value in CI tools on next. Not caring about regressions introduced
by a tree in next is harmful. Especially when we are close or in the
merge window. So please: either fix things quickly OR revert.

Christian



> ---
> drivers/virtio/Kconfig | 11 ++++++-----
> drivers/virtio/Makefile | 2 +-
> 2 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
> index 6b9b81f4b8c2..ce1b3f6ec325 100644
> --- a/drivers/virtio/Kconfig
> +++ b/drivers/virtio/Kconfig
> @@ -12,13 +12,13 @@ config ARCH_HAS_RESTRICTED_VIRTIO_MEMORY_ACCESS
> This option is selected if the architecture may need to enforce
> VIRTIO_F_ACCESS_PLATFORM
>
> -config VIRTIO_PCI_MODERN
> - tristate "Modern Virtio PCI Device"
> - depends on PCI
> +config VIRTIO_PCI_LIB
> + tristate
> help
> Modern PCI device implementation. This module implements the
> basic probe and control for devices which are based on modern
> - PCI device with possible vendor specific extensions.
> + PCI device with possible vendor specific extensions. Any
> + module that selects this module must depend on PCI.
>
> menuconfig VIRTIO_MENU
> bool "Virtio drivers"
> @@ -28,7 +28,8 @@ if VIRTIO_MENU
>
> config VIRTIO_PCI
> tristate "PCI driver for virtio devices"
> - depends on VIRTIO_PCI_MODERN
> + depends on PCI
> + select VIRTIO_PCI_LIB
> select VIRTIO
> help
> This driver provides support for virtio based paravirtual device
> diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile
> index f097578aaa8f..699bbea0465f 100644
> --- a/drivers/virtio/Makefile
> +++ b/drivers/virtio/Makefile
> @@ -1,6 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0
> obj-$(CONFIG_VIRTIO) += virtio.o virtio_ring.o
> -obj-$(CONFIG_VIRTIO_PCI_MODERN) += virtio_pci_modern_dev.o
> +obj-$(CONFIG_VIRTIO_PCI_LIB) += virtio_pci_modern_dev.o
> obj-$(CONFIG_VIRTIO_MMIO) += virtio_mmio.o
> obj-$(CONFIG_VIRTIO_PCI) += virtio_pci.o
> virtio_pci-y := virtio_pci_modern.o virtio_pci_common.o
>