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

[Cornelia Huck]

On Tue, 27 Mar 2018 16:45:02 +0200
Pierre Morel <pmorel@xxxxxxxxxxxxxxxxxx> wrote:

> On 27/03/2018 13:17, Cornelia Huck wrote:
> > On Thu, 15 Mar 2018 13:25:25 -0400
> > Tony Krowiak <akrowiak@xxxxxxxxxxxxxxxxxx> 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).