[dpdk-dev] [PATCH v7 2/9] eal/pci: get iommu class

santosh santosh.shukla at caviumnetworks.com
Mon Sep 4 17:31:58 CEST 2017


Hi Anatoly,


On Monday 04 September 2017 08:46 PM, Burakov, Anatoly wrote:
> Hi Santosh,
>
>> From: santosh [mailto:santosh.shukla at caviumnetworks.com]
>> Sent: Monday, September 4, 2017 4:14 PM
>> To: Burakov, Anatoly <anatoly.burakov at intel.com>; dev at dpdk.org
>> Cc: thomas at monjalon.net; jerin.jacob at caviumnetworks.com;
>> hemant.agrawal at nxp.com; olivier.matz at 6wind.com;
>> maxime.coquelin at redhat.com; Gonzalez Monroy, Sergio
>> <sergio.gonzalez.monroy at intel.com>; Richardson, Bruce
>> <bruce.richardson at intel.com>; shreyansh.jain at nxp.com;
>> gaetan.rivet at 6wind.com; stephen at networkplumber.org;
>> aconole at redhat.com
>> Subject: Re: [PATCH v7 2/9] eal/pci: get iommu class
>>
>> Hi Anatoly,
>>
>>
>> On Monday 04 September 2017 08:23 PM, Burakov, Anatoly wrote:
>>>> From: Santosh Shukla [mailto:santosh.shukla at caviumnetworks.com]
>>>> Sent: Thursday, August 31, 2017 4:26 AM
>>>> To: dev at dpdk.org
>>>> Cc: thomas at monjalon.net; jerin.jacob at caviumnetworks.com;
>>>> hemant.agrawal at nxp.com; olivier.matz at 6wind.com;
>>>> maxime.coquelin at redhat.com; Gonzalez Monroy, Sergio
>>>> <sergio.gonzalez.monroy at intel.com>; Richardson, Bruce
>>>> <bruce.richardson at intel.com>; shreyansh.jain at nxp.com;
>>>> gaetan.rivet at 6wind.com; Burakov, Anatoly
>> <anatoly.burakov at intel.com>;
>>>> stephen at networkplumber.org; aconole at redhat.com; Santosh Shukla
>>>> <santosh.shukla at caviumnetworks.com>
>>>> Subject: [PATCH v7 2/9] eal/pci: get iommu class
>>>>
>>>> Introducing rte_pci_get_iommu_class API which helps to get iommu
>>>> class of PCI device on the bus and returns preferred iova mapping mode
>> for PCI bus.
>>>> Patch also add rte_pci_get_iommu_class definition for bsdapp, in
>>>> bsdapp case - api returns default iova mode.
>>>>
>>>> Signed-off-by: Santosh Shukla <santosh.shukla at caviumnetworks.com>
>>>> Signed-off-by: Jerin Jacob <jerin.jacob at caviumnetworks.com>
>>>> Reviewed-by: Maxime Coquelin <maxime.coquelin at redhat.com>
>>>> ---
>>>> v6 --> v7:
>>>> - squashed v6 series patch [02/12] & [03/12] (Aaron comment).
>>>>
>>>>  lib/librte_eal/bsdapp/eal/eal_pci.c           | 10 ++++++++++
>>>>  lib/librte_eal/bsdapp/eal/rte_eal_version.map |  1 +
>>>>  lib/librte_eal/common/include/rte_bus.h       | 10 ++++++++++
>>>>  lib/librte_eal/common/include/rte_pci.h       | 11 +++++++++++
>>>>  4 files changed, 32 insertions(+)
>>>>
>>>> diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c
>>>> b/lib/librte_eal/bsdapp/eal/eal_pci.c
>>>> index 04eacdcc7..e2c252320 100644
>>>> --- a/lib/librte_eal/bsdapp/eal/eal_pci.c
>>>> +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
>>>> @@ -403,6 +403,16 @@ rte_pci_scan(void)
>>>>  	return -1;
>>>>  }
>>>>
>>>> +/*
>>>> + * Get iommu class of pci devices on the bus.
>>>> + */
>>>> +enum rte_iova_mode
>>>> +rte_pci_get_iommu_class(void)
>>>> +{
>>>> +	/* Supports only RTE_KDRV_NIC_UIO */
>>>> +	return RTE_IOVA_PA;
>>>> +}
>>>> +
>>>>  int
>>>>  pci_update_device(const struct rte_pci_addr *addr)  { diff --git
>>>> a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
>>>> b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
>>>> index c819e3084..1fdcfb460 100644
>>>> --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
>>>> +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
>>>> @@ -242,5 +242,6 @@ DPDK_17.11 {
>>>>  	global:
>>>>
>>>>  	rte_pci_match;
>>>> +	rte_pci_get_iommu_class;
>>>>
>>>>  } DPDK_17.08;
>>>> diff --git a/lib/librte_eal/common/include/rte_bus.h
>>>> b/lib/librte_eal/common/include/rte_bus.h
>>>> index c79368d3c..9e40687e5 100644
>>>> --- a/lib/librte_eal/common/include/rte_bus.h
>>>> +++ b/lib/librte_eal/common/include/rte_bus.h
>>>> @@ -55,6 +55,16 @@ extern "C" {
>>>>  /** Double linked list of buses */
>>>>  TAILQ_HEAD(rte_bus_list, rte_bus);
>>>>
>>>> +
>>>> +/**
>>>> + * IOVA mapping mode.
>>>> + */
>>>> +enum rte_iova_mode {
>>>> +	RTE_IOVA_DC = 0,	/* Don't care mode */
>>>> +	RTE_IOVA_PA = (1 << 0),
>>>> +	RTE_IOVA_VA = (1 << 1)
>>> Hi Santosh,
>>>
>>> No need to set values explicitly, standard C will take care of it.
>> no strong opinion, change queued for v8.

recalling myself on why expressed RTE_IOVA_PA/_VA as 1 << 0/1.
Since user in future (by mistake) may add new entry example: RTE_IOVA_XX = 3 then it will
enable both _pa and _va both, So to avoid such programming error, deliberately
kept _pa = 1 << 0 and _va = 1 << 1, if new entry comes (highly unlikely) then
should be programmed as _xx = 1 << 2;

If you convinced then I think - i don;t need to spin this change for v8. 

>>> I wonder the purpose of "don't care" mode. It's not used for anything but
>> cases when no driver is bound. All the libraries (e.g. rte_malloc) will only
>> check for IOVA_VA mode. Can't we just used PA in all cases where IOVA_DC
>> would be applicable?
>>
>> Indeed policy is to use iova_pa for _dc case, but we need a way to distinguish
>> between no device found vs device attached (if attached then decide upon
>> its iova scheme).
>>
>> For more detailed info pl. refer [1].
>>
>> [1] http://dpdk.org/dev/patchwork/patch/26762/
>>
> Maybe make your intentions more explicit then? I.e. instead of "don't care" use "no device" or some such. No strong opinion either way though, I'm fine with leaving it as is.

prefer keeping _DC, if you don;t mind, sounds more appropriate to me. 

> Thanks,
> Anatoly



More information about the dev mailing list