Received-SPF: pass (google.com: domain of linux-kernel+bounces-56785-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Wed, 7 Feb 2024 08:40:55 -0800 (PST)
From: matthew.gerlach@linux.intel.com
To: Xu Yilun <yilun.xu@linux.intel.com>
cc: hao.wu@intel.com, trix@redhat.com, mdf@kernel.org, yilun.xu@intel.com, 
    linux-fpga@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] fpga: dfl: afu: support Rev 2 of DFL Port feature
In-Reply-To: <ZcBIjcFJjGKf0qcO@yilunxu-OptiPlex-7050>
Message-ID: <alpine.DEB.2.22.394.2402051600190.122158@sj-4150-psse-sw-opae-dev2>
References: <20240125233715.861883-1-matthew.gerlach@linux.intel.com> <ZbjHl8ptQG5FdHvC@yilunxu-OptiPlex-7050> <alpine.DEB.2.22.394.2401300948590.112016@sj-4150-psse-sw-opae-dev2> <Zbnd8W1ciTKeoKc4@yilunxu-OptiPlex-7050> <alpine.DEB.2.22.394.2401311610020.112016@sj-4150-psse-sw-opae-dev2>
 <ZcBIjcFJjGKf0qcO@yilunxu-OptiPlex-7050>
User-Agent: Alpine 2.22 (DEB 394 2020-01-19)
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII; format=flowed


On Mon, 5 Feb 2024, Xu Yilun wrote:

> On Wed, Jan 31, 2024 at 04:26:27PM -0800, matthew.gerlach@linux.intel.com wrote:
>>
>>
>> On Wed, 31 Jan 2024, Xu Yilun wrote:
>>
>>> On Tue, Jan 30, 2024 at 10:00:16AM -0800, matthew.gerlach@linux.intel.com wrote:
>>>>
>>>>
>>>> On Tue, 30 Jan 2024, Xu Yilun wrote:
>>>>
>>>>> On Thu, Jan 25, 2024 at 03:37:15PM -0800, Matthew Gerlach wrote:
>>>>>> Revision 2 of the Device Feature List (DFL) Port feature
>>>>>> adds support for connecting the contents of the port to
>>>>>> multiple PCIe Physical Functions (PF).
>>>>>>
>>>>>> This new functionality requires changing the port reset
>>>>>> behavior during FPGA and software initialization from
>>>>>> revision 1 of the port feature. With revision 1, the initial
>>>>>> state of the logic inside the port was not guaranteed to
>>>>>> be valid until a port reset was performed by software during
>>>>>> driver initialization. With revision 2, the initial state
>>>>>> of the logic inside the port is guaranteed to be valid,
>>>>>> and a port reset is not required during driver initialization.
>>>>>>
>>>>>> This change in port reset behavior avoids a potential race
>>>>>> condition during PCI enumeration when a port is connected to
>>>>>> multiple PFs. Problems can occur if the driver attached to
>>>>>> the PF managing the port asserts reset in its probe function
>>>>>> when a driver attached to another PF accesses the port in its
>>>>>> own probe function. The potential problems include failed or hung
>>>>>
>>>>> Only racing during probe functions? I assume any time port_reset()
>>>>> would fail TLPs for the other PF. And port_reset() could be triggered
>>>>> at runtime by ioctl().
>>>>
>>>> Yes, a port_reset() triggered by ioctl could result in failed TLP for the
>>>> other PFs. The user space SW performing the ioctl needs to ensure all PFs
>>>> involved are properly quiesced before the port_reset is performed.
>>>
>>> How would user get an insight into other PF drivers to know everything
>>> is quiesced?  I mean do we need driver level management for this?
>>
>> Since this is an FPGA, the number of other PFs and the drivers bound to
>> those PFs depends on the FPGA image. There would also be user space software
>> stacks involved with the other PFs as well. The user would have to ensure
>> all the SW stacks and drivers are quiesced as appropriate for the FPGA
>
> User may not know everything about the device, they only get part of the
> controls that drivers grant. This is still true for vfio + userspace
> drivers.

A user performing a port reset would have to know the impact to the 
specific FPGA image being run in order to ensure all SW stacks are ready 
for the reset.

>
>> image. I don't think the driver performing the port_reset() can know all the
>
> Other PF drivers should know their own components. They should be aware
> that their devices are being reset.

The other PF drivers depend on the actual FPGA image being run.

>
>> components to be able to provide any meaningful management.
>
> If the reset provider and reset consumer are not in the same driver,
> they should interact with each other. IIRC, some reset controller class
> works for this purpose.

The other PFs in many cases can present as standard devices with existing 
drivers like virtio-net or virtio-blk. It does not seem desireable 
to have to change existing drivers for a particular FPGA implementation

Thanks,
Matthew
>
> Thanks,
> Yilun
>
>>
>> Thanks,
>> Matthew
>>
>>>
>>> Thanks,
>>> Yilun
>>>
>>>>
>>>> Do you want me to update the commit message with this information?
>>>>
>>>> Thanks,
>>>> Matthew
>>>
>>>
>
>