Re: [PATCH 2/2] ieee1275: Add TCG2 driver for ieee1275 PowerPC firmware

[Daniel Kiper]

On Tue, Nov 12, 2024 at 01:04:39PM -0500, Stefan Berger wrote:
> Follow recent extensions of EFI support providing a TCG2 driver with a
> public API for getting the maximum TPM command size and passing a TPM
> command through to the TPM 2. Implement this functionality using
> ieee1275 PowerPC firmware API calls.
>
> Move common initialization functions from the ibmvtpm driver module into
> the new built-in TCG2 driver. Make them available to the ibmvtpm driver as
> public functions and variables.
>
> The firmware support is readily available on pSeries platforms. For SLOF
> the new API calls have been recently merged upstream and will become
> available after QEMU 9.2.
>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
>  grub-core/Makefile.core.def           |   3 +
>  grub-core/commands/ieee1275/ibmvtpm.c |  46 +------
>  grub-core/lib/ieee1275/tcg2.c         | 167 ++++++++++++++++++++++++++
>  include/grub/ieee1275/tpm.h           |  31 +++++
>  4 files changed, 205 insertions(+), 42 deletions(-)
>  create mode 100644 grub-core/lib/ieee1275/tcg2.c
>  create mode 100644 include/grub/ieee1275/tpm.h
>
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 40427165e..f70e02e69 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -2575,8 +2575,10 @@ module = {
>    common = lib/tss2/tss2.c;
>    efi = lib/efi/tcg2.c;
>    emu = lib/tss2/tcg2_emu.c;
> +  powerpc_ieee1275 = lib/ieee1275/tcg2.c;
>    enable = efi;
>    enable = emu;
> +  enable = powerpc_ieee1275;
>    cppflags = '-I$(srcdir)/lib/tss2';
>  };
>
> @@ -2589,6 +2591,7 @@ module = {
>    /* The plaform support of tpm2_key_protector depends on the tcg2 
> implementation in tss2. */
>    enable = efi;
>    enable = emu;
> +  enable = powerpc_ieee1275;
>    cppflags = '-I$(srcdir)/lib/tss2 -I$(srcdir)/lib/libtasn1-grub';
>  };
>
> diff --git a/grub-core/commands/ieee1275/ibmvtpm.c 
> b/grub-core/commands/ieee1275/ibmvtpm.c
> index a6fee5c51..284673217 100644
> --- a/grub-core/commands/ieee1275/ibmvtpm.c
> +++ b/grub-core/commands/ieee1275/ibmvtpm.c
> @@ -23,48 +23,10 @@
>  #include <grub/types.h>
>  #include <grub/tpm.h>
>  #include <grub/ieee1275/ieee1275.h>
> +#include <grub/ieee1275/tpm.h>
>  #include <grub/mm.h>
>  #include <grub/misc.h>
>
> -static grub_ieee1275_ihandle_t tpm_ihandle;
> -static grub_uint8_t tpm_version;
> -
> -#define IEEE1275_IHANDLE_INVALID ((grub_ieee1275_ihandle_t) 0)
> -
> -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"))
> -    tpm_version = 2;
> -}
> -
> -static grub_err_t
> -tpm_init (void)
> -{
> -  static int init_success = 0;
> -
> -  if (!init_success)
> -    {
> -      if (grub_ieee1275_open ("/vdevice/vtpm", &tpm_ihandle) < 0)
> -     {
> -       tpm_ihandle = IEEE1275_IHANDLE_INVALID;
> -       return GRUB_ERR_UNKNOWN_DEVICE;
> -     }
> -
> -      init_success = 1;
> -
> -      tpm_get_tpm_version ();
> -    }
> -
> -  return GRUB_ERR_NONE;
> -}
> -
>  static int
>  ibmvtpm_2hash_ext_log (grub_uint8_t pcrindex,
>                      grub_uint32_t eventtype,
> @@ -90,7 +52,7 @@ ibmvtpm_2hash_ext_log (grub_uint8_t pcrindex,
>
>    INIT_IEEE1275_COMMON (&args.common, "call-method", 8, 2);
>    args.method = (grub_ieee1275_cell_t) "2hash-ext-log";
> -  args.ihandle = tpm_ihandle;
> +  args.ihandle = grub_ieee1275_tpm_ihandle;
>    args.pcrindex = pcrindex;
>    args.eventtype = eventtype;
>    args.description = (grub_ieee1275_cell_t) description;
> @@ -138,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 (tpm_version == 2)
> +  if (grub_ieee1275_tpm_version == 2)
>      return tpm2_log_event (buf, size, pcr, description);
>
>    return GRUB_ERR_NONE;
> @@ -151,5 +113,5 @@ grub_tpm_present (void)
>     * Call tpm_init() "late" rather than from GRUB_MOD_INIT() so that device 
> nodes
>     * can be found.
>     */
> -  return tpm_init() == GRUB_ERR_NONE;
> +  return grub_ieee1275_tpm_init() == GRUB_ERR_NONE;
>  }
> diff --git a/grub-core/lib/ieee1275/tcg2.c b/grub-core/lib/ieee1275/tcg2.c
> new file mode 100644
> index 000000000..cc7232099
> --- /dev/null
> +++ b/grub-core/lib/ieee1275/tcg2.c
> @@ -0,0 +1,167 @@
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2024 IBM Corporation
> + *  Copyright (C) 2024 Free Software Foundation, Inc.
> + *
> + *  GRUB is free software: you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation, either version 3 of the License, or
> + *  (at your option) any later version.
> + *
> + *  GRUB is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <grub/types.h>
> +#include <grub/tpm.h>
> +#include <grub/ieee1275/tpm.h>
> +#include <grub/mm.h>
> +#include <grub/misc.h>
> +
> +#include <tcg2.h>
> +
> +grub_ieee1275_ihandle_t grub_ieee1275_tpm_ihandle;
> +grub_uint8_t grub_ieee1275_tpm_version;
> +
> +#define IEEE1275_IHANDLE_INVALID ((grub_ieee1275_ihandle_t) 0)
> +
> +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;

I think this should be a part of the grub_ieee1275_tpm_init()...

> +}
> +
> +grub_err_t
> +grub_ieee1275_tpm_init (void)
> +{
> +  static int init_success = 0;
> +
> +  if (!init_success)
> +    {
> +      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 ();

... and probably the grub_ieee1275_tpm_init() should fail if TPM != 2.0...

If you agree with my suggestion you should do move and code change in
separate patches.

> +    }
> +
> +  return GRUB_ERR_NONE;
> +}
> +
> +grub_err_t
> +grub_tcg2_get_max_output_size (grub_size_t *size)

