Re: [edk2-devel] [PATCH 3/5] MdePkg: EFI_MM_COMUNICATION2_PROTOCOL

["Zhiguang Liu" <zhiguang.liu@intel.com>]

Hi Robert,

Give my comments inline

Thanks
Zhiguang

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Oleksiy
> Yakovlev
> Sent: Friday, May 8, 2020 3:03 AM
> To: devel@edk2.groups.io
> Cc: Gao, Liming <liming.gao@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Felixp@ami.com; oleksiyy@ami.com;
> robert@ami.com
> Subject: [edk2-devel] [PATCH 3/5] MdePkg:
> EFI_MM_COMUNICATION2_PROTOCOL
> 
> From: Robert Phelps <robert@ami.com>
> 
> Created a new header to support the new
> EFI_MM_COMMUNICATION2_PROTOCOL Protocol  as defined in the PI 1.7a
> Specification (PI 1.7a Mantis 1993)
> 
> Signed-off-by: Robert Phelps <robert@ami.com>
> ---
>  MdePkg/Include/Protocol/MmCommunication2.h | 66
> ++++++++++++++++++++++
>  MdePkg/MdePkg.dec                          |  7 +++
>  2 files changed, 73 insertions(+)
>  create mode 100644 MdePkg/Include/Protocol/MmCommunication2.h
> 
> diff --git a/MdePkg/Include/Protocol/MmCommunication2.h
> b/MdePkg/Include/Protocol/MmCommunication2.h
> new file mode 100644
> index 0000000000..b6fe5069e1
> --- /dev/null
> +++ b/MdePkg/Include/Protocol/MmCommunication2.h
> @@ -0,0 +1,66 @@
> +/** @file
> +    Provides a runtime service for communicating between DXE drivers
> +and a registered MMI handler **/ #ifndef _MM_COMMUNICATION2_H_
> #define
> +_MM_COMMUNICATION2_H_
> +
> +///
> +/// Global ID for EFI_MM_COMUNICATION@_PROTOCOL_GUID
> +///
> +#define EFI_MM_COMMUNICATION2_PROTOCOL_GUID \
> +        { 0x378daedc, 0xf06b, 0x4446, { 0x83, 0x14, 0x40, 0xab, 0x93,
> +0x3c, 0x87, 0xa3 }}
> +
> +
> +///
> +/// The forward declaration for
> EFI_MM_COMMUNICATION2_PROTOCOL_GUID
> +///
> +typedef struct _EFI_MM_COMMUNICATION2_PROTOCOL
> +EFI_MM_COMMUNICATION2_PROTOCOL;
> +
> +
> +/**
> + Communicates with a registered handler.
> +
> +    Usage is identical to
> EFI_MM_COMMUNICATION_PROTOCOL.Communicate() except for the
> notes below:
> +    - Instead of passing just the physical address via the CommBuffer
> parameter, the caller must pass both the physical and the virtual addresses
> of the communication buffer.
> +    - If no virtual remapping has taken place, the physical address will be
> equal to the virtual address, and so the caller is required to pass the same
> value for both parameters.
> +
> +    @param This                 The EFI_MM_COMMUNICATION2_PROTOCOL
> instance.
> +    @param mBufferPhysical      Physical address of the buffer to convey into
> MMRAM.

Shouldn't it be @param CommBufferPhysical

> +    @param CommBufferVirtual    Virtual address of the buffer to convey into
> MMRAM.

> +    @param 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   The CommBuffer** parameters do
> not refer to the same location in memory.
> +    @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.
> +**/
> +
> +typedef
> +EFI_STATUS
> +(EFIAPI *EFI_MM_COMMUNICATE2)(
> +IN CONST EFI_MM_COMMUNICATION2_PROTOCOL    *This,
> +IN OUT VOID                    *CommBufferPhysical,
> +IN OUT VOID                    *CommBufferVirtual,
> +IN OUT UINTN                    *CommSize OPTIONAL

Please intent the parameter names

> +);
> +
> +
> +/// This protocol provides a means of communicating between drivers
> +outside /// of MM and MMI handlers inside of MM, in a way that hides
> +the implementation /// details regarding whether traditional or standalone
> MM is being used.
> +
> +typedef struct _EFI_MM_COMMUNICATION2_PROTOCOL {
> +EFI_MM_COMMUNICATE2        Communicate;
> +} EFI_MM_COMMUNICATION2_PROTOCOL;
> +
> +
> +#endif
> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec index
> 6c37c2181c..c64cad75dd 100644
> --- a/MdePkg/MdePkg.dec
> +++ b/MdePkg/MdePkg.dec
> @@ -1277,6 +1277,13 @@
>    ## Include/Protocol/SpiSmmNorFlash.h
>    gEfiSpiSmmNorFlashProtocolGuid         = { 0xaab18f19, 0xfe14, 0x4666, { 0x86,
> 0x04, 0x87, 0xff, 0x6d, 0x66, 0x2c, 0x9a }}
> 
> +  #
> +  # Protocols defined in PI 1.7
> +  #
> +
> +  ## Include/Protocol/MmComunication2.h
> +  gEfiMmCommunication2ProtocolGuid       = { 0x378daedc, 0xf06b, 0x4446,
> { 0x83, 0x14, 0x40, 0xab, 0x93, 0x3c, 0x87, 0xa3 }}
> +
>    #
>    # Protocols defined in UEFI2.1/UEFI2.0/EFI1.1
>    #
> --
> 2.24.1.windows.2
> 
> 
> Please consider the environment before printing this email.
> 
> The information contained in this message may be confidential and
> proprietary to American Megatrends (AMI).  This communication is intended
> to be read only by the individual or entity to whom it is addressed or by their
> designee. If the reader of this message is not the intended recipient, you are
> on notice that any distribution of this message, in any form, is strictly
> prohibited.  Please promptly notify the sender by reply e-mail or by
> telephone at 770-246-8600, and then delete or destroy all copies of the
> transmission.
> 
>