[dpdk-dev,v3,1/6] Add Intel FPGA BUS Command Parse Code

Message ID 1522229396-17898-2-git-send-email-rosen.xu@intel.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Xu, Rosen March 28, 2018, 9:29 a.m. UTC
  Signed-off-by: Rosen Xu <rosen.xu@intel.com>
---
 lib/librte_eal/common/eal_common_options.c | 8 +++++++-
 lib/librte_eal/common/eal_options.h        | 2 ++
 2 files changed, 9 insertions(+), 1 deletion(-)
  

Comments

Gaëtan Rivet March 28, 2018, 1:26 p.m. UTC | #1
On Wed, Mar 28, 2018 at 05:29:51PM +0800, Rosen Xu wrote:
> Signed-off-by: Rosen Xu <rosen.xu@intel.com>
> ---
>  lib/librte_eal/common/eal_common_options.c | 8 +++++++-
>  lib/librte_eal/common/eal_options.h        | 2 ++
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> index 9f2f8d2..4fe0875 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -73,6 +73,7 @@
>  	{OPT_VDEV,              1, NULL, OPT_VDEV_NUM             },
>  	{OPT_VFIO_INTR,         1, NULL, OPT_VFIO_INTR_NUM        },
>  	{OPT_VMWARE_TSC_MAP,    0, NULL, OPT_VMWARE_TSC_MAP_NUM   },
> +	{OPT_IFPGA,             1, NULL, OPT_IFPGA_NUM            },
>  	{0,                     0, NULL, 0                        }
>  };
>  
> @@ -1160,7 +1161,12 @@ static int xdigit2val(unsigned char c)
>  
>  		core_parsed = LCORE_OPT_MAP;
>  		break;
> -
> +	case OPT_IFPGA_NUM:
> +		if (eal_option_device_add(RTE_DEVTYPE_VIRTUAL,
> +				optarg) < 0) {
> +			return -1;
> +		}
> +		break;

why do you need to add a new option if you only insert a virtual
devargs?

Why wouldn't --vdev option work instead? and for that matter, I was
expecting you to provide a PCI address. Can you give a command line
showing how you create your device? The devtype is essentially ignored
currently (at option stage, maybe there are still cruft left in PCI bus),
instead the devargs parsing will detect the bus from the given optarg.

This part of EAL will change rather soon, I'd prefer not to deal with
additional options unless necessary.

>  	/* don't know what to do, leave this to caller */
>  	default:
>  		return 1;
> diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h
> index e86c711..bdbb2c4 100644
> --- a/lib/librte_eal/common/eal_options.h
> +++ b/lib/librte_eal/common/eal_options.h
> @@ -55,6 +55,8 @@ enum {
>  	OPT_VFIO_INTR_NUM,
>  #define OPT_VMWARE_TSC_MAP    "vmware-tsc-map"
>  	OPT_VMWARE_TSC_MAP_NUM,
> +#define OPT_IFPGA              "ifpga"
> +	OPT_IFPGA_NUM,
>  	OPT_LONG_MAX_NUM
>  };
>  
> -- 
> 1.8.3.1
>
  
Xu, Rosen March 31, 2018, 4:25 p.m. UTC | #2
> -----Original Message-----
> From: Gaëtan Rivet [mailto:gaetan.rivet@6wind.com]
> Sent: Wednesday, March 28, 2018 21:26
> To: Xu, Rosen <rosen.xu@intel.com>
> Cc: dev@dpdk.org; Doherty, Declan <declan.doherty@intel.com>;
> Richardson, Bruce <bruce.richardson@intel.com>; shreyansh.jain@nxp.com;
> Zhang, Tianfei <tianfei.zhang@intel.com>; Wu, Hao <hao.wu@intel.com>
> Subject: Re: [PATCH v3 1/6] Add Intel FPGA BUS Command Parse Code
> 
> On Wed, Mar 28, 2018 at 05:29:51PM +0800, Rosen Xu wrote:
> > Signed-off-by: Rosen Xu <rosen.xu@intel.com>
> > ---
> >  lib/librte_eal/common/eal_common_options.c | 8 +++++++-
> >  lib/librte_eal/common/eal_options.h        | 2 ++
> >  2 files changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/librte_eal/common/eal_common_options.c
> > b/lib/librte_eal/common/eal_common_options.c
> > index 9f2f8d2..4fe0875 100644
> > --- a/lib/librte_eal/common/eal_common_options.c
> > +++ b/lib/librte_eal/common/eal_common_options.c
> > @@ -73,6 +73,7 @@
> >  	{OPT_VDEV,              1, NULL, OPT_VDEV_NUM             },
> >  	{OPT_VFIO_INTR,         1, NULL, OPT_VFIO_INTR_NUM        },
> >  	{OPT_VMWARE_TSC_MAP,    0, NULL,
> OPT_VMWARE_TSC_MAP_NUM   },
> > +	{OPT_IFPGA,             1, NULL, OPT_IFPGA_NUM            },
> >  	{0,                     0, NULL, 0                        }
> >  };
> >
> > @@ -1160,7 +1161,12 @@ static int xdigit2val(unsigned char c)
> >
> >  		core_parsed = LCORE_OPT_MAP;
> >  		break;
> > -
> > +	case OPT_IFPGA_NUM:
> > +		if (eal_option_device_add(RTE_DEVTYPE_VIRTUAL,
> > +				optarg) < 0) {
> > +			return -1;
> > +		}
> > +		break;
> 
> why do you need to add a new option if you only insert a virtual devargs?
> 
> Why wouldn't --vdev option work instead? and for that matter, I was
> expecting you to provide a PCI address. Can you give a command line
> showing how you create your device? The devtype is essentially ignored
> currently (at option stage, maybe there are still cruft left in PCI bus), instead
> the devargs parsing will detect the bus from the given optarg.
> 
> This part of EAL will change rather soon, I'd prefer not to deal with additional
> options unless necessary.

