Re: [PATCH v3 15/16] PCI/DOE: Relax restrictions on request and response size

["Li, Ming" <ming4.li@intel.com>]

On 2/11/2023 4:25 AM, Lukas Wunner 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.
> 
> To be clear, PCIe r6.0 sec 6.30.1 specifies the Data Object Header 2
> "Length" in dwords and pci_doe_send_req() and pci_doe_recv_resp()
> read/write dwords.  So from a spec point of view, DOE is still specified
> in dwords and allowing non-dword request/response buffers is merely for
> the convenience of callers.
> 
> Tested-by: Ira Weiny <ira.weiny@intel.com>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  Changes v2 -> v3:
>  * Fix byte order of last dword on big endian arches (Ira)
>  * Explain in commit message and kerneldoc that arbitrary-sized
>    payloads are not stipulated by the spec, but merely for
>    convenience of the caller (Bjorn, Jonathan)
>  * Add code comment that "remainder" in pci_doe_recv_resp() signifies
>    the number of data bytes in the last payload dword (Ira)
> 
>  drivers/pci/doe.c | 74 +++++++++++++++++++++++++++++++----------------
>  1 file changed, 49 insertions(+), 25 deletions(-)
>

......

> +
> +	if (payload_length) {
> +		/* Read all payload dwords except the last */
> +		for (; i < payload_length - 1; i++) {
> +			pci_read_config_dword(pdev, offset + PCI_DOE_READ,
> +					      &val);
> +			task->response_pl[i] = cpu_to_le32(val);
> +			pci_write_config_dword(pdev, offset + PCI_DOE_READ, 0);
> +		}
> +
> +		/* Read last payload dword */
>  		pci_read_config_dword(pdev, offset + PCI_DOE_READ, &val);
> -		task->response_pl[i] = cpu_to_le32(val);
> +		cpu_to_le32s(&val);
> +		memcpy(&task->response_pl[i], &val, remainder);

This "Read last payload dword" seems like making sense only when remainder != sizeof(u32).
If remainder == sizeof(u32), it should be read in above reading loops.
But this implementation also looks good to me.

Reviewed-by: Ming Li <ming4.li@intel.com>

>  		/* Prior to the last ack, ensure Data Object Ready */
> -		if (i == (payload_length - 1) && !pci_doe_data_obj_ready(doe_mb))
> +		if (!pci_doe_data_obj_ready(doe_mb))
>  			return -EIO;
>  		pci_write_config_dword(pdev, offset + PCI_DOE_READ, 0);
> +		i++;
>  	}
>  
>  	/* Flush excess length */