Hi
again many thanks for the quick review
On Sat, 22 Jun 2024 at 19:25, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 22.06.24 16:35, Ilias Apalodimas wrote:
commit 97707f12fdab ("tpm: Support boot measurements") moved out code from the EFI subsystem into the TPM one to support measurements when booting with !EFI.
Those were moved directly into the TPM subsystem and in the tpm-v2.c library. In hindsight, it would have been better to move it in new files since the TCG2 is governed by its own spec and it's cleaner when we want to enable certian parts of the TPM functionality.
Nits:
%s/certian/certain/
So let's create a header file and another library and move the TCG specific bits there.
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
boot/bootm.c | 1 + include/efi_tcg2.h | 1 + include/tpm-v2.h | 474 +++++------------------------- include/tpm_tcg2.h | 336 ++++++++++++++++++++++ lib/Makefile | 2 + lib/tpm-v2.c | 676 +------------------------------------------ lib/tpm_tcg2.c | 696 +++++++++++++++++++++++++++++++++++++++++++++
The patch series were easier to review if moving header definitions were separated from moving implementations.
This patch contains changes that are not described in the commit message, e.g.
if (elog->log_size) { if (log.found) { if (elog->log_size < log.log_position)
return -ENOBUFS;
return -ENOSPC;
And this is a great catch. this changed in patch#1 and the correct return is -ENOBUFS. I started working on 2 trees and obviously messed up this rebase... Thanks!
I guess you wanted to put this into patch 1.
Please, separate the patches adequately.
Fair enough. I thought it was going to be hard not breaking compilation hence the big patch. I'll try splitting it
- Copyright (c) 2020 Linaro
- Copyright (c) 2023 Linaro Limited
The copyright lines look inconsistent. Linaro Limited exists under this name since April 13th, 2010. Is the 2020 copyright for a different company?
I'll keep the older one, same company
[...]
+}
+__weak void tcg2_platform_startup_error(struct udevice *dev, int rc) {}
git warning: "new blank line at EOF".
Otherwise looks good.
Best regards
Heinrich
Thanks /Ilias