For PATCH v4, I have removed all the modification from eal library. 
Create vdev to take IFPGA parameters configuration.

The command line for PATCH v4 is(just take 2 AFU for example):
testpmd -c 0x3 -n 4 --socket-mem 1024,0 --huge-dir=/mnt/huge \
--vdev 'net_ifpga_cfg0,bdf=5e:00.0,port=0,afu_bts=./xxx.gbs'  \
--vdev 'net_ifpga_cfg1,bdf=be:00.0,port=0,afu_bts=./xxx.gbs'  -- -i --no-numa
Note: the parameter of "port" is used by OPAE for download bitstream.

> >  	/* don't know what to do, leave this to caller */
> >  	default:
> >  		return 1;
> > diff --git a/lib/librte_eal/common/eal_options.h
> > b/lib/librte_eal/common/eal_options.h
> > index e86c711..bdbb2c4 100644
> > --- a/lib/librte_eal/common/eal_options.h
> > +++ b/lib/librte_eal/common/eal_options.h
> > @@ -55,6 +55,8 @@ enum {
> >  	OPT_VFIO_INTR_NUM,
> >  #define OPT_VMWARE_TSC_MAP    "vmware-tsc-map"
> >  	OPT_VMWARE_TSC_MAP_NUM,
> > +#define OPT_IFPGA              "ifpga"
> > +	OPT_IFPGA_NUM,
> >  	OPT_LONG_MAX_NUM
> >  };
> >
> > --
> > 1.8.3.1
> >
> 
> --
> Gaëtan Rivet
> 6WIND
  
Xu, Rosen April 4, 2018, 1:58 a.m. UTC | #3
Hello Gaetan,

Are my answers ok?
If so, could you reply it?
Many thanks to you.

