Re: [PATCH 1/3] MdeModulePkg/Include: Add IOMMU protocol definition.

["Yao, Jiewen" <jiewen.yao@intel.com>]

Thanks for the info.

Comment inline.




From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
Sent: Wednesday, March 29, 2017 10:23 PM
To: Yao, Jiewen <jiewen.yao@intel.com>
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Leo Duran <leo.duran@amd.com>; edk2-devel@lists.01.org; Brijesh Singh <brijesh.singh@amd.com>
Subject: Re: [edk2] [PATCH 1/3] MdeModulePkg/Include: Add IOMMU protocol definition.

On 29 March 2017 at 00:45, Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> wrote:
> Agree. That is a good idea.
>
>
>
> I will add that in V2 patch.
>

Hello Jiewen,

As a bit of background, what I have in mind is an enhancement of the
PCI root bridge I/O allocate, map and unmap methods so that situations
that would currently lead to failure or to suboptimal performance are
left for the IOMMU protocol to handle if one is present. Note that
this may imply having IOMMU protocol instances for each PCI root
bridge, and for other masters as well. (For example, AMD Seattle has
separate IOMMUs for PCI and for the networking controllers, which are
wired to the internal interconnect directly)

[Jiewen] I am not very sure what do you mean.

Fist, let me explain Intel platform.

There might be multiple IOMMU engines on one platform. one for graphic and one for rest PCI device (such as ATA/USB).
But all IOMMU engines are reported by one “DMAR” ACPI table.

In such case, the Intel IOMMU driver just produces one IOMMU protocol, based upon DMAR ACPI table.

This IOMMU protocol provider can use UEFI device path to distinguish if the device is graphic or ATA/USB, and find out corresponding IOMMU engine.

I know AMD has “IVRS” ACPI table and ARM has “IORT” ACPI table.

In such case, I assume AMD may have one IOMMU protocol based upon IVRS table, and ARM may have one IOMMU protocol based upon IORT table.
And this single IOMMU protocol provider can handle multiple IOMMU engines on one system.

Is that understand same as yours?




So in RootBridgeIoMap(), for instance, we have this condition

  PhysicalAddress = (EFI_PHYSICAL_ADDRESS) (UINTN) HostAddress;
  if ((!RootBridge->DmaAbove4G ||
       (Operation != EfiPciOperationBusMasterRead64 &&
        Operation != EfiPciOperationBusMasterWrite64 &&
        Operation != EfiPciOperationBusMasterCommonBuffer64)) &&
      ((PhysicalAddress + *NumberOfBytes) > SIZE_4GB)) {

to decide whether bounce buffering is necessary (or even possible).
The mapping between DeviceAddress and HostAddress could be supplied by
the IOMMU protocol instance, which also means we should reinterpret
DmaAbove4G and other variables related to 32-bit addressing to apply
to the device address and not the physical address.

Similarly, in RootBridgeIoAllocateBuffer(), a failure to allocate
below 4 GB may not be an error if the IOMMU protocol instance can
provide a 32-bit addressable mapping for it.

[Jiewen] It is a good idea to remap based upon IOMMU.

However, one potential problem is that the memory size if not IOMMU page aligned.
In such case, PciRootBridge driver has to allocate another IOMMU page aligned memory for DMA buffer.

I believe the benefit will be got, only if the device driver who submit DMA request allocate IOMMU page aligned memory for DMA request.



I am aware that this complicates matters for you, but having IOMMU
support in the generic PCI host bridge driver is extremely useful for
us. I am happy to help out in any way I can.
[Jiewen] Yes, I definitely need comment for ARM/AMD/other system architecture.



Thanks,
Ard.