| Message ID | 20220718072144.2699487-1-airlied@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | docs: driver-api: firmware: add driver firmware guidelines. | expand |
On 18.07.22 09:21, Dave Airlie wrote: > From: Dave Airlie <airlied@redhat.com> > > A recent snafu where Intel ignored upstream feedback on a firmware > change, led to a late rc6 fix being required. In order to avoid this > in the future we should document some expectations around > linux-firmware. > > I was originally going to write this for drm, but it seems quite generic > advice. > > I'm cc'ing this quite widely to reach subsystems which use fw a lot. Thx for this, I kinda put "add a few words about firmware into Documentation/process/handling-regressions.rst" on my todo list already, but having a separate document is likely better. Took a quick look, here are a few suggestions for your consideration. > [...] > diff --git a/Documentation/driver-api/firmware/firmware-usage-guidelines.rst b/Documentation/driver-api/firmware/firmware-usage-guidelines.rst > new file mode 100644 > index 000000000000..34d2412e78c6 > --- /dev/null > +++ b/Documentation/driver-api/firmware/firmware-usage-guidelines.rst > @@ -0,0 +1,34 @@ > +=================== > +Firmware Guidelines > +=================== > + > +Drivers that use firmware from linux-firmware should attempt to follow > +the rules in this guide. How about spelling out the main aspect first clearly before going into the details about its consequence? Maybe something along these lines: ``` Users switching to a newer kernel should *not* have to install newer firmware files to keep their hardware working. At the same time updated firmware files must not cause any regressions for users of older kernel releases. Drivers that use such firmware (like that in linux-firmware) should thus follow these rules: ``` > +* Firmware should be versioned with at least a major/minor version it > + is suggested that the firmware files in linux-firmware be named with > + some device specific name, and just the major version. The > + major/minor/patch versions should be stored in a header in the > + firmware file for the driver to detect any non-ABI fixes/issues. The > + firmware files in linux-firmware should be overwritten with the newest > + compatible major version. Newer major version firmware should remain > + compatible with all kernels that load that major number. > + > +* Users should *not* have to install newer firmware to use existing > + hardware when they install a newer kernel. This will need changes if you pick up the suggestion above. > If the hardware isn't > + enabled by default or under development, Wondering if it might be better to drop the "or under development", as the "enabled by default" is the main part afaics. Maybe something like "If support for the hardware is normally inactive (e.g. has to be enabled manually by a kernel parameter)" would be better anyway. > this can be ignored, until > + the first kernel release that enables that hardware. This means no > + major version bumps without the kernel retaining backwards > + compatibility for the older major versions. Minor version bumps > + should not introduce new features that newer kernels depend on > + non-optionally. > + > +* If a security fix needs lockstep firmware and kernel fixes in order to > + be successful, then all supported major versions in the linux-firmware > + repo This made me wonder: what exactly are "all supported major versions" in this context? Do you mean something like "all major versions in the linux-firmware required by currently supported stable/longterm kernel series"? Then it might be wise to write that. > should be updated with the security fix, and the kernel patches > + should detect if the firmware is new enough to declare if the security > + issue is fixed. All communications around security fixes should point > + at both the firmware and kernel fixes. If a security fix requires > + deprecating old major versions, then this should only be done as a > + last option, and be stated clearly in all communications. > + HTH, Ciao, Thorsten
On Mon, Jul 18, 2022 at 05:21:44PM +1000, Dave Airlie wrote: > From: Dave Airlie <airlied@redhat.com> > > A recent snafu where Intel ignored upstream feedback on a firmware > change, led to a late rc6 fix being required. In order to avoid this > in the future we should document some expectations around > linux-firmware. > > I was originally going to write this for drm, but it seems quite generic > advice. > > I'm cc'ing this quite widely to reach subsystems which use fw a lot. > > Signed-off-by: Dave Airlie <airlied@redhat.com> > --- > Documentation/driver-api/firmware/core.rst | 1 + > .../firmware/firmware-usage-guidelines.rst | 34 +++++++++++++++++++ > 2 files changed, 35 insertions(+) > create mode 100644 Documentation/driver-api/firmware/firmware-usage-guidelines.rst > > diff --git a/Documentation/driver-api/firmware/core.rst b/Documentation/driver-api/firmware/core.rst > index 1d1688cbc078..803cd574bbd7 100644 > --- a/Documentation/driver-api/firmware/core.rst > +++ b/Documentation/driver-api/firmware/core.rst > @@ -13,4 +13,5 @@ documents these features. > direct-fs-lookup > fallback-mechanisms > lookup-order > + firmware-usage-guidelines > > diff --git a/Documentation/driver-api/firmware/firmware-usage-guidelines.rst b/Documentation/driver-api/firmware/firmware-usage-guidelines.rst > new file mode 100644 > index 000000000000..34d2412e78c6 > --- /dev/null > +++ b/Documentation/driver-api/firmware/firmware-usage-guidelines.rst > @@ -0,0 +1,34 @@ > +=================== > +Firmware Guidelines > +=================== > + > +Drivers that use firmware from linux-firmware should attempt to follow > +the rules in this guide. > + > +* Firmware should be versioned with at least a major/minor version. It > + is suggested that the firmware files in linux-firmware be named with > + some device specific name, and just the major version. The > + major/minor/patch versions should be stored in a header in the > + firmware file for the driver to detect any non-ABI fixes/issues. The > + firmware files in linux-firmware should be overwritten with the newest > + compatible major version. Newer major version firmware should remain > + compatible with all kernels that load that major number. would symbolic links be acceptable in the linux-firmware.git where the <fmw>_<major>.bin is a sym link to <fwm>_<major>.<minor>.bin or having the <fwm>_<major>.bin really to be the overwritten every minor update? > + > +* Users should *not* have to install newer firmware to use existing > + hardware when they install a newer kernel. If the hardware isn't > + enabled by default or under development, this can be ignored, until > + the first kernel release that enables that hardware. This means no > + major version bumps without the kernel retaining backwards > + compatibility for the older major versions. Minor version bumps > + should not introduce new features that newer kernels depend on > + non-optionally. > + > +* If a security fix needs lockstep firmware and kernel fixes in order to > + be successful, then all supported major versions in the linux-firmware > + repo should be updated with the security fix, and the kernel patches > + should detect if the firmware is new enough to declare if the security > + issue is fixed. All communications around security fixes should point > + at both the firmware and kernel fixes. If a security fix requires > + deprecating old major versions, then this should only be done as a > + last option, and be stated clearly in all communications. Everything makes sense to me. Thanks for writing this down. Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > + > -- > 2.36.1 > > > > -- > _______________________________________________ > Dri-devel mailing list > Dri-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/dri-devel
On Mon, Jul 18, 2022 at 05:21:44PM +1000, Dave Airlie wrote: > From: Dave Airlie <airlied@redhat.com> > > A recent snafu where Intel ignored upstream feedback on a firmware > change, led to a late rc6 fix being required. In order to avoid this > in the future we should document some expectations around > linux-firmware. > > I was originally going to write this for drm, but it seems quite generic > advice. > > I'm cc'ing this quite widely to reach subsystems which use fw a lot. > > Signed-off-by: Dave Airlie <airlied@redhat.com> Document well deserved to be written, thanks for making this happen. Modulo all the silly spelling / bike-shedding issues folks might find, in case you care to re-spin for a v2: Acked-by: Luis Chamberlain <mcgrof@kernel.org> Now let's think about the impact of two corner cases which *do* happen and so this poses security implications on enablement: 1) Devices which end up with a security issue which a vendor considers obsolete, and the only way to fix something is firmware. We're security-out-of-luck. For this I've previously sucessfully have put effort into organizations to open source the firmware. We were successful more than once: * https://github.com/qca/open-ath9k-htc-firmware * https://github.com/qca/ath6kl-firmware When these efforts fall short we have a slew of reverse engineering efforts which fortunately also have been sucessfull. 2) Vendor goes belly up Both implicate the need to help persuade early on a strategy for open source firmware, and I don't want to hear anyone tell me it is not possible. When that fails we can either reverse engineer and worst case, I am not sure if we have a process for annotations or should. Perhaps a kconfig symbol which drivers with buggy firmware can depend on, and only if you enable that kconfig symbol would these drivers be available to be enabled? Are we aware of such device drivers? They must exist... Luis
On Mon, 18 Jul 2022 11:33:11 +0200 Thorsten Leemhuis wrote: > > If the hardware isn't > > + enabled by default or under development, > > Wondering if it might be better to drop the "or under development", as > the "enabled by default" is the main part afaics. Maybe something like > "If support for the hardware is normally inactive (e.g. has to be > enabled manually by a kernel parameter)" would be better anyway. It's a tricky one, I'd say something like you can break the FW ABI "until HW becomes available for public consumption" or such. I'm guessing what we're after is letting people break the compatibility in early stages of the product development cycles. Pre-silicon and bring up, but not after there are products on the market?
> > +* Firmware should be versioned with at least a major/minor version. It > > + is suggested that the firmware files in linux-firmware be named with > > + some device specific name, and just the major version. The > > + major/minor/patch versions should be stored in a header in the > > + firmware file for the driver to detect any non-ABI fixes/issues. The > > + firmware files in linux-firmware should be overwritten with the newest > > + compatible major version. Newer major version firmware should remain > > + compatible with all kernels that load that major number. > > would symbolic links be acceptable in the linux-firmware.git where > the <fmw>_<major>.bin is a sym link to <fwm>_<major>.<minor>.bin > > or having the <fwm>_<major>.bin really to be the overwritten every minor > update? I don't think providing multiple minor versions of fw in linux-firmware is that interesting. Like if the major is the same, surely you always want the newer ones. As long as the ABI doesn't break. Otherwise we are just wasting disk space with fws nobody will be using. Dave.
On Tue, 19 Jul 2022 at 08:04, Jakub Kicinski <kuba@kernel.org> wrote: > > On Mon, 18 Jul 2022 11:33:11 +0200 Thorsten Leemhuis wrote: > > > If the hardware isn't > > > + enabled by default or under development, > > > > Wondering if it might be better to drop the "or under development", as > > the "enabled by default" is the main part afaics. Maybe something like > > "If support for the hardware is normally inactive (e.g. has to be > > enabled manually by a kernel parameter)" would be better anyway. > > It's a tricky one, I'd say something like you can break the FW ABI > "until HW becomes available for public consumption" or such. > I'm guessing what we're after is letting people break the compatibility > in early stages of the product development cycles. Pre-silicon and > bring up, but not after there are products on the market? I'll stick with enabled by default I think, "public consumption" invites efforts to describe corners of the cloud or other places where hw has shipped but is not technically "public", Dave.
On 7/18/22 19:29, Dave Airlie wrote: >>> +* Firmware should be versioned with at least a major/minor version. It >>> + is suggested that the firmware files in linux-firmware be named with >>> + some device specific name, and just the major version. The >>> + major/minor/patch versions should be stored in a header in the >>> + firmware file for the driver to detect any non-ABI fixes/issues. The >>> + firmware files in linux-firmware should be overwritten with the newest >>> + compatible major version. Newer major version firmware should remain >>> + compatible with all kernels that load that major number. >> >> would symbolic links be acceptable in the linux-firmware.git where >> the <fmw>_<major>.bin is a sym link to <fwm>_<major>.<minor>.bin >> >> or having the <fwm>_<major>.bin really to be the overwritten every minor >> update? > > I don't think providing multiple minor versions of fw in > linux-firmware is that interesting. > Like if the major is the same, surely you always want the newer ones. > As long as the > ABI doesn't break. Otherwise we are just wasting disk space with fws > nobody will be using. It was my understanding that once a firmware file is in linux-firmware it's there forever. There are tons of existing symlinks to point to the latest version, but the previous versions are not removed/overwritten. see random examples: ls -lR /lib/firmware | grep t4fw ls -lR /lib/firmware | grep fw_release
diff --git a/Documentation/driver-api/firmware/core.rst b/Documentation/driver-api/firmware/core.rst index 1d1688cbc078..803cd574bbd7 100644 --- a/Documentation/driver-api/firmware/core.rst +++ b/Documentation/driver-api/firmware/core.rst @@ -13,4 +13,5 @@ documents these features. direct-fs-lookup fallback-mechanisms lookup-order + firmware-usage-guidelines diff --git a/Documentation/driver-api/firmware/firmware-usage-guidelines.rst b/Documentation/driver-api/firmware/firmware-usage-guidelines.rst new file mode 100644 index 000000000000..34d2412e78c6 --- /dev/null +++ b/Documentation/driver-api/firmware/firmware-usage-guidelines.rst @@ -0,0 +1,34 @@ +=================== +Firmware Guidelines +=================== + +Drivers that use firmware from linux-firmware should attempt to follow +the rules in this guide. + +* Firmware should be versioned with at least a major/minor version. It + is suggested that the firmware files in linux-firmware be named with + some device specific name, and just the major version. The + major/minor/patch versions should be stored in a header in the + firmware file for the driver to detect any non-ABI fixes/issues. The + firmware files in linux-firmware should be overwritten with the newest + compatible major version. Newer major version firmware should remain + compatible with all kernels that load that major number. + +* Users should *not* have to install newer firmware to use existing + hardware when they install a newer kernel. If the hardware isn't + enabled by default or under development, this can be ignored, until + the first kernel release that enables that hardware. This means no + major version bumps without the kernel retaining backwards + compatibility for the older major versions. Minor version bumps + should not introduce new features that newer kernels depend on + non-optionally. + +* If a security fix needs lockstep firmware and kernel fixes in order to + be successful, then all supported major versions in the linux-firmware + repo should be updated with the security fix, and the kernel patches + should detect if the firmware is new enough to declare if the security + issue is fixed. All communications around security fixes should point + at both the firmware and kernel fixes. If a security fix requires + deprecating old major versions, then this should only be done as a + last option, and be stated clearly in all communications. +