[PATCH v6 3/6] tpm: Support boot measurements

[Ilias Apalodimas]

Hi Eddie,
One last thing...
Since you'll send a v7, mind adding the Linaro copyright on tpm-v2.c
since it's mostly my code moving around wrt to the TCG stuff?

Cheers
/Ilias


On Thu, 23 Feb 2023 at 20:07, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> Hi Eddie,
>
> [...]
>
> > > But at least the kernel can find the EFI config tables properly.
> > > > Also, note that the ret is efi_status_t and the new function you added
> > > > returns an int.  Just switch the ret here accordingly.
> > > >
> > > >
> > > >           * platforms can use different ways to do so.
> > > >           */
> > > >          ret = tcg2_init_log(dev, &elog);
> > > > -       if (ret != EFI_SUCCESS)
> > > > -               goto free_pool;
> > > > -
> > > > -       memcpy(event_log.buffer, elog.log, elog.log_position);
> > > > -       event_log.pos = elog.log_position;
> > > > +       if (ret == EFI_SUCCESS) {
> > > > +               memcpy(event_log.buffer, elog.log, elog.log_position);
> > > > +               event_log.pos = elog.log_position;
> > > > +       }
> > > >
> > > >
> > > > I'll try digging into the EFI issues more once I get some time
> > > Ok Ignore the previous mail. I just noticed that the specid event is
> > > generated by tcg2_log_init() and that's where the errors come from.
> >
> >
> > OK, I think I understand the problem. One problem with your solution below
> > is that it's only useful for EFI; writing the event log to a temporary
> > buffer in the non-EFI case is a waste of time, since it can't go anywhere.
>
> It's not even a 'solution', it's just an ugly hack to help you understand
> where things go sideways.
>
> >
> > I have a solution where the user can pass in their buffer in the
> > tcg2_event_log structure in the event that no memory region is discovered.
> > EFI can use that path.
>
> Yep that sounds sane,  that way EFI can pass the efi allocated memory and
> you can call call that with NULL for non-efi code.
> Please keep in mind that the EventLog, if it comes from TF-A, needs to be
> replayed into hardware as well, since TF-A doesn't extend PCRs.
>
> >
> >
> > Thanks for the debugging effort!
>
> yw
>
> Cheers
> /Ilias
> >
> > Eddie
> >
> >
> > >
> > > So something like the *really ugly hack* fixes EFI.  Can you please
> > > turn it into human-readable code
> > >
> > >   #include <linux/unaligned/be_byteshift.h>
> > >   #include <linux/unaligned/generic.h>
> > >   #include <linux/unaligned/le_byteshift.h>
> > > +#include <malloc.h>
> > >
> > >   #include "tpm-utils.h"
> > >
> > > @@ -634,7 +635,7 @@ __weak int tcg2_platform_get_log(struct udevice
> > > *dev, void **addr, u32 *size)
> > >
> > >                  if (dev_read_phandle_with_args(dev, "memory-region", NULL, 0,
> > >                                                 0, &args))
> > > -                       return -ENODEV;
> > > +                       goto alloc;
> > >
> > >                  a = ofnode_get_addr_size(args.node, "reg", &s);
> > >                  if (a == FDT_ADDR_T_NONE)
> > > @@ -643,6 +644,9 @@ __weak int tcg2_platform_get_log(struct udevice
> > > *dev, void **addr, u32 *size)
> > >                  *addr = map_physmem(a, s, MAP_NOCACHE);
> > >                  *size = (u32)s;
> > >          }
> > > +alloc:
> > > +       *addr = calloc(1, 4096);
> > > +       *size = 4096;
> > >
> > >          return 0;
> > >   }
> > >
> > > Cheers
> > > /Ilias
> > >
> > > > Cheers
> > > > /Ilias
> > > > > Thanks!
> > > > > /Ilias