On 4/20/2021 9:10 AM, Ard Biesheuvel wrote:
> On Tue, 20 Apr 2021 at 17:54, Rob Herring <robh+dt@xxxxxxxxxx> wrote:
>>
>> On Tue, Apr 20, 2021 at 10:12 AM Alexandre TORGUE
>> <alexandre.torgue@xxxxxxxxxxx> wrote:
>>>
>>>
>>>
>>> On 4/20/21 4:45 PM, Rob Herring wrote:
>>>> On Tue, Apr 20, 2021 at 9:03 AM Alexandre TORGUE
>>>> <alexandre.torgue@xxxxxxxxxxx> wrote:
>>>>>
>>>>> Hi,
>>>>
>>>> Greg or Sasha won't know what to do with this. Not sure who follows
>>>> the stable list either. Quentin sent the patch, but is not the author.
>>>> Given the patch in question is about consistency between EFI memory
>>>> map boot and DT memory map boot, copying EFI knowledgeable folks would
>>>> help (Ard B for starters).
>>>
>>> Ok thanks for the tips. I add Ard in the loop.
>>
>> Sigh. If it was only Ard I was suggesting I would have done that
>> myself. Now everyone on the patch in question and relevant lists are
>> Cc'ed.
>>
>
> Thanks for the cc.
>
>>>
>>> Ard, let me know if other people have to be directly added or if I have
>>> to resend to another mailing list.
>>>
>>> thanks
>>> alex
>>>
>>>>
>>>>>
>>>>> Since v5.4.102 I observe a regression on stm32mp1 platform: "no-map"
>>>>> reserved-memory regions are no more "reserved" and make part of the
>>>>> kernel System RAM. This causes allocation failure for devices which try
>>>>> to take a reserved-memory region.
>>>>>
>>>>> It has been introduced by the following path:
>>>>>
>>>>> "fdt: Properly handle "no-map" field in the memory region
>>>>> [ Upstream commit 86588296acbfb1591e92ba60221e95677ecadb43 ]"
>>>>> which replace memblock_remove by memblock_mark_nomap in no-map case.
>>>>>
>
> Why was this backported? It doesn't look like a bugfix to me.
>
>>>>> Reverting this patch it's fine.
>>>>>
>>>>> I add part of my DT (something is maybe wrong inside):
>>>>>
>>>>> memory@c0000000 {
>>>>> reg = <0xc0000000 0x20000000>;
>>>>> };
>>>>>
>>>>> reserved-memory {
>>>>> #address-cells = <1>;
>>>>> #size-cells = <1>;
>>>>> ranges;
>>>>>
>>>>> gpu_reserved: gpu@d4000000 {
>>>>> reg = <0xd4000000 0x4000000>;
>>>>> no-map;
>>>>> };
>>>>> };
>>>>>
>>>>> Sorry if this issue has already been raised and discussed.
>>>>>
>
> Could you explain why it fails? The region is clearly part of system
> memory, and tagged as no-map, so the patch in itself is not
> unreasonable. However, we obviously have code that relies on how the
> region is represented in /proc/iomem, so it would be helpful to get
> some insight into why this is the case.
I do wonder as well, we have a 32MB "no-map" reserved memory region on
our platforms located at 0xfe000000. Without the offending commit,
/proc/iomem looks like this:
40000000-fdffefff : System RAM
40008000-40ffffff : Kernel code
41e00000-41ef1d77 : Kernel data
100000000-13fffffff : System RAM
and with the patch applied, we have this:
40000000-fdffefff : System RAM
40008000-40ffffff : Kernel code
41e00000-41ef3db7 : Kernel data
fdfff000-ffffffff : System RAM
100000000-13fffffff : System RAM
so we can now see that the region 0xfe000000 - 0xfffffff is also cobbled
up with the preceding region which is a mailbox between Linux and the
secure monitor at 0xfdfff000 and of size 4KB. It seems like there is
The memblock=debug outputs is also different:
[ 0.000000] MEMBLOCK configuration:
[ 0.000000] memory size = 0xfdfff000 reserved size = 0x7ce4d20d
[ 0.000000] memory.cnt = 0x2
[ 0.000000] memory[0x0] [0x00000040000000-0x000000fdffefff],
0xbdfff000 bytes flags: 0x0
[ 0.000000] memory[0x1] [0x00000100000000-0x0000013fffffff],
0x40000000 bytes flags: 0x0
[ 0.000000] reserved.cnt = 0x6
[ 0.000000] reserved[0x0] [0x00000040003000-0x0000004000e494],
0xb495 bytes flags: 0x0
[ 0.000000] reserved[0x1] [0x00000040200000-0x00000041ef1d77],
0x1cf1d78 bytes flags: 0x0
[ 0.000000] reserved[0x2] [0x00000045000000-0x000000450fffff],
0x100000 bytes flags: 0x0
[ 0.000000] reserved[0x3] [0x00000047000000-0x0000004704ffff],
0x50000 bytes flags: 0x0
[ 0.000000] reserved[0x4] [0x000000c2c00000-0x000000fdbfffff],
0x3b000000 bytes flags: 0x0
[ 0.000000] reserved[0x5] [0x00000100000000-0x0000013fffffff],
0x40000000 bytes flags: 0x0
[ 0.000000] MEMBLOCK configuration:
[ 0.000000] memory size = 0x100000000 reserved size = 0x7ca4f24d
[ 0.000000] memory.cnt = 0x3
[ 0.000000] memory[0x0] [0x00000040000000-0x000000fdffefff],
0xbdfff000 bytes flags: 0x0
[ 0.000000] memory[0x1] [0x000000fdfff000-0x000000ffffffff],
0x2001000 bytes flags: 0x4
[ 0.000000] memory[0x2] [0x00000100000000-0x0000013fffffff],
0x40000000 bytes flags: 0x0
[ 0.000000] reserved.cnt = 0x6
[ 0.000000] reserved[0x0] [0x00000040003000-0x0000004000e494],
0xb495 bytes flags: 0x0
[ 0.000000] reserved[0x1] [0x00000040200000-0x00000041ef3db7],
0x1cf3db8 bytes flags: 0x0
[ 0.000000] reserved[0x2] [0x00000045000000-0x000000450fffff],
0x100000 bytes flags: 0x0
[ 0.000000] reserved[0x3] [0x00000047000000-0x0000004704ffff],
0x50000 bytes flags: 0x0
[ 0.000000] reserved[0x4] [0x000000c3000000-0x000000fdbfffff],
0x3ac00000 bytes flags: 0x0
[ 0.000000] reserved[0x5] [0x00000100000000-0x0000013fffffff],
0x40000000 bytes flags: 0x0
in the second case we can clearly see that the 32MB no-map region is now
considered as usable RAM.
Hope this helps.
>
> In any case, the mere fact that this causes a regression should be
> sufficient justification to revert/withdraw it from v5.4, as I don't
> see a reason why it was merged there in the first place. (It has no
> fixes tag or cc:stable)
Agreed, however that means we still need to find out whether a more
recent kernel is also broken, I should be able to tell you that a little
later.
--
Florian
