Linux-csky Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] riscv: Fixup compile error BUILD_BUG_ON failed
@ 2020-06-27 16:20 guoren
  2020-06-28  2:59 ` Masami Hiramatsu
  0 siblings, 1 reply; 5+ messages in thread
From: guoren @ 2020-06-27 16:20 UTC (permalink / raw)
  To: palmerdabbelt, paul.walmsley, anup, greentime.hu, zong.li
  Cc: linux-riscv, linux-kernel, linux-csky, Guo Ren, Masami Hiramatsu

From: Guo Ren <guoren@linux.alibaba.com>

Unfortunately, the current code couldn't be compiled, because
BUILD_BUG_ON needs a static defined value, not a dynamic
variable with a0 regs. Just use it inline as a solution.

  CC      arch/riscv/kernel/patch.o
In file included from ./include/linux/kernel.h:11,
                 from ./include/linux/list.h:9,
                 from ./include/linux/preempt.h:11,
                 from ./include/linux/spinlock.h:51,
                 from arch/riscv/kernel/patch.c:6:
In function ‘fix_to_virt’,
    inlined from ‘patch_map’ at arch/riscv/kernel/patch.c:37:17:
./include/linux/compiler.h:392:38: error: call to ‘__compiletime_assert_205’ declared with attribute error: BUILD_BUG_ON failed: idx >= __end_of_fixed_addresses
  _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
                                      ^
./include/linux/compiler.h:373:4: note: in definition of macro ‘__compiletime_assert’
    prefix ## suffix();    \
    ^~~~~~
./include/linux/compiler.h:392:2: note: in expansion of macro ‘_compiletime_assert’
  _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
  ^~~~~~~~~~~~~~~~~~~
./include/linux/build_bug.h:39:37: note: in expansion of macro ‘compiletime_assert’
 #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
                                     ^~~~~~~~~~~~~~~~~~
./include/linux/build_bug.h:50:2: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
  BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
  ^~~~~~~~~~~~~~~~
./include/asm-generic/fixmap.h:32:2: note: in expansion of macro ‘BUILD_BUG_ON’
  BUILD_BUG_ON(idx >= __end_of_fixed_addresses);
  ^~~~~~~~~~~~

Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Zong Li <zong.li@sifive.com>
---
 arch/riscv/kernel/patch.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/riscv/kernel/patch.c b/arch/riscv/kernel/patch.c
index d4a64df..f8e84f2 100644
--- a/arch/riscv/kernel/patch.c
+++ b/arch/riscv/kernel/patch.c
@@ -20,7 +20,7 @@ struct patch_insn {
 };
 
 #ifdef CONFIG_MMU
-static void *patch_map(void *addr, int fixmap)
+static inline void *patch_map(void *addr, int fixmap)
 {
 	uintptr_t uintaddr = (uintptr_t) addr;
 	struct page *page;
@@ -37,7 +37,6 @@ static void *patch_map(void *addr, int fixmap)
 	return (void *)set_fixmap_offset(fixmap, page_to_phys(page) +
 					 (uintaddr & ~PAGE_MASK));
 }
-NOKPROBE_SYMBOL(patch_map);
 
 static void patch_unmap(int fixmap)
 {
-- 
2.7.4


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] riscv: Fixup compile error BUILD_BUG_ON failed
  2020-06-27 16:20 [PATCH] riscv: Fixup compile error BUILD_BUG_ON failed guoren
@ 2020-06-28  2:59 ` Masami Hiramatsu
  2020-06-28  6:12   ` Guo Ren
  0 siblings, 1 reply; 5+ messages in thread
From: Masami Hiramatsu @ 2020-06-28  2:59 UTC (permalink / raw)
  To: guoren
  Cc: palmerdabbelt, paul.walmsley, anup, greentime.hu, zong.li,
	linux-riscv, linux-kernel, linux-csky, Guo Ren, Masami Hiramatsu

On Sat, 27 Jun 2020 16:20:02 +0000
guoren@kernel.org wrote:

> From: Guo Ren <guoren@linux.alibaba.com>
> 
> Unfortunately, the current code couldn't be compiled, because
> BUILD_BUG_ON needs a static defined value, not a dynamic
> variable with a0 regs. Just use it inline as a solution.
> 
>   CC      arch/riscv/kernel/patch.o
> In file included from ./include/linux/kernel.h:11,
>                  from ./include/linux/list.h:9,
>                  from ./include/linux/preempt.h:11,
>                  from ./include/linux/spinlock.h:51,
>                  from arch/riscv/kernel/patch.c:6:
> In function ‘fix_to_virt’,
>     inlined from ‘patch_map’ at arch/riscv/kernel/patch.c:37:17:
> ./include/linux/compiler.h:392:38: error: call to ‘__compiletime_assert_205’ declared with attribute error: BUILD_BUG_ON failed: idx >= __end_of_fixed_addresses
>   _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
>                                       ^
> ./include/linux/compiler.h:373:4: note: in definition of macro ‘__compiletime_assert’
>     prefix ## suffix();    \
>     ^~~~~~
> ./include/linux/compiler.h:392:2: note: in expansion of macro ‘_compiletime_assert’
>   _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
>   ^~~~~~~~~~~~~~~~~~~
> ./include/linux/build_bug.h:39:37: note: in expansion of macro ‘compiletime_assert’
>  #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
>                                      ^~~~~~~~~~~~~~~~~~
> ./include/linux/build_bug.h:50:2: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
>   BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
>   ^~~~~~~~~~~~~~~~
> ./include/asm-generic/fixmap.h:32:2: note: in expansion of macro ‘BUILD_BUG_ON’
>   BUILD_BUG_ON(idx >= __end_of_fixed_addresses);
>   ^~~~~~~~~~~~
> 
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Zong Li <zong.li@sifive.com>
> ---
>  arch/riscv/kernel/patch.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/riscv/kernel/patch.c b/arch/riscv/kernel/patch.c
> index d4a64df..f8e84f2 100644
> --- a/arch/riscv/kernel/patch.c
> +++ b/arch/riscv/kernel/patch.c
> @@ -20,7 +20,7 @@ struct patch_insn {
>  };
>  
>  #ifdef CONFIG_MMU
> -static void *patch_map(void *addr, int fixmap)
> +static inline void *patch_map(void *addr, int fixmap)

Would we be better to use "__always_inline" as same as fix_to_virt?
And also, could you add a comment why we need to make it inline?

Thank you,

>  {
>  	uintptr_t uintaddr = (uintptr_t) addr;
>  	struct page *page;
> @@ -37,7 +37,6 @@ static void *patch_map(void *addr, int fixmap)
>  	return (void *)set_fixmap_offset(fixmap, page_to_phys(page) +
>  					 (uintaddr & ~PAGE_MASK));
>  }
> -NOKPROBE_SYMBOL(patch_map);
>  
>  static void patch_unmap(int fixmap)
>  {
> -- 
> 2.7.4
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] riscv: Fixup compile error BUILD_BUG_ON failed
  2020-06-28  2:59 ` Masami Hiramatsu
