Re: [PATCH 5/7] iommu: Add domain_type_supported() callback in iommu_ops

["Suthikulpanit, Suravee via iommu" <iommu@lists.linux-foundation.org>]

Robin,

On 6/13/2022 4:31 PM, Robin Murphy wrote:
> On 2022-06-13 02:25, Suravee Suthikulpanit wrote:
>> When user requests to change IOMMU domain to a new type, IOMMU generic
>> layer checks the requested type against the default domain type returned
>> by vendor-specific IOMMU driver.
>>
>> However, there is only one default domain type, and current mechanism
>> does not allow if the requested type does not match the default type.
> 
> I don't really follow the reasoning here. If a driver's def_domain_type callback returns a specific type, it's saying that the device *has* to have that specific domain type for driver/platform-specific reasons. 

Agree, and I understand this part.

> If that's not the case, then the driver shouldn't say so in the first place.

Considering the case:
1. Boot w/ default domain = IOMMU_DOMAIN_DMA_FQ
2. User wants to change to IOMMU_DOMAIN_IDENTITY, which is not supported by IOMMU driver. In this case, IOMMU driver can return IOMMU_DOMAIN_DMA_FQ and prevent the mode change.
3. However, if user want to change to IOMMU_DOMAIN_DMA. The driver can support this. However, since the def_domain_type() returns IOMMU_DOMAIN_DMA_FQ, it ends up prevent the mode change.

IIUC, we should support step 3 above. Basically, with the newly proposed interface, it allows us to check with IOMMU driver if it can support certain domain types before trying
to allocate the domain.

> 
>> Introducing check_domain_type_supported() callback in iommu_ops,
>> which allows IOMMU generic layer to check with vendor-specific IOMMU driver
>> whether the requested type is supported. This allows user to request
>> types other than the default type.
> 
> Note also that you're only adding this in the sysfs path - what about the "iommu.passthrough=" parameter or CONFIG_IOMMU_DEFAULT_PASSTHROUGH?

For SNP case, we cannot enable SNP if iommu=off or iommu=pt or iommu.passthrough=1 or CONFIG_IOMMU_DEFAULT_PASSTHROUGH=y.
So, when another driver tries to enable SNP, the IOMMU driver prevents it (see iommu_sev_snp_supported() in patch 3).

Instead, if we boot with iommu.passhthrough=0, when another driver tries to enable SNP, the IOMMU driver allows this and switch to SNP enable mode.
Subsequently, if user tries to switch a domain (via sysfs) to IOMMU_DOMAIN_IDENTITY, the IOMMU needs to prevent this because it has already switch
to SNP-enabled mode.

> AFAICS there shouldn't need to be any core-level changes to support this. We already have drivers which don't support passthrough at all, so conditionally not supporting it should be no big deal. What should happen currently is that def_domain_type returns 0 for "don't care", then domain_alloc rejects IOMMU_DOMAIN_IDENTITY and and returns NULL, so iommu_group_alloc_default_domain() falls back to IOMMU_DOMAIN_DMA.

Technically, we can do it the way you suggest. But isn't this confusing? At first, def_domain_type() returns 0 for "don't care",
but then it rejects the request to change to IOMMU_DOMAIN_IDENTITY when trying to call domain_alloc().

Please let me know if I am missing something.

Best Regards,
Suravee
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu