On 01/07/2022 5:43 pm, Nicolin Chen wrote:
On Fri, Jul 01, 2022 at 11:21:48AM +0100, Robin Murphy wrote:
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 2ed3594f384e..072cac5ab5a4 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -1135,10 +1135,8 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
struct arm_smmu_device *smmu;
int ret;
- if (!fwspec || fwspec->ops != &arm_smmu_ops) {
- dev_err(dev, "cannot attach to SMMU, is it on the same bus?\n");
- return -ENXIO;
- }
+ if (!fwspec || fwspec->ops != &arm_smmu_ops)
+ return -EMEDIUMTYPE;
This is the wrong check, you want the "if (smmu_domain->smmu != smmu)"
condition further down. If this one fails it's effectively because the
device doesn't have an IOMMU at all, and similar to patch #3 it will be
Thanks for the review! I will fix that. The "on the same bus" is
quite eye-catching.
removed once the core code takes over properly (I even have both those
patches written now!)
Actually in my v1 the proposal for ops check returned -EMEDIUMTYPE
also upon an ops mismatch, treating that too as an incompatibility.
Do you mean that we should have fine-grained it further?
On second look, I think this particular check was already entirely
redundant by the time I made the fwspec conversion to it, oh well. Since
it remains harmless for the time being, let's just ignore it entirely
until we can confidently say goodbye to the whole lot[1].
I don't think there's any need to differentiate an instance mismatch
from a driver mismatch, once the latter becomes realistically possible,
mostly due to iommu_domain_alloc() also having to become device-aware to
know which driver to allocate from. Thus as far as a user is concerned,
if attaching a device to an existing domain fails with -EMEDIUMTYPE,
allocating a new domain using the given device, and attaching to that,
can be expected to succeed, regardless of why the original attempt was
rejected. In fact even in the theoretical different-driver-per-bus model
the same principle still holds up.
Thanks,
Robin.
[1]
https://gitlab.arm.com/linux-arm/linux-rm/-/commit/aa4accfa4a10e92daad0d51095918e8a89014393