@ 2020-06-28  6:12   ` Guo Ren
  2020-06-28 14:52     ` Masami Hiramatsu
  0 siblings, 1 reply; 5+ messages in thread
From: Guo Ren @ 2020-06-28  6:12 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Palmer Dabbelt, Paul Walmsley, Anup Patel, Greentime Hu, Zong Li,
	linux-riscv, Linux Kernel Mailing List, linux-csky, Guo Ren

On Sun, Jun 28, 2020 at 10:59 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> On Sat, 27 Jun 2020 16:20:02 +0000
> guoren@kernel.org wrote:
>
> > From: Guo Ren <guoren@linux.alibaba.com>
> >
> > Unfortunately, the current code couldn't be compiled, because
> > BUILD_BUG_ON needs a static defined value, not a dynamic
> > variable with a0 regs. Just use it inline as a solution.
> >
> >   CC      arch/riscv/kernel/patch.o
> > In file included from ./include/linux/kernel.h:11,
> >                  from ./include/linux/list.h:9,
> >                  from ./include/linux/preempt.h:11,
> >                  from ./include/linux/spinlock.h:51,
> >                  from arch/riscv/kernel/patch.c:6:
> > In function ‘fix_to_virt’,
> >     inlined from ‘patch_map’ at arch/riscv/kernel/patch.c:37:17:
> > ./include/linux/compiler.h:392:38: error: call to ‘__compiletime_assert_205’ declared with attribute error: BUILD_BUG_ON failed: idx >= __end_of_fixed_addresses
> >   _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
> >                                       ^
> > ./include/linux/compiler.h:373:4: note: in definition of macro ‘__compiletime_assert’
> >     prefix ## suffix();    \
> >     ^~~~~~
> > ./include/linux/compiler.h:392:2: note: in expansion of macro ‘_compiletime_assert’
> >   _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
> >   ^~~~~~~~~~~~~~~~~~~
> > ./include/linux/build_bug.h:39:37: note: in expansion of macro ‘compiletime_assert’
> >  #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
> >                                      ^~~~~~~~~~~~~~~~~~
> > ./include/linux/build_bug.h:50:2: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
> >   BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
> >   ^~~~~~~~~~~~~~~~
> > ./include/asm-generic/fixmap.h:32:2: note: in expansion of macro ‘BUILD_BUG_ON’
> >   BUILD_BUG_ON(idx >= __end_of_fixed_addresses);
> >   ^~~~~~~~~~~~
> >
> > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > Cc: Masami Hiramatsu <mhiramat@kernel.org>
> > Cc: Zong Li <zong.li@sifive.com>
> > ---
> >  arch/riscv/kernel/patch.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/arch/riscv/kernel/patch.c b/arch/riscv/kernel/patch.c
> > index d4a64df..f8e84f2 100644
> > --- a/arch/riscv/kernel/patch.c
> > +++ b/arch/riscv/kernel/patch.c
> > @@ -20,7 +20,7 @@ struct patch_insn {
> >  };
> >
> >  #ifdef CONFIG_MMU
> > -static void *patch_map(void *addr, int fixmap)
> > +static inline void *patch_map(void *addr, int fixmap)
>
> Would we be better to use "__always_inline" as same as fix_to_virt?
Ok

> And also, could you add a comment why we need to make it inline?
I've mentioned in comment:
> > BUILD_BUG_ON needs a static defined value, not a dynamic
> > variable with a0 regs.

idx must be a const unsigned int or it will cause compile error with
BUILD_BUG_ON.

/*
 * 'index to address' translation. If anyone tries to use the idx
 * directly without translation, we catch the bug with a NULL-deference
 * kernel oops. Illegal ranges of incoming indices are caught too.
 */
static __always_inline unsigned long fix_to_virt(const unsigned int idx)
{
        BUILD_BUG_ON(idx >= __end_of_fixed_addresses);
        return __fix_to_virt(idx);
}

-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] riscv: Fixup compile error BUILD_BUG_ON failed
  2020-06-28  6:12   ` Guo Ren
@ 2020-06-28 14:52     ` Masami Hiramatsu
  2020-06-28 15:58       ` Guo Ren
  0 siblings, 1 reply; 5+ messages in thread
From: Masami Hiramatsu @ 2020-06-28 14:52 UTC (permalink / raw)
  To: Guo Ren
  Cc: Palmer Dabbelt, Paul Walmsley, Anup Patel, Greentime Hu, Zong Li,
	linux-riscv, Linux Kernel Mailing List, linux-csky, Guo Ren

On Sun, 28 Jun 2020 14:12:07 +0800
Guo Ren <guoren@kernel.org> wrote:

> On Sun, Jun 28, 2020 at 10:59 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> >
> > On Sat, 27 Jun 2020 16:20:02 +0000
> > guoren@kernel.org wrote:
> >
> > > From: Guo Ren <guoren@linux.alibaba.com>
> > >
> > > Unfortunately, the current code couldn't be compiled, because
> > > BUILD_BUG_ON needs a static defined value, not a dynamic
> > > variable with a0 regs. Just use it inline as a solution.
> > >
> > >   CC      arch/riscv/kernel/patch.o
> > > In file included from ./include/linux/kernel.h:11,
> > >                  from ./include/linux/list.h:9,
> > >                  from ./include/linux/preempt.h:11,
> > >                  from ./include/linux/spinlock.h:51,
> > >                  from arch/riscv/kernel/patch.c:6:
> > > In function ‘fix_to_virt’,
> > >     inlined from ‘patch_map’ at arch/riscv/kernel/patch.c:37:17:
> > > ./include/linux/compiler.h:392:38: error: call to ‘__compiletime_assert_205’ declared with attribute error: BUILD_BUG_ON failed: idx >= __end_of_fixed_addresses
> > >   _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
> > >                                       ^
> > > ./include/linux/compiler.h:373:4: note: in definition of macro ‘__compiletime_assert’
> > >     prefix ## suffix();    \
> > >     ^~~~~~
> > > ./include/linux/compiler.h:392:2: note: in expansion of macro ‘_compiletime_assert’
> > >   _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
> > >   ^~~~~~~~~~~~~~~~~~~
> > > ./include/linux/build_bug.h:39:37: note: in expansion of macro ‘compiletime_assert’
> > >  #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
> > >                                      ^~~~~~~~~~~~~~~~~~
> > > ./include/linux/build_bug.h:50:2: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
> > >   BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
> > >   ^~~~~~~~~~~~~~~~
> > > ./include/asm-generic/fixmap.h:32:2: note: in expansion of macro ‘BUILD_BUG_ON’
> > >   BUILD_BUG_ON(idx >= __end_of_fixed_addresses);
> > >   ^~~~~~~~~~~~
> > >
> > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > > Cc: Masami Hiramatsu <mhiramat@kernel.org>
> > > Cc: Zong Li <zong.li@sifive.com>
> > > ---
> > >  arch/riscv/kernel/patch.c | 3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > >
> > > diff --git a/arch/riscv/kernel/patch.c b/arch/riscv/kernel/patch.c
> > > index d4a64df..f8e84f2 100644
> > > --- a/arch/riscv/kernel/patch.c
> > > +++ b/arch/riscv/kernel/patch.c
> > > @@ -20,7 +20,7 @@ struct patch_insn {
> > >  };
> > >
> > >  #ifdef CONFIG_MMU
> > > -static void *patch_map(void *addr, int fixmap)
> > > +static inline void *patch_map(void *addr, int fixmap)
> >
> > Would we be better to use "__always_inline" as same as fix_to_virt?
> Ok
> 
> > And also, could you add a comment why we need to make it inline?
> I've mentioned in comment:
> > > BUILD_BUG_ON needs a static defined value, not a dynamic
> > > variable with a0 regs.
> 
> idx must be a const unsigned int or it will cause compile error with
> BUILD_BUG_ON.

Ah, sorry for the confusion. I meant the comment line in the code.
BTW, can we also use "const unsigned int" for the fixmap index so that
no one miss it anymore?

Thank you,

> 
> /*
>  * 'index to address' translation. If anyone tries to use the idx
>  * directly without translation, we catch the bug with a NULL-deference
>  * kernel oops. Illegal ranges of incoming indices are caught too.
>  */
> static __always_inline unsigned long fix_to_virt(const unsigned int idx)
> {
>         BUILD_BUG_ON(idx >= __end_of_fixed_addresses);
>         return __fix_to_virt(idx);
> }
> 
> -- 
> Best Regards
>  Guo Ren
> 
> ML: https://lore.kernel.org/linux-csky/


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] riscv: Fixup compile error BUILD_BUG_ON failed
  2020-06-28 14:52     ` Masami Hiramatsu
