From nobody Thu Dec 7 01:35:04 2023 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4SlxfY4Jkgz53Lrt for ; Thu, 7 Dec 2023 01:35:17 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-ed1-x534.google.com (mail-ed1-x534.google.com [IPv6:2a00:1450:4864:20::534]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1D4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4SlxfY2SMrz4XKd for ; Thu, 7 Dec 2023 01:35:17 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-ed1-x534.google.com with SMTP id 4fb4d7f45d1cf-54cde11d0f4so549410a12.2 for ; Wed, 06 Dec 2023 17:35:17 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20230601.gappssmtp.com; s=20230601; t=1701912916; x=1702517716; darn=freebsd.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=fW4fp4PKnRufGZpdvbTs9AmQs/xS5639PEJF6E8CfOo=; b=EddC7IVGScJ2Ar+DMrx34MQIni6s2iaqzdjoM4/IFY3ZwbK/LjBwz8lwpM1KkQdZFJ y+VFpzvjv0BgzIvGlkwu/wWYgoR/1ma/1F+Xwecnzd3+qAs2ElGSSZd+OQ1qbMHzUP3k C2FLBACUQz45lsY/mFCKFg5N+AMEvTiXc4qLVb8YGCXA7ieMAv+9zstsgaRpHad5siRe EuBRbtY0B9lW+chFnzDbjjwMPNU6uGEbBa5tfjGqZ4ks4Gp1SCWUGmoT/O7irwRJxcat aBHCKlBwgTDsnZcUkomM7UXPpjcuQ3QZABVjiysIKHcjmvgLgCoDVhuua/c3Xx07njTf IyGg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701912916; x=1702517716; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=fW4fp4PKnRufGZpdvbTs9AmQs/xS5639PEJF6E8CfOo=; b=NG2bhsOlTezC515eCJbcQzN5iVYxIS2N9zsLHWZ83UYSUv0ktkxoNA7gego69movQ5 K1gdlpw3BKlN+tZ7WCt4aKrdUhfC7PDWDK6heF4QWM4DZQEUMU3x6RGAijliRb/IAUhu /PDEIuHQfrZg1qLrTAOAGM79+zP+gqSgvZF46uvpFHBBABCQjqISI/Os/VPU1j2ARyI6 dUECPq7M5VZDKX8Bg3HjGgbhGRNjW14FOaRryn9da7IC/ei4I/QPRKoj0GpwQVqjvk3k MXo9w3Ph1ljwGD8GzES3253WJVM3kvJHMsptqng0Dg8e3TV35pFQp0WXXQsc3Ol49Zea RnuQ== X-Gm-Message-State: AOJu0YxmtGOpIAGK543MWh7XfFeL9GX6bHyFp2LnsQ7HyhOWuPRUpa1v PIcLJlK27hs4QsqK3dVrFrEmtw3CVUx2MhgKGM68Nw== X-Google-Smtp-Source: AGHT+IFv0v8q8M/5qopI9zkLvMB7BqKxKoLoXeK4jMNr4vJK9WIy2ot46Xx7pg1nFKH483/7ZQX+OJH1mlzV4kMDB/8= X-Received: by 2002:a50:96c4:0:b0:54c:75eb:f8b1 with SMTP id z4-20020a5096c4000000b0054c75ebf8b1mr1219071eda.7.1701912915699; Wed, 06 Dec 2023 17:35:15 -0800 (PST) List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 References: <202302251737.31PHb2R8072300@gitrepo.freebsd.org> In-Reply-To: From: Warner Losh Date: Wed, 6 Dec 2023 18:35:04 -0700 Message-ID: Subject: Re: git: 773c13c686e4 - main - kldxref: skip .pkgsave files To: John Baldwin Cc: Warner Losh , src-committers , "" , "" Content-Type: multipart/alternative; boundary="000000000000c77e88060be17a5b" X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:15169, ipnet:2a00:1450::/32, country:US] X-Spamd-Bar: ---- X-Rspamd-Queue-Id: 4SlxfY2SMrz4XKd --000000000000c77e88060be17a5b Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, Dec 6, 2023, 2:53 PM John Baldwin wrote: > On 12/6/23 1:41 PM, Warner Losh wrote: > > Hey John, > > > > On Wed, Dec 6, 2023 at 2:13=E2=80=AFPM John Baldwin w= rote: > > > >> On 12/6/23 1:02 PM, Warner Losh wrote: > >>> On Wed, Dec 6, 2023, 1:04 PM John Baldwin 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=3D773c13c686e4b6ae9dbbc150b342b82= c3f47d73a > >>>>> > >>>>> commit 773c13c686e4b6ae9dbbc150b342b82c3f47d73a > >>>>> Author: Mina Gali=C4=87 > >>>>> AuthorDate: 2023-02-25 17:31:58 +0000 > >>>>> Commit: Warner Losh > >>>>> CommitDate: 2023-02-25 17:35:43 +0000 > >>>>> > >>>>> kldxref: skip .pkgsave files > >>>>> > >>>>> This should help people transitioning from traditional setup= s > 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/66= 1 > >>>>> 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 libel= f, > >>>> but I think here you want to probably revert this and just add pkgsa= ve > >>>> 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 b= y > >> 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 filenam= e > > 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 ca= se > 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 betwe= en > the native kldxref in the VM (before this change) and my cross-arch versi= on > 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. Warner --=20 > John Baldwin > > --000000000000c77e88060be17a5b Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


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=E2=80=AFPM 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=3D773c13c686e4b6ae9dbbc150b342b82= c3f47d73a
>>>>>
>>>>> commit 773c13c686e4b6ae9dbbc150b342b82c3f47d73a
>>>>> Author:=C2=A0 =C2=A0 =C2=A0Mina Gali=C4=87 <freebsd@= igalic.co>
>>>>> AuthorDate: 2023-02-25 17:31:58 +0000
>>>>> Commit:=C2=A0 =C2=A0 =C2=A0Warner Losh <imp@FreeBSD= .org>
>>>>> CommitDate: 2023-02-25 17:35:43 +0000
>>>>>
>>>>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 kldxref: skip .pkgsave file= s
>>>>>
>>>>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 This should help people tra= nsitioning from traditional setups to
>>>> pkgbase
>>>>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 experience a lot less frict= ion.
>>>>>
>>>>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 We do this by skipping all = files containing two dots.
>>>>>
>>>>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 Reviewed by: imp
>>>>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 Pull Request: https://github.com/freebsd/freebsd-src/pull/661
>>>>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 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 to= ol 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 ind= ex the
>> kernel
>>> too?
>>
>> We do index the kernel as well, yes.=C2=A0 However, we could proba= bly get by
>> with "kernel" and ends in ".ko" as a valid set= of files.=C2=A0 This would also
>> avoid bogusly warning about linker.hints not being a valid ELF fil= e on
>> re-runs if you use -v.
>>
>
> Yea, that sounds good. I'll code it up and add you to the review.<= br> >
> But why does it matter for these? Firmware is usually loaded by filena= me
> and need not be elf... or are these wrapped in elf sections...
>
> I haven't noticed it breaking my linuxkpi wifi driver that have au= toloaded
> firmware...

Hmm, afaik firmwares are loaded by "module name" where a firmware= .ko contains
one or more of the firmware modules.=C2=A0 We happen today to generally onl= y
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 certainl= y
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 tomorr= ow 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 take= s too long.

Warner=C2=A0=

--
John Baldwin

--000000000000c77e88060be17a5b--