Re: [PATCH net-next v2 2/4] NTB: ntb_transport: Use little-endian shared fields

[Dave Jiang <dave.jiang@intel.com>]

On 8/16/26 11:49 PM, Koichiro Den wrote:
> ntb_transport writes payload headers and the RX ring tail with
> iowrite32(), but reads peer-written copies from coherent memory as native
> integers. The values are therefore byte-swapped when read on a big-endian
> system.
> 
> Mark the shared fields as __le32 and convert coherent-memory accesses
> accordingly.
> 
> Fixes: 74465645cdb4 ("NTB: Fix Sparse Warnings")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Link: https://lore.kernel.org/r/20260815032932.151F11F000E9@smtp.kernel.org/
> Signed-off-by: Koichiro Den <den@valinux.co.jp>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
> Changes in v2:
>   - New patch. (Sashiko)
> 
>  drivers/ntb/ntb_transport.c | 47 +++++++++++++++++++++----------------
>  1 file changed, 27 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index d458a8b1de11..967a5ad38164 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
> @@ -132,7 +132,7 @@ struct ntb_queue_entry {
>  };
>  
>  struct ntb_rx_info {
> -	unsigned int entry;
> +	__le32 entry;
>  };
>  
>  struct ntb_transport_qp {
> @@ -265,9 +265,9 @@ enum {
>  };
>  
>  struct ntb_payload_header {
> -	unsigned int ver;
> -	unsigned int len;
> -	unsigned int flags;
> +	__le32 ver;
> +	__le32 len;
> +	__le32 flags;
>  };
>  
>  enum {
> @@ -514,7 +514,8 @@ static int ntb_qp_debugfs_stats_show(struct seq_file *s, void *v)
>  	seq_printf(s, "tx_err_no_buf - %llu\n", qp->tx_err_no_buf);
>  	seq_printf(s, "tx_mw - \t0x%p\n", qp->tx_mw);
>  	seq_printf(s, "tx_index (H) - \t%u\n", qp->tx_index);
> -	seq_printf(s, "RRI (T) - \t%u\n", qp->remote_rx_info->entry);
> +	seq_printf(s, "RRI (T) - \t%u\n",
> +		   le32_to_cpu(qp->remote_rx_info->entry));
>  	seq_printf(s, "tx_max_entry - \t%u\n", qp->tx_max_entry);
>  	seq_printf(s, "free tx - \t%u\n", ntb_transport_tx_free_entry(qp));
>  	seq_putc(s, '\n');
> @@ -633,7 +634,7 @@ static int ntb_transport_setup_qp_mw(struct ntb_transport_ctx *nt,
>  		qp->rx_alloc_entry++;
>  	}
>  
> -	qp->remote_rx_info->entry = qp->rx_max_entry - 1;
> +	qp->remote_rx_info->entry = cpu_to_le32(qp->rx_max_entry - 1);
>  
>  	/* setup the hdr offsets with 0's */
>  	for (i = 0; i < qp->rx_max_entry; i++) {
> @@ -919,7 +920,7 @@ static void ntb_qp_link_down_reset(struct ntb_transport_qp *qp)
>  {
>  	ntb_qp_link_context_reset(qp);
>  	if (qp->remote_rx_info)
> -		qp->remote_rx_info->entry = qp->rx_max_entry - 1;
> +		qp->remote_rx_info->entry = cpu_to_le32(qp->rx_max_entry - 1);
>  }
>  
>  static void ntb_qp_link_cleanup(struct ntb_transport_qp *qp)
> @@ -1445,7 +1446,7 @@ static void ntb_complete_rxc(struct ntb_transport_qp *qp)
>  		if (!(entry->flags & DESC_DONE_FLAG))
>  			break;
>  
> -		entry->rx_hdr->flags = 0;
> +		entry->rx_hdr->flags = cpu_to_le32(0);
>  		iowrite32(entry->rx_index, &qp->rx_info->entry);
>  
>  		cb_data = entry->cb_data;
> @@ -1609,13 +1610,15 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
>  {
>  	struct ntb_payload_header *hdr;
>  	struct ntb_queue_entry *entry;
> -	unsigned int flags;
>  	void *offset;
> +	u32 flags;
> +	u32 len;
> +	u32 ver;
>  
>  	offset = qp->rx_buff + qp->rx_max_frame * qp->rx_index;
>  	hdr = offset + qp->rx_max_frame - sizeof(struct ntb_payload_header);
>  
> -	flags = READ_ONCE(hdr->flags);
> +	flags = le32_to_cpu(READ_ONCE(hdr->flags));
>  	if (!(flags & DESC_DONE_FLAG)) {
>  		dev_dbg(&qp->ndev->pdev->dev, "done flag not set\n");
>  		qp->rx_ring_empty++;
> @@ -1623,21 +1626,23 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
>  	}
>  
>  	dma_rmb();
> +	ver = le32_to_cpu(hdr->ver);
> +	len = le32_to_cpu(hdr->len);
>  
>  	dev_dbg(&qp->ndev->pdev->dev, "qp %d: RX ver %u len %d flags %x\n",
> -		qp->qp_num, hdr->ver, hdr->len, flags);
> +		qp->qp_num, ver, len, flags);
>  
>  	if (flags & LINK_DOWN_FLAG) {
>  		dev_dbg(&qp->ndev->pdev->dev, "link down flag set\n");
>  		ntb_qp_link_down(qp);
> -		hdr->flags = 0;
> +		hdr->flags = cpu_to_le32(0);
>  		return -EAGAIN;
>  	}
>  
> -	if (hdr->ver != (u32)qp->rx_pkts) {
> +	if (ver != (u32)qp->rx_pkts) {
>  		dev_dbg(&qp->ndev->pdev->dev,
>  			"version mismatch, expected %llu - got %u\n",
> -			qp->rx_pkts, hdr->ver);
> +			qp->rx_pkts, ver);
>  		qp->rx_err_ver++;
>  		return -EIO;
>  	}
> @@ -1652,10 +1657,10 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
>  	entry->rx_hdr = hdr;
>  	entry->rx_index = qp->rx_index;
>  
> -	if (hdr->len > entry->len) {
> +	if (len > entry->len) {
>  		dev_dbg(&qp->ndev->pdev->dev,
>  			"receive buffer overflow! Wanted %d got %d\n",
> -			hdr->len, entry->len);
> +			len, entry->len);
>  		qp->rx_err_oflow++;
>  
>  		entry->len = -EIO;
> @@ -1665,12 +1670,12 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
>  	} else {
>  		dev_dbg(&qp->ndev->pdev->dev,
>  			"RX OK index %u ver %u size %d into buf size %d\n",
> -			qp->rx_index, hdr->ver, hdr->len, entry->len);
> +			qp->rx_index, ver, len, entry->len);
>  
> -		qp->rx_bytes += hdr->len;
> +		qp->rx_bytes += len;
>  		qp->rx_pkts++;
>  
> -		entry->len = hdr->len;
> +		entry->len = len;
>  
>  		ntb_async_rx(entry, offset);
>  	}
> @@ -2492,7 +2497,9 @@ EXPORT_SYMBOL_GPL(ntb_transport_max_size);
>  unsigned int ntb_transport_tx_free_entry(struct ntb_transport_qp *qp)
>  {
>  	unsigned int head = qp->tx_index;
> -	unsigned int tail = qp->remote_rx_info->entry;
> +	unsigned int tail;
> +
> +	tail = le32_to_cpu(READ_ONCE(qp->remote_rx_info->entry));
>  
>  	return tail >= head ? tail - head : qp->tx_max_entry + tail - head;
>  }