Date:   Fri, 27 Jan 2023 23:36:38 +0000
From:   Conor Dooley <conor@kernel.org>
To:     guoren@kernel.org
Cc:     arnd@arndb.de, palmer@rivosinc.com, conor.dooley@microchip.com,
        apatel@ventanamicro.com, atishp@atishpatra.org,
        mark.rutland@arm.com, bjorn@kernel.org, tongtiangen@huawei.com,
        ajones@ventanamicro.com, andrew@sifive.com,
        linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org,
        Guo Ren <guoren@linux.alibaba.com>
Subject: Re: [PATCH V2] riscv: Fixup race condition on PG_dcache_clean in
 flush_icache_pte
Message-ID: <Y9RgBh9X6KM1/uvp@spud>
References: <20230127035306.1819561-1-guoren@kernel.org>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="GuNVYafotbXvgZR0"
Content-Disposition: inline
In-Reply-To: <20230127035306.1819561-1-guoren@kernel.org>
Precedence: bulk


--GuNVYafotbXvgZR0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hey Guo Ren,

On Thu, Jan 26, 2023 at 10:53:06PM -0500, guoren@kernel.org wrote:
> From: Guo Ren <guoren@linux.alibaba.com>
>=20
> 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.
>=20
> Fixes: 08f051eda33b ("RISC-V: Flush I$ when making a dirty page executabl=
e")
> 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)
>=20
> V1:
> https://lore.kernel.org/linux-riscv/20221023133205.3493564-2-guoren@kerne=
l.org/
> ---
>  arch/riscv/mm/cacheflush.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>=20
> 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 =3D compound_head(page);
> =20
> -	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 */
> =20
> --=20
> 2.36.1
>=20

--GuNVYafotbXvgZR0
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYIAB0WIQRh246EGq/8RLhDjO14tDGHoIJi0gUCY9RgBgAKCRB4tDGHoIJi
0m+jAQCoWHcrkvq5cRc39KGXssFRtqfRT9ZMxIkLfGtPSIG16QD/fWAAQ8JWzxde
siNntjh6A6FImIAQ0pLweSyiW0bfEAk=
=qMS9
-----END PGP SIGNATURE-----

--GuNVYafotbXvgZR0--