Re: [PATCH net-next v4 1/4] octeon_ep: add PF-VF mailbox communication

From: shenjian (K)
Date: Thu Dec 14 2023 - 08:18:32 EST


A few nit comments


在 2023/12/13 11:58, Shinas Rasheed 写道:
Implement mailbox communication between PF and VFs.
PF-VF mailbox is used for all control commands from VF to PF and
asynchronous notification messages from PF to VF.

Signed-off-by: Shinas Rasheed <srasheed@xxxxxxxxxxx>
---
V4:
- Include [1/4] in the subject for this patch, which was lost in V3

V3: https://lore.kernel.org/all/20231211063355.2630028-2-srasheed@xxxxxxxxxxx/
- Corrected error cleanup logic for PF-VF mbox setup
- Removed double inclusion of types.h header file in octep_pfvf_mbox.c

V2: https://lore.kernel.org/all/20231209081450.2613561-2-srasheed@xxxxxxxxxxx/
- Remove unused variable

V1: https://lore.kernel.org/all/20231208070352.2606192-2-srasheed@xxxxxxxxxxx/

.../net/ethernet/marvell/octeon_ep/Makefile | 2 +-
.../marvell/octeon_ep/octep_cn9k_pf.c | 59 ++-
.../marvell/octeon_ep/octep_cnxk_pf.c | 46 ++-
.../marvell/octeon_ep/octep_ctrl_mbox.h | 4 +-
.../ethernet/marvell/octeon_ep/octep_main.c | 83 ++++-
.../ethernet/marvell/octeon_ep/octep_main.h | 45 ++-
.../marvell/octeon_ep/octep_pfvf_mbox.c | 335 ++++++++++++++++++
.../marvell/octeon_ep/octep_pfvf_mbox.h | 143 ++++++++
.../marvell/octeon_ep/octep_regs_cn9k_pf.h | 9 +
.../marvell/octeon_ep/octep_regs_cnxk_pf.h | 13 +
.../net/ethernet/marvell/octeon_ep/octep_tx.h | 24 +-
11 files changed, 714 insertions(+), 49 deletions(-)
create mode 100644 drivers/net/ethernet/marvell/octeon_ep/octep_pfvf_mbox.c
create mode 100644 drivers/net/ethernet/marvell/octeon_ep/octep_pfvf_mbox.h

