New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 724527 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

factory: cr50: provision board-id

Project Member Reported by hungte@chromium.org, May 19 2017

Issue description

Randall has made a new "board-id" design for Cr50 and we have to provision that ID in finalization (I believe we should do it as a new gooftool command, and let "gooftool finalize" run that sub command).

See go/cr50-boardid-lock for more details.
 
Cc: philipchen@chromium.org
Cc: marcochen@chromium.org
The necessary changes for board id have landed.
The most important one is:
https://chromium-review.googlesource.com/#/c/525096/
Have anyone started working on the factory flow to support board id?
Will 525096 work without stoping trunksd?
We need a way to get and set board ID with Chrome running, i.e., need trunksd to keep running.
Cc: vbendeb@chromium.org
+vadim
Cc: apronin@chromium.org
Re #4 - no, it will not work without stopping trunksd

but you should be able to stop it for a short duration, program the board ID and restart it, is this not acceptable?
Or, we can add this command to trunks_send. And call it without stopping trunks.
Re #7: we are planning to rename usb_updater into something line cr50_tool, as it acquires more and more features. Its great advantage is that the same tool can be used on the target over /dev/tpm0 and on the host over USB.

it's impossible to use trunks_send on the host, so this is not a viable option.
Hi Vdaim

What does that mean by "impossible to use trunks_send on the host"? I thought all Cr50 updates will be using trunks_send in the future?

And wasn't it same concern of stopping trunksd would cause problem that leads to the creation of trunks_send to allow updating Cr50 in background?

Hi Hung-Te,

yes, we added the ability to update Cr50 firmware to trunks_send to be able to do it at any time without stopping trunksd, as it is not something we could do on a prod image when the device is in user hands.

On the other hand we want to be able to control Cr50 both from the host (the workstation) and from the AP on the target during development.

Controlling from the host has got to happen over USB, controlling from the AP could happen over USB or /dev/tpm0, but to use /dev/tpm0 trunksd has to be stopped.

All use cases other than firmware update are either in development environment or early startup phase (before trunksd starts).
So for factory provisioning board-id I need to run it from AP, not from host. And I need to keep trunksd running since it may randomly crash Chrome if I stop trunksd.

So, can we have this feature supported as well in trunks_send?
why would it crash chrome, I have never seen it happen due to momentarily stopped trunkds.

Also, board id provisioning could be put into the startup scripts. In fact I am adding code there in prod image to do it for all existing boards, to make sure that there are no cr50s with default board IDs in the field, I'll add you to the patch review.

I really don't want to duplicate this functionality in trunks_send.
> why would it crash chrome, I have never seen it happen due to momentarily stopped trunkds.

Sometimes when Chrome is trying to load some user data, it'll try to access cryptohome and get user encryption info, and ends trying to get some service that will route to trunksd. We've seen random crash due to that in factory.

> board id provisioning could be put into the startup scripts.

How do you decide the flags? I remember it must be set according to build phase, which we have to start our factory software to retrieve related information, and can't be done without prior settings.

Also, board id (RLZ brand code) is not provisioned in the very beginning.

Currently we plan to do board provisioning as an individual test so it's executed inside Chrome.
I am sure there is something which can be done, say saving the required board ID in /var and then using it on the next startup. It is counter productive to duplicate functionality between different apps. 

Comment 15 Deleted

If your concern is dupe, what about moving the functionality to trunks_send and don't keep it in usb_updater, since no one should be setting board id using usb_updater?
And I do feel that if there will be more cr50 features coming, asking user to "stop trunksd then usb_updater" is really not the best approach, since stopping trunksd may cause unexpected results when cryptohome is running.

Maybe you should revise trunk_send design to make it more transparent and extensible, so we can easily support the features on both side (trunks_send and usb_updater), instead of causing more code duplication?
Or can you make boardid exported by kernel drivers in /sys, just like efuse on many ARM SOC?
I feel it may be time to combine usb_updater (future cr50-tool) and trunks_send into a single app that decides if it talks directly to /dev/tpm0 or goes through trunksd based on a command-line switch.
This way we can avoid code duplication, and just switch between two simple transports that implement send-command-and-get-response functionality at the low level.
We can even auto-detect the transport (use trunksd if /dev/tpm0 is busy; or vice versa: go through /dev/tpm0 if trunksd is not on dbus), but that may be an overkill.
Honestly to me it does not feel like it's time: we don't want anyone to mock with cr50 on a prod system, so it is even good that there is a threshold to cross to get using it (as in stopping trunksd).

