| Message ID | 20220813195639.1824765-7-sjg@chromium.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tpm: Various minor fixes and enhancements | expand |
Hi Simon, I know little of this device and the whole patch seems fine apart from the definitions and declarations of the state functions. On Sat, 13 Aug 2022 at 22:56, Simon Glass <sjg@chromium.org> wrote: > > > drivers/tpm/cr50_i2c.c | 117 +++++++++++++++++++++++++++++++++++++++++ > include/tpm-v2.h | 54 +++++++++++++++++++ > lib/tpm-v2.c | 24 +++++++++ [...] > diff --git a/include/tpm-v2.h b/include/tpm-v2.h > index e79c90b9395..8e90a616220 100644 > --- a/include/tpm-v2.h > +++ b/include/tpm-v2.h > @@ -419,6 +419,50 @@ enum { > HR_NV_INDEX = TPM_HT_NV_INDEX << HR_SHIFT, > }; > > +/* > + * Operations specific to the Cr50 TPM used on Chromium OS and Android devices > + * > + * FIXME: below is not enough to differentiate between vendors commands > + * of numerous devices. However, the current tpm2 APIs aren't very amenable > + * to extending generically because the marshaling code is assuming all > + * knowledge of all commands. > + */ > +#define TPM2_CC_VENDOR_BIT_MASK 0x20000000 > + > +#define TPM2_CR50_VENDOR_COMMAND (TPM2_CC_VENDOR_BIT_MASK | 0) > +#define TPM2_CR50_SUB_CMD_IMMEDIATE_RESET 19 > +#define TPM2_CR50_SUB_CMD_NVMEM_ENABLE_COMMITS 21 > +#define TPM2_CR50_SUB_CMD_REPORT_TPM_STATE 23 > +#define TPM2_CR50_SUB_CMD_TURN_UPDATE_ON 24 > +#define TPM2_CR50_SUB_CMD_GET_REC_BTN 29 > +#define TPM2_CR50_SUB_CMD_TPM_MODE 40 > +#define TPM2_CR50_SUB_CMD_GET_BOOT_MODE 52 > +#define TPM2_CR50_SUB_CMD_RESET_EC 53 > + > +/* Cr50 vendor-specific error codes. */ > +#define VENDOR_RC_ERR 0x00000500 > +enum cr50_vendor_rc { > + VENDOR_RC_INTERNAL_ERROR = (VENDOR_RC_ERR | 6), > + VENDOR_RC_NO_SUCH_SUBCOMMAND = (VENDOR_RC_ERR | 8), > + VENDOR_RC_NO_SUCH_COMMAND = (VENDOR_RC_ERR | 127), > +}; > + > +enum cr50_tpm_mode { > + /* > + * Default state: TPM is enabled, and may be set to either > + * TPM_MODE_ENABLED or TPM_MODE_DISABLED. > + */ > + TPM_MODE_ENABLED_TENTATIVE = 0, > + > + /* TPM is enabled, and mode may not be changed. */ > + TPM_MODE_ENABLED = 1, > + > + /* TPM is disabled, and mode may not be changed. */ > + TPM_MODE_DISABLED = 2, > + > + TPM_MODE_INVALID, > +}; > + > /** > * Issue a TPM2_Startup command. > * > @@ -658,4 +702,14 @@ u32 tpm2_disable_platform_hierarchy(struct udevice *dev); > u32 tpm2_submit_command(struct udevice *dev, const u8 *sendbuf, > u8 *recvbuf, size_t *recv_size); > > +/** > + * tpm_cr50_report_state() - Report the Cr50 internal state > + * > + * @dev: TPM device > + * @recvbuf: Buffer to save the response to > + * @recv_size: Pointer to the size of the response buffer > + * Return: result of the operation > + */ > +u32 tpm2_cr50_report_state(struct udevice *dev, u8 *recvbuf, size_t *recv_size); > + I think we should keep the generic include files clean for hardware specific details. > #endif /* __TPM_V2_H */ > diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c > index 3e240bb4c67..3de4841974a 100644 > --- a/lib/tpm-v2.c > +++ b/lib/tpm-v2.c > @@ -679,3 +679,27 @@ u32 tpm2_submit_command(struct udevice *dev, const u8 *sendbuf, > { > return tpm_sendrecv_command(dev, sendbuf, recvbuf, recv_size); > } > + > +u32 tpm2_cr50_report_state(struct udevice *dev, u8 *recvbuf, size_t *recv_size) > +{ > + u8 command_v2[COMMAND_BUFFER_SIZE] = { > + /* header 10 bytes */ > + tpm_u16(TPM2_ST_NO_SESSIONS), /* TAG */ > + tpm_u32(10 + 2), /* Length */ > + tpm_u32(TPM2_CR50_VENDOR_COMMAND), /* Command code */ > + > + tpm_u16(TPM2_CR50_SUB_CMD_REPORT_TPM_STATE), > + }; > + int ret; > + > + ret = tpm_sendrecv_command(dev, command_v2, recvbuf, recv_size); > + log_debug("ret=%s, %x\n", dev->name, ret); > + if (ret) > + return ret; > + if (*recv_size < 12) > + return -ENODATA; > + *recv_size -= 12; > + memcpy(recvbuf, recvbuf + 12, *recv_size); > + > + return 0; > +} Same here, this functions seems ok but shouldn't land in the generic TPM API Thanks /Ilias > -- > 2.37.1.595.g718a3a8f04-goog >
Hi Ilias, On Tue, 16 Aug 2022 at 06:43, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > Hi Simon, > > I know little of this device and the whole patch seems fine apart from > the definitions and declarations of the state functions. > > > On Sat, 13 Aug 2022 at 22:56, Simon Glass <sjg@chromium.org> wrote: > > > > > > > drivers/tpm/cr50_i2c.c | 117 +++++++++++++++++++++++++++++++++++++++++ > > include/tpm-v2.h | 54 +++++++++++++++++++ > > lib/tpm-v2.c | 24 +++++++++ > > [...] > > > diff --git a/include/tpm-v2.h b/include/tpm-v2.h > > index e79c90b9395..8e90a616220 100644 > > --- a/include/tpm-v2.h > > +++ b/include/tpm-v2.h > > @@ -419,6 +419,50 @@ enum { > > HR_NV_INDEX = TPM_HT_NV_INDEX << HR_SHIFT, > > }; > > > > +/* > > + * Operations specific to the Cr50 TPM used on Chromium OS and Android devices > > + * > > + * FIXME: below is not enough to differentiate between vendors commands > > + * of numerous devices. However, the current tpm2 APIs aren't very amenable > > + * to extending generically because the marshaling code is assuming all > > + * knowledge of all commands. > > + */ > > +#define TPM2_CC_VENDOR_BIT_MASK 0x20000000 > > + > > +#define TPM2_CR50_VENDOR_COMMAND (TPM2_CC_VENDOR_BIT_MASK | 0) > > +#define TPM2_CR50_SUB_CMD_IMMEDIATE_RESET 19 > > +#define TPM2_CR50_SUB_CMD_NVMEM_ENABLE_COMMITS 21 > > +#define TPM2_CR50_SUB_CMD_REPORT_TPM_STATE 23 > > +#define TPM2_CR50_SUB_CMD_TURN_UPDATE_ON 24 > > +#define TPM2_CR50_SUB_CMD_GET_REC_BTN 29 > > +#define TPM2_CR50_SUB_CMD_TPM_MODE 40 > > +#define TPM2_CR50_SUB_CMD_GET_BOOT_MODE 52 > > +#define TPM2_CR50_SUB_CMD_RESET_EC 53 > > + > > +/* Cr50 vendor-specific error codes. */ > > +#define VENDOR_RC_ERR 0x00000500 > > +enum cr50_vendor_rc { > > + VENDOR_RC_INTERNAL_ERROR = (VENDOR_RC_ERR | 6), > > + VENDOR_RC_NO_SUCH_SUBCOMMAND = (VENDOR_RC_ERR | 8), > > + VENDOR_RC_NO_SUCH_COMMAND = (VENDOR_RC_ERR | 127), > > +}; > > + > > +enum cr50_tpm_mode { > > + /* > > + * Default state: TPM is enabled, and may be set to either > > + * TPM_MODE_ENABLED or TPM_MODE_DISABLED. > > + */ > > + TPM_MODE_ENABLED_TENTATIVE = 0, > > + > > + /* TPM is enabled, and mode may not be changed. */ > > + TPM_MODE_ENABLED = 1, > > + > > + /* TPM is disabled, and mode may not be changed. */ > > + TPM_MODE_DISABLED = 2, > > + > > + TPM_MODE_INVALID, > > +}; > > + > > /** > > * Issue a TPM2_Startup command. > > * > > @@ -658,4 +702,14 @@ u32 tpm2_disable_platform_hierarchy(struct udevice *dev); > > u32 tpm2_submit_command(struct udevice *dev, const u8 *sendbuf, > > u8 *recvbuf, size_t *recv_size); > > > > +/** > > + * tpm_cr50_report_state() - Report the Cr50 internal state > > + * > > + * @dev: TPM device > > + * @recvbuf: Buffer to save the response to > > + * @recv_size: Pointer to the size of the response buffer > > + * Return: result of the operation > > + */ > > +u32 tpm2_cr50_report_state(struct udevice *dev, u8 *recvbuf, size_t *recv_size); > > + > > I think we should keep the generic include files clean for hardware > specific details. > > > #endif /* __TPM_V2_H */ > > diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c > > index 3e240bb4c67..3de4841974a 100644 > > --- a/lib/tpm-v2.c > > +++ b/lib/tpm-v2.c > > @@ -679,3 +679,27 @@ u32 tpm2_submit_command(struct udevice *dev, const u8 *sendbuf, > > { > > return tpm_sendrecv_command(dev, sendbuf, recvbuf, recv_size); > > } > > + > > +u32 tpm2_cr50_report_state(struct udevice *dev, u8 *recvbuf, size_t *recv_size) > > +{ > > + u8 command_v2[COMMAND_BUFFER_SIZE] = { > > + /* header 10 bytes */ > > + tpm_u16(TPM2_ST_NO_SESSIONS), /* TAG */ > > + tpm_u32(10 + 2), /* Length */ > > + tpm_u32(TPM2_CR50_VENDOR_COMMAND), /* Command code */ > > + > > + tpm_u16(TPM2_CR50_SUB_CMD_REPORT_TPM_STATE), > > + }; > > + int ret; > > + > > + ret = tpm_sendrecv_command(dev, command_v2, recvbuf, recv_size); > > + log_debug("ret=%s, %x\n", dev->name, ret); > > + if (ret) > > + return ret; > > + if (*recv_size < 12) > > + return -ENODATA; > > + *recv_size -= 12; > > + memcpy(recvbuf, recvbuf + 12, *recv_size); > > + > > + return 0; > > +} > > Same here, this functions seems ok but shouldn't land in the generic TPM API So shall I create a new tpm_cr50.h header file? What about the C file? > > Thanks > /Ilias > > -- > > 2.37.1.595.g718a3a8f04-goog > > Regards, Simon
Hi Simon, On Wed, 17 Aug 2022 at 21:54, Simon Glass <sjg@chromium.org> wrote: > > Hi Ilias, > > On Tue, 16 Aug 2022 at 06:43, Ilias Apalodimas > <ilias.apalodimas@linaro.org> wrote: > > > > Hi Simon, > > > > I know little of this device and the whole patch seems fine apart from > > the definitions and declarations of the state functions. > > > > > > On Sat, 13 Aug 2022 at 22:56, Simon Glass <sjg@chromium.org> wrote: > > > > > > > > > > > drivers/tpm/cr50_i2c.c | 117 +++++++++++++++++++++++++++++++++++++++++ > > > include/tpm-v2.h | 54 +++++++++++++++++++ > > > lib/tpm-v2.c | 24 +++++++++ > > > > [...] > > > > > diff --git a/include/tpm-v2.h b/include/tpm-v2.h > > > index e79c90b9395..8e90a616220 100644 > > > --- a/include/tpm-v2.h > > > +++ b/include/tpm-v2.h > > > @@ -419,6 +419,50 @@ enum { > > > HR_NV_INDEX = TPM_HT_NV_INDEX << HR_SHIFT, > > > }; > > > > > > +/* > > > + * Operations specific to the Cr50 TPM used on Chromium OS and Android devices > > > + * > > > + * FIXME: below is not enough to differentiate between vendors commands > > > + * of numerous devices. However, the current tpm2 APIs aren't very amenable > > > + * to extending generically because the marshaling code is assuming all > > > + * knowledge of all commands. > > > + */ > > > +#define TPM2_CC_VENDOR_BIT_MASK 0x20000000 > > > + > > > +#define TPM2_CR50_VENDOR_COMMAND (TPM2_CC_VENDOR_BIT_MASK | 0) > > > +#define TPM2_CR50_SUB_CMD_IMMEDIATE_RESET 19 > > > +#define TPM2_CR50_SUB_CMD_NVMEM_ENABLE_COMMITS 21 > > > +#define TPM2_CR50_SUB_CMD_REPORT_TPM_STATE 23 > > > +#define TPM2_CR50_SUB_CMD_TURN_UPDATE_ON 24 > > > +#define TPM2_CR50_SUB_CMD_GET_REC_BTN 29 > > > +#define TPM2_CR50_SUB_CMD_TPM_MODE 40 > > > +#define TPM2_CR50_SUB_CMD_GET_BOOT_MODE 52 > > > +#define TPM2_CR50_SUB_CMD_RESET_EC 53 > > > + > > > +/* Cr50 vendor-specific error codes. */ > > > +#define VENDOR_RC_ERR 0x00000500 > > > +enum cr50_vendor_rc { > > > + VENDOR_RC_INTERNAL_ERROR = (VENDOR_RC_ERR | 6), > > > + VENDOR_RC_NO_SUCH_SUBCOMMAND = (VENDOR_RC_ERR | 8), > > > + VENDOR_RC_NO_SUCH_COMMAND = (VENDOR_RC_ERR | 127), > > > +}; > > > + > > > +enum cr50_tpm_mode { > > > + /* > > > + * Default state: TPM is enabled, and may be set to either > > > + * TPM_MODE_ENABLED or TPM_MODE_DISABLED. > > > + */ > > > + TPM_MODE_ENABLED_TENTATIVE = 0, > > > + > > > + /* TPM is enabled, and mode may not be changed. */ > > > + TPM_MODE_ENABLED = 1, > > > + > > > + /* TPM is disabled, and mode may not be changed. */ > > > + TPM_MODE_DISABLED = 2, > > > + > > > + TPM_MODE_INVALID, > > > +}; > > > + > > > /** > > > * Issue a TPM2_Startup command. > > > * > > > @@ -658,4 +702,14 @@ u32 tpm2_disable_platform_hierarchy(struct udevice *dev); > > > u32 tpm2_submit_command(struct udevice *dev, const u8 *sendbuf, > > > u8 *recvbuf, size_t *recv_size); > > > > > > +/** > > > + * tpm_cr50_report_state() - Report the Cr50 internal state > > > + * > > > + * @dev: TPM device > > > + * @recvbuf: Buffer to save the response to > > > + * @recv_size: Pointer to the size of the response buffer > > > + * Return: result of the operation > > > + */ > > > +u32 tpm2_cr50_report_state(struct udevice *dev, u8 *recvbuf, size_t *recv_size); > > > + > > > > I think we should keep the generic include files clean for hardware > > specific details. > > > > > #endif /* __TPM_V2_H */ > > > diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c > > > index 3e240bb4c67..3de4841974a 100644 > > > --- a/lib/tpm-v2.c > > > +++ b/lib/tpm-v2.c > > > @@ -679,3 +679,27 @@ u32 tpm2_submit_command(struct udevice *dev, const u8 *sendbuf, > > > { > > > return tpm_sendrecv_command(dev, sendbuf, recvbuf, recv_size); > > > } > > > + > > > +u32 tpm2_cr50_report_state(struct udevice *dev, u8 *recvbuf, size_t *recv_size) > > > +{ > > > + u8 command_v2[COMMAND_BUFFER_SIZE] = { > > > + /* header 10 bytes */ > > > + tpm_u16(TPM2_ST_NO_SESSIONS), /* TAG */ > > > + tpm_u32(10 + 2), /* Length */ > > > + tpm_u32(TPM2_CR50_VENDOR_COMMAND), /* Command code */ > > > + > > > + tpm_u16(TPM2_CR50_SUB_CMD_REPORT_TPM_STATE), > > > + }; > > > + int ret; > > > + > > > + ret = tpm_sendrecv_command(dev, command_v2, recvbuf, recv_size); > > > + log_debug("ret=%s, %x\n", dev->name, ret); > > > + if (ret) > > > + return ret; > > > + if (*recv_size < 12) > > > + return -ENODATA; > > > + *recv_size -= 12; > > > + memcpy(recvbuf, recvbuf + 12, *recv_size); > > > + > > > + return 0; > > > +} > > > > Same here, this functions seems ok but shouldn't land in the generic TPM API > > So shall I create a new tpm_cr50.h header file? What about the C file? Yea the header file seems fine (I assume in drivers/tpm?) About the C file, tpm2_cr50_report_state() is called by cr50_i2c_report_state() which is static in the drivers. Can't we just move the function there? Regards /Ilias > > > > > Thanks > > /Ilias > > > -- > > > 2.37.1.595.g718a3a8f04-goog > > > > > Regards, > Simon
Hi Ilias, On Thu, 18 Aug 2022 at 01:29, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > Hi Simon, > > On Wed, 17 Aug 2022 at 21:54, Simon Glass <sjg@chromium.org> wrote: > > > > Hi Ilias, > > > > On Tue, 16 Aug 2022 at 06:43, Ilias Apalodimas > > <ilias.apalodimas@linaro.org> wrote: > > > > > > Hi Simon, > > > > > > I know little of this device and the whole patch seems fine apart from > > > the definitions and declarations of the state functions. > > > > > > > > > On Sat, 13 Aug 2022 at 22:56, Simon Glass <sjg@chromium.org> wrote: > > > > > > > > > > > > > > > drivers/tpm/cr50_i2c.c | 117 +++++++++++++++++++++++++++++++++++++++++ > > > > include/tpm-v2.h | 54 +++++++++++++++++++ > > > > lib/tpm-v2.c | 24 +++++++++ > > > > > > [...] > > > > > > > diff --git a/include/tpm-v2.h b/include/tpm-v2.h > > > > index e79c90b9395..8e90a616220 100644 > > > > --- a/include/tpm-v2.h > > > > +++ b/include/tpm-v2.h > > > > @@ -419,6 +419,50 @@ enum { > > > > HR_NV_INDEX = TPM_HT_NV_INDEX << HR_SHIFT, > > > > }; > > > > > > > > +/* > > > > + * Operations specific to the Cr50 TPM used on Chromium OS and Android devices > > > > + * > > > > + * FIXME: below is not enough to differentiate between vendors commands > > > > + * of numerous devices. However, the current tpm2 APIs aren't very amenable > > > > + * to extending generically because the marshaling code is assuming all > > > > + * knowledge of all commands. > > > > + */ > > > > +#define TPM2_CC_VENDOR_BIT_MASK 0x20000000 > > > > + > > > > +#define TPM2_CR50_VENDOR_COMMAND (TPM2_CC_VENDOR_BIT_MASK | 0) > > > > +#define TPM2_CR50_SUB_CMD_IMMEDIATE_RESET 19 > > > > +#define TPM2_CR50_SUB_CMD_NVMEM_ENABLE_COMMITS 21 > > > > +#define TPM2_CR50_SUB_CMD_REPORT_TPM_STATE 23 > > > > +#define TPM2_CR50_SUB_CMD_TURN_UPDATE_ON 24 > > > > +#define TPM2_CR50_SUB_CMD_GET_REC_BTN 29 > > > > +#define TPM2_CR50_SUB_CMD_TPM_MODE 40 > > > > +#define TPM2_CR50_SUB_CMD_GET_BOOT_MODE 52 > > > > +#define TPM2_CR50_SUB_CMD_RESET_EC 53 > > > > + > > > > +/* Cr50 vendor-specific error codes. */ > > > > +#define VENDOR_RC_ERR 0x00000500 > > > > +enum cr50_vendor_rc { > > > > + VENDOR_RC_INTERNAL_ERROR = (VENDOR_RC_ERR | 6), > > > > + VENDOR_RC_NO_SUCH_SUBCOMMAND = (VENDOR_RC_ERR | 8), > > > > + VENDOR_RC_NO_SUCH_COMMAND = (VENDOR_RC_ERR | 127), > > > > +}; > > > > + > > > > +enum cr50_tpm_mode { > > > > + /* > > > > + * Default state: TPM is enabled, and may be set to either > > > > + * TPM_MODE_ENABLED or TPM_MODE_DISABLED. > > > > + */ > > > > + TPM_MODE_ENABLED_TENTATIVE = 0, > > > > + > > > > + /* TPM is enabled, and mode may not be changed. */ > > > > + TPM_MODE_ENABLED = 1, > > > > + > > > > + /* TPM is disabled, and mode may not be changed. */ > > > > + TPM_MODE_DISABLED = 2, > > > > + > > > > + TPM_MODE_INVALID, > > > > +}; > > > > + > > > > /** > > > > * Issue a TPM2_Startup command. > > > > * > > > > @@ -658,4 +702,14 @@ u32 tpm2_disable_platform_hierarchy(struct udevice *dev); > > > > u32 tpm2_submit_command(struct udevice *dev, const u8 *sendbuf, > > > > u8 *recvbuf, size_t *recv_size); > > > > > > > > +/** > > > > + * tpm_cr50_report_state() - Report the Cr50 internal state > > > > + * > > > > + * @dev: TPM device > > > > + * @recvbuf: Buffer to save the response to > > > > + * @recv_size: Pointer to the size of the response buffer > > > > + * Return: result of the operation > > > > + */ > > > > +u32 tpm2_cr50_report_state(struct udevice *dev, u8 *recvbuf, size_t *recv_size); > > > > + > > > > > > I think we should keep the generic include files clean for hardware > > > specific details. > > > > > > > #endif /* __TPM_V2_H */ > > > > diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c > > > > index 3e240bb4c67..3de4841974a 100644 > > > > --- a/lib/tpm-v2.c > > > > +++ b/lib/tpm-v2.c > > > > @@ -679,3 +679,27 @@ u32 tpm2_submit_command(struct udevice *dev, const u8 *sendbuf, > > > > { > > > > return tpm_sendrecv_command(dev, sendbuf, recvbuf, recv_size); > > > > } > > > > + > > > > +u32 tpm2_cr50_report_state(struct udevice *dev, u8 *recvbuf, size_t *recv_size) > > > > +{ > > > > + u8 command_v2[COMMAND_BUFFER_SIZE] = { > > > > + /* header 10 bytes */ > > > > + tpm_u16(TPM2_ST_NO_SESSIONS), /* TAG */ > > > > + tpm_u32(10 + 2), /* Length */ > > > > + tpm_u32(TPM2_CR50_VENDOR_COMMAND), /* Command code */ > > > > + > > > > + tpm_u16(TPM2_CR50_SUB_CMD_REPORT_TPM_STATE), > > > > + }; > > > > + int ret; > > > > + > > > > + ret = tpm_sendrecv_command(dev, command_v2, recvbuf, recv_size); > > > > + log_debug("ret=%s, %x\n", dev->name, ret); > > > > + if (ret) > > > > + return ret; > > > > + if (*recv_size < 12) > > > > + return -ENODATA; > > > > + *recv_size -= 12; > > > > + memcpy(recvbuf, recvbuf + 12, *recv_size); > > > > + > > > > + return 0; > > > > +} > > > > > > Same here, this functions seems ok but shouldn't land in the generic TPM API > > > > So shall I create a new tpm_cr50.h header file? What about the C file? > > Yea the header file seems fine (I assume in drivers/tpm?) > > About the C file, tpm2_cr50_report_state() is called by > cr50_i2c_report_state() which is static in the drivers. Can't we just > move the function there? Just digging into this but I think you have the wrong end of the stick. I have added a new TPM method to the uclass for this. We cannot call functions 'sideways' of that mechanism as it violates how driver model works. The feature is implemented for cr50 and sandbox. It could be implemented for other TPMs if they have any info that can usefully be provided, such as the state of the PCRs or something else useful or important. So I think my patches are already doing all this correctly. Can you please take another look? I'll send v3 as Heinrich had a minor comment on a function comment. Also one of your changes. Regards, Simon
Hi Simon, On Fri, 19 Aug 2022 at 16:47, Simon Glass <sjg@chromium.org> wrote: > > Hi Ilias, > > On Thu, 18 Aug 2022 at 01:29, Ilias Apalodimas > <ilias.apalodimas@linaro.org> wrote: > > > > Hi Simon, > > > > On Wed, 17 Aug 2022 at 21:54, Simon Glass <sjg@chromium.org> wrote: > > > > > > Hi Ilias, > > > > > > On Tue, 16 Aug 2022 at 06:43, Ilias Apalodimas > > > <ilias.apalodimas@linaro.org> wrote: > > > > > > > > Hi Simon, > > > > > > > > I know little of this device and the whole patch seems fine apart from > > > > the definitions and declarations of the state functions. > > > > > > > > > > > > On Sat, 13 Aug 2022 at 22:56, Simon Glass <sjg@chromium.org> wrote: > > > > > > > > > > > > > > > > > > > drivers/tpm/cr50_i2c.c | 117 +++++++++++++++++++++++++++++++++++++++++ > > > > > include/tpm-v2.h | 54 +++++++++++++++++++ > > > > > lib/tpm-v2.c | 24 +++++++++ > > > > > > > > [...] > > > > > > > > > diff --git a/include/tpm-v2.h b/include/tpm-v2.h > > > > > index e79c90b9395..8e90a616220 100644 > > > > > --- a/include/tpm-v2.h > > > > > +++ b/include/tpm-v2.h > > > > > @@ -419,6 +419,50 @@ enum { > > > > > HR_NV_INDEX = TPM_HT_NV_INDEX << HR_SHIFT, > > > > > }; > > > > > > > > > > +/* > > > > > + * Operations specific to the Cr50 TPM used on Chromium OS and Android devices > > > > > + * > > > > > + * FIXME: below is not enough to differentiate between vendors commands > > > > > + * of numerous devices. However, the current tpm2 APIs aren't very amenable > > > > > + * to extending generically because the marshaling code is assuming all > > > > > + * knowledge of all commands. > > > > > + */ > > > > > +#define TPM2_CC_VENDOR_BIT_MASK 0x20000000 > > > > > + > > > > > +#define TPM2_CR50_VENDOR_COMMAND (TPM2_CC_VENDOR_BIT_MASK | 0) > > > > > +#define TPM2_CR50_SUB_CMD_IMMEDIATE_RESET 19 > > > > > +#define TPM2_CR50_SUB_CMD_NVMEM_ENABLE_COMMITS 21 > > > > > +#define TPM2_CR50_SUB_CMD_REPORT_TPM_STATE 23 > > > > > +#define TPM2_CR50_SUB_CMD_TURN_UPDATE_ON 24 > > > > > +#define TPM2_CR50_SUB_CMD_GET_REC_BTN 29 > > > > > +#define TPM2_CR50_SUB_CMD_TPM_MODE 40 > > > > > +#define TPM2_CR50_SUB_CMD_GET_BOOT_MODE 52 > > > > > +#define TPM2_CR50_SUB_CMD_RESET_EC 53 > > > > > + > > > > > +/* Cr50 vendor-specific error codes. */ > > > > > +#define VENDOR_RC_ERR 0x00000500 > > > > > +enum cr50_vendor_rc { > > > > > + VENDOR_RC_INTERNAL_ERROR = (VENDOR_RC_ERR | 6), > > > > > + VENDOR_RC_NO_SUCH_SUBCOMMAND = (VENDOR_RC_ERR | 8), > > > > > + VENDOR_RC_NO_SUCH_COMMAND = (VENDOR_RC_ERR | 127), > > > > > +}; > > > > > + > > > > > +enum cr50_tpm_mode { > > > > > + /* > > > > > + * Default state: TPM is enabled, and may be set to either > > > > > + * TPM_MODE_ENABLED or TPM_MODE_DISABLED. > > > > > + */ > > > > > + TPM_MODE_ENABLED_TENTATIVE = 0, > > > > > + > > > > > + /* TPM is enabled, and mode may not be changed. */ > > > > > + TPM_MODE_ENABLED = 1, > > > > > + > > > > > + /* TPM is disabled, and mode may not be changed. */ > > > > > + TPM_MODE_DISABLED = 2, > > > > > + > > > > > + TPM_MODE_INVALID, > > > > > +}; > > > > > + > > > > > /** > > > > > * Issue a TPM2_Startup command. > > > > > * > > > > > @@ -658,4 +702,14 @@ u32 tpm2_disable_platform_hierarchy(struct udevice *dev); > > > > > u32 tpm2_submit_command(struct udevice *dev, const u8 *sendbuf, > > > > > u8 *recvbuf, size_t *recv_size); > > > > > > > > > > +/** > > > > > + * tpm_cr50_report_state() - Report the Cr50 internal state > > > > > + * > > > > > + * @dev: TPM device > > > > > + * @recvbuf: Buffer to save the response to > > > > > + * @recv_size: Pointer to the size of the response buffer > > > > > + * Return: result of the operation > > > > > + */ > > > > > +u32 tpm2_cr50_report_state(struct udevice *dev, u8 *recvbuf, size_t *recv_size); > > > > > + > > > > > > > > I think we should keep the generic include files clean for hardware > > > > specific details. > > > > > > > > > #endif /* __TPM_V2_H */ > > > > > diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c > > > > > index 3e240bb4c67..3de4841974a 100644 > > > > > --- a/lib/tpm-v2.c > > > > > +++ b/lib/tpm-v2.c > > > > > @@ -679,3 +679,27 @@ u32 tpm2_submit_command(struct udevice *dev, const u8 *sendbuf, > > > > > { > > > > > return tpm_sendrecv_command(dev, sendbuf, recvbuf, recv_size); > > > > > } > > > > > + > > > > > +u32 tpm2_cr50_report_state(struct udevice *dev, u8 *recvbuf, size_t *recv_size) > > > > > +{ > > > > > + u8 command_v2[COMMAND_BUFFER_SIZE] = { > > > > > + /* header 10 bytes */ > > > > > + tpm_u16(TPM2_ST_NO_SESSIONS), /* TAG */ > > > > > + tpm_u32(10 + 2), /* Length */ > > > > > + tpm_u32(TPM2_CR50_VENDOR_COMMAND), /* Command code */ > > > > > + > > > > > + tpm_u16(TPM2_CR50_SUB_CMD_REPORT_TPM_STATE), > > > > > + }; > > > > > + int ret; > > > > > + > > > > > + ret = tpm_sendrecv_command(dev, command_v2, recvbuf, recv_size); > > > > > + log_debug("ret=%s, %x\n", dev->name, ret); > > > > > + if (ret) > > > > > + return ret; > > > > > + if (*recv_size < 12) > > > > > + return -ENODATA; > > > > > + *recv_size -= 12; > > > > > + memcpy(recvbuf, recvbuf + 12, *recv_size); > > > > > + > > > > > + return 0; > > > > > +} > > > > > > > > Same here, this functions seems ok but shouldn't land in the generic TPM API > > > > > > So shall I create a new tpm_cr50.h header file? What about the C file? > > > > Yea the header file seems fine (I assume in drivers/tpm?) > > > > About the C file, tpm2_cr50_report_state() is called by > > cr50_i2c_report_state() which is static in the drivers. Can't we just > > move the function there? > > Just digging into this but I think you have the wrong end of the stick. > > I have added a new TPM method to the uclass for this. We cannot call > functions 'sideways' of that mechanism as it violates how driver model > works. > I am not sure I am following here. I saw the uclass patches and they seem fine. The only thing that needs to be changed is move the cr50* generic functions outside the lib/tpm-v2.c API. This seems doable and the only thing that prevents us from doing it is some definitions in lib/tpm-utils.h which the new functions need. I'll go reply on the new patchset. Thanks /Ilias > The feature is implemented for cr50 and sandbox. It could be > implemented for other TPMs if they have any info that can usefully be > provided, such as the state of the PCRs or something else useful or > important. > > So I think my patches are already doing all this correctly. Can you > please take another look? I'll send v3 as Heinrich had a minor comment > on a function comment. Also one of your changes. > > Regards, > Simon
Hi Ilias, On Mon, 22 Aug 2022 at 00:00, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > Hi Simon, > > On Fri, 19 Aug 2022 at 16:47, Simon Glass <sjg@chromium.org> wrote: > > > > Hi Ilias, > > > > On Thu, 18 Aug 2022 at 01:29, Ilias Apalodimas > > <ilias.apalodimas@linaro.org> wrote: > > > > > > Hi Simon, > > > > > > On Wed, 17 Aug 2022 at 21:54, Simon Glass <sjg@chromium.org> wrote: > > > > > > > > Hi Ilias, > > > > > > > > On Tue, 16 Aug 2022 at 06:43, Ilias Apalodimas > > > > <ilias.apalodimas@linaro.org> wrote: > > > > > > > > > > Hi Simon, > > > > > > > > > > I know little of this device and the whole patch seems fine apart from > > > > > the definitions and declarations of the state functions. > > > > > > > > > > > > > > > On Sat, 13 Aug 2022 at 22:56, Simon Glass <sjg@chromium.org> wrote: > > > > > > > > > > > > > > > > > > > > > > > drivers/tpm/cr50_i2c.c | 117 +++++++++++++++++++++++++++++++++++++++++ > > > > > > include/tpm-v2.h | 54 +++++++++++++++++++ > > > > > > lib/tpm-v2.c | 24 +++++++++ > > > > > > > > > > [...] > > > > > > > > > > > diff --git a/include/tpm-v2.h b/include/tpm-v2.h > > > > > > index e79c90b9395..8e90a616220 100644 > > > > > > --- a/include/tpm-v2.h > > > > > > +++ b/include/tpm-v2.h > > > > > > @@ -419,6 +419,50 @@ enum { > > > > > > HR_NV_INDEX = TPM_HT_NV_INDEX << HR_SHIFT, > > > > > > }; > > > > > > > > > > > > +/* > > > > > > + * Operations specific to the Cr50 TPM used on Chromium OS and Android devices > > > > > > + * > > > > > > + * FIXME: below is not enough to differentiate between vendors commands > > > > > > + * of numerous devices. However, the current tpm2 APIs aren't very amenable > > > > > > + * to extending generically because the marshaling code is assuming all > > > > > > + * knowledge of all commands. > > > > > > + */ > > > > > > +#define TPM2_CC_VENDOR_BIT_MASK 0x20000000 > > > > > > + > > > > > > +#define TPM2_CR50_VENDOR_COMMAND (TPM2_CC_VENDOR_BIT_MASK | 0) > > > > > > +#define TPM2_CR50_SUB_CMD_IMMEDIATE_RESET 19 > > > > > > +#define TPM2_CR50_SUB_CMD_NVMEM_ENABLE_COMMITS 21 > > > > > > +#define TPM2_CR50_SUB_CMD_REPORT_TPM_STATE 23 > > > > > > +#define TPM2_CR50_SUB_CMD_TURN_UPDATE_ON 24 > > > > > > +#define TPM2_CR50_SUB_CMD_GET_REC_BTN 29 > > > > > > +#define TPM2_CR50_SUB_CMD_TPM_MODE 40 > > > > > > +#define TPM2_CR50_SUB_CMD_GET_BOOT_MODE 52 > > > > > > +#define TPM2_CR50_SUB_CMD_RESET_EC 53 > > > > > > + > > > > > > +/* Cr50 vendor-specific error codes. */ > > > > > > +#define VENDOR_RC_ERR 0x00000500 > > > > > > +enum cr50_vendor_rc { > > > > > > + VENDOR_RC_INTERNAL_ERROR = (VENDOR_RC_ERR | 6), > > > > > > + VENDOR_RC_NO_SUCH_SUBCOMMAND = (VENDOR_RC_ERR | 8), > > > > > > + VENDOR_RC_NO_SUCH_COMMAND = (VENDOR_RC_ERR | 127), > > > > > > +}; > > > > > > + > > > > > > +enum cr50_tpm_mode { > > > > > > + /* > > > > > > + * Default state: TPM is enabled, and may be set to either > > > > > > + * TPM_MODE_ENABLED or TPM_MODE_DISABLED. > > > > > > + */ > > > > > > + TPM_MODE_ENABLED_TENTATIVE = 0, > > > > > > + > > > > > > + /* TPM is enabled, and mode may not be changed. */ > > > > > > + TPM_MODE_ENABLED = 1, > > > > > > + > > > > > > + /* TPM is disabled, and mode may not be changed. */ > > > > > > + TPM_MODE_DISABLED = 2, > > > > > > + > > > > > > + TPM_MODE_INVALID, > > > > > > +}; > > > > > > + > > > > > > /** > > > > > > * Issue a TPM2_Startup command. > > > > > > * > > > > > > @@ -658,4 +702,14 @@ u32 tpm2_disable_platform_hierarchy(struct udevice *dev); > > > > > > u32 tpm2_submit_command(struct udevice *dev, const u8 *sendbuf, > > > > > > u8 *recvbuf, size_t *recv_size); > > > > > > > > > > > > +/** > > > > > > + * tpm_cr50_report_state() - Report the Cr50 internal state > > > > > > + * > > > > > > + * @dev: TPM device > > > > > > + * @recvbuf: Buffer to save the response to > > > > > > + * @recv_size: Pointer to the size of the response buffer > > > > > > + * Return: result of the operation > > > > > > + */ > > > > > > +u32 tpm2_cr50_report_state(struct udevice *dev, u8 *recvbuf, size_t *recv_size); > > > > > > + > > > > > > > > > > I think we should keep the generic include files clean for hardware > > > > > specific details. > > > > > > > > > > > #endif /* __TPM_V2_H */ > > > > > > diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c > > > > > > index 3e240bb4c67..3de4841974a 100644 > > > > > > --- a/lib/tpm-v2.c > > > > > > +++ b/lib/tpm-v2.c > > > > > > @@ -679,3 +679,27 @@ u32 tpm2_submit_command(struct udevice *dev, const u8 *sendbuf, > > > > > > { > > > > > > return tpm_sendrecv_command(dev, sendbuf, recvbuf, recv_size); > > > > > > } > > > > > > + > > > > > > +u32 tpm2_cr50_report_state(struct udevice *dev, u8 *recvbuf, size_t *recv_size) > > > > > > +{ > > > > > > + u8 command_v2[COMMAND_BUFFER_SIZE] = { > > > > > > + /* header 10 bytes */ > > > > > > + tpm_u16(TPM2_ST_NO_SESSIONS), /* TAG */ > > > > > > + tpm_u32(10 + 2), /* Length */ > > > > > > + tpm_u32(TPM2_CR50_VENDOR_COMMAND), /* Command code */ > > > > > > + > > > > > > + tpm_u16(TPM2_CR50_SUB_CMD_REPORT_TPM_STATE), > > > > > > + }; > > > > > > + int ret; > > > > > > + > > > > > > + ret = tpm_sendrecv_command(dev, command_v2, recvbuf, recv_size); > > > > > > + log_debug("ret=%s, %x\n", dev->name, ret); > > > > > > + if (ret) > > > > > > + return ret; > > > > > > + if (*recv_size < 12) > > > > > > + return -ENODATA; > > > > > > + *recv_size -= 12; > > > > > > + memcpy(recvbuf, recvbuf + 12, *recv_size); > > > > > > + > > > > > > + return 0; > > > > > > +} > > > > > > > > > > Same here, this functions seems ok but shouldn't land in the generic TPM API > > > > > > > > So shall I create a new tpm_cr50.h header file? What about the C file? > > > > > > Yea the header file seems fine (I assume in drivers/tpm?) > > > > > > About the C file, tpm2_cr50_report_state() is called by > > > cr50_i2c_report_state() which is static in the drivers. Can't we just > > > move the function there? > > > > Just digging into this but I think you have the wrong end of the stick. > > > > I have added a new TPM method to the uclass for this. We cannot call > > functions 'sideways' of that mechanism as it violates how driver model > > works. > > > > I am not sure I am following here. I saw the uclass patches and they > seem fine. The only thing that needs to be changed is move the cr50* > generic functions outside the lib/tpm-v2.c API. This seems doable and > the only thing that prevents us from doing it is some definitions in > lib/tpm-utils.h which the new functions need. > > I'll go reply on the new patchset. It would be good to get past this...see my comments on the other patch. Regards, Simon > > Thanks > /Ilias > > The feature is implemented for cr50 and sandbox. It could be > > implemented for other TPMs if they have any info that can usefully be > > provided, such as the state of the PCRs or something else useful or > > important. > > > > So I think my patches are already doing all this correctly. Can you > > please take another look? I'll send v3 as Heinrich had a minor comment > > on a function comment. Also one of your changes. > > > > Regards, > > Simon
diff --git a/drivers/tpm/cr50_i2c.c b/drivers/tpm/cr50_i2c.c index f8c30878947..dabf617be0e 100644 --- a/drivers/tpm/cr50_i2c.c +++ b/drivers/tpm/cr50_i2c.c @@ -13,11 +13,13 @@ #include <irq.h> #include <log.h> #include <spl.h> +#include <tpm-common.h> #include <tpm-v2.h> #include <acpi/acpigen.h> #include <acpi/acpi_device.h> #include <asm/gpio.h> #include <asm/io.h> +#include <asm/unaligned.h> #include <linux/delay.h> #include <dm/acpi.h> @@ -54,6 +56,41 @@ struct cr50_priv { bool use_irq; }; +/* + * The below structure represents the body of the response to the 'report tpm + * state' vendor command. + * + * It is transferred over the wire, so it needs to be serialized/deserialized, + * and it is likely to change, so its contents must be versioned. + */ +#define TPM_STATE_VERSION 1 +struct tpm_vendor_state { + u32 version; + /* + * The following three fields are set by the TPM in case of an assert. + * There is no other processing than setting the source code line + * number, error code and the first 4 characters of the function name. + * + * We don't expect this happening, but it is included in the report + * just in case. + */ + u32 fail_line; /* s_failLIne */ + u32 fail_code; /* s_failCode */ + char func_name[4]; /* s_failFunction, limited to 4 chars */ + + /* + * The following two fields are the current time filtered value of the + * 'failed tries' TPM counter, and the maximum allowed value of the + * counter. + * + * failed_tries == max_tries is the definition of the TPM lockout + * condition. + */ + u32 failed_tries; /* gp.failedTries */ + u32 max_tries; /* gp.maxTries */ + /* The below fields are present in version 2 and above */ +}; + /* Wait for interrupt to indicate TPM is ready */ static int cr50_i2c_wait_tpm_ready(struct udevice *dev) { @@ -573,6 +610,85 @@ static int cr50_i2c_get_desc(struct udevice *dev, char *buf, int size) return len; } +static int stringify_state(char *buf, int len, char *str, size_t max_size) +{ + struct tpm_vendor_state state; + size_t text_size = 0; + + state.version = get_unaligned_be32(buf + + offsetof(struct tpm_vendor_state, version)); + state.fail_line = get_unaligned_be32(buf + + offsetof(struct tpm_vendor_state, fail_line)); + state.fail_code = get_unaligned_be32(buf + + offsetof(struct tpm_vendor_state, fail_code)); + memcpy(state.func_name, + buf + offsetof(struct tpm_vendor_state, func_name), + sizeof(state.func_name)); + state.failed_tries = get_unaligned_be32(buf + + offsetof(struct tpm_vendor_state, failed_tries)); + state.max_tries = get_unaligned_be32(buf + + offsetof(struct tpm_vendor_state, max_tries)); + + text_size += snprintf(str + text_size, max_size - text_size, + "v=%d", state.version); + if (text_size >= max_size) + return -ENOSPC; + + if (state.version > TPM_STATE_VERSION) + text_size += snprintf(str + text_size, + max_size - text_size, + " not fully supported\n"); + if (text_size >= max_size) + return -ENOSPC; + + if (state.version == 0) + return -EINVAL; /* This should never happen */ + + text_size += snprintf(str + text_size, + max_size - text_size, + " failed_tries=%d max_tries=%d\n", + state.failed_tries, state.max_tries); + if (text_size >= max_size) + return -ENOSPC; + + if (state.fail_line) { + /* make sure function name is zero terminated. */ + char func_name[sizeof(state.func_name) + 1]; + + memcpy(func_name, state.func_name, sizeof(state.func_name)); + func_name[sizeof(state.func_name)] = '\0'; + + text_size += snprintf(str + text_size, + max_size - text_size, + "tpm failed: f %s line %d code %d", + func_name, + state.fail_line, + state.fail_code); + if (text_size >= max_size) + return -ENOSPC; + } + + return 0; +} + +static int cr50_i2c_report_state(struct udevice *dev, char *str, int str_max) +{ + char buf[50]; + int buf_size = sizeof(buf); + int ret; + + ret = tpm2_cr50_report_state(dev, buf, &buf_size); + if (ret) + return ret; + + /* TPM responded as expected */ + ret = stringify_state(buf, buf_size, str, str_max); + if (ret) + return ret; + + return 0; +} + static int cr50_i2c_open(struct udevice *dev) { char buf[80]; @@ -730,6 +846,7 @@ struct acpi_ops cr50_acpi_ops = { static const struct tpm_ops cr50_i2c_ops = { .open = cr50_i2c_open, .get_desc = cr50_i2c_get_desc, + .report_state = cr50_i2c_report_state, .send = cr50_i2c_send, .recv = cr50_i2c_recv, .cleanup = cr50_i2c_cleanup, diff --git a/include/tpm-v2.h b/include/tpm-v2.h index e79c90b9395..8e90a616220 100644 --- a/include/tpm-v2.h +++ b/include/tpm-v2.h @@ -419,6 +419,50 @@ enum { HR_NV_INDEX = TPM_HT_NV_INDEX << HR_SHIFT, }; +/* + * Operations specific to the Cr50 TPM used on Chromium OS and Android devices + * + * FIXME: below is not enough to differentiate between vendors commands + * of numerous devices. However, the current tpm2 APIs aren't very amenable + * to extending generically because the marshaling code is assuming all + * knowledge of all commands. + */ +#define TPM2_CC_VENDOR_BIT_MASK 0x20000000 + +#define TPM2_CR50_VENDOR_COMMAND (TPM2_CC_VENDOR_BIT_MASK | 0) +#define TPM2_CR50_SUB_CMD_IMMEDIATE_RESET 19 +#define TPM2_CR50_SUB_CMD_NVMEM_ENABLE_COMMITS 21 +#define TPM2_CR50_SUB_CMD_REPORT_TPM_STATE 23 +#define TPM2_CR50_SUB_CMD_TURN_UPDATE_ON 24 +#define TPM2_CR50_SUB_CMD_GET_REC_BTN 29 +#define TPM2_CR50_SUB_CMD_TPM_MODE 40 +#define TPM2_CR50_SUB_CMD_GET_BOOT_MODE 52 +#define TPM2_CR50_SUB_CMD_RESET_EC 53 + +/* Cr50 vendor-specific error codes. */ +#define VENDOR_RC_ERR 0x00000500 +enum cr50_vendor_rc { + VENDOR_RC_INTERNAL_ERROR = (VENDOR_RC_ERR | 6), + VENDOR_RC_NO_SUCH_SUBCOMMAND = (VENDOR_RC_ERR | 8), + VENDOR_RC_NO_SUCH_COMMAND = (VENDOR_RC_ERR | 127), +}; + +enum cr50_tpm_mode { + /* + * Default state: TPM is enabled, and may be set to either + * TPM_MODE_ENABLED or TPM_MODE_DISABLED. + */ + TPM_MODE_ENABLED_TENTATIVE = 0, + + /* TPM is enabled, and mode may not be changed. */ + TPM_MODE_ENABLED = 1, + + /* TPM is disabled, and mode may not be changed. */ + TPM_MODE_DISABLED = 2, + + TPM_MODE_INVALID, +}; + /** * Issue a TPM2_Startup command. * @@ -658,4 +702,14 @@ u32 tpm2_disable_platform_hierarchy(struct udevice *dev); u32 tpm2_submit_command(struct udevice *dev, const u8 *sendbuf, u8 *recvbuf, size_t *recv_size); +/** + * tpm_cr50_report_state() - Report the Cr50 internal state + * + * @dev: TPM device + * @recvbuf: Buffer to save the response to + * @recv_size: Pointer to the size of the response buffer + * Return: result of the operation + */ +u32 tpm2_cr50_report_state(struct udevice *dev, u8 *recvbuf, size_t *recv_size); + #endif /* __TPM_V2_H */ diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c index 3e240bb4c67..3de4841974a 100644 --- a/lib/tpm-v2.c +++ b/lib/tpm-v2.c @@ -679,3 +679,27 @@ u32 tpm2_submit_command(struct udevice *dev, const u8 *sendbuf, { return tpm_sendrecv_command(dev, sendbuf, recvbuf, recv_size); } + +u32 tpm2_cr50_report_state(struct udevice *dev, u8 *recvbuf, size_t *recv_size) +{ + u8 command_v2[COMMAND_BUFFER_SIZE] = { + /* header 10 bytes */ + tpm_u16(TPM2_ST_NO_SESSIONS), /* TAG */ + tpm_u32(10 + 2), /* Length */ + tpm_u32(TPM2_CR50_VENDOR_COMMAND), /* Command code */ + + tpm_u16(TPM2_CR50_SUB_CMD_REPORT_TPM_STATE), + }; + int ret; + + ret = tpm_sendrecv_command(dev, command_v2, recvbuf, recv_size); + log_debug("ret=%s, %x\n", dev->name, ret); + if (ret) + return ret; + if (*recv_size < 12) + return -ENODATA; + *recv_size -= 12; + memcpy(recvbuf, recvbuf + 12, *recv_size); + + return 0; +}
Add a vendor-specific TPM2 command for this and implement it for Cr50. Note: This is not part of the TPM spec, but is a Cr50 extension. Signed-off-by: Simon Glass <sjg@chromium.org> --- (no changes since v1) drivers/tpm/cr50_i2c.c | 117 +++++++++++++++++++++++++++++++++++++++++ include/tpm-v2.h | 54 +++++++++++++++++++ lib/tpm-v2.c | 24 +++++++++ 3 files changed, 195 insertions(+)