Re: [PATCH 1/4] cxl: mbox: Preparatory move of functions to core/mbox.c and cxlmbox.h

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Sun, 12 Nov 2023 16:09:35 -0800
Dan Williams <dan.j.williams@intel.com> wrote:

> Jonathan Cameron wrote:
> > A later patch will modify this code to separate out the mbox
> > functionality from the memdev.  This patch is intended to make
> > that a little more readable.  Related move of cxl_err(), cxl_cmd_err()
> > to make them accessible from other parts of the cxl core code.
> > 
> > Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > ---
> >  drivers/cxl/core/mbox.c | 261 ++++++++++++++++++++++++++++++++++++++-
> >  drivers/cxl/cxlmbox.h   | 146 ++++++++++++++++++++++
> >  drivers/cxl/cxlmem.h    | 148 ++--------------------
> >  drivers/cxl/pci.c       | 265 +---------------------------------------
> >  4 files changed, 418 insertions(+), 402 deletions(-)  
> 
> Given that the CXL mailbox format is being adopted by other standards
> efforts in PCIe and OCP I would expect that this functionality is better
> served moving out of cxl_core.ko into its own compilation unit.

Makes sense.

> 
> Something like drivers/cxl/cci/, that CXL can select. The s/mbox/cci/
> rename is a proposal to both drop a letter out of the acronym and
> perhaps make the code a bit more discoverable / palatable for folks
> coming from those non-CXL specs to find the Linux code. However, if
> folks think that's too much thrash I am ok with drivers/cxl/mbox/.

Medium term I might see how bad it is to just rip it out of CXL entirely
and push it to drivers/pci/mmpt.c (with a few hooks to deal with
CXL parts). However, right now I think that's a step 2 (as MMPT whilst
published isn't widely implemented yet)

> 
> As for the policy for raw commands. I would still like for there to be
> some discipline of registratants to this facility to classify production
> vs debug commands to give Linux distributors a single place to set policy.
> I.e. keep up the need for consumers of this to define "Linux" commands
> to avoid the "raw" warning.
> 
> That said, how much of what switch CCI wants to do should be an ioctl()
> ABI vs sysfs / configfs? The raw commands are really only there for
> prototyping until production flows are established.

I'd love us to be in the world where the kernel modeled the topology
being controlled nicely even though it would get complex. There is also
a clear argument that this complexity isn't of interest to the kernel
so it doesn't really belong there anyway.
The main issue here is that the 'other' interface I'd expect a userspace tool
to be poking is MCTP.  There the software model is a network port.
So all the smarts pretty much have to be in userspace.

If the assumption is that it's all in userspace, the kernel won't have enough
visibility to do high quality sanity checking + pretty much everything you
set with the FMAPI is destructive, most likely to 'other hosts'.

We could maybe add commands (so no taint) for the non destructive stuff like
querying topology, and maybe even DCD add / remove (not forced) with the
tunneling wrap up pushed down into the kernel. I'll admit I'm doubtful and
think it likely any software stack is going to use raw anyway because they
have it all implemented for the MCTP path.

I expect we'll get requests to drop the taint on the Switch Mailbox CCI
raw command and it's much harder to argue against that it is for type 3
devices.

Jonathan