diff mbox

[dpdk-dev] fm10k: support XEN domain0

Message ID 1431680162-13704-1-git-send-email-shaopeng.he@intel.com (mailing list archive)
State Accepted, archived
Headers

Commit Message

He, Shaopeng May 15, 2015, 8:56 a.m. UTC 
  fm10k was failing to run in XEN domain0, as the physical
memory for DMA should be allocated and translated
in a different way for XEN domain0. So
rte_memzone_reserve_bounded() should be used for DMA
memory allocation, and rte_mem_phy2mch() should be used
for DMA memory address translation to support running
fm10k PMD in XEN domain0.

Signed-off-by: Shaopeng He <shaopeng.he@intel.com>
---
 lib/librte_pmd_fm10k/fm10k_ethdev.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Thomas Monjalon May 15, 2015, 10:21 a.m. UTC | #1 
Hi,

2015-05-15 16:56, Shaopeng He:
> +#ifdef RTE_LIBRTE_XEN_DOM0
> +	q->hw_ring_phys_addr = rte_mem_phy2mch(mz->memseg_id, mz->phys_addr);
> +#else
>  	q->hw_ring_phys_addr = mz->phys_addr;
> +#endif

I know this is already done this way in other drivers, but don't you think
it's time to create a function to get physical address from a memzone?
So we could remove these "ifdef Xen" from every drivers.

Thanks
Stephen Hemminger May 15, 2015, 11:57 p.m. UTC | #2 
On Fri, 15 May 2015 16:56:02 +0800
Shaopeng He <shaopeng.he@intel.com> wrote:

> fm10k was failing to run in XEN domain0, as the physical
> memory for DMA should be allocated and translated
> in a different way for XEN domain0. So
> rte_memzone_reserve_bounded() should be used for DMA
> memory allocation, and rte_mem_phy2mch() should be used
> for DMA memory address translation to support running
> fm10k PMD in XEN domain0.
> 
> Signed-off-by: Shaopeng He <shaopeng.he@intel.com>

I agree with Thomas that this code has spread everywhere and should
be in a common spot.

Also, we discovered as part of the Xen net-front driver that it
should be a runtime determination, not a config option!
Jijiang Liu May 18, 2015, 2:23 a.m. UTC | #3 
Hi guys,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hemminger
> Sent: Saturday, May 16, 2015 7:58 AM
> To: He, Shaopeng
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] fm10k: support XEN domain0
> 
> On Fri, 15 May 2015 16:56:02 +0800
> Shaopeng He <shaopeng.he@intel.com> wrote:
> 
> > fm10k was failing to run in XEN domain0, as the physical memory for
> > DMA should be allocated and translated in a different way for XEN
> > domain0. So
> > rte_memzone_reserve_bounded() should be used for DMA memory
> > allocation, and rte_mem_phy2mch() should be used for DMA memory
> > address translation to support running fm10k PMD in XEN domain0.
> >
> > Signed-off-by: Shaopeng He <shaopeng.he@intel.com>
> 
> I agree with Thomas that this code has spread everywhere and should be in a
> common spot.
> 
> Also, we discovered as part of the Xen net-front driver that it should be a
> runtime determination, not a config option!

I also agree that it should be in a common spot.
But  it had better to apply the following Stephen's patch first. If so, Shaopeng just use the common function in the patch, which would be good.  
http://dpdk.org/ml/archives/dev/2015-March/014992.html
He, Shaopeng June 2, 2015, 3:27 a.m. UTC | #4 
Hi Stephen,

Do you see any problem of preventing your patch of "xen: allow choosing dom0 support at runtime" into upcoming 2.1 release? It will be better that fm10k's XEN support can base on your modifications, and I will rework this fm10k patch.

But in case your patch cannot make it into 2.1, to enable users to use fm10k with XEN domain0 in next release, should we enable this function by using current approach first?

Thanks,
--Shaopeng

-----Original Message-----
From: Liu, Jijiang 
Sent: Monday, May 18, 2015 10:23 AM
To: Stephen Hemminger; He, Shaopeng; Thomas Monjalon
Cc: dev@dpdk.org
Subject: RE: [dpdk-dev] [PATCH] fm10k: support XEN domain0

