Skip to content

xen/arch/x86/tpm.c: add CRB interface support + response validation fix#28

Open
accek-itl wants to merge 2 commits intoTrenchBoot:aem-staging-2026-03-13from
accek-itl:tpm-crb
Open

xen/arch/x86/tpm.c: add CRB interface support + response validation fix#28
accek-itl wants to merge 2 commits intoTrenchBoot:aem-staging-2026-03-13from
accek-itl:tpm-crb

Conversation

@accek-itl
Copy link
Copy Markdown

No description provided.

@macpijan macpijan requested a review from SergiiDmytruk April 23, 2026 13:27
Comment thread xen/arch/x86/tpm.c
* header (10) | parameterSize (4) | TPML_DIGEST_VALUES | authArea
* finish_r overlays: h (10) | paramSize (4) | hashCount (4) | hashes[]
*/
if ( o_size < sizeof(cmd_rsp.finish_r) + sizeof(uint32_t) )
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why sizeof(uint32_t)? That's first hash alg and 2 first bytes of the hash.

Comment thread xen/arch/x86/tpm.c
uint32_t data_buf_pa = TPM_BASE + CRB_DATA_BUFFER_(loc);
unsigned expected;

crb_cmd_ready(loc);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why, but coreboot's driver follows this with a wait for no error in CRB_CTRL_STS_ and then a wait for CRB_CTRL_START_ being zero.

Comment thread xen/arch/x86/tpm.c
tpm_write32(CRB_CTRL_RSP_ADDR_(loc), data_buf_pa);
tpm_write32(CRB_CTRL_RSP_ADDR_(loc) + 4, 0);

memcpy(__va(data_buf_pa), buf, i_size);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No check for i_size being too large.

Comment thread xen/arch/x86/tpm.c
}

/* Read header to learn the response length. */
memcpy(buf, __va(data_buf_pa), sizeof(struct tpm_rsp_hdr));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*o_size could be too small here.

Comment thread xen/arch/x86/tpm.c
static void crb_send_cmd(unsigned loc, uint8_t *buf, unsigned i_size,
unsigned *o_size)
{
uint32_t data_buf_pa = TPM_BASE + CRB_DATA_BUFFER_(loc);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should make no difference but upstream will likely prefer this version:

Suggested change
uint32_t data_buf_pa = TPM_BASE + CRB_DATA_BUFFER_(loc);
paddr_t data_buf_pa = TPM_BASE + CRB_DATA_BUFFER_(loc);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants