Hi Ard,
Thanks for you comments. Please see my comments below.
On 10/10/2017 03:59 AM, Ard Biesheuvel wrote:
On 10 October 2017 at 04:41, Daniil Egranov <daniil.egra...@arm.com> wrote:
Hi Ard, Ray,
Thanks for your comments.
On 10/09/2017 07:23 AM, Ard Biesheuvel wrote:
On 9 October 2017 at 11:40, Ard Biesheuvel <ard.biesheu...@linaro.org>
wrote:
On 9 October 2017 at 08:42, Ni, Ruiyu <ruiyu...@intel.com> wrote:
The "read"/"write" is from the Bus Master's point of view.
Thanks/Ray
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
Daniil
Egranov
Sent: Monday, October 9, 2017 9:16 AM
To: edk2-devel@lists.01.org
Cc: leif.lindh...@linaro.org; Zeng, Star <star.z...@intel.com>;
ard.biesheu...@linaro.org
Subject: [edk2] [PATCH] MdeModulePkg/PciHostBridgeDxe: Fixed PCI DMA
Map/Umap bounce buffer
The patch corrects the logic of transferring data between a bounce
buffer and a
real buffer above 4GB:
1. In the case of mapping a bounce buffer for the write operation, data
from a
real buffer should be copied into a bounce buffer.
2.In the case of unmapping a bounce buffer for the read operation, data
should
be copied from a bounce buffer into a real buffer.
The patch resolves a Juno board issue with the the grub and SATA
drives.
I am intrigued by this.
So as I suggested, this has to do with 64-bit DMA, but not in the way
I suspected. UEFI itself never hits this code path, because it runs
entirely < 32 GB, but as soon as GRUB starts allocating loader data
and use it for DMA, the bounce buffering kicks in because apparently,
the SATA controller is not 64-bit DMA capable.
Are you using the SataSiI3132Dxe driver on Juno? Does this help at all?
diff --git a/EmbeddedPkg/Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c
b/EmbeddedPkg/Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c
index 2fb5fd68db01..a938563ebdd6 100644
--- a/EmbeddedPkg/Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c
+++ b/EmbeddedPkg/Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c
@@ -104,7 +104,7 @@ SiI3132AtaPassThruCommand (
}
Status = PciIo->Map (
- PciIo, EfiPciIoOperationBusMasterRead,
+ PciIo, EfiPciIoOperationBusMasterWrite,
Packet->InDataBuffer, &InDataBufferLength,
&PhysInDataBuffer, &PciAllocMapping
);
if (EFI_ERROR (Status)) {
@@ -139,7 +139,7 @@ SiI3132AtaPassThruCommand (
OutDataBufferLength = Packet->OutTransferLength *
SataDevice->BlockSize;
Status = PciIo->Map (
- PciIo, EfiPciIoOperationBusMasterWrite,
+ PciIo, EfiPciIoOperationBusMasterRead,
Packet->OutDataBuffer, &OutDataBufferLength,
&PhysOutDataBuffer, &PciAllocMapping
);
if (EFI_ERROR (Status)) {
Also, it might make sense to find out if the hardware is really not
64-bit DMA capable, or whether the driver simply fails to set the
EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE attribute.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Swapping the EfiPciIoOperationBusMasterRead and
EfiPciIoOperationBusMasterWrite operations in the SiI3132AtaPassThru.c fixes
the problem as well. The driver's patch will be the correct fix for this
issue. I think i missed the point what these operations are from the Bus
Master's perspective.
Good!
The old PciHostBridge Juno driver was using NullDmaLib so it was direct
mapping. That explains why the SataSiI3132Dxe worked with the original host
bridge driver and failed with the new one.
NullDmaLib has nothing to do with this. The difference between the old
driver and the generic one is that the old driver always enables
64-bit DMA, while the generic one only does so if the driver sets the
EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE attribute. So to fix this
driver, we should
I meant what the NullDmaLib masked out this issue.
a) fix the swapped constants above
b) set EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE in the Start() hook
The PCIe is a serial bus protocol and does not implement DAC. The
meaning of this attribute is understandable but the name is incorrect.
PCIe designed with native 64-bit addressing so in context of PCIe this
attribute is not valid ... and I doubt what any legacy PCI devices are
still exist/usable.
Anyway, I set this attribute in the patch. In Juno case, bounce buffer
is not used anymore.
c) add code to disable DMA at ExitBootServices() [or the controller
may scribble over RAM when the kernel takes over]
d) replace mbStarted with a per-controller attribute, given that this
is a UEFI driver model implementation that could theoretically drive
multiple hardware instances concurrently.
I sent a set of patches with all changes. Please take a look.
Thanks,
Daniil
Thanks,
Ard.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
