Re: [PATCH 10/13] cxl/core: Map component registers for ports

From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
To: Dan Williams <dan.j.williams@intel.com>
Cc: Ben Widawsky <ben.widawsky@intel.com>,
	<linux-cxl@vger.kernel.org>,
	Alison Schofield <alison.schofield@intel.com>,
	Ira Weiny <ira.weiny@intel.com>,
	Vishal Verma <vishal.l.verma@intel.com>
Subject: Re: [PATCH 10/13] cxl/core: Map component registers for ports
Date: Mon, 13 Sep 2021 09:29:31 +0100	[thread overview]
Message-ID: <20210913092931.0000357b@Huawei.com> (raw)
In-Reply-To: <CAPcyv4jkw+QGwc3ZrQkVvOLcLpjstcyYror-TYKJQUZT1An9gA@mail.gmail.com>

On Fri, 10 Sep 2021 16:52:48 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> On Fri, Sep 3, 2021 at 9:14 AM Jonathan Cameron
> <Jonathan.Cameron@huawei.com> wrote:
> >
> > On Thu, 2 Sep 2021 12:50:14 -0700
> > Ben Widawsky <ben.widawsky@intel.com> wrote:
> >  
> > > Component registers are implemented for CXL.mem/cache operations. The
> > > cxl_pci driver handles enumerating CXL devices with the CXL.io protocol.
> > > The driver for managing CXL.mem/cache operations will need the component
> > > registers mapped and the mapping cannot be shared across two devices.
> > >
> > > For now, it's fine to relinquish this mapping in cxl_pci. CXL IDE is one
> > > exception (perhaps others will exist) where it might be desirable to
> > > have the cxl_pci driver do negotiation. For this case, it probably will
> > > make sense to create an ephemeral mapping. Further looking, there might
> > > need to be a cxl_core mechanism to allow arbitrating access to the
> > > component registers.  
> >
> >  
> > >
> > > Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>  
> >
> > As you predicted I don't like this. Needs some thought on how to get
> > around the mapping games though and it's Friday afternoon so I'm not
> > going to offer any concrete answers...
> >
> > Not totally obvious to me where RAS will be handled as well.
> > I think we definitely need an arbitration mechanism here.  
> 
> Poison consumption is handled via typical memory_failure(). Event
> record and event interrupts can be handled by the PCI driver
> potentially notifying the memdev driver if necessary. Port specific
> error handling (RAS component registers) can be handled by the port
> driver.
> 
> > Wouldn't it have been nice if all these capabilities had been nicely
> > padded so we could map them individually.  Oh well!
> > Gut feeling is this will only get worse for future versions of the spec
> > so we should assume there will be lots of stuff shoved in here.  
> 
> I'd prefer to run away from arbitration and towards sub-drivers
> providing services with distinct lines of ownership. In the rare cases
> where a driver can not act independently there should be a well
> defined driver API to request help from the register block owner, not
> pass around access to the register block.

Agreed. Where possible keep ownership well defined + API to deal
with the occasional cross over point.

J