<font face="Default Sans Serif,Verdana,Arial,Helvetica,sans-serif" size="2"><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><span style="font-size: 12.7272720336914px;">>+</span></div><font face="Verdana, Arial, Helvetica, sans-serif">>+A TPM is a crypto chip that is found in many systems. Besides it</font><br><font face="Verdana, Arial, Helvetica, sans-serif">>offering</font><br><font face="Verdana, Arial, Helvetica, sans-serif">>+a secure key store, among other functionality, it is also used to</font><br><font face="Verdana, Arial, Helvetica, sans-serif">>implement</font><br><font face="Verdana, Arial, Helvetica, sans-serif">>+'trusted boot'. This is realized by code in the firmware measuring</font><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><br></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;">I agree this needs a rephrase.</div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><br></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><br><br></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><span style="font-size: 12.7272720336914px;">>+ * Send a message via CRQ and wait for the response</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ */</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+static bool spapr_send_crq_and_wait(unsigned long unit,</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ struct crq *crq,</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ struct crq **response,</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ unsigned timeout,</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ vtpm_drv_state state1,</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ vtpm_drv_state state2)</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+{</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ long rc;</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ unsigned i;</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ *response = get_response_crq();</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ vtpm_drv_state_set(state1, VTPM_DRV_ERROR_NO_FAILURE);</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ rc = hv_send_crq(unit, (uint64_t *)crq);</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ if (rc != H_SUCCESS) {</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ vtpm_drv_state_set(VTPM_DRV_STATE_WAIT_INIT,</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ VTPM_DRV_ERROR_TPM_CRQ_ERROR);</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ return false;</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ }</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ vtpm_drv_state_set(state2, VTPM_DRV_ERROR_NO_FAILURE);</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ for (i = 0; i < timeout; i += 1000) {</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ if (((*response)->valid & PAPR_VTPM_MSG_RESULT))</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ return true;</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ SLOF_usleep(1000);</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ }</span><br style="font-size: 12.7272720336914px;"><br style="font-size: 12.7272720336914px;"></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><br></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;">The 'timeout' is given the value of '10' in number of the calls to the spapr_send_crq_and_wait(), which means the loop will be executed one time exactly.</div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;">Should <span style="font-size: 12.7272720336914px;">SLOF_usleep(1000) be done bef</span><span style="font-size: 12.7272720336914px;">ore the if-block? </span></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><span style="font-size: 12.7272720336914px;"><br></span></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><span style="font-size: 12.7272720336914px;"><br></span></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><span style="font-size: 12.7272720336914px;"><br></span></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><span style="font-size: 12.7272720336914px;">>+</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+/*</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ * Get parameters from the CRQ</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ */</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+static bool spapr_vtpm_get_params(void)</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+{</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ struct crq crq, *response;</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ static bool completed = false; /* only once */</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ if (completed)</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ return true;</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ /* get the TPM version */</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ crq.valid = PAPR_VTPM_VALID_COMMAND;</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ crq.msg = PAPR_VTPM_GET_VERSION;</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ if (!spapr_send_crq_and_wait(spapr_vtpm.unit, &crq, &response, 10,</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ VTPM_DRV_STATE_SEND_GET_VERSION,</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ VTPM_DRV_STATE_WAIT_VERSION)) {</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ dprintf("Failure getting TPM version from CRQ\n");</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ return false;</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ }</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ vtpm_drv_state_set(VTPM_DRV_STATE_CHECK_VERSION,</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ VTPM_DRV_ERROR_NO_FAILURE);</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ spapr_vtpm.tpm_version = be32_to_cpu(response->data);</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+ dprintf("TPM backend version: %d\n", spapr_vtpm.tpm_version);</span><br style="font-size: 12.7272720336914px;"><span style="font-size: 12.7272720336914px;">>+</span></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><br></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><span style="font-size: 12.7272720336914px;">This is minor. dprintf() can use %u format here.</span></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><br></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><br><br></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;">>+ if (!spapr_send_crq_and_wait(spapr_vtpm.unit, &crq, &response, 10,<br>>+ VTPM_DRV_STATE_SEND_BUFSIZE_REQ,<br>>+ VTPM_DRV_STATE_WAIT_BUFSIZE)) {<br>>+ dprintf("Failure getting RTCE buffer size from CRQ\n");<br>>+ return false;<br>>+ }<br>>+<br>>+ vtpm_drv_state_set(VTPM_DRV_STATE_ALLOC_RTCE_BUF,<br>>+ VTPM_DRV_ERROR_NO_FAILURE);<br>>+<br>>+ dprintf("RTCE buffer size: %u\n", be16_to_cpu(response->len));<br>>+ spapr_vtpm.buffer_size = response->len;</div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><span style="font-size: 12.7272720336914px;"><br></span></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><span style="font-size: 12.7272720336914px;">Should the above line be </span><span style="font-size: 12.7272720336914px;">spapr_vtpm.buffer_size = be16_to_cpu(response->len) ?</span></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><br></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><br></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><br></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><span style="font-size: 12.7272720336914px;">>+static bool spapr_vtpm_probe(void)</span></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;">>+{<br>>+ spapr_vtpm_init();<br>>+<br>>+ if (!spapr_vtpm.qaddr) {<br>>+ spapr_vtpm.qaddr = SLOF_alloc_mem(spapr_vtpm.qsize);</div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><br></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><br></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;">It's better with a cast to 'unsigned long' here, before the assignment.</div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><br></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><span style="font-size: 12.7272720336914px;"><br></span></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><span style="font-size: 12.7272720336914px;"><br></span></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><span style="font-size: 12.7272720336914px;">>+ spapr_vtpm.response = get_response_crq();</span></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;">>+ /* response CRQ has been set and valid field cleared */<br>>+<br>>+ crq.valid = PAPR_VTPM_VALID_COMMAND;<br>>+ crq.msg = PAPR_VTPM_TPM_COMMAND;<br>>+ crq.len = cpu_to_be16(len);<br>>+ crq.data = cpu_to_be32((uint64_t)data);</div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><br></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><br></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;">The cast should be <span style="font-size: 12.7272720336914px;">of type uint32_t instead?</span></div><div><span style="font-size: 12.7272720336914px;"><br></span></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><br></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><br></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><span style="font-size: 12.7272720336914px;"><br></span></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><span style="font-size: 12.7272720336914px;">>+static bool spapr_vtpm_readresponse(uint8_t *buffer, uint32_t *len)</span></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;">>+{<br>>+ if (vtpm_drv_error_get() != VTPM_DRV_ERROR_NO_FAILURE) {<br>>+ printf("VTPM CRQ: In failure mode\n");<br>>+ return false;<br>>+ }<br>>+<br>>+ /* response CRQ has been set */<br>>+ *len = MIN(*len, be32_to_cpu(spapr_vtpm.response->len));<br>>+</div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><br></div><div><font face="Verdana, Arial, Helvetica, sans-serif">Should it be using be16_to_cpu() for data instead?</font></div><div><font face="Verdana, Arial, Helvetica, sans-serif"><br></font></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><br></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><br></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;">>+ memcpy(buffer, (void *)(uint64_t)spapr_vtpm.response->data, *len);<br>>+</div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><br></div><div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;">Should it be casted to the type of uint32_t instead?</div><div style=""><div style=""><font face="Verdana, Arial, Helvetica, sans-serif"><br></font></div><div><br></div></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><br></div><div style="font-family: Verdana, Arial, Helvetica, sans-serif;"><span style="font-size: 12.7272720336914px;">>+bool spapr_is_vtpm_present(void)</span></div><font face="Verdana, Arial, Helvetica, sans-serif">>+{</font><br><font face="Verdana, Arial, Helvetica, sans-serif">>+ bool rc = false;</font><br><font face="Verdana, Arial, Helvetica, sans-serif">>+</font><br><font face="Verdana, Arial, Helvetica, sans-serif">>+ if (spapr_vtpm_probe()) {</font><br><font face="Verdana, Arial, Helvetica, sans-serif">>+ spapr_vtpm_init();</font></div><div><font face="Verdana, Arial, Helvetica, sans-serif">>+ rc = true;</font><br><font face="Verdana, Arial, Helvetica, sans-serif">>+ }</font></div><div><font face="Verdana, Arial, Helvetica, sans-serif"><br></font></div><div><font face="Verdana, Arial, Helvetica, sans-serif" style="font-size: 12.7272720336914px;">spapr_vtpm_probe() already called </font><span style="font-size: 12.7272720336914px; font-family: Verdana, Arial, Helvetica, sans-serif;">spapr_vtpm_init() within the function, why do we need to call </span><span style="font-size: 12.7272720336914px; font-family: Verdana, Arial, Helvetica, sans-serif;">spapr_vtpm_init() again in this if-block?</span></div><div><font face="Verdana, Arial, Helvetica, sans-serif"><br></font></div><div><font face="Verdana, Arial, Helvetica, sans-serif"><br></font></div><div><font face="Verdana, Arial, Helvetica, sans-serif"><br></font></div><div><font face="Verdana, Arial, Helvetica, sans-serif"><br></font></div><div><span style="font-family: Verdana, Arial, Helvetica, sans-serif; font-size: 12.7272720336914px;">Regards,</span><br style="font-family: Verdana, Arial, Helvetica, sans-serif; font-size: 12.7272720336914px;"><span style="font-family: Verdana, Arial, Helvetica, sans-serif; font-size: 12.7272720336914px;">Vicky</span><br style="font-family: Verdana, Arial, Helvetica, sans-serif; font-size: 12.7272720336914px;"><br style="font-family: Verdana, Arial, Helvetica, sans-serif; font-size: 12.7272720336914px;"><span style="font-family: Verdana, Arial, Helvetica, sans-serif; font-size: 12.7272720336914px;">Hon Ching (Vicky) Lo</span><br style="font-family: Verdana, Arial, Helvetica, sans-serif; font-size: 12.7272720336914px;"><span style="font-family: Verdana, Arial, Helvetica, sans-serif; font-size: 12.7272720336914px;">Linux Security Development</span><br style="font-family: Verdana, Arial, Helvetica, sans-serif; font-size: 12.7272720336914px;"><span style="font-family: Verdana, Arial, Helvetica, sans-serif; font-size: 12.7272720336914px;">Notes: </span><a href="mailto:lo1@us.ibm.com" style="font-family: Verdana, Arial, Helvetica, sans-serif; font-size: 12.7272720336914px;">lo1@us.ibm.com</a><br style="font-family: Verdana, Arial, Helvetica, sans-serif; font-size: 12.7272720336914px;"><span style="font-family: Verdana, Arial, Helvetica, sans-serif; font-size: 12.7272720336914px;">Office: 845-435-8946 (T/L: 295-8946)</span></div><div><span style="font-family: Verdana, Arial, Helvetica, sans-serif; font-size: 12.7272720336914px;"><br></span></div></font><BR>