From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753958AbcHANrI (ORCPT ); Mon, 1 Aug 2016 09:47:08 -0400 Received: from foss.arm.com ([217.140.101.70]:33971 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753525AbcHANpv (ORCPT ); Mon, 1 Aug 2016 09:45:51 -0400 Subject: Re: [PATCH v2] arm64: mm: convert __dma_* routines to use start, size To: "kwangwoo.lee@sk.com" , Russell King - ARM Linux , Catalin Marinas , Will Deacon , Mark Rutland , "linux-arm-kernel@lists.infradead.org" References: <1469518496-8177-1-git-send-email-kwangwoo.lee@sk.com> <15c12f9900fd4b31a875250c478023c6@nmail01.hynixad.com> <1c4ec01bd3594485b6a4b386994dc6ca@nmail01.hynixad.com> Cc: "hyunchul3.kim@sk.com" , "linux-kernel@vger.kernel.org" , "woosuk.chung@sk.com" From: Robin Murphy Message-ID: <21272fb2-bffb-15ee-41a9-af0251a33fd0@arm.com> Date: Mon, 1 Aug 2016 14:36:19 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <1c4ec01bd3594485b6a4b386994dc6ca@nmail01.hynixad.com> Content-Type: text/plain; charset=euc-kr Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 01/08/16 00:45, kwangwoo.lee@sk.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. From mboxrd@z Thu Jan 1 00:00:00 1970 From: robin.murphy@arm.com (Robin Murphy) Date: Mon, 1 Aug 2016 14:36:19 +0100 Subject: [PATCH v2] arm64: mm: convert __dma_* routines to use start, size In-Reply-To: <1c4ec01bd3594485b6a4b386994dc6ca@nmail01.hynixad.com> References: <1469518496-8177-1-git-send-email-kwangwoo.lee@sk.com> <15c12f9900fd4b31a875250c478023c6@nmail01.hynixad.com> <1c4ec01bd3594485b6a4b386994dc6ca@nmail01.hynixad.com> Message-ID: <21272fb2-bffb-15ee-41a9-af0251a33fd0@arm.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 01/08/16 00:45, kwangwoo.lee at sk.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.