[SeaBIOS] [PATCH 0/7] Some TPM code reorganization
stefanb at linux.vnet.ibm.com
Mon Nov 23 05:11:36 CET 2015
On 11/22/2015 08:26 PM, Kevin O'Connor wrote:
> On Sun, Nov 22, 2015 at 06:54:47PM -0500, Stefan Berger wrote:
>> On 11/22/2015 12:20 PM, Kevin O'Connor wrote:
>>> Hi Stefan,
>>> As part of trying to better understand the TPM code, I found some
>>> changes to tcgbios.c were helpful. It's mostly code movement. I've
>>> only compile tested these changes, but I think they are safe.
>>> I don't want to conflict with any changes you may have pending. So,
>>> let me know if this is a problem.
>> I do have other changes and extensions pending, but go ahead and apply it.
>>> The series is on top of your latest patches. (FYI, I made a minor
>>> change to a comment in your patch 2.) I've also put this series (and
>>> your series) up at:
>> I applied them locally. From what i can see from tests I have done, results
>> are still like before.
>> ACK to series.
> Thanks. I ran across a few other things - I sent an email series and
> put the changes up on github.
> There were a few other things I noticed, but that I have not tried
> - It's odd that "Booting from CD ROM device" is added twice for cdrom
> boots. Is that intentional?
That is not necessary. We can remove one of these log messages.
> - It's odd that is_tpm_present() is called from
> tpm_interrupt_handler32() , as that probes the hardware. I would
> think that if the has_working_tpm() check in
> tpm_interrupt_handler32() passes then is_tpm_present() would always
> - It seems the "TPM hardware interface" group of functions could be
> moved to tpm_drivers.c . Doing that seems like it could simpify the
> software/hardware interface as only a handful of functions would
> need to be exported (instead of the function table that
> tpm_drivers.c currently exports). Could tpm_drivers.c just export
> something like: tpmhw_probe(), tpmhw_set_timeouts(),
> tpmhw_transmit() ?
> I will hold off on further changes until you can merge any features
> you have pending.
It's mostly the menu that I would want to have merged next. All patches
after that are not that important and I'll only resurrect them once the
changes here settle.
I looked over and tried your 2nd series of 8 patches and the tests I did
all had the same results as without these patches applied. So from my
point of view, they can be applied.
I leave it up to you regarding 4/8 and the fixed-size buffer on the
stack. If we ever have a bigger message to pass then we can adjust the
buffer following the warning the code is emitting, otherwise if we pass
the event and its size around we won't have to do that. If stack usage
is not an issue I think your changes in probably the right direction.
All the arrays at the beginning of the file are basically the bodies of
TPM requests. We could extend those arrays by prefixing them with their
headers and won't have to build the header in the transmit function
anymore. Some messages will still need to be built, like TPM_Extend, but
those can be hard-coded. Is that a change we would want?
I am also trying to add similar code to SLOF, which is under BSD. I hope
that you agree that I can make similar transformations that you did to
the pending SLOF patches as well.
Thanks and regards,
More information about the SeaBIOS