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;
IronPort-SDR: Q6Ocy3OexFOcbWgPmG2vpNbaztXZcyY7xNmqJCfN0rgjvXYkQfey6xgBLhu+ndry7C5ymTMHqN
 H85j/170Noow==
IronPort-SDR: yPokJM+edFhLSi1AUvgzIz+eO5yACVbz2b6DGVcrsSrBJ+Dxg3u5brK9ZD8LH8RzjzAtyCew8X
 XQx4RzKdcfoA==
From:   "Wu, Hao" <hao.wu@intel.com>
To:     "matthew.gerlach@linux.intel.com" <matthew.gerlach@linux.intel.com>
CC:     "trix@redhat.com" <trix@redhat.com>,
        "linux-fpga@vger.kernel.org" <linux-fpga@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "Xu, Yilun" <yilun.xu@intel.com>,
        "Weight, Russell H" <russell.h.weight@intel.com>,
        "mdf@kernel.org" <mdf@kernel.org>
Subject: RE: [PATCH] fpga: dfl: pci: gracefully handle misconfigured port
 entries
Thread-Topic: [PATCH] fpga: dfl: pci: gracefully handle misconfigured port
 entries
Thread-Index: AQHXNgpK6sZ5wt8PgU2xWx05XVpyOqq+btGQgADGhICABuPGYA==
Date:   Mon, 26 Apr 2021 02:39:34 +0000
Message-ID: <DM6PR11MB3819A3CFA58B11427E813A2F85429@DM6PR11MB3819.namprd11.prod.outlook.com>
References: <20210420172740.707259-1-matthew.gerlach@linux.intel.com>
 <BYAPR11MB381637F0EFFAFD57902EE47985479@BYAPR11MB3816.namprd11.prod.outlook.com>
 <alpine.DEB.2.22.394.2104211001080.760078@rhweight-WRK1>
In-Reply-To: <alpine.DEB.2.22.394.2104211001080.760078@rhweight-WRK1>
Accept-Language: en-US
Content-Language: en-US
dlp-product: dlpe-windows
dlp-reaction: no-action
dlp-version: 11.5.1.3
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 15de05d0-02e2-40fb-aa50-08d9085c8723
X-MS-Exchange-CrossTenant-originalarrivaltime: 26 Apr 2021 02:39:34.6031
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: VBn/7CRklkeNK1wtKzaplbRRVDYxvyGV5YOKcaZQJDHl3qwwly+HCR2+XygXW4S1PmKcaZqX+e7dMDz8nL7Nyw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR11MB4674
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> On Wed, 21 Apr 2021, Wu, Hao wrote:
>=20
> >> Subject: [PATCH] fpga: dfl: pci: gracefully handle misconfigured port =
entries
> >>
> >> From: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> >>
> >> Gracefully ignore misconfigured port entries encountered in
> >> incorrect FPGA images.
> >>
> >> Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> >> ---
> >>  drivers/fpga/dfl-pci.c | 16 +++++++++++++++-
> >>  1 file changed, 15 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c
> >> index b44523e..660d3b6 100644
> >> --- a/drivers/fpga/dfl-pci.c
> >> +++ b/drivers/fpga/dfl-pci.c
> >> @@ -212,6 +212,7 @@ static int find_dfls_by_default(struct pci_dev *pc=
idev,
> >>  	int port_num, bar, i, ret =3D 0;
> >>  	resource_size_t start, len;
> >>  	void __iomem *base;
> >> +	int bars =3D 0;
> >>  	u32 offset;
> >>  	u64 v;
> >>
> >> @@ -228,6 +229,7 @@ static int find_dfls_by_default(struct pci_dev *pc=
idev,
> >>  	if (dfl_feature_is_fme(base)) {
> >>  		start =3D pci_resource_start(pcidev, 0);
> >>  		len =3D pci_resource_len(pcidev, 0);
> >> +		bars |=3D BIT(0);
> >>
> >>  		dfl_fpga_enum_info_add_dfl(info, start, len);
> >>
> >> @@ -253,9 +255,21 @@ static int find_dfls_by_default(struct pci_dev
> *pcidev,
> >>  			 */
> >>  			bar =3D FIELD_GET(FME_PORT_OFST_BAR_ID, v);
> >>  			offset =3D FIELD_GET(FME_PORT_OFST_DFH_OFST, v);
> >> +			if (bars & BIT(bar)) {
> >> +				dev_warn(&pcidev->dev, "skipping bad port
> >> BAR %d\n", bar);
> >> +				continue;
> >> +			}
> >
> > Will it be a real problem that multiple ports are inside one BAR but di=
fferent
> offsets?
> >
> > Hao
>=20
> I don't think multiple ports within a single BAR is something that has
> been supported in the past.  The genesis for this patch was a
> misconfigured port entry pointing to BAR0.  BAR0 had already been mapped
> for the FME and remapping BAR0 failed resulting in enumeration failure.

could some products put the port in BAR0? Or multiple ports in one BAR.
As we consider this is a common driver can be reused and maintained for
a long time, so I hope that we don't put limitation setup only for some=20
specific products.

Hao

>=20
> Matthew
>=20
> >
> >> +
> >>  			start =3D pci_resource_start(pcidev, bar) + offset;
> >> -			len =3D pci_resource_len(pcidev, bar) - offset;
> >> +			len =3D pci_resource_len(pcidev, bar);
> >> +			if (offset >=3D len) {
> >> +				dev_warn(&pcidev->dev, "bad port
> >> offset %u >=3D %pa\n",
> >> +					 offset, &len);
> >> +				continue;
> >> +			}
> >>
> >> +			len -=3D offset;
> >> +			bars |=3D BIT(bar);
> >>  			dfl_fpga_enum_info_add_dfl(info, start, len);
> >>  		}
> >>  	} else if (dfl_feature_is_port(base)) {
> >> --
> >> 1.8.3.1
> >
> >