RE: [PATCH] fpga: dfl: pci: gracefully handle misconfigured port entries

[Wu, Hao]

> On Wed, 21 Apr 2021, Wu, Hao wrote:
> 
> >> Subject: [PATCH] fpga: dfl: pci: gracefully handle misconfigured port entries
> >>
> >> From: Matthew Gerlach <matthew.gerlach@xxxxxxxxxxxxxxx>
> >>
> >> Gracefully ignore misconfigured port entries encountered in
> >> incorrect FPGA images.
> >>
> >> Signed-off-by: Matthew Gerlach <matthew.gerlach@xxxxxxxxxxxxxxx>
> >> ---
> >>  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 *pcidev,
> >>  	int port_num, bar, i, ret = 0;
> >>  	resource_size_t start, len;
> >>  	void __iomem *base;
> >> +	int bars = 0;
> >>  	u32 offset;
> >>  	u64 v;
> >>
> >> @@ -228,6 +229,7 @@ static int find_dfls_by_default(struct pci_dev *pcidev,
> >>  	if (dfl_feature_is_fme(base)) {
> >>  		start = pci_resource_start(pcidev, 0);
> >>  		len = pci_resource_len(pcidev, 0);
> >> +		bars |= 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 = FIELD_GET(FME_PORT_OFST_BAR_ID, v);
> >>  			offset = 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 different
> offsets?
> >
> > Hao
> 
> 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 
specific products.

Hao

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