Re: [PATCH v2 1/2] tpm: add retry logic

[James Bottomley <James.Bottomley@HansenPartnership.com>]

On Thu, 2018-03-22 at 21:43 +0530, Nayna Jain wrote:
> 
> > 
> >   /**
> > + * tpm_transmit - Internal kernel interface to transmit TPM
> > commands.
> > + *
> > + * @chip: TPM chip to use
> > + * @space: tpm space
> > + * @buf: TPM command buffer
> > + * @bufsiz: length of the TPM command buffer
> > + * @flags: tpm transmit flags - bitmap
> > + *
> > + * A wrapper around tpm_try_transmit that handles TPM2_RC_RETRY
> > + * returns from the TPM and retransmits the command after a delay
> > up
> > + * to a maximum wait of TPM2_DURATION_LONG.
> > + *
> > + * Note: TPM1 never returns TPM2_RC_RETRY so the retry logic is
> > TPM2
> > + * only
> > + *
> > + * Return:
> > + *     the length of the return when the operation is successful.
> > + *     A negative number for system errors (errno).
> > + */
> > +ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space
> > *space,
> > +		     u8 *buf, size_t bufsiz, unsigned int flags)
> > +{
> > +	struct tpm_output_header *header = (struct
> > tpm_output_header *)buf;
> > +	/* space for header and handles */
> > +	u8 save[TPM_HEADER_SIZE + 3*sizeof(u32)];
> > +	unsigned int delay_msec = TPM2_DURATION_SHORT;
> > +	u32 rc = 0;
> > +	ssize_t ret;
> > +	const size_t save_size = min(space ? sizeof(save) :
> > TPM_HEADER_SIZE,
> > +				     bufsiz);
> > +
> > +	/*
> > +	 * Subtlety here: if we have a space, the handles will be
> > +	 * transformed, so when we restore the header we also have
> > to
> > +	 * restore the handles.
> > +	 */
> > +	memcpy(save, buf, save_size);
> > +
> > +	for (;;) {
> > +		ret = tpm_try_transmit(chip, space, buf, bufsiz,
> > flags);
> > +		if (ret < 0)
> > +			break;
> > +		rc = be32_to_cpu(header->return_code);
> > +		if (rc != TPM2_RC_RETRY)
> > +			break;
> > +		delay_msec *= 2;
> 
> Was wondering if this increment could be moved after 
> tpm_msleep(delay_msec) ?

I thought about that when I saw the logic in the original but what it
would do is double the maximum delay (which is already nearly double
TPM2_DURATION_LONG).  I figured doubling the initial timeout was fine
rather than trying to do complex logic to get the delay exact (and then
having to litter the file with comments explaining why).

James