diff mbox series

lib/xz: Fix powerpc build with KERNEL_XZ

Message ID 20180918230756.26035-1-joel@jms.id.au
State New
Headers show
Series lib/xz: Fix powerpc build with KERNEL_XZ | expand

Commit Message

Joel Stanley Sept. 18, 2018, 11:07 p.m. UTC
This partially reverts faa16bc404d72a5 ("lib: Use existing define with
polynomial").

The cleanup added a dependency on include/linux, which broke the PowerPC
boot wrapper/decompresser when KERNEL_XZ is enabled:

  BOOTCC  arch/powerpc/boot/decompress.o
 In file included from arch/powerpc/boot/../../../lib/decompress_unxz.c:233,
                 from arch/powerpc/boot/decompress.c:42:
 arch/powerpc/boot/../../../lib/xz/xz_crc32.c:18:10: fatal error:
 linux/crc32poly.h: No such file or directory
  #include <linux/crc32poly.h>
           ^~~~~~~~~~~~~~~~~~~

The powerpc decompressor is a hairy corner of the kernel. Even while building
a 64-bit kernel it needs to build a 32-bit binary and therefore avoid including
files from include/linux.

Fixes: faa16bc404d7 ("lib: Use existing define with polynomial")
Signed-off-by: Joel Stanley <joel@jms.id.au>

---
We need to clean up the powerpc boot decompresser but that work will be
more involved than we would include in a late -rc. Please consider
merging this fix for 4.19. Thanks!

 lib/xz/xz_crc32.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

-- 
2.17.1

Comments

Christophe Leroy Sept. 19, 2018, 5:52 a.m. UTC | #1
Le 19/09/2018 à 01:07, Joel Stanley a écrit :
> This partially reverts faa16bc404d72a5 ("lib: Use existing define with

> polynomial").

> 

> The cleanup added a dependency on include/linux, which broke the PowerPC

> boot wrapper/decompresser when KERNEL_XZ is enabled:

> 

>    BOOTCC  arch/powerpc/boot/decompress.o

>   In file included from arch/powerpc/boot/../../../lib/decompress_unxz.c:233,

>                   from arch/powerpc/boot/decompress.c:42:

>   arch/powerpc/boot/../../../lib/xz/xz_crc32.c:18:10: fatal error:

>   linux/crc32poly.h: No such file or directory

>    #include <linux/crc32poly.h>

>             ^~~~~~~~~~~~~~~~~~~

> 

> The powerpc decompressor is a hairy corner of the kernel. Even while building

> a 64-bit kernel it needs to build a 32-bit binary and therefore avoid including

> files from include/linux.

> 

> Fixes: faa16bc404d7 ("lib: Use existing define with polynomial")

> Signed-off-by: Joel Stanley <joel@jms.id.au>

> ---

> We need to clean up the powerpc boot decompresser but that work will be

> more involved than we would include in a late -rc. Please consider

> merging this fix for 4.19. Thanks!

> 

>   lib/xz/xz_crc32.c | 3 +--

>   1 file changed, 1 insertion(+), 2 deletions(-)

> 

> diff --git a/lib/xz/xz_crc32.c b/lib/xz/xz_crc32.c

> index 25a5d87e2e4c..34532d14fd4c 100644

> --- a/lib/xz/xz_crc32.c

> +++ b/lib/xz/xz_crc32.c

> @@ -15,7 +15,6 @@

>    * but they are bigger and use more memory for the lookup table.

>    */

>   

> -#include <linux/crc32poly.h>

>   #include "xz_private.h"

>   

>   /*

> @@ -30,7 +29,7 @@ STATIC_RW_DATA uint32_t xz_crc32_table[256];

>   

>   XZ_EXTERN void xz_crc32_init(void)

>   {

> -	const uint32_t poly = CRC32_POLY_LE;

> +	const uint32_t poly = 0xEDB88320;


Maybe avoid capital letters ?

What about adding something like the following in xz_private.h instead:

#define CRC32_POLY_LE 0xedb88320

Christophe

>   

>   	uint32_t i;

>   	uint32_t j;

>
Krzysztof Kozlowski Sept. 19, 2018, 6:39 a.m. UTC | #2
On Wed, 19 Sep 2018 at 01:08, Joel Stanley <joel@jms.id.au> wrote:
>

> This partially reverts faa16bc404d72a5 ("lib: Use existing define with

> polynomial").

>

> The cleanup added a dependency on include/linux, which broke the PowerPC

> boot wrapper/decompresser when KERNEL_XZ is enabled:

>

>   BOOTCC  arch/powerpc/boot/decompress.o

>  In file included from arch/powerpc/boot/../../../lib/decompress_unxz.c:233,

>                  from arch/powerpc/boot/decompress.c:42:

>  arch/powerpc/boot/../../../lib/xz/xz_crc32.c:18:10: fatal error:

>  linux/crc32poly.h: No such file or directory

>   #include <linux/crc32poly.h>

>            ^~~~~~~~~~~~~~~~~~~

>

> The powerpc decompressor is a hairy corner of the kernel. Even while building

> a 64-bit kernel it needs to build a 32-bit binary and therefore avoid including

> files from include/linux.


I fixed the build error here:
https://lkml.org/lkml/2018/8/29/179

If you choose to remove any includes from /linux, then go ahead but
please use original reported-by :)

Best regards,
Krzysztof

>

> Fixes: faa16bc404d7 ("lib: Use existing define with polynomial")

> Signed-off-by: Joel Stanley <joel@jms.id.au>

> ---

> We need to clean up the powerpc boot decompresser but that work will be

> more involved than we would include in a late -rc. Please consider

> merging this fix for 4.19. Thanks!

>

>  lib/xz/xz_crc32.c | 3 +--

>  1 file changed, 1 insertion(+), 2 deletions(-)

>

> diff --git a/lib/xz/xz_crc32.c b/lib/xz/xz_crc32.c

> index 25a5d87e2e4c..34532d14fd4c 100644

> --- a/lib/xz/xz_crc32.c

> +++ b/lib/xz/xz_crc32.c

> @@ -15,7 +15,6 @@

>   * but they are bigger and use more memory for the lookup table.

>   */

>

> -#include <linux/crc32poly.h>

>  #include "xz_private.h"

>

>  /*

> @@ -30,7 +29,7 @@ STATIC_RW_DATA uint32_t xz_crc32_table[256];

>

>  XZ_EXTERN void xz_crc32_init(void)

>  {

> -       const uint32_t poly = CRC32_POLY_LE;

> +       const uint32_t poly = 0xEDB88320;

>

>         uint32_t i;

>         uint32_t j;

> --

> 2.17.1

>
Oliver O'Halloran Sept. 19, 2018, 6:44 a.m. UTC | #3
On Wed, Sep 19, 2018 at 3:52 PM, Christophe LEROY
<christophe.leroy@c-s.fr> wrote:
>

>

> Le 19/09/2018 à 01:07, Joel Stanley a écrit :

>>

>> This partially reverts faa16bc404d72a5 ("lib: Use existing define with

>> polynomial").

>>

>> The cleanup added a dependency on include/linux, which broke the PowerPC

>> boot wrapper/decompresser when KERNEL_XZ is enabled:

>>

>>    BOOTCC  arch/powerpc/boot/decompress.o

>>   In file included from

>> arch/powerpc/boot/../../../lib/decompress_unxz.c:233,

>>                   from arch/powerpc/boot/decompress.c:42:

>>   arch/powerpc/boot/../../../lib/xz/xz_crc32.c:18:10: fatal error:

>>   linux/crc32poly.h: No such file or directory

>>    #include <linux/crc32poly.h>

>>             ^~~~~~~~~~~~~~~~~~~

>>

>> The powerpc decompressor is a hairy corner of the kernel. Even while

>> building

>> a 64-bit kernel it needs to build a 32-bit binary and therefore avoid

>> including

>> files from include/linux.

>>

>> Fixes: faa16bc404d7 ("lib: Use existing define with polynomial")

>> Signed-off-by: Joel Stanley <joel@jms.id.au>

>> ---

>> We need to clean up the powerpc boot decompresser but that work will be

>> more involved than we would include in a late -rc. Please consider

>> merging this fix for 4.19. Thanks!

>>

>>   lib/xz/xz_crc32.c | 3 +--

>>   1 file changed, 1 insertion(+), 2 deletions(-)

>>

>> diff --git a/lib/xz/xz_crc32.c b/lib/xz/xz_crc32.c

>> index 25a5d87e2e4c..34532d14fd4c 100644

>> --- a/lib/xz/xz_crc32.c

>> +++ b/lib/xz/xz_crc32.c

>> @@ -15,7 +15,6 @@

>>    * but they are bigger and use more memory for the lookup table.

>>    */

>>   -#include <linux/crc32poly.h>

>>   #include "xz_private.h"

>>     /*

>> @@ -30,7 +29,7 @@ STATIC_RW_DATA uint32_t xz_crc32_table[256];

>>     XZ_EXTERN void xz_crc32_init(void)

>>   {

>> -       const uint32_t poly = CRC32_POLY_LE;

>> +       const uint32_t poly = 0xEDB88320;

>

>

> Maybe avoid capital letters ?

>

> What about adding something like the following in xz_private.h instead:

>

> #define CRC32_POLY_LE 0xedb88320


The problem is that it's pulling in linux/crc32poly.h. To support old
systems with a 32bit Open Firmware we boot wrapper we build the boot
wrapper as a 32bit ELF even for a 64 bit kernel. The headers generated
by Kbuild are only valid for the 64bit kernel so we have to avoid
pulling them into the boot wrapper.

> Christophe

>

>>         uint32_t i;

>>         uint32_t j;

>>

>
Christophe Leroy Sept. 19, 2018, 6:52 a.m. UTC | #4
Le 19/09/2018 à 08:44, Oliver a écrit :
> On Wed, Sep 19, 2018 at 3:52 PM, Christophe LEROY

> <christophe.leroy@c-s.fr> wrote:

>>

>>

>> Le 19/09/2018 à 01:07, Joel Stanley a écrit :

>>>

>>> This partially reverts faa16bc404d72a5 ("lib: Use existing define with

>>> polynomial").

>>>

>>> The cleanup added a dependency on include/linux, which broke the PowerPC

>>> boot wrapper/decompresser when KERNEL_XZ is enabled:

>>>

>>>     BOOTCC  arch/powerpc/boot/decompress.o

>>>    In file included from

>>> arch/powerpc/boot/../../../lib/decompress_unxz.c:233,

>>>                    from arch/powerpc/boot/decompress.c:42:

>>>    arch/powerpc/boot/../../../lib/xz/xz_crc32.c:18:10: fatal error:

>>>    linux/crc32poly.h: No such file or directory

>>>     #include <linux/crc32poly.h>

>>>              ^~~~~~~~~~~~~~~~~~~

>>>

>>> The powerpc decompressor is a hairy corner of the kernel. Even while

>>> building

>>> a 64-bit kernel it needs to build a 32-bit binary and therefore avoid

>>> including

>>> files from include/linux.

>>>

>>> Fixes: faa16bc404d7 ("lib: Use existing define with polynomial")

>>> Signed-off-by: Joel Stanley <joel@jms.id.au>

>>> ---

>>> We need to clean up the powerpc boot decompresser but that work will be

>>> more involved than we would include in a late -rc. Please consider

>>> merging this fix for 4.19. Thanks!

>>>

>>>    lib/xz/xz_crc32.c | 3 +--

>>>    1 file changed, 1 insertion(+), 2 deletions(-)

>>>

>>> diff --git a/lib/xz/xz_crc32.c b/lib/xz/xz_crc32.c

>>> index 25a5d87e2e4c..34532d14fd4c 100644

>>> --- a/lib/xz/xz_crc32.c

>>> +++ b/lib/xz/xz_crc32.c

>>> @@ -15,7 +15,6 @@

>>>     * but they are bigger and use more memory for the lookup table.

>>>     */

>>>    -#include <linux/crc32poly.h>

>>>    #include "xz_private.h"

>>>      /*

>>> @@ -30,7 +29,7 @@ STATIC_RW_DATA uint32_t xz_crc32_table[256];

>>>      XZ_EXTERN void xz_crc32_init(void)

>>>    {

>>> -       const uint32_t poly = CRC32_POLY_LE;

>>> +       const uint32_t poly = 0xEDB88320;

>>

>>

>> Maybe avoid capital letters ?

>>

>> What about adding something like the following in xz_private.h instead:

>>

>> #define CRC32_POLY_LE 0xedb88320

> 

> The problem is that it's pulling in linux/crc32poly.h. To support old

> systems with a 32bit Open Firmware we boot wrapper we build the boot

> wrapper as a 32bit ELF even for a 64 bit kernel. The headers generated

> by Kbuild are only valid for the 64bit kernel so we have to avoid

> pulling them into the boot wrapper.


Yes I understood, hence my proposal to redefine CRC32_POLY_LE in 
xz_private.h instead of including linux/crc32poly.h

It avoids modifying the C code.

Christophe

> 

>> Christophe

>>

>>>          uint32_t i;

>>>          uint32_t j;

>>>

>>
Joel Stanley Sept. 21, 2018, 2:30 a.m. UTC | #5
On Wed, 19 Sep 2018 at 16:09, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>

> On Wed, 19 Sep 2018 at 01:08, Joel Stanley <joel@jms.id.au> wrote:

> >

> > This partially reverts faa16bc404d72a5 ("lib: Use existing define with

> > polynomial").

> >

> > The cleanup added a dependency on include/linux, which broke the PowerPC

> > boot wrapper/decompresser when KERNEL_XZ is enabled:

> >

> >   BOOTCC  arch/powerpc/boot/decompress.o

> >  In file included from arch/powerpc/boot/../../../lib/decompress_unxz.c:233,

> >                  from arch/powerpc/boot/decompress.c:42:

> >  arch/powerpc/boot/../../../lib/xz/xz_crc32.c:18:10: fatal error:

> >  linux/crc32poly.h: No such file or directory

> >   #include <linux/crc32poly.h>

> >            ^~~~~~~~~~~~~~~~~~~

> >

> > The powerpc decompressor is a hairy corner of the kernel. Even while building

> > a 64-bit kernel it needs to build a 32-bit binary and therefore avoid including

> > files from include/linux.

>

> I fixed the build error here:

> https://lkml.org/lkml/2018/8/29/179

>

> If you choose to remove any includes from /linux, then go ahead but

> please use original reported-by :)


Okay. I'll try Christophe's suggestion instead.

Cheers,

Joel
diff mbox series

Patch

diff --git a/lib/xz/xz_crc32.c b/lib/xz/xz_crc32.c
index 25a5d87e2e4c..34532d14fd4c 100644
--- a/lib/xz/xz_crc32.c
+++ b/lib/xz/xz_crc32.c
@@ -15,7 +15,6 @@ 
  * but they are bigger and use more memory for the lookup table.
  */
 
-#include <linux/crc32poly.h>
 #include "xz_private.h"
 
 /*
@@ -30,7 +29,7 @@  STATIC_RW_DATA uint32_t xz_crc32_table[256];
 
 XZ_EXTERN void xz_crc32_init(void)
 {
-	const uint32_t poly = CRC32_POLY_LE;
+	const uint32_t poly = 0xEDB88320;
 
 	uint32_t i;
 	uint32_t j;