Re: Device driver location for the PCIe root port's DMA engine

[Vidya Sagar <vidyas@nvidia.com>]

On 4/13/2021 3:23 AM, Bjorn Helgaas wrote:
> External email: Use caution opening links or attachments
> 
> 
> [+cc Matthew for portdrv comment]
> 
> On Mon, Apr 12, 2021 at 10:31:02PM +0530, Vidya Sagar wrote:
>> Hi
>> I'm starting this mail to seek advice on the best approach to be taken to
>> add support for the driver of the PCIe root port's DMA engine.
>> To give some background, Tegra194's PCIe IPs are dual-mode PCIe IPs i.e.
>> they work either in the root port mode or in the endpoint mode based on the
>> boot time configuration.
>> Since the PCIe hardware IP as such is the same for both (RP and EP) modes,
>> the DMA engine sub-system of the PCIe IP is also made available to both
>> modes of operation.
>> Typically, the DMA engine is seen only in the endpoint mode, and that DMA
>> engine’s configuration registers are made available to the host through one
>> of its BARs.
>> In the situation that we have here, where there is a DMA engine present as
>> part of the root port, the DMA engine isn’t a typical general-purpose DMA
>> engine in the sense that it can’t have both source and destination addresses
>> targeting external memory addresses.
>> RP’s DMA engine, while doing a write operation,
>> would always fetch data (i.e. source) from local memory and write it to the
>> remote memory over PCIe link (i.e. destination would be the BAR of an
>> endpoint)
>> whereas while doing a read operation,
>> would always fetch/read data (i.e. source) from a remote memory over the
>> PCIe link and write it to the local memory.
>>
>> I see that there are at least two ways we can have a driver for this DMA
>> engine.
>> a) DMA engine driver as one of the port service drivers
>>        Since the DMA engine is a part of the root port hardware itself (although
>> it is not part of the standard capabilities of the root port), it is one of
>> the options to have the driver for the DMA engine go as one of the port
>> service drivers (along with AER, PME, hot-plug, etc...). Based on Vendor-ID
>> and Device-ID matching runtime, either it gets binded/enabled (like in the
>> case of Tegra194) or it doesn't.
>> b) DMA engine driver as a platform driver
>>        The DMA engine hardware can be described as a sub-node under the PCIe
>> controller's node in the device tree and a separate platform driver can be
>> written to work with it.
>>
>> I’m inclined to have the DMA engine driver as a port service driver as it
>> makes it cleaner and also in line with the design philosophy (the way I
>> understood it) of the port service drivers.
>> Please let me know your thoughts on this.
> 
> Personally I'm not a fan of the port service driver model.  It creates
> additional struct devices for things that are not separate devices.
> And it creates a parallel hierarchy in /sys/bus/pci_express/devices/
> that I think does not accurately model the hardware.
Agree.

> 
> The existing port services (AER, DPC, hotplug, etc) are things the
> device advertises via the PCI Capabilities defined by the generic PCIe
> spec, and in my opinion the support for them should be directly part
> of the PCI core and activated when the relevant Capability is present.
Is there an on-going activity to remove port service drivers are move 
AER/DPC/Hotplug etc.. handling within PCI core?

> 
> The DMA engine is different -- this is device-specific functionality
> and I think the obvious way to discover it and bind a driver to it is
> via the PCI Vendor and Device ID.
> 
> This *is* complicated by the fact that you can't just use
> pci_register_driver() to claim functionality implemented in Root Ports
> or Switch Ports because portdrv binds to them before you have a
> chance.  I think that's a defect in the portdrv design.  The usual
Yes. Hence I was thinking of adding a service driver for the DMA 
functionality

> workaround is to use pci_get_device(), which has its own issues (it's
> ugly, it's outside the normal driver binding model, doesn't work
> nicely with hotplug or udev, doesn't coordinate with other drivers
> using the same device, etc).  There are many examples of this in the
> EDAC code.
I didn't think of approaching this issue in this way. Thanks for the 
pointers to EDAC code. I'll wait for comments from Matthew before I 
proceed with this approach.

> 
> Bjorn
>