Received-SPF: pass (google.com: domain of linux-kernel+bounces-45073-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Tue, 30 Jan 2024 10:00:16 -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: <ZbjHl8ptQG5FdHvC@yilunxu-OptiPlex-7050>
Message-ID: <alpine.DEB.2.22.394.2401300948590.112016@sj-4150-psse-sw-opae-dev2>
References: <20240125233715.861883-1-matthew.gerlach@linux.intel.com> <ZbjHl8ptQG5FdHvC@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 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.

Do you want me to update the commit message with this information?

Thanks,
Matthew

>
> Thanks,
> Yilun
>
>> transaction layer packet (TLP) transactions and invalid data
>> being returned.
>>
>> Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
>>
>> ---
>> v2:
>> - Update commit message for clarity
>> - Remove potentially confusing dev_info message.
>> ---
>>  drivers/fpga/dfl-afu-main.c | 10 +++++++++-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
>> index c0a75ca360d6..42fe27660ab7 100644
>> --- a/drivers/fpga/dfl-afu-main.c
>> +++ b/drivers/fpga/dfl-afu-main.c
>> @@ -417,7 +417,15 @@ static const struct attribute_group port_hdr_group = {
>>  static int port_hdr_init(struct platform_device *pdev,
>>  			 struct dfl_feature *feature)
>>  {
>> -	port_reset(pdev);
>> +	void __iomem *base;
>> +	u8 rev;
>> +
>> +	base = dfl_get_feature_ioaddr_by_id(&pdev->dev, PORT_FEATURE_ID_HEADER);
>> +
>> +	rev = dfl_feature_revision(base);
>> +
>> +	if (rev < 2)
>> +		port_reset(pdev);
>>
>>  	return 0;
>>  }
>> --
>> 2.34.1
>>
>>
>