* [PATCH v2 0/2] fpga: dfl: optional VSEC for start of dfl @ 2020-11-18 19:01 matthew.gerlach 2020-11-18 19:01 ` [PATCH v2 1/2] fpga: dfl: refactor cci_enumerate_feature_devs() matthew.gerlach 2020-11-18 19:01 ` [PATCH v2 2/2] fpga: dfl: look for vendor specific capability matthew.gerlach 0 siblings, 2 replies; 5+ messages in thread From: matthew.gerlach @ 2020-11-18 19:01 UTC (permalink / raw) To: linux-fpga, linux-kernel, mdf, hao.wu, trix, linux-doc, corbet Cc: Matthew Gerlach From: Matthew Gerlach <matthew.gerlach@linux.intel.com> The start of a Device Feature List (DFL) is currently assumed to be at Bar0/Offset 0 on the PCIe bus by drivers/fpga/dfl-pci.c. This patchset adds support for the start one or more DFLs to be specified in a Vendor-Specific Capability (VSEC) structure in PCIe config space. If no such VSEC structure exists, then the start is assumed to be Bar0/Offset 0 for backward compatibility. Matthew Gerlach (2): fpga: dfl: refactor cci_enumerate_feature_devs() fpga: dfl: look for vendor specific capability Documentation/fpga/dfl.rst | 13 +++ drivers/fpga/dfl-pci.c | 163 +++++++++++++++++++++++++++++-------- 2 files changed, 141 insertions(+), 35 deletions(-) -- 2.25.2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] fpga: dfl: refactor cci_enumerate_feature_devs() 2020-11-18 19:01 [PATCH v2 0/2] fpga: dfl: optional VSEC for start of dfl matthew.gerlach @ 2020-11-18 19:01 ` matthew.gerlach 2020-11-18 19:01 ` [PATCH v2 2/2] fpga: dfl: look for vendor specific capability matthew.gerlach 1 sibling, 0 replies; 5+ messages in thread From: matthew.gerlach @ 2020-11-18 19:01 UTC (permalink / raw) To: linux-fpga, linux-kernel, mdf, hao.wu, trix, linux-doc, corbet Cc: Matthew Gerlach From: Matthew Gerlach <matthew.gerlach@linux.intel.com> In preparation of looking for dfls based on a vendor specific pcie capability, move code that assumes Bar0/offset0 as start of DFL to its own function. Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com> --- v2: remove spurious blank lines rename find_dfl_in_bar0 to find_dfls_by_default --- drivers/fpga/dfl-pci.c | 84 +++++++++++++++++++++++------------------- 1 file changed, 47 insertions(+), 37 deletions(-) diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c index a2203d03c9e2..b27fae045536 100644 --- a/drivers/fpga/dfl-pci.c +++ b/drivers/fpga/dfl-pci.c @@ -119,49 +119,20 @@ static int *cci_pci_create_irq_table(struct pci_dev *pcidev, unsigned int nvec) return table; } -/* enumerate feature devices under pci device */ -static int cci_enumerate_feature_devs(struct pci_dev *pcidev) +static int find_dfls_by_default(struct pci_dev *pcidev, + struct dfl_fpga_enum_info *info) { - struct cci_drvdata *drvdata = pci_get_drvdata(pcidev); - int port_num, bar, i, nvec, ret = 0; - struct dfl_fpga_enum_info *info; - struct dfl_fpga_cdev *cdev; resource_size_t start, len; + int port_num, bar, i; void __iomem *base; - int *irq_table; + int ret = 0; u32 offset; u64 v; - /* allocate enumeration info via pci_dev */ - info = dfl_fpga_enum_info_alloc(&pcidev->dev); - if (!info) - return -ENOMEM; - - /* add irq info for enumeration if the device support irq */ - nvec = cci_pci_alloc_irq(pcidev); - if (nvec < 0) { - dev_err(&pcidev->dev, "Fail to alloc irq %d.\n", nvec); - ret = nvec; - goto enum_info_free_exit; - } else if (nvec) { - irq_table = cci_pci_create_irq_table(pcidev, nvec); - if (!irq_table) { - ret = -ENOMEM; - goto irq_free_exit; - } - - ret = dfl_fpga_enum_info_add_irq(info, nvec, irq_table); - kfree(irq_table); - if (ret) - goto irq_free_exit; - } - - /* start to find Device Feature List in Bar 0 */ + /* start to find Device Feature List from Bar 0 */ base = cci_pci_ioremap_bar0(pcidev); - if (!base) { - ret = -ENOMEM; - goto irq_free_exit; - } + if (!base) + return -ENOMEM; /* * PF device has FME and Ports/AFUs, and VF device only has one @@ -208,12 +179,51 @@ static int cci_enumerate_feature_devs(struct pci_dev *pcidev) dfl_fpga_enum_info_add_dfl(info, start, len); } else { ret = -ENODEV; - goto irq_free_exit; } /* release I/O mappings for next step enumeration */ pcim_iounmap_regions(pcidev, BIT(0)); + return ret; +} + +/* enumerate feature devices under pci device */ +static int cci_enumerate_feature_devs(struct pci_dev *pcidev) +{ + struct cci_drvdata *drvdata = pci_get_drvdata(pcidev); + struct dfl_fpga_enum_info *info; + struct dfl_fpga_cdev *cdev; + int nvec, ret = 0; + int *irq_table; + + /* allocate enumeration info via pci_dev */ + info = dfl_fpga_enum_info_alloc(&pcidev->dev); + if (!info) + return -ENOMEM; + + /* add irq info for enumeration if the device support irq */ + nvec = cci_pci_alloc_irq(pcidev); + if (nvec < 0) { + dev_err(&pcidev->dev, "Fail to alloc irq %d.\n", nvec); + ret = nvec; + goto enum_info_free_exit; + } else if (nvec) { + irq_table = cci_pci_create_irq_table(pcidev, nvec); + if (!irq_table) { + ret = -ENOMEM; + goto irq_free_exit; + } + + ret = dfl_fpga_enum_info_add_irq(info, nvec, irq_table); + kfree(irq_table); + if (ret) + goto irq_free_exit; + } + + ret = find_dfls_by_default(pcidev, info); + if (ret) + goto irq_free_exit; + /* start enumeration with prepared enumeration information */ cdev = dfl_fpga_feature_devs_enumerate(info); if (IS_ERR(cdev)) { -- 2.25.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] fpga: dfl: look for vendor specific capability 2020-11-18 19:01 [PATCH v2 0/2] fpga: dfl: optional VSEC for start of dfl matthew.gerlach 2020-11-18 19:01 ` [PATCH v2 1/2] fpga: dfl: refactor cci_enumerate_feature_devs() matthew.gerlach @ 2020-11-18 19:01 ` matthew.gerlach 2020-11-22 1:18 ` Moritz Fischer 1 sibling, 1 reply; 5+ messages in thread From: matthew.gerlach @ 2020-11-18 19:01 UTC (permalink / raw) To: linux-fpga, linux-kernel, mdf, hao.wu, trix, linux-doc, corbet Cc: Matthew Gerlach From: Matthew Gerlach <matthew.gerlach@linux.intel.com> A DFL may not begin at offset 0 of BAR 0. A PCIe vendor specific capability can be used to specify the start of a number of DFLs. Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com> --- v2: Update documentation for clarity. Clean up macro names. Use GENMASK. Removed spurious blank lines. Changed some calls from dev_info to dev_dbg. Specifically check for VSEC not found, -ENODEV. Ensure correct pci vendor id. Remove check for page alignment. Rename find_dfl_in_cfg to find_dfls_by_vsec. Initialize target memory of pci_read_config_dword to invalid values before use. --- Documentation/fpga/dfl.rst | 13 ++++++ drivers/fpga/dfl-pci.c | 87 +++++++++++++++++++++++++++++++++++++- 2 files changed, 98 insertions(+), 2 deletions(-) diff --git a/Documentation/fpga/dfl.rst b/Documentation/fpga/dfl.rst index 0404fe6ffc74..37016ff35a90 100644 --- a/Documentation/fpga/dfl.rst +++ b/Documentation/fpga/dfl.rst @@ -501,6 +501,19 @@ Developer only needs to provide a sub feature driver with matched feature id. FME Partial Reconfiguration Sub Feature driver (see drivers/fpga/dfl-fme-pr.c) could be a reference. +Location of DFLs on PCI Device +=========================== +The start of the first DFL is assumed to be offset 0 of bar 0. +If the first node of the DFL is an FME, then further DFLs +in the port(s) are specified in FME header registers. +Alternatively, a vendor specific capability structure can be used to +specify the location of all the DFLs on the device, providing flexibility +for the type of starting node in the DFL. Intel has reserved the +VSEC ID of 0x43 for this purpose. The vendor specific +data begins with a 4 byte vendor specific register for the number of DFLs followed 4 byte +Offset/BIR vendor specific registers for each DFL. Bits 2:0 of Offset/BIR register +indicates the BAR, and bits 31:3 form the 8 byte aligned offset where bits 2:0 are +zero. Open discussion =============== diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c index b27fae045536..3a6807e3e10c 100644 --- a/drivers/fpga/dfl-pci.c +++ b/drivers/fpga/dfl-pci.c @@ -27,6 +27,14 @@ #define DRV_VERSION "0.8" #define DRV_NAME "dfl-pci" +#define PCI_VSEC_ID_INTEL_DFLS 0x43 + +#define PCI_VNDR_DFLS_CNT 8 +#define PCI_VNDR_DFLS_RES 0x0c + +#define PCI_VNDR_DFLS_RES_BAR_MASK GENMASK(2, 0) +#define PCI_VNDR_DFLS_RES_OFF_MASK GENMASK(31, 3) + struct cci_drvdata { struct dfl_fpga_cdev *cdev; /* container device */ }; @@ -119,8 +127,80 @@ static int *cci_pci_create_irq_table(struct pci_dev *pcidev, unsigned int nvec) return table; } +static int find_dfls_by_vsec(struct pci_dev *pcidev, struct dfl_fpga_enum_info *info) +{ + u32 bar, offset, vndr_hdr, dfl_cnt, dfl_res; + int dfl_res_off, i, voff = 0; + resource_size_t start, len; + + if (pcidev->vendor != PCI_VENDOR_ID_INTEL) + return -ENODEV; + + while ((voff = pci_find_next_ext_capability(pcidev, voff, PCI_EXT_CAP_ID_VNDR))) { + vndr_hdr = 0; + pci_read_config_dword(pcidev, voff + PCI_VNDR_HEADER, &vndr_hdr); + + dev_dbg(&pcidev->dev, + "vendor-specific capability id 0x%x, rev 0x%x len 0x%x\n", + PCI_VNDR_HEADER_ID(vndr_hdr), + PCI_VNDR_HEADER_REV(vndr_hdr), + PCI_VNDR_HEADER_LEN(vndr_hdr)); + + if (PCI_VNDR_HEADER_ID(vndr_hdr) == PCI_VSEC_ID_INTEL_DFLS) + break; + } + + if (!voff) { + dev_dbg(&pcidev->dev, "%s no VSEC found\n", __func__); + return -ENODEV; + } + + dfl_cnt = 0; + pci_read_config_dword(pcidev, voff + PCI_VNDR_DFLS_CNT, &dfl_cnt); + dev_dbg(&pcidev->dev, "dfl_cnt %d\n", dfl_cnt); + for (i = 0; i < dfl_cnt; i++) { + dfl_res_off = voff + PCI_VNDR_DFLS_RES + + (i * sizeof(dfl_res)); + dfl_res = GENMASK(31, 0); + pci_read_config_dword(pcidev, dfl_res_off, &dfl_res); + + dev_dbg(&pcidev->dev, "dfl_res 0x%x\n", dfl_res); + + bar = dfl_res & PCI_VNDR_DFLS_RES_BAR_MASK; + if (bar >= PCI_STD_NUM_BARS) { + dev_err(&pcidev->dev, "%s bad bar number %d\n", + __func__, bar); + return -EINVAL; + } + + len = pci_resource_len(pcidev, bar); + if (len == 0) { + dev_err(&pcidev->dev, "%s unmapped bar number %d\n", + __func__, bar); + return -EINVAL; + } + + offset = dfl_res & PCI_VNDR_DFLS_RES_OFF_MASK; + if (offset >= len) { + dev_err(&pcidev->dev, "%s bad offset %u >= %pa\n", + __func__, offset, &len); + return -EINVAL; + } + + dev_dbg(&pcidev->dev, "%s BAR %d offset 0x%x\n", __func__, bar, offset); + + len -= offset; + + start = pci_resource_start(pcidev, bar) + offset; + + dfl_fpga_enum_info_add_dfl(info, start, len); + } + + return 0; +} + static int find_dfls_by_default(struct pci_dev *pcidev, - struct dfl_fpga_enum_info *info) + struct dfl_fpga_enum_info *info) { resource_size_t start, len; int port_num, bar, i; @@ -220,7 +300,10 @@ static int cci_enumerate_feature_devs(struct pci_dev *pcidev) goto irq_free_exit; } - ret = find_dfls_by_default(pcidev, info); + ret = find_dfls_by_vsec(pcidev, info); + if (ret == -ENODEV) + ret = find_dfls_by_default(pcidev, info); + if (ret) goto irq_free_exit; -- 2.25.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] fpga: dfl: look for vendor specific capability 2020-11-18 19:01 ` [PATCH v2 2/2] fpga: dfl: look for vendor specific capability matthew.gerlach @ 2020-11-22 1:18 ` Moritz Fischer 2020-11-23 17:24 ` matthew.gerlach 0 siblings, 1 reply; 5+ messages in thread From: Moritz Fischer @ 2020-11-22 1:18 UTC (permalink / raw) To: matthew.gerlach Cc: linux-fpga, linux-kernel, mdf, hao.wu, trix, linux-doc, corbet Hi Matthew, On Wed, Nov 18, 2020 at 11:01:51AM -0800, matthew.gerlach@linux.intel.com wrote: > From: Matthew Gerlach <matthew.gerlach@linux.intel.com> > > A DFL may not begin at offset 0 of BAR 0. A PCIe vendor > specific capability can be used to specify the start of a > number of DFLs. > > Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com> > --- > v2: Update documentation for clarity. > Clean up macro names. > Use GENMASK. > Removed spurious blank lines. > Changed some calls from dev_info to dev_dbg. > Specifically check for VSEC not found, -ENODEV. > Ensure correct pci vendor id. > Remove check for page alignment. > Rename find_dfl_in_cfg to find_dfls_by_vsec. > Initialize target memory of pci_read_config_dword to invalid values before use. > --- > Documentation/fpga/dfl.rst | 13 ++++++ > drivers/fpga/dfl-pci.c | 87 +++++++++++++++++++++++++++++++++++++- > 2 files changed, 98 insertions(+), 2 deletions(-) > > diff --git a/Documentation/fpga/dfl.rst b/Documentation/fpga/dfl.rst > index 0404fe6ffc74..37016ff35a90 100644 > --- a/Documentation/fpga/dfl.rst > +++ b/Documentation/fpga/dfl.rst > @@ -501,6 +501,19 @@ Developer only needs to provide a sub feature driver with matched feature id. > FME Partial Reconfiguration Sub Feature driver (see drivers/fpga/dfl-fme-pr.c) > could be a reference. > > +Location of DFLs on PCI Device > +=========================== Maybe start with: "There are two ways of locating the DFLs" > +The start of the first DFL is assumed to be offset 0 of bar 0. > +If the first node of the DFL is an FME, then further DFLs > +in the port(s) are specified in FME header registers. > +Alternatively, a vendor specific capability structure can be used to > +specify the location of all the DFLs on the device, providing flexibility > +for the type of starting node in the DFL. Intel has reserved the > +VSEC ID of 0x43 for this purpose. The vendor specific > +data begins with a 4 byte vendor specific register for the number of DFLs followed 4 byte > +Offset/BIR vendor specific registers for each DFL. Bits 2:0 of Offset/BIR register > +indicates the BAR, and bits 31:3 form the 8 byte aligned offset where bits 2:0 are > +zero. It's nice to have details, thanks! Nit: This could be a table maybe? > > Open discussion > =============== > diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c > index b27fae045536..3a6807e3e10c 100644 > --- a/drivers/fpga/dfl-pci.c > +++ b/drivers/fpga/dfl-pci.c > @@ -27,6 +27,14 @@ > #define DRV_VERSION "0.8" > #define DRV_NAME "dfl-pci" > > +#define PCI_VSEC_ID_INTEL_DFLS 0x43 > + > +#define PCI_VNDR_DFLS_CNT 8 > +#define PCI_VNDR_DFLS_RES 0x0c > + > +#define PCI_VNDR_DFLS_RES_BAR_MASK GENMASK(2, 0) > +#define PCI_VNDR_DFLS_RES_OFF_MASK GENMASK(31, 3) > + > struct cci_drvdata { > struct dfl_fpga_cdev *cdev; /* container device */ > }; > @@ -119,8 +127,80 @@ static int *cci_pci_create_irq_table(struct pci_dev *pcidev, unsigned int nvec) > return table; > } > > +static int find_dfls_by_vsec(struct pci_dev *pcidev, struct dfl_fpga_enum_info *info) > +{ > + u32 bar, offset, vndr_hdr, dfl_cnt, dfl_res; > + int dfl_res_off, i, voff = 0; > + resource_size_t start, len; > + > + if (pcidev->vendor != PCI_VENDOR_ID_INTEL) > + return -ENODEV; > + > + while ((voff = pci_find_next_ext_capability(pcidev, voff, PCI_EXT_CAP_ID_VNDR))) { > + vndr_hdr = 0; > + pci_read_config_dword(pcidev, voff + PCI_VNDR_HEADER, &vndr_hdr); Are there concerns around those failing? > + > + dev_dbg(&pcidev->dev, > + "vendor-specific capability id 0x%x, rev 0x%x len 0x%x\n", > + PCI_VNDR_HEADER_ID(vndr_hdr), > + PCI_VNDR_HEADER_REV(vndr_hdr), > + PCI_VNDR_HEADER_LEN(vndr_hdr)); > + > + if (PCI_VNDR_HEADER_ID(vndr_hdr) == PCI_VSEC_ID_INTEL_DFLS) > + break; > + } > + > + if (!voff) { > + dev_dbg(&pcidev->dev, "%s no VSEC found\n", __func__); > + return -ENODEV; > + } > + > + dfl_cnt = 0; > + pci_read_config_dword(pcidev, voff + PCI_VNDR_DFLS_CNT, &dfl_cnt); I guess this could fall on it's face if you'd read back ~0 ... maybe I'm being too paranoid :) > + dev_dbg(&pcidev->dev, "dfl_cnt %d\n", dfl_cnt); > + for (i = 0; i < dfl_cnt; i++) { > + dfl_res_off = voff + PCI_VNDR_DFLS_RES + > + (i * sizeof(dfl_res)); > + dfl_res = GENMASK(31, 0); > + pci_read_config_dword(pcidev, dfl_res_off, &dfl_res); > + > + dev_dbg(&pcidev->dev, "dfl_res 0x%x\n", dfl_res); > + > + bar = dfl_res & PCI_VNDR_DFLS_RES_BAR_MASK; > + if (bar >= PCI_STD_NUM_BARS) { > + dev_err(&pcidev->dev, "%s bad bar number %d\n", > + __func__, bar); > + return -EINVAL; > + } > + > + len = pci_resource_len(pcidev, bar); > + if (len == 0) { > + dev_err(&pcidev->dev, "%s unmapped bar number %d\n", > + __func__, bar); > + return -EINVAL; > + } > + > + offset = dfl_res & PCI_VNDR_DFLS_RES_OFF_MASK; > + if (offset >= len) { > + dev_err(&pcidev->dev, "%s bad offset %u >= %pa\n", > + __func__, offset, &len); > + return -EINVAL; > + } > + > + dev_dbg(&pcidev->dev, "%s BAR %d offset 0x%x\n", __func__, bar, offset); > + > + len -= offset; > + > + start = pci_resource_start(pcidev, bar) + offset; > + > + dfl_fpga_enum_info_add_dfl(info, start, len); > + } > + > + return 0; > +} > + > static int find_dfls_by_default(struct pci_dev *pcidev, > - struct dfl_fpga_enum_info *info) > + struct dfl_fpga_enum_info *info) > { > resource_size_t start, len; > int port_num, bar, i; > @@ -220,7 +300,10 @@ static int cci_enumerate_feature_devs(struct pci_dev *pcidev) > goto irq_free_exit; > } > > - ret = find_dfls_by_default(pcidev, info); > + ret = find_dfls_by_vsec(pcidev, info); > + if (ret == -ENODEV) > + ret = find_dfls_by_default(pcidev, info); > + > if (ret) > goto irq_free_exit; > > -- > 2.25.2 > Thanks, Moritz ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] fpga: dfl: look for vendor specific capability 2020-11-22 1:18 ` Moritz Fischer @ 2020-11-23 17:24 ` matthew.gerlach 0 siblings, 0 replies; 5+ messages in thread From: matthew.gerlach @ 2020-11-23 17:24 UTC (permalink / raw) To: Moritz Fischer; +Cc: linux-fpga, linux-kernel, hao.wu, trix, linux-doc, corbet On Sat, 21 Nov 2020, Moritz Fischer wrote: > Hi Matthew, > > On Wed, Nov 18, 2020 at 11:01:51AM -0800, matthew.gerlach@linux.intel.com wrote: >> From: Matthew Gerlach <matthew.gerlach@linux.intel.com> >> >> A DFL may not begin at offset 0 of BAR 0. A PCIe vendor >> specific capability can be used to specify the start of a >> number of DFLs. >> >> Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com> >> --- >> v2: Update documentation for clarity. >> Clean up macro names. >> Use GENMASK. >> Removed spurious blank lines. >> Changed some calls from dev_info to dev_dbg. >> Specifically check for VSEC not found, -ENODEV. >> Ensure correct pci vendor id. >> Remove check for page alignment. >> Rename find_dfl_in_cfg to find_dfls_by_vsec. >> Initialize target memory of pci_read_config_dword to invalid values before use. >> --- >> Documentation/fpga/dfl.rst | 13 ++++++ >> drivers/fpga/dfl-pci.c | 87 +++++++++++++++++++++++++++++++++++++- >> 2 files changed, 98 insertions(+), 2 deletions(-) >> >> diff --git a/Documentation/fpga/dfl.rst b/Documentation/fpga/dfl.rst >> index 0404fe6ffc74..37016ff35a90 100644 >> --- a/Documentation/fpga/dfl.rst >> +++ b/Documentation/fpga/dfl.rst >> @@ -501,6 +501,19 @@ Developer only needs to provide a sub feature driver with matched feature id. >> FME Partial Reconfiguration Sub Feature driver (see drivers/fpga/dfl-fme-pr.c) >> could be a reference. >> >> +Location of DFLs on PCI Device >> +=========================== > > Maybe start with: "There are two ways of locating the DFLs" This seems to be a reasonable suggestion. I will update the text. >> +The start of the first DFL is assumed to be offset 0 of bar 0. >> +If the first node of the DFL is an FME, then further DFLs >> +in the port(s) are specified in FME header registers. >> +Alternatively, a vendor specific capability structure can be used to >> +specify the location of all the DFLs on the device, providing flexibility >> +for the type of starting node in the DFL. Intel has reserved the >> +VSEC ID of 0x43 for this purpose. The vendor specific >> +data begins with a 4 byte vendor specific register for the number of DFLs followed 4 byte >> +Offset/BIR vendor specific registers for each DFL. Bits 2:0 of Offset/BIR register >> +indicates the BAR, and bits 31:3 form the 8 byte aligned offset where bits 2:0 are >> +zero. > It's nice to have details, thanks! Nit: This could be a table maybe? I will look into some sort of table if it looks helpful. >> >> Open discussion >> =============== >> diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c >> index b27fae045536..3a6807e3e10c 100644 >> --- a/drivers/fpga/dfl-pci.c >> +++ b/drivers/fpga/dfl-pci.c >> @@ -27,6 +27,14 @@ >> #define DRV_VERSION "0.8" >> #define DRV_NAME "dfl-pci" >> >> +#define PCI_VSEC_ID_INTEL_DFLS 0x43 >> + >> +#define PCI_VNDR_DFLS_CNT 8 >> +#define PCI_VNDR_DFLS_RES 0x0c >> + >> +#define PCI_VNDR_DFLS_RES_BAR_MASK GENMASK(2, 0) >> +#define PCI_VNDR_DFLS_RES_OFF_MASK GENMASK(31, 3) >> + >> struct cci_drvdata { >> struct dfl_fpga_cdev *cdev; /* container device */ >> }; >> @@ -119,8 +127,80 @@ static int *cci_pci_create_irq_table(struct pci_dev *pcidev, unsigned int nvec) >> return table; >> } >> >> +static int find_dfls_by_vsec(struct pci_dev *pcidev, struct dfl_fpga_enum_info *info) >> +{ >> + u32 bar, offset, vndr_hdr, dfl_cnt, dfl_res; >> + int dfl_res_off, i, voff = 0; >> + resource_size_t start, len; >> + >> + if (pcidev->vendor != PCI_VENDOR_ID_INTEL) >> + return -ENODEV; >> + >> + while ((voff = pci_find_next_ext_capability(pcidev, voff, PCI_EXT_CAP_ID_VNDR))) { >> + vndr_hdr = 0; >> + pci_read_config_dword(pcidev, voff + PCI_VNDR_HEADER, &vndr_hdr); > Are there concerns around those failing? The intent is that failing pci_read_config would be handled by initializing the target to an invalid value. >> + >> + dev_dbg(&pcidev->dev, >> + "vendor-specific capability id 0x%x, rev 0x%x len 0x%x\n", >> + PCI_VNDR_HEADER_ID(vndr_hdr), >> + PCI_VNDR_HEADER_REV(vndr_hdr), >> + PCI_VNDR_HEADER_LEN(vndr_hdr)); >> + >> + if (PCI_VNDR_HEADER_ID(vndr_hdr) == PCI_VSEC_ID_INTEL_DFLS) >> + break; >> + } >> + >> + if (!voff) { >> + dev_dbg(&pcidev->dev, "%s no VSEC found\n", __func__); >> + return -ENODEV; >> + } >> + >> + dfl_cnt = 0; >> + pci_read_config_dword(pcidev, voff + PCI_VNDR_DFLS_CNT, &dfl_cnt); > I guess this could fall on it's face if you'd read back ~0 ... maybe > I'm being too paranoid :) I don't think you are being too paranoid. I can add a check to make sure dfl_res_off never outgrows the size of PCIe config space. >> + dev_dbg(&pcidev->dev, "dfl_cnt %d\n", dfl_cnt); >> + for (i = 0; i < dfl_cnt; i++) { >> + dfl_res_off = voff + PCI_VNDR_DFLS_RES + >> + (i * sizeof(dfl_res)); >> + dfl_res = GENMASK(31, 0); >> + pci_read_config_dword(pcidev, dfl_res_off, &dfl_res); >> + >> + dev_dbg(&pcidev->dev, "dfl_res 0x%x\n", dfl_res); >> + >> + bar = dfl_res & PCI_VNDR_DFLS_RES_BAR_MASK; >> + if (bar >= PCI_STD_NUM_BARS) { >> + dev_err(&pcidev->dev, "%s bad bar number %d\n", >> + __func__, bar); >> + return -EINVAL; >> + } >> + >> + len = pci_resource_len(pcidev, bar); >> + if (len == 0) { >> + dev_err(&pcidev->dev, "%s unmapped bar number %d\n", >> + __func__, bar); >> + return -EINVAL; >> + } >> + >> + offset = dfl_res & PCI_VNDR_DFLS_RES_OFF_MASK; >> + if (offset >= len) { >> + dev_err(&pcidev->dev, "%s bad offset %u >= %pa\n", >> + __func__, offset, &len); >> + return -EINVAL; >> + } >> + >> + dev_dbg(&pcidev->dev, "%s BAR %d offset 0x%x\n", __func__, bar, offset); >> + >> + len -= offset; >> + >> + start = pci_resource_start(pcidev, bar) + offset; >> + >> + dfl_fpga_enum_info_add_dfl(info, start, len); >> + } >> + >> + return 0; >> +} >> + >> static int find_dfls_by_default(struct pci_dev *pcidev, >> - struct dfl_fpga_enum_info *info) >> + struct dfl_fpga_enum_info *info) >> { >> resource_size_t start, len; >> int port_num, bar, i; >> @@ -220,7 +300,10 @@ static int cci_enumerate_feature_devs(struct pci_dev *pcidev) >> goto irq_free_exit; >> } >> >> - ret = find_dfls_by_default(pcidev, info); >> + ret = find_dfls_by_vsec(pcidev, info); >> + if (ret == -ENODEV) >> + ret = find_dfls_by_default(pcidev, info); >> + >> if (ret) >> goto irq_free_exit; >> >> -- >> 2.25.2 >> > > Thanks, > Moritz > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-11-23 17:23 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-18 19:01 [PATCH v2 0/2] fpga: dfl: optional VSEC for start of dfl matthew.gerlach 2020-11-18 19:01 ` [PATCH v2 1/2] fpga: dfl: refactor cci_enumerate_feature_devs() matthew.gerlach 2020-11-18 19:01 ` [PATCH v2 2/2] fpga: dfl: look for vendor specific capability matthew.gerlach 2020-11-22 1:18 ` Moritz Fischer 2020-11-23 17:24 ` matthew.gerlach
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.