Re: [PATCH resend] kbuild: provide include/asm/asm-prototypes.h for x86

2016-11-21 Thread Peter Wu
On Mon, Nov 21, 2016 at 06:27:14PM +1100, Nicholas Piggin wrote:
> Hi Adam,
> 
> Thanks. I'd suggest doing x86: or x86/kbuild: prefix for the patch. Also
> possibly consider describing what the patch does at a higher level in your
> subject line, e.g.:
> 
>   x86/kbuild: enable modversions for symbols exported from asm
> 
> Also, it wouldn't hurt to add a little changelog of your own. Describe
> problem then solution, e.g.,
> 
>   Commit 4efca4ed ("kbuild: modversions for EXPORT_SYMBOL() for asm") adds
>   modversion support for symbols exported from asm files. Architectures
>   must include C-style declarations for those symbols in asm/asm-prototypes.h
>   in order for them to be versioned.
> 
>   Add these declarations for x86, and an architecture-independent file that
>   can be used for common symbols.
> 
> (if you want to use that as-is or rewrite it, no problem).
> 
> You can add Acked-by: Nicholas Piggin 

Hi Adam, Nicholas,

Thanks for the patch! I have confirmed that this fixes the defconfig and
the normal .config build I use for debugging, so:

Tested-by: Peter Wu 

It might also be worth to mention in the changelog the user impact,
namely that kernels may fail to load modules at all when
CONFIG_MODVERSIONS=y.

Kind regards,
Peter

> Also it's not a big deal, but if you redo the patch, you could consider
> splitting it into two (first add the generic header, then the x86 header),
> but both can go via the x86 tree.
> 
> Thanks,
> Nick
> 
> On Mon, 21 Nov 2016 07:39:45 +0100
> Adam Borowski  wrote:
> 
> > Nicholas Piggin wrote:
> > > Architectures will need to have an include/asm/asm-prototypes.h that
> > > defines or #include<>s C-style prototypes for exported asm functions.
> > > We can do an asm-generic version for the common ones like memset so
> > > there's not a lot of pointless duplication there.  
> > 
> > Signed-off-by: Adam Borowski 
> > Tested-by: Kalle Valo 
> > ---
> >  arch/x86/include/asm/asm-prototypes.h | 12 
> >  include/asm-generic/asm-prototypes.h  |  7 +++
> >  2 files changed, 19 insertions(+)
> >  create mode 100644 arch/x86/include/asm/asm-prototypes.h
> >  create mode 100644 include/asm-generic/asm-prototypes.h
> > 
> > diff --git a/arch/x86/include/asm/asm-prototypes.h 
> > b/arch/x86/include/asm/asm-prototypes.h
> > new file mode 100644
> > index 000..ae87224
> > --- /dev/null
> > +++ b/arch/x86/include/asm/asm-prototypes.h
> > @@ -0,0 +1,12 @@
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include 
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > diff --git a/include/asm-generic/asm-prototypes.h 
> > b/include/asm-generic/asm-prototypes.h
> > new file mode 100644
> > index 000..df13637
> > --- /dev/null
> > +++ b/include/asm-generic/asm-prototypes.h
> > @@ -0,0 +1,7 @@
> > +#include 
> > +extern void *__memset(void *, int, __kernel_size_t);
> > +extern void *__memcpy(void *, const void *, __kernel_size_t);
> > +extern void *__memmove(void *, const void *, __kernel_size_t);
> > +extern void *memset(void *, int, __kernel_size_t);
> > +extern void *memcpy(void *, const void *, __kernel_size_t);
> > +extern void *memmove(void *, const void *, __kernel_size_t);
> 


Re: [PATCH resend] kbuild: provide include/asm/asm-prototypes.h for x86

2016-11-20 Thread Nicholas Piggin
Hi Adam,

Thanks. I'd suggest doing x86: or x86/kbuild: prefix for the patch. Also
possibly consider describing what the patch does at a higher level in your
subject line, e.g.:

  x86/kbuild: enable modversions for symbols exported from asm

Also, it wouldn't hurt to add a little changelog of your own. Describe
problem then solution, e.g.,

  Commit 4efca4ed ("kbuild: modversions for EXPORT_SYMBOL() for asm") adds
  modversion support for symbols exported from asm files. Architectures
  must include C-style declarations for those symbols in asm/asm-prototypes.h
  in order for them to be versioned.

  Add these declarations for x86, and an architecture-independent file that
  can be used for common symbols.

(if you want to use that as-is or rewrite it, no problem).

You can add Acked-by: Nicholas Piggin 

Also it's not a big deal, but if you redo the patch, you could consider
splitting it into two (first add the generic header, then the x86 header),
but both can go via the x86 tree.

Thanks,
Nick

On Mon, 21 Nov 2016 07:39:45 +0100
Adam Borowski  wrote:

