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

[Palmer Dabbelt]

On Wed, 20 Apr 2022 23:45:23 PDT (-0700), guoren@xxxxxxxxxx wrote:
> On Thu, Apr 21, 2022 at 2:48 AM Palmer Dabbelt <palmer@xxxxxxxxxxxx> wrote:
> > From: Palmer Dabbelt <palmer@xxxxxxxxxxxx>
> >
> > 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@xxxxxxxxxxxx>
> > ---
> >  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.

Thanks, not quite shure how I pulled the old version over.  I've fixed 
this one up.

> Others:
> Reviewed-by: Guo Ren <guoren@xxxxxxxxxx>
>
> > +        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