[PATCH 2/2] Revert "XhciDxe: Use common buffer for AsyncInterruptTransfer"

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

There is concern at the thread
https://lists.01.org/pipermail/edk2-devel/2018-November/031951.html.
And the time point is a little sensitive as it is near edk2-stable201811.

This reverts commit 777920997152a2e68f664241f6080b64ff21edd6.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@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, 103 insertions(+), 63 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
index 64855a4c158c..7f64f9c7c982 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
@@ -769,7 +769,6 @@ 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 9dd45e93a272..75959ae08363 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
@@ -118,18 +118,17 @@ 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  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
+  @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
 
   @return Created URB or NULL
 
@@ -143,7 +142,6 @@ 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,
@@ -171,24 +169,8 @@ 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;
 
@@ -196,7 +178,7 @@ XhcCreateUrb (
   ASSERT_EFI_ERROR (Status);
   if (EFI_ERROR (Status)) {
     DEBUG ((EFI_D_ERROR, "XhcCreateUrb: XhcCreateTransferTrb Failed, Status = %r\n", Status));
-    XhcFreeUrb (Xhc, Urb);
+    FreePool (Urb);
     Urb = NULL;
   }
 
@@ -224,14 +206,6 @@ 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);
 }
 
@@ -290,14 +264,10 @@ XhcCreateTransferTrb (
   // No need to remap.
   //
   if ((Urb->Data != NULL) && (Urb->DataMap == NULL)) {
-    if (Urb->AllocateCommonBuffer) {
-      MapOp = EfiPciIoOperationBusMasterCommonBuffer;
+    if (((UINT8) (Urb->Ep.Direction)) == EfiUsbDataIn) {
+      MapOp = EfiPciIoOperationBusMasterWrite;
     } else {
-      if (((UINT8) (Urb->Ep.Direction)) == EfiUsbDataIn) {
-        MapOp = EfiPciIoOperationBusMasterWrite;
-      } else {
-        MapOp = EfiPciIoOperationBusMasterRead;
-      }
+      MapOp = EfiPciIoOperationBusMasterRead;
     }
 
     Len = Urb->DataLen;
@@ -1397,6 +1367,7 @@ XhciDelAsyncIntTransfer (
       }
 
       RemoveEntryList (&Urb->UrbList);
+      FreePool (Urb->Data);
       XhcFreeUrb (Xhc, Urb);
       return EFI_SUCCESS;
     }
@@ -1434,6 +1405,7 @@ XhciDelAllAsyncIntTransfers (
     }
 
     RemoveEntryList (&Urb->UrbList);
+    FreePool (Urb->Data);
     XhcFreeUrb (Xhc, Urb);
   }
 }
@@ -1466,8 +1438,15 @@ 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,
@@ -1476,14 +1455,14 @@ XhciInsertAsyncIntTransfer (
           MaxPacket,
           XHC_INT_TRANSFER_ASYNC,
           NULL,
-          TRUE,
-          NULL,
+          Data,
           DataLen,
           Callback,
           Context
           );
   if (Urb == NULL) {
     DEBUG ((DEBUG_ERROR, "%a: failed to create URB\n", __FUNCTION__));
+    FreePool (Data);
     return NULL;
   }
 
@@ -1524,6 +1503,61 @@ 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.
@@ -1543,6 +1577,7 @@ XhcMonitorAsyncRequests (
   UINT8                   *ProcBuf;
   URB                     *Urb;
   UINT8                   SlotId;
+  EFI_STATUS              Status;
   EFI_TPL                 OldTpl;
 
   OldTpl = gBS->RaiseTPL (XHC_TPL);
@@ -1571,6 +1606,15 @@ 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 1a95d98996c1..b5e192c3b589 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h
@@ -172,7 +172,6 @@ typedef struct _URB {
   //
   USB_ENDPOINT                    Ep;
   EFI_USB_DEVICE_REQUEST          *Request;
-  BOOLEAN                         AllocateCommonBuffer;
   VOID                            *Data;
   UINTN                           DataLen;
   VOID                            *DataPhy;
@@ -1433,18 +1432,17 @@ XhcSetTrDequeuePointer (
 /**
   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  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
+  @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
 
   @return Created URB or NULL
 
@@ -1452,13 +1450,12 @@ XhcSetTrDequeuePointer (
 URB*
 XhcCreateUrb (
   IN USB_XHCI_INSTANCE                  *Xhc,
-  IN UINT8                              BusAddr,
+  IN UINT8                              DevAddr,
   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