From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id CD7B321DF9692 for ; Thu, 24 Aug 2017 21:54:29 -0700 (PDT) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 24 Aug 2017 21:57:05 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.41,424,1498546800"; d="scan'208";a="1007477819" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by orsmga003.jf.intel.com with ESMTP; 24 Aug 2017 21:57:05 -0700 Received: from fmsmsx115.amr.corp.intel.com (10.18.116.19) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 24 Aug 2017 21:57:04 -0700 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by fmsmsx115.amr.corp.intel.com (10.18.116.19) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 24 Aug 2017 21:57:04 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.183]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.128]) with mapi id 14.03.0319.002; Fri, 25 Aug 2017 12:57:01 +0800 From: "Gao, Liming" To: "Zhu, Yonghong" , "edk2-devel@lists.01.org" CC: "Verkamp, Daniel" , Tomas Pilar Thread-Topic: [Patch] BaseTools/EfiRom: Add multiple device id support Thread-Index: AQHTGxgQ6XivADsVQkO4xD78YvnpiqKUhu9A Date: Fri, 25 Aug 2017 04:57:00 +0000 Message-ID: <4A89E2EF3DFEDB4C8BFDE51014F606A14D77653C@shsmsx102.ccr.corp.intel.com> References: <1503386821-50364-1-git-send-email-yonghong.zhu@intel.com> In-Reply-To: <1503386821-50364-1-git-send-email-yonghong.zhu@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [Patch] BaseTools/EfiRom: Add multiple device id support X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 25 Aug 2017 04:54:30 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Liming Gao >-----Original Message----- >From: Zhu, Yonghong >Sent: Tuesday, August 22, 2017 3:27 PM >To: edk2-devel@lists.01.org >Cc: Verkamp, Daniel ; Gao, Liming >; Tomas Pilar >Subject: [Patch] BaseTools/EfiRom: Add multiple device id support > >From: Daniel Verkamp > >This is a patch to implement writing and dumping of PCI 3.0 Device ID >lists in EFI option ROMs in the EfiRom tool. >Using this modification, multiple space-delimited device IDs can be >specified after -i. The first device in the list is used for the main >PCI ROM header Device ID field and is also written in the list. The >list is only written when more than one device ID has been specified; >when only one device ID is given on the command line, the EfiRom output >should be identical to the current code. > >Fixes: https://bugzilla.tianocore.org/show_bug.cgi?id=3D666 >Cc: Liming Gao >Cc: Tomas Pilar >Contributed-under: TianoCore Contribution Agreement 1.0 >Signed-off-by: Daniel Verkamp >--- > BaseTools/Source/C/EfiRom/EfiRom.c | 146 >++++++++++++++++++++++++++++++++----- > BaseTools/Source/C/EfiRom/EfiRom.h | 6 +- > 2 files changed, 129 insertions(+), 23 deletions(-) > >diff --git a/BaseTools/Source/C/EfiRom/EfiRom.c >b/BaseTools/Source/C/EfiRom/EfiRom.c >index 84322e3..6b30596 100644 >--- a/BaseTools/Source/C/EfiRom/EfiRom.c >+++ b/BaseTools/Source/C/EfiRom/EfiRom.c >@@ -142,11 +142,11 @@ Returns: > if ((FList->FileFlags & FILE_FLAG_EFI) !=3D 0) { > if (mOptions.Verbose) { > VerboseMsg("Processing EFI file %s\n", FList->FileName); > } > >- Status =3D ProcessEfiFile (FptrOut, FList, mOptions.VendId, mOption= s.DevId, >&Size); >+ Status =3D ProcessEfiFile (FptrOut, FList, mOptions.VendId, >mOptions.DevIdList[0], &Size); > } else if ((FList->FileFlags & FILE_FLAG_BINARY) !=3D0 ) { > if (mOptions.Verbose) { > VerboseMsg("Processing binary file %s\n", FList->FileName); > } > >@@ -182,12 +182,18 @@ BailOut: > while (mOptions.FileList !=3D NULL) { > FList =3D mOptions.FileList->Next; > free (mOptions.FileList); > mOptions.FileList =3D FList; > } >+ >+ // >+ // Clean up device ID list >+ // >+ if (mOptions.DevIdList !=3D NULL) { >+ free (mOptions.DevIdList); >+ } > } >- > if (FptrOut !=3D NULL) { > fclose (FptrOut); > } > > if (mOptions.Verbose) { >@@ -449,10 +455,11 @@ Returns: > UINT16 MachineType; > UINT16 SubSystem; > UINT32 HeaderPadBytes; > UINT32 PadBytesBeforeImage; > UINT32 PadBytesAfterImage; >+ UINT32 DevIdListSize; > > // > // Try to open the input file > // > if ((InFptr =3D fopen (LongFilePath (InFile->FileName), "rb")) =3D=3D N= ULL) { >@@ -492,11 +499,20 @@ Returns: > // For Pci3.0 to use the different data structure. > // > if (mOptions.Pci23 =3D=3D 1) { > HeaderSize =3D sizeof (PCI_DATA_STRUCTURE) + HeaderPadBytes + sizeof >(EFI_PCI_EXPANSION_ROM_HEADER); > } else { >- HeaderSize =3D sizeof (PCI_3_0_DATA_STRUCTURE) + HeaderPadBytes + >sizeof (EFI_PCI_EXPANSION_ROM_HEADER); >+ if (mOptions.DevIdCount > 1) { >+ // >+ // Write device ID list when more than one device ID is specified. >+ // Leave space for list plus terminator. >+ // >+ DevIdListSize =3D (mOptions.DevIdCount + 1) * sizeof (UINT16); >+ } else { >+ DevIdListSize =3D 0; >+ } >+ HeaderSize =3D sizeof (PCI_3_0_DATA_STRUCTURE) + HeaderPadBytes + >DevIdListSize + sizeof (EFI_PCI_EXPANSION_ROM_HEADER); > } > > if (mOptions.Verbose) { > VerboseMsg(" File size =3D 0x%X\n", (unsigned) FileSize); > } >@@ -629,11 +645,18 @@ Returns: > PciDs23.CodeType =3D PCI_CODE_TYPE_EFI_IMAGE; > } else { > PciDs30.Signature =3D PCI_DATA_STRUCTURE_SIGNATURE; > PciDs30.VendorId =3D VendId; > PciDs30.DeviceId =3D DevId; >- PciDs30.DeviceListOffset =3D 0; // to be fixed >+ if (mOptions.DevIdCount > 1) { >+ // >+ // Place device list immediately after PCI structure >+ // >+ PciDs30.DeviceListOffset =3D (UINT16) sizeof (PCI_3_0_DATA_STRUCTUR= E); >+ } else { >+ PciDs30.DeviceListOffset =3D 0; >+ } > PciDs30.Length =3D (UINT16) sizeof (PCI_3_0_DATA_STRUCTURE); > PciDs30.Revision =3D 0x3; > // > // Class code and code revision from the command line (optional) > // >@@ -699,10 +722,30 @@ Returns: > Status =3D STATUS_ERROR; > goto BailOut; > } > } > >+ // >+ // Write the Device ID list to the output file >+ // >+ if (mOptions.DevIdCount > 1) { >+ if (fwrite (mOptions.DevIdList, sizeof (UINT16), mOptions.DevIdCount, >OutFptr) !=3D mOptions.DevIdCount) { >+ Error (NULL, 0, 0002, "Failed to write PCI device list to output fi= le!", NULL); >+ Status =3D STATUS_ERROR; >+ goto BailOut; >+ } >+ // >+ // Write two-byte terminating 0 at the end of the device list >+ // >+ if (putc (0, OutFptr) =3D=3D EOF || putc (0, OutFptr) =3D=3D EOF) { >+ Error (NULL, 0, 0002, "Failed to write PCI device list to output fi= le!", NULL); >+ Status =3D STATUS_ERROR; >+ goto BailOut; >+ } >+ } >+ >+ > // > // Pad head to make it a multiple of 512 bytes > // > while (PadBytesBeforeImage > 0) { > if (putc (~0, OutFptr) =3D=3D EOF) { >@@ -885,10 +928,12 @@ Returns: > UINT32 CodeRevision; > EFI_STATUS Status; > INTN ReturnStatus; > BOOLEAN EfiRomFlag; > UINT64 TempValue; >+ char *OptionName; >+ UINT16 *DevIdList; > > ReturnStatus =3D 0; > FileFlags =3D 0; > EfiRomFlag =3D FALSE; > >@@ -900,10 +945,13 @@ Returns: > // > // To avoid compile warnings > // > FileList =3D PrevFileList =3D NULL; > >+ Options->DevIdList =3D NULL; >+ Options->DevIdCount =3D 0; >+ > ClassCode =3D 0; > CodeRevision =3D 0; > // > // Skip over the program name > // >@@ -955,30 +1003,57 @@ Returns: > Options->VendIdValid =3D 1; > > Argv++; > Argc--; > } else if (stricmp (Argv[0], "-i") =3D=3D 0) { >+ >+ OptionName =3D Argv[0]; >+ > // >- // Device ID specified with -i >- // Make sure there's another parameter >+ // Device IDs specified with -i >+ // Make sure there's at least one more parameter > // >- Status =3D AsciiStringToUint64(Argv[1], FALSE, &TempValue); >- if (EFI_ERROR (Status)) { >- Error (NULL, 0, 2000, "Invalid option value", "%s =3D %s", Argv= [0], Argv[1]); >+ if (Argc < 1) { >+ Error (NULL, 0, 2000, "Invalid parameter", "Missing Device Id w= ith %s >option!", OptionName); > ReturnStatus =3D 1; > goto Done; > } >- if (TempValue >=3D 0x10000) { >- Error (NULL, 0, 2000, "Invalid option value", "Device Id %s out= of range!", >Argv[1]); >- ReturnStatus =3D 1; >- goto Done; >+ >+ // >+ // Process until another dash-argument parameter or the end of th= e list >+ // >+ while (Argc > 1 && Argv[1][0] !=3D '-') { >+ Status =3D AsciiStringToUint64(Argv[1], FALSE, &TempValue); >+ if (EFI_ERROR (Status)) { >+ Error (NULL, 0, 2000, "Invalid option value", "%s =3D %s", Op= tionName, >Argv[1]); >+ ReturnStatus =3D 1; >+ goto Done; >+ } >+ // >+ // Don't allow device IDs greater than 16 bits >+ // Don't allow 0, since it is used as a list terminator >+ // >+ if (TempValue >=3D 0x10000 || TempValue =3D=3D 0) { >+ Error (NULL, 0, 2000, "Invalid option value", "Device Id %s o= ut of >range!", Argv[1]); >+ ReturnStatus =3D 1; >+ goto Done; >+ } >+ >+ DevIdList =3D (UINT16*) realloc (Options->DevIdList, (Options- >>DevIdCount + 1) * sizeof (UINT16)); >+ if (DevIdList =3D=3D NULL) { >+ Error (NULL, 0, 4001, "Resource", "memory cannot be allocated= !", >NULL); >+ ReturnStatus =3D 1; >+ goto Done; >+ } >+ Options->DevIdList =3D DevIdList; >+ >+ Options->DevIdList[Options->DevIdCount++] =3D (UINT16) TempValu= e; >+ >+ Argv++; >+ Argc--; > } >- Options->DevId =3D (UINT16) TempValue; >- Options->DevIdValid =3D 1; > >- Argv++; >- Argc--; > } else if ((stricmp (Argv[0], "-o") =3D=3D 0) || (stricmp (Argv[0],= "--output") =3D=3D >0)) { > // > // Output filename specified with -o > // Make sure there's another parameter > // >@@ -1055,11 +1130,11 @@ Returns: > // command line. > // > Options->DumpOption =3D 1; > > Options->VendIdValid =3D 1; >- Options->DevIdValid =3D 1; >+ Options->DevIdCount =3D 1; > FileFlags =3D FILE_FLAG_BINARY; > } else if ((stricmp (Argv[0], "-l") =3D=3D 0) || (stricmp (Argv[0],= "--class-code") >=3D=3D 0)) { > // > // Class code value for the next file in the list. > // Make sure there's another parameter >@@ -1189,16 +1264,22 @@ Returns: > Error (NULL, 0, 2000, "Missing Vendor ID in command line", NULL); > ReturnStatus =3D STATUS_ERROR; > goto Done; > } > >- if (!Options->DevIdValid) { >+ if (!Options->DevIdCount) { > Error (NULL, 0, 2000, "Missing Device ID in command line", NULL); > ReturnStatus =3D STATUS_ERROR; > goto Done; > } > } >+ >+ if (Options->DevIdCount > 1 && Options->Pci23) { >+ Error (NULL, 0, 2000, "Invalid parameter", "PCI 3.0 is required when >specifying multiple Device IDs"); >+ ReturnStatus =3D STATUS_ERROR; >+ goto Done; >+ } > > Done: > if (ReturnStatus !=3D 0) { > while (Options->FileList !=3D NULL) { > FileList =3D Options->FileList->Next; >@@ -1281,11 +1362,11 @@ Returns: > fprintf (stdout, " -r Rev Hex Revision in the PCI data structure he= ader.\n"); > fprintf (stdout, " -n Not to automatically set the LAST bit in = the last >file.\n"); > fprintf (stdout, " -f VendorId\n\ > Hex PCI Vendor ID for the device OpROM, must be specified\n")= ; > fprintf (stdout, " -i DeviceId\n\ >- Hex PCI Device ID for the device OpROM, must be specified\n")= ; >+ One or more hex PCI Device IDs for the device OpROM, must be >specified\n"); > fprintf (stdout, " -p, --pci23\n\ > Default layout meets PCI 3.0 specifications\n\ > specifying this flag will for a PCI 2.3 layout.\n"); > fprintf (stdout, " -d, --dump\n\ > Dump the headers of an existing option ROM image.\n"); >@@ -1326,10 +1407,11 @@ Returns: > UINT32 ImageStart; > UINT32 ImageCount; > EFI_PCI_EXPANSION_ROM_HEADER EfiRomHdr; > PCI_DATA_STRUCTURE PciDs23; > PCI_3_0_DATA_STRUCTURE PciDs30; >+ UINT16 DevId; > > // > // Open the input file > // > if ((InFptr =3D fopen (LongFilePath (InFile->FileName), "rb")) =3D=3D N= ULL) { >@@ -1424,10 +1506,34 @@ Returns: > fprintf (stdout, " Vendor ID 0x%04X\n", PciDs30.Vend= orId); > fprintf (stdout, " Device ID 0x%04X\n", PciDs30.Devi= ceId); > fprintf (stdout, " Length 0x%04X\n", PciDs30.Leng= th); > fprintf (stdout, " Revision 0x%04X\n", PciDs30.Revi= sion); > fprintf (stdout, " DeviceListOffset 0x%02X\n", >PciDs30.DeviceListOffset); >+ if (PciDs30.DeviceListOffset) { >+ // >+ // Print device ID list >+ // >+ fprintf (stdout, " Device list contents\n"); >+ if (fseek (InFptr, ImageStart + PciRomHdr.PcirOffset + >PciDs30.DeviceListOffset, SEEK_SET)) { >+ Error (NULL, 0, 3001, "Not supported", "Failed to seek to PCI dev= ice ID >list!"); >+ goto BailOut; >+ } >+ >+ // >+ // Loop until terminating 0 >+ // >+ do { >+ if (fread (&DevId, sizeof (DevId), 1, InFptr) !=3D 1) { >+ Error (NULL, 0, 3001, "Not supported", "Failed to read PCI devi= ce ID list >from file %s!", InFile->FileName); >+ goto BailOut; >+ } >+ if (DevId) { >+ fprintf (stdout, " 0x%04X\n", DevId); >+ } >+ } while (DevId); >+ >+ } > fprintf ( > stdout, > " Class Code 0x%06X\n", > (unsigned) (PciDs30.ClassCode[0] | (PciDs30.ClassCode[1] << 8) | >(PciDs30.ClassCode[2] << 16)) > ); >diff --git a/BaseTools/Source/C/EfiRom/EfiRom.h >b/BaseTools/Source/C/EfiRom/EfiRom.h >index 6763d6b..f90c63f 100644 >--- a/BaseTools/Source/C/EfiRom/EfiRom.h >+++ b/BaseTools/Source/C/EfiRom/EfiRom.h >@@ -1,9 +1,9 @@ > /** @file > This file contains the relevant declarations required to generate Option = Rom >File > >-Copyright (c) 1999 - 2014, Intel Corporation. All rights reserved.
>+Copyright (c) 1999 - 2017, 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 which accompanies this > distribution. The full text of the license may be found at > http://opensource.org/licenses/bsd-license.php > >@@ -81,13 +81,13 @@ typedef struct { > CHAR8 OutFileName[MAX_PATH]; > INT8 NoLast; > UINT16 ClassCode; > UINT16 PciRevision; > UINT16 VendId; >- UINT16 DevId; >+ UINT16 *DevIdList; >+ UINT32 DevIdCount; > UINT8 VendIdValid; >- UINT8 DevIdValid; > INT8 Verbose; > INT8 Quiet; > INT8 Debug; > INT8 Pci23; > INT8 Pci30; >-- >2.6.1.windows.1