From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=217.140.101.70; helo=foss.arm.com; envelope-from=supreeth.venkatesh@arm.com; receiver=edk2-devel@lists.01.org Received: from foss.arm.com (usa-sjc-mx-foss1.foss.arm.com [217.140.101.70]) by ml01.01.org (Postfix) with ESMTP id C660621CAD998 for ; Tue, 16 Oct 2018 02:48:38 -0700 (PDT) Received: by usa-sjc-mx-foss1.foss.arm.com (Postfix, from userid 105) id 9A764354C; Tue, 16 Oct 2018 03:08:30 -0700 (PDT) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 2BBF434FE; Sun, 14 Oct 2018 19:34:01 -0700 (PDT) Received: from [10.6.43.238] (bc-c3-3-14.eu.iaas.arm.com [10.6.43.238]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id EF1203F5B1; Sun, 14 Oct 2018 19:33:39 -0700 (PDT) To: Eric Jin , edk2-devel@lists.01.org Cc: Jiaxin Wu References: <20181014022552.4800-1-eric.jin@intel.com> From: Supreeth Venkatesh Message-ID: <2acaf652-b73d-e0de-d98c-89c2a039bdc5@arm.com> Date: Mon, 15 Oct 2018 03:33:37 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20181014022552.4800-1-eric.jin@intel.com> Subject: Re: [PATCH] uefi-sct/SctPkg:Implement the iSCSI devicepath to text X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 16 Oct 2018 09:48:39 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US On 10/14/2018 03:25 AM, Eric Jin wrote: > Cc: Supreeth Venkatesh > Cc: Jiaxin Wu > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Eric Jin > --- > .../DevicePathToTextBBTestCoverage.c | 43 ++++++++++++++++++- > .../BlackBoxTest/DevicePathToTextBBTestMain.c | 26 +++++++++-- > .../DevicePathToText/BlackBoxTest/Guid.c | 1 + > .../DevicePathToText/BlackBoxTest/Guid.h | 7 +++ > 4 files changed, 73 insertions(+), 4 deletions(-) > > diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathToText/BlackBoxTest/DevicePathToTextBBTestCoverage.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathToText/BlackBoxTest/DevicePathToTextBBTestCoverage.c > index c30af434..14c4c460 100644 > --- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathToText/BlackBoxTest/DevicePathToTextBBTestCoverage.c > +++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathToText/BlackBoxTest/DevicePathToTextBBTestCoverage.c > @@ -2202,7 +2202,48 @@ DevicePathToTextConvertDeviceNodeToTextCoverageTest ( > SctFreePool (Text); > } > > - > + // > + // iSCSI(Name,0x12AB,0x0000005678000000,CRC32C,None,CHAP_BI,TCP) > + // > + pDeviceNode1 = DevicePathUtilities->CreateDeviceNode (0x3, 0x13, sizeof (ISCSI_DEVICE_PATH_WITH_NAME) + 4); Why 0x03, 0x13 and + 4? - Magic Numbers. > > + ((ISCSI_DEVICE_PATH_WITH_NAME *)pDeviceNode1)->NetworkProtocol = 0x0; //TCP > + ((ISCSI_DEVICE_PATH_WITH_NAME *)pDeviceNode1)->LoginOption = 0x0002; Magic Number 2. > + ((ISCSI_DEVICE_PATH_WITH_NAME *)pDeviceNode1)->Lun = 0x0000007856000000; Magic Number > + ((ISCSI_DEVICE_PATH_WITH_NAME *)pDeviceNode1)->TargetPortalGroupTag = 0x12AB; Magic Number. > + ((ISCSI_DEVICE_PATH_WITH_NAME *)pDeviceNode1)->iSCSITargetName[0] = 'N'; > + ((ISCSI_DEVICE_PATH_WITH_NAME *)pDeviceNode1)->iSCSITargetName[1] = 'a'; > + ((ISCSI_DEVICE_PATH_WITH_NAME *)pDeviceNode1)->iSCSITargetName[2] = 'm'; > + ((ISCSI_DEVICE_PATH_WITH_NAME *)pDeviceNode1)->iSCSITargetName[3] = 'e'; > + > + Text = DevicePathToText->ConvertDeviceNodeToText (pDeviceNode1, FALSE, FALSE); > + pDeviceNode2 = SctConvertTextToDeviceNode(Text); > + > + if ((pDeviceNode2 != NULL) && (SctCompareMem (pDeviceNode2, pDeviceNode1, SctDevicePathNodeLength(pDeviceNode1)) == 0)) { > + AssertionType = EFI_TEST_ASSERTION_PASSED; > + } else { > + AssertionType = EFI_TEST_ASSERTION_FAILED; > + } > + > + StandardLib->RecordAssertion ( > + StandardLib, > + AssertionType, > + gDevicePathToTextBBTestFunctionAssertionGuid135, > + L"EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL - ConvertDeviceNodeToText must correctly recover the converting ConvertTextToDeviceNode has acted on the device node string", > + L"%a:%d: Convert result: %s - Expected:iSCSI(MyTargetName,0x12AB,0x0000005678000000,CRC32C,None,CHAP_BI,TCP)", Magic Numbers. > + __FILE__, > + (UINTN)__LINE__, > + Text > + ); > + if (pDeviceNode1 != NULL) { > + SctFreePool (pDeviceNode1); > + } > + if (pDeviceNode2 != NULL) { > + SctFreePool (pDeviceNode2); > + } > + if (Text != NULL) { > + SctFreePool (Text); > + } > + > return EFI_SUCCESS; > } > > diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathToText/BlackBoxTest/DevicePathToTextBBTestMain.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathToText/BlackBoxTest/DevicePathToTextBBTestMain.c > index 41cf217b..d755227c 100644 > --- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathToText/BlackBoxTest/DevicePathToTextBBTestMain.c > +++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathToText/BlackBoxTest/DevicePathToTextBBTestMain.c > @@ -2437,6 +2437,7 @@ BuildiSCSIDeviceNode ( > CHAR16 *LunStr; > UINT16 Options; > UINT64 LunNum; > + UINT64 TempLunNum; > > Status = GetNextRequiredParam(&TextDeviceNode, L"TargetName", &ParamIdentifierStr, &ParamIdentifierVal); > if ((!EFI_ERROR(Status)) && (ParamIdentifierVal != NULL)) { > @@ -2459,7 +2460,7 @@ BuildiSCSIDeviceNode ( > return NULL; > } > > - Length = sizeof (ISCSI_DEVICE_PATH_WITH_NAME) + (UINT16) (SctStrLen (NameStr) * 2 + 2); > + Length = sizeof (ISCSI_DEVICE_PATH_WITH_NAME) + (UINT16) (SctStrLen (NameStr)); > iSCSI = (ISCSI_DEVICE_PATH_WITH_NAME *) CreateDeviceNode (0x3, 0x13, Length); > if (iSCSI == NULL) { > return NULL; > @@ -2499,7 +2500,7 @@ BuildiSCSIDeviceNode ( > if (SctStrCmp (ParamIdentifierVal, L"CRC32C") == 0) { > Options |= 0x0002; > } > - break; > + break; > > case 1: // DataDigest > if (SctStrCmp (ParamIdentifierVal, L"CRC32C") == 0) { > @@ -2514,6 +2515,9 @@ BuildiSCSIDeviceNode ( > if (SctStrCmp (ParamIdentifierVal, L"CHAP_UNI") == 0) { > Options |= 0x1000; > } > + if (SctStrCmp (ParamIdentifierVal, L"CHAP_BI") == 0) { > + Options |= 0x0000; > + } > break; > > case 3: // Protocol > @@ -2533,8 +2537,24 @@ BuildiSCSIDeviceNode ( > > SctUnicodeToAscii (iSCSI->iSCSITargetName, NameStr, SctStrLen (NameStr)); > iSCSI->TargetPortalGroupTag = (UINT16) SctStrToUInt (PortalGroupStr); > - SctStrToUInt64 (LunStr, &LunNum); > + > + // > + // The LUN is an 8 byte array that is displayed in hexadecimal format with byte > + // 0 first (i.e., on the left) and byte 7 last (i.e, on the right), and is required. > + // > + SctStrToUInt64 (LunStr, &TempLunNum); > + LunNum = 0; > + > + LunNum = SctLShiftU64((TempLunNum & 0x00000000000000FF), 56);; > + LunNum = LunNum | SctLShiftU64((TempLunNum & 0x000000000000FF00), 40); > + LunNum = LunNum | SctLShiftU64((TempLunNum & 0x0000000000FF0000), 24); > + LunNum = LunNum | SctLShiftU64((TempLunNum & 0x00000000FF000000), 8); > + LunNum = LunNum | SctRShiftU64((TempLunNum & 0x000000FF00000000), 8); > + LunNum = LunNum | SctRShiftU64((TempLunNum & 0x0000FF0000000000), 24); > + LunNum = LunNum | SctRShiftU64((TempLunNum & 0x00FF000000000000), 40); > + LunNum = LunNum | SctRShiftU64((TempLunNum & 0xFF00000000000000), 56); Deinge Macros for magic numbers. > iSCSI->Lun = LunNum; > + > iSCSI->LoginOption = (UINT16) Options; > > return (EFI_DEVICE_PATH_PROTOCOL *) iSCSI; > diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathToText/BlackBoxTest/Guid.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathToText/BlackBoxTest/Guid.c > index 9cad8d31..21944cee 100644 > --- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathToText/BlackBoxTest/Guid.c > +++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathToText/BlackBoxTest/Guid.c > @@ -136,3 +136,4 @@ EFI_GUID gDevicePathToTextBBTestFunctionAssertionGuid133 = EFI_TEST_DEVICEPATHT > > EFI_GUID gDevicePathToTextBBTestFunctionAssertionGuid134 = EFI_TEST_DEVICEPATHTOTEXTBBTESTFUNCTION_ASSERTION_134_GUID; > > +EFI_GUID gDevicePathToTextBBTestFunctionAssertionGuid135 = EFI_TEST_DEVICEPATHTOTEXTBBTESTFUNCTION_ASSERTION_135_GUID; > diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathToText/BlackBoxTest/Guid.h b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathToText/BlackBoxTest/Guid.h > index 41d2a8a8..e00186db 100644 > --- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathToText/BlackBoxTest/Guid.h > +++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathToText/BlackBoxTest/Guid.h > @@ -400,3 +400,10 @@ extern EFI_GUID gDevicePathToTextBBTestFunctionAssertionGuid133; > } > > extern EFI_GUID gDevicePathToTextBBTestFunctionAssertionGuid134; > + > +#define EFI_TEST_DEVICEPATHTOTEXTBBTESTFUNCTION_ASSERTION_135_GUID \ > + { \ > + 0xbf73c00e, 0x5a62, 0x4a20, { 0xbe, 0x15, 0x4d, 0x83, 0x7e, 0xe5, 0x7d, 0xdf } \ > + } > +extern EFI_GUID gDevicePathToTextBBTestFunctionAssertionGuid135; > +