On Wed, Mar 30, 2011 at 01:55:40PM -0400, Stefan Berger wrote:
This patch adds invocactions of functions that measure various parts of the code and data through various parts of the BIOS code. It follows TCG specifications on what needs to be measured. It also adds the implementation of the called functions.
[...]
void VISIBLE32FLAT startBoot(void) {
- tcpa_calling_int19h();
- tcpa_add_event_separators();
Why add two functions here, instead of just one function that does both actions?
[...]
// Initialize tpm (after acpi tables were written) tcpa_acpi_init(); tcpa_startup();
- tcpa_measure_post((void *)0xE0000, (void *)0xFFFFF);
- tcpa_smbios_measure();
Same here. Also, I'm not sure I understand why you're measuring 0xE0000-0xFFFFF - if the intent is to measure the code, then it's a bit more complicated as the init code has already been relocated by this point.
[...]
- tcpa_option_rom(FLATPTR_TO_SEG(rom), len);
You're better off just defining tcpa_option_rom() to take a regular pointer.
[...]
- /* specs: 8.2.3 step 5 and 8.2.5.6, measure El Torito boot catalog */
- /* measure 2048 bytes (one sector) */
- tcpa_add_bootdevice(1, 0);
- tcpa_ipl(IPL_EL_TORITO_2, GET_SEG(SS), (u32)buffer, 2048);
Same here for tcpa_ipl(). GET_SEG() always returns 0 in 32bit mode and this code is only run in 32bit mode.
[...]
+static struct smbios_entry_point * +find_smbios_entry_point(void) +{
The smbios table is built in smbios.c:smbios_entry_point_init() - instead of scanning for the table, just record where the table is in a global variable.
[...]
+u32 +tcpa_smbios_measure(void) +{
- u32 rc;
- struct pcctes pcctes = {
.eventid = 1, /* 10.4.2.3.1 */.eventdatasize = SHA1_BUFSIZE,- };
- struct smbios_entry_point *sep = find_smbios_entry_point();
- if (!has_working_tpm())
return 0;
BTW - SeaBIOS uses C99, so it's fine to move "if (!has_working_tpm())" above the variable declarations.
-Kevin