linux-kernel - Re: [PATCH] iommu/iova: Fix module config properly

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]

Message-ID: <bdb9abaf-2703-6d4c-9cab-f6e15e3e8b30@huawei.com>
Date:   Thu, 15 Sep 2022 12:45:43 +0100
From:   John Garry <john.garry@...wei.com>
To:     Thierry Reding <thierry.reding@...il.com>,
        Robin Murphy <robin.murphy@....com>
CC:     <joro@...tes.org>, <will@...nel.org>, <iommu@...ts.linux.dev>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] iommu/iova: Fix module config properly

On 14/09/2022 10:47, Thierry Reding wrote:
> On Tue, Sep 13, 2022 at 03:15:18PM +0100, Robin Murphy wrote:
>> On 2022-09-13 14:01, John Garry wrote:
>>> On 13/09/2022 12:47, Robin Murphy wrote:
>>>> IOMMU_IOVA is intended to be an optional library for users to select as
>>>> and when they desire. Since it can be a module now, this means that
>>>> built-in code which has chosen not to select it should not fail to link
>>>> if it happens to have selected as a module by someone else. Replace
>>>> IS_ENABLED() with IS_REACHABLE() to do the right thing.
>>>
>>> Hi Robin,
>>>
>>> Recently you mentioned "I wonder if we couldn't replace the IS_ENABLED()
>>> with IS_REACHABLE() and restore some of the previously-conditional
>>> selects", and pointed me to 84db889e6d82 as an example of when a
>>> conditional select was made unconditional.
>>>
>>> So will you also restore some previously-conditional selects next?
>>
>> I figured I'd leave that up to Thierry (and/or anyone else with a vested
>> interest), but having mulled it over since that previous thread, there's
>> really no excuse for the API itself not to do the right thing either way, so
>> I felt compelled to write up this much.
> 
> On Tegra specifically, as the commit message says, we don't really care
> about the conditional selection because practically we always want IOMMU
> support enabled. So instead of adding back the conditional select it
> would make more sense to select IOMMU_API instead and then get rid of
> the handful of #ifdef blocks we have for that.

Out of curiosity, does the same go to host1x, whose kconfig got the same 
treatment as tegra with regards to selecting IOMMU_IOVA? I mean, will 
you not go back to conditionally selecting IOMMU_IOVA, and instead 
select IOMMU_API and IOMMU_IOVA always?

Thanks,
John

> 
> On a side note: I'm looking at a subtle regression right now where some
> corner case no longer works and that's primarily due to the fact that we
> can have four different scenarios: non-IOMMU, IOMMU, DMA/IOMMU and IOMMU
> but devices with no iommus property in their DT nodes. This has all
> become very unwieldy and becomes increasingly difficult to test. So
> anything that we can do to keep down the number of permutations is
> welcome.
> 
> While that's not directly related to this issue, the lesson learned is
> that, at least on the Tegra side, we have in the past made things
> unnecessarily difficult for ourselves based on the misguided assumption
> that maximum configurability is key.
> 
> Thierry