Hi Anatoly, > -----Original Message----- > From: Burakov, Anatoly > Sent: Wednesday, March 14, 2018 8:08 PM > To: Wang, Xiao W <xiao.w.w...@intel.com>; dev@dpdk.org > Cc: Wang, Zhihong <zhihong.w...@intel.com>; > maxime.coque...@redhat.com; y...@fridaylinux.org; Liang, Cunming > <cunming.li...@intel.com>; Xu, Rosen <rosen...@intel.com>; Chen, Junjie J > <junjie.j.c...@intel.com>; Daly, Dan <dan.d...@intel.com> > Subject: Re: [dpdk-dev] [PATCH 1/3] eal/vfio: add support for multiple > container > > On 09-Mar-18 11:08 PM, Xiao Wang wrote: > > From: Junjie Chen <junjie.j.c...@intel.com> > > > > Currently eal vfio framework binds vfio group fd to the default > > container fd, while in some cases, e.g. vDPA (vhost data path > > acceleration), we want to set vfio group to a new container and > > program DMA mapping via this new container, so this patch adds > > APIs to support multiple container. > > > > Signed-off-by: Junjie Chen <junjie.j.c...@intel.com> > > Signed-off-by: Xiao Wang <xiao.w.w...@intel.com> > > --- > > I'm not going to get into virtual vs. real device debate, but i do have > some issues with VFIO side of things. > > I'm not completely convinced this change is needed in the first place. > If the device driver manages its own groups anyway, it knows which VFIO > groups belong to it, so it can add/remove them without putting them into > separate containers. What is the purpose of keeping them in a separate > container as opposed to just keeping track of group id's?
The device driver needs to have a separate container to program IOMMU For the device, with the VM's addr translation table. So driver needs the Devices be put into new containers, rather than the default one. > > <...> > > > > + vfio_cfg->vfio_container_fd = vfio_get_container_fd(); > > + > > + if (vfio_cfg->vfio_container_fd < 0) > > + return -1; > > + > > + return vfio_cfg->vfio_container_fd; > > +} > > Please correct me if i'm wrong, but this patch appears to be mistitled. > You're not really creating multiple containers, you're just partitioning > existing one. Do we really need to open/store/close container fd's > separately, if all we have is a single container anyway? This driver are creating new containers for devices, it needs each device to have its own container, then we can dma_map/ummap for the device via it's associated container. BRs, Xiao > > The semantics of this are also weird in multiprocess. When secondary > process requests a container, we always create a new one, send it over > IPC and close it afterwards. It seems to be oblivious that you may have > several container fd's, and does not know which one you are asking for. > We know it's all the same container, but that's clearly not what the > code appears to be doing. > > -- > Thanks, > Anatoly
