diff mbox series

[v6,2/4] lib/kni: add PCI related information

Message ID 20190625035700.2953-3-vattunuru@marvell.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series add IOVA = VA support in KNI |

Checks

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

Commit Message

Vamsi Krishna Attunuru June 25, 2019, 3:56 a.m. UTC 
  From: Vamsi Attunuru <vattunuru@marvell.com>

PCI related information is needed in KNI kernel module,
since it requires iommu domain info for address
translations(using iommu_iova_to_phys() call) when
KNI runs in IOVA = VA mode.

Signed-off-by: Vamsi Attunuru <vattunuru@marvell.com>
---
 lib/librte_eal/linux/eal/include/rte_kni_common.h | 7 +++++++
 lib/librte_kni/rte_kni.c                          | 5 +++++
 2 files changed, 12 insertions(+)

Comments

Stephen Hemminger June 25, 2019, 5:41 p.m. UTC | #1 
On Tue, 25 Jun 2019 09:26:58 +0530
<vattunuru@marvell.com> wrote:

> From: Vamsi Attunuru <vattunuru@marvell.com>
> 
> PCI related information is needed in KNI kernel module,
> since it requires iommu domain info for address
> translations(using iommu_iova_to_phys() call) when
> KNI runs in IOVA = VA mode.
> 
> Signed-off-by: Vamsi Attunuru <vattunuru@marvell.com>
> ---
>  lib/librte_eal/linux/eal/include/rte_kni_common.h | 7 +++++++
>  lib/librte_kni/rte_kni.c                          | 5 +++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/lib/librte_eal/linux/eal/include/rte_kni_common.h b/lib/librte_eal/linux/eal/include/rte_kni_common.h
> index 91a1c14..5db5a13 100644
> --- a/lib/librte_eal/linux/eal/include/rte_kni_common.h
> +++ b/lib/librte_eal/linux/eal/include/rte_kni_common.h
> @@ -111,6 +111,13 @@ struct rte_kni_device_info {
>  	void * mbuf_va;
>  	phys_addr_t mbuf_phys;
>  
> +	/* PCI info */
> +	uint16_t vendor_id;           /**< Vendor ID or PCI_ANY_ID. */
> +	uint16_t device_id;           /**< Device ID or PCI_ANY_ID. */
> +	uint8_t bus;                  /**< Device bus */
> +	uint8_t devid;                /**< Device ID */
> +	uint8_t function;             /**< Device function. */
> +
>  	uint16_t group_id;            /**< Group ID */
>  	uint32_t core_id;             /**< core ID to bind for kernel thread */
>  
> diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c
> index e29d0cc..99c4bf5 100644
> --- a/lib/librte_kni/rte_kni.c
> +++ b/lib/librte_kni/rte_kni.c
> @@ -242,6 +242,11 @@ rte_kni_alloc(struct rte_mempool *pktmbuf_pool,
>  		kni->ops.port_id = UINT16_MAX;
>  
>  	memset(&dev_info, 0, sizeof(dev_info));
> +	dev_info.bus = conf->addr.bus;
> +	dev_info.devid = conf->addr.devid;
> +	dev_info.function = conf->addr.function;
> +	dev_info.vendor_id = conf->id.vendor_id;
> +	dev_info.device_id = conf->id.device_id;
>  	dev_info.core_id = conf->core_id;
>  	dev_info.force_bind = conf->force_bind;
>  	dev_info.group_id = conf->group_id;


NAK

Why is PCI info part of KNI. KNI can be used with non-PCI devices now
Stephen Hemminger June 26, 2019, 2:58 p.m. UTC | #2 
On Wed, 26 Jun 2019 03:48:12 +0000
Vamsi Krishna Attunuru <vattunuru@marvell.com> wrote:

> ________________________________
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Tuesday, June 25, 2019 11:11 PM
> To: Vamsi Krishna Attunuru
> Cc: dev@dpdk.org; ferruh.yigit@intel.com; olivier.matz@6wind.com; arybchenko@solarflare.com
> Subject: [EXT] Re: [dpdk-dev] [PATCH v6 2/4] lib/kni: add PCI related information
> 
> External Email
> 
> ----------------------------------------------------------------------
> On Tue, 25 Jun 2019 09:26:58 +0530
> <vattunuru@marvell.com> wrote:
> 
> > From: Vamsi Attunuru <vattunuru@marvell.com>
> >
> > PCI related information is needed in KNI kernel module,
> > since it requires iommu domain info for address
> > translations(using iommu_iova_to_phys() call) when
> > KNI runs in IOVA = VA mode.
> >
> > Signed-off-by: Vamsi Attunuru <vattunuru@marvell.com>
> > ---
> >  lib/librte_eal/linux/eal/include/rte_kni_common.h | 7 +++++++
> >  lib/librte_kni/rte_kni.c                          | 5 +++++
> >  2 files changed, 12 insertions(+)
> >
> > diff --git a/lib/librte_eal/linux/eal/include/rte_kni_common.h b/lib/librte_eal/linux/eal/include/rte_kni_common.h
> > index 91a1c14..5db5a13 100644
> > --- a/lib/librte_eal/linux/eal/include/rte_kni_common.h
> > +++ b/lib/librte_eal/linux/eal/include/rte_kni_common.h
> > @@ -111,6 +111,13 @@ struct rte_kni_device_info {
> >        void * mbuf_va;
> >        phys_addr_t mbuf_phys;
> >
> > +     /* PCI info */
> > +     uint16_t vendor_id;           /**< Vendor ID or PCI_ANY_ID. */
> > +     uint16_t device_id;           /**< Device ID or PCI_ANY_ID. */
> > +     uint8_t bus;                  /**< Device bus */
> > +     uint8_t devid;                /**< Device ID */
> > +     uint8_t function;             /**< Device function. */
> > +
> >        uint16_t group_id;            /**< Group ID */
> >        uint32_t core_id;             /**< core ID to bind for kernel thread */
> >
> > diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c
> > index e29d0cc..99c4bf5 100644
> > --- a/lib/librte_kni/rte_kni.c
> > +++ b/lib/librte_kni/rte_kni.c
> > @@ -242,6 +242,11 @@ rte_kni_alloc(struct rte_mempool *pktmbuf_pool,
> >                kni->ops.port_id = UINT16_MAX;
> >
> >        memset(&dev_info, 0, sizeof(dev_info));
> > +     dev_info.bus = conf->addr.bus;
> > +     dev_info.devid = conf->addr.devid;
> > +     dev_info.function = conf->addr.function;
> > +     dev_info.vendor_id = conf->id.vendor_id;
> > +     dev_info.device_id = conf->id.device_id;
> >        dev_info.core_id = conf->core_id;
> >        dev_info.force_bind = conf->force_bind;
> >        dev_info.group_id = conf->group_id;  
> 
> 
> NAK
> 
> Why is PCI info part of KNI. KNI can be used with non-PCI devices now
> 
> Kernel KNI module needs device info(PCI in this case) to figure out iommu domain details to work with IOVA=VA mode and the struct rte_kni_device_info only carries KNI device info to the kernel module, if not here, where do you suggest me to put this info.

The kernel KNI driver works as is with netvsc PMD (which is vmbus).
Your code may break that. If needs to work for both.
Ferruh Yigit July 11, 2019, 4:22 p.m. UTC | #3 
On 6/25/2019 4:56 AM, vattunuru@marvell.com wrote:
> From: Vamsi Attunuru <vattunuru@marvell.com>
> 
> PCI related information is needed in KNI kernel module,
> since it requires iommu domain info for address
> translations(using iommu_iova_to_phys() call) when
> KNI runs in IOVA = VA mode.
> 
> Signed-off-by: Vamsi Attunuru <vattunuru@marvell.com>
> ---
>  lib/librte_eal/linux/eal/include/rte_kni_common.h | 7 +++++++
>  lib/librte_kni/rte_kni.c                          | 5 +++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/lib/librte_eal/linux/eal/include/rte_kni_common.h b/lib/librte_eal/linux/eal/include/rte_kni_common.h
> index 91a1c14..5db5a13 100644
> --- a/lib/librte_eal/linux/eal/include/rte_kni_common.h
> +++ b/lib/librte_eal/linux/eal/include/rte_kni_common.h
> @@ -111,6 +111,13 @@ struct rte_kni_device_info {
>  	void * mbuf_va;
>  	phys_addr_t mbuf_phys;
>  
> +	/* PCI info */
> +	uint16_t vendor_id;           /**< Vendor ID or PCI_ANY_ID. */
> +	uint16_t device_id;           /**< Device ID or PCI_ANY_ID. */
> +	uint8_t bus;                  /**< Device bus */
> +	uint8_t devid;                /**< Device ID */
> +	uint8_t function;             /**< Device function. */
> +
>  	uint16_t group_id;            /**< Group ID */
>  	uint32_t core_id;             /**< core ID to bind for kernel thread */
>  
> diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c
> index e29d0cc..99c4bf5 100644
> --- a/lib/librte_kni/rte_kni.c
> +++ b/lib/librte_kni/rte_kni.c
> @@ -242,6 +242,11 @@ rte_kni_alloc(struct rte_mempool *pktmbuf_pool,
>  		kni->ops.port_id = UINT16_MAX;
>  
>  	memset(&dev_info, 0, sizeof(dev_info));
> +	dev_info.bus = conf->addr.bus;
> +	dev_info.devid = conf->addr.devid;
> +	dev_info.function = conf->addr.function;
> +	dev_info.vendor_id = conf->id.vendor_id;
> +	dev_info.device_id = conf->id.device_id;

Sample application part to set 'conf' values were also removed, need to add them
back too.

The device part was removed from KNI but I can see why this patch requires it back.
Ferruh Yigit July 12, 2019, 11:11 a.m. UTC | #4 
On 7/12/2019 12:02 PM, Vamsi Krishna Attunuru wrote:
> 
> 
> 
> --------------------------------------------------------------------------------
> *From:* dev <dev-bounces@dpdk.org> on behalf of Ferruh Yigit
> <ferruh.yigit@intel.com>
> *Sent:* Thursday, July 11, 2019 9:52 PM
> *To:* Vamsi Krishna Attunuru; dev@dpdk.org
> *Cc:* olivier.matz@6wind.com; arybchenko@solarflare.com
> *Subject:* [EXT] Re: [dpdk-dev] [PATCH v6 2/4] lib/kni: add PCI related information
>  
> External Email
> 
> ----------------------------------------------------------------------
> On 6/25/2019 4:56 AM, vattunuru@marvell.com wrote:
>> From: Vamsi Attunuru <vattunuru@marvell.com>
>> 
>> PCI related information is needed in KNI kernel module,
>> since it requires iommu domain info for address
>> translations(using iommu_iova_to_phys() call) when
>> KNI runs in IOVA = VA mode.
>> 
>> Signed-off-by: Vamsi Attunuru <vattunuru@marvell.com>
>> ---
>>  lib/librte_eal/linux/eal/include/rte_kni_common.h | 7 +++++++
>>  lib/librte_kni/rte_kni.c                          | 5 +++++
>>  2 files changed, 12 insertions(+)
>> 
>> diff --git a/lib/librte_eal/linux/eal/include/rte_kni_common.h b/lib/librte_eal/linux/eal/include/rte_kni_common.h
>> index 91a1c14..5db5a13 100644
>> --- a/lib/librte_eal/linux/eal/include/rte_kni_common.h
>> +++ b/lib/librte_eal/linux/eal/include/rte_kni_common.h
>> @@ -111,6 +111,13 @@ struct rte_kni_device_info {
>>        void * mbuf_va;
>>        phys_addr_t mbuf_phys;
>>  
>> +     /* PCI info */
>> +     uint16_t vendor_id;           /**< Vendor ID or PCI_ANY_ID. */
>> +     uint16_t device_id;           /**< Device ID or PCI_ANY_ID. */
>> +     uint8_t bus;                  /**< Device bus */
>> +     uint8_t devid;                /**< Device ID */
>> +     uint8_t function;             /**< Device function. */
>> +
>>        uint16_t group_id;            /**< Group ID */
>>        uint32_t core_id;             /**< core ID to bind for kernel thread */
>>  
>> diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c
>> index e29d0cc..99c4bf5 100644
>> --- a/lib/librte_kni/rte_kni.c
>> +++ b/lib/librte_kni/rte_kni.c
>> @@ -242,6 +242,11 @@ rte_kni_alloc(struct rte_mempool *pktmbuf_pool,
>>                kni->ops.port_id = UINT16_MAX;
>>  
>>        memset(&dev_info, 0, sizeof(dev_info));
>> +     dev_info.bus = conf->addr.bus;
>> +     dev_info.devid = conf->addr.devid;
>> +     dev_info.function = conf->addr.function;
>> +     dev_info.vendor_id = conf->id.vendor_id;
>> +     dev_info.device_id = conf->id.device_id;
> 
> Sample application part to set 'conf' values were also removed, need to add them
> back too.
> 
> Ack, adding in next version of patches.
> 
> The device part was removed from KNI but I can see why this patch requires it back.
> 
> Yes, rte_kni_device_info is the only struct carries all required info to kernel
> module,
> all these device info can be protected under the config check though.

But still, lets make the device info requirement only for 'iova_mode', not for
all KNI
diff mbox series

Patch

diff --git a/lib/librte_eal/linux/eal/include/rte_kni_common.h b/lib/librte_eal/linux/eal/include/rte_kni_common.h
index 91a1c14..5db5a13 100644
--- a/lib/librte_eal/linux/eal/include/rte_kni_common.h
+++ b/lib/librte_eal/linux/eal/include/rte_kni_common.h
@@ -111,6 +111,13 @@  struct rte_kni_device_info {
 	void * mbuf_va;
 	phys_addr_t mbuf_phys;
 
+	/* PCI info */
+	uint16_t vendor_id;           /**< Vendor ID or PCI_ANY_ID. */
+	uint16_t device_id;           /**< Device ID or PCI_ANY_ID. */
+	uint8_t bus;                  /**< Device bus */
+	uint8_t devid;                /**< Device ID */
+	uint8_t function;             /**< Device function. */
+
 	uint16_t group_id;            /**< Group ID */
 	uint32_t core_id;             /**< core ID to bind for kernel thread */
 
diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c
index e29d0cc..99c4bf5 100644
--- a/lib/librte_kni/rte_kni.c
+++ b/lib/librte_kni/rte_kni.c
@@ -242,6 +242,11 @@  rte_kni_alloc(struct rte_mempool *pktmbuf_pool,
 		kni->ops.port_id = UINT16_MAX;
 
 	memset(&dev_info, 0, sizeof(dev_info));
+	dev_info.bus = conf->addr.bus;
+	dev_info.devid = conf->addr.devid;
+	dev_info.function = conf->addr.function;
+	dev_info.vendor_id = conf->id.vendor_id;
+	dev_info.device_id = conf->id.device_id;
 	dev_info.core_id = conf->core_id;
 	dev_info.force_bind = conf->force_bind;
 	dev_info.group_id = conf->group_id;