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

Issue 732325 link

Starred by 2 users

Issue metadata

Status: Archived
Owner:
Last visit > 30 days ago
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

gooftool: Revise firmware reading time

Project Member Reported by hungte@chromium.org, Jun 12 2017

Issue description

Currently gooftool uses crosfw module to read firmware, and crosfw used to read whole firmware image.

This used to work well because most devices have firmware SPI image 4M and readig whole image does not take too much time.

However, for modern Chromebooks that uses 16G SPI, reading whole SPI becomes too long.

If we look at gooftool, what it needs are mostly in RO (GBB, RO_SECTION, FMAP, GBB) - or maybe plus RW keblocks (RW_VBLOCK_A, RW_VBLOCK_B).

time flashrom -r /tmp/bios.bin
 9s

time flashrom -r /tmp/bios.bin -i WP_RO -i VBLOCK_A -i VBLOCK_B -i RW_VPD
 3.1s

which means we can reduce each flashrom invocation to 1/3 time.
 
Gooftool also need FW_MAIN_A/B
(https://chromium.googlesource.com/chromiumos/platform/factory/+/master/py/gooftool/core.py#290)
Only 25% faster if these two blocks included, 

$ time flashrom -r /tmp/bios.bin -i WP_RO -i VBLOCK_A -i VBLOCK_B -i RW_VPD -i FW_MAIN_A -i FW_MAIN_B
7.7s

$ time flashrom -r /tmp/bios.bin
9.7s

Comment 2 by hungte@chromium.org, Jun 13 2017

I think MAIN_A and MAIN_B are only needed by verify_keys?

So at least we can make probe running faster, and only fetch MAIN_A/MAIN_B if needed.
Yes, you are right.
How about make FirmwareContent.Load(TARGET_MAIN) only loads and caches the sections listed in #0 by default. If user specifies extra sections to load, e.g. FirmwareContent.Load(TARGET_MAIN, sections=...) it will load and return an uncached instance with the sections included.

Comment 4 by hungte@chromium.org, Jun 13 2017

Not sure if "default read all" or "default read only few sections" is better. You can try to make some CL and see which works better.

Comment 5 by hungte@chromium.org, Jul 14 2017

Cc: -phoenixshen@chromium.org -stimim@chromium.org chromeos-factory-eng@google.com

Comment 6 by hungte@chromium.org, Jul 26 2017

Owner: yhong@chromium.org
Status: Assigned (was: Untriaged)
Assign to yhong since this is probe related

Comment 7 by yhong@chromium.org, Aug 30 2017

Status: Started (was: Assigned)
Project Member

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

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

commit 5b4f0b4bfee6125aebf0746c1f4788cf72cd978b
Author: Yong Hong <yhong@chromium.org>
Date: Wed Aug 30 20:04:26 2017

probe: Speed up firmware probing process.

The probing process only needs RO_SECTION/EC_RO from the firmware
so this CL revises the interface of `cros.factory.gooftool.crosfw`
to be able to load only specific sections.

Before:
$ time probe eval-function chromeos_firmware firmware_keys
13.7s

After:
$ time probe eval-function chromeos_firmware firmware_keys
5.3s

BUG= chromium:732325 
TEST=manually test on DUT; make test

Change-Id: Ie79d57ad8e79ff707155fe9b680ebae687d4ec62
Reviewed-on: https://chromium-review.googlesource.com/642667
Commit-Ready: Yong Hong <yhong@chromium.org>
Tested-by: Yong Hong <yhong@chromium.org>
Reviewed-by: Hung-Te Lin <hungte@chromium.org>

[modify] https://crrev.com/5b4f0b4bfee6125aebf0746c1f4788cf72cd978b/py/gooftool/crosfw.py
[modify] https://crrev.com/5b4f0b4bfee6125aebf0746c1f4788cf72cd978b/py/probe/functions/chromeos_firmware.py

Project Member

Comment 9 by bugdroid1@chromium.org, Sep 28 2017

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

commit bd47f18ca80f96b4cc0809690a53c45018d6717d
Author: Yong Hong <yhong@chromium.org>
Date: Thu Sep 28 16:16:27 2017

hwid: Speed up getting/setting HWID string process.

The HWID String is stored in the GBB section of the main firmware.
This CL revises the getting/setting process so that it only loads
the GBB section instead of all sections.

Before:
$ time hwid read
0m10.045s
$ time hwid write --hwid <HWID_STRING>
0m14.134s

After:
$ time hwid read
0m1.775s
$ time hwid write --hwid <HWID_STRING>
0m11.458s

BUG= chromium:732325 
TEST=None

Change-Id: Ie2f77ab08b8e99d541aa059ceb4f9e60104b3286
Reviewed-on: https://chromium-review.googlesource.com/643042
Commit-Ready: Yong Hong <yhong@google.com>
Tested-by: Yong Hong <yhong@google.com>
Reviewed-by: Hung-Te Lin <hungte@chromium.org>

[modify] https://crrev.com/bd47f18ca80f96b4cc0809690a53c45018d6717d/py/hwid/v3/hwid_utils.py

Project Member

Comment 10 by bugdroid1@chromium.org, Nov 9 2017

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

commit dad230a0bc37b591e0d11d76d678d12a3f04c821
Author: Yong Hong <yhong@chromium.org>
Date: Thu Nov 09 08:38:13 2017

gooftool: Speed up write_protect sub-command.

This CL speeds up write_protect sub-command by loading only `WP_RO`
section for calculating the write protect range.

Before:
$ time gooftool write_protect
0m18.413s

After:
$ time gooftool write_protect
0m11.028s

BUG= chromium:732325 
TEST=manually run `gooftool write_protect` on DUT; make test

Change-Id: I50a1755ce6d085a3fc3c26872895982e76970578
Reviewed-on: https://chromium-review.googlesource.com/643011
Commit-Ready: Yong Hong <yhong@google.com>
Tested-by: Yong Hong <yhong@google.com>
Reviewed-by: Hung-Te Lin <hungte@chromium.org>

[modify] https://crrev.com/dad230a0bc37b591e0d11d76d678d12a3f04c821/py/gooftool/crosfw.py
[modify] https://crrev.com/dad230a0bc37b591e0d11d76d678d12a3f04c821/py/gooftool/commands.py

Comment 11 by yhong@chromium.org, Dec 27 2017

Status: Fixed (was: Started)
Status: Archived (was: Fixed)

Sign in to add a comment