From d98fc9adfb6f4d0da5556c4a3592f01fb8731b24 Mon Sep 17 00:00:00 2001 From: Star Zeng Date: Tue, 6 Nov 2018 21:47:39 +0800 Subject: [PATCH] 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 Reviewed-by: Ruiyu Ni --- 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 64855a4c15..7f64f9c7c9 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 9dd45e93a2..75959ae083 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; } @@ -1523,6 +1502,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. @@ -1543,6 +1577,7 @@ XhcMonitorAsyncRequests ( UINT8 *ProcBuf; URB *Urb; UINT8 SlotId; + EFI_STATUS Status; EFI_TPL OldTpl; OldTpl = gBS->RaiseTPL (XHC_TPL); @@ -1570,6 +1605,15 @@ XhcMonitorAsyncRequests ( continue; } + // + // 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 diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h index 1a95d98996..b5e192c3b5 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,