Robin,
On 6/14/2022 4:51 PM, Robin Murphy wrote:
On 2022-06-13 15:38, Suthikulpanit, Suravee wrote:
Robin,
On 6/13/2022 4:31 PM, Robin Murphy wrote:
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).
Ugh, I hadn't looked too closely at the other patches, but an interface that looks like a
simple "is this feature supported?" check with a secret side-effect of changing
global behaviour as well? Yuck :(
What external drivers are expected to have the authority to affect the entire
system and call that? The fact that you're exporting it suggests they could be
loaded from modules *after* v2 features have been enabled and/or the user has
configured a non-default identity domain for a group via sysfs... Fun!
I see your point.
Currently, the function to enable SNP will be called from SEV driver when it
tries to enable SNP support globally on the system.
This is done during fs_initcall(), which is early in the boot process. I can
also add a guard code to make sure that this won't
be done after a certain phase.
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().
Yes, that's how it works; def_domain_type is responsible for quirking
individual *devices* that need to have a specific domain type (in practice,
devices which need identity mapping), while domain_alloc is responsible for
saying which domain types the driver supports as a whole, by allocating them or
not as appropriate.
We don't have a particularly neat way to achieve the negative of
def_domain_type - i.e. saying that a specific device *can't* use a specific
otherwise-supported domain type - other than subsequently failing in
attach_dev, but so far we've not needed such a thing. And if SNP is expected to
be mutually exclusive with identity domain support globally, then we still
shouldn't need it.
Thanks for your feedback.
Suravee
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
