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

[Vasant Hegde <vasant.hegde@amd.com>]

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.
Also based on base domain type we do adjust the GCR3 table.

-Vasant

> 
> gcr3 touching and enforcing should be part of generic PASID code and
> be triggered naturally when the SVA domain is attached to a PASID.
> 
> Jason