Re: [PATCH v3 04/15] iommu: Move bus setup to IOMMU device registration

Robin Murphy
2022-07-06T14:37:21Z

On 2022-07-06 03:35, Baolu Lu wrote:
> On 2022/7/6 01:08, Robin Murphy wrote:
>> @@ -202,12 +210,32 @@ int iommu_device_register(struct iommu_device
>> *iommu,
>>       spin_lock(&iommu_device_lock);
>>       list_add_tail(&iommu->list, &iommu_device_list);
>>       spin_unlock(&iommu_device_lock);
>> +
>> +    for (int i = 0; i < ARRAY_SIZE(iommu_buses); i++) {
>> +        struct bus_type *bus = iommu_buses[i];
>> +        int err;
>> +
>> +        if (bus->iommu_ops && bus->iommu_ops != ops) {
>> +            err = -EBUSY;
>> +        } else {
>> +            bus->iommu_ops = ops;
>> +            err = bus_iommu_probe(bus);
>> +        }
>> +        if (err) {
>> +            iommu_device_unregister(iommu);
>> +            return err;
>> +        }
>> +    }
>> +
>>       return 0;
>>   }
>>   EXPORT_SYMBOL_GPL(iommu_device_register);
>
> With bus_set_iommu() retired, my understanding is that now we embrace
> the first-come-first-serve policy for bus->iommu_ops setting. This will
> lead to problem in different iommu_ops for different bus case. Did I
> overlook anything?

This is just formalising the de-facto situation that we don't actually
have any combination of drivers that could load on the same system
without already attempting to claim at least one bus in common. It's
also only temporary until the bus ops are removed completely and we
fully support multiple drivers coexisting, which only actually takes a
handful more patches - I've realised I could even bring that change
*ahead* of the big job of converting iommu_domain_alloc() (I'm not
convinced that the tree-wide flag-day patch for that I currently have in
the dev branch is really viable, nor that I've actually got the correct
device at some of the callsites), although whether it's worth the
potentially-surprising behaviour that might result I'm less sure.

If we already had systems where in-tree drivers successfully coexisted
on different buses then I'd have split this up and done something a bit
more involved to keep a vestigial bus_set_iommu() around until the final
bus ops removal, but since we don't, it seemed neatest to do all the
related work in one go.

Thanks,
Robin.