Bug#1029843: brcmfmac: requested firmware filename inconsistent with linux-firmware.git on non-devicetree systems

[Cyril Brulebois]

Hi,

Diederik de Haas  (2023-05-04):
> Assuming the '55' stands for max 55 chars, that could be an issue?

That's not how format strings work. :)

That happily overflows:

$ printf "%-10s and the rest of the line\n" kibi
kibi   and the rest of the line

$ printf "%-10s and the rest of the line\n" "Diederik de Haas"
Diederik de Haas and the rest of the line

> It turns out that spaces (and or backslashes to escape them) seems to
> be an issue for the Debian scripts used to make the Debian
> firmware-nonfree packages too. See https://bugs.debian.org/1035505 for
> details. Once that is fixed, I can submit my MR to add those missing
> symlinks.

Oh, d-i isn't the only one expecting non-crazy paths! :)

> Seems doable. I'm not going to spend time trying to make that though.
> I know virtually nothing about d-i/hw-detect internals, so it seems
> very unwise for me to even try it.
> Given the (specific) subject at hand, you won't be surprised that
> there's also a motivational issue ;-)

I was merely writing it out so that it could be sanity-checked, I wasn't
suggesting you would be the one to implement that. (Instead, I was
implicitly volunteering myself…)


Cheers,
-- 
Cyril Brulebois (k...@debian.org)
D-I release manager -- Release team member -- Freelance Consultant


signature.asc
Description: PGP signature

Bug#1029843: brcmfmac: requested firmware filename inconsistent with linux-firmware.git on non-devicetree systems

[Diederik de Haas]

Control: block -1 1035505

On Thursday, 4 May 2023 01:41:12 CEST Cyril Brulebois wrote:
> Diederik de Haas  (2023-05-04):
> > And that makes it a firmware-brcm80211 issue and now it all does make
> > sense as it now all does tie together :-)
> 
> Great, that's what it looked like to me but I was afraid I could have
> misunderstood the situation and I didn't want to digress in a wrong
> direction…
> 
> > So while upstream does define the link from what I earlier called
> > 'weird' name to brcmfmac43455-sdio.raspberrypi,4-model-b.txt, the Debian
> > firmware-brcm80211 package does not define that link and is therefor
> > missing.
> 
> Adding the links will at least make those paths shows up in the
> Contents-firmware indices generated by debian-cd. Those contain 3
> “columns”: path, package, component (the current format string is
> "%-55s %s %s\n").

Assuming the '55' stands for max 55 chars, that could be an issue?

> Even if hw-detect didn't misbehave because of spaces in filenames (i.e.

It turns out that spaces (and or backslashes to escape them) seems to be an 
issue for the Debian scripts used to make the Debian firmware-nonfree packages 
too. See https://bugs.debian.org/1035505 for details.
Once that is fixed, I can submit my MR to add those missing symlinks.

> For Bookworm, given we expect the firmware package to be adjusted to
> include those symlinks, what if hw-detect had some little cheatsheet,

Seems doable. I'm not going to spend time trying to make that though.
I know virtually nothing about d-i/hw-detect internals, so it seems very 
unwise for me to even try it.
Given the (specific) subject at hand, you won't be surprised that there's also 
a motivational issue ;-)

Cheers, 
 Diederik

signature.asc
Description: This is a digitally signed message part.

Bug#1029843: brcmfmac: requested firmware filename inconsistent with linux-firmware.git on non-devicetree systems

[Cyril Brulebois]

Hi,

Diederik de Haas  (2023-05-04):
> On Thursday, 4 May 2023 00:21:25 CEST James Addison wrote:
> > I added a note[1] on the rpi4-uefi.dev GitHub repository, and from
> > one of your fellow contributors' responses, it seems that in fact
> > the filename-with-spaces format _is_ referenced from
> > linux-firmware.git, in a 'WHENCE' file that is used to create
> > symlinks (I hadn't been aware of that previously).
> 
> And that makes it a firmware-brcm80211 issue and now it all does make
> sense as it now all does tie together :-)

Great, that's what it looked like to me but I was afraid I could have
misunderstood the situation and I didn't want to digress in a wrong
direction…

> So while upstream does define the link from what I earlier called
> 'weird' name to brcmfmac43455-sdio.raspberrypi,4-model-b.txt, the Debian
> firmware-brcm80211 package does not define that link and is therefor
> missing.

Adding the links will at least make those paths shows up in the
Contents-firmware indices generated by debian-cd. Those contain 3
“columns”: path, package, component (the current format string is
"%-55s %s %s\n").

Even if hw-detect didn't misbehave because of spaces in filenames (i.e.
the way the list of missing files is constructed, which I've been
assuming is one of the main issue here, but I didn't dive into it
because that's not getting rewritten for Bookworm anyway)… it wouldn't
be able to perform the required lookup, given how it extracts those
“columns” from those indices.

(Looking at regular Contents file again, I see there's nothing done
specifically — like some kind of escaping — for paths like
“/etc/testssl/DST Root CA X3.txt” in the testssl.sh package; so I guess
it wouldn't be crazy to just change nothing in debian-cd, and have
hw-detect deal with spotting package and component at the end of the
line, reading the whole beginning as a path; of course, if someone goes
as far as including spaces at the end of some firmware filename, we'll
have to just change the format… Cc-ing debian-cd@ for information.)

Anyway, you know how stupidly pragmatic I can be…

For Bookworm, given we expect the firmware package to be adjusted to
include those symlinks, what if hw-detect had some little cheatsheet,
turning the space-powered filenames into their normal counterparts right
off the bat (while going through the dmesg lines), so that hw-detect
would:
 - not split those into smaller parts, therefore building a list of
   paths correctly;
 - additionally succeed in performing the firmware file to firmware
   package (and component) lookup;
 - deploy the relevant firmware package;

… while the linux kernel would ultimately request the space-powered
filenames again, this time finding the symlinks that the updated package
would ship, being happy, no longer complaining, and hw-detect would give
a green light and carry on.

Embedding a little symlinks → files mapping for one single package,
that's really not expected to change too much this close to the release…
looks like (1) a very ugly kludge, granted; but also (2) a very small
price to pay to get immediate support for that particular way of booting
Pi devices, without waiting on any major, post-release rewrite.


If that looks fine to you, feel free to clone this against hw-detect
with the symlinks → files mapping. Cc-ing debian-boot@ for information
as well.


Cheers,
-- 
Cyril Brulebois (k...@debian.org)
D-I release manager -- Release team member -- Freelance Consultant


signature.asc
Description: PGP signature

Bug#1029843: brcmfmac: requested firmware filename inconsistent with linux-firmware.git on non-devicetree systems

[Diederik de Haas]

On Thursday, 4 May 2023 01:03:08 CEST Diederik de Haas wrote:
> Control: affects -1 -raspi-firmware

That it affects a RPi device does NOT mean it affects the raspi-firmware 
package, 
so I removed that field (hopefully)

signature.asc
Description: This is a digitally signed message part.

Bug#1029843: brcmfmac: requested firmware filename inconsistent with linux-firmware.git on non-devicetree systems

[Diederik de Haas]

Control: reassign -1 firmware-brcm80211
Control: affects -1 -raspi-firmware
Control: retitle -1 Missing symlinks for RPi 4 (to 
brcmfmac43455-sdio.raspberrypi,4-model-b.txt)

On Thursday, 4 May 2023 00:21:25 CEST James Addison wrote:
> I added a note[1] on the rpi4-uefi.dev GitHub repository, and from one of
> your fellow contributors' responses, it seems that in fact the
> filename-with-spaces format _is_ referenced from linux-firmware.git, in a
> 'WHENCE' file that is used to create symlinks (I hadn't been aware of that
> previously).

And that makes it a firmware-brcm80211 issue and now it all does make sense
as it now all does tie together :-)
So while upstream does define the link from what I earlier called 'weird' name
to brcmfmac43455-sdio.raspberrypi,4-model-b.txt, the Debian firmware-brcm80211
package does not define that link and is therefor missing.

> As a result: I feel that maybe this bugreport is not valid.

It is actually a valid bug report, although the discussion got rather messy
and I've therefor dropped the whole list of other bug reports.
I'm not even sure which bug report is exactly about what, but my current
best guess is that this is the right bug for this issue.
(I was about to question why you'd reassigned it to src:linux)

> [1] - https://github.com/pftf/RPi4/issues/76#issuecomment-1533295773



signature.asc
Description: This is a digitally signed message part.

Bug#1029843: brcmfmac: requested firmware filename inconsistent with linux-firmware.git on non-devicetree systems

[James Addison]

Package: src:linux
Followup-For: Bug #1029843
X-Debbugs-Cc: p...@akeo.ie, k...@debian.org, didi.deb...@cknow.org, 
debian-b...@lists.debian.org, 1029...@bugs.debian.org, 1035...@bugs.debian.org, 
989...@bugs.debian.org, debian-...@lists.debian.org
Control: retitle -1 brcmfmac: requested firmware filename inconsistent with 
linux-firmware.git on non-devicetree systems

Thanks, Pete.

I added a note[1] on the rpi4-uefi.dev GitHub repository, and from one of your
fellow contributors' responses, it seems that in fact the filename-with-spaces
format _is_ referenced from linux-firmware.git, in a 'WHENCE' file that is
used to create symlinks (I hadn't been aware of that previously).

As a result: I feel that maybe this bugreport is not valid.


I suppose that some of the confusion stemmed from the fact that a single binary
of a kernel module in combination with a single physical hardware device probed
different firmware filenames at runtime depending on the context (ACPI vs
Devicetree, in this case).

(it's code, so yep, I get that it's technically _possible_ for that to happen,
and perhaps it's useful to workaround limitations of existing standards, but
it's not clear to me whether that's necessary here)

> Note that, in case you think there may be something that we can improve 
> in the SMBIOS data reported by the UEFI firmware (which is currently 
> generated from the source code at [1], with the full output from a 
> Raspberry Pi 4, from UEFI Shell's smbiosview command at [2]) we can look 
> into updating the UEFI firmware to alter the data we output.

Thank you - I'll take a look at those to learn more.

[1] - https://github.com/pftf/RPi4/issues/76#issuecomment-1533295773

Bug#1029843: brcmfmac: requested firmware filename inconsistent with linux-firmware.git on non-devicetree systems

[James Addison]

Control: unmerge 1029843 1030519
Control: reassign 1029843 src:linux
Control: retitle 1029843 brcmfmac: requested firmware filename
inconsistent with linux-firmware.git on non-devicetree systems
Control: affects 1029843 firmware-brcm80211 raspi-firmware

Dear Maintainer,

This bugreport relates to the brcmfmac kernel module and the firmware
filename that it probes for during load.

It looks like this may have been a cause of some problems reported in
bugs #989593, #1029843 (this bug) and #1035392.

The last of those three bugs is an installation-report of mine, and as
far as I can tell the problem is that when the affected system (an
RPi) was configured without devicetree support in its UEFI bootloader,
the kernel module was unable to determine a precise filename and used
some fallback logic to determine one approximately here:

https://sources.debian.org/src/linux/6.1.25-1/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c/?hl=487#L487

Please let me know if I can provide any further details to help track this down.

Thank you,
James