Re: [PATCH] boot: image-fdt: suppress spurious error on expected reservation failures

Hi,

On 8/11/2026 6:43 PM, Tom Rini wrote:
> On Tue, Aug 11, 2026 at 09:57:52AM +0530, Balaji Selvanathan wrote:
>> Hi Tom,
>>
>> On 8/10/2026 11:46 PM, Tom Rini wrote:
>>> On Mon, Aug 10, 2026 at 03:04:53PM +0530, Balaji Selvanathan wrote:
>>>
>>>> Before commit 623f6c5b6ab7 ("boot: image-fdt: free old dtb
>>>> reservations"), boot_fdt_handle_region() filtered out -EEXIST and
>>>> -EINVAL when reserving an FDT memory region, treating both as benign.
>>>> That commit added the free/reserve toggle but dropped the filter, so
>>>> these now print as errors.
>>>>
>>>> -EINVAL in particular is expected: when a reserved-memory region falls
>>>> outside the available memory LMB knows about, _lmb_alloc_addr() returns
>>>> -EINVAL. This happens when a carve-out described in the kernel DT lies
>>>> in a range the platform does not report as usable RAM, so its
>>>> reservation fails with -EINVAL and prints:
>>>>
>>>>    ERROR: reserving fdt memory region failed (addr=91a80000 size=80000 flags=2): -22
>>>>
>>>> on an otherwise clean boot. The region is still described in the
>>>> kernel DT and Linux reserves it itself, so U-Boot failing to track it
>>>> in LMB is harmless.
>>>>
>>>> Restore the old filter, scoped to the reserve path: print the error
>>>> only when freeing, or when a reservation fails with something other
>>>> than -EEXIST or -EINVAL. The "free ||" guard keeps free-path failures
>>>> always visible.
>>>>
>>>> Fixes: 623f6c5b6ab7 ("boot: image-fdt: free old dtb reservations")
>>>> Signed-off-by: Balaji Selvanathan <[email protected]>
>>> This is the same as:
>>> https://patchwork.ozlabs.org/project/uboot/patch/[email protected]/
>>> So I've added people from that thread to this one.
>>>
>>> It seems to me like the last message there wasn't quite everyone being
>>> in agreement on the next step, so, what is required next at this point?
>>> This is clearly a problem in some cases, in terms of "scary" message to
>>> the user which isn't meaningful.
>>
>> AFAIK, lmb_alloc_mem returns -EINVAL for 2 reasons: a NULL addr pointer, and
>> "the region isn't part of the available LMB map" (_lmb_alloc_addr()).
>>
>> In boot_fdt_handle_region, I think we always pass a valid &addr and bail
>> early on size 0, so the NULL-pointer -EINVAL can't be hit from here. So
>> -EINVAL could only mean "the region isn't part of the available LMB map" in
>> boot_fdt_handle_region.
>>
>> Maybe we can give "region outside the LMB map" its own return value
>> (-EFAULT) as Rudolph suggested and have boot_fdt_handle_region() ignore that
>> specific case (to make it more clean). That seems like a middle ground.
> 
> OK. I think a patch doing that might help move the discussion forward,
> can you please do so? Thanks.

I have a v2 prepared that changes the return value for both alloc and
free in my outgoing queue, used a different return value but will change
it to EFAULT before I send it out.

Regards,
Jonas

>