public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: edk2-devel-01 <edk2-devel@lists.01.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Jordan Justen <jordan.l.justen@intel.com>
Subject: [PATCH v2 7/7] OvmfPkg/Virtio10Dxe: convert to PciCapLib
Date: Wed, 23 May 2018 22:21:21 +0200	[thread overview]
Message-ID: <20180523202121.8125-8-lersek@redhat.com> (raw)
In-Reply-To: <20180523202121.8125-1-lersek@redhat.com>

Replace the manual capability list parsing in OvmfPkg/Virtio10Dxe with
PciCapLib and PciCapPciIoLib API calls.

The VIRTIO_PCI_CAP_LINK structure type is now superfluous. (Well, it
always has been; we should have used EFI_PCI_CAPABILITY_HDR.)

Also, EFI_PCI_CAPABILITY_VENDOR_HDR is now included at the front of
VIRTIO_PCI_CAP. No driver other than Virtio10Dxe relies on VIRTIO_PCI_CAP.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    v2:
    - no changes

 OvmfPkg/Virtio10Dxe/Virtio10.inf            |   2 +
 OvmfPkg/Include/IndustryStandard/Virtio10.h |   7 +-
 OvmfPkg/Virtio10Dxe/Virtio10.c              | 135 +++++++-------------
 3 files changed, 52 insertions(+), 92 deletions(-)

diff --git a/OvmfPkg/Virtio10Dxe/Virtio10.inf b/OvmfPkg/Virtio10Dxe/Virtio10.inf
index c4ef15d94bfc..db0cb1189a29 100644
--- a/OvmfPkg/Virtio10Dxe/Virtio10.inf
+++ b/OvmfPkg/Virtio10Dxe/Virtio10.inf
@@ -32,6 +32,8 @@ [LibraryClasses]
   BaseMemoryLib
   DebugLib
   MemoryAllocationLib
+  PciCapLib
+  PciCapPciIoLib
   UefiBootServicesTableLib
   UefiDriverEntryPoint
   UefiLib
diff --git a/OvmfPkg/Include/IndustryStandard/Virtio10.h b/OvmfPkg/Include/IndustryStandard/Virtio10.h
index c5efb5cfcb8a..7d51aa36b326 100644
--- a/OvmfPkg/Include/IndustryStandard/Virtio10.h
+++ b/OvmfPkg/Include/IndustryStandard/Virtio10.h
@@ -16,6 +16,7 @@
 #ifndef _VIRTIO_1_0_H_
 #define _VIRTIO_1_0_H_
 
+#include <IndustryStandard/Pci23.h>
 #include <IndustryStandard/Virtio095.h>
 
 //
@@ -29,11 +30,7 @@
 //
 #pragma pack (1)
 typedef struct {
-  UINT8 CapId;   // Capability identifier (generic)
-  UINT8 CapNext; // Link to next capability (generic)
-} VIRTIO_PCI_CAP_LINK;
-
-typedef struct {
+  EFI_PCI_CAPABILITY_VENDOR_HDR VendorHdr;
   UINT8  ConfigType; // Identifies the specific VirtIo 1.0 config structure
   UINT8  Bar;        // The BAR that contains the structure
   UINT8  Padding[3];
diff --git a/OvmfPkg/Virtio10Dxe/Virtio10.c b/OvmfPkg/Virtio10Dxe/Virtio10.c
index e9b50b6e437b..9ebb72c76bfd 100644
--- a/OvmfPkg/Virtio10Dxe/Virtio10.c
+++ b/OvmfPkg/Virtio10Dxe/Virtio10.c
@@ -21,6 +21,8 @@
 #include <Library/BaseMemoryLib.h>
 #include <Library/DebugLib.h>
 #include <Library/MemoryAllocationLib.h>
+#include <Library/PciCapLib.h>
+#include <Library/PciCapPciIoLib.h>
 #include <Library/UefiBootServicesTableLib.h>
 #include <Library/UefiLib.h>
 
@@ -184,48 +186,6 @@ GetBarType (
 }
 
 
-/**
-  Read a slice from PCI config space at the given offset, then advance the
-  offset.
-
-  @param [in]     PciIo   The EFI_PCI_IO_PROTOCOL instance that represents the
-                          device.
-
-  @param [in,out] Offset  On input, the offset in PCI config space to start
-                          reading from. On output, the offset of the first byte
-                          that was not read. On error, Offset is not modified.
-
-  @param [in]     Size    The number of bytes to read.
-
-  @param [out]    Buffer  On output, the bytes read from PCI config space are
-                          stored in this object.
-
-  @retval EFI_SUCCESS  Size bytes have been transferred from PCI config space
-                       (from Offset) to Buffer, and Offset has been incremented
-                       by Size.
-
-  @return              Error codes from PciIo->Pci.Read().
-**/
-STATIC
-EFI_STATUS
-ReadConfigSpace (
-  IN     EFI_PCI_IO_PROTOCOL *PciIo,
-  IN OUT UINT32              *Offset,
-  IN     UINTN               Size,
-     OUT VOID                *Buffer
-  )
-{
-  EFI_STATUS Status;
-
-  Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint8, *Offset, Size, Buffer);
-  if (EFI_ERROR (Status)) {
-    return Status;
-  }
-  *Offset += (UINT32)Size;
-  return EFI_SUCCESS;
-}
-
-
 /*
   Traverse the PCI capabilities list of a virtio-1.0 device, and capture the
   locations of the interesting virtio-1.0 register blocks.
@@ -239,57 +199,51 @@ ReadConfigSpace (
                                 will have been updated from the PCI
                                 capabilities found.
 
-  @param[in]     CapabilityPtr  The offset of the first capability in PCI
-                                config space, taken from the standard PCI
-                                device header.
-
   @retval EFI_SUCCESS  Traversal successful.
 
-  @return              Error codes from the ReadConfigSpace() and GetBarType()
-                       helper functions.
+  @return              Error codes from PciCapPciIoLib, PciCapLib, and the
+                       GetBarType() helper function.
 */
 STATIC
 EFI_STATUS
 ParseCapabilities (
-  IN OUT VIRTIO_1_0_DEV *Device,
-  IN     UINT8          CapabilityPtr
+  IN OUT VIRTIO_1_0_DEV *Device
   )
 {
-  UINT32              Offset;
-  VIRTIO_PCI_CAP_LINK CapLink;
+  EFI_STATUS   Status;
+  PCI_CAP_DEV  *PciDevice;
+  PCI_CAP_LIST *CapList;
+  UINT16       VendorInstance;
+  PCI_CAP      *VendorCap;
 
-  for (Offset = CapabilityPtr & 0xFC;
-       Offset > 0;
-       Offset = CapLink.CapNext & 0xFC
-       ) {
-    EFI_STATUS        Status;
+  Status = PciCapPciIoDeviceInit (Device->PciIo, &PciDevice);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+  Status = PciCapListInit (PciDevice, &CapList);
+  if (EFI_ERROR (Status)) {
+    goto UninitPciDevice;
+  }
+
+  for (VendorInstance = 0;
+       !EFI_ERROR (PciCapListFindCap (CapList, PciCapNormal,
+                     EFI_PCI_CAPABILITY_ID_VENDOR, VendorInstance,
+                     &VendorCap));
+       VendorInstance++) {
     UINT8             CapLen;
     VIRTIO_PCI_CAP    VirtIoCap;
     VIRTIO_1_0_CONFIG *ParsedConfig;
 
-    //
-    // Read capability identifier and link to next capability.
-    //
-    Status = ReadConfigSpace (Device->PciIo, &Offset, sizeof CapLink,
-               &CapLink);
-    if (EFI_ERROR (Status)) {
-      return Status;
-    }
-    if (CapLink.CapId != 0x09) {
-      //
-      // Not a vendor-specific capability, move to the next one.
-      //
-      continue;
-    }
-
     //
     // Big enough to accommodate a VIRTIO_PCI_CAP structure?
     //
-    Status = ReadConfigSpace (Device->PciIo, &Offset, sizeof CapLen, &CapLen);
+    Status = PciCapRead (PciDevice, VendorCap,
+               OFFSET_OF (EFI_PCI_CAPABILITY_VENDOR_HDR, Length), &CapLen,
+               sizeof CapLen);
     if (EFI_ERROR (Status)) {
-      return Status;
+      goto UninitCapList;
     }
-    if (CapLen < sizeof CapLink + sizeof CapLen + sizeof VirtIoCap) {
+    if (CapLen < sizeof VirtIoCap) {
       //
       // Too small, move to next.
       //
@@ -299,11 +253,11 @@ ParseCapabilities (
     //
     // Read interesting part of capability.
     //
-    Status = ReadConfigSpace (Device->PciIo, &Offset, sizeof VirtIoCap,
-               &VirtIoCap);
+    Status = PciCapRead (PciDevice, VendorCap, 0, &VirtIoCap, sizeof VirtIoCap);
     if (EFI_ERROR (Status)) {
-      return Status;
+      goto UninitCapList;
     }
+
     switch (VirtIoCap.ConfigType) {
     case VIRTIO_PCI_CAP_COMMON_CFG:
       ParsedConfig = &Device->CommonConfig;
@@ -326,7 +280,7 @@ ParseCapabilities (
     //
     Status = GetBarType (Device->PciIo, VirtIoCap.Bar, &ParsedConfig->BarType);
     if (EFI_ERROR (Status)) {
-      return Status;
+      goto UninitCapList;
     }
     ParsedConfig->Bar    = VirtIoCap.Bar;
     ParsedConfig->Offset = VirtIoCap.Offset;
@@ -337,19 +291,18 @@ ParseCapabilities (
       // This capability has an additional field called NotifyOffsetMultiplier;
       // parse it too.
       //
-      if (CapLen < sizeof CapLink + sizeof CapLen + sizeof VirtIoCap +
-                   sizeof Device->NotifyOffsetMultiplier) {
+      if (CapLen < sizeof VirtIoCap + sizeof Device->NotifyOffsetMultiplier) {
         //
         // Too small, move to next.
         //
         continue;
       }
 
-      Status = ReadConfigSpace (Device->PciIo, &Offset,
-                 sizeof Device->NotifyOffsetMultiplier,
-                 &Device->NotifyOffsetMultiplier);
+      Status = PciCapRead (PciDevice, VendorCap, sizeof VirtIoCap,
+                 &Device->NotifyOffsetMultiplier,
+                 sizeof Device->NotifyOffsetMultiplier);
       if (EFI_ERROR (Status)) {
-        return Status;
+        goto UninitCapList;
       }
     }
 
@@ -359,7 +312,15 @@ ParseCapabilities (
     ParsedConfig->Exists = TRUE;
   }
 
-  return EFI_SUCCESS;
+  ASSERT_EFI_ERROR (Status);
+
+UninitCapList:
+  PciCapListUninit (CapList);
+
+UninitPciDevice:
+  PciCapPciIoDeviceUninit (PciDevice);
+
+  return Status;
 }
 
 
@@ -1015,7 +976,7 @@ Virtio10BindingStart (
 
   Device->VirtIo.SubSystemDeviceId = Pci.Hdr.DeviceId - 0x1040;
 
-  Status = ParseCapabilities (Device, Pci.Device.CapabilityPtr);
+  Status = ParseCapabilities (Device);
   if (EFI_ERROR (Status)) {
     goto ClosePciIo;
   }
-- 
2.14.1.3.gb7cf6e02401b



  parent reply	other threads:[~2018-05-23 20:21 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-23 20:21 [PATCH v2 0/7] OvmfPkg, ArmVirtPkg: add and use PCI(E) Capabilities Library Laszlo Ersek
2018-05-23 20:21 ` [PATCH v2 1/7] OvmfPkg: introduce PciCapLib Laszlo Ersek
2018-05-24  7:53   ` Ard Biesheuvel
2018-05-24 14:39     ` Laszlo Ersek
2018-05-24 14:41       ` Ard Biesheuvel
2018-05-24 17:25         ` Laszlo Ersek
2018-05-23 20:21 ` [PATCH v2 2/7] OvmfPkg: introduce PciCapPciSegmentLib Laszlo Ersek
2018-05-24  8:08   ` Ard Biesheuvel
2018-05-24 14:43     ` Laszlo Ersek
2018-05-24 14:55       ` Ard Biesheuvel
2018-05-23 20:21 ` [PATCH v2 3/7] OvmfPkg: introduce PciCapPciIoLib Laszlo Ersek
2018-05-24  8:13   ` Ard Biesheuvel
2018-05-24 14:50     ` Laszlo Ersek
2018-05-24 14:54       ` Ard Biesheuvel
2018-05-24 14:54         ` Ard Biesheuvel
2018-05-24 17:22         ` Laszlo Ersek
2018-05-23 20:21 ` [PATCH v2 4/7] OvmfPkg: resolve PciCapLib, PciCapPciSegmentLib, PciCapPciIoLib Laszlo Ersek
2018-05-24  8:14   ` Ard Biesheuvel
2018-05-23 20:21 ` [PATCH v2 5/7] ArmVirtPkg: " Laszlo Ersek
2018-05-24  8:14   ` Ard Biesheuvel
2018-05-23 20:21 ` [PATCH v2 6/7] OvmfPkg/PciHotPlugInitDxe: convert to PciCapLib Laszlo Ersek
2018-05-24  8:15   ` Ard Biesheuvel
2018-05-23 20:21 ` Laszlo Ersek [this message]
2018-05-24  8:16   ` [PATCH v2 7/7] OvmfPkg/Virtio10Dxe: " Ard Biesheuvel
2018-05-24 14:55     ` Laszlo Ersek
2018-05-24 20:04 ` [PATCH v2 0/7] OvmfPkg, ArmVirtPkg: add and use PCI(E) Capabilities Library Laszlo Ersek

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=20180523202121.8125-8-lersek@redhat.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