@ 2020-06-28 15:58       ` Guo Ren
  0 siblings, 0 replies; 5+ messages in thread
From: Guo Ren @ 2020-06-28 15:58 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Palmer Dabbelt, Paul Walmsley, Anup Patel, Greentime Hu, Zong Li,
	linux-riscv, Linux Kernel Mailing List, linux-csky, Guo Ren

On Sun, Jun 28, 2020 at 10:52 PM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> On Sun, 28 Jun 2020 14:12:07 +0800
> Guo Ren <guoren@kernel.org> wrote:
>
> > On Sun, Jun 28, 2020 at 10:59 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > >
> > > On Sat, 27 Jun 2020 16:20:02 +0000
> > > guoren@kernel.org wrote:
> > >
> > > > From: Guo Ren <guoren@linux.alibaba.com>
> > > >
> > > > Unfortunately, the current code couldn't be compiled, because
> > > > BUILD_BUG_ON needs a static defined value, not a dynamic
> > > > variable with a0 regs. Just use it inline as a solution.
> > > >
> > > >   CC      arch/riscv/kernel/patch.o
> > > > In file included from ./include/linux/kernel.h:11,
> > > >                  from ./include/linux/list.h:9,
> > > >                  from ./include/linux/preempt.h:11,
> > > >                  from ./include/linux/spinlock.h:51,
> > > >                  from arch/riscv/kernel/patch.c:6:
> > > > In function ‘fix_to_virt’,
> > > >     inlined from ‘patch_map’ at arch/riscv/kernel/patch.c:37:17:
> > > > ./include/linux/compiler.h:392:38: error: call to ‘__compiletime_assert_205’ declared with attribute error: BUILD_BUG_ON failed: idx >= __end_of_fixed_addresses
> > > >   _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
> > > >                                       ^
> > > > ./include/linux/compiler.h:373:4: note: in definition of macro ‘__compiletime_assert’
> > > >     prefix ## suffix();    \
> > > >     ^~~~~~
> > > > ./include/linux/compiler.h:392:2: note: in expansion of macro ‘_compiletime_assert’
> > > >   _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
> > > >   ^~~~~~~~~~~~~~~~~~~
> > > > ./include/linux/build_bug.h:39:37: note: in expansion of macro ‘compiletime_assert’
> > > >  #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
> > > >                                      ^~~~~~~~~~~~~~~~~~
> > > > ./include/linux/build_bug.h:50:2: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
> > > >   BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
> > > >   ^~~~~~~~~~~~~~~~
> > > > ./include/asm-generic/fixmap.h:32:2: note: in expansion of macro ‘BUILD_BUG_ON’
> > > >   BUILD_BUG_ON(idx >= __end_of_fixed_addresses);
> > > >   ^~~~~~~~~~~~
> > > >
> > > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > > > Cc: Masami Hiramatsu <mhiramat@kernel.org>
> > > > Cc: Zong Li <zong.li@sifive.com>
> > > > ---
> > > >  arch/riscv/kernel/patch.c | 3 +--
> > > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > > >
> > > > diff --git a/arch/riscv/kernel/patch.c b/arch/riscv/kernel/patch.c
> > > > index d4a64df..f8e84f2 100644
> > > > --- a/arch/riscv/kernel/patch.c
> > > > +++ b/arch/riscv/kernel/patch.c
> > > > @@ -20,7 +20,7 @@ struct patch_insn {
> > > >  };
> > > >
> > > >  #ifdef CONFIG_MMU
> > > > -static void *patch_map(void *addr, int fixmap)
> > > > +static inline void *patch_map(void *addr, int fixmap)
> > >
> > > Would we be better to use "__always_inline" as same as fix_to_virt?
> > Ok
> >
> > > And also, could you add a comment why we need to make it inline?
> > I've mentioned in comment:
> > > > BUILD_BUG_ON needs a static defined value, not a dynamic
> > > > variable with a0 regs.
> >
> > idx must be a const unsigned int or it will cause compile error with
> > BUILD_BUG_ON.
>
> Ah, sorry for the confusion. I meant the comment line in the code.
Ok.

> BTW, can we also use "const unsigned int" for the fixmap index so that
> no one miss it anymore?
Sure, actually I've done it in my patch V2. Thx for mention.

-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-27 16:20 [PATCH] riscv: Fixup compile error BUILD_BUG_ON failed guoren
2020-06-28  2:59 ` Masami Hiramatsu
2020-06-28  6:12   ` Guo Ren
2020-06-28 14:52     ` Masami Hiramatsu
2020-06-28 15:58       ` Guo Ren

Linux-csky Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-csky/0 linux-csky/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-csky linux-csky/ https://lore.kernel.org/linux-csky \
		linux-csky@vger.kernel.org
	public-inbox-index linux-csky

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-csky


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git