Re: [dpdk-dev] [PATCH v2] eal/devargs: add option to supply PCI dev args

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 6/26/2018 1:48 PM, Shahaf Shuler wrote:
> Hi Pavan,
> 
> Friday, June 15, 2018 7:44 AM, Pavan Nikhilesh:
>> Subject: [dpdk-dev] [PATCH v2] eal/devargs: add option to supply PCI dev
>> args
>>
>> Currently, the only way of supplying device argument to a pci device is to
>> whitelist it i.e. -w 000X:00:0X.0,self_test=1. This is not a very feasible method
>> as whitelisting a device has its own side effects i.e only the whitelisted pci
>> devices are probed.
>>
>> Add a new eal command line option --pci-args to pass device args without the
>> need to whitelist the devices.
>> 		--pci-args 000X:00:0X.0,self_test=1
>>
>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> 
> Tested-by: Shahaf Shuler <shahafs@mellanox.com>
> 
> It seems to work. 
> Please see small comments below

Isn't this conflict with Gaetan's devarg work which has wider scope?

> 
>> ---
>>  v2 Changes:
>>  - Document the option usage in eal_common_usage.
>>  - Update commit log to be more informative.
>>
>>  lib/librte_eal/common/eal_common_devargs.c  | 3 +++
>> lib/librte_eal/common/eal_common_options.c  | 9 +++++++++
>>  lib/librte_eal/common/eal_options.h         | 2 ++
>>  lib/librte_eal/common/include/rte_dev.h     | 1 +
>>  lib/librte_eal/common/include/rte_devargs.h | 1 +
>>  5 files changed, 16 insertions(+)
> 
> Should we also update the manual of testpmd (doc/guides/testpmd_app_ug/run_app.rst ) for the new eal arg?
> 
>>
>> diff --git a/lib/librte_eal/common/eal_common_devargs.c
>> b/lib/librte_eal/common/eal_common_devargs.c
>> index b0434158b..a56bfeea0 100644
>> --- a/lib/librte_eal/common/eal_common_devargs.c
>> +++ b/lib/librte_eal/common/eal_common_devargs.c
>> @@ -156,6 +156,9 @@ rte_devargs_add(enum rte_devtype devtype, const
>> char *devargs_str)
>>  	bus = devargs->bus;
>>  	if (devargs->type == RTE_DEVTYPE_BLACKLISTED_PCI)
>>  		devargs->policy = RTE_DEV_BLACKLISTED;
>> +	else if (devargs->type == RTE_DEVTYPE_WHITELISTED_PCI)
>> +		devargs->policy = RTE_DEV_WHITELISTED;
>> +
>>  	if (bus->conf.scan_mode == RTE_BUS_SCAN_UNDEFINED) {
>>  		if (devargs->policy == RTE_DEV_WHITELISTED)
>>  			bus->conf.scan_mode = RTE_BUS_SCAN_WHITELIST;
>> diff --git a/lib/librte_eal/common/eal_common_options.c
>> b/lib/librte_eal/common/eal_common_options.c
>> index ecebb2923..31eebaa53 100644
>> --- a/lib/librte_eal/common/eal_common_options.c
>> +++ b/lib/librte_eal/common/eal_common_options.c
>> @@ -76,6 +76,7 @@ eal_long_options[] = {
>>  	{OPT_VMWARE_TSC_MAP,    0, NULL,
>> OPT_VMWARE_TSC_MAP_NUM   },
>>  	{OPT_LEGACY_MEM,        0, NULL, OPT_LEGACY_MEM_NUM       },
>>  	{OPT_SINGLE_FILE_SEGMENTS, 0, NULL,
>> OPT_SINGLE_FILE_SEGMENTS_NUM},
>> +	{OPT_PCI_DEVARGS,       1, NULL, OPT_PCI_DEVARGS_NUM},
>>  	{0,                     0, NULL, 0                        }
>>  };
>>
>> @@ -1224,6 +1225,12 @@ eal_parse_common_option(int opt, const char
>> *optarg,
>>  	case OPT_SINGLE_FILE_SEGMENTS_NUM:
>>  		conf->single_file_segments = 1;
>>  		break;
>> +	case OPT_PCI_DEVARGS_NUM:
>> +		if (eal_option_device_add(RTE_DEVTYPE_NORMAL,
>> +				optarg) < 0) {
>> +			return -1;
>> +		}
>> +		break;
>>
>>  	/* don't know what to do, leave this to caller */
>>  	default:
>> @@ -1360,6 +1367,8 @@ eal_common_usage(void)
>>  	       "  --"OPT_VDEV"              Add a virtual device.\n"
>>  	       "                      The argument format is <driver><id>[,key=val,...]\n"
>>  	       "                      (ex: --vdev=net_pcap0,iface=eth2).\n"
>> +	       "  --"OPT_PCI_DEVARGS"          Pass key-value arguments to a pci
>> device\n"
>> +	       "                      The argument format is
>> <domain:bus:devid.func>[,key=val,...]\n"
>>  	       "  -d LIB.so|DIR       Add a driver or driver directory\n"
>>  	       "                      (can be used multiple times)\n"
>>  	       "  --"OPT_VMWARE_TSC_MAP"    Use VMware TSC map instead
>> of native RDTSC\n"
>> diff --git a/lib/librte_eal/common/eal_options.h
>> b/lib/librte_eal/common/eal_options.h
>> index 211ae06ae..d52d38e32 100644
>> --- a/lib/librte_eal/common/eal_options.h
>> +++ b/lib/librte_eal/common/eal_options.h
>> @@ -59,6 +59,8 @@ enum {
>>  	OPT_LEGACY_MEM_NUM,
>>  #define OPT_SINGLE_FILE_SEGMENTS    "single-file-segments"
>>  	OPT_SINGLE_FILE_SEGMENTS_NUM,
>> +#define OPT_PCI_DEVARGS       "pci-args"
>> +	OPT_PCI_DEVARGS_NUM,
>>  	OPT_LONG_MAX_NUM
>>  };
>>
>> diff --git a/lib/librte_eal/common/include/rte_dev.h
>> b/lib/librte_eal/common/include/rte_dev.h
>> index 3879ff3ca..fb3e5033f 100644
>> --- a/lib/librte_eal/common/include/rte_dev.h
>> +++ b/lib/librte_eal/common/include/rte_dev.h
>> @@ -124,6 +124,7 @@ enum rte_kernel_driver {
>>   * Device policies.
>>   */
>>  enum rte_dev_policy {
>> +	RTE_DEV_NORMAL,
>>  	RTE_DEV_WHITELISTED,
>>  	RTE_DEV_BLACKLISTED,
>>  };
>> diff --git a/lib/librte_eal/common/include/rte_devargs.h
>> b/lib/librte_eal/common/include/rte_devargs.h
>> index 58fbd90a2..78c600bf2 100644
>> --- a/lib/librte_eal/common/include/rte_devargs.h
>> +++ b/lib/librte_eal/common/include/rte_devargs.h
>> @@ -29,6 +29,7 @@ extern "C" {
>>   * Type of generic device
>>   */
>>  enum rte_devtype {
>> +	RTE_DEVTYPE_NORMAL, /* Normal dev with special pci args */
> 
> What is "Normal" device? Can we find a better name? 
> 
>>  	RTE_DEVTYPE_WHITELISTED_PCI,
>>  	RTE_DEVTYPE_BLACKLISTED_PCI,
>>  	RTE_DEVTYPE_VIRTUAL,
>> --
>> 2.17.1
>