On Mon, Feb 11, 2019 at 8:39 AM Jarkko Sakkinen
<jarkko.sakkinen@xxxxxxxxxxxxxxx> wrote:
> On Mon, Feb 04, 2019 at 01:33:01PM -0800, Matthew Garrett wrote:
> > +int tpm2_event_log_length(void *data, int count, void *size_info)
>
> Should be a static function and for consistency's sake the name should
> be tpm2_calc_event_log_size().
Done.
> > + size += event_size;
> > + }
>
> Would add a newline here.
Done.
> Not really sure why this change to branching in the first part of this
> function. Would reduce the diff and review the actually relevant parts
> if you didn't do this.
Done.
> > +
> > + if (efi.tpm_final_log != EFI_INVALID_TABLE_ADDR) {
>
> Wondering that should this function instead do right `in the beginning:
>
> if (efi.tpm_log == EFI_INVALID_TABLE_ADDR &&
> efi.tpm_final_log == EFI_INVALID_TABLE_ADDR)
> return 0;
>
> Feels odd condition that the log would not be invalid but the post log
> (using post would be more self-describing than final imho) would be. Can
> that legitly happen?
The spec name is final, so I kept it that way for consistency. Keeping
a separate check for the final event log is partly out of
defensiveness against firmware implementations getting this wrong -
I've definitely found implementations that just don't produce any
final events, so it wouldn't surprise me if there are some that don't
install the table.
> > +struct efi_tcg2_final_events_table {
> > + u64 version;
> > + u64 number_of_events;
>
> Would name this simply as nr_events as nr_ is so commonly used prefix in
> the kernel.
Done.
> > static inline int _calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
> > - struct tcg_pcr_event *event_header)
> > + struct tcg_pcr_event *event_header,
> > + void *(*map)(resource_size_t, unsigned long),
> > + void (*unmap)(void *, unsigned long))
>
> Should replace with bool do_mapping instead of doing indirect calls for
> on reason (and thus introducing retpoline's). I don't think call sites
> should be enumerated. A proper kdoc would just describe parameters and
> their purpose.
Done. I'll send an updated patchset.
