linux-kernel - Re: [PATCH v3 05/14] s390: vfio-ap: base implementation of VFIO AP device driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 3 Apr 2018 11:56:08 +0200
From:   Cornelia Huck <cohuck@...hat.com>
To:     Pierre Morel <pmorel@...ux.vnet.ibm.com>
Cc:     Tony Krowiak <akrowiak@...ux.vnet.ibm.com>,
        linux-s390@...r.kernel.org, linux-kernel@...r.kernel.org,
        kvm@...r.kernel.org, freude@...ibm.com, schwidefsky@...ibm.com,
        heiko.carstens@...ibm.com, borntraeger@...ibm.com,
        kwankhede@...dia.com, bjsdjshi@...ux.vnet.ibm.com,
        pbonzini@...hat.com, alex.williamson@...hat.com,
        alifm@...ux.vnet.ibm.com, mjrosato@...ux.vnet.ibm.com,
        jjherne@...ux.vnet.ibm.com, thuth@...hat.com,
        pasic@...ux.vnet.ibm.com, berrange@...hat.com,
        fiuczy@...ux.vnet.ibm.com, buendgen@...ibm.com
Subject: Re: [PATCH v3 05/14] s390: vfio-ap: base implementation of VFIO AP
 device driver

On Tue, 27 Mar 2018 16:45:02 +0200
Pierre Morel <pmorel@...ux.vnet.ibm.com> wrote:

> On 27/03/2018 13:17, Cornelia Huck wrote:
> > On Thu, 15 Mar 2018 13:25:25 -0400
> > Tony Krowiak <akrowiak@...ux.vnet.ibm.com> wrote:
> >  
> >> On 03/15/2018 09:25 AM, Pierre Morel wrote:  
> >>> On 14/03/2018 19:25, Tony Krowiak wrote:  
> >>>> +config VFIO_AP
> >>>> +    def_tristate m  
> >>> not sure it must be module by default.
> >>> I would not set it by default.  
> >> Connie also asked about this in the last review, so I will go ahead
> >> and change it.  
> >>>     
> >>>> +    prompt "VFIO support for AP devices"
> >>>> +    depends on ZCRYPT && VFIO_MDEV_DEVICE  
> >>> VFIO_MDEV_DEVICE is a general feature *needed* by VFIO_AP
> >>> and has no use case by its own. If it is set it is obviously because some
> >>> mediated device drivers needs it.
> >>> while ZCRYPT is a Z feature which may be set without VFIO_AP.
> >>>
> >>> So you need:
> >>>
> >>> config VFIO_AP
> >>>      def_tristate n
> >>>      prompt "VFIO support for AP devices"
> >>>      depends on ZCRYPT
> >>>      select VFIO_MDEV
> >>>      select VFIO_MDEV_DEVICE
> >>> ...  
> >> I was thinking the same just yesterday and I agree, this makes sense.  
> > OTOH, nobody else seems to do a select on these symbols so far.
> >
> > If you decide to go that route, you'll also need to depend on VFIO  
> 
> I think a select is better (again).
> 
> > (otherwise you could end up selecting symbols with unmet dependencies).
> > All in all, I prefer the 'depends' approach.
> >  
> Why do you prefer this approach?

Hm, I thought I had already written a mail, but apparently I didn't....
 
> I can tell you why I prefer a mixed approach:
> 
> We have two tools, depends and select.
> 
> It seems to me that depends should be used for things we can not choose 
> to be there or not, but things that just are there, like hardware 
> dependencies. For example MMU, CPU type, CRYPTO hardware...
> 
> Select on the other hand is useful to choose things that we need like 
> libraries, VFIO, VIRTIO, crypto libraries etc.
> 
> Using this policy is clear and makes easy to choose functionalities and 
> get the utilities automatically.
> 
> On the other hand, only using depends makes things to hide the 
> functionalities behind the utilities.

My view is the following:
- select is useful for library functionality or for enabling
  architecture-specific optimizations (the HAVE_xxx symbols),
  especially things you don't want the user to deal with. If you select
  something, you need to take care of any dependencies yourself.
- depends is useful for more complex dependencies, and especially
  things you don't want automagically enabled. [In modern menuconfig,
  it is easy to figure out any missing dependencies for a config option
  anyway.]

The mdev infrastructure is too complex to be considered a simple
library IMO (cf. the missing VFIO dependency).