Chromium Code Reviews (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out

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

9 years, 8 months ago by Hung-Te
9 years, 7 months ago
Che-Liang Chiou


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, bios # seeing rw|ro by FMAP on x86, 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 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 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


Total messages: 2 (0 generated)
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.
File (right): 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. 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. ErrorDie('Invalid RO section in layout: %s (%s,%s)' %
error_reason is a one single string, but your format string has 3 %s. Did I miss

Powered by Google App Engine
This is Rietveld 408576698