Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OvmfPkg: DxeTcg2PhysicalPresenceLib: fix changing of PCR banks #164

Merged
merged 3 commits into from
Sep 16, 2024

Conversation

arturkow2
Copy link
Contributor

Tcg2PhysicalPresenceLibSubmitRequestToPreOSFunction writes to TPM2 physical presence PPI provided by coreboot (a memory region preserved across reboots). CPU caches must be explicitly flushed prior to platform reboot or request written to PPI will be lost.

Tcg2PhysicalPresenceLibSubmitRequestToPreOSFunction writes to TPM2
physical presence PPI provided by coreboot (a memory region preserved
across reboots). CPU caches must be explicitly flushed prior to platform
reboot or request written to PPI will be lost.

Signed-off-by: Artur Kowalski <artur.kowalski@3mdeb.com>
@miczyg1
Copy link
Contributor

miczyg1 commented Sep 4, 2024

@arturkow2 please do not open PRs based outdated history of target branch... Be sure to pull most recent code before yo ubranch out for development. I already rebased this branch on dasharo.

@miczyg1
Copy link
Contributor

miczyg1 commented Sep 4, 2024

I have tested the changes on VP6650 with Infineon dTPM and it didn't work... I was not prompted to confirm the SHA bank change before entering boot menu or setup.

EDIT: Wait, this platform is actually the worst possible piece to test this on. This platform can't reset properly, as it always goes through power cycle...

@miczyg1
Copy link
Contributor

miczyg1 commented Sep 4, 2024

Ok it works, but once I apply new PCR banks settings and reset the platform I am stuck on the prompt to change PCR banks forever:

A configuration change was requested to change the boot measurements to use PCR 
bank(s) of this computer's TPM (Trusted Platform Module)

WARNING: Changing the PCR bank(s) of the boot measurements may prevent the Opera
ting System from properly processing the measurements. Please check if your Oper
ating System supports the new PCR bank(s).

WARNING: Secrets in the TPM that are bound to the boot state of your machine may
 become unusable.

Current PCRBanks is 0x2. (SHA256)
New PCRBanks is 0x2. (SHA256)
 
Press F12 to change the boot measurements to use PCR bank(s) of the TPM 
Press ESC to reject this change request and continue

I have to press ESC to exit the loop. It looks like the request is not cleared after it is serviced. We have to clear it so that it does not persist in RAM and is handled on each boot... This behavior was observed on VP4670

@arturkow2 arturkow2 marked this pull request as draft September 11, 2024 10:40
Flush cache not only when placing request in PPI, but also after
clearing old request from PPI.

Signed-off-by: Artur Kowalski <artur.kowalski@3mdeb.com>
@arturkow2 arturkow2 marked this pull request as ready for review September 11, 2024 11:03
Signed-off-by: Michał Żygowski <michal.zygowski@3mdeb.com>
@miczyg1
Copy link
Contributor

miczyg1 commented Sep 16, 2024

Ok, now it works when the request is cleared and flushed

@miczyg1 miczyg1 merged commit 250710a into dasharo Sep 16, 2024
3 checks passed
@miczyg1 miczyg1 deleted the pcr-bank-change-fix branch September 16, 2024 11:03
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.

2 participants