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.mc...@intel.com> 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@lists.freedesktop.org > > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > _______________________________________________ > > > > Intel-gfx mailing list > > > > Intel-gfx@lists.freedesktop.org > > > > 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@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
