Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(513)

Issue 6788005: factory_test_tools: support write protection for ARM (Closed)

Created:
9 years, 8 months ago by Hung-Te
Modified:
9 years, 7 months ago
Reviewers:
Che-Liang Chiou
CC:
chromium-os-reviews_chromium.org
Visibility:
Public.

Description

factory_test_tools: support write protection for ARM The RO/RW layout is different for ARM/x86, so we must trust FMAP to build the write protection layout. BUG=chromium-os:12193 TEST=on x86, gft_wpfw.py bios # seeing rw|ro by FMAP on x86, gft_wpfw.py ec # seeing ro|rw by FMAP assuming ARM will contain correct FMAP so the behavior is correct. gooftool --wpfw # success gooftool --verify_hwid --db_path DB --wpfw --verbose # seeing wpfw avoids reading EC/BIOS firmware again Change-Id: Idb015522f4c3922f476beb43763107be416aa45b R=clchiou@chromium.org Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=529424b

Patch Set 1 #

Total comments: 3

Patch Set 2 : speed up finalization, by using cached bios/ec data #

Patch Set 3 : fix by reviewer's comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+67 lines, -29 lines) Patch
M gft_wpfw.py View 1 2 3 chunks +48 lines, -24 lines 0 comments Download
M gooftool View 1 2 2 chunks +19 lines, -5 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
Hung-Te
9 years, 8 months ago (2011-04-01 11:56:46 UTC) #1
Che-Liang Chiou
9 years, 8 months ago (2011-04-01 12:22:01 UTC) #2
LGTM with nits.

http://codereview.chromium.org/6788005/diff/1/gft_wpfw.py
File gft_wpfw.py (right):

http://codereview.chromium.org/6788005/diff/1/gft_wpfw.py#newcode82
gft_wpfw.py:82: slot_size = int(eeprom_size / 2)
Are you assuming that RO and RW takes exactly half space of eeprom? I am okay
with this assumption, but please adds a comment for it.

http://codereview.chromium.org/6788005/diff/1/gft_wpfw.py#newcode86
gft_wpfw.py:86: ro_end_slot = int(ro_end_offset / slot_size)
I could be wrong about this. It looks like it is a right-exclusive range:
[ro_begin_offset, ro_end_offset). If this is the case, the check on line 88
might be problematic when ro_end_offset is right on the boundary or ro/rw.

For example, consider the following fmap
  LSB [ RO | RW ] RSB
and eeprom_size = 1000, ro_begin_offset = 0, ro_end_offset = 500.
Then, ro_begin_slot = 0 and ro_end_slot = 1, and check on line 88 will fail.

http://codereview.chromium.org/6788005/diff/1/gft_wpfw.py#newcode93
gft_wpfw.py:93: ErrorDie('Invalid RO section in layout: %s (%s,%s)' %
error_reason)
error_reason is a one single string, but your format string has 3 %s. Did I miss
anything?

Powered by Google App Engine
This is Rietveld 408576698