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

[Mike Rapoport <rppt@kernel.org>]

On Tue, Sep 22, 2026 at 05:24:32PM +0800, Wandun wrote:
> 
> 
> On 9/22/26 16:48, Mike Rapoport wrote:
> > On Sun, Sep 20, 2026 at 05:28:47PM +0800, Wandun Chen wrote:
> >> From: Wandun Chen <chenwandun@lixiang.com>
> >>
> >> This series fixes several error-handling issues in the reserved-memory
> >> initialization paths.
> >>
> >> The first two patches fix cleanup of no-map regions after driver
> >> initialization failure.
> >>
> >> Static reserved-memory nodes are reserved during the early DT scan but
> >> initialized later. The third patch tags regions whose early reservation
> >> succeeded, so the late scan can skip the nodes whose early reservation
> >> failed.
> >>
> >> The last two patches reject overlapping static regions. Without
> >> these checks, overlapping nodes can be initialized over the same
> >> physical memory, result in data corrupt.
> >>
> >> Sashiko reported these issues in [1] [2] [3].
> >>
> >> [1] https://sashiko.dev/#/message/20260814090305.4C8741F00A3D%40smtp.kernel.org
> >> [2] https://sashiko.dev/#/message/20260814084718.29C341F000E9%40smtp.kernel.org
> >> [3] https://sashiko.dev/#/message/20260806100605.2C2C01F000E9%40smtp.kernel.org
> >>
> >> 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().

> 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.
 
> 
> [1] https://lore.kernel.org/lkml/20260818092420.2859026-1-chenwandun1@gmail.com/
> 
> 
> Best regards,
> Wandun
> 
> >  
> >>    Patches 4 and 5 are reworked to reject regions that overlap or are
> >>    contained by an existing reservation. The code makes a little different
> >>    from what was acked in v2, so the Acked-by tags for these patches are not
> >>    carried over.
> >>
> >> 2. Reorder the patches: the two cleanup fixes in v2 now come first.
> >>    In v3, the first patch now introduces the 'dynamic' distinction in
> >>    fdt_init_reserved_mem_node(), which the following patche 3 build on,
> >>    so the series reads more fluently.
> >>
> >> v1 --> v2:
> >> 1. Rework failed-node tracking in patch 1: do not track zero-sized nodes,
> >>    and keep a reserved_mem slot when tracking overflows.
> >> 2. Reject static reserved regions overlapping existing no-map regions.
> >> 3. Keep MEMBLOCK_NOMAP flag for static no-map regions when init failure.
> >>
> >>
> >> Wandun Chen (5):
> >>   of: reserved_mem: release dynamically allocated no-map region on init
> >>     failure
> >>   of: reserved_mem: retain static no-map memory on init failure
> >>   of: reserved_mem: skip init for regions whose early reservation failed
> >>   of: reserved_mem: reject static regions overlapping no-map memory
> >>   of: reserved_mem: reject static mapped regions overlapping existing
> >>     reservations
> >>
> >>  drivers/of/of_reserved_mem.c | 86 +++++++++++++++++++++++++-------
> >>  include/linux/memblock.h     |  7 +++
> >>  mm/memblock.c                | 95 ++++++++++++++++++++++++++++++++++++
> >>  3 files changed, 170 insertions(+), 18 deletions(-)
> >>
> >> -- 
> >> 2.43.0
> >>
> > 
> 

-- 
Sincerely yours,
Mike.