> Nicholas Piggin wrote:
> > Architectures will need to have an include/asm/asm-prototypes.h that
> > defines or #include<>s C-style prototypes for exported asm functions.
> > We can do an asm-generic version for the common ones like memset so
> > there's not a lot of pointless duplication there.  
> 
> Signed-off-by: Adam Borowski 
> Tested-by: Kalle Valo 
> ---
>  arch/x86/include/asm/asm-prototypes.h | 12 
>  include/asm-generic/asm-prototypes.h  |  7 +++
>  2 files changed, 19 insertions(+)
>  create mode 100644 arch/x86/include/asm/asm-prototypes.h
>  create mode 100644 include/asm-generic/asm-prototypes.h
> 
> diff --git a/arch/x86/include/asm/asm-prototypes.h 
> b/arch/x86/include/asm/asm-prototypes.h
> new file mode 100644
> index 000..ae87224
> --- /dev/null
> +++ b/arch/x86/include/asm/asm-prototypes.h
> @@ -0,0 +1,12 @@
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> diff --git a/include/asm-generic/asm-prototypes.h 
> b/include/asm-generic/asm-prototypes.h
> new file mode 100644
> index 000..df13637
> --- /dev/null
> +++ b/include/asm-generic/asm-prototypes.h
> @@ -0,0 +1,7 @@
> +#include 
> +extern void *__memset(void *, int, __kernel_size_t);
> +extern void *__memcpy(void *, const void *, __kernel_size_t);
> +extern void *__memmove(void *, const void *, __kernel_size_t);
> +extern void *memset(void *, int, __kernel_size_t);
> +extern void *memcpy(void *, const void *, __kernel_size_t);
> +extern void *memmove(void *, const void *, __kernel_size_t);



[PATCH resend] kbuild: provide include/asm/asm-prototypes.h for x86

2016-11-20 Thread Adam Borowski
Nicholas Piggin wrote:
> Architectures will need to have an include/asm/asm-prototypes.h that
> defines or #include<>s C-style prototypes for exported asm functions.
> We can do an asm-generic version for the common ones like memset so
> there's not a lot of pointless duplication there.

Signed-off-by: Adam Borowski 
Tested-by: Kalle Valo 
---
 arch/x86/include/asm/asm-prototypes.h | 12 
 include/asm-generic/asm-prototypes.h  |  7 +++
 2 files changed, 19 insertions(+)
 create mode 100644 arch/x86/include/asm/asm-prototypes.h
 create mode 100644 include/asm-generic/asm-prototypes.h

diff --git a/arch/x86/include/asm/asm-prototypes.h 
b/arch/x86/include/asm/asm-prototypes.h
new file mode 100644
index 000..ae87224
--- /dev/null
+++ b/arch/x86/include/asm/asm-prototypes.h
@@ -0,0 +1,12 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#include 
+#include 
+#include 
+#include 
diff --git a/include/asm-generic/asm-prototypes.h 
b/include/asm-generic/asm-prototypes.h
new file mode 100644
index 000..df13637
--- /dev/null
+++ b/include/asm-generic/asm-prototypes.h
@@ -0,0 +1,7 @@
+#include 
+extern void *__memset(void *, int, __kernel_size_t);
+extern void *__memcpy(void *, const void *, __kernel_size_t);
+extern void *__memmove(void *, const void *, __kernel_size_t);
+extern void *memset(void *, int, __kernel_size_t);
+extern void *memcpy(void *, const void *, __kernel_size_t);
+extern void *memmove(void *, const void *, __kernel_size_t);
-- 
2.10.2

Nicholas Piggin wrote:
> On Sun, 20 Nov 2016 19:26:23 +0100 Peter Wu  wrote:
>
>> Current git master (v4.9-rc5-364-g77079b1) with the latest kbuild fixes
>> is still failing to load modules when built with CONFIG_MODVERSIONS=y on
>> x86_64 using GCC 6.2.1.
>>
>> It can still be reproduced with make defconfig, then enabling
>> CONFIG_MODVERSIONS=y. The build output contains:
>>
>> WARNING: "memcpy" [net/netfilter/nf_nat.ko] has no CRC!
>> WARNING: "memmove" [net/netfilter/nf_nat.ko] has no CRC!
>> WARNING: "_copy_to_user" [fs/efivarfs/efivarfs.ko] has no CRC!
>> WARNING: "memcpy" [fs/efivarfs/efivarfs.ko] has no CRC!
>> WARNING: "_copy_from_user" [fs/efivarfs/efivarfs.ko] has no CRC!
>
> Sorry it's taken some time, bear with us. The arch specific patches need
> to be merged now. Adam, what is the status of your patch? Please submit
> to x86 maintainers if you haven't already.

I've re-checked against 4.9-rc6.

It'd probably fit better with kbuild parts, but it's up to you to decide;
I'm sending to x86 guys as you wish.


Meow!