[PATCH] docs: driver-api: firmware: add driver firmware guidelines.

[PATCH] docs: driver-api: firmware: add driver firmware guidelines.

[Dave Airlie]

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.
+
+* 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.
+
-- 
2.36.1

Re: [PATCH] docs: driver-api: firmware: add driver firmware guidelines.

[Thorsten Leemhuis]

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

Re: [PATCH] docs: driver-api: firmware: add driver firmware guidelines.

[Rodrigo Vivi]

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

Re: [PATCH] docs: driver-api: firmware: add driver firmware guidelines.

[Luis Chamberlain]

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

Re: [PATCH] docs: driver-api: firmware: add driver firmware guidelines.

[Jakub Kicinski]

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?

Re: [PATCH] docs: driver-api: firmware: add driver firmware guidelines.

[Dave Airlie]

> > +* 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.

Re: [PATCH] docs: driver-api: firmware: add driver firmware guidelines.

[Dave Airlie]

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.

Re: [PATCH] docs: driver-api: firmware: add driver firmware guidelines.

[Pierre-Louis Bossart]

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

[PATCH] docs: driver-api: firmware: add driver firmware guidelines.

[Dave Airlie]

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.

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.
+
+* 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.
+
-- 
2.36.1