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

StandaloneMm: Remove hob creation in Arm and Support FF-A v1.2 #6116

Open
wants to merge 27 commits into
base: master
Choose a base branch
from

Conversation

LeviYeoReum
Copy link
Contributor

@LeviYeoReum LeviYeoReum commented Aug 27, 2024

Description

This patch series remove hob creation in Arm/StandaloneMmCoreEntryPoint.
For this, This patch series includes:
1. Simplify StandaloneMm entrypoint code for Arm
2. Replace CpuEntryPoint Hob with the protocol
3. Remove arm specific HobLib implementation used in StandaloneMmCore.
4. Support FF-A v1.2, add ArmFfaLib
5. Apply FF-A boot protocol for StandaloneMm
6. To apply booting with FF-A, use embedded stack for StandaloneMm because TF-A doesn't set stack for StandaloneMm
7. Apply FF-A Library in Mmcommunication Driver in Arm
8. StandaloneMm in Arm is UP migratable. Remove per-cpu feature
9. Support ExtractSectionLib for StandaloneMm.
10. Move StandaloneMmCoreEntryPoint & StandaloneMmCpu driver for Arm to ArmPkg.

How This Was Tested

Tested in Base FVP RevC platform by checking Boot and dumping uefi variables.

Integration Instructions

.

@ardbiesheuvel
Copy link
Member

cc @apalos

Please asses the impact of these changes on the code in Platform/StandaloneMm/PlatformStandaloneMmPkg/PlatformStandaloneMmRpmb.dsc in the edk2-platforms repo.

Also, please merge all patches that rename the MM #defines into a single one, so we don't break the build intermittently.

@LeviYeoReum
Copy link
Contributor Author

Hi @ardbiesheuvel.

Please asses the impact of these changes on the code in Platform/StandaloneMm/PlatformStandaloneMmPkg/PlatformStandaloneMmRpmb.dsc in the edk2-platforms repo.

Yes. It would break until I post the patchset related to FF-A boot protocol usage and it requires for optee to create phit hob using FF-A boot protocol.

I have a plan to FF-A boot protocol patch for edk2 in couple of week, and later I'll post the optee patch for this.

Also, please merge all patches that rename the MM #defines into a single one, so we don't break the build intermittently.

Thanks. I'll merge it

@LeviYeoReum LeviYeoReum force-pushed the levi/2999_stmm_milestone_1 branch from ad497a7 to dece401 Compare August 29, 2024 08:49
@LeviYeoReum LeviYeoReum requested review from apalos and niruiyu August 29, 2024 08:53
@apalos
Copy link
Contributor

apalos commented Sep 4, 2024

Hi @ardbiesheuvel.

Please asses the impact of these changes on the code in Platform/StandaloneMm/PlatformStandaloneMmPkg/PlatformStandaloneMmRpmb.dsc in the edk2-platforms repo.

Yes. It would break until I post the patchset related to FF-A boot protocol usage and it requires for optee to create phit hob using FF-A boot protocol.

I have a plan to FF-A boot protocol patch for edk2 in couple of week, and later I'll post the optee patch for this.

I'd still like to understand a bit more how this affects the whole situation. Wouldn't that make certain versions of stmm incompatible with OP-TEE? Do we have a versioning field in there we could use and at least report an error message that makes sense?

Also, please merge all patches that rename the MM #defines into a single one, so we don't break the build intermittently.

Thanks. I'll merge it

@LeviYeoReum
Copy link
Contributor Author

Hi @apalos.

I'd still like to understand a bit more how this affects the whole situation. Wouldn't that make certain versions of stmm incompatible with OP-TEE? Do we have a versioning field in there we could use and at least report an error message that makes sense?

Here is what I think:

optee standaloneMm Working Comments
old old O Legacy
new old X load_stmm() in optee, doesn't set stack pointer. So as soon as enter standaloneMm, It will crash TA.
old new X StandaloneMm check FF-A version, If it's under v1.2, StandaloneMm prints error message and fail to load TA.
new new O .

Because, FF-A v1.2's. manifest doesn't have stack memory region for partition. So new standaloneMm set stack by itself.

