Re: [PATCH v4 1/5] iommu: Return -EMEDIUMTYPE for incompatible domain and device/group

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization



[Index of Archives]     [KVM Development]     [Libvirt Development]     [Libvirt Users]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux