New rollback_index API.

New rollback_index API.

[Luigi Semenzato, 2010-06-24 00:17:34]

Randall,

this is a rough first cut.  I have to catch the bus now but I plan to go over
the changes more carefully.  In the interest of speed, I am publishing the CL. 
You may look at it now or you may wait until my next message.

[Luigi Semenzato, 2010-06-24 01:19:19]

OK---I think this does everything as discussed.  Thank you.

On 2010/06/24 00:17:34, Luigi Semenzato wrote:
> Randall,
> 
> this is a rough first cut.  I have to catch the bus now but I plan to go over
> the changes more carefully.  In the interest of speed, I am publishing the CL.

> You may look at it now or you may wait until my next message.

[gauravsh, 2010-06-24 19:18:55]

just a few minor comments

http://codereview.chromium.org/2869022/diff/3003/7002
File firmware/lib/rollback_index.c (right):

http://codereview.chromium.org/2869022/diff/3003/7002#newcode233
firmware/lib/rollback_index.c:233: int developer_mode,
update the comment to reflect that SetupTPM also has the side effect of reading
the firmware version indices

As a side note: Is there a good reason to not split SetupTPM() and the reading
of firmware version indices into separate functions?

http://codereview.chromium.org/2869022/diff/3003/7002#newcode234
firmware/lib/rollback_index.c:234: uint16_t* key_version,
change this firmware_key_version and firmware_version to make it clear which
versions we are talking about

http://codereview.chromium.org/2869022/diff/3003/7002#newcode273
firmware/lib/rollback_index.c:273: *key_version = (uint16_t) (firmware_versions
>> 16);
since SetupTPM can be called with NULL arguments, it would be a good idea to
make sure it is not the case before doing this. (this should help in catching
any incorrect usage of SetupTPM() instead of a silent failure)

[Luigi Semenzato, 2010-06-28 18:10:28]

This changes the API slightly following Gaurav's comments.

[Randall Spangler, 2010-06-28 18:18:58]

http://codereview.chromium.org/2869022/diff/3003/7001
File firmware/include/rollback_index.h (right):

http://codereview.chromium.org/2869022/diff/3003/7001#newcode86
firmware/include/rollback_index.h:86: * recovery mode, the effect is undefined.
*/
Read should explicitly return versions=0.
Write should do nothing.

[gauravsh, 2010-06-28 18:47:10]

lgtm

[Luigi Semenzato, 2010-06-28 19:48:43]

Addressed Randall's API request.  (Sanity check: we have global variables in
recovery mode, even though it's RO firmware, correct?)

[Randall Spangler, 2010-06-28 20:13:42]

LGTM

Yes, recovery mode has globals, since this will be called from LoadKernel(),
which is in BDS phase rather than PEI.