linux-kernel - Re: [PATCH v3 10/10] riscv: dts: eswin: add HiFive Premier P550 board device tree

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <b82ae3ed-442c-4a87-82be-a973cd333287@collabora.com>
Date: Wed, 16 Apr 2025 16:50:51 -0300
From: Ariel D'Alessandro <ariel.dalessandro@...labora.com>
To: Sjoerd Simons <sjoerd@...labora.com>,
 Samuel Holland <samuel.holland@...ive.com>,
 Pinkesh Vaghela <pinkesh.vaghela@...fochips.com>,
 Conor Dooley <conor@...nel.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Thomas Gleixner <tglx@...utronix.de>
Cc: Paul Walmsley <paul.walmsley@...ive.com>,
 Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>,
 Daniel Lezcano <daniel.lezcano@...aro.org>,
 Min Lin <linmin@...incomputing.com>,
 Pritesh Patel <pritesh.patel@...fochips.com>, Yangyu Chen
 <cyy@...self.name>, Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>,
 Yu Chien Peter Lin <peterlin@...estech.com>,
 Charlie Jenkins <charlie@...osinc.com>,
 Kanak Shilledar <kanakshilledar@...il.com>,
 Darshan Prajapati <darshan.prajapati@...fochips.com>,
 Neil Armstrong <neil.armstrong@...aro.org>, Heiko Stuebner
 <heiko@...ech.de>, Aradhya Bhatia <a-bhatia1@...com>, rafal@...ecki.pl,
 Anup Patel <anup@...infault.org>, devicetree@...r.kernel.org,
 linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org,
 "kernel@...labora.com" <kernel@...labora.com>
Subject: Re: [PATCH v3 10/10] riscv: dts: eswin: add HiFive Premier P550 board
 device tree

Samuel, Sjoerd,

On 4/15/25 4:39 AM, Sjoerd Simons wrote:
> Hey,
> 
> On Mon, 2025-04-14 at 11:00 -0500, Samuel Holland wrote:
>> Hi Ariel,
>>
>> On 2025-04-14 7:55 AM, Ariel D'Alessandro wrote:
>>> Hi Pinkesh,
>>>
>>> On 4/10/25 12:25 PM, Pinkesh Vaghela wrote:
>>>> From: Min Lin <linmin@...incomputing.com>
>>>
> <snip>
> 
>>> Although commit log says that this includes DRAM configuration, looks like
>>> it's
>>> missing? In order to test this patchset, had to add this following memory
>>> definition (picked from vendor kernel repository):
>>>
>>>      L50: memory@...00000 {
>>>              compatible = "sifive,axi4-mem-port", "sifive,axi4-port",
>>> "sifive,mem-port";
>>>              device_type = "memory";
>>>              reg = <0x0 0x80000000 0x7f 0x80000000>;
>>>              sifive,port-width-bytes = <32>;
>>>      };
>>
>> That is a misstatement in the commit message. The memory node is not included
>> in
>> the static devicetree because the amount of RAM installed on the board is
>> variable. It is the responsibility of firmware to provide the memory map,
>> either
>> through EFI or by patching the memory node into the DT at runtime. I believe
>> the
>> current BSP U-Boot does the former but not the latter.
> 
> Amount of RAM being variable is pretty common on devices using FDT these days;
> Typically the dts still gets a memory node that's a reasonable default, with the
> expectation that e.g. u-boot will fix it up. If you look at other risc-v
> devicetrees in upstream they (almost?) all come with a pre-defined memory node.
> For the P550 board a default memory node for 16G ram seems reasonable (as that
> seems the minimal SKU?)
> 
> That all being said. Indeed the sifive BSP u-boot doesn't seem to call the
> relevant `fdt_fixup_memory` to fixup the memory node, hence us having issues
> booting with u-boot directly (without going through EFI). Honestly this was a
> bit of a surprise to me as only most other architectures that's just done by
> common code, but that doesn't seem to be the case for risc-v (either upstream or
> downstream)

As Samuel mentioned, the latest BSP U-Boot is now patching/populating 
the DT memory node at runtime, after commit [0]. And this indeed ends up 
calling `fdt_fixup_memory()` as Sjoerd pointed out.

In conclusion, this is working properly with the current BSP U-Boot. 
Feel free to add:

   Tested-by: Ariel D'Alessandro <ariel.dalessandro@...labora.com>

Thanks!

[0] 
https://github.com/eswincomputing/u-boot/commit/7fab50468f19efea72ff27ac08cb388fbf5be307

-- 
Ariel D'Alessandro
Software Engineer

Collabora Ltd.
Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK 
Registered in England & Wales, no. 5513718