Mailing List Archive

On Fri, Apr 26, 2024 at 11:20:11AM -0700, Vamsi Attunuru wrote:
> Adds a misc driver for Marvell CN10K DPI(DMA Engine) device's physical
> function which initializes DPI DMA hardware's global configuration and
> enables hardware mailbox channels between physical function (PF) and
> it's virtual functions (VF). VF device drivers (User space drivers) use
> this hw mailbox to communicate any required device configuration on it's
> respective VF device. Accordingly, this DPI PF driver provisions the
> VF device resources.
>
> At the hardware level, the DPI physical function (PF) acts as a management
> interface to setup the VF device resources, VF devices are only provisioned
> to handle or control the actual DMA Engine's data transfer capabilities.
>
> Signed-off-by: Vamsi Attunuru <vattunuru@marvell.com>
> ---
> Changes V5 -> V6:
> - Updated documentation

The documentation for where to find the userspace code needs to be in
the documentation file, not buried in a comment here in a changelog
section that will never show up anywhere.

thanks,

greg k-h

> -----Original Message-----
> From: Greg KH <gregkh@linuxfoundation.org>
> Sent: Saturday, April 27, 2024 4:37 PM
> To: Vamsi Krishna Attunuru <vattunuru@marvell.com>
> Cc: arnd@arndb.de; Jerin Jacob <jerinj@marvell.com>; linux-
> kernel@vger.kernel.org
> Subject: [EXTERNAL] Re: [PATCH v6 1/1] misc: mrvl-cn10k-dpi: add Octeon
> CN10K DPI administrative driver
>
> Prioritize security for external emails: Confirm sender and content safety
> before clicking links or opening attachments
>
> ----------------------------------------------------------------------
> On Fri, Apr 26, 2024 at 11:20:11AM -0700, Vamsi Attunuru wrote:
> > Adds a misc driver for Marvell CN10K DPI(DMA Engine) device's physical
> > function which initializes DPI DMA hardware's global configuration and
> > enables hardware mailbox channels between physical function (PF) and
> > it's virtual functions (VF). VF device drivers (User space drivers)
> > use this hw mailbox to communicate any required device configuration
> > on it's respective VF device. Accordingly, this DPI PF driver
> > provisions the VF device resources.
> >
> > At the hardware level, the DPI physical function (PF) acts as a
> > management interface to setup the VF device resources, VF devices are
> > only provisioned to handle or control the actual DMA Engine's data transfer
> capabilities.
> >
> > Signed-off-by: Vamsi Attunuru <vattunuru@marvell.com>
> > ---
> > Changes V5 -> V6:
> > - Updated documentation
>
> The documentation for where to find the userspace code needs to be in the
> documentation file, not buried in a comment here in a changelog section that
> will never show up anywhere.

Sure, I will add the details in documentation file and resend. I assumed like you wanted the user space code references for the review purpose, so mentioned as separately.

Thanks
Vamsi
>
> thanks,
>
> greg k-h

Adds a misc driver for Marvell CN10K DPI(DMA Engine) device's physical
function which initializes DPI DMA hardware's global configuration and
enables hardware mailbox channels between physical function (PF) and
it's virtual functions (VF). VF device drivers (User space drivers) use
this hw mailbox to communicate any required device configuration on it's
respective VF device. Accordingly, this DPI PF driver provisions the
VF device resources.

At the hardware level, the DPI physical function (PF) acts as a management
interface to setup the VF device resources, VF devices are only provisioned
to handle or control the actual DMA Engine's data transfer capabilities.

Signed-off-by: Vamsi Attunuru <vattunuru@marvell.com>
---
Changes V5 -> V6:
- Updated documentation
- Fixed data types in uapi file

Changes V4 -> V5:
- Fixed license and data types in uapi file

Changes V3 -> V4:
- Moved ioctl definations to .h file
- Fixed structure alignements which are passed in ioctl

Changes V2 -> V3:
- Added ioctl operation to the fops
- Used managed version of kzalloc & request_irq
- Addressed miscellaneous comments

Changes V1 -> V2:
- Fixed return values and busy-wait loops
- Merged .h file into .c file
- Fixed directory structure
- Removed module params
- Registered the device as misc device

Documentation/misc-devices/index.rst | 1 +
Documentation/misc-devices/mrvl_cn10k_dpi.rst | 57 ++
.../userspace-api/ioctl/ioctl-number.rst | 1 +
MAINTAINERS | 5 +
drivers/misc/Kconfig | 13 +
drivers/misc/Makefile | 2 +
drivers/misc/mrvl_cn10k_dpi.c | 685 ++++++++++++++++++
include/uapi/misc/mrvl_cn10k_dpi.h | 37 +
8 files changed, 801 insertions(+)

diff --git a/Documentation/misc-devices/index.rst b/Documentation/misc-devices/index.rst
index 2d0ce9138588..10f2e0f74e45 100644
--- a/Documentation/misc-devices/index.rst
+++ b/Documentation/misc-devices/index.rst
@@ -20,6 +20,7 @@ fit into other categories.
ics932s401
isl29003
lis3lv02d
+ mrvl_cn10k_dpi
max6875
oxsemi-tornado
pci-endpoint-test
diff --git a/Documentation/misc-devices/mrvl_cn10k_dpi.rst b/Documentation/misc-devices/mrvl_cn10k_dpi.rst
new file mode 100644
index 000000000000..cce202f114b7
--- /dev/null
+++ b/Documentation/misc-devices/mrvl_cn10k_dpi.rst
@@ -0,0 +1,57 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+===============================================
+Marvell CN10K DMA packet interface (DPI) driver
+===============================================
+
+Overview
+========
+
+DPI is a DMA packet interface hardware block in Marvell's CN10K silicon.
+DPI hardware comprises a physical function (PF), its virtual functions,
+mailbox logic, and a set of DMA engines & DMA command queues.
+
+DPI PF function is an administrative function which services the mailbox
+requests from its VF functions and provisions DMA engine resources to
+it's VF functions.
+
+mrvl_cn10k_dpi.ko misc driver loads on DPI PF device and services the
+mailbox commands submitted by the VF devices and accordingly initializes
+the DMA engines and VF device's DMA command queues. Also, driver creates
+/dev/mrvl-cn10k-dpi node to set DMA engine and PEM (PCIe interface) port
+attributes like fifo length, molr, mps & mrrs.
+
+DPI PF driver is just an administrative driver to setup its VF device's
+queues and provisions the hardware resources, it can not initiate any
+DMA operations. Only VF devices are provisioned with DMA capabilities.
+
+Driver location
+===============
+
+drivers/misc/mrvl_cn10k_dpi.c
+
+Driver IOCTLs
+=============
+
+:c:macro::`DPI_MPS_MRRS_CFG`
+ioctl that sets max payload size & max read request size parameters of
+a pem port to which DMA engines are wired.
+
+
+:c:macro::`DPI_ENGINE_CFG`
+ioctl that sets DMA engine's fifo sizes & max outstanding load request
+thresholds.
+
+Userspace code example
+----------------------
+
+DPI VF devices are managed by user space drivers, below is a reference
+code to the user space driver's mailbox command exchange with DPI PF
+driver through hardware mailbox.
+
+https://github.com/VamsiKrishnaA99/dpi-dma/blob/main/driver/roc_dpi.c
+
+Below is a sample application that uses driver IOCTLs to setup DMA engine
+and PEM port attributes over `/dev/mrvl-cn10k-dpi` node.
+
+https://github.com/VamsiKrishnaA99/dpi-dma/blob/main/application/main.c
diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst b/Documentation/userspace-api/ioctl/ioctl-number.rst
index 457e16f06e04..e6fd0c386b59 100644
--- a/Documentation/userspace-api/ioctl/ioctl-number.rst
+++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
@@ -358,6 +358,7 @@ Code Seq# Include File Comments
0xB6 all linux/fpga-dfl.h
0xB7 all uapi/linux/remoteproc_cdev.h <mailto:linux-remoteproc@vger.kernel.org>
0xB7 all uapi/linux/nsfs.h <mailto:Andrei Vagin <avagin@openvz.org>>
+0xB8 01-02 uapi/misc/mrvl_cn10k_dpi.h Marvell CN10K DPI driver
0xC0 00-0F linux/usb/iowarrior.h
0xCA 00-0F uapi/misc/cxl.h
0xCA 10-2F uapi/misc/ocxl.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 960512bec428..ab77232d583e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13104,6 +13104,11 @@ S: Supported
F: Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml
F: drivers/mmc/host/sdhci-xenon*

+MARVELL OCTEON CN10K DPI DRIVER
+M: Vamsi Attunuru <vattunuru@marvell.com>
+S: Maintained
+F: drivers/misc/mrvl_cn10k_dpi.c
+
MATROX FRAMEBUFFER DRIVER
L: linux-fbdev@vger.kernel.org
S: Orphan
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 4fb291f0bf7c..78470ef2538f 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -574,6 +574,19 @@ config NSM
To compile this driver as a module, choose M here.
The module will be called nsm.

+config MARVELL_CN10K_DPI
+ tristate "Octeon CN10K DPI driver"
+ depends on ARM64 && PCI
+ help
+ Enables Octeon CN10K DMA packet interface (DPI) driver which intializes
+ DPI hardware's physical function (PF) device's global configuration and
+ its virtual function's (VFs) resource configuration to enable DMA transfers.
+ DPI PF device does not have any data movement functionality, it only serves
+ VF's resource configuration requests.
+
+ To compile this driver as a module, choose M here: the module
+ will be called mrvl_cn10k_dpi.
+
source "drivers/misc/c2port/Kconfig"
source "drivers/misc/eeprom/Kconfig"
source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index ea6ea5bbbc9c..5106bf96ea5c 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -68,3 +68,5 @@ obj-$(CONFIG_TMR_INJECT) += xilinx_tmr_inject.o
obj-$(CONFIG_TPS6594_ESM) += tps6594-esm.o
obj-$(CONFIG_TPS6594_PFSM) += tps6594-pfsm.o
obj-$(CONFIG_NSM) += nsm.o
+obj-$(CONFIG_MARVELL_CN10K_DPI) += mrvl_cn10k_dpi.o
+obj-y += mrvl_cn10k_dpi.o
diff --git a/drivers/misc/mrvl_cn10k_dpi.c b/drivers/misc/mrvl_cn10k_dpi.c
new file mode 100644
index 000000000000..bd99583994f9
--- /dev/null
+++ b/drivers/misc/mrvl_cn10k_dpi.c
@@ -0,0 +1,685 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Marvell Octeon CN10K DPI driver
+ *
+ * Copyright (C) 2024 Marvell.
+ *
+ */
+
+#include <linux/compat.h>
+#include <linux/delay.h>
+#include <linux/miscdevice.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+
+#include <uapi/misc/mrvl_cn10k_dpi.h>
+
+#define DPI_DRV_NAME "mrvl-cn10k-dpi"
+
+/* PCI device IDs */
+#define PCI_DEVID_MRVL_CN10K_DPI_PF 0xA080
+#define PCI_SUBDEVID_MRVL_CN10K_DPI_PF 0xB900
+
+/* PCI BAR nos */
+#define PCI_DPI_CFG_BAR 0
+
+/* MSI-X interrupts */
+#define DPI_MAX_REQQ_INT 32
+#define DPI_MAX_CC_INT 64
+
+/* MBOX MSI-X interrupt vector index */
+#define DPI_MBOX_PF_VF_INT_IDX 0x75
+
+#define DPI_MAX_IRQS (DPI_MBOX_PF_VF_INT_IDX + 1)
+
+#define DPI_MAX_VFS 32
+
+#define DPI_ENGINE_MASK GENMASK(2, 0)
+
+#define DPI_DMA_IDS_DMA_NPA_PF_FUNC(x) (((x) & GENMASK_ULL(15, 0)) << 16)
+#define DPI_DMA_IDS_INST_STRM(x) (((x) & GENMASK_ULL(7, 0)) << 40)
+#define DPI_DMA_IDS_DMA_STRM(x) (((x) & GENMASK_ULL(7, 0)) << 32)
+#define DPI_DMA_ENG_EN_MOLR(x) (((x) & GENMASK_ULL(9, 0)) << 32)
+#define DPI_EBUS_PORTX_CFG_MPS(x) (((x) & GENMASK(2, 0)) << 4)
+#define DPI_DMA_IDS_DMA_SSO_PF_FUNC(x) ((x) & GENMASK(15, 0))
+#define DPI_DMA_IDS2_INST_AURA(x) ((x) & GENMASK(19, 0))
+#define DPI_DMA_IBUFF_CSIZE_CSIZE(x) ((x) & GENMASK(13, 0))
+#define DPI_EBUS_PORTX_CFG_MRRS(x) ((x) & GENMASK(2, 0))
+#define DPI_ENG_BUF_BLKS(x) ((x) & GENMASK(5, 0))
+#define DPI_DMA_CONTROL_DMA_ENB GENMASK_ULL(53, 48)
+
+#define DPI_DMA_CONTROL_O_MODE BIT_ULL(14)
+#define DPI_DMA_CONTROL_LDWB BIT_ULL(32)
+#define DPI_DMA_CONTROL_WQECSMODE1 BIT_ULL(37)
+#define DPI_DMA_CONTROL_ZBWCSEN BIT_ULL(39)
+#define DPI_DMA_CONTROL_WQECSOFF(offset) (((u64)offset) << 40)
+#define DPI_DMA_CONTROL_WQECSDIS BIT_ULL(47)
+#define DPI_DMA_CONTROL_PKT_EN BIT_ULL(56)
+#define DPI_DMA_IBUFF_CSIZE_NPA_FREE BIT(16)
+
+#define DPI_CTL_EN BIT_ULL(0)
+#define DPI_DMA_CC_INT BIT_ULL(0)
+#define DPI_DMA_QRST BIT_ULL(0)
+
+#define DPI_REQQ_INT_INSTRFLT BIT_ULL(0)
+#define DPI_REQQ_INT_RDFLT BIT_ULL(1)
+#define DPI_REQQ_INT_WRFLT BIT_ULL(2)
+#define DPI_REQQ_INT_CSFLT BIT_ULL(3)
+#define DPI_REQQ_INT_INST_DBO BIT_ULL(4)
+#define DPI_REQQ_INT_INST_ADDR_NULL BIT_ULL(5)
+#define DPI_REQQ_INT_INST_FILL_INVAL BIT_ULL(6)
+#define DPI_REQQ_INT_INSTR_PSN BIT_ULL(7)
+
+#define DPI_REQQ_INT \
+ (DPI_REQQ_INT_INSTRFLT | \
+ DPI_REQQ_INT_RDFLT | \
+ DPI_REQQ_INT_WRFLT | \
+ DPI_REQQ_INT_CSFLT | \
+ DPI_REQQ_INT_INST_DBO | \
+ DPI_REQQ_INT_INST_ADDR_NULL | \
+ DPI_REQQ_INT_INST_FILL_INVAL | \
+ DPI_REQQ_INT_INSTR_PSN)
+
+#define DPI_PF_RAS_EBI_DAT_PSN BIT_ULL(0)
+#define DPI_PF_RAS_NCB_DAT_PSN BIT_ULL(1)
+#define DPI_PF_RAS_NCB_CMD_PSN BIT_ULL(2)
+
+#define DPI_PF_RAS_INT \
+ (DPI_PF_RAS_EBI_DAT_PSN | \
+ DPI_PF_RAS_NCB_DAT_PSN | \
+ DPI_PF_RAS_NCB_CMD_PSN)
+
+#define DPI_DMAX_IBUFF_CSIZE(x) (0x0ULL | ((x) << 11))
+#define DPI_DMAX_IDS(x) (0x18ULL | ((x) << 11))
+#define DPI_DMAX_IDS2(x) (0x20ULL | ((x) << 11))
+#define DPI_DMAX_QRST(x) (0x30ULL | ((x) << 11))
+
+#define DPI_CTL 0x10010ULL
+#define DPI_DMA_CONTROL 0x10018ULL
+#define DPI_DMA_ENGX_EN(x) (0x10040ULL | ((x) << 3))
+#define DPI_ENGX_BUF(x) (0x100C0ULL | ((x) << 3))
+
+#define DPI_EBUS_PORTX_CFG(x) (0x10100ULL | ((x) << 3))
+
+#define DPI_PF_RAS 0x10308ULL
+#define DPI_PF_RAS_ENA_W1C 0x10318ULL
+
+#define DPI_DMA_CCX_INT(x) (0x11000ULL | ((x) << 3))
+#define DPI_DMA_CCX_INT_ENA_W1C(x) (0x11800ULL | ((x) << 3))
+
+#define DPI_REQQX_INT(x) (0x12C00ULL | ((x) << 5))
+#define DPI_REQQX_INT_ENA_W1C(x) (0x13800ULL | ((x) << 5))
+
+#define DPI_MBOX_PF_VF_DATA0(x) (0x16000ULL | ((x) << 4))
+#define DPI_MBOX_PF_VF_DATA1(x) (0x16008ULL | ((x) << 4))
+
+#define DPI_MBOX_VF_PF_INT 0x16300ULL
+#define DPI_MBOX_VF_PF_INT_W1S 0x16308ULL
+#define DPI_MBOX_VF_PF_INT_ENA_W1C 0x16310ULL
+#define DPI_MBOX_VF_PF_INT_ENA_W1S 0x16318ULL
+
+#define DPI_WCTL_FIF_THR 0x17008ULL
+
+#define DPI_EBUS_MAX_PORTS 2
+
+#define DPI_EBUS_MRRS_MIN 128
+#define DPI_EBUS_MRRS_MAX 1024
+#define DPI_EBUS_MPS_MIN 128
+#define DPI_EBUS_MPS_MAX 1024
+#define DPI_WCTL_FIFO_THRESHOLD 0x30
+
+#define DPI_QUEUE_OPEN 0x1
+#define DPI_QUEUE_CLOSE 0x2
+#define DPI_REG_DUMP 0x3
+#define DPI_GET_REG_CFG 0x4
+#define DPI_QUEUE_OPEN_V2 0x5
+
+enum dpi_mbox_rsp_type {
+ DPI_MBOX_TYPE_CMD,
+ DPI_MBOX_TYPE_RSP_ACK,
+ DPI_MBOX_TYPE_RSP_NACK,
+};
+
+struct dpivf_config {
+ u16 csize;
+ u32 aura;
+ u16 sso_pf_func;
+ u16 npa_pf_func;
+};
+
+struct dpipf_vf {
+ u8 this_vfid;
+ bool setup_done;
+ struct dpivf_config vf_config;
+};
+
+/* DPI device mailbox */
+struct dpi_mbox {
+ struct work_struct work;
+ /* lock to serialize mbox requests */
+ struct mutex lock;
+ struct dpipf *pf;
+ u8 __iomem *pf_vf_data_reg;
+ u8 __iomem *vf_pf_data_reg;
+};
+
+struct dpipf {
+ struct miscdevice miscdev;
+ void __iomem *reg_base;
+ struct pci_dev *pdev;
+ struct dpipf_vf vf[DPI_MAX_VFS];
+ /* Mailbox to talk to VFs */
+ struct dpi_mbox *mbox[DPI_MAX_VFS];
+};
+
+union dpi_mbox_message_t {
+ u64 u[2];
+ struct dpi_mbox_message_s {
+ /* VF ID to configure */
+ u64 vfid :8;
+ /* Command code */
+ u64 cmd :4;
+ /* Command buffer size in 8-byte words */
+ u64 csize :14;
+ /* Aura of the command buffer */
+ u64 aura :20;
+ /* SSO PF function */
+ u64 sso_pf_func :16;
+ /* NPA PF function */
+ u64 npa_pf_func :16;
+ /* Work queue completion status enable */
+ u64 wqecs :1;
+ /* Work queue completion status byte offset */
+ u64 wqecsoff :7;
+ /* Reserved */
+ u64 rsvd :42;
+ } s __packed;
+};
+
+static inline void dpi_reg_write(struct dpipf *dpi, u64 offset, u64 val)
+{
+ writeq(val, dpi->reg_base + offset);
+}
+
+static inline u64 dpi_reg_read(struct dpipf *dpi, u64 offset)
+{
+ return readq(dpi->reg_base + offset);
+}
+
+static void dpi_wqe_cs_offset(struct dpipf *dpi, u8 offset)
+{
+ u64 reg;
+
+ reg = dpi_reg_read(dpi, DPI_DMA_CONTROL);
+ reg &= ~DPI_DMA_CONTROL_WQECSDIS;
+ reg |= DPI_DMA_CONTROL_ZBWCSEN | DPI_DMA_CONTROL_WQECSMODE1;
+ reg |= DPI_DMA_CONTROL_WQECSOFF(offset);
+ dpi_reg_write(dpi, DPI_DMA_CONTROL, reg);
+}
+
+static int dpi_queue_init(struct dpipf *dpi, struct dpipf_vf *dpivf, u8 vf)
+{
+ u16 sso_pf_func = dpivf->vf_config.sso_pf_func;
+ u16 npa_pf_func = dpivf->vf_config.npa_pf_func;
+ u16 csize = dpivf->vf_config.csize;
+ u32 aura = dpivf->vf_config.aura;
+ unsigned long timeout;
+ u64 reg;
+
+ dpi_reg_write(dpi, DPI_DMAX_QRST(vf), DPI_DMA_QRST);
+
+ /* Wait for a maximum of 3 sec */
+ timeout = jiffies + msecs_to_jiffies(3000);
+ while (!time_after(jiffies, timeout)) {
+ reg = dpi_reg_read(dpi, DPI_DMAX_QRST(vf));
+ if (!(reg & DPI_DMA_QRST))
+ break;
+
+ usleep_range(500, 1000);
+ }
+
+ if (reg & DPI_DMA_QRST) {
+ dev_err(&dpi->pdev->dev, "Queue reset failed\n");
+ return -EBUSY;
+ }
+
+ dpi_reg_write(dpi, DPI_DMAX_IDS2(vf), 0);
+ dpi_reg_write(dpi, DPI_DMAX_IDS(vf), 0);
+
+ reg = DPI_DMA_IBUFF_CSIZE_CSIZE(csize) | DPI_DMA_IBUFF_CSIZE_NPA_FREE;
+ dpi_reg_write(dpi, DPI_DMAX_IBUFF_CSIZE(vf), reg);
+
+ reg = dpi_reg_read(dpi, DPI_DMAX_IDS2(vf));
+ reg |= DPI_DMA_IDS2_INST_AURA(aura);
+ dpi_reg_write(dpi, DPI_DMAX_IDS2(vf), reg);
+
+ reg = dpi_reg_read(dpi, DPI_DMAX_IDS(vf));
+ reg |= DPI_DMA_IDS_DMA_NPA_PF_FUNC(npa_pf_func);
+ reg |= DPI_DMA_IDS_DMA_SSO_PF_FUNC(sso_pf_func);
+ reg |= DPI_DMA_IDS_DMA_STRM(vf + 1);
+ reg |= DPI_DMA_IDS_INST_STRM(vf + 1);
+ dpi_reg_write(dpi, DPI_DMAX_IDS(vf), reg);
+
+ return 0;
+}
+
+static void dpi_queue_fini(struct dpipf *dpi, struct dpipf_vf *dpivf, u8 vf)
+{
+ dpi_reg_write(dpi, DPI_DMAX_QRST(vf), DPI_DMA_QRST);
+
+ /* Reset IDS and IDS2 registers */
+ dpi_reg_write(dpi, DPI_DMAX_IDS2(vf), 0);
+ dpi_reg_write(dpi, DPI_DMAX_IDS(vf), 0);
+}
+
+static void dpi_poll_pfvf_mbox(struct dpipf *dpi)
+{
+ u64 reg;
+ u32 vf;
+
+ reg = dpi_reg_read(dpi, DPI_MBOX_VF_PF_INT);
+ if (reg) {
+ for (vf = 0; vf < pci_num_vf(dpi->pdev); vf++) {
+ if (reg & BIT_ULL(vf))
+ schedule_work(&dpi->mbox[vf]->work);
+ }
+ dpi_reg_write(dpi, DPI_MBOX_VF_PF_INT, reg);
+ }
+}
+
+static irqreturn_t dpi_mbox_intr_handler(int irq, void *data)
+{
+ struct dpipf *dpi = data;
+
+ dpi_poll_pfvf_mbox(dpi);
+
+ return IRQ_HANDLED;
+}
+
+static int queue_config(struct dpipf *dpi, struct dpipf_vf *dpivf, union dpi_mbox_message_t *msg)
+{
+ int ret = 0;
+
+ switch (msg->s.cmd) {
+ case DPI_QUEUE_OPEN:
+ case DPI_QUEUE_OPEN_V2:
+ dpivf->vf_config.aura = msg->s.aura;
+ dpivf->vf_config.csize = msg->s.cmd == DPI_QUEUE_OPEN ? msg->s.csize / 8 :
+ msg->s.csize;
+ dpivf->vf_config.sso_pf_func = msg->s.sso_pf_func;
+ dpivf->vf_config.npa_pf_func = msg->s.npa_pf_func;
+ ret = dpi_queue_init(dpi, dpivf, msg->s.vfid);
+ if (!ret) {
+ if (msg->s.wqecs)
+ dpi_wqe_cs_offset(dpi, msg->s.wqecsoff);
+ dpivf->setup_done = true;
+ }
+ break;
+ case DPI_QUEUE_CLOSE:
+ dpivf->vf_config.aura = 0;
+ dpivf->vf_config.csize = 0;
+ dpivf->vf_config.sso_pf_func = 0;
+ dpivf->vf_config.npa_pf_func = 0;
+ dpi_queue_fini(dpi, dpivf, msg->s.vfid);
+ dpivf->setup_done = false;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return ret;
+}
+
+static void dpi_pfvf_mbox_work(struct work_struct *work)
+{
+ struct dpi_mbox *mbox = container_of(work, struct dpi_mbox, work);
+ union dpi_mbox_message_t msg = { 0 };
+ struct dpipf_vf *dpivf;
+ struct dpipf *dpi;
+ int ret;
+
+ dpi = mbox->pf;
+
+ mutex_lock(&mbox->lock);
+ msg.u[0] = readq(mbox->vf_pf_data_reg);
+ if (unlikely(msg.u[0] == (u64)-1))
+ goto exit;
+
+ if (unlikely(msg.s.vfid >= pci_num_vf(dpi->pdev))) {
+ dev_err(&dpi->pdev->dev, "Invalid vfid:%d\n", msg.s.vfid);
+ goto exit;
+ }
+
+ dpivf = &dpi->vf[msg.s.vfid];
+ msg.u[1] = readq(mbox->pf_vf_data_reg);
+
+ ret = queue_config(dpi, dpivf, &msg);
+ if (ret < 0)
+ writeq(DPI_MBOX_TYPE_RSP_NACK, mbox->pf_vf_data_reg);
+ else
+ writeq(DPI_MBOX_TYPE_RSP_ACK, mbox->pf_vf_data_reg);
+exit:
+ mutex_unlock(&mbox->lock);
+}
+
+/* Setup registers for a PF mailbox */
+static void dpi_setup_mbox_regs(struct dpipf *dpi, int vf)
+{
+ struct dpi_mbox *mbox = dpi->mbox[vf];
+
+ mbox->pf_vf_data_reg = dpi->reg_base + DPI_MBOX_PF_VF_DATA0(vf);
+ mbox->vf_pf_data_reg = dpi->reg_base + DPI_MBOX_PF_VF_DATA1(vf);
+}
+
+static int dpi_pfvf_mbox_setup(struct dpipf *dpi)
+{
+ int vf;
+
+ for (vf = 0; vf < DPI_MAX_VFS; vf++) {
+ dpi->mbox[vf] = devm_kzalloc(&dpi->pdev->dev, sizeof(*dpi->mbox[vf]), GFP_KERNEL);
+
+ if (!dpi->mbox[vf])
+ return -ENOMEM;
+
+ mutex_init(&dpi->mbox[vf]->lock);
+ INIT_WORK(&dpi->mbox[vf]->work, dpi_pfvf_mbox_work);
+ dpi->mbox[vf]->pf = dpi;
+ dpi_setup_mbox_regs(dpi, vf);
+ }
+
+ return 0;
+}
+
+static void dpi_pfvf_mbox_destroy(struct dpipf *dpi)
+{
+ unsigned int vf;
+
+ for (vf = 0; vf < DPI_MAX_VFS; vf++) {
+ if (work_pending(&dpi->mbox[vf]->work))
+ cancel_work_sync(&dpi->mbox[vf]->work);
+
+ dpi->mbox[vf] = NULL;
+ }
+}
+
+static void dpi_init(struct dpipf *dpi)
+{
+ unsigned int engine, port;
+ u8 mrrs_val, mps_val;
+ u64 reg;
+
+ for (engine = 0; engine < DPI_MAX_ENGINES; engine++) {
+ if (engine == 4 || engine == 5)
+ reg = DPI_ENG_BUF_BLKS(16);
+ else
+ reg = DPI_ENG_BUF_BLKS(8);
+
+ dpi_reg_write(dpi, DPI_ENGX_BUF(engine), reg);
+ }
+
+ reg = DPI_DMA_CONTROL_ZBWCSEN | DPI_DMA_CONTROL_PKT_EN | DPI_DMA_CONTROL_LDWB |
+ DPI_DMA_CONTROL_O_MODE | DPI_DMA_CONTROL_DMA_ENB;
+
+ dpi_reg_write(dpi, DPI_DMA_CONTROL, reg);
+ dpi_reg_write(dpi, DPI_CTL, DPI_CTL_EN);
+
+ mrrs_val = 2; /* 512B */
+ mps_val = 1; /* 256B */
+
+ for (port = 0; port < DPI_EBUS_MAX_PORTS; port++) {
+ reg = dpi_reg_read(dpi, DPI_EBUS_PORTX_CFG(port));
+ reg &= ~(DPI_EBUS_PORTX_CFG_MRRS(7) | DPI_EBUS_PORTX_CFG_MPS(7));
+ reg |= DPI_EBUS_PORTX_CFG_MPS(mps_val) | DPI_EBUS_PORTX_CFG_MRRS(mrrs_val);
+ dpi_reg_write(dpi, DPI_EBUS_PORTX_CFG(port), reg);
+ }
+
+ dpi_reg_write(dpi, DPI_WCTL_FIF_THR, DPI_WCTL_FIFO_THRESHOLD);
+}
+
+static void dpi_fini(struct dpipf *dpi)
+{
+ unsigned int engine;
+
+ for (engine = 0; engine < DPI_MAX_ENGINES; engine++)
+ dpi_reg_write(dpi, DPI_ENGX_BUF(engine), 0);
+
+ dpi_reg_write(dpi, DPI_DMA_CONTROL, 0);
+ dpi_reg_write(dpi, DPI_CTL, 0);
+}
+
+static void dpi_free_irq_vectors(void *pdev)
+{
+ pci_free_irq_vectors((struct pci_dev *)pdev);
+}
+
+static int dpi_irq_init(struct dpipf *dpi)
+{
+ struct pci_dev *pdev = dpi->pdev;
+ struct device *dev = &pdev->dev;
+ int i, ret;
+
+ /* Clear all RAS interrupts */
+ dpi_reg_write(dpi, DPI_PF_RAS, DPI_PF_RAS_INT);
+
+ /* Clear all RAS interrupt enable bits */
+ dpi_reg_write(dpi, DPI_PF_RAS_ENA_W1C, DPI_PF_RAS_INT);
+
+ for (i = 0; i < DPI_MAX_REQQ_INT; i++) {
+ dpi_reg_write(dpi, DPI_REQQX_INT(i), DPI_REQQ_INT);
+ dpi_reg_write(dpi, DPI_REQQX_INT_ENA_W1C(i), DPI_REQQ_INT);
+ }
+
+ for (i = 0; i < DPI_MAX_CC_INT; i++) {
+ dpi_reg_write(dpi, DPI_DMA_CCX_INT(i), DPI_DMA_CC_INT);
+ dpi_reg_write(dpi, DPI_DMA_CCX_INT_ENA_W1C(i), DPI_DMA_CC_INT);
+ }
+
+ ret = pci_alloc_irq_vectors(pdev, DPI_MAX_IRQS, DPI_MAX_IRQS, PCI_IRQ_MSIX);
+ if (ret != DPI_MAX_IRQS) {
+ dev_err(dev, "DPI: Failed to alloc %d msix irqs\n", DPI_MAX_IRQS);
+ return ret;
+ }
+
+ ret = devm_add_action_or_reset(dev, dpi_free_irq_vectors, pdev);
+ if (ret) {
+ dev_err(dev, "DPI: Failed to add irq free action\n");
+ return ret;
+ }
+
+ ret = devm_request_irq(dev, pci_irq_vector(pdev, DPI_MBOX_PF_VF_INT_IDX),
+ dpi_mbox_intr_handler, 0, "dpi-mbox", dpi);
+ if (ret) {
+ dev_err(dev, "DPI: request_irq failed for mbox; err=%d\n", ret);
+ return ret;
+ }
+
+ dpi_reg_write(dpi, DPI_MBOX_VF_PF_INT_ENA_W1S, GENMASK_ULL(31, 0));
+
+ return 0;
+}
+
+static int dpi_mps_mrrs_config(struct dpipf *dpi, void __user *arg)
+{
+ struct dpi_mps_mrrs_cfg cfg;
+ u8 mrrs_val, mps_val;
+ u64 reg;
+
+ if (copy_from_user(&cfg, arg, sizeof(struct dpi_mps_mrrs_cfg)))
+ return -EFAULT;
+
+ if (cfg.max_read_req_sz < DPI_EBUS_MRRS_MIN || cfg.max_read_req_sz > DPI_EBUS_MRRS_MAX ||
+ !is_power_of_2(cfg.max_read_req_sz)) {
+ dev_err(&dpi->pdev->dev, "Invalid MRRS size:%u\n", cfg.max_read_req_sz);
+ return -EINVAL;
+ }
+
+ if (cfg.max_payload_sz < DPI_EBUS_MPS_MIN || cfg.max_payload_sz > DPI_EBUS_MPS_MAX ||
+ !is_power_of_2(cfg.max_payload_sz)) {
+ dev_err(&dpi->pdev->dev, "Invalid MPS size:%u\n", cfg.max_payload_sz);
+ return -EINVAL;
+ }
+
+ if (cfg.port >= DPI_EBUS_MAX_PORTS) {
+ dev_err(&dpi->pdev->dev, "Invalid EBUS port:%u\n", cfg.port);
+ return -EINVAL;
+ }
+
+ mrrs_val = fls(cfg.max_read_req_sz >> 8);
+ mps_val = fls(cfg.max_payload_sz >> 8);
+
+ reg = dpi_reg_read(dpi, DPI_EBUS_PORTX_CFG(cfg.port));
+ reg &= ~(DPI_EBUS_PORTX_CFG_MRRS(0x7) | DPI_EBUS_PORTX_CFG_MPS(0x7));
+ reg |= DPI_EBUS_PORTX_CFG_MPS(mps_val) | DPI_EBUS_PORTX_CFG_MRRS(mrrs_val);
+ dpi_reg_write(dpi, DPI_EBUS_PORTX_CFG(cfg.port), reg);
+
+ return 0;
+}
+
+static int dpi_engine_config(struct dpipf *dpi, void __user *arg)
+{
+ struct dpi_engine_cfg cfg;
+ unsigned int engine;
+ u8 *eng_buf;
+ u64 reg;
+
+ if (copy_from_user(&cfg, arg, sizeof(struct dpi_engine_cfg)))
+ return -EFAULT;
+
+ eng_buf = (u8 *)&cfg.fifo_mask;
+
+ for (engine = 0; engine < DPI_MAX_ENGINES; engine++) {
+ reg = DPI_ENG_BUF_BLKS(eng_buf[engine & DPI_ENGINE_MASK]);
+ dpi_reg_write(dpi, DPI_ENGX_BUF(engine), reg);
+
+ if (cfg.update_molr) {
+ reg = DPI_DMA_ENG_EN_MOLR(cfg.molr[engine & DPI_ENGINE_MASK]);
+ dpi_reg_write(dpi, DPI_DMA_ENGX_EN(engine), reg);
+ }
+ }
+
+ return 0;
+}
+
+static long dpi_dev_ioctl(struct file *fptr, unsigned int cmd, unsigned long data)
+{
+ void __user *arg = (void __user *)data;
+ struct dpipf *dpi;
+ int ret = -EINVAL;
+
+ dpi = container_of(fptr->private_data, struct dpipf, miscdev);
+
+ switch (cmd) {
+ case DPI_MPS_MRRS_CFG:
+ ret = dpi_mps_mrrs_config(dpi, arg);
+ break;
+ case DPI_ENGINE_CFG:
+ ret = dpi_engine_config(dpi, arg);
+ break;
+ default:
+ break;
+ }
+
+ return ret;
+}
+
+static const struct file_operations dpi_device_fops = {
+ .owner = THIS_MODULE,
+ .unlocked_ioctl = dpi_dev_ioctl,
+ .compat_ioctl = compat_ptr_ioctl,
+};
+
+static int dpi_probe(struct pci_dev *pdev, const struct pci_device_id *id)
+{
+ struct device *dev = &pdev->dev;
+ struct dpipf *dpi;
+ int ret;
+
+ dpi = devm_kzalloc(dev, sizeof(*dpi), GFP_KERNEL);
+ if (!dpi)
+ return -ENOMEM;
+
+ dpi->pdev = pdev;
+
+ ret = pcim_enable_device(pdev);
+ if (ret) {
+ dev_err(dev, "DPI: Failed to enable PCI device\n");
+ return ret;
+ }
+
+ ret = pcim_iomap_regions(pdev, BIT(0) | BIT(4), DPI_DRV_NAME);
+ if (ret) {
+ dev_err(dev, "DPI: Failed to request MMIO region\n");
+ return ret;
+ }
+
+ dpi->reg_base = pcim_iomap_table(pdev)[PCI_DPI_CFG_BAR];
+
+ /* Initialize global PF registers */
+ dpi_init(dpi);
+
+ /* Setup PF-VF mailbox */
+ ret = dpi_pfvf_mbox_setup(dpi);
+ if (ret) {
+ dev_err(dev, "DPI: Failed to setup pf-vf mbox\n");
+ goto err_dpi_fini;
+ }
+
+ /* Register interrupts */
+ ret = dpi_irq_init(dpi);
+ if (ret) {
+ dev_err(dev, "DPI: Failed to initialize irq vectors\n");
+ goto err_dpi_mbox_free;
+ }
+
+ pci_set_drvdata(pdev, dpi);
+ dpi->miscdev.minor = MISC_DYNAMIC_MINOR;
+ dpi->miscdev.name = DPI_DRV_NAME;
+ dpi->miscdev.fops = &dpi_device_fops;
+ dpi->miscdev.parent = dev;
+
+ ret = misc_register(&dpi->miscdev);
+ if (ret) {
+ dev_err(dev, "DPI: Failed to register misc device\n");
+ goto err_dpi_mbox_free;
+ }
+
+ return 0;
+
+err_dpi_mbox_free:
+ dpi_pfvf_mbox_destroy(dpi);
+err_dpi_fini:
+ dpi_fini(dpi);
+ return ret;
+}
+
+static void dpi_remove(struct pci_dev *pdev)
+{
+ struct dpipf *dpi = pci_get_drvdata(pdev);
+
+ misc_deregister(&dpi->miscdev);
+ pci_sriov_configure_simple(pdev, 0);
+ dpi_pfvf_mbox_destroy(dpi);
+ dpi_fini(dpi);
+ pci_set_drvdata(pdev, NULL);
+}
+
+static const struct pci_device_id dpi_id_table[] = {
+ { PCI_DEVICE_SUB(PCI_VENDOR_ID_CAVIUM, PCI_DEVID_MRVL_CN10K_DPI_PF,
+ PCI_VENDOR_ID_CAVIUM, PCI_SUBDEVID_MRVL_CN10K_DPI_PF) },
+ { 0, } /* end of table */
+};
+
+static struct pci_driver dpi_driver = {
+ .name = DPI_DRV_NAME,
+ .id_table = dpi_id_table,
+ .probe = dpi_probe,
+ .remove = dpi_remove,
+ .sriov_configure = pci_sriov_configure_simple,
+};
+
+module_pci_driver(dpi_driver);
+MODULE_DEVICE_TABLE(pci, dpi_id_table);
+MODULE_AUTHOR("Marvell.");
+MODULE_DESCRIPTION("Marvell Octeon CN10K DPI Driver");
+MODULE_LICENSE("GPL");
diff --git a/include/uapi/misc/mrvl_cn10k_dpi.h b/include/uapi/misc/mrvl_cn10k_dpi.h
new file mode 100644
index 000000000000..a1951644448a
--- /dev/null
+++ b/include/uapi/misc/mrvl_cn10k_dpi.h
@@ -0,0 +1,37 @@
+/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
+/*
+ * Marvell Octeon CN10K DPI driver
+ *
+ * Copyright (C) 2024 Marvell.
+ *
+ */
+
+#ifndef __MRVL_CN10K_DPI_H__
+#define __MRVL_CN10K_DPI_H__
+
+#include <linux/types.h>
+
+#define DPI_MAX_ENGINES 6
+
+struct dpi_mps_mrrs_cfg {
+ __u16 max_read_req_sz; /* Max read request size */
+ __u16 max_payload_sz; /* Max payload size */
+ __u8 port; /* Ebus port */
+};
+
+struct dpi_engine_cfg {
+ __u64 fifo_mask; /* FIFO size mask in KBytes */
+ __u16 molr[DPI_MAX_ENGINES]; /* Max outstanding load requests */
+ __u8 update_molr; /* '1' to update engine MOLR */
+};
+
+/* DPI ioctl numbers */
+#define DPI_MAGIC_NUM 0xB8
+
+/* Set MPS & MRRS parameters */
+#define DPI_MPS_MRRS_CFG _IOW(DPI_MAGIC_NUM, 1, struct dpi_mps_mrrs_cfg)
+
+/* Set Engine FIFO configuration */
+#define DPI_ENGINE_CFG _IOW(DPI_MAGIC_NUM, 2, struct dpi_engine_cfg)
+
+#endif /* __MRVL_CN10K_DPI_H__ */
--
2.25.1

On Sun, Apr 28, 2024 at 10:50:27PM -0700, Vamsi Attunuru wrote:
> Adds a misc driver for Marvell CN10K DPI(DMA Engine) device's physical
> function which initializes DPI DMA hardware's global configuration and
> enables hardware mailbox channels between physical function (PF) and
> it's virtual functions (VF). VF device drivers (User space drivers) use
> this hw mailbox to communicate any required device configuration on it's
> respective VF device. Accordingly, this DPI PF driver provisions the
> VF device resources.
>
> At the hardware level, the DPI physical function (PF) acts as a management
> interface to setup the VF device resources, VF devices are only provisioned
> to handle or control the actual DMA Engine's data transfer capabilities.
>
> Signed-off-by: Vamsi Attunuru <vattunuru@marvell.com>

No one else at Marvell has reviewed this before you submitted this?

> ---
> Changes V5 -> V6:
> - Updated documentation
> - Fixed data types in uapi file
>
> Changes V4 -> V5:
> - Fixed license and data types in uapi file
>
> Changes V3 -> V4:
> - Moved ioctl definations to .h file
> - Fixed structure alignements which are passed in ioctl
>
> Changes V2 -> V3:
> - Added ioctl operation to the fops
> - Used managed version of kzalloc & request_irq
> - Addressed miscellaneous comments
>
> Changes V1 -> V2:
> - Fixed return values and busy-wait loops
> - Merged .h file into .c file
> - Fixed directory structure
> - Removed module params
> - Registered the device as misc device
>
> Documentation/misc-devices/index.rst | 1 +
> Documentation/misc-devices/mrvl_cn10k_dpi.rst | 57 ++
> .../userspace-api/ioctl/ioctl-number.rst | 1 +
> MAINTAINERS | 5 +
> drivers/misc/Kconfig | 13 +
> drivers/misc/Makefile | 2 +
> drivers/misc/mrvl_cn10k_dpi.c | 685 ++++++++++++++++++
> include/uapi/misc/mrvl_cn10k_dpi.h | 37 +
> 8 files changed, 801 insertions(+)
>
> diff --git a/Documentation/misc-devices/index.rst b/Documentation/misc-devices/index.rst
> index 2d0ce9138588..10f2e0f74e45 100644
> --- a/Documentation/misc-devices/index.rst
> +++ b/Documentation/misc-devices/index.rst
> @@ -20,6 +20,7 @@ fit into other categories.
> ics932s401
> isl29003
> lis3lv02d
> + mrvl_cn10k_dpi
> max6875
> oxsemi-tornado
> pci-endpoint-test

Why not in sorted order?

> diff --git a/Documentation/misc-devices/mrvl_cn10k_dpi.rst b/Documentation/misc-devices/mrvl_cn10k_dpi.rst
> new file mode 100644
> index 000000000000..cce202f114b7
> --- /dev/null
> +++ b/Documentation/misc-devices/mrvl_cn10k_dpi.rst
> @@ -0,0 +1,57 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +===============================================
> +Marvell CN10K DMA packet interface (DPI) driver
> +===============================================
> +
> +Overview
> +========
> +
> +DPI is a DMA packet interface hardware block in Marvell's CN10K silicon.
> +DPI hardware comprises a physical function (PF), its virtual functions,
> +mailbox logic, and a set of DMA engines & DMA command queues.
> +
> +DPI PF function is an administrative function which services the mailbox
> +requests from its VF functions and provisions DMA engine resources to
> +it's VF functions.
> +
> +mrvl_cn10k_dpi.ko misc driver loads on DPI PF device and services the
> +mailbox commands submitted by the VF devices and accordingly initializes
> +the DMA engines and VF device's DMA command queues. Also, driver creates
> +/dev/mrvl-cn10k-dpi node to set DMA engine and PEM (PCIe interface) port
> +attributes like fifo length, molr, mps & mrrs.
> +
> +DPI PF driver is just an administrative driver to setup its VF device's
> +queues and provisions the hardware resources, it can not initiate any
> +DMA operations. Only VF devices are provisioned with DMA capabilities.
> +
> +Driver location
> +===============
> +
> +drivers/misc/mrvl_cn10k_dpi.c
> +
> +Driver IOCTLs
> +=============
> +
> +:c:macro::`DPI_MPS_MRRS_CFG`
> +ioctl that sets max payload size & max read request size parameters of
> +a pem port to which DMA engines are wired.
> +
> +
> +:c:macro::`DPI_ENGINE_CFG`
> +ioctl that sets DMA engine's fifo sizes & max outstanding load request
> +thresholds.
> +
> +Userspace code example
> +----------------------
> +
> +DPI VF devices are managed by user space drivers, below is a reference
> +code to the user space driver's mailbox command exchange with DPI PF
> +driver through hardware mailbox.
> +
> +https://github.com/VamsiKrishnaA99/dpi-dma/blob/main/driver/roc_dpi.c
> +
> +Below is a sample application that uses driver IOCTLs to setup DMA engine
> +and PEM port attributes over `/dev/mrvl-cn10k-dpi` node.
> +
> +https://github.com/VamsiKrishnaA99/dpi-dma/blob/main/application/main.c

I appreciate the code being open, but this is on a personal account that
was created for only this one package, and it doesn't even have a
README. Yet it is fully owned/copyrighted by Marvell and the code has
been around since 2021? Why isn't this on a Marvell-controlled page to
show that this really is supported and is the proper interface to use
this driver that it provides to their customers?



> diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst b/Documentation/userspace-api/ioctl/ioctl-number.rst
> index 457e16f06e04..e6fd0c386b59 100644
> --- a/Documentation/userspace-api/ioctl/ioctl-number.rst
> +++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
> @@ -358,6 +358,7 @@ Code Seq# Include File Comments
> 0xB6 all linux/fpga-dfl.h
> 0xB7 all uapi/linux/remoteproc_cdev.h <mailto:linux-remoteproc@vger.kernel.org>
> 0xB7 all uapi/linux/nsfs.h <mailto:Andrei Vagin <avagin@openvz.org>>
> +0xB8 01-02 uapi/misc/mrvl_cn10k_dpi.h Marvell CN10K DPI driver
> 0xC0 00-0F linux/usb/iowarrior.h
> 0xCA 00-0F uapi/misc/cxl.h
> 0xCA 10-2F uapi/misc/ocxl.h
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 960512bec428..ab77232d583e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13104,6 +13104,11 @@ S: Supported
> F: Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml
> F: drivers/mmc/host/sdhci-xenon*
>
> +MARVELL OCTEON CN10K DPI DRIVER
> +M: Vamsi Attunuru <vattunuru@marvell.com>
> +S: Maintained

So this is not part of your job to support this code? Why isn't Marvell
allowing that to happen?

> +F: drivers/misc/mrvl_cn10k_dpi.c
> +
> MATROX FRAMEBUFFER DRIVER
> L: linux-fbdev@vger.kernel.org
> S: Orphan
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 4fb291f0bf7c..78470ef2538f 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -574,6 +574,19 @@ config NSM
> To compile this driver as a module, choose M here.
> The module will be called nsm.
>
> +config MARVELL_CN10K_DPI
> + tristate "Octeon CN10K DPI driver"
> + depends on ARM64 && PCI
> + help
> + Enables Octeon CN10K DMA packet interface (DPI) driver which intializes
> + DPI hardware's physical function (PF) device's global configuration and
> + its virtual function's (VFs) resource configuration to enable DMA transfers.
> + DPI PF device does not have any data movement functionality, it only serves
> + VF's resource configuration requests.

Nit, please wrap at 72 columns, didn't checkpatch.pl complain about
this?

> +
> + To compile this driver as a module, choose M here: the module
> + will be called mrvl_cn10k_dpi.
> +
> source "drivers/misc/c2port/Kconfig"
> source "drivers/misc/eeprom/Kconfig"
> source "drivers/misc/cb710/Kconfig"
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index ea6ea5bbbc9c..5106bf96ea5c 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -68,3 +68,5 @@ obj-$(CONFIG_TMR_INJECT) += xilinx_tmr_inject.o
> obj-$(CONFIG_TPS6594_ESM) += tps6594-esm.o
> obj-$(CONFIG_TPS6594_PFSM) += tps6594-pfsm.o
> obj-$(CONFIG_NSM) += nsm.o
> +obj-$(CONFIG_MARVELL_CN10K_DPI) += mrvl_cn10k_dpi.o
> +obj-y += mrvl_cn10k_dpi.o

That is odd, why are you saying obj-y here? You want everyone to always
build this code into the kernel image no matter what

> diff --git a/drivers/misc/mrvl_cn10k_dpi.c b/drivers/misc/mrvl_cn10k_dpi.c
> new file mode 100644
> index 000000000000..bd99583994f9
> --- /dev/null
> +++ b/drivers/misc/mrvl_cn10k_dpi.c
> @@ -0,0 +1,685 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Marvell Octeon CN10K DPI driver
> + *
> + * Copyright (C) 2024 Marvell.
> + *
> + */
> +
> +#include <linux/compat.h>
> +#include <linux/delay.h>
> +#include <linux/miscdevice.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/irq.h>
> +#include <linux/interrupt.h>
> +
> +#include <uapi/misc/mrvl_cn10k_dpi.h>
> +
> +#define DPI_DRV_NAME "mrvl-cn10k-dpi"

KBUILD_MODNAME please.

> +
> +/* PCI device IDs */
> +#define PCI_DEVID_MRVL_CN10K_DPI_PF 0xA080
> +#define PCI_SUBDEVID_MRVL_CN10K_DPI_PF 0xB900
> +
> +/* PCI BAR nos */

What is "nos"?

> +#define PCI_DPI_CFG_BAR 0
> +
> +/* MSI-X interrupts */
> +#define DPI_MAX_REQQ_INT 32
> +#define DPI_MAX_CC_INT 64
> +
> +/* MBOX MSI-X interrupt vector index */
> +#define DPI_MBOX_PF_VF_INT_IDX 0x75
> +
> +#define DPI_MAX_IRQS (DPI_MBOX_PF_VF_INT_IDX + 1)
> +
> +#define DPI_MAX_VFS 32
> +
> +#define DPI_ENGINE_MASK GENMASK(2, 0)
> +
> +#define DPI_DMA_IDS_DMA_NPA_PF_FUNC(x) (((x) & GENMASK_ULL(15, 0)) << 16)
> +#define DPI_DMA_IDS_INST_STRM(x) (((x) & GENMASK_ULL(7, 0)) << 40)
> +#define DPI_DMA_IDS_DMA_STRM(x) (((x) & GENMASK_ULL(7, 0)) << 32)
> +#define DPI_DMA_ENG_EN_MOLR(x) (((x) & GENMASK_ULL(9, 0)) << 32)
> +#define DPI_EBUS_PORTX_CFG_MPS(x) (((x) & GENMASK(2, 0)) << 4)
> +#define DPI_DMA_IDS_DMA_SSO_PF_FUNC(x) ((x) & GENMASK(15, 0))
> +#define DPI_DMA_IDS2_INST_AURA(x) ((x) & GENMASK(19, 0))
> +#define DPI_DMA_IBUFF_CSIZE_CSIZE(x) ((x) & GENMASK(13, 0))
> +#define DPI_EBUS_PORTX_CFG_MRRS(x) ((x) & GENMASK(2, 0))
> +#define DPI_ENG_BUF_BLKS(x) ((x) & GENMASK(5, 0))
> +#define DPI_DMA_CONTROL_DMA_ENB GENMASK_ULL(53, 48)
> +
> +#define DPI_DMA_CONTROL_O_MODE BIT_ULL(14)
> +#define DPI_DMA_CONTROL_LDWB BIT_ULL(32)
> +#define DPI_DMA_CONTROL_WQECSMODE1 BIT_ULL(37)
> +#define DPI_DMA_CONTROL_ZBWCSEN BIT_ULL(39)
> +#define DPI_DMA_CONTROL_WQECSOFF(offset) (((u64)offset) << 40)
> +#define DPI_DMA_CONTROL_WQECSDIS BIT_ULL(47)
> +#define DPI_DMA_CONTROL_PKT_EN BIT_ULL(56)
> +#define DPI_DMA_IBUFF_CSIZE_NPA_FREE BIT(16)
> +
> +#define DPI_CTL_EN BIT_ULL(0)
> +#define DPI_DMA_CC_INT BIT_ULL(0)
> +#define DPI_DMA_QRST BIT_ULL(0)
> +
> +#define DPI_REQQ_INT_INSTRFLT BIT_ULL(0)
> +#define DPI_REQQ_INT_RDFLT BIT_ULL(1)
> +#define DPI_REQQ_INT_WRFLT BIT_ULL(2)
> +#define DPI_REQQ_INT_CSFLT BIT_ULL(3)
> +#define DPI_REQQ_INT_INST_DBO BIT_ULL(4)
> +#define DPI_REQQ_INT_INST_ADDR_NULL BIT_ULL(5)
> +#define DPI_REQQ_INT_INST_FILL_INVAL BIT_ULL(6)
> +#define DPI_REQQ_INT_INSTR_PSN BIT_ULL(7)
> +
> +#define DPI_REQQ_INT \
> + (DPI_REQQ_INT_INSTRFLT | \
> + DPI_REQQ_INT_RDFLT | \
> + DPI_REQQ_INT_WRFLT | \
> + DPI_REQQ_INT_CSFLT | \
> + DPI_REQQ_INT_INST_DBO | \
> + DPI_REQQ_INT_INST_ADDR_NULL | \
> + DPI_REQQ_INT_INST_FILL_INVAL | \
> + DPI_REQQ_INT_INSTR_PSN)
> +
> +#define DPI_PF_RAS_EBI_DAT_PSN BIT_ULL(0)
> +#define DPI_PF_RAS_NCB_DAT_PSN BIT_ULL(1)
> +#define DPI_PF_RAS_NCB_CMD_PSN BIT_ULL(2)
> +
> +#define DPI_PF_RAS_INT \
> + (DPI_PF_RAS_EBI_DAT_PSN | \
> + DPI_PF_RAS_NCB_DAT_PSN | \
> + DPI_PF_RAS_NCB_CMD_PSN)
> +
> +#define DPI_DMAX_IBUFF_CSIZE(x) (0x0ULL | ((x) << 11))
> +#define DPI_DMAX_IDS(x) (0x18ULL | ((x) << 11))
> +#define DPI_DMAX_IDS2(x) (0x20ULL | ((x) << 11))
> +#define DPI_DMAX_QRST(x) (0x30ULL | ((x) << 11))
> +
> +#define DPI_CTL 0x10010ULL
> +#define DPI_DMA_CONTROL 0x10018ULL
> +#define DPI_DMA_ENGX_EN(x) (0x10040ULL | ((x) << 3))
> +#define DPI_ENGX_BUF(x) (0x100C0ULL | ((x) << 3))
> +
> +#define DPI_EBUS_PORTX_CFG(x) (0x10100ULL | ((x) << 3))
> +
> +#define DPI_PF_RAS 0x10308ULL
> +#define DPI_PF_RAS_ENA_W1C 0x10318ULL
> +
> +#define DPI_DMA_CCX_INT(x) (0x11000ULL | ((x) << 3))
> +#define DPI_DMA_CCX_INT_ENA_W1C(x) (0x11800ULL | ((x) << 3))
> +
> +#define DPI_REQQX_INT(x) (0x12C00ULL | ((x) << 5))
> +#define DPI_REQQX_INT_ENA_W1C(x) (0x13800ULL | ((x) << 5))
> +
> +#define DPI_MBOX_PF_VF_DATA0(x) (0x16000ULL | ((x) << 4))
> +#define DPI_MBOX_PF_VF_DATA1(x) (0x16008ULL | ((x) << 4))
> +
> +#define DPI_MBOX_VF_PF_INT 0x16300ULL
> +#define DPI_MBOX_VF_PF_INT_W1S 0x16308ULL
> +#define DPI_MBOX_VF_PF_INT_ENA_W1C 0x16310ULL
> +#define DPI_MBOX_VF_PF_INT_ENA_W1S 0x16318ULL
> +
> +#define DPI_WCTL_FIF_THR 0x17008ULL
> +
> +#define DPI_EBUS_MAX_PORTS 2
> +
> +#define DPI_EBUS_MRRS_MIN 128
> +#define DPI_EBUS_MRRS_MAX 1024
> +#define DPI_EBUS_MPS_MIN 128
> +#define DPI_EBUS_MPS_MAX 1024
> +#define DPI_WCTL_FIFO_THRESHOLD 0x30
> +
> +#define DPI_QUEUE_OPEN 0x1
> +#define DPI_QUEUE_CLOSE 0x2
> +#define DPI_REG_DUMP 0x3
> +#define DPI_GET_REG_CFG 0x4
> +#define DPI_QUEUE_OPEN_V2 0x5
> +
> +enum dpi_mbox_rsp_type {
> + DPI_MBOX_TYPE_CMD,
> + DPI_MBOX_TYPE_RSP_ACK,
> + DPI_MBOX_TYPE_RSP_NACK,
> +};
> +
> +struct dpivf_config {
> + u16 csize;
> + u32 aura;
> + u16 sso_pf_func;
> + u16 npa_pf_func;

Do you intend to have unaligned accesses to the fields in this
structure?

> +};
> +
> +struct dpipf_vf {
> + u8 this_vfid;
> + bool setup_done;
> + struct dpivf_config vf_config;
> +};
> +
> +/* DPI device mailbox */
> +struct dpi_mbox {
> + struct work_struct work;
> + /* lock to serialize mbox requests */
> + struct mutex lock;
> + struct dpipf *pf;
> + u8 __iomem *pf_vf_data_reg;
> + u8 __iomem *vf_pf_data_reg;
> +};
> +
> +struct dpipf {
> + struct miscdevice miscdev;
> + void __iomem *reg_base;
> + struct pci_dev *pdev;
> + struct dpipf_vf vf[DPI_MAX_VFS];
> + /* Mailbox to talk to VFs */
> + struct dpi_mbox *mbox[DPI_MAX_VFS];
> +};
> +
> +union dpi_mbox_message_t {

Didn't checkpatch complain about the "_t" here?

> + u64 u[2];

What is "u"?

> + struct dpi_mbox_message_s {
> + /* VF ID to configure */
> + u64 vfid :8;
> + /* Command code */
> + u64 cmd :4;
> + /* Command buffer size in 8-byte words */
> + u64 csize :14;
> + /* Aura of the command buffer */
> + u64 aura :20;
> + /* SSO PF function */
> + u64 sso_pf_func :16;
> + /* NPA PF function */
> + u64 npa_pf_func :16;
> + /* Work queue completion status enable */
> + u64 wqecs :1;
> + /* Work queue completion status byte offset */
> + u64 wqecsoff :7;
> + /* Reserved */
> + u64 rsvd :42;

reserved for what?

If you are reading this from hardware, the bit fields you created here
will NOT work or be portable at all. Please do this properly.

Also you have a mix of tabs and spaces for some reason in this
structure, again, checkpatch should have caught that.

> + } s __packed;

"s"?

What is "u" and "s" here?


> +};
> +
> +static inline void dpi_reg_write(struct dpipf *dpi, u64 offset, u64 val)
> +{
> + writeq(val, dpi->reg_base + offset);

No need to read to ensure that the write succeeded? Or are you doing
that in individual places where you want to make sure it happened? If
so, I didn't see that in the use of this.

> +}
> +
> +static inline u64 dpi_reg_read(struct dpipf *dpi, u64 offset)
> +{
> + return readq(dpi->reg_base + offset);
> +}
> +
> +static void dpi_wqe_cs_offset(struct dpipf *dpi, u8 offset)
> +{
> + u64 reg;
> +
> + reg = dpi_reg_read(dpi, DPI_DMA_CONTROL);
> + reg &= ~DPI_DMA_CONTROL_WQECSDIS;
> + reg |= DPI_DMA_CONTROL_ZBWCSEN | DPI_DMA_CONTROL_WQECSMODE1;
> + reg |= DPI_DMA_CONTROL_WQECSOFF(offset);
> + dpi_reg_write(dpi, DPI_DMA_CONTROL, reg);
> +}
> +
> +static int dpi_queue_init(struct dpipf *dpi, struct dpipf_vf *dpivf, u8 vf)
> +{
> + u16 sso_pf_func = dpivf->vf_config.sso_pf_func;
> + u16 npa_pf_func = dpivf->vf_config.npa_pf_func;
> + u16 csize = dpivf->vf_config.csize;
> + u32 aura = dpivf->vf_config.aura;
> + unsigned long timeout;
> + u64 reg;
> +
> + dpi_reg_write(dpi, DPI_DMAX_QRST(vf), DPI_DMA_QRST);
> +
> + /* Wait for a maximum of 3 sec */
> + timeout = jiffies + msecs_to_jiffies(3000);
> + while (!time_after(jiffies, timeout)) {
> + reg = dpi_reg_read(dpi, DPI_DMAX_QRST(vf));
> + if (!(reg & DPI_DMA_QRST))
> + break;
> +
> + usleep_range(500, 1000);

Why sleep this value? Please document.

> + }
> +
> + if (reg & DPI_DMA_QRST) {
> + dev_err(&dpi->pdev->dev, "Queue reset failed\n");
> + return -EBUSY;
> + }
> +
> + dpi_reg_write(dpi, DPI_DMAX_IDS2(vf), 0);
> + dpi_reg_write(dpi, DPI_DMAX_IDS(vf), 0);
> +
> + reg = DPI_DMA_IBUFF_CSIZE_CSIZE(csize) | DPI_DMA_IBUFF_CSIZE_NPA_FREE;
> + dpi_reg_write(dpi, DPI_DMAX_IBUFF_CSIZE(vf), reg);
> +
> + reg = dpi_reg_read(dpi, DPI_DMAX_IDS2(vf));
> + reg |= DPI_DMA_IDS2_INST_AURA(aura);
> + dpi_reg_write(dpi, DPI_DMAX_IDS2(vf), reg);
> +
> + reg = dpi_reg_read(dpi, DPI_DMAX_IDS(vf));
> + reg |= DPI_DMA_IDS_DMA_NPA_PF_FUNC(npa_pf_func);
> + reg |= DPI_DMA_IDS_DMA_SSO_PF_FUNC(sso_pf_func);
> + reg |= DPI_DMA_IDS_DMA_STRM(vf + 1);
> + reg |= DPI_DMA_IDS_INST_STRM(vf + 1);
> + dpi_reg_write(dpi, DPI_DMAX_IDS(vf), reg);
> +
> + return 0;
> +}
> +
> +static void dpi_queue_fini(struct dpipf *dpi, struct dpipf_vf *dpivf, u8 vf)
> +{
> + dpi_reg_write(dpi, DPI_DMAX_QRST(vf), DPI_DMA_QRST);
> +
> + /* Reset IDS and IDS2 registers */
> + dpi_reg_write(dpi, DPI_DMAX_IDS2(vf), 0);
> + dpi_reg_write(dpi, DPI_DMAX_IDS(vf), 0);
> +}
> +
> +static void dpi_poll_pfvf_mbox(struct dpipf *dpi)
> +{
> + u64 reg;
> + u32 vf;
> +
> + reg = dpi_reg_read(dpi, DPI_MBOX_VF_PF_INT);
> + if (reg) {
> + for (vf = 0; vf < pci_num_vf(dpi->pdev); vf++) {
> + if (reg & BIT_ULL(vf))
> + schedule_work(&dpi->mbox[vf]->work);
> + }
> + dpi_reg_write(dpi, DPI_MBOX_VF_PF_INT, reg);
> + }

No error if reg was not read properly?

> +}
> +
> +static irqreturn_t dpi_mbox_intr_handler(int irq, void *data)
> +{
> + struct dpipf *dpi = data;
> +
> + dpi_poll_pfvf_mbox(dpi);
> +
> + return IRQ_HANDLED;

So this can always succeed?

> +}
> +
> +static int queue_config(struct dpipf *dpi, struct dpipf_vf *dpivf, union dpi_mbox_message_t *msg)
> +{
> + int ret = 0;
> +
> + switch (msg->s.cmd) {
> + case DPI_QUEUE_OPEN:
> + case DPI_QUEUE_OPEN_V2:
> + dpivf->vf_config.aura = msg->s.aura;
> + dpivf->vf_config.csize = msg->s.cmd == DPI_QUEUE_OPEN ? msg->s.csize / 8 :
> + msg->s.csize;
> + dpivf->vf_config.sso_pf_func = msg->s.sso_pf_func;
> + dpivf->vf_config.npa_pf_func = msg->s.npa_pf_func;
> + ret = dpi_queue_init(dpi, dpivf, msg->s.vfid);
> + if (!ret) {
> + if (msg->s.wqecs)
> + dpi_wqe_cs_offset(dpi, msg->s.wqecsoff);
> + dpivf->setup_done = true;
> + }
> + break;
> + case DPI_QUEUE_CLOSE:
> + dpivf->vf_config.aura = 0;
> + dpivf->vf_config.csize = 0;
> + dpivf->vf_config.sso_pf_func = 0;
> + dpivf->vf_config.npa_pf_func = 0;
> + dpi_queue_fini(dpi, dpivf, msg->s.vfid);
> + dpivf->setup_done = false;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return ret;
> +}
> +
> +static void dpi_pfvf_mbox_work(struct work_struct *work)
> +{
> + struct dpi_mbox *mbox = container_of(work, struct dpi_mbox, work);
> + union dpi_mbox_message_t msg = { 0 };

Are you sure this can be on the stack?

> + struct dpipf_vf *dpivf;
> + struct dpipf *dpi;
> + int ret;
> +
> + dpi = mbox->pf;
> +
> + mutex_lock(&mbox->lock);
> + msg.u[0] = readq(mbox->vf_pf_data_reg);
> + if (unlikely(msg.u[0] == (u64)-1))
> + goto exit;

Only use likely/unlikely if you can prove with a benchmark that it makes
a difference. Otherwise let the CPU and compiler choose for you, as it
knows better than you do (and will know better over time.)

If you want to use unlikely, you have to document it as to how it
matters.

> +
> + if (unlikely(msg.s.vfid >= pci_num_vf(dpi->pdev))) {
> + dev_err(&dpi->pdev->dev, "Invalid vfid:%d\n", msg.s.vfid);

You send this to the kernel log if a device is broken?

> + goto exit;
> + }
> +
> + dpivf = &dpi->vf[msg.s.vfid];
> + msg.u[1] = readq(mbox->pf_vf_data_reg);
> +
> + ret = queue_config(dpi, dpivf, &msg);
> + if (ret < 0)
> + writeq(DPI_MBOX_TYPE_RSP_NACK, mbox->pf_vf_data_reg);
> + else
> + writeq(DPI_MBOX_TYPE_RSP_ACK, mbox->pf_vf_data_reg);
> +exit:
> + mutex_unlock(&mbox->lock);
> +}
> +
> +/* Setup registers for a PF mailbox */
> +static void dpi_setup_mbox_regs(struct dpipf *dpi, int vf)
> +{
> + struct dpi_mbox *mbox = dpi->mbox[vf];
> +
> + mbox->pf_vf_data_reg = dpi->reg_base + DPI_MBOX_PF_VF_DATA0(vf);
> + mbox->vf_pf_data_reg = dpi->reg_base + DPI_MBOX_PF_VF_DATA1(vf);
> +}
> +
> +static int dpi_pfvf_mbox_setup(struct dpipf *dpi)
> +{
> + int vf;
> +
> + for (vf = 0; vf < DPI_MAX_VFS; vf++) {
> + dpi->mbox[vf] = devm_kzalloc(&dpi->pdev->dev, sizeof(*dpi->mbox[vf]), GFP_KERNEL);
> +
> + if (!dpi->mbox[vf])
> + return -ENOMEM;
> +
> + mutex_init(&dpi->mbox[vf]->lock);
> + INIT_WORK(&dpi->mbox[vf]->work, dpi_pfvf_mbox_work);
> + dpi->mbox[vf]->pf = dpi;
> + dpi_setup_mbox_regs(dpi, vf);
> + }
> +
> + return 0;
> +}
> +
> +static void dpi_pfvf_mbox_destroy(struct dpipf *dpi)
> +{
> + unsigned int vf;
> +
> + for (vf = 0; vf < DPI_MAX_VFS; vf++) {
> + if (work_pending(&dpi->mbox[vf]->work))
> + cancel_work_sync(&dpi->mbox[vf]->work);
> +
> + dpi->mbox[vf] = NULL;
> + }
> +}
> +
> +static void dpi_init(struct dpipf *dpi)
> +{
> + unsigned int engine, port;
> + u8 mrrs_val, mps_val;
> + u64 reg;
> +
> + for (engine = 0; engine < DPI_MAX_ENGINES; engine++) {
> + if (engine == 4 || engine == 5)
> + reg = DPI_ENG_BUF_BLKS(16);
> + else
> + reg = DPI_ENG_BUF_BLKS(8);
> +
> + dpi_reg_write(dpi, DPI_ENGX_BUF(engine), reg);
> + }
> +
> + reg = DPI_DMA_CONTROL_ZBWCSEN | DPI_DMA_CONTROL_PKT_EN | DPI_DMA_CONTROL_LDWB |
> + DPI_DMA_CONTROL_O_MODE | DPI_DMA_CONTROL_DMA_ENB;
> +
> + dpi_reg_write(dpi, DPI_DMA_CONTROL, reg);
> + dpi_reg_write(dpi, DPI_CTL, DPI_CTL_EN);
> +
> + mrrs_val = 2; /* 512B */
> + mps_val = 1; /* 256B */
> +
> + for (port = 0; port < DPI_EBUS_MAX_PORTS; port++) {
> + reg = dpi_reg_read(dpi, DPI_EBUS_PORTX_CFG(port));
> + reg &= ~(DPI_EBUS_PORTX_CFG_MRRS(7) | DPI_EBUS_PORTX_CFG_MPS(7));
> + reg |= DPI_EBUS_PORTX_CFG_MPS(mps_val) | DPI_EBUS_PORTX_CFG_MRRS(mrrs_val);
> + dpi_reg_write(dpi, DPI_EBUS_PORTX_CFG(port), reg);
> + }
> +
> + dpi_reg_write(dpi, DPI_WCTL_FIF_THR, DPI_WCTL_FIFO_THRESHOLD);
> +}
> +
> +static void dpi_fini(struct dpipf *dpi)
> +{
> + unsigned int engine;
> +
> + for (engine = 0; engine < DPI_MAX_ENGINES; engine++)
> + dpi_reg_write(dpi, DPI_ENGX_BUF(engine), 0);
> +
> + dpi_reg_write(dpi, DPI_DMA_CONTROL, 0);
> + dpi_reg_write(dpi, DPI_CTL, 0);
> +}
> +
> +static void dpi_free_irq_vectors(void *pdev)
> +{
> + pci_free_irq_vectors((struct pci_dev *)pdev);
> +}
> +
> +static int dpi_irq_init(struct dpipf *dpi)
> +{
> + struct pci_dev *pdev = dpi->pdev;
> + struct device *dev = &pdev->dev;
> + int i, ret;
> +
> + /* Clear all RAS interrupts */
> + dpi_reg_write(dpi, DPI_PF_RAS, DPI_PF_RAS_INT);
> +
> + /* Clear all RAS interrupt enable bits */
> + dpi_reg_write(dpi, DPI_PF_RAS_ENA_W1C, DPI_PF_RAS_INT);
> +
> + for (i = 0; i < DPI_MAX_REQQ_INT; i++) {
> + dpi_reg_write(dpi, DPI_REQQX_INT(i), DPI_REQQ_INT);
> + dpi_reg_write(dpi, DPI_REQQX_INT_ENA_W1C(i), DPI_REQQ_INT);
> + }
> +
> + for (i = 0; i < DPI_MAX_CC_INT; i++) {
> + dpi_reg_write(dpi, DPI_DMA_CCX_INT(i), DPI_DMA_CC_INT);
> + dpi_reg_write(dpi, DPI_DMA_CCX_INT_ENA_W1C(i), DPI_DMA_CC_INT);
> + }
> +
> + ret = pci_alloc_irq_vectors(pdev, DPI_MAX_IRQS, DPI_MAX_IRQS, PCI_IRQ_MSIX);
> + if (ret != DPI_MAX_IRQS) {
> + dev_err(dev, "DPI: Failed to alloc %d msix irqs\n", DPI_MAX_IRQS);
> + return ret;
> + }
> +
> + ret = devm_add_action_or_reset(dev, dpi_free_irq_vectors, pdev);
> + if (ret) {
> + dev_err(dev, "DPI: Failed to add irq free action\n");
> + return ret;
> + }
> +
> + ret = devm_request_irq(dev, pci_irq_vector(pdev, DPI_MBOX_PF_VF_INT_IDX),
> + dpi_mbox_intr_handler, 0, "dpi-mbox", dpi);
> + if (ret) {
> + dev_err(dev, "DPI: request_irq failed for mbox; err=%d\n", ret);
> + return ret;
> + }
> +
> + dpi_reg_write(dpi, DPI_MBOX_VF_PF_INT_ENA_W1S, GENMASK_ULL(31, 0));
> +
> + return 0;
> +}
> +
> +static int dpi_mps_mrrs_config(struct dpipf *dpi, void __user *arg)
> +{
> + struct dpi_mps_mrrs_cfg cfg;
> + u8 mrrs_val, mps_val;
> + u64 reg;
> +
> + if (copy_from_user(&cfg, arg, sizeof(struct dpi_mps_mrrs_cfg)))
> + return -EFAULT;
> +
> + if (cfg.max_read_req_sz < DPI_EBUS_MRRS_MIN || cfg.max_read_req_sz > DPI_EBUS_MRRS_MAX ||
> + !is_power_of_2(cfg.max_read_req_sz)) {
> + dev_err(&dpi->pdev->dev, "Invalid MRRS size:%u\n", cfg.max_read_req_sz);

You are allowing userspace to spam the kernel log with messages by
sending the driver invalid data. THat is a denial of service, please
never do that.

> + return -EINVAL;
> + }
> +
> + if (cfg.max_payload_sz < DPI_EBUS_MPS_MIN || cfg.max_payload_sz > DPI_EBUS_MPS_MAX ||
> + !is_power_of_2(cfg.max_payload_sz)) {
> + dev_err(&dpi->pdev->dev, "Invalid MPS size:%u\n", cfg.max_payload_sz);
> + return -EINVAL;
> + }
> +
> + if (cfg.port >= DPI_EBUS_MAX_PORTS) {
> + dev_err(&dpi->pdev->dev, "Invalid EBUS port:%u\n", cfg.port);
> + return -EINVAL;
> + }
> +
> + mrrs_val = fls(cfg.max_read_req_sz >> 8);
> + mps_val = fls(cfg.max_payload_sz >> 8);
> +
> + reg = dpi_reg_read(dpi, DPI_EBUS_PORTX_CFG(cfg.port));
> + reg &= ~(DPI_EBUS_PORTX_CFG_MRRS(0x7) | DPI_EBUS_PORTX_CFG_MPS(0x7));
> + reg |= DPI_EBUS_PORTX_CFG_MPS(mps_val) | DPI_EBUS_PORTX_CFG_MRRS(mrrs_val);
> + dpi_reg_write(dpi, DPI_EBUS_PORTX_CFG(cfg.port), reg);
> +
> + return 0;
> +}
> +
> +static int dpi_engine_config(struct dpipf *dpi, void __user *arg)
> +{
> + struct dpi_engine_cfg cfg;
> + unsigned int engine;
> + u8 *eng_buf;
> + u64 reg;
> +
> + if (copy_from_user(&cfg, arg, sizeof(struct dpi_engine_cfg)))
> + return -EFAULT;

No need to ever validate any information in that structure from
userspace? You always blindly copy it on to the device? Userspace can
never get this wrong?

> +
> + eng_buf = (u8 *)&cfg.fifo_mask;
> +
> + for (engine = 0; engine < DPI_MAX_ENGINES; engine++) {
> + reg = DPI_ENG_BUF_BLKS(eng_buf[engine & DPI_ENGINE_MASK]);
> + dpi_reg_write(dpi, DPI_ENGX_BUF(engine), reg);
> +
> + if (cfg.update_molr) {
> + reg = DPI_DMA_ENG_EN_MOLR(cfg.molr[engine & DPI_ENGINE_MASK]);
> + dpi_reg_write(dpi, DPI_DMA_ENGX_EN(engine), reg);
> + }
> + }
> +
> + return 0;
> +}
> +
> +static long dpi_dev_ioctl(struct file *fptr, unsigned int cmd, unsigned long data)
> +{
> + void __user *arg = (void __user *)data;
> + struct dpipf *dpi;
> + int ret = -EINVAL;

Wrong error code for an invalid ioctl command :(

> +
> + dpi = container_of(fptr->private_data, struct dpipf, miscdev);
> +
> + switch (cmd) {
> + case DPI_MPS_MRRS_CFG:
> + ret = dpi_mps_mrrs_config(dpi, arg);
> + break;
> + case DPI_ENGINE_CFG:
> + ret = dpi_engine_config(dpi, arg);
> + break;
> + default:
> + break;
> + }
> +
> + return ret;
> +}
> +
> +static const struct file_operations dpi_device_fops = {
> + .owner = THIS_MODULE,
> + .unlocked_ioctl = dpi_dev_ioctl,
> + .compat_ioctl = compat_ptr_ioctl,
> +};
> +
> +static int dpi_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> +{
> + struct device *dev = &pdev->dev;
> + struct dpipf *dpi;
> + int ret;
> +
> + dpi = devm_kzalloc(dev, sizeof(*dpi), GFP_KERNEL);
> + if (!dpi)
> + return -ENOMEM;
> +
> + dpi->pdev = pdev;
> +
> + ret = pcim_enable_device(pdev);
> + if (ret) {
> + dev_err(dev, "DPI: Failed to enable PCI device\n");
> + return ret;
> + }
> +
> + ret = pcim_iomap_regions(pdev, BIT(0) | BIT(4), DPI_DRV_NAME);
> + if (ret) {
> + dev_err(dev, "DPI: Failed to request MMIO region\n");
> + return ret;
> + }
> +
> + dpi->reg_base = pcim_iomap_table(pdev)[PCI_DPI_CFG_BAR];
> +
> + /* Initialize global PF registers */
> + dpi_init(dpi);
> +
> + /* Setup PF-VF mailbox */
> + ret = dpi_pfvf_mbox_setup(dpi);
> + if (ret) {
> + dev_err(dev, "DPI: Failed to setup pf-vf mbox\n");
> + goto err_dpi_fini;
> + }
> +
> + /* Register interrupts */
> + ret = dpi_irq_init(dpi);
> + if (ret) {
> + dev_err(dev, "DPI: Failed to initialize irq vectors\n");
> + goto err_dpi_mbox_free;
> + }
> +
> + pci_set_drvdata(pdev, dpi);
> + dpi->miscdev.minor = MISC_DYNAMIC_MINOR;
> + dpi->miscdev.name = DPI_DRV_NAME;
> + dpi->miscdev.fops = &dpi_device_fops;
> + dpi->miscdev.parent = dev;
> +
> + ret = misc_register(&dpi->miscdev);
> + if (ret) {
> + dev_err(dev, "DPI: Failed to register misc device\n");
> + goto err_dpi_mbox_free;
> + }
> +
> + return 0;
> +
> +err_dpi_mbox_free:
> + dpi_pfvf_mbox_destroy(dpi);
> +err_dpi_fini:
> + dpi_fini(dpi);
> + return ret;
> +}
> +
> +static void dpi_remove(struct pci_dev *pdev)
> +{
> + struct dpipf *dpi = pci_get_drvdata(pdev);
> +
> + misc_deregister(&dpi->miscdev);
> + pci_sriov_configure_simple(pdev, 0);
> + dpi_pfvf_mbox_destroy(dpi);
> + dpi_fini(dpi);
> + pci_set_drvdata(pdev, NULL);
> +}
> +
> +static const struct pci_device_id dpi_id_table[] = {
> + { PCI_DEVICE_SUB(PCI_VENDOR_ID_CAVIUM, PCI_DEVID_MRVL_CN10K_DPI_PF,
> + PCI_VENDOR_ID_CAVIUM, PCI_SUBDEVID_MRVL_CN10K_DPI_PF) },
> + { 0, } /* end of table */
> +};
> +
> +static struct pci_driver dpi_driver = {
> + .name = DPI_DRV_NAME,
> + .id_table = dpi_id_table,
> + .probe = dpi_probe,
> + .remove = dpi_remove,
> + .sriov_configure = pci_sriov_configure_simple,
> +};
> +
> +module_pci_driver(dpi_driver);
> +MODULE_DEVICE_TABLE(pci, dpi_id_table);
> +MODULE_AUTHOR("Marvell.");
> +MODULE_DESCRIPTION("Marvell Octeon CN10K DPI Driver");
> +MODULE_LICENSE("GPL");
> diff --git a/include/uapi/misc/mrvl_cn10k_dpi.h b/include/uapi/misc/mrvl_cn10k_dpi.h
> new file mode 100644
> index 000000000000..a1951644448a
> --- /dev/null
> +++ b/include/uapi/misc/mrvl_cn10k_dpi.h
> @@ -0,0 +1,37 @@
> +/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */

Why is this file "GPL-2.0+" but your driver is "GPL-2.0"? Is that what
your lawyers want to have happen (sorry, I have to ask.)

thanks,

greg k-h

Hi Vamsi,

kernel test robot noticed the following build errors:

[auto build test ERROR on char-misc/char-misc-testing]
[.also build test ERROR on char-misc/char-misc-next char-misc/char-misc-linus soc/for-next linus/master v6.9-rc6 next-20240430]
[.If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Vamsi-Attunuru/misc-mrvl-cn10k-dpi-add-Octeon-CN10K-DPI-administrative-driver/20240430-142354
base: char-misc/char-misc-testing
patch link: https://lore.kernel.org/r/20240429055027.2162310-1-vattunuru%40marvell.com
patch subject: [PATCH v6 1/1] misc: mrvl-cn10k-dpi: add Octeon CN10K DPI administrative driver
config: arc-allnoconfig (https://download.01.org/0day-ci/archive/20240430/202404302147.7X1w59Jg-lkp@intel.com/config)
compiler: arc-elf-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240430/202404302147.7X1w59Jg-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202404302147.7X1w59Jg-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

drivers/misc/mrvl_cn10k_dpi.c: In function 'dpi_reg_write':
>> drivers/misc/mrvl_cn10k_dpi.c:202:9: error: implicit declaration of function 'writeq'; did you mean 'writeb'? [-Werror=implicit-function-declaration]
202 | writeq(val, dpi->reg_base + offset);
| ^~~~~~
| writeb
drivers/misc/mrvl_cn10k_dpi.c: In function 'dpi_reg_read':
>> drivers/misc/mrvl_cn10k_dpi.c:207:16: error: implicit declaration of function 'readq'; did you mean 'readb'? [-Werror=implicit-function-declaration]
207 | return readq(dpi->reg_base + offset);
| ^~~~~
| readb
drivers/misc/mrvl_cn10k_dpi.c: In function 'dpi_free_irq_vectors':
>> drivers/misc/mrvl_cn10k_dpi.c:453:9: error: implicit declaration of function 'pci_free_irq_vectors'; did you mean 'dpi_free_irq_vectors'? [-Werror=implicit-function-declaration]
453 | pci_free_irq_vectors((struct pci_dev *)pdev);
| ^~~~~~~~~~~~~~~~~~~~
| dpi_free_irq_vectors
In file included from include/uapi/linux/posix_types.h:5,
from include/uapi/linux/types.h:14,
from include/linux/types.h:6,
from include/linux/compat.h:9,
from drivers/misc/mrvl_cn10k_dpi.c:8:
drivers/misc/mrvl_cn10k_dpi.c: In function 'dpi_remove':
include/linux/stddef.h:8:14: error: called object is not a function or function pointer
8 | #define NULL ((void *)0)
| ^
include/linux/pci.h:2445:41: note: in expansion of macro 'NULL'
2445 | #define pci_sriov_configure_simple NULL
| ^~~~
drivers/misc/mrvl_cn10k_dpi.c:661:9: note: in expansion of macro 'pci_sriov_configure_simple'
661 | pci_sriov_configure_simple(pdev, 0);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/misc/mrvl_cn10k_dpi.c: At top level:
>> drivers/misc/mrvl_cn10k_dpi.c:681:1: warning: data definition has no type or storage class
681 | module_pci_driver(dpi_driver);
| ^~~~~~~~~~~~~~~~~
>> drivers/misc/mrvl_cn10k_dpi.c:681:1: error: type defaults to 'int' in declaration of 'module_pci_driver' [-Werror=implicit-int]
>> drivers/misc/mrvl_cn10k_dpi.c:681:1: warning: parameter names (without types) in function declaration
>> drivers/misc/mrvl_cn10k_dpi.c:673:26: warning: 'dpi_driver' defined but not used [-Wunused-variable]
673 | static struct pci_driver dpi_driver = {
| ^~~~~~~~~~
cc1: some warnings being treated as errors


vim +202 drivers/misc/mrvl_cn10k_dpi.c

199
200 static inline void dpi_reg_write(struct dpipf *dpi, u64 offset, u64 val)
201 {
> 202 writeq(val, dpi->reg_base + offset);
203 }
204
205 static inline u64 dpi_reg_read(struct dpipf *dpi, u64 offset)
206 {
> 207 return readq(dpi->reg_base + offset);
208 }
209

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Hi Vamsi,

kernel test robot noticed the following build errors:

[auto build test ERROR on char-misc/char-misc-testing]
[.also build test ERROR on char-misc/char-misc-next char-misc/char-misc-linus soc/for-next linus/master v6.9-rc6 next-20240430]
[.If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Vamsi-Attunuru/misc-mrvl-cn10k-dpi-add-Octeon-CN10K-DPI-administrative-driver/20240430-142354
base: char-misc/char-misc-testing
patch link: https://lore.kernel.org/r/20240429055027.2162310-1-vattunuru%40marvell.com
patch subject: [PATCH v6 1/1] misc: mrvl-cn10k-dpi: add Octeon CN10K DPI administrative driver
config: um-allmodconfig (https://download.01.org/0day-ci/archive/20240430/202404302217.urdwvcqg-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 37ae4ad0eef338776c7e2cffb3896153d43dcd90)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240430/202404302217.urdwvcqg-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202404302217.urdwvcqg-lkp@intel.com/

All errors (new ones prefixed by >>):

In file included from drivers/misc/mrvl_cn10k_dpi.c:12:
In file included from include/linux/pci.h:38:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
547 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
560 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
| ^
In file included from drivers/misc/mrvl_cn10k_dpi.c:12:
In file included from include/linux/pci.h:38:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
573 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
| ^
In file included from drivers/misc/mrvl_cn10k_dpi.c:12:
In file included from include/linux/pci.h:38:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
584 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
594 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
604 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:692:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
692 | readsb(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:700:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
700 | readsw(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:708:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
708 | readsl(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:717:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
717 | writesb(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:726:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
726 | writesw(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:735:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
735 | writesl(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
In file included from drivers/misc/mrvl_cn10k_dpi.c:12:
In file included from include/linux/pci.h:2693:
In file included from include/linux/dma-mapping.h:11:
In file included from include/linux/scatterlist.h:8:
In file included from include/linux/mm.h:2208:
include/linux/vmstat.h:522:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
522 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
>> drivers/misc/mrvl_cn10k_dpi.c:453:2: error: call to undeclared function 'pci_free_irq_vectors'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
453 | pci_free_irq_vectors((struct pci_dev *)pdev);
| ^
drivers/misc/mrvl_cn10k_dpi.c:453:2: note: did you mean 'dpi_free_irq_vectors'?
drivers/misc/mrvl_cn10k_dpi.c:451:13: note: 'dpi_free_irq_vectors' declared here
451 | static void dpi_free_irq_vectors(void *pdev)
| ^
452 | {
453 | pci_free_irq_vectors((struct pci_dev *)pdev);
| ~~~~~~~~~~~~~~~~~~~~
| dpi_free_irq_vectors
>> drivers/misc/mrvl_cn10k_dpi.c:661:28: error: called object type 'void *' is not a function or function pointer
661 | pci_sriov_configure_simple(pdev, 0);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~^
>> drivers/misc/mrvl_cn10k_dpi.c:681:1: error: type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int [-Wimplicit-int]
681 | module_pci_driver(dpi_driver);
| ^
| int
>> drivers/misc/mrvl_cn10k_dpi.c:681:19: error: a parameter list without types is only allowed in a function definition
681 | module_pci_driver(dpi_driver);
| ^
13 warnings and 4 errors generated.


vim +/pci_free_irq_vectors +453 drivers/misc/mrvl_cn10k_dpi.c

450
451 static void dpi_free_irq_vectors(void *pdev)
452 {
> 453 pci_free_irq_vectors((struct pci_dev *)pdev);
454 }
455

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Hi Vamsi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on char-misc/char-misc-testing]
[.also build test WARNING on char-misc/char-misc-next char-misc/char-misc-linus soc/for-next linus/master v6.9-rc6 next-20240430]
[.If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Vamsi-Attunuru/misc-mrvl-cn10k-dpi-add-Octeon-CN10K-DPI-administrative-driver/20240430-142354
base: char-misc/char-misc-testing
patch link: https://lore.kernel.org/r/20240429055027.2162310-1-vattunuru%40marvell.com
patch subject: [PATCH v6 1/1] misc: mrvl-cn10k-dpi: add Octeon CN10K DPI administrative driver
config: m68k-m5208evb_defconfig (https://download.01.org/0day-ci/archive/20240430/202404302343.PSH0M0Cy-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240430/202404302343.PSH0M0Cy-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202404302343.PSH0M0Cy-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/misc/mrvl_cn10k_dpi.c:197:9: warning: 'packed' attribute ignored for field of type 'struct dpi_mbox_message_s' [-Wattributes]
197 | } s __packed;
| ^
drivers/misc/mrvl_cn10k_dpi.c: In function 'dpi_reg_write':
drivers/misc/mrvl_cn10k_dpi.c:202:9: error: implicit declaration of function 'writeq'; did you mean 'writeb'? [-Werror=implicit-function-declaration]
202 | writeq(val, dpi->reg_base + offset);
| ^~~~~~
| writeb
drivers/misc/mrvl_cn10k_dpi.c: In function 'dpi_reg_read':
drivers/misc/mrvl_cn10k_dpi.c:207:16: error: implicit declaration of function 'readq'; did you mean 'readb'? [-Werror=implicit-function-declaration]
207 | return readq(dpi->reg_base + offset);
| ^~~~~
| readb
drivers/misc/mrvl_cn10k_dpi.c: In function 'dpi_free_irq_vectors':
drivers/misc/mrvl_cn10k_dpi.c:453:9: error: implicit declaration of function 'pci_free_irq_vectors'; did you mean 'dpi_free_irq_vectors'? [-Werror=implicit-function-declaration]
453 | pci_free_irq_vectors((struct pci_dev *)pdev);
| ^~~~~~~~~~~~~~~~~~~~
| dpi_free_irq_vectors
In file included from include/uapi/linux/posix_types.h:5,
from include/uapi/linux/types.h:14,
from include/linux/types.h:6,
from include/linux/compat.h:9,
from drivers/misc/mrvl_cn10k_dpi.c:8:
drivers/misc/mrvl_cn10k_dpi.c: In function 'dpi_remove':
include/linux/stddef.h:8:14: error: called object is not a function or function pointer
8 | #define NULL ((void *)0)
| ^
include/linux/pci.h:2445:41: note: in expansion of macro 'NULL'
2445 | #define pci_sriov_configure_simple NULL
| ^~~~
drivers/misc/mrvl_cn10k_dpi.c:661:9: note: in expansion of macro 'pci_sriov_configure_simple'
661 | pci_sriov_configure_simple(pdev, 0);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/misc/mrvl_cn10k_dpi.c: At top level:
drivers/misc/mrvl_cn10k_dpi.c:681:1: warning: data definition has no type or storage class
681 | module_pci_driver(dpi_driver);
| ^~~~~~~~~~~~~~~~~
drivers/misc/mrvl_cn10k_dpi.c:681:1: error: type defaults to 'int' in declaration of 'module_pci_driver' [-Werror=implicit-int]
drivers/misc/mrvl_cn10k_dpi.c:681:1: warning: parameter names (without types) in function declaration
drivers/misc/mrvl_cn10k_dpi.c:673:26: warning: 'dpi_driver' defined but not used [-Wunused-variable]
673 | static struct pci_driver dpi_driver = {
| ^~~~~~~~~~
cc1: some warnings being treated as errors


vim +197 drivers/misc/mrvl_cn10k_dpi.c

175
176 union dpi_mbox_message_t {
177 u64 u[2];
178 struct dpi_mbox_message_s {
179 /* VF ID to configure */
180 u64 vfid :8;
181 /* Command code */
182 u64 cmd :4;
183 /* Command buffer size in 8-byte words */
184 u64 csize :14;
185 /* Aura of the command buffer */
186 u64 aura :20;
187 /* SSO PF function */
188 u64 sso_pf_func :16;
189 /* NPA PF function */
190 u64 npa_pf_func :16;
191 /* Work queue completion status enable */
192 u64 wqecs :1;
193 /* Work queue completion status byte offset */
194 u64 wqecsoff :7;
195 /* Reserved */
196 u64 rsvd :42;
> 197 } s __packed;
198 };
199

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

> -----Original Message-----
> From: Greg KH <gregkh@linuxfoundation.org>
> Sent: Monday, April 29, 2024 2:44 PM
> To: Vamsi Krishna Attunuru <vattunuru@marvell.com>
> Cc: arnd@arndb.de; Jerin Jacob <jerinj@marvell.com>; linux-
> kernel@vger.kernel.org
> Subject: [EXTERNAL] Re: [PATCH v6 1/1] misc: mrvl-cn10k-dpi: add Octeon
> CN10K DPI administrative driver
>
> Prioritize security for external emails: Confirm sender and content safety
> before clicking links or opening attachments
>
> ----------------------------------------------------------------------
> On Sun, Apr 28, 2024 at 10:50:27PM -0700, Vamsi Attunuru wrote:
> > Adds a misc driver for Marvell CN10K DPI(DMA Engine) device's physical
> > function which initializes DPI DMA hardware's global configuration and
> > enables hardware mailbox channels between physical function (PF) and
> > it's virtual functions (VF). VF device drivers (User space drivers)
> > use this hw mailbox to communicate any required device configuration
> > on it's respective VF device. Accordingly, this DPI PF driver
> > provisions the VF device resources.
> >
> > At the hardware level, the DPI physical function (PF) acts as a
> > management interface to setup the VF device resources, VF devices are
> > only provisioned to handle or control the actual DMA Engine's data transfer
> capabilities.
> >
> > Signed-off-by: Vamsi Attunuru <vattunuru@marvell.com>
>
> No one else at Marvell has reviewed this before you submitted this?

It was reviewed until V3, sorry for the inconvenience. I will get the next version
thoroughly reviewed and will send with reviewed-by sign off.
>
> > ---
> > Changes V5 -> V6:
> > - Updated documentation
> > - Fixed data types in uapi file
> >
> > Changes V4 -> V5:
> > - Fixed license and data types in uapi file
> >
> > Changes V3 -> V4:
> > - Moved ioctl definations to .h file
> > - Fixed structure alignements which are passed in ioctl
> >
> > Changes V2 -> V3:
> > - Added ioctl operation to the fops
> > - Used managed version of kzalloc & request_irq
> > - Addressed miscellaneous comments
> >
> > Changes V1 -> V2:
> > - Fixed return values and busy-wait loops
> > - Merged .h file into .c file
> > - Fixed directory structure
> > - Removed module params
> > - Registered the device as misc device
> >
> > Documentation/misc-devices/index.rst | 1 +
> > Documentation/misc-devices/mrvl_cn10k_dpi.rst | 57 ++
> > .../userspace-api/ioctl/ioctl-number.rst | 1 +
> > MAINTAINERS | 5 +
> > drivers/misc/Kconfig | 13 +
> > drivers/misc/Makefile | 2 +
> > drivers/misc/mrvl_cn10k_dpi.c | 685 ++++++++++++++++++
> > include/uapi/misc/mrvl_cn10k_dpi.h | 37 +
> > 8 files changed, 801 insertions(+)
> >
> > diff --git a/Documentation/misc-devices/index.rst
> > b/Documentation/misc-devices/index.rst
> > index 2d0ce9138588..10f2e0f74e45 100644
> > --- a/Documentation/misc-devices/index.rst
> > +++ b/Documentation/misc-devices/index.rst
> > @@ -20,6 +20,7 @@ fit into other categories.
> > ics932s401
> > isl29003
> > lis3lv02d
> > + mrvl_cn10k_dpi
> > max6875
> > oxsemi-tornado
> > pci-endpoint-test
>
> Why not in sorted order?
Ack, will fix it.

>
> > diff --git a/Documentation/misc-devices/mrvl_cn10k_dpi.rst
> > b/Documentation/misc-devices/mrvl_cn10k_dpi.rst
> > new file mode 100644
> > index 000000000000..cce202f114b7
> > --- /dev/null
> > +++ b/Documentation/misc-devices/mrvl_cn10k_dpi.rst
> > @@ -0,0 +1,57 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +
> > +===============================================
> > +Marvell CN10K DMA packet interface (DPI) driver
> > +===============================================
> > +
> > +Overview
> > +========
> > +
> > +DPI is a DMA packet interface hardware block in Marvell's CN10K silicon.
> > +DPI hardware comprises a physical function (PF), its virtual
> > +functions, mailbox logic, and a set of DMA engines & DMA command
> queues.
> > +
> > +DPI PF function is an administrative function which services the
> > +mailbox requests from its VF functions and provisions DMA engine
> > +resources to it's VF functions.
> > +
> > +mrvl_cn10k_dpi.ko misc driver loads on DPI PF device and services the
> > +mailbox commands submitted by the VF devices and accordingly
> > +initializes the DMA engines and VF device's DMA command queues. Also,
> > +driver creates /dev/mrvl-cn10k-dpi node to set DMA engine and PEM
> > +(PCIe interface) port attributes like fifo length, molr, mps & mrrs.
> > +
> > +DPI PF driver is just an administrative driver to setup its VF
> > +device's queues and provisions the hardware resources, it can not
> > +initiate any DMA operations. Only VF devices are provisioned with DMA
> capabilities.
> > +
> > +Driver location
> > +===============
> > +
> > +drivers/misc/mrvl_cn10k_dpi.c
> > +
> > +Driver IOCTLs
> > +=============
> > +
> > +:c:macro::`DPI_MPS_MRRS_CFG`
> > +ioctl that sets max payload size & max read request size parameters
> > +of a pem port to which DMA engines are wired.
> > +
> > +
> > +:c:macro::`DPI_ENGINE_CFG`
> > +ioctl that sets DMA engine's fifo sizes & max outstanding load
> > +request thresholds.
> > +
> > +Userspace code example
> > +----------------------
> > +
> > +DPI VF devices are managed by user space drivers, below is a
> > +reference code to the user space driver's mailbox command exchange
> > +with DPI PF driver through hardware mailbox.
> > +
> > +https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__github.com_Vamsi
> > +KrishnaA99_dpi-2Ddma_blob_main_driver_roc-
> 5Fdpi.c&d=DwIBAg&c=nKjWec2b
> >
> +6R0mOyPaz7xtfQ&r=WllrYaumVkxaWjgKto6E_rtDQshhIhik2jkvzFyRhW8&m
> =7PlVgM
> > +7n0EhJ17QxPm3mXq6PT0CQzsOn2YFnRB8RMsIQdFAsZ5UKdTFmkcChUrf-
> &s=8wLYfXsc
> > +lLz7Fuvz9FfyEehC5xTSiIa88PJzturPF4U&e=
> > +
> > +Below is a sample application that uses driver IOCTLs to setup DMA
> > +engine and PEM port attributes over `/dev/mrvl-cn10k-dpi` node.
> > +
> > +https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__github.com_Vamsi
> > +KrishnaA99_dpi-
> 2Ddma_blob_main_application_main.c&d=DwIBAg&c=nKjWec2b
> >
> +6R0mOyPaz7xtfQ&r=WllrYaumVkxaWjgKto6E_rtDQshhIhik2jkvzFyRhW8&m
> =7PlVgM
> > +7n0EhJ17QxPm3mXq6PT0CQzsOn2YFnRB8RMsIQdFAsZ5UKdTFmkcChUrf-
> &s=BMRvLLAx
> > +cnlsyZlVCgOoUXju_jKom8zbKQ9GxlxlApg&e=
>
> I appreciate the code being open, but this is on a personal account that was
> created for only this one package, and it doesn't even have a README. Yet it
> is fully owned/copyrighted by Marvell and the code has been around since
> 2021? Why isn't this on a Marvell-controlled page to show that this really is
> supported and is the proper interface to use this driver that it provides to
> their customers?
>
User space driver is already part of open-source code base(github.com/DPDK/dpdk.git).
The mailbox related changes in user driver (roc_dpi.c file) are not yet upstreamed
which has the dependency with this kernel driver. So, I put the latest user driver
in personal account and shared so that it's helpful for reviewing kernel driver.

I will work internally and get it moved to Marvell-controlled page, will host at
https://github.com/MarvellEmbeddedProcessors and update the same in
Documentation page in next version.

>
>
> > diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst
> > b/Documentation/userspace-api/ioctl/ioctl-number.rst
> > index 457e16f06e04..e6fd0c386b59 100644
> > --- a/Documentation/userspace-api/ioctl/ioctl-number.rst
> > +++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
> > @@ -358,6 +358,7 @@ Code Seq# Include File
> Comments
> > 0xB6 all linux/fpga-dfl.h
> > 0xB7 all uapi/linux/remoteproc_cdev.h <mailto:linux-
> remoteproc@vger.kernel.org>
> > 0xB7 all uapi/linux/nsfs.h <mailto:Andrei Vagin
> <avagin@openvz.org>>
> > +0xB8 01-02 uapi/misc/mrvl_cn10k_dpi.h Marvell CN10K DPI
> driver
> > 0xC0 00-0F linux/usb/iowarrior.h
> > 0xCA 00-0F uapi/misc/cxl.h
> > 0xCA 10-2F uapi/misc/ocxl.h
> > diff --git a/MAINTAINERS b/MAINTAINERS index
> > 960512bec428..ab77232d583e 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -13104,6 +13104,11 @@ S: Supported
> > F: Documentation/devicetree/bindings/mmc/marvell,xenon-
> sdhci.yaml
> > F: drivers/mmc/host/sdhci-xenon*
> >
> > +MARVELL OCTEON CN10K DPI DRIVER
> > +M: Vamsi Attunuru <vattunuru@marvell.com>
> > +S: Maintained
>
> So this is not part of your job to support this code? Why isn't Marvell allowing
> that to happen?

No, we are happy to support it further, will fix this.

>
> > +F: drivers/misc/mrvl_cn10k_dpi.c
> > +
> > MATROX FRAMEBUFFER DRIVER
> > L: linux-fbdev@vger.kernel.org
> > S: Orphan
> > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index
> > 4fb291f0bf7c..78470ef2538f 100644
> > --- a/drivers/misc/Kconfig
> > +++ b/drivers/misc/Kconfig
> > @@ -574,6 +574,19 @@ config NSM
> > To compile this driver as a module, choose M here.
> > The module will be called nsm.
> >
> > +config MARVELL_CN10K_DPI
> > + tristate "Octeon CN10K DPI driver"
> > + depends on ARM64 && PCI
> > + help
> > + Enables Octeon CN10K DMA packet interface (DPI) driver which
> intializes
> > + DPI hardware's physical function (PF) device's global configuration
> and
> > + its virtual function's (VFs) resource configuration to enable DMA
> transfers.
> > + DPI PF device does not have any data movement functionality, it
> only serves
> > + VF's resource configuration requests.
>
> Nit, please wrap at 72 columns, didn't checkpatch.pl complain about this?
>

Yes, I did not see any complains, not sure if I am missing any options. I will
double check and ensure clean patch in next version.

> > +
> > + To compile this driver as a module, choose M here: the module
> > + will be called mrvl_cn10k_dpi.
> > +
> > source "drivers/misc/c2port/Kconfig"
> > source "drivers/misc/eeprom/Kconfig"
> > source "drivers/misc/cb710/Kconfig"
> > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index
> > ea6ea5bbbc9c..5106bf96ea5c 100644
> > --- a/drivers/misc/Makefile
> > +++ b/drivers/misc/Makefile
> > @@ -68,3 +68,5 @@ obj-$(CONFIG_TMR_INJECT) += xilinx_tmr_inject.o
> > obj-$(CONFIG_TPS6594_ESM) += tps6594-esm.o
> > obj-$(CONFIG_TPS6594_PFSM) += tps6594-pfsm.o
> > obj-$(CONFIG_NSM) += nsm.o
> > +obj-$(CONFIG_MARVELL_CN10K_DPI) += mrvl_cn10k_dpi.o
> > +obj-y += mrvl_cn10k_dpi.o
>
> That is odd, why are you saying obj-y here? You want everyone to always
> build this code into the kernel image no matter what
>

Sorry, it's my bad, this change(2nd line) was mistakenly added, will remove it.

> > diff --git a/drivers/misc/mrvl_cn10k_dpi.c
> > b/drivers/misc/mrvl_cn10k_dpi.c new file mode 100644 index
> > 000000000000..bd99583994f9
> > --- /dev/null
> > +++ b/drivers/misc/mrvl_cn10k_dpi.c
> > @@ -0,0 +1,685 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Marvell Octeon CN10K DPI driver
> > + *
> > + * Copyright (C) 2024 Marvell.
> > + *
> > + */
> > +
> > +#include <linux/compat.h>
> > +#include <linux/delay.h>
> > +#include <linux/miscdevice.h>
> > +#include <linux/module.h>
> > +#include <linux/pci.h>
> > +#include <linux/irq.h>
> > +#include <linux/interrupt.h>
> > +
> > +#include <uapi/misc/mrvl_cn10k_dpi.h>
> > +
> > +#define DPI_DRV_NAME "mrvl-cn10k-dpi"
>
> KBUILD_MODNAME please.

ack
>
> > +
> > +/* PCI device IDs */
> > +#define PCI_DEVID_MRVL_CN10K_DPI_PF 0xA080
> > +#define PCI_SUBDEVID_MRVL_CN10K_DPI_PF 0xB900
> > +
> > +/* PCI BAR nos */
>
> What is "nos"?
It's numbers, will add full wording.
>
> > +#define PCI_DPI_CFG_BAR 0
> > +
> > +/* MSI-X interrupts */
> > +#define DPI_MAX_REQQ_INT 32
> > +#define DPI_MAX_CC_INT 64
> > +
> > +/* MBOX MSI-X interrupt vector index */ #define
> > +DPI_MBOX_PF_VF_INT_IDX 0x75
> > +
> > +#define DPI_MAX_IRQS (DPI_MBOX_PF_VF_INT_IDX + 1)
> > +
> > +#define DPI_MAX_VFS 32
> > +
> > +#define DPI_ENGINE_MASK GENMASK(2, 0)
> > +
> > +#define DPI_DMA_IDS_DMA_NPA_PF_FUNC(x) (((x) &
> GENMASK_ULL(15, 0)) << 16)
> > +#define DPI_DMA_IDS_INST_STRM(x) (((x) &
> GENMASK_ULL(7, 0)) << 40)
> > +#define DPI_DMA_IDS_DMA_STRM(x) (((x) &
> GENMASK_ULL(7, 0)) << 32)
> > +#define DPI_DMA_ENG_EN_MOLR(x) (((x) &
> GENMASK_ULL(9, 0)) << 32)
> > +#define DPI_EBUS_PORTX_CFG_MPS(x) (((x) & GENMASK(2,
> 0)) << 4)
> > +#define DPI_DMA_IDS_DMA_SSO_PF_FUNC(x) ((x) &
> GENMASK(15, 0))
> > +#define DPI_DMA_IDS2_INST_AURA(x) ((x) & GENMASK(19,
> 0))
> > +#define DPI_DMA_IBUFF_CSIZE_CSIZE(x) ((x) & GENMASK(13,
> 0))
> > +#define DPI_EBUS_PORTX_CFG_MRRS(x) ((x) & GENMASK(2,
> 0))
> > +#define DPI_ENG_BUF_BLKS(x) ((x) & GENMASK(5,
> 0))
> > +#define DPI_DMA_CONTROL_DMA_ENB
> GENMASK_ULL(53, 48)
> > +
> > +#define DPI_DMA_CONTROL_O_MODE BIT_ULL(14)
> > +#define DPI_DMA_CONTROL_LDWB BIT_ULL(32)
> > +#define DPI_DMA_CONTROL_WQECSMODE1 BIT_ULL(37)
> > +#define DPI_DMA_CONTROL_ZBWCSEN BIT_ULL(39)
> > +#define DPI_DMA_CONTROL_WQECSOFF(offset) (((u64)offset) << 40)
> > +#define DPI_DMA_CONTROL_WQECSDIS BIT_ULL(47)
> > +#define DPI_DMA_CONTROL_PKT_EN BIT_ULL(56)
> > +#define DPI_DMA_IBUFF_CSIZE_NPA_FREE BIT(16)
> > +
> > +#define DPI_CTL_EN BIT_ULL(0)
> > +#define DPI_DMA_CC_INT BIT_ULL(0)
> > +#define DPI_DMA_QRST BIT_ULL(0)
> > +
> > +#define DPI_REQQ_INT_INSTRFLT BIT_ULL(0)
> > +#define DPI_REQQ_INT_RDFLT BIT_ULL(1)
> > +#define DPI_REQQ_INT_WRFLT BIT_ULL(2)
> > +#define DPI_REQQ_INT_CSFLT BIT_ULL(3)
> > +#define DPI_REQQ_INT_INST_DBO BIT_ULL(4)
> > +#define DPI_REQQ_INT_INST_ADDR_NULL BIT_ULL(5)
> > +#define DPI_REQQ_INT_INST_FILL_INVAL BIT_ULL(6)
> > +#define DPI_REQQ_INT_INSTR_PSN BIT_ULL(7)
> > +
> > +#define DPI_REQQ_INT \
> > + (DPI_REQQ_INT_INSTRFLT | \
> > + DPI_REQQ_INT_RDFLT | \
> > + DPI_REQQ_INT_WRFLT | \
> > + DPI_REQQ_INT_CSFLT | \
> > + DPI_REQQ_INT_INST_DBO | \
> > + DPI_REQQ_INT_INST_ADDR_NULL | \
> > + DPI_REQQ_INT_INST_FILL_INVAL | \
> > + DPI_REQQ_INT_INSTR_PSN)
> > +
> > +#define DPI_PF_RAS_EBI_DAT_PSN BIT_ULL(0)
> > +#define DPI_PF_RAS_NCB_DAT_PSN BIT_ULL(1)
> > +#define DPI_PF_RAS_NCB_CMD_PSN BIT_ULL(2)
> > +
> > +#define DPI_PF_RAS_INT \
> > + (DPI_PF_RAS_EBI_DAT_PSN | \
> > + DPI_PF_RAS_NCB_DAT_PSN | \
> > + DPI_PF_RAS_NCB_CMD_PSN)
> > +
> > +#define DPI_DMAX_IBUFF_CSIZE(x) (0x0ULL | ((x) << 11))
> > +#define DPI_DMAX_IDS(x) (0x18ULL | ((x) << 11))
> > +#define DPI_DMAX_IDS2(x) (0x20ULL | ((x) << 11))
> > +#define DPI_DMAX_QRST(x) (0x30ULL | ((x) << 11))
> > +
> > +#define DPI_CTL 0x10010ULL
> > +#define DPI_DMA_CONTROL 0x10018ULL
> > +#define DPI_DMA_ENGX_EN(x) (0x10040ULL | ((x) << 3))
> > +#define DPI_ENGX_BUF(x) (0x100C0ULL | ((x) << 3))
> > +
> > +#define DPI_EBUS_PORTX_CFG(x) (0x10100ULL | ((x) << 3))
> > +
> > +#define DPI_PF_RAS 0x10308ULL
> > +#define DPI_PF_RAS_ENA_W1C 0x10318ULL
> > +
> > +#define DPI_DMA_CCX_INT(x) (0x11000ULL | ((x) << 3)) #define
> > +DPI_DMA_CCX_INT_ENA_W1C(x) (0x11800ULL | ((x) << 3))
> > +
> > +#define DPI_REQQX_INT(x) (0x12C00ULL | ((x) << 5)) #define
> > +DPI_REQQX_INT_ENA_W1C(x) (0x13800ULL | ((x) << 5))
> > +
> > +#define DPI_MBOX_PF_VF_DATA0(x) (0x16000ULL | ((x) << 4)) #define
> > +DPI_MBOX_PF_VF_DATA1(x) (0x16008ULL | ((x) << 4))
> > +
> > +#define DPI_MBOX_VF_PF_INT 0x16300ULL #define
> DPI_MBOX_VF_PF_INT_W1S
> > +0x16308ULL #define DPI_MBOX_VF_PF_INT_ENA_W1C 0x16310ULL
> #define
> > +DPI_MBOX_VF_PF_INT_ENA_W1S 0x16318ULL
> > +
> > +#define DPI_WCTL_FIF_THR 0x17008ULL
> > +
> > +#define DPI_EBUS_MAX_PORTS 2
> > +
> > +#define DPI_EBUS_MRRS_MIN 128
> > +#define DPI_EBUS_MRRS_MAX 1024
> > +#define DPI_EBUS_MPS_MIN 128
> > +#define DPI_EBUS_MPS_MAX 1024
> > +#define DPI_WCTL_FIFO_THRESHOLD 0x30
> > +
> > +#define DPI_QUEUE_OPEN 0x1
> > +#define DPI_QUEUE_CLOSE 0x2
> > +#define DPI_REG_DUMP 0x3
> > +#define DPI_GET_REG_CFG 0x4
> > +#define DPI_QUEUE_OPEN_V2 0x5
> > +
> > +enum dpi_mbox_rsp_type {
> > + DPI_MBOX_TYPE_CMD,
> > + DPI_MBOX_TYPE_RSP_ACK,
> > + DPI_MBOX_TYPE_RSP_NACK,
> > +};
> > +
> > +struct dpivf_config {
> > + u16 csize;
> > + u32 aura;
> > + u16 sso_pf_func;
> > + u16 npa_pf_func;
>
> Do you intend to have unaligned accesses to the fields in this structure?
>
No, I will fix it.

> > +};
> > +
> > +struct dpipf_vf {
> > + u8 this_vfid;
> > + bool setup_done;
> > + struct dpivf_config vf_config;
> > +};
> > +
> > +/* DPI device mailbox */
> > +struct dpi_mbox {
> > + struct work_struct work;
> > + /* lock to serialize mbox requests */
> > + struct mutex lock;
> > + struct dpipf *pf;
> > + u8 __iomem *pf_vf_data_reg;
> > + u8 __iomem *vf_pf_data_reg;
> > +};
> > +
> > +struct dpipf {
> > + struct miscdevice miscdev;
> > + void __iomem *reg_base;
> > + struct pci_dev *pdev;
> > + struct dpipf_vf vf[DPI_MAX_VFS];
> > + /* Mailbox to talk to VFs */
> > + struct dpi_mbox *mbox[DPI_MAX_VFS];
> > +};
> > +
> > +union dpi_mbox_message_t {
>
> Didn't checkpatch complain about the "_t" here?
>
No, I will double check and fix it.
> > + u64 u[2];
>
> What is "u"?
>
> > + struct dpi_mbox_message_s {
> > + /* VF ID to configure */
> > + u64 vfid :8;
> > + /* Command code */
> > + u64 cmd :4;
> > + /* Command buffer size in 8-byte words */
> > + u64 csize :14;
> > + /* Aura of the command buffer */
> > + u64 aura :20;
> > + /* SSO PF function */
> > + u64 sso_pf_func :16;
> > + /* NPA PF function */
> > + u64 npa_pf_func :16;
> > + /* Work queue completion status enable */
> > + u64 wqecs :1;
> > + /* Work queue completion status byte offset */
> > + u64 wqecsoff :7;
> > + /* Reserved */
> > + u64 rsvd :42;
>
> reserved for what?
>
> If you are reading this from hardware, the bit fields you created here will
> NOT work or be portable at all. Please do this properly.

Sure, I will fix it properly.
>
> Also you have a mix of tabs and spaces for some reason in this structure,
> again, checkpatch should have caught that.
>
> > + } s __packed;
>
> "s"?
>
> What is "u" and "s" here?
>
>
> > +};
> > +
> > +static inline void dpi_reg_write(struct dpipf *dpi, u64 offset, u64
> > +val) {
> > + writeq(val, dpi->reg_base + offset);
>
> No need to read to ensure that the write succeeded? Or are you doing that
> in individual places where you want to make sure it happened? If so, I didn't
> see that in the use of this.

Yes, writes are guaranteed as it's an onboard pcie device and register writes
are uncached memory accesses.
>
> > +}
> > +
> > +static inline u64 dpi_reg_read(struct dpipf *dpi, u64 offset) {
> > + return readq(dpi->reg_base + offset); }
> > +
> > +static void dpi_wqe_cs_offset(struct dpipf *dpi, u8 offset) {
> > + u64 reg;
> > +
> > + reg = dpi_reg_read(dpi, DPI_DMA_CONTROL);
> > + reg &= ~DPI_DMA_CONTROL_WQECSDIS;
> > + reg |= DPI_DMA_CONTROL_ZBWCSEN |
> DPI_DMA_CONTROL_WQECSMODE1;
> > + reg |= DPI_DMA_CONTROL_WQECSOFF(offset);
> > + dpi_reg_write(dpi, DPI_DMA_CONTROL, reg); }
> > +
> > +static int dpi_queue_init(struct dpipf *dpi, struct dpipf_vf *dpivf,
> > +u8 vf) {
> > + u16 sso_pf_func = dpivf->vf_config.sso_pf_func;
> > + u16 npa_pf_func = dpivf->vf_config.npa_pf_func;
> > + u16 csize = dpivf->vf_config.csize;
> > + u32 aura = dpivf->vf_config.aura;
> > + unsigned long timeout;
> > + u64 reg;
> > +
> > + dpi_reg_write(dpi, DPI_DMAX_QRST(vf), DPI_DMA_QRST);
> > +
> > + /* Wait for a maximum of 3 sec */
> > + timeout = jiffies + msecs_to_jiffies(3000);
> > + while (!time_after(jiffies, timeout)) {
> > + reg = dpi_reg_read(dpi, DPI_DMAX_QRST(vf));
> > + if (!(reg & DPI_DMA_QRST))
> > + break;
> > +
> > + usleep_range(500, 1000);
>
> Why sleep this value? Please document.

ack
>
> > + }
> > +
> > + if (reg & DPI_DMA_QRST) {
> > + dev_err(&dpi->pdev->dev, "Queue reset failed\n");
> > + return -EBUSY;
> > + }
> > +
> > + dpi_reg_write(dpi, DPI_DMAX_IDS2(vf), 0);
> > + dpi_reg_write(dpi, DPI_DMAX_IDS(vf), 0);
> > +
> > + reg = DPI_DMA_IBUFF_CSIZE_CSIZE(csize) |
> DPI_DMA_IBUFF_CSIZE_NPA_FREE;
> > + dpi_reg_write(dpi, DPI_DMAX_IBUFF_CSIZE(vf), reg);
> > +
> > + reg = dpi_reg_read(dpi, DPI_DMAX_IDS2(vf));
> > + reg |= DPI_DMA_IDS2_INST_AURA(aura);
> > + dpi_reg_write(dpi, DPI_DMAX_IDS2(vf), reg);
> > +
> > + reg = dpi_reg_read(dpi, DPI_DMAX_IDS(vf));
> > + reg |= DPI_DMA_IDS_DMA_NPA_PF_FUNC(npa_pf_func);
> > + reg |= DPI_DMA_IDS_DMA_SSO_PF_FUNC(sso_pf_func);
> > + reg |= DPI_DMA_IDS_DMA_STRM(vf + 1);
> > + reg |= DPI_DMA_IDS_INST_STRM(vf + 1);
> > + dpi_reg_write(dpi, DPI_DMAX_IDS(vf), reg);
> > +
> > + return 0;
> > +}
> > +
> > +static void dpi_queue_fini(struct dpipf *dpi, struct dpipf_vf *dpivf,
> > +u8 vf) {
> > + dpi_reg_write(dpi, DPI_DMAX_QRST(vf), DPI_DMA_QRST);
> > +
> > + /* Reset IDS and IDS2 registers */
> > + dpi_reg_write(dpi, DPI_DMAX_IDS2(vf), 0);
> > + dpi_reg_write(dpi, DPI_DMAX_IDS(vf), 0); }
> > +
> > +static void dpi_poll_pfvf_mbox(struct dpipf *dpi) {
> > + u64 reg;
> > + u32 vf;
> > +
> > + reg = dpi_reg_read(dpi, DPI_MBOX_VF_PF_INT);
> > + if (reg) {
> > + for (vf = 0; vf < pci_num_vf(dpi->pdev); vf++) {
> > + if (reg & BIT_ULL(vf))
> > + schedule_work(&dpi->mbox[vf]->work);
> > + }
> > + dpi_reg_write(dpi, DPI_MBOX_VF_PF_INT, reg);
> > + }
>
> No error if reg was not read properly?
Yes, ideally no errors as it's an on board pcie device.
>
> > +}
> > +
> > +static irqreturn_t dpi_mbox_intr_handler(int irq, void *data) {
> > + struct dpipf *dpi = data;
> > +
> > + dpi_poll_pfvf_mbox(dpi);
> > +
> > + return IRQ_HANDLED;
>
> So this can always succeed?
Yes. It's msix interrupt and not shared, I will check and fix if any thing
need to be validated.

>
> > +}
> > +
> > +static int queue_config(struct dpipf *dpi, struct dpipf_vf *dpivf,
> > +union dpi_mbox_message_t *msg) {
> > + int ret = 0;
> > +
> > + switch (msg->s.cmd) {
> > + case DPI_QUEUE_OPEN:
> > + case DPI_QUEUE_OPEN_V2:
> > + dpivf->vf_config.aura = msg->s.aura;
> > + dpivf->vf_config.csize = msg->s.cmd == DPI_QUEUE_OPEN ?
> msg->s.csize / 8 :
> > + msg->s.csize;
> > + dpivf->vf_config.sso_pf_func = msg->s.sso_pf_func;
> > + dpivf->vf_config.npa_pf_func = msg->s.npa_pf_func;
> > + ret = dpi_queue_init(dpi, dpivf, msg->s.vfid);
> > + if (!ret) {
> > + if (msg->s.wqecs)
> > + dpi_wqe_cs_offset(dpi, msg->s.wqecsoff);
> > + dpivf->setup_done = true;
> > + }
> > + break;
> > + case DPI_QUEUE_CLOSE:
> > + dpivf->vf_config.aura = 0;
> > + dpivf->vf_config.csize = 0;
> > + dpivf->vf_config.sso_pf_func = 0;
> > + dpivf->vf_config.npa_pf_func = 0;
> > + dpi_queue_fini(dpi, dpivf, msg->s.vfid);
> > + dpivf->setup_done = false;
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static void dpi_pfvf_mbox_work(struct work_struct *work) {
> > + struct dpi_mbox *mbox = container_of(work, struct dpi_mbox,
> work);
> > + union dpi_mbox_message_t msg = { 0 };
>
> Are you sure this can be on the stack?

No, it's not guaranteed, will use memset. Thanks.
>
> > + struct dpipf_vf *dpivf;
> > + struct dpipf *dpi;
> > + int ret;
> > +
> > + dpi = mbox->pf;
> > +
> > + mutex_lock(&mbox->lock);
> > + msg.u[0] = readq(mbox->vf_pf_data_reg);
> > + if (unlikely(msg.u[0] == (u64)-1))
> > + goto exit;
>
> Only use likely/unlikely if you can prove with a benchmark that it makes a
> difference. Otherwise let the CPU and compiler choose for you, as it knows
> better than you do (and will know better over time.)

Ack.
>
> If you want to use unlikely, you have to document it as to how it matters.
>
> > +
> > + if (unlikely(msg.s.vfid >= pci_num_vf(dpi->pdev))) {
> > + dev_err(&dpi->pdev->dev, "Invalid vfid:%d\n", msg.s.vfid);
>
> You send this to the kernel log if a device is broken?

Ok, ideally, it's not case, will remove log.

>
> > + goto exit;
> > + }
> > +
> > + dpivf = &dpi->vf[msg.s.vfid];
> > + msg.u[1] = readq(mbox->pf_vf_data_reg);
> > +
> > + ret = queue_config(dpi, dpivf, &msg);
> > + if (ret < 0)
> > + writeq(DPI_MBOX_TYPE_RSP_NACK, mbox-
> >pf_vf_data_reg);
> > + else
> > + writeq(DPI_MBOX_TYPE_RSP_ACK, mbox->pf_vf_data_reg);
> > +exit:
> > + mutex_unlock(&mbox->lock);
> > +}
> > +
> > +/* Setup registers for a PF mailbox */ static void
> > +dpi_setup_mbox_regs(struct dpipf *dpi, int vf) {
> > + struct dpi_mbox *mbox = dpi->mbox[vf];
> > +
> > + mbox->pf_vf_data_reg = dpi->reg_base +
> DPI_MBOX_PF_VF_DATA0(vf);
> > + mbox->vf_pf_data_reg = dpi->reg_base +
> DPI_MBOX_PF_VF_DATA1(vf); }
> > +
> > +static int dpi_pfvf_mbox_setup(struct dpipf *dpi) {
> > + int vf;
> > +
> > + for (vf = 0; vf < DPI_MAX_VFS; vf++) {
> > + dpi->mbox[vf] = devm_kzalloc(&dpi->pdev->dev,
> > +sizeof(*dpi->mbox[vf]), GFP_KERNEL);
> > +
> > + if (!dpi->mbox[vf])
> > + return -ENOMEM;
> > +
> > + mutex_init(&dpi->mbox[vf]->lock);
> > + INIT_WORK(&dpi->mbox[vf]->work, dpi_pfvf_mbox_work);
> > + dpi->mbox[vf]->pf = dpi;
> > + dpi_setup_mbox_regs(dpi, vf);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void dpi_pfvf_mbox_destroy(struct dpipf *dpi) {
> > + unsigned int vf;
> > +
> > + for (vf = 0; vf < DPI_MAX_VFS; vf++) {
> > + if (work_pending(&dpi->mbox[vf]->work))
> > + cancel_work_sync(&dpi->mbox[vf]->work);
> > +
> > + dpi->mbox[vf] = NULL;
> > + }
> > +}
> > +
> > +static void dpi_init(struct dpipf *dpi) {
> > + unsigned int engine, port;
> > + u8 mrrs_val, mps_val;
> > + u64 reg;
> > +
> > + for (engine = 0; engine < DPI_MAX_ENGINES; engine++) {
> > + if (engine == 4 || engine == 5)
> > + reg = DPI_ENG_BUF_BLKS(16);
> > + else
> > + reg = DPI_ENG_BUF_BLKS(8);
> > +
> > + dpi_reg_write(dpi, DPI_ENGX_BUF(engine), reg);
> > + }
> > +
> > + reg = DPI_DMA_CONTROL_ZBWCSEN |
> DPI_DMA_CONTROL_PKT_EN | DPI_DMA_CONTROL_LDWB |
> > + DPI_DMA_CONTROL_O_MODE |
> DPI_DMA_CONTROL_DMA_ENB;
> > +
> > + dpi_reg_write(dpi, DPI_DMA_CONTROL, reg);
> > + dpi_reg_write(dpi, DPI_CTL, DPI_CTL_EN);
> > +
> > + mrrs_val = 2; /* 512B */
> > + mps_val = 1; /* 256B */
> > +
> > + for (port = 0; port < DPI_EBUS_MAX_PORTS; port++) {
> > + reg = dpi_reg_read(dpi, DPI_EBUS_PORTX_CFG(port));
> > + reg &= ~(DPI_EBUS_PORTX_CFG_MRRS(7) |
> DPI_EBUS_PORTX_CFG_MPS(7));
> > + reg |= DPI_EBUS_PORTX_CFG_MPS(mps_val) |
> DPI_EBUS_PORTX_CFG_MRRS(mrrs_val);
> > + dpi_reg_write(dpi, DPI_EBUS_PORTX_CFG(port), reg);
> > + }
> > +
> > + dpi_reg_write(dpi, DPI_WCTL_FIF_THR,
> DPI_WCTL_FIFO_THRESHOLD); }
> > +
> > +static void dpi_fini(struct dpipf *dpi) {
> > + unsigned int engine;
> > +
> > + for (engine = 0; engine < DPI_MAX_ENGINES; engine++)
> > + dpi_reg_write(dpi, DPI_ENGX_BUF(engine), 0);
> > +
> > + dpi_reg_write(dpi, DPI_DMA_CONTROL, 0);
> > + dpi_reg_write(dpi, DPI_CTL, 0);
> > +}
> > +
> > +static void dpi_free_irq_vectors(void *pdev) {
> > + pci_free_irq_vectors((struct pci_dev *)pdev); }
> > +
> > +static int dpi_irq_init(struct dpipf *dpi) {
> > + struct pci_dev *pdev = dpi->pdev;
> > + struct device *dev = &pdev->dev;
> > + int i, ret;
> > +
> > + /* Clear all RAS interrupts */
> > + dpi_reg_write(dpi, DPI_PF_RAS, DPI_PF_RAS_INT);
> > +
> > + /* Clear all RAS interrupt enable bits */
> > + dpi_reg_write(dpi, DPI_PF_RAS_ENA_W1C, DPI_PF_RAS_INT);
> > +
> > + for (i = 0; i < DPI_MAX_REQQ_INT; i++) {
> > + dpi_reg_write(dpi, DPI_REQQX_INT(i), DPI_REQQ_INT);
> > + dpi_reg_write(dpi, DPI_REQQX_INT_ENA_W1C(i),
> DPI_REQQ_INT);
> > + }
> > +
> > + for (i = 0; i < DPI_MAX_CC_INT; i++) {
> > + dpi_reg_write(dpi, DPI_DMA_CCX_INT(i),
> DPI_DMA_CC_INT);
> > + dpi_reg_write(dpi, DPI_DMA_CCX_INT_ENA_W1C(i),
> DPI_DMA_CC_INT);
> > + }
> > +
> > + ret = pci_alloc_irq_vectors(pdev, DPI_MAX_IRQS, DPI_MAX_IRQS,
> PCI_IRQ_MSIX);
> > + if (ret != DPI_MAX_IRQS) {
> > + dev_err(dev, "DPI: Failed to alloc %d msix irqs\n",
> DPI_MAX_IRQS);
> > + return ret;
> > + }
> > +
> > + ret = devm_add_action_or_reset(dev, dpi_free_irq_vectors, pdev);
> > + if (ret) {
> > + dev_err(dev, "DPI: Failed to add irq free action\n");
> > + return ret;
> > + }
> > +
> > + ret = devm_request_irq(dev, pci_irq_vector(pdev,
> DPI_MBOX_PF_VF_INT_IDX),
> > + dpi_mbox_intr_handler, 0, "dpi-mbox", dpi);
> > + if (ret) {
> > + dev_err(dev, "DPI: request_irq failed for mbox; err=%d\n",
> ret);
> > + return ret;
> > + }
> > +
> > + dpi_reg_write(dpi, DPI_MBOX_VF_PF_INT_ENA_W1S,
> GENMASK_ULL(31, 0));
> > +
> > + return 0;
> > +}
> > +
> > +static int dpi_mps_mrrs_config(struct dpipf *dpi, void __user *arg) {
> > + struct dpi_mps_mrrs_cfg cfg;
> > + u8 mrrs_val, mps_val;
> > + u64 reg;
> > +
> > + if (copy_from_user(&cfg, arg, sizeof(struct dpi_mps_mrrs_cfg)))
> > + return -EFAULT;
> > +
> > + if (cfg.max_read_req_sz < DPI_EBUS_MRRS_MIN ||
> cfg.max_read_req_sz > DPI_EBUS_MRRS_MAX ||
> > + !is_power_of_2(cfg.max_read_req_sz)) {
> > + dev_err(&dpi->pdev->dev, "Invalid MRRS size:%u\n",
> > +cfg.max_read_req_sz);
>
> You are allowing userspace to spam the kernel log with messages by sending
> the driver invalid data. THat is a denial of service, please never do that.

Sure, will fix it.
>
> > + return -EINVAL;
> > + }
> > +
> > + if (cfg.max_payload_sz < DPI_EBUS_MPS_MIN ||
> cfg.max_payload_sz > DPI_EBUS_MPS_MAX ||
> > + !is_power_of_2(cfg.max_payload_sz)) {
> > + dev_err(&dpi->pdev->dev, "Invalid MPS size:%u\n",
> cfg.max_payload_sz);
> > + return -EINVAL;
> > + }
> > +
> > + if (cfg.port >= DPI_EBUS_MAX_PORTS) {
> > + dev_err(&dpi->pdev->dev, "Invalid EBUS port:%u\n",
> cfg.port);
> > + return -EINVAL;
> > + }
> > +
> > + mrrs_val = fls(cfg.max_read_req_sz >> 8);
> > + mps_val = fls(cfg.max_payload_sz >> 8);
> > +
> > + reg = dpi_reg_read(dpi, DPI_EBUS_PORTX_CFG(cfg.port));
> > + reg &= ~(DPI_EBUS_PORTX_CFG_MRRS(0x7) |
> DPI_EBUS_PORTX_CFG_MPS(0x7));
> > + reg |= DPI_EBUS_PORTX_CFG_MPS(mps_val) |
> DPI_EBUS_PORTX_CFG_MRRS(mrrs_val);
> > + dpi_reg_write(dpi, DPI_EBUS_PORTX_CFG(cfg.port), reg);
> > +
> > + return 0;
> > +}
> > +
> > +static int dpi_engine_config(struct dpipf *dpi, void __user *arg) {
> > + struct dpi_engine_cfg cfg;
> > + unsigned int engine;
> > + u8 *eng_buf;
> > + u64 reg;
> > +
> > + if (copy_from_user(&cfg, arg, sizeof(struct dpi_engine_cfg)))
> > + return -EFAULT;
>
> No need to ever validate any information in that structure from userspace?
> You always blindly copy it on to the device? Userspace can never get this
> wrong?
>
Yes, couple of range checks are needed, will address in next patch.
> > +
> > + eng_buf = (u8 *)&cfg.fifo_mask;
> > +
> > + for (engine = 0; engine < DPI_MAX_ENGINES; engine++) {
> > + reg = DPI_ENG_BUF_BLKS(eng_buf[engine &
> DPI_ENGINE_MASK]);
> > + dpi_reg_write(dpi, DPI_ENGX_BUF(engine), reg);
> > +
> > + if (cfg.update_molr) {
> > + reg = DPI_DMA_ENG_EN_MOLR(cfg.molr[engine &
> DPI_ENGINE_MASK]);
> > + dpi_reg_write(dpi, DPI_DMA_ENGX_EN(engine),
> reg);
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static long dpi_dev_ioctl(struct file *fptr, unsigned int cmd,
> > +unsigned long data) {
> > + void __user *arg = (void __user *)data;
> > + struct dpipf *dpi;
> > + int ret = -EINVAL;
>
> Wrong error code for an invalid ioctl command :(
ack
>
> > +
> > + dpi = container_of(fptr->private_data, struct dpipf, miscdev);
> > +
> > + switch (cmd) {
> > + case DPI_MPS_MRRS_CFG:
> > + ret = dpi_mps_mrrs_config(dpi, arg);
> > + break;
> > + case DPI_ENGINE_CFG:
> > + ret = dpi_engine_config(dpi, arg);
> > + break;
> > + default:
> > + break;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static const struct file_operations dpi_device_fops = {
> > + .owner = THIS_MODULE,
> > + .unlocked_ioctl = dpi_dev_ioctl,
> > + .compat_ioctl = compat_ptr_ioctl,
> > +};
> > +
> > +static int dpi_probe(struct pci_dev *pdev, const struct pci_device_id
> > +*id) {
> > + struct device *dev = &pdev->dev;
> > + struct dpipf *dpi;
> > + int ret;
> > +
> > + dpi = devm_kzalloc(dev, sizeof(*dpi), GFP_KERNEL);
> > + if (!dpi)
> > + return -ENOMEM;
> > +
> > + dpi->pdev = pdev;
> > +
> > + ret = pcim_enable_device(pdev);
> > + if (ret) {
> > + dev_err(dev, "DPI: Failed to enable PCI device\n");
> > + return ret;
> > + }
> > +
> > + ret = pcim_iomap_regions(pdev, BIT(0) | BIT(4), DPI_DRV_NAME);
> > + if (ret) {
> > + dev_err(dev, "DPI: Failed to request MMIO region\n");
> > + return ret;
> > + }
> > +
> > + dpi->reg_base = pcim_iomap_table(pdev)[PCI_DPI_CFG_BAR];
> > +
> > + /* Initialize global PF registers */
> > + dpi_init(dpi);
> > +
> > + /* Setup PF-VF mailbox */
> > + ret = dpi_pfvf_mbox_setup(dpi);
> > + if (ret) {
> > + dev_err(dev, "DPI: Failed to setup pf-vf mbox\n");
> > + goto err_dpi_fini;
> > + }
> > +
> > + /* Register interrupts */
> > + ret = dpi_irq_init(dpi);
> > + if (ret) {
> > + dev_err(dev, "DPI: Failed to initialize irq vectors\n");
> > + goto err_dpi_mbox_free;
> > + }
> > +
> > + pci_set_drvdata(pdev, dpi);
> > + dpi->miscdev.minor = MISC_DYNAMIC_MINOR;
> > + dpi->miscdev.name = DPI_DRV_NAME;
> > + dpi->miscdev.fops = &dpi_device_fops;
> > + dpi->miscdev.parent = dev;
> > +
> > + ret = misc_register(&dpi->miscdev);
> > + if (ret) {
> > + dev_err(dev, "DPI: Failed to register misc device\n");
> > + goto err_dpi_mbox_free;
> > + }
> > +
> > + return 0;
> > +
> > +err_dpi_mbox_free:
> > + dpi_pfvf_mbox_destroy(dpi);
> > +err_dpi_fini:
> > + dpi_fini(dpi);
> > + return ret;
> > +}
> > +
> > +static void dpi_remove(struct pci_dev *pdev) {
> > + struct dpipf *dpi = pci_get_drvdata(pdev);
> > +
> > + misc_deregister(&dpi->miscdev);
> > + pci_sriov_configure_simple(pdev, 0);
> > + dpi_pfvf_mbox_destroy(dpi);
> > + dpi_fini(dpi);
> > + pci_set_drvdata(pdev, NULL);
> > +}
> > +
> > +static const struct pci_device_id dpi_id_table[] = {
> > + { PCI_DEVICE_SUB(PCI_VENDOR_ID_CAVIUM,
> PCI_DEVID_MRVL_CN10K_DPI_PF,
> > + PCI_VENDOR_ID_CAVIUM,
> PCI_SUBDEVID_MRVL_CN10K_DPI_PF) },
> > + { 0, } /* end of table */
> > +};
> > +
> > +static struct pci_driver dpi_driver = {
> > + .name = DPI_DRV_NAME,
> > + .id_table = dpi_id_table,
> > + .probe = dpi_probe,
> > + .remove = dpi_remove,
> > + .sriov_configure = pci_sriov_configure_simple, };
> > +
> > +module_pci_driver(dpi_driver);
> > +MODULE_DEVICE_TABLE(pci, dpi_id_table);
> MODULE_AUTHOR("Marvell.");
> > +MODULE_DESCRIPTION("Marvell Octeon CN10K DPI Driver");
> > +MODULE_LICENSE("GPL");
> > diff --git a/include/uapi/misc/mrvl_cn10k_dpi.h
> > b/include/uapi/misc/mrvl_cn10k_dpi.h
> > new file mode 100644
> > index 000000000000..a1951644448a
> > --- /dev/null
> > +++ b/include/uapi/misc/mrvl_cn10k_dpi.h
> > @@ -0,0 +1,37 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
>
> Why is this file "GPL-2.0+" but your driver is "GPL-2.0"? Is that what your
> lawyers want to have happen (sorry, I have to ask.)

No specific reason, I think I need to use " GPL-2.0-or-later" instead of
"GPL-2.0+", will fix it.

Thanks
Vamsi

>
> thanks,
>
> greg k-h

On Wed, May 01, 2024 at 07:46:55AM +0000, Vamsi Krishna Attunuru wrote:
> > > --- /dev/null
> > > +++ b/include/uapi/misc/mrvl_cn10k_dpi.h
> > > @@ -0,0 +1,37 @@
> > > +/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
> >
> > Why is this file "GPL-2.0+" but your driver is "GPL-2.0"? Is that what your
> > lawyers want to have happen (sorry, I have to ask.)
>
> No specific reason, I think I need to use " GPL-2.0-or-later" instead of
> "GPL-2.0+", will fix it.

No, that's not the problem here at all, either of those are the same.
The problem is that it does not match with the C file. Please work with
your legal department to choose what license you want for the files and
be consistent.

thanks,

greg k-h