Re: [PATCH v2 10/10] PCI/DOE: Relax restrictions on request and response size

From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: Lukas Wunner <lukas@wunner.de>, <linux-pci@vger.kernel.org>,
	Gregory Price <gregory.price@memverge.com>,
	Ira Weiny <ira.weiny@intel.com>,
	Dan Williams <dan.j.williams@intel.com>,
	Alison Schofield <alison.schofield@intel.com>,
	Vishal Verma <vishal.l.verma@intel.com>,
	Dave Jiang <dave.jiang@intel.com>,
	"Li, Ming" <ming4.li@intel.com>, Hillf Danton <hdanton@sina.com>,
	"Ben Widawsky" <bwidawsk@kernel.org>, <linuxarm@huawei.com>,
	<linux-cxl@vger.kernel.org>
Subject: Re: [PATCH v2 10/10] PCI/DOE: Relax restrictions on request and response size
Date: Wed, 25 Jan 2023 09:47:21 +0000	[thread overview]
Message-ID: <20230125094721.000061c1@Huawei.com> (raw)
In-Reply-To: <20230124235155.GA1114594@bhelgaas>

On Tue, 24 Jan 2023 17:51:55 -0600
Bjorn Helgaas <helgaas@kernel.org> wrote:

> On Tue, Jan 24, 2023 at 12:43:15PM +0000, Jonathan Cameron wrote:
> > On Mon, 23 Jan 2023 11:20:00 +0100
> > Lukas Wunner <lukas@wunner.de> wrote:
> >   
> > > An upcoming user of DOE is CMA (Component Measurement and Authentication,
> > > PCIe r6.0 sec 6.31).
> > > 
> > > It builds on SPDM (Security Protocol and Data Model):
> > > https://www.dmtf.org/dsp/DSP0274
> > > 
> > > SPDM message sizes are not always a multiple of dwords.  To transport
> > > them over DOE without using bounce buffers, allow sending requests and
> > > receiving responses whose final dword is only partially populated.
> > > 
> > > Tested-by: Ira Weiny <ira.weiny@intel.com>
> > > Signed-off-by: Lukas Wunner <lukas@wunner.de>
> > > Cc: Jonathan Cameron <Jonathan.Cameron@Huawei.com>  
> > Ah. This...
> > 
> > I can't immediately find the original discussion thread, but I'm fairly
> > sure we had a version of the DOE code that did this (maybe we just
> > discussed doing it and never had code...)
> > 
> > IIRC, at the time feedback was strongly in favour of making
> > the handling of non dword payloads a problem for the caller (e.g. PCI/CMA)
> > not the DOE core, mainly so that we could keep the layering simple.
> > DOE part of PCI spec says DWORD multiples only, CMA has non dword
> > entries.  
> 
> I can't remember, but I might have been the voice in favor of making
> it the caller's problem.  Your argument about dealing with it here
> makes a lot of sense, and I'm OK with it, but I *would* like to add
> some text to the commit log and comments in the code about what is
> happening here.  Otherwise there's an unexplained disconnect between
> the DWORD spec language and the byte-oriented code.

Absolutely agree. Calling out why we have a mismatch with the specification
will avoid a bunch of head scratching in the future!

> 
> > Personally I'm fully in favour of making our lives easier and handling
> > this at the DOE layer!  The CMA padding code is nasty as we have to deal
> > with caching just the right bits of the payload for the running hashes.
> > With it at this layer I'd imagine that code gets much simpler
> > 
> > Assuming resolution to Ira's question on endianness is resolved.
> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>  