Re: [PATCH] RISC-V: Always define XIP_FIXUP

[Alex Ghiti]

Le 4/29/21 à 12:43 AM, Palmer Dabbelt a écrit :
> On Wed, 28 Apr 2021 01:25:55 PDT (-0700), alex@xxxxxxxx wrote:
> > Le 4/27/21 à 11:34 PM, Palmer Dabbelt a écrit :
> > > From: Palmer Dabbelt <palmerdabbelt@xxxxxxxxxx>
> > >
> > > XIP depends on MMU, but XIP_FIXUP is defined throughout the kernel in
> > > order to avoid excessive ifdefs.  This just makes sure to always define
> > > XIP_FIXIP, which will fix MMU=n builds.
> >
> > A small typo here.
>
> Actually two: "defined" should have been "used".  Both are fixed.
>
> > > Fixes: 44c922572952 ("RISC-V: enable XIP")
> > > Reported-by: Guenter Roeck <linux@xxxxxxxxxxxx>
> > > Signed-off-by: Palmer Dabbelt <palmerdabbelt@xxxxxxxxxx>
> > > ---
> > >   arch/riscv/include/asm/pgtable.h | 24 ++++++++++++------------
> > >   1 file changed, 12 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/arch/riscv/include/asm/pgtable.h 
> > > b/arch/riscv/include/asm/pgtable.h
> > > index 2f1384e14e31..fd749351f432 100644
> > > --- a/arch/riscv/include/asm/pgtable.h
> > > +++ b/arch/riscv/include/asm/pgtable.h
> > > @@ -73,18 +73,6 @@
> > >   #endif
> > >   #define FIXADDR_START    (FIXADDR_TOP - FIXADDR_SIZE)
> > >
> > > -#ifdef CONFIG_XIP_KERNEL
> > > -#define XIP_OFFSET        SZ_8M
> > > -#define XIP_FIXUP(addr) ({                            \
> > > -    uintptr_t __a = (uintptr_t)(addr);                    \
> > > -    (__a >= CONFIG_XIP_PHYS_ADDR && __a < CONFIG_XIP_PHYS_ADDR + 
> > > SZ_16M) ?    \
> > > -        __a - CONFIG_XIP_PHYS_ADDR + CONFIG_PHYS_RAM_BASE - 
> > > XIP_OFFSET :\
> > > -        __a;                                \
> > > -    })
> > > -#else
> > > -#define XIP_FIXUP(addr)        (addr)
> > > -#endif /* CONFIG_XIP_KERNEL */
> > > -
> > >   #endif
> > >
> > >   #ifndef __ASSEMBLY__
> > > @@ -101,6 +89,18 @@
> > >   #include <asm/pgtable-32.h>
> > >   #endif /* CONFIG_64BIT */
> > >
> > > +#ifdef CONFIG_XIP_KERNEL
> > > +#define XIP_OFFSET        SZ_8M
> >
> >
> > XIP_OFFSET is used in head.S and then this breaks XIP_KERNEL. XIP_OFFSET
> > must live outside the ifndef __ASSEMBLY__.
>
> Thanks, I hadn't even seen XIP_OFFSET.  This is fixed in the v2.
>
> Do you have an XIP config that will run on QEMU, and a way to run it? If 
> so, can you post a defconfig and some instructions?  That'll make it 
> easier to test on my end.

Yes, I'll do that later today or tomorrow.

> > > +#define XIP_FIXUP(addr) ({                            \
> > > +    uintptr_t __a = (uintptr_t)(addr);                    \
> > > +    (__a >= CONFIG_XIP_PHYS_ADDR && __a < CONFIG_XIP_PHYS_ADDR + 
> > > SZ_16M) ?    \
> > > +        __a - CONFIG_XIP_PHYS_ADDR + CONFIG_PHYS_RAM_BASE - 
> > > XIP_OFFSET :\
> > > +        __a;                                \
> > > +    })
> > > +#else
> > > +#define XIP_FIXUP(addr)        (addr)
> > > +#endif /* CONFIG_XIP_KERNEL */
> > > +
> > >   #ifdef CONFIG_MMU
> > >   /* Number of entries in the page global directory */
> > >   #define PTRS_PER_PGD    (PAGE_SIZE / sizeof(pgd_t))
> >
> > Thank you for doing that!
> >
> > Alex
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-riscv