linux-kernel - Re: [PATCH v2 3/4] RISC-V: Split out the XIP fixups into their own file

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJF2gTTC4V_vSFFranTB6PZrHK08J8KygjfACxfQ91POpRtY-Q@mail.gmail.com>
Date:   Thu, 21 Apr 2022 14:45:23 +0800
From:   Guo Ren <guoren@...nel.org>
To:     Palmer Dabbelt <palmer@...osinc.com>
Cc:     linux-riscv <linux-riscv@...ts.infradead.org>,
        Randy Dunlap <rdunlap@...radead.org>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Palmer Dabbelt <palmer@...belt.com>,
        Albert Ou <aou@...s.berkeley.edu>,
        Anup Patel <anup@...infault.org>,
        Vincent Chen <vincent.chen@...ive.com>,
        Atish Patra <atishp@...osinc.com>,
        Alexandre Ghiti <alexandre.ghiti@...onical.com>,
        jszhang@...nel.org, Vitaly Wool <vitaly.wool@...sulko.com>,
        gatecat@....me, Kefeng Wang <wangkefeng.wang@...wei.com>,
        Nick Kossifidis <mick@....forth.gr>,
        潘庆霖 <panqinglin2020@...as.ac.cn>,
        Mike Rapoport <rppt@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux@...osinc.com
Subject: Re: [PATCH v2 3/4] RISC-V: Split out the XIP fixups into their own file

On Thu, Apr 21, 2022 at 2:48 AM Palmer Dabbelt <palmer@...osinc.com> wrote:
>
> From: Palmer Dabbelt <palmer@...osinc.com>
>
> This was broken by the original refactoring (as the XIP definitions
> depend on <asm/pgtable.h>) and then more broken by the merge (as I
> accidentally took the old version).  This fixes both breakages, while
> also pulling this out of <asm/asm.h> to avoid polluting most assembly
> files with the XIP fixups.
>
> Fixes: bee7fbc38579 ("RISC-V CPU Idle Support")
> Fixes: 63b13e64a829 ("RISC-V: Add arch functions for non-retentive suspend entry/exit")
> Signed-off-by: Palmer Dabbelt <palmer@...osinc.com>
> ---
>  arch/riscv/include/asm/asm.h       | 26 ------------------------
>  arch/riscv/include/asm/xip_fixup.h | 32 ++++++++++++++++++++++++++++++
>  arch/riscv/kernel/head.S           |  1 +
>  arch/riscv/kernel/suspend_entry.S  |  1 +
>  4 files changed, 34 insertions(+), 26 deletions(-)
>  create mode 100644 arch/riscv/include/asm/xip_fixup.h
>
> diff --git a/arch/riscv/include/asm/asm.h b/arch/riscv/include/asm/asm.h
> index 8c2549b16ac0..618d7c5af1a2 100644
> --- a/arch/riscv/include/asm/asm.h
> +++ b/arch/riscv/include/asm/asm.h
> @@ -67,30 +67,4 @@
>  #error "Unexpected __SIZEOF_SHORT__"
>  #endif
>
> -#ifdef __ASSEMBLY__
> -
> -/* Common assembly source macros */
> -
> -#ifdef CONFIG_XIP_KERNEL
> -.macro XIP_FIXUP_OFFSET reg
> -       REG_L t0, _xip_fixup
> -       add \reg, \reg, t0
> -.endm
> -.macro XIP_FIXUP_FLASH_OFFSET reg
> -       la t1, __data_loc
> -       REG_L t1, _xip_phys_offset
> -       sub \reg, \reg, t1
> -       add \reg, \reg, t0
> -.endm
> -_xip_fixup: .dword CONFIG_PHYS_RAM_BASE - CONFIG_XIP_PHYS_ADDR - XIP_OFFSET
> -_xip_phys_offset: .dword CONFIG_XIP_PHYS_ADDR + XIP_OFFSET
> -#else
> -.macro XIP_FIXUP_OFFSET reg
> -.endm
> -.macro XIP_FIXUP_FLASH_OFFSET reg
> -.endm
> -#endif /* CONFIG_XIP_KERNEL */
> -
> -#endif /* __ASSEMBLY__ */
> -
>  #endif /* _ASM_RISCV_ASM_H */
> diff --git a/arch/riscv/include/asm/xip_fixup.h b/arch/riscv/include/asm/xip_fixup.h
> new file mode 100644
> index 000000000000..0d0754305324
> --- /dev/null
> +++ b/arch/riscv/include/asm/xip_fixup.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * XIP fixup macros, only useful in assembly.
> + */
> +#ifndef _ASM_RISCV_XIP_FIXUP_H
> +#define _ASM_RISCV_XIP_FIXUP_H
> +
> +#include <linux/pgtable.h>
> +
> +#ifdef CONFIG_XIP_KERNEL
> +.macro XIP_FIXUP_OFFSET reg
> +        REG_L t0, _xip_fixup
> +        add \reg, \reg, t0
> +.endm
> +.macro XIP_FIXUP_FLASH_OFFSET reg
> +        la t1, __data_loc
> +        li t0, XIP_OFFSET_MASK
> +        and t1, t1, t0
> +        li t1, XIP_OFFSET
I still prefer the style:
REG_L t1, _xip_phys_offset
...
_xip_phys_offset: .dword CONFIG_XIP_PHYS_ADDR + XIP_OFFSET

Because it's more clear and has the same style as your _xip_fixup.

Others:
Reviewed-by: Guo Ren <guoren@...nel.org>

> +        sub t0, t0, t1
> +        sub \reg, \reg, t0
> +.endm
-.endm
> +
> +_xip_fixup: .dword CONFIG_PHYS_RAM_BASE - CONFIG_XIP_PHYS_ADDR - XIP_OFFSET
> +#else
> +.macro XIP_FIXUP_OFFSET reg
> +.endm
> +.macro XIP_FIXUP_FLASH_OFFSET reg
> +.endm
> +#endif /* CONFIG_XIP_KERNEL */
> +
> +#endif
> diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> index 893b8bb69391..822c33aa7f45 100644
> --- a/arch/riscv/kernel/head.S
> +++ b/arch/riscv/kernel/head.S
> @@ -14,6 +14,7 @@
>  #include <asm/hwcap.h>
>  #include <asm/image.h>
> +#include <asm/xip_fixup.h>
>  #include "efi-header.S"
>
>  __HEAD
> diff --git a/arch/riscv/kernel/suspend_entry.S b/arch/riscv/kernel/suspend_entry.S
> index 4b07b809a2b8..aafcca58c19d 100644
> --- a/arch/riscv/kernel/suspend_entry.S
> +++ b/arch/riscv/kernel/suspend_entry.S
> @@ -8,6 +8,7 @@
>  #include <asm/asm.h>
>  #include <asm/asm-offsets.h>
>  #include <asm/csr.h>
> +#include <asm/xip_fixup.h>
>
>         .text
>         .altmacro
> --
> 2.34.1
>


-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/