On Mon, Nov 25, 2024 at 05:41:40PM -0500, Stefan Berger wrote:
> Move tpm_get_tpm_version into grub_ieee1275_tpm_init and invalidate
s/tpm_get_tpm_version/tpm_get_tpm_version()/
s/grub_ieee1275_tpm_init/grub_ieee1275_tpm_init()/
> grub_ieee1275_tpm_ihandle in case no TPM 2 could be detected. Try the
> initialization only once. Use the grub_ieee1275_tpm_ihandle as
Could you explain why you change logic here? I am not against that but
I think it should be clarified why...
> indicator for an available TPM instead of grub_ieee1275_tpm_version,
> which can now be removed.
>
> Signed-off-by: Stefan Berger <stef...@linux.ibm.com>
> ---
> grub-core/commands/ieee1275/ibmvtpm.c | 2 +-
> grub-core/lib/ieee1275/tcg2.c | 43 +++++++++++----------------
> include/grub/ieee1275/tpm.h | 1 -
> 3 files changed, 18 insertions(+), 28 deletions(-)
>
> diff --git a/grub-core/commands/ieee1275/ibmvtpm.c
> b/grub-core/commands/ieee1275/ibmvtpm.c
> index 284673217..16552b136 100644
> --- a/grub-core/commands/ieee1275/ibmvtpm.c
> +++ b/grub-core/commands/ieee1275/ibmvtpm.c
> @@ -100,7 +100,7 @@ grub_tpm_measure (unsigned char *buf, grub_size_t size,
> grub_uint8_t pcr,
> grub_dprintf ("tpm", "log_event, pcr = %d, size = 0x%" PRIxGRUB_SIZE ",
> %s\n",
> pcr, size, description);
>
> - if (grub_ieee1275_tpm_version == 2)
> + if (grub_ieee1275_tpm_ihandle != IEEE1275_IHANDLE_INVALID)
> return tpm2_log_event (buf, size, pcr, description);
>
> return GRUB_ERR_NONE;
> diff --git a/grub-core/lib/ieee1275/tcg2.c b/grub-core/lib/ieee1275/tcg2.c
> index 8c4306ae8..8f29b3c1e 100644
> --- a/grub-core/lib/ieee1275/tcg2.c
> +++ b/grub-core/lib/ieee1275/tcg2.c
> @@ -23,39 +23,30 @@
> #include <grub/mm.h>
> #include <grub/misc.h>
>
> -grub_ieee1275_ihandle_t grub_ieee1275_tpm_ihandle;
> -grub_uint8_t grub_ieee1275_tpm_version;
> -
> -static void
> -tpm_get_tpm_version (void)
> -{
> - grub_ieee1275_phandle_t vtpm;
> - char buffer[20];
> -
> - if (!grub_ieee1275_finddevice ("/vdevice/vtpm", &vtpm) &&
> - !grub_ieee1275_get_property (vtpm, "compatible", buffer,
> - sizeof (buffer), NULL) &&
> - !grub_strcmp (buffer, "IBM,vtpm20"))
> - grub_ieee1275_tpm_version = 2;
> -}
> +grub_ieee1275_ihandle_t grub_ieee1275_tpm_ihandle = IEEE1275_IHANDLE_INVALID;
>
> grub_err_t
> grub_ieee1275_tpm_init (void)
> {
> - static int init_success = 0;
> + grub_ieee1275_phandle_t vtpm;
> + static int init_tried = 0;
We have a bool in the GRUB. Please use it...
> + char buffer[20];
>
> - if (!init_success)
> + if (!init_tried)
> {
> - if (grub_ieee1275_open ("/vdevice/vtpm", &grub_ieee1275_tpm_ihandle) <
> 0)
> - {
> - grub_ieee1275_tpm_ihandle = IEEE1275_IHANDLE_INVALID;
> - return GRUB_ERR_UNKNOWN_DEVICE;
> - }
> -
> - init_success = 1;
> -
> - tpm_get_tpm_version ();
> + init_tried = 1;
> +
> + if (grub_ieee1275_open ("/vdevice/vtpm",
> + &grub_ieee1275_tpm_ihandle) < 0 ||
You do not need wrap this line...
> + grub_ieee1275_finddevice ("/vdevice/vtpm", &vtpm) ||
> + grub_ieee1275_get_property (vtpm, "compatible", buffer,
> + sizeof (buffer), NULL) ||
Same here...
> + grub_strcmp (buffer, "IBM,vtpm20"))
Should not you call grub_ieee1275_close() before invalidating
grub_ieee1275_tpm_ihandle?
> + grub_ieee1275_tpm_ihandle = IEEE1275_IHANDLE_INVALID;
> }
>
> + if (grub_ieee1275_tpm_ihandle == IEEE1275_IHANDLE_INVALID)
> + return GRUB_ERR_UNKNOWN_DEVICE;
> +
> return GRUB_ERR_NONE;
> }
Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
