Re: [PATCH V2 1/1] PCI/DOE: Fix maximum data object length miscalculation

[Bjorn Helgaas <helgaas@kernel.org>]

On Wed, Nov 16, 2022 at 09:56:37AM +0800, Li Ming wrote:
> The value of data object length 0x0 indicates 2^18 dwords being
> transferred. This patch adjusts the value of data object length for the
> above case on both sending side and receiving side.
> 
> Besides, it is unnecessary to check whether length is greater than
> SZ_1M while receiving a response data object, because length from LENGTH
> field of data object header, max value is 2^18.
> 
> Signed-off-by: Li Ming <ming4.li@intel.com>

Applied with Reviewed-by from Jonathan and Lukas, thank you very much
to all of you!

I touched up the commit log; let me know if I made anything worse:

    PCI/DOE: Fix maximum data object length miscalculation
    
    Per PCIe r6.0, sec 6.30.1, a data object Length of 0x0 indicates 2^18
    DWORDs (256K DW or 1MB) being transferred.  Adjust the value of data object
    length for this case on both sending side and receiving side.
    
    Don't bother checking whether Length is greater than SZ_1M because all
    values of the 18-bit Length field are valid, and it is impossible to
    represent anything larger than SZ_1M:
    
      0x00000    256K DW (1M bytes)
      0x00001       1 DW (4 bytes)
      ...
      0x3ffff  256K-1 DW (1M - 4 bytes)

> ---
> v1->v2:
>  * Fix the value of PCI_DOE_MAX_LENGTH
>  * Moving length checking closer to transferring process
>  * Add a missing bracket
>  * Adjust patch description
> ---
>  drivers/pci/doe.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/doe.c b/drivers/pci/doe.c
> index e402f05068a5..66d9ab288646 100644
> --- a/drivers/pci/doe.c
> +++ b/drivers/pci/doe.c
> @@ -29,6 +29,9 @@
>  #define PCI_DOE_FLAG_CANCEL	0
>  #define PCI_DOE_FLAG_DEAD	1
>  
> +/* Max data object length is 2^18 dwords */
> +#define PCI_DOE_MAX_LENGTH	(1 << 18)
> +
>  /**
>   * struct pci_doe_mb - State for a single DOE mailbox
>   *
> @@ -107,6 +110,7 @@ static int pci_doe_send_req(struct pci_doe_mb *doe_mb,
>  {
>  	struct pci_dev *pdev = doe_mb->pdev;
>  	int offset = doe_mb->cap_offset;
> +	size_t length;
>  	u32 val;
>  	int i;
>  
> @@ -123,15 +127,20 @@ static int pci_doe_send_req(struct pci_doe_mb *doe_mb,
>  	if (FIELD_GET(PCI_DOE_STATUS_ERROR, val))
>  		return -EIO;
>  
> +	/* Length is 2 DW of header + length of payload in DW */
> +	length = 2 + task->request_pl_sz / sizeof(u32);
> +	if (length > PCI_DOE_MAX_LENGTH)
> +		return -EIO;
> +	if (length == PCI_DOE_MAX_LENGTH)
> +		length = 0;
> +
>  	/* Write DOE Header */
>  	val = FIELD_PREP(PCI_DOE_DATA_OBJECT_HEADER_1_VID, task->prot.vid) |
>  		FIELD_PREP(PCI_DOE_DATA_OBJECT_HEADER_1_TYPE, task->prot.type);
>  	pci_write_config_dword(pdev, offset + PCI_DOE_WRITE, val);
> -	/* Length is 2 DW of header + length of payload in DW */
>  	pci_write_config_dword(pdev, offset + PCI_DOE_WRITE,
>  			       FIELD_PREP(PCI_DOE_DATA_OBJECT_HEADER_2_LENGTH,
> -					  2 + task->request_pl_sz /
> -						sizeof(u32)));
> +					  length));
>  	for (i = 0; i < task->request_pl_sz / sizeof(u32); i++)
>  		pci_write_config_dword(pdev, offset + PCI_DOE_WRITE,
>  				       task->request_pl[i]);
> @@ -178,7 +187,10 @@ static int pci_doe_recv_resp(struct pci_doe_mb *doe_mb, struct pci_doe_task *tas
>  	pci_write_config_dword(pdev, offset + PCI_DOE_READ, 0);
>  
>  	length = FIELD_GET(PCI_DOE_DATA_OBJECT_HEADER_2_LENGTH, val);
> -	if (length > SZ_1M || length < 2)
> +	/* A value of 0x0 indicates max data object length */
> +	if (!length)
> +		length = PCI_DOE_MAX_LENGTH;
> +	if (length < 2)
>  		return -EIO;
>  
>  	/* First 2 dwords have already been read */
> -- 
> 2.25.1
>