[PATCH V3 3/4] MdeModulePkg XhciDxe: Use common buffer for AsyncInterruptTransfer

[Star Zeng <star.zeng@intel.com>]

V3:
Call XhcFreeUrb after XhcCreateTransferTrb fails in XhcCreateTrb.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1274

In current code, XhcMonitorAsyncRequests (timer handler) will do
unmap and map operations for AsyncIntTransfers to "Flush data from
PCI controller specific address to mapped system memory address".
XhcMonitorAsyncRequests
  XhcFlushAsyncIntMap
    PciIo->Unmap
      IoMmu->SetAttribute
    PciIo->Map
      IoMmu->SetAttribute

This may impact the boot performance.

Since the data buffer for XhcMonitorAsyncRequests is internal
buffer, we can allocate common buffer by PciIo->AllocateBuffer
and map the buffer with EfiPciIoOperationBusMasterCommonBuffer,
then the unmap and map operations can be removed.

///
/// Provides both read and write access to system memory by
/// both the processor and a bus master. The buffer is coherent
/// from both the processor's and the bus master's point of view.
///
EfiPciIoOperationBusMasterCommonBuffer,

Test done:
USB KB works normally.
USB disk read/write works normally.

Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
---
 MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c      |   1 +
 MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 138 +++++++++++--------------------
 MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h |  27 +++---
 3 files changed, 63 insertions(+), 103 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
index 7f64f9c7c982..64855a4c158c 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
@@ -769,6 +769,7 @@ XhcTransfer (
           MaximumPacketLength,
           Type,
           Request,
+          FALSE,
           Data,
           *DataLength,
           NULL,
diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
index 75959ae08363..9dd45e93a272 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
@@ -118,17 +118,18 @@ ON_EXIT:
 /**
   Create a new URB for a new transaction.
 
-  @param  Xhc       The XHCI Instance
-  @param  BusAddr   The logical device address assigned by UsbBus driver
-  @param  EpAddr    Endpoint addrress
-  @param  DevSpeed  The device speed
-  @param  MaxPacket The max packet length of the endpoint
-  @param  Type      The transaction type
-  @param  Request   The standard USB request for control transfer
-  @param  Data      The user data to transfer
-  @param  DataLen   The length of data buffer
-  @param  Callback  The function to call when data is transferred
-  @param  Context   The context to the callback
+  @param  Xhc                   The XHCI Instance
+  @param  BusAddr               The logical device address assigned by UsbBus driver
+  @param  EpAddr                Endpoint addrress
+  @param  DevSpeed              The device speed
+  @param  MaxPacket             The max packet length of the endpoint
+  @param  Type                  The transaction type
+  @param  Request               The standard USB request for control transfer
+  @param  AllocateCommonBuffer  Indicate whether need to allocate common buffer for data transfer
+  @param  Data                  The user data to transfer, NULL if AllocateCommonBuffer is TRUE
+  @param  DataLen               The length of data buffer
+  @param  Callback              The function to call when data is transferred
+  @param  Context               The context to the callback
 
   @return Created URB or NULL
 
@@ -142,6 +143,7 @@ XhcCreateUrb (
   IN UINTN                              MaxPacket,
   IN UINTN                              Type,
   IN EFI_USB_DEVICE_REQUEST             *Request,
+  IN BOOLEAN                            AllocateCommonBuffer,
   IN VOID                               *Data,
   IN UINTN                              DataLen,
   IN EFI_ASYNC_USB_TRANSFER_CALLBACK    Callback,
@@ -169,8 +171,24 @@ XhcCreateUrb (
   Ep->Type      = Type;
 
   Urb->Request  = Request;
+  if (AllocateCommonBuffer) {
+    ASSERT (Data == NULL);
+    Status = Xhc->PciIo->AllocateBuffer (
+                           Xhc->PciIo,
+                           AllocateAnyPages,
+                           EfiBootServicesData,
+                           EFI_SIZE_TO_PAGES (DataLen),
+                           &Data,
+                           0
+                           );
+    if (EFI_ERROR (Status) || (Data == NULL)) {
+      FreePool (Urb);
+      return NULL;
+    }
+  }
   Urb->Data     = Data;
   Urb->DataLen  = DataLen;
+  Urb->AllocateCommonBuffer = AllocateCommonBuffer;
   Urb->Callback = Callback;
   Urb->Context  = Context;
 
@@ -178,7 +196,7 @@ XhcCreateUrb (
   ASSERT_EFI_ERROR (Status);
   if (EFI_ERROR (Status)) {
     DEBUG ((EFI_D_ERROR, "XhcCreateUrb: XhcCreateTransferTrb Failed, Status = %r\n", Status));
-    FreePool (Urb);
+    XhcFreeUrb (Xhc, Urb);
     Urb = NULL;
   }
 
@@ -206,6 +224,14 @@ XhcFreeUrb (
     Xhc->PciIo->Unmap (Xhc->PciIo, Urb->DataMap);
   }
 
+  if (Urb->AllocateCommonBuffer) {
+    Xhc->PciIo->FreeBuffer (
+                  Xhc->PciIo,
+                  EFI_SIZE_TO_PAGES (Urb->DataLen),
+                  Urb->Data
+                  );
+  }
+
   FreePool (Urb);
 }
 
@@ -264,10 +290,14 @@ XhcCreateTransferTrb (
   // No need to remap.
   //
   if ((Urb->Data != NULL) && (Urb->DataMap == NULL)) {
-    if (((UINT8) (Urb->Ep.Direction)) == EfiUsbDataIn) {
-      MapOp = EfiPciIoOperationBusMasterWrite;
+    if (Urb->AllocateCommonBuffer) {
+      MapOp = EfiPciIoOperationBusMasterCommonBuffer;
     } else {
-      MapOp = EfiPciIoOperationBusMasterRead;
+      if (((UINT8) (Urb->Ep.Direction)) == EfiUsbDataIn) {
+        MapOp = EfiPciIoOperationBusMasterWrite;
+      } else {
+        MapOp = EfiPciIoOperationBusMasterRead;
+      }
     }
 
     Len = Urb->DataLen;
@@ -1367,7 +1397,6 @@ XhciDelAsyncIntTransfer (
       }
 
       RemoveEntryList (&Urb->UrbList);
-      FreePool (Urb->Data);
       XhcFreeUrb (Xhc, Urb);
       return EFI_SUCCESS;
     }
@@ -1405,7 +1434,6 @@ XhciDelAllAsyncIntTransfers (
     }
 
     RemoveEntryList (&Urb->UrbList);
-    FreePool (Urb->Data);
     XhcFreeUrb (Xhc, Urb);
   }
 }
@@ -1438,15 +1466,8 @@ XhciInsertAsyncIntTransfer (
   IN VOID                               *Context
   )
 {
-  VOID      *Data;
   URB       *Urb;
 
-  Data = AllocateZeroPool (DataLen);
-  if (Data == NULL) {
-    DEBUG ((DEBUG_ERROR, "%a: failed to allocate buffer\n", __FUNCTION__));
-    return NULL;
-  }
-
   Urb = XhcCreateUrb (
           Xhc,
           BusAddr,
@@ -1455,14 +1476,14 @@ XhciInsertAsyncIntTransfer (
           MaxPacket,
           XHC_INT_TRANSFER_ASYNC,
           NULL,
-          Data,
+          TRUE,
+          NULL,
           DataLen,
           Callback,
           Context
           );
   if (Urb == NULL) {
     DEBUG ((DEBUG_ERROR, "%a: failed to create URB\n", __FUNCTION__));
-    FreePool (Data);
     return NULL;
   }
 
@@ -1503,61 +1524,6 @@ XhcUpdateAsyncRequest (
 }
 
 /**
-  Flush data from PCI controller specific address to mapped system
-  memory address.
-
-  @param  Xhc                The XHCI device.
-  @param  Urb                The URB to unmap.
-
-  @retval EFI_SUCCESS        Success to flush data to mapped system memory.
-  @retval EFI_DEVICE_ERROR   Fail to flush data to mapped system memory.
-
-**/
-EFI_STATUS
-XhcFlushAsyncIntMap (
-  IN  USB_XHCI_INSTANCE   *Xhc,
-  IN  URB                 *Urb
-  )
-{
-  EFI_STATUS                    Status;
-  EFI_PHYSICAL_ADDRESS          PhyAddr;
-  EFI_PCI_IO_PROTOCOL_OPERATION MapOp;
-  EFI_PCI_IO_PROTOCOL           *PciIo;
-  UINTN                         Len;
-  VOID                          *Map;
-
-  PciIo = Xhc->PciIo;
-  Len   = Urb->DataLen;
-
-  if (Urb->Ep.Direction == EfiUsbDataIn) {
-    MapOp = EfiPciIoOperationBusMasterWrite;
-  } else {
-    MapOp = EfiPciIoOperationBusMasterRead;
-  }
-
-  if (Urb->DataMap != NULL) {
-    Status = PciIo->Unmap (PciIo, Urb->DataMap);
-    if (EFI_ERROR (Status)) {
-      goto ON_ERROR;
-    }
-  }
-
-  Urb->DataMap = NULL;
-
-  Status = PciIo->Map (PciIo, MapOp, Urb->Data, &Len, &PhyAddr, &Map);
-  if (EFI_ERROR (Status) || (Len != Urb->DataLen)) {
-    goto ON_ERROR;
-  }
-
-  Urb->DataPhy  = (VOID *) ((UINTN) PhyAddr);
-  Urb->DataMap  = Map;
-  return EFI_SUCCESS;
-
-ON_ERROR:
-  return EFI_DEVICE_ERROR;
-}
-
-/**
   Interrupt transfer periodic check handler.
 
   @param  Event                 Interrupt event.
@@ -1577,7 +1543,6 @@ XhcMonitorAsyncRequests (
   UINT8                   *ProcBuf;
   URB                     *Urb;
   UINT8                   SlotId;
-  EFI_STATUS              Status;
   EFI_TPL                 OldTpl;
 
   OldTpl = gBS->RaiseTPL (XHC_TPL);
@@ -1606,15 +1571,6 @@ XhcMonitorAsyncRequests (
     }
 
     //
-    // Flush any PCI posted write transactions from a PCI host
-    // bridge to system memory.
-    //
-    Status = XhcFlushAsyncIntMap (Xhc, Urb);
-    if (EFI_ERROR (Status)) {
-      DEBUG ((EFI_D_ERROR, "XhcMonitorAsyncRequests: Fail to Flush AsyncInt Mapped Memeory\n"));
-    }
-
-    //
     // Allocate a buffer then copy the transferred data for user.
     // If failed to allocate the buffer, update the URB for next
     // round of transfer. Ignore the data of this round.
diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h
index b5e192c3b589..1a95d98996c1 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h
@@ -172,6 +172,7 @@ typedef struct _URB {
   //
   USB_ENDPOINT                    Ep;
   EFI_USB_DEVICE_REQUEST          *Request;
+  BOOLEAN                         AllocateCommonBuffer;
   VOID                            *Data;
   UINTN                           DataLen;
   VOID                            *DataPhy;
@@ -1432,17 +1433,18 @@ XhcSetTrDequeuePointer (
 /**
   Create a new URB for a new transaction.
 
-  @param  Xhc       The XHCI Instance
-  @param  DevAddr   The device address
-  @param  EpAddr    Endpoint addrress
-  @param  DevSpeed  The device speed
-  @param  MaxPacket The max packet length of the endpoint
-  @param  Type      The transaction type
-  @param  Request   The standard USB request for control transfer
-  @param  Data      The user data to transfer
-  @param  DataLen   The length of data buffer
-  @param  Callback  The function to call when data is transferred
-  @param  Context   The context to the callback
+  @param  Xhc                   The XHCI Instance
+  @param  BusAddr               The logical device address assigned by UsbBus driver
+  @param  EpAddr                Endpoint addrress
+  @param  DevSpeed              The device speed
+  @param  MaxPacket             The max packet length of the endpoint
+  @param  Type                  The transaction type
+  @param  Request               The standard USB request for control transfer
+  @param  AllocateCommonBuffer  Indicate whether need to allocate common buffer for data transfer
+  @param  Data                  The user data to transfer, NULL if AllocateCommonBuffer is TRUE
+  @param  DataLen               The length of data buffer
+  @param  Callback              The function to call when data is transferred
+  @param  Context               The context to the callback
 
   @return Created URB or NULL
 
@@ -1450,12 +1452,13 @@ XhcSetTrDequeuePointer (
 URB*
 XhcCreateUrb (
   IN USB_XHCI_INSTANCE                  *Xhc,
-  IN UINT8                              DevAddr,
+  IN UINT8                              BusAddr,
   IN UINT8                              EpAddr,
   IN UINT8                              DevSpeed,
   IN UINTN                              MaxPacket,
   IN UINTN                              Type,
   IN EFI_USB_DEVICE_REQUEST             *Request,
+  IN BOOLEAN                            AllocateCommonBuffer,
   IN VOID                               *Data,
   IN UINTN                              DataLen,
   IN EFI_ASYNC_USB_TRANSFER_CALLBACK    Callback,
-- 
2.7.0.windows.1