* [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