linux-kernel - Re: [PATCH 2/3] swiotlb: prefix dma_to_phys and phys_to

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:	Tue, 29 Mar 2016 08:57:45 -0400
From:	Sinan Kaya <okaya@...eaurora.org>
To:	Stefano Stabellini <sstabellini@...nel.org>,
	Konrad Rzeszutek Wilk <konrad@...nel.org>
Cc:	Robin Murphy <robin.murphy@....com>,
	linux-arm-kernel@...ts.infradead.org, timur@...eaurora.org,
	cov@...eaurora.org, nwatters@...eaurora.org,
	linux-mips@...ux-mips.org, linux-ia64@...r.kernel.org,
	linux-xtensa@...ux-xtensa.org,
	Catalin Marinas <catalin.marinas@....com>,
	Will Deacon <will.deacon@....com>,
	Max Filippov <jcmvbkbc@...il.com>,
	Chris Metcalf <cmetcalf@...hip.com>,
	Paul Mackerras <paulus@...ba.org>,
	"H. Peter Anvin" <hpa@...or.com>, Huacai Chen <chenhc@...ote.com>,
	Guan Xuetao <gxt@...c.pku.edu.cn>,
	Marek Szyprowski <m.szyprowski@...sung.com>,
	Jisheng Zhang <jszhang@...vell.com>,
	Florian Fainelli <f.fainelli@...il.com>,
	Russell King <linux@....linux.org.uk>,
	Aaro Koskinen <aaro.koskinen@....fi>,
	Michael Ellerman <mpe@...erman.id.au>, X86 <x86@...nel.org>,
	Ingo Molnar <mingo@...hat.com>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Guenter Roeck <linux@...ck-us.net>,
	Fenghua Yu <fenghua.yu@...el.com>,
	Denys Vlasenko <dvlasenk@...hat.com>,
	Arnd Bergmann <arnd@...db.de>,
	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
	Akinobu Mita <akinobu.mita@...il.com>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Valentin Rothberg <valentinrothberg@...il.com>,
	Chris Zankel <chris@...kel.net>,
	Tony Luck <tony.luck@...el.com>, linux-pci@...r.kernel.org,
	linuxppc-dev@...ts.ozlabs.org, Dean Nelson <dnelson@...hat.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Ralf Baechle <ralf@...ux-mips.org>,
	"Suthikulpanit, Suravee" <Suravee.Suthikulpanit@....com>,
	Joe Perches <joe@...ches.com>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH 2/3] swiotlb: prefix dma_to_phys and phys_to_dma functions

On 3/29/2016 8:44 AM, Stefano Stabellini wrote:
> Could you please explain what is the problem that you are trying to
> solve? In other words, what is the issue with assuming that physical
> address = dma address (and the current dma_to_phys and phys_to_dma
> static inlines) if no arm64 platforms violate it? That's pretty much
> what is done on x86 too (without X86_DMA_REMAP).
> 
> If you want to make sure that the assumption is not violated, you can
> introduce a boot time check or a BUG_ON somewhere.
> 
> If there is an arm64 platform with phys_addr != dma_addr, we need proper
> support for it. In fact even if there is an IOMMU on that platform, when
> running Xen on it, the IOMMU would be used by the hypervisor and Linux
> would still end up without it, using the swiotlb.


The problem is that device drivers are trying to use dma_to_phys and phys_to_dma
APIs to do address translation even though these two API do not exist in DMA mapping
layer. (see patch #1 and I made the same mistake in my HIDMA code)

Especially, when a device is behind an IOMMU; the DMA addresses are not equal
to physical addresses. Usage of dma_to_phys causes a crash on the system.

I'm trying to prefix the dma_to_phys and phys_to_dma API with swiotlb so that
we know what it is intended for and usage of these API in drivers need to be
discouraged in the future during code reviews.

Since I renamed the API, Robin Murphy made a suggestion to convert this 

phys_to_virt(swiotlb_dma_to_phys(dev, dma_handle))

to this

#define swiotlb_to_virt(addr) phys_to_virt((phys_addr_t)(addr))

swiotlb_to_virt(dma_handle)

just to reduce code clutter since we know swiotlb_dma_to_phys returns phys already
for ARM64 architecture.

I think we can do this exercise later. I'll undo this change for the moment. 
Let's focus on the API rename. 

I don't want this general purpose dma_to_phys API returning phys=dma value. This is
not correct.

-- 
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project