Combining trunks_send and usb_updater is easier said than done: they live in two different repositories and each rely on the code in those repositories.

There is no problem in development environment with using usb_updater extensions, and this is where it is mostly supposed to be used. We need to address issues when using it in the factory process, I just don't feel that re-writing this utility and combining two unrelated repositories is the way to go.

Maybe modifying usb_updater to use trunks_send --raw interface instead of /dev/tpm0 based on command flag is an option, but again, I would rather limit the ability of cr50_tool running on a prod system.

As an alternative crazy idea: can both cryptohomed and trunksd be stopped for board_id management. Programming board ID would take fraction of the second, do we really need to make this such a bid deal?

At the very least, we need a way to *read* the Board ID from Cr50 during boot, and if it is blank, write the RLZ code into the Board ID.  This may be doable before Chrome starts, so maybe the tpm0/trunksd issue doesn't apply.

When we implement case-closed debugging "V1", it will be necessary to send vendor commands to (re-)configure CCD when enterprise policy changes.  Looking at how they set the FWMP, that policy is set via Chrome.  So it looks like we'll need a way to do that via trunksd?

Non-enterprise device owners will also be able to set a CCD password.  The current proposal is to do that via crosh, though in the last eng review it was suggested that may need to be done via chrome settings because we need to determine if the current user is the device owner.  So again, may need to be able to send these commands when Chrome is running, without crashing it.

ok, I will add a mode to use trunks_send --raw as a TPM driver option in usb_updater. This is not going to look pretty, but will do the trick.

Hung-Te, do you think it is OK for now to stop trunksd/cryptohomed momentarily during factory process to configure Board ID?
If, with all the package inter-dependencies, usb_updater can link to libtrunks.so from trunks package, it can also use TrunksDBusProxy::SendCommandAndWait() from there instead of calling "trunks_send --raw".

trunks_send itself does just that: https://chromium.googlesource.com/aosp/platform/system/tpm/+/master/trunks/trunks_send.cc#163
after a simple setup:
https://chromium.googlesource.com/aosp/platform/system/tpm/+/master/trunks/trunks_send.cc#633
we want to be able to run this utility on the workstation, so we better not link with DUT only libraries.
> Hung-Te, do you think it is OK for now to stop trunksd/cryptohomed momentarily during factory process to configure Board ID?

Do you mean in future we can read/set bid by trunks_send, and for the incoming project we must do with usb_updater?

The incoming project is not MP build yet so we may hard-code the board id without reading factory data, but we definitely need a real solution that won't need to stop trunksd before MP (or for future projects) otherwise there's a big chance that TPMs will be provisioned with wrong id.
I'll look into using trunks_send as a tpm driver interface option (in
addition to USB and /dev/tpm0) in the meanwhile
I thought I have asked somewhere but can't find it anymore.

Vadim, Randall, for the short term solution using usb_updater, can you give some real example of how we should invoke in factory to provision right ID?

Comment 29 by vbendeb@google.com, Jul 10 2017

With the latest changes to cr50-result.conf you don't need to do anything extra, it will set default board ID values based on VPD RLZ string contents.

Just make sure that the latest usb_updater and cr50-result.conf are either in the factory image, or in the image installed on the device before shipping.

It would be great to have a test added to verify that Board IS is indeed set though.

initctl stop trunksd
usb_updater -s -i
initctl start trunksd

would allow to check the currently set value.
So you mean "$(usb_updater -s -i)" will return something, so what should I compare with? Just war RLZ 4 byte string, no extra encoding needed?

Also, I remember we still have to set build state (MP/Non-MP) - was that also covered in cr50-result?
it will return something like this:

localhost ~ # usb_updater -s -i            
Board ID space: 41424344:bebdbcbb:0000ff00

where the first two values are the RLZ code in hex, direct and inverted, and the third field is the flags.

this is what cr50-result.conf will set it to, if the factory flow has not set it to something else.

As suggested by the document in the opening comment, maybe the factory process should set the flags field to 0xFF7F unless a different value is requested explicitly.

0x00FF7F means "development board".  0x00FF80 means "MP board".  0x00FF00 means "don't know whether it's dev or MP".

Do we not know whether a board is dev or MP?

