| Message ID | 20210428033415.107756-1-palmer@dabbelt.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | RISC-V: Always define XIP_FIXUP | expand |
On Wed, Apr 28, 2021 at 9:05 AM Palmer Dabbelt <palmer@dabbelt.com> wrote: > > From: Palmer Dabbelt <palmerdabbelt@google.com> > > 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. > > Fixes: 44c922572952 ("RISC-V: enable XIP") > Reported-by: Guenter Roeck <linux@roeck-us.net> > Signed-off-by: Palmer Dabbelt <palmerdabbelt@google.com> Looks good to me. Reviewed-by: Anup Patel <anup@brainfault.org> Regards, Anup > --- > 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 > +#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)) > -- > 2.31.1.498.g6c1eba8ee3d-goog >
Le 4/27/21 à 11:34 PM, Palmer Dabbelt a écrit : > From: Palmer Dabbelt <palmerdabbelt@google.com> > > 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. > > Fixes: 44c922572952 ("RISC-V: enable XIP") > Reported-by: Guenter Roeck <linux@roeck-us.net> > Signed-off-by: Palmer Dabbelt <palmerdabbelt@google.com> > --- > 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__. > +#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
On Wed, 28 Apr 2021 01:25:55 PDT (-0700), alex@ghiti.fr wrote: > Le 4/27/21 à 11:34 PM, Palmer Dabbelt a écrit : >> From: Palmer Dabbelt <palmerdabbelt@google.com> >> >> 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@roeck-us.net> >> Signed-off-by: Palmer Dabbelt <palmerdabbelt@google.com> >> --- >> 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. >> +#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
Le 4/29/21 à 12:43 AM, Palmer Dabbelt a écrit : > On Wed, 28 Apr 2021 01:25:55 PDT (-0700), alex@ghiti.fr wrote: >> Le 4/27/21 à 11:34 PM, Palmer Dabbelt a écrit : >>> From: Palmer Dabbelt <palmerdabbelt@google.com> >>> >>> 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@roeck-us.net> >>> Signed-off-by: Palmer Dabbelt <palmerdabbelt@google.com> >>> --- >>> 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@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
Hi Palmer, Le 4/29/21 à 12:43 AM, Palmer Dabbelt a écrit : > On Wed, 28 Apr 2021 01:25:55 PDT (-0700), alex@ghiti.fr wrote: >> Le 4/27/21 à 11:34 PM, Palmer Dabbelt a écrit : >>> From: Palmer Dabbelt <palmerdabbelt@google.com> >>> >>> 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@roeck-us.net> >>> Signed-off-by: Palmer Dabbelt <palmerdabbelt@google.com> >>> --- >>> 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. I posted a tutorial here on how I test XIP kernel: https://alexghiti.github.io/xip/XIP.html If something does not work for you, please tell me. Alex > >>> +#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@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
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 +#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))