diff mbox series

[U-Boot,11/30] riscv: fix use of incorrectly sized variables

Message ID 20181019220743.15020-12-lukas.auer@aisec.fraunhofer.de
State Superseded
Delegated to: Andes
Headers show
Series General fixes / cleanup for RISC-V and improvements to qemu-riscv | expand

Commit Message

Lukas Auer Oct. 19, 2018, 10:07 p.m. UTC
The RISC-V arch incorrectly uses 32-bit instead of 64-bit variables in
several places. Fix this.
In addition, BITS_PER_LONG is set to 64 on RV64I systems.

Signed-off-by: Lukas Auer <lukas.auer@aisec.fraunhofer.de>
---

 arch/riscv/include/asm/io.h          |  6 +++---
 arch/riscv/include/asm/posix_types.h |  6 +++---
 arch/riscv/include/asm/types.h       |  4 ++++
 arch/riscv/lib/interrupts.c          | 10 +++++-----
 4 files changed, 15 insertions(+), 11 deletions(-)

Comments

Bin Meng Oct. 22, 2018, 7:36 a.m. UTC | #1
Hi Lukas,

On Sat, Oct 20, 2018 at 6:10 AM Lukas Auer
<lukas.auer@aisec.fraunhofer.de> wrote:
>
> The RISC-V arch incorrectly uses 32-bit instead of 64-bit variables in
> several places. Fix this.
> In addition, BITS_PER_LONG is set to 64 on RV64I systems.
>
> Signed-off-by: Lukas Auer <lukas.auer@aisec.fraunhofer.de>
> ---
>
>  arch/riscv/include/asm/io.h          |  6 +++---
>  arch/riscv/include/asm/posix_types.h |  6 +++---
>  arch/riscv/include/asm/types.h       |  4 ++++
>  arch/riscv/lib/interrupts.c          | 10 +++++-----
>  4 files changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/arch/riscv/include/asm/io.h b/arch/riscv/include/asm/io.h
> index f4a76d8720..472814a13e 100644
> --- a/arch/riscv/include/asm/io.h
> +++ b/arch/riscv/include/asm/io.h
> @@ -74,12 +74,12 @@ static inline phys_addr_t virt_to_phys(void *vaddr)
>  #define __arch_getb(a)                 (*(unsigned char *)(a))
>  #define __arch_getw(a)                 (*(unsigned short *)(a))
>  #define __arch_getl(a)                 (*(unsigned int *)(a))
> -#define __arch_getq(a)                 (*(unsigned long *)(a))
> +#define __arch_getq(a)                 (*(unsigned long long *)(a))
>
>  #define __arch_putb(v, a)              (*(unsigned char *)(a) = (v))
>  #define __arch_putw(v, a)              (*(unsigned short *)(a) = (v))
>  #define __arch_putl(v, a)              (*(unsigned int *)(a) = (v))
> -#define __arch_putq(v, a)              (*(unsigned long *)(a) = (v))
> +#define __arch_putq(v, a)              (*(unsigned long long *)(a) = (v))
>
>  #define __raw_writeb(v, a)             __arch_putb(v, a)
>  #define __raw_writew(v, a)             __arch_putw(v, a)
> @@ -152,7 +152,7 @@ static inline u32 readl(const volatile void __iomem *addr)
>
>  static inline u64 readq(const volatile void __iomem *addr)
>  {
> -       u32     val;
> +       u64     val;
>
>         val = __arch_getq(addr);
>         __iormb();
> diff --git a/arch/riscv/include/asm/posix_types.h b/arch/riscv/include/asm/posix_types.h
> index 7438dbeb03..0fc052082a 100644
> --- a/arch/riscv/include/asm/posix_types.h
> +++ b/arch/riscv/include/asm/posix_types.h
> @@ -37,10 +37,10 @@ typedef unsigned short              __kernel_gid_t;
>  #ifdef __GNUC__
>  typedef __SIZE_TYPE__          __kernel_size_t;
>  #else
> -typedef unsigned int           __kernel_size_t;
> +typedef unsigned long          __kernel_size_t;
>  #endif
> -typedef int                    __kernel_ssize_t;
> -typedef int                    __kernel_ptrdiff_t;
> +typedef long                   __kernel_ssize_t;
> +typedef long                   __kernel_ptrdiff_t;
>  typedef long                   __kernel_time_t;
>  typedef long                   __kernel_suseconds_t;
>  typedef long                   __kernel_clock_t;
> diff --git a/arch/riscv/include/asm/types.h b/arch/riscv/include/asm/types.h
> index bd8627196d..403cf9a48f 100644
> --- a/arch/riscv/include/asm/types.h
> +++ b/arch/riscv/include/asm/types.h
> @@ -21,7 +21,11 @@ typedef unsigned short umode_t;
>   */
>  #ifdef __KERNEL__
>
> +#ifdef CONFIG_ARCH_RV64I
> +#define BITS_PER_LONG 64
> +#else
>  #define BITS_PER_LONG 32
> +#endif
>
>  #include <stddef.h>
>
> diff --git a/arch/riscv/lib/interrupts.c b/arch/riscv/lib/interrupts.c
> index 0a0995a7af..1c16980a64 100644
> --- a/arch/riscv/lib/interrupts.c
> +++ b/arch/riscv/lib/interrupts.c
> @@ -12,7 +12,7 @@
>  #include <asm/system.h>
>  #include <asm/encoding.h>
>
> -static void _exit_trap(int code, uint epc, struct pt_regs *regs);
> +static void _exit_trap(ulong code, ulong epc, struct pt_regs *regs);
>
>  int interrupt_init(void)
>  {
> @@ -34,9 +34,9 @@ int disable_interrupts(void)
>         return 0;
>  }
>
> -uint handle_trap(uint mcause, uint epc, struct pt_regs *regs)
> +ulong handle_trap(ulong mcause, ulong epc, struct pt_regs *regs)
>  {
> -       uint is_int;
> +       ulong is_int;
>
>         is_int = (mcause & MCAUSE_INT);
>         if ((is_int) && ((mcause & MCAUSE_CAUSE)  == IRQ_M_EXT))
> @@ -60,7 +60,7 @@ __attribute__((weak)) void timer_interrupt(struct pt_regs *regs)
>  {
>  }
>
> -static void _exit_trap(int code, uint epc, struct pt_regs *regs)
> +static void _exit_trap(ulong code, ulong epc, struct pt_regs *regs)
>  {
>         static const char * const exception_code[] = {
>                 "Instruction address misaligned",
> @@ -70,6 +70,6 @@ static void _exit_trap(int code, uint epc, struct pt_regs *regs)
>                 "Load address misaligned"
>         };
>
> -       printf("exception code: %d , %s , epc %08x , ra %08lx\n",
> +       printf("exception code: %ld , %s , epc %016lx , ra %016lx\n",

We should not print 16 digits on a RV32 system.

>                 code, exception_code[code], epc, regs->ra);
>  }
> --

Others look good to me.

Regards,
Bin
Rick Chen Oct. 23, 2018, 5:52 a.m. UTC | #2
> > -static void _exit_trap(int code, uint epc, struct pt_regs *regs)
> > +static void _exit_trap(ulong code, ulong epc, struct pt_regs *regs)
> >  {
> >         static const char * const exception_code[] = {
> >                 "Instruction address misaligned",
> > @@ -70,6 +70,6 @@ static void _exit_trap(int code, uint epc, struct pt_regs
> *regs)
> >                 "Load address misaligned"
> >         };
> >
> > -       printf("exception code: %d , %s , epc %08x , ra %08lx\n",
> > +       printf("exception code: %ld , %s , epc %016lx , ra %016lx\n",
>
> We should not print 16 digits on a RV32 system.

Hi Lukas

Maybe you can try as below :

printf("exception code: %ld , %s , epc %p , ra %p\n",
      code, exception_code[code], (void *)epc, (void *)regs->ra);

Rick

>
> >                 code, exception_code[code], epc, regs->ra);
> >  }
> > --
>
> Others look good to me.
>
> Regards,
> Bin
Lukas Auer Oct. 25, 2018, 11:47 a.m. UTC | #3
Hi Rick and Bin,

On Tue, 2018-10-23 at 13:52 +0800, Rick Chen wrote:
> > > -static void _exit_trap(int code, uint epc, struct pt_regs *regs)
> > > +static void _exit_trap(ulong code, ulong epc, struct pt_regs
> > > *regs)
> > >  {
> > >         static const char * const exception_code[] = {
> > >                 "Instruction address misaligned",
> > > @@ -70,6 +70,6 @@ static void _exit_trap(int code, uint epc,
> > > struct pt_regs
> > 
> > *regs)
> > >                 "Load address misaligned"
> > >         };
> > > 
> > > -       printf("exception code: %d , %s , epc %08x , ra %08lx\n",
> > > +       printf("exception code: %ld , %s , epc %016lx , ra
> > > %016lx\n",
> > 
> > We should not print 16 digits on a RV32 system.
> 
> Hi Lukas
> 
> Maybe you can try as below :
> 
> printf("exception code: %ld , %s , epc %p , ra %p\n",
>       code, exception_code[code], (void *)epc, (void *)regs->ra);
> 
> Rick
> 

Thanks, I will update the patch to use this format string in v2.

Thanks,
Lukas

> > 
> > >                 code, exception_code[code], epc, regs->ra);
> > >  }
> > > --
> > 
> > Others look good to me.
> > 
> > Regards,
> > Bin
diff mbox series

Patch

diff --git a/arch/riscv/include/asm/io.h b/arch/riscv/include/asm/io.h
index f4a76d8720..472814a13e 100644
--- a/arch/riscv/include/asm/io.h
+++ b/arch/riscv/include/asm/io.h
@@ -74,12 +74,12 @@  static inline phys_addr_t virt_to_phys(void *vaddr)
 #define __arch_getb(a)			(*(unsigned char *)(a))
 #define __arch_getw(a)			(*(unsigned short *)(a))
 #define __arch_getl(a)			(*(unsigned int *)(a))
-#define __arch_getq(a)			(*(unsigned long *)(a))
+#define __arch_getq(a)			(*(unsigned long long *)(a))
 
 #define __arch_putb(v, a)		(*(unsigned char *)(a) = (v))
 #define __arch_putw(v, a)		(*(unsigned short *)(a) = (v))
 #define __arch_putl(v, a)		(*(unsigned int *)(a) = (v))
-#define __arch_putq(v, a)		(*(unsigned long *)(a) = (v))
+#define __arch_putq(v, a)		(*(unsigned long long *)(a) = (v))
 
 #define __raw_writeb(v, a)		__arch_putb(v, a)
 #define __raw_writew(v, a)		__arch_putw(v, a)
@@ -152,7 +152,7 @@  static inline u32 readl(const volatile void __iomem *addr)
 
 static inline u64 readq(const volatile void __iomem *addr)
 {
-	u32	val;
+	u64	val;
 
 	val = __arch_getq(addr);
 	__iormb();
diff --git a/arch/riscv/include/asm/posix_types.h b/arch/riscv/include/asm/posix_types.h
index 7438dbeb03..0fc052082a 100644
--- a/arch/riscv/include/asm/posix_types.h
+++ b/arch/riscv/include/asm/posix_types.h
@@ -37,10 +37,10 @@  typedef unsigned short		__kernel_gid_t;
 #ifdef __GNUC__
 typedef __SIZE_TYPE__		__kernel_size_t;
 #else
-typedef unsigned int		__kernel_size_t;
+typedef unsigned long		__kernel_size_t;
 #endif
-typedef int			__kernel_ssize_t;
-typedef int			__kernel_ptrdiff_t;
+typedef long			__kernel_ssize_t;
+typedef long			__kernel_ptrdiff_t;
 typedef long			__kernel_time_t;
 typedef long			__kernel_suseconds_t;
 typedef long			__kernel_clock_t;
diff --git a/arch/riscv/include/asm/types.h b/arch/riscv/include/asm/types.h
index bd8627196d..403cf9a48f 100644
--- a/arch/riscv/include/asm/types.h
+++ b/arch/riscv/include/asm/types.h
@@ -21,7 +21,11 @@  typedef unsigned short umode_t;
  */
 #ifdef __KERNEL__
 
+#ifdef CONFIG_ARCH_RV64I
+#define BITS_PER_LONG 64
+#else
 #define BITS_PER_LONG 32
+#endif
 
 #include <stddef.h>
 
diff --git a/arch/riscv/lib/interrupts.c b/arch/riscv/lib/interrupts.c
index 0a0995a7af..1c16980a64 100644
--- a/arch/riscv/lib/interrupts.c
+++ b/arch/riscv/lib/interrupts.c
@@ -12,7 +12,7 @@ 
 #include <asm/system.h>
 #include <asm/encoding.h>
 
-static void _exit_trap(int code, uint epc, struct pt_regs *regs);
+static void _exit_trap(ulong code, ulong epc, struct pt_regs *regs);
 
 int interrupt_init(void)
 {
@@ -34,9 +34,9 @@  int disable_interrupts(void)
 	return 0;
 }
 
-uint handle_trap(uint mcause, uint epc, struct pt_regs *regs)
+ulong handle_trap(ulong mcause, ulong epc, struct pt_regs *regs)
 {
-	uint is_int;
+	ulong is_int;
 
 	is_int = (mcause & MCAUSE_INT);
 	if ((is_int) && ((mcause & MCAUSE_CAUSE)  == IRQ_M_EXT))
@@ -60,7 +60,7 @@  __attribute__((weak)) void timer_interrupt(struct pt_regs *regs)
 {
 }
 
-static void _exit_trap(int code, uint epc, struct pt_regs *regs)
+static void _exit_trap(ulong code, ulong epc, struct pt_regs *regs)
 {
 	static const char * const exception_code[] = {
 		"Instruction address misaligned",
@@ -70,6 +70,6 @@  static void _exit_trap(int code, uint epc, struct pt_regs *regs)
 		"Load address misaligned"
 	};
 
-	printf("exception code: %d , %s , epc %08x , ra %08lx\n",
+	printf("exception code: %ld , %s , epc %016lx , ra %016lx\n",
 		code, exception_code[code], epc, regs->ra);
 }