Re: [RFC] GuC firmware versioning change

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Oct 18, 2018 at 11:12:21AM -0700, Rodrigo Vivi wrote:
> On Thu, Oct 18, 2018 at 10:32:37AM -0700, Jeff McGee wrote:
> > On Thu, Oct 18, 2018 at 11:57:06AM +0200, Daniel Vetter wrote:
> > > On Fri, Oct 12, 2018 at 11:45 PM Jeff McGee <jeff.mcgee@xxxxxxxxx> wrote:
> > > >
> > > > On Fri, Oct 12, 2018 at 02:33:26PM -0700, Jeff McGee wrote:
> > > > > On Fri, Oct 12, 2018 at 01:51:46PM -0700, Rodrigo Vivi wrote:
> > > > > > On Fri, Oct 12, 2018 at 01:24:30PM -0700, Jeff McGee wrote:
> > > > > > > The GuC firmware team is proposing a change to the firmware versioning scheme.
> > > > > > > The goal is to more accurately track the firmware interface to help users
> > > > > > > manage dependencies on that interface. The proposed scheme is based on
> > > > > > > semver.org with some additions to handle branching.
> > > > > > >
> > > > > > > The proposed version number would have 4 fields: BASE.MAJOR.MINOR.PATCH.
> > > > > > > Contrast this with the 2 fields in the current version number: MAJOR.MINOR.
> > > > > > > Side note, the current firmware encodes a BRANCH and CLIENT number as well, but
> > > > > > > they have not been needed by i915. So a firmware released with the proposed
> > > > > > > scheme would be named <platform>_guc_ver<base>_<major>_<minor>_<patch>.bin
> > > > > > > (ex: skl_guc_ver1_5_4_7.bin) instead of the current
> > > > > > > <platform>_guc_ver<major>_<minor>.bin (ex: skl_guc_ver9_33.bin).
> > > > > > >
> > > > > > > The BASE number is an ID that is used to identify a set of releases in which
> > > > > > > the MAJOR.MINOR.PATCH semantics are consistent. In other words, two releases
> > > > > > > from the same BASE can be compared via their MAJOR.MINOR.PATCH to infer their
> > > > > > > relationship as described below. Two releases from a different BASE cannot be
> > > > > > > reliably compared. The BASE number facilitates arbitrary branching which can
> > > > > > > create duplicate and/or disconnected MAJOR.MINOR.PATCH versions. This type of
> > > > > > > branching is expected to be rare, and so BASE will rarely change. When a new
> > > > > > > BASE is created, the MAJOR.MINOR.PATCH reset to starting values.
> > > > > >
> > > > > > Could you please clarify a bit what BASE means?
> > > > > > What would be a different BASE?
> > > > > >
> > > > >
> > > > > The BASE number supports general branching that would cause version number
> > > > > conflicts. Branching for firmware releases is not desirable, but it is a
> > > > > practical reality. Therefore the versioning scheme must accomodate it. Let's
> > > > > say that a high-priority request is made to put specific updates on an old
> > > > > release that said customer is locked on. Those updates could include any sort
> > > > > of change including interface change. Then we have a sequence like below:
> > > > >
> > > > > v1.1.0.0   v1.1.0.1   v1.1.0.2   v1.1.0.3   v1.1.1.0
> > > > > ----O----------O----------O----------O----------O
> > > > >                            \
> > > > >                             \
> > > > >                              \
> > > > >                               O----------O
> > > > >                           v2.1.1.0   v2.1.1.1
> > > > >
> > > > > You can see that if we don't have a BASE number that changes from 1 to 2, then
> > > > > we end up with duplicated v1.1.0 along the different branches which are not the
> > > > > same. As I wrote, this should be a very rare scenario, but it can happen. Maybe
> > > > > upstream will always be supplied with releases from the "main" BASE, and you
> > > > > can ignore this field, but it needs to be there for other firmware
> > > > > distributions.
> > > 
> > > The way this is usually solved in semver is to not prepend a BASE, but
> > > postfix a branch-specific version, while keeping the mainline version
> > > unchanged. So
> > > 
> > > v2.1.1.0 becomes v1.1.0.1-branch1-0
> > > v2.1.1.1 becomes v1.1.0.1-branch1-1
> 
> +1. This would be much cleaner.
> 
> 
> > > 
> > > With the rule that branches are explicitly unsorted (that's denoted by
> > > the - - around them, instead o using dots), so not comparable. Bonus
> > > points if you name the branch points by the customers (could be the
> > > product, or internal customer group or whatever) to make these names
> > > slightly more meaningful.
> 
> +1 for meaningful names.
> 
> > > 
> > I don't see any mention of branch handling on semver.org.
> 
> Also I don't see an extra field for base on semver.org.
> So Daniel is showing how it is usually solved on many Linux distros
> and packages without creating this complexity.
> 
> > It discusses using
> > hyphen for pre-release versions and plus for metadata. Any other references
> > you can share would be appreciated.
> > 
> > We can certainly use an approach like this, where the branch ID (changed from
> > BASE ID) is appended via the hyphen only if necessary. We would probably stick
> > with a simple numerical branch ID rather than a string,
> 
> so branch1, branch2... at least it gets clear for everyone
> that this is the branch and not the mainline, while mainline keeps
> the clean semver.
> 
> > so that the full
> > version encoded in firmware is compact. We have 64 bits currently allocated
> > for version info in the CSS. We could define an enumeration in the interface
> > to name each ID more verbosely.
> 
> save one int for branch number. if 0 it is mainline and filename doesn't
> need to have any string, otherwise -branch<int>.
> 
> > 
> > I'm still not clear on how the major.minor.patch should change in this model
> > when the first and subsequent releases from a branch are made. Your example
> > seems to indicate that major.minor.patch are locked, and each release for
> > the branch gets one incremented version at the end. I would prefer that the
> > major.minor.patch continue to evolve on the branch per normal convention, but
> > then the branch ID would indicate that there is no gauranteed relationship to
> > any major.minor.patch on another branch. So in below sequence the "1" branch
> > resets to v1.0.0 for first release, then next release is v1.1.0 if for example
> > a backwards compatible interface change is made, and so on. Does that seem
> > reasonable?
> > 
> 
> If I was one of the users of branched versioning I'd like to know how
> it compares with the mainline.
> 
> So if I recieve a v1.0.0.1 I would imediatelly assume this was derivated
> from 1.0.0 instead of 1.0.2.
> 

