Add components which is requried to generate a GBB during factory.

Add components which is requried to generate a GBB during factory.

As the conclusion after discussed with Nick, Bill, and Randall, we need the
following components in the component list: 'key_recovery', 'key_root', 'data_bitmap_fv', 'data_display_geometry', 'data_recovery_url'. Using this data can
automatically generate a GBB during factory instead of a handcraftied one.

The rootkey and recoverykey are used to detect existing keys in the firmware in
order to differentiate the same BOM but different keys (dev or pvt keys).

TEST=run it on Mario and compare with keys
BUG=chrome-os-partner:501
Change-Id: If84686223cf769845e06889463fa2e1b51033d72

[Hung-Te, 2010-09-02 02:03:49]

http://codereview.chromium.org/3343003/diff/3001/4001
File client/common_lib/flashrom_util.py (right):

http://codereview.chromium.org/3343003/diff/3001/4001#newcode402
client/common_lib/flashrom_util.py:402: def gbb_utility(self, args='',
output_file=''):
I think GBB is beyond the scope of flashrom_util.
Can you move GBB stuff back to hardware_Components, and only invoke
flashrom_util for writing to flashrom?

http://codereview.chromium.org/3343003/diff/3001/4003
File client/site_tests/hardware_Components/hardware_Components.py (right):

http://codereview.chromium.org/3343003/diff/3001/4003#newcode119
client/site_tests/hardware_Components/hardware_Components.py:119: result =
filecmp.cmp(target_key_filename, current_key_filename) == 1
filecmp.cmp should return True or False, not 0/1.

[Tom Wai-Hong Tam, 2010-09-02 02:47:41]

http://codereview.chromium.org/3343003/diff/3001/4001
File client/common_lib/flashrom_util.py (right):

http://codereview.chromium.org/3343003/diff/3001/4001#newcode402
client/common_lib/flashrom_util.py:402: def gbb_utility(self, args='',
output_file=''):
That's OK. Since I think a lot of the logic are the same so move here. Will move
to a better place later.

http://codereview.chromium.org/3343003/diff/3001/4003
File client/site_tests/hardware_Components/hardware_Components.py (right):

http://codereview.chromium.org/3343003/diff/3001/4003#newcode119
client/site_tests/hardware_Components/hardware_Components.py:119: result =
filecmp.cmp(target_key_filename, current_key_filename) == 1
Done. I read an old document. In Python 2.2.3, it used 1/0.

[Tom Wai-Hong Tam, 2010-09-02 23:17:17]

Now I moved all GBB things to a new library gbb_util.py and made flashrom_util
only focus on flashrom. gbb_util.py need further feature improvement and now it
only supports get function. Some logic on hardware_Components also changed.

[Tom Wai-Hong Tam, 2010-09-03 18:25:01]

Please someone takes a look at it. Thanks!

[Hung-Te, 2010-09-04 00:31:01]

http://codereview.chromium.org/3343003/diff/21001/22001
File client/common_lib/flashrom_util.py (right):

http://codereview.chromium.org/3343003/diff/21001/22001#newcode500
client/common_lib/flashrom_util.py:500: return utils.system(cmd,
ignore_status=True)
I think no one cares about the return value here, since we've output the
complete error messages :-) Why not make the output a simple True/False so that
it's easier for every part else to figure out if the read was success?

http://codereview.chromium.org/3343003/diff/21001/22002
File client/common_lib/gbb_util.py (right):

http://codereview.chromium.org/3343003/diff/21001/22002#newcode74
client/common_lib/gbb_util.py:74: def _get_gbb_value_to_file(self, key,
to_file=None):
The semantic looks like confusing here: return value would change if to_file is
different (None / content), and the function does not always do like its name
("to_file").

After checking the parts in hardware_Components, I think there's no need to
support outputing to a file.
Maybe a simple _get_gbb_value(self, key) which always return value, would work
better.

If you really want to keep the "output to file" version, create another
_get_gbb_value_to_file.

http://codereview.chromium.org/3343003/diff/21001/22002#newcode94
client/common_lib/gbb_util.py:94: def get_bmpfv(self, to_file=None):
Seems like there no need to support to_file here? Since hardware_Components only
wants its value (and I think anyone who wants things in the file can easily
write value to that file by himself)

http://codereview.chromium.org/3343003/diff/21001/22002#newcode103
client/common_lib/gbb_util.py:103: def get_recoverykey(self, to_file=None):
Seems like we don't need to_file here

http://codereview.chromium.org/3343003/diff/21001/22002#newcode107
client/common_lib/gbb_util.py:107: def get_rootkey(self, to_file=None):
Seems like that we don't need to_file here

[Tom Wai-Hong Tam, 2010-09-04 01:27:05]

http://codereview.chromium.org/3343003/diff/21001/22001
File client/common_lib/flashrom_util.py (right):

http://codereview.chromium.org/3343003/diff/21001/22001#newcode500
client/common_lib/flashrom_util.py:500: return utils.system(cmd,
ignore_status=True)
Done.

http://codereview.chromium.org/3343003/diff/21001/22002
File client/common_lib/gbb_util.py (right):

http://codereview.chromium.org/3343003/diff/21001/22002#newcode74
client/common_lib/gbb_util.py:74: def _get_gbb_value_to_file(self, key,
to_file=None):
Done.

http://codereview.chromium.org/3343003/diff/21001/22002#newcode94
client/common_lib/gbb_util.py:94: def get_bmpfv(self, to_file=None):
Done.

http://codereview.chromium.org/3343003/diff/21001/22002#newcode103
client/common_lib/gbb_util.py:103: def get_recoverykey(self, to_file=None):
Done.

