From: Robin Murphy <robin.murphy@arm.com> To: "kwangwoo.lee@sk.com" <kwangwoo.lee@sk.com>, Russell King - ARM Linux <linux@armlinux.org.uk>, Catalin Marinas <catalin.marinas@arm.com>, Will Deacon <will.deacon@arm.com>, Mark Rutland <mark.rutland@arm.com>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org> Cc: "hyunchul3.kim@sk.com" <hyunchul3.kim@sk.com>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "woosuk.chung@sk.com" <woosuk.chung@sk.com> Subject: Re: [PATCH v2] arm64: mm: convert __dma_* routines to use start, size Date: Wed, 27 Jul 2016 10:56:18 +0100 [thread overview] Message-ID: <c355debc-0238-ff51-ca90-6e0c8085366b@arm.com> (raw) In-Reply-To: <15c12f9900fd4b31a875250c478023c6@nmail01.hynixad.com> 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
WARNING: multiple messages have this Message-ID (diff)
From: robin.murphy@arm.com (Robin Murphy) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v2] arm64: mm: convert __dma_* routines to use start, size Date: Wed, 27 Jul 2016 10:56:18 +0100 [thread overview] Message-ID: <c355debc-0238-ff51-ca90-6e0c8085366b@arm.com> (raw) In-Reply-To: <15c12f9900fd4b31a875250c478023c6@nmail01.hynixad.com> On 27/07/16 02:55, kwangwoo.lee at 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
next prev parent reply other threads:[~2016-07-27 9:56 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-07-26 7:34 [PATCH v2] arm64: mm: convert __dma_* routines to use start, size Kwangwoo Lee 2016-07-26 7:34 ` Kwangwoo Lee 2016-07-26 10:43 ` Robin Murphy 2016-07-26 10:43 ` Robin Murphy 2016-07-27 1:55 ` kwangwoo.lee 2016-07-27 1:55 ` kwangwoo.lee at sk.com 2016-07-27 9:56 ` Robin Murphy [this message] 2016-07-27 9:56 ` Robin Murphy 2016-07-28 0:08 ` kwangwoo.lee 2016-07-28 0:08 ` kwangwoo.lee at sk.com 2016-07-29 17:06 ` Robin Murphy 2016-07-29 17:06 ` Robin Murphy 2016-07-31 23:45 ` kwangwoo.lee 2016-07-31 23:45 ` kwangwoo.lee at sk.com 2016-08-01 13:36 ` Robin Murphy 2016-08-01 13:36 ` Robin Murphy 2016-08-01 13:53 ` Robin Murphy 2016-08-01 13:53 ` Robin Murphy 2016-08-01 23:24 ` kwangwoo.lee 2016-08-01 23:24 ` kwangwoo.lee at sk.com
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=c355debc-0238-ff51-ca90-6e0c8085366b@arm.com \ --to=robin.murphy@arm.com \ --cc=catalin.marinas@arm.com \ --cc=hyunchul3.kim@sk.com \ --cc=kwangwoo.lee@sk.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux@armlinux.org.uk \ --cc=mark.rutland@arm.com \ --cc=will.deacon@arm.com \ --cc=woosuk.chung@sk.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.