mbox series

[v2,0/2] ARM: decompressor: support AUTO_ZRELADDR and appended DTB

Message ID 20240121203009.9257-1-ansuelsmth@gmail.com (mailing list archive)
Headers show
Series ARM: decompressor: support AUTO_ZRELADDR and appended DTB | expand

Message

Christian Marangi Jan. 21, 2024, 8:29 p.m. UTC
This series try to address a long lasting problem with legacy device
that require an appended DTB and the use of AUTO_ZRELADDR.

With these device AUTO_ZRELADDR is not possible if for some reason at
the start of the RAM it's needed to reserve some space. (example qcom SoC
that allocate reserved space for SMEM)

In the current implementation with appended DTB and AUTO_ZRELADDR,
the memory start is only derived from the PC register and it can't be
changed by declaring additional info in the DTS.

In a normal setup, we have an intentional undocumented chosen property
to handle this and the memory node to declare the start of the memory.

With this applied and ARM_ATAG_DTB_COMPAT_IGNORE_MEM enabled (more 
info in the related patch) ipq806x can boot right away with AUTO_ZRELADDR
enabled and a correct memory node defined in DTS.

It's needed to ignore MEM ATAGs as most of the time the values from the
bootloader are hardcoded and OEM didn't care to actually provide them
resulting in funny situation where a Netgear R7800 with 512Mb of RAM
have Uboot passing 1.7GB of RAM with ATAGS.

While MEM ATAG may be broken, other ATAG like serial number or bootargs
might still be useful for partition declaration (cmdlinepart) or other
info hence DTB_COMPAT is still needed in these case and can't be
disabled.

I'm open to any suggestion on how this can be improved and I would love
some additional testing on other legacy platform but I assume this will
permit many legacy device to be correctly supported without having to
hardcode address.

Changes v2:
- Add Review and Ack Tags
- Use IS_ENABLED instead of global variable

Christian Marangi (2):
  ARM: decompressor: support memory start validation for appended DTB
  ARM: decompressor: add option to ignore MEM ATAGs

 arch/arm/Kconfig                        | 12 ++++++++++++
 arch/arm/boot/compressed/atags_to_fdt.c |  4 ++++
 arch/arm/boot/compressed/head.S         | 22 ++++++++++++++++++++++
 3 files changed, 38 insertions(+)

Comments

Christian Marangi Feb. 21, 2024, 4:57 p.m. UTC | #1
On Sun, Jan 21, 2024 at 09:29:32PM +0100, Christian Marangi wrote:
> This series try to address a long lasting problem with legacy device
> that require an appended DTB and the use of AUTO_ZRELADDR.
> 
> With these device AUTO_ZRELADDR is not possible if for some reason at
> the start of the RAM it's needed to reserve some space. (example qcom SoC
> that allocate reserved space for SMEM)
> 
> In the current implementation with appended DTB and AUTO_ZRELADDR,
> the memory start is only derived from the PC register and it can't be
> changed by declaring additional info in the DTS.
> 
> In a normal setup, we have an intentional undocumented chosen property
> to handle this and the memory node to declare the start of the memory.
> 
> With this applied and ARM_ATAG_DTB_COMPAT_IGNORE_MEM enabled (more 
> info in the related patch) ipq806x can boot right away with AUTO_ZRELADDR
> enabled and a correct memory node defined in DTS.
> 
> It's needed to ignore MEM ATAGs as most of the time the values from the
> bootloader are hardcoded and OEM didn't care to actually provide them
> resulting in funny situation where a Netgear R7800 with 512Mb of RAM
> have Uboot passing 1.7GB of RAM with ATAGS.
> 
> While MEM ATAG may be broken, other ATAG like serial number or bootargs
> might still be useful for partition declaration (cmdlinepart) or other
> info hence DTB_COMPAT is still needed in these case and can't be
> disabled.
> 
> I'm open to any suggestion on how this can be improved and I would love
> some additional testing on other legacy platform but I assume this will
> permit many legacy device to be correctly supported without having to
> hardcode address.
> 
> Changes v2:
> - Add Review and Ack Tags
> - Use IS_ENABLED instead of global variable
> 
> Christian Marangi (2):
>   ARM: decompressor: support memory start validation for appended DTB
>   ARM: decompressor: add option to ignore MEM ATAGs
> 
>  arch/arm/Kconfig                        | 12 ++++++++++++
>  arch/arm/boot/compressed/atags_to_fdt.c |  4 ++++
>  arch/arm/boot/compressed/head.S         | 22 ++++++++++++++++++++++
>  3 files changed, 38 insertions(+)
> 
>

