Message-ID:
 <CH0PR18MB433942172399A580C86D6BA9CD9C2@CH0PR18MB4339.namprd18.prod.outlook.com>
Date: Wed, 4 Sep 2024 06:20:55 +0000
From: Geethasowjanya Akula <gakula@...vell.com>
To: Jiri Pirko <jiri@...nulli.us>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "davem@...emloft.net"
	<davem@...emloft.net>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        Sunil Kovvuri Goutham
	<sgoutham@...vell.com>,
        Subbaraya Sundeep Bhatta <sbhatta@...vell.com>,
        Hariprasad Kelam <hkelam@...vell.com>
Subject: RE: [EXTERNAL] Re: [net-next PATCH 4/4] octeontx2-pf: Export common
 APIs



>-----Original Message-----
>From: Jiri Pirko <jiri@...nulli.us>
>Sent: Tuesday, September 3, 2024 8:31 PM
>To: Geethasowjanya Akula <gakula@...vell.com>
>Cc: netdev@...r.kernel.org; linux-kernel@...r.kernel.org; kuba@...nel.org;
>davem@...emloft.net; pabeni@...hat.com; edumazet@...gle.com; Sunil
>Kovvuri Goutham <sgoutham@...vell.com>; Subbaraya Sundeep Bhatta
><sbhatta@...vell.com>; Hariprasad Kelam <hkelam@...vell.com>
>Subject: [EXTERNAL] Re: [net-next PATCH 4/4] octeontx2-pf: Export common
>APIs
>
>Tue, Sep 03, 2024 at 02: 40: 48PM CEST, gakula@ marvell. com wrote: >Export
>mbox, hw resources and interrupt configuration functions. >So, that they can
>be used later by the RVU representor driver. > >Signed-off-by: Geetha sowjanya
><gakula@ marvell. com> 
>Tue, Sep 03, 2024 at 02:40:48PM CEST, gakula@...vell.com wrote:
>>Export mbox, hw resources and interrupt configuration functions.
>>So, that they can be used later by the RVU representor driver.
>>
>>Signed-off-by: Geetha sowjanya <gakula@...vell.com>
>>---
>> .../marvell/octeontx2/nic/otx2_common.c       |  2 +
>> .../marvell/octeontx2/nic/otx2_common.h       | 11 +++++
>> .../ethernet/marvell/octeontx2/nic/otx2_pf.c  | 40 +++++++++++++------
>> .../marvell/octeontx2/nic/otx2_txrx.c         | 17 +++++---
>> .../marvell/octeontx2/nic/otx2_txrx.h         |  3 +-
>> .../ethernet/marvell/octeontx2/nic/otx2_vf.c  |  6 +--
>> 6 files changed, 56 insertions(+), 23 deletions(-)
>>
>>diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
>>b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
>>index 34e76cfd941b..e38b3eea11f3 100644
>>--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
>>+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
>>@@ -246,6 +246,7 @@ int otx2_hw_set_mtu(struct otx2_nic *pfvf, int mtu)
>> 	mutex_unlock(&pfvf->mbox.lock);
>> 	return err;
>> }
>>+EXPORT_SYMBOL(otx2_hw_set_mtu);
>>
>> int otx2_config_pause_frm(struct otx2_nic *pfvf)  { @@ -1782,6 +1783,7
>>@@ void otx2_free_cints(struct otx2_nic *pfvf, int n)
>> 		free_irq(vector, &qset->napi[qidx]);
>> 	}
>> }
>>+EXPORT_SYMBOL(otx2_free_cints);
>>
>> void otx2_set_cints_affinity(struct otx2_nic *pfvf)  { diff --git
>>a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
>>b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
>>index b36b87dae2cb..327254e578d5 100644
>>--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
>>+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
>>@@ -1000,6 +1000,17 @@ int otx2_aura_init(struct otx2_nic *pfvf, int
>>aura_id,  int otx2_init_rsrc(struct pci_dev *pdev, struct otx2_nic
>>*pf);  void otx2_free_queue_mem(struct otx2_qset *qset);  int
>>otx2_alloc_queue_mem(struct otx2_nic *pf);
>>+int otx2_init_hw_resources(struct otx2_nic *pfvf); void
>>+otx2_free_hw_resources(struct otx2_nic *pf); int otx2_wq_init(struct
>>+otx2_nic *pf); int otx2_check_pf_usable(struct otx2_nic *pf); int
>>+otx2_pfaf_mbox_init(struct otx2_nic *pf); int
>>+otx2_register_mbox_intr(struct otx2_nic *pf, bool probe_af); int
>>+otx2_realloc_msix_vectors(struct otx2_nic *pf); void
>>+otx2_pfaf_mbox_destroy(struct otx2_nic *pf); void
>>+otx2_disable_mbox_intr(struct otx2_nic *pf); void
>>+otx2_disable_napi(struct otx2_nic *pf); irqreturn_t
>>+otx2_cq_intr_handler(int irq, void *cq_irq);
>>
>> /* RSS configuration APIs*/
>> int otx2_rss_init(struct otx2_nic *pfvf); diff --git
>>a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
>>b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
>>index 5bb6db5a3a73..b4fa2c12721d 100644
>>--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
>>+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
>>@@ -1008,7 +1008,7 @@ static irqreturn_t otx2_pfaf_mbox_intr_handler(int
>irq, void *pf_irq)
>> 	return IRQ_HANDLED;
>> }
>>
>>-static void otx2_disable_mbox_intr(struct otx2_nic *pf)
>>+void otx2_disable_mbox_intr(struct otx2_nic *pf)
>> {
>> 	int vector = pci_irq_vector(pf->pdev, RVU_PF_INT_VEC_AFPF_MBOX);
>>
>>@@ -1016,8 +1016,9 @@ static void otx2_disable_mbox_intr(struct otx2_nic
>*pf)
>> 	otx2_write64(pf, RVU_PF_INT_ENA_W1C, BIT_ULL(0));
>> 	free_irq(vector, pf);
>> }
>>+EXPORT_SYMBOL(otx2_disable_mbox_intr);
>>
>>-static int otx2_register_mbox_intr(struct otx2_nic *pf, bool probe_af)
>>+int otx2_register_mbox_intr(struct otx2_nic *pf, bool probe_af)
>> {
>> 	struct otx2_hw *hw = &pf->hw;
>> 	struct msg_req *req;
>>@@ -1060,8 +1061,9 @@ static int otx2_register_mbox_intr(struct
>>otx2_nic *pf, bool probe_af)
>>
>> 	return 0;
>> }
>>+EXPORT_SYMBOL(otx2_register_mbox_intr);
>>
>>-static void otx2_pfaf_mbox_destroy(struct otx2_nic *pf)
>>+void otx2_pfaf_mbox_destroy(struct otx2_nic *pf)
>> {
>> 	struct mbox *mbox = &pf->mbox;
>>
>>@@ -1076,8 +1078,9 @@ static void otx2_pfaf_mbox_destroy(struct otx2_nic
>*pf)
>> 	otx2_mbox_destroy(&mbox->mbox);
>> 	otx2_mbox_destroy(&mbox->mbox_up);
>> }
>>+EXPORT_SYMBOL(otx2_pfaf_mbox_destroy);
>>
>>-static int otx2_pfaf_mbox_init(struct otx2_nic *pf)
>>+int otx2_pfaf_mbox_init(struct otx2_nic *pf)
>> {
>> 	struct mbox *mbox = &pf->mbox;
>> 	void __iomem *hwbase;
>>@@ -1124,6 +1127,7 @@ static int otx2_pfaf_mbox_init(struct otx2_nic *pf)
>> 	otx2_pfaf_mbox_destroy(pf);
>> 	return err;
>> }
>>+EXPORT_SYMBOL(otx2_pfaf_mbox_init);
>>
>> static int otx2_cgx_config_linkevents(struct otx2_nic *pf, bool
>>enable)  { @@ -1379,7 +1383,7 @@ static irqreturn_t
>>otx2_q_intr_handler(int irq, void *data)
>> 	return IRQ_HANDLED;
>> }
>>
>>-static irqreturn_t otx2_cq_intr_handler(int irq, void *cq_irq)
>>+irqreturn_t otx2_cq_intr_handler(int irq, void *cq_irq)
>> {
>> 	struct otx2_cq_poll *cq_poll = (struct otx2_cq_poll *)cq_irq;
>> 	struct otx2_nic *pf = (struct otx2_nic *)cq_poll->dev; @@ -1398,20
>>+1402,25 @@ static irqreturn_t otx2_cq_intr_handler(int irq, void
>>*cq_irq)
>>
>> 	return IRQ_HANDLED;
>> }
>>+EXPORT_SYMBOL(otx2_cq_intr_handler);
>>
>>-static void otx2_disable_napi(struct otx2_nic *pf)
>>+void otx2_disable_napi(struct otx2_nic *pf)
>> {
>> 	struct otx2_qset *qset = &pf->qset;
>> 	struct otx2_cq_poll *cq_poll;
>>+	struct work_struct *work;
>> 	int qidx;
>>
>> 	for (qidx = 0; qidx < pf->hw.cint_cnt; qidx++) {
>> 		cq_poll = &qset->napi[qidx];
>>-		cancel_work_sync(&cq_poll->dim.work);
>>+		work = &cq_poll->dim.work;
>>+		if (work->func)
>>+			cancel_work_sync(work);
>> 		napi_disable(&cq_poll->napi);
>> 		netif_napi_del(&cq_poll->napi);
>> 	}
>> }
>>+EXPORT_SYMBOL(otx2_disable_napi);
>>
>> static void otx2_free_cq_res(struct otx2_nic *pf)  { @@ -1477,7
>>+1486,7 @@ static int otx2_get_rbuf_size(struct otx2_nic *pf, int mtu)
>> 	return ALIGN(rbuf_size, 2048);
>> }
>>
>>-static int otx2_init_hw_resources(struct otx2_nic *pf)
>>+int otx2_init_hw_resources(struct otx2_nic *pf)
>> {
>> 	struct nix_lf_free_req *free_req;
>> 	struct mbox *mbox = &pf->mbox;
>>@@ -1601,8 +1610,9 @@ static int otx2_init_hw_resources(struct otx2_nic
>*pf)
>> 	mutex_unlock(&mbox->lock);
>> 	return err;
>> }
>>+EXPORT_SYMBOL(otx2_init_hw_resources);
>>
>>-static void otx2_free_hw_resources(struct otx2_nic *pf)
>>+void otx2_free_hw_resources(struct otx2_nic *pf)
>> {
>> 	struct otx2_qset *qset = &pf->qset;
>> 	struct nix_lf_free_req *free_req;
>>@@ -1688,6 +1698,7 @@ static void otx2_free_hw_resources(struct otx2_nic
>*pf)
>> 	}
>> 	mutex_unlock(&mbox->lock);
>> }
>>+EXPORT_SYMBOL(otx2_free_hw_resources);
>>
>> static bool otx2_promisc_use_mce_list(struct otx2_nic *pfvf)  { @@
>>-1781,6 +1792,7 @@ void otx2_free_queue_mem(struct otx2_qset *qset)
>> 	kfree(qset->napi);
>> }
>> EXPORT_SYMBOL(otx2_free_queue_mem);
>>+
>> int otx2_alloc_queue_mem(struct otx2_nic *pf)  {
>> 	struct otx2_qset *qset = &pf->qset;
>>@@ -2103,7 +2115,7 @@ static netdev_tx_t otx2_xmit(struct sk_buff *skb,
>struct net_device *netdev)
>> 	sq = &pf->qset.sq[sq_idx];
>> 	txq = netdev_get_tx_queue(netdev, qidx);
>>
>>-	if (!otx2_sq_append_skb(netdev, sq, skb, qidx)) {
>>+	if (!otx2_sq_append_skb(pf, txq, sq, skb, qidx)) {
>> 		netif_tx_stop_queue(txq);
>>
>> 		/* Check again, incase SQBs got freed up */ @@ -2808,7
>+2820,7 @@
>>static const struct net_device_ops otx2_netdev_ops = {
>> 	.ndo_set_vf_trust	= otx2_ndo_set_vf_trust,
>> };
>>
>>-static int otx2_wq_init(struct otx2_nic *pf)
>>+int otx2_wq_init(struct otx2_nic *pf)
>> {
>> 	pf->otx2_wq = create_singlethread_workqueue("otx2_wq");
>> 	if (!pf->otx2_wq)
>>@@ -2819,7 +2831,7 @@ static int otx2_wq_init(struct otx2_nic *pf)
>> 	return 0;
>> }
>>
>>-static int otx2_check_pf_usable(struct otx2_nic *nic)
>>+int otx2_check_pf_usable(struct otx2_nic *nic)
>> {
>> 	u64 rev;
>>
>>@@ -2836,8 +2848,9 @@ static int otx2_check_pf_usable(struct otx2_nic
>*nic)
>> 	}
>> 	return 0;
>> }
>>+EXPORT_SYMBOL(otx2_check_pf_usable);
>>
>>-static int otx2_realloc_msix_vectors(struct otx2_nic *pf)
>>+int otx2_realloc_msix_vectors(struct otx2_nic *pf)
>> {
>> 	struct otx2_hw *hw = &pf->hw;
>> 	int num_vec, err;
>>@@ -2859,6 +2872,7 @@ static int otx2_realloc_msix_vectors(struct
>>otx2_nic *pf)
>>
>> 	return otx2_register_mbox_intr(pf, false); }
>>+EXPORT_SYMBOL(otx2_realloc_msix_vectors);
>>
>> static int otx2_sriov_vfcfg_init(struct otx2_nic *pf)  { diff --git
>>a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
>>b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
>>index 3eb85949677a..fbd9fe98259f 100644
>>--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
>>+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
>>@@ -131,6 +131,7 @@ static void otx2_xdp_snd_pkt_handler(struct
>>otx2_nic *pfvf,  }
>>
>> static void otx2_snd_pkt_handler(struct otx2_nic *pfvf,
>>+				 struct net_device *ndev,
>> 				 struct otx2_cq_queue *cq,
>> 				 struct otx2_snd_queue *sq,
>> 				 struct nix_cqe_tx_s *cqe,
>>@@ -145,7 +146,7 @@ static void otx2_snd_pkt_handler(struct otx2_nic
>>*pfvf,
>>
>> 	if (unlikely(snd_comp->status) && netif_msg_tx_err(pfvf))
>> 		net_err_ratelimited("%s: TX%d: Error in send CQ status:%x\n",
>>-				    pfvf->netdev->name, cq->cint_idx,
>>+				    ndev->name, cq->cint_idx,
>> 				    snd_comp->status);
>>
>> 	sg = &sq->sg[snd_comp->sqe_id];
>>@@ -453,6 +454,7 @@ static int otx2_tx_napi_handler(struct otx2_nic *pfvf,
>> 	int tx_pkts = 0, tx_bytes = 0, qidx;
>> 	struct otx2_snd_queue *sq;
>> 	struct nix_cqe_tx_s *cqe;
>>+	struct net_device *ndev;
>> 	int processed_cqe = 0;
>>
>> 	if (cq->pend_cqe >= budget)
>>@@ -464,6 +466,7 @@ static int otx2_tx_napi_handler(struct otx2_nic
>>*pfvf,
>> process_cqe:
>> 	qidx = cq->cq_idx - pfvf->hw.rx_queues;
>> 	sq = &pfvf->qset.sq[qidx];
>>+	ndev = pfvf->netdev;
>>
>> 	while (likely(processed_cqe < budget) && cq->pend_cqe) {
>> 		cqe = (struct nix_cqe_tx_s *)otx2_get_next_cqe(cq); @@ -
>478,7 +481,8
>>@@ static int otx2_tx_napi_handler(struct otx2_nic *pfvf,
>> 		if (cq->cq_type == CQ_XDP)
>> 			otx2_xdp_snd_pkt_handler(pfvf, sq, cqe);
>> 		else
>>-			otx2_snd_pkt_handler(pfvf, cq, &pfvf->qset.sq[qidx],
>>+			otx2_snd_pkt_handler(pfvf, ndev, cq,
>>+					     &pfvf->qset.sq[qidx],
>> 					     cqe, budget, &tx_pkts, &tx_bytes);
>>
>> 		cqe->hdr.cqe_type = NIX_XQE_TYPE_INVALID; @@ -505,7
>+509,7 @@ static
>>int otx2_tx_napi_handler(struct otx2_nic *pfvf,
>> 		/* Check if queue was stopped earlier due to ring full */
>> 		smp_mb();
>> 		if (netif_tx_queue_stopped(txq) &&
>>-		    netif_carrier_ok(pfvf->netdev))
>>+		    netif_carrier_ok(ndev))
>> 			netif_tx_wake_queue(txq);
>
>
>I don't understand the this change you do in otx2_tx_napi_handler() and
>otx2_snd_pkt_handler(). What are you trying to achieve?
Will add this changes into "Introduce RVU representors"
patchset where it explains why this changes are required.

>Also, it is unrelated to the rest of the patch (export functions)
>
>
>> 	}
>> 	return 0;
>>@@ -594,6 +598,7 @@ int otx2_napi_handler(struct napi_struct *napi, int
>budget)
>> 	}
>> 	return workdone;
>> }
>>+EXPORT_SYMBOL(otx2_napi_handler);
>>
>> void otx2_sqe_flush(void *dev, struct otx2_snd_queue *sq,
>> 		    int size, int qidx)
>>@@ -1141,13 +1146,13 @@ static void otx2_set_txtstamp(struct otx2_nic
>*pfvf, struct sk_buff *skb,
>> 	}
>> }
>>
>>-bool otx2_sq_append_skb(struct net_device *netdev, struct
>>otx2_snd_queue *sq,
>>+bool otx2_sq_append_skb(void *dev, struct netdev_queue *txq,
>
>1) This looks like unrelated to the rest of the patch (export functions)
>2) Why void *? Avoid that please. Just have arg struct otx2_nic *pfvf no?
Will change it to otx2_nic and move this change to " Introduce RVU representors"
patchset where related changes are present. 
>
>
>
>>+			struct otx2_snd_queue *sq,
>> 			struct sk_buff *skb, u16 qidx)
>> {
>>-	struct netdev_queue *txq = netdev_get_tx_queue(netdev, qidx);
>>-	struct otx2_nic *pfvf = netdev_priv(netdev);
>> 	int offset, num_segs, free_desc;
>> 	struct nix_sqe_hdr_s *sqe_hdr;
>>+	struct otx2_nic *pfvf = dev;
>>
>> 	/* Check if there is enough room between producer
>> 	 * and consumer index.
>>diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.h
>>b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.h
>>index 3f1d2655ff77..e1db5f961877 100644
>>--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.h
>>+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.h
>>@@ -167,7 +167,8 @@ static inline u64 otx2_iova_to_phys(void
>>*iommu_domain, dma_addr_t dma_addr)  }
>>
>> int otx2_napi_handler(struct napi_struct *napi, int budget); -bool
>>otx2_sq_append_skb(struct net_device *netdev, struct otx2_snd_queue
>>*sq,
>>+bool otx2_sq_append_skb(void *dev, struct netdev_queue *txq,
>>+			struct otx2_snd_queue *sq,
>> 			struct sk_buff *skb, u16 qidx);
>> void cn10k_sqe_flush(void *dev, struct otx2_snd_queue *sq,
>> 		     int size, int qidx);
>>diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
>>b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
>>index 79a8acac6283..0486fca8b573 100644
>>--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
>>+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
>>@@ -395,7 +395,7 @@ static netdev_tx_t otx2vf_xmit(struct sk_buff *skb,
>struct net_device *netdev)
>> 	sq = &vf->qset.sq[qidx];
>> 	txq = netdev_get_tx_queue(netdev, qidx);
>>
>>-	if (!otx2_sq_append_skb(netdev, sq, skb, qidx)) {
>>+	if (!otx2_sq_append_skb(vf, txq, sq, skb, qidx)) {
>> 		netif_tx_stop_queue(txq);
>>
>> 		/* Check again, incase SQBs got freed up */ @@ -500,7 +500,7
>@@
>>static const struct net_device_ops otx2vf_netdev_ops = {
>> 	.ndo_setup_tc = otx2_setup_tc,
>> };
>>
>>-static int otx2_wq_init(struct otx2_nic *vf)
>>+static int otx2_vf_wq_init(struct otx2_nic *vf)
>> {
>> 	vf->otx2_wq = create_singlethread_workqueue("otx2vf_wq");
>> 	if (!vf->otx2_wq)
>>@@ -689,7 +689,7 @@ static int otx2vf_probe(struct pci_dev *pdev, const
>struct pci_device_id *id)
>> 		goto err_ptp_destroy;
>> 	}
>>
>>-	err = otx2_wq_init(vf);
>>+	err = otx2_vf_wq_init(vf);
>> 	if (err)
>> 		goto err_unreg_netdev;
>>
>>--
>>2.25.1
>>

Powered by blists - more mailing lists