Re: [PATCH 01/18] arm64: mm: don't use x18 in idmap_kpti_install_ng_mappings

[Nick Desaulniers]

On Fri, Oct 18, 2019 at 9:10 AM Sami Tolvanen <samitolvanen@xxxxxxxxxx> wrote:
>
> idmap_kpti_install_ng_mappings uses x18 as a temporary register, which
> will result in a conflict when x18 is reserved. Use x16 and x17 instead
> where needed.
>
> Signed-off-by: Sami Tolvanen <samitolvanen@xxxxxxxxxx>

TIL about .req/.unreq.  Seems like a nice way of marking "variable"
lifetime.  Technically, only `pte` needed to be moved to reuse
{w|x}16, but moving most the unreqs together is nicer than splitting
them apart. The usage all looks correct to me.
Reviewed-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>

> ---
>  arch/arm64/mm/proc.S | 63 ++++++++++++++++++++++----------------------
>  1 file changed, 32 insertions(+), 31 deletions(-)
>
> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
> index a1e0592d1fbc..fdabf40a83c8 100644
> --- a/arch/arm64/mm/proc.S
> +++ b/arch/arm64/mm/proc.S
> @@ -250,15 +250,15 @@ ENTRY(idmap_kpti_install_ng_mappings)
>         /* We're the boot CPU. Wait for the others to catch up */
>         sevl
>  1:     wfe
> -       ldaxr   w18, [flag_ptr]
> -       eor     w18, w18, num_cpus
> -       cbnz    w18, 1b
> +       ldaxr   w17, [flag_ptr]
> +       eor     w17, w17, num_cpus
> +       cbnz    w17, 1b
>
>         /* We need to walk swapper, so turn off the MMU. */
>         pre_disable_mmu_workaround
> -       mrs     x18, sctlr_el1
> -       bic     x18, x18, #SCTLR_ELx_M
> -       msr     sctlr_el1, x18
> +       mrs     x17, sctlr_el1
> +       bic     x17, x17, #SCTLR_ELx_M
> +       msr     sctlr_el1, x17
>         isb
>
>         /* Everybody is enjoying the idmap, so we can rewrite swapper. */
> @@ -281,9 +281,9 @@ skip_pgd:
>         isb
>
>         /* We're done: fire up the MMU again */
> -       mrs     x18, sctlr_el1
> -       orr     x18, x18, #SCTLR_ELx_M
> -       msr     sctlr_el1, x18
> +       mrs     x17, sctlr_el1
> +       orr     x17, x17, #SCTLR_ELx_M
> +       msr     sctlr_el1, x17
>         isb
>
>         /*
> @@ -353,46 +353,47 @@ skip_pte:
>         b.ne    do_pte
>         b       next_pmd
>
> +       .unreq  cpu
> +       .unreq  num_cpus
> +       .unreq  swapper_pa
> +       .unreq  cur_pgdp
> +       .unreq  end_pgdp
> +       .unreq  pgd
> +       .unreq  cur_pudp
> +       .unreq  end_pudp
> +       .unreq  pud
> +       .unreq  cur_pmdp
> +       .unreq  end_pmdp
> +       .unreq  pmd
> +       .unreq  cur_ptep
> +       .unreq  end_ptep
> +       .unreq  pte
> +
>         /* Secondary CPUs end up here */
>  __idmap_kpti_secondary:
>         /* Uninstall swapper before surgery begins */
> -       __idmap_cpu_set_reserved_ttbr1 x18, x17
> +       __idmap_cpu_set_reserved_ttbr1 x16, x17
>
>         /* Increment the flag to let the boot CPU we're ready */
> -1:     ldxr    w18, [flag_ptr]
> -       add     w18, w18, #1
> -       stxr    w17, w18, [flag_ptr]
> +1:     ldxr    w16, [flag_ptr]
> +       add     w16, w16, #1
> +       stxr    w17, w16, [flag_ptr]
>         cbnz    w17, 1b
>
>         /* Wait for the boot CPU to finish messing around with swapper */
>         sevl
>  1:     wfe
> -       ldxr    w18, [flag_ptr]
> -       cbnz    w18, 1b
> +       ldxr    w16, [flag_ptr]
> +       cbnz    w16, 1b
>
>         /* All done, act like nothing happened */
> -       offset_ttbr1 swapper_ttb, x18
> +       offset_ttbr1 swapper_ttb, x16
>         msr     ttbr1_el1, swapper_ttb
>         isb
>         ret
>
> -       .unreq  cpu
> -       .unreq  num_cpus
> -       .unreq  swapper_pa
>         .unreq  swapper_ttb
>         .unreq  flag_ptr
> -       .unreq  cur_pgdp
> -       .unreq  end_pgdp
> -       .unreq  pgd
> -       .unreq  cur_pudp
> -       .unreq  end_pudp
> -       .unreq  pud
> -       .unreq  cur_pmdp
> -       .unreq  end_pmdp
> -       .unreq  pmd
> -       .unreq  cur_ptep
> -       .unreq  end_ptep
> -       .unreq  pte
>  ENDPROC(idmap_kpti_install_ng_mappings)
>         .popsection
>  #endif
> --
> 2.23.0.866.gb869b98d4c-goog
>


-- 
Thanks,
~Nick Desaulniers