For this. I think optee's stmm.c code should be modified with:
- At the load, create Hob list and passes it with FF-A boot protocol
- update stmm svc handler.

BTW, to confirm the optee modification, could you share me what is proper build command for TF-A / optee / u-boot for vexpress-fvp platform?

Thanks.

@apalos
Copy link
Contributor

apalos commented Sep 4, 2024

Hi @apalos.

I'd still like to understand a bit more how this affects the whole situation. Wouldn't that make certain versions of stmm incompatible with OP-TEE? Do we have a versioning field in there we could use and at least report an error message that makes sense?

Here is what I think:
optee standaloneMm Working Comments
old old O Legacy
new old X load_stmm() in optee, doesn't set stack pointer. So as soon as enter standaloneMm, It will crash TA.
old new X StandaloneMm check FF-A version, If it's under v1.2, StandaloneMm prints error message and fail to load TA.
new new O .

Because, FF-A v1.2's. manifest doesn't have stack memory region for partition. So new standaloneMm set stack by itself.

Yes, that's what I assumed when I read the patches, thanks for the confirmation.

I don't personally mind if we remove the 1.0 variant, but this needs to be done properly and in coordination with OP-TEE. Parsing the FF-A version and not launching StMM if an older version is loaded is the bare minimum we should fix.

In any case, I'll ask around and see if anyone is affected by removing 1.0.

For this. I think optee's stmm.c code should be modified with: - At the load, create Hob list and passes it with FF-A boot protocol - update stmm svc handler.

BTW, to confirm the optee modification, could you share me what is proper build command for TF-A / optee / u-boot for vexpress-fvp platform?

I don't have anything for the FVP.
I do keep a hacky branch that emulates an RPMB partition (in memory) for U-Boot though and you can test Get/Set variable from the bootloader. Just clone https://git.linaro.org/people/ilias.apalodimas/efi_optee_variables.git/, change your cross compiler in the build.sh script and run it. Once the build is done, the script will print a qemu cmd line for you to run. Once you reach the U-Boot consol do printenv -e .

Thanks.

@LeviYeoReum LeviYeoReum force-pushed the levi/2999_stmm_milestone_1 branch from dece401 to 79f6e6f Compare September 24, 2024 12:38
@LeviYeoReum LeviYeoReum requested a review from niruiyu September 24, 2024 12:39
@LeviYeoReum LeviYeoReum force-pushed the levi/2999_stmm_milestone_1 branch 2 times, most recently from e2de4dd to 4aaa79e Compare September 24, 2024 13:37
ArmPkg/Library/ArmFfaLib/ArmFfaCommon.c Outdated Show resolved Hide resolved
ArmPkg/Library/ArmFfaLib/ArmFfaCommon.c Outdated Show resolved Hide resolved
ArmPkg/Library/ArmFfaLib/ArmFfaDxeLib.c Show resolved Hide resolved
ArmPkg/Library/ArmFfaLib/ArmFfaCommon.c Show resolved Hide resolved
ArmPkg/Library/ArmFfaLib/ArmFfaCommon.h Show resolved Hide resolved
@LeviYeoReum LeviYeoReum requested a review from kuqin12 September 24, 2024 20:04
@LeviYeoReum LeviYeoReum force-pushed the levi/2999_stmm_milestone_1 branch 6 times, most recently from 4ccfe5b to cc51b9a Compare September 26, 2024 09:01
This patch introduces a PI_MM_CPU_DRIVER_EP protocol to handle
Mmcommunication request based on the CPU driver.
Previously the CPU driver entry point was retrieved using the
gEfiArmTfCpuDriverEntryPoint HOB.
However, this practice is incorrect as StandaloneMM must be a HOB
consumer and not a HOB producer.

Therefore, remove the CPU entry HOB gEfiArmTfCpuDriverEntryPoint,
and replace it with the CPU driver entry protocol
EDKII_PI_MM_CPU_DRIVER_EP_PROTOCOL.
The EDKII_PI_MM_CPU_DRIVER_EP_PROTOCOL installed in
StandaloneMmCpuInitialize() will be used by the code in
Arm/StandaloneMmCoreEntryPoint.