Any news for this?
Linus Walleij Feb. 22, 2024, 12:07 p.m. UTC | #2
On Wed, Feb 21, 2024 at 5:57 PM Christian Marangi <ansuelsmth@gmail.com> wrote:

> Any news for this?

Can you please put the patches into Russell's patch tracker so he can
apply them?
https://www.arm.linux.org.uk/developer/patches/

If you run into any problems, ping me on IRC and I'll help out.

Yours,
Linus Walleij
Christian Marangi May 5, 2024, 4:22 p.m. UTC | #3
On Thu, Feb 22, 2024 at 01:07:56PM +0100, Linus Walleij wrote:
> On Wed, Feb 21, 2024 at 5:57 PM Christian Marangi <ansuelsmth@gmail.com> wrote:
> 
> > Any news for this?
> 
> Can you please put the patches into Russell's patch tracker so he can
> apply them?
> https://www.arm.linux.org.uk/developer/patches/
> 
> If you run into any problems, ping me on IRC and I'll help out.
>

God I totally missed this email where I had to put the 2 patch in the
tracking system. Just did now... my bad!
Christian Marangi June 13, 2024, 11:24 a.m. UTC | #4
On Wed, Feb 21, 2024 at 05:57:00PM +0100, Christian Marangi wrote:
> On Sun, Jan 21, 2024 at 09:29:32PM +0100, Christian Marangi wrote:
> > This series try to address a long lasting problem with legacy device
> > that require an appended DTB and the use of AUTO_ZRELADDR.
> > 
> > With these device AUTO_ZRELADDR is not possible if for some reason at
> > the start of the RAM it's needed to reserve some space. (example qcom SoC
> > that allocate reserved space for SMEM)
> > 
> > In the current implementation with appended DTB and AUTO_ZRELADDR,
> > the memory start is only derived from the PC register and it can't be
> > changed by declaring additional info in the DTS.
> > 
> > In a normal setup, we have an intentional undocumented chosen property
> > to handle this and the memory node to declare the start of the memory.
> > 
> > With this applied and ARM_ATAG_DTB_COMPAT_IGNORE_MEM enabled (more 
> > info in the related patch) ipq806x can boot right away with AUTO_ZRELADDR
> > enabled and a correct memory node defined in DTS.
> > 
> > It's needed to ignore MEM ATAGs as most of the time the values from the
> > bootloader are hardcoded and OEM didn't care to actually provide them
> > resulting in funny situation where a Netgear R7800 with 512Mb of RAM
> > have Uboot passing 1.7GB of RAM with ATAGS.
> > 
> > While MEM ATAG may be broken, other ATAG like serial number or bootargs
> > might still be useful for partition declaration (cmdlinepart) or other
> > info hence DTB_COMPAT is still needed in these case and can't be
> > disabled.
> > 
> > I'm open to any suggestion on how this can be improved and I would love
> > some additional testing on other legacy platform but I assume this will
> > permit many legacy device to be correctly supported without having to
> > hardcode address.
> > 
> > Changes v2:
> > - Add Review and Ack Tags
> > - Use IS_ENABLED instead of global variable
> > 
> > Christian Marangi (2):
> >   ARM: decompressor: support memory start validation for appended DTB
> >   ARM: decompressor: add option to ignore MEM ATAGs
> > 
> >  arch/arm/Kconfig                        | 12 ++++++++++++
> >  arch/arm/boot/compressed/atags_to_fdt.c |  4 ++++
> >  arch/arm/boot/compressed/head.S         | 22 ++++++++++++++++++++++
> >  3 files changed, 38 insertions(+)
> > 
> >
> 
> Any news for this?

Sorry for asking again... but any news for this?

I have also added the 2 patch here [1] [2].

Been in incoming from a long time and I have seen other patch getting
accepted. Did I do something wrong in submitting the 2 patch?

[1] https://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=9394/1
[2] https://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=9395/1
Christian Marangi June 13, 2024, 1:42 p.m. UTC | #5
On Thu, Jun 13, 2024 at 04:59:49PM +0100, Russell King (Oracle) wrote:
> On Thu, Jun 13, 2024 at 03:50:58PM +0200, Linus Walleij wrote:
> > On Thu, Jun 13, 2024 at 1:24 PM Christian Marangi <ansuelsmth@gmail.com> wrote:
> > 
> > > Sorry for asking again... but any news for this?
> > >
> > > I have also added the 2 patch here [1] [2].
> > >
> > > Been in incoming from a long time and I have seen other patch getting
> > > accepted. Did I do something wrong in submitting the 2 patch?
> > 
> > Hm Russell must have had some concerns, Russell?
> 
> I've been snowed under for about the last six weeks - with only the
> occasional day that isn't silly. It's that kind of frustrating snowed
> under where each problem is a bit like a brick wall placed every 1m
> and you're supposed to be doing a 100m sprint race - you can't see
> the next brick wall until you've climbed over the first.
> 
> Whether I have time to read the mailing lists or not depends entirely
> on what is happening on any particular day.
> 
> > If for nothing else I think some Tested-by:s would be appreciated,
> > do we have some people who use this that can provide Tested-by
> > tags?
> 
> Yes, tested-by's would be a really good idea, because my gut feeling
> is that this change has moderate risk of causing regressions. I'm
> not talking about "it works for me on the setup it's intended for"
> I'm talking about other platforms.
> 
> I'm also wondering about distros, and what they're supposed to do
> with the config option with their "universal" kernel that's
> supposed to boot across as many platforms as possible, what they
> should set the config option to, and what impact it has when enabled
> on platforms that it isn't originally intended for.
> 
> I haven't really read much of the patch because I've been so busy,
> so I may be being overly cautious. Given that I am quite busy, I
> would appreciate a summary of the situation rather than being fed
> with lots of results! In other words, the tested-bys, and "it works
> on all the xyz platforms that we've testsed, nothing appears to have
> regressed" would be ideal.
>

The current patch are used downstream on the OpenWrt ipq806x target that
is a mix of legacy (what this affects) and non legacy targets. (old
bootloader support loading DTB from the image and older ones require it
to be appended)

I think I need some help from the community to test this.

I can also move these patches to our "generic" target on OpenWrt so that
they will be enabled by every arm target we support.

Anyway thanks for the feedback, my only concern was that I messed
submitting the patch on the tracking system. Hope community can help
with this since it's a big feature for legacy devices that was broken
from a looong time (and only solution currently is to hardcode the PHY
offset values)
Linus Walleij June 13, 2024, 1:50 p.m. UTC | #6
On Thu, Jun 13, 2024 at 1:24 PM Christian Marangi <ansuelsmth@gmail.com> wrote:

> Sorry for asking again... but any news for this?
>
> I have also added the 2 patch here [1] [2].
>
> Been in incoming from a long time and I have seen other patch getting
> accepted. Did I do something wrong in submitting the 2 patch?

Hm Russell must have had some concerns, Russell?

If for nothing else I think some Tested-by:s would be appreciated,
do we have some people who use this that can provide Tested-by
tags?

I would rebase on v6.10-rc1 and mark the old versions as superseded
in any case so it is clear it will merge fine.

Yours,
Linus Walleij
Russell King (Oracle) June 13, 2024, 3:59 p.m. UTC | #7
On Thu, Jun 13, 2024 at 03:50:58PM +0200, Linus Walleij wrote:
> On Thu, Jun 13, 2024 at 1:24 PM Christian Marangi <ansuelsmth@gmail.com> wrote:
> 
> > Sorry for asking again... but any news for this?
> >
> > I have also added the 2 patch here [1] [2].
> >
> > Been in incoming from a long time and I have seen other patch getting
> > accepted. Did I do something wrong in submitting the 2 patch?
> 
> Hm Russell must have had some concerns, Russell?

I've been snowed under for about the last six weeks - with only the
occasional day that isn't silly. It's that kind of frustrating snowed
under where each problem is a bit like a brick wall placed every 1m
and you're supposed to be doing a 100m sprint race - you can't see
the next brick wall until you've climbed over the first.

Whether I have time to read the mailing lists or not depends entirely
on what is happening on any particular day.

> If for nothing else I think some Tested-by:s would be appreciated,
> do we have some people who use this that can provide Tested-by
> tags?

Yes, tested-by's would be a really good idea, because my gut feeling
is that this change has moderate risk of causing regressions. I'm
not talking about "it works for me on the setup it's intended for"
I'm talking about other platforms.

I'm also wondering about distros, and what they're supposed to do
with the config option with their "universal" kernel that's
supposed to boot across as many platforms as possible, what they
should set the config option to, and what impact it has when enabled
on platforms that it isn't originally intended for.

I haven't really read much of the patch because I've been so busy,
so I may be being overly cautious. Given that I am quite busy, I
would appreciate a summary of the situation rather than being fed
with lots of results! In other words, the tested-bys, and "it works
on all the xyz platforms that we've testsed, nothing appears to have
regressed" would be ideal.

Thanks.