http://codereview.chromium.org/3343003/diff/21001/22002#newcode107
client/common_lib/gbb_util.py:107: def get_rootkey(self, to_file=None):
Done.

[Hung-Te, 2010-09-04 02:48:27]

Please do test it to run properly again :-)

http://codereview.chromium.org/3343003/diff/29001/30002
File client/common_lib/gbb_util.py (right):

http://codereview.chromium.org/3343003/diff/29001/30002#newcode60
client/common_lib/gbb_util.py:60: if flashrom.read_whole_to_file(bios_file) !=
0:
The return value has been changed to True/False

http://codereview.chromium.org/3343003/diff/29001/30002#newcode75
client/common_lib/gbb_util.py:75: """Gets the GBB value which needs to be output
to a file in gbb_utility.
not a file anymore...

http://codereview.chromium.org/3343003/diff/29001/30002#newcode84
client/common_lib/gbb_util.py:84: value = f.read()
Do we have to take care if file does not exist (i.e., I/O exception)?

http://codereview.chromium.org/3343003/diff/29001/30002#newcode90
client/common_lib/gbb_util.py:90: return self._get_gbb_value_to_file('bmpfv')
function name not changed here?

http://codereview.chromium.org/3343003/diff/29001/30002#newcode99
client/common_lib/gbb_util.py:99: return
self._get_gbb_value_to_file('recoverykey')
function name not changed here?

http://codereview.chromium.org/3343003/diff/29001/30002#newcode103
client/common_lib/gbb_util.py:103: return self._get_gbb_value_to_file('rootkey')
function name not changed here?

[Hung-Te, 2010-09-06 09:31:35]

Just tested this code with Tammo today, and found few bugs in code.

http://codereview.chromium.org/3343003/diff/33001/34002
File client/common_lib/gbb_util.py (right):

http://codereview.chromium.org/3343003/diff/33001/34002#newcode15
client/common_lib/gbb_util.py:15: from autotest_lib.client.common_lib import
utils
need to import "error" as well.

http://codereview.chromium.org/3343003/diff/33001/34002#newcode60
client/common_lib/gbb_util.py:60: if flashrom.read_whole_to_file(bios_file):
"if not ..." here.

http://codereview.chromium.org/3343003/diff/33001/34004
File client/site_tests/hardware_Components/hardware_Components.py (right):

http://codereview.chromium.org/3343003/diff/33001/34004#newcode111
client/site_tests/hardware_Components/hardware_Components.py:111: gbb =
gbb_util.GBBUtility()
this would read (physically) flash content by invoking flashrom, and each
iteration takes about 30 seconds - and that means if we have dozens of
qualified_*, it will take N*30 seconds, a really long time.

please try to find some way to cache this value... (You can create another CL
for it)

http://codereview.chromium.org/3343003/diff/33001/34004#newcode118
client/site_tests/hardware_Components/hardware_Components.py:118: gbb =
gbb_util.GBBUtility()
Just like the check_existence_key_recovery, this function also creates another
N*30 seconds to run :)

[Tom Wai-Hong Tam, 2010-09-06 16:40:45]

Sorry. Since I was not in the office. I changed the code through VPN and hadn't
tested it in real machine. Thanks for testing it for me.

http://codereview.chromium.org/3343003/diff/29001/30002
File client/common_lib/gbb_util.py (right):

http://codereview.chromium.org/3343003/diff/29001/30002#newcode60
client/common_lib/gbb_util.py:60: if flashrom.read_whole_to_file(bios_file) !=
0:
Done.

http://codereview.chromium.org/3343003/diff/29001/30002#newcode75
client/common_lib/gbb_util.py:75: """Gets the GBB value which needs to be output
to a file in gbb_utility.
I mean gbb_utility needs to output the value to a file.

http://codereview.chromium.org/3343003/diff/29001/30002#newcode84
client/common_lib/gbb_util.py:84: value = f.read()
I think the most case of failure is the utils.system_output(cmd) in
_run_gbb_utility. If the above command successes, I don't think there is any
case makes f.read() failed.

In addition, I don't know how to handle this error except re-raising it to other
name. So leave it to upper application to handle it.

http://codereview.chromium.org/3343003/diff/29001/30002#newcode90
client/common_lib/gbb_util.py:90: return self._get_gbb_value_to_file('bmpfv')
Done.

http://codereview.chromium.org/3343003/diff/29001/30002#newcode99
client/common_lib/gbb_util.py:99: return
self._get_gbb_value_to_file('recoverykey')
Done.

http://codereview.chromium.org/3343003/diff/29001/30002#newcode103
client/common_lib/gbb_util.py:103: return self._get_gbb_value_to_file('rootkey')
Done.

http://codereview.chromium.org/3343003/diff/33001/34002#newcode15
client/common_lib/gbb_util.py:15: from autotest_lib.client.common_lib import
utils
Done.

http://codereview.chromium.org/3343003/diff/33001/34002#newcode60
client/common_lib/gbb_util.py:60: if flashrom.read_whole_to_file(bios_file) !=
0:
Done.

http://codereview.chromium.org/3343003/diff/33001/34004
File client/site_tests/hardware_Components/hardware_Components.py (right):

http://codereview.chromium.org/3343003/diff/33001/34004#newcode111
client/site_tests/hardware_Components/hardware_Components.py:111: gbb =
gbb_util.GBBUtility()
Make the variable gbb as a member variable of the class. I think the HWID
matching is in the runin phase, the time is not a critical factor. After that,
the test will run for only one qualified components.

[Hung-Te, 2010-09-07 01:41:53]

LGTM