From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f67.google.com (mail-wm1-f67.google.com [209.85.128.67]) by mx.groups.io with SMTP id smtpd.web10.2480.1589275839285525383 for ; Tue, 12 May 2020 02:30:39 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=2Jv9k791; spf=pass (domain: nuviainc.com, ip: 209.85.128.67, mailfrom: leif@nuviainc.com) Received: by mail-wm1-f67.google.com with SMTP id g12so22453044wmh.3 for ; Tue, 12 May 2020 02:30:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuviainc-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=Puj7Ukgn9QCE2rSVCQQvs1r7zo6y7s8OLIBP5JaLarE=; b=2Jv9k791tZaLb2rBcQ7CBFWgoAqFUHM9ph2p+HsSSnLg2HfaaWwDuuKzSzkxtrDHng YhMIKCop1C9xPLHmg/+BxjJnJaKa4C2bo4DjV7yC1smgCl2Zps3I+7Iz528fuO/dqHJM KS+gQ+2+8C5v1fOU51A0lYxt6zBVeH6jgqKAitKgCrTbSulh5TLWt4VKqs3xy/py3+4K LUBSgL6o5cktUH9qVlFxQSG9NngHcletOpFoOMuHDt6ClbS/UPfs6UJBH6TJIDaJVKSu aJs/eat1Yw59KOCOBiz2J8+juOTpGTDO8BpAGuDzdxszCF8MNVXuDrpfHYQjNOJk4/Ot wsVQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=Puj7Ukgn9QCE2rSVCQQvs1r7zo6y7s8OLIBP5JaLarE=; b=mubj6VHsmxIndkBF99i6ntB8ilP/Z1BGX97fiIWZShm4HyPNePWj/tZuyeB3YfxWv2 aPg9uzF2xrQ+T6HFB1Apa7MgLsqUSradMCkGB4P1Awp5hyPwlvAPHO4I2MQS2jXX4AzJ Wue7YFVu7RGRdTwdBOv/Mif38kmVMkYfnREQDW4LXzq6Jw6sEuNCbzftoxyg168syYKE +II7sOoLnPP/qzOnosWPsyPWV3MSlcLwynasjhoF9ieq6ghXbGGad2djslIgq/Ik0d1W 4p7kSZTiEAaZCqKRK6925hBQ4radSE1G9sAY7Zsqxq9CY5PcwyWWP9+7bj/bEKfhd7Ba 6p9Q== X-Gm-Message-State: AGi0PuYqsXOT4UTqgcVZO5zVUwwBLFJ1MosqmFN26LzmYgzTLM8TL/MB QQaEfRJMEW+DQMKSdDOpHC66FQ== X-Google-Smtp-Source: APiQypL0N8GZIljUIdN6EIB8fDyn13M6CnbceNDd0cIFyWJHC/bRzun+dWCuHsnLX2oyF7xXdtFcGQ== X-Received: by 2002:a1c:9c15:: with SMTP id f21mr36805871wme.139.1589275837767; Tue, 12 May 2020 02:30:37 -0700 (PDT) Return-Path: Received: from vanye ([2001:470:1f09:12f0:b26e:bfff:fea9:f1b8]) by smtp.gmail.com with ESMTPSA id g10sm20658399wrx.4.2020.05.12.02.30.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 12 May 2020 02:30:37 -0700 (PDT) Date: Tue, 12 May 2020 10:30:35 +0100 From: "Leif Lindholm" To: Ard Biesheuvel Cc: devel@edk2.groups.io, Michael D Kinney , Liming Gao , Jian J Wang , Hao A Wu , Sami Mujawar , Jiewen Yao Subject: Re: [PATCH resend 3/7] ArmPkg/MmCommunicationDxe: expose MM Communicate 2 protocol Message-ID: <20200512093035.GX21486@vanye> References: <20200506172734.2475-1-ard.biesheuvel@arm.com> <20200506172734.2475-4-ard.biesheuvel@arm.com> MIME-Version: 1.0 In-Reply-To: <20200506172734.2475-4-ard.biesheuvel@arm.com> User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Wed, May 06, 2020 at 19:27:30 +0200, Ard Biesheuvel wrote: > Implement the new MmCommunication2 protocol which supports the use > of standalone MM at runtime inside an address space that has been > virtually remapped by the OS. > > Note that the implementation of the old MM Communicate protocol is > removed: it never worked correctly so there is no point in keeping it. > > Signed-off-by: Ard Biesheuvel Reviewed-by: Leif Lindholm I agree the old protocol should be dropped. Any current users would be well warned that their foundation is a swamp. / Leif > --- > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 81 +++++++++----------- > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf | 6 +- > 2 files changed, 41 insertions(+), 46 deletions(-) > > diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c > index a9e06be1adc2..9457eaf1d809 100644 > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c > @@ -1,6 +1,6 @@ > /** @file > > - Copyright (c) 2016-2018, ARM Limited. All rights reserved. > + Copyright (c) 2016-2020, ARM Limited. All rights reserved. > > SPDX-License-Identifier: BSD-2-Clause-Patent > > @@ -16,7 +16,7 @@ > #include > #include > > -#include > +#include > > #include > > @@ -39,39 +39,34 @@ STATIC EFI_HANDLE mMmCommunicateHandle; > /** > 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(). > + This function provides a service to send and receive messages from a registered UEFI service. > > - @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. This is optional. > + @param[in] This The EFI_MM_COMMUNICATION_PROTOCOL instance. > + @param[in] CommBufferPhysical Physical address of the MM communication buffer > + @param[in] CommBufferVirtual Virtual address of the MM communication buffer > + @param[in] 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. > + This parameter is optional and may be NULL. > + > + @retval EFI_SUCCESS The message was successfully posted. > + @retval EFI_INVALID_PARAMETER CommBufferPhysical was NULL or CommBufferVirtual 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. > > - @retval EFI_SUCCESS The message was successfully posted. > - @retval EFI_INVALID_PARAMETER The CommBuffer was NULL. > - @retval EFI_BAD_BUFFER_SIZE The buffer size is incorrect 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 > EFI_STATUS > EFIAPI > -MmCommunicationCommunicate ( > - IN CONST EFI_MM_COMMUNICATION_PROTOCOL *This, > - IN OUT VOID *CommBuffer, > - IN OUT UINTN *CommSize OPTIONAL > +MmCommunication2Communicate ( > + IN CONST EFI_MM_COMMUNICATION2_PROTOCOL *This, > + IN OUT VOID *CommBufferPhysical, > + IN OUT VOID *CommBufferVirtual, > + IN OUT UINTN *CommSize OPTIONAL > ) > { > EFI_MM_COMMUNICATE_HEADER *CommunicateHeader; > @@ -87,11 +82,11 @@ MmCommunicationCommunicate ( > // > // Check parameters > // > - if (CommBuffer == NULL) { > + if (CommBufferVirtual == NULL) { > return EFI_INVALID_PARAMETER; > } > > - CommunicateHeader = CommBuffer; > + CommunicateHeader = CommBufferVirtual; > // CommBuffer is a mandatory parameter. Hence, Rely on > // MessageLength + Header to ascertain the > // total size of the communication payload rather than > @@ -136,7 +131,7 @@ MmCommunicationCommunicate ( > CommunicateSmcArgs.Arg1 = 0; > > // Copy Communication Payload > - CopyMem ((VOID *)mNsCommBuffMemRegion.VirtualBase, CommBuffer, BufferSize); > + CopyMem ((VOID *)mNsCommBuffMemRegion.VirtualBase, CommBufferVirtual, BufferSize); > > // comm_buffer_address (64-bit physical address) > CommunicateSmcArgs.Arg2 = (UINTN)mNsCommBuffMemRegion.PhysicalBase; > @@ -149,7 +144,7 @@ MmCommunicationCommunicate ( > > switch (CommunicateSmcArgs.Arg0) { > case ARM_SMC_MM_RET_SUCCESS: > - ZeroMem (CommBuffer, BufferSize); > + ZeroMem (CommBufferVirtual, BufferSize); > // On successful return, the size of data being returned is inferred from > // MessageLength + Header. > CommunicateHeader = (EFI_MM_COMMUNICATE_HEADER *)mNsCommBuffMemRegion.VirtualBase; > @@ -158,7 +153,7 @@ MmCommunicationCommunicate ( > sizeof (CommunicateHeader->MessageLength); > > CopyMem ( > - CommBuffer, > + CommBufferVirtual, > (VOID *)mNsCommBuffMemRegion.VirtualBase, > BufferSize > ); > @@ -191,8 +186,8 @@ MmCommunicationCommunicate ( > // > // MM Communication Protocol instance > // > -EFI_MM_COMMUNICATION_PROTOCOL mMmCommunication = { > - MmCommunicationCommunicate > +STATIC EFI_MM_COMMUNICATION2_PROTOCOL mMmCommunication2 = { > + MmCommunication2Communicate > }; > > /** > @@ -293,7 +288,7 @@ MmGuidedEventNotify ( > Header.Data[0] = 0; > > Size = sizeof (Header); > - MmCommunicationCommunicate (&mMmCommunication, &Header, &Size); > + MmCommunication2Communicate (&mMmCommunication2, &Header, &Header, &Size); > } > > /** > @@ -312,7 +307,7 @@ MmGuidedEventNotify ( > **/ > EFI_STATUS > EFIAPI > -MmCommunicationInitialize ( > +MmCommunication2Initialize ( > IN EFI_HANDLE ImageHandle, > IN EFI_SYSTEM_TABLE *SystemTable > ) > @@ -363,9 +358,9 @@ MmCommunicationInitialize ( > // Install the communication protocol > Status = gBS->InstallProtocolInterface ( > &mMmCommunicateHandle, > - &gEfiMmCommunicationProtocolGuid, > + &gEfiMmCommunication2ProtocolGuid, > EFI_NATIVE_INTERFACE, > - &mMmCommunication > + &mMmCommunication2 > ); > if (EFI_ERROR(Status)) { > DEBUG ((DEBUG_ERROR, "MmCommunicationInitialize: " > @@ -402,8 +397,8 @@ MmCommunicationInitialize ( > UninstallProtocol: > gBS->UninstallProtocolInterface ( > mMmCommunicateHandle, > - &gEfiMmCommunicationProtocolGuid, > - &mMmCommunication > + &gEfiMmCommunication2ProtocolGuid, > + &mMmCommunication2 > ); > > CleanAddedMemorySpace: > diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > index 505228704ea5..2465fb77c58e 100644 > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf > @@ -2,7 +2,7 @@ > # > # DXE MM Communicate driver > # > -# Copyright (c) 2016 - 2018, ARM Limited. All rights reserved. > +# Copyright (c) 2016 - 2020, ARM Limited. All rights reserved. > # > # SPDX-License-Identifier: BSD-2-Clause-Patent > # > @@ -14,7 +14,7 @@ [Defines] > FILE_GUID = 09EE81D3-F15E-43F4-85B4-CB9873DA5D6B > MODULE_TYPE = DXE_RUNTIME_DRIVER > VERSION_STRING = 1.0 > - ENTRY_POINT = MmCommunicationInitialize > + ENTRY_POINT = MmCommunication2Initialize > > # > # The following is for reference only and not required by > @@ -40,7 +40,7 @@ [LibraryClasses] > UefiDriverEntryPoint > > [Protocols] > - gEfiMmCommunicationProtocolGuid ## PRODUCES > + gEfiMmCommunication2ProtocolGuid ## PRODUCES > > [Guids] > gEfiEndOfDxeEventGroupGuid > -- > 2.17.1 >