Re: [PATCH v19 21/33] tss2: Add TPM2 Software Stack (TSS2) support

[Gary Lin]

On Tue, Oct 01, 2024 at 04:48:34PM +0200, Daniel Kiper wrote:
> On Wed, Sep 18, 2024 at 03:28:44PM +0800, Gary Lin via Grub-devel wrote:
> > On Tue, Sep 17, 2024 at 11:14:33PM -0400, Stefan Berger wrote:
> > > On 9/6/24 5:11 AM, Gary Lin wrote:
> > > > A Trusted Platform Module (TPM) Software Stack (TSS) provides logic to
> > > > compose and submit TPM commands and parse reponses.
> > > >
> > >
> > > > +static TPM_RC_t
> > > > +tpm2_submit_command_real (const TPMI_ST_COMMAND_TAG_t tag,
> > > > +                         const TPM_CC_t commandCode,
> > > > +                         TPM_RC_t *responseCode,
> > > > +                         const struct grub_tpm2_buffer *in,
> > > > +                         struct grub_tpm2_buffer *out)
> > > > +{
> > > > +  grub_err_t err;
> > > > +  struct grub_tpm2_buffer buf;
> > > > +  TPMI_ST_COMMAND_TAG_t tag_out;
> > > > +  grub_uint32_t command_size;
> > > > +  grub_size_t max_output_size;
> > > > +
> > > > +  /* Marshal */
> > > > +  grub_tpm2_buffer_init (&buf);
> > > > +  grub_tpm2_buffer_pack_u16 (&buf, tag);
> > > > +  grub_tpm2_buffer_pack_u32 (&buf, 0);
> > > > +  grub_tpm2_buffer_pack_u32 (&buf, commandCode);
> > > > +  grub_tpm2_buffer_pack (&buf, in->data, in->size);
> > > > +
> > > > +  if (buf.error != 0)
> > > > +    return TPM_RC_FAILURE;
> > > > +
> > > > +  command_size = grub_swap_bytes32 (buf.size);
> > > > +  grub_memcpy (&buf.data[sizeof (grub_uint16_t)], &command_size,
> > > > +              sizeof (command_size));
> > >
> > > This is odd. So UEFI requires that the command size is in little endian
> > > while the TPM commands are typically all in big endian format? If so, 
> > > could
> > > you push this into the grub_tcg2_submit_command? Other firmware doesn't 
> > > need
> > > this odd endianess switch and could just leave the size as-is and just 
> > > pass
> > > the TPM command to the device without switching endianess again.
> >
> > EFI_TCG2_PROTOCOL.SubmitCommand() expects the little-endian
> > InputParameterBlockSize, and InputParameterBlock in big-endian for the
> > TPM command stream.
> >
> > Actually, the code is also needed when running grub-emu on a
> > little-endian system. Maybe we can add an additional check for the
> > endianness here and only run the code for the little-endian systems.
> 
> Why do not define grub_cpu_to_tpm32()/grub_tpm_to_cpu32() et consores
> macros and make them noop on some archs?
> 
Actually I found that grub_cpu_to_be32() is sufficient here since TPM
expects big endian integers. grub_be_to_cpu16() and grub_be_to_cpu32()
are already used for buffer unpacking.

Gary Lin