[dpdk-dev] [PATCH v2 2/2] vhost: enable IOMMU for async vhost

[Ding, Xuan]

> -----Original Message-----
> From: Xia, Chenbo <chenbo.xia at intel.com>
> Sent: Friday, September 24, 2021 3:36 PM
> To: Maxime Coquelin <maxime.coquelin at redhat.com>; Hu, Jiayu
> <jiayu.hu at intel.com>; Ding, Xuan <xuan.ding at intel.com>; dev at dpdk.org;
> Burakov, Anatoly <anatoly.burakov at intel.com>
> Cc: Jiang, Cheng1 <cheng1.jiang at intel.com>; Richardson, Bruce
> <bruce.richardson at intel.com>; Pai G, Sunil <sunil.pai.g at intel.com>; Wang,
> Yinan <yinan.wang at intel.com>; Yang, YvonneX <yvonnex.yang at intel.com>
> Subject: RE: [PATCH v2 2/2] vhost: enable IOMMU for async vhost
> 
> > -----Original Message-----
> > From: Maxime Coquelin <maxime.coquelin at redhat.com>
> > Sent: Friday, September 24, 2021 3:14 PM
> > To: Xia, Chenbo <chenbo.xia at intel.com>; Hu, Jiayu <jiayu.hu at intel.com>;
> Ding,
> > Xuan <xuan.ding at intel.com>; dev at dpdk.org; Burakov, Anatoly
> > <anatoly.burakov at intel.com>
> > Cc: Jiang, Cheng1 <cheng1.jiang at intel.com>; Richardson, Bruce
> > <bruce.richardson at intel.com>; Pai G, Sunil <sunil.pai.g at intel.com>; Wang,
> > Yinan <yinan.wang at intel.com>; Yang, YvonneX <yvonnex.yang at intel.com>
> > Subject: Re: [PATCH v2 2/2] vhost: enable IOMMU for async vhost
> >
> >
> >
> > On 9/24/21 03:53, Xia, Chenbo wrote:
> > >> -----Original Message-----
> > >> From: Maxime Coquelin <maxime.coquelin at redhat.com>
> > >> Sent: Thursday, September 23, 2021 10:56 PM
> > >> To: Hu, Jiayu <jiayu.hu at intel.com>; Ding, Xuan <xuan.ding at intel.com>;
> > >> dev at dpdk.org; Burakov, Anatoly <anatoly.burakov at intel.com>; Xia,
> Chenbo
> > >> <chenbo.xia at intel.com>
> > >> Cc: Jiang, Cheng1 <cheng1.jiang at intel.com>; Richardson, Bruce
> > >> <bruce.richardson at intel.com>; Pai G, Sunil <sunil.pai.g at intel.com>; Wang,
> > >> Yinan <yinan.wang at intel.com>; Yang, YvonneX <yvonnex.yang at intel.com>
> > >> Subject: Re: [PATCH v2 2/2] vhost: enable IOMMU for async vhost
> > >>
> > >>
> > >>
> > >> On 9/23/21 16:39, Hu, Jiayu wrote:
> > >>> Hi Xuan,
> > >>>
> > >>>> -----Original Message-----
> > >>>> From: Ding, Xuan <xuan.ding at intel.com>
> > >>>> Sent: Friday, September 17, 2021 1:26 PM
> > >>>> To: dev at dpdk.org; Burakov, Anatoly <anatoly.burakov at intel.com>;
> > >>>> maxime.coquelin at redhat.com; Xia, Chenbo <chenbo.xia at intel.com>
> > >>>> Cc: Hu, Jiayu <jiayu.hu at intel.com>; Jiang, Cheng1
> > <cheng1.jiang at intel.com>;
> > >>>> Richardson, Bruce <bruce.richardson at intel.com>; Pai G, Sunil
> > >>>> <sunil.pai.g at intel.com>; Wang, Yinan <yinan.wang at intel.com>; Yang,
> > >>>> YvonneX <yvonnex.yang at intel.com>; Ding, Xuan <xuan.ding at intel.com>
> > >>>> Subject: [PATCH v2 2/2] vhost: enable IOMMU for async vhost
> > >>>>
> > >>>> The use of IOMMU has many advantages, such as isolation and address
> > >>>> translation. This patch extends the capbility of DMA engine to use
> IOMMU
> > if
> > >>>> the DMA engine is bound to vfio.
> > >>>>
> > >>>> When set memory table, the guest memory will be mapped into the
> default
> > >>>> container of DPDK.
> > >>>>
> > >>>> Signed-off-by: Xuan Ding <xuan.ding at intel.com>
> > >>>> ---
> > >>>>    lib/vhost/rte_vhost.h  |  1 +
> > >>>>    lib/vhost/vhost_user.c | 57
> > >>>> +++++++++++++++++++++++++++++++++++++++++-
> > >>>>    2 files changed, 57 insertions(+), 1 deletion(-)
> > >>>>
> > >>>> diff --git a/lib/vhost/rte_vhost.h b/lib/vhost/rte_vhost.h index
> > >>>> 8d875e9322..e0537249f3 100644
> > >>>> --- a/lib/vhost/rte_vhost.h
> > >>>> +++ b/lib/vhost/rte_vhost.h
> > >>>> @@ -127,6 +127,7 @@ struct rte_vhost_mem_region {
> > >>>>    	void	 *mmap_addr;
> > >>>>    	uint64_t mmap_size;
> > >>>>    	int fd;
> > >>>> +	uint64_t dma_map_success;
> > >>>
> > >>> How about using bool for dma_map_success?
> > >>
> > >> The bigger problem here is that you are breaking the ABI.
> > >
> > > Maybe this kind of driver-facing structs/functions should be removed
> > > from ABI, since we are refactoring DPDK ABI recently.
> >
> > It has actually been exposed for SPDK, we cannot just remove it from
> > API.
> 
> 'exposed' does not mean it has to be ABI. Like 'driver_sdk_headers' in
> ethdev lib, those headers can be exposed but do not include ABI. I see
> SPDK is using that for building its lib. Not sure in this case, the SPDK
> Vhost lib should be considered as application.
> 
> Thanks,
> Chenbo

Thanks for the discussion. Since the possible ABI changing is in the future,
I consider adding the dma_map_success in the virtio_net structure, to indicate
the map status of each region. This flag can even be removed if it is not considering
the restrictions on user(kernel driver support). Details can be provided in next version's patch.

Hope to get your insights. :)

Thanks,
Xuan

> 
> >
> > Maxime
> >
> > > /Chenbo
> > >
> > >>
> > >>>>    };
> > >>>>
> > >>>>    /**
> > >