I've been reading through the tcgbios and tpm_drivers code in
SeaBIOS. I have a couple of questions:
Why does the driver sometimes use tpm_sha1_calc() and sometimes use
sha1()? It seems the software sha1 implementation is always superior,
so why bothering implementing the hardware version? (The spec seems
to agree with this as well.) It seems like dropping tpm_sha1_calc()
would simplify the code.
What is SCALAR in tpm_drivers() for - it seems like all the timeouts
in the spec are increased by 10? Also, it seems like tpm_drivers.c
uses durations and timeouts in milliseconds, while
tcgbios.c:determine_timeouts() uses values in microseconds.
I don't understand the error handling in tpm_extend_acpi_log() and
tpm_extend(). Why does a log overflow in tpm_extend_acpi_log()
shutdown the tpm chip (via tpm_set_failure() )? In particular,
tpm_extend_acpi_log can be called from clients via the 16bit BIOS
interface, and it's the only way a client could cause the tpm chip to
shutdown. Why does tpm_extend() call reset_acpi_log() on failure? It
seems odd that a failure in communication with the TPM chip would
result in an ACPI log reset - no other TPM chip failure does that.
Is it expected that the tcpa ACPI table could move or be modified at
runtime? The code rescans for the table twice on every call to
tpm_extend_acpi_log() - if it can't move or be modified then I think
it would be simpler to cache the values.
On Thu, Nov 19, 2015 at 07:44:27PM -0500, Stefan Berger wrote:
> "Kevin O'Connor" <kevin(a)koconnor.net> wrote on 11/19/2015 07:07:27 PM:
> > From: "Kevin O'Connor" <kevin(a)koconnor.net>
> > On Thu, Nov 19, 2015 at 06:33:59PM -0500, Stefan Berger wrote:
> > > Let me know how to proceed on the menu part. Do you agree with the
> > > code I had posted module the fact that once we leave the menu you want
> > > machine to be reset? It is not necessary to do that, but we can do it.
> > > From a TPM perspective machine resets are only necessary when certain
> > > state changes are done.
> > It's been a while since I last looked at that. My only real concern
> > is minimizing the impact to the non-tpm seabios code. I'd prefer not
> > to change the main SeaBIOS boot prompt, and adding loops (or loops via
> Though adding an additional boot *menu item* is ok?
> > gotos) to the boot menu code seems strange to me. Outside of that, I
> Once the user selects the TPM submenu and then leaves it, what should
> happen? If we reset the machine, we wouldn't need to add a loop because
> there is no way to go back to the main menu. Otherwise getting back into
> the previous menu seem 'normal'.
I don't understand what you are asking.
It seems to me selecting the TPM config from the boot menu could be
considered "booting into the TPM configuration" instead of selecting
the "TPM submenu". When treated as "booting into the TPM
configuration" then at the completion of configuration the next
natural step would be to reboot. This is similar to how a traditional
BIOS handles its "BIOS setup menu" - the machine reboots upon exiting
the setup menu.
I'm sure there are other ways to do this, but it does seem strange to
me to restructure the boot menu for something that I understand is
going to be very infrequently accessed.
On Thu, Nov 19, 2015 at 06:33:59PM -0500, Stefan Berger wrote:
> Let me know how to proceed on the menu part. Do you agree with the last
> code I had posted module the fact that once we leave the menu you want the
> machine to be reset? It is not necessary to do that, but we can do it.
> From a TPM perspective machine resets are only necessary when certain TPM
> state changes are done.
It's been a while since I last looked at that. My only real concern
is minimizing the impact to the non-tpm seabios code. I'd prefer not
to change the main SeaBIOS boot prompt, and adding loops (or loops via
gotos) to the boot menu code seems strange to me. Outside of that, I
don't have a preference on whether TPM resets the machine or not.
On Thu, Nov 19, 2015 at 08:00:27PM +0000, Stefan Berger wrote:
> "Kevin O'Connor" <kevin(a)koconnor.net> wrote on 11/19/2015 09:33:09 AM:
> > Signed-off-by: Kevin O'Connor <kevin(a)koconnor.net>
> Tested-by: Stefan Berger <stefanb(a)us.ibm.com>
Thanks. It looks like I inadvertently moved "enum ipltype" from
tcgbios.h to std/tcg.h . I moved it back in my local copy.
On Thu, Nov 19, 2015 at 02:59:43PM +0100, Jenkins Build Host wrote:
> See <http://jenkins.xeni.kraxel.org:8080/job/seabios/198/changes>
> [kevin] acpi: Don't build SSDT files on every build; store them in git
> Total size: 130700 Fixed: 58336 Free: 372 (used 99.7% of 128KiB rom)
> Creating out/bios.bin
> + make EXTRAVERSION=-16.b198.gdc8eb67-git-snapshot-by-kraxel.org out/bios.bin
> make: `out/bios.bin' is up to date.
> + cp out/bios.bin rpm.bin/bios-qemu.bin
> + cp 'out/src/fw/*dsdt*.aml' rpm.bin/
> cp: cannot stat 'out/src/fw/*dsdt*.aml': No such file or directory
> error: Bad exit status from /var/tmp/rpm-tmp.PsDRHn (%build)
Looks like the rpm build has a dependency on the acpi-dsdt.aml and
I'd prefer not to require iasl for every seabios build. We could
create a new makefile target (eg, make aml) and then update the rpm to
call both "make" and "make aml".
It's been over 2 years since the last meaningful change to the DSDT
files in the SeaBIOS repo. (And that's with good cause, as the
content is now only used by old QEMU machine types and it should be
static.) It does not make sense to require iasl on every build to
generate these files. Instead, it's simpler to just commit the four
generated files into the code repo.
Kevin O'Connor (5):
acpi_extract: Move main code to new function main()
acpi_extract: Make the generated .hex files more human readable
acpi_extract: Don't generate unused (and empty) q35-acpi-dsdt.hex file
acpi: Don't build SSDT files on every build; store them in git
acpi: Remove build check for iasl
Makefile | 4 +-
scripts/acpi_extract.py | 232 ++++++++++----------
scripts/test-build.sh | 13 --
src/fw/acpi-dsdt.hex | 554 ++++++++++++++++++++++++++++++++++++++++++++++++
src/fw/acpi.c | 8 +-
src/fw/ssdt-misc.hex | 88 ++++++++
src/fw/ssdt-pcihp.hex | 38 ++++
src/fw/ssdt-proc.hex | 35 +++
8 files changed, 843 insertions(+), 129 deletions(-)
create mode 100644 src/fw/acpi-dsdt.hex
create mode 100644 src/fw/ssdt-misc.hex
create mode 100644 src/fw/ssdt-pcihp.hex
create mode 100644 src/fw/ssdt-proc.hex