Hi guys,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hemminger
> Sent: Saturday, May 16, 2015 7:58 AM
> To: He, Shaopeng
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] fm10k: support XEN domain0
> 
> On Fri, 15 May 2015 16:56:02 +0800
> Shaopeng He <shaopeng.he@intel.com> wrote:
> 
> > fm10k was failing to run in XEN domain0, as the physical memory for 
> > DMA should be allocated and translated in a different way for XEN 
> > domain0. So
> > rte_memzone_reserve_bounded() should be used for DMA memory 
> > allocation, and rte_mem_phy2mch() should be used for DMA memory 
> > address translation to support running fm10k PMD in XEN domain0.
> >
> > Signed-off-by: Shaopeng He <shaopeng.he@intel.com>
> 
> I agree with Thomas that this code has spread everywhere and should be 
> in a common spot.
> 
> Also, we discovered as part of the Xen net-front driver that it should 
> be a runtime determination, not a config option!

I also agree that it should be in a common spot.
But  it had better to apply the following Stephen's patch first. If so, Shaopeng just use the common function in the patch, which would be good.  
http://dpdk.org/ml/archives/dev/2015-March/014992.html
Jijiang Liu June 5, 2015, 3:17 a.m. UTC | #5 
Acked-by: Jijiang Liu <Jijiang.liu@intel.com>

I think this patch could be merged before Stephen's following patch[1] is merged, then Stephen should rework the patch[1].
Thanks.

