From:   =?utf-8?B?QmrDtnJuIFTDtnBlbA==?= <bjorn@kernel.org>
To:     Alexandre Ghiti <alexghiti@rivosinc.com>,
        Paul Walmsley <paul.walmsley@sifive.com>,
        Palmer Dabbelt <palmer@dabbelt.com>,
        Albert Ou <aou@eecs.berkeley.edu>,
        Andrey Ryabinin <ryabinin.a.a@gmail.com>,
        Alexander Potapenko <glider@google.com>,
        Andrey Konovalov <andreyknvl@gmail.com>,
        Dmitry Vyukov <dvyukov@google.com>,
        Vincenzo Frascino <vincenzo.frascino@arm.com>,
        Ard Biesheuvel <ardb@kernel.org>,
        Conor Dooley <conor@kernel.org>,
        linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
        kasan-dev@googlegroups.com, linux-efi@vger.kernel.org
Cc:     Alexandre Ghiti <alexghiti@rivosinc.com>
Subject: Re: [PATCH v4 2/6] riscv: Rework kasan population functions
In-Reply-To: <20230203075232.274282-3-alexghiti@rivosinc.com>
References: <20230203075232.274282-1-alexghiti@rivosinc.com>
 <20230203075232.274282-3-alexghiti@rivosinc.com>
Date:   Fri, 17 Feb 2023 15:54:06 +0100
Message-ID: <87lekwmjg1.fsf@all.your.base.are.belong.to.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Alexandre Ghiti <alexghiti@rivosinc.com> writes:

> Our previous kasan population implementation used to have the final kasan
> shadow region mapped with kasan_early_shadow_page, because we did not cle=
an
> the early mapping and then we had to populate the kasan region "in-place"
> which made the code cumbersome.
>
> So now we clear the early mapping, establish a temporary mapping while we
> populate the kasan shadow region with just the kernel regions that will
> be used.
>
> This new version uses the "generic" way of going through a page table
> that may be folded at runtime (avoid the XXX_next macros).
>
> It was tested with outline instrumentation on an Ubuntu kernel
> configuration successfully.
>
> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>

(One minor nit, that can be addressed later.)

Reviewed-by: Bj=C3=B6rn T=C3=B6pel <bjorn@rivosinc.com>

>  arch/riscv/mm/kasan_init.c | 361 +++++++++++++++++++------------------
>  1 file changed, 183 insertions(+), 178 deletions(-)


> @@ -482,7 +437,37 @@ static void __init kasan_shallow_populate(void *star=
t, void *end)
>  	unsigned long vend =3D PAGE_ALIGN((unsigned long)end);
>=20=20
>  	kasan_shallow_populate_pgd(vaddr, vend);
> -	local_flush_tlb_all();
> +}
> +
> +static void create_tmp_mapping(void)
> +{
> +	void *ptr;
> +	p4d_t *base_p4d;
> +
> +	/*
> +	 * We need to clean the early mapping: this is hard to achieve "in-plac=
e",
> +	 * so install a temporary mapping like arm64 and x86 do.
> +	 */
> +	memcpy(tmp_pg_dir, swapper_pg_dir, sizeof(pgd_t) * PTRS_PER_PGD);
> +
> +	/* Copy the last p4d since it is shared with the kernel mapping. */
> +	if (pgtable_l5_enabled) {
> +		ptr =3D (p4d_t *)pgd_page_vaddr(*pgd_offset_k(KASAN_SHADOW_END));
> +		memcpy(tmp_p4d, ptr, sizeof(p4d_t) * PTRS_PER_P4D);
> +		set_pgd(&tmp_pg_dir[pgd_index(KASAN_SHADOW_END)],
> +			pfn_pgd(PFN_DOWN(__pa(tmp_p4d)), PAGE_TABLE));
> +		base_p4d =3D tmp_p4d;
> +	} else {
> +		base_p4d =3D (p4d_t *)tmp_pg_dir;
> +	}
> +
> +	/* Copy the last pud since it is shared with the kernel mapping. */
> +	if (pgtable_l4_enabled) {
> +		ptr =3D (pud_t *)p4d_page_vaddr(*(base_p4d + p4d_index(KASAN_SHADOW_EN=
D)));
> +		memcpy(tmp_pud, ptr, sizeof(pud_t) * PTRS_PER_PUD);
> +		set_p4d(&base_p4d[p4d_index(KASAN_SHADOW_END)],
> +			pfn_p4d(PFN_DOWN(__pa(tmp_pud)), PAGE_TABLE));
> +	}
>  }
>=20=20
>  void __init kasan_init(void)
> @@ -490,10 +475,27 @@ void __init kasan_init(void)
>  	phys_addr_t p_start, p_end;
>  	u64 i;
>=20=20
> -	if (IS_ENABLED(CONFIG_KASAN_VMALLOC))
> +	create_tmp_mapping();
> +	csr_write(CSR_SATP, PFN_DOWN(__pa(tmp_pg_dir)) | satp_mode);

Nit: Maybe add a comment, why the sfence.vma is *not* required here. I
tripped over it.


Bj=C3=B6rn