From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.byosoft.com.cn (mail.byosoft.com.cn [58.240.74.242]) by mx.groups.io with SMTP id smtpd.web09.8515.1616737112900627631 for ; Thu, 25 Mar 2021 22:38:35 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=none, err=permanent DNS error (domain: byosoft.com.cn, ip: 58.240.74.242, mailfrom: gaojie@byosoft.com.cn) Received: from [192.168.0.33] ([223.166.104.131]) (envelope-sender ) by 192.168.6.13 with ESMTP for ; Fri, 26 Mar 2021 13:38:26 +0800 X-WM-Sender: gaojie@byosoft.com.cn X-Originating-IP: 223.166.104.131 X-WM-AuthFlag: YES X-WM-AuthUser: gaojie@byosoft.com.cn Subject: Re: [edk2-devel] [edk2-test][Patch 1/1] ImageIndex need be corrected to range 1 ~ DescriptorCount. To: devel@edk2.groups.io, eric.jin@intel.com Cc: G Edhaya Chandran , Samer El-Haj-Mahmoud , Arvin Chen References: <20210322061254.6272-1-eric.jin@intel.com> From: "Gao Jie" Message-ID: <70489bf6-af18-ddc8-7d5e-67b50d61b808@byosoft.com.cn> Date: Fri, 26 Mar 2021 13:38:26 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 MIME-Version: 1.0 In-Reply-To: <20210322061254.6272-1-eric.jin@intel.com> Content-Type: multipart/alternative; boundary="------------12767D6F9E88211C8A16A7AF" Content-Language: en-US --------------12767D6F9E88211C8A16A7AF Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Reviewed-by: Barton Gao > On 2021/3/22 14:12, Eric Jin wrote: > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3272 > > The goal of the Support Routine: CheckForSupportGetImage is to detect the > current FMP instance's capability for supporting GetImage(). > > In current code, "ImageIndex = 0" is the input parameter, but it is > inconsistent with spec -"A unique number identifying the firmware image(s) > within the device. The number is between 1 and DescriptorCount." > > It is a bug and need the fix. > The similar errors exist in CheckForSupportSetImage/CheckForSupportCheckImage. > > Cc: G Edhaya Chandran > Cc: Barton Gao > Cc: Samer El-Haj-Mahmoud > Cc: Arvin Chen > Signed-off-by: Eric Jin > --- > .../BlackBoxTest/FirmwareManagementBBTestConformance.c | 7 ++++--- > .../BlackBoxTest/FirmwareManagementBBTestConformance.c | 7 ++++--- > 2 files changed, 8 insertions(+), 6 deletions(-) > > diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/FirmwareManagement/BlackBoxTest/FirmwareManagementBBTestConformance.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/FirmwareManagement/BlackBoxTest/FirmwareManagementBBTestConformance.c > index 257f2756dbcb..44d01d27a8f2 100644 > --- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/FirmwareManagement/BlackBoxTest/FirmwareManagementBBTestConformance.c > +++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/FirmwareManagement/BlackBoxTest/FirmwareManagementBBTestConformance.c > @@ -2,6 +2,7 @@ > > Copyright 2006 - 2016 Unified EFI, Inc.
> Copyright (c) 2010 - 2016, Dell Inc. All rights reserved.
> + Copyright (c) 2021, Intel Corporation. All rights reserved.
> > This program and the accompanying materials > are licensed and made available under the terms and conditions of the BSD License > @@ -208,7 +209,7 @@ CheckForSupportGetImage ( > TestGuid = gFirmwareManagementBBTestConformanceSupportGuid001; > ResultMessageLabel = L"GetImage, function support check"; > > - ImageIndex = 0; > + ImageIndex = 1; > Image = NULL; > ImageSize = 0; > > @@ -283,7 +284,7 @@ CheckForSupportSetImage ( > TestGuid = gFirmwareManagementBBTestConformanceSupportGuid002; > ResultMessageLabel = L"SetImage, function support check"; > > - ImageIndex = 0; > + ImageIndex = 1; > Image = NULL; > ImageSize = 0; > AbortReason = NULL; > @@ -362,7 +363,7 @@ CheckForSupportCheckImage ( > TestGuid = gFirmwareManagementBBTestConformanceSupportGuid003; > ResultMessageLabel = L"CheckImage, function support check"; > > - ImageIndex = 0; > + ImageIndex = 1; > Image = NULL; > ImageSize = 0; > > diff --git a/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/FirmwareManagement/BlackBoxTest/FirmwareManagementBBTestConformance.c b/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/FirmwareManagement/BlackBoxTest/FirmwareManagementBBTestConformance.c > index 797229adc8f6..65715f6c33e6 100644 > --- a/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/FirmwareManagement/BlackBoxTest/FirmwareManagementBBTestConformance.c > +++ b/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/FirmwareManagement/BlackBoxTest/FirmwareManagementBBTestConformance.c > @@ -3,6 +3,7 @@ > Copyright 2006 - 2016 Unified EFI, Inc.
> Copyright (c) 2010 - 2016, Dell Inc. All rights reserved.
> Copyright (c) 2019,Microchip Technology Inc.
> + Copyright (c) 2021, Intel Corporation. All rights reserved.
> > This program and the accompanying materials > are licensed and made available under the terms and conditions of the BSD License > @@ -209,7 +210,7 @@ CheckForSupportGetImage ( > TestGuid = gFirmwareManagementBBTestConformanceSupportGuid001; > ResultMessageLabel = L"GetImage, function support check"; > > - ImageIndex = 0; > + ImageIndex = 1; > Image = NULL; > ImageSize = 0; > > @@ -284,7 +285,7 @@ CheckForSupportSetImage ( > TestGuid = gFirmwareManagementBBTestConformanceSupportGuid002; > ResultMessageLabel = L"SetImage, function support check"; > > - ImageIndex = 0; > + ImageIndex = 1; > Image = NULL; > ImageSize = 0; > AbortReason = NULL; > @@ -363,7 +364,7 @@ CheckForSupportCheckImage ( > TestGuid = gFirmwareManagementBBTestConformanceSupportGuid003; > ResultMessageLabel = L"CheckImage, function support check"; > > - ImageIndex = 0; > + ImageIndex = 1; > Image = NULL; > ImageSize = 0; > --------------12767D6F9E88211C8A16A7AF Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: 8bit

Reviewed-by: Barton Gao <gaojie@byosoft.com.cn>

On 2021/3/22 14:12, Eric Jin wrote:
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3272

The goal of the Support Routine: CheckForSupportGetImage is to detect the
current FMP instance's capability for supporting GetImage().

In current code, "ImageIndex = 0" is the input parameter, but it is
inconsistent with spec -"A unique number identifying the firmware image(s)
within the device. The number is between 1 and DescriptorCount."

It is a bug and need the fix.
The similar errors exist in CheckForSupportSetImage/CheckForSupportCheckImage.

Cc: G Edhaya Chandran <Edhaya.Chandran@arm.com>
Cc: Barton Gao <gaojie@byosoft.com.cn>
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
Cc: Arvin Chen <arvinx.chen@intel.com>
Signed-off-by: Eric Jin <eric.jin@intel.com>
---
 .../BlackBoxTest/FirmwareManagementBBTestConformance.c     | 7 ++++---
 .../BlackBoxTest/FirmwareManagementBBTestConformance.c     | 7 ++++---
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/FirmwareManagement/BlackBoxTest/FirmwareManagementBBTestConformance.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/FirmwareManagement/BlackBoxTest/FirmwareManagementBBTestConformance.c
index 257f2756dbcb..44d01d27a8f2 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/FirmwareManagement/BlackBoxTest/FirmwareManagementBBTestConformance.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/FirmwareManagement/BlackBoxTest/FirmwareManagementBBTestConformance.c
@@ -2,6 +2,7 @@
 
   Copyright 2006 - 2016 Unified EFI, Inc.<BR>
   Copyright (c) 2010 - 2016, Dell Inc. All rights reserved.<BR>
+  Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
 
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD License
@@ -208,7 +209,7 @@ CheckForSupportGetImage (
   TestGuid = gFirmwareManagementBBTestConformanceSupportGuid001;
   ResultMessageLabel = L"GetImage, function support check";
 
-  ImageIndex = 0;
+  ImageIndex = 1;
   Image = NULL;
   ImageSize = 0;
 
@@ -283,7 +284,7 @@ CheckForSupportSetImage (
   TestGuid = gFirmwareManagementBBTestConformanceSupportGuid002;
   ResultMessageLabel = L"SetImage, function support check";
 
-  ImageIndex = 0;
+  ImageIndex = 1;
   Image = NULL;
   ImageSize = 0;
   AbortReason = NULL;
@@ -362,7 +363,7 @@ CheckForSupportCheckImage (
   TestGuid = gFirmwareManagementBBTestConformanceSupportGuid003;
   ResultMessageLabel = L"CheckImage, function support check";
 
-  ImageIndex = 0;
+  ImageIndex = 1;
   Image = NULL;
   ImageSize = 0;
 
diff --git a/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/FirmwareManagement/BlackBoxTest/FirmwareManagementBBTestConformance.c b/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/FirmwareManagement/BlackBoxTest/FirmwareManagementBBTestConformance.c
index 797229adc8f6..65715f6c33e6 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/FirmwareManagement/BlackBoxTest/FirmwareManagementBBTestConformance.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/FirmwareManagement/BlackBoxTest/FirmwareManagementBBTestConformance.c
@@ -3,6 +3,7 @@
   Copyright 2006 - 2016 Unified EFI, Inc.<BR>
   Copyright (c) 2010 - 2016, Dell Inc. All rights reserved.<BR>
   Copyright (c) 2019,Microchip Technology Inc.<BR>
+  Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
 
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD License
@@ -209,7 +210,7 @@ CheckForSupportGetImage (
   TestGuid = gFirmwareManagementBBTestConformanceSupportGuid001;
   ResultMessageLabel = L"GetImage, function support check";
 
-  ImageIndex = 0;
+  ImageIndex = 1;
   Image = NULL;
   ImageSize = 0;
 
@@ -284,7 +285,7 @@ CheckForSupportSetImage (
   TestGuid = gFirmwareManagementBBTestConformanceSupportGuid002;
   ResultMessageLabel = L"SetImage, function support check";
 
-  ImageIndex = 0;
+  ImageIndex = 1;
   Image = NULL;
   ImageSize = 0;
   AbortReason = NULL;
@@ -363,7 +364,7 @@ CheckForSupportCheckImage (
   TestGuid = gFirmwareManagementBBTestConformanceSupportGuid003;
   ResultMessageLabel = L"CheckImage, function support check";
 
-  ImageIndex = 0;
+  ImageIndex = 1;
   Image = NULL;
   ImageSize = 0;
 
--------------12767D6F9E88211C8A16A7AF--