RE: [EXTERNAL] Re: [PATCH v5 1/1] misc: mrvl-cn10k-dpi: add Octeon CN10K DPI administrative driver

[Vamsi Krishna Attunuru]

> -----Original Message-----
> From: Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx>
> Sent: Saturday, April 13, 2024 4:55 PM
> To: Vamsi Krishna Attunuru <vattunuru@xxxxxxxxxxx>
> Cc: arnd@xxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Jerin Jacob
> <jerinj@xxxxxxxxxxx>
> Subject: Re: [EXTERNAL] Re: [PATCH v5 1/1] misc: mrvl-cn10k-dpi: add
> Octeon CN10K DPI administrative driver
> 
> On Sat, Apr 13, 2024 at 10:58:37AM +0000, Vamsi Krishna Attunuru wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx>
> > > Sent: Saturday, April 13, 2024 11:18 AM
> > > To: Vamsi Krishna Attunuru <vattunuru@xxxxxxxxxxx>
> > > Cc: arnd@xxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Jerin Jacob
> > > <jerinj@xxxxxxxxxxx>
> > > Subject: Re: [EXTERNAL] Re: [PATCH v5 1/1] misc: mrvl-cn10k-dpi: add
> > > Octeon CN10K DPI administrative driver
> > >
> > > On Fri, Apr 12, 2024 at 04:19:58PM +0000, Vamsi Krishna Attunuru wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx>
> > > > > Sent: Friday, April 12, 2024 9:05 PM
> > > > > To: Vamsi Krishna Attunuru <vattunuru@xxxxxxxxxxx>
> > > > > Cc: arnd@xxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> > > > > Subject: Re: [EXTERNAL] Re: [PATCH v5 1/1] misc: mrvl-cn10k-dpi:
> > > > > add Octeon CN10K DPI administrative driver
> > > > >
> > > > > On Fri, Apr 12, 2024 at 01:56:36PM +0000, Vamsi Krishna Attunuru
> wrote:
> > > > > >
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx>
> > > > > > > Sent: Friday, April 12, 2024 5:57 PM
> > > > > > > To: Vamsi Krishna Attunuru <vattunuru@xxxxxxxxxxx>
> > > > > > > Cc: arnd@xxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> > > > > > > Subject: [EXTERNAL] Re: [PATCH v5 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 12, 2024 at 05:10:05AM -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.
> > > > > > >
> > > > > > > No pointer to the userspace code that uses this?  Why not?
> > > > > > > How are we supposed to be able to review this?
> > > > > >
> > > > > > Userspace code will use two functionalities (mailbox & ioctl)
> > > > > > from this driver. DPDK DMA driver uses the mailbox and the
> > > > > > dpdk application uses the ioctl to setup the device
> > > > > > attributes. We are waiting for this kernel driver get merged
> > > > > > to update the corresponding support in DPDK
> > > > > driver and applications. I will provide the pointers to both the
> > > > > use cases in userspace code.
> > > > > > Meanwhile below is the current dpdk dma driver that uses sysfs
> > > > > > based scheme to convey mbox requests to the kernel DPI driver
> > > > > > which gets
> > > > > replaced with hardware mailbox scheme once mrvl-cn10k-dpi kernel
> > > > > driver is merged.
> > > > > > https://urldefense.proofpoint.com/v2/url?u=https-
> > > > > 3A__github.com_DPDK_d
> > > > > > pdk_blob_main_drivers_common_cnxk_roc-
> > > > > 5Fdpi.c&d=DwIBAg&c=nKjWec2b6R0mO
> > > > > >
> > > > >
> > >
> yPaz7xtfQ&r=WllrYaumVkxaWjgKto6E_rtDQshhIhik2jkvzFyRhW8&m=o3EhoL
> > > > > s7dsod
> > > > > > -YHS438Wl2Pf_MKMBYegGSKteoX3qFTB0HV897ykpCVbTp-
> > > > > nmj4e&s=A6TJDFUtPm3ksJh
> > > > > > qop89CL8GgKj4sjkJIVi1-RdnUr8&e=
> > > > >
> > > > > So this is a DPDK thing?  Ugh, do the networking people know about
> this?
> > > > > If not, why aren't they reviewing this?
> > > >
> > > > Actually, It's not networking related. Like the Linux kernel, DPDK
> > > > also supports multiple subsystems like network, scheduler, DMA,
> > > > mempool etc. Regarding the usecases, the DPDK Marvell DMA/DPI VF
> > > > driver interacts(over hardware mailbox) with the mrvl-cn10k-dpi
> > > > misc
> > > kernel driver(administrative driver) for setting up the VF device
> resources.
> > >
> > > So this is something that the PCI core should be concerned about then?
> >
> > No, it's a normal PCIe sriov capability implemented in all sriov capable PCIe
> devices.
> > Our PF device aka this driver in kernel space service mailbox requests
> > from userspace applications via VF devices. For instance, DPI VF
> > device from user space writes into mailbox registers and the DPI hardware
> triggers an interrupt to DPI PF device.
> > Upon PF interrupt, this driver services the mailbox requests.
> 
> Isn't that a "normal" PCI thing?  How is this different from other devices that
> have VF?

Looks like there is a lot of confusion for this device. Let me explain
There are two aspects for this DPI PF device.
a) It's a PCIe device so it is "using" some of the PCI services provided PCIe HW or PCI subsystem
b) It is "providing" non PCIe service(DPI HW administrative function) by using (a)
Let me enumerate PF device operations with above aspects.
1) Means to create VF(s) from PF. It's category (a) service and driver uses API (pci_sriov_configure_simple()) from PCI subsystem to implement it.
2) Means to get the interrupt(mailbox or any device specific interrupt). It's category (a) service and driver uses API (pci_alloc_irq_vectors()) from PCI subsystem to implement it.
3) Means to get the mailbox content from VF by using (2). It's category (b) service. This service is not part of PCI specification.
DPI PF device has the mailbox registers(DPI_MBOX_PF_VF_DATA registers) in its PCIe BAR space which are device specific.
4) Upon receiving DPI HW administrative function mailbox request, service it. Its category (b) service. This service is not part of PCI specification.
For instance, dpi_queue_open & close are requests sent from DPI VF device to DPI PF device for setting up the DPI VF queue resources. Once its setup by DPI PF,
then DPI VF device can use these queues. These queues are not part of PCIe specification. These queues are used for making DMA by DPI VF device/driver.

