From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out30-54.freemail.mail.aliyun.com (out30-54.freemail.mail.aliyun.com [115.124.30.54]) by mx.groups.io with SMTP id smtpd.web11.3348.1623987736880611677 for ; Thu, 17 Jun 2021 20:42:17 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: linux.alibaba.com, ip: 115.124.30.54, mailfrom: huangming@linux.alibaba.com) X-Alimail-AntiSpam: AC=PASS;BC=-1|-1;BR=01201311R601e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=alimailimapcm10staff010182156082;MF=huangming@linux.alibaba.com;NM=1;PH=DS;RN=9;SR=0;TI=SMTPD_---0Ucmotr7_1623987733; Received: from MingdeMacBook-Pro.local(mailfrom:huangming@linux.alibaba.com fp:SMTPD_---0Ucmotr7_1623987733) by smtp.aliyun-inc.com(127.0.0.1); Fri, 18 Jun 2021 11:42:14 +0800 Subject: Re: [edk2-devel] [Patch] StandaloneMmPkg: Fixed communicating from TF-A failed issue To: Ard Biesheuvel , Omkar Kulkarni Cc: "devel@edk2.groups.io" , Ard Biesheuvel , Sami Mujawar , Jiewen Yao , Supreeth Venkatesh , "guoheyi@linux.alibaba.com" , nd References: <20210608142112.87183-1-huangming@linux.alibaba.com> <7536fafd-fa8e-9940-beec-a1cd357ecb03@linux.alibaba.com> From: "Ming Huang" Message-ID: <4c209aa5-b3f3-d643-2791-6a57ca16200b@linux.alibaba.com> Date: Fri, 18 Jun 2021 11:42:13 +0800 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 6/16/21 10:10 PM, Ard Biesheuvel wrote: > On Wed, 16 Jun 2021 at 07:30, Omkar Kulkarni 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 >>> 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 >>>>> --- >>>>> 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 >>>>> >>> >>> >>> >>> >>