[Skiboot] [PATCH] tpm_i2c_nuvoton: check TPM vendor id register during probe
Oliver O'Halloran
oohall at gmail.com
Fri Mar 6 13:39:08 AEDT 2020
On Fri, Mar 6, 2020 at 8:17 AM Klaus Heinrich Kiwi
<klaus at linux.vnet.ibm.com> wrote:
>
>
>
> On 3/5/2020 3:54 PM, Eric Richter wrote:
> > + /* ensure there's really the TPM we expect at that address */
> > + if (tpm_i2c_request_send(tpm_device, SMBUS_READ, TPM_VID_DID,
> > + 1, &vendor, sizeof(vendor)) ||
> > + (vendor & TPM_VENDOR_ID_MASK) != TPM_650_VENDOR_ID) {
> if tpm_i2c_request_send() fails (negative error code), you skip this
> block and still tries to tpm_register_chip()?
Any non-zero value is considered true so it'll go down the error path
if the I2C transaction fails. I'm not 100% sure I understand what
you're saying here though...
> > + prlog(PR_ERR, "NUVOTON: i2c device inaccessible, or "
> > + "expected vendor id mismatch\n");
> > + if (vendor) {
> > + prlog(PR_ERR, "NUVOTON: expected 0x%X, "
> > + "got 0x%X\n", TPM_650_VENDOR_ID, vendor);
> > + }
> > + free(tpm_device);
> > + continue;
> > + }
> > if (tpm_register_chip(node, tpm_device,
> > &tpm_i2c_nuvoton_driver)) {
> > free(tpm_device);
> >
>
> Perhaps registering it would then fail? Since tpm_register_chip() sounds
> both like an action as well as a predicate, perhaps be more explicit
> about what we are testing? (I know this is outside the proposed patch
> but still...)
It doesn't fail, that's the problem. Right now we just keep trucking
along until skiboot tries to write a measurement to the TPM at which
point we notice it's not there.
>
> -Klaus
>
> --
> Klaus Heinrich Kiwi <klaus at linux.vnet.ibm.com>
> _______________________________________________
> Skiboot mailing list
> Skiboot at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot
More information about the Skiboot
mailing list