Re: [PATCH v2 2/2] drivers: gpio: add virtio-gpio guest driver

On Sat, Dec 05, 2020 at 09:05:16PM +0100, Enrico Weigelt, metux IT consult wrote:

On 05.12.20 20:32, Michael S. Tsirkin wrote:

Hi,

quoted

It seems a bit of a mess, at this point I'm not entirely sure when
should drivers select VIRTIO and when depend on it.

if VIRTIO just enables something that could be seen as library
functions, then select should be right, IMHO.

quoted

The text near it says:

# SPDX-License-Identifier: GPL-2.0-only
config VIRTIO
        tristate

oh, wait, doesn't have an menu text, so we can't even explicitly enable
it (not shown in menu) - only implicitly. Which means that some other
option must select it, in order to become availe at all, and in order
to make others depending on it becoming available.

IMHO, therefore select is the correct approach.

quoted

        help
          This option is selected by any driver which implements the virtio
          bus, such as CONFIG_VIRTIO_PCI, CONFIG_VIRTIO_MMIO, CONFIG_RPMSG
          or CONFIG_S390_GUEST.

Which seems clear enough and would indicate drivers for devices *behind*
the bus should not select VIRTIO and thus presumably should "depend on" it.
This is violated in virtio console and virtio fs drivers.

See above: NAK. because it can't even be enabled directly (by the user).
If it wasn't meant otherwise, we'd have to add an menu text.

The point is that user enables one of the bindings.
That in turn enables drivers. If we merely select VIRTIO
there's a chance user won't remember to select any bindings
and will be surprised not to see any devices.

quoted

For console it says:

commit 9f30eb29c514589e16f2999ea070598583d1f6ec
Author: Michal Suchanek [off-list ref]
Date:   Mon Aug 31 18:58:50 2020 +0200

    char: virtio: Select VIRTIO from VIRTIO_CONSOLE.
    
    Make it possible to have virtio console built-in when
    other virtio drivers are modular.
    
    Signed-off-by: Michal Suchanek [off-list ref]
    Reviewed-by: Amit Shah [off-list ref]
    Link: https://lore.kernel.org/r/20200831165850.26163-1-msuchanek@suse.de (local)
    Signed-off-by: Greg Kroah-Hartman [off-list ref]

which seems kind of bogus - why do we care about allowing a builtin
virtio console driver if the pci virtio bus driver is a module?
There won't be any devices on the bus to attach to ...

When using other transports ?

Any transport selects VIRTIO so if you enable that, you get
VIRTIO and thus it's enough to depend on it.

In my current project, eg. I'm using mmio - my kernel has pci completely
disabled.

quoted

I am inclined to fix console and virtio fs to depend on VIRTIO:
select is harder to use correctly ...

I don't thinkt that would be good - instead everybody should just select
VIRTIO, never depend on it (maybe depend on VIRTIO_MENU instead)

GPU depends on VIRTIO and on VIRTIO_MENU ... which seems even messier
...

--mtx

-- 
---
Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert
werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren
GPG/PGP-Schlüssel zu.
---
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization