From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754713AbcG0J4Y (ORCPT ); Wed, 27 Jul 2016 05:56:24 -0400 Received: from foss.arm.com ([217.140.101.70]:43695 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753285AbcG0J4W (ORCPT ); Wed, 27 Jul 2016 05:56:22 -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> Cc: "hyunchul3.kim@sk.com" , "linux-kernel@vger.kernel.org" , "woosuk.chung@sk.com" From: Robin Murphy Message-ID: Date: Wed, 27 Jul 2016 10:56:18 +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: <15c12f9900fd4b31a875250c478023c6@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 27/07/16 02:55, kwangwoo.lee@sk.com wrote: [...] >>> /* >>> - * __dma_clean_range(start, end) >>> + * __dma_clean_area(start, size) >>> * - start - virtual start address of region >>> - * - end - virtual end address of region >>> + * - size - size in question >>> */ >>> -__dma_clean_range: >>> - dcache_line_size x2, x3 >>> - sub x3, x2, #1 >>> - bic x0, x0, x3 >>> -1: >>> +__dma_clean_area: >>> alternative_if_not ARM64_WORKAROUND_CLEAN_CACHE >>> - dc cvac, x0 >>> + dcache_by_line_op cvac, sy, x0, x1, x2, x3 >>> alternative_else >>> - dc civac, x0 >>> + dcache_by_line_op civac, sy, x0, x1, x2, x3 >> >> dcache_by_line_op is a relatively large macro - is there any way we can >> still apply the alternative to just the one instruction which needs it, >> as opposed to having to patch the entire mostly-identical routine? > > I agree with your opinion. Then, how do you think about using CONFIG_* options > like below? I think that alternative_* macros seems to keep the space for > unused instruction. Is it necessary? Please, share your thought about the > space. Thanks! > > +__dma_clean_area: > +#if defined(CONFIG_ARM64_ERRATUM_826319) || \ > + defined(CONFIG_ARM64_ERRATUM_827319) || \ > + defined(CONFIG_ARM64_ERRATUM_824069) || \ > + defined(CONFIG_ARM64_ERRATUM_819472) > + dcache_by_line_op civac, sy, x0, x1, x2, x3 > +#else > + dcache_by_line_op cvac, sy, x0, x1, x2, x3 > +#endif That's not ideal, because we still only really want to use the workaround if we detect a CPU which needs it, rather than baking it in at compile time. I was thinking more along the lines of pushing the alternative down into dcache_by_line_op, something like the idea below (compile-tested only, may not actually be viable). Robin. -----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