[1]http://dpdk.org/ml/archives/dev/2015-March/014992.html

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Shaopeng He
> Sent: Friday, May 15, 2015 4:56 PM
> To: dev@dpdk.org
> Cc: He, Shaopeng
> Subject: [dpdk-dev] [PATCH] fm10k: support XEN domain0
> 
> fm10k was failing to run in XEN domain0, as the physical memory for DMA
> should be allocated and translated in a different way for XEN domain0. So
> rte_memzone_reserve_bounded() should be used for DMA memory
> allocation, and rte_mem_phy2mch() should be used for DMA memory
> address translation to support running fm10k PMD in XEN domain0.
> 
> Signed-off-by: Shaopeng He <shaopeng.he@intel.com>
> ---
>  lib/librte_pmd_fm10k/fm10k_ethdev.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/lib/librte_pmd_fm10k/fm10k_ethdev.c
> b/lib/librte_pmd_fm10k/fm10k_ethdev.c
> index 275c19c..c85c856 100644
> --- a/lib/librte_pmd_fm10k/fm10k_ethdev.c
> +++ b/lib/librte_pmd_fm10k/fm10k_ethdev.c
> @@ -1004,7 +1004,11 @@ fm10k_rx_queue_setup(struct rte_eth_dev *dev,
> uint16_t queue_id,
>  		return (-ENOMEM);
>  	}
>  	q->hw_ring = mz->addr;
> +#ifdef RTE_LIBRTE_XEN_DOM0
> +	q->hw_ring_phys_addr = rte_mem_phy2mch(mz->memseg_id, mz-
> >phys_addr);
> +#else
>  	q->hw_ring_phys_addr = mz->phys_addr;
> +#endif
> 
>  	dev->data->rx_queues[queue_id] = q;
>  	return 0;
> @@ -1150,7 +1154,11 @@ fm10k_tx_queue_setup(struct rte_eth_dev *dev,
> uint16_t queue_id,
>  		return (-ENOMEM);
>  	}
>  	q->hw_ring = mz->addr;
> +#ifdef RTE_LIBRTE_XEN_DOM0
> +	q->hw_ring_phys_addr = rte_mem_phy2mch(mz->memseg_id, mz-
> >phys_addr);
> +#else
>  	q->hw_ring_phys_addr = mz->phys_addr;
> +#endif
> 
>  	/*
>  	 * allocate memory for the RS bit tracker. Enough slots to hold the
> --
> 1.9.3
He, Shaopeng June 23, 2015, 1:21 a.m. UTC | #6 
Hi Thomas,

> -----Original Message-----
> From: Liu, Jijiang
> Sent: Friday, June 05, 2015 11:18 AM
> To: dev@dpdk.org
> Cc: He, Shaopeng
> Subject: RE: [dpdk-dev] [PATCH] fm10k: support XEN domain0
> 
> 
> Acked-by: Jijiang Liu <Jijiang.liu@intel.com>
> 
> I think this patch could be merged before Stephen's following patch[1] is
> merged, then Stephen should rework the patch[1].
> Thanks.
> 
> [1]http://dpdk.org/ml/archives/dev/2015-March/014992.html

Do you think we can accept this patch in current no-so-elegant way, so user can
use XEN with fm10k from release 2.1; or better to wait for Stephen's patch?
Thank you in advance for your attention to this matter.

Best Regards,
--Shaopeng

> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Shaopeng He
> > Sent: Friday, May 15, 2015 4:56 PM
> > To: dev@dpdk.org
> > Cc: He, Shaopeng
> > Subject: [dpdk-dev] [PATCH] fm10k: support XEN domain0
> >
> > fm10k was failing to run in XEN domain0, as the physical memory for DMA
> > should be allocated and translated in a different way for XEN domain0. So
> > rte_memzone_reserve_bounded() should be used for DMA memory
> > allocation, and rte_mem_phy2mch() should be used for DMA memory
> > address translation to support running fm10k PMD in XEN domain0.
> >
> > Signed-off-by: Shaopeng He <shaopeng.he@intel.com>
> > ---
> >  lib/librte_pmd_fm10k/fm10k_ethdev.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/lib/librte_pmd_fm10k/fm10k_ethdev.c
> > b/lib/librte_pmd_fm10k/fm10k_ethdev.c
> > index 275c19c..c85c856 100644
> > --- a/lib/librte_pmd_fm10k/fm10k_ethdev.c
> > +++ b/lib/librte_pmd_fm10k/fm10k_ethdev.c
> > @@ -1004,7 +1004,11 @@ fm10k_rx_queue_setup(struct rte_eth_dev
> *dev,
> > uint16_t queue_id,
> >  		return (-ENOMEM);
> >  	}
> >  	q->hw_ring = mz->addr;
> > +#ifdef RTE_LIBRTE_XEN_DOM0
> > +	q->hw_ring_phys_addr = rte_mem_phy2mch(mz->memseg_id, mz-
> > >phys_addr);
> > +#else
> >  	q->hw_ring_phys_addr = mz->phys_addr;
> > +#endif
> >
> >  	dev->data->rx_queues[queue_id] = q;
> >  	return 0;
> > @@ -1150,7 +1154,11 @@ fm10k_tx_queue_setup(struct rte_eth_dev
> *dev,
> > uint16_t queue_id,
> >  		return (-ENOMEM);
> >  	}
> >  	q->hw_ring = mz->addr;
> > +#ifdef RTE_LIBRTE_XEN_DOM0
> > +	q->hw_ring_phys_addr = rte_mem_phy2mch(mz->memseg_id, mz-
> > >phys_addr);
> > +#else
> >  	q->hw_ring_phys_addr = mz->phys_addr;
> > +#endif
> >
> >  	/*
> >  	 * allocate memory for the RS bit tracker. Enough slots to hold the
> > --
> > 1.9.3
>
He, Shaopeng June 30, 2015, 3:27 a.m. UTC | #7 
Hi Thomas, Stephen,

> -----Original Message-----
> From: He, Shaopeng
> Sent: Tuesday, June 23, 2015 9:21 AM
> To: Thomas Monjalon
> Cc: Liu, Jijiang; dev@dpdk.org; Stephen Hemminger
> Subject: RE: [dpdk-dev] [PATCH] fm10k: support XEN domain0
> 
> Hi Thomas,
> 
> > -----Original Message-----
> > From: Liu, Jijiang
> > Sent: Friday, June 05, 2015 11:18 AM
> > To: dev@dpdk.org
> > Cc: He, Shaopeng
> > Subject: RE: [dpdk-dev] [PATCH] fm10k: support XEN domain0
> >
> >
> > Acked-by: Jijiang Liu <Jijiang.liu@intel.com>
> >
> > I think this patch could be merged before Stephen's following patch[1] is
> > merged, then Stephen should rework the patch[1].
> > Thanks.
> >
> > [1]http://dpdk.org/ml/archives/dev/2015-March/014992.html
> 
> Do you think we can accept this patch in current no-so-elegant way, so user
> can
> use XEN with fm10k from release 2.1; or better to wait for Stephen's patch?
> Thank you in advance for your attention to this matter.
> 
> Best Regards,
> --Shaopeng

This patch is necessary for fm10k to use on XEN environment with DPDK. How could
we move forward, could you please kindly give some advice?

Thanks,
--Shaopeng

> 
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Shaopeng He
> > > Sent: Friday, May 15, 2015 4:56 PM
> > > To: dev@dpdk.org
> > > Cc: He, Shaopeng
> > > Subject: [dpdk-dev] [PATCH] fm10k: support XEN domain0
> > >
> > > fm10k was failing to run in XEN domain0, as the physical memory for DMA
> > > should be allocated and translated in a different way for XEN domain0. So
> > > rte_memzone_reserve_bounded() should be used for DMA memory
> > > allocation, and rte_mem_phy2mch() should be used for DMA memory
> > > address translation to support running fm10k PMD in XEN domain0.
> > >
> > > Signed-off-by: Shaopeng He <shaopeng.he@intel.com>
> > > ---
> > >  lib/librte_pmd_fm10k/fm10k_ethdev.c | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > >
> > > diff --git a/lib/librte_pmd_fm10k/fm10k_ethdev.c
> > > b/lib/librte_pmd_fm10k/fm10k_ethdev.c
> > > index 275c19c..c85c856 100644
> > > --- a/lib/librte_pmd_fm10k/fm10k_ethdev.c
> > > +++ b/lib/librte_pmd_fm10k/fm10k_ethdev.c
> > > @@ -1004,7 +1004,11 @@ fm10k_rx_queue_setup(struct rte_eth_dev
> > *dev,
> > > uint16_t queue_id,
> > >  		return (-ENOMEM);
> > >  	}
> > >  	q->hw_ring = mz->addr;
> > > +#ifdef RTE_LIBRTE_XEN_DOM0
> > > +	q->hw_ring_phys_addr = rte_mem_phy2mch(mz->memseg_id, mz-
> > > >phys_addr);
> > > +#else
> > >  	q->hw_ring_phys_addr = mz->phys_addr;
> > > +#endif
> > >
> > >  	dev->data->rx_queues[queue_id] = q;
> > >  	return 0;
> > > @@ -1150,7 +1154,11 @@ fm10k_tx_queue_setup(struct rte_eth_dev
> > *dev,
> > > uint16_t queue_id,
> > >  		return (-ENOMEM);
> > >  	}
> > >  	q->hw_ring = mz->addr;
> > > +#ifdef RTE_LIBRTE_XEN_DOM0
> > > +	q->hw_ring_phys_addr = rte_mem_phy2mch(mz->memseg_id, mz-
> > > >phys_addr);
> > > +#else
> > >  	q->hw_ring_phys_addr = mz->phys_addr;
> > > +#endif
> > >
> > >  	/*
> > >  	 * allocate memory for the RS bit tracker. Enough slots to hold the
> > > --
> > > 1.9.3
> >
Thomas Monjalon July 1, 2015, 10:47 a.m. UTC | #8 
2015-06-30 03:27, He, Shaopeng:
> From: He, Shaopeng
> > From: Liu, Jijiang
> > > Acked-by: Jijiang Liu <Jijiang.liu@intel.com>
> > >
> > > I think this patch could be merged before Stephen's following patch[1] is
> > > merged, then Stephen should rework the patch[1].
> > > Thanks.
> > >
> > > [1]http://dpdk.org/ml/archives/dev/2015-March/014992.html
> > 
> > Do you think we can accept this patch in current no-so-elegant way, so user
> > can
> > use XEN with fm10k from release 2.1; or better to wait for Stephen's patch?
> > Thank you in advance for your attention to this matter.
> 
> This patch is necessary for fm10k to use on XEN environment with DPDK. How could
> we move forward, could you please kindly give some advice?

Applied.
It's probably the last time this ifdef is used. Next time, we'll have to introduce
a function which is common to Xen and standard case. A rework is welcome.
The Xen case could also be switched at run-time. Stephen's patch must be
reviewed for that.
diff mbox

Patch

diff --git a/lib/librte_pmd_fm10k/fm10k_ethdev.c b/lib/librte_pmd_fm10k/fm10k_ethdev.c
index 275c19c..c85c856 100644
--- a/lib/librte_pmd_fm10k/fm10k_ethdev.c
+++ b/lib/librte_pmd_fm10k/fm10k_ethdev.c
@@ -1004,7 +1004,11 @@  fm10k_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_id,
 		return (-ENOMEM);
 	}
 	q->hw_ring = mz->addr;
+#ifdef RTE_LIBRTE_XEN_DOM0
+	q->hw_ring_phys_addr = rte_mem_phy2mch(mz->memseg_id, mz->phys_addr);
+#else
 	q->hw_ring_phys_addr = mz->phys_addr;
+#endif
 
 	dev->data->rx_queues[queue_id] = q;
 	return 0;
@@ -1150,7 +1154,11 @@  fm10k_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_id,
 		return (-ENOMEM);
 	}
 	q->hw_ring = mz->addr;
+#ifdef RTE_LIBRTE_XEN_DOM0
+	q->hw_ring_phys_addr = rte_mem_phy2mch(mz->memseg_id, mz->phys_addr);
+#else
 	q->hw_ring_phys_addr = mz->phys_addr;
+#endif
 
 	/*
 	 * allocate memory for the RS bit tracker. Enough slots to hold the