From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mout02.posteo.de (mout02.posteo.de [185.67.36.66]) by mx.groups.io with SMTP id smtpd.web08.45387.1640353969890661103 for ; Fri, 24 Dec 2021 05:52:50 -0800 Authentication-Results: mx.groups.io; dkim=fail reason="body hash did not verify" header.i=@posteo.de header.s=2017 header.b=pN7yewWW; spf=pass (domain: posteo.de, ip: 185.67.36.66, mailfrom: mhaeuser@posteo.de) Received: from submission (posteo.de [89.146.220.130]) by mout02.posteo.de (Postfix) with ESMTPS id E8A94240101 for ; Fri, 24 Dec 2021 14:52:46 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.de; s=2017; t=1640353966; bh=KH0sjAiMJGdMQMDYxZEKYswrPjQN4veHipF4hQdz4os=; h=Date:Subject:To:Cc:From:From; b=pN7yewWWgxZe2BiA+n4s5IyG1gM3F270Mo7y01uuuHtlTjKg1KYRStBPkUEKbCimu 2i/JX+9AaxgxCCDEb8Q5MYZ7e4HR+lvw6D0s5VykrUIk7IFOnGK9ktrwl8VzBZUtOz kG11pFGNcOtMfMbTxmas0kOh9drkOaA8awSwi8uNd1+YIZvdsUOUsoadDBLSLec81d Ge1lvisOiHXaS5ubGLOqNosNgDdqCp65mLtdzlHNM0teBeefWUuxIOeoeZBsbXSXRr aHi7i33MdjJrt1KfpLeV/0zSKxK1u+67/2f4exI/XXjmsiqJmzKr815DAy36O9qHeV VOfUOzB9S5+Ig== Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4JL7mX1ktyz6tp8; Fri, 24 Dec 2021 14:52:43 +0100 (CET) Message-ID: Date: Fri, 24 Dec 2021 13:52:43 +0000 MIME-Version: 1.0 Subject: Re: [edk2-devel] [PATCH edk2 v1 3/3] StandaloneMmPkg: Fix check buffer address failed issue from TF-A To: Ming Huang , devel@edk2.groups.io Cc: omkar.kulkarni@arm.com, Sami Mujawar , ardb+tianocore@kernel.org, jiewen.yao@intel.com, Supreeth Venkatesh , ming.huang-@outlook.com References: <898f19b9-a0eb-dfdb-0690-86cca1acb716@posteo.de> <8c4866d1-e312-4c80-0b39-37cc2a42c22a@linux.alibaba.com> From: =?UTF-8?B?TWFydmluIEjDpHVzZXI=?= In-Reply-To: <8c4866d1-e312-4c80-0b39-37cc2a42c22a@linux.alibaba.com> Content-Language: en-GB Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable On 24.12.21 02:18, Ming Huang wrote: > > =E5=9C=A8 12/23/21 7:05 PM, Marvin H=C3=A4user =E5=86=99=E9=81=93: >> On 23.12.21 11:46, Ming Huang wrote: >>> =E5=9C=A8 12/16/21 5:15 PM, Marvin H=C3=A4user =E5=86=99=E9=81=93: >>>> =EF=BB=BFHey all, >>>> >>>>> On 15. Dec 2021, at 16:02, Ming Huang w= rote: >>>>> >>>>> =EF=BB=BF >>>>> >>>>>> On 12/9/21 1:46 AM, Omkar Anand Kulkarni wrote: >>>>>> Hi Ming, >>>>>> Thanks for this patch. This patch helps to resolve Standalone MM iss= ue while exercising RAS use case. >>>>>> Few comments mentioned inline. >>>>>> - Omkar >>>>>>> On 10/15/21 2:39 PM, Ming Huang via groups.io wrote: >>>>>>> There are two scene communicate with StandaloneMm(MM): >>>>>>> 1 edk2 -> TF-A -> MM, communicate MM use non-secure buffer which >>>>>>> specify by EFI_SECURE_PARTITION_BOOT_INFO.SpNsCommBufBase; >>>>>>> 2 RAS scene: fiq -> TF-A -> MM, use secure buffer which >>>>>>> specify by EFI_SECURE_PARTITION_BOOT_INFO.SpShareBufBase; >>>>>>> For now, the second scene will failed because check buffer address. >>>>>>> This patch add CheckBufferAddr() to support check address for secur= e buffer. >>>>>>> Signed-off-by: Ming Huang >>>>>>> --- >>>>>>> StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c=C2=A0=C2=A0= =C2=A0=C2=A0 | 70 >>>>>>> ++++++++++++++++---- >>>>>>> StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c | 21 >>>>>>> ++++++ >>>>>>> StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.h |=C2=A0 1= + >>>>>>> 3 files changed, 79 insertions(+), 13 deletions(-) >>>>>>> diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c >>>>>>> b/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c >>>>>>> index 5dfaf9d751..63fab1bd78 100644 >>>>>>> --- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c >>>>>>> +++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/EventHandle.c >>>>>>> @@ -50,6 +50,7 @@ EFI_MM_COMMUNICATE_HEADER >>>>>>> **PerCpuGuidedEventContext =3D NULL; >>>>>>> // Descriptor with whereabouts of memory used for communication wit= h >>>>>>> the normal world=C2=A0 EFI_MMRAM_DESCRIPTOR=C2=A0 mNsCommBuffer; >>>>>>> +EFI_MMRAM_DESCRIPTOR=C2=A0 mSCommBuffer; >>>>>>> MP_INFORMATION_HOB_DATA *mMpInformationHobData; >>>>>>> @@ -60,6 +61,58 @@ EFI_MM_CONFIGURATION_PROTOCOL mMmConfig =3D >>>>>>> { >>>>>>> STATIC EFI_MM_ENTRY_POINT=C2=A0=C2=A0=C2=A0=C2=A0 mMmEntryPoint =3D= NULL; >>>>>>> +STATIC >>>>>>> +EFI_STATUS >>>>>>> +CheckBufferAddr ( >>>>>>> +=C2=A0 IN UINTN CommBufferAddr >>>>>>> +=C2=A0 ) >>>>>>> +{ >>>>>>> +=C2=A0 UINTN=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 CommBufferSize; >>>>>>> +=C2=A0 EFI_STATUS Status; >>>>>>> + >>>>>>> +=C2=A0 Status =3D=C2=A0 EFI_SUCCESS; >>>>>>> +=C2=A0 if (CommBufferAddr < mNsCommBuffer.PhysicalStart) { >>>>>>> +=C2=A0=C2=A0=C2=A0 Status =3D EFI_ACCESS_DENIED; >>>>>>> +=C2=A0 } >>>>>>> + >>>>>>> +=C2=A0 if ((CommBufferAddr + sizeof (EFI_MM_COMMUNICATE_HEADER)) >= =3D >>>>>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 (mNsCommBuffer.PhysicalStart + mNsC= ommBuffer.PhysicalSize)) { >>>>>>> +=C2=A0=C2=A0=C2=A0 Status =3D=C2=A0 EFI_INVALID_PARAMETER; >>>>>> Single space after "Status =3D " >>>>> Modify it in v2. >>>>> >>>>>> - Omkar >>>>>>> +=C2=A0 } >>>>>>> + >>>>>>> +=C2=A0 // Find out the size of the buffer passed=C2=A0 CommBufferS= ize =3D >>>>>>> + ((EFI_MM_COMMUNICATE_HEADER *) CommBufferAddr)->MessageLength >>>>>>> + >>>>>>> +=C2=A0=C2=A0=C2=A0 sizeof (EFI_MM_COMMUNICATE_HEADER); >>>>>>> + >>>>>>> +=C2=A0 // perform bounds check. >>>>>>> +=C2=A0 if (CommBufferAddr + CommBufferSize >=3D >>>>>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 mNsCommBuffer.PhysicalStart + mNsCo= mmBuffer.PhysicalSize) { >>>>>>> +=C2=A0=C2=A0=C2=A0 Status =3D=C2=A0 EFI_ACCESS_DENIED; >>>>>> Single space after "Status =3D " >>>>> Modify it in v2. >>>>> >>>>>> - Omkar >>>>>>> +=C2=A0 } >>>>>>> + >>>>>>> +=C2=A0 if (!EFI_ERROR (Status)) { >>>>>> In case of error this function call will not return from here. It wi= ll execute the code below comparing the MM Communicate buffer address with = the Secure buffer address, which may cause wrong return type being returned= . Can you check this, please? >>>>>> - Omkar >>>>>>> +=C2=A0=C2=A0=C2=A0 return EFI_SUCCESS; >>>>>>> +=C2=A0 } >>>>>>> + >>>>>>> +=C2=A0 Status =3D=C2=A0 EFI_SUCCESS; >>>>>>> +=C2=A0 if (CommBufferAddr < mSCommBuffer.PhysicalStart) { >>>>>>> +=C2=A0=C2=A0=C2=A0 Status =3D EFI_ACCESS_DENIED; >>>>>>> +=C2=A0 } >>>>>>> + >>>>>>> +=C2=A0 if ((CommBufferAddr + sizeof (EFI_MM_COMMUNICATE_HEADER)) >= =3D >>>>>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 (mSCommBuffer.PhysicalStart + mSCom= mBuffer.PhysicalSize)) { >>>>>>> +=C2=A0=C2=A0=C2=A0 Status =3D=C2=A0 EFI_INVALID_PARAMETER; >>>>>>> +=C2=A0 } >>>>>>> + >>>>>>> +=C2=A0 // perform bounds check. >>>>>>> +=C2=A0 if (CommBufferAddr + CommBufferSize >=3D >>>>>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 mSCommBuffer.PhysicalStart + mSComm= Buffer.PhysicalSize) { >>>>>>> +=C2=A0=C2=A0=C2=A0 Status =3D=C2=A0 EFI_ACCESS_DENIED; >>>>>>> +=C2=A0 } >>>>>>> + >>>>>>> +=C2=A0 return Status; >>>>>>> +} >>>>>>> + >>>>>> CheckBufferAddr() function performs validity and overflow checks on = the Communication buffers. These checks are same for both the non-secure >>>>>> MM communicate buffer and secure buffer shared between EL3 and S-EL0= . Can this code be combined ( example below)? This will help mitigate the a= bove mentioned return type issue as well. >>>>> Your example is a good idea to solve this case. I may modify it like = below in v2: >>>>> >>>>> STATIC >>>>> EFI_STATUS >>>>> CheckBufferAddr ( >>>>> IN UINTN CommBufferAddr >>>>> ) >>>>> { >>>>> UINTN=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 CommBufferSize; >>>>> EFI_STATUS Status; >>>>> UINT64=C2=A0=C2=A0=C2=A0=C2=A0 NsCommBufferEnd; >>>>> UINT64=C2=A0=C2=A0=C2=A0=C2=A0 SCommBufferEnd; >>>>> UINT64=C2=A0=C2=A0=C2=A0=C2=A0 CommBufferEnd; >>>>> >>>>> Status =3D=C2=A0 EFI_SUCCESS; >>>>> NsCommBufferEnd =3D mNsCommBuffer.PhysicalStart + mNsCommBuffer.Physi= calSize; >>>>> SCommBufferEnd =3D mSCommBuffer.PhysicalStart + mSCommBuffer.Physical= Size; >>>>> >>>>> if ((CommBufferAddr >=3D mNsCommBuffer.PhysicalStart) && >>>>> (CommBufferAddr < NsCommBufferEnd)) { >>>>> CommBufferEnd =3D NsCommBufferEnd; >>>>> } else if ((CommBufferAddr >=3D mSCommBuffer.PhysicalStart) && >>>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 (CommBufferAddr <=3D SCommBufferEnd))= { >>>> I find it odd the check here (lesser-equals) is inconsistent with the = check above (lesser). It=E2=80=99d be caught below anyway, but I=E2=80=99d = change this to lesser to keep the return codes consistent. >>> Should be lesser, modify it in v3. >>> >>>>> CommBufferEnd =3D SCommBufferEnd; >>>>> } else { >>>>> return EFI_ACCESS_DENIED; >>>>> } >>>>> >>>>> if ((CommBufferAddr + sizeof (EFI_MM_COMMUNICATE_HEADER)) >=3D CommBu= fferEnd) { >>>> Why is greater-equals used here? MessageLength =3D=3D 0 is not filtere= d below, so this looks odd to be honest, as this is only the theoretical ma= ximum buffer end. >>>> >>>> How do you know this cannot wraparound? I actually don=E2=80=99t think= we do. We do know it holds that CommBufferAddr <=3D CommBufferEnd though, = so checking CommBufferEnd - CommBufferAddr < sizeof (EFI_MM_COMMUNICATE_HEA= DER) would give you that for free, if we assume the UINT64 variables above = are actually bounded by UINTN, which seems reasonable - could ASSERT. >>> In my mind, (CommBufferAddr + sizeof (EFI_MM_COMMUNICATE_HEADER)) >=3D = CommBufferEnd) >>> is the same with: CommBufferEnd - CommBufferAddr < sizeof (EFI_MM_COMMU= NICATE_HEADER) >> Okay, assume: >> CommBufferEnd =3D MAX_UINTN >> CommBufferAddr =3D MAX_UINTN - 1 (MAX_UINTN - 1 < MAX_UINTN, so the chec= k above would pass) >> sizeof (EFI_MM_COMMUNICATE_HEADER) =3D 16 (should be accurate for 64-bit= architectures) >> >> Then (assume implicit mod 2^N on both sides due to bounded integers!): >> (CommBufferAddr + sizeof (EFI_MM_COMMUNICATE_HEADER)) >=3D CommBufferEnd= ) <=3D> ((MAX_UINTN - 1) + 16) >=3D MAX_UINTN <=3D> MAX_UINTN + 15 >=3D MAX= _UINTN <=3D>(wraparound!!) 14 >=3D MAX_UINTN <=3D> FALSE >> >> And (assume implicit mod 2^N on both sides due to bounded integers!): >> CommBufferEnd - CommBufferAddr < sizeof (EFI_MM_COMMUNICATE_HEADER) <=3D= > MAX_UINTN - (MAX_UINTN - 1) < 16 <=3D> 1 < 16 <=3D> TRUE >> >> Due to wraparound semantics and the knowledge derived by the if-checks a= bove they are by no means the same. The left term of the first equation can= wrap around (or it cannot, but then we need some concrete proof that it ca= nnot), and the left term of the second equation obviously cannot (directly = follows from the if statements before). > I got it. Thanks for your proper comments. > I may modify it like below in v3: > ---------------------------------------- > STATIC > EFI_STATUS > CheckBufferAddr ( > IN UINTN BufferAddr > ) > { > UINTN BufferSize; > UINT64 NsCommBufferEnd; > UINT64 SCommBufferEnd; > UINT64 CommBufferEnd; > > NsCommBufferEnd =3D mNsCommBuffer.PhysicalStart + mNsCommBuffer.Physic= alSize; > SCommBufferEnd =3D mSCommBuffer.PhysicalStart + mSCommBuffer.PhysicalS= ize; > > if ((BufferAddr >=3D mNsCommBuffer.PhysicalStart) && > (BufferAddr < NsCommBufferEnd)) { > CommBufferEnd =3D NsCommBufferEnd; > } else if ((BufferAddr >=3D mSCommBuffer.PhysicalStart) && > (BufferAddr < SCommBufferEnd)) { > CommBufferEnd =3D SCommBufferEnd; > } else { > return EFI_ACCESS_DENIED; > } > > if ((CommBufferEnd - BufferAddr) < sizeof (EFI_MM_COMMUNICATE_HEADER))= { > return EFI_INVALID_PARAMETER; Just another cosmetic thing I just noticed, why is this Invalid=20 Parameter? The check above yields Access Denied when the buffer start is=20 out of a trusted range, OK. The check below yields Access Denied when=20 the buffer data extends beyond the trusted range, OK. What makes this=20 check different from the other ones that it gets a different return=20 code? I'm not sure on the policy of function documentation (i.e. whether=20 one is needed), but what would the difference be in their descriptions? > } > > // Find out the size of the buffer passed > BufferSize =3D ((EFI_MM_COMMUNICATE_HEADER *) BufferAddr)->MessageLeng= th + > sizeof (EFI_MM_COMMUNICATE_HEADER); Same issue as above, can also be solved by rewriting as subtraction.=20 Because you now know that (CommBufferEnd - BufferAddr) >=3D sizeof=20 (EFI_MM_COMMUNICATE_HEADER). Best regards, Marvin > // perform bounds check. > if ((CommBufferEnd - BufferAddr) < BufferSize) { > return EFI_ACCESS_DENIED; > } > > return EFI_SUCCESS; > } > ---------------------------------------- > > - Ming > >>>> Alternatively, you could not store the maximum buffer end but the maxi= mum buffer size, so the additions of the buffer start would just vanish. Th= is might be more readable too I think. >>> As CommBufferAddr may be not at the begin of communicate buffer, >>> so check size with the maximum buffer size is not enough. >>> >>>>> Status =3D=C2=A0 EFI_INVALID_PARAMETER; >>>> Why is there no return here? This can proceed when the buffer cannot f= it this header, and yet below the header is dereferenced. >>> Modify it in v3. >>> >>>>> } >>>>> >>>>> // Find out the size of the buffer passed >>>>> CommBufferSize =3D ((EFI_MM_COMMUNICATE_HEADER *) CommBufferAddr)->Me= ssageLength + >>>>> sizeof (EFI_MM_COMMUNICATE_HEADER); >>>> Same wraparound concern, same suggestion for solving it. >>>> >>>>> // perform bounds check. >>>>> if (CommBufferAddr + CommBufferSize >=3D CommBufferEnd) { >>>>> Status =3D=C2=A0 EFI_ACCESS_DENIED; >>>> It=E2=80=99s obviously not bad here, but for consistency=E2=80=99s sak= e, to mitigate bugs introduced by future changes, and readability, I=E2=80= =99d return here and just return EFI_SUCCESS below, removing the code requi= rement of keeping Status consistent with the check results. >>> Modify it in v3. >> Thanks for the modifications. >> >> Best regards, >> Marvin >> >>> - Ming >>> >>>> Finally, I really believe this kind of function should be abstracted i= n a way that it can be consumed by all places that accept any sort of commu= nication buffer. Buffer validity checking is too critical than to duplicate= it in every consumer. >>>> >>>> Thanks! >>>> >>>> Best regards, >>>> Marvin >>>> >>>>> } >>>>> >>>>> return Status; >>>>> } >>>>> >>>>> - Ming >>>>> >>>>>> STATIC >>>>>> EFI_STATUS >>>>>> CheckBufferAddr ( >>>>>> IN UINTN CommBufferAddr >>>>>> ) >>>>>> { >>>>>> UINTN=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0 CommBufferSize; >>>>>> EFI_STATUS=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0 Status; >>>>>> EFI_MMRAM_DESCRIPTOR CommBuffer; >>>>>> if (CommBufferAddr < mNsCommBuffer.PhysicalStart || >>>>>> CommBufferAddr > (mNsCommBuffer.PhysicalStart + mNsCommBuffer.Physic= alSize)) { >>>>>> CommBuffer =3D mSCommBuffer; >>>>>> } else { >>>>>> CommBuffer =3D mNsCommBuffer; >>>>>> } >>>>>> if (CommBufferAddr < CommBuffer.PhysicalStart) { >>>>>> Status =3D EFI_ACCESS_DENIED; >>>>>> } >>>>>> if ((CommBufferAddr + sizeof (EFI_MM_COMMUNICATE_HEADER)) >=3D >>>>>> (CommBuffer.PhysicalStart + CommBuffer.PhysicalSize)) { >>>>>> Status =3D EFI_INVALID_PARAMETER; >>>>>> } >>>>>> // Find out the size of the buffer passed >>>>>> CommBufferSize =3D ((EFI_MM_COMMUNICATE_HEADER *) CommBufferAddr)->M= essageLength + >>>>>> sizeof (EFI_MM_COMMUNICATE_HEADER); >>>>>> // perform bounds check. >>>>>> if (CommBufferAddr + CommBufferSize >=3D >>>>>> CommBuffer.PhysicalStart + CommBuffer.PhysicalSize) { >>>>>> Status =3D EFI_ACCESS_DENIED; >>>>>> } >>>>>> return Status; >>>>>> } >>>>>> - Omkar >>>>>>> /** >>>>>>> The PI Standalone MM entry point for the TF-A CPU driver. >>>>>>> @@ -104,25 +157,16 @@ PiMmStandaloneArmTfCpuDriverEntry ( >>>>>>> return EFI_INVALID_PARAMETER; >>>>>>> } >>>>>>> -=C2=A0 if (NsCommBufferAddr < mNsCommBuffer.PhysicalStart) { >>>>>>> -=C2=A0=C2=A0=C2=A0 return EFI_ACCESS_DENIED; >>>>>>> -=C2=A0 } >>>>>>> - >>>>>>> -=C2=A0 if ((NsCommBufferAddr + sizeof (EFI_MM_COMMUNICATE_HEADER))= >=3D >>>>>>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 (mNsCommBuffer.PhysicalStart + mNsC= ommBuffer.PhysicalSize)) { >>>>>>> -=C2=A0=C2=A0=C2=A0 return EFI_INVALID_PARAMETER; >>>>>>> +=C2=A0 Status =3D CheckBufferAddr (NsCommBufferAddr);=C2=A0 if (EF= I_ERROR (Status)) >>>>>>> + { >>>>>>> +=C2=A0=C2=A0=C2=A0 DEBUG ((DEBUG_ERROR, "Check Buffer failed: %r\n= ", Status)); >>>>>>> +=C2=A0=C2=A0=C2=A0 return Status; >>>>>>> } >>>>>>> // Find out the size of the buffer passed >>>>>>> NsCommBufferSize =3D ((EFI_MM_COMMUNICATE_HEADER *) >>>>>>> NsCommBufferAddr)->MessageLength + >>>>>>> sizeof (EFI_MM_COMMUNICATE_HEADER); >>>>>>> -=C2=A0 // perform bounds check. >>>>>>> -=C2=A0 if (NsCommBufferAddr + NsCommBufferSize >=3D >>>>>>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 mNsCommBuffer.PhysicalStart + mNsCo= mmBuffer.PhysicalSize) { >>>>>>> -=C2=A0=C2=A0=C2=A0 return EFI_ACCESS_DENIED; >>>>>>> -=C2=A0 } >>>>>>> - >>>>>>> GuidedEventContext =3D NULL; >>>>>>> // Now that the secure world can see the normal world buffer, alloc= ate >>>>>>> // memory to copy the communication buffer to the secure world. >>>>>>> diff --git >>>>>>> a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c >>>>>>> b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c >>>>>>> index fd9c59b4da..96dad20dd1 100644 >>>>>>> --- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c >>>>>>> +++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.c >>>>>>> @@ -107,6 +107,7 @@ StandaloneMmCpuInitialize ( >>>>>>> UINTN=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0 Index; >>>>>>> UINTN=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0 ArraySize; >>>>>>> VOID=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0 *HobStart; >>>>>>> +=C2=A0 EFI_MMRAM_HOB_DESCRIPTOR_BLOCK=C2=A0 *MmramRangesHob; >>>>>>> ASSERT (SystemTable !=3D NULL); >>>>>>> mMmst =3D SystemTable; >>>>>>> @@ -186,6 +187,26 @@ StandaloneMmCpuInitialize ( >>>>>>> CopyMem (&mNsCommBuffer, NsCommBufMmramRange, >>>>>>> sizeof(EFI_MMRAM_DESCRIPTOR)); >>>>>>> DEBUG ((DEBUG_INFO, "mNsCommBuffer: 0x%016lx - 0x%lx\n", >>>>>>> mNsCommBuffer.CpuStart, mNsCommBuffer.PhysicalSize)); >>>>>>> +=C2=A0 Status =3D GetGuidedHobData ( >>>>>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0 HobStart, >>>>>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0 &gEfiMmPeiMmramMemoryReserveGuid, >>>>>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0 (VOID **) &MmramRangesHob >>>>>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0 ); >>>>>>> +=C2=A0 if (EFI_ERROR (Status)) { >>>>>>> +=C2=A0=C2=A0=C2=A0 DEBUG ((DEBUG_ERROR, "MmramRangesHob data extra= ction failed - >>>>>>> 0x%x\n", Status)); >>>>>>> +=C2=A0=C2=A0=C2=A0 return Status; >>>>>>> +=C2=A0 } >>>>>>> + >>>>>>> +=C2=A0 // >>>>>>> +=C2=A0 // As CreateHobListFromBootInfo(), the base and size of buf= fer shared >>>>>>> + with=C2=A0 // privileged Secure world software is in second one. >>>>>>> +=C2=A0 // >>>>>>> +=C2=A0 CopyMem ( >>>>>>> +=C2=A0=C2=A0=C2=A0 &mSCommBuffer, >>>>>>> +=C2=A0=C2=A0=C2=A0 &MmramRangesHob->Descriptor[0] + 1, >>>>>> Can this be changed to >>>>>> &MmramRangesHob->Descriptor[1], >>>>>> - Omkar >>>>>>> +=C2=A0=C2=A0=C2=A0 sizeof(EFI_MMRAM_DESCRIPTOR) >>>>>>> +=C2=A0 ); >>>>>>> + >>>>>>> // >>>>>>> // Extract the MP information from the Hoblist >>>>>>> // >>>>>>> diff --git >>>>>>> a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.h >>>>>>> b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.h >>>>>>> index 2c96439c15..2e03b20d85 100644 >>>>>>> --- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.h >>>>>>> +++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.h >>>>>>> @@ -30,6 +30,7 @@ extern EFI_MM_CPU_PROTOCOL mMmCpuState;=C2=A0 // >>>>>>> extern EFI_MM_COMMUNICATE_HEADER=C2=A0=C2=A0=C2=A0 **PerCpuGuidedEv= entContext; >>>>>>> extern EFI_MMRAM_DESCRIPTOR=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0 mNsCommBuffer; >>>>>>> +extern EFI_MMRAM_DESCRIPTOR=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0 mSCommBuffer; >>>>>>> extern MP_INFORMATION_HOB_DATA=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 = *mMpInformationHobData; >>>>>>> extern EFI_MM_CONFIGURATION_PROTOCOL mMmConfig; >>>>>>> --=20 >>>>>>> 2.17.1 >>>>>> IMPORTANT NOTICE: The contents of this email and any attachments are= confidential and may also be privileged. If you are not the intended recip= ient, please notify the sender immediately and do not disclose the contents= to any other person, use it for any purpose, or store or copy the informat= ion in any medium. Thank you. >>>>>=20