> 
> > > > DPDK is one example that uses this driver, there can be other
> > > > userspace generic frameworks/applications where the virtual
> > > > functions are binded to userspace drivers and interact with
> > > > physical/administrative
> > > function driver running in the kernel.
> > >
> > > Are there other devices/drivers that do this today in Linux?  Why
> > > make a device-specific api for this common functionality?
> >
> > The apis defined in this driver are specific to Marvell DPI hardware.
> 
> The api, yes, but that's the point, shouldn't this be generic for all hardware
> that supports this?  Implementation should be device specific, in the driver.

No. As mentioned above, it is not generic for all devices. (3) and (4) are specific to Marvell DPI PF device/driver. (1) and (2) are common for all the PCIe PF
device, which, this driver is using APIs from PCI subsystem.

> 
> > For instance,
> > the variables molr(max outstanding load requests), fifo_max, ebus_port
> > are DPI hardware specific. Generally, drivers use driver-specific api
> > to configure any device-specific configuration which does not fit in common
> functionality right.
> >
> > Mailbox operations like dpi_queue_open & close are requests sent from
> > VF device to PF device for setting up the VF queue resources.
> 
> Why is an ioctl to a random character device the correct api to userspace for
> this type of thing?  Shouldn't this go through the PCI layer api instead?

No. Because it is not part of PCIe specification. PCI specification operations are controlled through PCI config space and extended config space.
These are some DPI device global configuration operations(ex: DPI_EBUS_PORTX_CFG, DPI_ENGX_BUF cfg) which are NOT part of PCI config space or extended config space.
There by, it does have any role in PCI layer API.

> 
> thanks,
> 
> greg k-h