Adding support for TPM over FF-A on ARM platforms by kuqin12 · Pull Request #10874 · tianocore/edk2

kuqin12 · 2025-03-19T00:28:17Z

Description

This change adds the support of TPM over FF-A. It covers the TPM device library to allow TCG2 modules to communicate with the TPM service in the secure world through FF-A commands. In addition, it also supports the publication of TPM2 and the corresponding SSDT table that is responsible for supporting the physical presence interface through ASL methods during OS runtime.

Breaking change?
- Breaking change - Does this PR cause a break in build or boot behavior?
- Examples: Does it add a new library class or move a module to a different repo.
Impacts security?
- Security - Does this PR have a direct security impact?
- Examples: Crypto algorithm change or buffer overflow fix.
Includes tests?
- Tests - Does this PR include any explicit test code?
- Examples: Unit tests or integration tests.

How This Was Tested

This change is tested on QEMU SBSA virtual platform and proprietary hardware platforms and booted on Windows platform.

Integration Instructions

Add the following modules to the platform DSC file:
SecurityPkg/Tcg/Tcg2StandaloneMmArm/Tcg2StandaloneMmArm.inf
SecurityPkg/Tcg/Tcg2AcpiFfa/Tcg2AcpiFfa.inf

Link the following TPM library for TCG2 modules:
Tpm2DeviceLib|ArmPkg/Library/Tpm2DeviceLibFfa/Tpm2DeviceLibFfa.inf

LeviYeoReum

Why do you make it with Tpm2DeviceLib?
Would It better to make it Tpm2InstanceLib so that Tpm2DeviceLibRouter can choise this FF-A CRB library?

kuqin12 · 2025-03-19T23:03:46Z

Why do you make it with Tpm2DeviceLib? Would It better to make it Tpm2InstanceLib so that Tpm2DeviceLibRouter can choise this FF-A CRB library?

I started with device lib because it is able to be tested by directly linking to the driver without jumping through hoops.

But on the other hand, the router library is just picking among instance libraries using PcdTpmInstanceGuid, which only has TPM_NONE, TPM_1.2 and TPM_2. I do not see the benefits of add an extra layer of abstraction in this case, assuming all ARM platforms from this point and on are TPM2 ready.

Can you please let me know your usage?

LeviYeoReum · 2025-03-20T10:34:04Z

Why do you make it with Tpm2DeviceLib? Would It better to make it Tpm2InstanceLib so that Tpm2DeviceLibRouter can choise this FF-A CRB library?

I started with device lib because it is able to be tested by directly linking to the driver without jumping through hoops.

But on the other hand, the router library is just picking among instance libraries using PcdTpmInstanceGuid, which only has TPM_NONE, TPM_1.2 and TPM_2. I do not see the benefits of add an extra layer of abstraction in this case, assuming all ARM platforms from this point and on are TPM2 ready.

Can you please let me know your usage?

PcdTpmInstanceGuid, which only has TPM_NONE, TPM_1.2 and TPM_2.

But we can makes it with Tpm Service UUID right?
I'm just asking. But I've implemented with this with Tpm2InstanceLib.
Actually there is no reason to hinder your patch for this Library.

kuqin12 · 2025-03-20T16:26:04Z

Why do you make it with Tpm2DeviceLib? Would It better to make it Tpm2InstanceLib so that Tpm2DeviceLibRouter can choise this FF-A CRB library?

I started with device lib because it is able to be tested by directly linking to the driver without jumping through hoops.
But on the other hand, the router library is just picking among instance libraries using PcdTpmInstanceGuid, which only has TPM_NONE, TPM_1.2 and TPM_2. I do not see the benefits of add an extra layer of abstraction in this case, assuming all ARM platforms from this point and on are TPM2 ready.
Can you please let me know your usage?

PcdTpmInstanceGuid, which only has TPM_NONE, TPM_1.2 and TPM_2.

