* Re: [PATCH V2] riscv: Fixup race condition on PG_dcache_clean in flush_icache_pte
2023-01-27 3:53 [PATCH V2] riscv: Fixup race condition on PG_dcache_clean in flush_icache_pte guoren
@ 2023-01-27 11:34 ` Andrew Jones
2023-01-27 23:36 ` Conor Dooley
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Andrew Jones @ 2023-01-27 11:34 UTC (permalink / raw)
To: guoren
Cc: arnd, palmer, conor.dooley, apatel, atishp, mark.rutland, bjorn,
tongtiangen, andrew, linux-kernel, linux-riscv, Guo Ren
On Thu, Jan 26, 2023 at 10:53:06PM -0500, guoren@kernel.org wrote:
> From: Guo Ren <guoren@linux.alibaba.com>
>
> In commit 588a513d3425 ("arm64: Fix race condition on PG_dcache_clean
> in __sync_icache_dcache()"), we found RISC-V has the same issue as the
> previous arm64. The previous implementation didn't guarantee the correct
> sequence of operations, which means flush_icache_all() hasn't been
> called when the PG_dcache_clean was set. That would cause a risk of page
> synchronization.
>
> Fixes: 08f051eda33b ("RISC-V: Flush I$ when making a dirty page executable")
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> Signed-off-by: Guo Ren <guoren@kernel.org>
> ---
> Changelog:
> V2:
> - Optimize commit log
> - Rebase on riscv for-next (20230127)
>
> V1:
> https://lore.kernel.org/linux-riscv/20221023133205.3493564-2-guoren@kernel.org/
> ---
> arch/riscv/mm/cacheflush.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c
> index 3cc07ed45aeb..fcd6145fbead 100644
> --- a/arch/riscv/mm/cacheflush.c
> +++ b/arch/riscv/mm/cacheflush.c
> @@ -90,8 +90,10 @@ void flush_icache_pte(pte_t pte)
> if (PageHuge(page))
> page = compound_head(page);
>
> - if (!test_and_set_bit(PG_dcache_clean, &page->flags))
> + if (!test_bit(PG_dcache_clean, &page->flags)) {
> flush_icache_all();
> + set_bit(PG_dcache_clean, &page->flags);
> + }
> }
> #endif /* CONFIG_MMU */
>
> --
> 2.36.1
>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Thanks,
drew
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH V2] riscv: Fixup race condition on PG_dcache_clean in flush_icache_pte
2023-01-27 3:53 [PATCH V2] riscv: Fixup race condition on PG_dcache_clean in flush_icache_pte guoren
2023-01-27 11:34 ` Andrew Jones
@ 2023-01-27 23:36 ` Conor Dooley
2023-01-28 2:47 ` Guo Ren
2023-02-09 23:37 ` Palmer Dabbelt
2023-02-09 23:40 ` patchwork-bot+linux-riscv
3 siblings, 1 reply; 6+ messages in thread
From: Conor Dooley @ 2023-01-27 23:36 UTC (permalink / raw)
To: guoren
Cc: arnd, palmer, conor.dooley, apatel, atishp, mark.rutland, bjorn,
tongtiangen, ajones, andrew, linux-kernel, linux-riscv, Guo Ren
[-- Attachment #1: Type: text/plain, Size: 1833 bytes --]
Hey Guo Ren,
On Thu, Jan 26, 2023 at 10:53:06PM -0500, guoren@kernel.org wrote:
> From: Guo Ren <guoren@linux.alibaba.com>
>
> In commit 588a513d3425 ("arm64: Fix race condition on PG_dcache_clean
> in __sync_icache_dcache()"), we found RISC-V has the same issue as the
> previous arm64. The previous implementation didn't guarantee the correct
> sequence of operations, which means flush_icache_all() hasn't been
> called when the PG_dcache_clean was set. That would cause a risk of page
> synchronization.
>
> Fixes: 08f051eda33b ("RISC-V: Flush I$ when making a dirty page executable")
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> Signed-off-by: Guo Ren <guoren@kernel.org>
> ---
> Changelog:
> V2:
> - Optimize commit log
Probably would have benefited from providing the analysis that the arm64
commit did, for riscv, rather than referring to theirs.
But that's not really important and the diff itself seems sound, so:
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Thanks,
Conor.
> - Rebase on riscv for-next (20230127)
>
> V1:
> https://lore.kernel.org/linux-riscv/20221023133205.3493564-2-guoren@kernel.org/
> ---
> arch/riscv/mm/cacheflush.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c
> index 3cc07ed45aeb..fcd6145fbead 100644
> --- a/arch/riscv/mm/cacheflush.c
> +++ b/arch/riscv/mm/cacheflush.c
> @@ -90,8 +90,10 @@ void flush_icache_pte(pte_t pte)
> if (PageHuge(page))
> page = compound_head(page);
>
> - if (!test_and_set_bit(PG_dcache_clean, &page->flags))
> + if (!test_bit(PG_dcache_clean, &page->flags)) {
> flush_icache_all();
> + set_bit(PG_dcache_clean, &page->flags);
> + }
> }
> #endif /* CONFIG_MMU */
>
> --
> 2.36.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH V2] riscv: Fixup race condition on PG_dcache_clean in flush_icache_pte
2023-01-27 23:36 ` Conor Dooley
@ 2023-01-28 2:47 ` Guo Ren
0 siblings, 0 replies; 6+ messages in thread
From: Guo Ren @ 2023-01-28 2:47 UTC (permalink / raw)
To: Conor Dooley
Cc: arnd, palmer, conor.dooley, apatel, atishp, mark.rutland, bjorn,
tongtiangen, ajones, andrew, linux-kernel, linux-riscv, Guo Ren
On Sat, Jan 28, 2023 at 7:36 AM Conor Dooley <conor@kernel.org> wrote:
>
> Hey Guo Ren,
>
> On Thu, Jan 26, 2023 at 10:53:06PM -0500, guoren@kernel.org wrote:
> > From: Guo Ren <guoren@linux.alibaba.com>
> >
> > In commit 588a513d3425 ("arm64: Fix race condition on PG_dcache_clean
> > in __sync_icache_dcache()"), we found RISC-V has the same issue as the
> > previous arm64. The previous implementation didn't guarantee the correct
> > sequence of operations, which means flush_icache_all() hasn't been
> > called when the PG_dcache_clean was set. That would cause a risk of page
> > synchronization.
> >
> > Fixes: 08f051eda33b ("RISC-V: Flush I$ when making a dirty page executable")
> > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > Signed-off-by: Guo Ren <guoren@kernel.org>
> > ---
> > Changelog:
> > V2:
> > - Optimize commit log
>
> Probably would have benefited from providing the analysis that the arm64
> commit did, for riscv, rather than referring to theirs.
Maybe you are right, but they are almost the same for this patch. So I
didn't want to duplicate the commit log but gave out the original
instead.
> But that's not really important and the diff itself seems sound, so:
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Thx.
>
> Thanks,
> Conor.
>
> > - Rebase on riscv for-next (20230127)
> >
> > V1:
> > https://lore.kernel.org/linux-riscv/20221023133205.3493564-2-guoren@kernel.org/
> > ---
> > arch/riscv/mm/cacheflush.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c
> > index 3cc07ed45aeb..fcd6145fbead 100644
> > --- a/arch/riscv/mm/cacheflush.c
> > +++ b/arch/riscv/mm/cacheflush.c
> > @@ -90,8 +90,10 @@ void flush_icache_pte(pte_t pte)
> > if (PageHuge(page))
> > page = compound_head(page);
> >
> > - if (!test_and_set_bit(PG_dcache_clean, &page->flags))
> > + if (!test_bit(PG_dcache_clean, &page->flags)) {
> > flush_icache_all();
> > + set_bit(PG_dcache_clean, &page->flags);
> > + }
> > }
> > #endif /* CONFIG_MMU */
> >
> > --
> > 2.36.1
> >
--
Best Regards
Guo Ren
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V2] riscv: Fixup race condition on PG_dcache_clean in flush_icache_pte
2023-01-27 3:53 [PATCH V2] riscv: Fixup race condition on PG_dcache_clean in flush_icache_pte guoren
2023-01-27 11:34 ` Andrew Jones
2023-01-27 23:36 ` Conor Dooley
@ 2023-02-09 23:37 ` Palmer Dabbelt
2023-02-09 23:40 ` patchwork-bot+linux-riscv
3 siblings, 0 replies; 6+ messages in thread
From: Palmer Dabbelt @ 2023-02-09 23:37 UTC (permalink / raw)
To: Arnd Bergmann, Conor Dooley, apatel, atishp, Mark Rutland, bjorn,
tongtiangen, ajones, Andrew Waterman, guoren
Cc: linux-kernel, linux-riscv, Guo Ren
On Thu, 26 Jan 2023 22:53:06 -0500, guoren@kernel.org wrote:
> In commit 588a513d3425 ("arm64: Fix race condition on PG_dcache_clean
> in __sync_icache_dcache()"), we found RISC-V has the same issue as the
> previous arm64. The previous implementation didn't guarantee the correct
> sequence of operations, which means flush_icache_all() hasn't been
> called when the PG_dcache_clean was set. That would cause a risk of page
> synchronization.
>
> [...]
Applied, thanks!
[1/1] riscv: Fixup race condition on PG_dcache_clean in flush_icache_pte
https://git.kernel.org/palmer/c/950b879b7f02
Best regards,
--
Palmer Dabbelt <palmer@rivosinc.com>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH V2] riscv: Fixup race condition on PG_dcache_clean in flush_icache_pte
2023-01-27 3:53 [PATCH V2] riscv: Fixup race condition on PG_dcache_clean in flush_icache_pte guoren
` (2 preceding siblings ...)
2023-02-09 23:37 ` Palmer Dabbelt
@ 2023-02-09 23:40 ` patchwork-bot+linux-riscv
3 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+linux-riscv @ 2023-02-09 23:40 UTC (permalink / raw)
To: Guo Ren
Cc: linux-riscv, arnd, palmer, conor.dooley, apatel, atishp,
mark.rutland, bjorn, tongtiangen, ajones, andrew, linux-kernel,
guoren
Hello:
This patch was applied to riscv/linux.git (fixes)
by Palmer Dabbelt <palmer@rivosinc.com>:
On Thu, 26 Jan 2023 22:53:06 -0500 you wrote:
> From: Guo Ren <guoren@linux.alibaba.com>
>
> In commit 588a513d3425 ("arm64: Fix race condition on PG_dcache_clean
> in __sync_icache_dcache()"), we found RISC-V has the same issue as the
> previous arm64. The previous implementation didn't guarantee the correct
> sequence of operations, which means flush_icache_all() hasn't been
> called when the PG_dcache_clean was set. That would cause a risk of page
> synchronization.
>
> [...]
Here is the summary with links:
- [V2] riscv: Fixup race condition on PG_dcache_clean in flush_icache_pte
https://git.kernel.org/riscv/c/950b879b7f02
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 6+ messages in thread