Messages in this thread | | | Subject | Re: [PATCH v1 4/7] fpga: dfl: fix VF creation when ports have no local BAR space | From | Tom Rix <> | Date | Fri, 18 Feb 2022 06:55:58 -0800 |
| |
On 2/18/22 12:14 AM, Zhang, Tianfei wrote: > >> -----Original Message----- >> From: Tom Rix <trix@redhat.com> >> Sent: Tuesday, February 15, 2022 11:51 PM >> To: Zhang, Tianfei <tianfei.zhang@intel.com>; Wu, Hao <hao.wu@intel.com>; >> mdf@kernel.org; Xu, Yilun <yilun.xu@intel.com>; linux-fpga@vger.kernel.org; >> linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org >> Cc: corbet@lwn.net; Matthew Gerlach <matthew.gerlach@linux.intel.com> >> Subject: Re: [PATCH v1 4/7] fpga: dfl: fix VF creation when ports have no local >> BAR space >> >> >> On 2/14/22 3:26 AM, Tianfei zhang wrote: >>> From: Matthew Gerlach <matthew.gerlach@linux.intel.com> >>> >>> When a port is not connected to the same PCIe endpoint as the FME, the >>> port does not need to be released before being virtualized. Fix VF >>> creation code to handle this new use >> Similar, how does this fit in with iofs, this looks like it would not be valid for the >> existing cards > IOFS introducing multiple methods for PR and AFU access. > 1. Legacy Model. > 2. Micro-Personas in AFU. > 3. Multiple VFs per PR slot. > > For 1 and 2 model, there are 1:1 mapping between Port device and PR slot (or entire AFU). In virtualization, > it should release the Port device firstly and then assign to VM. In this models, the DFL driver will track that > the number of port devices has released (cdev->released_port_num in dfl_fpga_cdev_config_ports_vf() function) > are equal with the numbers of SRIOV or not. But in model 3, it has multiple VFs per PR slot, and assign the VF to VM > without release the port device, so the tracking mechanism of cdev->released_port_num is not workable on new
If ->release_port_num is not workable, then it needs to be generalized.
Refactor to handle all the cases.
Tom
> model. This patch want to handle this new model during VF creation. > >>> case. >>> >>> Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com> >>> Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com> >>> --- >>> drivers/fpga/dfl.c | 11 +++++++++-- >>> 1 file changed, 9 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c index >>> 26f8cf890700..cfc539a656f0 100644 >>> --- a/drivers/fpga/dfl.c >>> +++ b/drivers/fpga/dfl.c >>> @@ -1705,15 +1705,22 @@ >> EXPORT_SYMBOL_GPL(dfl_fpga_cdev_config_ports_pf); >>> int dfl_fpga_cdev_config_ports_vf(struct dfl_fpga_cdev *cdev, int num_vfs) >>> { >>> struct dfl_feature_platform_data *pdata; >>> - int ret = 0; >>> + int ret = 0, port_count = 0; >>> >>> mutex_lock(&cdev->lock); >>> + >>> + list_for_each_entry(pdata, &cdev->port_dev_list, node) { >>> + if (pdata->dev) >> This looks wrong, >> >> pdata->dev is dereferenced below, if there is a case were (!pdata->dev) >> here there would be crash later. >> >>> + continue; >>> + port_count++; >> how does this work when only some of the ports are handled in the new way ? > This code want to handle the " Multiple VFs per PR slot" model as I mentioned above. > In new model, the port_count want to count that how many port devices have released. > This code looks not good readability, I try to re-write it. > >> Tom >> >>> + } >>> + >>> /* >>> * can't turn multiple ports into 1 VF device, only 1 port for 1 VF >>> * device, so if released port number doesn't match VF device number, >>> * then reject the request with -EINVAL error code. >>> */ >>> - if (cdev->released_port_num != num_vfs) { >>> + if (port_count && cdev->released_port_num != num_vfs) { >>> ret = -EINVAL; >>> goto done; >>> }
| |