From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.88; helo=mga01.intel.com; envelope-from=star.zeng@intel.com; receiver=edk2-devel@lists.01.org Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 9A69B2117CEBE for ; Tue, 6 Nov 2018 06:34:33 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 06 Nov 2018 06:34:33 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,472,1534834800"; d="scan'208";a="94069933" Received: from shwdeopenpsi068.ccr.corp.intel.com ([10.239.158.46]) by FMSMGA003.fm.intel.com with ESMTP; 06 Nov 2018 06:34:33 -0800 From: Star Zeng To: edk2-devel@lists.01.org Cc: Star Zeng Date: Tue, 6 Nov 2018 22:34:26 +0800 Message-Id: <1541514867-173440-3-git-send-email-star.zeng@intel.com> X-Mailer: git-send-email 2.7.0.windows.1 In-Reply-To: <1541514867-173440-1-git-send-email-star.zeng@intel.com> References: <1541514867-173440-1-git-send-email-star.zeng@intel.com> Subject: [PATCH 2/2] Revert "XhciDxe: Use common buffer for AsyncInterruptTransfer" X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 06 Nov 2018 14:34:33 -0000 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 --- 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