But we can makes it with Tpm Service UUID right? I'm just asking. But I've implemented with this with Tpm2InstanceLib. Actually there is no reason to hinder your patch for this Library.

I can add one library as instance lib if so desired. It should be minor update.

kuqin12 · 2025-03-20T21:40:52Z

Why do you make it with Tpm2DeviceLib? Would It better to make it Tpm2InstanceLib so that Tpm2DeviceLibRouter can choise this FF-A CRB library?

I started with device lib because it is able to be tested by directly linking to the driver without jumping through hoops.
But on the other hand, the router library is just picking among instance libraries using PcdTpmInstanceGuid, which only has TPM_NONE, TPM_1.2 and TPM_2. I do not see the benefits of add an extra layer of abstraction in this case, assuming all ARM platforms from this point and on are TPM2 ready.
Can you please let me know your usage?

PcdTpmInstanceGuid, which only has TPM_NONE, TPM_1.2 and TPM_2.

But we can makes it with Tpm Service UUID right? I'm just asking. But I've implemented with this with Tpm2InstanceLib. Actually there is no reason to hinder your patch for this Library.

I can add one library as instance lib if so desired. It should be minor update.

Actually, I went ahead and added it in a new commit. Please let me know if that looks reasonable to you.

jyao1 · 2025-04-07T08:20:10Z

Hello
Is there any special reason to put Tpm2DeviceLibFfa to ArmPkg, while Tcg2AcpiFfa and Tcg2StandaloneMmArm to SecurityPkg?

Shouldn't they go together? Either ArmPkg or SecurityPkg?

leiflindholm · 2025-04-07T09:22:12Z

Hello Is there any special reason to put Tpm2DeviceLibFfa to ArmPkg, while Tcg2AcpiFfa and Tcg2StandaloneMmArm to SecurityPkg?

Shouldn't they go together? Either ArmPkg or SecurityPkg?

Agreed. And ArmPkg is not it.

kuqin12 · 2025-04-07T16:47:08Z

Hello Is there any special reason to put Tpm2DeviceLibFfa to ArmPkg, while Tcg2AcpiFfa and Tcg2StandaloneMmArm to SecurityPkg?
Shouldn't they go together? Either ArmPkg or SecurityPkg?

Agreed. And ArmPkg is not it.

@leiflindholm @jyao1 I do not disagree, either. But the Tpm2DeviceLib will depend on FFA lib directly, which seems inverted dependency. Any suggestions to avoid such entanglement?

leiflindholm · 2025-04-07T17:44:11Z

@leiflindholm @jyao1 I do not disagree, either. But the Tpm2DeviceLib will depend on FFA lib directly, which seems inverted dependency. Any suggestions to avoid such entanglement?

ArmFfaLib is an implementation of an industry standard specification, so belongs in MdePkg.

jyao1 · 2025-04-15T01:59:09Z

@kuqin12 , thanks for the update. I am OK with this patch.

Since you are expert on ARM-FFA part, would you mind to add yourself to be the reviewer and help me maintain the ARM-FFA related module?

If you agree, please add a section to https://github.com/tianocore/edk2/blob/master/Maintainers.txt, with following