So are you saying that when branching from mainline v1.0.2, the first release
should use v1.0.2-branch1? But what would the subsequent versions use? If I
lock the v1.0.2 portion to preserve branch-off point, I will need a 5th field
just to number the releases from this branch, e.g. v1.0.2-branch1-0,
v1.0.2-branch1-1, etc. I guess that is what Daniel suggested. But if the branch
is long-lived, I've now lost visibility into what sort of changes (interface or
internal) are going on. I guess that's the trade-off.

> >  v1.0.0     v1.0.1     v1.0.2     v1.0.3     v1.1.0
> > ----O----------O----------O----------O----------O
> >                            \
> >                             \
> >                              \
> >                               O----------O
> >                           v1.0.0-1   v1.1.0-1
> 
> For the repensentation I thing that internally you can save only number
> one, but when building final file name for distribution we should print
> v1.0.0-branch1 ....  v1.1.0-branch1
> 
> or even better with meaningful strings instead of branch
> as Daniel suggested.
> 
> maybe an external mapping table from branch numbers to customer strings?
> 

Agreed. The convention for naming the firmware file from the 4 basic numeric
fields is completely up to the OS team. So 3.4.1-3, 3.4.1-branch3, or
3.4.1-customerX (where customerX maps to branch id 3 through some table that
driver knows about) are all valid options. I think the mapping table should be
incorporated into the public interface spec in some fashion.
-Jeff