This protocol is used like below:
 +=====+
 |StandaloneMmCore|
 +=====+
    |
    CEntryPoint()
    ===================
            |
            ProcessModuleEntryPointList()
                    |
                    +--> StandaloneMmMain()
                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                         |   // Load StandaloneMmCpu driver which implements
                         |   // CpuDriverEntryPoint used by DelegatedEventLoop().
                         |   // and install the gEdkiiPiMmCpuDriverEpProtocolGuid.
            --------------
            |
            ... // Get CpuDriverEntryPoint implemented by
                // StandaloneMmCpu driver with gEdkiiPiMmCpuDriverEpProtocolGuid
            |
            DelegatedEventLoop() // Handle request by delegating CpuDriverEntryPoint.

Signed-off-by: Levi Yun <[email protected]>
ArmTransferList is used to pass boot information according to
firmware handoff protocol specification [0].

When initializing StandaloneMm, it gets boot information from
the PHIT HOB in the TransferList.

[0] https://github.com/FirmwareHandoff/firmware_handoff

Signed-off-by: Levi Yun <[email protected]>
By using transfer list passed by TF-A,
StandaloneMmCore is no more producer of HOBs, But it is consumer.
So, the Arm-specific implementation of StandaloneMmCoreHobLib
is no longer needed.
This change removes the Arm-specific HOB creation code and
integrates the necessary adjustments.

Signed-off-by: Levi Yun <[email protected]>
Change naming convention in ArmMmSvc.h with

  MM to SPM_MM

This would make it clear to discern ABI protocol used to communicate
with secure partition.

Continuous-integration-options: PatchCheck.ignore-multi-package
Signed-off-by: Levi Yun <[email protected]>
Introduce a new helper function SpmMmStatusToEfiStatus() to convert
the SPM_MM status values to EFI_STATUS values.

Signed-off-by: Levi yun <[email protected]>
To remove hob creation in StandaloneMm entrypoint,
TF-A should pass PHIT hob information to StandaloneMm.
When it passes PHIT hob, it passes according to
firmware handoff specification[0].

This patch applies boot protocol using transfer list with firmware
handoff specification and remove hob creation in StandaloneMm
entrypoint.

Link: https://github.com/FirmwareHandoff/firmware_handoff [0]

Signed-off-by: Levi Yun <[email protected]>
Update ArmFfaSvc.h defines & macros with FF-A v1.2 [0] and
FF-A Memory Mangement Protocol [1].

This also changes naming convention used in it.

Link: https://developer.arm.com/documentation/den0077/latest/ [0]
Link: https://developer.arm.com/documentation/den0140/latest/ [1]

Signed-off-by: Levi Yun <[email protected]>
ArmFfaBootInfo.h contains boot information used in FF-A [0].
This boot information will be used to
initalize firmware (i.e) StandaloneMm.

Links: https://developer.arm.com/documentation/den0077/latest/ [0]

Signed-off-by: Levi Yun <[email protected]>
Partition descriptor is used to get partition information via
FFA_PARTITION_INFO_GET or FFA_PARTITION_INFO_GET_REGS FF-A ABI.

Adds defines for partition descriptor and some macros used to call above
ABIs.

Signed-off-by: Levi Yun <[email protected]>
To communicate with spmc or spmd, UEFI needs to map the Rx/Tx buffer
(which is a global resource in a partition)
by getting the required information from the partition descriptor.

for this, Define ArmFfaLib related Pcd and Guid.

Pcd:
  - PcdFfaLibConduitSmc
      conudit to use ArmFfaLib.

  - PcdFfaTxBufeer
      address of Tx buffer.

  - PcdFfaRxBuffer:
      address of Rx buffer.

  - PcdTxRxPageCount:
      specify buffer size with EFI_PAGE_SIZE unit.

  - PcdFfaExitBootEventRegistered:
      check exit boot event registered to unmap rx/tx buffer.

Guid:
   - gArmFfaRxTxBufferInfoGuid:
       This is used in Hob to get Rx/Tx buffer information to pass
       Rx/Tx buffer information via HobList if Rx/Tx Buffer mapped in
       PEI phase.

