Re: [Linaro-uefi] [PATCH] Platforms/AMD: remove bogus 4 GB limit in PciHostBridgeDxe

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; ^ /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?

Thanks,