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

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

On Mon, 13 Nov 2023 12:39:15 -0700
Sumanesh Samanta <sumanesh.samanta@broadcom.com> wrote:

> >>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 to break it out to a separate module. I was kind of assuming that
would happen later and the fact it isn't is more about history of this patch
set than anything else (predates MMPT surfacing)

> 
> Hi Dan, Jonathan,
> 
> Agree with the statement above, and in fact, I think that there should
> be a way applications can send CCI commands even from hosts without
> CXL root port.
> Consider a CXL switch that is connected to two hosts, one with CXL
> root port, and other having a pure PCIe root port.
> Ideally applications on either host should be able to communicate
> with/configure the switch in the same way from both the hosts (CXL
> capable or not).


Absolutely agree, but I think this should just work with the current code.
It's using stuff from the library module, not stuff that is loaded just
when the ACPI tables say it's a CXL system. We can reduce what is pulled
in by doing what Dan suggests but that's a software modularity question
rather than anything about the hardware supported.

> 
> If the CCI mailbox driver is completely dependent on the CXL root
> port, then it will not even load on a PCIe root port, even if we
> implement the 0x0c0b00 class code in a CXL switch.
> In this respect, a CXL switch can implement the PCIe MMPT interface
> too, so that PCIe based drivers can access that interface to send
> mailbox commands.
> The idea is, even if the CCI mailbox driver does not load in a non-CXL
> root port, an application can use the MMPT interface to manage the
> driver.
> Please let me know if you think that will work?
> 
> One potential problem I see is that in CXL root port, both CCI and
> MMPT mailbox will be available, which might lead to conflict if two
> applications use the two interfaces at the same time.
> Ultimately I think having a root port independent mailbox driver (that
> works in both CXL and PCIe root port) would be helpful for switches
> that can connect to both CXL and PCIe root ports.

'Watch this space' as interaction of MMPT and CXL is not
something the CXL spec speaks about and there are some corners
that need to be resolved.  Take questions about this to appropriate
standards orgs as we can't resolve this here.

However, the switch CCI is not associated with the root port it's associated
with a PCIe function that can be on a PCIe bus.  Whether we end up with
PCIe software support MMPT without needing to be part of a driver binding
to the class code (which inherently here means it understands the CXL mailbox)
is an interesting question.  I find it unlikely that Linux will support such
'bare' MMPT instances but maybe...

Jonathan


> 
> Would appreciate any insight on this.
> 
> sincerely,
> Sumanesh
> 
> 
> 
> 
> On Sun, Nov 12, 2023 at 5:10 PM 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.
> >
> > 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/.
> >
> > 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.
> >  
>