diff mbox series

[v6,3/6] fpga: dfl: check released_port_num and num_vfs for legacy model

Message ID 20220316070814.1916017-4-tianfei.zhang@intel.com (mailing list archive)
State New
Headers show
Series Add OFS support for DFL driver | expand

Commit Message

Zhang, Tianfei March 16, 2022, 7:08 a.m. UTC
From: Tianfei zhang <tianfei.zhang@intel.com>

In OFS legacy model, there is 1:1 mapping for Port device and VF,
so it need to check the number of released port match the number of
VFs or not. But in "Multiple VFs per PR slot" model, there is 1:N
mapping for the Port device and VFs.

Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
---
 drivers/fpga/dfl.c | 10 ++++++----
 drivers/fpga/dfl.h |  2 ++
 2 files changed, 8 insertions(+), 4 deletions(-)

Comments

Wu, Hao March 17, 2022, 8:49 a.m. UTC | #1
> Subject: [PATCH v6 3/6] fpga: dfl: check released_port_num and num_vfs for
> legacy model
> 
> From: Tianfei zhang <tianfei.zhang@intel.com>
> 
> In OFS legacy model, there is 1:1 mapping for Port device and VF,
> so it need to check the number of released port match the number of
> VFs or not. But in "Multiple VFs per PR slot" model, there is 1:N
> mapping for the Port device and VFs.

The title and commit message seems not matching the code..
From code it sounds like we are trying to skip the PORT 
(PF access-> VF access) function, as new SRIOV usage model is introduced.
Probably we can skip it early in this function or even skip this function
directly. It doesn't matter it's 1:N or 1:1, we always want to keep PF
access to port, right?

