Re: [PATCH 3/5] cxl/pci: Add DOE Auxiliary Devices

From: Dan Williams <dan.j.williams@intel.com>
To: Christoph Hellwig <hch@lst.de>
Cc: Bjorn Helgaas <helgaas@kernel.org>,
	Jonathan Cameron <Jonathan.Cameron@huawei.com>,
	"Weiny, Ira" <ira.weiny@intel.com>,
	Alison Schofield <alison.schofield@intel.com>,
	Vishal Verma <vishal.l.verma@intel.com>,
	Ben Widawsky <ben.widawsky@intel.com>,
	Bjorn Helgaas <bhelgaas@google.com>,
	linux-cxl@vger.kernel.org, Linux PCI <linux-pci@vger.kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH 3/5] cxl/pci: Add DOE Auxiliary Devices
Date: Mon, 29 Nov 2021 15:59:25 -0800	[thread overview]
Message-ID: <CAPcyv4gYBpx6Anw__gW-3xZfbcTaVv5eUR6wuxt_Ert1N6hDZA@mail.gmail.com> (raw)
In-Reply-To: <CAPcyv4g+=fkMyzoKtRbJfFyM=hq3B=RMJotNWyGoJDZk0d38uQ@mail.gmail.com>

On Mon, Nov 29, 2021 at 3:37 PM Dan Williams <dan.j.williams@intel.com> wrote:
>
> On Thu, Nov 18, 2021 at 10:48 PM Christoph Hellwig <hch@lst.de> wrote:
> >
> > On Wed, Nov 17, 2021 at 04:15:36PM -0600, Bjorn Helgaas wrote:
> > > > Agreed though how it all gets tied together isn't totally clear
> > > > to me yet. The messy bit is interrupts given I don't think we have
> > > > a model for enabling those anywhere other than in individual PCI drivers.
> > >
> > > Ah.  Yeah, that is a little messy.  The only real precedent where the
> > > PCI core and a driver might need to coordinate on interrupts is the
> > > portdrv.  So far we've pretended that bridges do not have
> > > device-specific functionality that might require interrupts.  I don't
> > > think that's actually true, but we haven't integrated drivers for the
> > > tuning, performance monitoring, and similar features that bridges may
> > > have.  Yet.
> >
> > And portdrv really is conceptually part of the core PCI core, and
> > should eventually be fully integrated..
>
> What does a fully integrated portdrv look like? DOE enabling could
> follow a similar model.
>
> >
> > > In any case, I think the argument that DOE capabilities are not
> > > CXL-specific still holds.
> >
> > Agreed.
>
> I don't think anyone is arguing that DOE is something CXL specific.
> The enabling belongs only in drivers/pci/ as a DOE core, and then that
> core is referenced by any other random PCI driver that needs to
> interact with a DOE.
>
> The question is what does that DOE core look like? A Linux device
> representing the DOE capability and a common driver for the
> data-transfer seems a reasonable abstraction to me and that's what
> Auxiliary Bus offers.

I will also add that this is not just an argument to use a
device+driver organization for its own sake, it also allows for an
idiomatic ABI for determining when the kernel is using a device
capability vs when userspace is using it. For example,
IO_STRICT_DEVMEM currently locks out userspace MMIO when a kernel
driver is attached to a device. I see a need for a similar policy to
lock out userspace from configuration writes to the DOE while the
kernel is using the DOE. If userspace wants / needs access returned to
it then it can force unbind just that one DOE aux-driver rather than
unbind the driver for the entire device if DOE was just a library that
all drivers linked against.

DOE negotiates security features like SPDM and IDE. I think it is
important for the kernel to be able to control access to DOE instances
even though it has not cared about protecting itself from userspace
initiated configuration writes in the past.