RE: [EXTERNAL] Re: [net-next PATCH v7 02/10] octeontx2-pf: RVU representor driver

Geethasowjanya Akula
2024-07-04T09:33:34Z

>-----Original Message-----
>From: William Tu <witu@nvidia.com>
>Sent: Wednesday, July 3, 2024 11:03 PM
>To: Geethasowjanya Akula <gakula@marvell.com>; netdev@vger.kernel.org;
>linux-kernel@vger.kernel.org
>Cc: kuba@kernel.org; davem@davemloft.net; pabeni@redhat.com;
>edumazet@google.com; Sunil Kovvuri Goutham <sgoutham@marvell.com>;
>Subbaraya Sundeep Bhatta <sbhatta@marvell.com>; Hariprasad Kelam
><hkelam@marvell.com>
>Subject: [EXTERNAL] Re: [net-next PATCH v7 02/10] octeontx2-pf: RVU
>representor driver
>
>On 6/28/24 6:35 AM, Geetha sowjanya wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> Adds basic driver for the RVU representor.
>>
>> Driver on probe does pci specific initialization and does hw resources
>> configuration. Introduces RVU_ESWITCH kernel config to enable/disable
>> the driver. Representor and NIC shares the code but representors
>> netdev support subset of NIC functionality. Hence "otx2_rep_dev" API
>> helps to skip the features initialization that are not supported by
>> the representors.
>Hi Geetha,
>I'm trying to summarize how all representor device works, here:
>https://urldefense.proofpoint.com/v2/url?u=https-
>3A__lore.kernel.org_netdev_39dbf7f6-2D76e0-2D4319-2D97d8-
>2D24b54e788435-
>40nvidia.com_&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=UiEt_nUeYFctu7J
>VLXVlXDhTmq_EAfooaZEYInfGuEQ&m=JalPLLghK9_rJ6gTXJikQqDlTQc5MqXAmd
>A2eGmCWGYsc8lXfjLk9VROkFRfzhCC&s=JM-
>nw9SH1X_jvuUQNmrGLe5ap8YMx8kO9GVHztjb49I&e=
>
>So in the RVU representor case, IIUC,
>- it has its own PCI func id?
Yes. Our's is a multi-PF device and each of the PF has it's own VFs.
And in HW, packet parser identifies pkts sent or received by each of these PF/VFs by a unique PF_FUNC (15-11bits PF & 10-0bits PFs' VF).
If representor netdev is registered from a separate PF (ie a seperate PF_FUNC) then at packet parser it's easy to install pkt forwarding rules.
eg: representee <=> representor
>- it has its own receive queue (unlike ice which shares the rxq with PF), does
>representor netdev support multiple rx queues?
>- it has its own send queue (always use qidx=0)
RVU representors has their own TX and RX queues.
For now we support single tx and rx queues.
>> Signed-off-by: Geetha sowjanya <gakula@marvell.com>
>> ---
>> .../net/ethernet/marvell/octeontx2/Kconfig | 8 +
>> .../ethernet/marvell/octeontx2/af/Makefile | 3 +-
>> .../net/ethernet/marvell/octeontx2/af/mbox.h | 8 +
>> .../net/ethernet/marvell/octeontx2/af/rvu.h | 11 +
>> .../ethernet/marvell/octeontx2/af/rvu_nix.c | 21 +-
>> .../ethernet/marvell/octeontx2/af/rvu_rep.c | 47 ++++
>> .../ethernet/marvell/octeontx2/nic/Makefile | 2 +
>> .../marvell/octeontx2/nic/otx2_common.h | 12 +-
>> .../ethernet/marvell/octeontx2/nic/otx2_pf.c | 17 +-
>> .../marvell/octeontx2/nic/otx2_txrx.c | 23 +-
>> .../net/ethernet/marvell/octeontx2/nic/rep.c | 223 ++++++++++++++++++
>> .../net/ethernet/marvell/octeontx2/nic/rep.h | 31 +++
>> 12 files changed, 388 insertions(+), 18 deletions(-)
>> create mode 100644 drivers/net/ethernet/marvell/octeontx2/af/rvu_rep.c
>> create mode 100644 drivers/net/ethernet/marvell/octeontx2/nic/rep.c
>> create mode 100644 drivers/net/ethernet/marvell/octeontx2/nic/rep.h
>>
>> diff --git a/drivers/net/ethernet/marvell/octeontx2/Kconfig
>> b/drivers/net/ethernet/marvell/octeontx2/Kconfig
>> index a32d85d6f599..ff86a5f267c3 100644
>> --- a/drivers/net/ethernet/marvell/octeontx2/Kconfig
>> +++ b/drivers/net/ethernet/marvell/octeontx2/Kconfig
>> @@ -46,3 +46,11 @@ config OCTEONTX2_VF
>> depends on OCTEONTX2_PF
>> help
>> This driver supports Marvell's OcteonTX2 NIC virtual function.
>> +
>> +config RVU_ESWITCH
>> + tristate "Marvell RVU E-Switch support"
>> + depends on OCTEONTX2_PF
>> + default m
>> + help
>> + This driver supports Marvell's RVU E-Switch that
>> + provides internal SRIOV packet steering and switching for
>> +the
>> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/Makefile
>> b/drivers/net/ethernet/marvell/octeontx2/af/Makefile
>> index 3cf4c8285c90..ccea37847df8 100644
>> --- a/drivers/net/ethernet/marvell/octeontx2/af/Makefile
>> +++ b/drivers/net/ethernet/marvell/octeontx2/af/Makefile
>> @@ -11,4 +11,5 @@ rvu_mbox-y := mbox.o rvu_trace.o
>> rvu_af-y := cgx.o rvu.o rvu_cgx.o rvu_npa.o rvu_nix.o \
>> rvu_reg.o rvu_npc.o rvu_debugfs.o ptp.o rvu_npc_fs.o \
>> rvu_cpt.o rvu_devlink.o rpm.o rvu_cn10k.o rvu_switch.o \
>> - rvu_sdp.o rvu_npc_hash.o mcs.o mcs_rvu_if.o mcs_cnf10kb.o
>> + rvu_sdp.o rvu_npc_hash.o mcs.o mcs_rvu_if.o mcs_cnf10kb.o \
>> + rvu_rep.o
>> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
>> b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
>> index e6d7d6e862c0..befb327e8aff 100644
>> --- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
>> +++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
>> @@ -144,6 +144,7 @@ M(LMTST_TBL_SETUP, 0x00a, lmtst_tbl_setup,
>lmtst_tbl_setup_req, \
>> msg_rsp) \
>> M(SET_VF_PERM, 0x00b, set_vf_perm, set_vf_perm, msg_rsp) \
>> M(PTP_GET_CAP, 0x00c, ptp_get_cap, msg_req, ptp_get_cap_rsp) \
>> +M(GET_REP_CNT, 0x00d, get_rep_cnt, msg_req, get_rep_cnt_rsp) \
>> /* CGX mbox IDs (range 0x200 - 0x3FF) */ \
>> M(CGX_START_RXTX, 0x200, cgx_start_rxtx, msg_req, msg_rsp) \
>> M(CGX_STOP_RXTX, 0x201, cgx_stop_rxtx, msg_req, msg_rsp) \
>> @@ -1525,6 +1526,13 @@ struct ptp_get_cap_rsp {
>> u64 cap;
>> };
>>
>> +struct get_rep_cnt_rsp {
>> + struct mbox_msghdr hdr;
>> + u16 rep_cnt;
>> + u16 rep_pf_map[64];
>> + u64 rsvd;
>> +};
>> +
>> struct flow_msg {
>> unsigned char dmac[6];
>> unsigned char smac[6];
>> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
>> b/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
>> index 30efa5607c58..cbdc7aeaccfc 100644
>> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
>> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
>> @@ -594,6 +594,9 @@ struct rvu {
>> spinlock_t cpt_intr_lock;
>>
>> struct mutex mbox_lock; /* Serialize mbox up and down msgs */
>> + u16 rep_pcifunc;
>
>does rep_pcifunc mean the VF/SF's pcifunc, or rep itself has its own pcifunc?
>> + int rep_cnt;
>
>does rep_cnt mean number of rx/tx queues in representor?
>> + u16 *rep2pfvf_map;
>> };
>>
>> static inline void rvu_write64(struct rvu *rvu, u64 block, u64 offset, u64 val)
>> @@ -822,6 +825,14 @@ bool is_sdp_pfvf(u16 pcifunc);
>> bool is_sdp_pf(u16 pcifunc);
>> bool is_sdp_vf(struct rvu *rvu, u16 pcifunc);
>>
>> +static inline bool is_rep_dev(struct rvu *rvu, u16 pcifunc)
>> +{
>> + if (rvu->rep_pcifunc && rvu->rep_pcifunc == pcifunc)
>> + return true;
>> +
>> + return false;
>> +}
>> +
>> /* CGX APIs */
>> static inline bool is_pf_cgxmapped(struct rvu *rvu, u8 pf)
>> {
>> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
>b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
>> index 785ef71a5ead..02d83c4958d9 100644
>> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
>> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
>> @@ -329,7 +329,9 @@ static bool is_valid_txschq(struct rvu *rvu, int
>blkaddr,
>>
>> /* TLs aggegating traffic are shared across PF and VFs */
>> if (lvl >= hw->cap.nix_tx_aggr_lvl) {
>> - if (rvu_get_pf(map_func) != rvu_get_pf(pcifunc))
>> + if ((nix_get_tx_link(rvu, map_func) !=
>> + nix_get_tx_link(rvu, pcifunc)) &&
>> + (rvu_get_pf(map_func) != rvu_get_pf(pcifunc)))
>> return false;
>> else
>> return true;
>> @@ -1634,6 +1636,12 @@ int rvu_mbox_handler_nix_lf_alloc(struct rvu
>*rvu,
>> cfg = NPC_TX_DEF_PKIND;
>> rvu_write64(rvu, blkaddr, NIX_AF_LFX_TX_PARSE_CFG(nixlf), cfg);
>>
>> + if (is_rep_dev(rvu, pcifunc)) {
>> + pfvf->tx_chan_base = RVU_SWITCH_LBK_CHAN;
>> + pfvf->tx_chan_cnt = 1;
>> + goto exit;
>> + }
>> +
>> intf = is_lbk_vf(rvu, pcifunc) ? NIX_INTF_TYPE_LBK :
>NIX_INTF_TYPE_CGX;
>> if (is_sdp_pfvf(pcifunc))
>> intf = NIX_INTF_TYPE_SDP;
>> @@ -1704,6 +1712,9 @@ int rvu_mbox_handler_nix_lf_free(struct rvu *rvu,
>struct nix_lf_free_req *req,
>> if (nixlf < 0)
>> return NIX_AF_ERR_AF_LF_INVALID;
>>
>> + if (is_rep_dev(rvu, pcifunc))
>> + goto free_lf;
>> +
>> if (req->flags & NIX_LF_DISABLE_FLOWS)
>> rvu_npc_disable_mcam_entries(rvu, pcifunc, nixlf);
>> else
>> @@ -1715,6 +1726,7 @@ int rvu_mbox_handler_nix_lf_free(struct rvu *rvu,
>struct nix_lf_free_req *req,
>>
>> nix_interface_deinit(rvu, pcifunc, nixlf);
>>
>> +free_lf:
>> /* Reset this NIX LF */
>> err = rvu_lf_reset(rvu, block, nixlf);
>> if (err) {
>> @@ -2010,7 +2022,8 @@ static void nix_get_txschq_range(struct rvu *rvu,
>u16 pcifunc,
>> struct rvu_hwinfo *hw = rvu->hw;
>> int pf = rvu_get_pf(pcifunc);
>>
>> - if (is_lbk_vf(rvu, pcifunc)) { /* LBK links */
>> + /* LBK links */
>> + if (is_lbk_vf(rvu, pcifunc) || is_rep_dev(rvu, pcifunc)) {
>> *start = hw->cap.nix_txsch_per_cgx_lmac * link;
>> *end = *start + hw->cap.nix_txsch_per_lbk_lmac;
>> } else if (is_pf_cgxmapped(rvu, pf)) { /* CGX links */
>> @@ -4523,7 +4536,7 @@ int rvu_mbox_handler_nix_set_hw_frs(struct rvu
>*rvu, struct nix_frs_cfg *req,
>> if (!nix_hw)
>> return NIX_AF_ERR_INVALID_NIXBLK;
>>
>> - if (is_lbk_vf(rvu, pcifunc))
>> + if (is_lbk_vf(rvu, pcifunc) || is_rep_dev(rvu, pcifunc))
>> rvu_get_lbk_link_max_frs(rvu, &max_mtu);
>> else
>> rvu_get_lmac_link_max_frs(rvu, &max_mtu);
>> @@ -4551,6 +4564,8 @@ int rvu_mbox_handler_nix_set_hw_frs(struct rvu
>*rvu, struct nix_frs_cfg *req,
>> /* For VFs of PF0 ingress is LBK port, so config LBK link */
>> pfvf = rvu_get_pfvf(rvu, pcifunc);
>> link = hw->cgx_links + pfvf->lbkid;
>> + } else if (is_rep_dev(rvu, pcifunc)) {
>> + link = hw->cgx_links + 0;
>> }
>>
>> if (link < 0)
>> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_rep.c
>b/drivers/net/ethernet/marvell/octeontx2/af/rvu_rep.c
>> new file mode 100644
>> index 000000000000..cf13c5f0a3c5
>> --- /dev/null
>> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_rep.c
>> @@ -0,0 +1,47 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Marvell RVU Admin Function driver
>> + *
>> + * Copyright (C) 2024 Marvell.
>> + *
>> + */
>> +
>> +#include <linux/types.h>
>> +#include <linux/device.h>
>> +#include <linux/module.h>
>> +#include <linux/pci.h>
>> +
>> +#include "rvu.h"
>> +#include "rvu_reg.h"
>> +
>> +int rvu_mbox_handler_get_rep_cnt(struct rvu *rvu, struct msg_req *req,
>> + struct get_rep_cnt_rsp *rsp)
>> +{
>> + int pf, vf, numvfs, hwvf, rep = 0;
>> + u16 pcifunc;
>> +
>> + rvu->rep_pcifunc = req->hdr.pcifunc;
>> + rsp->rep_cnt = rvu->cgx_mapped_pfs + rvu->cgx_mapped_vfs;
>> + rvu->rep_cnt = rsp->rep_cnt;
>> +
>> + rvu->rep2pfvf_map = devm_kzalloc(rvu->dev, rvu->rep_cnt *
>> + sizeof(u16), GFP_KERNEL);
>> + if (!rvu->rep2pfvf_map)
>> + return -ENOMEM;
>> +
>> + for (pf = 0; pf < rvu->hw->total_pfs; pf++) {
>> + if (!is_pf_cgxmapped(rvu, pf))
>> + continue;
>> + pcifunc = pf << RVU_PFVF_PF_SHIFT;
>> + rvu->rep2pfvf_map[rep] = pcifunc;
>> + rsp->rep_pf_map[rep] = pcifunc;
>> + rep++;
>> + rvu_get_pf_numvfs(rvu, pf, &numvfs, &hwvf);
>> + for (vf = 0; vf < numvfs; vf++) {
>> + rvu->rep2pfvf_map[rep] = pcifunc |
>> + ((vf + 1) & RVU_PFVF_FUNC_MASK);
>> + rsp->rep_pf_map[rep] = rvu->rep2pfvf_map[rep];
>> + rep++;
>> + }
>> + }
>> + return 0;
>> +}
>> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/Makefile
>b/drivers/net/ethernet/marvell/octeontx2/nic/Makefile
>> index 64a97a0a10ed..dbc971266865 100644
>> --- a/drivers/net/ethernet/marvell/octeontx2/nic/Makefile
>> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/Makefile
>> @@ -5,11 +5,13 @@
>>
>> obj-$(CONFIG_OCTEONTX2_PF) += rvu_nicpf.o otx2_ptp.o
>> obj-$(CONFIG_OCTEONTX2_VF) += rvu_nicvf.o otx2_ptp.o
>> +obj-$(CONFIG_RVU_ESWITCH) += rvu_rep.o
>>
>> rvu_nicpf-y := otx2_pf.o otx2_common.o otx2_txrx.o otx2_ethtool.o \
>> otx2_flows.o otx2_tc.o cn10k.o otx2_dmac_flt.o \
>> otx2_devlink.o qos_sq.o qos.o
>> rvu_nicvf-y := otx2_vf.o
>> +rvu_rep-y := rep.o
>>
>> rvu_nicpf-$(CONFIG_DCB) += otx2_dcbnl.o
>> rvu_nicpf-$(CONFIG_MACSEC) += cn10k_macsec.o
>> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
>b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
>> index 772fe01bdf98..d297138c356e 100644
>> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
>> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
>> @@ -29,6 +29,7 @@
>> #include "otx2_devlink.h"
>> #include <rvu_trace.h>
>> #include "qos.h"
>> +#include "rep.h"
>>
>> /* IPv4 flag more fragment bit */
>> #define IPV4_FLAG_MORE 0x20
>> @@ -439,6 +440,7 @@ struct otx2_nic {
>> #define OTX2_FLAG_PTP_ONESTEP_SYNC BIT_ULL(15)
>> #define OTX2_FLAG_ADPTV_INT_COAL_ENABLED BIT_ULL(16)
>> #define OTX2_FLAG_TC_MARK_ENABLED BIT_ULL(17)
>> +#define OTX2_FLAG_REP_MODE_ENABLED BIT_ULL(18)
>> u64 flags;
>> u64 *cq_op_addr;
>>
>> @@ -506,11 +508,19 @@ struct otx2_nic {
>> #if IS_ENABLED(CONFIG_MACSEC)
>> struct cn10k_mcs_cfg *macsec_cfg;
>> #endif
>> +
>> +#if IS_ENABLED(CONFIG_RVU_ESWITCH)
>> + struct rep_dev **reps;
>> + int rep_cnt;
>> + u16 rep_pf_map[RVU_MAX_REP];
>> + u16 esw_mode;
>> +#endif
>> };
>>
>> static inline bool is_otx2_lbkvf(struct pci_dev *pdev)
>> {
>> - return pdev->device == PCI_DEVID_OCTEONTX2_RVU_AFVF;
>> + return (pdev->device == PCI_DEVID_OCTEONTX2_RVU_AFVF) ||
>> + (pdev->device == PCI_DEVID_RVU_REP);
>> }
>>
>> static inline bool is_96xx_A0(struct pci_dev *pdev)
>> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
>b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
>> index 2b2afcc4b921..8078d1c1fff9 100644
>> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
>> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
>> @@ -1502,10 +1502,11 @@ int otx2_init_hw_resources(struct otx2_nic *pf)
>> hw->sqpool_cnt = otx2_get_total_tx_queues(pf);
>> hw->pool_cnt = hw->rqpool_cnt + hw->sqpool_cnt;
>>
>> - /* Maximum hardware supported transmit length */
>> - pf->tx_max_pktlen = pf->netdev->max_mtu + OTX2_ETH_HLEN;
>> -
>> - pf->rbsize = otx2_get_rbuf_size(pf, pf->netdev->mtu);
>> + if (!otx2_rep_dev(pf->pdev)) {
>> + /* Maximum hardware supported transmit length */
>> + pf->tx_max_pktlen = pf->netdev->max_mtu + OTX2_ETH_HLEN;
>> + pf->rbsize = otx2_get_rbuf_size(pf, pf->netdev->mtu);
>> + }
>>
>> mutex_lock(&mbox->lock);
>> /* NPA init */
>> @@ -1634,11 +1635,12 @@ void otx2_free_hw_resources(struct otx2_nic
>*pf)
>> otx2_pfc_txschq_stop(pf);
>> #endif
>>
>> - otx2_clean_qos_queues(pf);
>> + if (!otx2_rep_dev(pf->pdev))
>> + otx2_clean_qos_queues(pf);
>>
>> mutex_lock(&mbox->lock);
>> /* Disable backpressure */
>> - if (!(pf->pcifunc & RVU_PFVF_FUNC_MASK))
>> + if (!is_otx2_lbkvf(pf->pdev))
>> otx2_nix_config_bp(pf, false);
>> mutex_unlock(&mbox->lock);
>>
>> @@ -1670,7 +1672,8 @@ void otx2_free_hw_resources(struct otx2_nic *pf)
>> otx2_free_cq_res(pf);
>>
>> /* Free all ingress bandwidth profiles allocated */
>> - cn10k_free_all_ipolicers(pf);
>> + if (!otx2_rep_dev(pf->pdev))
>> + cn10k_free_all_ipolicers(pf);
>>
>> mutex_lock(&mbox->lock);
>> /* Reset NIX LF */
>> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
>b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
>> index fbd9fe98259f..5dcdd8b65837 100644
>> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
>> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
>> @@ -375,11 +375,13 @@ static void otx2_rcv_pkt_handler(struct otx2_nic
>*pfvf,
>> }
>> start += sizeof(*sg);
>> }
>> - otx2_set_rxhash(pfvf, cqe, skb);
>>
>> - skb_record_rx_queue(skb, cq->cq_idx);
>> - if (pfvf->netdev->features & NETIF_F_RXCSUM)
>> - skb->ip_summed = CHECKSUM_UNNECESSARY;
>> + if (!(pfvf->flags & OTX2_FLAG_REP_MODE_ENABLED)) {
>> + otx2_set_rxhash(pfvf, cqe, skb);
>> + skb_record_rx_queue(skb, cq->cq_idx);
>> + if (pfvf->netdev->features & NETIF_F_RXCSUM)
>> + skb->ip_summed = CHECKSUM_UNNECESSARY;
>> + }
>
>Does this mean representor have only one rxq, and it doesn't support
>checksum offload?
>
>Thanks
>William