diff mbox

[dpdk-dev,v8,2/9] eal/pci: get iommu class

Message ID 20170918104234.9149-3-santosh.shukla@caviumnetworks.com (mailing list archive)
State Superseded, archived
Headers

Checks

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

Commit Message

Santosh Shukla Sept. 18, 2017, 10:42 a.m. UTC 
  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@caviumnetworks.com>
Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@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(+)

Comments

Anatoly Burakov Sept. 19, 2017, 4:37 p.m. UTC | #1 
On 18-Sep-17 11:42 AM, Santosh Shukla wrote:
> 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>
> ---

Hi Santosh,

You have probably missed my comment on previous version of this patch, 
but for commit history reasons i really think you should add a linuxapp 
stub in this commit as well as a FreeBSD stub, even though you are 
adding a linuxapp function in the next commit. Any linuxapp application 
using that function will fail to compile with this commit, despite this 
API being already present and declared as public.
Santosh Shukla Sept. 19, 2017, 5:29 p.m. UTC | #2 
Hi Anatoly,


On Tuesday 19 September 2017 10:07 PM, Burakov, Anatoly wrote:
> On 18-Sep-17 11:42 AM, Santosh Shukla wrote:
>> 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>
>> ---
>
> Hi Santosh,
>
> You have probably missed my comment on previous version of this patch, but for commit history reasons i really think you should add a linuxapp stub in this commit as well as a FreeBSD stub, even though you are adding a linuxapp function in the next commit. Any linuxapp application using that function will fail to compile with this commit, despite this API being already present and declared as public.
>
First, apologies for not following up on your note:

I prefer to keep less context in each patch and 
for [03/9], its already has _IOVA_AS_VA flag + whole autodetection 
algo inside (squashed per Aron suggestion).
 
Now if I squash [2/9] into [3/9], then would be too much info 
for future reader to digest for (imo). Its a kind of trade-off.

On any linuxapp appl breaking with this commit: 
This series exposes eal api for application to use and identify iova mode.

If you still feel not convinced with my explanation then I'll spin v9 and squash
[02/09], [03/09] in v9.

Thanks.
Anatoly Burakov Sept. 20, 2017, 9:09 a.m. UTC | #3 
Hi Santosh,

On 19-Sep-17 6:29 PM, santosh wrote:
> Hi Anatoly,
> 
> 
> On Tuesday 19 September 2017 10:07 PM, Burakov, Anatoly wrote:
>> On 18-Sep-17 11:42 AM, Santosh Shukla wrote:
>>> 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>
>>> ---
>>
>> Hi Santosh,
>>
>> You have probably missed my comment on previous version of this patch, but for commit history reasons i really think you should add a linuxapp stub in this commit as well as a FreeBSD stub, even though you are adding a linuxapp function in the next commit. Any linuxapp application using that function will fail to compile with this commit, despite this API being already present and declared as public.
>>
> First, apologies for not following up on your note:
> 
> I prefer to keep less context in each patch and
> for [03/9], its already has _IOVA_AS_VA flag + whole autodetection
> algo inside (squashed per Aron suggestion).
>   
> Now if I squash [2/9] into [3/9], then would be too much info
> for future reader to digest for (imo). Its a kind of trade-off.
> 
> On any linuxapp appl breaking with this commit:
> This series exposes eal api for application to use and identify iova mode.
> 
> If you still feel not convinced with my explanation then I'll spin v9 and squash
> [02/09], [03/09] in v9.

No, i don't mean squashing these two patches into one. I mean, provide a 
stub like for FreeBSD, and then edit it to be a proper implementation in 
the next commit.

I.e. in this commit, add a stub that just returns 0, like for FreeBSD. 
Next commit, instead of starting from scratch, start from this stub.

Thanks,
Anatoly

> 
> Thanks.
> 
> 
>
Santosh Shukla Sept. 20, 2017, 10:24 a.m. UTC | #4 
Hi Anatoly,


On Wednesday 20 September 2017 02:39 PM, Burakov, Anatoly wrote:
> Hi Santosh,
>
> On 19-Sep-17 6:29 PM, santosh wrote:
>> Hi Anatoly,
>>
>>
>> On Tuesday 19 September 2017 10:07 PM, Burakov, Anatoly wrote:
>>> On 18-Sep-17 11:42 AM, Santosh Shukla wrote:
>>>> 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>
>>>> ---
>>>
>>> Hi Santosh,
>>>
>>> You have probably missed my comment on previous version of this patch, but for commit history reasons i really think you should add a linuxapp stub in this commit as well as a FreeBSD stub, even though you are adding a linuxapp function in the next commit. Any linuxapp application using that function will fail to compile with this commit, despite this API being already present and declared as public.
>>>
>> First, apologies for not following up on your note:
>>
>> I prefer to keep less context in each patch and
>> for [03/9], its already has _IOVA_AS_VA flag + whole autodetection
>> algo inside (squashed per Aron suggestion).
>>   Now if I squash [2/9] into [3/9], then would be too much info
>> for future reader to digest for (imo). Its a kind of trade-off.
>>
>> On any linuxapp appl breaking with this commit:
>> This series exposes eal api for application to use and identify iova mode.
>>
>> If you still feel not convinced with my explanation then I'll spin v9 and squash
>> [02/09], [03/09] in v9.
>
> No, i don't mean squashing these two patches into one. I mean, provide a stub like for FreeBSD, and then edit it to be a proper implementation in the next commit.
>
> I.e. in this commit, add a stub that just returns 0, like for FreeBSD. Next commit, instead of starting from scratch, start from this stub.
>
+1, Sending v9.

Thanks.

> Thanks,
> Anatoly
>
>>
>> Thanks.
>>
>>
>>
>
>
diff mbox

Patch

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 cfbf8fbd0..c6ffd9399 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -243,5 +243,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)
+};
+
 /**
  * Bus specific scan for devices attached on the bus.
  * For each bus object, the scan would be responsible for finding devices and
diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index eab84c7a4..0e36de093 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -381,6 +381,17 @@  int
 rte_pci_match(const struct rte_pci_driver *pci_drv,
 	      const struct rte_pci_device *pci_dev);
 
+
+/**
+ * Get iommu class of PCI devices on the bus.
+ * And return their preferred iova mapping mode.
+ *
+ * @return
+ *   - enum rte_iova_mode.
+ */
+enum rte_iova_mode
+rte_pci_get_iommu_class(void);
+
 /**
  * Map the PCI device resources in user space virtual memory address
  *