Re: [PATCH RESEND 04/10] iommu/amd: Add support to enable/disable SVA feature

[Jason Gunthorpe <jgg@ziepe.ca>]

On Mon, Aug 28, 2023 at 04:15:45PM +0530, Vasant Hegde wrote:
> Jason,
> 
> 
> On 8/23/2023 8:58 PM, Jason Gunthorpe wrote:
> > On Wed, Aug 23, 2023 at 02:04:09PM +0000, Vasant Hegde wrote:
> > 
> >> +int amd_iommu_sva_enable(struct device *dev)
> >> +{
> >> +	struct pci_dev *pdev = dev_is_pci(dev) ? to_pci_dev(dev) : NULL;
> >> +	struct amd_iommu *iommu = get_amd_iommu_from_dev(dev);
> >> +	struct iommu_dev_data *dev_data = dev_iommu_priv_get(dev);
> >> +
> >> +	if (!pdev || !iommu || !dev_data)
> >> +		return -EINVAL;
> >> +
> >> +	if (!amd_iommu_sva_supported())
> >> +		return -ENODEV;
> >> +
> >> +	if (!dev_data->pasid_enabled)
> >> +		return -EINVAL;
> >> +
> >> +	return amd_iommu_sva_gcr3_init(dev_data, dev->iommu->max_pasids);
> >> +}
> >> +
> >> +int amd_iommu_sva_disable(struct device *dev)
> >> +{
> >> +	struct amd_iommu *iommu = get_amd_iommu_from_dev(dev);
> >> +	struct iommu_dev_data *dev_data = dev_iommu_priv_get(dev);
> >> +
> >> +	if (!iommu || !dev_data)
> >> +		return -EINVAL;
> >> +
> >> +	return amd_iommu_sva_gcr3_uninit(dev_data);
> >> +}
> > 
> > I think these features are a mistake, you need to implement them for
> > now but I wouldn't touch the gcr3 table, and disable should be a NOP.
> 
> We need to configure Device Table Entry when we enable/disable SVA.

The DTE should only be changed by bind/unbind of RID/PASID - it has
nothing to do with these APIs.

Again, start with enabling native PASID support for UNAMANGED v2
domains and your SVA will make alot more sense and be alot cleaner.

Jason