Re: git: 773c13c686e4 - main - kldxref: skip .pkgsave files

From: John Baldwin <jhb_at_FreeBSD.org>
Date: Thu, 18 Jan 2024 21:35:43 UTC
On 12/6/23 5:35 PM, Warner Losh wrote:
> On Wed, Dec 6, 2023, 2:53 PM John Baldwin <jhb@freebsd.org> wrote:
> 
>> On 12/6/23 1:41 PM, Warner Losh wrote:
>>> Hey John,
>>>
>>> On Wed, Dec 6, 2023 at 2:13 PM John Baldwin <jhb@freebsd.org> wrote:
>>>
>>>> On 12/6/23 1:02 PM, Warner Losh wrote:
>>>>> On Wed, Dec 6, 2023, 1:04 PM John Baldwin <jhb@freebsd.org> wrote:
>>>>>
>>>>>> On 2/25/23 9:37 AM, Warner Losh wrote:
>>>>>>> The branch main has been updated by imp:
>>>>>>>
>>>>>>> URL:
>>>>>>
>>>>
>> https://cgit.FreeBSD.org/src/commit/?id=773c13c686e4b6ae9dbbc150b342b82c3f47d73a
>>>>>>>
>>>>>>> commit 773c13c686e4b6ae9dbbc150b342b82c3f47d73a
>>>>>>> Author:     Mina Galić <freebsd@igalic.co>
>>>>>>> AuthorDate: 2023-02-25 17:31:58 +0000
>>>>>>> Commit:     Warner Losh <imp@FreeBSD.org>
>>>>>>> CommitDate: 2023-02-25 17:35:43 +0000
>>>>>>>
>>>>>>>         kldxref: skip .pkgsave files
>>>>>>>
>>>>>>>         This should help people transitioning from traditional setups
>> to
>>>>>> pkgbase
>>>>>>>         experience a lot less friction.
>>>>>>>
>>>>>>>         We do this by skipping all files containing two dots.
>>>>>>>
>>>>>>>         Reviewed by: imp
>>>>>>>         Pull Request: https://github.com/freebsd/freebsd-src/pull/661
>>>>>>>         Differential Revision: https://reviews.freebsd.org/D27959
>>>>>>
>>>>>> This restriction is too broad and omits all of the modern wifi
>> firmware
>>>>>> klds from linker.hints, e.g.
>>>>>>
>>>>>> /boot/kernel/iwlwifi-3160-17.ucode.ko
>>>>>> /boot/kernel/iwlwifi-3168-29.ucode.ko
>>>>>> /boot/kernel/iwlwifi-7260-17.ucode.ko
>>>>>> /boot/kernel/iwlwifi-7265-17.ucode.ko
>>>>>> /boot/kernel/iwlwifi-7265D-29.ucode.ko
>>>>>> /boot/kernel/iwlwifi-8000C-36.ucode.ko
>>>>>> /boot/kernel/iwlwifi-8265-36.ucode.ko
>>>>>> /boot/kernel/iwlwifi-9000-pu-b0-jf-b0-46.ucode.ko
>>>>>> /boot/kernel/iwlwifi-9260-th-b0-jf-b0-46.ucode.ko
>>>>>> /boot/kernel/iwlwifi-Qu-b0-hr-b0-77.ucode.ko
>>>>>> /boot/kernel/iwlwifi-Qu-b0-jf-b0-77.ucode.ko
>>>>>> /boot/kernel/iwlwifi-Qu-c0-hr-b0-77.ucode.ko
>>>>>> /boot/kernel/iwlwifi-Qu-c0-jf-b0-77.ucode.ko
>>>>>> /boot/kernel/iwlwifi-QuZ-a0-hr-b0-77.ucode.ko
>>>>>> /boot/kernel/iwlwifi-QuZ-a0-jf-b0-77.ucode.ko
>>>>>> /boot/kernel/iwlwifi-cc-a0-77.ucode.ko
>>>>>> /boot/kernel/iwlwifi-so-a0-gf-a0-83.ucode.ko
>>>>>> /boot/kernel/iwlwifi-so-a0-gf-a0.pnvm.ko
>>>>>> /boot/kernel/iwlwifi-so-a0-gf4-a0-83.ucode.ko
>>>>>> /boot/kernel/iwlwifi-so-a0-gf4-a0.pnvm.ko
>>>>>> /boot/kernel/iwlwifi-so-a0-hr-b0-81.ucode.ko
>>>>>> /boot/kernel/iwlwifi-so-a0-jf-b0-77.ucode.ko
>>>>>> /boot/kernel/iwlwifi-ty-a0-gf-a0-83.ucode.ko
>>>>>> /boot/kernel/iwlwifi-ty-a0-gf-a0.pnvm.ko
>>>>>> /boot/kernel/rtw8723d_fw.bin.ko
>>>>>> /boot/kernel/rtw8821c_fw.bin.ko
>>>>>> /boot/kernel/rtw8822b_fw.bin.ko
>>>>>> /boot/kernel/rtw8822c_fw.bin.ko
>>>>>> /boot/kernel/rtw8822c_wow_fw.bin.ko
>>>>>>
>>>>>> all match this pattern and are skipped.
>>>>>>
>>>>>> I'm busy rewriting a bunch of kldxref to be a cross tool using libelf,
>>>>>> but I think here you want to probably revert this and just add pkgsave
>>>>>> to the list of "known bad" suffixes.
>>>>>>
>>>>>
>>>>> Sure. Any reason to not just require .ko? Or do we have to index the
>>>> kernel
>>>>> too?
>>>>
>>>> We do index the kernel as well, yes.  However, we could probably get by
>>>> with "kernel" and ends in ".ko" as a valid set of files.  This would
>> also
>>>> avoid bogusly warning about linker.hints not being a valid ELF file on
>>>> re-runs if you use -v.
>>>>
>>>
>>> Yea, that sounds good. I'll code it up and add you to the review.
>>>
>>> But why does it matter for these? Firmware is usually loaded by filename
>>> and need not be elf... or are these wrapped in elf sections...
>>>
>>> I haven't noticed it breaking my linuxkpi wifi driver that have
>> autoloaded
>>> firmware...
>>
>> Hmm, afaik firmwares are loaded by "module name" where a firmware .ko
>> contains
>> one or more of the firmware modules.  We happen today to generally only
>> store one module in a single .ko (and with the same name), and in that case
>> kern_linker.c may fallback to just trying to load "foo".ko if it doesn't
>> find
>> an entry in linker.hints, but if that is why it is working that is
>> certainly
>> by happy accident.
>>
>> I only found this by comparing klxref output btw on a stale i386 VM between
>> the native kldxref in the VM (before this change) and my cross-arch version
>> of kldxref.
>>
> 
> Ok. That all makes sense. I'll update my working tree tomorrow with the
> revert and the replacement. Since it "works" today, I'll push the revert
> and the fix at the same time unless the review takes too long.

Ping, do you still have this fix in your pending tree?  I noticed it is
still there when doing MFC's of the libelf kldxref changes today.

-- 
John Baldwin