Signed-off-by: Levi Yun <[email protected]>
PcdFfaEnabled is no more used because ArmFfaLib could find whether FF-A
is supported dynamically.

This patch removes usage of PcdFfaEnabled.

Continuous-integration-options: PatchCheck.ignore-multi-package
Signed-off-by: Levi Yun <[email protected]>
Add ArmFfaLib.h which defines interfaces correspond to FF-A ABIs.

Signed-off-by: Levi Yun <[email protected]>
Add ArmFfaLib used in Dxe driver

Signed-off-by: Levi Yun <[email protected]>
Add ArmFfaLib used in PEIM.

Signed-off-by: Levi Yun <[email protected]>
Add ArmFfaLib used in StandaloneMmCore/StandaloneMm Driver.

Continuous-integration-options: PatchCheck.ignore-multi-package
Signed-off-by: Levi Yun <[email protected]>
…ulirty

The StandaloneMm implementation for Arm sets up the stack in
the early startup code using the data section reserved in the
assembly code.

When TF-A loads the StandaloneMM binary in the DRAM it maps
the entire StandaloneMM memory region as Read Only.

Therefore, the initial startup assembly code updates the mem
permissions of the stack region to Read Write.

However, when the StandaloneMmCore is loaded the function
UpdateMmFoundationPeCoffPermissions() starts applying the
memory permissions based on the PE COFF sections as below:

A. If the section is not executable, it first removes the
executable permission of the section by calling TextUpdate().
TextUpdate() is the StandaloneMM MMU library function
ArmSetMemoryRegionNoExec().

B. It then checks if the section is writable, and if it is
it calls ReadWriteUpdater(), which invokes the StandaloneMM
MMU library function ArmClearMemoryRegionReadOnly() to make
the section writable.

However, this results in the stack being made read-only
between A and B. To understand this please see the following
flow.

1. TF-A sets the entire StandaloneMM region as Read Only.
2. The stack is reserved in the data section by the early
   assembly entry point code.
    +--------------------+   <--- Start of Data Section
    |                    |
    |  Data Section      |
    |                    |
    | +----------------+ |   <--- Stack region
    | |   Stack        | |
    | +----------------+ |
    |                    |
    +--------------------+

3. The StanaloneMM early entry point code updates the
   attributes of the stack to Read Write.
4. When UpdateMmFoundationPeCoffPermissions() sets the
   permission of the data section to remove the Execute
   attribute, it calls ArmSetMemoryRegionNoExec().
5. The ArmSetMemoryRegionNoExec() implementation gets the
   attributes of the first granule which is at the start
   of the data section, then clears the execute permission
   and applies the attribute for the entire data section.
6. Since TF-A has mapped the entire section as read only
   the first granule of the data section is read only and
   therefore the stack region attributes are changed to
   Read Only no execute.
7. Since the stack is read only after point A any updates
   to the stack result in an exception.

To resolve this issue with update the library with FF-A v1.2,
get/set memory permission per page unit.

Links: https://developer.arm.com/documentation/den0140/latest/ [0]
Signed-off-by: Levi Yun <[email protected]>
To support the service other than Mmcommunication service,
StandaloneMm should use FF-A v1.2 or later [0].

For this, StandaloneMm needs to change:
  1. apply FF-A boot protocol
      - FF-A uses its own boot protocol and it can deliver phit hob.
        So, StandaloneMm should understand FF-A boot protocol and
        get phit hob from it to initialize.

  2. Use DIRECT_REQ2 / DIRECT_RESP2
      - To support the other service via FF-A protocol
        than MmCommunication,
        StandaloneMm should use DIRECT_REQ2 / DIRECT_RESP2.
        In case of service provided with MmCommunication protocol,
        register x2/x3 value is set as gEfiMmCommunication2ProtocolGuid and
        register x4 value is set with MmCommunication Buffer
        (ServiceTypeMmCommunication).
        In case of service with each speicifiation via FF-A,
        register x2/x3 value is set as each service guid
        and StandaloneMmCoreEntryPoint genreates Mm communication header
        with passed arguments to dispatch this service
        provided by StandaloneMm.
        i.e) Tpm service, Firmware update service and etc.
        (ServiceTypeMisc)

Link: https://developer.arm.com/documentation/den0077/latest/ [0]

Signed-off-by: Levi Yun <[email protected]>
Support Mmcommunication protocol via FF-A with StandaloneMm.

Signed-off-by: Levi Yun <[email protected]>
Support Mmcommunication protocol via FF-A with StandaloneMm.
For this, FF-A v1.2 is required.

Signed-off-by: Levi Yun <[email protected]>
On Arm all requests are handled as Asynchronous events by the Root
MMI handler.
Since the communication data is conveyed using either the NS shared
buffer or the Secure shared buffer, the Arm implementation does not
setup the mCommunicationBuffer. Therefore, the mCommunicationBuffer
being NULL is not an error case.

Moreover, the existing code switches to Asynchronous event processing
when the mCommunicationBuffer is NULL, which means that the log is an
info log rather than an error.

Therefore, change the log level from ERROR to INFO when the
mCommunicationBuffer is NULL.

Signed-off-by: Levi Yun <[email protected]>
StandaloneMm Arm uses stack allocated in data section.
This patch adds Pcd which specify the stack size of StandaloneMm.

Signed-off-by: levi.yun <[email protected]>
There are 2 communication interfaces between the SPMC and StandaloneMM
1. SpmMM
2. FF-A

When SpmMM is enabled, TF-A acts as the SPMC at EL3 and the stack is setup
by TF-A for use by StandaloneMm. However, when FF-A is enabled, the SPMC
does not setup the stack for StandaloneMm and it is expected that the
StandaloneMm code will setup its own stack.

Therefore, reserve an area in the data region for use as the stack for
StandaloneMM. This stack will be used in both the scenarios described
above, i.e. when either SpmMM or FF-A is enabled.

Although the stack is reserved from the data section which is expected
to be Read-Write enabled, when TF-A maps the StandaloneMM binary into
the DRAM it configures the entire StandaloneMM memory as Read-Only.

Therefore, before the stack can be utilised, the PE Coff sections
need to be scanned to change the the stack region from Read-Only to
Read-Write.

Signed-off-by: Levi Yun <[email protected]>
Arm has three types of communication buffer
    - Non secure communication buffer: shared buffer with NS partition
    - Secure communication buffer: shared buffer with Secure partition
    - Internal Misc service buffer: For misc service
        i.e. for services that do not use MmCommunication protocol,
        a buffer is created and populated internally.

Since, internal Misc service buffer is allocated dynamically in
StandaloneMm there is no additional check required for the Misc
service buffer.

This patch move sanity check of communication buffer from
StandaloneMmCpu Driver to StandaloneMmEntryPoint.

Signed-off-by: Levi Yun <[email protected]>
StandaloneMm in Arm is UP-migratable which means StandaloneMm
cannot run concurrently.

Therefore, remove per-cpu feature in StandaloneMm.

Signed-off-by: Levi Yun <[email protected]>
The default ExtractGuidedSectionLib used by Standalone MM is the
implementation in EmbeddedPkg. However, the ExtractGuidedSectionLib
implementation in EmbeddedPkg builds HOBs to save the extract handler
information.
Since StandaloneMm is consumer of HOB, not a HOB producer, introduce
a StandaloneMmExtraGuidedSectionLib implementation that saves the
extract handler information in the ConfigurationTable.
This StandaloneMmExtraGuidedSectionLib can be used by MM_STANDALONE
and MM_CORE_STANDALONE modules.

Signed-off-by: Levi Yun <[email protected]>
StandaloneMmCpu driver is only used for Arm architecture and
StandaloneMmCoreEntryPointLib for Arm has specific implementation with
StandaloneMmCpu driver.

Move StandaloneMmCpu Driver and StandaloneMmCoreEntryPointLib for Arm
to ArmPkg.

Continuous-integration-options: PatchCheck.ignore-multi-package
Signed-off-by: Levi Yun <[email protected]>
@LeviYeoReum LeviYeoReum force-pushed the levi/2999_stmm_milestone_1 branch from b0e1cf6 to 191a023 Compare December 24, 2024 20:12
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.

8 participants