Received-SPF: pass (google.com: domain of linux-kernel+bounces-44337-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Tue, 30 Jan 2024 17:55:35 +0800
From: Xu Yilun <yilun.xu@linux.intel.com>
To: Matthew Gerlach <matthew.gerlach@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
Message-ID: <ZbjHl8ptQG5FdHvC@yilunxu-OptiPlex-7050>
References: <20240125233715.861883-1-matthew.gerlach@linux.intel.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240125233715.861883-1-matthew.gerlach@linux.intel.com>

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().

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
> 
>