linux-kernel - Re: [PATCH v2] arm64: mm: convert __dma

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:	Mon, 1 Aug 2016 14:36:19 +0100
From:	Robin Murphy <robin.murphy@....com>
To:	"kwangwoo.lee@...com" <kwangwoo.lee@...com>,
	Russell King - ARM Linux <linux@...linux.org.uk>,
	Catalin Marinas <catalin.marinas@....com>,
	Will Deacon <will.deacon@....com>,
	Mark Rutland <mark.rutland@....com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>
Cc:	"hyunchul3.kim@...com" <hyunchul3.kim@...com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"woosuk.chung@...com" <woosuk.chung@...com>
Subject: Re: [PATCH v2] arm64: mm: convert __dma_* routines to use start, size

On 01/08/16 00:45, kwangwoo.lee@...com wrote:
[...]
>>>> -----8<-----
>>>> diff --git a/arch/arm64/include/asm/assembler.h
>>>> b/arch/arm64/include/asm/assembler.h
>>>> index 10b017c4bdd8..1c005c90387e 100644
>>>> --- a/arch/arm64/include/asm/assembler.h
>>>> +++ b/arch/arm64/include/asm/assembler.h
>>>> @@ -261,7 +261,16 @@ lr	.req	x30		// link register
>>>>  	add	\size, \kaddr, \size
>>>>  	sub	\tmp2, \tmp1, #1
>>>>  	bic	\kaddr, \kaddr, \tmp2
>>>> -9998:	dc	\op, \kaddr
>>>> +9998:
>>>> +	.ifeqs "\op", "cvac"
>>>> +alternative_if_not ARM64_WORKAROUND_CLEAN_CACHE
>>>> +	dc	cvac, \kaddr
>>>> +alternative_else
>>>> +	dc	civac, \kaddr
>>>> +alternative_endif
>>>> +	.else
>>>> +	dc	\op, \kaddr
>>>> +	.endif
>>>>  	add	\kaddr, \kaddr, \tmp1
>>>>  	cmp	\kaddr, \size
>>>>  	b.lo	9998b
>>>
>>> I agree that it looks not viable because it makes the macro bigger and
>>> conditional specifically with CVAC op.
>>
>> Actually, having had a poke around in the resulting disassembly, it
>> looks like this does work correctly. I can't think of a viable reason
>> for the whole dcache_by_line_op to ever be wrapped in yet another
>> alternative (which almost certainly would go horribly wrong), and it
>> would mean that any other future users are automatically covered for
>> free. It's just horrible to look at at the source level.
> 
> Then, Are you going to send a patch for this? Or should I include this change?

I'll do a bit more testing just to make sure, then spin a separate patch
(and try to remember to keep you on CC..)

Robin.