[PATCH] habanalabs: select CRC32

[PATCH] habanalabs: select CRC32

[Vegard Nossum]

Fix the following build/link error by adding a dependency on the CRC32
routines:

  ld: drivers/misc/habanalabs/common/firmware_if.o: in function `hl_fw_dynamic_request_descriptor':
  firmware_if.c:(.text.unlikely+0xc89): undefined reference to `crc32_le'

Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
---
 drivers/misc/habanalabs/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/misc/habanalabs/Kconfig b/drivers/misc/habanalabs/Kconfig
index 293d79811372c..0068ff9a34abb 100644
--- a/drivers/misc/habanalabs/Kconfig
+++ b/drivers/misc/habanalabs/Kconfig
@@ -8,6 +8,7 @@ config HABANA_AI
 	depends on PCI && HAS_IOMEM
 	select GENERIC_ALLOCATOR
 	select HWMON
+	select CRC32
 	help
 	  Enables PCIe card driver for Habana's AI Processors (AIP) that are
 	  designed to accelerate Deep Learning inference and training workloads.
-- 
2.23.0.718.g5ad94255a8

Re: [PATCH] habanalabs: select CRC32

[Arnd Bergmann]

On Mon, Oct 11, 2021 at 5:14 PM Vegard Nossum <vegard.nossum@oracle.com> wrote:
>
> Fix the following build/link error by adding a dependency on the CRC32
> routines:
>
>   ld: drivers/misc/habanalabs/common/firmware_if.o: in function `hl_fw_dynamic_request_descriptor':
>   firmware_if.c:(.text.unlikely+0xc89): undefined reference to `crc32_le'
>
> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>

Patch looks good to me, I wonder how I never caught that with my own randconfig
build testing. May I ask how you found it?

Fixes: 8a43c83fec12 ("habanalabs: load boot fit to device")
Acked-by: Arnd Bergmann <arnd@arndb.de>

Re: [PATCH] habanalabs: select CRC32

[Vegard Nossum]

On 10/11/21 5:33 PM, Arnd Bergmann wrote:
> On Mon, Oct 11, 2021 at 5:14 PM Vegard Nossum <vegard.nossum@oracle.com> wrote:
>>
>> Fix the following build/link error by adding a dependency on the CRC32
>> routines:
>>
>>   ld: drivers/misc/habanalabs/common/firmware_if.o: in function `hl_fw_dynamic_request_descriptor':
>>   firmware_if.c:(.text.unlikely+0xc89): undefined reference to `crc32_le'
>>
>> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
> 
> Patch looks good to me, I wonder how I never caught that with my own randconfig
> build testing. May I ask how you found it?
> 
> Fixes: 8a43c83fec12 ("habanalabs: load boot fit to device")
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> 

Thanks.

I'm working on a couple of new make targets, satconfig and
satrandconfig, which can generate .configs that respect a set of choices
specified in a .satconfig file (and, of course, the Kconfig files).

In this case, I was just doing satrandconfig builds and noticed that the
crc32_le ones popped up quite often so I just added CONFIG_CRC32=n to my
.satconfig, which means that satrandconfig will generate only configs
that have CRC32=n.

I also suspect that satrandconfig samples the configuration space MUCH
more uniformly than randconfig, in the sense that "make randconfig" will
tend to have CRC32 enabled just because there are so many drivers that
_do_ select it. (Think of it this way, every prompt for an option that
has "select CRC32" in it will be another 50% probability of actually
enabling the option.)

I just tried doing 'make randconfig' 100 times and the result was
CRC32=y every single time.

If I do the same for 'make satrandconfig', I get the following
distribution of values:

48 CRC32=y
10 CRC32=m
42 CRC32=n

That's probably still not really uniform, but quite a bit better, and as
I said I can still put CONFIG_CRC32=n in my .satconfig and get _only_
configs that respect this choice.

The really cool thing about sat{,rand}config IMHO is that you can fairly
easily generate random configs that you know will be bootable (because
you've forced it to only output configs that respect options that you
need to boot on your particular setup).

This project itself is about 11 years old, but I made a breakthrough
recently and I'm polishing the patches for submission now. I thought I
would try to fix the breakage that I could find first so it doesn't
appear as if the tool itself is broken... (that's not to say it doesn't
have any bugs, however!)


Vegard

Re: [PATCH] habanalabs: select CRC32

[Arnd Bergmann]

On Mon, Oct 11, 2021 at 6:29 PM Vegard Nossum <vegard.nossum@oracle.com> wrote:
> On 10/11/21 5:33 PM, Arnd Bergmann wrote:
> I'm working on a couple of new make targets, satconfig and
> satrandconfig, which can generate .configs that respect a set of choices
> specified in a .satconfig file (and, of course, the Kconfig files).
>
> In this case, I was just doing satrandconfig builds and noticed that the
> crc32_le ones popped up quite often so I just added CONFIG_CRC32=n to my
> .satconfig, which means that satrandconfig will generate only configs
> that have CRC32=n.

Ah nice! I had gone through some of those last year, but it was a rather
tedious manual process, and having some tool for it sound really
useful.

