Received-SPF: pass (google.com: domain of linux-kernel+bounces-158972-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Thu, 25 Apr 2024 18:26:05 +0100
From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
To: Dan Williams <dan.j.williams@intel.com>
CC: <linux-cxl@vger.kernel.org>, Sreenivas Bagalkote
	<sreenivas.bagalkote@broadcom.com>, Brett Henning
	<brett.henning@broadcom.com>, Harold Johnson <harold.johnson@broadcom.com>,
	Sumanesh Samanta <sumanesh.samanta@broadcom.com>,
	<linux-kernel@vger.kernel.org>, Davidlohr Bueso <dave@stgolabs.net>, "Dave
 Jiang" <dave.jiang@intel.com>, Alison Schofield <alison.schofield@intel.com>,
	Vishal Verma <vishal.l.verma@intel.com>, "Ira Weiny" <ira.weiny@intel.com>,
	<linuxarm@huawei.com>, <linux-api@vger.kernel.org>, Lorenzo Pieralisi
	<lpieralisi@kernel.org>, "Natu, Mahesh" <mahesh.natu@intel.com>,
	<gregkh@linuxfoundation.org>
Subject: Re: RFC: Restricting userspace interfaces for CXL fabric management
Message-ID: <20240425182605.00005f22@Huawei.com>
In-Reply-To: <662a826dbeeff_b6e02943c@dwillia2-mobl3.amr.corp.intel.com.notmuch>
References: <20240321174423.00007e0d@Huawei.com>
	<66109191f3d6d_2583ad29468@dwillia2-xfh.jf.intel.com.notmuch>
	<20240410124517.000075f2@Huawei.com>
	<66284d5e8ac01_690a29431@dwillia2-xfh.jf.intel.com.notmuch>
	<20240425123344.000001a9@Huawei.com>
	<662a826dbeeff_b6e02943c@dwillia2-mobl3.amr.corp.intel.com.notmuch>
Organization: Huawei Technologies Research and Development (UK) Ltd.
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset="US-ASCII"
Content-Transfer-Encoding: 7bit

On Thu, 25 Apr 2024 09:18:53 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> Jonathan Cameron wrote:
> [..]
> > > Also, the assertion that these kernels will be built with
> > > CONFIG_SECURITY_LOCKDOWN_LSM=n and likely CONFIG_STRICT_DEVMEM=n, then
> > > the entire user-mode driver ABI is available for use. CXL commands are
> > > simple polled mmio, does Linux really benefit from carrying drivers in
> > > the kernel that the kernel itself does not care about?  
> > 
> > Sure we could it in userspace...  It's bad engineering, limits the design
> > to polling only and uses a bunch of interfaces we put a lot of effort into
> > telling people not to use except for debug.
> > 
> > I really don't see the advantage in pushing a project/group of projects
> > all of which are picking the upstream kernel up directly, to do a dirty
> > hack. We loose all the advantages of a proper well maintained kernel
> > driver purely on the argument that one use model is not the same as
> > this one.  Sensible security lockdown requirements is fine (along
> > with all the other kernel features that must be disable for that
> > to work), making open kernel development on for a large Linux
> > market harder is not.  
> 
> The minimum requirement for justifying an in kernel driver is that
> something else in the kernel consumes that facility. So, again, I want
> to get back to specifics what else in the kernel is going to leverage
> the Switch CCI mailbox?

Why?  I've never heard of such as a requirement and numerous drivers
provide fairly direct access to hardware. Sometimes there is a subsystem
aiding the data formatting etc, but fundamentally that's a convenience.

Taking this to a silly level, on this basis all networking drivers would
not be in the kernel.  They are there mainly to provide userspace access to
a network.  Any of the hardware access subsystems such hwmon, input, IIO
etc are primarily about providing a convenient way to get data to/from
a device.  They are kernel drivers because that is the cleaner path
for data marshaling, interrupt handling etc.

In kernel users are a perfectly valid reason to have a kernel driver,
but it's far from the only one.  None of the AI accelerators have in kernel
users today (maybe they will in future). Sure there are other arguments
that mean only a few such devices have been upstreamed, but it's not
that they need in kernel users. If it's really an issue I'll just submit
it to driver/misc and Greg can take a view on whether it's an acceptable
device to have driver for... (after he's asked the obvious question of
why aren't the CXL folk taking it!) +cc Greg to save providing info later.

For background this is a PCI function with a mailbox used for switch
configuration. The mailbox is identical to the one found on CXL type3
devices. Whole thing defined in the CXL spec. It gets a little complex
because you can tunnel commands to devices connected to the switch,
potentially affecting other hosts.  Typical Linux device doing this
would be a BMC, but there have been repeated questions about providing
a subset of access to any Linux system (avoiding the foot guns)
Whole thing fully discoverable - proposal is a standard PCI driver.

> 
> The generic-Type-3-device mailbox has an in kernel driver because the
> kernel has need to send mailbox commands internally and it is
> fundamental to RAS and provisioning flows that the kernel have this
> coordination. What are the motivations for an in-band Switch CCI command
> submission path?
> 
> It could be the case that you have a self-evident example in mind that I
> have thus far failed to realize.
> 

There are possibilities, but for now it's a transport driver just like
MCTP etc with a well defined chardev interface, with documented ioctl
interface etc (which I'd keep inline with one the CXL mailbox uses
just to avoid reinventing the wheel - I'd prefer to use that directly
to avoid divergence but I don't care that much).

As far as I can see, with the security / blast radius concern alleviated
by disabling this if lockdown is in use + taint for unaudited commands
(and a nasty sounding config similar to the cxl mailbox one)
there is little reason not to take such a driver into the kernel.
It has next to no maintenance impact outside of itself and a bit of
library code which I've proposed pushing down to the level of MMPT
(so PCI not CXL) if you think that is necessary. 

We want interrupt handling and basic access controls / command
interface to userspace.

Apologies if I'm grumpy - several long days of battling cpu hotplug code.

Jonathan