Re: [dpdk-dev] [PATCH] net/hns3: support Tx push quick doorbell to improve perf

[Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>]

Why is the same patch sent twice? Could you cleanup patchwork, please,
and set appropriate status?

On 6/13/21 4:42 AM, Min Hu (Connor) wrote:
> From: Chengwen Feng <fengchengwen@huawei.com>
> 
> Kunpeng 930 support tx push mode which could improve performance, It

tx -> Tx

> works like below:
> 1. Add pcie bar45 which support driver direct write the Tx descriptor

pcie -> PCIe (as an official abbreviation of the PCI express)

> or tail reg to it.
> 2. Support three operations: a) direct write one Tx descriptor, b)
> direct write two Tx descriptors, c) direct write tail reg.
> 3. The original tail reg located at bar23, the above bar45 tail reg
> could provide better bandwidth from the hardware perspective.
> 
> The hns3 driver only support direct write tail reg (also have the name
> of quick doorbell), the detail:
> 1. Considering compatibility, firmware will report tx push capa if the

tx -> Tx

> hardware support it.
> 2. Add control macro RTE_LIBRTE_HNS3_ENABLE_TX_PUSH which was not
> defined default.
> 3. If user define macro RTE_LIBRTE_HNS3_ENABLE_TX_PUSH and hardware
> support, then driver will direct write bar45 tail reg to inform the
> hardware.
> 
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
>   drivers/net/hns3/hns3_ethdev.c        |  4 +-
>   drivers/net/hns3/hns3_ethdev.h        |  4 ++
>   drivers/net/hns3/hns3_ethdev_vf.c     |  4 +-
>   drivers/net/hns3/hns3_rxtx.c          | 73 ++++++++++++++++++++++++++++++++++-
>   drivers/net/hns3/hns3_rxtx.h          | 19 +++++++++
>   drivers/net/hns3/hns3_rxtx_vec_neon.h |  2 +-
>   drivers/net/hns3/hns3_rxtx_vec_sve.c  |  2 +-
>   7 files changed, 102 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
> index 2049130..467d770 100644
> --- a/drivers/net/hns3/hns3_ethdev.c
> +++ b/drivers/net/hns3/hns3_ethdev.c
> @@ -5177,6 +5177,8 @@ hns3_init_pf(struct rte_eth_dev *eth_dev)
>   		goto err_cmd_init;
>   	}
>   
> +	hns3_tx_push_init(eth_dev);
> +
>   	/*
>   	 * To ensure that the hardware environment is clean during
>   	 * initialization, the driver actively clear the hardware environment
> @@ -7422,8 +7424,8 @@ hns3_dev_init(struct rte_eth_dev *eth_dev)
>   				     "process, ret = %d", ret);
>   			goto err_mp_init_secondary;
>   		}
> -
>   		hw->secondary_cnt++;
> +		hns3_tx_push_init(eth_dev);
>   		return 0;
>   	}
>   
> diff --git a/drivers/net/hns3/hns3_ethdev.h b/drivers/net/hns3/hns3_ethdev.h
> index 1714eba..575bacd 100644
> --- a/drivers/net/hns3/hns3_ethdev.h
> +++ b/drivers/net/hns3/hns3_ethdev.h
> @@ -862,6 +862,7 @@ enum {
>   	HNS3_DEV_SUPPORT_COPPER_B,
>   	HNS3_DEV_SUPPORT_FD_QUEUE_REGION_B,
>   	HNS3_DEV_SUPPORT_PTP_B,
> +	HNS3_DEV_SUPPORT_TX_PUSH_B,
>   	HNS3_DEV_SUPPORT_INDEP_TXRX_B,
>   	HNS3_DEV_SUPPORT_STASH_B,
>   	HNS3_DEV_SUPPORT_RXD_ADV_LAYOUT_B,
> @@ -900,6 +901,9 @@ enum {
>   #define hns3_dev_ras_imp_supported(hw) \
>   	hns3_get_bit((hw)->capability, HNS3_DEV_SUPPORT_RAS_IMP_B)
>   
> +#define hns3_dev_tx_push_supported(hw) \
> +		hns3_get_bit((hw)->capability, HNS3_DEV_SUPPORT_TX_PUSH_B)
> +
>   #define HNS3_DEV_PRIVATE_TO_HW(adapter) \
>   	(&((struct hns3_adapter *)adapter)->hw)
>   #define HNS3_DEV_PRIVATE_TO_PF(adapter) \
> diff --git a/drivers/net/hns3/hns3_ethdev_vf.c b/drivers/net/hns3/hns3_ethdev_vf.c
> index 41dd8ee..1e94a1e 100644
> --- a/drivers/net/hns3/hns3_ethdev_vf.c
> +++ b/drivers/net/hns3/hns3_ethdev_vf.c
> @@ -1921,6 +1921,8 @@ hns3vf_init_vf(struct rte_eth_dev *eth_dev)
>   		goto err_cmd_init;
>   	}
>   
> +	hns3_tx_push_init(eth_dev);
> +
>   	/* Get VF resource */
>   	ret = hns3_query_vf_resource(hw);
>   	if (ret)
> @@ -2928,8 +2930,8 @@ hns3vf_dev_init(struct rte_eth_dev *eth_dev)
>   					  "process, ret = %d", ret);
>   			goto err_mp_init_secondary;
>   		}
> -
>   		hw->secondary_cnt++;
> +		hns3_tx_push_init(eth_dev);
>   		return 0;
>   	}
>   
> diff --git a/drivers/net/hns3/hns3_rxtx.c b/drivers/net/hns3/hns3_rxtx.c
> index 1d7a769..88d4fc0 100644
> --- a/drivers/net/hns3/hns3_rxtx.c
> +++ b/drivers/net/hns3/hns3_rxtx.c
> @@ -2892,6 +2892,69 @@ hns3_tx_queue_conf_check(struct hns3_hw *hw, const struct rte_eth_txconf *conf,
>   	return 0;
>   }
>   
> +static void *
> +hns3_tx_push_get_queue_tail_reg(struct rte_eth_dev *dev, uint16_t queue_id)
> +{
> +#define HNS3_TX_PUSH_TQP_REGION_SIZE		0x10000
> +#define HNS3_TX_PUSH_QUICK_DOORBELL_OFFSET	64
> +#define HNS3_TX_PUSH_PCI_BAR_INDEX		4
> +
> +	struct rte_pci_device *pci_dev = RTE_DEV_TO_PCI(dev->device);
> +	uint8_t bar_id = HNS3_TX_PUSH_PCI_BAR_INDEX;
> +
> +	/*
> +	 * If device support tx push then its pcie bar45 must exist, and DPDK

tx -> Tx

> +	 * framework will mmap the bar45 default in pci probe stage.

pci -> PCI

> +	 *
> +	 * In the bar45, the first half is for roce, and the second half is for

roce ?

> +	 * nic, every TQP occupy 64KB.


nic -> NIC

> +	 *
> +	 * The quick doorbell located at 64B offset in the TQP region.
> +	 */
> +	return (void *)((char *)pci_dev->mem_resource[bar_id].addr +
> +			(pci_dev->mem_resource[bar_id].len >> 1) +
> +			HNS3_TX_PUSH_TQP_REGION_SIZE * queue_id +
> +			HNS3_TX_PUSH_QUICK_DOORBELL_OFFSET);
> +}
> +
> +void
> +hns3_tx_push_init(struct rte_eth_dev *dev)
> +{
> +	struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	volatile uint32_t *reg;
> +	uint32_t val;
> +
> +	if (!hns3_dev_tx_push_supported(hw))
> +		return;
> +
> +	reg = (volatile uint32_t *)hns3_tx_push_get_queue_tail_reg(dev, 0);
> +	/*
> +	 * Because the size of bar45 is about 8GB size, it may take a long time
> +	 * to do the page fault in Tx process when work with vfio-pci, so use
> +	 * one read operation to make kernel setup page table mapping for bar45
> +	 * in the init stage.
> +	 * Note: the bar45 is readable but the result is all 1.
> +	 */
> +	val = *reg;
> +	RTE_SET_USED(val);
> +}
> +
> +static void
> +hns3_tx_push_queue_init(struct rte_eth_dev *dev,
> +			uint16_t queue_id,
> +			struct hns3_tx_queue *txq)
> +{
> +	struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	if (!hns3_dev_tx_push_supported(hw)) {
> +		txq->tx_push_enable = false;
> +		return;
> +	}
> +
> +	txq->io_tail_reg = (volatile void *)hns3_tx_push_get_queue_tail_reg(dev,
> +						queue_id);
> +	txq->tx_push_enable = true;
> +}
> +
>   int
>   hns3_tx_queue_setup(struct rte_eth_dev *dev, uint16_t idx, uint16_t nb_desc,
>   		    unsigned int socket_id, const struct rte_eth_txconf *conf)
> @@ -2983,6 +3046,12 @@ hns3_tx_queue_setup(struct rte_eth_dev *dev, uint16_t idx, uint16_t nb_desc,
>   	memset(&txq->basic_stats, 0, sizeof(struct hns3_tx_basic_stats));
>   	memset(&txq->dfx_stats, 0, sizeof(struct hns3_tx_dfx_stats));
>   
> +	/*
> +	 * Call hns3_tx_push_queue_init after assigned io_tail_reg field because
> +	 * it may overwrite the io_tail_reg field.
> +	 */
> +	hns3_tx_push_queue_init(dev, idx, txq);
> +
>   	rte_spinlock_lock(&hw->lock);
>   	dev->data->tx_queues[idx] = txq;
>   	rte_spinlock_unlock(&hw->lock);
> @@ -4029,7 +4098,7 @@ hns3_xmit_pkts_simple(void *tx_queue,
>   	hns3_tx_fill_hw_ring(txq, tx_pkts + nb_tx, nb_pkts - nb_tx);
>   	txq->next_to_use += nb_pkts - nb_tx;
>   
> -	hns3_write_reg_opt(txq->io_tail_reg, nb_pkts);
> +	hns3_write_txq_tail_reg(txq, nb_pkts);
>   
>   	return nb_pkts;
>   }
> @@ -4146,7 +4215,7 @@ hns3_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
>   end_of_tx:
>   
>   	if (likely(nb_tx))
> -		hns3_write_reg_opt(txq->io_tail_reg, nb_hold);
> +		hns3_write_txq_tail_reg(txq, nb_hold);
>   
>   	return nb_tx;
>   }
> diff --git a/drivers/net/hns3/hns3_rxtx.h b/drivers/net/hns3/hns3_rxtx.h
> index ba24e00..7419386 100644
> --- a/drivers/net/hns3/hns3_rxtx.h
> +++ b/drivers/net/hns3/hns3_rxtx.h
> @@ -419,6 +419,7 @@ struct hns3_tx_dfx_stats {
>   };
>   
>   struct hns3_tx_queue {
> +	/* The io_tail_reg is write-only if working in tx push mode */
>   	volatile void *io_tail_reg;
>   	struct hns3_desc *tx_ring;
>   	struct hns3_entry *sw_ring;
> @@ -659,6 +660,23 @@ hns3_rx_calc_ptype(struct hns3_rx_queue *rxq, const uint32_t l234_info,
>   		return ptype_tbl->l3table[l3id] | ptype_tbl->l4table[l4id];
>   }
>   
> +/*
> + * If enable using tx push feature and also device support it, then use qick

tx -> Tx

qick -> quick ?

> + * doorbell (bar45) to inform the hardware.
> + *
> + * The other cases (such as: device don't support or user don't enable using)
> + * then use normal doorbell (bar23) to inform the hardware.
> + */
> +static inline void
> +hns3_write_txq_tail_reg(struct hns3_tx_queue *txq, uint32_t value)
> +{
> +	rte_io_wmb();
> +	if (txq->tx_push_enable)
> +		rte_write64_relaxed(rte_cpu_to_le_32(value), txq->io_tail_reg);
> +	else
> +		rte_write32_relaxed(rte_cpu_to_le_32(value), txq->io_tail_reg);
> +}
> +
>   void hns3_dev_rx_queue_release(void *queue);
>   void hns3_dev_tx_queue_release(void *queue);
>   void hns3_free_all_queues(struct rte_eth_dev *dev);
> @@ -741,5 +759,6 @@ int hns3_tx_done_cleanup(void *txq, uint32_t free_cnt);
>   void hns3_enable_rxd_adv_layout(struct hns3_hw *hw);
>   int hns3_dev_rx_descriptor_status(void *rx_queue, uint16_t offset);
>   int hns3_dev_tx_descriptor_status(void *tx_queue, uint16_t offset);
> +void hns3_tx_push_init(struct rte_eth_dev *dev);
>   
>   #endif /* _HNS3_RXTX_H_ */
> diff --git a/drivers/net/hns3/hns3_rxtx_vec_neon.h b/drivers/net/hns3/hns3_rxtx_vec_neon.h
> index e5c7d69..74c848d 100644
> --- a/drivers/net/hns3/hns3_rxtx_vec_neon.h
> +++ b/drivers/net/hns3/hns3_rxtx_vec_neon.h
> @@ -84,7 +84,7 @@ hns3_xmit_fixed_burst_vec(void *__restrict tx_queue,
>   	txq->next_to_use = next_to_use;
>   	txq->tx_bd_ready -= nb_tx;
>   
> -	hns3_write_reg_opt(txq->io_tail_reg, nb_tx);
> +	hns3_write_txq_tail_reg(txq, nb_tx);
>   
>   	return nb_tx;
>   }
> diff --git a/drivers/net/hns3/hns3_rxtx_vec_sve.c b/drivers/net/hns3/hns3_rxtx_vec_sve.c
> index bf7f704..d5c4933 100644
> --- a/drivers/net/hns3/hns3_rxtx_vec_sve.c
> +++ b/drivers/net/hns3/hns3_rxtx_vec_sve.c
> @@ -475,7 +475,7 @@ hns3_xmit_fixed_burst_vec_sve(void *__restrict tx_queue,
>   	txq->next_to_use += nb_pkts - nb_tx;
>   
>   	txq->tx_bd_ready -= nb_pkts;
> -	hns3_write_reg_opt(txq->io_tail_reg, nb_pkts);
> +	hns3_write_txq_tail_reg(txq, nb_pkts);
>   
>   	return nb_pkts;
>   }
>