linux-kernel - Re: [PATCH 12/21] dma-iommu: factor atomic pool allocations into helpers

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: <290377cd-8cd3-aa7a-0057-36df3d7cd728@arm.com>
Date:   Tue, 23 Apr 2019 10:48:49 +0100
From:   Robin Murphy <robin.murphy@....com>
To:     Christoph Hellwig <hch@....de>
Cc:     Joerg Roedel <joro@...tes.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will.deacon@....com>,
        Tom Lendacky <thomas.lendacky@....com>,
        iommu@...ts.linux-foundation.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 12/21] dma-iommu: factor atomic pool allocations into
 helpers

On 2019-04-19 10:07 am, Christoph Hellwig wrote:
> On Thu, Apr 18, 2019 at 05:41:00PM +0100, Robin Murphy wrote:
>>>   From a very high level POV this looks ok, but sometimes a bit to
>>> convoluted to me.  The major issue why I went with the version I posted
>>> is that I can cleanly ifdef out the remap code in just a few sections.
>>> In this version it is spread out a lot more, and the use of IS_ENABLED
>>> means that we'd need a lot more stubs for functionality that won't
>>> ever be called but needs to be compilable.
>>
>> What functionality do you have planned in that regard? I did do a quick
>> build test of my arm64 config with DMA_DIRECT_REMAP hacked out, and
>> dma-iommu.o appeared to link OK (although other bits of arm64 and
>> dma-direct didn't, as expected). I will try x86 with IOMMU_DMA to make
>> sure, though.
> 
> Yeah, this seems to actually work, there just is a huge chunk of
> remapping that is hopefully discarded by the compiler even without the
> ifdefs.

Right, the major point of this approach is that you don't need stubs; 
indeed, stubs themselves fall into the category of "#ifdefed code I'd 
prefer to avoid". The preprocessing essentially resolves to:

	if (0)
		function_that_doesnt_exist();

so compilation treats it as an external reference, but since constant 
folding ends up eliding the call, that symbol isn't referenced in the 
final object, so linking never has to resolve it. All it needs is a 
declaration to avoid a compiler warning, but that's the same declaration 
that's needed anyway for when the function does exist. Similarly, static 
functions like iommu_dma_alloc_remap() still get compiled - so we don't 
lose coverage - but then get discarded at the link stage (via 
gc-sections) since they end up unreferenced. It's really pretty neat.

Robin.