public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 0/7] MdeModulePkg ArmVirtPkg: fixes for 64-bit PCI DMA
@ 2016-09-05  9:17 Ard Biesheuvel
  2016-09-05  9:17 ` [PATCH 1/7] MdeModulePkg/AtaAtapiPassThru: enable " Ard Biesheuvel
                   ` (7 more replies)
  0 siblings, 8 replies; 20+ messages in thread
From: Ard Biesheuvel @ 2016-09-05  9:17 UTC (permalink / raw)
  To: edk2-devel, feng.tian, star.zeng, liming.gao
  Cc: lersek, leif.lindholm, Ard Biesheuvel

After moving ArmVirtQemu to the generic PciHostBridgeDxe, we noticed that
setting DmaAbove4G resulted in problems with the emulated EHCI USB host
controller, which were caused by the fact that the PCI layer was providing
DMA buffers allocated above 4 GB while the emulated EHCI controller in QEMU
does not indicate support for 64-bit addressing.

As it turns out, the PCI drivers in MdeModulePkg *completely* ignore the
EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE attribute, and simply assume that no
PCI root bridge driver will produce mappings above 4 GB. On ARM, this is
problematic, since not all platforms have memory below 4 GB, and so having
full support for DMA above 4 GB is indispensable.

So first, make the various drivers under MdeModulePkg/Pci/Bus set the
EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE attributes for devices that can
support 64-bit DMA addressing (patches #1 - #5). Then, we can update the
host bridge driver to actually take these attributes into account, and only
create mappings above 4 GB for devices that have indicated support for it.

Finally, in patch #7 we can remove the 4 GB DMA limit from ArmVirtPkg.

Branch can be found here:
https://git.linaro.org/people/ard.biesheuvel/uefi-next.git/shortlog/refs/heads/pci-64bit-dma-fixes

Ard Biesheuvel (7):
  MdeModulePkg/AtaAtapiPassThru: enable 64-bit PCI DMA
  MdeModulePkg/EhciDxe: enable 64-bit PCI DMA
  MdeModulePkg/NvmExpressDxe: enable 64-bit PCI DMA
  MdeModulePkg/SdMmcPciHcDxe: enable 64-bit PCI DMA
  MdeModulePkg/XhciDxe: enable 64-bit PCI DMA
  MdeModulePkg/PciHostBridgeDxe: restrict 64-bit DMA to devices that
    support it
  ArmVirtPkg/FdtPciHostBridgeLib: enable 64-bit PCI DMA

 ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c |  2 +-
 MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c             | 20 +++++++++++++++++-
 MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c                          | 22 +++++++++++++++++++-
 MdeModulePkg/Bus/Pci/EhciDxe/Ehci.h                          |  2 ++
 MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c                     |  2 +-
 MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c           | 13 ++++++++++++
 MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c      | 14 +++++++++----
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c           | 20 ++++++++++++++++++
 MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c                          | 22 +++++++++++++++++++-
 MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h                          |  2 ++
 10 files changed, 110 insertions(+), 9 deletions(-)

-- 
2.7.4



^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/7] MdeModulePkg/AtaAtapiPassThru: enable 64-bit PCI DMA
  2016-09-05  9:17 [PATCH 0/7] MdeModulePkg ArmVirtPkg: fixes for 64-bit PCI DMA Ard Biesheuvel
@ 2016-09-05  9:17 ` Ard Biesheuvel
  2016-09-05  9:17 ` [PATCH 2/7] MdeModulePkg/EhciDxe: " Ard Biesheuvel
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: Ard Biesheuvel @ 2016-09-05  9:17 UTC (permalink / raw)
  To: edk2-devel, feng.tian, star.zeng, liming.gao
  Cc: lersek, leif.lindholm, Ard Biesheuvel

PCI controller drivers must set the EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE
attribute if the controller supports 64-bit DMA addressing.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
index 469a40ac392a..68bce94810a6 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
@@ -2258,7 +2258,25 @@ AhciModeInitialization (
   if ((Capability & EFI_AHCI_CAP_SAM) == 0) {
     AhciOrReg (PciIo, EFI_AHCI_GHC_OFFSET, EFI_AHCI_GHC_ENABLE);
   }
-  
+
+  //
+  // Enable 64-bit DMA support in the PCI layer if this controller
+  // supports it.
+  //
+  if ((Capability & EFI_AHCI_CAP_S64A) != 0) {
+    Status = PciIo->Attributes (
+                      PciIo,
+                      EfiPciIoAttributeOperationEnable,
+                      EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE,
+                      NULL
+                      );
+    if (EFI_ERROR (Status)) {
+      DEBUG ((EFI_D_WARN,
+        "AhciModeInitialization: failed to enable 64-bit DMA on 64-bit capable controller (%r)\n",
+        Status));
+    }
+  }
+
   //
   // Get the number of command slots per port supported by this HBA.
   //
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/7] MdeModulePkg/EhciDxe: enable 64-bit PCI DMA
  2016-09-05  9:17 [PATCH 0/7] MdeModulePkg ArmVirtPkg: fixes for 64-bit PCI DMA Ard Biesheuvel
  2016-09-05  9:17 ` [PATCH 1/7] MdeModulePkg/AtaAtapiPassThru: enable " Ard Biesheuvel
@ 2016-09-05  9:17 ` Ard Biesheuvel
  2016-09-05 12:19   ` Laszlo Ersek
  2016-09-05  9:17 ` [PATCH 3/7] MdeModulePkg/NvmExpressDxe: " Ard Biesheuvel
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Ard Biesheuvel @ 2016-09-05  9:17 UTC (permalink / raw)
  To: edk2-devel, feng.tian, star.zeng, liming.gao
  Cc: lersek, leif.lindholm, Ard Biesheuvel

PCI controller drivers must set the EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE
attribute if the controller supports 64-bit DMA addressing.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c      | 22 +++++++++++++++++++-
 MdeModulePkg/Bus/Pci/EhciDxe/Ehci.h      |  2 ++
 MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c |  2 +-
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
index 4e9e05f0e431..e4c7e59526de 100644
--- a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
+++ b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
@@ -89,7 +89,7 @@ EhcGetCapability (
 
   *MaxSpeed       = EFI_USB_SPEED_HIGH;
   *PortNumber     = (UINT8) (Ehc->HcStructParams & HCSP_NPORTS);
-  *Is64BitCapable = (UINT8) (Ehc->HcCapParams & HCCP_64BIT);
+  *Is64BitCapable = (UINT8) Ehc->Support64BitDma;
 
   DEBUG ((EFI_D_INFO, "EhcGetCapability: %d ports, 64 bit %d\n", *PortNumber, *Is64BitCapable));
 
@@ -1877,6 +1877,26 @@ EhcDriverBindingStart (
     goto CLOSE_PCIIO;
   }
 
+  //
+  // Enable 64-bit DMA support in the PCI layer if this controller
+  // supports it.
+  //
+  if ((Ehc->HcCapParams & HCCP_64BIT) != 0) {
+    Status = PciIo->Attributes (
+                      PciIo,
+                      EfiPciIoAttributeOperationEnable,
+                      EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE,
+                      NULL
+                      );
+    if (!EFI_ERROR (Status)) {
+      Ehc->Support64BitDma = TRUE;
+    } else {
+      DEBUG ((EFI_D_WARN,
+        "EhcDriverBindingStart: failed to enable 64-bit DMA on 64-bit capable controller @ %p (%r)\n",
+        Controller, Status));
+    }
+  }
+
   Status = gBS->InstallProtocolInterface (
                   &Controller,
                   &gEfiUsb2HcProtocolGuid,
diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.h b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.h
index 7177658092c3..be81bde40d9b 100644
--- a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.h
+++ b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.h
@@ -173,6 +173,8 @@ struct _USB2_HC_DEV {
   UINT16                    DebugPortOffset; // The offset of debug port mmio register
   UINT8                     DebugPortBarNum; // The bar number of debug port mmio register
   UINT8                     DebugPortNum;    // The port number of usb debug port
+
+  BOOLEAN                   Support64BitDma; // Whether 64 bit DMA may be used with this device
 };
 
 
diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c b/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c
index 5594e6699ea6..036c00b32b40 100644
--- a/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c
+++ b/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c
@@ -178,7 +178,7 @@ EhcInitSched (
   //
   Ehc->MemPool = UsbHcInitMemPool (
                    PciIo,
-                   EHC_BIT_IS_SET (Ehc->HcCapParams, HCCP_64BIT),
+                   Ehc->Support64BitDma,
                    EHC_HIGH_32BIT (PhyAddr)
                    );
 
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 3/7] MdeModulePkg/NvmExpressDxe: enable 64-bit PCI DMA
  2016-09-05  9:17 [PATCH 0/7] MdeModulePkg ArmVirtPkg: fixes for 64-bit PCI DMA Ard Biesheuvel
  2016-09-05  9:17 ` [PATCH 1/7] MdeModulePkg/AtaAtapiPassThru: enable " Ard Biesheuvel
  2016-09-05  9:17 ` [PATCH 2/7] MdeModulePkg/EhciDxe: " Ard Biesheuvel
@ 2016-09-05  9:17 ` Ard Biesheuvel
  2016-09-05  9:17 ` [PATCH 4/7] MdeModulePkg/SdMmcPciHcDxe: " Ard Biesheuvel
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: Ard Biesheuvel @ 2016-09-05  9:17 UTC (permalink / raw)
  To: edk2-devel, feng.tian, star.zeng, liming.gao
  Cc: lersek, leif.lindholm, Ard Biesheuvel

PCI controller drivers must set the EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE
attribute if the controller supports 64-bit DMA addressing.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c
index a173504cdf4d..51cff3c96c29 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c
@@ -863,6 +863,19 @@ NvmeControllerInit (
   }
 
   //
+  // Enable 64-bit DMA support in the PCI layer.
+  //
+  Status = PciIo->Attributes (
+                    PciIo,
+                    EfiPciIoAttributeOperationEnable,
+                    EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE,
+                    NULL
+                    );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((EFI_D_WARN, "NvmeControllerInit: failed to enable 64-bit DMA (%r)\n", Status));
+  }
+
+  //
   // Read the Controller Capabilities register and verify that the NVM command set is supported
   //
   Status = ReadNvmeControllerCapabilities (Private, &Private->Cap);
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 4/7] MdeModulePkg/SdMmcPciHcDxe: enable 64-bit PCI DMA
  2016-09-05  9:17 [PATCH 0/7] MdeModulePkg ArmVirtPkg: fixes for 64-bit PCI DMA Ard Biesheuvel
                   ` (2 preceding siblings ...)
  2016-09-05  9:17 ` [PATCH 3/7] MdeModulePkg/NvmExpressDxe: " Ard Biesheuvel
@ 2016-09-05  9:17 ` Ard Biesheuvel
  2016-09-05  9:17 ` [PATCH 5/7] MdeModulePkg/XhciDxe: " Ard Biesheuvel
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: Ard Biesheuvel @ 2016-09-05  9:17 UTC (permalink / raw)
  To: edk2-devel, feng.tian, star.zeng, liming.gao
  Cc: lersek, leif.lindholm, Ard Biesheuvel

PCI controller drivers must set the EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE
attribute if the controller supports 64-bit DMA addressing.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
index 0be081dad0bc..5de1dd6fd9e6 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
@@ -527,6 +527,7 @@ SdMmcPciHcDriverBindingStart (
   CARD_TYPE_DETECT_ROUTINE        *Routine;
   UINT32                          RoutineNum;
   BOOLEAN                         MediaPresent;
+  BOOLEAN                         Support64BitDma;
 
   DEBUG ((EFI_D_INFO, "SdMmcPciHcDriverBindingStart: Start\n"));
 
@@ -600,6 +601,7 @@ SdMmcPciHcDriverBindingStart (
     goto Done;
   }
 
+  Support64BitDma = TRUE;
   for (Slot = FirstBar; Slot < (FirstBar + SlotNum); Slot++) {
     Private->Slot[Slot].Enable = TRUE;
 
@@ -609,6 +611,8 @@ SdMmcPciHcDriverBindingStart (
     }
     DumpCapabilityReg (Slot, &Private->Capability[Slot]);
 
+    Support64BitDma &= Private->Capability[Slot].SysBus64;
+
     Status = SdMmcHcGetMaxCurrent (PciIo, Slot, &Private->MaxCurrent[Slot]);
     if (EFI_ERROR (Status)) {
       continue;
@@ -664,6 +668,22 @@ SdMmcPciHcDriverBindingStart (
   }
 
   //
+  // Enable 64-bit DMA support in the PCI layer if this controller
+  // supports it.
+  //
+  if (Support64BitDma) {
+    Status = PciIo->Attributes (
+                      PciIo,
+                      EfiPciIoAttributeOperationEnable,
+                      EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE,
+                      NULL
+                      );
+    if (EFI_ERROR (Status)) {
+      DEBUG ((EFI_D_WARN, "SdMmcPciHcDriverBindingStart: failed to enable 64-bit DMA (%r)\n", Status));
+    }
+  }
+
+  //
   // Start the asynchronous I/O monitor
   //
   Status = gBS->CreateEvent (
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 5/7] MdeModulePkg/XhciDxe: enable 64-bit PCI DMA
  2016-09-05  9:17 [PATCH 0/7] MdeModulePkg ArmVirtPkg: fixes for 64-bit PCI DMA Ard Biesheuvel
                   ` (3 preceding siblings ...)
  2016-09-05  9:17 ` [PATCH 4/7] MdeModulePkg/SdMmcPciHcDxe: " Ard Biesheuvel
@ 2016-09-05  9:17 ` Ard Biesheuvel
  2016-09-05 12:14   ` Laszlo Ersek
  2016-09-05  9:17 ` [PATCH 6/7] MdeModulePkg/PciHostBridgeDxe: restrict 64-bit DMA to devices that support it Ard Biesheuvel
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Ard Biesheuvel @ 2016-09-05  9:17 UTC (permalink / raw)
  To: edk2-devel, feng.tian, star.zeng, liming.gao
  Cc: lersek, leif.lindholm, Ard Biesheuvel

PCI controller drivers must set the EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE
attribute if the controller supports 64-bit DMA addressing.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c | 22 +++++++++++++++++++-
 MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h |  2 ++
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
index 4798bea86061..cdff1c3b8849 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
@@ -125,7 +125,7 @@ XhcGetCapability (
   Xhc             = XHC_FROM_THIS (This);
   *MaxSpeed       = EFI_USB_SPEED_SUPER;
   *PortNumber     = (UINT8) (Xhc->HcSParams1.Data.MaxPorts);
-  *Is64BitCapable = (UINT8) (Xhc->HcCParams.Data.Ac64);
+  *Is64BitCapable = (UINT8) Xhc->Support64BitDma;
   DEBUG ((EFI_D_INFO, "XhcGetCapability: %d ports, 64 bit %d\n", *PortNumber, *Is64BitCapable));
 
   gBS->RestoreTPL (OldTpl);
@@ -2020,6 +2020,26 @@ XhcDriverBindingStart (
     return EFI_OUT_OF_RESOURCES;
   }
 
+  //
+  // Enable 64-bit DMA support in the PCI layer if this controller
+  // supports it.
+  //
+  if ((Xhc->HcCParams.Data.Ac64) != 0) {
+    Status = PciIo->Attributes (
+                      PciIo,
+                      EfiPciIoAttributeOperationEnable,
+                      EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE,
+                      NULL
+                      );
+    if (!EFI_ERROR (Status)) {
+      Xhc->Support64BitDma = TRUE;
+    } else {
+      DEBUG ((EFI_D_WARN,
+        "XhcDriverBindingStart: failed to enable 64-bit DMA on 64-bit capable controller @ %p (%r)\n",
+        Controller, Status));
+    }
+  }
+
   XhcSetBiosOwnership (Xhc);
 
   XhcResetHC (Xhc, XHC_RESET_TIMEOUT);
diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
index 7999151b3fde..0f53bb0eff7c 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
@@ -256,6 +256,8 @@ struct _USB_XHCI_INSTANCE {
   // The array supports up to 255 devices, entry 0 is reserved and should not be used.
   //
   USB_DEV_CONTEXT           UsbDevContext[256];
+
+  BOOLEAN                   Support64BitDma; // Whether 64 bit DMA may be used with this device
 };
 
 
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 6/7] MdeModulePkg/PciHostBridgeDxe: restrict 64-bit DMA to devices that support it
  2016-09-05  9:17 [PATCH 0/7] MdeModulePkg ArmVirtPkg: fixes for 64-bit PCI DMA Ard Biesheuvel
                   ` (4 preceding siblings ...)
  2016-09-05  9:17 ` [PATCH 5/7] MdeModulePkg/XhciDxe: " Ard Biesheuvel
@ 2016-09-05  9:17 ` Ard Biesheuvel
  2016-09-05 12:04   ` Laszlo Ersek
  2016-09-05  9:17 ` [PATCH 7/7] ArmVirtPkg/FdtPciHostBridgeLib: enable 64-bit PCI DMA Ard Biesheuvel
  2016-09-06  7:48 ` [PATCH 0/7] MdeModulePkg ArmVirtPkg: fixes for " Ard Biesheuvel
  7 siblings, 1 reply; 20+ messages in thread
From: Ard Biesheuvel @ 2016-09-05  9:17 UTC (permalink / raw)
  To: edk2-devel, feng.tian, star.zeng, liming.gao
  Cc: lersek, leif.lindholm, Ard Biesheuvel

Currently, the EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE attribute is completely
ignored by the PCI host bridge driver, which means that, on an implementation
that supports DMA above 4 GB, allocations above 4 GB may be provided to
devices that have not expressed support for it.

So in addition to checking 'RootBridge->DmaAbove4G' to establish whether the
root bridge itself supports DMA above 4 GB, we must also take into account
the operation type (EfiPciOperationBusMaster{Read|Write|CommBuffer}64),
and the EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE attribute, when mapping and
allocating DMA memory, respectively.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
index b2d76d67afa2..8af131b0af37 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
@@ -1073,10 +1073,15 @@ RootBridgeIoMap (
   RootBridge = ROOT_BRIDGE_FROM_THIS (This);
 
   PhysicalAddress = (EFI_PHYSICAL_ADDRESS) (UINTN) HostAddress;
-  if (!RootBridge->DmaAbove4G && ((PhysicalAddress + *NumberOfBytes) > SIZE_4GB)) {
+  if ((!RootBridge->DmaAbove4G ||
+       (Operation != EfiPciOperationBusMasterRead64 &&
+        Operation != EfiPciOperationBusMasterWrite64 &&
+        Operation != EfiPciOperationBusMasterCommonBuffer64)) &&
+      ((PhysicalAddress + *NumberOfBytes) > SIZE_4GB)) {
+
     //
-    // If the root bridge can not handle performing DMA above 4GB but
-    // any part of the DMA transfer being mapped is above 4GB, then
+    // If the root bridge or the device cannot handle performing DMA above
+    // 4GB but any part of the DMA transfer being mapped is above 4GB, then
     // map the DMA transfer to a buffer below 4GB.
     //
 
@@ -1308,7 +1313,8 @@ RootBridgeIoAllocateBuffer (
   RootBridge = ROOT_BRIDGE_FROM_THIS (This);
 
   AllocateType = AllocateAnyPages;
-  if (!RootBridge->DmaAbove4G) {
+  if (!RootBridge->DmaAbove4G ||
+      (Attributes & EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE) == 0) {
     //
     // Limit allocations to memory below 4GB
     //
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 7/7] ArmVirtPkg/FdtPciHostBridgeLib: enable 64-bit PCI DMA
  2016-09-05  9:17 [PATCH 0/7] MdeModulePkg ArmVirtPkg: fixes for 64-bit PCI DMA Ard Biesheuvel
                   ` (5 preceding siblings ...)
  2016-09-05  9:17 ` [PATCH 6/7] MdeModulePkg/PciHostBridgeDxe: restrict 64-bit DMA to devices that support it Ard Biesheuvel
@ 2016-09-05  9:17 ` Ard Biesheuvel
  2016-09-05 12:04   ` Laszlo Ersek
  2016-09-06  7:48 ` [PATCH 0/7] MdeModulePkg ArmVirtPkg: fixes for " Ard Biesheuvel
  7 siblings, 1 reply; 20+ messages in thread
From: Ard Biesheuvel @ 2016-09-05  9:17 UTC (permalink / raw)
  To: edk2-devel, feng.tian, star.zeng, liming.gao
  Cc: lersek, leif.lindholm, Ard Biesheuvel

Now that the PCI root bridge driver and various host controller drivers
have been fixed, remove the 4 GB limit on PCI DMA allocation for QEMU's
ECAM PCI host bridge.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
index bb3742386c63..5b9c887db35d 100644
--- a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
+++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
@@ -317,7 +317,7 @@ PciHostBridgeGetRootBridges (
                                       EFI_PCI_ATTRIBUTE_VGA_PALETTE_IO_16;
   mRootBridge.Attributes            = mRootBridge.Supports;
 
-  mRootBridge.DmaAbove4G            = FALSE;
+  mRootBridge.DmaAbove4G            = TRUE;
   mRootBridge.NoExtendedConfigSpace = FALSE;
   mRootBridge.ResourceAssigned      = FALSE;
 
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 6/7] MdeModulePkg/PciHostBridgeDxe: restrict 64-bit DMA to devices that support it
  2016-09-05  9:17 ` [PATCH 6/7] MdeModulePkg/PciHostBridgeDxe: restrict 64-bit DMA to devices that support it Ard Biesheuvel
@ 2016-09-05 12:04   ` Laszlo Ersek
  0 siblings, 0 replies; 20+ messages in thread
From: Laszlo Ersek @ 2016-09-05 12:04 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel, feng.tian, star.zeng, liming.gao
  Cc: leif.lindholm

On 09/05/16 11:17, Ard Biesheuvel wrote:
> Currently, the EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE attribute is completely
> ignored by the PCI host bridge driver, which means that, on an implementation
> that supports DMA above 4 GB, allocations above 4 GB may be provided to
> devices that have not expressed support for it.
> 
> So in addition to checking 'RootBridge->DmaAbove4G' to establish whether the
> root bridge itself supports DMA above 4 GB, we must also take into account
> the operation type (EfiPciOperationBusMaster{Read|Write|CommBuffer}64),
> and the EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE attribute, when mapping and
> allocating DMA memory, respectively.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> index b2d76d67afa2..8af131b0af37 100644
> --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> @@ -1073,10 +1073,15 @@ RootBridgeIoMap (
>    RootBridge = ROOT_BRIDGE_FROM_THIS (This);
>  
>    PhysicalAddress = (EFI_PHYSICAL_ADDRESS) (UINTN) HostAddress;
> -  if (!RootBridge->DmaAbove4G && ((PhysicalAddress + *NumberOfBytes) > SIZE_4GB)) {
> +  if ((!RootBridge->DmaAbove4G ||
> +       (Operation != EfiPciOperationBusMasterRead64 &&
> +        Operation != EfiPciOperationBusMasterWrite64 &&
> +        Operation != EfiPciOperationBusMasterCommonBuffer64)) &&
> +      ((PhysicalAddress + *NumberOfBytes) > SIZE_4GB)) {
> +
>      //
> -    // If the root bridge can not handle performing DMA above 4GB but
> -    // any part of the DMA transfer being mapped is above 4GB, then
> +    // If the root bridge or the device cannot handle performing DMA above
> +    // 4GB but any part of the DMA transfer being mapped is above 4GB, then
>      // map the DMA transfer to a buffer below 4GB.
>      //
>  
> @@ -1308,7 +1313,8 @@ RootBridgeIoAllocateBuffer (
>    RootBridge = ROOT_BRIDGE_FROM_THIS (This);
>  
>    AllocateType = AllocateAnyPages;
> -  if (!RootBridge->DmaAbove4G) {
> +  if (!RootBridge->DmaAbove4G ||
> +      (Attributes & EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE) == 0) {
>      //
>      // Limit allocations to memory below 4GB
>      //
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 7/7] ArmVirtPkg/FdtPciHostBridgeLib: enable 64-bit PCI DMA
  2016-09-05  9:17 ` [PATCH 7/7] ArmVirtPkg/FdtPciHostBridgeLib: enable 64-bit PCI DMA Ard Biesheuvel
@ 2016-09-05 12:04   ` Laszlo Ersek
  0 siblings, 0 replies; 20+ messages in thread
From: Laszlo Ersek @ 2016-09-05 12:04 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel, feng.tian, star.zeng, liming.gao
  Cc: leif.lindholm

On 09/05/16 11:17, Ard Biesheuvel wrote:
> Now that the PCI root bridge driver and various host controller drivers
> have been fixed, remove the 4 GB limit on PCI DMA allocation for QEMU's
> ECAM PCI host bridge.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
> index bb3742386c63..5b9c887db35d 100644
> --- a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
> +++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
> @@ -317,7 +317,7 @@ PciHostBridgeGetRootBridges (
>                                        EFI_PCI_ATTRIBUTE_VGA_PALETTE_IO_16;
>    mRootBridge.Attributes            = mRootBridge.Supports;
>  
> -  mRootBridge.DmaAbove4G            = FALSE;
> +  mRootBridge.DmaAbove4G            = TRUE;
>    mRootBridge.NoExtendedConfigSpace = FALSE;
>    mRootBridge.ResourceAssigned      = FALSE;
>  
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 5/7] MdeModulePkg/XhciDxe: enable 64-bit PCI DMA
  2016-09-05  9:17 ` [PATCH 5/7] MdeModulePkg/XhciDxe: " Ard Biesheuvel
@ 2016-09-05 12:14   ` Laszlo Ersek
  0 siblings, 0 replies; 20+ messages in thread
From: Laszlo Ersek @ 2016-09-05 12:14 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel, feng.tian, star.zeng, liming.gao
  Cc: leif.lindholm

On 09/05/16 11:17, Ard Biesheuvel wrote:
> PCI controller drivers must set the EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE
> attribute if the controller supports 64-bit DMA addressing.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c | 22 +++++++++++++++++++-
>  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h |  2 ++
>  2 files changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> index 4798bea86061..cdff1c3b8849 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> @@ -125,7 +125,7 @@ XhcGetCapability (
>    Xhc             = XHC_FROM_THIS (This);
>    *MaxSpeed       = EFI_USB_SPEED_SUPER;
>    *PortNumber     = (UINT8) (Xhc->HcSParams1.Data.MaxPorts);
> -  *Is64BitCapable = (UINT8) (Xhc->HcCParams.Data.Ac64);
> +  *Is64BitCapable = (UINT8) Xhc->Support64BitDma;
>    DEBUG ((EFI_D_INFO, "XhcGetCapability: %d ports, 64 bit %d\n", *PortNumber, *Is64BitCapable));
>  
>    gBS->RestoreTPL (OldTpl);
> @@ -2020,6 +2020,26 @@ XhcDriverBindingStart (
>      return EFI_OUT_OF_RESOURCES;
>    }
>  
> +  //
> +  // Enable 64-bit DMA support in the PCI layer if this controller
> +  // supports it.
> +  //
> +  if ((Xhc->HcCParams.Data.Ac64) != 0) {

I think the inner parens are superfluous.

> +    Status = PciIo->Attributes (
> +                      PciIo,
> +                      EfiPciIoAttributeOperationEnable,
> +                      EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE,
> +                      NULL
> +                      );
> +    if (!EFI_ERROR (Status)) {
> +      Xhc->Support64BitDma = TRUE;
> +    } else {
> +      DEBUG ((EFI_D_WARN,
> +        "XhcDriverBindingStart: failed to enable 64-bit DMA on 64-bit capable controller @ %p (%r)\n",

I prefer to use "%a" with __FUNCTION__, rather than open-coding the
containing function's name; for brevity and for robustness against code
movement.

I'll leave it to you if you want to change these things.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo

> +        Controller, Status));
> +    }
> +  }
> +
>    XhcSetBiosOwnership (Xhc);
>  
>    XhcResetHC (Xhc, XHC_RESET_TIMEOUT);
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
> index 7999151b3fde..0f53bb0eff7c 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
> @@ -256,6 +256,8 @@ struct _USB_XHCI_INSTANCE {
>    // The array supports up to 255 devices, entry 0 is reserved and should not be used.
>    //
>    USB_DEV_CONTEXT           UsbDevContext[256];
> +
> +  BOOLEAN                   Support64BitDma; // Whether 64 bit DMA may be used with this device
>  };
>  
>  
> 



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/7] MdeModulePkg/EhciDxe: enable 64-bit PCI DMA
  2016-09-05  9:17 ` [PATCH 2/7] MdeModulePkg/EhciDxe: " Ard Biesheuvel
@ 2016-09-05 12:19   ` Laszlo Ersek
  2016-09-05 12:44     ` Ard Biesheuvel
  0 siblings, 1 reply; 20+ messages in thread
From: Laszlo Ersek @ 2016-09-05 12:19 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel, feng.tian, star.zeng, liming.gao
  Cc: leif.lindholm

On 09/05/16 11:17, Ard Biesheuvel wrote:
> PCI controller drivers must set the EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE
> attribute if the controller supports 64-bit DMA addressing.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c      | 22 +++++++++++++++++++-
>  MdeModulePkg/Bus/Pci/EhciDxe/Ehci.h      |  2 ++
>  MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c |  2 +-
>  3 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
> index 4e9e05f0e431..e4c7e59526de 100644
> --- a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
> +++ b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
> @@ -89,7 +89,7 @@ EhcGetCapability (
>  
>    *MaxSpeed       = EFI_USB_SPEED_HIGH;
>    *PortNumber     = (UINT8) (Ehc->HcStructParams & HCSP_NPORTS);
> -  *Is64BitCapable = (UINT8) (Ehc->HcCapParams & HCCP_64BIT);
> +  *Is64BitCapable = (UINT8) Ehc->Support64BitDma;
>  
>    DEBUG ((EFI_D_INFO, "EhcGetCapability: %d ports, 64 bit %d\n", *PortNumber, *Is64BitCapable));
>  
> @@ -1877,6 +1877,26 @@ EhcDriverBindingStart (
>      goto CLOSE_PCIIO;
>    }
>  
> +  //
> +  // Enable 64-bit DMA support in the PCI layer if this controller
> +  // supports it.
> +  //
> +  if ((Ehc->HcCapParams & HCCP_64BIT) != 0) {

How about using the nice EHC_BIT_IS_SET macro here (inspired by the
previous use in EhcInitSched(), at the bottom of this patch)?

> +    Status = PciIo->Attributes (
> +                      PciIo,
> +                      EfiPciIoAttributeOperationEnable,
> +                      EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE,
> +                      NULL
> +                      );
> +    if (!EFI_ERROR (Status)) {
> +      Ehc->Support64BitDma = TRUE;
> +    } else {
> +      DEBUG ((EFI_D_WARN,
> +        "EhcDriverBindingStart: failed to enable 64-bit DMA on 64-bit capable controller @ %p (%r)\n",
> +        Controller, Status));

Same comment as for 5/7, i.e. %a and __FUNCTION__.

Again, whether you want to change these is up to you.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

(I won't try to review the rest of the patches.)

Thanks
Laszlo

> +    }
> +  }
> +
>    Status = gBS->InstallProtocolInterface (
>                    &Controller,
>                    &gEfiUsb2HcProtocolGuid,
> diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.h b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.h
> index 7177658092c3..be81bde40d9b 100644
> --- a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.h
> +++ b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.h
> @@ -173,6 +173,8 @@ struct _USB2_HC_DEV {
>    UINT16                    DebugPortOffset; // The offset of debug port mmio register
>    UINT8                     DebugPortBarNum; // The bar number of debug port mmio register
>    UINT8                     DebugPortNum;    // The port number of usb debug port
> +
> +  BOOLEAN                   Support64BitDma; // Whether 64 bit DMA may be used with this device
>  };
>  
>  
> diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c b/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c
> index 5594e6699ea6..036c00b32b40 100644
> --- a/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c
> +++ b/MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c
> @@ -178,7 +178,7 @@ EhcInitSched (
>    //
>    Ehc->MemPool = UsbHcInitMemPool (
>                     PciIo,
> -                   EHC_BIT_IS_SET (Ehc->HcCapParams, HCCP_64BIT),
> +                   Ehc->Support64BitDma,
>                     EHC_HIGH_32BIT (PhyAddr)
>                     );
>  
> 



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/7] MdeModulePkg/EhciDxe: enable 64-bit PCI DMA
  2016-09-05 12:19   ` Laszlo Ersek
@ 2016-09-05 12:44     ` Ard Biesheuvel
  2016-09-05 13:06       ` Laszlo Ersek
  0 siblings, 1 reply; 20+ messages in thread
From: Ard Biesheuvel @ 2016-09-05 12:44 UTC (permalink / raw)
  To: Laszlo Ersek
  Cc: edk2-devel@lists.01.org, Tian, Feng, Zeng, Star, Gao, Liming,
	Leif Lindholm

On 5 September 2016 at 13:19, Laszlo Ersek <lersek@redhat.com> wrote:
> On 09/05/16 11:17, Ard Biesheuvel wrote:
>> PCI controller drivers must set the EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE
>> attribute if the controller supports 64-bit DMA addressing.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c      | 22 +++++++++++++++++++-
>>  MdeModulePkg/Bus/Pci/EhciDxe/Ehci.h      |  2 ++
>>  MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c |  2 +-
>>  3 files changed, 24 insertions(+), 2 deletions(-)
>>
>> diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
>> index 4e9e05f0e431..e4c7e59526de 100644
>> --- a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
>> +++ b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
>> @@ -89,7 +89,7 @@ EhcGetCapability (
>>
>>    *MaxSpeed       = EFI_USB_SPEED_HIGH;
>>    *PortNumber     = (UINT8) (Ehc->HcStructParams & HCSP_NPORTS);
>> -  *Is64BitCapable = (UINT8) (Ehc->HcCapParams & HCCP_64BIT);
>> +  *Is64BitCapable = (UINT8) Ehc->Support64BitDma;
>>
>>    DEBUG ((EFI_D_INFO, "EhcGetCapability: %d ports, 64 bit %d\n", *PortNumber, *Is64BitCapable));
>>
>> @@ -1877,6 +1877,26 @@ EhcDriverBindingStart (
>>      goto CLOSE_PCIIO;
>>    }
>>
>> +  //
>> +  // Enable 64-bit DMA support in the PCI layer if this controller
>> +  // supports it.
>> +  //
>> +  if ((Ehc->HcCapParams & HCCP_64BIT) != 0) {
>
> How about using the nice EHC_BIT_IS_SET macro here (inspired by the
> previous use in EhcInitSched(), at the bottom of this patch)?
>

I just moved the test from the previous hunk here. I don't care either
way, so I will let the maintainers decide. Feng, Star?

>> +    Status = PciIo->Attributes (
>> +                      PciIo,
>> +                      EfiPciIoAttributeOperationEnable,
>> +                      EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE,
>> +                      NULL
>> +                      );
>> +    if (!EFI_ERROR (Status)) {
>> +      Ehc->Support64BitDma = TRUE;
>> +    } else {
>> +      DEBUG ((EFI_D_WARN,
>> +        "EhcDriverBindingStart: failed to enable 64-bit DMA on 64-bit capable controller @ %p (%r)\n",
>> +        Controller, Status));
>
> Same comment as for 5/7, i.e. %a and __FUNCTION__.
>

The surrounding code never uses that. I started out using %a and
__FUNCTION__, and then removed it again to align with the existing
code.

> Again, whether you want to change these is up to you.
>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>

Thanks a lot!

> (I won't try to review the rest of the patches.)
>

No worries. I did build test all of them, but I currently have no way
of testing the NVM and SDHCI patches, so I am hoping they are
'obviously correct'

-- 
Ard.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/7] MdeModulePkg/EhciDxe: enable 64-bit PCI DMA
  2016-09-05 12:44     ` Ard Biesheuvel
@ 2016-09-05 13:06       ` Laszlo Ersek
  0 siblings, 0 replies; 20+ messages in thread
From: Laszlo Ersek @ 2016-09-05 13:06 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: edk2-devel@lists.01.org, Tian, Feng, Zeng, Star, Gao, Liming,
	Leif Lindholm

On 09/05/16 14:44, Ard Biesheuvel wrote:
> On 5 September 2016 at 13:19, Laszlo Ersek <lersek@redhat.com> wrote:
>> On 09/05/16 11:17, Ard Biesheuvel wrote:
>>> PCI controller drivers must set the EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE
>>> attribute if the controller supports 64-bit DMA addressing.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> ---
>>>  MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c      | 22 +++++++++++++++++++-
>>>  MdeModulePkg/Bus/Pci/EhciDxe/Ehci.h      |  2 ++
>>>  MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c |  2 +-
>>>  3 files changed, 24 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
>>> index 4e9e05f0e431..e4c7e59526de 100644
>>> --- a/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
>>> +++ b/MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c
>>> @@ -89,7 +89,7 @@ EhcGetCapability (
>>>
>>>    *MaxSpeed       = EFI_USB_SPEED_HIGH;
>>>    *PortNumber     = (UINT8) (Ehc->HcStructParams & HCSP_NPORTS);
>>> -  *Is64BitCapable = (UINT8) (Ehc->HcCapParams & HCCP_64BIT);
>>> +  *Is64BitCapable = (UINT8) Ehc->Support64BitDma;
>>>
>>>    DEBUG ((EFI_D_INFO, "EhcGetCapability: %d ports, 64 bit %d\n", *PortNumber, *Is64BitCapable));
>>>
>>> @@ -1877,6 +1877,26 @@ EhcDriverBindingStart (
>>>      goto CLOSE_PCIIO;
>>>    }
>>>
>>> +  //
>>> +  // Enable 64-bit DMA support in the PCI layer if this controller
>>> +  // supports it.
>>> +  //
>>> +  if ((Ehc->HcCapParams & HCCP_64BIT) != 0) {
>>
>> How about using the nice EHC_BIT_IS_SET macro here (inspired by the
>> previous use in EhcInitSched(), at the bottom of this patch)?
>>
> 
> I just moved the test from the previous hunk here. I don't care either
> way, so I will let the maintainers decide. Feng, Star?
> 
>>> +    Status = PciIo->Attributes (
>>> +                      PciIo,
>>> +                      EfiPciIoAttributeOperationEnable,
>>> +                      EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE,
>>> +                      NULL
>>> +                      );
>>> +    if (!EFI_ERROR (Status)) {
>>> +      Ehc->Support64BitDma = TRUE;
>>> +    } else {
>>> +      DEBUG ((EFI_D_WARN,
>>> +        "EhcDriverBindingStart: failed to enable 64-bit DMA on 64-bit capable controller @ %p (%r)\n",
>>> +        Controller, Status));
>>
>> Same comment as for 5/7, i.e. %a and __FUNCTION__.
>>
> 
> The surrounding code never uses that. I started out using %a and
> __FUNCTION__, and then removed it again to align with the existing
> code.
> 
>> Again, whether you want to change these is up to you.
>>
>> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>>
> 
> Thanks a lot!
> 
>> (I won't try to review the rest of the patches.)
>>
> 
> No worries. I did build test all of them, but I currently have no way
> of testing the NVM and SDHCI patches, so I am hoping they are
> 'obviously correct'
> 

You can test the NVMe change with QEMU (x86_64) + OVMF:

https://tianocore.acgmultimedia.com/show_bug.cgi?id=79

Thanks
Laszlo


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/7] MdeModulePkg ArmVirtPkg: fixes for 64-bit PCI DMA
  2016-09-05  9:17 [PATCH 0/7] MdeModulePkg ArmVirtPkg: fixes for 64-bit PCI DMA Ard Biesheuvel
                   ` (6 preceding siblings ...)
  2016-09-05  9:17 ` [PATCH 7/7] ArmVirtPkg/FdtPciHostBridgeLib: enable 64-bit PCI DMA Ard Biesheuvel
@ 2016-09-06  7:48 ` Ard Biesheuvel
  2016-09-06  8:04   ` Zeng, Star
  2016-09-06  8:54   ` Ni, Ruiyu
  7 siblings, 2 replies; 20+ messages in thread
From: Ard Biesheuvel @ 2016-09-06  7:48 UTC (permalink / raw)
  To: edk2-devel-01, Tian, Feng, Zeng, Star, Gao, Liming
  Cc: Laszlo Ersek, Leif Lindholm, Ard Biesheuvel

Feng, Star: do you have any feedback on these patches? Thanks.

On 5 September 2016 at 10:17, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> After moving ArmVirtQemu to the generic PciHostBridgeDxe, we noticed that
> setting DmaAbove4G resulted in problems with the emulated EHCI USB host
> controller, which were caused by the fact that the PCI layer was providing
> DMA buffers allocated above 4 GB while the emulated EHCI controller in QEMU
> does not indicate support for 64-bit addressing.
>
> As it turns out, the PCI drivers in MdeModulePkg *completely* ignore the
> EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE attribute, and simply assume that no
> PCI root bridge driver will produce mappings above 4 GB. On ARM, this is
> problematic, since not all platforms have memory below 4 GB, and so having
> full support for DMA above 4 GB is indispensable.
>
> So first, make the various drivers under MdeModulePkg/Pci/Bus set the
> EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE attributes for devices that can
> support 64-bit DMA addressing (patches #1 - #5). Then, we can update the
> host bridge driver to actually take these attributes into account, and only
> create mappings above 4 GB for devices that have indicated support for it.
>
> Finally, in patch #7 we can remove the 4 GB DMA limit from ArmVirtPkg.
>
> Branch can be found here:
> https://git.linaro.org/people/ard.biesheuvel/uefi-next.git/shortlog/refs/heads/pci-64bit-dma-fixes
>
> Ard Biesheuvel (7):
>   MdeModulePkg/AtaAtapiPassThru: enable 64-bit PCI DMA
>   MdeModulePkg/EhciDxe: enable 64-bit PCI DMA
>   MdeModulePkg/NvmExpressDxe: enable 64-bit PCI DMA
>   MdeModulePkg/SdMmcPciHcDxe: enable 64-bit PCI DMA
>   MdeModulePkg/XhciDxe: enable 64-bit PCI DMA
>   MdeModulePkg/PciHostBridgeDxe: restrict 64-bit DMA to devices that
>     support it
>   ArmVirtPkg/FdtPciHostBridgeLib: enable 64-bit PCI DMA
>
>  ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c |  2 +-
>  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c             | 20 +++++++++++++++++-
>  MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c                          | 22 +++++++++++++++++++-
>  MdeModulePkg/Bus/Pci/EhciDxe/Ehci.h                          |  2 ++
>  MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c                     |  2 +-
>  MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c           | 13 ++++++++++++
>  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c      | 14 +++++++++----
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c           | 20 ++++++++++++++++++
>  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c                          | 22 +++++++++++++++++++-
>  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h                          |  2 ++
>  10 files changed, 110 insertions(+), 9 deletions(-)
>
> --
> 2.7.4
>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/7] MdeModulePkg ArmVirtPkg: fixes for 64-bit PCI DMA
  2016-09-06  7:48 ` [PATCH 0/7] MdeModulePkg ArmVirtPkg: fixes for " Ard Biesheuvel
@ 2016-09-06  8:04   ` Zeng, Star
  2016-09-06  8:54   ` Ni, Ruiyu
  1 sibling, 0 replies; 20+ messages in thread
From: Zeng, Star @ 2016-09-06  8:04 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel-01, Tian, Feng, Gao, Liming
  Cc: Laszlo Ersek, Leif Lindholm, Ni, Ruiyu

Also cc Ruiyu to see if any comments.

Thanks,
Star
-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] 
Sent: Tuesday, September 6, 2016 3:48 PM
To: edk2-devel-01 <edk2-devel@lists.01.org>; Tian, Feng <feng.tian@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming <liming.gao@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>; Leif Lindholm <leif.lindholm@linaro.org>; Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: Re: [PATCH 0/7] MdeModulePkg ArmVirtPkg: fixes for 64-bit PCI DMA

Feng, Star: do you have any feedback on these patches? Thanks.

On 5 September 2016 at 10:17, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> After moving ArmVirtQemu to the generic PciHostBridgeDxe, we noticed 
> that setting DmaAbove4G resulted in problems with the emulated EHCI 
> USB host controller, which were caused by the fact that the PCI layer 
> was providing DMA buffers allocated above 4 GB while the emulated EHCI 
> controller in QEMU does not indicate support for 64-bit addressing.
>
> As it turns out, the PCI drivers in MdeModulePkg *completely* ignore 
> the EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE attribute, and simply assume 
> that no PCI root bridge driver will produce mappings above 4 GB. On 
> ARM, this is problematic, since not all platforms have memory below 4 
> GB, and so having full support for DMA above 4 GB is indispensable.
>
> So first, make the various drivers under MdeModulePkg/Pci/Bus set the 
> EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE attributes for devices that 
> can support 64-bit DMA addressing (patches #1 - #5). Then, we can 
> update the host bridge driver to actually take these attributes into 
> account, and only create mappings above 4 GB for devices that have indicated support for it.
>
> Finally, in patch #7 we can remove the 4 GB DMA limit from ArmVirtPkg.
>
> Branch can be found here:
> https://git.linaro.org/people/ard.biesheuvel/uefi-next.git/shortlog/re
> fs/heads/pci-64bit-dma-fixes
>
> Ard Biesheuvel (7):
>   MdeModulePkg/AtaAtapiPassThru: enable 64-bit PCI DMA
>   MdeModulePkg/EhciDxe: enable 64-bit PCI DMA
>   MdeModulePkg/NvmExpressDxe: enable 64-bit PCI DMA
>   MdeModulePkg/SdMmcPciHcDxe: enable 64-bit PCI DMA
>   MdeModulePkg/XhciDxe: enable 64-bit PCI DMA
>   MdeModulePkg/PciHostBridgeDxe: restrict 64-bit DMA to devices that
>     support it
>   ArmVirtPkg/FdtPciHostBridgeLib: enable 64-bit PCI DMA
>
>  ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c |  2 +-
>  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c             | 20 +++++++++++++++++-
>  MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c                          | 22 +++++++++++++++++++-
>  MdeModulePkg/Bus/Pci/EhciDxe/Ehci.h                          |  2 ++
>  MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c                     |  2 +-
>  MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c           | 13 ++++++++++++
>  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c      | 14 +++++++++----
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c           | 20 ++++++++++++++++++
>  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c                          | 22 +++++++++++++++++++-
>  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h                          |  2 ++
>  10 files changed, 110 insertions(+), 9 deletions(-)
>
> --
> 2.7.4
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/7] MdeModulePkg ArmVirtPkg: fixes for 64-bit PCI DMA
  2016-09-06  7:48 ` [PATCH 0/7] MdeModulePkg ArmVirtPkg: fixes for " Ard Biesheuvel
  2016-09-06  8:04   ` Zeng, Star
@ 2016-09-06  8:54   ` Ni, Ruiyu
  2016-09-06 10:36     ` Ard Biesheuvel
  1 sibling, 1 reply; 20+ messages in thread
From: Ni, Ruiyu @ 2016-09-06  8:54 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel-01, Tian, Feng, Zeng, Star,
	Gao, Liming
  Cc: Laszlo Ersek, Leif Lindholm

Ard,
The patch to MdeModulePkg/PciHostBridgeDxe is good.

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

Thanks/Ray

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Tuesday, September 6, 2016 3:48 PM
> To: edk2-devel-01 <edk2-devel@lists.01.org>; Tian, Feng
> <feng.tian@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming
> <liming.gao@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>; Leif Lindholm
> <leif.lindholm@linaro.org>; Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Subject: Re: [edk2] [PATCH 0/7] MdeModulePkg ArmVirtPkg: fixes for 64-bit
> PCI DMA
> 
> Feng, Star: do you have any feedback on these patches? Thanks.
> 
> On 5 September 2016 at 10:17, Ard Biesheuvel <ard.biesheuvel@linaro.org>
> wrote:
> > After moving ArmVirtQemu to the generic PciHostBridgeDxe, we noticed
> > that setting DmaAbove4G resulted in problems with the emulated EHCI
> > USB host controller, which were caused by the fact that the PCI layer
> > was providing DMA buffers allocated above 4 GB while the emulated EHCI
> > controller in QEMU does not indicate support for 64-bit addressing.
> >
> > As it turns out, the PCI drivers in MdeModulePkg *completely* ignore
> > the EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE attribute, and simply
> assume
> > that no PCI root bridge driver will produce mappings above 4 GB. On
> > ARM, this is problematic, since not all platforms have memory below 4
> > GB, and so having full support for DMA above 4 GB is indispensable.
> >
> > So first, make the various drivers under MdeModulePkg/Pci/Bus set the
> > EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE attributes for devices that
> > can support 64-bit DMA addressing (patches #1 - #5). Then, we can
> > update the host bridge driver to actually take these attributes into
> > account, and only create mappings above 4 GB for devices that have
> indicated support for it.
> >
> > Finally, in patch #7 we can remove the 4 GB DMA limit from ArmVirtPkg.
> >
> > Branch can be found here:
> > https://git.linaro.org/people/ard.biesheuvel/uefi-next.git/shortlog/re
> > fs/heads/pci-64bit-dma-fixes
> >
> > Ard Biesheuvel (7):
> >   MdeModulePkg/AtaAtapiPassThru: enable 64-bit PCI DMA
> >   MdeModulePkg/EhciDxe: enable 64-bit PCI DMA
> >   MdeModulePkg/NvmExpressDxe: enable 64-bit PCI DMA
> >   MdeModulePkg/SdMmcPciHcDxe: enable 64-bit PCI DMA
> >   MdeModulePkg/XhciDxe: enable 64-bit PCI DMA
> >   MdeModulePkg/PciHostBridgeDxe: restrict 64-bit DMA to devices that
> >     support it
> >   ArmVirtPkg/FdtPciHostBridgeLib: enable 64-bit PCI DMA
> >
> >  ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c |  2 +-
> >  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c             | 20
> +++++++++++++++++-
> >  MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c                          | 22
> +++++++++++++++++++-
> >  MdeModulePkg/Bus/Pci/EhciDxe/Ehci.h                          |  2 ++
> >  MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c                     |  2 +-
> >  MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c           | 13
> ++++++++++++
> >  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c      | 14
> +++++++++----
> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c           | 20
> ++++++++++++++++++
> >  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c                          | 22
> +++++++++++++++++++-
> >  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h                          |  2 ++
> >  10 files changed, 110 insertions(+), 9 deletions(-)
> >
> > --
> > 2.7.4
> >
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/7] MdeModulePkg ArmVirtPkg: fixes for 64-bit PCI DMA
  2016-09-06  8:54   ` Ni, Ruiyu
@ 2016-09-06 10:36     ` Ard Biesheuvel
  2016-09-06 12:41       ` Tian, Feng
  0 siblings, 1 reply; 20+ messages in thread
From: Ard Biesheuvel @ 2016-09-06 10:36 UTC (permalink / raw)
  To: Ni, Ruiyu
  Cc: edk2-devel-01, Tian, Feng, Zeng, Star, Gao, Liming, Laszlo Ersek,
	Leif Lindholm

On 6 September 2016 at 09:54, Ni, Ruiyu <ruiyu.ni@intel.com> wrote:
> Ard,
> The patch to MdeModulePkg/PciHostBridgeDxe is good.
>
> Reviewed-by: Ruiyu Ni <Ruiyu.ni@intel.com>
>

Thanks Ray!

Who is responsible for the AtapPassThru, USB, NVME and SDHCI drivers?

>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
>> Ard Biesheuvel
>> Sent: Tuesday, September 6, 2016 3:48 PM
>> To: edk2-devel-01 <edk2-devel@lists.01.org>; Tian, Feng
>> <feng.tian@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming
>> <liming.gao@intel.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>; Leif Lindholm
>> <leif.lindholm@linaro.org>; Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Subject: Re: [edk2] [PATCH 0/7] MdeModulePkg ArmVirtPkg: fixes for 64-bit
>> PCI DMA
>>
>> Feng, Star: do you have any feedback on these patches? Thanks.
>>
>> On 5 September 2016 at 10:17, Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> wrote:
>> > After moving ArmVirtQemu to the generic PciHostBridgeDxe, we noticed
>> > that setting DmaAbove4G resulted in problems with the emulated EHCI
>> > USB host controller, which were caused by the fact that the PCI layer
>> > was providing DMA buffers allocated above 4 GB while the emulated EHCI
>> > controller in QEMU does not indicate support for 64-bit addressing.
>> >
>> > As it turns out, the PCI drivers in MdeModulePkg *completely* ignore
>> > the EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE attribute, and simply
>> assume
>> > that no PCI root bridge driver will produce mappings above 4 GB. On
>> > ARM, this is problematic, since not all platforms have memory below 4
>> > GB, and so having full support for DMA above 4 GB is indispensable.
>> >
>> > So first, make the various drivers under MdeModulePkg/Pci/Bus set the
>> > EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE attributes for devices that
>> > can support 64-bit DMA addressing (patches #1 - #5). Then, we can
>> > update the host bridge driver to actually take these attributes into
>> > account, and only create mappings above 4 GB for devices that have
>> indicated support for it.
>> >
>> > Finally, in patch #7 we can remove the 4 GB DMA limit from ArmVirtPkg.
>> >
>> > Branch can be found here:
>> > https://git.linaro.org/people/ard.biesheuvel/uefi-next.git/shortlog/re
>> > fs/heads/pci-64bit-dma-fixes
>> >
>> > Ard Biesheuvel (7):
>> >   MdeModulePkg/AtaAtapiPassThru: enable 64-bit PCI DMA
>> >   MdeModulePkg/EhciDxe: enable 64-bit PCI DMA
>> >   MdeModulePkg/NvmExpressDxe: enable 64-bit PCI DMA
>> >   MdeModulePkg/SdMmcPciHcDxe: enable 64-bit PCI DMA
>> >   MdeModulePkg/XhciDxe: enable 64-bit PCI DMA
>> >   MdeModulePkg/PciHostBridgeDxe: restrict 64-bit DMA to devices that
>> >     support it
>> >   ArmVirtPkg/FdtPciHostBridgeLib: enable 64-bit PCI DMA
>> >
>> >  ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c |  2 +-
>> >  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c             | 20
>> +++++++++++++++++-
>> >  MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c                          | 22
>> +++++++++++++++++++-
>> >  MdeModulePkg/Bus/Pci/EhciDxe/Ehci.h                          |  2 ++
>> >  MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c                     |  2 +-
>> >  MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c           | 13
>> ++++++++++++
>> >  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c      | 14
>> +++++++++----
>> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c           | 20
>> ++++++++++++++++++
>> >  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c                          | 22
>> +++++++++++++++++++-
>> >  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h                          |  2 ++
>> >  10 files changed, 110 insertions(+), 9 deletions(-)
>> >
>> > --
>> > 2.7.4
>> >
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/7] MdeModulePkg ArmVirtPkg: fixes for 64-bit PCI DMA
  2016-09-06 10:36     ` Ard Biesheuvel
@ 2016-09-06 12:41       ` Tian, Feng
  2016-09-06 14:48         ` Ard Biesheuvel
  0 siblings, 1 reply; 20+ messages in thread
From: Tian, Feng @ 2016-09-06 12:41 UTC (permalink / raw)
  To: Ard Biesheuvel, Ni, Ruiyu
  Cc: edk2-devel-01, Zeng, Star, Gao, Liming, Laszlo Ersek,
	Leif Lindholm, Tian, Feng

ATA/USB/NVME/SD are ok to me

Reviewed-by: Feng Tian <feng.tian@Intel.com>

-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] 
Sent: Tuesday, September 6, 2016 6:36 PM
To: Ni, Ruiyu <ruiyu.ni@intel.com>
Cc: edk2-devel-01 <edk2-devel@lists.01.org>; Tian, Feng <feng.tian@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming <liming.gao@intel.com>; Laszlo Ersek <lersek@redhat.com>; Leif Lindholm <leif.lindholm@linaro.org>
Subject: Re: [edk2] [PATCH 0/7] MdeModulePkg ArmVirtPkg: fixes for 64-bit PCI DMA

On 6 September 2016 at 09:54, Ni, Ruiyu <ruiyu.ni@intel.com> wrote:
> Ard,
> The patch to MdeModulePkg/PciHostBridgeDxe is good.
>
> Reviewed-by: Ruiyu Ni <Ruiyu.ni@intel.com>
>

Thanks Ray!

Who is responsible for the AtapPassThru, USB, NVME and SDHCI drivers?

>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf 
>> Of Ard Biesheuvel
>> Sent: Tuesday, September 6, 2016 3:48 PM
>> To: edk2-devel-01 <edk2-devel@lists.01.org>; Tian, Feng 
>> <feng.tian@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming 
>> <liming.gao@intel.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>; Leif Lindholm 
>> <leif.lindholm@linaro.org>; Ard Biesheuvel 
>> <ard.biesheuvel@linaro.org>
>> Subject: Re: [edk2] [PATCH 0/7] MdeModulePkg ArmVirtPkg: fixes for 
>> 64-bit PCI DMA
>>
>> Feng, Star: do you have any feedback on these patches? Thanks.
>>
>> On 5 September 2016 at 10:17, Ard Biesheuvel 
>> <ard.biesheuvel@linaro.org>
>> wrote:
>> > After moving ArmVirtQemu to the generic PciHostBridgeDxe, we 
>> > noticed that setting DmaAbove4G resulted in problems with the 
>> > emulated EHCI USB host controller, which were caused by the fact 
>> > that the PCI layer was providing DMA buffers allocated above 4 GB 
>> > while the emulated EHCI controller in QEMU does not indicate support for 64-bit addressing.
>> >
>> > As it turns out, the PCI drivers in MdeModulePkg *completely* 
>> > ignore the EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE attribute, and 
>> > simply
>> assume
>> > that no PCI root bridge driver will produce mappings above 4 GB. On 
>> > ARM, this is problematic, since not all platforms have memory below 
>> > 4 GB, and so having full support for DMA above 4 GB is indispensable.
>> >
>> > So first, make the various drivers under MdeModulePkg/Pci/Bus set 
>> > the EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE attributes for devices 
>> > that can support 64-bit DMA addressing (patches #1 - #5). Then, we 
>> > can update the host bridge driver to actually take these attributes 
>> > into account, and only create mappings above 4 GB for devices that 
>> > have
>> indicated support for it.
>> >
>> > Finally, in patch #7 we can remove the 4 GB DMA limit from ArmVirtPkg.
>> >
>> > Branch can be found here:
>> > https://git.linaro.org/people/ard.biesheuvel/uefi-next.git/shortlog
>> > /re
>> > fs/heads/pci-64bit-dma-fixes
>> >
>> > Ard Biesheuvel (7):
>> >   MdeModulePkg/AtaAtapiPassThru: enable 64-bit PCI DMA
>> >   MdeModulePkg/EhciDxe: enable 64-bit PCI DMA
>> >   MdeModulePkg/NvmExpressDxe: enable 64-bit PCI DMA
>> >   MdeModulePkg/SdMmcPciHcDxe: enable 64-bit PCI DMA
>> >   MdeModulePkg/XhciDxe: enable 64-bit PCI DMA
>> >   MdeModulePkg/PciHostBridgeDxe: restrict 64-bit DMA to devices that
>> >     support it
>> >   ArmVirtPkg/FdtPciHostBridgeLib: enable 64-bit PCI DMA
>> >
>> >  ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c |  2 +-
>> >  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c             | 20
>> +++++++++++++++++-
>> >  MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c                          | 22
>> +++++++++++++++++++-
>> >  MdeModulePkg/Bus/Pci/EhciDxe/Ehci.h                          |  2 ++
>> >  MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c                     |  2 +-
>> >  MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c           | 13
>> ++++++++++++
>> >  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c      | 14
>> +++++++++----
>> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c           | 20
>> ++++++++++++++++++
>> >  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c                          | 22
>> +++++++++++++++++++-
>> >  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h                          |  2 ++
>> >  10 files changed, 110 insertions(+), 9 deletions(-)
>> >
>> > --
>> > 2.7.4
>> >
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/7] MdeModulePkg ArmVirtPkg: fixes for 64-bit PCI DMA
  2016-09-06 12:41       ` Tian, Feng
@ 2016-09-06 14:48         ` Ard Biesheuvel
  0 siblings, 0 replies; 20+ messages in thread
From: Ard Biesheuvel @ 2016-09-06 14:48 UTC (permalink / raw)
  To: Tian, Feng
  Cc: Ni, Ruiyu, edk2-devel-01, Zeng, Star, Gao, Liming, Laszlo Ersek,
	Leif Lindholm

On 6 September 2016 at 13:41, Tian, Feng <feng.tian@intel.com> wrote:
> ATA/USB/NVME/SD are ok to me
>
> Reviewed-by: Feng Tian <feng.tian@Intel.com>
>

Thanks everyone

Committed as

a2c9b0873a77 MdeModulePkg/AtaAtapiPassThru: enable 64-bit PCI DMA
167c3fb45674 MdeModulePkg/EhciDxe: enable 64-bit PCI DMA
4e28ea2c29e0 MdeModulePkg/NvmExpressDxe: enable 64-bit PCI DMA
df0a0e4b6fae MdeModulePkg/SdMmcPciHcDxe: enable 64-bit PCI DMA
5c1b371a8839 MdeModulePkg/XhciDxe: enable 64-bit PCI DMA
e58a71d9c50b MdeModulePkg/PciHostBridgeDxe: restrict 64-bit DMA to
devices that support it
4c0b2d25c61c ArmVirtPkg/FdtPciHostBridgeLib: enable 64-bit PCI DMA



> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Tuesday, September 6, 2016 6:36 PM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>
> Cc: edk2-devel-01 <edk2-devel@lists.01.org>; Tian, Feng <feng.tian@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming <liming.gao@intel.com>; Laszlo Ersek <lersek@redhat.com>; Leif Lindholm <leif.lindholm@linaro.org>
> Subject: Re: [edk2] [PATCH 0/7] MdeModulePkg ArmVirtPkg: fixes for 64-bit PCI DMA
>
> On 6 September 2016 at 09:54, Ni, Ruiyu <ruiyu.ni@intel.com> wrote:
>> Ard,
>> The patch to MdeModulePkg/PciHostBridgeDxe is good.
>>
>> Reviewed-by: Ruiyu Ni <Ruiyu.ni@intel.com>
>>
>
> Thanks Ray!
>
> Who is responsible for the AtapPassThru, USB, NVME and SDHCI drivers?
>
>>> -----Original Message-----
>>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf
>>> Of Ard Biesheuvel
>>> Sent: Tuesday, September 6, 2016 3:48 PM
>>> To: edk2-devel-01 <edk2-devel@lists.01.org>; Tian, Feng
>>> <feng.tian@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming
>>> <liming.gao@intel.com>
>>> Cc: Laszlo Ersek <lersek@redhat.com>; Leif Lindholm
>>> <leif.lindholm@linaro.org>; Ard Biesheuvel
>>> <ard.biesheuvel@linaro.org>
>>> Subject: Re: [edk2] [PATCH 0/7] MdeModulePkg ArmVirtPkg: fixes for
>>> 64-bit PCI DMA
>>>
>>> Feng, Star: do you have any feedback on these patches? Thanks.
>>>
>>> On 5 September 2016 at 10:17, Ard Biesheuvel
>>> <ard.biesheuvel@linaro.org>
>>> wrote:
>>> > After moving ArmVirtQemu to the generic PciHostBridgeDxe, we
>>> > noticed that setting DmaAbove4G resulted in problems with the
>>> > emulated EHCI USB host controller, which were caused by the fact
>>> > that the PCI layer was providing DMA buffers allocated above 4 GB
>>> > while the emulated EHCI controller in QEMU does not indicate support for 64-bit addressing.
>>> >
>>> > As it turns out, the PCI drivers in MdeModulePkg *completely*
>>> > ignore the EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE attribute, and
>>> > simply
>>> assume
>>> > that no PCI root bridge driver will produce mappings above 4 GB. On
>>> > ARM, this is problematic, since not all platforms have memory below
>>> > 4 GB, and so having full support for DMA above 4 GB is indispensable.
>>> >
>>> > So first, make the various drivers under MdeModulePkg/Pci/Bus set
>>> > the EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE attributes for devices
>>> > that can support 64-bit DMA addressing (patches #1 - #5). Then, we
>>> > can update the host bridge driver to actually take these attributes
>>> > into account, and only create mappings above 4 GB for devices that
>>> > have
>>> indicated support for it.
>>> >
>>> > Finally, in patch #7 we can remove the 4 GB DMA limit from ArmVirtPkg.
>>> >
>>> > Branch can be found here:
>>> > https://git.linaro.org/people/ard.biesheuvel/uefi-next.git/shortlog
>>> > /re
>>> > fs/heads/pci-64bit-dma-fixes
>>> >
>>> > Ard Biesheuvel (7):
>>> >   MdeModulePkg/AtaAtapiPassThru: enable 64-bit PCI DMA
>>> >   MdeModulePkg/EhciDxe: enable 64-bit PCI DMA
>>> >   MdeModulePkg/NvmExpressDxe: enable 64-bit PCI DMA
>>> >   MdeModulePkg/SdMmcPciHcDxe: enable 64-bit PCI DMA
>>> >   MdeModulePkg/XhciDxe: enable 64-bit PCI DMA
>>> >   MdeModulePkg/PciHostBridgeDxe: restrict 64-bit DMA to devices that
>>> >     support it
>>> >   ArmVirtPkg/FdtPciHostBridgeLib: enable 64-bit PCI DMA
>>> >
>>> >  ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c |  2 +-
>>> >  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c             | 20
>>> +++++++++++++++++-
>>> >  MdeModulePkg/Bus/Pci/EhciDxe/Ehci.c                          | 22
>>> +++++++++++++++++++-
>>> >  MdeModulePkg/Bus/Pci/EhciDxe/Ehci.h                          |  2 ++
>>> >  MdeModulePkg/Bus/Pci/EhciDxe/EhciSched.c                     |  2 +-
>>> >  MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c           | 13
>>> ++++++++++++
>>> >  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c      | 14
>>> +++++++++----
>>> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c           | 20
>>> ++++++++++++++++++
>>> >  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c                          | 22
>>> +++++++++++++++++++-
>>> >  MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h                          |  2 ++
>>> >  10 files changed, 110 insertions(+), 9 deletions(-)
>>> >
>>> > --
>>> > 2.7.4
>>> >
>>> _______________________________________________
>>> edk2-devel mailing list
>>> edk2-devel@lists.01.org
>>> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2016-09-06 14:48 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-05  9:17 [PATCH 0/7] MdeModulePkg ArmVirtPkg: fixes for 64-bit PCI DMA Ard Biesheuvel
2016-09-05  9:17 ` [PATCH 1/7] MdeModulePkg/AtaAtapiPassThru: enable " Ard Biesheuvel
2016-09-05  9:17 ` [PATCH 2/7] MdeModulePkg/EhciDxe: " Ard Biesheuvel
2016-09-05 12:19   ` Laszlo Ersek
2016-09-05 12:44     ` Ard Biesheuvel
2016-09-05 13:06       ` Laszlo Ersek
2016-09-05  9:17 ` [PATCH 3/7] MdeModulePkg/NvmExpressDxe: " Ard Biesheuvel
2016-09-05  9:17 ` [PATCH 4/7] MdeModulePkg/SdMmcPciHcDxe: " Ard Biesheuvel
2016-09-05  9:17 ` [PATCH 5/7] MdeModulePkg/XhciDxe: " Ard Biesheuvel
2016-09-05 12:14   ` Laszlo Ersek
2016-09-05  9:17 ` [PATCH 6/7] MdeModulePkg/PciHostBridgeDxe: restrict 64-bit DMA to devices that support it Ard Biesheuvel
2016-09-05 12:04   ` Laszlo Ersek
2016-09-05  9:17 ` [PATCH 7/7] ArmVirtPkg/FdtPciHostBridgeLib: enable 64-bit PCI DMA Ard Biesheuvel
2016-09-05 12:04   ` Laszlo Ersek
2016-09-06  7:48 ` [PATCH 0/7] MdeModulePkg ArmVirtPkg: fixes for " Ard Biesheuvel
2016-09-06  8:04   ` Zeng, Star
2016-09-06  8:54   ` Ni, Ruiyu
2016-09-06 10:36     ` Ard Biesheuvel
2016-09-06 12:41       ` Tian, Feng
2016-09-06 14:48         ` Ard Biesheuvel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox