public inbox for linux-integrity@vger.kernel.org
 help / color / mirror / Atom feed
From: Jarkko Sakkinen <jarkko@kernel.org>
To: amirmizi6@gmail.com, Eyal.Cohen@nuvoton.com,
	oshrialkoby85@gmail.com, alexander.steffen@infineon.com,
	robh+dt@kernel.org, mark.rutland@arm.com, peterhuewe@gmx.de,
	jgg@ziepe.ca, arnd@arndb.de, gregkh@linuxfoundation.org,
	benoit.houyere@st.com, eajames@linux.ibm.com, joel@jms.id.au
Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-integrity@vger.kernel.org, oshri.alkoby@nuvoton.com,
	tmaimon77@gmail.com, gcwilson@us.ibm.com, kgoldman@us.ibm.com,
	Dan.Morav@nuvoton.com, oren.tanami@nuvoton.com,
	shmulik.hager@nuvoton.com, amir.mizinski@nuvoton.com
Subject: Re: [PATCH v14 2/7] tpm: tpm_tis: Fix expected bit handling and send all bytes in one shot without last byte in exception
Date: Tue, 14 Sep 2021 00:10:26 +0300	[thread overview]
Message-ID: <5659e538b806c64cd738b13d89663890034447d0.camel@kernel.org> (raw)
In-Reply-To: <20210913144351.101167-3-amirmizi6@gmail.com>

On Mon, 2021-09-13 at 17:43 +0300, amirmizi6@gmail.com wrote:
> From: Amir Mizinski <amirmizi6@gmail.com>
> 
> Detected an incorrect implementation of the send command.
> Currently, the driver polls the TPM_STS.stsValid field until TRUE; then it
> reads TPM_STS register again to verify only that TPM_STS.expect field is
> FALSE (i.e., it ignores TPM_STS.stsValid).
> Since TPM_STS.stsValid represents the TPM_STS.expect validity, both fields
> fields should be checked in the same TPM_STS register read value.
> 
> This fix modifies the signature of 'wait_for_tpm_stat()', adding an
> additional "mask_result" parameter to its call and renaming it to
> 'tpm_tis_wait_for_stat()' for better alignment with other naming.
> 'tpm_tis_wait_for_stat()' is now polling the TPM_STS with a mask and waits
> for the value in mask_result. The fix adds the ability to check if certain
> TPM_STS bits have been cleared.

Use imprative form, e.g. "Modify the signature...".

> 
> This change is also aligned to verifying the CRC on I2C TPM. The CRC
> verification should be done after the TPM_STS.expect field is cleared
> (TPM received all expected command bytes and set the calculated CRC value
> in the register).

What does it mean when you "align to verifying"?

> In addition, the send command was changed to comply with
> TCG_DesignPrinciples_TPM2p0Driver_vp24_pubrev.pdf as follows:
> - send all command bytes in one loop
> - remove special handling of the last byte
> 
> Suggested-by: Benoit Houyere <benoit.houyere@st.com>
> Signed-off-by: Amir Mizinski <amirmizi6@gmail.com>


I don't think the rename is important enough to be done, and it
definitely should not be melded into another patch.

/Jarkko


  reply	other threads:[~2021-09-13 21:10 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-13 14:43 [PATCH v14 0/7] Add tpm i2c ptp driver amirmizi6
2021-09-13 14:43 ` [PATCH v14 1/7] tpm: Make read{16, 32}() and write32() in tpm_tis_phy_ops optional amirmizi6
2021-09-13 21:06   ` Jarkko Sakkinen
2021-09-13 14:43 ` [PATCH v14 2/7] tpm: tpm_tis: Fix expected bit handling and send all bytes in one shot without last byte in exception amirmizi6
2021-09-13 21:10   ` Jarkko Sakkinen [this message]
2021-09-13 14:43 ` [PATCH v14 3/7] tpm: tpm_tis: Rewrite "tpm_tis_req_canceled()" amirmizi6
2021-09-13 14:43 ` [PATCH v14 4/7] tpm: Handle an exception for TPM Firmware Update mode amirmizi6
2021-09-13 14:43 ` [PATCH v14 5/7] tpm: tpm_tis: verify TPM_STS register is valid after locality request amirmizi6
2021-09-13 14:43 ` [PATCH v14 6/7] tpm: tpm_tis: add tpm_tis_i2c driver amirmizi6
2021-09-13 14:43 ` [PATCH v14 7/7] \tpm: Add YAML schema for TPM TIS I2C options amirmizi6

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5659e538b806c64cd738b13d89663890034447d0.camel@kernel.org \
    --to=jarkko@kernel.org \
    --cc=Dan.Morav@nuvoton.com \
    --cc=Eyal.Cohen@nuvoton.com \
    --cc=alexander.steffen@infineon.com \
    --cc=amir.mizinski@nuvoton.com \
    --cc=amirmizi6@gmail.com \
    --cc=arnd@arndb.de \
    --cc=benoit.houyere@st.com \
    --cc=devicetree@vger.kernel.org \
    --cc=eajames@linux.ibm.com \
    --cc=gcwilson@us.ibm.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jgg@ziepe.ca \
    --cc=joel@jms.id.au \
    --cc=kgoldman@us.ibm.com \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=oren.tanami@nuvoton.com \
    --cc=oshri.alkoby@nuvoton.com \
    --cc=oshrialkoby85@gmail.com \
    --cc=peterhuewe@gmx.de \
    --cc=robh+dt@kernel.org \
    --cc=shmulik.hager@nuvoton.com \
    --cc=tmaimon77@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox