On Mon, 2017-11-13 at 11:48 +0000, Ard Biesheuvel wrote:
> On 13 November 2017 at 11:40, Supreeth Venkatesh
> <[email protected]> wrote:
> >
> > On Mon, 2017-11-13 at 10:30 +0000, Ard Biesheuvel wrote:
> > >
> > > On 25 October 2017 at 17:32, Supreeth Venkatesh
> > > <[email protected]> wrote:
> > > >
> > > >
> > > > PI v1.5 Specification Volume 4 defines Management Mode Core
> > > > Interface
> > > > and defines EFI_MM_COMMUNICATION_PROTOCOL. This protocol
> > > > provides a
> > > > means of communicating between drivers outside of MM and MMI
> > > > handlers inside of MM.
> > > >
> > > > This patch implements the EFI_MM_COMMUNICATION_PROTOCOL DXE
> > > > runtime
> > > > driver for AARCH64 platforms. It uses SMCs allocated from the
> > > > standard
> > > > SMC range defined in
> > > > http://infocenter.arm.com/help/topic/com.arm.doc.den0060a/DEN00
> > > > 60A_
> > > > ARM_MM_Interface_Specification.pdf
> > > > to communicate with the standalone MM environment in the secure
> > > > world.
> > > >
> > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > Signed-off-by: Achin Gupta <[email protected]>
> > > > Signed-off-by: Supreeth Venkatesh <[email protected]>
> > > > ---
> > > > .../Drivers/MmCommunicationDxe/MmCommunication.c | 339
> > > > +++++++++++++++++++++
> > > No need to separate the .inf and .c changes, so this patch can be
> > > merged with the previous one.
> > Thanks for more comments/feedback.
> > Ok. Please wait for v3.
> > >
> > >
> > > >
> > > >
> > > > 1 file changed, 339 insertions(+)
> > > > create mode 100644
> > > > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > >
> > > > diff --git
> > > > a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > > new file mode 100644
> > > > index 0000000000..ecf70e666c
> > > > --- /dev/null
> > > > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > > @@ -0,0 +1,339 @@
> > > > +/** @file
> > > > +
> > > > + Copyright (c) 2016-2017, ARM Limited. All rights reserved.
> > > > +
> > > > + This program and the accompanying materials
> > > > + are licensed and made available under the terms and
> > > > conditions
> > > > of the BSD License
> > > > + which accompanies this distribution. The full text of the
> > > > license may be found at
> > > > + http://opensource.org/licenses/bsd-license.php
> > > > +
> > > > + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS
> > > > IS"
> > > > BASIS,
> > > > + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> > > > EXPRESS OR IMPLIED.
> > > > +
> > > > +**/
> > > > +
> > > > +#include <Library/ArmLib.h>
> > > > +#include <Library/ArmSmcLib.h>
> > > > +#include <Library/BaseMemoryLib.h>
> > > > +#include <Library/DebugLib.h>
> > > > +#include <Library/DxeServicesTableLib.h>
> > > > +#include <Library/HobLib.h>
> > > > +#include <Library/PcdLib.h>
> > > > +#include <Library/UefiBootServicesTableLib.h>
> > > > +#include <Library/UefiRuntimeServicesTableLib.h>
> > > > +
> > > > +#include <Protocol/MmCommunication.h>
> > > > +
> > > > +#include <IndustryStandard/ArmStdSmc.h>
> > > > +
> > > > +/**
> > > > + Communicates with a registered handler.
> > > > +
> > > > + This function provides an interface to send and receive
> > > > messages
> > > > to the
> > > > + Standalone MM environment on behalf of UEFI services. This
> > > > function is part
> > > > + of the MM Communication Protocol that may be called in
> > > > physical
> > > > mode prior to
> > > > + SetVirtualAddressMap() and in virtual mode after
> > > > SetVirtualAddressMap().
> > > > +
> > > > + @param[in] This The
> > > > EFI_MM_COMMUNICATION_PROTOCOL instance.
> > > > + @param[in, out] CommBuffer A pointer to the buffer
> > > > to
> > > > convey into MMRAM.
> > > > + @param[in, out] CommSize The size of the data
> > > > buffer
> > > > being passed in.On exit, the size of data
> > > > + being returned. Zero if
> > > > the
> > > > handler does not wish to reply with any data.
> > > > +
> > > > + @retval EFI_SUCCESS The message was
> > > > successfully
> > > > posted.
> > > > + @retval EFI_INVALID_PARAMETER The CommBuffer was NULL.
> > > > + @retval EFI_BAD_BUFFER_SIZE The buffer is too large
> > > > for
> > > > the MM implementation. If this error is
> > > > + returned, the
> > > > MessageLength
> > > > field in the CommBuffer header or the integer
> > > > + pointed by CommSize are
> > > > updated to reflect the maximum payload size the
> > > > + implementation can
> > > > accommodate.
> > > > + @retval EFI_ACCESS_DENIED The CommunicateBuffer
> > > > parameter or CommSize parameter, if not omitted,
> > > > + are in address range
> > > > that
> > > > cannot be accessed by the MM environment
> > > > +**/
> > > Please don't use lines > 80 characters.
> > Ok.
> > >
> > >
> > > >
> > > >
> > > > +EFI_STATUS
> > > > +EFIAPI
> > > > +MmCommunicationCommunicate (
> > > > + IN CONST EFI_MM_COMMUNICATION_PROTOCOL *This,
> > > > + IN OUT VOID *CommBuffer,
> > > > + IN OUT
> > > > UINTN *CommSize OPTIONAL
> > > > + );
> > > > +
> > > > +//
> > > > +// Address, Length of the pre-allocated buffer for
> > > > communication
> > > > with the secure
> > > > +// world.
> > > > +//
> > > > +STATIC ARM_MEMORY_REGION_DESCRIPTOR mNsCommBuffMemRegion;
> > > > +
> > > > +// Notification event when virtual address map is set.
> > > > +STATIC EFI_EVENT mSetVirtualAddressMapEvent;
> > > > +
> > > > +//
> > > > +// Handle to install the MM Communication Protocol
> > > > +//
> > > > +STATIC EFI_HANDLE mMmCommunicateHandle;
> > > > +
> > > > +//
> > > > +// MM Communication Protocol instance
> > > > +//
> > > > +EFI_MM_COMMUNICATION_PROTOCOL mMmCommunication = {
> > > > + MmCommunicationCommunicate
> > > > +};
> > > > +
> > > Please move this below the definition of
> > > MmCommunicationCommunicate(),
> > > and drop the forward declaration.
> > Ok.
> > >
> > >
> > > >
> > > >
> > > > +/**
> > > > + Communicates with a registered handler.
> > > > +
> > > > + This function provides an interface to send and receive
> > > > messages
> > > > to the
> > > > + Standalone MM environment on behalf of UEFI services. This
> > > > function is part
> > > > + of the MM Communication Protocol that may be called in
> > > > physical
> > > > mode prior to
> > > > + SetVirtualAddressMap() and in virtual mode after
> > > > SetVirtualAddressMap().
> > > > +
> > > > + @param[in] This The
> > > > EFI_MM_COMMUNICATION_PROTOCOL instance.
> > > > + @param[in, out] CommBuffer A pointer to the buffer
> > > > to
> > > > convey into SMRAM.
> > > What does 'convey into SMRAM' mean?
> > >
This is to mean Management Mode RAM.
There is a concept of System Management RAM which is separate from
Normal World UEFI.
> > > >
> > > >
> > > > + @param[in, out] CommSize The size of the data
> > > > buffer
> > > > being passed in.On exit, the size of data
> > > > + being returned. Zero if
> > > > the
> > > > handler does not wish to reply with any data.
> > > > +
> > > CommSize is a pointer so it could be NULL but not zero. Or do you
> > > mean
> > > *CommSize?
> > >
> > CommSize is an optional parameter.
> OK, please add OPTIONAL the prototype
Ok.
>
> >
> > Yeah, if Commsize is used, then its
> > value (*CommSize) will be zero, if there is no data. This is
> > something
> > the specification implicitly states.
> OK
>
>
> >
> > >
> > > >
> > > >
> > > > + @retval EFI_SUCCESS The message was
> > > > successfully
> > > > posted.
> > > > + @retval EFI_INVALID_PARAMETER The CommBuffer was NULL.
> > > > + @retval EFI_BAD_BUFFER_SIZE The buffer is too large
> > > > for
> > > > the MM implementation. If this error is
> > > > + returned, the
> > > > MessageLength
> > > > field in the CommBuffer header or the integer
> > > > + pointed by CommSize are
> > > > updated to reflect the maximum payload size the
> > > > + implementation can
> > > > accommodate.
> > > > + @retval EFI_ACCESS_DENIED The CommunicateBuffer
> > > > parameter or CommSize parameter, if not omitted,
> > > > + are in address range
> > > > that
> > > > cannot be accessed by the MM environment
> > > > +**/
> > > STATIC ?
> > >
> > This implements an EFIAPI for the protocol
> > MM_COMMUNICATION_PROTOCOL in
> > MdePkg.Not sure why you want this function static?
> Any function that is only callable via function pointers should be
> static.
>
Ok.
> >
> > >
> > > >
> > > >
> > > > +EFI_STATUS
> > > > +EFIAPI
> > > > +MmCommunicationCommunicate (
> > > > + IN CONST EFI_MM_COMMUNICATION_PROTOCOL *This,
> > > > + IN OUT VOID *CommBuffer,
> > > > + IN OUT UINTN *CommSize
> > > > + )
> > > > +{
> > > > + EFI_MM_COMMUNICATE_HEADER *CommunicateHeader;
> > > > + ARM_SMC_ARGS CommunicateSmcArgs;
> > > > + EFI_STATUS Status;
> > > > + UINTN BufferSize;
> > > > +
> > > > + CommunicateHeader = CommBuffer;
> > > > + Status = EFI_SUCCESS;
> > > > + BufferSize = 0;
> > > > +
> > > > + ZeroMem (&CommunicateSmcArgs, sizeof (ARM_SMC_ARGS));
> > > > +
> > > > + //
> > > > + // Check parameters
> > > > + //
> > > > + if (CommBuffer == NULL) {
> > > > + return EFI_INVALID_PARAMETER;
> > > > + }
> > > > +
> > > > + // If the length of the CommBuffer is 0 then return the
> > > > expected
> > > > length.
> > > > + if (CommSize) {
> > > > + if (*CommSize == 0) {
> > > > + *CommSize = mNsCommBuffMemRegion.Length;
> > > > + return EFI_BAD_BUFFER_SIZE;
> > > Here you return EFI_BAD_BUFFER_SIZE for a 0 sized buffer, while
> > > the
> > > comment says is it returned when the buffer is too large.
> > >
> > Specification states this:
> > "If the CommSize parameter is passed into the call, but the integer
> > it
> > points to, has a value of 0, then this must be updated to reflect
> > the
> > maximum size of the CommBuffer that the implementation can
> > tolerate."
> > Also, Return Values are
> > EFI_INVALID_PARAMETER - The buffer was NULL.
> > EFI_BAD_BUFFER_SIZE - The buffer is too large for the MM
> > implementation. If this error is returned, the MessageLength field
> > in
> > the CommBuffer header or the integer pointed by CommSize, are
> > updated
> > to reflect the maximum payload size the implementation can
> > accommodate.
> > See the function description above for more details
> >
> > So, it does not specify what should be the return value, when
> > *CommSize
> > is zero. It is open to interpretation between EFI_INVALID_PARAMETER
> > and
> > EFI_BAD_BUFFER_SIZE. I choose EFI_BAD_BUFFER_SIZE.
> > Let me know what you think.
> >
> If the specification does not state that an error should be returned,
> we should clarify that first.
I have this in my to-do list, its an error condition (no doubt about
that), its just that the return code in the specification need to have
an updated statement to reflect this.
>
> >
> > >
> > > >
> > > >
> > > > + }
> > > > + //
> > > > + // CommSize must hold HeaderGuid and MessageLength
> > > > + //
> > > > + if (*CommSize < sizeof (EFI_MM_COMMUNICATE_HEADER)) {
> > > > + return EFI_INVALID_PARAMETER;
> > > > + }
> > > > + BufferSize = *CommSize;
> > > > + } else {
> > > > + BufferSize = CommunicateHeader->MessageLength +
> > > > + sizeof (CommunicateHeader->HeaderGuid) +
> > > > + sizeof (CommunicateHeader->MessageLength);
> > > > + }
> > > > +
> > > > + //
> > > > + // If the buffer size is 0 or greater than what can be
> > > > tolerated
> > > > by the MM
> > > > + // environment then return the expected size.
> > > > + //
> > > > + if ((BufferSize == 0) ||
> > > > + (BufferSize > mNsCommBuffMemRegion.Length)) {
> > > > + CommunicateHeader->MessageLength =
> > > > mNsCommBuffMemRegion.Length
> > > > -
> > > > + sizeof
> > > > (CommunicateHeader-
> > > > >
> > > > > HeaderGuid) -
> > > > + sizeof
> > > > (CommunicateHeader-
> > > > >
> > > > > MessageLength);
> > > > + return EFI_BAD_BUFFER_SIZE;
> > > > + }
> > > > +
> > > > + // SMC Function ID
> > > > + CommunicateSmcArgs.Arg0 = ARM_SMC_ID_MM_COMMUNICATE_AARCH64;
> > > > +
> > > > + // Reserved for Future. Must be Zero.
> > > > + CommunicateSmcArgs.Arg1 = 0;
> > > > +
> > > > + if (mNsCommBuffMemRegion.VirtualBase) {
> > > > + CopyMem ((VOID *)mNsCommBuffMemRegion.VirtualBase,
> > > > CommBuffer,
> > > > BufferSize);
> > > > + }
> > > > +
> > > else? What happens if VirtualBase == NULL? Can we still proceed
> > > without copying?
> > Yup. This was Achin's comment as well. Please wait for v3.
> > >
> > >
> > > >
> > > >
> > > > + // For the SMC64 version, this parameter is a 64-bit
> > > > Physical
> > > > Address (PA)
> > > > + // or Intermediate Physical Address (IPA).
> > > > + // For the SMC32 version, this parameter is a 32-bit PA or
> > > > IPA.
> > > > + CommunicateSmcArgs.Arg2 =
> > > > (UINTN)mNsCommBuffMemRegion.PhysicalBase;
> > > > +
> > > > + // comm_size_address is a PA or an IPA that holds the size
> > > > of
> > > > the
> > > > + // communication buffer being passed in. This parameter is
> > > > optional
> > > > + // and can be omitted by passing a zero.
> > > > + // ARM does not recommend using it since this might require
> > > > the
> > > > + // implementation to create a separate memory mapping for
> > > > the
> > > > parameter.
> > > > + // ARM recommends storing the buffer size in the buffer
> > > > itself.
> > > > + CommunicateSmcArgs.Arg3 = 0;
> > > > +
> > > > + // Call the Standalone MM environment.
> > > > + ArmCallSmc (&CommunicateSmcArgs);
> > > > +
> > > > + Status = CommunicateSmcArgs.Arg0;
> > > > + switch (Status) {
> > > I think I mentioned this before, but please don't use Status for
> > > the
> > > ARM_SMC_MM_xxx enum space. Use a separate, correctly typed
> > > variable
> > > instead.
> > Oh. your last email asks to replace EFI_SUCCESS with MM Specific
> > RETURN
> > SUCCESS. Status is still "UINTN" type variable for both MM and
> > general
> > UEFI status codes.
> > Let me know if you feel strongly about it, I will have two
> > different
> > "UINTN" type variables.
> Status should be EFI_STATUS not UINTN.
>
> The ARM_SMC_MM_xxx symbolic constants are fundamentally a different
> type, and you are relying on the fact that EFI_SUCCESS and
> ARM_SMC_MM_RET_SUCCESS happen to be #defined to the same numeric
> value. That is very bad coding style.
No. I am *not* relying on the fact ARM_SMC_MM_RET_SUCCESS and
EFI_SUCCESS are same. I am just relying on the fact that
EFI_STATUS and MM return Status happen to be "UINTN" type ultimately.
EFI_STATUS -> RETURN_STATUS -> UINTN (MdePkg/Include/Base.h).
However, I get that you are trying to say these can be implemented as
"enums" and hence the reason to have two different variables to convert
from one type to the other. I will change it.
>
> >
> > >
> > >
> > > >
> > > >
> > > > + case ARM_SMC_MM_RET_SUCCESS:
> > > > + // On exit, the size of data being returned is inferred
> > > > from
> > > > + // CommSize or MessageLength + Header.
> > > > + CopyMem (CommBuffer, (const VOID
> > > > *)mNsCommBuffMemRegion.VirtualBase, BufferSize);
> > > > + break;
> > > > +
> > > > + case ARM_SMC_MM_RET_NOT_SUPPORTED:
> > > > + case ARM_SMC_MM_RET_INVALID_PARAMS:
> > > > + Status = EFI_INVALID_PARAMETER;
> > > > + break;
> > > > +
> > > > + case ARM_SMC_MM_RET_DENIED:
> > > > + Status = EFI_ACCESS_DENIED;
> > > > + break;
> > > > +
> > > > + case ARM_SMC_MM_RET_NO_MEMORY:
> > > > + // Unexpected error since the CommSize was checked for
> > > > zero
> > > > length
> > > > + // prior to issuing the SMC
> > > > + default:
> > > > + Status = EFI_ACCESS_DENIED;
> > > > + ASSERT (0);
> > > > + }
> > > > +
> > > > + return Status;
> > > > +}
> > > > +
> > > > +/**
> > > > + Notification callback on SetVirtualAddressMap event.
> > > > +
> > > > + This function notifies the MM communication protocol
> > > > interface
> > > > on
> > > > + SetVirtualAddressMap event and converts pointers used in
> > > > this
> > > > driver
> > > > + from physical to virtual address.
> > > > +
> > > > + @param Event SetVirtualAddressMap event.
> > > > + @param Context A context when the
> > > > SetVirtualAddressMap
> > > > triggered.
> > > > +
> > > > + @retval EFI_SUCCESS The function executed successfully.
> > > > + @retval Other Some error occurred when executing
> > > > this
> > > > function.
> > > > +
> > > > +**/
> > > > +STATIC
> > > > +VOID
> > > > +EFIAPI
> > > > +NotifySetVirtualAddressMap (
> > > > + IN EFI_EVENT Event,
> > > > + IN VOID *Context
> > > > + )
> > > > +{
> > > > + EFI_STATUS Status;
> > > > +
> > > > + Status = gRT->ConvertPointer (EFI_OPTIONAL_PTR,
> > > > + (VOID
> > > > **)&mNsCommBuffMemRegion.VirtualBase
> > > > + );
> > > > + if (EFI_ERROR (Status)) {
> > > > + DEBUG ((DEBUG_ERROR, "NotifySetVirtualAddressMap(): Unable
> > > > to
> > > > convert MM runtime pointer. Status:0x%x\n", Status));
> > > Please use %r for reporting EFI_STATUS values.
> > >
> > Ok.
> > >
> > > >
> > > >
> > > > + }
> > > > +
> > > > +}
> > > > +
> > > > +/**
> > > > + The Entry Point for MM Communication
> > > > +
> > > > + This function installs the MM communication protocol
> > > > interface
> > > > and finds out
> > > > + what type of buffer management will be required prior to
> > > > invoking the
> > > > + communication SMC.
> > > > +
> > > > + @param ImageHandle The firmware allocated handle for the
> > > > EFI
> > > > image.
> > > > + @param SystemTable A pointer to the EFI System Table.
> > > > +
> > > > + @retval EFI_SUCCESS The entry point is executed
> > > > successfully.
> > > > + @retval Other Some error occurred when executing
> > > > this
> > > > entry point.
> > > > +
> > > > +**/
> > > > +EFI_STATUS
> > > > +EFIAPI
> > > > +MmCommunicationInitialize (
> > > > + IN EFI_HANDLE ImageHandle,
> > > > + IN EFI_SYSTEM_TABLE *SystemTable
> > > > + )
> > > > +{
> > > > + EFI_STATUS Status;
> > > > +
> > > > + mNsCommBuffMemRegion.PhysicalBase = 0;
> > > > + mNsCommBuffMemRegion.VirtualBase = 0;
> > > > + mNsCommBuffMemRegion.Length = 0;
> > > > +
> > > Please drop the 0 assignmentsm since they are overridden
> > > immediately.
> > Ok.
> > >
> > >
> > > >
> > > >
> > > > + mNsCommBuffMemRegion.PhysicalBase = PcdGet64
> > > > (PcdMmBufferBase);
> > > > + // During boot , Virtual and Physical are same
> > > > + mNsCommBuffMemRegion.VirtualBase =
> > > > mNsCommBuffMemRegion.PhysicalBase;
> > > > + mNsCommBuffMemRegion.Length = PcdGet64 (PcdMmBufferSize);
> > > > +
> > > > + if (mNsCommBuffMemRegion.PhysicalBase == 0) {
> > > > + DEBUG ((DEBUG_ERROR, "MmCommunicateInitialize: Invalid MM
> > > > Buffer Base Address.\n"));
> > > > + return EFI_INVALID_PARAMETER;
> > > > + }
> > > > +
> > > > + if (mNsCommBuffMemRegion.Length == 0) {
> > > > + DEBUG ((DEBUG_ERROR, "MmCommunicateInitialize: Maximum
> > > > Buffer
> > > > Size is zero.\n"));
> > > > + return EFI_INVALID_PARAMETER;
> > > > + }
> > > > +
> > > > + Status = gDS->AddMemorySpace (EfiGcdMemoryTypeSystemMemory,
> > > > + mNsCommBuffMemRegion.PhysicalB
> > > > ase,
> > > > + mNsCommBuffMemRegion.Length,
> > > > + EFI_MEMORY_WB |
> > > > + EFI_MEMORY_XP |
> > > > + EFI_MEMORY_RUNTIME);
> > > > + if (EFI_ERROR (Status)) {
> > > > + DEBUG ((DEBUG_ERROR, "MmCommunicateInitialize: Failed to
> > > > add
> > > > MM-NS Buffer Memory Space\n"));
> > > > + return EFI_INVALID_PARAMETER;
> > > > + }
> > > > +
> > > > + Status = gDS-
> > > > >
> > > > > SetMemorySpaceAttributes(mNsCommBuffMemRegion.PhysicalBase,
> > > > + mNsCommBuffMemRegion.
> > > > Leng
> > > > th,
> > > > + EFI_MEMORY_WB |
> > > > EFI_MEMORY_XP);
> > > > + if (EFI_ERROR (Status)) {
> > > > + DEBUG ((DEBUG_ERROR, "MmCommunicateInitialize: Failed to
> > > > set
> > > > MM-NS Buffer Memory attributes\n"));
> > > cleanup?
> > Yup. Wait for v3.
> > >
> > >
> > > >
> > > >
> > > > + return EFI_INVALID_PARAMETER;
> > > > + }
> > > > +
> > > > + Status = gBS->AllocatePages (AllocateAddress,
> > > > + EfiRuntimeServicesData,
> > > > + EFI_SIZE_TO_PAGES
> > > > (mNsCommBuffMemRegion.Length),
> > > > + &mNsCommBuffMemRegion.PhysicalB
> > > > ase)
> > > > ;
> > > > + if (EFI_ERROR (Status)) {
> > > > + DEBUG ((DEBUG_ERROR, "MmCommunicateInitialize: Failed to
> > > > allocate MM-NS Buffer Memory Space\n"));
> > > cleanup?
> > >
> > This was Udit's comment as well. Wait for v3.
> > >
> > > >
> > > >
> > > > + return EFI_INVALID_PARAMETER;
> > > > + }
> > > > +
> > > > + // Install the communication protocol
> > > > + Status = gBS->InstallProtocolInterface
> > > > (&mMmCommunicateHandle,
> > > > + &gEfiMmCommunication
> > > > Prot
> > > > ocolGuid,
> > > > + EFI_NATIVE_INTERFACE
> > > > ,
> > > > + &mMmCommunication);
> > > > + if (EFI_ERROR(Status)) {
> > > > + DEBUG ((DEBUG_ERROR, "MmCommunicationInitialize: Failed to
> > > > install MM communication protocol\n"));
> > > cleanup?
> > Yup. Wait for v3.
> > >
> > >
> > > >
> > > >
> > > > + return EFI_INVALID_PARAMETER;
> > > > + }
> > > > +
> > > > + // Register notification callback when virtual address is
> > > > associated
> > > > + // with the physical address.
> > > > + // Create a Set Virtual Address Map event.
> > > > + //
> > > > + Status = gBS->CreateEvent
> > > > (EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE, // Type
> > > > + TPL_NOTIFY,
> > > > /
> > > > / NotifyTpl
> > > > + NotifySetVirtualAddressMap,
> > > > /
> > > > / NotifyFunction
> > > > + NULL,
> > > > /
> > > > / NotifyContext
> > > > + &mSetVirtualAddressMapEvent
> > > > /
> > > > / Event
> > > > + );
> > > > + ASSERT_EFI_ERROR (Status);
> > > > +
> > > cleanup?
> > Not required. Since it will still work at boot time.
> If gBS->CreateEvent () fails in a RELEASE build, it will ignore the
> ASSERT () and return the error code, which will result in the driver
> to be unloaded. Without cleanup, that will leave protocol pointers
> pointing into memory which has now been freed. Also, the buffer
> allocation will not be freed either.
Ok. So the cleanup here will be to free the buffer and uninstall the
protocol. Is that what you had in mind?
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
