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

Reply via email to