> 
> Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
> ---
>  drivers/fpga/dfl.c | 10 ++++++----
>  drivers/fpga/dfl.h |  2 ++
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
> index 712c53363fda..b95b29c5c81d 100644
> --- a/drivers/fpga/dfl.c
> +++ b/drivers/fpga/dfl.c
> @@ -1707,11 +1707,13 @@ int dfl_fpga_cdev_config_ports_vf(struct
> dfl_fpga_cdev *cdev, int num_vfs)
> 
>  	mutex_lock(&cdev->lock);
>  	/*
> -	 * 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.
> +	 * In the OFS legacy model, it can't turn multiple ports into 1 VF
> +	 * device, because only 1 port conneced to 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 ((dfl_has_port_connected_afu(cdev) &&

Could we really use this as indication for which SRIOV model of hardware?

> +	     cdev->released_port_num != num_vfs)) {
>  		ret = -EINVAL;
>  		goto done;
>  	}
> diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
> index bc56b7e8c01b..83c2c50975e5 100644
> --- a/drivers/fpga/dfl.h
> +++ b/drivers/fpga/dfl.h
> @@ -471,6 +471,8 @@ void dfl_fpga_enum_info_free(struct
> dfl_fpga_enum_info *info);
>  #define DFL_PORT_CONNECT_BITS  MAX_DFL_FPGA_PORT_NUM
>  #define DFL_FEAT_PORT_CONNECT_MASK ((1UL <<
> (DFL_PORT_CONNECT_BITS)) - 1)
> 
> +#define dfl_has_port_connected_afu(cdev) ((cdev)->flags &
> DFL_FEAT_PORT_CONNECT_MASK)
> +
>  /**
>   * struct dfl_fpga_cdev - container device of DFL based FPGA
>   *
> --
> 2.26.2
Zhang, Tianfei March 17, 2022, 9:02 a.m. UTC | #2
> -----Original Message-----
> From: Wu, Hao <hao.wu@intel.com>
> Sent: Thursday, March 17, 2022 4:49 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; linux-kernel@vger.kernel.org;
> rdunlap@infradead.org
> Cc: corbet@lwn.net; Matthew Gerlach <matthew.gerlach@linux.intel.com>
> Subject: RE: [PATCH v6 3/6] fpga: dfl: check released_port_num and num_vfs for
> legacy model
> 
> > Subject: [PATCH v6 3/6] fpga: dfl: check released_port_num and num_vfs
> > for legacy model
> >
> > From: Tianfei zhang <tianfei.zhang@intel.com>
> >
> > In OFS legacy model, there is 1:1 mapping for Port device and VF, so
> > it need to check the number of released port match the number of VFs
> > or not. But in "Multiple VFs per PR slot" model, there is 1:N mapping
> > for the Port device and VFs.
> 
> The title and commit message seems not matching the code..
> From code it sounds like we are trying to skip the PORT (PF access-> VF access)
> function, as new SRIOV usage model is introduced.
> Probably we can skip it early in this function or even skip this function directly. It
> doesn't matter it's 1:N or 1:1, we always want to keep PF access to port, right?

For 1:1, there is 1 port mapping the 1 VFs, and we check the num_vfs was equal  with released ports or not.
But for 1:N, it break this checking.

> 
> >
> > Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> > Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
> > ---
> >  drivers/fpga/dfl.c | 10 ++++++----
> >  drivers/fpga/dfl.h |  2 ++
> >  2 files changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c index
> > 712c53363fda..b95b29c5c81d 100644
> > --- a/drivers/fpga/dfl.c
> > +++ b/drivers/fpga/dfl.c
> > @@ -1707,11 +1707,13 @@ int dfl_fpga_cdev_config_ports_vf(struct
> > dfl_fpga_cdev *cdev, int num_vfs)
> >
> >  	mutex_lock(&cdev->lock);
> >  	/*
> > -	 * 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.
> > +	 * In the OFS legacy model, it can't turn multiple ports into 1 VF
> > +	 * device, because only 1 port conneced to 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 ((dfl_has_port_connected_afu(cdev) &&
> 
> Could we really use this as indication for which SRIOV model of hardware?
> 
> > +	     cdev->released_port_num != num_vfs)) {
> >  		ret = -EINVAL;
> >  		goto done;
> >  	}
> > diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h index
> > bc56b7e8c01b..83c2c50975e5 100644
> > --- a/drivers/fpga/dfl.h
> > +++ b/drivers/fpga/dfl.h
> > @@ -471,6 +471,8 @@ void dfl_fpga_enum_info_free(struct
> > dfl_fpga_enum_info *info);  #define DFL_PORT_CONNECT_BITS
> > MAX_DFL_FPGA_PORT_NUM  #define DFL_FEAT_PORT_CONNECT_MASK
> ((1UL <<
> > (DFL_PORT_CONNECT_BITS)) - 1)
> >
> > +#define dfl_has_port_connected_afu(cdev) ((cdev)->flags &
> > DFL_FEAT_PORT_CONNECT_MASK)
> > +
> >  /**
> >   * struct dfl_fpga_cdev - container device of DFL based FPGA
> >   *
> > --
> > 2.26.2
diff mbox series

Patch

diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
index 712c53363fda..b95b29c5c81d 100644
--- a/drivers/fpga/dfl.c
+++ b/drivers/fpga/dfl.c
@@ -1707,11 +1707,13 @@  int dfl_fpga_cdev_config_ports_vf(struct dfl_fpga_cdev *cdev, int num_vfs)
 
 	mutex_lock(&cdev->lock);
 	/*
-	 * 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.
+	 * In the OFS legacy model, it can't turn multiple ports into 1 VF
+	 * device, because only 1 port conneced to 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 ((dfl_has_port_connected_afu(cdev) &&
+	     cdev->released_port_num != num_vfs)) {
 		ret = -EINVAL;
 		goto done;
 	}
diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
index bc56b7e8c01b..83c2c50975e5 100644
--- a/drivers/fpga/dfl.h
+++ b/drivers/fpga/dfl.h
@@ -471,6 +471,8 @@  void dfl_fpga_enum_info_free(struct dfl_fpga_enum_info *info);
 #define DFL_PORT_CONNECT_BITS  MAX_DFL_FPGA_PORT_NUM
 #define DFL_FEAT_PORT_CONNECT_MASK ((1UL << (DFL_PORT_CONNECT_BITS)) - 1)
 
+#define dfl_has_port_connected_afu(cdev) ((cdev)->flags & DFL_FEAT_PORT_CONNECT_MASK)
+
 /**
  * struct dfl_fpga_cdev - container device of DFL based FPGA
  *