bus/pci: restricted bus scanning to allowed devices
Checks
Commit Message
rte_bus_scan API scans all the available PCI devices irrespective of white
or black listing parameters then further devices are probed based on white
or black listing parameters. So unnecessary CPU cycles are wasted during
rte_pci_scan.
For Octeontx2 platform with core frequency 2.4 Ghz, rte_bus_scan consumes
around 26ms to scan around 90 PCI devices but all may not be used by the
application. So for the application which uses 2 NICs, rte_bus_scan
consumes few microseconds and rest time is saved with this patch.
Patch restricts devices to be scanned as per below mentioned conditions:
- All devices will be scanned if no parameters are passed.
- Only white listed devices will be scanned if white list is available.
- All devices, except black listed, will be scanned if black list is
available.
Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
---
drivers/bus/pci/bsd/pci.c | 18 +++++++++++++++++-
drivers/bus/pci/linux/pci.c | 11 +++++++++++
drivers/bus/pci/pci_common.c | 4 ++--
drivers/bus/pci/private.h | 20 ++++++++++++++++++++
4 files changed, 50 insertions(+), 3 deletions(-)
Comments
> /* Create dummy pci device to get devargs */
> + dummy_dev.addr.domain = matches[i].pc_sel.pc_domain;
> + dummy_dev.addr.bus = matches[i].pc_sel.pc_bus;
> + dummy_dev.addr.devid = matches[i].pc_sel.pc_dev;
> + dummy_dev.addr.function = matches[i].pc_sel.pc_func;
> + dummy_dev.device.devargs =
> + pci_devargs_lookup(&dummy_dev);
> +
> + /* Check that device should be ignored or not */
> + if (pci_ignore_device(&dummy_dev))
> + continue;
It seems that you are creating dummy_dev as an alternative to passing
just the PCI bus/device/function. Wouldn't be easier to just use BDF
instead. Dummy arguments on the stack can lead to more corner cases
in the future if device subsystem changes.
> +/**
> + * Get the devargs of a PCI device.
> + *
> + * @param pci_dev
> + * PCI device to be validated
> + * @return
> + * devargs on succes, NULL otherwise
> + */
> +struct rte_devargs *pci_devargs_lookup(struct rte_pci_device *pci_dev);
Must be marked experimental (or internal).
The pci_device should be marked const.
> +
> +/**
> + * Validate whether a pci device should be ignored or not.
> + *
> + * @param pci_dev
> + * PCI device to be validated
> + * @return
> + * 1 if device is to be ignored, 0 otherwise
> + */
> +bool pci_ignore_device(const struct rte_pci_device *pci_dev);
ditto
Regards
Sunil Kumar Kori
>-----Original Message-----
>From: Stephen Hemminger <stephen@networkplumber.org>
>Sent: Monday, December 16, 2019 9:43 PM
>To: Sunil Kumar Kori <skori@marvell.com>
>Cc: dev@dpdk.org
>Subject: [EXT] Re: [dpdk-dev] [PATCH] bus/pci: restricted bus scanning to
>allowed devices
>
>External Email
>
>----------------------------------------------------------------------
>> /* Create dummy pci device to get devargs */
>> + dummy_dev.addr.domain =
>matches[i].pc_sel.pc_domain;
>> + dummy_dev.addr.bus = matches[i].pc_sel.pc_bus;
>> + dummy_dev.addr.devid = matches[i].pc_sel.pc_dev;
>> + dummy_dev.addr.function =
>matches[i].pc_sel.pc_func;
>> + dummy_dev.device.devargs =
>> +
> pci_devargs_lookup(&dummy_dev);
>> +
>> + /* Check that device should be ignored or not */
>> + if (pci_ignore_device(&dummy_dev))
>> + continue;
>
>It seems that you are creating dummy_dev as an alternative to passing just the
>PCI bus/device/function. Wouldn't be easier to just use BDF instead. Dummy
>arguments on the stack can lead to more corner cases in the future if device
>subsystem changes.
Agreed and initially I have implemented using BDF only instead of using dummy device.
But using that approach, I was not able to use existing APIs to get devargs and ignore device.
I had to write almost same functions to solve the purpose. So just to avoid having replica of
same code, I followed this approach. Please suggest.
>
>> +/**
>> + * Get the devargs of a PCI device.
>> + *
>> + * @param pci_dev
>> + * PCI device to be validated
>> + * @return
>> + * devargs on succes, NULL otherwise
>> + */
>> +struct rte_devargs *pci_devargs_lookup(struct rte_pci_device
>> +*pci_dev);
>
>Must be marked experimental (or internal).
>The pci_device should be marked const.
Okay but If I go with BDF one then this change is not required anyway.
>
>
>> +
>> +/**
>> + * Validate whether a pci device should be ignored or not.
>> + *
>> + * @param pci_dev
>> + * PCI device to be validated
>> + * @return
>> + * 1 if device is to be ignored, 0 otherwise
>> + */
>> +bool pci_ignore_device(const struct rte_pci_device *pci_dev);
>
>ditto
ditto
Hello Stephen,
Any suggestions ?
Regards
Sunil Kumar Kori
>-----Original Message-----
>From: dev <dev-bounces@dpdk.org> On Behalf Of Sunil Kumar Kori
>Sent: Tuesday, December 17, 2019 4:30 PM
>To: Stephen Hemminger <stephen@networkplumber.org>
>Cc: dev@dpdk.org
>Subject: Re: [dpdk-dev] [EXT] Re: [PATCH] bus/pci: restricted bus scanning to
>allowed devices
>
>
>
>Regards
>Sunil Kumar Kori
>
>>-----Original Message-----
>>From: Stephen Hemminger <stephen@networkplumber.org>
>>Sent: Monday, December 16, 2019 9:43 PM
>>To: Sunil Kumar Kori <skori@marvell.com>
>>Cc: dev@dpdk.org
>>Subject: [EXT] Re: [dpdk-dev] [PATCH] bus/pci: restricted bus scanning
>>to allowed devices
>>
>>External Email
>>
>>----------------------------------------------------------------------
>>> /* Create dummy pci device to get devargs */
>>> + dummy_dev.addr.domain =
>>matches[i].pc_sel.pc_domain;
>>> + dummy_dev.addr.bus = matches[i].pc_sel.pc_bus;
>>> + dummy_dev.addr.devid = matches[i].pc_sel.pc_dev;
>>> + dummy_dev.addr.function =
>>matches[i].pc_sel.pc_func;
>>> + dummy_dev.device.devargs =
>>> +
>> pci_devargs_lookup(&dummy_dev);
>>> +
>>> + /* Check that device should be ignored or not */
>>> + if (pci_ignore_device(&dummy_dev))
>>> + continue;
>>
>>It seems that you are creating dummy_dev as an alternative to passing
>>just the PCI bus/device/function. Wouldn't be easier to just use BDF
>>instead. Dummy arguments on the stack can lead to more corner cases in
>>the future if device subsystem changes.
>Agreed and initially I have implemented using BDF only instead of using
>dummy device.
>But using that approach, I was not able to use existing APIs to get devargs and
>ignore device.
>I had to write almost same functions to solve the purpose. So just to avoid
>having replica of same code, I followed this approach. Please suggest.
>>
>>> +/**
>>> + * Get the devargs of a PCI device.
>>> + *
>>> + * @param pci_dev
>>> + * PCI device to be validated
>>> + * @return
>>> + * devargs on succes, NULL otherwise
>>> + */
>>> +struct rte_devargs *pci_devargs_lookup(struct rte_pci_device
>>> +*pci_dev);
>>
>>Must be marked experimental (or internal).
>>The pci_device should be marked const.
>Okay but If I go with BDF one then this change is not required anyway.
>>
>>
>>> +
>>> +/**
>>> + * Validate whether a pci device should be ignored or not.
>>> + *
>>> + * @param pci_dev
>>> + * PCI device to be validated
>>> + * @return
>>> + * 1 if device is to be ignored, 0 otherwise
>>> + */
>>> +bool pci_ignore_device(const struct rte_pci_device *pci_dev);
>>
>>ditto
>ditto
Hello All,
Is there any thought on this ? Otherwise it can be merged.
Regards
Sunil Kumar Kori
>-----Original Message-----
>From: Sunil Kumar Kori
>Sent: Thursday, January 23, 2020 2:13 PM
>To: Stephen Hemminger <stephen@networkplumber.org>; Jerin Jacob
>Kollanukkaran <jerinj@marvell.com>
>Subject: FW: [dpdk-dev] [EXT] Re: [PATCH] bus/pci: restricted bus scanning to
>allowed devices
>
>Hello Stephen,
>
>Can you please look into this patch or provide your thought in this ? So that it
>can be merged within 20.02 release.
>
>Regards
>Sunil Kumar Kori
>
>-----Original Message-----
>From: Sunil Kumar Kori <skori@marvell.com>
>Sent: Tuesday, January 21, 2020 2:09 PM
>To: Sunil Kumar Kori <skori@marvell.com>; Stephen Hemminger
><stephen@networkplumber.org>
>Cc: dev@dpdk.org
>Subject: RE: [dpdk-dev] [EXT] Re: [PATCH] bus/pci: restricted bus scanning to
>allowed devices
>
>Hello Stephen,
>Any suggestions ?
>
>Regards
>Sunil Kumar Kori
>
>>-----Original Message-----
>>From: dev <dev-bounces@dpdk.org> On Behalf Of Sunil Kumar Kori
>>Sent: Tuesday, December 17, 2019 4:30 PM
>>To: Stephen Hemminger <stephen@networkplumber.org>
>>Cc: dev@dpdk.org
>>Subject: Re: [dpdk-dev] [EXT] Re: [PATCH] bus/pci: restricted bus
>>scanning to allowed devices
>>
>>
>>
>>Regards
>>Sunil Kumar Kori
>>
>>>-----Original Message-----
>>>From: Stephen Hemminger <stephen@networkplumber.org>
>>>Sent: Monday, December 16, 2019 9:43 PM
>>>To: Sunil Kumar Kori <skori@marvell.com>
>>>Cc: dev@dpdk.org
>>>Subject: [EXT] Re: [dpdk-dev] [PATCH] bus/pci: restricted bus scanning
>>>to allowed devices
>>>
>>>External Email
>>>
>>>----------------------------------------------------------------------
>>>> /* Create dummy pci device to get devargs */
>>>> + dummy_dev.addr.domain =
>>>matches[i].pc_sel.pc_domain;
>>>> + dummy_dev.addr.bus = matches[i].pc_sel.pc_bus;
>>>> + dummy_dev.addr.devid = matches[i].pc_sel.pc_dev;
>>>> + dummy_dev.addr.function =
>>>matches[i].pc_sel.pc_func;
>>>> + dummy_dev.device.devargs =
>>>> +
>>> pci_devargs_lookup(&dummy_dev);
>>>> +
>>>> + /* Check that device should be ignored or not */
>>>> + if (pci_ignore_device(&dummy_dev))
>>>> + continue;
>>>
>>>It seems that you are creating dummy_dev as an alternative to passing
>>>just the PCI bus/device/function. Wouldn't be easier to just use BDF
>>>instead. Dummy arguments on the stack can lead to more corner cases in
>>>the future if device subsystem changes.
>>Agreed and initially I have implemented using BDF only instead of using
>>dummy device.
>>But using that approach, I was not able to use existing APIs to get
>>devargs and ignore device.
>>I had to write almost same functions to solve the purpose. So just to
>>avoid having replica of same code, I followed this approach. Please suggest.
>>>
>>>> +/**
>>>> + * Get the devargs of a PCI device.
>>>> + *
>>>> + * @param pci_dev
>>>> + * PCI device to be validated
>>>> + * @return
>>>> + * devargs on succes, NULL otherwise
>>>> + */
>>>> +struct rte_devargs *pci_devargs_lookup(struct rte_pci_device
>>>> +*pci_dev);
>>>
>>>Must be marked experimental (or internal).
>>>The pci_device should be marked const.
>>Okay but If I go with BDF one then this change is not required anyway.
>>>
>>>
>>>> +
>>>> +/**
>>>> + * Validate whether a pci device should be ignored or not.
>>>> + *
>>>> + * @param pci_dev
>>>> + * PCI device to be validated
>>>> + * @return
>>>> + * 1 if device is to be ignored, 0 otherwise
>>>> + */
>>>> +bool pci_ignore_device(const struct rte_pci_device *pci_dev);
>>>
>>>ditto
>>ditto
Hello Stephen,
Please provide ack on below change if there is no concern so that it can be accepted on 20.05.
Regards
Sunil Kumar Kori
>-----Original Message-----
>From: Sunil Kumar Kori
>Sent: Thursday, February 27, 2020 2:00 PM
>To: Stephen Hemminger <stephen@networkplumber.org>; Jerin Jacob
>Kollanukkaran <jerinj@marvell.com>; dev@dpdk.org
>Subject: RE: [dpdk-dev] [EXT] Re: [PATCH] bus/pci: restricted bus scanning to
>allowed devices
>
>Hello All,
>
>Is there any thought on this ? Otherwise it can be merged.
>
>Regards
>Sunil Kumar Kori
>
>>-----Original Message-----
>>From: Sunil Kumar Kori
>>Sent: Thursday, January 23, 2020 2:13 PM
>>To: Stephen Hemminger <stephen@networkplumber.org>; Jerin Jacob
>>Kollanukkaran <jerinj@marvell.com>
>>Subject: FW: [dpdk-dev] [EXT] Re: [PATCH] bus/pci: restricted bus
>>scanning to allowed devices
>>
>>Hello Stephen,
>>
>>Can you please look into this patch or provide your thought in this ?
>>So that it can be merged within 20.02 release.
>>
>>Regards
>>Sunil Kumar Kori
>>
>>-----Original Message-----
>>From: Sunil Kumar Kori <skori@marvell.com>
>>Sent: Tuesday, January 21, 2020 2:09 PM
>>To: Sunil Kumar Kori <skori@marvell.com>; Stephen Hemminger
>><stephen@networkplumber.org>
>>Cc: dev@dpdk.org
>>Subject: RE: [dpdk-dev] [EXT] Re: [PATCH] bus/pci: restricted bus
>>scanning to allowed devices
>>
>>Hello Stephen,
>>Any suggestions ?
>>
>>Regards
>>Sunil Kumar Kori
>>
>>>-----Original Message-----
>>>From: dev <dev-bounces@dpdk.org> On Behalf Of Sunil Kumar Kori
>>>Sent: Tuesday, December 17, 2019 4:30 PM
>>>To: Stephen Hemminger <stephen@networkplumber.org>
>>>Cc: dev@dpdk.org
>>>Subject: Re: [dpdk-dev] [EXT] Re: [PATCH] bus/pci: restricted bus
>>>scanning to allowed devices
>>>
>>>
>>>
>>>Regards
>>>Sunil Kumar Kori
>>>
>>>>-----Original Message-----
>>>>From: Stephen Hemminger <stephen@networkplumber.org>
>>>>Sent: Monday, December 16, 2019 9:43 PM
>>>>To: Sunil Kumar Kori <skori@marvell.com>
>>>>Cc: dev@dpdk.org
>>>>Subject: [EXT] Re: [dpdk-dev] [PATCH] bus/pci: restricted bus
>>>>scanning to allowed devices
>>>>
>>>>External Email
>>>>
>>>>---------------------------------------------------------------------
>>>>-
>>>>> /* Create dummy pci device to get devargs */
>>>>> + dummy_dev.addr.domain =
>>>>matches[i].pc_sel.pc_domain;
>>>>> + dummy_dev.addr.bus = matches[i].pc_sel.pc_bus;
>>>>> + dummy_dev.addr.devid = matches[i].pc_sel.pc_dev;
>>>>> + dummy_dev.addr.function =
>>>>matches[i].pc_sel.pc_func;
>>>>> + dummy_dev.device.devargs =
>>>>> +
>>>> pci_devargs_lookup(&dummy_dev);
>>>>> +
>>>>> + /* Check that device should be ignored or not */
>>>>> + if (pci_ignore_device(&dummy_dev))
>>>>> + continue;
>>>>
>>>>It seems that you are creating dummy_dev as an alternative to passing
>>>>just the PCI bus/device/function. Wouldn't be easier to just use BDF
>>>>instead. Dummy arguments on the stack can lead to more corner cases
>>>>in the future if device subsystem changes.
>>>Agreed and initially I have implemented using BDF only instead of
>>>using dummy device.
>>>But using that approach, I was not able to use existing APIs to get
>>>devargs and ignore device.
>>>I had to write almost same functions to solve the purpose. So just to
>>>avoid having replica of same code, I followed this approach. Please suggest.
>>>>
>>>>> +/**
>>>>> + * Get the devargs of a PCI device.
>>>>> + *
>>>>> + * @param pci_dev
>>>>> + * PCI device to be validated
>>>>> + * @return
>>>>> + * devargs on succes, NULL otherwise
>>>>> + */
>>>>> +struct rte_devargs *pci_devargs_lookup(struct rte_pci_device
>>>>> +*pci_dev);
>>>>
>>>>Must be marked experimental (or internal).
>>>>The pci_device should be marked const.
>>>Okay but If I go with BDF one then this change is not required anyway.
>>>>
>>>>
>>>>> +
>>>>> +/**
>>>>> + * Validate whether a pci device should be ignored or not.
>>>>> + *
>>>>> + * @param pci_dev
>>>>> + * PCI device to be validated
>>>>> + * @return
>>>>> + * 1 if device is to be ignored, 0 otherwise
>>>>> + */
>>>>> +bool pci_ignore_device(const struct rte_pci_device *pci_dev);
>>>>
>>>>ditto
>>>ditto
Hello,
It looks like there is no comment/objection on following patch and it can be merged.
I would request to @David Marchand, please take care of this towards the merging process for 20.05.
Regards
Sunil Kumar Kori
>-----Original Message-----
>From: Sunil Kumar Kori
>Sent: Monday, March 9, 2020 11:37 AM
>To: 'Stephen Hemminger' <stephen@networkplumber.org>; Jerin Jacob
>Kollanukkaran <jerinj@marvell.com>; 'dev@dpdk.org' <dev@dpdk.org>
>Subject: RE: [dpdk-dev] [EXT] Re: [PATCH] bus/pci: restricted bus scanning to
>allowed devices
>
>Hello Stephen,
>
>Please provide ack on below change if there is no concern so that it can be
>accepted on 20.05.
>
>Regards
>Sunil Kumar Kori
>
>>-----Original Message-----
>>From: Sunil Kumar Kori
>>Sent: Thursday, February 27, 2020 2:00 PM
>>To: Stephen Hemminger <stephen@networkplumber.org>; Jerin Jacob
>>Kollanukkaran <jerinj@marvell.com>; dev@dpdk.org
>>Subject: RE: [dpdk-dev] [EXT] Re: [PATCH] bus/pci: restricted bus
>>scanning to allowed devices
>>
>>Hello All,
>>
>>Is there any thought on this ? Otherwise it can be merged.
>>
>>Regards
>>Sunil Kumar Kori
>>
>>>-----Original Message-----
>>>From: Sunil Kumar Kori
>>>Sent: Thursday, January 23, 2020 2:13 PM
>>>To: Stephen Hemminger <stephen@networkplumber.org>; Jerin Jacob
>>>Kollanukkaran <jerinj@marvell.com>
>>>Subject: FW: [dpdk-dev] [EXT] Re: [PATCH] bus/pci: restricted bus
>>>scanning to allowed devices
>>>
>>>Hello Stephen,
>>>
>>>Can you please look into this patch or provide your thought in this ?
>>>So that it can be merged within 20.02 release.
>>>
>>>Regards
>>>Sunil Kumar Kori
>>>
>>>-----Original Message-----
>>>From: Sunil Kumar Kori <skori@marvell.com>
>>>Sent: Tuesday, January 21, 2020 2:09 PM
>>>To: Sunil Kumar Kori <skori@marvell.com>; Stephen Hemminger
>>><stephen@networkplumber.org>
>>>Cc: dev@dpdk.org
>>>Subject: RE: [dpdk-dev] [EXT] Re: [PATCH] bus/pci: restricted bus
>>>scanning to allowed devices
>>>
>>>Hello Stephen,
>>>Any suggestions ?
>>>
>>>Regards
>>>Sunil Kumar Kori
>>>
>>>>-----Original Message-----
>>>>From: dev <dev-bounces@dpdk.org> On Behalf Of Sunil Kumar Kori
>>>>Sent: Tuesday, December 17, 2019 4:30 PM
>>>>To: Stephen Hemminger <stephen@networkplumber.org>
>>>>Cc: dev@dpdk.org
>>>>Subject: Re: [dpdk-dev] [EXT] Re: [PATCH] bus/pci: restricted bus
>>>>scanning to allowed devices
>>>>
>>>>
>>>>
>>>>Regards
>>>>Sunil Kumar Kori
>>>>
>>>>>-----Original Message-----
>>>>>From: Stephen Hemminger <stephen@networkplumber.org>
>>>>>Sent: Monday, December 16, 2019 9:43 PM
>>>>>To: Sunil Kumar Kori <skori@marvell.com>
>>>>>Cc: dev@dpdk.org
>>>>>Subject: [EXT] Re: [dpdk-dev] [PATCH] bus/pci: restricted bus
>>>>>scanning to allowed devices
>>>>>
>>>>>External Email
>>>>>
>>>>>--------------------------------------------------------------------
>>>>>-
>>>>>-
>>>>>> /* Create dummy pci device to get devargs */
>>>>>> + dummy_dev.addr.domain =
>>>>>matches[i].pc_sel.pc_domain;
>>>>>> + dummy_dev.addr.bus =
>matches[i].pc_sel.pc_bus;
>>>>>> + dummy_dev.addr.devid =
>matches[i].pc_sel.pc_dev;
>>>>>> + dummy_dev.addr.function =
>>>>>matches[i].pc_sel.pc_func;
>>>>>> + dummy_dev.device.devargs =
>>>>>> +
>>>>> pci_devargs_lookup(&dummy_dev);
>>>>>> +
>>>>>> + /* Check that device should be ignored or not
>*/
>>>>>> + if (pci_ignore_device(&dummy_dev))
>>>>>> + continue;
>>>>>
>>>>>It seems that you are creating dummy_dev as an alternative to
>>>>>passing just the PCI bus/device/function. Wouldn't be easier to just
>>>>>use BDF instead. Dummy arguments on the stack can lead to more
>>>>>corner cases in the future if device subsystem changes.
>>>>Agreed and initially I have implemented using BDF only instead of
>>>>using dummy device.
>>>>But using that approach, I was not able to use existing APIs to get
>>>>devargs and ignore device.
>>>>I had to write almost same functions to solve the purpose. So just to
>>>>avoid having replica of same code, I followed this approach. Please
>suggest.
>>>>>
>>>>>> +/**
>>>>>> + * Get the devargs of a PCI device.
>>>>>> + *
>>>>>> + * @param pci_dev
>>>>>> + * PCI device to be validated
>>>>>> + * @return
>>>>>> + * devargs on succes, NULL otherwise
>>>>>> + */
>>>>>> +struct rte_devargs *pci_devargs_lookup(struct rte_pci_device
>>>>>> +*pci_dev);
>>>>>
>>>>>Must be marked experimental (or internal).
>>>>>The pci_device should be marked const.
>>>>Okay but If I go with BDF one then this change is not required anyway.
>>>>>
>>>>>
>>>>>> +
>>>>>> +/**
>>>>>> + * Validate whether a pci device should be ignored or not.
>>>>>> + *
>>>>>> + * @param pci_dev
>>>>>> + * PCI device to be validated
>>>>>> + * @return
>>>>>> + * 1 if device is to be ignored, 0 otherwise
>>>>>> + */
>>>>>> +bool pci_ignore_device(const struct rte_pci_device *pci_dev);
>>>>>
>>>>>ditto
>>>>ditto
On Mon, Apr 6, 2020 at 11:32 AM Sunil Kumar Kori <skori@marvell.com> wrote:
> It looks like there is no comment/objection on following patch and it can be merged.
The title does not reflect what this patch is about.
I understand this as an optimisation for the pci bus scanning.
I agree with Stephen comment.
If you change pci_ignore_device and pci_devargs_lookup to take a
rte_pci_addr as input, then pci_ignore_device can call
pci_devargs_lookup itself.
And pci_devargs_lookup does not need to be exported in the private header.
Untested (just tried compilation),
https://github.com/david-marchand/dpdk/commit/e7860231ecdce91f9f70027d4090a7057b8fd5f7
Regards
Sunil Kumar Kori
>-----Original Message-----
>From: David Marchand <david.marchand@redhat.com>
>Sent: Monday, April 6, 2020 6:52 PM
>To: Sunil Kumar Kori <skori@marvell.com>
>Cc: Stephen Hemminger <stephen@networkplumber.org>; Jerin Jacob
>Kollanukkaran <jerinj@marvell.com>; dev@dpdk.org
>Subject: Re: [dpdk-dev] [EXT] Re: [PATCH] bus/pci: restricted bus scanning to
>allowed devices
>
>On Mon, Apr 6, 2020 at 11:32 AM Sunil Kumar Kori <skori@marvell.com>
>wrote:
>> It looks like there is no comment/objection on following patch and it can be
>merged.
>
>The title does not reflect what this patch is about.
>I understand this as an optimisation for the pci bus scanning.
>
>
>I agree with Stephen comment.
>
>If you change pci_ignore_device and pci_devargs_lookup to take a
>rte_pci_addr as input, then pci_ignore_device can call pci_devargs_lookup
>itself.
>And pci_devargs_lookup does not need to be exported in the private header.
>
>Untested (just tried compilation),
>https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_david-
>2Dmarchand_dpdk_commit_e7860231ecdce91f9f70027d4090a7057b8fd5f7&
>d=DwIFaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=dXeXaAMkP5COgn1zxHMyaF1_d
>9IIuq6vHQO6NrIPjaE&m=H7IQgd6gnpQBcbVgN952C_oIcMj5Z34kRhYngZr8Pyc
>&s=eq-hCdZ-C4xZ0Y7rQEkYwJS04r5sh7G2I9CXHTuBmws&e=
>
>
>--
>David Marchand
Okay. I will share updated version of this based on review comments.
@@ -338,6 +338,9 @@ rte_pci_scan(void)
.match_buf_len = sizeof(matches),
.matches = &matches[0],
};
+ struct rte_pci_device dummy_dev;
+
+ memset(&dummy_dev, 0, sizeof(struct rte_pci_device));
/* for debug purposes, PCI can be disabled */
if (!rte_eal_has_pci())
@@ -357,9 +360,22 @@ rte_pci_scan(void)
goto error;
}
- for (i = 0; i < conf_io.num_matches; i++)
+ for (i = 0; i < conf_io.num_matches; i++) {
+ /* Create dummy pci device to get devargs */
+ dummy_dev.addr.domain = matches[i].pc_sel.pc_domain;
+ dummy_dev.addr.bus = matches[i].pc_sel.pc_bus;
+ dummy_dev.addr.devid = matches[i].pc_sel.pc_dev;
+ dummy_dev.addr.function = matches[i].pc_sel.pc_func;
+ dummy_dev.device.devargs =
+ pci_devargs_lookup(&dummy_dev);
+
+ /* Check that device should be ignored or not */
+ if (pci_ignore_device(&dummy_dev))
+ continue;
+
if (pci_scan_one(fd, &matches[i]) < 0)
goto error;
+ }
dev_count += conf_io.num_matches;
} while(conf_io.status == PCI_GETCONF_MORE_DEVS);
@@ -458,6 +458,9 @@ rte_pci_scan(void)
DIR *dir;
char dirname[PATH_MAX];
struct rte_pci_addr addr;
+ struct rte_pci_device dummy_dev;
+
+ memset(&dummy_dev, 0, sizeof(struct rte_pci_device));
/* for debug purposes, PCI can be disabled */
if (!rte_eal_has_pci())
@@ -482,6 +485,14 @@ rte_pci_scan(void)
if (parse_pci_addr_format(e->d_name, sizeof(e->d_name), &addr) != 0)
continue;
+ /* Create dummy pci device to get devargs */
+ dummy_dev.addr = addr;
+ dummy_dev.device.devargs = pci_devargs_lookup(&dummy_dev);
+
+ /* Check that device should be ignored or not */
+ if (pci_ignore_device(&dummy_dev))
+ continue;
+
snprintf(dirname, sizeof(dirname), "%s/%s",
rte_pci_get_sysfs_path(), e->d_name);
@@ -42,7 +42,7 @@ const char *rte_pci_get_sysfs_path(void)
return path;
}
-static struct rte_devargs *pci_devargs_lookup(struct rte_pci_device *dev)
+struct rte_devargs *pci_devargs_lookup(struct rte_pci_device *dev)
{
struct rte_devargs *devargs;
struct rte_pci_addr addr;
@@ -589,7 +589,7 @@ pci_dma_unmap(struct rte_device *dev, void *addr, uint64_t iova, size_t len)
return -1;
}
-static bool
+bool
pci_ignore_device(const struct rte_pci_device *dev)
{
struct rte_devargs *devargs = dev->device.devargs;
@@ -42,6 +42,26 @@ int rte_pci_scan(void);
void
pci_name_set(struct rte_pci_device *dev);
+/**
+ * Get the devargs of a PCI device.
+ *
+ * @param pci_dev
+ * PCI device to be validated
+ * @return
+ * devargs on succes, NULL otherwise
+ */
+struct rte_devargs *pci_devargs_lookup(struct rte_pci_device *pci_dev);
+
+/**
+ * Validate whether a pci device should be ignored or not.
+ *
+ * @param pci_dev
+ * PCI device to be validated
+ * @return
+ * 1 if device is to be ignored, 0 otherwise
+ */
+bool pci_ignore_device(const struct rte_pci_device *pci_dev);
+
/**
* Add a PCI device to the PCI Bus (append to PCI Device list). This function
* also updates the bus references of the PCI Device (and the generic device