0x00FF00 is set as a default when a "normal" chrome os image starts up, IF the value was not yet set explicitly.

The idea is that the factory process sets the value to something matching the board development phase before "normal" chrome os image has a chance to run.
One minor question. How do we detect if a device has CR50 or not (i.e., need to do board-id or not)?

There are many tricks I believe (for example tpm_version | grep Manufacturer |grep CROS?) but I'd like to get an official answer.

Comment 35 by vben...@gmail.com, Jul 11 2017

boards where you can't control board ID will not have neither usb_updater, nor the /etc/init/cr50-* scripts.

I tried introducing a file as an immutable means of indicating the presence of cr50, but this was adamantly shut down by vapier@ 
I'm not sure if usb_updater is a good idea - wasn't it a general tool?
Will checking trunksd or trunks_send be better?

Or just will this be better:
 tpm_version | grep 'Manufacturer Info: *43524f53'
usb_updater will soon become cr50tool.  It will be the cr50 equivalent of ectool - the utility which understand how to send all the TPM vendor commands we're adding.  Firmware update, Board ID, CCD config, etc.

This was a lower pain direction than trying to make a single usb_updater which understands every USB update protocol (camera, trackpad, cr50, etc.)
Cc: mruthven@chromium.org
guys, so is there a definitive plan when board ID setting will become a part of the factory process.

It would be great if all Cr50 devices built from now on had the correct board ID flags set (indicating the build phase)
You changed the upstart job to provision board ID from $(mosys platform brand) right?
Since factory runs test images, I think that implies all boards should have been set if they migrate to new test images (i.e., new projects).

For build phase, (1) Need a reliable, portable way to detect if the system has Cr50 (2) Need some example of real command line for how to provision the phase without touching board ID.
I did change the job to set the board ID in case it is not set, to catch all cr50 devices in the wild which do not have Board ID set. The default flags are set to 0xff0000

The factory process should be setting the flags to properly match the device hardware phase (evt/devt/mp, etc).
Re#40 yes we can set phase, but it'd be great if you can post the real command as a reference.

As I've explained, this may be related to endianess so I'd like the author of tool to give an real example of how we should use it - with flag 0x00FF80 and 0x00FF7F.

And BTW, in fact we can't give a trusted answer of EVT/DVT/MP. There are some indications of phase in factory process but they may be not reliable, especially that many projects consider PVT as MP.

The only thing we plan to do is set MP flag based on if WP flags set. So actually you can do same thing in the upstart script.
What about this: we move the part setting board ID & phase from upstart conf to a standalone script, so both upstart and factory can call it.

So factory does not need to 'detect' if system has cr50 or not - we simply invoke  the script if it exists (which implies the existence of cr50).

The command, for example /usr/sbin/cr50_set_board_id, should:

1. Fetch $(mosys platform brand) and compare that with existing board ID.
2. If mosys returns NULL, error and abort.
3. If Board ID not set, provision with the value from mosys.
4. If Board ID was set, error and abort if that's not same as value from mosys.
5. If Phase was set, exit with 0 # can't change anymore.
6. Find system WP states. If WP set (flashrom -p host --wp-status), change Phase to MP.
7. If a param is given (say 'finalize' or 'factory'), set Phase to Dev.
8. Otherwise, keep Phase unchanged (this is the case of standard test image, or in factory before finalization)

Sounds ok?
Cc: -yhong@chromium.org -pihsun@chromium.org chromeos-factory-eng@google.com
Owner: youcheng@chromium.org
Status: Assigned (was: Untriaged)
Assign to youcheng - see my proposal in c#42. Feel free to let me know if your plate is full.
A few comments:

1. Fetch $(mosys platform brand) and compare that with existing board ID. #
Existing where? Or you mean check for validity?

2. If mosys returns NULL, error and abort.
3. If Board ID not set, provision with the value from mosys.

4. If Board ID was set, error and abort if that's not same as value from
mosys.  ## That is this Chrome OS device would fail factory process, right?

5. If Phase was set, exit with 0 # can't change anymore.

6. Find system WP states. If WP set (flashrom -p host --wp-status), change
Phase to MP.  ## Board ID fields can not be changed, they can be programmed
only once.

7. If a param is given (say 'finalize' or 'factory'), set Phase to Dev.
## Again, phase can be set only once

