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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 6 Jul 2022 15:37:06 +0100
From:   Robin Murphy <robin.murphy@....com>
To:     Baolu Lu <baolu.lu@...ux.intel.com>, joro@...tes.org
Cc:     will@...nel.org, iommu@...ts.linux.dev,
        linux-arm-kernel@...ts.infradead.org,
        suravee.suthikulpanit@....com, vasant.hegde@....com,
        mjrosato@...ux.ibm.com, gerald.schaefer@...ux.ibm.com,
        schnelle@...ux.ibm.com, linux-s390@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 04/15] iommu: Move bus setup to IOMMU device
 registration

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.