On 18/10/2023 23:58, Jason Gunthorpe wrote:
> On Wed, Oct 18, 2023 at 09:27:07PM +0100, Joao Martins wrote:
>> -static struct iommu_domain *amd_iommu_domain_alloc(unsigned type)
>> +static struct iommu_domain *do_iommu_domain_alloc(unsigned int type,
>> + struct device *dev,
>> + u32 flags)
>> {
>> struct protection_domain *domain;
>> + struct amd_iommu *iommu = NULL;
>> +
>> + if (dev) {
>> + iommu = rlookup_amd_iommu(dev);
>> + if (!iommu)
>> + return ERR_PTR(-ENODEV);
>> + }
>>
>> /*
>> * Since DTE[Mode]=0 is prohibited on SNP-enabled system,
>> * default to use IOMMU_DOMAIN_DMA[_FQ].
>> */
>> if (amd_iommu_snp_en && (type == IOMMU_DOMAIN_IDENTITY))
>> - return NULL;
>> + return ERR_PTR(-EINVAL);
>>
>> domain = protection_domain_alloc(type);
>> if (!domain)
>> - return NULL;
>> + return ERR_PTR(-ENOMEM);
>>
>> domain->domain.geometry.aperture_start = 0;
>> domain->domain.geometry.aperture_end = dma_max_address();
>> domain->domain.geometry.force_aperture = true;
>>
>> + if (iommu) {
>> + domain->domain.type = type;
>> + domain->domain.pgsize_bitmap =
>> + iommu->iommu.ops->pgsize_bitmap;
>> + domain->domain.ops =
>> + iommu->iommu.ops->default_domain_ops;
>> + }
>> +
>> return &domain->domain;
>> }
>
> In the end this is probably not enough refactoring, but this driver
> needs so much work we should just wait till the already written series
> get merged.
>
That is quite a road ahead :(( -- and AMD IOMMU hardware is the only thing I am
best equipped to test this.
But I understand if we end up going this way
> eg domain_alloc_paging should just invoke domain_alloc_user with some
> null arguments if the driver is constructed this way
>
>> +static struct iommu_domain *amd_iommu_domain_alloc_user(struct device *dev,
>> + u32 flags)
>> +{
>> + unsigned int type = IOMMU_DOMAIN_UNMANAGED;
>> +
>> + if (flags & IOMMU_HWPT_ALLOC_NEST_PARENT)
>> + return ERR_PTR(-EOPNOTSUPP);
>
> This should be written as a list of flags the driver *supports* not
> that it rejects
>
> if (flags)
> return ERR_PTR(-EOPNOTSUPP);
Will fix (this was a silly mistake, as I'm doing the right thing on Intel).
