Re: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Fix DMA on SDHC v3 64-bit systems

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On Fri, 1 Mar 2019 at 11:32, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> On Fri, 1 Mar 2019 at 01:19, Ashish Singhal <ashishsingha@nvidia.com> wrote:
> >
> > Eugene,
> >
> > Small question. Did the issue appear after the V4 patch went in? Looking at the code before that patch, we were enabling 64b dma in pci based on capability register already despite of driver supporting only 32b dma.
> >
>
> I think this may have been an oversight on my part when I originally
> added the DUAL_ADDRESS_CYCLE handling.
>
> The following commit added EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE to the
> host bridge driver
>
> commit e58a71d9c50ba641b5ab19f5ce2cbf772187de4d
> Author: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Date:   Mon Sep 5 09:55:16 2016 +0100
>
>     MdeModulePkg/PciHostBridgeDxe: restrict 64-bit DMA to devices that
> support it
>
>     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.
>
> and the SDHCI driver was fixed accordingly in
>
> Author: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Date:   Mon Sep 5 09:51:48 2016 +0100
>
>     MdeModulePkg/SdMmcPciHcDxe: enable 64-bit PCI DMA
>
>     PCI controller drivers must set the EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE
>     attribute if the controller supports 64-bit DMA addressing.
>
> So before these changes, we were in the exact same situation, but
> since PC platforms never enable DMA above 4 GB in the first place,
> nobody ever noticed until we started running this code on arm64
> platforms that have no 32-bit addressable DRAM to begin with.
>
> The obvious conclusion is that the driver should not set the
> EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE attribute if the device does
> not support it, or, which seems to be our case, if the driver does not
> implement the 64-bit DMA mode that the driver does support.

Correction: that the *device* does support.

> However,
> since there are platforms for which bounce buffering is not an option
> (since there is no 32-bit addressable memory to bounce to), this is
> not just a performance optimization, and so it would be useful to fix
> the code so it can drive all 64-bit DMA capable hardware.