diff --git a/drivers/net/ethernet/marvell/octeon_ep/Makefile b/drivers/net/ethernet/marvell/octeon_ep/Makefile
index 02a4a21bc298..62162ed63f34 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/Makefile
+++ b/drivers/net/ethernet/marvell/octeon_ep/Makefile
@@ -7,4 +7,4 @@ obj-$(CONFIG_OCTEON_EP) += octeon_ep.o
octeon_ep-y := octep_main.o octep_cn9k_pf.o octep_tx.o octep_rx.o \
octep_ethtool.o octep_ctrl_mbox.o octep_ctrl_net.o \
- octep_cnxk_pf.o
+ octep_pfvf_mbox.o octep_cnxk_pf.o
diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c b/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c
index 9209f1ec1b52..b5805969404f 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_cn9k_pf.c
@@ -362,16 +362,55 @@ static void octep_setup_mbox_regs_cn93_pf(struct octep_device *oct, int q_no)
{
struct octep_mbox *mbox = oct->mbox[q_no];
- mbox->q_no = q_no;
-
- /* PF mbox interrupt reg */
- mbox->mbox_int_reg = oct->mmio[0].hw_addr + CN93_SDP_EPF_MBOX_RINT(0);
-
/* PF to VF DATA reg. PF writes into this reg */
- mbox->mbox_write_reg = oct->mmio[0].hw_addr + CN93_SDP_R_MBOX_PF_VF_DATA(q_no);
+ mbox->pf_vf_data_reg = oct->mmio[0].hw_addr + CN93_SDP_MBOX_PF_VF_DATA(q_no);
/* VF to PF DATA reg. PF reads from this reg */
- mbox->mbox_read_reg = oct->mmio[0].hw_addr + CN93_SDP_R_MBOX_VF_PF_DATA(q_no);
+ mbox->vf_pf_data_reg = oct->mmio[0].hw_addr + CN93_SDP_MBOX_VF_PF_DATA(q_no);
+}
+
+/* Poll for mailbox messages from VF */
+static void octep_poll_pfvf_mailbox(struct octep_device *oct)
+{
+ u32 vf, active_vfs, active_rings_per_vf, vf_mbox_queue;
+ u64 reg0, reg1;
+
+ reg0 = octep_read_csr64(oct, CN93_SDP_EPF_MBOX_RINT(0));
+ reg1 = octep_read_csr64(oct, CN93_SDP_EPF_MBOX_RINT(1));
+ if (reg0 || reg1) {
+ active_vfs = CFG_GET_ACTIVE_VFS(oct->conf);
+ active_rings_per_vf = CFG_GET_ACTIVE_RPVF(oct->conf);
+ for (vf = 0; vf < active_vfs; vf++) {
+ vf_mbox_queue = vf * active_rings_per_vf;
+
+ if (vf_mbox_queue < 64) {
+ if (!(reg0 & (0x1UL << vf_mbox_queue)))
+ continue;
+ } else {
+ if (!(reg1 & (0x1UL << (vf_mbox_queue - 64))))
+ continue;
+ }
+
+ if (!oct->mbox[vf_mbox_queue]) {
+ dev_err(&oct->pdev->dev, "bad mbox vf %d\n", vf);
+ continue;
+ }
+ schedule_work(&oct->mbox[vf_mbox_queue]->wk.work);
+ }
+ if (reg0)
+ octep_write_csr64(oct, CN93_SDP_EPF_MBOX_RINT(0), reg0);
+ if (reg1)
+ octep_write_csr64(oct, CN93_SDP_EPF_MBOX_RINT(1), reg1);
+ }
+}
+
+/* PF-VF mailbox interrupt handler */
+static irqreturn_t octep_pfvf_mbox_intr_handler_cn93_pf(void *dev)
+{
+ struct octep_device *oct = (struct octep_device *)dev;
+
+ octep_poll_pfvf_mailbox(oct);
+ return IRQ_HANDLED;
}
/* Poll OEI events like heartbeat */
@@ -403,6 +442,7 @@ static irqreturn_t octep_oei_intr_handler_cn93_pf(void *dev)
*/
static void octep_poll_non_ioq_interrupts_cn93_pf(struct octep_device *oct)
{
+ octep_poll_pfvf_mailbox(oct);
octep_poll_oei_cn93_pf(oct);
}
@@ -646,6 +686,8 @@ static void octep_enable_interrupts_cn93_pf(struct octep_device *oct)
octep_write_csr64(oct, CN93_SDP_EPF_MISC_RINT_ENA_W1S, intr_mask);
octep_write_csr64(oct, CN93_SDP_EPF_DMA_RINT_ENA_W1S, intr_mask);
+ octep_write_csr64(oct, CN93_SDP_EPF_MBOX_RINT_ENA_W1S(0), -1ULL);
+ octep_write_csr64(oct, CN93_SDP_EPF_MBOX_RINT_ENA_W1S(1), -1ULL);
octep_write_csr64(oct, CN93_SDP_EPF_DMA_VF_RINT_ENA_W1S(0), -1ULL);
octep_write_csr64(oct, CN93_SDP_EPF_PP_VF_RINT_ENA_W1S(0), -1ULL);
@@ -672,6 +714,8 @@ static void octep_disable_interrupts_cn93_pf(struct octep_device *oct)
octep_write_csr64(oct, CN93_SDP_EPF_MISC_RINT_ENA_W1C, intr_mask);
octep_write_csr64(oct, CN93_SDP_EPF_DMA_RINT_ENA_W1C, intr_mask);
+ octep_write_csr64(oct, CN93_SDP_EPF_MBOX_RINT_ENA_W1C(0), -1ULL);
+ octep_write_csr64(oct, CN93_SDP_EPF_MBOX_RINT_ENA_W1C(1), -1ULL);
octep_write_csr64(oct, CN93_SDP_EPF_DMA_VF_RINT_ENA_W1C(0), -1ULL);
octep_write_csr64(oct, CN93_SDP_EPF_PP_VF_RINT_ENA_W1C(0), -1ULL);
@@ -807,6 +851,7 @@ void octep_device_setup_cn93_pf(struct octep_device *oct)
oct->hw_ops.setup_oq_regs = octep_setup_oq_regs_cn93_pf;
oct->hw_ops.setup_mbox_regs = octep_setup_mbox_regs_cn93_pf;
+ oct->hw_ops.mbox_intr_handler = octep_pfvf_mbox_intr_handler_cn93_pf;
oct->hw_ops.oei_intr_handler = octep_oei_intr_handler_cn93_pf;
oct->hw_ops.ire_intr_handler = octep_ire_intr_handler_cn93_pf;
oct->hw_ops.ore_intr_handler = octep_ore_intr_handler_cn93_pf;
diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_cnxk_pf.c b/drivers/net/ethernet/marvell/octeon_ep/octep_cnxk_pf.c
index 098a0c5c4d1c..5de0b5ecbc5f 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_cnxk_pf.c
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_cnxk_pf.c
@@ -392,16 +392,44 @@ static void octep_setup_mbox_regs_cnxk_pf(struct octep_device *oct, int q_no)
{
struct octep_mbox *mbox = oct->mbox[q_no];
- mbox->q_no = q_no;
-
- /* PF mbox interrupt reg */
- mbox->mbox_int_reg = oct->mmio[0].hw_addr + CNXK_SDP_EPF_MBOX_RINT(0);
-
/* PF to VF DATA reg. PF writes into this reg */
- mbox->mbox_write_reg = oct->mmio[0].hw_addr + CNXK_SDP_R_MBOX_PF_VF_DATA(q_no);
+ mbox->pf_vf_data_reg = oct->mmio[0].hw_addr + CNXK_SDP_MBOX_PF_VF_DATA(q_no);
/* VF to PF DATA reg. PF reads from this reg */
- mbox->mbox_read_reg = oct->mmio[0].hw_addr + CNXK_SDP_R_MBOX_VF_PF_DATA(q_no);
+ mbox->vf_pf_data_reg = oct->mmio[0].hw_addr + CNXK_SDP_MBOX_VF_PF_DATA(q_no);
+}
+
+static void octep_poll_pfvf_mailbox_cnxk_pf(struct octep_device *oct)
+{
+ u32 vf, active_vfs, active_rings_per_vf, vf_mbox_queue;
+ u64 reg0;
+
+ reg0 = octep_read_csr64(oct, CNXK_SDP_EPF_MBOX_RINT(0));
+ if (reg0) {
+ active_vfs = CFG_GET_ACTIVE_VFS(oct->conf);
+ active_rings_per_vf = CFG_GET_ACTIVE_RPVF(oct->conf);
+ for (vf = 0; vf < active_vfs; vf++) {
+ vf_mbox_queue = vf * active_rings_per_vf;
+ if (!(reg0 & (0x1UL << vf_mbox_queue)))
+ continue;
+
+ if (!oct->mbox[vf_mbox_queue]) {
+ dev_err(&oct->pdev->dev, "bad mbox vf %d\n", vf);
+ continue;
+ }
+ schedule_work(&oct->mbox[vf_mbox_queue]->wk.work);
+ }
+ if (reg0)
the checking of reg0 here seems unnecessary.

+ octep_write_csr64(oct, CNXK_SDP_EPF_MBOX_RINT(0), reg0);
+ }
+}
+
+static irqreturn_t octep_pfvf_mbox_intr_handler_cnxk_pf(void *dev)
+{
+ struct octep_device *oct = (struct octep_device *)dev;
+
+ octep_poll_pfvf_mailbox_cnxk_pf(oct);
+ return IRQ_HANDLED;
}

