[PATCH 1/2] ARM: kexec: Add missing memory clobber to inline asm in crash_setup_regs()

[PATCH 1/2] ARM: kexec: Add missing memory clobber to inline asm in crash_setup_regs()

[Dave Martin]

Currently, the inline asm is passed &newregs->ARM_r0 as in input,
when modifying multiple fields of newregs.

It's plausible to assume that GCC will assume newregs->ARM_r0 is modified
when passed the address, but unfortunately this assumption is incorrect.

Also, GCC has no way to guess that the other ARM_r* fields are modified
without the addition of a "memory" clobber.

Applies cleanly on v2.6.37-rc1.

Signed-off-by: Dave Martin <dave.martin@linaro.org>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/include/asm/kexec.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/include/asm/kexec.h b/arch/arm/include/asm/kexec.h
index 8ec9ef5..b37e02c 100644
--- a/arch/arm/include/asm/kexec.h
+++ b/arch/arm/include/asm/kexec.h
@@ -34,7 +34,7 @@ static inline void crash_setup_regs(struct pt_regs *newregs,
 		memcpy(newregs, oldregs, sizeof(*newregs));
 	} else {
 		__asm__ __volatile__ ("stmia %0, {r0 - r15}"
-				      : : "r" (&newregs->ARM_r0));
+				      : : "r" (&newregs->ARM_r0) : "memory");
 		__asm__ __volatile__ ("mrs %0, cpsr"
 				      : "=r" (newregs->ARM_cpsr));
 	}
-- 
1.7.1

[PATCH 2/2] ARM: kexec: Fix crash_setup_regs() for ARMv7 and CONFIG_THUMB2_KERNEL

[Dave Martin]

 * Fix kexec build failure with CONFIG_THUMB2_KERNEL.

 * Avoids deprecated/forbidden sp and pc usage in for ARMv7 onwards,
retaining compatibility with older architecture versions.

 * The pc value saved to newregs is now aligned on a predictable
instruction boundary.

(stmia { ... pc } or str pc has implementation-defined results
in most versions of the ARM architecutre, and is prohibited
(unpredictable) in Thumb-2.)

 * Switch to named inline asm arguments (else I get readily confused ...)

The resulting code should be compatible with all architecture versions >= v3,
with or without CONFIG_THUMB2_KERNEL.

Applies cleanly on v2.6.37-rc1.

Signed-off-by: Dave Martin <dave.martin@linaro.org>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/include/asm/kexec.h |   18 ++++++++++++++----
 1 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/arch/arm/include/asm/kexec.h b/arch/arm/include/asm/kexec.h
index b37e02c..c0094d8 100644
--- a/arch/arm/include/asm/kexec.h
+++ b/arch/arm/include/asm/kexec.h
@@ -33,10 +33,20 @@ static inline void crash_setup_regs(struct pt_regs *newregs,
 	if (oldregs) {
 		memcpy(newregs, oldregs, sizeof(*newregs));
 	} else {
-		__asm__ __volatile__ ("stmia %0, {r0 - r15}"
-				      : : "r" (&newregs->ARM_r0) : "memory");
-		__asm__ __volatile__ ("mrs %0, cpsr"
-				      : "=r" (newregs->ARM_cpsr));
+		__asm__ __volatile__ (
+			"stmia	%[regs_base], {r0-r12}\n\t"
+			"mov	%[_ARM_sp], sp\n\t"
+			"str	lr, %[_ARM_lr]\n\t"
+			"adr	%[_ARM_pc], 1f\n\t"
+			"mrs	%[_ARM_cpsr], cpsr\n\t"
+		"1:"
+			: [_ARM_pc] "=r" (newregs->ARM_pc),
+			  [_ARM_cpsr] "=r" (newregs->ARM_cpsr),
+			  [_ARM_sp] "=r" (newregs->ARM_sp),
+			  [_ARM_lr] "=o" (newregs->ARM_lr)
+			: [regs_base] "r" (&newregs->ARM_r0)
+			: "memory"
+		);
 	}
 }
 
-- 
1.7.1

[PATCH 2/2] ARM: kexec: Fix crash_setup_regs() for ARMv7 and CONFIG_THUMB2_KERNEL

[Mika Westerberg]

Hi,

On Tue, Nov 16, 2010 at 01:12:21PM +0000, Dave Martin wrote:
>  * Fix kexec build failure with CONFIG_THUMB2_KERNEL.
> 
>  * Avoids deprecated/forbidden sp and pc usage in for ARMv7 onwards,
> retaining compatibility with older architecture versions.
> 
>  * The pc value saved to newregs is now aligned on a predictable
> instruction boundary.
> 
> (stmia { ... pc } or str pc has implementation-defined results
> in most versions of the ARM architecutre, and is prohibited
> (unpredictable) in Thumb-2.)
> 
>  * Switch to named inline asm arguments (else I get readily confused ...)
> 
> The resulting code should be compatible with all architecture versions >= v3,
> with or without CONFIG_THUMB2_KERNEL.
> 
> Applies cleanly on v2.6.37-rc1.

Quickly tested this and seems to work. So FWIW:

Acked-by: Mika Westerberg <mika.westerberg@iki.fi>

> 
> Signed-off-by: Dave Martin <dave.martin@linaro.org>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> Reviewed-by: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm/include/asm/kexec.h |   18 ++++++++++++++----
>  1 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/include/asm/kexec.h b/arch/arm/include/asm/kexec.h
> index b37e02c..c0094d8 100644
> --- a/arch/arm/include/asm/kexec.h
> +++ b/arch/arm/include/asm/kexec.h
> @@ -33,10 +33,20 @@ static inline void crash_setup_regs(struct pt_regs *newregs,
>  	if (oldregs) {
>  		memcpy(newregs, oldregs, sizeof(*newregs));
>  	} else {
> -		__asm__ __volatile__ ("stmia %0, {r0 - r15}"
> -				      : : "r" (&newregs->ARM_r0) : "memory");
> -		__asm__ __volatile__ ("mrs %0, cpsr"
> -				      : "=r" (newregs->ARM_cpsr));
> +		__asm__ __volatile__ (
> +			"stmia	%[regs_base], {r0-r12}\n\t"
> +			"mov	%[_ARM_sp], sp\n\t"
> +			"str	lr, %[_ARM_lr]\n\t"
> +			"adr	%[_ARM_pc], 1f\n\t"
> +			"mrs	%[_ARM_cpsr], cpsr\n\t"
> +		"1:"
> +			: [_ARM_pc] "=r" (newregs->ARM_pc),
> +			  [_ARM_cpsr] "=r" (newregs->ARM_cpsr),
> +			  [_ARM_sp] "=r" (newregs->ARM_sp),
> +			  [_ARM_lr] "=o" (newregs->ARM_lr)
> +			: [regs_base] "r" (&newregs->ARM_r0)
> +			: "memory"
> +		);
>  	}
>  }
>  
> -- 
> 1.7.1

[PATCH 2/2] ARM: kexec: Fix crash_setup_regs() for ARMv7 and CONFIG_THUMB2_KERNEL

[Catalin Marinas]

On 17 November 2010 13:11, Mika Westerberg <mika.westerberg@iki.fi> wrote:
> On Tue, Nov 16, 2010 at 01:12:21PM +0000, Dave Martin wrote:
>> ?* Fix kexec build failure with CONFIG_THUMB2_KERNEL.
>>
>> ?* Avoids deprecated/forbidden sp and pc usage in for ARMv7 onwards,
>> retaining compatibility with older architecture versions.
>>
>> ?* The pc value saved to newregs is now aligned on a predictable
>> instruction boundary.
>>
>> (stmia { ... pc } or str pc has implementation-defined results
>> in most versions of the ARM architecutre, and is prohibited
>> (unpredictable) in Thumb-2.)
>>
>> ?* Switch to named inline asm arguments (else I get readily confused ...)
>>
>> The resulting code should be compatible with all architecture versions >= v3,
>> with or without CONFIG_THUMB2_KERNEL.
>>
>> Applies cleanly on v2.6.37-rc1.
>
> Quickly tested this and seems to work. So FWIW:
>
> Acked-by: Mika Westerberg <mika.westerberg@iki.fi>

It's more like Tested-by :)  (unless you reviewed it as well, though
there seems to be some confusion between Reviewed-by and Acked-by).

-- 
Catalin

[PATCH 2/2] ARM: kexec: Fix crash_setup_regs() for ARMv7 and CONFIG_THUMB2_KERNEL

[Mika Westerberg]

On Wed, Nov 17, 2010 at 01:26:02PM +0000, Catalin Marinas wrote:
> On 17 November 2010 13:11, Mika Westerberg <mika.westerberg@iki.fi> wrote:
[...]
> >
> > Quickly tested this and seems to work. So FWIW:
> >
> > Acked-by: Mika Westerberg <mika.westerberg@iki.fi>
> 
> It's more like Tested-by :)  (unless you reviewed it as well, though
> there seems to be some confusion between Reviewed-by and Acked-by).

Well, I was the original author if this code so I thought that
Acked-by was right but Tested-by is fine :)

Regards,
MW

[PATCH 2/2] ARM: kexec: Fix crash_setup_regs() for ARMv7 and CONFIG_THUMB2_KERNEL

[Catalin Marinas]

On 17 November 2010 13:35, Mika Westerberg <mika.westerberg@iki.fi> wrote:
> On Wed, Nov 17, 2010 at 01:26:02PM +0000, Catalin Marinas wrote:
>> On 17 November 2010 13:11, Mika Westerberg <mika.westerberg@iki.fi> wrote:
> [...]
>> >
>> > Quickly tested this and seems to work. So FWIW:
>> >
>> > Acked-by: Mika Westerberg <mika.westerberg@iki.fi>
>>
>> It's more like Tested-by :) ?(unless you reviewed it as well, though
>> there seems to be some confusion between Reviewed-by and Acked-by).
>
> Well, I was the original author if this code so I thought that
> Acked-by was right but Tested-by is fine :)

We can have both Tested-by and Acked-by then :)

-- 
Catalin

[PATCH 2/2] ARM: kexec: Fix crash_setup_regs() for ARMv7 and CONFIG_THUMB2_KERNEL

[Dave P. Martin]

On Wed, Nov 17, 2010 at 03:35:26PM +0200, Mika Westerberg wrote:
> On Wed, Nov 17, 2010 at 01:26:02PM +0000, Catalin Marinas wrote:
> > On 17 November 2010 13:11, Mika Westerberg <mika.westerberg@iki.fi> wrote:
> [...]
> > >
> > > Quickly tested this and seems to work. So FWIW:
> > >
> > > Acked-by: Mika Westerberg <mika.westerberg@iki.fi>
> > 
> > It's more like Tested-by :)  (unless you reviewed it as well, though
> > there seems to be some confusion between Reviewed-by and Acked-by).
> 
> Well, I was the original author if this code so I thought that
> Acked-by was right but Tested-by is fine :)

Well, I don't know either ... so I'll make it Tested-by for now
if that's OK.

Thanks for trying it out, anyway.

---Dave