From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932724AbcG1AI3 (ORCPT ); Wed, 27 Jul 2016 20:08:29 -0400 Received: from exvmail4.hynix.com ([166.125.252.92]:59631 "EHLO invmail4.hynix.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1758277AbcG1AIW (ORCPT ); Wed, 27 Jul 2016 20:08:22 -0400 X-AuditID: a67dfc5b-f79386d000007ffb-e4-57994ceac208 From: "kwangwoo.lee@sk.com" To: Robin Murphy , Russell King - ARM Linux , Catalin Marinas , "Will Deacon" , Mark Rutland , "linux-arm-kernel@lists.infradead.org" CC: "hyunchul3.kim@sk.com" , "linux-kernel@vger.kernel.org" , "woosuk.chung@sk.com" Subject: RE: [PATCH v2] arm64: mm: convert __dma_* routines to use start, size Thread-Topic: [PATCH v2] arm64: mm: convert __dma_* routines to use start, size Thread-Index: AQHR5xBcaKHsJZrE6EmNL7MPsHijzaAp7/iAgAFuegCAABbLAIABfUjA Date: Thu, 28 Jul 2016 00:08:09 +0000 Message-ID: References: <1469518496-8177-1-git-send-email-kwangwoo.lee@sk.com> <15c12f9900fd4b31a875250c478023c6@nmail01.hynixad.com> In-Reply-To: Accept-Language: ko-KR, en-US Content-Language: ko-KR X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.92.132.196] Content-Type: text/plain; charset="ks_c_5601-1987" MIME-Version: 1.0 X-CFilter-Loop: Reflected X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrKIsWRmVeSWpSXmKPExsXCNUdUWfeVz8xwg7uXTC3eL+thtNj0+Bqr xeVdc9gsDk3dy2ix9PpFJouDH56wWrz8eILFgd1jzbw1jB6Xr11k9ti8pN7j8ya5AJYoLpuU 1JzMstQifbsErozutm72glP6FVMvvWBrYFyj18XIwSEhYCJxYbdrFyMnkCkmceHeerYuRi4O IYE1jBJLtu1khXAWM0p0PfjHCFLFJmAosffWPlYQW0RgMZPE/mUyIEXMAgsZJba2fQRLCAsE SPzdsJgNoihQ4trpuywQtpvE+8ermUBsFgFVicVtn8HqeQWcJLZ/m8AEse0uo8Tcrl1gRZwC 1hIrd85jBTmVUUBW4uo1GZAws4C4xOKv15ghzhaQWLLnPJQtKvHy8T9WCFtR4tvr7+wQ9UYS S1bPZ4KwFSWmdD9kh9grKHFy5hMWiHpJiYMrbrBMYBSfhWTFLCTts5C0z0LSvoCRZRWjSGZe WW5iZo6JXnF2RmVeZoVecn7uJkZgZC6r/RO9g3HJX49DjAIcjEo8vBNezAgXYk0sK67MPcQo wcGsJMJr4T0zXIg3JbGyKrUoP76oNCe1+BCjNAeLkjjvt/994UIC6YklqdmpqQWpRTBZJg5O qQbGytYrv0TMim+f2tv97G3/85sfOU71+J/S3cV8TDHF7s/OztDtV3U8omaJH2FPWmd5nf+T 1uf70xI+7lO6y+Wf1RZgGTXBTKdTdpLgycczTTfPXcVdnbe3vvPNZKPdcru3aGz89UP5SlB6 s+nHC2zpqrZbxJli7S7eNqkI57nyhYtL/njJdQYvJZbijERDLeai4kQA/bS2GsgCAAA= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id u6S08YWA030712 > -----Original Message----- > From: Robin Murphy [mailto:robin.murphy@arm.com] > Sent: Wednesday, July 27, 2016 6:56 PM > To: À̱¤¿ì(LEE KWANGWOO) MS SW; Russell King - ARM Linux; Catalin Marinas; Will Deacon; Mark Rutland; > linux-arm-kernel@lists.infradead.org > Cc: ±èÇöö(KIM HYUNCHUL) MS SW; linux-kernel@vger.kernel.org; Á¤¿ì¼®(CHUNG WOO SUK) MS SW > Subject: Re: [PATCH v2] arm64: mm: convert __dma_* routines to use start, size > > 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). OK. Using the capability of CPU features seems to be preferred. > 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 I agree that it looks not viable because it makes the macro bigger and conditional specifically with CVAC op. Then.. if the number of the usage of alternative_* macros for erratum is few (just one in this case for cache clean), I think only small change like below seems to be optimal and there is no need to create a variant macro of dcache_cache_by_line_op. How do you think about it? /* - * __dma_clean_range(start, end) + * __clean_dcache_area_poc(kaddr, size) + * + * Ensure that any D-cache lines for the interval [kaddr, kaddr+size) + * are cleaned to the PoC. + * + * - kaddr - kernel address + * - size - size in question + */ +ENTRY(__clean_dcache_area_poc) + /* FALLTHROUGH */ + +/* + * __dma_clean_area(start, size) * - start - virtual start address of region - * - end - virtual end address of region + * - size - size in question */ -__dma_clean_range: +__dma_clean_area: + add x1, x1, x0 dcache_line_size x2, x3 sub x3, x2, #1 bic x0, x0, x3 @@ -158,24 +172,21 @@ alternative_endif b.lo 1b dsb sy ret -ENDPROC(__dma_clean_range) +ENDPIPROC(__clean_dcache_area_poc) +ENDPROC(__dma_clean_area) Regards, Kwangwoo Lee