RE: [PATCH v7 09/11] iommu/vt-d: Add bind guest PASID support

["Tian, Kevin" <kevin.tian@intel.com>]

> From: Jacob Pan [mailto:jacob.jun.pan@linux.intel.com]
> Sent: Wednesday, October 30, 2019 12:12 AM
> 
> On Tue, 29 Oct 2019 07:57:21 +0000
> "Tian, Kevin" <kevin.tian@intel.com> wrote:
> 
> > > From: Jacob Pan [mailto:jacob.jun.pan@linux.intel.com]
> > > Sent: Tuesday, October 29, 2019 12:03 AM
> > >
> > > On Mon, 28 Oct 2019 06:03:36 +0000
> > > "Tian, Kevin" <kevin.tian@intel.com> wrote:
> > >
> > > > > > > +	.sva_bind_gpasid	= intel_svm_bind_gpasid,
> > > > > > > +	.sva_unbind_gpasid	=
> > > > > > > intel_svm_unbind_gpasid, +#endif
> > > > > >
> > > > > > again, pure PASID management logic should be separated from
> > > > > > SVM.
> > > > > I am not following, these two functions are SVM functionality,
> > > > > not pure PASID management which is already separated in
> > > > > ioasid.c
> > > >
> > > > I should say pure "scalable mode" logic. Above callbacks are not
> > > > related to host SVM per se. They are serving gpasid requests from
> > > > guest side, thus part of generic scalable mode capability.
> > > Got your point, but we are sharing data structures with host SVM,
> > > it is very difficult and inefficient to separate the two.
> >
> > I don't think difficulty is the reason against such direction. We
> > need do things right. :-) I'm fine with putting it in a TODO list,
> > but at least need the right information in the 1st place to tell that
> > current way is just a short-term approach, and we should revisit
> > later.
> I guess the fundamental question is: Should the scalable mode logic,
> i.e. guest SVA at PASID granu device, be perceived as part of the
> overall SVA functionality?

guest SVA != guest scalable mode. I'm not sure whether the definition
of SVA has been changed. but iirc it simply means shared virtual
memory usage i.e. sharing CPU page table with device. But with
scalable mode, you can have PASID tagged 1st-level for whatever
usage, guest IOVA, guest SVA, guest nested GPA, etc.

> 
> My view is yes, we shall share SVA and gSVA whenever we can.

sharing is based on scalable mode, not based on sva itself.

> 
> The longer term, which I am working on right now, is to converge
> intel_svm_bind_mm to the generic iommu_sva_bind_device() and use
> common
> data structures as well. It is conceivable that these common structures
> span across hardware architectures, also guest vs host SVA usages.
> 
> i.e. iommu_ops have
> iommu_sva_bind_gpasid() for SM/gSVA
> iommu_sva_bind_device() for native SVA
> 
> Or I am missing your point completely?

since sva is already used in VFIO for broader purpose, it's fine to leave
the name there. But again, it's incorrect to tie iommu_sva_bind_gpasid
under CONFIG_INTEL_IOMMU_SVM. The former is for SM, while the
latter is only for SVA. As long as host IOMMU is in scalable mode, 
bind_gpasid can be supported. If you want a config option, then it should
be a new one instead of IOMMU_SVM.

Thanks
Kevin