"Kevin O'Connor" <[email protected]> wrote on 01/08/2016 01:05:05 PM:
> From: "Kevin O'Connor" <[email protected]> > To: Stefan Berger/Watson/IBM@IBMUS > Cc: "[email protected]" <[email protected]>, Stefan Berger > <[email protected]> > Date: 01/08/2016 01:05 PM > Subject: Re: [SeaBIOS] SeaBIOS Digest, Vol 72, Issue 33 > > On Fri, Jan 08, 2016 at 12:19:52PM -0500, Stefan Berger wrote: > > "Kevin O'Connor" <[email protected]> wrote on 01/08/2016 11:41:13 AM: > > > On Thu, Jan 07, 2016 at 03:39:13PM -0500, Stefan Berger wrote: > > > > "Kevin O'Connor" <[email protected]> wrote on 01/07/2016 03:14:37 PM: > > > > > I don't have input on what TPM2 organization should look like, > > mainly > > > > > because I don't know what TPM2 entails. I gather the TIS commands > > are > > > > > changing, but what else changes? Does the ACPI log, BIOS interface, > > > > > or tpm menu change? Do you have a pointer to the TPM2 spec (when I > > > > > last looked it seemed that TPM2 was still being worked on). > > > > > > > > The TIS got more registers; some flags allow detection of the TPM > > version. > > > > > > > > All commands changed -- no backwards compatibility. The header > > 'fields' > > > > are the same, their ordinal and tag values are not. > > > > > > > > Spec: > > > > > > http://www.trustedcomputinggroup.org/resources/tpm_library_specification > > > > > > Thanks. Does the hardware interface change as well (ie, is it still > > > the same reads/writes to MMIO at 0xfed40000)? > > > > > > > It has the same address, but one or two more registers. > > Does it require a different tpm_drivers.c implementation - with > something like tpmhw1_transmit() and tpmhw2_transmit() functions? No. Only an extension to the probing function that interprets the flags from the new registers to determine TPM1.2 or TPM2. > > > > My initial thought would be to do what you've proposed - have wrapper > > > functions around the TPM commands (eg, tpm_extend, tpm_get_capability, > > > read_permanent_flags) and teach those functions how to send the two > > > different styles of commands (and translate the responses if > > > necessary). > > > > So the good thing is that some of the code can be shared between 1.2 and > > 2.0, > > to a certain 'depth' at least. An example of a shared function would be > > this one. > > > > static void > > tpm_add_event_separators(void) > > { > > static const u8 evt_separator[] = {0xff,0xff,0xff,0xff}; > > u32 pcrIndex; > > for (pcrIndex = 0; pcrIndex <= 7; pcrIndex++) > > tpm_add_measurement_to_log(pcrIndex, EV_SEPARATOR, > > NULL, 0, > > evt_separator, > > sizeof(evt_separator)); > > } > > > > Following this function further down: > > > > tpm_add_measurement_to_log() [on current master] can be completely > > shared as well. tpm_log_extend_event would need to become a function that > > branches into tpm12_log_extend_event and tpm2_log_extend_event, depending > > on detected version of TPM. > > Sounds like a new tpm_extend() function could be made with just the > hardware command. And then it could handle the v1 and v2 cases and > thus reduce the amount of duplicated code. Yes, there should be a tpm_extend() function branching into tpm12_extend() and tpm2_extend(). > > > tpm_log_event could again be shared since ACPI logging is the same. > > Same for tpm_fill_hash for as long as we only support sha1. > > > > Basically all functions where commands are created cannot be shared. > > Also TPM 2's initialization is a bit different and it supports more > > hashes. > > If the init is notable different maybe just do tpm1_startup() and > tpm2_startup()? Yes, something like that. > > > So it actually speaks against splitting this up into different files, but > > the > > outcome may be that the code would show a mix of tpm12_*, tpm2_*, and > > tpm_* functions in the format of > > > > tpm12_foo() { [...] } > > > > tpm2_foo() { [...] } > > > > tpm_foo() { > > switch (tpmversion) { > > 1.2: > > return tpm12_foo() > > 2: > > return tpm2_foo() > > } > > } > > Okay. But, I would say that if both tpm12_foo() and tpm2_foo() are > both only a few lines that it might be better to just inline it all > into tpm_foo(). An 'if' might be more succinct also - hopefully there > isn't a tpm3 in the works.. Not that I know of :-) > > > tpm_xyz() { [...] } > > > > tpm12_bar() { [...] } > > > > tpm2_bar() { [...] } > > > > [...] > > > > That's what I did before... > > Oh, were there tpm2 patches available? I must have missed them. No, I never showed them. Stefan > > > If none of the code could be shared the decision to split it up completely > > would be a lot easier. > > Agreed. > > -Kevin >
_______________________________________________ SeaBIOS mailing list [email protected] http://www.seabios.org/mailman/listinfo/seabios
