[PATCH v5 3/3] tpm: get tpm event log from bloblist

[Ilias Apalodimas]

On Wed, 15 Jan 2025 at 03:17, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Raymond,
>
> On Tue, 14 Jan 2025 at 08:20, Raymond Mao <raymond.mao at linaro.org> wrote:
> >
> > Get tpm event log from bloblist instead of FDT when bloblist is
> > enabled and valid from previous boot stage.
> >
> > As a fallback, when no event log from previous stage is observed
> > and no user buffer is passed, malloc a default buffer to initialize
> > the event log.
> >
> > Signed-off-by: Raymond Mao <raymond.mao at linaro.org>
> > ---
> > Changes in v2
> > - Remove patch dependency.
> > - Remove the fallback to FDT when BLOBLIST is selected.
> > Changes in v3
> > - Malloc an 8KB buffer when user eventlog buffer does not exist.
> > Changes in v4
> > - Replace the default eventlog size with TPM2_EVENT_LOG_SIZE.
> > Changes in v5
> > - Replace TPM2_EVENT_LOG_SIZE with CONFIG_TPM2_EVENT_LOG_SIZE.
> > - Add an inline TODO comment.
> >
> >  lib/tpm_tcg2.c | 61 ++++++++++++++++++++++++++++++++++----------------
> >  1 file changed, 42 insertions(+), 19 deletions(-)
>
> Reviewed-by: Simon Glass <sjg at chromium.org>
>
> nit in case you respin again: -ENODEV is for driver model, when there
> is no device. It is actually handling specially by driver model. So
> -ENOENT is what I try to use for things that don't exist but aren't
> devices.
>

+1,


> >
> > diff --git a/lib/tpm_tcg2.c b/lib/tpm_tcg2.c
> > index 4134d93a35..2d9076f091 100644
> > --- a/lib/tpm_tcg2.c
> > +++ b/lib/tpm_tcg2.c
> > @@ -5,6 +5,7 @@
> >
> >  #include <dm.h>
> >  #include <dm/of_access.h>
> > +#include <malloc.h>
> >  #include <tpm_api.h>
> >  #include <tpm-common.h>
> >  #include <tpm-v2.h>
> > @@ -19,6 +20,7 @@
> >  #include <linux/unaligned/generic.h>
> >  #include <linux/unaligned/le_byteshift.h>
> >  #include "tpm-utils.h"
> > +#include <bloblist.h>
> >
> >  int tcg2_get_pcr_info(struct udevice *dev, u32 *supported_bank, u32 *active_bank,
> >                       u32 *bank_num)
> > @@ -615,15 +617,24 @@ int tcg2_log_prepare_buffer(struct udevice *dev, struct tcg2_event_log *elog,
> >                 elog->found = log.found;
> >         }
> >
> > +       if (elog->found)
> > +               return 0;
> > +
> >         /*
> > -        * Initialize the log buffer if no log was discovered and the buffer is
> > -        * valid. User's can pass in their own buffer as a fallback if no
> > -        * memory region is found.
> > +        * Initialize the log buffer if no log was discovered.
> > +        * User can pass in their own buffer as a fallback if no memory region
> > +        * is found, else malloc a buffer if it does not exist.
> >          */
> > -       if (!elog->found && elog->log_size)
> > -               rc = tcg2_log_init(dev, elog);
> > +       if (!elog->log_size) {
> > +               elog->log = malloc(CONFIG_TPM2_EVENT_LOG_SIZE);
> > +               if (!elog->log)
> > +                       return -ENOMEM;
> > +
> > +               memset(elog->log, 0, CONFIG_TPM2_EVENT_LOG_SIZE);
> > +               elog->log_size = CONFIG_TPM2_EVENT_LOG_SIZE;

Why are you doing this? There are 2 ways to pass an EventLog to the
kernel. Either via a config table from EFI, or using sml-base and
sml-size in a DT (which are both required). IOW you will create an
EventLog no one will discover

Cheers
/Ilias


> > +       }
> >
> > -       return rc;
> > +       return tcg2_log_init(dev, elog);
> >  }
> >
> >  int tcg2_measurement_init(struct udevice **dev, struct tcg2_event_log *elog,
> > @@ -676,10 +687,25 @@ __weak int tcg2_platform_get_log(struct udevice *dev, void **addr, u32 *size)
> >         const __be32 *size_prop;
> >         int asize;
> >         int ssize;
> > +       struct ofnode_phandle_args args;
> > +       phys_addr_t a;
> > +       fdt_size_t s;
> >
> >         *addr = NULL;
> >         *size = 0;
> >
> > +       *addr = bloblist_get_blob(BLOBLISTT_TPM_EVLOG, size);
> > +       if (*addr && *size)
> > +               return 0;
> > +       /*
> > +        * TODO:
> > +        * replace BLOBLIST with a new kconfig for handoff all components
> > +        * (fdt, tpm event log, etc...) from previous boot stage via bloblist
> > +        * mandatorily following Firmware Handoff spec.
> > +        */
> > +       else if (CONFIG_IS_ENABLED(BLOBLIST))
> > +               return -ENODEV;
> > +
> >         addr_prop = dev_read_prop(dev, "tpm_event_log_addr", &asize);
> >         if (!addr_prop)
> >                 addr_prop = dev_read_prop(dev, "linux,sml-base", &asize);
> > @@ -694,22 +720,19 @@ __weak int tcg2_platform_get_log(struct udevice *dev, void **addr, u32 *size)
> >
> >                 *addr = map_physmem(a, s, MAP_NOCACHE);
> >                 *size = (u32)s;
> > -       } else {
> > -               struct ofnode_phandle_args args;
> > -               phys_addr_t a;
> > -               fdt_size_t s;
> >
> > -               if (dev_read_phandle_with_args(dev, "memory-region", NULL, 0,
> > -                                              0, &args))
> > -                       return -ENODEV;
> > +               return 0;
> > +       }
> >
> > -               a = ofnode_get_addr_size(args.node, "reg", &s);
> > -               if (a == FDT_ADDR_T_NONE)
> > -                       return -ENOMEM;
> > +       if (dev_read_phandle_with_args(dev, "memory-region", NULL, 0, 0, &args))
> > +               return -ENODEV;
> >
> > -               *addr = map_physmem(a, s, MAP_NOCACHE);
> > -               *size = (u32)s;
> > -       }
> > +       a = ofnode_get_addr_size(args.node, "reg", &s);
> > +       if (a == FDT_ADDR_T_NONE)
> > +               return -ENOMEM;
> > +
> > +       *addr = map_physmem(a, s, MAP_NOCACHE);
> > +       *size = (u32)s;
> >
> >         return 0;
> >  }
> > --
> > 2.25.1
> >
>
> Regards,
> SImon