> I also suspect that satrandconfig samples the configuration space MUCH
> more uniformly than randconfig, in the sense that "make randconfig" will
> tend to have CRC32 enabled just because there are so many drivers that
> _do_ select it. (Think of it this way, every prompt for an option that
> has "select CRC32" in it will be another 50% probability of actually
> enabling the option.)

Yes, of course. I've seen other problems like this often happen with

CONFIG_I2C=m
CONFIG_LEDS_CLASS=m
CONFIG_IPV6=m
CONFIG_CRYPTO=m
CONFIG_PM_SLEEP=n

Not sure if you're already hitting those or if you need to add something
to your setup to make those more easy to find.

> I just tried doing 'make randconfig' 100 times and the result was>
> This project itself is about 11 years old, but I made a breakthrough
> recently and I'm polishing the patches for submission now. I thought I
> would try to fix the breakage that I could find first so it doesn't
> appear as if the tool itself is broken... (that's not to say it doesn't
> have any bugs, however!)

Sounds great.

       Arnd

Re: [PATCH] habanalabs: select CRC32

[Vegard Nossum]

On 10/11/21 9:37 PM, Arnd Bergmann wrote:
> On Mon, Oct 11, 2021 at 6:29 PM Vegard Nossum <vegard.nossum@oracle.com> wrote:
>> I also suspect that satrandconfig samples the configuration space MUCH
>> more uniformly than randconfig, in the sense that "make randconfig" will
>> tend to have CRC32 enabled just because there are so many drivers that
>> _do_ select it. (Think of it this way, every prompt for an option that
>> has "select CRC32" in it will be another 50% probability of actually
>> enabling the option.)
> 
> Yes, of course. I've seen other problems like this often happen with
> 
> CONFIG_I2C=m
> CONFIG_LEDS_CLASS=m
> CONFIG_IPV6=m
> CONFIG_CRYPTO=m
> CONFIG_PM_SLEEP=n
> 
> Not sure if you're already hitting those or if you need to add something
> to your setup to make those more easy to find.

The CRYPTO one I've definitely hit a few times (usually combined with
CRYPTO_HASH) and a failure to link crypto_ahash_* functions.

Otherwise it's mostly CRC32 and a couple of odd ones like this that I've
seen so far:

ERROR: modpost: "dell_privacy_has_mic_mute"
[drivers/platform/x86/dell/dell-laptop.ko] undefined!

  CC      drivers/scsi/aic7xxx/aic79xx_reg_print.o
gcc: error: drivers/scsi/aic7xxx/aic79xx_reg_print.c: No such file or
directory
gcc: fatal error: no input files
compilation terminated.
make[3]: *** [scripts/Makefile.build:271:
drivers/scsi/aic7xxx/aic79xx_reg_print.o] Error 1

I'll give your suggestions a try, I have about 9 patches queued up so
far so I think there are definitely more problems lurking around.

Thanks,


Vegard

Re: [PATCH] habanalabs: select CRC32

[Arnd Bergmann]

On Tue, Oct 12, 2021 at 7:54 AM Vegard Nossum <vegard.nossum@oracle.com> wrote:
> On 10/11/21 9:37 PM, Arnd Bergmann wrote:
> > On Mon, Oct 11, 2021 at 6:29 PM Vegard Nossum <vegard.nossum@oracle.com> wrote:
>
> Otherwise it's mostly CRC32 and a couple of odd ones like this that I've
> seen so far:
>
> ERROR: modpost: "dell_privacy_has_mic_mute"
> [drivers/platform/x86/dell/dell-laptop.ko] undefined!

I have a patch for this one that I should send out.

>   CC      drivers/scsi/aic7xxx/aic79xx_reg_print.o
> gcc: error: drivers/scsi/aic7xxx/aic79xx_reg_print.c: No such file or
> directory
> gcc: fatal error: no input files
> compilation terminated.
> make[3]: *** [scripts/Makefile.build:271:
> drivers/scsi/aic7xxx/aic79xx_reg_print.o] Error 1

I never saw this one, as I only build with CONFIG_COMPILE_TEST=y
and CONFIG_PREVENT_FIRMWARE_BUILD=y. I think it's generally
a good idea to force these two options, to avoid known problems
and long compile times, but the aic7xxx error might be something
worth fixing regardless.

          Arnd

Re: [PATCH] habanalabs: select CRC32

[Randy Dunlap]

On 10/11/21 11:44 PM, Arnd Bergmann wrote:
> On Tue, Oct 12, 2021 at 7:54 AM Vegard Nossum <vegard.nossum@oracle.com> wrote:
>> On 10/11/21 9:37 PM, Arnd Bergmann wrote:
>>> On Mon, Oct 11, 2021 at 6:29 PM Vegard Nossum <vegard.nossum@oracle.com> wrote:
>>
>> Otherwise it's mostly CRC32 and a couple of odd ones like this that I've
>> seen so far:
>>
>> ERROR: modpost: "dell_privacy_has_mic_mute"
>> [drivers/platform/x86/dell/dell-laptop.ko] undefined!
> 
> I have a patch for this one that I should send out.