> Thanks,
> Rodrigo.
> 
> > 
> > -Jeff
> > 
> > > Prefixing a BASE, with a dot, is very much not how it's done and very
> > > confusing. There's one thing debian does, it's prepending an EPOCH,
> > > which is used for e.g. C++ packages when the compiler abi changes. I
> > > think reading up on debian's rules would be good inspiration for this
> > > problem:
> > > 
> > > https://serverfault.com/questions/604541/debian-packages-version-convention
> > > 
> > > Cheers, Daniel
> > > 
> > > > > -Jeff
> > > > >
> > > >
> > > > Sorry, I misrepresented how to the numbers would change in the above example.
> > > > The change of BASE from 1 to 2 would reset MAJOR.MINOR.PATCH. So the sequence
> > > > should be v1.1.0.2 -> v2.1.0.0 -> v2.1.0.1 and so on.
> > > >
> > > > If we don't have BASE, the pure semver.org sequence would be:
> > > >
> > > >   v1.0.0     v1.0.1     v1.0.2     v1.0.3     v1.1.0
> > > > ----O----------O----------O----------O----------O
> > > >                            \
> > > >                             \
> > > >                              \
> > > >                               O----------O
> > > >                             v1.1.0     v1.1.1
> > > >
> > > > And so you see the duplication of version number.
> > > > - Jeff
> > > >
> > > > > > >
> > > > > > > The MAJOR number conforms to the major in semver.org. It increments on a
> > > > > > > backwards incompatible change of the interface. It resets to 1 on a change of
> > > > > > > BASE. The MAJOR number basically works the same between the current and
> > > > > > > proposed versioning schemes.
> > > > > > >
> > > > > > > The MINOR number conforms to the minor in semver.org. It increments on a
> > > > > > > backwards compatible change of the interface (new interfaces that are optional
> > > > > > > to use). It will also increment on substantial new internal functionality that
> > > > > > > doesn't affect the interface but should be called out to the user. It resets to
> > > > > > > 0 on a change of MAJOR. The MINOR number in the current versioning scheme
> > > > > > > increments on any backwards compatible change. The proposed versioning scheme
> > > > > > > breaks this into the MINOR number just described and the PATCH number below.
> > > > > > >
> > > > > > > The PATCH number conforms to the patch in semver.org. It increments on a
> > > > > > > backwards compatible internal change, usually a bug fix. It resets to 0 on a
> > > > > > > change of MINOR.
> > > > > >
> > > > > > I like the idea of MAJOR.MINOR.PATCH following semver.org.
> > > > > >
> > > > > > I think if we remove the BASE out of picture and just use semver clear,
> > > > > > but maybe it is just because I didn't quite understand BASE.
> > > > > >
> > > > > > >
> > > > > > > The MAJOR.MINOR collectively define the interface version. Because the MINOR
> > > > > > > may also increment on a substantial internal change, it doesn't always mark an
> > > > > > > interface change, e.g. 4.5 and 4.6 may have identical interfaces. But the
> > > > > > > determination of interface compatibility is unchanged, e.g. 4.6 is always
> > > > > > > backwards compatible with 4.5.
> > > > > > >
> > > > > > > Each MAJOR.MINOR may continue to receive internal fixes along a branch even
> > > > > > > after the main branch for that BASE has moved on to another MAJOR.MINOR.
> > > > > > > Releases from these fix-only branches increment only the PATCH number on that
> > > > > > > MAJOR.MINOR, and therefore remain semantically consistent with the main branch.
> > > > > > > No change of BASE is therefore needed. Consider an example:
> > > > > > >
> > > > > > > v1.1.0.0   v1.1.0.1   v1.1.0.2   v1.1.1.0   v1.1.1.1
> > > > > > > ----O----------O----------O----------O----------O    <-- main adopts v1.1.1.x
> > > > > > >                            \
> > > > > > >                             \
> > > > > > >                              \
> > > > > > >                               O----------O     <-- fixes for interface v1.1.0.x
> > > > > > >                           v1.1.0.3   v1.1.0.4
> > > > > >
> > > > > > This approach is cool and more or less how Mesa handles their releases,
> > > > > > except by the fact that their Major is the year and minor is the month.
> > > > > >
> > > > > > However, on the firmware side I have a concern because we are so far trying
> > > > > > to make sure that we have 1-1 relationship on kernel-firmware version.
> > > > > >
> > > > > > But based on this view and what Anusha told me yesterday it seems
> > > > > > that GuC is getting constant releases. With the constant patches we will
> > > > > > soon explode linux-firmware.git repository size.
> > > > > >
> > > > > > But this maybe is something to be solved on linux-firmware side and we make
> > > > > > sure that we clean up and remove firmware that were never released in any
> > > > > > official Linux kernel. Anusha or Antonio, thoughts?
> > > > > >
> > > > > > Thanks,
> > > > > > Rodrigo.
> > > > > >
> > > > >
> > > > > I expect that i915 will still require a single version of firmware per platform
> > > > > as it does today. This change should not impact that other than to require i915
> > > > > to check 4 numbers instead of 2.
> > > > >
> > > > > It is true that firmware releases are made *internally* (from the firmware
> > > > > team to the operating system teams) at a frequent rate. It is up to each OS
> > > > > team to decide their own cadence for integrating and distributing those
> > > > > firmware based on their unique situation. So the Linux team may filter these
> > > > > releases and update the upstream much less frequently.
> > > > >
> > > > > > >
> > > > > > > There is no need to change the BASE because the branching happened from the
> > > > > > > last fix (v1.1.0.2) on the main branch prior to the change of interface
> > > > > > > (v1.1.1.0). As long as only fixes are applied to v1.1.0.x, there is no risk of
> > > > > > > version number clash. All of these release versions remain semantically
> > > > > > > connected with one small caveat. If this set of release versions came
> > > > > > > sequentially along a single branch, one could infer that the exact fixes in
> > > > > > > v1.1.0.4 were inherited by v1.1.1.0. With this "hidden" branching, this may
> > > > > > > not be true as in this example. One would need to review the v1.1.1.0 release
> > > > > > > notes to check.
> > > > > > >
> > > > > > > Please provide any feedback on the proposed change.
> > > > > > >
> > > > > > > Thanks,
> > > > > > > Jeff
> > > > > _______________________________________________
> > > > > Intel-gfx mailing list
> > > > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > 
> > > 
> > > 
> > > -- 
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux