From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pg1-f171.google.com (mail-pg1-f171.google.com [209.85.215.171]) by mx.groups.io with SMTP id smtpd.web08.4423.1623289392666176456 for ; Wed, 09 Jun 2021 18:43:12 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20161025 header.b=di+3/bx4; spf=pass (domain: gmail.com, ip: 209.85.215.171, mailfrom: kuqin12@gmail.com) Received: by mail-pg1-f171.google.com with SMTP id i34so14807778pgl.9 for ; Wed, 09 Jun 2021 18:43:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=rI1VDqvv5xDXWz4cDwEHZQBieP0i8SgS5AfgMuo9aAc=; b=di+3/bx4H8m/d6Q/CiHpya8TGEmnbr+OyxeenzRurYdnBd+//CriYBfWPo7tMtvL/F bL58kSqNZVqf1CTib5/yh1HeIEQsIg9OZm2RJWnQ91q2VItnP5xkrkQ8a3JTXClzjv8B ji6/W3HonV4tPkVBgRs+1KKrcMv5Hi4kshF+6h5YMn1qOWreym0rh27dqmMosHMuVpNt A2oPxHt8vJO1EFoFHkV/lTdWm89vnyDbZWOiaZwNxtsOC3OMRCby0+oRB/1Kb4TvqRNY 4k3NtvpYFse/9eDRbSsHBsp1D/CejUIh85xHIPTHXEWv7je97x+GA0vIlceACsp7HVXv Ih5g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=rI1VDqvv5xDXWz4cDwEHZQBieP0i8SgS5AfgMuo9aAc=; b=uoFoU3bnU7ApY1/E6kxnczgJfCfLXSbKRvYePLd0SM/36wFEBf0Ly/0xuEud7xXxxo 4RGD3Ap09GBh3loXRtgdGllIICe/y+lGWCEFJXa/4UpgtH+jWKX1KeZRXQSf4Hz05K7Z 5cZnQdoFhLUNYbTo0vNWDx4T3l8fhSLSKOt7HLRWp8oadbuSTDSWAoRe6klp3RpSQMf4 Xj917YPq8cR5QxFM0XDTmDYTRCjHFrn7/Ylq179CWgIXtKcBiqUgeyaoDCsVhHe5m8FN 3b/giPlTjGgEdeDE1X1UvZNgecRxe0NXwGuqQXbnx/KjL6uX/WO2oBstI1qHY81OMmYT yHBQ== X-Gm-Message-State: AOAM531M0PoIYAfw0sZu0ermQnaSgyCqpU89UHV1Jfp7yGs4VfYXI5LK hMo4IwGZQYj/kt7nhkac9DgTDh2sEaP7Xg== X-Google-Smtp-Source: ABdhPJx7xLBJ27+qhVQ2geaZ50wDsCyCymzzyFQ4LTC/aYy09JizfWJLS0CaAOQJEaJEpYELXC60sg== X-Received: by 2002:a63:4e50:: with SMTP id o16mr2534893pgl.140.1623289392060; Wed, 09 Jun 2021 18:43:12 -0700 (PDT) Return-Path: Received: from localhost.localdomain ([50.35.88.161]) by smtp.gmail.com with ESMTPSA id p20sm624990pff.204.2021.06.09.18.43.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 09 Jun 2021 18:43:11 -0700 (PDT) From: "Kun Qin" To: devel@edk2.groups.io Cc: Michael D Kinney , Liming Gao , Zhiguang Liu , Andrew Fish , Laszlo Ersek , Leif Lindholm Subject: [PATCH v1 1/5] EDK2 Code First: PI Specification: EFI_MM_COMMUNICATE_HEADER Update Date: Wed, 9 Jun 2021 18:42:55 -0700 Message-Id: <20210610014259.1151-2-kuqin12@gmail.com> X-Mailer: git-send-email 2.31.1.windows.1 In-Reply-To: <20210610014259.1151-1-kuqin12@gmail.com> References: <20210610014259.1151-1-kuqin12@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3430 This change includes specification update markdown file that describes the proposed PI Specification v1.7 Errata A in detail and potential impact to the existing codebase. Cc: Michael D Kinney Cc: Liming Gao Cc: Zhiguang Liu Cc: Andrew Fish Cc: Laszlo Ersek Cc: Leif Lindholm Signed-off-by: Kun Qin --- BZ3430-SpecChange.md | 88 ++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/BZ3430-SpecChange.md b/BZ3430-SpecChange.md new file mode 100644 index 000000000000..33a1ffda447b --- /dev/null +++ b/BZ3430-SpecChange.md @@ -0,0 +1,88 @@ +# Title: Change MessageLength Field of EFI_MM_COMMUNICATE_HEADER to UINT64 + +## Status: Draft + +## Document: UEFI Platform Initialization Specification Version 1.7 Errata A + +## License + +SPDX-License-Identifier: CC-BY-4.0 + +## Submitter: [TianoCore Community](https://www.tianocore.org) + +## Summary of the change + +Change the `MessageLength` Field of `EFI_MM_COMMUNICATE_HEADER` from UINTN to UINT64 to remove architecture dependency: + +```c +typedef struct { + EFI_GUID HeaderGuid; + UINT64 MessageLength; + UINT8 Data[ANYSIZE_ARRAY]; +} EFI_MM_COMMUNICATE_HEADER; +``` + +## Benefits of the change + +In PI Spec v1.7 Errata A, Vol.4, Sec 5.7 MM Communication Protocol, the MessageLength field of `EFI_MM_COMMUNICATE_HEADER` (also defined as `EFI_SMM_COMMUNICATE_HEADER`) is defined as type UINTN. + +But this structure, as a generic definition, could be used for both PEI and DXE MM communication. Thus for 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. + +## Impact of the change + +This change will impact the known structure consumers including: + +```bash +MdeModulePkg/Core/PiSmmCore/PiSmmIpl +MdeModulePkg/Application/SmiHandlerProfileInfo +MdeModulePkg/Application/MemoryProfileInfo +``` + +For consumers that are not using `OFFSET_OF(EFI_MM_COMMUNICATE_HEADER, Data)`, but performing explicit addition such as the existing MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c, one will need to change code implementation to match new structure definition. Otherwise, the code compiled on IA32 architecture will experience structure field dereference error. + +User who currently uses UINTN local variables as place holder of MessageLength will need to use caution to make cast from UINTN to UINT64 and vice versa. It is recommended to use `SafeUint64ToUintn` for such operations when the value is indeterministic. + +Note: MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib is also consuming this structure, but it handled this size discrepancy internally. If this potential spec change is not applied, all applicable PEI MM communicate callers will need to use the same routine as that of SmmLockBoxPeiLib to invoke a properly populated EFI_MM_COMMUNICATE_HEADER to be used in X64 MM foundation. + +## Detailed description of the change [normative updates] + +### Specification Changes + +1. In PI Specification v1.7 Errata A: Vol. 4 Page-91, the definition of `EFI_MM_COMMUNICATE_HEADER` should be changed from current: + +```c +typedef struct { + EFI_GUID HeaderGuid; + UINTN MessageLength; + UINT8 Data[ANYSIZE_ARRAY]; +} EFI_MM_COMMUNICATE_HEADER; +``` + +to: + +```c +typedef struct { + EFI_GUID HeaderGuid; + UINT64 MessageLength; + UINT8 Data[ANYSIZE_ARRAY]; +} EFI_MM_COMMUNICATE_HEADER; +``` + +### Code Changes + +1. Remove the explicit calculation of the offset of `Data` in `EFI_MM_COMMUNICATE_HEADER`. Thus applicable calculations of `sizeof(EFI_GUID) + sizeof(UINTN)` should be replaced with `OFFSET_OF(EFI_MM_COMMUNICATE_HEADER, Data)` or similar alternatives. These calculations are identified in: + +```bash +MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c +MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c +``` + +1. Resolve potentially mismatched type between `UINTN` and `UINT64`. This would occur when `MessageLength` or its derivitive are used for local calculation with existing `UINTN` typed variables. Code change regarding this perspective is per case evaluation: if the variables involved are all deterministic values, and there is no overflow or underflow risk, a cast operation (from `UINTN` to `UINT64`) can be safely used. Otherwise, the calculation will be performed in `UINT64` bitwidth and then convert to `UINTN` using `SafeUint64*` and `SafeUint64ToUintn`, respectively. These operations are identified in: + +```bash +MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c +MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c +MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c +``` + +1. After all above changes applied and specification updated, `MdePkg/Include/Protocol/MmCommunication.h` will need to be updated to match new definition that includes the field type update. -- 2.31.1.windows.1