This function seems unused. If it is true please drop it.

> +{
> +  struct tpm_get_maximum_cmd_size
> +  {
> +    struct grub_ieee1275_common_hdr common;
> +    grub_ieee1275_cell_t method;
> +    grub_ieee1275_cell_t ihandle;
> +    grub_ieee1275_cell_t catch_result;
> +    grub_ieee1275_cell_t size;
> +  };
> +  struct tpm_get_maximum_cmd_size args;
> +  static int error_displayed = 0;
> +  grub_err_t err;
> +
> +  err = grub_ieee1275_tpm_init ();
> +  if (err != GRUB_ERR_NONE)
> +      return err;
> +
> +  INIT_IEEE1275_COMMON (&args.common, "call-method", 2, 2);
> +  args.method = (grub_ieee1275_cell_t) "get-maximum-cmd-size";
> +  args.ihandle = grub_ieee1275_tpm_ihandle;
> +
> +  if (IEEE1275_CALL_ENTRY_FN (&args) == -1)
> +    return GRUB_ERR_INVALID_COMMAND;
> +
> +  /*
> +   * catch_result is set if firmware does not support get-maximum-cmd-size
> +   * rc is GRUB_IEEE1275_CELL_FALSE (0) on failure
> +   */
> +  if (args.catch_result)
> +    {
> +      if (!error_displayed)
> +     {
> +       error_displayed++;
> +       return grub_error (GRUB_ERR_BAD_DEVICE,
> +                          "get-maximum-cmd-size failed: Firmware is likely 
> too old.\n");
> +     }
> +      return GRUB_ERR_INVALID_COMMAND;
> +    }
> +
> +  *size = args.size;
> +
> +  return GRUB_ERR_NONE;
> +}
> +
> +grub_err_t
> +grub_tcg2_submit_command (grub_size_t input_size,
> +                       grub_uint8_t *input,
> +                       grub_size_t output_size,
> +                       grub_uint8_t *output)

I think you should separate this function addition from earlier code
shuffling and update, i.e, I want separate patch for it.

> +{
> +  struct tpm_pass_through_to_tpm
> +  {
> +    struct grub_ieee1275_common_hdr common;
> +    grub_ieee1275_cell_t method;
> +    grub_ieee1275_cell_t ihandle;
> +    grub_ieee1275_cell_t buf_size;
> +    grub_ieee1275_cell_t buf_addr;
> +    grub_ieee1275_cell_t catch_result;
> +    grub_ieee1275_cell_t resp_size;
> +  };
> +  struct tpm_pass_through_to_tpm args;
> +  static int error_displayed = 0;
> +  grub_err_t err;
> +
> +  if (input_size == 0 || input == NULL ||
> +      output_size == 0 || output == NULL)
> +    return GRUB_ERR_BAD_ARGUMENT;
> +
> +  err = grub_ieee1275_tpm_init ();
> +  if (err != GRUB_ERR_NONE)
> +      return err;
> +
> +  INIT_IEEE1275_COMMON (&args.common, "call-method", 4, 2);
> +  args.method = (grub_ieee1275_cell_t) "pass-through-to-tpm";
> +  args.ihandle = grub_ieee1275_tpm_ihandle;
> +  args.buf_size = (grub_ieee1275_cell_t) input_size;
> +  args.buf_addr = (grub_ieee1275_cell_t) input;
> +
> +  if (IEEE1275_CALL_ENTRY_FN (&args) == -1)
> +    return GRUB_ERR_INVALID_COMMAND;
> +
> +  /*
> +   * catch_result is set if firmware does not support pass-through-to-tpm
> +   */
> +  if (args.catch_result)
> +    {
> +      if (!error_displayed)
> +     {
> +       error_displayed++;
> +       return grub_error (GRUB_ERR_BAD_DEVICE,
> +                          "pass-through-to-tpm failed: Firmware is likely 
> too old.\n");
> +     }
> +      return GRUB_ERR_INVALID_COMMAND;
> +    }
> +
> +  grub_memcpy (output, input, args.resp_size);
> +
> +  return GRUB_ERR_NONE;
> +}
> diff --git a/include/grub/ieee1275/tpm.h b/include/grub/ieee1275/tpm.h
> new file mode 100644
> index 000000000..e3a7eccbf
> --- /dev/null
> +++ b/include/grub/ieee1275/tpm.h
> @@ -0,0 +1,31 @@
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2019  Free Software Foundation, Inc.
> + *
> + *  GRUB is free software: you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation, either version 3 of the License, or
> + *  (at your option) any later version.
> + *
> + *  GRUB is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef GRUB_IEEE1275_TPM_HEADER
> +#define GRUB_IEEE1275_TPM_HEADER      1
> +
> +#include <grub/err.h>
> +#include <grub/types.h>
> +#include <grub/ieee1275/ieee1275.h>
> +
> +extern grub_ieee1275_ihandle_t grub_ieee1275_tpm_ihandle;
> +extern grub_uint8_t grub_ieee1275_tpm_version;
> +
> +grub_err_t grub_ieee1275_tpm_init (void);

extern please...

Daniel