<html><body><p><font size="2">A few comments:</font><br><br><font size="2">1024 is not guaranteed, and the PC client uses 512. The safest approach is to use getcapability. You also have to handle back level TPMs that don't implement that capability. See readNvBufferMax() for the implementation.</font><br><br><font size="2">A int64_t signed value is strange for a two byte unsigned length Letting it go negative is risky, especially when adding it to a pointer. Always subtracting 1024 is a C trick - as a minimum, it needs lots of comments. .I think it's cleaner to calculate the bytes to read once and put that value into in->size, then subtract in->size, terminating at zero.</font><br><br><tt><font size="2"><br>> From: Eric Richter <erichte@linux.ibm.com></font></tt><br><tt><font size="2">> To: skiboot@lists.ozlabs.org</font></tt><br><tt><font size="2">> Cc: klaus@linux.ibm.com, nayna@linux.ibm.com</font></tt><br><tt><font size="2">> Date: 09/28/2020 06:07 PM</font></tt><br><tt><font size="2">> Subject: [EXTERNAL] [Skiboot] [PATCH v6a 1/4] tssskiboot.c: chunk <br>> reads/writes in 1024-sized buffers to support larger nv indices</font></tt><br><tt><font size="2">> Sent by: "Skiboot" <skiboot-bounces+kgoldman=us.ibm.com@lists.ozlabs.org></font></tt><br><tt><font size="2">> <br>> The Nuvoton npct650 chip has a command buffer max size of 1024.<br>> Attempting to read or write from an NV index larger than this value<br>> would return an error.<br>> <br>> This patch changes the tss_nv_read and tss_nv_write commands to chunk<br>> their operations in 1024-byte batches to allow support for larger NV<br>> indices.<br>> <br>> Signed-off-by: Eric Richter <erichte@linux.ibm.com><br>> ---<br>> libstb/tss2/tssskiboot.c | 82 ++++++++++++++++++++++++++--------------<br>> 1 file changed, 54 insertions(+), 28 deletions(-)<br>> <br>> diff --git a/libstb/tss2/tssskiboot.c b/libstb/tss2/tssskiboot.c<br>> index 5f6d7611..b97159e7 100644<br>> --- a/libstb/tss2/tssskiboot.c<br>> +++ b/libstb/tss2/tssskiboot.c<br>> @@ -13,6 +13,8 @@<br>> #include <ibmtss/tssmarshal.h><br>> #include <ibmtss/tssresponsecode.h><br>> <br>> +#define TSS_MAX_NV_BUFFER_SIZE 1024<br>> +<br>> /*<br>> * Helper to string-fy TSS error response codes.<br>> */<br>> @@ -98,6 +100,7 @@ int tss_nv_read(TPMI_RH_NV_INDEX nv_index, void *buffer,<br>> NV_Read_Out *out = NULL;<br>> NV_Read_In *in = NULL;<br>> TPM_RC rc = OPAL_SUCCESS;<br>> + int64_t buffer_remaining;<br>> <br>> if (!buffer) {<br>> rc = OPAL_PARAMETER;<br>> @@ -125,21 +128,30 @@ int tss_nv_read(TPMI_RH_NV_INDEX nv_index, void *buffer,<br>> <br>> in->nvIndex = nv_index;<br>> in->authHandle = nv_index;<br>> - in->offset = offset;<br>> - in->size = buffer_size;<br>> <br>> - rc = TSS_Execute(context,<br>> - (RESPONSE_PARAMETERS *) out,<br>> - (COMMAND_PARAMETERS *) in,<br>> - NULL,<br>> - TPM_CC_NV_Read,<br>> - TPM_RS_PW, NULL, 0,<br>> - TPM_RH_NULL, NULL, 0);<br>> + buffer_remaining = buffer_size;<br>> + while (buffer_remaining > 0) {<br>> + in->offset = offset;<br>> + in->size = MIN(TSS_MAX_NV_BUFFER_SIZE, buffer_remaining);<br>> <br>> - if (!rc)<br>> - memcpy(buffer, out->data.b.buffer, buffer_size);<br>> - else<br>> - tss_error_trace("tss_nv_read", rc);<br>> + rc = TSS_Execute(context,<br>> + (RESPONSE_PARAMETERS *) out,<br>> + (COMMAND_PARAMETERS *) in,<br>> + NULL,<br>> + TPM_CC_NV_Read,<br>> + TPM_RS_PW, NULL, 0,<br>> + TPM_RH_NULL, NULL, 0);<br>> +<br>> + if (rc) {<br>> + tss_error_trace("tss_nv_read", rc);<br>> + goto cleanup;<br>> + }<br>> +<br>> + memcpy(buffer, out->data.b.buffer, in->size);<br>> + buffer += TSS_MAX_NV_BUFFER_SIZE;<br>> + buffer_remaining -= TSS_MAX_NV_BUFFER_SIZE;<br>> + offset += TSS_MAX_NV_BUFFER_SIZE;<br>> + }<br>> <br>> cleanup:<br>> TSS_Delete(context);<br>> @@ -161,6 +173,7 @@ int tss_nv_write(TPMI_RH_NV_INDEX nv_index, void *buffer,<br>> TSS_CONTEXT *context = NULL;<br>> NV_Write_In *in = NULL;<br>> TPM_RC rc = OPAL_SUCCESS;<br>> + int64_t buffer_remaining;<br>> <br>> if (!buffer) {<br>> rc = OPAL_PARAMETER;<br>> @@ -182,23 +195,36 @@ int tss_nv_write(TPMI_RH_NV_INDEX nv_index, <br>> void *buffer,<br>> <br>> in->nvIndex = nv_index;<br>> in->authHandle = TPM_RH_PLATFORM;<br>> - in->offset = offset;<br>> - rc = TSS_TPM2B_Create(&in->data.b, buffer, buffer_size,<br>> - sizeof(in->data.t.buffer));<br>> - if (rc) {<br>> - tss_error_trace("tss_nv_write", rc);<br>> - goto cleanup;<br>> +<br>> + buffer_remaining = buffer_size;<br>> + while (buffer_remaining > 0) {<br>> + in->offset = offset;<br>> + rc = TSS_TPM2B_Create(&in->data.b, buffer,<br>> + MIN(TSS_MAX_NV_BUFFER_SIZE, buffer_remaining),<br>> + sizeof(in->data.t.buffer));<br>> +<br>> + if (rc) {<br>> + tss_error_trace("tss_nv_write", rc);<br>> + goto cleanup;<br>> + }<br>> +<br>> + rc = TSS_Execute(context,<br>> + NULL,<br>> + (COMMAND_PARAMETERS *) in,<br>> + NULL,<br>> + TPM_CC_NV_Write,<br>> + TPM_RS_PW, NULL, 0,<br>> + TPM_RH_NULL, NULL, 0);<br>> + if (rc) {<br>> + tss_error_trace("tss_nv_write", rc);<br>> + goto cleanup;<br>> + }<br>> +<br>> + buffer += TSS_MAX_NV_BUFFER_SIZE;<br>> + buffer_remaining -= TSS_MAX_NV_BUFFER_SIZE;<br>> + offset += TSS_MAX_NV_BUFFER_SIZE;<br>> }<br>> <br>> - rc = TSS_Execute(context,<br>> - NULL,<br>> - (COMMAND_PARAMETERS *) in,<br>> - NULL,<br>> - TPM_CC_NV_Write,<br>> - TPM_RS_PW, NULL, 0,<br>> - TPM_RH_NULL, NULL, 0);<br>> - if (rc)<br>> - tss_error_trace("tss_nv_write", rc);<br>> cleanup:<br>> TSS_Delete(context);<br>> free(in);<br>> -- <br>> 2.21.1<br>> <br>> _______________________________________________<br>> Skiboot mailing list<br>> Skiboot@lists.ozlabs.org<br>> <a href="INVALID URI REMOVED">INVALID URI REMOVED</a><br>> u=https-3A__lists.ozlabs.org_listinfo_skiboot&d=DwICAg&c=jf_iaSHvJObTbx-<br>> siA1ZOg&r=DZCVG43VcL8GTneMZb8k8lEwb-O1GZktFfre1-<br>> mlmiA&m=Lhg9Fbycamsl8ljo_qgfAqufCe7ygbaEdyA5oatS8Sg&s=UCqK31nTMpsZxNo632V67tYvuDdZbwEZudPrVrWuZnQ&e=<br>> <br></font></tt><BR>
</body></html>