...

+#include <linux/types.h>
+#include <linux/errno.h>
+#include <linux/string.h>
+#include <linux/mutex.h>
+#include <linux/jiffies.h>
+#include <linux/sched.h>
+#include <linux/sched/signal.h>
+#include <linux/io.h>
+#include <linux/pci.h>
+#include <linux/etherdevice.h>
+
+#include "octep_config.h"
+#include "octep_main.h"
+#include "octep_pfvf_mbox.h"
+#include "octep_ctrl_net.h"
+
+static void octep_pfvf_validate_version(struct octep_device *oct, u32 vf_id,
redundant space before "u32 vf_id"

+ union octep_pfvf_mbox_word cmd,
+ union octep_pfvf_mbox_word *rsp)
+{
+ u32 vf_version = (u32)cmd.s_version.version;
+
+ if (vf_version <= OCTEP_PFVF_MBOX_VERSION_V1)
+ rsp->s_version.type = OCTEP_PFVF_MBOX_TYPE_RSP_ACK;
+ else
+ rsp->s_version.type = OCTEP_PFVF_MBOX_TYPE_RSP_NACK;
+}

...
+static void octep_pfvf_dev_remove(struct octep_device *oct, u32 vf_id,
+ union octep_pfvf_mbox_word cmd,
+ union octep_pfvf_mbox_word *rsp)
+{
+ int err;
+
+ err = octep_ctrl_net_dev_remove(oct, vf_id);
+ if (err) {
+ rsp->s.type = OCTEP_PFVF_MBOX_TYPE_RSP_NACK;
+ dev_err(&oct->pdev->dev, "Failed to acknowledge fw of vf %d removal\n",
+ vf_id);
+ return;
+ }
+ rsp->s.type = OCTEP_PFVF_MBOX_TYPE_RSP_ACK;
+}
+
+int octep_setup_pfvf_mbox(struct octep_device *oct)
+{
+ int i = 0, num_vfs = 0, rings_per_vf = 0;
looks unnecessary initialiazation here.

+ int ring = 0;
+
+ num_vfs = oct->conf->sriov_cfg.active_vfs;
+ rings_per_vf = oct->conf->sriov_cfg.max_rings_per_vf;
+
+ for (i = 0; i < num_vfs; i++) {
+ ring = rings_per_vf * i;
redundant space after 'ring'? also exist at other places.
+ oct->mbox[ring] = vzalloc(sizeof(*oct->mbox[ring]));
+
+ if (!oct->mbox[ring])
+ goto free_mbox;
+
+ memset(oct->mbox[ring], 0, sizeof(struct octep_mbox));
+ mutex_init(&oct->mbox[ring]->lock);
+ INIT_WORK(&oct->mbox[ring]->wk.work, octep_pfvf_mbox_work);
+ oct->mbox[ring]->wk.ctxptr = oct->mbox[ring];
+ oct->mbox[ring]->oct = oct;
+ oct->mbox[ring]->vf_id = i;
+ oct->hw_ops.setup_mbox_regs(oct, ring);
+ }
+ return 0;
+
+free_mbox:
+ while (i) {
+ i--;
+ ring = rings_per_vf * i;
+ cancel_work_sync(&oct->mbox[ring]->wk.work);
+ mutex_destroy(&oct->mbox[ring]->lock);
+ vfree(oct->mbox[ring]);
+ oct->mbox[ring] = NULL;
+ }
+ return -ENOMEM;
+}
+
+void octep_delete_pfvf_mbox(struct octep_device *oct)
+{
+ int rings_per_vf = oct->conf->sriov_cfg.max_rings_per_vf;
+ int num_vfs = oct->conf->sriov_cfg.active_vfs;
+ int i = 0, ring = 0, vf_srn = 0;
+
+ for (i = 0; i < num_vfs; i++) {
+ ring = vf_srn + rings_per_vf * i;
+ if (!oct->mbox[ring])
+ continue;
+
+ if (work_pending(&oct->mbox[ring]->wk.work))
+ cancel_work_sync(&oct->mbox[ring]->wk.work);
+
+ mutex_destroy(&oct->mbox[ring]->lock);
+ vfree(oct->mbox[ring]);
+ oct->mbox[ring] = NULL;
+ }
+}
+
+static void octep_pfvf_pf_get_data(struct octep_device *oct,
+ struct octep_mbox *mbox, int vf_id,
+ union octep_pfvf_mbox_word cmd,
+ union octep_pfvf_mbox_word *rsp)
+{
+ int length = 0;
+ int i = 0;
+ int err;
+ struct octep_iface_link_info link_info;
+ struct octep_iface_rx_stats rx_stats;
+ struct octep_iface_tx_stats tx_stats;
Variables should be placed with Revert-Chris Tree  sequency


...