| Message ID | 20220301062123.818687-5-tianfei.zhang@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add OFS support for DFL driver | expand |
> -----Original Message----- > From: Zhang, Tianfei <tianfei.zhang@intel.com> > Sent: Tuesday, March 1, 2022 2:21 PM > To: Wu, Hao <hao.wu@intel.com>; trix@redhat.com; mdf@kernel.org; Xu, Yilun > <yilun.xu@intel.com>; linux-fpga@vger.kernel.org; linux-doc@vger.kernel.org > Cc: linux-kernel@vger.kernel.org; corbet@lwn.net; Matthew Gerlach > <matthew.gerlach@linux.intel.com>; Zhang, Tianfei <tianfei.zhang@intel.com> > Subject: [PATCH v3 4/5] fpga: dfl: Handle dfl's starting with AFU > > From: Matthew Gerlach <matthew.gerlach@linux.intel.com> > > Allow for a Device Feature List (DFL) to start with > a Device Feature Header (DFH) of type Accelerator Function Unit (AFU) > by doing nothing. This allows for PCIe VFs to be created. Why this is related to VFs creation? We don't have AFU in PF in OFS case, right? > > Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com> > Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com> > --- > drivers/fpga/dfl-pci.c | 7 ++++++- > drivers/fpga/dfl.c | 22 +++++++++++++--------- > 2 files changed, 19 insertions(+), 10 deletions(-) > > diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c > index 33545c999c06..e7d58e7b1bbd 100644 > --- a/drivers/fpga/dfl-pci.c > +++ b/drivers/fpga/dfl-pci.c > @@ -275,7 +275,12 @@ static int find_dfls_by_default(struct pci_dev *pcidev, > > dfl_fpga_enum_info_add_dfl(info, start, len); > } else { Can be something like else if dfl_feature_is_afu(base) following the same style. > - ret = -ENODEV; > + v = readq(base + DFH); > + if (FIELD_GET(DFH_TYPE, v) != DFH_TYPE_AFU) { > + dev_info(&pcidev->dev, "Unknown feature type 0x%llx > id 0x%llx\n", > + FIELD_GET(DFH_TYPE, v), FIELD_GET(DFH_ID, > v)); > + ret = -ENODEV; > + } But nothing else done for AFU so far? How it works? Sounds like more patches are required. > } > > /* release I/O mappings for next step enumeration */ > diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c > index fd04ef5c8b03..e30bbb3039cd 100644 > --- a/drivers/fpga/dfl.c > +++ b/drivers/fpga/dfl.c > @@ -900,9 +900,11 @@ static void build_info_free(struct > build_feature_devs_info *binfo) > dfl_id_free(feature_dev_id_type(binfo->feature_dev), > binfo->feature_dev->id); > > - list_for_each_entry_safe(finfo, p, &binfo->sub_features, node) > { > - list_del(&finfo->node); > - kfree(finfo); > + if (!list_empty(&binfo->sub_features)) { > + list_for_each_entry_safe(finfo, p, &binfo- > >sub_features, node) { > + list_del(&finfo->node); > + kfree(finfo); > + } > } > } > > @@ -1439,12 +1441,14 @@ dfl_fpga_feature_devs_enumerate(struct > dfl_fpga_enum_info *info) > * start enumeration for all feature devices based on Device Feature > * Lists. > */ > - list_for_each_entry(dfl, &info->dfls, node) { > - ret = parse_feature_list(binfo, dfl->start, dfl->len); > - if (ret) { > - remove_feature_devs(cdev); > - build_info_free(binfo); > - goto unregister_region_exit; > + if (!list_empty(&info->dfls)) { > + list_for_each_entry(dfl, &info->dfls, node) { > + ret = parse_feature_list(binfo, dfl->start, dfl->len); > + if (ret) { > + remove_feature_devs(cdev); > + build_info_free(binfo); > + goto unregister_region_exit; > + } > } > } > > -- > 2.26.2
> -----Original Message----- > From: Wu, Hao <hao.wu@intel.com> > Sent: Tuesday, March 1, 2022 3:28 PM > To: Zhang, Tianfei <tianfei.zhang@intel.com>; trix@redhat.com; > mdf@kernel.org; Xu, Yilun <yilun.xu@intel.com>; linux-fpga@vger.kernel.org; > linux-doc@vger.kernel.org > Cc: linux-kernel@vger.kernel.org; corbet@lwn.net; Matthew Gerlach > <matthew.gerlach@linux.intel.com> > Subject: RE: [PATCH v3 4/5] fpga: dfl: Handle dfl's starting with AFU > > > -----Original Message----- > > From: Zhang, Tianfei <tianfei.zhang@intel.com> > > Sent: Tuesday, March 1, 2022 2:21 PM > > To: Wu, Hao <hao.wu@intel.com>; trix@redhat.com; mdf@kernel.org; Xu, > > Yilun <yilun.xu@intel.com>; linux-fpga@vger.kernel.org; > > linux-doc@vger.kernel.org > > Cc: linux-kernel@vger.kernel.org; corbet@lwn.net; Matthew Gerlach > > <matthew.gerlach@linux.intel.com>; Zhang, Tianfei > > <tianfei.zhang@intel.com> > > Subject: [PATCH v3 4/5] fpga: dfl: Handle dfl's starting with AFU > > > > From: Matthew Gerlach <matthew.gerlach@linux.intel.com> > > > > Allow for a Device Feature List (DFL) to start with a Device Feature > > Header (DFH) of type Accelerator Function Unit (AFU) by doing nothing. > > This allows for PCIe VFs to be created. > > Why this is related to VFs creation? We don't have AFU in PF in OFS case, right? Yes, in " Multiple VFs per PR slot" model, the AFU was not connected with Port. > > > > > Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com> > > Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com> > > --- > > drivers/fpga/dfl-pci.c | 7 ++++++- > > drivers/fpga/dfl.c | 22 +++++++++++++--------- > > 2 files changed, 19 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c index > > 33545c999c06..e7d58e7b1bbd 100644 > > --- a/drivers/fpga/dfl-pci.c > > +++ b/drivers/fpga/dfl-pci.c > > @@ -275,7 +275,12 @@ static int find_dfls_by_default(struct pci_dev > > *pcidev, > > > > dfl_fpga_enum_info_add_dfl(info, start, len); > > } else { > > Can be something like else if dfl_feature_is_afu(base) following the same style. dfl_feature_is_afu(base) will be better. > > > - ret = -ENODEV; > > + v = readq(base + DFH); > > + if (FIELD_GET(DFH_TYPE, v) != DFH_TYPE_AFU) { > > + dev_info(&pcidev->dev, "Unknown feature type 0x%llx > > id 0x%llx\n", > > + FIELD_GET(DFH_TYPE, v), FIELD_GET(DFH_ID, > > v)); > > + ret = -ENODEV; > > + } > > But nothing else done for AFU so far? How it works? Sounds like more patches > are required. In " Multiple VFs per PR slot" model, we access the AFU resource via VFs for example VFIO-PIC driver, and we cannot access the AFU MMIO resource by AFU APIs. > > > } > > > > /* release I/O mappings for next step enumeration */ diff --git > > a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c index > > fd04ef5c8b03..e30bbb3039cd 100644 > > --- a/drivers/fpga/dfl.c > > +++ b/drivers/fpga/dfl.c > > @@ -900,9 +900,11 @@ static void build_info_free(struct > > build_feature_devs_info *binfo) > > dfl_id_free(feature_dev_id_type(binfo->feature_dev), > > binfo->feature_dev->id); > > > > - list_for_each_entry_safe(finfo, p, &binfo->sub_features, node) > > { > > - list_del(&finfo->node); > > - kfree(finfo); > > + if (!list_empty(&binfo->sub_features)) { > > + list_for_each_entry_safe(finfo, p, &binfo- > > >sub_features, node) { > > + list_del(&finfo->node); > > + kfree(finfo); > > + } > > } > > } > > > > @@ -1439,12 +1441,14 @@ dfl_fpga_feature_devs_enumerate(struct > > dfl_fpga_enum_info *info) > > * start enumeration for all feature devices based on Device Feature > > * Lists. > > */ > > - list_for_each_entry(dfl, &info->dfls, node) { > > - ret = parse_feature_list(binfo, dfl->start, dfl->len); > > - if (ret) { > > - remove_feature_devs(cdev); > > - build_info_free(binfo); > > - goto unregister_region_exit; > > + if (!list_empty(&info->dfls)) { > > + list_for_each_entry(dfl, &info->dfls, node) { > > + ret = parse_feature_list(binfo, dfl->start, dfl->len); > > + if (ret) { > > + remove_feature_devs(cdev); > > + build_info_free(binfo); > > + goto unregister_region_exit; > > + } > > } > > } > > > > -- > > 2.26.2
diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c index 33545c999c06..e7d58e7b1bbd 100644 --- a/drivers/fpga/dfl-pci.c +++ b/drivers/fpga/dfl-pci.c @@ -275,7 +275,12 @@ static int find_dfls_by_default(struct pci_dev *pcidev, dfl_fpga_enum_info_add_dfl(info, start, len); } else { - ret = -ENODEV; + v = readq(base + DFH); + if (FIELD_GET(DFH_TYPE, v) != DFH_TYPE_AFU) { + dev_info(&pcidev->dev, "Unknown feature type 0x%llx id 0x%llx\n", + FIELD_GET(DFH_TYPE, v), FIELD_GET(DFH_ID, v)); + ret = -ENODEV; + } } /* release I/O mappings for next step enumeration */ diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c index fd04ef5c8b03..e30bbb3039cd 100644 --- a/drivers/fpga/dfl.c +++ b/drivers/fpga/dfl.c @@ -900,9 +900,11 @@ static void build_info_free(struct build_feature_devs_info *binfo) dfl_id_free(feature_dev_id_type(binfo->feature_dev), binfo->feature_dev->id); - list_for_each_entry_safe(finfo, p, &binfo->sub_features, node) { - list_del(&finfo->node); - kfree(finfo); + if (!list_empty(&binfo->sub_features)) { + list_for_each_entry_safe(finfo, p, &binfo->sub_features, node) { + list_del(&finfo->node); + kfree(finfo); + } } } @@ -1439,12 +1441,14 @@ dfl_fpga_feature_devs_enumerate(struct dfl_fpga_enum_info *info) * start enumeration for all feature devices based on Device Feature * Lists. */ - list_for_each_entry(dfl, &info->dfls, node) { - ret = parse_feature_list(binfo, dfl->start, dfl->len); - if (ret) { - remove_feature_devs(cdev); - build_info_free(binfo); - goto unregister_region_exit; + if (!list_empty(&info->dfls)) { + list_for_each_entry(dfl, &info->dfls, node) { + ret = parse_feature_list(binfo, dfl->start, dfl->len); + if (ret) { + remove_feature_devs(cdev); + build_info_free(binfo); + goto unregister_region_exit; + } } }