MdeModulePkg EhciDxe: Use common buffer for AsyncInterruptTransfer

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

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

This may impact the boot performance.

Since the data buffer for EhcMonitorAsyncRequests 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>
Reviewed-by: Hao Wu <hao.a.wu@intel.com>

diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
index 5569f4f9618b9f2b4997398e959c83f7c5e50258..764eeda58ba1fcacd152d6488638624a0ab96486 100644 (file)
--- a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
+++ b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
@@ -763,6 +763,7 @@ EhcControlTransfer (
           Translator,\r
           EHC_CTRL_TRANSFER,\r
           Request,\r
+          FALSE,\r
           Data,\r
           *DataLength,\r
           NULL,\r
@@ -906,6 +907,7 @@ EhcBulkTransfer (
           Translator,\r
           EHC_BULK_TRANSFER,\r
           NULL,\r
+          FALSE,\r
           Data[0],\r
           *DataLength,\r
           NULL,\r
@@ -1163,6 +1165,7 @@ EhcSyncInterruptTransfer (
           Translator,\r
           EHC_INT_TRANSFER_SYNC,\r
           NULL,\r
+          FALSE,\r
           Data,\r
           *DataLength,\r
           NULL,\r

diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c b/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c
index ec8d796fab110995d086ce722d7c7922d7bd40c7..b067fd02d1ce2e6c68262d83f22aea746f7df44b 100644 (file)
--- a/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c
+++ b/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c
@@ -778,7 +778,6 @@ EhciDelAsyncIntTransfer (
       EhcUnlinkQhFromPeriod (Ehc, Urb->Qh);\r
       RemoveEntryList (&Urb->UrbList);\r
 \r
-      gBS->FreePool (Urb->Data);\r
       EhcFreeUrb (Ehc, Urb);\r
       return EFI_SUCCESS;\r
     }\r
@@ -809,7 +808,6 @@ EhciDelAllAsyncIntTransfers (
     EhcUnlinkQhFromPeriod (Ehc, Urb->Qh);\r
     RemoveEntryList (&Urb->UrbList);\r
 \r
-    gBS->FreePool (Urb->Data);\r
     EhcFreeUrb (Ehc, Urb);\r
   }\r
 }\r
@@ -848,16 +846,8 @@ EhciInsertAsyncIntTransfer (
   IN UINTN                              Interval\r
   )\r
 {\r
-  VOID      *Data;\r
   URB       *Urb;\r
 \r
-  Data = AllocatePool (DataLen);\r
-\r
-  if (Data == NULL) {\r
-    DEBUG ((DEBUG_ERROR, "%a: failed to allocate buffer\n", __FUNCTION__));\r
-    return NULL;\r
-  }\r
-\r
   Urb = EhcCreateUrb (\r
           Ehc,\r
           DevAddr,\r
@@ -868,7 +858,8 @@ EhciInsertAsyncIntTransfer (
           Hub,\r
           EHC_INT_TRANSFER_ASYNC,\r
           NULL,\r
-          Data,\r
+          TRUE,\r
+          NULL,\r
           DataLen,\r
           Callback,\r
           Context,\r
@@ -877,7 +868,6 @@ EhciInsertAsyncIntTransfer (
 \r
   if (Urb == NULL) {\r
     DEBUG ((DEBUG_ERROR, "%a: failed to create URB\n", __FUNCTION__));\r
-    gBS->FreePool (Data);\r
     return NULL;\r
   }\r
 \r
@@ -891,60 +881,6 @@ EhciInsertAsyncIntTransfer (
   return Urb;\r
 }\r
 \r
-/**\r
-  Flush data from PCI controller specific address to mapped system\r
-  memory address.\r
-\r
-  @param  Ehc                The EHCI device.\r
-  @param  Urb                The URB to unmap.\r
-\r
-  @retval EFI_SUCCESS        Success to flush data to mapped system memory.\r
-  @retval EFI_DEVICE_ERROR   Fail to flush data to mapped system memory.\r
-\r
-**/\r
-EFI_STATUS\r
-EhcFlushAsyncIntMap (\r
-  IN  USB2_HC_DEV         *Ehc,\r
-  IN  URB                 *Urb\r
-  )\r
-{\r
-  EFI_STATUS                    Status;\r
-  EFI_PHYSICAL_ADDRESS          PhyAddr;\r
-  EFI_PCI_IO_PROTOCOL_OPERATION MapOp;\r
-  EFI_PCI_IO_PROTOCOL           *PciIo;\r
-  UINTN                         Len;\r
-  VOID                          *Map;\r
-\r
-  PciIo = Ehc->PciIo;\r
-  Len   = Urb->DataLen;\r
-\r
-  if (Urb->Ep.Direction == EfiUsbDataIn) {\r
-    MapOp = EfiPciIoOperationBusMasterWrite;\r
-  } else {\r
-    MapOp = EfiPciIoOperationBusMasterRead;\r
-  }\r
-\r
-  Status = PciIo->Unmap (PciIo, Urb->DataMap);\r
-  if (EFI_ERROR (Status)) {\r
-    goto ON_ERROR;\r
-  }\r
-\r
-  Urb->DataMap = NULL;\r
-\r
-  Status = PciIo->Map (PciIo, MapOp, Urb->Data, &Len, &PhyAddr, &Map);\r
-  if (EFI_ERROR (Status) || (Len != Urb->DataLen)) {\r
-    goto ON_ERROR;\r
-  }\r
-\r
-  Urb->DataPhy  = (VOID *) ((UINTN) PhyAddr);\r
-  Urb->DataMap  = Map;\r
-  return EFI_SUCCESS;\r
-\r
-ON_ERROR:\r
-  return EFI_DEVICE_ERROR;\r
-}\r
-\r
-\r
 /**\r
   Update the queue head for next round of asynchronous transfer.\r
 \r
@@ -1050,7 +986,6 @@ EhcMonitorAsyncRequests (
   BOOLEAN                 Finished;\r
   UINT8                   *ProcBuf;\r
   URB                     *Urb;\r
-  EFI_STATUS              Status;\r
 \r
   OldTpl  = gBS->RaiseTPL (EHC_TPL);\r
   Ehc     = (USB2_HC_DEV *) Context;\r
@@ -1068,15 +1003,6 @@ EhcMonitorAsyncRequests (
       continue;\r
     }\r
 \r
-    //\r
-    // Flush any PCI posted write transactions from a PCI host\r
-    // bridge to system memory.\r
-    //\r
-    Status = EhcFlushAsyncIntMap (Ehc, Urb);\r
-    if (EFI_ERROR (Status)) {\r
-      DEBUG ((EFI_D_ERROR, "EhcMonitorAsyncRequests: Fail to Flush AsyncInt Mapped Memeory\n"));\r
-    }\r
-\r
     //\r
     // Allocate a buffer then copy the transferred data for user.\r
     // If failed to allocate the buffer, update the URB for next\r

diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/EhciUrb.c b/MdeModulePkg/Bus/Pci/EhciDxe/EhciUrb.c
index 6afb327df165aa81cf559a903a090391fa9e82bf..09c12776ecdbe7445149a95f6c78c690c477d513 100644 (file)
--- a/MdeModulePkg/Bus/Pci/EhciDxe/EhciUrb.c
+++ b/MdeModulePkg/Bus/Pci/EhciDxe/EhciUrb.c
@@ -339,6 +339,14 @@ EhcFreeUrb (
     PciIo->Unmap (PciIo, Urb->DataMap);\r
   }\r
 \r
+  if (Urb->AllocateCommonBuffer) {\r
+    PciIo->FreeBuffer (\r
+             PciIo,\r
+             EFI_SIZE_TO_PAGES (Urb->DataLen),\r
+             Urb->Data\r
+             );\r
+  }\r
+\r
   if (Urb->Qh != NULL) {\r
     //\r
     // Ensure that this queue head has been unlinked from the\r
@@ -529,7 +537,8 @@ ON_ERROR:
   @param  Hub                   The transaction translator to use.\r
   @param  Type                  The transaction type.\r
   @param  Request               The standard USB request for control transfer.\r
-  @param  Data                  The user data to transfer.\r
+  @param  AllocateCommonBuffer  Indicate whether need to allocate common buffer for data transfer.\r
+  @param  Data                  The user data to transfer, NULL if AllocateCommonBuffer is TRUE.\r
   @param  DataLen               The length of data buffer.\r
   @param  Callback              The function to call when data is transferred.\r
   @param  Context               The context to the callback.\r
@@ -549,6 +558,7 @@ EhcCreateUrb (
   IN EFI_USB2_HC_TRANSACTION_TRANSLATOR *Hub,\r
   IN UINTN                              Type,\r
   IN EFI_USB_DEVICE_REQUEST             *Request,\r
+  IN BOOLEAN                            AllocateCommonBuffer,\r
   IN VOID                               *Data,\r
   IN UINTN                              DataLen,\r
   IN EFI_ASYNC_USB_TRANSFER_CALLBACK    Callback,\r
@@ -596,8 +606,24 @@ EhcCreateUrb (
   Ep->PollRate    = EhcConvertPollRate (Interval);\r
 \r
   Urb->Request    = Request;\r
+  if (AllocateCommonBuffer) {\r
+    ASSERT (Data == NULL);\r
+    Status = Ehc->PciIo->AllocateBuffer (\r
+                           Ehc->PciIo,\r
+                           AllocateAnyPages,\r
+                           EfiBootServicesData,\r
+                           EFI_SIZE_TO_PAGES (DataLen),\r
+                           &Data,\r
+                           0\r
+                           );\r
+    if (EFI_ERROR (Status) || (Data == NULL)) {\r
+      FreePool (Urb);\r
+      return NULL;\r
+    }\r
+  }\r
   Urb->Data       = Data;\r
   Urb->DataLen    = DataLen;\r
+  Urb->AllocateCommonBuffer = AllocateCommonBuffer;\r
   Urb->Callback   = Callback;\r
   Urb->Context    = Context;\r
 \r
@@ -627,10 +653,14 @@ EhcCreateUrb (
   if (Data != NULL) {\r
     Len     = DataLen;\r
 \r
-    if (Ep->Direction == EfiUsbDataIn) {\r
-      MapOp = EfiPciIoOperationBusMasterWrite;\r
+    if (Urb->AllocateCommonBuffer) {\r
+      MapOp = EfiPciIoOperationBusMasterCommonBuffer;\r
     } else {\r
-      MapOp = EfiPciIoOperationBusMasterRead;\r
+      if (Ep->Direction == EfiUsbDataIn) {\r
+        MapOp = EfiPciIoOperationBusMasterWrite;\r
+      } else {\r
+        MapOp = EfiPciIoOperationBusMasterRead;\r
+      }\r
     }\r
 \r
     Status  = PciIo->Map (PciIo, MapOp, Data, &Len, &PhyAddr, &Map);\r

diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/EhciUrb.h b/MdeModulePkg/Bus/Pci/EhciDxe/EhciUrb.h
index 02e9af81be391d873d975a68ba09d70600248ca4..cb3599f9d36112be382ffd37e4893d2e5b827443 100644 (file)
--- a/MdeModulePkg/Bus/Pci/EhciDxe/EhciUrb.h
+++ b/MdeModulePkg/Bus/Pci/EhciDxe/EhciUrb.h
@@ -3,7 +3,7 @@
     This file contains URB request, each request is warpped in a\r
     URB (Usb Request Block).\r
 \r
-Copyright (c) 2007 - 2010, Intel Corporation. All rights reserved.<BR>\r
+Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.<BR>\r
 This program and the accompanying materials\r
 are licensed and made available under the terms and conditions of the BSD License\r
 which accompanies this distribution.  The full text of the license may be found at\r
@@ -216,6 +216,7 @@ struct _URB {
   EFI_USB_DEVICE_REQUEST          *Request;     // Control transfer only\r
   VOID                            *RequestPhy;  // Address of the mapped request\r
   VOID                            *RequestMap;\r
+  BOOLEAN                         AllocateCommonBuffer;\r
   VOID                            *Data;\r
   UINTN                           DataLen;\r
   VOID                            *DataPhy;     // Address of the mapped user data\r
@@ -298,20 +299,21 @@ EhcFreeUrb (
 /**\r
   Create a new URB and its associated QTD.\r
 \r
-  @param  Ehc        The EHCI device.\r
-  @param  DevAddr    The device address.\r
-  @param  EpAddr     Endpoint addrress & its direction.\r
-  @param  DevSpeed   The device speed.\r
-  @param  Toggle     Initial data toggle to use.\r
-  @param  MaxPacket  The max packet length of the endpoint.\r
-  @param  Hub        The transaction translator to use.\r
-  @param  Type       The transaction type.\r
-  @param  Request    The standard USB request for control transfer.\r
-  @param  Data       The user data to transfer.\r
-  @param  DataLen    The length of data buffer.\r
-  @param  Callback   The function to call when data is transferred.\r
-  @param  Context    The context to the callback.\r
-  @param  Interval   The interval for interrupt transfer.\r
+  @param  Ehc                   The EHCI device.\r
+  @param  DevAddr               The device address.\r
+  @param  EpAddr                Endpoint addrress & its direction.\r
+  @param  DevSpeed              The device speed.\r
+  @param  Toggle                Initial data toggle to use.\r
+  @param  MaxPacket             The max packet length of the endpoint.\r
+  @param  Hub                   The transaction translator to use.\r
+  @param  Type                  The transaction type.\r
+  @param  Request               The standard USB request for control transfer.\r
+  @param  AllocateCommonBuffer  Indicate whether need to allocate common buffer for data transfer.\r
+  @param  Data                  The user data to transfer, NULL if AllocateCommonBuffer is TRUE.\r
+  @param  DataLen               The length of data buffer.\r
+  @param  Callback              The function to call when data is transferred.\r
+  @param  Context               The context to the callback.\r
+  @param  Interval              The interval for interrupt transfer.\r
 \r
   @return Created URB or NULL.\r
 \r
@@ -327,6 +329,7 @@ EhcCreateUrb (
   IN EFI_USB2_HC_TRANSACTION_TRANSLATOR *Hub,\r
   IN UINTN                              Type,\r
   IN EFI_USB_DEVICE_REQUEST             *Request,\r
+  IN BOOLEAN                            AllocateCommonBuffer,\r
   IN VOID                               *Data,\r
   IN UINTN                              DataLen,\r
   IN EFI_ASYNC_USB_TRANSFER_CALLBACK    Callback,\r