[PATCH v2] tpm: Allow committing non-volatile data

Add an option to tell the TPM to commit non-volatile data immediately it is changed, rather than waiting until later. This is needed in some situations, since if the device reboots it may not write the data.
Add definitions for the rest of the Cr50 commands while we are here.
Signed-off-by: Simon Glass sjg@chromium.org --- I am resending this as I think it got lost.
Changes in v2: - Rebase to master
include/tpm-v2.h | 14 ++++++++++++++ lib/tpm-v2.c | 20 ++++++++++++++++++++ 2 files changed, 34 insertions(+)
diff --git a/include/tpm-v2.h b/include/tpm-v2.h index 8e90a616220..0a03994740d 100644 --- a/include/tpm-v2.h +++ b/include/tpm-v2.h @@ -712,4 +712,18 @@ u32 tpm2_submit_command(struct udevice *dev, const u8 *sendbuf, */ u32 tpm2_cr50_report_state(struct udevice *dev, u8 *recvbuf, size_t *recv_size);
+/* + * tpm2_cr50_enable_nvcommits() - Tell Cr50 to commit NV data immediately + * + * For Chromium OS verified boot, we may reboot or reset at different times, + * possibly leaving non-volatile data unwritten by the TPM. + * + * This vendor command is used to indicate that non-volatile data should be + * written to its store immediately. + * + * @dev TPM device + * Return: result of the operation + */ +u32 tpm2_cr50_enable_nvcommits(struct udevice *dev); + #endif /* __TPM_V2_H */ diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c index bdf019b0f93..5fcd3649b74 100644 --- a/lib/tpm-v2.c +++ b/lib/tpm-v2.c @@ -699,3 +699,23 @@ u32 tpm2_cr50_report_state(struct udevice *dev, u8 *recvbuf, size_t *recv_size)
return 0; } + +u32 tpm2_cr50_enable_nvcommits(struct udevice *dev) +{ + 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_NVMEM_ENABLE_COMMITS), + }; + int ret; + + ret = tpm_sendrecv_command(dev, command_v2, NULL, NULL); + log_debug("ret=%s, %x\n", dev->name, ret); + if (ret) + return ret; + + return 0; +}

Hi Simon,
On Mon, Feb 20, 2023 at 09:31:24AM -0700, Simon Glass wrote:
Add an option to tell the TPM to commit non-volatile data immediately it is changed, rather than waiting until later. This is needed in some situations, since if the device reboots it may not write the data.
Add definitions for the rest of the Cr50 commands while we are here.
This defines a function that's unused. IIRC you said U-Boot doesn't use it, but some code that run for that laptop does right? In any case the function declaration doesn't belong to the TPMv2 library. I think we are better off adding it to the cr50 driver itself. I also assume you compile u-boot in a 'special' way so the linker doesn't get rid of the emitted code? Does t hat mean we can define it as __unused as well?
Thanks /Ilias
Signed-off-by: Simon Glass sjg@chromium.org
I am resending this as I think it got lost.
Changes in v2:
- Rebase to master
include/tpm-v2.h | 14 ++++++++++++++ lib/tpm-v2.c | 20 ++++++++++++++++++++ 2 files changed, 34 insertions(+)
diff --git a/include/tpm-v2.h b/include/tpm-v2.h index 8e90a616220..0a03994740d 100644 --- a/include/tpm-v2.h +++ b/include/tpm-v2.h @@ -712,4 +712,18 @@ u32 tpm2_submit_command(struct udevice *dev, const u8 *sendbuf, */ u32 tpm2_cr50_report_state(struct udevice *dev, u8 *recvbuf, size_t *recv_size);
+/*
- tpm2_cr50_enable_nvcommits() - Tell Cr50 to commit NV data immediately
- For Chromium OS verified boot, we may reboot or reset at different times,
- possibly leaving non-volatile data unwritten by the TPM.
- This vendor command is used to indicate that non-volatile data should be
- written to its store immediately.
- @dev TPM device
- Return: result of the operation
- */
+u32 tpm2_cr50_enable_nvcommits(struct udevice *dev);
#endif /* __TPM_V2_H */ diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c index bdf019b0f93..5fcd3649b74 100644 --- a/lib/tpm-v2.c +++ b/lib/tpm-v2.c @@ -699,3 +699,23 @@ u32 tpm2_cr50_report_state(struct udevice *dev, u8 *recvbuf, size_t *recv_size)
return 0; }
+u32 tpm2_cr50_enable_nvcommits(struct udevice *dev) +{
- 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_NVMEM_ENABLE_COMMITS),
- };
- int ret;
- ret = tpm_sendrecv_command(dev, command_v2, NULL, NULL);
- log_debug("ret=%s, %x\n", dev->name, ret);
- if (ret)
return ret;
- return 0;
+}
2.39.2.637.g21b0678d19-goog

Hi Ilias,
On Tue, 21 Feb 2023 at 06:58, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Simon,
On Mon, Feb 20, 2023 at 09:31:24AM -0700, Simon Glass wrote:
Add an option to tell the TPM to commit non-volatile data immediately it is changed, rather than waiting until later. This is needed in some situations, since if the device reboots it may not write the data.
Add definitions for the rest of the Cr50 commands while we are here.
This defines a function that's unused. IIRC you said U-Boot doesn't use it, but some code that run for that laptop does right?
Yes it is used by ChromeOS code which is not upstream at present.
In any case the function declaration doesn't belong to the TPMv2 library. I think we are better off adding it to the cr50 driver itself. I also
We cannot call tpm_sendrecv_command() from a TPM driver..it is in lib/ and that would be a violation of the software layers. This is a TPM2 command, even if it is specific to cr50.
assume you compile u-boot in a 'special' way so the linker doesn't get rid of the emitted code? Does t hat mean we can define it as __unused as well?
Nothing special, but this allows the ChromeOS code to build correctly. I could also add a command to use it, if that helps?
Regards, Simon
Thanks /Ilias
Signed-off-by: Simon Glass sjg@chromium.org
I am resending this as I think it got lost.
Changes in v2:
- Rebase to master
include/tpm-v2.h | 14 ++++++++++++++ lib/tpm-v2.c | 20 ++++++++++++++++++++ 2 files changed, 34 insertions(+)
diff --git a/include/tpm-v2.h b/include/tpm-v2.h index 8e90a616220..0a03994740d 100644 --- a/include/tpm-v2.h +++ b/include/tpm-v2.h @@ -712,4 +712,18 @@ u32 tpm2_submit_command(struct udevice *dev, const u8 *sendbuf, */ u32 tpm2_cr50_report_state(struct udevice *dev, u8 *recvbuf, size_t *recv_size);
+/*
- tpm2_cr50_enable_nvcommits() - Tell Cr50 to commit NV data immediately
- For Chromium OS verified boot, we may reboot or reset at different times,
- possibly leaving non-volatile data unwritten by the TPM.
- This vendor command is used to indicate that non-volatile data should be
- written to its store immediately.
- @dev TPM device
- Return: result of the operation
- */
+u32 tpm2_cr50_enable_nvcommits(struct udevice *dev);
#endif /* __TPM_V2_H */ diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c index bdf019b0f93..5fcd3649b74 100644 --- a/lib/tpm-v2.c +++ b/lib/tpm-v2.c @@ -699,3 +699,23 @@ u32 tpm2_cr50_report_state(struct udevice *dev, u8 *recvbuf, size_t *recv_size)
return 0;
}
+u32 tpm2_cr50_enable_nvcommits(struct udevice *dev) +{
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_NVMEM_ENABLE_COMMITS),
};
int ret;
ret = tpm_sendrecv_command(dev, command_v2, NULL, NULL);
log_debug("ret=%s, %x\n", dev->name, ret);
if (ret)
return ret;
return 0;
+}
2.39.2.637.g21b0678d19-goog

