Re: [RFC PATCH 1/6] riscv: Add a custom, simplified version of Svpbmt "XPbmtUC"

[Bo Gan <ganboing@gmail.com>]

Hi Conor,

On 3/13/26 16:55, Conor Dooley wrote:
> On Fri, Mar 13, 2026 at 02:33:16PM -0700, Bo Gan wrote:
>> Hi Conor,
>>
>> Thanks so much for the prompt review. See inline.
>>
>> On 3/13/26 06:24, Conor Dooley wrote:
>>> Hey,
>>>
>>> Gonna offer some feedback on the detail of what's been done in this
>>> series, without providing any commentary on whether this is the correct
>>> approach to take.
>>>
>>> On Fri, Mar 13, 2026 at 01:44:02AM -0700, Bo Gan wrote:
>>>> On platforms that doesn't support Svpbmt or XTheadMae, SoC vendors
>>>> sometimes map the system memory twice in physical address space, one
>>>> as cached, and the other as uncached. Through the uncached window,
>>>> device drivers will be able to map DMA buffer for noncoherent devices.
>>>> Such setup is usually found in SoC with pre-Svpbmt Sifive cores.
>>>> Make use of such feature by modeling it as "XPbmtUC", a customized
>>>> version of Svpbmt, where a single bit in PTE is used for UC control.
>>>> There's no IO bit with such scheme, as it's assumed that the PMA
>>>> (usually hard-wired on these SoCs) will properly convey the strongly-
>>>> ordered, non-idempotent attribute of the MMIO region.
>>>>
>>>> The enablement of such position of "XPbmtUC" is controlled by the
>>>> device-tree property "riscv,xpbmt-uncache-bit".
>>>
>>> Firstly, the naming generally I take some exception to. If this is some
>>> fake vendor extension for linux purposes, it needs to have "xlinux" in
>>> it, like our xlinuxenvcfg does. It should also be consistent, don't use
>>> "xpmbtuc" and "xpbmt-uncache-bit", pick one and stick to it.
>>>
>> Makes sense. I can certainly change that to be conformant.
>>
>>> Athough, I think I disagree fundamentally with this property, as it seems
>>> to me like "software configuration" that shouldn't be permitted in
>>> devicetree. Maybe I am misunderstanding, but the numbers you chose are
>>> convenient, not set in stone by the specific hardware, right?
>>
>> For JH7110, the bit 32 (PPN bit 34) matches exactly with the HW. Meaning
>> toggling this bit would re-map the page to the uncached window, which
>> matches perfectly with the synthetic UC bit in the scheme.
> 
> What does "matches exactly with the hardware" mean? AFAICT, you picked
> it because it was the best value, but you could also have picked another
> less optimal value?
> 
>>
>> For EIC770X, the bit 38 (PPN bit 40) is hand picked to be able to map all
>> physical memory space (40 bit), while making it very easy for the thin-
>> hypervisor, which can utilize Sv39x4 (41 bit) page scheme in G-stage.
>>
>> I also considered the sbi call approach, where the kernel can query for
>> the support and position of the uncache bit. The thing is that JH7110
>> can just hard-code the bit without any changes to firmware, and I want
>> to have a consistent way for both SoC, thus the device-tree approach, to
>> let the EIC770X firmware/bootloader adding the property to dt at runtime.
>> Any better ideas?
> 
> Is the only thing that's variable on your eic770x platform whether or
> not the bit is enabled? Or are you looking to vary the bit depending on
> the specific platform?
> 

It'll be "fixed" for eic770x if a thin-hypervisor re-mapping is enabled
underneath. It just so happens that the physical address space is 40 bits
(ignoring the 40bit+ upper uncached region for interleaved memory, which
we don't need when the "xpbmt-uc" is enabled anyway), and the hypervisor
can use Sv39x4 (also 41bit) to re-map everything.

The variation comes with different SoCs, JH7110 vs. EIC770X. I'd like to
make it a variable, to make a unified kernel binary boot on all SoCs, so
I need to fix the alternative logic for PC-relative instructions to read
from a global variable "xpbmtuc_bit/mask". Also I want to avoid adding
too many branches to the alternative macro.

>>> I'd be much more comfortable with adding xlinuxwhatever to
>>> riscv,isa-extensions, to signal that a soc supports this stuff than with
>>> a property for the bit itself. I suppose that bit information could then
>>> come from a LUT in the vendor extensions, that a validate callback could
>>> check (via root compatible) before enabling. There's not a super neat
>>> way to do that at the moment though I don't think, code currently
>>> expects that vendor extensions are in a different "namespace" to
>>> standard ones, and this would blur the lines because it's not from a
>>> specific vendor, nor is it a standard extension.
>>> I guess, it could be done by keeping it as a standard number, but then
>>> it's a bit trickier to neatly access the LUT while keeping it split
>>> apart.
>>> I know this means having to modify the kernel if there's a new device,
>>> but I'm inclined to say "deal with it" because they could've done
>>> something standard and opted not to.
>>>
>>> Could also argue that this should be shoved into a sifive specific
>>> thing, but I don't expect that they're the only ones with devices like
>>> this that could benefit.
>>>
>>
>> I've thought about riscv,isa-extensions. The issue with that is that it's
>> a per-CPU thing, but I'm adding a global extension, and I don't want to
> 
> Most of the extensions in that string are effectively global. There's no
> need to worry about "polluting" it.
> 

Got it. So I can use something like "xlinuxpbmtuc38" in isa-string? (until
someone comes up with a better naming. Naming things is hard...)

>> pollute the isa-extension string. Thus, I followed Samuel's approach --
>> He uses "riscv,physical-memory-regions" in the root node.

Bo

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