Date   

Re: [PATCH 1/1] UsbBusDxe: fix NOOPT build error

Wu, Hao A
 

-----Original Message-----
From: Gerd Hoffmann <kraxel@...>
Sent: Monday, December 20, 2021 10:33 PM
To: devel@edk2.groups.io
Cc: Wu, Hao A <hao.a.wu@...>; Philippe Mathieu-Daudé
<philmd@...>; Wang, Jian J <jian.j.wang@...>; Pawel
Polawski <ppolawsk@...>; Ni, Ray <ray.ni@...>; Gao, Liming
<gaoliming@...>; Gerd Hoffmann <kraxel@...>
Subject: [PATCH 1/1] UsbBusDxe: fix NOOPT build error

Reviewed-by: Hao A Wu <hao.a.wu@...>
Will tweak the subject to "MdeModulePkg/UsbBusDxe: fix NOOPT build error" before merging. If concerns, please help to raise.

Best Regards,
Hao Wu



gcc-11 (fedora 35):

/home/kraxel/projects/edk2/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus
.c: In function ‘UsbIoBulkTransfer’:
/home/kraxel/projects/edk2/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBus
.c:277:12: error: ‘UsbHcBulkTransfer’ accessing 80 bytes in a region of size 8 [-
Werror=stringop-overflow=]

Signed-off-by: Gerd Hoffmann <kraxel@...>
---
MdeModulePkg/Bus/Usb/UsbBusDxe/UsbUtility.h | 2 +-
MdeModulePkg/Bus/Usb/UsbBusDxe/UsbUtility.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbUtility.h
b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbUtility.h
index 04cf36d3c860..d93370a6c21e 100644
--- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbUtility.h
+++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbUtility.h
@@ -148,7 +148,7 @@ UsbHcBulkTransfer (
IN UINT8 DevSpeed,
IN UINTN MaxPacket,
IN UINT8 BufferNum,
- IN OUT VOID *Data[EFI_USB_MAX_BULK_BUFFER_NUM],
+ IN OUT VOID *Data[],
IN OUT UINTN *DataLength,
IN OUT UINT8 *DataToggle,
IN UINTN TimeOut,
diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbUtility.c
b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbUtility.c
index 12d08c0b740f..740e7babb0ca 100644
--- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbUtility.c
+++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbUtility.c
@@ -267,7 +267,7 @@ UsbHcBulkTransfer (
IN UINT8 DevSpeed,
IN UINTN MaxPacket,
IN UINT8 BufferNum,
- IN OUT VOID *Data[EFI_USB_MAX_BULK_BUFFER_NUM],
+ IN OUT VOID *Data[],
IN OUT UINTN *DataLength,
IN OUT UINT8 *DataToggle,
IN UINTN TimeOut,
--
2.33.1


Re: [edk2-platforms: PATCH] System will occur a CPU exception error when sorting CPU APIC map, because of a pointer point to an wrong space.

Ni, Ray
 

Reviewed-by: Ray Ni <ray.ni@...>

-----Original Message-----
From: Lin, JackX <jackx.lin@...>
Sent: Tuesday, December 21, 2021 2:48 PM
To: devel@edk2.groups.io
Cc: Lin, JackX <jackx.lin@...>; Lin, JackX <jackx.lin@...>; Chiu, Chasel <chasel.chiu@...>; Dong, Eric
<eric.dong@...>; Yao, Jiewen <jiewen.yao@...>; Ni, Ray <ray.ni@...>; Chaganty, Rangasai V
<rangasai.v.chaganty@...>; Kuo, Donald <donald.kuo@...>; Kumar, Chandana C <chandana.c.kumar@...>
Subject: [edk2-platforms: PATCH] System will occur a CPU exception error when sorting CPU APIC map, because of a pointer
point to an wrong space.

Signed-off-by: JackX Lin <JackX.Lin@...>
Cc: Chasel Chiu <chasel.chiu@...>
Cc: Dong Eric <eric.dong@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Ray Ni <ray.ni@...>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@...>
Cc: Donald Kuo <Donald.Kuo@...>
Cc: Chandana C Kumar <chandana.c.kumar@...>
Cc: JackX Lin <JackX.Lin@...>
---
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index 785cf4c2f9..05fc7799fb 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -169,7 +169,7 @@ SortCpuLocalApicInTable (
UINT32 Index;
UINT32 CurrProcessor;
UINT32 BspApicId;
- EFI_CPU_ID_ORDER_MAP *TempVal;
+ EFI_CPU_ID_ORDER_MAP TempVal;
EFI_CPU_ID_ORDER_MAP *CpuIdMapPtr;
UINT32 CoreThreadMask;
EFI_CPU_ID_ORDER_MAP *TempCpuApicIdOrderTable;
@@ -183,7 +183,6 @@ SortCpuLocalApicInTable (
}

TempCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof (EFI_CPU_ID_ORDER_MAP));
- TempVal = AllocateZeroPool (sizeof (EFI_CPU_ID_ORDER_MAP));
CoreThreadMask = (UINT32) ((1 << mNumOfBitShift) - 1);

for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus; CurrProcessor++, Index++) {
--
2.32.0.windows.2


[edk2-platforms: PATCH] System will occur a CPU exception error when sorting CPU APIC map, because of a pointer point to an wrong space.

JackX Lin
 

Signed-off-by: JackX Lin <JackX.Lin@...>
Cc: Chasel Chiu <chasel.chiu@...>
Cc: Dong Eric <eric.dong@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Ray Ni <ray.ni@...>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@...>
Cc: Donald Kuo <Donald.Kuo@...>
Cc: Chandana C Kumar <chandana.c.kumar@...>
Cc: JackX Lin <JackX.Lin@...>
---
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index 785cf4c2f9..05fc7799fb 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -169,7 +169,7 @@ SortCpuLocalApicInTable (
UINT32 Index;
UINT32 CurrProcessor;
UINT32 BspApicId;
- EFI_CPU_ID_ORDER_MAP *TempVal;
+ EFI_CPU_ID_ORDER_MAP TempVal;
EFI_CPU_ID_ORDER_MAP *CpuIdMapPtr;
UINT32 CoreThreadMask;
EFI_CPU_ID_ORDER_MAP *TempCpuApicIdOrderTable;
@@ -183,7 +183,6 @@ SortCpuLocalApicInTable (
}

TempCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof (EFI_CPU_ID_ORDER_MAP));
- TempVal = AllocateZeroPool (sizeof (EFI_CPU_ID_ORDER_MAP));
CoreThreadMask = (UINT32) ((1 << mNumOfBitShift) - 1);

for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus; CurrProcessor++, Index++) {
--
2.32.0.windows.2


Re: [PATCH v4] MdeModulePkg: Replace with UFS_UNIT_DESC to fix timeout problem

Wu, Hao A
 

Reviewed-by: Hao A Wu <hao.a.wu@...>
Will merge in a couple of days.

Best Regards,
Hao Wu

-----Original Message-----
From: Ke, VincentX <vincentx.ke@...>
Sent: Friday, December 17, 2021 4:10 PM
To: devel@edk2.groups.io
Cc: Ke, VincentX <vincentx.ke@...>; Wu, Hao A <hao.a.wu@...>;
Ni, Ray <ray.ni@...>; Chiu, Ian <Ian.chiu@...>; Chu, Maggie
<maggie.chu@...>
Subject: [PATCH v4] MdeModulePkg: Replace with UFS_UNIT_DESC to fix
timeout problem

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3714

Replace with UFS_UNIT_DESC to fix response timeout problem.

Cc: Hao A Wu <hao.a.wu@...>
Cc: Ray Ni <ray.ni@...>
Cc: Ian Chiu <Ian.chiu@...>
Cc: Maggie Chu <maggie.chu@...>
Signed-off-by: VincentX Ke <vincentx.ke@...>
---
.../Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c | 20 +++++++++----------
1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
index b331c0f3e3..9ad1e19fe0 100644
--- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
+++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
@@ -1037,9 +1037,9 @@ InitializeUfsBlockIoPeim (
UFS_PEIM_HC_PRIVATE_DATA *Private;
EDKII_UFS_HOST_CONTROLLER_PPI *UfsHcPpi;
UINT32 Index;
- UFS_CONFIG_DESC Config;
UINTN MmioBase;
UINT8 Controller;
+ UFS_UNIT_DESC UnitDescriptor;

//
// Shadow this PEIM to run from memory @@ -1126,19 +1126,17 @@
InitializeUfsBlockIoPeim (
}

//
- // Get Ufs Device's Lun Info by reading Configuration Descriptor.
+ // Check if 8 common luns are active and set corresponding bit mask.
//
- Status = UfsRwDeviceDesc (Private, TRUE, UfsConfigDesc, 0, 0, &Config,
sizeof (UFS_CONFIG_DESC));
- if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_ERROR, "Ufs Get Configuration Descriptor Error, Status
= %r\n", Status));
- Controller++;
- continue;
- }
-
for (Index = 0; Index < UFS_PEIM_MAX_LUNS; Index++) {
- if (Config.UnitDescConfParams[Index].LunEn != 0) {
- Private->Luns.BitMask |= (BIT0 << Index);
+ Status = UfsRwDeviceDesc (Private, TRUE, UfsUnitDesc, (UINT8) Index, 0,
&UnitDescriptor, sizeof (UFS_UNIT_DESC));
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Fail to read UFS Unit Descriptor, Index = %X,
Status = %r\n", Index, Status));
+ continue;
+ }
+ if (UnitDescriptor.LunEn == 0x1) {
DEBUG ((DEBUG_INFO, "Ufs %d Lun %d is enabled\n", Controller, Index));
+ Private->Luns.BitMask |= (BIT0 << Index);
}
}

--
2.18.0.windows.1


Event: TianoCore Bug Triage - APAC / NAMO - 12/21/2021 #cal-reminder

devel@edk2.groups.io Calendar <noreply@...>
 

Reminder: TianoCore Bug Triage - APAC / NAMO

When:
12/21/2021
6:30pm to 7:30pm
(UTC-08:00) America/Los Angeles

Where:
https://teams.microsoft.com/l/meetup-join/19%3ameeting_OTUyZTg2NjgtNDhlNS00ODVlLTllYTUtYzg1OTNjNjdiZjFh%40thread.v2/0?context=%7b%22Tid%22%3a%2246c98d88-e344-4ed4-8496-4ed7712e255d%22%2c%22Oid%22%3a%22b286b53a-1218-4db3-bfc9-3d4c5aa7669e%22%7d

Organizer: Liming Gao gaoliming@...

View Event

Description:

TianoCore Bug Triage - APAC / NAMO

Hosted by Liming Gao

 

________________________________________________________________________________

Microsoft Teams meeting

Join on your computer or mobile app

Click here to join the meeting

Join with a video conferencing device

teams@...

Video Conference ID: 116 062 094 0

Alternate VTC dialing instructions

Or call in (audio only)

+1 916-245-6934,,77463821#   United States, Sacramento

Phone Conference ID: 774 638 21#

Find a local number | Reset PIN

Learn More | Meeting options


Re: [PATCH v1 2/2] ArmPkg: MmCommunicationDxe: Update MM communicate input arguments checks

Kun Qin
 

Hi Sami,

Thanks for your review. But this v1 patch was splitted into multiple patches as in https://edk2.groups.io/g/devel/message/85116. Please feel free leave feedback on the new series.

Regards,
Kun

On 12/13/2021 13:03, Sami Mujawar wrote:
Hi Kun,
Thank you for this patch.
These changes look good to me.
Reviewed-by: Sami Mujawar <sami.mujawar@...>
Regards,
Sami Mujawar
On 30/11/2021 12:39 AM, Kun Qin via groups.io wrote:
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3751

Current MM communicate routine from ArmPkg would conduct few steps before
proceeding with SMC calls. However, some inspection steps are different
from PI specification.

This patch updated MM communicate input argument inspection routine to
match the following PI descriptions:
1. Return code `EFI_INVALID_PARAMETER` represents "the `CommBuffer**`
parameters do not refer to the same location in memory".
2. `CommSize` represents "the size of the data buffer being passed in"
instead of "the size of the data being used from data buffer".
3. Regarding `MessageLength`, "if the `MessageLength` is zero, or too
large for the MM implementation to manage, the MM implementation must
update the `MessageLength` to reflect the size of the `Data` buffer that
it can tolerate".

Cc: Leif Lindholm <leif@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Bret Barkelew <Bret.Barkelew@...>
Cc: Michael Kubacki <michael.kubacki@...>

Signed-off-by: Kun Qin <kuqin12@...>
---
  ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 44 ++++++++++++--------
  1 file changed, 27 insertions(+), 17 deletions(-)

diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
index b1e309580988..8a2bd222957f 100644
--- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
+++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
@@ -41,15 +41,19 @@ STATIC EFI_HANDLE  mMmCommunicateHandle;
    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] 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.
+  @param[in] This                     The EFI_MM_COMMUNICATION_PROTOCOL instance.
+  @param[in, out] CommBufferPhysical  Physical address of the MM communication buffer
+  @param[in, out] CommBufferVirtual   Virtual address of the MM communication buffer
+  @param[in, out] CommSize            The size of the data buffer being passed in. On input, when not
+                                      omitted, the buffer should cover EFI_MM_COMMUNICATE_HEADER and the
+                                      value of MessageLength field. 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_INVALID_PARAMETER  CommBufferPhysical or CommBufferVirtual was NULL, or integer value
+                                 pointed by CommSize does not cover EFI_MM_COMMUNICATE_HEADER and the
+                                 value of MessageLength field.
    @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
@@ -82,10 +86,11 @@ MmCommunication2Communicate (
    //
    // Check parameters
    //
-  if (CommBufferVirtual == NULL) {
+  if (CommBufferVirtual == NULL || CommBufferPhysical == NULL) {
      return EFI_INVALID_PARAMETER;
    }
+  Status = EFI_SUCCESS;
    CommunicateHeader = CommBufferVirtual;
    // CommBuffer is a mandatory parameter. Hence, Rely on
    // MessageLength + Header to ascertain the
@@ -95,33 +100,38 @@ MmCommunication2Communicate (
                 sizeof (CommunicateHeader->HeaderGuid) +
                 sizeof (CommunicateHeader->MessageLength);
-  // If the length of the CommBuffer is 0 then return the expected length.
-  if (CommSize != 0) {
+  // If CommSize is not omitted, perform size inspection before proceeding.
+  if (CommSize != NULL) {
      // This case can be used by the consumer of this driver to find out the
      // max size that can be used for allocating CommBuffer.
      if ((*CommSize == 0) ||
          (*CommSize > mNsCommBuffMemRegion.Length)) {
        *CommSize = mNsCommBuffMemRegion.Length;
-      return EFI_BAD_BUFFER_SIZE;
+      Status = EFI_BAD_BUFFER_SIZE;
      }
      //
-    // CommSize must match MessageLength + sizeof (EFI_MM_COMMUNICATE_HEADER);
+    // CommSize should cover at least MessageLength + sizeof (EFI_MM_COMMUNICATE_HEADER);
      //
-    if (*CommSize != BufferSize) {
-        return EFI_INVALID_PARAMETER;
+    if (*CommSize < BufferSize) {
+      Status = EFI_INVALID_PARAMETER;
      }
    }
    //
-  // If the buffer size is 0 or greater than what can be tolerated by the MM
+  // If the message length is 0 or greater than what can be tolerated by the MM
    // environment then return the expected size.
    //
-  if ((BufferSize == 0) ||
+  if ((CommunicateHeader->MessageLength == 0) ||
        (BufferSize > mNsCommBuffMemRegion.Length)) {
      CommunicateHeader->MessageLength = mNsCommBuffMemRegion.Length -
                                         sizeof (CommunicateHeader->HeaderGuid) -
                                         sizeof (CommunicateHeader->MessageLength);
-    return EFI_BAD_BUFFER_SIZE;
+    Status = EFI_BAD_BUFFER_SIZE;
+  }
+
+  // MessageLength or CommSize check has failed, return here.
+  if (EFI_ERROR (Status)) {
+    return Status;
    }
    // SMC Function ID


Re: [PATCH v1 2/2] ArmPkg: MmCommunicationDxe: Update MM communicate input arguments checks

Kun Qin
 

Sure, the updated patches are sent here: https://edk2.groups.io/g/devel/message/85116. Please provide any further feedback. Any input is appreciated.

Regards,
Kun

On 12/15/2021 00:52, Ard Biesheuvel wrote:
On Tue, 30 Nov 2021 at 01:39, Kun Qin <kuqin12@...> wrote:

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3751

Current MM communicate routine from ArmPkg would conduct few steps before
proceeding with SMC calls. However, some inspection steps are different
from PI specification.

This patch updated MM communicate input argument inspection routine to
match the following PI descriptions:
1. Return code `EFI_INVALID_PARAMETER` represents "the `CommBuffer**`
parameters do not refer to the same location in memory".
2. `CommSize` represents "the size of the data buffer being passed in"
instead of "the size of the data being used from data buffer".
3. Regarding `MessageLength`, "if the `MessageLength` is zero, or too
large for the MM implementation to manage, the MM implementation must
update the `MessageLength` to reflect the size of the `Data` buffer that
it can tolerate".
A bullet list like this is usually a strong hint that the patch should
be split up, and this case is no different. Please split this up into
3 separate patches so that us poor reviewers have an easier job.

Cc: Leif Lindholm <leif@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Bret Barkelew <Bret.Barkelew@...>
Cc: Michael Kubacki <michael.kubacki@...>

Signed-off-by: Kun Qin <kuqin12@...>
---
ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 44 ++++++++++++--------
1 file changed, 27 insertions(+), 17 deletions(-)

diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
index b1e309580988..8a2bd222957f 100644
--- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
+++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
@@ -41,15 +41,19 @@ STATIC EFI_HANDLE mMmCommunicateHandle;

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] 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.
+ @param[in] This The EFI_MM_COMMUNICATION_PROTOCOL instance.
+ @param[in, out] CommBufferPhysical Physical address of the MM communication buffer
+ @param[in, out] CommBufferVirtual Virtual address of the MM communication buffer
+ @param[in, out] CommSize The size of the data buffer being passed in. On input, when not
+ omitted, the buffer should cover EFI_MM_COMMUNICATE_HEADER and the
+ value of MessageLength field. 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_INVALID_PARAMETER CommBufferPhysical or CommBufferVirtual was NULL, or integer value
+ pointed by CommSize does not cover EFI_MM_COMMUNICATE_HEADER and the
+ value of MessageLength field.
@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
@@ -82,10 +86,11 @@ MmCommunication2Communicate (
//
// Check parameters
//
- if (CommBufferVirtual == NULL) {
+ if (CommBufferVirtual == NULL || CommBufferPhysical == NULL) {
return EFI_INVALID_PARAMETER;
}

+ Status = EFI_SUCCESS;
CommunicateHeader = CommBufferVirtual;
// CommBuffer is a mandatory parameter. Hence, Rely on
// MessageLength + Header to ascertain the
@@ -95,33 +100,38 @@ MmCommunication2Communicate (
sizeof (CommunicateHeader->HeaderGuid) +
sizeof (CommunicateHeader->MessageLength);

- // If the length of the CommBuffer is 0 then return the expected length.
- if (CommSize != 0) {
+ // If CommSize is not omitted, perform size inspection before proceeding.
+ if (CommSize != NULL) {
// This case can be used by the consumer of this driver to find out the
// max size that can be used for allocating CommBuffer.
if ((*CommSize == 0) ||
(*CommSize > mNsCommBuffMemRegion.Length)) {
*CommSize = mNsCommBuffMemRegion.Length;
- return EFI_BAD_BUFFER_SIZE;
+ Status = EFI_BAD_BUFFER_SIZE;
}
//
- // CommSize must match MessageLength + sizeof (EFI_MM_COMMUNICATE_HEADER);
+ // CommSize should cover at least MessageLength + sizeof (EFI_MM_COMMUNICATE_HEADER);
//
- if (*CommSize != BufferSize) {
- return EFI_INVALID_PARAMETER;
+ if (*CommSize < BufferSize) {
+ Status = EFI_INVALID_PARAMETER;
}
}

//
- // If the buffer size is 0 or greater than what can be tolerated by the MM
+ // If the message length is 0 or greater than what can be tolerated by the MM
// environment then return the expected size.
//
- if ((BufferSize == 0) ||
+ if ((CommunicateHeader->MessageLength == 0) ||
(BufferSize > mNsCommBuffMemRegion.Length)) {
CommunicateHeader->MessageLength = mNsCommBuffMemRegion.Length -
sizeof (CommunicateHeader->HeaderGuid) -
sizeof (CommunicateHeader->MessageLength);
- return EFI_BAD_BUFFER_SIZE;
+ Status = EFI_BAD_BUFFER_SIZE;
+ }
+
+ // MessageLength or CommSize check has failed, return here.
+ if (EFI_ERROR (Status)) {
+ return Status;
}

// SMC Function ID
--
2.32.0.windows.1


[PATCH v2 6/6] ArmPkg: MmCommunicationDxe: Update MM communicate `MessageLength` check

Kun Qin
 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3751

Current MM communicate routine from ArmPkg would conduct few checks prior
to proceeding with SMC calls. However, the inspection step is different
from PI specification.

This patch updated MM communicate input argument inspection routine to
assure that "if the `MessageLength` is zero, or too large for the MM
implementation to manage, the MM implementation must update the
`MessageLength` to reflect the size of the `Data` buffer that it can
tolerate", as described by `EFI_MM_COMMUNICATION_PROTOCOL.Communicate()`
section in PI specification.

Cc: Leif Lindholm <leif@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Bret Barkelew <Bret.Barkelew@...>
Cc: Michael Kubacki <michael.kubacki@...>

Signed-off-by: Kun Qin <kuqin12@...>
---

Notes:
v2:
- Splitting patch into 4 of 4 [Ard]
- Uncrustify style update

ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
index 2f89b7c5b6c4..85d9034555f0 100644
--- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
+++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
@@ -92,6 +92,7 @@ MmCommunication2Communicate (
return EFI_INVALID_PARAMETER;
}

+ Status = EFI_SUCCESS;
CommunicateHeader = CommBufferVirtual;
// CommBuffer is a mandatory parameter. Hence, Rely on
// MessageLength + Header to ascertain the
@@ -109,28 +110,33 @@ MmCommunication2Communicate (
(*CommSize > mNsCommBuffMemRegion.Length))
{
*CommSize = mNsCommBuffMemRegion.Length;
- return EFI_BAD_BUFFER_SIZE;
+ Status = EFI_BAD_BUFFER_SIZE;
}

//
// CommSize should cover at least MessageLength + sizeof (EFI_MM_COMMUNICATE_HEADER);
//
if (*CommSize < BufferSize) {
- return EFI_INVALID_PARAMETER;
+ Status = EFI_INVALID_PARAMETER;
}
}

//
- // If the buffer size is 0 or greater than what can be tolerated by the MM
+ // If the message length is 0 or greater than what can be tolerated by the MM
// environment then return the expected size.
//
- if ((BufferSize == 0) ||
+ if ((CommunicateHeader->MessageLength == 0) ||
(BufferSize > mNsCommBuffMemRegion.Length))
{
CommunicateHeader->MessageLength = mNsCommBuffMemRegion.Length -
sizeof (CommunicateHeader->HeaderGuid) -
sizeof (CommunicateHeader->MessageLength);
- return EFI_BAD_BUFFER_SIZE;
+ Status = EFI_BAD_BUFFER_SIZE;
+ }
+
+ // MessageLength or CommSize check has failed, return here.
+ if (EFI_ERROR (Status)) {
+ return Status;
}

// SMC Function ID
--
2.32.0.windows.1


[PATCH v2 5/6] ArmPkg: MmCommunicationDxe: Update MM communicate `CommSize` check

Kun Qin
 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3751

Current MM communicate routine from ArmPkg would conduct few checks prior
to proceeding with SMC calls. However, the inspection step is different
from PI specification.

This patch updated MM communicate input argument inspection routine to
assure `CommSize` represents "the size of the data buffer being passed
in" instead of the size of the data being used from data buffer, as
described by section `EFI_MM_COMMUNICATION2_PROTOCOL.Communicate()` in PI
specification.

Cc: Leif Lindholm <leif@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Bret Barkelew <Bret.Barkelew@...>
Cc: Michael Kubacki <michael.kubacki@...>

Signed-off-by: Kun Qin <kuqin12@...>
---

Notes:
v2:
- Splitting patch into 3 of 4 [Ard]

ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
index 0283be430dff..2f89b7c5b6c4 100644
--- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
+++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
@@ -44,13 +44,18 @@ STATIC EFI_HANDLE mMmCommunicateHandle;
@param[in] This The EFI_MM_COMMUNICATION_PROTOCOL instance.
@param[in, out] CommBufferPhysical Physical address of the MM communication buffer
@param[in, out] CommBufferVirtual Virtual address of the MM communication buffer
- @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
+ @param[in, out] CommSize The size of the data buffer being passed in. On input,
+ when not omitted, the buffer should cover EFI_MM_COMMUNICATE_HEADER
+ and the value of MessageLength field. 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_INVALID_PARAMETER CommBufferPhysical or CommBufferVirtual was NULL, or
+ integer value pointed by CommSize does not cover
+ EFI_MM_COMMUNICATE_HEADER and the value of MessageLength
+ field.
@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
@@ -96,8 +101,8 @@ MmCommunication2Communicate (
sizeof (CommunicateHeader->HeaderGuid) +
sizeof (CommunicateHeader->MessageLength);

- // If the length of the CommBuffer is 0 then return the expected length.
- if (CommSize != 0) {
+ // If CommSize is not omitted, perform size inspection before proceeding.
+ if (CommSize != NULL) {
// This case can be used by the consumer of this driver to find out the
// max size that can be used for allocating CommBuffer.
if ((*CommSize == 0) ||
@@ -108,9 +113,9 @@ MmCommunication2Communicate (
}

//
- // CommSize must match MessageLength + sizeof (EFI_MM_COMMUNICATE_HEADER);
+ // CommSize should cover at least MessageLength + sizeof (EFI_MM_COMMUNICATE_HEADER);
//
- if (*CommSize != BufferSize) {
+ if (*CommSize < BufferSize) {
return EFI_INVALID_PARAMETER;
}
}
--
2.32.0.windows.1


[PATCH v2 4/6] ArmPkg: MmCommunicationDxe: Update MM communicate `CommBuffer**` checks

Kun Qin
 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3751

Current MM communicate routine from ArmPkg would conduct few checks prior
to proceeding with SMC calls. However, the inspection step is different
from PI specification.

This patch updated MM communicate input argument inspection routine to
assure that return code `EFI_INVALID_PARAMETER` represents "the
`CommBuffer**` parameters do not refer to the same location in memory",
as described by `EFI_MM_COMMUNICATION2_PROTOCOL.Communicate()` section
in PI specification.

Cc: Leif Lindholm <leif@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Bret Barkelew <Bret.Barkelew@...>
Cc: Michael Kubacki <michael.kubacki@...>

Signed-off-by: Kun Qin <kuqin12@...>
---

Notes:
v2:
- Splitting patch into 2 of 4 [Ard]
- Uncrustify style update

ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
index 7f756a32d4e0..0283be430dff 100644
--- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
+++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
@@ -83,7 +83,7 @@ MmCommunication2Communicate (
//
// Check parameters
//
- if (CommBufferVirtual == NULL) {
+ if ((CommBufferVirtual == NULL) || (CommBufferPhysical == NULL)) {
return EFI_INVALID_PARAMETER;
}

--
2.32.0.windows.1


[PATCH v2 3/6] ArmPkg: MmCommunicationDxe: MM communicate function argument attributes

Kun Qin
 

Current MM communicate2 function from ArmPkg described input arguments
`CommBufferPhysical`, `CommBufferVirtual` and `CommSize` as input only,
which mismatches with the "input and output type" as in PI specification.

This change updated function descriptions of MM communite2 to match input
argument types.

Cc: Leif Lindholm <leif@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Bret Barkelew <Bret.Barkelew@...>
Cc: Michael Kubacki <michael.kubacki@...>

Signed-off-by: Kun Qin <kuqin12@...>
---

Notes:
v2:
- Splitting patch into 1 of 4 [Ard]

ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
index 7c8284104d87..7f756a32d4e0 100644
--- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
+++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
@@ -41,12 +41,13 @@ STATIC EFI_HANDLE mMmCommunicateHandle;

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] 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.
+ @param[in] This The EFI_MM_COMMUNICATION_PROTOCOL instance.
+ @param[in, out] CommBufferPhysical Physical address of the MM communication buffer
+ @param[in, out] CommBufferVirtual Virtual address of the MM communication buffer
+ @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. 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.
--
2.32.0.windows.1


[PATCH v2 2/6] MdePkg: MmCommunication2: Update MM communicate2 function description

Kun Qin
 

Current MM communicate2 function definition described input arguments
`CommBufferPhysical`, `CommBufferVirtual` and `CommSize` as input only,
which mismatches with the "input and output type" as in PI specification.

This change updated function descriptions of MM communite2 definition to
match input argument types.

Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>

Signed-off-by: Kun Qin <kuqin12@...>
---

Notes:
v2:
- Newly added

MdePkg/Include/Protocol/MmCommunication2.h | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/MdePkg/Include/Protocol/MmCommunication2.h b/MdePkg/Include/Protocol/MmCommunication2.h
index 3495a7327f76..1b56320c7fff 100644
--- a/MdePkg/Include/Protocol/MmCommunication2.h
+++ b/MdePkg/Include/Protocol/MmCommunication2.h
@@ -27,12 +27,13 @@ typedef struct _EFI_MM_COMMUNICATION2_PROTOCOL EFI_MM_COMMUNICATION2_PROTOCOL;

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] 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.
+ @param[in] This The EFI_MM_COMMUNICATION_PROTOCOL instance.
+ @param[in, out] CommBufferPhysical Physical address of the MM communication buffer
+ @param[in, out] CommBufferVirtual Virtual address of the MM communication buffer
+ @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. 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.
--
2.32.0.windows.1


[PATCH v2 1/6] MdeModulePkg: VariableSmmRuntimeDxe: Fix Variable Policy Message Length

Kun Qin
 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3709

In EDKII implementation of variable policy, the DXE runtime agent would
communicate to MM to disable, register or query policies. However, these
operations populate the value of MessageLength that includes communicate
header to include MM communicate header, which mismatches with the
description of PI specification.

This fix will correct the MessageLength field calculation to exclude
the size of MM_COMMUNICATE_HEADER.

Cc: Jian J Wang <jian.j.wang@...>
Cc: Liming Gao <gaoliming@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Bret Barkelew <Bret.Barkelew@...>
Cc: Michael Kubacki <michael.kubacki@...>

Signed-off-by: Kun Qin <kuqin12@...>
---

Notes:
v2:
- No review, no updates

MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c
index 672a2293bcb1..b2094fbcd6ea 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c
@@ -89,7 +89,7 @@ ProtocolDisableVariablePolicy (
CommHeader = mMmCommunicationBuffer;
PolicyHeader = (VAR_CHECK_POLICY_COMM_HEADER *)&CommHeader->Data;
CopyGuid (&CommHeader->HeaderGuid, &gVarCheckPolicyLibMmiHandlerGuid);
- CommHeader->MessageLength = BufferSize;
+ CommHeader->MessageLength = BufferSize - OFFSET_OF (EFI_MM_COMMUNICATE_HEADER, Data);
PolicyHeader->Signature = VAR_CHECK_POLICY_COMM_SIG;
PolicyHeader->Revision = VAR_CHECK_POLICY_COMM_REVISION;
PolicyHeader->Command = VAR_CHECK_POLICY_COMMAND_DISABLE;
@@ -138,7 +138,7 @@ ProtocolIsVariablePolicyEnabled (
PolicyHeader = (VAR_CHECK_POLICY_COMM_HEADER *)&CommHeader->Data;
CommandParams = (VAR_CHECK_POLICY_COMM_IS_ENABLED_PARAMS *)(PolicyHeader + 1);
CopyGuid (&CommHeader->HeaderGuid, &gVarCheckPolicyLibMmiHandlerGuid);
- CommHeader->MessageLength = BufferSize;
+ CommHeader->MessageLength = BufferSize - OFFSET_OF (EFI_MM_COMMUNICATE_HEADER, Data);
PolicyHeader->Signature = VAR_CHECK_POLICY_COMM_SIG;
PolicyHeader->Revision = VAR_CHECK_POLICY_COMM_REVISION;
PolicyHeader->Command = VAR_CHECK_POLICY_COMMAND_IS_ENABLED;
@@ -213,7 +213,7 @@ ProtocolRegisterVariablePolicy (
PolicyHeader = (VAR_CHECK_POLICY_COMM_HEADER *)&CommHeader->Data;
PolicyBuffer = (VOID *)(PolicyHeader + 1);
CopyGuid (&CommHeader->HeaderGuid, &gVarCheckPolicyLibMmiHandlerGuid);
- CommHeader->MessageLength = BufferSize;
+ CommHeader->MessageLength = BufferSize - OFFSET_OF (EFI_MM_COMMUNICATE_HEADER, Data);
PolicyHeader->Signature = VAR_CHECK_POLICY_COMM_SIG;
PolicyHeader->Revision = VAR_CHECK_POLICY_COMM_REVISION;
PolicyHeader->Command = VAR_CHECK_POLICY_COMMAND_REGISTER;
@@ -270,7 +270,7 @@ DumpVariablePolicyHelper (
PolicyHeader = (VAR_CHECK_POLICY_COMM_HEADER *)&CommHeader->Data;
CommandParams = (VAR_CHECK_POLICY_COMM_DUMP_PARAMS *)(PolicyHeader + 1);
CopyGuid (&CommHeader->HeaderGuid, &gVarCheckPolicyLibMmiHandlerGuid);
- CommHeader->MessageLength = BufferSize;
+ CommHeader->MessageLength = BufferSize - OFFSET_OF (EFI_MM_COMMUNICATE_HEADER, Data);
PolicyHeader->Signature = VAR_CHECK_POLICY_COMM_SIG;
PolicyHeader->Revision = VAR_CHECK_POLICY_COMM_REVISION;
PolicyHeader->Command = VAR_CHECK_POLICY_COMMAND_DUMP;
@@ -397,7 +397,7 @@ ProtocolLockVariablePolicy (
CommHeader = mMmCommunicationBuffer;
PolicyHeader = (VAR_CHECK_POLICY_COMM_HEADER *)&CommHeader->Data;
CopyGuid (&CommHeader->HeaderGuid, &gVarCheckPolicyLibMmiHandlerGuid);
- CommHeader->MessageLength = BufferSize;
+ CommHeader->MessageLength = BufferSize - OFFSET_OF (EFI_MM_COMMUNICATE_HEADER, Data);
PolicyHeader->Signature = VAR_CHECK_POLICY_COMM_SIG;
PolicyHeader->Revision = VAR_CHECK_POLICY_COMM_REVISION;
PolicyHeader->Command = VAR_CHECK_POLICY_COMMAND_LOCK;
--
2.32.0.windows.1


[PATCH v2 0/6] MM communicate functionality in variable policy

Kun Qin
 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3709
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3751

This patch series is a follow up of previous submission:
https://edk2.groups.io/g/devel/message/84140

v2 patches mainly focus on feedback for commits submitted in v1 patches:
a. Splitted the original ArmPkg patch into 4 separate patches;
b. Updated patches according to Uncrustify scanning results;

Patch v2 branch: https://github.com/kuqin12/edk2/tree/mm_communicate_check_v2

Cc: Jian J Wang <jian.j.wang@...>
Cc: Liming Gao <gaoliming@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Cc: Leif Lindholm <leif@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Bret Barkelew <Bret.Barkelew@...>
Cc: Michael Kubacki <michael.kubacki@...>

Kun Qin (6):
MdeModulePkg: VariableSmmRuntimeDxe: Fix Variable Policy Message
Length
MdePkg: MmCommunication2: Update MM communicate2 function description
ArmPkg: MmCommunicationDxe: MM communicate function argument
attributes
ArmPkg: MmCommunicationDxe: Update MM communicate `CommBuffer**`
checks
ArmPkg: MmCommunicationDxe: Update MM communicate `CommSize` check
ArmPkg: MmCommunicationDxe: Update MM communicate `MessageLength`
check

ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 46 ++++++++++++--------
MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c | 10 ++---
MdePkg/Include/Protocol/MmCommunication2.h | 13 +++---
3 files changed, 41 insertions(+), 28 deletions(-)

--
2.32.0.windows.1


Re: [PATCH] IntelFsp2WrapperPkg : Remove EFIAPI from local functions.

Zeng, Star
 

Reviewed-by: Star Zeng <star.zeng@...>

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chiu, Chasel
Sent: Tuesday, December 21, 2021 8:40 AM
To: devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Zeng, Star <star.zeng@...>; Ni, Ray <ray.ni@...>; S, Ashraf Ali <ashraf.ali.s@...>
Subject: [edk2-devel] [PATCH] IntelFsp2WrapperPkg : Remove EFIAPI from local functions.

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3642

Local functions do not need EFIAPI.

Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Star Zeng <star.zeng@...>
Cc: Ray Ni <ray.ni@...>
Cc: Ashraf Ali S <ashraf.ali.s@...>
Signed-off-by: Chasel Chiu <chasel.chiu@...>
---
IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c | 1 -
IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c | 1 -
2 files changed, 2 deletions(-)

diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
index 49fbb27eca..b0c6b2f8a6 100644
--- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
+++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
@@ -45,7 +45,6 @@ extern EFI_GUID gFspHobGuid;
**/



UINTN

-EFIAPI

GetFspmUpdDataAddress (

VOID

)

diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
index ddee9cd029..fadadd40e6 100644
--- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
+++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
@@ -188,7 +188,6 @@ FspSiliconInitDoneGetFspHobList (
**/



UINTN

-EFIAPI

GetFspsUpdDataAddress (

VOID

)

--
2.28.0.windows.1



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#85113): https://edk2.groups.io/g/devel/message/85113
Mute This Topic: https://groups.io/mt/87869015/1779220
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [star.zeng@...]
-=-=-=-=-=-=


Re: [PATCH v8] IntelFsp2WrapperPkg : FSPM/S UPD data address based on Build Type

Chiu, Chasel
 

Thanks Ray! I have sent a code review for removing EFIAPI, please help to review.

Thanks,
Chasel

-----Original Message-----
From: Ni, Ray <ray.ni@...>
Sent: Monday, December 20, 2021 1:15 PM
To: S, Ashraf Ali <ashraf.ali.s@...>; devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@...>; Desimone, Nathaniel L
<nathaniel.l.desimone@...>; Zeng, Star <star.zeng@...>; Kuo, Ted
<ted.kuo@...>; Duggapu, Chinni B <chinni.b.duggapu@...>;
Chaganty, Rangasai V <rangasai.v.chaganty@...>; Solanki, Digant H
<digant.h.solanki@...>; V, Sangeetha <sangeetha.v@...>
Subject: RE: [PATCH v8] IntelFsp2WrapperPkg : FSPM/S UPD data address based
on Build Type


+UINTN
+EFIAPI
+GetFspmUpdDataAddress (
+ VOID
+ )

This is internal function. Please remove "EFIAPI".


[PATCH] IntelFsp2WrapperPkg : Remove EFIAPI from local functions.

Chiu, Chasel
 

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D3642

Local functions do not need EFIAPI.

Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Star Zeng <star.zeng@...>
Cc: Ray Ni <ray.ni@...>
Cc: Ashraf Ali S <ashraf.ali.s@...>
Signed-off-by: Chasel Chiu <chasel.chiu@...>
---
IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c | 1 -
IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c | 1 -
2 files changed, 2 deletions(-)

diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c b/IntelF=
sp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
index 49fbb27eca..b0c6b2f8a6 100644
--- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
+++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
@@ -45,7 +45,6 @@ extern EFI_GUID gFspHobGuid;
**/=0D
=0D
UINTN=0D
-EFIAPI=0D
GetFspmUpdDataAddress (=0D
VOID=0D
)=0D
diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c b/IntelF=
sp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
index ddee9cd029..fadadd40e6 100644
--- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
+++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
@@ -188,7 +188,6 @@ FspSiliconInitDoneGetFspHobList (
**/=0D
=0D
UINTN=0D
-EFIAPI=0D
GetFspsUpdDataAddress (=0D
VOID=0D
)=0D
--=20
2.28.0.windows.1


Re: [PATCH] BaseTools/Brotli: update to latest brotli submodule

Pedro Falcato
 

Ross,

This is a duplicate of https://edk2.groups.io/g/devel/message/84497, which has already been Rb'd and is just waiting for a maintainer to merge it.

Best regards,
Pedro

On Mon, Dec 20, 2021 at 4:55 PM Ross Burton <ross@...> wrote:
Update to the latest Brotli commit, and extend the makefiles as needed.

Specifically, this is needed for 0a3944 in brotli, to fix VLA parameter
warnings with new compilers.

Signed-off-by: Ross Burton <ross.burton@...>
Change-Id: I36d28cadb9bf2d9e01ac0341e69509fa1b8d6969
---
 BaseTools/Source/C/BrotliCompress/GNUmakefile |  7 +++++++
 BaseTools/Source/C/BrotliCompress/Makefile    | 11 ++++++++++-
 BaseTools/Source/C/BrotliCompress/brotli      |  2 +-
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/C/BrotliCompress/GNUmakefile b/BaseTools/Source/C/BrotliCompress/GNUmakefile
index b150e5dd2b..1a946140a3 100644
--- a/BaseTools/Source/C/BrotliCompress/GNUmakefile
+++ b/BaseTools/Source/C/BrotliCompress/GNUmakefile
@@ -10,8 +10,12 @@ APPNAME = BrotliCompress

 OBJECTS = \
   BrotliCompress.o \
+  brotli/c/common/constants.o \
+  brotli/c/common/context.o \
   brotli/c/common/dictionary.o \
   brotli/c/common/transform.o \
+  brotli/c/common/platform.o \
+  brotli/c/common/shared_dictionary.o \
   brotli/c/dec/bit_reader.o \
   brotli/c/dec/decode.o \
   brotli/c/dec/huffman.o \
@@ -22,12 +26,15 @@ OBJECTS = \
   brotli/c/enc/block_splitter.o \
   brotli/c/enc/brotli_bit_stream.o \
   brotli/c/enc/cluster.o \
+  brotli/c/enc/compound_dictionary.o \
   brotli/c/enc/compress_fragment.o \
   brotli/c/enc/compress_fragment_two_pass.o \
+  brotli/c/enc/command.o \
   brotli/c/enc/dictionary_hash.o \
   brotli/c/enc/encode.o \
   brotli/c/enc/encoder_dict.o \
   brotli/c/enc/entropy_encode.o \
+  brotli/c/enc/fast_log.o \
   brotli/c/enc/histogram.o \
   brotli/c/enc/literal_cost.o \
   brotli/c/enc/memory.o \
diff --git a/BaseTools/Source/C/BrotliCompress/Makefile b/BaseTools/Source/C/BrotliCompress/Makefile
index 038d1ec242..918497dc7b 100644
--- a/BaseTools/Source/C/BrotliCompress/Makefile
+++ b/BaseTools/Source/C/BrotliCompress/Makefile
@@ -13,7 +13,13 @@ APPNAME = BrotliCompress

 #LIBS = $(LIB_PATH)\Common.lib

-COMMON_OBJ = brotli\c\common\dictionary.obj brotli\c\common\transform.obj
+COMMON_OBJ = \
+  brotli\c\common\constants.obj \
+  brotli\c\common\context.obj \
+  brotli\c\common\dictionary.obj \
+  brotli\c\common\transform.obj \
+  brotli\c\common\platform.obj \
+  brotli\c\common\shared_dictionary.obj
 DEC_OBJ = \
   brotli\c\dec\bit_reader.obj \
   brotli\c\dec\decode.obj \
@@ -26,12 +32,15 @@ ENC_OBJ = \
   brotli\c\enc\block_splitter.obj \
   brotli\c\enc\brotli_bit_stream.obj \
   brotli\c\enc\cluster.obj \
+  brotli\c\enc\compound_dictionary.obj \
   brotli\c\enc\compress_fragment.obj \
   brotli\c\enc\compress_fragment_two_pass.obj \
+  brotli\c\enc\command.obj \
   brotli\c\enc\dictionary_hash.obj \
   brotli\c\enc\encode.obj \
   brotli\c\enc\encoder_dict.obj \
   brotli\c\enc\entropy_encode.obj \
+  brotli\c\enc\fast_log.obj \
   brotli\c\enc\histogram.obj \
   brotli\c\enc\literal_cost.obj \
   brotli\c\enc\memory.obj \
diff --git a/BaseTools/Source/C/BrotliCompress/brotli b/BaseTools/Source/C/BrotliCompress/brotli
index 666c3280cc..e83c7b8e8f 160000
--- a/BaseTools/Source/C/BrotliCompress/brotli
+++ b/BaseTools/Source/C/BrotliCompress/brotli
@@ -1 +1 @@
-Subproject commit 666c3280cc11dc433c303d79a83d4ffbdd12cc8d
+Subproject commit e83c7b8e8fb8b696a1df6866bc46cbb76d7e0348
--
2.25.1



------------
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#85111): https://edk2.groups.io/g/devel/message/85111
Mute This Topic: https://groups.io/mt/87860390/5946980
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [pedro.falcato@...]
------------




--
Pedro Falcato


[PATCH] BaseTools/Brotli: update to latest brotli submodule

Ross Burton <ross@...>
 

Update to the latest Brotli commit, and extend the makefiles as needed.

Specifically, this is needed for 0a3944 in brotli, to fix VLA parameter
warnings with new compilers.

Signed-off-by: Ross Burton <ross.burton@...>
Change-Id: I36d28cadb9bf2d9e01ac0341e69509fa1b8d6969
---
BaseTools/Source/C/BrotliCompress/GNUmakefile | 7 +++++++
BaseTools/Source/C/BrotliCompress/Makefile | 11 ++++++++++-
BaseTools/Source/C/BrotliCompress/brotli | 2 +-
3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/C/BrotliCompress/GNUmakefile b/BaseTools/Sour=
ce/C/BrotliCompress/GNUmakefile
index b150e5dd2b..1a946140a3 100644
--- a/BaseTools/Source/C/BrotliCompress/GNUmakefile
+++ b/BaseTools/Source/C/BrotliCompress/GNUmakefile
@@ -10,8 +10,12 @@ APPNAME =3D BrotliCompress
=0D
OBJECTS =3D \=0D
BrotliCompress.o \=0D
+ brotli/c/common/constants.o \=0D
+ brotli/c/common/context.o \=0D
brotli/c/common/dictionary.o \=0D
brotli/c/common/transform.o \=0D
+ brotli/c/common/platform.o \=0D
+ brotli/c/common/shared_dictionary.o \=0D
brotli/c/dec/bit_reader.o \=0D
brotli/c/dec/decode.o \=0D
brotli/c/dec/huffman.o \=0D
@@ -22,12 +26,15 @@ OBJECTS =3D \
brotli/c/enc/block_splitter.o \=0D
brotli/c/enc/brotli_bit_stream.o \=0D
brotli/c/enc/cluster.o \=0D
+ brotli/c/enc/compound_dictionary.o \=0D
brotli/c/enc/compress_fragment.o \=0D
brotli/c/enc/compress_fragment_two_pass.o \=0D
+ brotli/c/enc/command.o \=0D
brotli/c/enc/dictionary_hash.o \=0D
brotli/c/enc/encode.o \=0D
brotli/c/enc/encoder_dict.o \=0D
brotli/c/enc/entropy_encode.o \=0D
+ brotli/c/enc/fast_log.o \=0D
brotli/c/enc/histogram.o \=0D
brotli/c/enc/literal_cost.o \=0D
brotli/c/enc/memory.o \=0D
diff --git a/BaseTools/Source/C/BrotliCompress/Makefile b/BaseTools/Source/=
C/BrotliCompress/Makefile
index 038d1ec242..918497dc7b 100644
--- a/BaseTools/Source/C/BrotliCompress/Makefile
+++ b/BaseTools/Source/C/BrotliCompress/Makefile
@@ -13,7 +13,13 @@ APPNAME =3D BrotliCompress
=0D
#LIBS =3D $(LIB_PATH)\Common.lib=0D
=0D
-COMMON_OBJ =3D brotli\c\common\dictionary.obj brotli\c\common\transform.ob=
j=0D
+COMMON_OBJ =3D \=0D
+ brotli\c\common\constants.obj \=0D
+ brotli\c\common\context.obj \=0D
+ brotli\c\common\dictionary.obj \=0D
+ brotli\c\common\transform.obj \=0D
+ brotli\c\common\platform.obj \=0D
+ brotli\c\common\shared_dictionary.obj=0D
DEC_OBJ =3D \=0D
brotli\c\dec\bit_reader.obj \=0D
brotli\c\dec\decode.obj \=0D
@@ -26,12 +32,15 @@ ENC_OBJ =3D \
brotli\c\enc\block_splitter.obj \=0D
brotli\c\enc\brotli_bit_stream.obj \=0D
brotli\c\enc\cluster.obj \=0D
+ brotli\c\enc\compound_dictionary.obj \=0D
brotli\c\enc\compress_fragment.obj \=0D
brotli\c\enc\compress_fragment_two_pass.obj \=0D
+ brotli\c\enc\command.obj \=0D
brotli\c\enc\dictionary_hash.obj \=0D
brotli\c\enc\encode.obj \=0D
brotli\c\enc\encoder_dict.obj \=0D
brotli\c\enc\entropy_encode.obj \=0D
+ brotli\c\enc\fast_log.obj \=0D
brotli\c\enc\histogram.obj \=0D
brotli\c\enc\literal_cost.obj \=0D
brotli\c\enc\memory.obj \=0D
diff --git a/BaseTools/Source/C/BrotliCompress/brotli b/BaseTools/Source/C/=
BrotliCompress/brotli
index 666c3280cc..e83c7b8e8f 160000
--- a/BaseTools/Source/C/BrotliCompress/brotli
+++ b/BaseTools/Source/C/BrotliCompress/brotli
@@ -1 +1 @@
-Subproject commit 666c3280cc11dc433c303d79a83d4ffbdd12cc8d
+Subproject commit e83c7b8e8fb8b696a1df6866bc46cbb76d7e0348
--=20
2.25.1


Re: EDK build error

Andrew Fish
 

Jacek,

Since you are not subscribed to the mailing list this mail got sent to moderation….

Sometimes syntax errors in your build config files can crash the tools. This looks like a PCD value in a DSC might not be formatted correctly? If you have modified something in that area you can look into that. 

Thanks,

Andrew Fish

On Dec 18, 2021, at 12:57 PM, Kolakowski, Jacek <Jacek.Kolakowski@...> wrote:

Hi
I got the below build error. Can anybody help resolve it?
 
Thanks,
Jacek 
 
(Please send email to devel@edk2.groups.io for help, attaching following call stack trace!)
 
  (Python 3.8.6 on win32) Traceback (most recent call last):
  File "C:\source\7nmGnr2\edk2\BaseTools\Source\Python\build\build.py", line 2695, in Main
  MyBuild.Launch()
  File "C:\source\7nmGnr2\edk2\BaseTools\Source\Python\build\build.py", line 2490, in Launch
  self._MultiThreadBuildPlatform()
  File "C:\source\7nmGnr2\edk2\BaseTools\Source\Python\build\build.py", line 2282, in _MultiThreadBuildPlatform
  Wa, self.BuildModules = self.PerformAutoGen(BuildTarget,ToolChain)
  File "C:\source\7nmGnr2\edk2\BaseTools\Source\Python\build\build.py", line 2133, in PerformAutoGen
  Wa = WorkspaceAutoGen(
  File "C:\source\7nmGnr2\edk2\BaseTools\Source\Python\AutoGen\WorkspaceAutoGen.py", line 43, in __init__
  self._InitWorker(Workspace, MetaFile, Target, Toolchain, Arch, *args, **kwargs)
  File "C:\source\7nmGnr2\edk2\BaseTools\Source\Python\AutoGen\WorkspaceAutoGen.py", line 117, in _InitWorker
  self.ProcessPcdType()
  File "C:\source\7nmGnr2\edk2\BaseTools\Source\Python\AutoGen\WorkspaceAutoGen.py", line 264, in ProcessPcdType
  Platform.Pcds
  File "C:\source\7nmGnr2\edk2\BaseTools\Source\Python\Workspace\DscBuildData.py", line 1218, in Pcds
  self._Pcds = self.UpdateStructuredPcds(MODEL_PCD_TYPE_LIST, self._Pcds)
  File "C:\source\7nmGnr2\edk2\BaseTools\Source\Python\Workspace\DscBuildData.py", line 1600, in UpdateStructuredPcds
  Str_Pcd_Values = self.GenerateByteArrayValue(S_pcd_set)
  File "C:\source\7nmGnr2\edk2\BaseTools\Source\Python\Workspace\DscBuildData.py", line 3036, in GenerateByteArrayValue
  EdkLogger.warn('Build', COMMAND_FAILURE, 'Can not collect output from command: %s\n%s\n' % (Command, StdOut, StdErr))
  TypeError: not all arguments converted during string formatting
 
 
  - Failed -
  Build end time: 18:29:42, Dec.18 2021
  Build total time: 00:00:13


Intel Technology Poland sp. z o.o.
ul. Słowackiego 173 | 80-298 Gdańsk | Sąd Rejonowy Gdańsk Północ | VII Wydział Gospodarczy Krajowego Rejestru Sądowego - KRS 101882 | NIP 957-07-52-316 | Kapitał zakładowy 200.000 PLN.

Ta wiadomość wraz z załącznikami jest przeznaczona dla określonego adresata i może zawierać informacje poufne. W razie przypadkowego otrzymania tej wiadomości, prosimy o powiadomienie nadawcy oraz trwałe jej usunięcie; jakiekolwiek przeglądanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited.


11601 - 11620 of 96644