> -----Original Message-----
> From: Xu, Rosen
> Sent: Sunday, April 01, 2018 0:26
> To: gaetan.rivet@6wind.com
> Cc: dev@dpdk.org; Doherty, Declan <declan.doherty@intel.com>;
> Richardson, Bruce <bruce.richardson@intel.com>; shreyansh.jain@nxp.com;
> Zhang, Tianfei <tianfei.zhang@intel.com>; Wu, Hao <hao.wu@intel.com>
> Subject: RE: [PATCH v3 1/6] Add Intel FPGA BUS Command Parse Code
> 
> 
> 
> > -----Original Message-----
> > From: Gaëtan Rivet [mailto:gaetan.rivet@6wind.com]
> > Sent: Wednesday, March 28, 2018 21:26
> > To: Xu, Rosen <rosen.xu@intel.com>
> > Cc: dev@dpdk.org; Doherty, Declan <declan.doherty@intel.com>;
> > Richardson, Bruce <bruce.richardson@intel.com>;
> > shreyansh.jain@nxp.com; Zhang, Tianfei <tianfei.zhang@intel.com>; Wu,
> > Hao <hao.wu@intel.com>
> > Subject: Re: [PATCH v3 1/6] Add Intel FPGA BUS Command Parse Code
> >
> > On Wed, Mar 28, 2018 at 05:29:51PM +0800, Rosen Xu wrote:
> > > Signed-off-by: Rosen Xu <rosen.xu@intel.com>
> > > ---
> > >  lib/librte_eal/common/eal_common_options.c | 8 +++++++-
> > >  lib/librte_eal/common/eal_options.h        | 2 ++
> > >  2 files changed, 9 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/lib/librte_eal/common/eal_common_options.c
> > > b/lib/librte_eal/common/eal_common_options.c
> > > index 9f2f8d2..4fe0875 100644
> > > --- a/lib/librte_eal/common/eal_common_options.c
> > > +++ b/lib/librte_eal/common/eal_common_options.c
> > > @@ -73,6 +73,7 @@
> > >  	{OPT_VDEV,              1, NULL, OPT_VDEV_NUM             },
> > >  	{OPT_VFIO_INTR,         1, NULL, OPT_VFIO_INTR_NUM        },
> > >  	{OPT_VMWARE_TSC_MAP,    0, NULL,
> > OPT_VMWARE_TSC_MAP_NUM   },
> > > +	{OPT_IFPGA,             1, NULL, OPT_IFPGA_NUM            },
> > >  	{0,                     0, NULL, 0                        }
> > >  };
> > >
> > > @@ -1160,7 +1161,12 @@ static int xdigit2val(unsigned char c)
> > >
> > >  		core_parsed = LCORE_OPT_MAP;
> > >  		break;
> > > -
> > > +	case OPT_IFPGA_NUM:
> > > +		if (eal_option_device_add(RTE_DEVTYPE_VIRTUAL,
> > > +				optarg) < 0) {
> > > +			return -1;
> > > +		}
> > > +		break;
> >
> > why do you need to add a new option if you only insert a virtual devargs?
> >
> > Why wouldn't --vdev option work instead? and for that matter, I was
> > expecting you to provide a PCI address. Can you give a command line
> > showing how you create your device? The devtype is essentially ignored
> > currently (at option stage, maybe there are still cruft left in PCI
> > bus), instead the devargs parsing will detect the bus from the given optarg.
> >
> > This part of EAL will change rather soon, I'd prefer not to deal with
> > additional options unless necessary.
> 
> For PATCH v4, I have removed all the modification from eal library.
> Create vdev to take IFPGA parameters configuration.
> 
> The command line for PATCH v4 is(just take 2 AFU for example):
> testpmd -c 0x3 -n 4 --socket-mem 1024,0 --huge-dir=/mnt/huge \ --vdev
> 'net_ifpga_cfg0,bdf=5e:00.0,port=0,afu_bts=./xxx.gbs'  \ --vdev
> 'net_ifpga_cfg1,bdf=be:00.0,port=0,afu_bts=./xxx.gbs'  -- -i --no-numa
> Note: the parameter of "port" is used by OPAE for download bitstream.
> 
> > >  	/* don't know what to do, leave this to caller */
> > >  	default:
> > >  		return 1;
> > > diff --git a/lib/librte_eal/common/eal_options.h
> > > b/lib/librte_eal/common/eal_options.h
> > > index e86c711..bdbb2c4 100644
> > > --- a/lib/librte_eal/common/eal_options.h
> > > +++ b/lib/librte_eal/common/eal_options.h
> > > @@ -55,6 +55,8 @@ enum {
> > >  	OPT_VFIO_INTR_NUM,
> > >  #define OPT_VMWARE_TSC_MAP    "vmware-tsc-map"
> > >  	OPT_VMWARE_TSC_MAP_NUM,
> > > +#define OPT_IFPGA              "ifpga"
> > > +	OPT_IFPGA_NUM,
> > >  	OPT_LONG_MAX_NUM
> > >  };
> > >
> > > --
> > > 1.8.3.1
> > >
> >
> > --
> > Gaëtan Rivet
> > 6WIND
  

Patch

diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 9f2f8d2..4fe0875 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -73,6 +73,7 @@ 
 	{OPT_VDEV,              1, NULL, OPT_VDEV_NUM             },
 	{OPT_VFIO_INTR,         1, NULL, OPT_VFIO_INTR_NUM        },
 	{OPT_VMWARE_TSC_MAP,    0, NULL, OPT_VMWARE_TSC_MAP_NUM   },
+	{OPT_IFPGA,             1, NULL, OPT_IFPGA_NUM            },
 	{0,                     0, NULL, 0                        }
 };
 
@@ -1160,7 +1161,12 @@  static int xdigit2val(unsigned char c)
 
 		core_parsed = LCORE_OPT_MAP;
 		break;
-
+	case OPT_IFPGA_NUM:
+		if (eal_option_device_add(RTE_DEVTYPE_VIRTUAL,
+				optarg) < 0) {
+			return -1;
+		}
+		break;
 	/* don't know what to do, leave this to caller */
 	default:
 		return 1;
diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h
index e86c711..bdbb2c4 100644
--- a/lib/librte_eal/common/eal_options.h
+++ b/lib/librte_eal/common/eal_options.h
@@ -55,6 +55,8 @@  enum {
 	OPT_VFIO_INTR_NUM,
 #define OPT_VMWARE_TSC_MAP    "vmware-tsc-map"
 	OPT_VMWARE_TSC_MAP_NUM,
+#define OPT_IFPGA              "ifpga"
+	OPT_IFPGA_NUM,
 	OPT_LONG_MAX_NUM
 };