From: Star Zeng <star.zeng@intel.com>
To: edk2-devel@lists.01.org
Cc: Star Zeng <star.zeng@intel.com>, Ruiyu Ni <ruiyu.ni@intel.com>,
Hao Wu <hao.a.wu@intel.com>, Jian J Wang <jian.j.wang@intel.com>,
Jiewen Yao <jiewen.yao@intel.com>
Subject: [PATCH V2 3/4] MdeModulePkg XhciDxe: Use common buffer for AsyncInterruptTransfer
Date: Fri, 26 Oct 2018 13:42:20 +0800 [thread overview]
Message-ID: <1540532541-115032-4-git-send-email-star.zeng@intel.com> (raw)
In-Reply-To: <1540532541-115032-1-git-send-email-star.zeng@intel.com>
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 | 141 +++++++++++--------------------
MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h | 27 +++---
3 files changed, 67 insertions(+), 102 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..d03a6681ce0d 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,6 +196,11 @@ XhcCreateUrb (
ASSERT_EFI_ERROR (Status);
if (EFI_ERROR (Status)) {
DEBUG ((EFI_D_ERROR, "XhcCreateUrb: XhcCreateTransferTrb Failed, Status = %r\n", Status));
+ Xhc->PciIo->FreeBuffer (
+ Xhc->PciIo,
+ EFI_SIZE_TO_PAGES (Urb->DataLen),
+ Urb->Data
+ );
FreePool (Urb);
Urb = NULL;
}
@@ -206,6 +229,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 +295,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 +1402,6 @@ XhciDelAsyncIntTransfer (
}
RemoveEntryList (&Urb->UrbList);
- FreePool (Urb->Data);
XhcFreeUrb (Xhc, Urb);
return EFI_SUCCESS;
}
@@ -1405,7 +1439,6 @@ XhciDelAllAsyncIntTransfers (
}
RemoveEntryList (&Urb->UrbList);
- FreePool (Urb->Data);
XhcFreeUrb (Xhc, Urb);
}
}
@@ -1438,15 +1471,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 +1481,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 +1529,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 +1548,6 @@ XhcMonitorAsyncRequests (
UINT8 *ProcBuf;
URB *Urb;
UINT8 SlotId;
- EFI_STATUS Status;
EFI_TPL OldTpl;
OldTpl = gBS->RaiseTPL (XHC_TPL);
@@ -1606,15 +1576,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 cd1403f2842a..2b5b95b7fb60 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
next prev parent reply other threads:[~2018-10-26 5:42 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-26 5:42 [PATCH V2 0/4] Remove unnecessary Map/Unmap in XhciDxe/EhciDxe for AsyncInterruptTransfer Star Zeng
2018-10-26 5:42 ` [PATCH V2 1/4] MdeModulePkg XhciDxe: Extract new XhciInsertAsyncIntTransfer function Star Zeng
2018-10-26 7:33 ` Wu, Hao A
2018-10-26 13:42 ` Zeng, Star
2018-10-26 5:42 ` [PATCH V2 2/4] MdeModulePkg EhciDxe: Extract new EhciInsertAsyncIntTransfer function Star Zeng
2018-10-26 7:54 ` Wu, Hao A
2018-10-26 5:42 ` Star Zeng [this message]
2018-10-26 7:38 ` [PATCH V2 3/4] MdeModulePkg XhciDxe: Use common buffer for AsyncInterruptTransfer Wu, Hao A
2018-10-26 8:14 ` Wu, Hao A
2018-10-26 13:43 ` Zeng, Star
2018-10-26 5:42 ` [PATCH V2 4/4] MdeModulePkg EhciDxe: " Star Zeng
2018-10-26 8:12 ` Wu, Hao A
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=1540532541-115032-4-git-send-email-star.zeng@intel.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