lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <DM6PR11MB381900B67DE65EEBF936CA6885E20@DM6PR11MB3819.namprd11.prod.outlook.com>
Date:   Tue, 17 Nov 2020 10:00:22 +0000
From:   "Wu, Hao" <hao.wu@...el.com>
To:     "Xu, Yilun" <yilun.xu@...el.com>,
        "matthew.gerlach@...ux.intel.com" <matthew.gerlach@...ux.intel.com>
CC:     "linux-fpga@...r.kernel.org" <linux-fpga@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "mdf@...nel.org" <mdf@...nel.org>,
        "trix@...hat.com" <trix@...hat.com>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "corbet@....net" <corbet@....net>, "Xu, Yilun" <yilun.xu@...el.com>
Subject: RE: [PATCH 2/2] fpga: dfl: look for vendor specific capability

> > +
> > +		start = pci_resource_start(pcidev, bar) + offset;
> > +		len -= offset;
> 
> With these code, I have the following assumption:
> 
> 1. There is only one DFL in one bar, multiple DFLs requires multiple
> bars.
> 
> 2. The DFL region is from the "offset" to the end of the bar.

I think we should not have such kind of limitation, but at least it 
requires user clearly from spec, to make sure no overlap case could
happen. We all know that BAR number is small, but we won't limit
DFL numbers by BAR number here.

> 
> Are they correct? If yes maybe we should specify them clearly in Doc.
> 
> > +
> > +		if (!PAGE_ALIGNED(start)) {
> > +			dev_err(&pcidev->dev, "%s unaliged start 0x%llx\n",
> > +				__func__, start);
> > +			return -EINVAL;
> > +		}
> > +
> > +		dfl_fpga_enum_info_add_dfl(info, start, len);
> 
> Do we need some region overlapping check in this func? So we could find
> the HW problem (e.g. same bar num for multiple DFLs) in early stage.

Not sure if VSEC can add a length field for this purpose, otherwise overlapping
check only can be done after enumeration (walk the DFL to know the end).

> 
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  static int find_dfl_in_bar0(struct pci_dev *pcidev,
> >  			    struct dfl_fpga_enum_info *info)
> >  {
> > @@ -221,7 +304,10 @@ static int cci_enumerate_feature_devs(struct
> pci_dev *pcidev)
> >  			goto irq_free_exit;
> >  	}
> >
> > -	ret = find_dfl_in_bar0(pcidev, info);
> > +	ret = find_dfl_in_cfg(pcidev, info);
> > +
> > +	if (ret)
> > +		ret = find_dfl_in_bar0(pcidev, info);
> 
> The patch is more than the relocation support for DFL. Actually it
> introduced a different way of DFL finding.
> 
> Previously it starts at bar0 offset 0, find dfl fme first, then find
> dfl port according to fme header registers. Now it enumerates every DFL
> by PCIe VSEC.

Yes, the name is a little confusing, maybe we can rename them.

find_dfls_by_default or find_dfls - to handle original cases.
find_dfls_by_vsec - to handle vsec case.

Thanks
Hao

> 
> Maybe we should add more description about the change and why.
> 
> Thanks,
> Yilun
> 
> >
> >  	if (ret)
> >  		goto irq_free_exit;
> > --
> > 2.25.2

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