8. Otherwise, keep Phase unchanged (this is the case of standard test
image, or in factory before finalization)
> Existing where? Or you mean check for validity?
 The Board ID read from Cr50 on DUT.

> ## That is this Chrome OS device would fail factory process, right?
 Correct. Although there'll be nothing we can do then...

> ## Board ID fields can not be changed, they can be programmed
only once.
 Yes I meant program, as in step 5 we checked it's not programmed yet.
 And we should directly exit with 0 on step 6 if after programmed.

 7 should say "program" instead of "set".


As long as it is understood that Board ID (including flags) can be set only once and only if this field is empty, the procedure makes sense.

It just looked that some steps would be sequential while you mean they would be just one of many.
Owner: vbendeb@chromium.org
Ok. But we still need a real example of setting phase to 0x00FF7F or 0x00FF80, that will work for both ARM and X86 platforms.

Vadim, can you provide that, or Randall will?
Re#42: That looks ok.

Re#47: Not sure I understand the question.  It's a parameter to usb_updater (which is poorly named in this case, since it's not updating, and not over usb)

BOARDID_TYPE=ZZCR
BOARDID_FLAGS=0xFF80
usb_updater --board-id ${BOARDID_TYPE}:${BOARDID_FLAGS}

Note you cannot set type and flags separately.  You get a single call to set both.


Owner: petershih@chromium.org
Re#48

Thanks, that's what I'm looking for. So there's no need to call -s / -i, just --board-id?

Re-assign to petershih@. Peter, let me know if you have further questions or if your plate is fulll. This has higher priority and we want to have it done before next week, if possible...
Labels: -Pri-3 Pri-1
in general case you still need a -s and to stop trunksd if you are doing it on the device.

Randall's case good for doing it over suzy-q or on x86 machines which have a dedicated usb interface to H1.
Since setting --board-id is a single command, I think the flow should be slightly revised:

1. BOARDID_TYPE="$(mosys platform brand)".
2. Error and abort if BOARDID_TYPE is empty.
3. Get current BOARDID by usb_updater -s -i
4. Decide target flags by checking command line args
5. If target flags is empty (not specified - from upstart), set to current flags.
6. Write the target board ID (with flags) if it's different from current value.
6. Write the target board ID (with flags) if it's different from current value.

it is impossible to change Board ID fields (board type and flags) once they are set. The script should verify that the Board ID space is empty, and if it is not - check if the current contents match the desired contents.

If there is no match - the board should be failed.


> 0x00FF7F means "development board".  0x00FF80 means "MP board".  0x00FF00 means "don't know whether it's dev or MP".
> it is impossible to change Board ID fields (board type and flags) once they are set.

