Re: [v5.4 stable] arm: stm32: Regression observed on "no-map" reserved memory region
On Tue, Apr 20, 2021 at 11:10 AM Ard Biesheuvel wrote:
>
> On Tue, 20 Apr 2021 at 17:54, Rob Herring wrote:
> >
> > On Tue, Apr 20, 2021 at 10:12 AM Alexandre TORGUE
> > wrote:
> > >
> > >
> > >
> > > On 4/20/21 4:45 PM, Rob Herring wrote:
> > > > On Tue, Apr 20, 2021 at 9:03 AM Alexandre TORGUE
> > > > 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.
Probably because of commit 8a5a75e5e9e5 ("of/fdt: Make sure no-map
does not remove already reserved regions") which was in the same
series.
'Properly handle' implies before it was 'improperly handled', so
sounds like a fix.
Rob
Re: [v5.4 stable] arm: stm32: Regression observed on "no-map" reserved memory region
On 4/20/2021 9:10 AM, Ard Biesheuvel wrote:
> On Tue, 20 Apr 2021 at 17:54, Rob Herring wrote:
>>
>> On Tue, Apr 20, 2021 at 10:12 AM Alexandre TORGUE
>> wrote:
>>>
>>>
>>>
>>> On 4/20/21 4:45 PM, Rob Herring wrote:
On Tue, Apr 20, 2021 at 9:03 AM Alexandre TORGUE
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@c000 {
> reg = <0xc000 0x2000>;
> };
>
> reserved-memory {
> #address-cells = <1>;
> #size-cells = <1>;
> ranges;
>
> gpu_reserved: gpu@d400 {
> reg = <0xd400 0x400>;
> 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 0xfe00. Without the offending commit,
/proc/iomem looks like this:
4000-fdffefff : System RAM
40008000-40ff : Kernel code
41e0-41ef1d77 : Kernel data
1-13fff : System RAM
and with the patch applied, we have this:
4000-fdffefff : System RAM
40008000-40ff : Kernel code
41e0-41ef3db7 : Kernel data
fdfff000- : System RAM
1-13fff : System RAM
so we can now see that the region 0xfe00 - 0xfff 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.00] MEMBLOCK configuration:
[0.00] memory size = 0xfdfff000 reserved size = 0x7ce4d20d
[0.00] memory.cnt = 0x2
[0.00] memory[0x0] [0x004000-0x00fdffefff],
0xbdfff000 bytes flags: 0x0
[0.00] memory[0x1] [0x01-0x013fff],
0x4000 bytes flags: 0x0
[0.00] reserved.cnt = 0x6
[0.00] reserved[0x0] [0x0040003000-0x004000e494],
0xb495 bytes flags: 0x0
[0.00] reserved[0x1] [0x004020-0x0041ef1d77],
0x1cf1d78 bytes flags: 0x0
[0.00] reserved[0x2] [0x004500-0x00450f],
0x10 bytes flags: 0x0
[0.00] reserved[0x3] [0x004700-0x004704],
0x5 bytes flags: 0x0
[0.00] reserved[0x4] [0x00c2c0-0x00fdbf],
0x3b00 bytes flags: 0x0
[0.00] reserved[0x5] [0x01-0x013fff],
0x4000 bytes flags: 0x0
[0.00] MEMBLOCK configuration:
[0.00] memory size = 0x1 reserved size = 0x7ca4f24d
[0.00] memory.cnt = 0x3
[0.00] memory[0x0] [0x004000-0x00fdffefff],
0xbdfff000 bytes flags: 0x0
[0.00] memory[0x1] [0x00fdfff000-0x00],
0x2001000 bytes flags: 0x4
[0.00] memory[0x2] [0x01-0x013fff],
0x4000 bytes flags: 0x0
[0.00] reserved.cnt = 0x6
[0.00] reserved[0x0] [0x0040003000-0x004000e494],
0xb495 bytes flags: 0x0
[0.00] reserved[0x1] [0x004020-0x0041ef3db7],
0x1cf3db8 bytes flags: 0x0
[0.00] reserved[0x2] [0x004500-0x00450f],
0x10 bytes flags: 0x0
[0.00]
Re: [v5.4 stable] arm: stm32: Regression observed on "no-map" reserved memory region
On Tue, 20 Apr 2021 at 17:54, Rob Herring wrote:
>
> On Tue, Apr 20, 2021 at 10:12 AM Alexandre TORGUE
> wrote:
> >
> >
> >
> > On 4/20/21 4:45 PM, Rob Herring wrote:
> > > On Tue, Apr 20, 2021 at 9:03 AM Alexandre TORGUE
> > > 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@c000 {
> > >> reg = <0xc000 0x2000>;
> > >> };
> > >>
> > >> reserved-memory {
> > >> #address-cells = <1>;
> > >> #size-cells = <1>;
> > >> ranges;
> > >>
> > >> gpu_reserved: gpu@d400 {
> > >> reg = <0xd400 0x400>;
> > >> 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.
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)
Re: [v5.4 stable] arm: stm32: Regression observed on "no-map" reserved memory region
On Tue, Apr 20, 2021 at 10:12 AM Alexandre TORGUE
wrote:
>
>
>
> On 4/20/21 4:45 PM, Rob Herring wrote:
> > On Tue, Apr 20, 2021 at 9:03 AM Alexandre TORGUE
> > 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.
>
> 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.
> >>
> >> Reverting this patch it's fine.
> >>
> >> I add part of my DT (something is maybe wrong inside):
> >>
> >> memory@c000 {
> >> reg = <0xc000 0x2000>;
> >> };
> >>
> >> reserved-memory {
> >> #address-cells = <1>;
> >> #size-cells = <1>;
> >> ranges;
> >>
> >> gpu_reserved: gpu@d400 {
> >> reg = <0xd400 0x400>;
> >> no-map;
> >> };
> >> };
> >>
> >> Sorry if this issue has already been raised and discussed.
> >>
> >> Thanks
> >> alex