SecurityPkg: ARM-FFA related modules
F: SecurityPkg/*Arm
F: SecurityPkg/*Ffa
R: name <email> [github-id]

kuqin12 · 2025-04-15T16:03:45Z

@kuqin12 , thanks for the update. I am OK with this patch.

Since you are expert on ARM-FFA part, would you mind to add yourself to be the reviewer and help me maintain the ARM-FFA related module?

If you agree, please add a section to https://github.com/tianocore/edk2/blob/master/Maintainers.txt, with following
SecurityPkg: ARM-FFA related modules
F: SecurityPkg/*Arm
F: SecurityPkg/*Ffa
R: name <email> [github-id]

Thanks, @jyao1. I can't claim to be an expert but I'd be willing to help review the incoming changes to these functional components :) I will add change in the next push.

This change adds a GUID for the physical presence interface. This is defined in TCG Physical Presence Interface v1.30, Rev. 00.52: Section 8.1 ACPI Functions. Signed-off-by: Kun Qin <kun.qin@microsoft.com>

This change adds a new driver Tcg2StandaloneMmArm. It will register an MMI handler that is responsible for supporting the physical presence interface from ASL methods during OS runtime. Platforms need to expose the PPI ACPI function GUID in the Standalone MM secure partition. Signed-off-by: Kun Qin <kun.qin@microsoft.com>

…tion This change adds a new library instance of SmmTcg2PhysicalPresenceLib. It will directly check on the PCD value instead of relying on the HOB value, which will require change on the TFA/SPMC side. Signed-off-by: Kun Qin <kun.qin@microsoft.com>

This change adds a new driver Tcg2AcpiFfa. It will publish the TPM2 and the corresponding SSDT table that is responsible for supporting the physical presence interface through ASL methods during OS runtime. Co-authored-by: Raymond Diaz <raymonddiaz@microsoft.com> Signed-off-by: Kun Qin <kun.qin@microsoft.com>

TPM over FF-A is a mechanism enabling the normal world to communicate with TPM devices offered as a FF-A service in the secure world. This update introduces a header file containing definitions from the TPM over FF-A specification, as detailed in the following documentation: https://developer.arm.com/documentation/den0138/latest/ Signed-off-by: Kun Qin <kun.qin@microsoft.com>

This change introduces a `Tpm2DeviceLibFfa` library to support TPM over FF-A. The implementation follows the TPM over FF-A spec v1.0 BET: https://developer.arm.com/documentation/den0138/latest/ The change is tested on QEMU SBSA virtual platform and proprietary hardware platforms. Co-authored-by: Raymond Diaz <raymonddiaz@microsoft.com> Signed-off-by: Kun Qin <kun.qin@microsoft.com>

This change introduces a `Tpm2InstanceLibFfa` library to support TPM over FF-A and works with Tpm2DeviceLibRouter* libraries. The implementation follows the TPM over FF-A spec v1.0 BET: https://developer.arm.com/documentation/den0138/latest/ The change is tested on QEMU SBSA virtual platform and proprietary hardware platforms. Signed-off-by: Kun Qin <kun.qin@microsoft.com>

…ityPkg - Updated Maintainers.txt to include Kun Qin as a maintainer for the ARM- FFA sections in SecurityPkg. - Added his contact information: email and GitHub username. Signed-off-by: Kun Qin <kun.qin@microsoft.com>

github-actions Bot added the impact:security This change has a direct security impact such as changing a crypto algorithm. label Mar 19, 2025

kuqin12 force-pushed the tpm_ffa branch 3 times, most recently from 4e75df4 to f5f82ac Compare March 19, 2025 08:19

kuqin12 marked this pull request as ready for review March 19, 2025 08:42

tianocore-assign-reviewers Bot requested review from ardbiesheuvel, jyao1, leiflindholm, rahul1-kumar and samimujawar March 19, 2025 08:43

LeviYeoReum reviewed Mar 19, 2025

View reviewed changes

Comment thread SecurityPkg/Tcg/Tcg2AcpiFfa/Tpm2Ffa.asl

Comment thread SecurityPkg/Tcg/Tcg2AcpiFfa/Tpm2Ffa.asl Outdated

Comment thread SecurityPkg/Tcg/Tcg2AcpiFfa/Tcg2AcpiFfa.c

Comment thread SecurityPkg/Tcg/Tcg2AcpiFfa/Tcg2AcpiFfa.c Outdated

kuqin12 closed this Mar 19, 2025

kuqin12 reopened this Mar 19, 2025

kuqin12 force-pushed the tpm_ffa branch 3 times, most recently from 5092015 to 52b3a75 Compare March 19, 2025 23:54

ardbiesheuvel approved these changes Mar 20, 2025

View reviewed changes

kuqin12 force-pushed the tpm_ffa branch 2 times, most recently from 217bc22 to 2aa9b4b Compare March 20, 2025 21:34

kuqin12 force-pushed the tpm_ffa branch 2 times, most recently from 9a532ad to 029b215 Compare March 20, 2025 22:33

kuqin12 force-pushed the tpm_ffa branch 2 times, most recently from 966633d to 9e3d801 Compare April 10, 2025 20:52

kuqin12 marked this pull request as ready for review April 10, 2025 21:35

azizidomar-arm reviewed Apr 11, 2025

View reviewed changes

Comment thread SecurityPkg/Library/Tpm2DeviceLibFfa/Tpm2ServiceFfaRaw.c

Comment thread SecurityPkg/Library/Tpm2DeviceLibFfa/Tpm2Ptp.c

kuqin12 force-pushed the tpm_ffa branch from 9e3d801 to 0ff0e98 Compare April 11, 2025 19:20

jyao1 reviewed Apr 15, 2025

View reviewed changes

Comment thread SecurityPkg/Include/Guid/Tpm2ServiceFfa.h Outdated

jyao1 reviewed Apr 15, 2025

View reviewed changes

Comment thread SecurityPkg/Include/Guid/Tpm2ServiceFfa.h Outdated

kuqin12 force-pushed the tpm_ffa branch from 0ff0e98 to 2aff683 Compare April 15, 2025 23:10

tianocore-assign-reviewers Bot requested review from ajfish, makubacki and mdkinney April 15, 2025 23:10

kuqin12 force-pushed the tpm_ffa branch from 2aff683 to a9d5bd0 Compare April 15, 2025 23:16

samimujawar reviewed Apr 16, 2025

View reviewed changes

Comment thread SecurityPkg/Tcg/Tcg2StandaloneMmArm/Tcg2StandaloneMmArm.c

Comment thread SecurityPkg/Tcg/Tcg2AcpiFfa/Tcg2AcpiFfa.c

SecurityPkg: PhysicalPresenceData: Add a GUID used for ACPI functions

ddec1d8

This change adds a GUID for the physical presence interface. This is defined in TCG Physical Presence Interface v1.30, Rev. 00.52: Section 8.1 ACPI Functions. Signed-off-by: Kun Qin <kun.qin@microsoft.com>

kuqin12 force-pushed the tpm_ffa branch from a9d5bd0 to f76a941 Compare April 16, 2025 18:17

rahul1-kumar reviewed Apr 16, 2025

View reviewed changes

Comment thread SecurityPkg/Tcg/Tcg2StandaloneMmArm/Tcg2StandaloneMmArm.c

kuqin12 and others added 7 commits April 16, 2025 12:24

kuqin12 force-pushed the tpm_ffa branch from f76a941 to a8ff895 Compare April 16, 2025 19:24

jyao1 added the push Auto push patch series in PR if all checks pass label Apr 17, 2025

jyao1 approved these changes Apr 17, 2025

View reviewed changes

Merge branch 'master' into tpm_ffa

8ab5722

mergify Bot merged commit 5e5ca20 into tianocore:master Apr 18, 2025
126 checks passed

Conversation

kuqin12 commented Mar 19, 2025

Description

How This Was Tested

Integration Instructions

Uh oh!

LeviYeoReum left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kuqin12 commented Mar 19, 2025

Uh oh!

LeviYeoReum commented Mar 20, 2025

Uh oh!

kuqin12 commented Mar 20, 2025

Uh oh!

kuqin12 commented Mar 20, 2025

Uh oh!

jyao1 commented Apr 7, 2025

Uh oh!

leiflindholm commented Apr 7, 2025

Uh oh!

kuqin12 commented Apr 7, 2025

Uh oh!

leiflindholm commented Apr 7, 2025

Uh oh!

Uh oh!

Uh oh!

jyao1 commented Apr 15, 2025

Uh oh!

Uh oh!

Uh oh!

kuqin12 commented Apr 15, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants