[PATCH] MdeModulePkg XhciDxe: Fix Map and Unmap inconsistency

[PATCH] MdeModulePkg XhciDxe: Fix Map and Unmap inconsistency

[Star Zeng]

We found there are loops of *2* Maps and only *1* Unmap and
the DMA buffer address is decreasing.

It is caused by the below code flow.
XhcAsyncInterruptTransfer ->
  XhcCreateUrb ->
    XhcCreateTransferTrb ->
      Map Urb->DataMap           (1)

Timer: loops of *2* Maps and only *1* Unmap
XhcMonitorAsyncRequests ->
  XhcFlushAsyncIntMap ->
    Unmap and Map Urb->DataMap   (2)
  XhcUpdateAsyncRequest ->
    XhcCreateTransferTrb ->
      Map Urb->DataMap           (3)

This patch is to eliminate (3).

Cc: Ruiyu Ni <ruiyu.ni@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>
---
 MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
index 6013d7620fe5..21fdcf130a50 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
@@ -259,8 +259,11 @@ XhcCreateTransferTrb (
   } else {
     EPType  = (UINT8) ((DEVICE_CONTEXT_64 *)OutputContext)->EP[Dci-1].EPType;
   }
-  
-  if (Urb->Data != NULL) {
+
+  //
+  // No need to remap.
+  //
+  if ((Urb->Data != NULL) && (Urb->DataMap == NULL)) {
     if (((UINT8) (Urb->Ep.Direction)) == EfiUsbDataIn) {
       MapOp = EfiPciIoOperationBusMasterWrite;
     } else {
-- 
2.7.0.windows.1

Re: [PATCH] MdeModulePkg XhciDxe: Fix Map and Unmap inconsistency

[Ni, Ruiyu]

Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>

Thanks/Ray

> -----Original Message-----
> From: Zeng, Star
> Sent: Thursday, August 24, 2017 5:47 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Yao,
> Jiewen <jiewen.yao@intel.com>
> Subject: [PATCH] MdeModulePkg XhciDxe: Fix Map and Unmap
> inconsistency
> 
> We found there are loops of *2* Maps and only *1* Unmap and the DMA
> buffer address is decreasing.
> 
> It is caused by the below code flow.
> XhcAsyncInterruptTransfer ->
>   XhcCreateUrb ->
>     XhcCreateTransferTrb ->
>       Map Urb->DataMap           (1)
> 
> Timer: loops of *2* Maps and only *1* Unmap XhcMonitorAsyncRequests ->
>   XhcFlushAsyncIntMap ->
>     Unmap and Map Urb->DataMap   (2)
>   XhcUpdateAsyncRequest ->
>     XhcCreateTransferTrb ->
>       Map Urb->DataMap           (3)
> 
> This patch is to eliminate (3).
> 
> Cc: Ruiyu Ni <ruiyu.ni@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>
> ---
>  MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> index 6013d7620fe5..21fdcf130a50 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> @@ -259,8 +259,11 @@ XhcCreateTransferTrb (
>    } else {
>      EPType  = (UINT8) ((DEVICE_CONTEXT_64 *)OutputContext)->EP[Dci-
> 1].EPType;
>    }
> -
> -  if (Urb->Data != NULL) {
> +
> +  //
> +  // No need to remap.
> +  //
> +  if ((Urb->Data != NULL) && (Urb->DataMap == NULL)) {
>      if (((UINT8) (Urb->Ep.Direction)) == EfiUsbDataIn) {
>        MapOp = EfiPciIoOperationBusMasterWrite;
>      } else {
> --
> 2.7.0.windows.1