[edk2] Multiple Device ID support in EfiRom

2017-08-07 Thread Tomas Pilar (tpilar)

Hi,

I am looking at this patch (attached) from Daniel:

--

Hello,

Attached 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.


Let me know if there's anything I missed.

Thanks,
-- Daniel Verkamp

--

Was this patch ever considered for inclusion in the edk2 trunk? If not, 
what is the current state of multiple device id support for rom creation?


Cheers,
Tom
Index: EfiRom.c
===
--- EfiRom.c	(revision 2129)
+++ EfiRom.c	(working copy)
@@ -150,7 +150,7 @@
 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);
@@ -193,6 +193,13 @@
   free (mOptions.FileList);
   mOptions.FileList = FList;
 }
+
+//
+// Clean up device ID list
+//
+if (mOptions.DevIdList != NULL) {
+  free (mOptions.DevIdList);
+}
   }
 
   if (mOptions.Verbose) {
@@ -449,6 +456,7 @@
   UINT16MachineType;
   UINT16SubSystem;
   UINT32HeaderPadBytes;
+  UINT32DevIdListSize;
 
   //
   // Try to open the input file
@@ -492,7 +500,16 @@
   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) {
@@ -617,7 +634,14 @@
 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;
 //
@@ -686,7 +710,27 @@
   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;
+}
+  }
+
+  //
   // Keep track of how many bytes left to write
   //
   TotalSize -= HeaderSize;
@@ -866,6 +910,8 @@
   EFI_STATUS Status;
   BOOLEANEfiRomFlag;
   UINT64 TempValue;
+  char   *OptionName;
+  UINT16 *DevIdList;
 
   FileFlags = 0;
   EfiRomFlag = FALSE;
@@ -880,6 +926,9 @@
   //
   FileList= PrevFileList = NULL;
 
+  Options->DevIdList  = NULL;
+  Options->DevIdCount = 0;
+
   ClassCode   = 0;
   CodeRevision= 0;
   //
@@ -933,24 +982,49 @@
 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, 

Re: [edk2] Multiple Device ID support in EfiRom

2017-08-10 Thread Gao, Liming
This patch is not integrated into edk2 trunk. Could you file this issue in 
bugzillar (https://bugzilla.tianocore.org/) and attach this patch? We will add 
it into edk2 trunk. 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Tomas 
> Pilar (tpilar)
> Sent: Monday, August 7, 2017 10:19 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] Multiple Device ID support in EfiRom
> 
> Hi,
> 
> I am looking at this patch (attached) from Daniel:
> 
> --
> 
> Hello,
> 
> Attached 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.
> 
> Let me know if there's anything I missed.
> 
> Thanks,
> -- Daniel Verkamp
> 
> --
> 
> Was this patch ever considered for inclusion in the edk2 trunk? If not,
> what is the current state of multiple device id support for rom creation?
> 
> Cheers,
> Tom
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel