From: Michael Ellerman <mpe@ellerman•id.au>
To: Christophe Leroy <christophe.leroy@c-s•fr>,
Benjamin Herrenschmidt <benh@kernel•crashing.org>,
Paul Mackerras <paulus@samba•org>,
Oliver O'Halloran <oohall@gmail•com>,
Segher Boessenkool <segher@kernel•crashing.org>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux•ibm.com>,
linuxppc-dev@lists•ozlabs.org, linux-kernel@vger•kernel.org,
Alastair D'Silva <alastair@d-silva•org>
Subject: powerpc flush_inval_dcache_range() was buggy until v5.3-rc1 (was Re: [PATCH 4/4] powerpc/64: reuse PPC32 static inline flush_dcache_range())
Date: Thu, 08 Aug 2019 16:53:21 +1000 [thread overview]
Message-ID: <87ef1wtafy.fsf@concordia.ellerman.id.au> (raw)
In-Reply-To: d6f628ffdeb9c7863da722a8f6ef2949e57bb360.1557824379.git.christophe.leroy@c-s.fr
[ deliberately broke threading so this doesn't get buried ]
Christophe Leroy <christophe.leroy@c-s•fr> writes:
> diff --git a/arch/powerpc/kernel/misc_64.S b/arch/powerpc/kernel/misc_64.S
> index a4fd536efb44..1b0a42c50ef1 100644
> --- a/arch/powerpc/kernel/misc_64.S
> +++ b/arch/powerpc/kernel/misc_64.S
> @@ -115,35 +115,6 @@ _ASM_NOKPROBE_SYMBOL(flush_icache_range)
> EXPORT_SYMBOL(flush_icache_range)
>
> /*
> - * Like above, but only do the D-cache.
> - *
> - * flush_dcache_range(unsigned long start, unsigned long stop)
> - *
> - * flush all bytes from start to stop-1 inclusive
> - */
> -
> -_GLOBAL_TOC(flush_dcache_range)
> - ld r10,PPC64_CACHES@toc(r2)
> - lwz r7,DCACHEL1BLOCKSIZE(r10) /* Get dcache block size */
> - addi r5,r7,-1
> - andc r6,r3,r5 /* round low to line bdy */
> - subf r8,r6,r4 /* compute length */
> - add r8,r8,r5 /* ensure we get enough */
> - lwz r9,DCACHEL1LOGBLOCKSIZE(r10)/* Get log-2 of dcache block size */
> - srw. r8,r8,r9 /* compute line count */
^
> - beqlr /* nothing to do? */
Alastair noticed that this was a 32-bit right shift.
Meaning if you called flush_dcache_range() with a range larger than 4GB,
it did nothing and returned.
That code (which was previously called flush_inval_dcache_range()) was
merged back in 2005:
https://github.com/mpe/linux-fullhistory/commit/faa5ee3743ff9b6df9f9a03600e34fdae596cfb2#diff-67c7ffa8e420c7d4206cae4a9e888e14
Back then it was only used by the smu.c driver, which presumably wasn't
flushing more than 4GB.
Over time it grew more users:
v4.17 (Apr 2018): fb5924fddf9e ("powerpc/mm: Flush cache on memory hot(un)plug")
v4.15 (Nov 2017): 6c44741d75a2 ("powerpc/lib: Implement UACCESS_FLUSHCACHE API")
v4.15 (Nov 2017): 32ce3862af3c ("powerpc/lib: Implement PMEM API")
v4.8 (Jul 2016): c40785ad305b ("powerpc/dart: Use a cachable DART")
The DART case doesn't matter, but the others probably could. I assume
the lack of bug reports is due to the fact that pmem stuff is still in
development and the lack of flushing usually doesn't actually matter? Or
are people flushing/hotplugging < 4G at a time?
Anyway we probably want to backport the fix below to various places?
cheers
diff --git a/arch/powerpc/kernel/misc_64.S b/arch/powerpc/kernel/misc_64.S
index 1ad4089dd110..802f5abbf061 100644
--- a/arch/powerpc/kernel/misc_64.S
+++ b/arch/powerpc/kernel/misc_64.S
@@ -148,7 +148,7 @@ _GLOBAL(flush_inval_dcache_range)
subf r8,r6,r4 /* compute length */
add r8,r8,r5 /* ensure we get enough */
lwz r9,DCACHEL1LOGBLOCKSIZE(r10)/* Get log-2 of dcache block size */
- srw. r8,r8,r9 /* compute line count */
+ srd. r8,r8,r9 /* compute line count */
beqlr /* nothing to do? */
sync
isync
next prev parent reply other threads:[~2019-08-08 6:55 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-14 9:05 [PATCH 1/4] powerpc/64: flush_inval_dcache_range() becomes flush_dcache_range() Christophe Leroy
2019-05-14 9:05 ` [PATCH 2/4] powerpc/32: activate ARCH_HAS_PMEM_API and ARCH_HAS_UACCESS_FLUSHCACHE Christophe Leroy
2019-07-15 6:49 ` Michael Ellerman
2019-07-15 7:02 ` Oliver O'Halloran
2019-07-15 17:03 ` Christophe Leroy
2019-05-14 9:05 ` [PATCH 3/4] powerpc/32: define helpers to get L1 cache sizes Christophe Leroy
2019-07-08 1:19 ` Michael Ellerman
2019-05-14 9:05 ` [PATCH 4/4] powerpc/64: reuse PPC32 static inline flush_dcache_range() Christophe Leroy
2019-07-08 1:19 ` Michael Ellerman
2019-07-08 14:21 ` Aneesh Kumar K.V
2019-07-09 2:20 ` Oliver O'Halloran
2019-07-09 2:51 ` Aneesh Kumar K.V
2019-07-09 5:29 ` Oliver O'Halloran
2019-07-09 17:04 ` Segher Boessenkool
2019-08-08 6:53 ` Michael Ellerman [this message]
2019-07-08 1:19 ` [PATCH 1/4] powerpc/64: flush_inval_dcache_range() becomes flush_dcache_range() Michael Ellerman
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=87ef1wtafy.fsf@concordia.ellerman.id.au \
--to=mpe@ellerman$(echo .)id.au \
--cc=alastair@d-silva$(echo .)org \
--cc=aneesh.kumar@linux$(echo .)ibm.com \
--cc=benh@kernel$(echo .)crashing.org \
--cc=christophe.leroy@c-s$(echo .)fr \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
--cc=oohall@gmail$(echo .)com \
--cc=paulus@samba$(echo .)org \
--cc=segher@kernel$(echo .)crashing.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox