Re: [PATCH] drm/i915/dmc: Accept symbolic link in firmware name

Imre Deak
2016-07-21T13:32:18Z

If you look back we did have both no-black listing (only required a
proper major version) and black listing before we ended up deciding
that we'll use white listing. The reasons for this were too many
obscure firmware issues, undocumented interoperability details between
firmware and the driver and as a result what we saw the only viable way
to validate driver functionality and deal with bug reports.

--Imre

On ke, 2016-07-20 at 01:39 +0300, Vivi, Rodrigo wrote:
> Marc kicked me to the other side of the fence. I'm now cheering for
> the
> symbolic links back again.
>
> We cannot block users to use some new firmware version if they
> like/want/need.
>
> Also, also as Chris highlighted the hardcoded version doesn't help
> the
> bisect but make it unlikely. I don't believe any users out there will
> save and install all firmware versions when bisecting.
>
> This is not a new problem and we can use the linux libraries as an
> example here. like libmeh.so.2.1, libmeh.so.2 link and libmeh.so
> link.
>
> We don't hardcode all userspace libraries in the userspace side for
> the graphics stack and we do not validate all possible combinations
> of libdrm, mesa, ddx, libva, etc... Why should we need this with
> firmware?
>
> Also I'd like to take this libmeh.so example here and go even further
> with the symbolic link fixing https://bugs.freedesktop.org/show_bug.c
> gi?id=96800 in the right way:
>
> - providing a firmware_platform.bin that is a symbolic link to the
> lastest firmware_platform_<abi version>.bin (our soname) that is a
> symbolic link to firmware_platform_<abi version>_<release
> number>.bin
>
> Our driver should also search for the firmware_platform.bin and have
> a
> check for the minimal version like userspace libraries does.
>
> In any case our drm_debug should have the information of the version
> loaded and version expected, in case users find issues that leads us
> to
> blame the firmware versions.
>
> Thoughts?
>
>
> On Tue, 2016-07-19 at 14:58 -0700, Herbert, Marc wrote:
> > Versioning dependencies isn't exactly a new problem outside the
> > kernel,
> > so please allow me to share my experience below. Thanks to Rodrigo
> > for
> > glancing over this email and preventing me from writing anything
> > too
> > stupid or that was said already.
> >
> >
> > >
> > > >
> > > > The firmware should be side-ways compatible for
> > > > everything with the same minor version (thus resolvable from
> > > > the
> > > > same
> > > > symlink), right?
> > > From the same major version I guess it should, but the reason
> > > things
> > > don't work that way is why we introduced version white listing.
> > In an ideal world you look only at the major version of your
> > dependencies
> > since they mark ABI changes. This is what the firmware symbolic
> > links
> > do.
> >
> > Back to the real world, some minor versions are so bad that you
> > really don't want to inflict them on your users. So you go and
> > blacklist at
> > the hardest level: in the source code itself. Fine.
> > However keep in mind blacklisting should stay exceptional since
> > leaking
> > system configuration into source code means taking away from users
> > control
> > they normally have. You need a pretty strong reason for this like
> > some very
> > obscure bugs that can't be Googled easily or some "less than alpha"
> > version.
> > Please keep in mind it's a hack/workaround: blacklisting shouldn't
> > never become a normal policy.
> >
> > On the other hand, never WHITE-list since it blocks users from
> > *UPGRADING*
> > the dependency.
> >
> > If you're tempted to white-list the most current version then just
> > blacklist
> > all versions older than today and job done. White-listing is making
> > a
> > very bold
> > and most likely wrong statement about the future: that no future
> > revision
> > of the dependency will ever be a pure bugfix and fully compatible
> > with the
> > current revision.
> >
> > Please don't try to abstract away the independent versioning of the
> > firmware and make it look like firmware and kernel are developed,
> > versioned,
> > released and validated on the same cycles and by the same team.
> > Granted: it's important to publish which versions
> > were validated together. However most software projects don't embed
> > their validation results in their source code as a default policy,
> > there are better ways to document things like this.
> >
> > >
> > > >
> > > > No. You want to be changing exactly one variable, which means
> > > > leaving
> > > > the firmware constant.
> > Yes! Constant at least for a range large enough for bisections to
> > be
> > practical (ABI changes happen too).
> >
> > >
> > > Hm, not sure. When looking for a working snapshot you also want
> > > to
> > > consider bugs introduced by the firmware itself. This is in a way
> > > the
> > > exact reason why we want stricter control on the firmware version
> > > and
> > > introduced a white list. This also means that loading a firmware
> > > version other than what the driver allows (at a given commit)
> > > won't
> > > work anyway.
> > Please trust and respect your users. If someone is technical and
> > motivated
> > enough to bisect the kernel and/or the firmware then please don't
> > get
> > in her way and trust that she will: 1. try the latest releases
> > first,
> > and 2. more generally have some clue about what she's doing. Or at
> > least learn quickly enough.
> >
> > Again feel free to blacklist the known broken past but don't block
> > the
> > unknown future, thanks!
> >
> > Marc