Re: [PATCH 06/11] riscv: mm: Fix up memory types when writing page tables

From: Samuel Holland <samuel.holland@sifive.com>
To: Alexandre Ghiti <alex@ghiti.fr>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	linux-riscv@lists.infradead.org, Conor Dooley <conor@kernel.org>
Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	Alexandre Ghiti <alexghiti@rivosinc.com>,
	Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>,
	Emil Renner Berthing <kernel@esmil.dk>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>
Subject: Re: [PATCH 06/11] riscv: mm: Fix up memory types when writing page tables
Date: Wed, 8 Oct 2025 21:12:28 -0500	[thread overview]
Message-ID: <e155e1f8-0b54-432e-a155-eec5ca41c341@sifive.com> (raw)
In-Reply-To: <4123bd5d-33b8-4248-a03a-de784c433040@ghiti.fr>

Hi Alex,

On 2024-11-05 5:03 AM, Alexandre Ghiti wrote:
> On 02/11/2024 01:08, Samuel Holland wrote:
>> Currently, Linux on RISC-V has three ways to specify the cacheability
>> and ordering PMAs of a page:
>>   1) Do nothing; assume the system is entirely cache-coherent and rely on
>>      the hardware for any ordering requirements
>>   2) Use the page table bits specified by Svpbmt
>>   3) Use the page table bits specified by XTheadMae
>>
>> To support all three methods, the kernel dynamically determines the
>> definitions of the _PAGE_NOCACHE and _PAGE_IO fields. However, this
>> alone is not sufficient, as XTheadMae uses a nonzero memory type value
>> for normal memory pages. So the kernel has an additional alternative
>> sequence (ALT_THEAD_PMA) to insert the correct memory type when writing
>> page table entries.
> 
> 
> I have just taken a look, and it's not exactly when the page table is written
> but rather when the page table entry is being created.
> 
> And I have to admit that I find it weird, moving that to the set_pXd() functions
> seems way more robust. Indeed those functions must be used to write a page table
> entry but a page table entry can be created by other means than with the
> pfn_pXd() functions.
> 
> This is what I did for NAPOT to hide the size of the mapping contained in the
> pfn from the generic kernel here https://lore.kernel.org/linux-
> riscv/20240802151430.99114-1-alexghiti@rivosinc.com/

I've just sent out a v2 which follows your suggestion[1]. It turns out that the
pXXp_get()/set_pXX() functions aren't 100% robust either, with some examples
shown in patch 5 of the new series[2]. And there's some ongoing discussion about
if it's appropriate to put nontrivial code in those helper functions[3]. So I
don't know what's really the best strategy here.

Regards,
Samuel

[1]:
https://lore.kernel.org/linux-riscv/20251009015839.3460231-1-samuel.holland@sifive.com/
[2]:
https://lore.kernel.org/linux-riscv/20251009015839.3460231-6-samuel.holland@sifive.com/
[3]:
https://lore.kernel.org/all/20251006082238.GQ3245006@noisy.programming.kicks-ass.net/

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv