Re: [RFC] MdeModulePkg/PciHostBridge: Add address translation support

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

Hi Heyi,

On 20 December 2017 at 08:21, Heyi Guo <heyi.guo@linaro.org> wrote:
> PCIe on some ARM platforms requires address translation, not only for
> legacy IO access, but also for 32bit memory BAR access as well. There
> will be "Address Translation Unit" or something similar in PCI host
> bridges to translation CPU address to PCI address and vice versa. So
> we think it may be useful to add address translation support to the
> generic PCI host bridge driver.
>

I agree. While unusual on a PC, it is quite common on other
architectures to have more complex non 1:1 topologies, which currently
require a forked PciHostBridgeDxe driver with local changes applied.

> This RFC only contains one minor change to the definition of
> PciHostBridgeLib, and there certainly will be a lot of other changes
> to make it work, including:
>
> 1. Use CPU address for GCD space add and allocate operations, instead
> of PCI address; also IO space will be changed to memory space if
> translation exists.
>

For I/O space, the translation should simply be applied to the I/O
range. I don't think it makes sense to use memory space here, given
that it is specific to architectures that lack native port I/O.

> 2. RootBridgeIoMemRead/Write, RootBridgeIoRead/Write need to get
> translation of the corresponding aperture, add the translation to the
> input address, and then call CpuIo2 protocol; IO access will also be
> converted to memory access if IO translation exists.
>

Again, why is this necessary? A host bridge that implements a non 1:1
translation for port I/O ranges may be part of a system that has
native port I/O, and so the translation should be based on that.

> 3. RootBridgeIoConfiguration needs to fill AddrTranslationOffset in
> the discriptor.
>

Indeed. Note that this has been a source of much confusion lately,
should we should discuss this carefully before spending time on an
implementation.

> If it makes sense, then I'll continue to prepare the formal patch.
>
> Any comments?
>
> Thanks,
>
> Gary (Heyi Guo)
>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Jason Zhang <jason.zhang@linaro.org>
>
> ---
>  MdeModulePkg/Include/Library/PciHostBridgeLib.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/MdeModulePkg/Include/Library/PciHostBridgeLib.h b/MdeModulePkg/Include/Library/PciHostBridgeLib.h
> index d42e9ec..b9e8c0f 100644
> --- a/MdeModulePkg/Include/Library/PciHostBridgeLib.h
> +++ b/MdeModulePkg/Include/Library/PciHostBridgeLib.h
> @@ -22,6 +22,7 @@
>  typedef struct {
>    UINT64 Base;
>    UINT64 Limit;
> +  UINT64 Translation;
>  } PCI_ROOT_BRIDGE_APERTURE;
>
>  typedef struct {
> --
> 2.7.4
>