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