Sorry, should have sent all in one email.
Stefan Berger wrote:
- if (enable)
dprintf(DEBUG_tcg, "Return code from TPM_PhysicalEnable = 0x%08x\n",*returnCode);- else
dprintf(DEBUG_tcg, "Return code from TPM_PhysicalDisable = 0x%08x\n",*returnCode);
Maybe simplify this to one string with %s + enable ? "Enable" : "Disable" ?
+err_exit:
- if (enable)
dprintf(DEBUG_tcg, "TCGBIOS: Enabling the TPM failed.\n");- else
dprintf(DEBUG_tcg, "TCGBIOS: Disabling the TPM failed.\n");
Same here.
- dprintf(DEBUG_tcg, "TCGBIOS: TPM malfunctioning (line %d).\n", __LINE__);
See last email. This dprintf() seems a bit redundant with the error in the lines above.
- if (pf.flags[PERM_FLAG_IDX_DISABLE]) {
if (verbose)printf("TPM must first be enable.\n");
Typo: enabled
+tpm_process_cfg(const tpm_bios_cfg *cfg, int verbose, u32 *returnCode)
..
- if (rc)
printf("Op %d: An error occurred: 0x%x TPM return code: 0x%x\n",cfg->op, rc, *returnCode);
Maybe add a %s __func__ or some word about where this happened.
+show_tpm_menu(int state, int next_scancodes[7])
Are scancodes really 32-bit? If you use a fixed size that's presumably to save a bit of memory. I would prefer next_scancodes to be dynamically sized according to some number of possible scancodes in an enum or somesuch which is known at compile time, and for it to be an array of unsigned char/uint8_t or uint16_t, as appropriate for the hardware. IIRC scancodes are 8 bit?
- if (state & TPM_STATE_ENABLED)
printf(" Enabled");- else
printf(" Disabled");
I'd use %s ? : for these but that's a matter of taste.
+void +tpm_menu(void) +{
..
- printf("The Trusted Platform Module (TPM) is a hardware device in "
"this machine.\n"
This is not universally true. I believe that the ME implements TPM in software on recent platforms.
In this patch there is also an msleep(2000) call. Please avoid adding unneccessary delays to the code.
Thanks!
//Peter