On Thu, Apr 21, 2016 at 2:31 PM, Ard Biesheuvel ard.biesheuvel@linaro.org wrote:
On 21 April 2016 at 19:29, Ricardo Salveti ricardo.salveti@linaro.org wrote:
Ircloud is off for me, so replying here instead.
On Fri, Apr 15, 2016 at 8:52 AM, Ard Biesheuvel ard.biesheuvel@linaro.org wrote:
The copy-pasted PciHostBridgeDxe driver has some interesting restrictions that are difficult to fulfil on Seattle, i.e., its AllocateBuffer() and IoMap() operations require the DMA address to be below 4 GB. This would only makes sense in the presence of 32-bit only DMA bus masters that are not behind a SMMU, but in the Seattle case, it is completely pointless since it does not have any RAM below 4 GB in the first place.
So simply drop these restrictions.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel ard.biesheuvel@linaro.org
.../Drivers/PciHostBridgeDxe/PciRootBridgeIo.c | 91 +--------------------- 1 file changed, 2 insertions(+), 89 deletions(-)
diff --git a/Platforms/AMD/Styx/Drivers/PciHostBridgeDxe/PciRootBridgeIo.c b/Platforms/AMD/Styx/Drivers/PciHostBridgeDxe/PciRootBridgeIo.c index 4ac89fb7548f..941c330a4228 100644 --- a/Platforms/AMD/Styx/Drivers/PciHostBridgeDxe/PciRootBridgeIo.c +++ b/Platforms/AMD/Styx/Drivers/PciHostBridgeDxe/PciRootBridgeIo.c @@ -1715,89 +1715,7 @@ DEBUG((EFI_D_ERROR, "RootBridgeIoMap() - %d\n", __LINE__)); return EFI_INVALID_PARAMETER; }
- //
- // Most PCAT like chipsets can not handle performing DMA above 4GB.
- // If any part of the DMA transfer being mapped is above 4GB, then
- // map the DMA transfer to a buffer below 4GB.
- //
- PhysicalAddress = (EFI_PHYSICAL_ADDRESS) (UINTN) HostAddress;
- if ((PhysicalAddress + *NumberOfBytes) > 0x100000000ULL) {
- //
- // Common Buffer operations can not be remapped. If the common buffer
- // if above 4GB, then it is not possible to generate a mapping, so return
- // an error.
- //
- if (Operation == EfiPciOperationBusMasterCommonBuffer || Operation == EfiPciOperationBusMasterCommonBuffer64) {
return EFI_UNSUPPORTED;- }
- //
- // Allocate a MAP_INFO structure to remember the mapping when Unmap() is
- // called later.
- //
- Status = gBS->AllocatePool (
EfiBootServicesData,sizeof(MAP_INFO),(VOID **)&MapInfo);- if (EFI_ERROR (Status)) {
*NumberOfBytes = 0;return Status;- }
- //
- // Return a pointer to the MAP_INFO structure in Mapping
- //
- *Mapping = MapInfo;
- //
- // Initialize the MAP_INFO structure
- //
- MapInfo->Operation = Operation;
- MapInfo->NumberOfBytes = *NumberOfBytes;
- MapInfo->NumberOfPages = EFI_SIZE_TO_PAGES(*NumberOfBytes);
- MapInfo->HostAddress = PhysicalAddress;
- MapInfo->MappedHostAddress = 0x00000000ffffffff;
- //
- // Allocate a buffer below 4GB to map the transfer to.
- //
- Status = gBS->AllocatePages (
AllocateMaxAddress,EfiBootServicesData,MapInfo->NumberOfPages,&MapInfo->MappedHostAddress);- if (EFI_ERROR (Status)) {
gBS->FreePool (MapInfo);*NumberOfBytes = 0;return Status;- }
- //
- // If this is a read operation from the Bus Master's point of view,
- // then copy the contents of the real buffer into the mapped buffer
- // so the Bus Master can read the contents of the real buffer.
- //
- if (Operation == EfiPciOperationBusMasterRead || Operation == EfiPciOperationBusMasterRead64) {
CopyMem ((VOID *)(UINTN)MapInfo->MappedHostAddress,(VOID *)(UINTN)MapInfo->HostAddress,MapInfo->NumberOfBytes);- }
- //
- // The DeviceAddress is the address of the maped buffer below 4GB
- //
- *DeviceAddress = MapInfo->MappedHostAddress;
- } else {
- //
- // The transfer is below 4GB, so the DeviceAddress is simply the HostAddress
- //
- *DeviceAddress = PhysicalAddress;
- }
- *DeviceAddress = PhysicalAddress;
The 3 errors I had: /tmp/build/edk2/OpenPlatformPkg/Platforms/AMD/Styx/Drivers/PciHostBridgeDxe/PciRootBridgeIo.c: In function 'RootBridgeIoMap': /tmp/build/edk2/OpenPlatformPkg/Platforms/AMD/Styx/Drivers/PciHostBridgeDxe/PciRootBridgeIo.c:1699:13: error: unused variable 'MapInfo' [-Werror=unused-variable] MAP_INFO *MapInfo; ^ /tmp/build/edk2/OpenPlatformPkg/Platforms/AMD/Styx/Drivers/PciHostBridgeDxe/PciRootBridgeIo.c:1697:14: error: unused variable 'Status' [-Werror=unused-variable] EFI_STATUS Status; ^
These can both simply be dropped
/tmp/build/edk2/OpenPlatformPkg/Platforms/AMD/Styx/Drivers/PciHostBridgeDxe/PciRootBridgeIo.c:1718:18: error: 'PhysicalAddress' may be used uninitialized in this function [-Werror=maybe-uninitialized] *DeviceAddress = PhysicalAddress; ^ cc1: all warnings being treated as errors
Here should *DeviceAddress be just HostAddress?
No, please use
*DeviceAddress = (EFI_PHYSICAL_ADDRESS) (UINTN) HostAddress;
and drop the declaration of PhysicalAddress
Tried the fixed version and didn't help, but pushed to the private dev-FDK101 branch as we would need it anyway.
Thanks,