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

From: Bjorn Helgaas <helgaas@kernel.org>
To: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
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: Tue, 24 Jan 2023 17:51:55 -0600	[thread overview]
Message-ID: <20230124235155.GA1114594@bhelgaas> (raw)
In-Reply-To: <20230124124315.00000a5c@Huawei.com>

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.

> 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>