Hi Simon,
We had that discussion in the past.
On Tue, 21 Feb 2023 at 16:09, Simon Glass sjg@chromium.org wrote:
Hi Ilias,
On Tue, 21 Feb 2023 at 06:58, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Simon,
On Mon, Feb 20, 2023 at 09:31:24AM -0700, Simon Glass wrote:
Add an option to tell the TPM to commit non-volatile data immediately it is changed, rather than waiting until later. This is needed in some situations, since if the device reboots it may not write the data.
Add definitions for the rest of the Cr50 commands while we are here.
This defines a function that's unused. IIRC you said U-Boot doesn't use it, but some code that run for that laptop does right?
Yes it is used by ChromeOS code which is not upstream at present.
In any case the function declaration doesn't belong to the TPMv2 library. I think we are better off adding it to the cr50 driver itself. I also
We cannot call tpm_sendrecv_command() from a TPM driver..it is in lib/ and that would be a violation of the software layers. This is a TPM2 command, even if it is specific to cr50.
assume you compile u-boot in a 'special' way so the linker doesn't get rid of the emitted code? Does t hat mean we can define it as __unused as well?
Nothing special, but this allows the ChromeOS code to build correctly. I could also add a command to use it, if that helps?
5208ed187cb6 ("tpm: Allow committing non-volatile data") is what you need. That uses a generic name and takes the command as an argument. IOW calling tpm2_enable_nvcommits(dev, TPM2_CR50_SUB_CMD_NVMEM_ENABLE_COMMITS) will do the right thing for you.
Regards /Ilias
Regards, Simon
Thanks /Ilias
Signed-off-by: Simon Glass sjg@chromium.org
I am resending this as I think it got lost.
Changes in v2:
- Rebase to master
include/tpm-v2.h | 14 ++++++++++++++ lib/tpm-v2.c | 20 ++++++++++++++++++++ 2 files changed, 34 insertions(+)
diff --git a/include/tpm-v2.h b/include/tpm-v2.h index 8e90a616220..0a03994740d 100644 --- a/include/tpm-v2.h +++ b/include/tpm-v2.h @@ -712,4 +712,18 @@ u32 tpm2_submit_command(struct udevice *dev, const u8 *sendbuf, */ u32 tpm2_cr50_report_state(struct udevice *dev, u8 *recvbuf, size_t *recv_size);
+/*
- tpm2_cr50_enable_nvcommits() - Tell Cr50 to commit NV data immediately
- For Chromium OS verified boot, we may reboot or reset at different times,
- possibly leaving non-volatile data unwritten by the TPM.
- This vendor command is used to indicate that non-volatile data should be
- written to its store immediately.
- @dev TPM device
- Return: result of the operation
- */
+u32 tpm2_cr50_enable_nvcommits(struct udevice *dev);
#endif /* __TPM_V2_H */ diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c index bdf019b0f93..5fcd3649b74 100644 --- a/lib/tpm-v2.c +++ b/lib/tpm-v2.c @@ -699,3 +699,23 @@ u32 tpm2_cr50_report_state(struct udevice *dev, u8 *recvbuf, size_t *recv_size)
return 0;
}
+u32 tpm2_cr50_enable_nvcommits(struct udevice *dev) +{
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_NVMEM_ENABLE_COMMITS),
};
int ret;
ret = tpm_sendrecv_command(dev, command_v2, NULL, NULL);
log_debug("ret=%s, %x\n", dev->name, ret);
if (ret)
return ret;
return 0;
+}
2.39.2.637.g21b0678d19-goog

Hi Ilias,
On Tue, 21 Feb 2023 at 07:18, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Simon,
We had that discussion in the past.
On Tue, 21 Feb 2023 at 16:09, Simon Glass sjg@chromium.org wrote:
Hi Ilias,
On Tue, 21 Feb 2023 at 06:58, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Simon,
On Mon, Feb 20, 2023 at 09:31:24AM -0700, Simon Glass wrote:
Add an option to tell the TPM to commit non-volatile data immediately it is changed, rather than waiting until later. This is needed in some situations, since if the device reboots it may not write the data.
Add definitions for the rest of the Cr50 commands while we are here.
This defines a function that's unused. IIRC you said U-Boot doesn't use it, but some code that run for that laptop does right?
Yes it is used by ChromeOS code which is not upstream at present.
In any case the function declaration doesn't belong to the TPMv2 library. I think we are better off adding it to the cr50 driver itself. I also
We cannot call tpm_sendrecv_command() from a TPM driver..it is in lib/ and that would be a violation of the software layers. This is a TPM2 command, even if it is specific to cr50.
assume you compile u-boot in a 'special' way so the linker doesn't get rid of the emitted code? Does t hat mean we can define it as __unused as well?
Nothing special, but this allows the ChromeOS code to build correctly. I could also add a command to use it, if that helps?
5208ed187cb6 ("tpm: Allow committing non-volatile data") is what you need. That uses a generic name and takes the command as an argument. IOW calling tpm2_enable_nvcommits(dev, TPM2_CR50_SUB_CMD_NVMEM_ENABLE_COMMITS) will do the right thing for you.
Thanks for the reminder. I'll try to remember to stop asking :-)
Regards, Simon
participants (2)
-
Ilias Apalodimas
-
Simon Glass