Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Received-SPF: Pass (protection.outlook.com: domain of nvidia.com designates
 216.228.112.34 as permitted sender) receiver=protection.outlook.com;
 client-ip=216.228.112.34; helo=mail.nvidia.com;
Subject: Re: Device driver location for the PCIe root port's DMA engine
To:     Bjorn Helgaas <helgaas@kernel.org>,
        Matthew Wilcox <willy@infradead.org>
CC:     <bhelgaas@google.com>, <lorenzo.pieralisi@arm.com>,
        <amurray@thegoodpenguin.co.uk>, <robh@kernel.org>,
        <jingoohan1@gmail.com>, <gustavo.pimentel@synopsys.com>,
        Krishna Thota <kthota@nvidia.com>,
        "Manikanta Maddireddy" <mmaddireddy@nvidia.com>,
        Thierry Reding <treding@nvidia.com>,
        Jonathan Hunter <jonathanh@nvidia.com>,
        <linux-pci@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <linux-tegra@vger.kernel.org>
References: <20210412215333.GA2144302@bjorn-Precision-5520>
From:   Vidya Sagar <vidyas@nvidia.com>
Message-ID: <5fe57865-41f8-67c5-e6a6-9abe7252591c@nvidia.com>
Date:   Tue, 13 Apr 2021 23:42:15 +0530
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101
 Thunderbird/78.8.0
MIME-Version: 1.0
In-Reply-To: <20210412215333.GA2144302@bjorn-Precision-5520>
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Apr 2021 18:12:23.9514
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 1ba32f84-0bb7-4303-2226-08d8fea7b01e
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=43083d15-7273-40c1-b7db-39efd9ccc17a;Ip=[216.228.112.34];Helo=[mail.nvidia.com]
X-MS-Exchange-CrossTenant-AuthSource: DM6NAM11FT021.eop-nam11.prod.protection.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR12MB1684
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


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
>