Hi Anatoly,
On Monday 04 September 2017 08:46 PM, Burakov, Anatoly wrote: > Hi Santosh, > >> From: santosh [mailto:santosh.shu...@caviumnetworks.com] >> Sent: Monday, September 4, 2017 4:14 PM >> To: Burakov, Anatoly <anatoly.bura...@intel.com>; dev@dpdk.org >> Cc: tho...@monjalon.net; jerin.ja...@caviumnetworks.com; >> hemant.agra...@nxp.com; olivier.m...@6wind.com; >> maxime.coque...@redhat.com; Gonzalez Monroy, Sergio >> <sergio.gonzalez.mon...@intel.com>; Richardson, Bruce >> <bruce.richard...@intel.com>; shreyansh.j...@nxp.com; >> gaetan.ri...@6wind.com; step...@networkplumber.org; >> acon...@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.shu...@caviumnetworks.com] >>>> Sent: Thursday, August 31, 2017 4:26 AM >>>> To: dev@dpdk.org >>>> Cc: tho...@monjalon.net; jerin.ja...@caviumnetworks.com; >>>> hemant.agra...@nxp.com; olivier.m...@6wind.com; >>>> maxime.coque...@redhat.com; Gonzalez Monroy, Sergio >>>> <sergio.gonzalez.mon...@intel.com>; Richardson, Bruce >>>> <bruce.richard...@intel.com>; shreyansh.j...@nxp.com; >>>> gaetan.ri...@6wind.com; Burakov, Anatoly >> <anatoly.bura...@intel.com>; >>>> step...@networkplumber.org; acon...@redhat.com; Santosh Shukla >>>> <santosh.shu...@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.shu...@caviumnetworks.com> >>>> Signed-off-by: Jerin Jacob <jerin.ja...@caviumnetworks.com> >>>> Reviewed-by: Maxime Coquelin <maxime.coque...@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
