[SeaBIOS] [Qemu-devel] [PATCH V1 6/8] Add measurement code to the BIOS

[Kevin O'Connor]

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