Revert "XhciDxe: Use common buffer for AsyncInterruptTransfer"

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>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>

3 files changed, 103 insertions, 63 deletions

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
index 64855a4..7f64f9c 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 9dd45e9..75959ae 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 1a95d98..b5e192c 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,