linux-kernel - Re: [PATCH V4 0/5] mlx5 ConnectX control misc 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] [day] [month] [year] [list]

Message-ID: <2024043046-morbidly-marbling-fe0d@gregkh>
Date: Tue, 30 Apr 2024 09:09:06 +0200
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: David Ahern <dsahern@...nel.org>
Cc: Jason Gunthorpe <jgg@...dia.com>, Christoph Hellwig <hch@...radead.org>,
	Saeed Mahameed <saeed@...nel.org>, Arnd Bergmann <arnd@...db.de>,
	Leon Romanovsky <leonro@...dia.com>, Jiri Pirko <jiri@...dia.com>,
	Leonid Bloch <lbloch@...dia.com>, Itay Avraham <itayavr@...dia.com>,
	Jakub Kicinski <kuba@...nel.org>,
	Saeed Mahameed <saeedm@...dia.com>,
	Aron Silverton <aron.silverton@...cle.com>,
	linux-kernel@...r.kernel.org,
	Andy Gospodarek <andrew.gospodarek@...adcom.com>
Subject: Re: [PATCH V4 0/5] mlx5 ConnectX control misc driver

On Mon, Apr 29, 2024 at 07:36:41PM -0600, David Ahern wrote:
> On 3/4/24 9:02 AM, Jason Gunthorpe wrote:
> > On Wed, Feb 14, 2024 at 01:57:35PM -0400, Jason Gunthorpe wrote:
> > 
> >> I also like this, I don't want the outcome of this discussion to be
> >> that only mlx5ctl gets merged. I want all the HW that has this problem
> >> to have support in the mainline kernel.
> > 
> > To this end here is my proposal to move forward with a new
> > mini-subsystem to provide rules for this common approach. Get the
> > existing tools out of hacky unstructured direct hardware access via
> > /sys/XX and into a formalized lockdown compatible system. I've talked
> > to enough people now to think we have a critical mass.
> > 
> > ===============
> > fwctl subsystem
> > ===============
> 
> 
> The aux bus infrastructure was created specifically for multifunction
> devices -- it allows a core PCI device driver with smaller, subsystem
> focused drivers for vendor specific implementations of various S/W stack
> APIs (IB, netdev, etc). One aspect not addressed in that design is where
> to park the various drivers and extensions that are not solely tied to
> any one subsystem.
> 
> Given, that how about moving the existing auxbus code into a new
> directory, drivers/auxbus. From there, create a subdirectory for this
> new fwctl subsystem which is most likely to be realized as a new auxbus
> device and then subdirectories for vendor specific drivers for the aux
> bus device. Then new drivers being developed in this auxbus world can
> put the core PCI device handling code under drivers/auxbus/core.
> 
> In short:
> - drivers/auxbus/auxiliary.c

Ah, the ever-frequent "where do we put the files" discussion :)

Is it "per bus" or "per functionality"?  In the end, it's a bit of both,
HOWEVER, no, auxiliary.c belongs in drivers/base/ for now, why move it?

And really, stuff on the auxbus has nothing to do with the auxbus, it
has everything to do with with the common device that auxbus really is
emulating (i.e. a pci device with multi-functions, or whatever.)

> - drivers/auxbus/core/<vendor>/  - per h/w device driver for managing
> the PCI device which is shared across multiple auxbus devices

Why "vendor"?  What happens when vendor names change?  :)

> - drivers/auxbus/fwctl/fwctl.c   - this FW interface

fwctl has nothing to do with auxbus other than it might be a user of it,
right?  So again, no need for it to be under auxbus/ as that is not
needed.

> - drivers/auxbus/fwctl/<vendor>/ - vendor specific driver for a fwctl
> auxbus device


if you want drivers/fwctl/<VENDOR>/ if <VENDOR> numbers get huge, sure
make subdirs, but really, why create a subdir like that if you don't
need it?

Start simple please, you can always move files later if it gets too
complex.  I think that discussing where to place non-existant files at
this point in time is kind of funny, given we have yet to see any actual
code...

thanks,

greg k-h