Re: [PATCH v3 0/5] of: reserved_mem: several fixes about reserved memory

[Mike Rapoport]

On Tue, Oct 06, 2026 at 03:26:21PM +0200, Marek Szyprowski wrote:
> On 03.10.2026 10:04, Mike Rapoport wrote:
> > On Tue, Sep 22, 2026 at 05:24:32PM +0800, Wandun wrote:
> >>>> From: Wandun Chen <chenwandun@xxxxxxxxxxx>
> >>>>
> >>>> This series fixes several error-handling issues in the reserved-memory
> >>>> initialization paths.
> >>>>
> >>>> v2 --> v3:
> >>>> 1. Rework the mechanism that checks in the late scan whether the early
> >>>>    reservation succeeded (patches 3-5, suggested by Marek, thanks).
> >>>>
> >>>>    Patch 3 adds a new memblock flag MEMBLOCK_RSRV_RMEM, which is set when
> >>>>    the early reservation of a static region succeeds and checked in the
> >>>>    late scan.
> >>> Can we keep this local to of_reserved_mem please?
> >> Probably not. I do not see a way to keep this entirely local to
> >> of_reserved_mem while handling the issue robustly.
> >>
> >> I previously implemented an approach in of_reserved_mem that records
> >> static reserved-memory nodes whose early reservation failed in a local
> >> array [1]. However, the early scan runs before paging_init(), so the array
> >> cannot be dynamically expanded. If the number of failed nodes exceeds
> >> the array size, some failures cannot be recorded and the issue remains,
> >> and that is why Marek said "partial solution", although in practice
> >> having that many failed nodes is unlikely.
> > Even before paging_init() there is memblock_alloc().
> 
> One can call it, but such memory cannot be dereferenced/accessed for example
> on ARM64, because it is not yet mapped in the linear map.

Can't we use early_memremap() for it?

If there are many regions that need MEMBLOCK_RSRV_RMEM set, memblock will
need to allocate memory. If that memory is still not mapped we'll end up
with a crash in memblock.
 
> >> To handle this robustly, the late scan needs a way to determine whether
> >> the corresponding early reservation actually succeeded. Current approach
> >> uses memblock to retain that state.
> > I can't say I like the idea of keeping this state in memblock.
> > This add flags and code to memblock to deal with corner cases of bad
> > firmware that reports weird memory layouts, and once there is a flag in
> > the common infrastructure, people tend to abuse it.
> 
> So far I found no better place to store the information about successful
> region reservation.

We can have reserve_failed_nodes larger than MAX_RESERVED_REGIONS, it's
__initdata and anyway discarded after boot.

But even a DT with more than 64 bad regions seems broken enough to WARN()
and ignore remaining errors.

> Best regards
> -- 
> Marek Szyprowski, PhD

-- 
Sincerely yours,
Mike.