public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* Multiple Device ID support in EfiRom
@ 2017-08-07 14:18 Tomas Pilar (tpilar)
  2017-08-10 13:48 ` Gao, Liming
  0 siblings, 1 reply; 2+ messages in thread
From: Tomas Pilar (tpilar) @ 2017-08-07 14:18 UTC (permalink / raw)
  To: edk2-devel@lists.01.org

[-- Attachment #1: Type: text/plain, Size: 821 bytes --]

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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: EfiRom_device_list.patch --]
[-- Type: text/x-patch; name="EfiRom_device_list.patch", Size: 9119 bytes --]

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 @@
   UINT16                        MachineType;
   UINT16                        SubSystem;
   UINT32                        HeaderPadBytes;
+  UINT32                        DevIdListSize;
 
   //
   // 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;
   BOOLEAN    EfiRomFlag;
   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, "Invalid parameter", "Missing Device Id with %s option!", OptionName);
           return 1;
         }
-        if (TempValue >= 0x10000) {
-          Error (NULL, 0, 2000, "Invalid option value", "Device Id %s out of range!", Argv[1]);
-          return 1;
+
+        //
+        // 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]);
+            return 1;
+          }
+          //
+          // 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]);
+            return 1;
+          }
+
+          DevIdList = (UINT16*) realloc (Options->DevIdList, (Options->DevIdCount + 1) * sizeof (UINT16));
+          if (DevIdList == NULL) {
+            Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!", NULL);
+            return 1;
+          }
+          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
@@ -1021,7 +1095,7 @@
         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)) {
         //
@@ -1144,10 +1218,15 @@
       return STATUS_ERROR;
     }
   
-    if (!Options->DevIdValid) {
+    if (!Options->DevIdCount) {
       Error (NULL, 0, 2000, "Missing Device ID in command line", NULL);
       return STATUS_ERROR;
     }
+
+    if (Options->DevIdCount > 1 && Options->Pci23) {
+      Error (NULL, 0, 2000, "Invalid parameter", "PCI 3.0 is required when specifying multiple Device IDs");
+      return STATUS_ERROR;
+    }
   }
 
   return 0;
@@ -1226,7 +1305,7 @@
   fprintf (stdout, "  -f VendorId\n\
             Hex PCI Vendor ID for the device OpROM.\n");
   fprintf (stdout, "  -i DeviceId\n\
-            Hex PCI Device ID for the device OpROM.\n");
+            One or more hex PCI Device IDs for the device OpROM.\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");
@@ -1271,6 +1350,7 @@
   EFI_PCI_EXPANSION_ROM_HEADER  EfiRomHdr;
   PCI_DATA_STRUCTURE            PciDs23;
   PCI_3_0_DATA_STRUCTURE        PciDs30;
+  UINT16                        DevId;
 
   //
   // Open the input file
@@ -1369,6 +1449,30 @@
     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",
Index: EfiRom.h
===================================================================
--- EfiRom.h	(revision 2129)
+++ EfiRom.h	(working copy)
@@ -92,9 +92,9 @@
   UINT16    ClassCode;
   UINT16    PciRevision;
   UINT16    VendId;
-  UINT16    DevId;
+  UINT16    *DevIdList;
+  UINT32    DevIdCount;
   UINT8     VendIdValid;
-  UINT8     DevIdValid;
   INT8      Verbose;
   INT8      Quiet;
   INT8      Debug;

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: Multiple Device ID support in EfiRom
  2017-08-07 14:18 Multiple Device ID support in EfiRom Tomas Pilar (tpilar)
@ 2017-08-10 13:48 ` Gao, Liming
  0 siblings, 0 replies; 2+ messages in thread
From: Gao, Liming @ 2017-08-10 13:48 UTC (permalink / raw)
  To: Tomas Pilar (tpilar), edk2-devel@lists.01.org

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-bounces@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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2017-08-10 13:45 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-07 14:18 Multiple Device ID support in EfiRom Tomas Pilar (tpilar)
2017-08-10 13:48 ` Gao, Liming

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox