I took a closer look and it seems Cyan, Glados, Strago, and Reef and all of their variants obtain board version by reading the EC.

We're probably not going to change the firmware to report board version correctly in SMBIOS for products that have already launched. Fortunately, mosys can read the version from the EC as well, so I went ahead and added support for that:
https://chromium-review.googlesource.com/408816
https://chromium-review.googlesource.com/408817

Note: This is a good time to suggest formatting rules. Right now I have it simply return the raw number, but we might want to add "revN" to match the format used by the non-x86 platforms.

I agree doing that would safe cyan, strago, and glados.
But at least for reef, which has implemented board_id correctly (or, at least it supposed to be) by https://review.coreboot.org/#/c/15561/, the SMBIOS is still wrong.

Can we fix the SMBIOS on reef?

What exactly do you want to have spit out in smbios? Just the number? I suggest that because there's talks about using board_id for other purposes. Nothing has happened just yet along those lines, but it keeps surfacing.

> What exactly do you want to have spit out in smbios? Just the number?

At least for Samus, I think Duncan has recommended to replace numbers with meaningful words like EVT, DVT1, DVT2, PVT (to align with what we call them) inside firmware ( https://chromium.googlesource.com/chromiumos/third_party/coreboot/+log/firmware-samus-6300.B/src/mainboard/google/samus/board_version.c ) , so it's easier for people to check and enforce rules like "if the board ID says it's PVT/MP then WP must enabled".

But since most ARM boards are doing rev%d, and if you prefer to have only number for it, I think we (factory) are also fine with that.

samus is a fixed device while we are doing reference boards that are supposed to support potential zergs with minor changes, etc. It becomes complicated for a straight up EVT, DVT, PVT, etc. I'll just push rev%d.

SMBIOS patches pushed:
https://review.coreboot.org/17290 
https://review.coreboot.org/17291

Both good points w.r.t. meaningful names. For those reasons IMO it's best to have the firmware report the raw value and do the translation in a userspace tool (mosys, gooftool, etc) that can be easily patched and deployed as revision numbers are sorted out.

sounds good to me. thanks!

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/coreboot/+/308ee2479a964dd25570f9874dedf3b554cd8a8e

commit 308ee2479a964dd25570f9874dedf3b554cd8a8e
Author: Aaron Durbin <adurbin@chromium.org>
Date: Tue Nov 08 16:04:04 2016

UPSTREAM: vendorcode/google: add common smbios mainboard version support

Provide an option to deliver the mainboard smbios version in the
form of 'rev%d' based on the board_id() value.

BUG= chromium:663243 
BRANCH=None
TEST=None

Signed-off-by: Aaron Durbin <adurbin@chromium.org>
Reviewed-on: https://review.coreboot.org/17290
Reviewed-by: Furquan Shaikh <furquan@google.com>
Tested-by: build bot (Jenkins)

Change-Id: If0a34935f570612da6e0c950fd7e8f0d92b6984f
Reviewed-on: https://chromium-review.googlesource.com/410074
Commit-Ready: Furquan Shaikh <furquan@chromium.org>
Tested-by: Furquan Shaikh <furquan@chromium.org>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>

[modify] https://crrev.com/308ee2479a964dd25570f9874dedf3b554cd8a8e/src/vendorcode/google/Makefile.inc
[add] https://crrev.com/308ee2479a964dd25570f9874dedf3b554cd8a8e/src/vendorcode/google/smbios.c
[modify] https://crrev.com/308ee2479a964dd25570f9874dedf3b554cd8a8e/src/vendorcode/google/Kconfig

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/coreboot/+/c45546f7cf85f71568600b61bec95cc9d7bdae93

commit c45546f7cf85f71568600b61bec95cc9d7bdae93
Author: Aaron Durbin <adurbin@chromium.org>
Date: Tue Nov 08 16:06:10 2016

UPSTREAM: mainboard/google/reef: use common google smbios mainboard version

BUG= chromium:663243 
BRANCH=None
TEST=None

Signed-off-by: Aaron Durbin <adurbin@chromium.org>
Reviewed-on: https://review.coreboot.org/17291
Reviewed-by: Furquan Shaikh <furquan@google.com>
Tested-by: build bot (Jenkins)
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>

Change-Id: Ic78a6aac11a8e842911245c59e8ced7ed2c4e27a
Reviewed-on: https://chromium-review.googlesource.com/410075
Commit-Ready: Furquan Shaikh <furquan@chromium.org>
Tested-by: Furquan Shaikh <furquan@chromium.org>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>

[modify] https://crrev.com/c45546f7cf85f71568600b61bec95cc9d7bdae93/src/mainboard/google/reef/Kconfig

> Both good points w.r.t. meaningful names.

What do you think if we support both?
I think:
 - What put in SMBIOS should be simple version number since we won't want to change firmware source simply for build phase changes.
 - "mosys platform version" can keep doing rev%d since it's "version"
 - We can add a new command like "mosys platform phase" that prints meaningful names, converted from version string, for example EVT, DVT, PVT, MP.

I don't think phase is going to be legit for a lot of devices that are zerg since we're trying keep boards in lockstep across devices in that category. This bug covers pushing out some more info so we can keep a little bit more info. 
https://code.google.com/p/chrome-os-partner/issues/detail?id=59897 I'm not sure where I'm going to stuff that information but it'll be a number in smbios again. 

Re #13: Maybe just populate the manufacturer in the type 1 table with OEM1, OEM2, etc. (assuming we need to obfuscate OEM).

How will Zerg deal with board revisions?

Assume reference board has rev 1 = EVT, rev 2 = DVT, rev 3 = PVT,
will first zerg board do their EVT with rev 3?

If that's the case, we can still remap according to customization ID so that
 if customization id = 0 (reference board), 1 = EVT, 2 = DVT
 if customization id = 1 (zerg 1), 3 = EVT... etc

Anyway phase in mosys is not a high priority job, although I still hear people asking similar request "to enforce WP and security stuff in PVT".

I think it might be rev 3, but I don't want to map names to those things because names get wrong really quickly when people insert a new build where one wasn't expecting it.

The reference board is being taken as is which had its own notion of phases. The products have their own phases. So which one should be used? Right now we're not really using the products' notion of phases because the same board is being used with a single firmware. For none-zerg boards they'd have their own phase sequence.

We don't have the ability to encode both currently. I'd like to have such a notion in the future, but we don't have that ability now while trying to juggle multiple projects on the same board.

As for the 'enforce WP and security stuff in PVT', where is that request coming from where one needs to enforce it? Is this concept something we can put into HWID? i.e. when to do X? Is it purely binary for many of these things? Yes or no?

> names get wrong really quickly 
 I agree, so firmware is definitely not the best place for it.

> where is that request coming from where one needs to enforce it? 
 peng team, everytime when there was doubt about if a device has been write-protected properly.

> Is this concept something we can put into HWID?
 We already did, since http://crosbug.com/p/28988 .
 The factory software will check "board phase" and enforce WP if phase >= PVT. But those were all software. The problem is, how do you make sure partner is using right software? That's why there were discussions to  put it in hardware.

 In fact, if you look at the design doc in  issue 28988 , Jon has previously requested board ID to be the indicator of phase.

> Is it purely binary for many of these things? Yes or no?

 I believe it's a "final enforcement" than binary.
 Most of the things can be done early in many builds, but since write-protection is the root of trust for all our security model, people were concerned about if partner is doing that correctly, especially that WP is usually not enabled in early builds (also PVT_DOGFOOD).

 It is very possible that partner may forget to switch to right software when they're going to start main build. That's why phase enforcement has been always a request.

But well, I think we can try to solve this by factory software first, for example marking all unknown boards versions as PVT, so it's safer even partner forgot to update software.

Conclusion:

  Please keep using rev%d for both firmware and "mosys platform version".
  Factory can implement rev 2 phase mapping by ourselves.

Thanks!

I think we can agree phase enforcement at a hardware level demands a hardware stuffing change. How does that fix 'using the right software' question? We can add a pin going forward which indicates 'final enforcement' required that can be exported, but it's something we need to get into the designs early. Is that something you'd like to propose? It's not terrible, but it's also kinda like the WP screw. Except in this case it's an indicator the WP screw and lock down better have been done!

There are things in EC that need to happen too. Which we don't have an automatic way of indicating that out. That should be explored as well.

Wow, that sounds great!

Yes I don't need detail phase names, just a bit of "phase enforcement" so we can read it somewhere would be enough.

> How does that fix 'using the right software' question?
 Currently in our finalization step, we allow few customization like "skip WP", "allow few steps to be waived", and these are all disallowed if phase is PVT (or, if sees "phase enforcement" bit). Every release of software has same code for finalization.
 The only difference is the "phase" definition in software.
 If the "phase enforcement" bit is a hardware thing, then even if partner installed wrong version of software, the finalization code can detect that by reading hardware bit.
 For example, if the phase in software indicates "EVT" but the hardware enforcement flag is set, then finalization should abort and alert that "the software version is wrong".

> it's something we need to get into the designs early.
 I'm not familiar with the process of adding pin into board designs.
 Is there some example or anyone can help proposing that?

Hungte, can you start a doc describing this bit and what you need it for? Then we can pass it off to hardware group after review to incorporate into boards going forward. That will help provide context to others, etc. I'll read/review it in my morning and we can go from there. Sound good?

Ok I can try. Thanks!

Re#21: https://docs.google.com/document/d/1NzDfm8GjBPV735JmJvtBiizLklsGdtQI5OWmDyZ9QiY , thanks!

If there's anything else to do here, let Martin know.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/coreboot/+/ccd71f48506ea9309dfcac893f6f9003487ca84b

commit ccd71f48506ea9309dfcac893f6f9003487ca84b
Author: Aaron Durbin <adurbin@chromium.org>
Date: Thu May 11 17:31:12 2017

UPSTREAM: string.h: only guard snprintf() with __ROMCC__

There's no need to keep the snprintf() declaration hidden
for early stages. romcc is the entity that has issues. Therefore,
be explicit about when to guard snprintf().

BUG= chromium:663243 
BRANCH=None
TEST=None

Signed-off-by: Aaron Durbin <adurbin@chromium.org>
Reviewed-on: https://review.coreboot.org/17289
Reviewed-by: Furquan Shaikh <furquan@google.com>
Tested-by: build bot (Jenkins)
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>

Change-Id: Ib4d0879e52c3f73c6ca61ab75f672f0003fca71f
Reviewed-on: https://chromium-review.googlesource.com/410073
Commit-Ready: Furquan Shaikh <furquan@chromium.org>
Tested-by: Furquan Shaikh <furquan@chromium.org>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/502670
Commit-Queue: Philip Chen <philipchen@chromium.org>
Tested-by: Philip Chen <philipchen@chromium.org>

[modify] https://crrev.com/ccd71f48506ea9309dfcac893f6f9003487ca84b/src/include/string.h