On Wed, Feb 02 2022, Alex Williamson <alex.williamson@xxxxxxxxxx> wrote: > On Tue, 1 Feb 2022 20:24:59 -0400 > Jason Gunthorpe <jgg@xxxxxxxxxx> wrote: > >> On Tue, Feb 01, 2022 at 02:49:16PM -0700, Alex Williamson wrote: >> > On Tue, 1 Feb 2022 14:36:20 -0400 >> > Jason Gunthorpe <jgg@xxxxxxxxxx> wrote: >> > > I don't want to touch capabilities, but we can try to use feature for >> > > set state. Please confirm this is what you want. >> > >> > It's a team sport, but to me it seems like it fits well both in my >> > mental model of interacting with a device feature, without >> > significantly altering the uAPI you're defining anyway. >> >> Well, my advice is that ioctls are fine, and a bit easier all around. >> eg strace and syzkaller are a bit easier if everything neatly maps >> into one struct per ioctl - their generator tools are optimized for >> this common case. >> >> Simple multiplexors are next-best-fine, but there should be a clear >> idea when to use the multiplexer, or not. >> >> Things like the cap chains enter a whole world of adventure for >> strace/syzkaller :) > > vfio's argsz/flags is not only a standard framework, but it's one that > promotes extensions. We were able to add capability chains with > backwards compatibility because of this design. IMO, that's avoided > ioctl sprawl; we've been able to maintain a fairly small set of core > ioctls rather than add add a new ioctl every time we want to describe > some new property of a device or region or IOMMU. I think that > improves the usability of the uAPI. I certainly wouldn't want to > program to a uAPI with a million ioctls. A counter argument is that > we're making the interface more complex, but at the same time we're > adding shared infrastructure for dealing with that complexity. > > Of course we do continue to add new ioctls as necessary, including this > FEATURE ioctl, and I recognize that with such a generic multiplexer we > run the risk of over using it, ie. everything looks like a nail. You > initially did not see the fit for setting device state as interacting > with a device feature, but it doesn't seem like you had a strong > objection to my explanation of it in that context. > > So I think if the FEATURE ioctl has an ongoing place in our uAPI (using > it to expose migration flags would seem to be a point in that > direction) and it doesn't require too many contortions to think of the > operation we're trying to perform on the device as interacting with a > device FEATURE, and there are no functional or performance implications > of it, I would think we should use it. To do otherwise would suggest > that we should consider the FEATURE ioctl a failed experiment and not > continue to expand its use. > > I'd be interested to hear more input on this from the community. My personal take would be: a new ioctl is more suitable for things that may be implemented by different backends, but in a non-generic way, and for mandatory functionality; the FEATURE ioctl is more suitable for things that either are very specific to a certain backend (i.e. don't reserve an ioctl for something that will only ever be used on one platform), or for things that have a lot of commonality for the backends that implement them (i.e. you are using a familiar scheme to interact with them.)
