From: "Gao, Liming" <liming.gao@intel.com>
To: "Zhu, Yonghong" <yonghong.zhu@intel.com>,
"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Verkamp, Daniel" <daniel.verkamp@intel.com>,
Tomas Pilar <tpilar@solarflare.com>
Subject: Re: [Patch] BaseTools/EfiRom: Add multiple device id support
Date: Fri, 25 Aug 2017 04:57:00 +0000 [thread overview]
Message-ID: <4A89E2EF3DFEDB4C8BFDE51014F606A14D77653C@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <1503386821-50364-1-git-send-email-yonghong.zhu@intel.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
>-----Original Message-----
>From: Zhu, Yonghong
>Sent: Tuesday, August 22, 2017 3:27 PM
>To: edk2-devel@lists.01.org
>Cc: Verkamp, Daniel <daniel.verkamp@intel.com>; Gao, Liming
><liming.gao@intel.com>; Tomas Pilar <tpilar@solarflare.com>
>Subject: [Patch] BaseTools/EfiRom: Add multiple device id support
>
>From: Daniel Verkamp <daniel.verkamp@intel.com>
>
>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=666
>Cc: Liming Gao <liming.gao@intel.com>
>Cc: Tomas Pilar <tpilar@solarflare.com>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Daniel Verkamp <daniel.verkamp@intel.com>
>---
> 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) != 0) {
> if (mOptions.Verbose) {
> VerboseMsg("Processing EFI file %s\n", FList->FileName);
> }
>
>- Status = ProcessEfiFile (FptrOut, FList, mOptions.VendId, mOptions.DevId,
>&Size);
>+ Status = ProcessEfiFile (FptrOut, FList, mOptions.VendId,
>mOptions.DevIdList[0], &Size);
> } else if ((FList->FileFlags & FILE_FLAG_BINARY) !=0 ) {
> if (mOptions.Verbose) {
> VerboseMsg("Processing binary file %s\n", FList->FileName);
> }
>
>@@ -182,12 +182,18 @@ BailOut:
> while (mOptions.FileList != NULL) {
> FList = mOptions.FileList->Next;
> free (mOptions.FileList);
> mOptions.FileList = FList;
> }
>+
>+ //
>+ // Clean up device ID list
>+ //
>+ if (mOptions.DevIdList != NULL) {
>+ free (mOptions.DevIdList);
>+ }
> }
>-
> if (FptrOut != 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 = fopen (LongFilePath (InFile->FileName), "rb")) == NULL) {
>@@ -492,11 +499,20 @@ Returns:
> // For Pci3.0 to use the different data structure.
> //
> if (mOptions.Pci23 == 1) {
> HeaderSize = sizeof (PCI_DATA_STRUCTURE) + HeaderPadBytes + sizeof
>(EFI_PCI_EXPANSION_ROM_HEADER);
> } else {
>- HeaderSize = 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 = (mOptions.DevIdCount + 1) * sizeof (UINT16);
>+ } else {
>+ DevIdListSize = 0;
>+ }
>+ HeaderSize = sizeof (PCI_3_0_DATA_STRUCTURE) + HeaderPadBytes +
>DevIdListSize + sizeof (EFI_PCI_EXPANSION_ROM_HEADER);
> }
>
> if (mOptions.Verbose) {
> VerboseMsg(" File size = 0x%X\n", (unsigned) FileSize);
> }
>@@ -629,11 +645,18 @@ Returns:
> PciDs23.CodeType = PCI_CODE_TYPE_EFI_IMAGE;
> } else {
> PciDs30.Signature = PCI_DATA_STRUCTURE_SIGNATURE;
> PciDs30.VendorId = VendId;
> PciDs30.DeviceId = DevId;
>- PciDs30.DeviceListOffset = 0; // to be fixed
>+ if (mOptions.DevIdCount > 1) {
>+ //
>+ // Place device list immediately after PCI structure
>+ //
>+ PciDs30.DeviceListOffset = (UINT16) sizeof (PCI_3_0_DATA_STRUCTURE);
>+ } else {
>+ PciDs30.DeviceListOffset = 0;
>+ }
> PciDs30.Length = (UINT16) sizeof (PCI_3_0_DATA_STRUCTURE);
> PciDs30.Revision = 0x3;
> //
> // Class code and code revision from the command line (optional)
> //
>@@ -699,10 +722,30 @@ Returns:
> Status = 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) != mOptions.DevIdCount) {
>+ Error (NULL, 0, 0002, "Failed to write PCI device list to output file!", NULL);
>+ Status = STATUS_ERROR;
>+ goto BailOut;
>+ }
>+ //
>+ // Write two-byte terminating 0 at the end of the device list
>+ //
>+ if (putc (0, OutFptr) == EOF || putc (0, OutFptr) == EOF) {
>+ Error (NULL, 0, 0002, "Failed to write PCI device list to output file!", NULL);
>+ Status = STATUS_ERROR;
>+ goto BailOut;
>+ }
>+ }
>+
>+
> //
> // Pad head to make it a multiple of 512 bytes
> //
> while (PadBytesBeforeImage > 0) {
> if (putc (~0, OutFptr) == EOF) {
>@@ -885,10 +928,12 @@ Returns:
> UINT32 CodeRevision;
> EFI_STATUS Status;
> INTN ReturnStatus;
> BOOLEAN EfiRomFlag;
> UINT64 TempValue;
>+ char *OptionName;
>+ UINT16 *DevIdList;
>
> ReturnStatus = 0;
> FileFlags = 0;
> EfiRomFlag = FALSE;
>
>@@ -900,10 +945,13 @@ Returns:
> //
> // To avoid compile warnings
> //
> FileList = PrevFileList = NULL;
>
>+ Options->DevIdList = NULL;
>+ Options->DevIdCount = 0;
>+
> ClassCode = 0;
> CodeRevision = 0;
> //
> // Skip over the program name
> //
>@@ -955,30 +1003,57 @@ Returns:
> Options->VendIdValid = 1;
>
> Argv++;
> Argc--;
> } else if (stricmp (Argv[0], "-i") == 0) {
>+
>+ OptionName = 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 = AsciiStringToUint64(Argv[1], FALSE, &TempValue);
>- if (EFI_ERROR (Status)) {
>- Error (NULL, 0, 2000, "Invalid option value", "%s = %s", Argv[0], Argv[1]);
>+ if (Argc < 1) {
>+ Error (NULL, 0, 2000, "Invalid parameter", "Missing Device Id with %s
>option!", OptionName);
> ReturnStatus = 1;
> goto Done;
> }
>- if (TempValue >= 0x10000) {
>- Error (NULL, 0, 2000, "Invalid option value", "Device Id %s out of range!",
>Argv[1]);
>- ReturnStatus = 1;
>- goto Done;
>+
>+ //
>+ // Process until another dash-argument parameter or the end of the list
>+ //
>+ while (Argc > 1 && Argv[1][0] != '-') {
>+ Status = AsciiStringToUint64(Argv[1], FALSE, &TempValue);
>+ if (EFI_ERROR (Status)) {
>+ Error (NULL, 0, 2000, "Invalid option value", "%s = %s", OptionName,
>Argv[1]);
>+ ReturnStatus = 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 >= 0x10000 || TempValue == 0) {
>+ Error (NULL, 0, 2000, "Invalid option value", "Device Id %s out of
>range!", Argv[1]);
>+ ReturnStatus = 1;
>+ goto Done;
>+ }
>+
>+ DevIdList = (UINT16*) realloc (Options->DevIdList, (Options-
>>DevIdCount + 1) * sizeof (UINT16));
>+ if (DevIdList == NULL) {
>+ Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!",
>NULL);
>+ ReturnStatus = 1;
>+ goto Done;
>+ }
>+ Options->DevIdList = DevIdList;
>+
>+ Options->DevIdList[Options->DevIdCount++] = (UINT16) TempValue;
>+
>+ Argv++;
>+ Argc--;
> }
>- Options->DevId = (UINT16) TempValue;
>- Options->DevIdValid = 1;
>
>- Argv++;
>- Argc--;
> } else if ((stricmp (Argv[0], "-o") == 0) || (stricmp (Argv[0], "--output") ==
>0)) {
> //
> // Output filename specified with -o
> // Make sure there's another parameter
> //
>@@ -1055,11 +1130,11 @@ Returns:
> // command line.
> //
> Options->DumpOption = 1;
>
> Options->VendIdValid = 1;
>- Options->DevIdValid = 1;
>+ Options->DevIdCount = 1;
> FileFlags = FILE_FLAG_BINARY;
> } else if ((stricmp (Argv[0], "-l") == 0) || (stricmp (Argv[0], "--class-code")
>== 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 = STATUS_ERROR;
> goto Done;
> }
>
>- if (!Options->DevIdValid) {
>+ if (!Options->DevIdCount) {
> Error (NULL, 0, 2000, "Missing Device ID in command line", NULL);
> ReturnStatus = 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 = STATUS_ERROR;
>+ goto Done;
>+ }
>
> Done:
> if (ReturnStatus != 0) {
> while (Options->FileList != NULL) {
> FileList = Options->FileList->Next;
>@@ -1281,11 +1362,11 @@ Returns:
> fprintf (stdout, " -r Rev Hex Revision in the PCI data structure header.\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 = fopen (LongFilePath (InFile->FileName), "rb")) == NULL) {
>@@ -1424,10 +1506,34 @@ Returns:
> fprintf (stdout, " Vendor ID 0x%04X\n", PciDs30.VendorId);
> fprintf (stdout, " Device ID 0x%04X\n", PciDs30.DeviceId);
> fprintf (stdout, " Length 0x%04X\n", PciDs30.Length);
> fprintf (stdout, " Revision 0x%04X\n", PciDs30.Revision);
> 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 device ID
>list!");
>+ goto BailOut;
>+ }
>+
>+ //
>+ // Loop until terminating 0
>+ //
>+ do {
>+ if (fread (&DevId, sizeof (DevId), 1, InFptr) != 1) {
>+ Error (NULL, 0, 3001, "Not supported", "Failed to read PCI device 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.<BR>
>+Copyright (c) 1999 - 2017, 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 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
prev parent reply other threads:[~2017-08-25 4:54 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-22 7:27 [Patch] BaseTools/EfiRom: Add multiple device id support Yonghong Zhu
2017-08-25 4:57 ` Gao, Liming [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4A89E2EF3DFEDB4C8BFDE51014F606A14D77653C@shsmsx102.ccr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox