Re: [PATCH v3] platform: generic: Keep some empty space in FDT passed to next stage

From: Samuel Holland <samuel.holland@sifive.com>
To: Anup Patel <anup@brainfault.org>, Anup Patel <apatel@ventanamicro.com>
Cc: Atish Patra <atishp@rivosinc.com>,
	Andrew Jones <ajones@ventanamicro.com>,
	opensbi@lists.infradead.org
Subject: Re: [PATCH v3] platform: generic: Keep some empty space in FDT passed to next stage
Date: Tue, 16 Dec 2025 09:55:48 -0600	[thread overview]
Message-ID: <bdfa024a-ad3d-47af-b629-ede7d4ecd607@sifive.com> (raw)
In-Reply-To: <CAAhSdy3wZ=ih970E1ZT9tW+=gz1gYbjMWELriHX1ya-pr40iUw@mail.gmail.com>

Anup,

On 2025-12-16 8:45 AM, Anup Patel wrote:
> On Tue, Dec 9, 2025 at 11:01 AM Anup Patel <apatel@ventanamicro.com> wrote:
>>
>> Leaving no empty space in the FDT passed to the next booting stage
>> causes the following U-Boot crash on Ventana internal platforms:
>>
>> Unhandled exception: Load access fault
>> EPC: 00000000fffa6372 RA: 00000000fffa7418 TVAL: 0001746174730068
>> EPC: 0000000080245372 RA: 0000000080246418 reloc adjusted
>>
>> SP:  00000000fef38440 GP:  00000000fef40e60 TP:  0000000000000000
>> T0:  00000000fef40a70 T1:  000000000000ff00 T2:  0000000000000000
>> S0:  00000000fffc17a8 S1:  00000000fef38d40 A0:  7375746174730068
>> A1:  00000000fffc17a8 A2:  0000000000000010 A3:  0000000000000010
>> A4:  0000000000000000 A5:  00000000fffc17b8 A6:  0000000000ff0000
>> A7:  000000000000b100 S2:  0000000000000000 S3:  0000000000000001
>> S4:  00000000fef38d40 S5:  7375746174730068 S6:  0000000000000000
>> S7:  00000000fef4eef0 S8:  00000000fef4ef90 S9:  0000000000000000
>> S10: 0000000000000000 S11: 00000000fef4efc0 T3:  00000000fef40ea8
>> T4:  0000000000ff0000 T5:  00000000fef40a60 T6:  00000000fef40a6c
>>
>> To address the above issue, keep some minimal empty space in the
>> FDT instead of no empty space.
>>
>> Fixes: bbe9a23060e9 ("platform: generic: Pack the FDT after applying fixups")
>> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> 
> Applied this patch to the riscv/opensbi repo.

Again, this patch reintroduces the bug that was fixed by the "Fixes:" commit.
You can't just say "somebody will add the memset() somewhere later" and ignore
the bug you are introducing. You need to add the memset() *now* along with the
fdt_open_into() call.

Regards,
Samuel

>> ---
>> Changes since v2:
>> - Fix comments in generic_final_init()
>> Changes since v1:
>> - Add Kconfig option for amount of empty space in FDT with default
>>   value as 4KB
>> ---
>>  platform/generic/Kconfig    | 5 +++++
>>  platform/generic/platform.c | 3 +++
>>  2 files changed, 8 insertions(+)
>>
>> diff --git a/platform/generic/Kconfig b/platform/generic/Kconfig
>> index 25b8886b..b1808012 100644
>> --- a/platform/generic/Kconfig
>> +++ b/platform/generic/Kconfig
>> @@ -23,6 +23,11 @@ config PLATFORM_GENERIC_MINOR_VER
>>         range 0 65535
>>         default 1
>>
>> +config PLATFORM_GENERIC_FDT_EMPTY_SPACE
>> +       int "Platform FDT empty space (KB)"
>> +       range 0 1024
>> +       default 4
>> +
>>  config PLATFORM_ALLWINNER_D1
>>         bool "Allwinner D1 support"
>>         depends on FDT_IRQCHIP_PLIC
>> diff --git a/platform/generic/platform.c b/platform/generic/platform.c
>> index e66f99fa..01fa03e0 100644
>> --- a/platform/generic/platform.c
>> +++ b/platform/generic/platform.c
>> @@ -245,7 +245,10 @@ int generic_final_init(bool cold_boot)
>>         fdt_fixups(fdt);
>>         fdt_domain_fixup(fdt);
>>
>> +       /* Set the empty space in FDT based on kconfig option */
>>         fdt_pack(fdt);
>> +       fdt_open_into(fdt, fdt, fdt_totalsize(fdt) +
>> +                     CONFIG_PLATFORM_GENERIC_FDT_EMPTY_SPACE * 1024);
>>
>>         return 0;
>>  }
>> --
>> 2.43.0
>>
> 

-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi