diff mbox series

ARM: boot: Fix ATAGs with appended DTB

Message ID 20200225144749.19815-1-geert+renesas@glider.be (mailing list archive)
State Accepted
Delegated to: Geert Uytterhoeven
Headers show
Series ARM: boot: Fix ATAGs with appended DTB | expand

Commit Message

Geert Uytterhoeven Feb. 25, 2020, 2:47 p.m. UTC
At early boot, register r8 may contain an ATAGs or DTB pointer.
When an appended DTB is found, its address is stored in r8, for
extraction of the RAM base address later.

However, if r8 contained an ATAGs pointer before, that pointer will be
lost, and the provided ATAGs is no longer folded into the provided DTB.

Fix this by leaving r8 untouched.

Fixes: 137e522593918be2 ("ARM: 8960/1: boot: Obtain start of physical memory from DTB")
Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Not tested with ATAGs, only with [uz]Image + DTB, and zImage with
appended DTB.
---
 arch/arm/boot/compressed/head.S | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Marek Szyprowski Feb. 26, 2020, 6:35 a.m. UTC | #1
Hi Geert,

On 25.02.2020 15:47, Geert Uytterhoeven wrote:
> At early boot, register r8 may contain an ATAGs or DTB pointer.
> When an appended DTB is found, its address is stored in r8, for
> extraction of the RAM base address later.
>
> However, if r8 contained an ATAGs pointer before, that pointer will be
> lost, and the provided ATAGs is no longer folded into the provided DTB.
>
> Fix this by leaving r8 untouched.
>
> Fixes: 137e522593918be2 ("ARM: 8960/1: boot: Obtain start of physical memory from DTB")
> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Not tested with ATAGs, only with [uz]Image + DTB, and zImage with
> appended DTB.

Works fine with zImage + appended DTB + cmdline/memory info passed via ATAGs

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

> ---
>   arch/arm/boot/compressed/head.S | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index 339d4b4cfbbeed15..a351ed2bc195ed8d 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -267,16 +267,18 @@ not_angel:
>   		cmp	r0, r1		@ do we have a DTB there?
>   		bne	1f
>   
> -		mov	r8, r6		@ use it if so
>   		/* preserve 64-bit alignment */
>   		add	r5, r5, #7
>   		bic	r5, r5, #7
> -		add	sp, sp, r5	@ and move stack above it
> +		add	sp, sp, r5	@ if so, move stack above DTB
> +		mov	r0, r6		@ and extract memory start from DTB
> +		b	2f
>   
>   1:
>   #endif /* CONFIG_ARM_APPENDED_DTB */
>   
>   		mov	r0, r8
> +2:
>   		bl	fdt_get_mem_start
>   		mov	r4, r0
>   		cmp	r0, #-1

Best regards
Russell King (Oracle) Feb. 26, 2020, 5:49 p.m. UTC | #2
On Wed, Feb 26, 2020 at 07:35:14AM +0100, Marek Szyprowski wrote:
> Hi Geert,
> 
> On 25.02.2020 15:47, Geert Uytterhoeven wrote:
> > At early boot, register r8 may contain an ATAGs or DTB pointer.
> > When an appended DTB is found, its address is stored in r8, for
> > extraction of the RAM base address later.
> >
> > However, if r8 contained an ATAGs pointer before, that pointer will be
> > lost, and the provided ATAGs is no longer folded into the provided DTB.
> >
> > Fix this by leaving r8 untouched.
> >
> > Fixes: 137e522593918be2 ("ARM: 8960/1: boot: Obtain start of physical memory from DTB")
> > Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

The original commit hasn't been submitted, so it can be fixed before it
hits mainline if you want.  Let me know what you want to do.  Thanks.
Geert Uytterhoeven Feb. 26, 2020, 5:56 p.m. UTC | #3
Hi Russell,

On Wed, Feb 26, 2020 at 6:49 PM Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
> On Wed, Feb 26, 2020 at 07:35:14AM +0100, Marek Szyprowski wrote:
> > On 25.02.2020 15:47, Geert Uytterhoeven wrote:
> > > At early boot, register r8 may contain an ATAGs or DTB pointer.
> > > When an appended DTB is found, its address is stored in r8, for
> > > extraction of the RAM base address later.
> > >
> > > However, if r8 contained an ATAGs pointer before, that pointer will be
> > > lost, and the provided ATAGs is no longer folded into the provided DTB.
> > >
> > > Fix this by leaving r8 untouched.
> > >
> > > Fixes: 137e522593918be2 ("ARM: 8960/1: boot: Obtain start of physical memory from DTB")
> > > Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> The original commit hasn't been submitted, so it can be fixed before it
> hits mainline if you want.  Let me know what you want to do.  Thanks.

Fixing the original is fine for me, of course.
Thanks!

Gr{oetje,eeting}s,

                        Geert
Russell King (Oracle) Feb. 26, 2020, 5:57 p.m. UTC | #4
On Wed, Feb 26, 2020 at 06:56:06PM +0100, Geert Uytterhoeven wrote:
> Hi Russell,
> 
> On Wed, Feb 26, 2020 at 6:49 PM Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
> > On Wed, Feb 26, 2020 at 07:35:14AM +0100, Marek Szyprowski wrote:
> > > On 25.02.2020 15:47, Geert Uytterhoeven wrote:
> > > > At early boot, register r8 may contain an ATAGs or DTB pointer.
> > > > When an appended DTB is found, its address is stored in r8, for
> > > > extraction of the RAM base address later.
> > > >
> > > > However, if r8 contained an ATAGs pointer before, that pointer will be
> > > > lost, and the provided ATAGs is no longer folded into the provided DTB.
> > > >
> > > > Fix this by leaving r8 untouched.
> > > >
> > > > Fixes: 137e522593918be2 ("ARM: 8960/1: boot: Obtain start of physical memory from DTB")
> > > > Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >
> > The original commit hasn't been submitted, so it can be fixed before it
> > hits mainline if you want.  Let me know what you want to do.  Thanks.
> 
> Fixing the original is fine for me, of course.
> Thanks!

Please submit a replacement for 8960/1, thanks.
Geert Uytterhoeven Feb. 26, 2020, 8:48 p.m. UTC | #5
Hi Russell,

On Wed, Feb 26, 2020 at 6:57 PM Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
> On Wed, Feb 26, 2020 at 06:56:06PM +0100, Geert Uytterhoeven wrote:
> > On Wed, Feb 26, 2020 at 6:49 PM Russell King - ARM Linux admin
> > <linux@armlinux.org.uk> wrote:
> > > On Wed, Feb 26, 2020 at 07:35:14AM +0100, Marek Szyprowski wrote:
> > > > On 25.02.2020 15:47, Geert Uytterhoeven wrote:
> > > > > At early boot, register r8 may contain an ATAGs or DTB pointer.
> > > > > When an appended DTB is found, its address is stored in r8, for
> > > > > extraction of the RAM base address later.
> > > > >
> > > > > However, if r8 contained an ATAGs pointer before, that pointer will be
> > > > > lost, and the provided ATAGs is no longer folded into the provided DTB.
> > > > >
> > > > > Fix this by leaving r8 untouched.
> > > > >
> > > > > Fixes: 137e522593918be2 ("ARM: 8960/1: boot: Obtain start of physical memory from DTB")
> > > > > Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > >
> > > The original commit hasn't been submitted, so it can be fixed before it
> > > hits mainline if you want.  Let me know what you want to do.  Thanks.
> >
> > Fixing the original is fine for me, of course.
> > Thanks!
>
> Please submit a replacement for 8960/1, thanks.

Done.

Gr{oetje,eeting}s,

                        Geert
Marek Szyprowski March 12, 2020, 12:23 p.m. UTC | #6
Hi,

On 26.02.2020 21:48, Geert Uytterhoeven wrote:
> Hi Russell,
>
> On Wed, Feb 26, 2020 at 6:57 PM Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
>> On Wed, Feb 26, 2020 at 06:56:06PM +0100, Geert Uytterhoeven wrote:
>>> On Wed, Feb 26, 2020 at 6:49 PM Russell King - ARM Linux admin
>>> <linux@armlinux.org.uk> wrote:
>>>> On Wed, Feb 26, 2020 at 07:35:14AM +0100, Marek Szyprowski wrote:
>>>>> On 25.02.2020 15:47, Geert Uytterhoeven wrote:
>>>>>> At early boot, register r8 may contain an ATAGs or DTB pointer.
>>>>>> When an appended DTB is found, its address is stored in r8, for
>>>>>> extraction of the RAM base address later.
>>>>>>
>>>>>> However, if r8 contained an ATAGs pointer before, that pointer will be
>>>>>> lost, and the provided ATAGs is no longer folded into the provided DTB.
>>>>>>
>>>>>> Fix this by leaving r8 untouched.
>>>>>>
>>>>>> Fixes: 137e522593918be2 ("ARM: 8960/1: boot: Obtain start of physical memory from DTB")
>>>>>> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>>>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>>> The original commit hasn't been submitted, so it can be fixed before it
>>>> hits mainline if you want.  Let me know what you want to do.  Thanks.
>>> Fixing the original is fine for me, of course.
>>> Thanks!
>> Please submit a replacement for 8960/1, thanks.
> Done.

Gentle ping. This fix is still not present in linux-next for over 2 weeks...

Best regards
Russell King (Oracle) March 12, 2020, 12:28 p.m. UTC | #7
On Thu, Mar 12, 2020 at 01:23:09PM +0100, Marek Szyprowski wrote:
> Hi,
> 
> On 26.02.2020 21:48, Geert Uytterhoeven wrote:
> > Hi Russell,
> >
> > On Wed, Feb 26, 2020 at 6:57 PM Russell King - ARM Linux admin
> > <linux@armlinux.org.uk> wrote:
> >> On Wed, Feb 26, 2020 at 06:56:06PM +0100, Geert Uytterhoeven wrote:
> >>> On Wed, Feb 26, 2020 at 6:49 PM Russell King - ARM Linux admin
> >>> <linux@armlinux.org.uk> wrote:
> >>>> On Wed, Feb 26, 2020 at 07:35:14AM +0100, Marek Szyprowski wrote:
> >>>>> On 25.02.2020 15:47, Geert Uytterhoeven wrote:
> >>>>>> At early boot, register r8 may contain an ATAGs or DTB pointer.
> >>>>>> When an appended DTB is found, its address is stored in r8, for
> >>>>>> extraction of the RAM base address later.
> >>>>>>
> >>>>>> However, if r8 contained an ATAGs pointer before, that pointer will be
> >>>>>> lost, and the provided ATAGs is no longer folded into the provided DTB.
> >>>>>>
> >>>>>> Fix this by leaving r8 untouched.
> >>>>>>
> >>>>>> Fixes: 137e522593918be2 ("ARM: 8960/1: boot: Obtain start of physical memory from DTB")
> >>>>>> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> >>>>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >>>> The original commit hasn't been submitted, so it can be fixed before it
> >>>> hits mainline if you want.  Let me know what you want to do.  Thanks.
> >>> Fixing the original is fine for me, of course.
> >>> Thanks!
> >> Please submit a replacement for 8960/1, thanks.
> > Done.
> 
> Gentle ping. This fix is still not present in linux-next for over 2 weeks...

The 32-bit ARM tree is now a low priority ad-hoc effort; as a result,
I now only merge patches around once or twice each cycle. I've merged
the replacement earlier this morning.

So, it will take longer than two weeks for patches to make it into my
tree, as has been the case ever since I lost funding for maintaining
32-bit ARM support, and, therefore, I now have other priorities.
Geert Uytterhoeven March 12, 2020, 12:29 p.m. UTC | #8
Hi Marek,

On Thu, Mar 12, 2020 at 1:23 PM Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
> On 26.02.2020 21:48, Geert Uytterhoeven wrote:
> > On Wed, Feb 26, 2020 at 6:57 PM Russell King - ARM Linux admin
> > <linux@armlinux.org.uk> wrote:
> >> On Wed, Feb 26, 2020 at 06:56:06PM +0100, Geert Uytterhoeven wrote:
> >>> On Wed, Feb 26, 2020 at 6:49 PM Russell King - ARM Linux admin
> >>> <linux@armlinux.org.uk> wrote:
> >>>> On Wed, Feb 26, 2020 at 07:35:14AM +0100, Marek Szyprowski wrote:
> >>>>> On 25.02.2020 15:47, Geert Uytterhoeven wrote:
> >>>>>> At early boot, register r8 may contain an ATAGs or DTB pointer.
> >>>>>> When an appended DTB is found, its address is stored in r8, for
> >>>>>> extraction of the RAM base address later.
> >>>>>>
> >>>>>> However, if r8 contained an ATAGs pointer before, that pointer will be
> >>>>>> lost, and the provided ATAGs is no longer folded into the provided DTB.
> >>>>>>
> >>>>>> Fix this by leaving r8 untouched.
> >>>>>>
> >>>>>> Fixes: 137e522593918be2 ("ARM: 8960/1: boot: Obtain start of physical memory from DTB")
> >>>>>> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> >>>>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >>>> The original commit hasn't been submitted, so it can be fixed before it
> >>>> hits mainline if you want.  Let me know what you want to do.  Thanks.
> >>> Fixing the original is fine for me, of course.
> >>> Thanks!
> >> Please submit a replacement for 8960/1, thanks.
> > Done.
>
> Gentle ping. This fix is still not present in linux-next for over 2 weeks...

According to
https://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8963
the fixed version was applied less than one hour ago.

It's now part of arm/for-next.

Gr{oetje,eeting}s,

                        Geert
diff mbox series

Patch

diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index 339d4b4cfbbeed15..a351ed2bc195ed8d 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -267,16 +267,18 @@  not_angel:
 		cmp	r0, r1		@ do we have a DTB there?
 		bne	1f
 
-		mov	r8, r6		@ use it if so
 		/* preserve 64-bit alignment */
 		add	r5, r5, #7
 		bic	r5, r5, #7
-		add	sp, sp, r5	@ and move stack above it
+		add	sp, sp, r5	@ if so, move stack above DTB
+		mov	r0, r6		@ and extract memory start from DTB
+		b	2f
 
 1:
 #endif /* CONFIG_ARM_APPENDED_DTB */
 
 		mov	r0, r8
+2:
 		bl	fdt_get_mem_start
 		mov	r4, r0
 		cmp	r0, #-1