On 6/16/21 10:10 PM, Ard Biesheuvel wrote:
> On Wed, 16 Jun 2021 at 07:30, Omkar Kulkarni <omkar.kulka...@arm.com> wrote:
>>
>>
>> On 6/10/21 6:44 AM, Ming Huang via groups.io wrote:
>>> On 6/9/21 3:10 PM, Ard Biesheuvel wrote:
>>>> On Tue, 8 Jun 2021 at 16:21, Ming Huang <huangm...@linux.alibaba.com>
>>> wrote:
>>>>>
>>>>> TF-A: TrustedFirmware-a
>>>>> SPM: Secure Partition Manager(MM)
>>>>>
>>>>> For AArch64, when SPM enable in TF-A, TF-A may communicate to MM
>>> with
>>>>> buffer address (PLAT_SPM_BUF_BASE). The address is different from
>>>>> PcdMmBufferBase which use in edk2.
>>>>
>>>> Then why do we have PcdMmBufferBase?
>>>
>>> ArmPkg use this Pcd for the base address of non-secure communication
>>> buffer.
>>>
>>>>
>>>> Is it possible to set PcdMmBufferBase to the correct value?
>>>
>>> The secure communication may interrupt the non-secure communication. if
>>> we use the same address (PcdMmBufferBase and PLAT_SPM_BUF_BASE), the
>>> date in communication buffer may be corrupted.
>>>
>>> Best Regards,
>>> Ming
>>
>> In case where an interrupt handler executing from EL3 makes a call into
>> StandaloneMM, the handler in EL3 makes an spm call into StandaloneMM using
>> PLAT_SPM_BUF_BASE buffer base address. This PLAT_SPM_BUF_BASE is a shared
>> buffer between EL3 and S-EL0. This is where the following check fails and
>> leads to spm call failure. So this change would help resolve this issue.
>>
>
> But is it the right fix? Why would EDK2 even be aware of how EL3 and
> S-EL0 communicate with each other, and where the buffer is located?
The root cause for this problem is that the StandaloneMmPkg and TF-A
are not full cooperative.
PLAT_SPM_BUF_BASE is not dynamic buffer just like PcdMmBufferBase.
Please refer PcdMmBufferBase comments in
edk2-platforms/Platform/ARM/SgiPkg/SgiPlatform.dsc.inc:
#
# Set the base address and size of the buffer used
# for communication between the Normal world edk2
# with StandaloneMm image at S-EL0 through MM_COMMUNICATE.
# This buffer gets allocated in ATF and since we do not have
# a mechanism currently to communicate the base address and
# size of this buffer from ATF, hard-code it here
#
## MM Communicate
gArmTokenSpaceGuid.PcdMmBufferBase|0xFF600000
gArmTokenSpaceGuid.PcdMmBufferSize|0x10000
Best Regards,
Ming
>
>
>>>
>>>>
>>>>> Checking address will let TF-A communicate failed to MM. So remove
>>>>> below checking code:
>>>>> if (NsCommBufferAddr < mNsCommBuffer.PhysicalStart) {
>>>>> return EFI_ACCESS_DENIED;
>>>>> }
>>>>>
>>>>> Signed-off-by: Ming Huang <huangm...@linux.alibaba.com>
>>>>> ---
>>>>> StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c |
>>> 4
>>>>> ----
>>>>> 1 file changed, 4 deletions(-)
>>>>>
>>>>> diff --git
>>>>> a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c
>>>>> b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c
>>>>> index 63fbe26642..fe98d3181d 100644
>>>>> ---
>>> a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c
>>>>> +++
>>> b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c
>>>>> @@ -103,10 +103,6 @@ PiMmStandaloneArmTfCpuDriverEntry (
>>>>> return EFI_INVALID_PARAMETER;
>>>>> }
>>>>>
>>>>> - if (NsCommBufferAddr < mNsCommBuffer.PhysicalStart) {
>>>>> - return EFI_ACCESS_DENIED;
>>>>> - }
>>>>> -
>>>>> if ((NsCommBufferAddr + sizeof (EFI_MM_COMMUNICATE_HEADER)) >=
>>>>> (mNsCommBuffer.PhysicalStart + mNsCommBuffer.PhysicalSize)) {
>>>>> return EFI_INVALID_PARAMETER;
>>>>> --
>>>>> 2.17.1
>>>>>
>>>
>>>
>>>
>>>
>>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#76734): https://edk2.groups.io/g/devel/message/76734
Mute This Topic: https://groups.io/mt/83396678/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
