Changes since v2:
- Update the uaccess.h to address issues pointed out by Luc
Changes since v1:
- Now a series, v1 was only the "Reserve memblock for initrd" patch
- Added fixes for compiler issues pointed out by the kbuild robot
- Updated "Reserve memblock for initrd", as per Mike's comments
This is a few fixes found during testing 5.9.
Stafford Horne (3):
openrisc: Reserve memblock for initrd
openrisc: Fix cache API compile issue when not inlining
openrisc: Fix issue with get_user for 64-bit values
arch/openrisc/include/asm/uaccess.h | 33 ++++++++++++++++++-----------
arch/openrisc/kernel/setup.c | 10 +++++++++
arch/openrisc/mm/cache.c | 2 +-
3 files changed, 32 insertions(+), 13 deletions(-)
--
2.26.2
Recently OpenRISC added support for external initrd images, but I found
some instability when using larger buildroot initrd images. It turned
out that I forgot to reserve the memblock space for the initrd image.
This patch fixes the instability issue by reserving memblock space.
Fixes: ff6c923dbec3 ("openrisc: Add support for external initrd images")
Signed-off-by: Stafford Horne <[email protected]>
Reviewed-by: Mike Rapoport <[email protected]>
---
Changes since v2:
- None
Changes since v1:
- Updated to use separate variables as suggested by Mike.
arch/openrisc/kernel/setup.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/openrisc/kernel/setup.c b/arch/openrisc/kernel/setup.c
index b18e775f8be3..13c87f1f872b 100644
--- a/arch/openrisc/kernel/setup.c
+++ b/arch/openrisc/kernel/setup.c
@@ -80,6 +80,16 @@ static void __init setup_memory(void)
*/
memblock_reserve(__pa(_stext), _end - _stext);
+#ifdef CONFIG_BLK_DEV_INITRD
+ /* Then reserve the initrd, if any */
+ if (initrd_start && (initrd_end > initrd_start)) {
+ unsigned long aligned_start = ALIGN_DOWN(initrd_start, PAGE_SIZE);
+ unsigned long aligned_end = ALIGN(initrd_end, PAGE_SIZE);
+
+ memblock_reserve(__pa(aligned_start), aligned_end - aligned_start);
+ }
+#endif /* CONFIG_BLK_DEV_INITRD */
+
early_init_fdt_reserve_self();
early_init_fdt_scan_reserved_mem();
--
2.26.2
A build failure was raised by kbuild with the following error.
drivers/android/binder.c: Assembler messages:
drivers/android/binder.c:3861: Error: unrecognized keyword/register name `l.lwz ?ap,4(r24)'
drivers/android/binder.c:3866: Error: unrecognized keyword/register name `l.addi ?ap,r0,0'
The issue is with 64-bit get_user() calls on openrisc. I traced this to
a problem where in the internally in the get_user macros there is a cast
to long __gu_val this causes GCC to think the get_user call is 32-bit.
This binder code is really long and GCC allocates register r30, which
triggers the issue. The 64-bit get_user asm tries to get the 64-bit pair
register, which for r30 overflows the general register names and returns
the dummy register ?ap.
The fix here is to move the temporary variables into the asm macros. We
use a 32-bit __gu_tmp for 32-bit and smaller macro and a 64-bit tmp in
the 64-bit macro. The cast in the 64-bit macro has a trick of casting
through __typeof__((x)-(x)) which avoids the below warning. This was
barrowed from riscv.
arch/openrisc/include/asm/uaccess.h:240:8: warning: cast to pointer from integer of different size
I tested this is a small unit test to check reading between 64-bit and
32-bit pointers to 64-bit and 32-bit values in all combinations. Also I
ran make C=1 to confirm no new sparse warnings came up. It all looks
clean to me.
Link: https://lore.kernel.org/lkml/202008200453.ohnhqkjQ%[email protected]/
Signed-off-by: Stafford Horne <[email protected]>
Cc: Luc Van Oostenryck <[email protected]>
---
Changes since v2:
- Add back temporary variables but move to a different place, as described in
commit message.
Changes since v1:
- New
arch/openrisc/include/asm/uaccess.h | 33 ++++++++++++++++++-----------
1 file changed, 21 insertions(+), 12 deletions(-)
diff --git a/arch/openrisc/include/asm/uaccess.h b/arch/openrisc/include/asm/uaccess.h
index f0390211236b..120f5005461b 100644
--- a/arch/openrisc/include/asm/uaccess.h
+++ b/arch/openrisc/include/asm/uaccess.h
@@ -165,19 +165,19 @@ struct __large_struct {
#define __get_user_nocheck(x, ptr, size) \
({ \
- long __gu_err, __gu_val; \
- __get_user_size(__gu_val, (ptr), (size), __gu_err); \
- (x) = (__force __typeof__(*(ptr)))__gu_val; \
+ long __gu_err; \
+ __get_user_size((x), (ptr), (size), __gu_err); \
__gu_err; \
})
#define __get_user_check(x, ptr, size) \
({ \
- long __gu_err = -EFAULT, __gu_val = 0; \
+ long __gu_err = -EFAULT; \
const __typeof__(*(ptr)) __user *__gu_addr = (ptr); \
- if (access_ok(__gu_addr, size)) \
- __get_user_size(__gu_val, __gu_addr, (size), __gu_err); \
- (x) = (__force __typeof__(*(ptr)))__gu_val; \
+ if (access_ok(__gu_addr, size)) \
+ __get_user_size((x), __gu_addr, (size), __gu_err); \
+ else \
+ (x) = (__typeof__(*(ptr))) 0; \
__gu_err; \
})
@@ -191,11 +191,13 @@ do { \
case 2: __get_user_asm(x, ptr, retval, "l.lhz"); break; \
case 4: __get_user_asm(x, ptr, retval, "l.lwz"); break; \
case 8: __get_user_asm2(x, ptr, retval); break; \
- default: (x) = __get_user_bad(); \
+ default: (x) = (__typeof__(*(ptr)))__get_user_bad(); \
} \
} while (0)
#define __get_user_asm(x, addr, err, op) \
+{ \
+ unsigned long __gu_tmp; \
__asm__ __volatile__( \
"1: "op" %1,0(%2)\n" \
"2:\n" \
@@ -209,10 +211,14 @@ do { \
" .align 2\n" \
" .long 1b,3b\n" \
".previous" \
- : "=r"(err), "=r"(x) \
- : "r"(addr), "i"(-EFAULT), "0"(err))
+ : "=r"(err), "=r"(__gu_tmp) \
+ : "r"(addr), "i"(-EFAULT), "0"(err)); \
+ (x) = (__typeof__(*(addr)))__gu_tmp; \
+}
#define __get_user_asm2(x, addr, err) \
+{ \
+ unsigned long long __gu_tmp; \
__asm__ __volatile__( \
"1: l.lwz %1,0(%2)\n" \
"2: l.lwz %H1,4(%2)\n" \
@@ -229,8 +235,11 @@ do { \
" .long 1b,4b\n" \
" .long 2b,4b\n" \
".previous" \
- : "=r"(err), "=&r"(x) \
- : "r"(addr), "i"(-EFAULT), "0"(err))
+ : "=r"(err), "=&r"(__gu_tmp) \
+ : "r"(addr), "i"(-EFAULT), "0"(err)); \
+ (x) = (__typeof__(*(addr)))( \
+ (__typeof__((x)-(x)))__gu_tmp); \
+}
/* more complex routines */
--
2.26.2
I found this when compiling a kbuild random config with GCC 11. The
config enables CONFIG_DEBUG_SECTION_MISMATCH, which sets CFLAGS
-fno-inline-functions-called-once. This causes the call to cache_loop in
cache.c to not be inlined causing the below compile error.
In file included from arch/openrisc/mm/cache.c:13:
arch/openrisc/mm/cache.c: In function 'cache_loop':
./arch/openrisc/include/asm/spr.h:16:27: warning: 'asm' operand 0 probably does not match constraints
16 | #define mtspr(_spr, _val) __asm__ __volatile__ ( \
| ^~~~~~~
arch/openrisc/mm/cache.c:25:3: note: in expansion of macro 'mtspr'
25 | mtspr(reg, line);
| ^~~~~
./arch/openrisc/include/asm/spr.h:16:27: error: impossible constraint in 'asm'
16 | #define mtspr(_spr, _val) __asm__ __volatile__ ( \
| ^~~~~~~
arch/openrisc/mm/cache.c:25:3: note: in expansion of macro 'mtspr'
25 | mtspr(reg, line);
| ^~~~~
make[1]: *** [scripts/Makefile.build:283: arch/openrisc/mm/cache.o] Error 1
The asm constraint "K" requires a immediate constant argument to mtspr,
however because of no inlining a register argument is passed causing a
failure. Fix this by using __always_inline.
Link: https://lore.kernel.org/lkml/202008200453.ohnhqkjQ%[email protected]/
Signed-off-by: Stafford Horne <[email protected]>
---
Changes since v2:
- None
Changes since v1:
- New
arch/openrisc/mm/cache.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/openrisc/mm/cache.c b/arch/openrisc/mm/cache.c
index 08f56af387ac..534a52ec5e66 100644
--- a/arch/openrisc/mm/cache.c
+++ b/arch/openrisc/mm/cache.c
@@ -16,7 +16,7 @@
#include <asm/cacheflush.h>
#include <asm/tlbflush.h>
-static void cache_loop(struct page *page, const unsigned int reg)
+static __always_inline void cache_loop(struct page *page, const unsigned int reg)
{
unsigned long paddr = page_to_pfn(page) << PAGE_SHIFT;
unsigned long line = paddr & ~(L1_CACHE_BYTES - 1);
--
2.26.2
On Fri, Sep 11, 2020 at 08:39:40AM +0900, Stafford Horne wrote:
> A build failure was raised by kbuild with the following error.
>
> drivers/android/binder.c: Assembler messages:
> drivers/android/binder.c:3861: Error: unrecognized keyword/register name `l.lwz ?ap,4(r24)'
> drivers/android/binder.c:3866: Error: unrecognized keyword/register name `l.addi ?ap,r0,0'
>
> The issue is with 64-bit get_user() calls on openrisc. I traced this to
> a problem where in the internally in the get_user macros there is a cast
> to long __gu_val this causes GCC to think the get_user call is 32-bit.
> This binder code is really long and GCC allocates register r30, which
> triggers the issue. The 64-bit get_user asm tries to get the 64-bit pair
> register, which for r30 overflows the general register names and returns
> the dummy register ?ap.
>
> The fix here is to move the temporary variables into the asm macros. We
> use a 32-bit __gu_tmp for 32-bit and smaller macro and a 64-bit tmp in
> the 64-bit macro. The cast in the 64-bit macro has a trick of casting
> through __typeof__((x)-(x)) which avoids the below warning. This was
> barrowed from riscv.
>
> arch/openrisc/include/asm/uaccess.h:240:8: warning: cast to pointer from integer of different size
>
> I tested this is a small unit test to check reading between 64-bit and
> 32-bit pointers to 64-bit and 32-bit values in all combinations. Also I
> ran make C=1 to confirm no new sparse warnings came up. It all looks
> clean to me.
It looks correct to me too now at C & assembly level.
Feel free to add my:
Reviewed-by: Luc Van Oostenryck <[email protected]>
-- Luc
On Fri, Sep 11, 2020 at 10:55:26PM +0200, Luc Van Oostenryck wrote:
> On Fri, Sep 11, 2020 at 08:39:40AM +0900, Stafford Horne wrote:
> > A build failure was raised by kbuild with the following error.
> >
> > drivers/android/binder.c: Assembler messages:
> > drivers/android/binder.c:3861: Error: unrecognized keyword/register name `l.lwz ?ap,4(r24)'
> > drivers/android/binder.c:3866: Error: unrecognized keyword/register name `l.addi ?ap,r0,0'
> >
> > The issue is with 64-bit get_user() calls on openrisc. I traced this to
> > a problem where in the internally in the get_user macros there is a cast
> > to long __gu_val this causes GCC to think the get_user call is 32-bit.
> > This binder code is really long and GCC allocates register r30, which
> > triggers the issue. The 64-bit get_user asm tries to get the 64-bit pair
> > register, which for r30 overflows the general register names and returns
> > the dummy register ?ap.
> >
> > The fix here is to move the temporary variables into the asm macros. We
> > use a 32-bit __gu_tmp for 32-bit and smaller macro and a 64-bit tmp in
> > the 64-bit macro. The cast in the 64-bit macro has a trick of casting
> > through __typeof__((x)-(x)) which avoids the below warning. This was
> > barrowed from riscv.
> >
> > arch/openrisc/include/asm/uaccess.h:240:8: warning: cast to pointer from integer of different size
> >
> > I tested this is a small unit test to check reading between 64-bit and
> > 32-bit pointers to 64-bit and 32-bit values in all combinations. Also I
> > ran make C=1 to confirm no new sparse warnings came up. It all looks
> > clean to me.
>
> It looks correct to me too now at C & assembly level.
> Feel free to add my:
> Reviewed-by: Luc Van Oostenryck <[email protected]>
Thanks a lot.
-Stafford