arm64: kernel: Fix unmasked debug exceptions when restoring mdscr_el1
PATCH 1/1 arm64: kernel: Fix unmasked debug exceptions when restoring mdscr_el1
Changes to make the resume from cpu_suspend() code behave more like secondary boot caused debug exceptions to be unmasked early by __cpu_setup(). We then go on to restore mdscr_el1 in cpu_do_resume(), potentially taking break or watch points based on uninitialised registers.
Mask debug exceptions in cpu_do_resume(), which is specific to resume from cpu_suspend(). Debug exceptions will be restored to their original state by local_dbg_restore() in cpu_suspend(), which runs after hw_breakpoint_restore() has re-initialised the other registers.
--- arch/arm64/mm/proc.S | 9 +++++++++ 1 file changed, 9 insertions(+)
arch/arm64/mm/proc.S +9 -0
@@ -100,7 +100,16 @@ ENTRY(cpu_do_resume) msr tcr_el1, x8 msr vbar_el1, x9 + + /* + * __cpu_setup() cleared MDSCR_EL1.MDE and friends, before unmasking + * debug exceptions. By restoring MDSCR_EL1 here, we may take a debug + * exception. Mask them until local_dbg_restore() in cpu_suspend() + * resets them. + */ + disable_dbg msr mdscr_el1, x10 + msr sctlr_el1, x12 /* * Restore oslsr_el1 by writing oslar_el1
Signature · 1 line
2.8.0.rc3
PATCH 1/1 arm64: kernel: Fix unmasked debug exceptions when restoring mdscr_el1
Quoted 4 lines · James Morse
Changes to make the resume from
cpu_suspend()code behave more like secondary boot caused debug exceptions to be unmasked early by__cpu_setup(). We then go on to restoremdscr_el1incpu_do_resume(), potentially taking break or watch points based on uninitialised registers.
Another option would be moving enable_dbg() out of __cpu_setup() and calling when it returns in the cold boot path, that would not change much in terms of debugging but we would avoid fiddling about with daif three times in the resume path just to restore it to what it was on suspend entry :)
Thanks !
Lorenzo
Quoted 37 lines
Mask debug exceptions in
cpu_do_resume(), which is specific to resume fromcpu_suspend(). Debug exceptions will be restored to their original state bylocal_dbg_restore()incpu_suspend(), which runs afterhw_breakpoint_restore()has re-initialised the other registers.Reported-by Lorenzo Pieralisi <[email protected]>Fixes cabe1c81ea5b ("arm64: Change cpu_resume() to enable mmu early then access sleep_sp by va")Cc <[email protected]> #4.7Signed-off-by James Morse <[email protected]>--- arch/arm64/mm/proc.S | 9 +++++++++ 1 file changed, 9 insertions(+)arch/arm64/mm/proc.S +9
-0@@ -100,7 +100,16 @@ ENTRY(cpu_do_resume) msr tcr_el1, x8 msr vbar_el1, x9 + + /* + * __cpu_setup() cleared MDSCR_EL1.MDE and friends, before unmasking + * debug exceptions. By restoring MDSCR_EL1 here, we may take a debug + * exception. Mask them until local_dbg_restore() in cpu_suspend() + * resets them. + */ + disable_dbg msr mdscr_el1, x10 + msr sctlr_el1, x12 /* * Restore oslsr_el1 by writing oslar_el1Signature · 1 line
2.8.0.rc3
PATCH 1/1 arm64: kernel: Fix unmasked debug exceptions when restoring mdscr_el1
Hi Lorenzo,
Quoted 11 lines · Lorenzo Pieralisi
Quoted 4 lines · James Morse
Changes to make the resume from
cpu_suspend()code behave more like secondary boot caused debug exceptions to be unmasked early by__cpu_setup(). We then go on to restoremdscr_el1incpu_do_resume(), potentially taking break or watch points based on uninitialised registers.Another option would be moving
enable_dbg()out of__cpu_setup()and calling when it returns in the cold boot path, that would not change much in terms of debugging but we would avoid fiddling about with daif three times in the resume path just to restore it to what it was on suspend entry :)
I agree, but there is/will-be quite a lot of churn in the code that calls enable_mmu() if Ard's cleanup series is taken. I think this is the tidiest fix to backport. I will post a patch to tidy this up as you suggest for v4.9-rc1.
Thanks,
James
PATCH 1/1 arm64: kernel: Fix unmasked debug exceptions when restoring mdscr_el1
In reply to James MorseQuoted 27 lines · James Morse
Changes to make the resume from
cpu_suspend()code behave more like secondary boot caused debug exceptions to be unmasked early by__cpu_setup(). We then go on to restoremdscr_el1incpu_do_resume(), potentially taking break or watch points based on uninitialised registers.Mask debug exceptions in
cpu_do_resume(), which is specific to resume fromcpu_suspend(). Debug exceptions will be restored to their original state bylocal_dbg_restore()incpu_suspend(), which runs afterhw_breakpoint_restore()has re-initialised the other registers.Reported-by Lorenzo Pieralisi <[email protected]>Fixes cabe1c81ea5b ("arm64: Change cpu_resume() to enable mmu early then access sleep_sp by va")Cc <[email protected]> #4.7Signed-off-by James Morse <[email protected]>--- arch/arm64/mm/proc.S | 9 +++++++++ 1 file changed, 9 insertions(+)arch/arm64/mm/proc.S +1
-0@@ -100,7 +100,16 @@ ENTRY(cpu_do_resume) msr tcr_el1, x8 msr vbar_el1, x9 +
arch/arm64/mm/proc.S ENTRY(cpu_do_resume) · line 113
Quoted 10 lines
arch/arm64/mm/proc.S +8
-0+ /* + * __cpu_setup() cleared MDSCR_EL1.MDE and friends, before unmasking + * debug exceptions. By restoring MDSCR_EL1 here, we may take a debug + * exception. Mask them until local_dbg_restore() in cpu_suspend() + * resets them. + */ + disable_dbg msr mdscr_el1, x10 + msr sctlr_el1, x12
Looks good to me:
Will