Wait, I thought we can change the bits from "don't know" to "MP" or "DEV"? (I know we can't change from dev to MP)

Since the factory will boot several times, cr50-result.conf will be executed before our factory flow can provision the board ID, so the flag will be set to 0xff00 (by cr50-result) before we can figure out the right phase to set.


This may be a problem.

Also, I think it's somehow hard specify if the board is MP or DEV correctly if the value can't be changed by WP settings.

For example, some projects may run factory flow before final firmware is confirmed, so they will try to build, finalize (for OQC) and not enabling WP.

If we set the board to "DEV" at that time, it'll fail when the device is reflashed with real final firmware and enable WP (which should program as MP). So I think for factory, the best thing we can do is to set the MP flags when the factory claims the build to be "PVT" or "MP". This implies few dogfood and lab units may still get board ID set to "MP". Is that OK?

Had some quick offline discussion with vadim.

Facts:
 - BoardID can be set only one time, for both type and flag.
 - Factory is running a standard test image with factory software, so we can't expect the cr50-result.conf to be different on factory branches.
 - DUTs in manufacturing line may try to boot test images any time before or after finalized, but it will be always in developer mode (or recovery boot) before OQC.
 - We have to set board ID before OQC, which means we can't trust WP status.
 - crossystem phase_enforcement is not implemented on all boards, so it's not trusted.
 - Projects may choose to ship PVT units.

Proposal:
 - The logic of setting board ID should be moved to a standalone script.
 - The flag to set should be decided by "Phase". Factory Phase PVT or MP should set FLAGS=MP, otherwise DEV.
 - Upstart (cr50-result.conf) should only run the script to set board ID if the system boots in normal mode (crossystem mainfw_type=normal).
 - Factory should add a pytest or part of gooftool command to call the script setting Cr50 Board ID.

Project Member

Comment 56 by bugdroid1@chromium.org, Aug 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/152c9c83d64529bbe64664206e4fc53ff74bf58e

commit 152c9c83d64529bbe64664206e4fc53ff74bf58e
Author: Hung-Te Lin <hungte@chromium.org>
Date: Fri Aug 04 07:27:49 2017

chromeos-cr50-scripts: Only set Board ID in normal mode.

During factory and developer testing, we may not have enough information
to figure out Board ID; so the script to set Board ID as 0xff00 (not
sure if it's dev or MP) should be executed only when device has been
booted in normal mode (i.e., shipping state).

BUG= chromium:724527 
TEST=None

Change-Id: Iabf82562cb7ec6109795b6b7a34b0b42a1747e98
Reviewed-on: https://chromium-review.googlesource.com/599132
Commit-Ready: Hung-Te Lin <hungte@chromium.org>
Tested-by: Hung-Te Lin <hungte@chromium.org>
Reviewed-by: Vadim Bendebury <vbendeb@chromium.org>

[modify] https://crrev.com/152c9c83d64529bbe64664206e4fc53ff74bf58e/chromeos-base/chromeos-cr50-scripts/files/cr50-result.conf
[rename] https://crrev.com/152c9c83d64529bbe64664206e4fc53ff74bf58e/chromeos-base/chromeos-cr50-scripts/chromeos-cr50-scripts-0.0.1-r2.ebuild

Project Member

Comment 57 by bugdroid1@chromium.org, Aug 4 2017

Labels: merge-merged-factory-eve-9667.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/02c0823176650a3d917cd8d115c911fa5e0424bf

commit 02c0823176650a3d917cd8d115c911fa5e0424bf
Author: Hung-Te Lin <hungte@chromium.org>
Date: Fri Aug 04 08:00:29 2017

chromeos-cr50-scripts: Only set Board ID in normal mode.

During factory and developer testing, we may not have enough information
to figure out Board ID; so the script to set Board ID as 0xff00 (not
sure if it's dev or MP) should be executed only when device has been
booted in normal mode (i.e., shipping state).

BUG= chromium:724527 
TEST=None

Change-Id: Iabf82562cb7ec6109795b6b7a34b0b42a1747e98
Reviewed-on: https://chromium-review.googlesource.com/599132
Commit-Ready: Hung-Te Lin <hungte@chromium.org>
Tested-by: Hung-Te Lin <hungte@chromium.org>
Reviewed-by: Vadim Bendebury <vbendeb@chromium.org>
(cherry picked from commit 152c9c83d64529bbe64664206e4fc53ff74bf58e)
Reviewed-on: https://chromium-review.googlesource.com/601633
Reviewed-by: Hung-Te Lin <hungte@chromium.org>
Commit-Queue: Hung-Te Lin <hungte@chromium.org>
Trybot-Ready: Hung-Te Lin <hungte@chromium.org>

[modify] https://crrev.com/02c0823176650a3d917cd8d115c911fa5e0424bf/chromeos-base/chromeos-cr50-scripts/files/cr50-result.conf
[rename] https://crrev.com/02c0823176650a3d917cd8d115c911fa5e0424bf/chromeos-base/chromeos-cr50-scripts/chromeos-cr50-scripts-0.0.1-r2.ebuild

Project Member

Comment 59 by bugdroid1@chromium.org, Aug 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/factory/+/8d40fcf23db3010d7e17c7839f4974fa9ff099d4

commit 8d40fcf23db3010d7e17c7839f4974fa9ff099d4
Author: Shen-En Shih <petershih@chromium.org>
Date: Mon Aug 07 08:05:31 2017

gooftool: Set cr50 board id in finalize

Set the board ID and flag to the Cr50 during the factory
finalization step.

BUG= chromium:724527 
TEST=make test

Change-Id: I3ab7ec247f84a0c4db68cfe696c1cc6e71469516
Reviewed-on: https://chromium-review.googlesource.com/603039
Reviewed-by: Hung-Te Lin <hungte@chromium.org>
Tested-by: Shen-En Shih <petershih@chromium.org>
Commit-Queue: Shen-En Shih <petershih@chromium.org>

[modify] https://crrev.com/8d40fcf23db3010d7e17c7839f4974fa9ff099d4/py/gooftool/commands.py
[modify] https://crrev.com/8d40fcf23db3010d7e17c7839f4974fa9ff099d4/py/gooftool/core.py

Project Member

Comment 61 by bugdroid1@chromium.org, Aug 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/factory/+/d078a7c5b6f34f9610187e63b0ccf69964feae7c

commit d078a7c5b6f34f9610187e63b0ccf69964feae7c
Author: Shen-En Shih <petershih@chromium.org>
Date: Mon Aug 07 12:51:16 2017

gooftool: Set cr50 board id in finalize

Set the board ID and flag to the Cr50 during the factory
finalization step.

BUG= chromium:724527 
TEST=make test

Change-Id: I3ab7ec247f84a0c4db68cfe696c1cc6e71469516
Reviewed-on: https://chromium-review.googlesource.com/601774
Commit-Ready: Shen-En Shih <petershih@chromium.org>
Tested-by: Shen-En Shih <petershih@chromium.org>
Reviewed-by: Shen-En Shih <petershih@chromium.org>

[modify] https://crrev.com/d078a7c5b6f34f9610187e63b0ccf69964feae7c/py/gooftool/commands.py
[modify] https://crrev.com/d078a7c5b6f34f9610187e63b0ccf69964feae7c/py/gooftool/core.py

Status: Fixed (was: Assigned)
ToT and Eve factory branch have merged all needed changes.

I've created  issue 753227  to track the implementation of board ID provisioning based on trunks_send.
Project Member

Comment 63 by bugdroid1@chromium.org, Aug 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/c027bf2e81edb6fe6180d9f6340fd0aa9d77c0f9

commit c027bf2e81edb6fe6180d9f6340fd0aa9d77c0f9
Author: Hung-Te Lin <hungte@chromium.org>
Date: Sat Aug 26 18:20:52 2017

chromeos-cr50-scripts: Set default MP features to be 0x7f.

All projects may have released devices without board ID set, for example
due to not able to run finalization in Proto stage. As a result, for any
board-locked images it seems more reasonable to use flags=0x7f80
(features=0x7f, phase=0x80) for MP units.

BUG= chromium:724527 
TEST=None

Change-Id: I4d55fd86f1c82680204f2020aa58d4d28c0a837f
Reviewed-on: https://chromium-review.googlesource.com/634725
Commit-Ready: Hung-Te Lin <hungte@chromium.org>
Tested-by: Hung-Te Lin <hungte@chromium.org>
Reviewed-by: Randall Spangler <rspangler@chromium.org>
Reviewed-by: Vadim Bendebury <vbendeb@chromium.org>

[modify] https://crrev.com/c027bf2e81edb6fe6180d9f6340fd0aa9d77c0f9/chromeos-base/chromeos-cr50-scripts/files/cr50-set-board-id.sh
[rename] https://crrev.com/c027bf2e81edb6fe6180d9f6340fd0aa9d77c0f9/chromeos-base/chromeos-cr50-scripts/chromeos-cr50-scripts-0.0.1-r4.ebuild

Project Member

Comment 64 by bugdroid1@chromium.org, Aug 30 2017

Labels: merge-merged-factory-gru-9017.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/184718e5c9db8d82630c5a3fabb6e4bf057bf3a5

commit 184718e5c9db8d82630c5a3fabb6e4bf057bf3a5
Author: Hung-Te Lin <hungte@chromium.org>
Date: Wed Aug 30 22:29:09 2017

chromeos-cr50-scripts: Only set Board ID in normal mode.

During factory and developer testing, we may not have enough information
to figure out Board ID; so the script to set Board ID as 0xff00 (not
sure if it's dev or MP) should be executed only when device has been
booted in normal mode (i.e., shipping state).

BUG= chromium:724527 
TEST=None

Original-change-Id: Iabf82562cb7ec6109795b6b7a34b0b42a1747e98
Reviewed-on: https://chromium-review.googlesource.com/599132
Commit-Ready: Hung-Te Lin <hungte@chromium.org>
Tested-by: Hung-Te Lin <hungte@chromium.org>
Reviewed-by: Vadim Bendebury <vbendeb@chromium.org>
Change-Id: Iae1e7dd0de8af487002c7096f8797c45c48dce1e
Reviewed-on: https://chromium-review.googlesource.com/644566
Reviewed-by: Philip Chen <philipchen@chromium.org>
Commit-Queue: Philip Chen <philipchen@chromium.org>
Tested-by: Philip Chen <philipchen@chromium.org>

[modify] https://crrev.com/184718e5c9db8d82630c5a3fabb6e4bf057bf3a5/chromeos-base/chromeos-cr50-scripts/files/cr50-result.conf
[rename] https://crrev.com/184718e5c9db8d82630c5a3fabb6e4bf057bf3a5/chromeos-base/chromeos-cr50-scripts/chromeos-cr50-scripts-0.0.1-r4.ebuild

Project Member

Comment 65 by bugdroid1@chromium.org, Aug 30 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/38b60ba8b66b4dd7aa0f6afeacd27b9d6ce9d01f

commit 38b60ba8b66b4dd7aa0f6afeacd27b9d6ce9d01f
Author: Shen-En Shih <petershih@google.com>
Date: Wed Aug 30 22:31:42 2017

chromeos-cr50-scripts: A script to set board id and flag

The board id and flag should be set properly in the factory flow.
This script helps the factory flow to set them properly.

BUG= chromium:724527 
TEST=none

Original-change-Id: I16bb028158736038f6dd8e04dad8e74cd2485c35
Reviewed-on: https://chromium-review.googlesource.com/598755
Commit-Ready: Shen-En Shih <petershih@chromium.org>
Tested-by: Shen-En Shih <petershih@chromium.org>
Reviewed-by: Hung-Te Lin <hungte@chromium.org>
Change-Id: I6d2e83cd741584db05abc6df5b462c2af395783e
Reviewed-on: https://chromium-review.googlesource.com/644567
Reviewed-by: Philip Chen <philipchen@chromium.org>
Commit-Queue: Philip Chen <philipchen@chromium.org>
Tested-by: Philip Chen <philipchen@chromium.org>

[modify] https://crrev.com/38b60ba8b66b4dd7aa0f6afeacd27b9d6ce9d01f/chromeos-base/chromeos-cr50-scripts/files/cr50-result.conf
[add] https://crrev.com/38b60ba8b66b4dd7aa0f6afeacd27b9d6ce9d01f/chromeos-base/chromeos-cr50-scripts/files/cr50-set-board-id.sh
[rename] https://crrev.com/38b60ba8b66b4dd7aa0f6afeacd27b9d6ce9d01f/chromeos-base/chromeos-cr50-scripts/chromeos-cr50-scripts-0.0.1-r5.ebuild
[modify] https://crrev.com/38b60ba8b66b4dd7aa0f6afeacd27b9d6ce9d01f/chromeos-base/chromeos-cr50-scripts/chromeos-cr50-scripts-0.0.1.ebuild

Project Member

Comment 66 by bugdroid1@chromium.org, Aug 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/factory/+/86e8d35bc2b1d7f8f5343ca1b999a49d8867dce4

commit 86e8d35bc2b1d7f8f5343ca1b999a49d8867dce4
Author: Shen-En Shih <petershih@chromium.org>
Date: Thu Aug 31 18:13:27 2017

gooftool: Set cr50 board id in finalize

Set the board ID and flag to the Cr50 during the factory
finalization step.

Fix the conflict from the import libs.
 Conflicts:
	py/gooftool/core.py

BUG= chromium:724527 
TEST=make test

Change-Id: Id02080ef576c7e10bcc4f6ec3b557a3decffc6c1
Original-change-Id: I3ab7ec247f84a0c4db68cfe696c1cc6e71469516
Reviewed-on: https://chromium-review.googlesource.com/601774
Commit-Ready: Shen-En Shih <petershih@chromium.org>
Tested-by: Shen-En Shih <petershih@chromium.org>
Reviewed-by: Shen-En Shih <petershih@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/644101
Commit-Queue: Philip Chen <philipchen@chromium.org>
Tested-by: Philip Chen <philipchen@chromium.org>

[modify] https://crrev.com/86e8d35bc2b1d7f8f5343ca1b999a49d8867dce4/py/gooftool/commands.py
[modify] https://crrev.com/86e8d35bc2b1d7f8f5343ca1b999a49d8867dce4/py/gooftool/core.py

Status: Assigned (was: Fixed)
I missed the fact that the changes removed fallback code which is supposed to set board ID in an H1 which does not have it set yet.

A common case would be those already manufactured/released reef and bob family devices.

part of the changes in this patch https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-overlay/+/598755 need to be reverted.
Hi Vadim,

Yes we're aware of that. The board ID and flags should still be set in startup script.
Lines 32-34 in "cr50-result.conf" calls the script to set the board-id/flags.
Lines 25-28 should not be bypassed in a released device.

Do you see anything goes unexpected in any device?
Status: Fixed (was: Assigned)
You are right, false alarm. I had a device where board ID was not set, but the device is in dev mode, where board ID is not supposed to be set.

Sorry for the noise, closing.
Project Member

Comment 70 by bugdroid1@chromium.org, Sep 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/dddd53e5275b74d9b4cb0e1e3a7dabcef00017f3

commit dddd53e5275b74d9b4cb0e1e3a7dabcef00017f3
Author: Hung-Te Lin <hungte@chromium.org>
Date: Wed Sep 20 18:31:54 2017

chromeos-cr50-scripts: Set default MP features to be 0x7f.

All projects may have released devices without board ID set, for example
due to not able to run finalization in Proto stage. As a result, for any
board-locked images it seems more reasonable to use flags=0x7f80
(features=0x7f, phase=0x80) for MP units.

BUG= chromium:724527 
TEST=None

Change-Id: I4d55fd86f1c82680204f2020aa58d4d28c0a837f
Reviewed-on: https://chromium-review.googlesource.com/634725
Commit-Ready: Hung-Te Lin <hungte@chromium.org>
Tested-by: Hung-Te Lin <hungte@chromium.org>
Reviewed-by: Randall Spangler <rspangler@chromium.org>
Reviewed-by: Vadim Bendebury <vbendeb@chromium.org>
(cherry picked from commit c027bf2e81edb6fe6180d9f6340fd0aa9d77c0f9)
Reviewed-on: https://chromium-review.googlesource.com/667122
Commit-Queue: Philip Chen <philipchen@chromium.org>
Tested-by: Philip Chen <philipchen@chromium.org>
Reviewed-by: Philip Chen <philipchen@chromium.org>

[modify] https://crrev.com/dddd53e5275b74d9b4cb0e1e3a7dabcef00017f3/chromeos-base/chromeos-cr50-scripts/files/cr50-set-board-id.sh
[rename] https://crrev.com/dddd53e5275b74d9b4cb0e1e3a7dabcef00017f3/chromeos-base/chromeos-cr50-scripts/chromeos-cr50-scripts-0.0.1-r7.ebuild

Comment 71 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Comment 72 by dchan@chromium.org, Jan 23 2018

Status: Fixed (was: Archived)
Project Member

Comment 73 by bugdroid1@chromium.org, Mar 14 2018

Labels: merge-merged-factory-reef-8811.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/factory/+/c088844628ea1f1c004bcef73da4d189cc18f56f

commit c088844628ea1f1c004bcef73da4d189cc18f56f
Author: Shen-En Shih <petershih@chromium.org>
Date: Wed Mar 14 10:39:40 2018

gooftool: Set cr50 board id in finalize

Set the board ID and flag to the Cr50 during the factory
finalization step.

Fix the conflict from the import libs.
 Conflicts:
	py/gooftool/core.py

BUG= chromium:724527 
TEST=make test

Original Change-Id: Id02080ef576c7e10bcc4f6ec3b557a3decffc6c1
Original-change-Id: I3ab7ec247f84a0c4db68cfe696c1cc6e71469516
Reviewed-on: https://chromium-review.googlesource.com/601774
Commit-Ready: Shen-En Shih <petershih@chromium.org>
Tested-by: Shen-En Shih <petershih@chromium.org>
Reviewed-by: Shen-En Shih <petershih@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/644101
Commit-Queue: Philip Chen <philipchen@chromium.org>
Tested-by: Philip Chen <philipchen@chromium.org>

Change-Id: I4881ada47ab94374d0b914d5c4037eca20bd8c03
Reviewed-on: https://chromium-review.googlesource.com/962203
Reviewed-by: Marco Chen <marcochen@chromium.org>
Commit-Queue: Marco Chen <marcochen@chromium.org>
Tested-by: Marco Chen <marcochen@chromium.org>

[modify] https://crrev.com/c088844628ea1f1c004bcef73da4d189cc18f56f/py/gooftool/commands.py
[modify] https://crrev.com/c088844628ea1f1c004bcef73da4d189cc18f56f/py/gooftool/core.py

Sign in to add a comment