From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pj1-f53.google.com (mail-pj1-f53.google.com [209.85.216.53]) by mx.groups.io with SMTP id smtpd.web09.35591.1629180021600024954 for ; Mon, 16 Aug 2021 23:00:21 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20161025 header.b=npOShXnb; spf=pass (domain: gmail.com, ip: 209.85.216.53, mailfrom: kuqin12@gmail.com) Received: by mail-pj1-f53.google.com with SMTP id qe12-20020a17090b4f8c00b00179321cbae7so4590440pjb.2 for ; Mon, 16 Aug 2021 23:00:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=F+FNIUeGZg8hJusU7AlyOK0zl/RRiq0HstjH6pS/i28=; b=npOShXnbDMC7WGpAuPI8WJyq/L4ZZfiKkpthIr+xQAc0HdCE8nxGzdL/NE8aLm3OJD PUeILqlIDsoo5w8H8G/p9yMFqpYzgiRJ11Jv2mhKhv90tImq8WbL2F14MzqCbNuVt98P +IBqYK2gRJ5fQ5GOEf8gKL23A6b0m/5mnML5XM3+6XX2rhL2qAIP28GuRQHvA3b5JX/P lLxumEYDBeEj6RM0eci7wW3D4mn1+ZGZZHIE8+isx+OKMDaQatRa9TcelkFBf5DuVacT lFJccaNhrFORUHJby2+f1VXoqwfiLJDU+JEjFZhpBm/IP3UjPAQvGOFXAI3KviPP4h/y Fx8g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=F+FNIUeGZg8hJusU7AlyOK0zl/RRiq0HstjH6pS/i28=; b=LL+oFPHaoAv3mqQXCj3f0xP1B/uYQmSvB7I56N6DuUuILwg3AXBvrg2UWqeCBvJi26 dzZfneeb3K87TOJ3DcWZ4fUCQhXUFP+D2OQW39TnLlp8klr49AA0VNA9lZWWgmWSCzdf m3HGhI3bId7m+nD2/EdF0EBpfyx2+1+je23rnxD6a8VGtDepnF5/R0DKxVbn1JsQSzNG n1I289h8bohrUzzUlbTCBZ9JIgsLU4bEZ7+8/Aal+v5ISWGf/zWUewIok5N4YLJUzOzH O/+tJmK4wTZYJoc1VRHXU6kh+kYxmxlvLCzGlCUIMit0ebx2pQhMCQHZ8REWOOU0YrDz AnqQ== X-Gm-Message-State: AOAM531m9A1r43T4gNOFj4lwuiLyyYIR4PtdLFwjCuSaenOx/e59jU8f 8Bcri+5JGBGN9WBPruA7Lgw= X-Google-Smtp-Source: ABdhPJzKyTXXW8CphBByx+HMGjTEqooVGRvlr/DSCj1AmKZRr0Xedj289S0we05ewc5hn1yEf3SyGA== X-Received: by 2002:a17:902:7282:b029:12c:75a0:faa5 with SMTP id d2-20020a1709027282b029012c75a0faa5mr1587293pll.35.1629180021192; Mon, 16 Aug 2021 23:00:21 -0700 (PDT) Return-Path: Received: from [192.168.50.18] ([50.35.88.161]) by smtp.gmail.com with ESMTPSA id i1sm953532pjs.31.2021.08.16.23.00.20 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 16 Aug 2021 23:00:20 -0700 (PDT) Subject: Re: [edk2-devel] [PATCH v3 2/7] MdePkg: MmCommunication: Introduce EFI_MM_COMMUNICATE_HEADER_V3 to MdePkg To: "Ni, Ray" , "devel@edk2.groups.io" Cc: "Kinney, Michael D" , Liming Gao , "Liu, Zhiguang" , "Wu, Hao A" , =?UTF-8?Q?Marvin_H=c3=a4user?= , Bret Barkelew , Michael Kubacki References: <20210817050807.766-1-kuqin12@gmail.com> <20210817050807.766-3-kuqin12@gmail.com> From: "Kun Qin" Message-ID: Date: Mon, 16 Aug 2021 23:00:20 -0700 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Hi Ray, This same header will be commonly used for both MM communicate PPI and Protocol. If including this definition in MmCommunicate protocol header, the potential PEIM that installs MM communicate PPI will need to include a PPI header as well as the aforementioned protocol header, which to me seems counter-intuitive. I think PiMultiPhase header seems to be a file more ideal to resolve the entanglement above, as this structure indeed covers multiple phases during boot process. Please let me know if you have any other thoughts. Thanks, Kun On 08/16/2021 22:50, Ni, Ray wrote: > Can you kindly explain why you choose to define the definitions in PiMultiPhase.h instead of MmCommunication.h? > > Thanks, > Ray > >> -----Original Message----- >> From: devel@edk2.groups.io On Behalf Of Kun Qin >> Sent: Tuesday, August 17, 2021 1:08 PM >> To: devel@edk2.groups.io >> Cc: Kinney, Michael D ; Liming Gao ; Liu, Zhiguang >> ; Wu, Hao A ; Marvin Häuser ; Bret Barkelew >> ; Michael Kubacki >> Subject: [edk2-devel] [PATCH v3 2/7] MdePkg: MmCommunication: Introduce EFI_MM_COMMUNICATE_HEADER_V3 to >> MdePkg >> >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3398 >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3430 >> >> This change introduces a new definition for MM communicate header >> structure, intending to provide better portability between different >> architectures (IA32 & X64) and adapt to flexible array supported by >> modern compilers. >> >> The original MessageLength field of EFI_MM_COMMUNICATE_HEADER, as a >> generic definition, was used for both PEI and DXE MM communication. On a >> system that supports PEI MM launch, but operates PEI in 32bit mode and MM >> foundation in 64bit, the current EFI_MM_COMMUNICATE_HEADER definition >> will cause structure parse error due to UINTN used. This introduction >> removes the architecture dependent field by defining this field as >> UINT64. >> >> The new signature could help identifying whether the data received is >> compiliant with this new data structure, which will help for binary >> release modules to identify usage of legacy data structure. >> >> Version field is also added to indicate the current version of header in >> case there is need for minor modification in the future. >> >> The data field of MM communicate message is replaced with flexible array >> to allow users not having to consume extra data during communicate and >> author code more intrinsically. >> >> Cc: Michael D Kinney >> Cc: Liming Gao >> Cc: Zhiguang Liu >> Cc: Hao A Wu >> Cc: Marvin Häuser >> Cc: Bret Barkelew >> Cc: Michael Kubacki >> >> Signed-off-by: Kun Qin >> --- >> >> Notes: >> v3: >> - Newly introduced communicate v3 >> - Used flexible arrays [Marvin] >> - Added static assert [Michael] >> >> MdePkg/Include/Pi/PiMultiPhase.h | 55 ++++++++++++++++++++ >> MdePkg/MdePkg.dec | 5 ++ >> 2 files changed, 60 insertions(+) >> >> diff --git a/MdePkg/Include/Pi/PiMultiPhase.h b/MdePkg/Include/Pi/PiMultiPhase.h >> index 89280d9d3506..8c60338091b3 100644 >> --- a/MdePkg/Include/Pi/PiMultiPhase.h >> +++ b/MdePkg/Include/Pi/PiMultiPhase.h >> @@ -103,6 +103,17 @@ SPDX-License-Identifier: BSD-2-Clause-Patent >> #define EFI_SMRAM_CLOSED EFI_MMRAM_CLOSED >> #define EFI_SMRAM_LOCKED EFI_MMRAM_LOCKED >> >> +/// >> +/// MM Communicate header constants >> +/// >> +#define COMMUNICATE_HEADER_V3_GUID \ >> + { \ >> + 0x68e8c853, 0x2ba9, 0x4dd7, { 0x9a, 0xc0, 0x91, 0xe1, 0x61, 0x55, 0xc9, 0x35 } \ >> + } >> + >> +#define EFI_MM_COMMUNICATE_HEADER_V3_SIGNATURE 0x4D434833 // "MCH3" >> +#define EFI_MM_COMMUNICATE_HEADER_V3_VERSION 3 >> + >> /// >> /// Structure describing a MMRAM region and its accessibility attributes. >> /// >> @@ -149,6 +160,48 @@ typedef struct _EFI_MM_RESERVED_MMRAM_REGION { >> UINT64 MmramReservedSize; >> } EFI_MM_RESERVED_MMRAM_REGION; >> >> +#pragma pack(1) >> + >> +/// >> +/// To avoid confusion in interpreting frames, the buffer communicating to MM core through >> +/// EFI_MM_COMMUNICATE3 or later should always start with EFI_MM_COMMUNICATE_HEADER_V3. >> +/// >> +typedef struct { >> + /// >> + /// Indicator GUID for MM core that the communication buffer is compliant with this v3 header. >> + /// Must be gCommunicateHeaderV3Guid. >> + /// >> + EFI_GUID HeaderGuid; >> + /// >> + /// Signature to indicate data is compliant with new MM communicate header structure. >> + /// Must be "MCH3". >> + /// >> + UINT32 Signature; >> + /// >> + /// MM communicate data format version, MM foundation entry point should check if incoming >> + /// data is a supported format before proceeding. >> + /// Must be 3. >> + /// >> + UINT32 Version; >> + /// >> + /// Allows for disambiguation of the message format. >> + /// >> + EFI_GUID MessageGuid; >> + /// >> + /// Describes the size of MessageData (in bytes) and does not include the size of the header. >> + /// >> + UINT64 MessageSize; >> + /// >> + /// Designates an array of bytes that is MessageSize in size. >> + /// >> + UINT8 MessageData[]; >> +} EFI_MM_COMMUNICATE_HEADER_V3; >> + >> +#pragma pack() >> + >> +STATIC_ASSERT ((sizeof (EFI_MM_COMMUNICATE_HEADER_V3) == OFFSET_OF (EFI_MM_COMMUNICATE_HEADER_V3, >> MessageData)), \ >> + "sizeof (EFI_MM_COMMUNICATE_HEADER_V3) does not align with the beginning of flexible array MessageData"); >> + >> typedef enum { >> EFI_PCD_TYPE_8, >> EFI_PCD_TYPE_16, >> @@ -208,4 +261,6 @@ EFI_STATUS >> IN VOID *ProcedureArgument >> ); >> >> +extern EFI_GUID gCommunicateHeaderV3Guid; >> + >> #endif >> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec >> index a28a2daaffa8..411079a4343e 100644 >> --- a/MdePkg/MdePkg.dec >> +++ b/MdePkg/MdePkg.dec >> @@ -823,6 +823,11 @@ [Guids] >> # >> gLinuxEfiInitrdMediaGuid = {0x5568e427, 0x68fc, 0x4f3d, {0xac, 0x74, 0xca, 0x55, 0x52, 0x31, 0xcc, 0x68}} >> >> + # >> + # GUID indicates the MM communication data is compliant with v3 communication header. >> + # >> + gCommunicateHeaderV3Guid = { 0x68e8c853, 0x2ba9, 0x4dd7, { 0x9a, 0xc0, 0x91, 0xe1, 0x61, 0x55, 0xc9, 0x35 } } >> + >> [Guids.IA32, Guids.X64] >> ## Include/Guid/Cper.h >> gEfiIa32X64ErrorTypeCacheCheckGuid = { 0xA55701F5, 0xE3EF, 0x43de, { 0xAC, 0x72, 0x24, 0x9B, 0x57, 0x3F, 0xAD, 0x2C }} >> -- >> 2.32.0.windows.1 >> >> >> >> >> >