[PATCH v2] arm64: Fix the kernel panic() on QDF2400 platform

[PATCH v2] arm64: Fix the kernel panic() on QDF2400 platform

[Shanker Donthineni]

The commit 38fd94b0275c 'arm64: Work around Falkor erratum 1003' has
been added to fix the hardware bug but causing a system crash. The
value of the register x1 which contains 'struct mm_struct *' should
be preserved inside macro pre_ttbr0_update_workaround.

Macro pre_ttbr0_update_workaround expects 2nd and 3rd arguments are
temporary registers. Don't clobber register x1, Otherwise the next
load operation 'mmid x1 x1' access the invalid address.

[<ffff0000080989a0>] cpu_do_switch_mm+0x20/0x40
[<ffff000008b18614>] efi_virtmap_load+0x34/0x40
[<ffff000008b1812c>] virt_efi_get_next_variable+0x64/0xc8
[<ffff000008b16204>] efivar_init+0x8c/0x348
[<ffff0000092b777c>] efisubsys_init+0xd4/0x270
[<ffff000009270c74>] do_one_initcall+0x80/0x110
[<ffff000009270ea0>] kernel_init_freeable+0x19c/0x240
[<ffff000008d8cef0>] kernel_init+0x10/0x100
[<ffff000008082ec0>] ret_from_fork+0x10/0x50
 Code: d5033fdf b340bc01 d5182001 d5033fdf (f9416821)
 ---[ end trace 15247ca922eb6bb7 ]---
 note: swapper/0[1] exited with preempt_count 2
 Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b

 SMP: stopping secondary CPUs
 ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b

Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
Tested-by: Manoj Iyer <manoj.iyer@canonical.com>
---
v2: include 12 digit commit-id in commit description.

 arch/arm64/mm/proc.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
index cd4d53d..877d42f 100644
--- a/arch/arm64/mm/proc.S
+++ b/arch/arm64/mm/proc.S
@@ -138,7 +138,7 @@ ENDPROC(cpu_do_resume)
  *	- pgd_phys - physical address of new TTB
  */
 ENTRY(cpu_do_switch_mm)
-	pre_ttbr0_update_workaround x0, x1, x2
+	pre_ttbr0_update_workaround x0, x2, x3
 	mmid	x1, x1				// get mm->context.id
 	bfi	x0, x1, #48, #16		// set the ASID
 	msr	ttbr0_el1, x0			// set TTBR0
-- 
Qualcomm Datacenter Technologies, Inc. on behalf of the Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH v2] arm64: Fix the kernel panic() on QDF2400 platform

[Timur Tabi]

On 02/23/2017 02:02 PM, Shanker Donthineni wrote:
> The commit 38fd94b0275c 'arm64: Work around Falkor erratum 1003' has
> been added to fix the hardware bug but causing a system crash. The

"causes"

> value of the register x1 which contains 'struct mm_struct *' should
> be preserved inside macro pre_ttbr0_update_workaround.

The subject line seems a bit vague and alarmist to me.  In my opinion, this 
is more descriptive:

arm64: qcom: pre_ttbr0_update_workaround should not clobber register x1



-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH v2] arm64: Fix the kernel panic() on QDF2400 platform

[Shanker Donthineni]

On 02/23/2017 02:46 PM, Timur Tabi wrote:
> On 02/23/2017 02:02 PM, Shanker Donthineni wrote:
>> The commit 38fd94b0275c 'arm64: Work around Falkor erratum 1003' has
>> been added to fix the hardware bug but causing a system crash. The
>
> "causes"
>
>> value of the register x1 which contains 'struct mm_struct *' should
>> be preserved inside macro pre_ttbr0_update_workaround.
>
> The subject line seems a bit vague and alarmist to me.  In my opinion, 
> this is more descriptive:
>
> arm64: qcom: pre_ttbr0_update_workaround should not clobber register x1
>
>
Why do you want keep 'pre_ttbr0_update_workaround' in subject, nothing 
wrong with macro definition itself. Problem with the caller, not passing 
the right arguments.

-- 
Shanker Donthineni
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH v2] arm64: Fix the kernel panic() on QDF2400 platform

[Timur Tabi]

On 02/23/2017 03:05 PM, Shanker Donthineni wrote:
> Why do you want keep 'pre_ttbr0_update_workaround' in subject, nothing
> wrong with macro definition itself. Problem with the caller, not passing
> the right arguments.

Ok, how about this:

arm64: qcom: do not use x1 when calling pre_ttbr0_update_workaround

"Fix the kernel panic() on QDF2400 platform" could mean almost anything. 
There could plenty of future kernel panics on the QDF2400.

Also, I think our legal review would insist on saying "Qualcomm Technologies 
QDF2400", which would make the line too long.

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH v2] arm64: Fix the kernel panic() on QDF2400 platform

[Shanker Donthineni]

Hi Timur,


On 02/23/2017 03:11 PM, Timur Tabi wrote:
> On 02/23/2017 03:05 PM, Shanker Donthineni wrote:
>> Why do you want keep 'pre_ttbr0_update_workaround' in subject, nothing
>> wrong with macro definition itself. Problem with the caller, not passing
>> the right arguments.
>
> Ok, how about this:
>
> arm64: qcom: do not use x1 when calling pre_ttbr0_update_workaround
>
> "Fix the kernel panic() on QDF2400 platform" could mean almost 
> anything. There could plenty of future kernel panics on the QDF2400.
>
> Also, I think our legal review would insist on saying "Qualcomm 
> Technologies QDF2400", which would make the line too long.
>

I don't understand why do you want to insert keyword 'qcom' in subject. 
None of the commits "git log --oneline arch/arm64/mm/proc.S" shows 
platform specific keywords.
I would like to see comments from Will Deacon, and follow his suggestions.

38fd94b arm64: Work around Falkor erratum 1003
f33bcf0 arm64: Factor out TTBR0_EL1 post-update workaround into a 
specific asm macro
623b476 arm64: move sp_el0 and tpidr_el1 into cpu_suspend_ctx
6ba3b55 arm64: use alternative auto-nop
744c6c3 arm64: kernel: Fix unmasked debug exceptions when restoring 
mdscr_el1
b611303 arm64: vmlinux.ld: Add mmuoff data sections and move mmuoff text 
into idmap
2ce39ad arm64: debug: unmask PSTATE.D earlier
cabe1c8 arm64: Change cpu_resume() to enable mmu early then access 
sleep_sp by va
7b7293a arm64: Fold proc-macros.S into assembler.h
104a0c0 arm64: Add workaround for Cavium erratum 27456
50e1881 arm64: mm: add code to safely replace TTBR1_EL1
f436b2a arm64: kernel: fix architected PMU registers unconditional access
60792ad arm64: kernel: enforce pmuserenr_el0 initialization and restore
f00083c arm64: mm: place __cpu_setup in .text
44eaacf arm64: Add 16K page size support
5aec715 arm64: mm: rewrite ASID allocator and MM context-switching code
fa7aae8 arm64: proc: de-scope TLBI operation during cold boot
d8d23fa arm64: mdscr_el1: avoid exposing DCC to userspace
8d446c8 arm64/mm: Add PROT_DEVICE_nGnRnE and PROT_NORMAL_WT
8ec4198 arm64: mm: ensure patched kernel text is fetched from PoU
4b3dc96 arm64: force CONFIG_SMP=y and remove redundant #ifdefs
2f4b829 arm64: Add support for hardware updates of the access and dirty 
pte bits

Shanker Donthineni
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.