I sent a patch for that. Hans de Goede replaced with:
[PATCH] platform/x86: dell: Make DELL_WMI_PRIVACY depend on DELL_WMI

<20211011132338.407571-1-hdegoede@redhat.com>

so no need to send your patch.

>>    CC      drivers/scsi/aic7xxx/aic79xx_reg_print.o
>> gcc: error: drivers/scsi/aic7xxx/aic79xx_reg_print.c: No such file or
>> directory
>> gcc: fatal error: no input files
>> compilation terminated.
>> make[3]: *** [scripts/Makefile.build:271:
>> drivers/scsi/aic7xxx/aic79xx_reg_print.o] Error 1
> 
> I never saw this one, as I only build with CONFIG_COMPILE_TEST=y
> and CONFIG_PREVENT_FIRMWARE_BUILD=y. I think it's generally
> a good idea to force these two options, to avoid known problems
> and long compile times, but the aic7xxx error might be something
> worth fixing regardless.

thanks.
-- 
~Randy

Re: [PATCH] habanalabs: select CRC32

[Arnd Bergmann]

On Tue, Oct 12, 2021 at 10:10 PM Randy Dunlap <rdunlap@infradead.org> wrote:
>
> On 10/11/21 11:44 PM, Arnd Bergmann wrote:
> > On Tue, Oct 12, 2021 at 7:54 AM Vegard Nossum <vegard.nossum@oracle.com> wrote:
> >> On 10/11/21 9:37 PM, Arnd Bergmann wrote:
> >>> On Mon, Oct 11, 2021 at 6:29 PM Vegard Nossum <vegard.nossum@oracle.com> wrote:
> >>
> >> Otherwise it's mostly CRC32 and a couple of odd ones like this that I've
> >> seen so far:
> >>
> >> ERROR: modpost: "dell_privacy_has_mic_mute"
> >> [drivers/platform/x86/dell/dell-laptop.ko] undefined!
> >
> > I have a patch for this one that I should send out.
>
> I sent a patch for that. Hans de Goede replaced with:
> [PATCH] platform/x86: dell: Make DELL_WMI_PRIVACY depend on DELL_WMI
>
> <20211011132338.407571-1-hdegoede@redhat.com>
>
> so no need to send your patch.

Unfortunately there is still a related problem that remains with
"DELL_WMI=m && DELL_SMBIOS=y", and another one with
"DELL_WMI=m && DELL_LAPTOP=y && DELL_WMI_PRIVACY=y".

      Arnd

Re: [PATCH] habanalabs: select CRC32

[Randy Dunlap]

On 10/12/21 1:42 PM, Arnd Bergmann wrote:
> On Tue, Oct 12, 2021 at 10:10 PM Randy Dunlap <rdunlap@infradead.org> wrote:
>>
>> On 10/11/21 11:44 PM, Arnd Bergmann wrote:
>>> On Tue, Oct 12, 2021 at 7:54 AM Vegard Nossum <vegard.nossum@oracle.com> wrote:
>>>> On 10/11/21 9:37 PM, Arnd Bergmann wrote:
>>>>> On Mon, Oct 11, 2021 at 6:29 PM Vegard Nossum <vegard.nossum@oracle.com> wrote:
>>>>
>>>> Otherwise it's mostly CRC32 and a couple of odd ones like this that I've
>>>> seen so far:
>>>>
>>>> ERROR: modpost: "dell_privacy_has_mic_mute"
>>>> [drivers/platform/x86/dell/dell-laptop.ko] undefined!
>>>
>>> I have a patch for this one that I should send out.
>>
>> I sent a patch for that. Hans de Goede replaced with:
>> [PATCH] platform/x86: dell: Make DELL_WMI_PRIVACY depend on DELL_WMI
>>
>> <20211011132338.407571-1-hdegoede@redhat.com>
>>
>> so no need to send your patch.
> 
> Unfortunately there is still a related problem that remains with
> "DELL_WMI=m && DELL_SMBIOS=y", and another one with
> "DELL_WMI=m && DELL_LAPTOP=y && DELL_WMI_PRIVACY=y".

OK.  That probably would have been fixed by my patch (just guessing here),
but Hans decided that it wasn't needed.


-- 
~Randy

Re: [PATCH] habanalabs: select CRC32

[Oded Gabbay]

On Mon, Oct 11, 2021 at 6:33 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Mon, Oct 11, 2021 at 5:14 PM Vegard Nossum <vegard.nossum@oracle.com> wrote:
> >
> > Fix the following build/link error by adding a dependency on the CRC32
> > routines:
> >
> >   ld: drivers/misc/habanalabs/common/firmware_if.o: in function `hl_fw_dynamic_request_descriptor':
> >   firmware_if.c:(.text.unlikely+0xc89): undefined reference to `crc32_le'
> >
> > Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
>
> Patch looks good to me, I wonder how I never caught that with my own randconfig
> build testing. May I ask how you found it?
>
> Fixes: 8a43c83fec12 ("habanalabs: load boot fit to device")
> Acked-by: Arnd Bergmann <arnd@arndb.de>

Thanks!
Applied to my -next tree.
Oded