Thanks. Please let me know if any further changes are needed.

Regards,

Heyi

On Wed, Mar 07, 2018 at 12:30:59PM +0800, Ni, Ruiyu wrote:
> On 3/6/2018 10:44 AM, Guo Heyi wrote:
> >Hi Ray,
> >
> >Any comments for v5?
> 
> Heyi,
> Some backward compatibility concerns were received from internal production
> teams. Current change will cause silent failure on old platforms because
> TranslationOffset might be random if uninitialized.
> I will solve the concern and then send out updates to you, hopefully by end
> of next week.
> 
> >
> >Regards,
> >
> >Heyi
> >
> >On Thu, Mar 01, 2018 at 02:57:22PM +0800, Heyi Guo wrote:
> >>PCI address translation is necessary for some non-x86 platforms. On
> >>such platforms, address value (denoted as "device address" or "address
> >>in PCI view") set to PCI BAR registers in configuration space might be
> >>different from the address which is used by CPU to access the
> >>registers in memory BAR or IO BAR spaces (denoted as "host address" or
> >>"address in CPU view"). The difference between the two addresses is
> >>called "Address Translation Offset" or simply "translation", and can
> >>be represented by "Address Translation Offset" in ACPI QWORD Address
> >>Space Descriptor (Offset 0x1E). However UEFI and ACPI differs on the
> >>definitions of QWORD Address Space Descriptor, and we will follow UEFI
> >>definition on UEFI protocols, such as PCI root bridge IO protocol and
> >>PCI IO protocol. In UEFI 2.7, "Address Translation Offset" is "Offset
> >>to apply to the Starting address to convert it to a PCI address". This
> >>means:
> >>
> >>1. Translation = device address - host address.
> >>
> >>2. PciRootBridgeIo->Configuration should return CPU view address, as
> >>well as PciIo->GetBarAttributes.
> >>
> >>Summary of addresses used in protocol interfaces and internal
> >>implementations:
> >>
> >>1. *Only* the following protocol interfaces assume Address is Device
> >>    Address:
> >>(1). PciHostBridgeResourceAllocation.GetProposedResources()
> >>      Otherwise PCI bus driver cannot set correct address into PCI
> >>      BARs.
> >>(2). PciRootBridgeIo.Mem.Read() and PciRootBridgeIo.Mem.Write()
> >>(3). PciRootBridgeIo.CopyMem()
> >>UEFI and PI spec have clear statements for all other protocol
> >>interfaces about the address type.
> >>
> >>2. Library interfaces and internal implementation:
> >>(1). Base and Limit in PCI_ROOT_BRIDGE_APERTURE are device address.
> >>      It is easy to check whether the address is below 4G or above 4G.
> >>(2). Addresses in PCI_ROOT_BRIDGE_INSTANCE.ResAllocNode are host
> >>      address, for they are allocated from GCD.
> >>(3). Address passed to PciHostBridgeResourceConflict is host address,
> >>      for it comes from PCI_ROOT_BRIDGE_INSTANCE.ResAllocNode.
> >>
> >>RESTRICTION: to simplify the situation, we require the alignment of
> >>Translation must be larger than any BAR alignment in the same root
> >>bridge, so that resource allocation alignment can be applied to both
> >>device address and host address.
> >>
> >>Contributed-under: TianoCore Contribution Agreement 1.1
> >>Signed-off-by: Heyi Guo <heyi....@linaro.org>
> >>Cc: Ruiyu Ni <ruiyu...@intel.com>
> >>Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>
> >>Cc: Star Zeng <star.z...@intel.com>
> >>Cc: Eric Dong <eric.d...@intel.com>
> >>Cc: Laszlo Ersek <ler...@redhat.com>
> >>Cc: Michael D Kinney <michael.d.kin...@intel.com>
> >>---
> >>
> >>Notes:
> >>     v5:
> >>     - Add check for the alignment of Translation against the BAR alignment
> >>       [Ray]
> >>     - Keep coding style of comments consistent with the context [Ray]
> >>     - Comment change for Base in PCI_RES_NODE [Ray]
> >>     - Add macros of TO_HOST_ADDRESS and TO_DEVICE_ADDRESS for address type
> >>       conversion (After that we can also simply the comments about the
> >>       calculation [Ray]
> >>     - Add check for bus translation offset in CreateRootBridge(), making
> >>       sure it is zero, and unify code logic for all types of resource
> >>       after that [Ray]
> >>     - Use GetTranslationByResourceType() to simplify the code in
> >>       RootBridgeIoConfiguration() (also fix a bug in previous patch
> >>       version of missing a break after case TypePMem64) [Ray]
> >>     - Commit message refinement [Ray]
> >>     v4:
> >>     - Add ASSERT (FALSE) to default branch in GetTranslationByResourceType
> >>       [Laszlo]
> >>     - Fix bug when passing BaseAddress to gDS->AllocateIoSpace and
> >>       gDS->AllocateMemorySpace [Laszlo]
> >>     - Add comment for applying alignment to both device address and host
> >>       address, and add NOTE for the alignment requirement of Translation,
> >>       as well as in commit message [Laszlo][Ray]
> >>     - Improve indention for the code in CreateRootBridge [Laszlo]
> >>     - Improve comment for Translation in PCI_ROOT_BRIDGE_APERTURE
> >>       definition [Laszlo]
> >>     - Ignore translation of bus in CreateRootBridge
> >>     v4:
> >>     - Add ASSERT (FALSE) to default branch in GetTranslationByResourceType
> >>       [Laszlo]
> >>     - Fix bug when passing BaseAddress to gDS->AllocateIoSpace and
> >>       gDS->AllocateMemorySpace [Laszlo]
> >>     - Add comment for applying alignment to both device address and host
> >>       address, and add NOTE for the alignment requirement of Translation,
> >>       as well as in commit message [Laszlo][Ray]
> >>     - Improve indention for the code in CreateRootBridge [Laszlo]
> >>     - Improve comment for Translation in PCI_ROOT_BRIDGE_APERTURE
> >>       definition [Laszlo]
> >>     - Ignore translation of bus in CreateRootBridge
> >>
> >>  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.h   |  21 ++++
> >>  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostResource.h |   3 +
> >>  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c   | 129 
> >> +++++++++++++++++---
> >>  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 118 
> >> ++++++++++++++++--
> >>  4 files changed, 245 insertions(+), 26 deletions(-)
> >>
> >>diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.h 
> >>b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.h
> >>index 9a8ca21f4819..c2791ea5c2a4 100644
> >>--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.h
> >>+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.h
> >>@@ -38,6 +38,13 @@ typedef struct {
> >>  #define PCI_HOST_BRIDGE_FROM_THIS(a) CR (a, PCI_HOST_BRIDGE_INSTANCE, 
> >> ResAlloc, PCI_HOST_BRIDGE_SIGNATURE)
> >>  //
> >>+// Macros to translate device address to host address and vice versa. 
> >>According
> >>+// to UEFI 2.7, device address = host address + translation offset.
> >>+//
> >>+#define TO_HOST_ADDRESS(DeviceAddress,TranslationOffset) ((DeviceAddress) 
> >>- (TranslationOffset))
> >>+#define TO_DEVICE_ADDRESS(HostAddress,TranslationOffset) ((HostAddress) + 
> >>(TranslationOffset))
> >>+
> >>+//
> >>  // Driver Entry Point
> >>  //
> >>  /**
> >>@@ -247,6 +254,20 @@ ResourceConflict (
> >>    IN  PCI_HOST_BRIDGE_INSTANCE *HostBridge
> >>    );
> >>+/**
> >>+  This routine gets translation offset from a root bridge instance by 
> >>resource type.
> >>+
> >>+  @param RootBridge The Root Bridge Instance for the resources.
> >>+  @param ResourceType The Resource Type of the translation offset.
> >>+
> >>+  @retval The Translation Offset of the specified resource.
> >>+**/
> >>+UINT64
> >>+GetTranslationByResourceType (
> >>+  IN  PCI_ROOT_BRIDGE_INSTANCE     *RootBridge,
> >>+  IN  PCI_RESOURCE_TYPE            ResourceType
> >>+  );
> >>+
> >>  extern EFI_METRONOME_ARCH_PROTOCOL *mMetronome;
> >>  extern EFI_CPU_IO2_PROTOCOL        *mCpuIo;
> >>  #endif
> >>diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostResource.h 
> >>b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostResource.h
> >>index 8612c0c3251b..a6c3739db368 100644
> >>--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostResource.h
> >>+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostResource.h
> >>@@ -38,6 +38,9 @@ typedef enum {
> >>  typedef struct {
> >>    PCI_RESOURCE_TYPE Type;
> >>+  //
> >>+  // Base is a host address
> >>+  //
> >>    UINT64            Base;
> >>    UINT64            Length;
> >>    UINT64            Alignment;
> >>diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c 
> >>b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> >>index 1494848c3e8c..42ded2855c71 100644
> >>--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> >>+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> >>@@ -33,6 +33,39 @@ EFI_EVENT                   mIoMmuEvent;
> >>  VOID                        *mIoMmuRegistration;
> >>  /**
> >>+  This routine gets translation offset from a root bridge instance by 
> >>resource type.
> >>+
> >>+  @param RootBridge The Root Bridge Instance for the resources.
> >>+  @param ResourceType The Resource Type of the translation offset.
> >>+
> >>+  @retval The Translation Offset of the specified resource.
> >>+**/
> >>+UINT64
> >>+GetTranslationByResourceType (
> >>+  IN  PCI_ROOT_BRIDGE_INSTANCE     *RootBridge,
> >>+  IN  PCI_RESOURCE_TYPE            ResourceType
> >>+  )
> >>+{
> >>+  switch (ResourceType) {
> >>+    case TypeIo:
> >>+      return RootBridge->Io.Translation;
> >>+    case TypeMem32:
> >>+      return RootBridge->Mem.Translation;
> >>+    case TypePMem32:
> >>+      return RootBridge->PMem.Translation;
> >>+    case TypeMem64:
> >>+      return RootBridge->MemAbove4G.Translation;
> >>+    case TypePMem64:
> >>+      return RootBridge->PMemAbove4G.Translation;
> >>+    case TypeBus:
> >>+      return RootBridge->Bus.Translation;
> >>+    default:
> >>+      ASSERT (FALSE);
> >>+      return 0;
> >>+  }
> >>+}
> >>+
> >>+/**
> >>    Ensure the compatibility of an IO space descriptor with the IO aperture.
> >>    The IO space descriptor can come from the GCD IO space map, or it can
> >>@@ -366,6 +399,7 @@ InitializePciHostBridge (
> >>    UINTN                       MemApertureIndex;
> >>    BOOLEAN                     ResourceAssigned;
> >>    LIST_ENTRY                  *Link;
> >>+  UINT64                      HostAddress;
> >>    RootBridges = PciHostBridgeGetRootBridges (&RootBridgeCount);
> >>    if ((RootBridges == NULL) || (RootBridgeCount == 0)) {
> >>@@ -411,8 +445,15 @@ InitializePciHostBridge (
> >>      }
> >>      if (RootBridges[Index].Io.Base <= RootBridges[Index].Io.Limit) {
> >>+      //
> >>+      // Base and Limit in PCI_ROOT_BRIDGE_APERTURE are device address.
> >>+      // For GCD resource manipulation, we need to use host address.
> >>+      //
> >>+      HostAddress = TO_HOST_ADDRESS (RootBridges[Index].Io.Base,
> >>+        RootBridges[Index].Io.Translation);
> >>+
> >>        Status = AddIoSpace (
> >>-                 RootBridges[Index].Io.Base,
> >>+                 HostAddress,
> >>                   RootBridges[Index].Io.Limit - RootBridges[Index].Io.Base 
> >> + 1
> >>                   );
> >>        ASSERT_EFI_ERROR (Status);
> >>@@ -422,7 +463,7 @@ InitializePciHostBridge (
> >>                          EfiGcdIoTypeIo,
> >>                          0,
> >>                          RootBridges[Index].Io.Limit - 
> >> RootBridges[Index].Io.Base + 1,
> >>-                        &RootBridges[Index].Io.Base,
> >>+                        &HostAddress,
> >>                          gImageHandle,
> >>                          NULL
> >>                          );
> >>@@ -443,14 +484,20 @@ InitializePciHostBridge (
> >>      for (MemApertureIndex = 0; MemApertureIndex < ARRAY_SIZE 
> >> (MemApertures); MemApertureIndex++) {
> >>        if (MemApertures[MemApertureIndex]->Base <= 
> >> MemApertures[MemApertureIndex]->Limit) {
> >>+        //
> >>+        // Base and Limit in PCI_ROOT_BRIDGE_APERTURE are device address.
> >>+        // For GCD resource manipulation, we need to use host address.
> >>+        //
> >>+        HostAddress = TO_HOST_ADDRESS 
> >>(MemApertures[MemApertureIndex]->Base,
> >>+          MemApertures[MemApertureIndex]->Translation);
> >>          Status = AddMemoryMappedIoSpace (
> >>-                   MemApertures[MemApertureIndex]->Base,
> >>+                   HostAddress,
> >>                     MemApertures[MemApertureIndex]->Limit - 
> >> MemApertures[MemApertureIndex]->Base + 1,
> >>                     EFI_MEMORY_UC
> >>                     );
> >>          ASSERT_EFI_ERROR (Status);
> >>          Status = gDS->SetMemorySpaceAttributes (
> >>-                        MemApertures[MemApertureIndex]->Base,
> >>+                        HostAddress,
> >>                          MemApertures[MemApertureIndex]->Limit - 
> >> MemApertures[MemApertureIndex]->Base + 1,
> >>                          EFI_MEMORY_UC
> >>                          );
> >>@@ -463,7 +510,7 @@ InitializePciHostBridge (
> >>                            EfiGcdMemoryTypeMemoryMappedIo,
> >>                            0,
> >>                            MemApertures[MemApertureIndex]->Limit - 
> >> MemApertures[MemApertureIndex]->Base + 1,
> >>-                          &MemApertures[MemApertureIndex]->Base,
> >>+                          &HostAddress,
> >>                            gImageHandle,
> >>                            NULL
> >>                            );
> >>@@ -654,6 +701,11 @@ AllocateResource (
> >>    if (BaseAddress < Limit) {
> >>      //
> >>      // Have to make sure Aligment is handled since we are doing direct 
> >> address allocation
> >>+    // Strictly speaking, alignment requirement should be applied to device
> >>+    // address instead of host address which is used in GCD manipulation 
> >>below,
> >>+    // but as we restrict the alignment of Translation to be larger than 
> >>any BAR
> >>+    // alignment in the root bridge, we can simplify the situation and 
> >>consider
> >>+    // the same alignment requirement is also applied to host address.
> >>      //
> >>      BaseAddress = ALIGN_VALUE (BaseAddress, LShiftU64 (1, 
> >> BitsOfAlignment));
> >>@@ -721,6 +773,7 @@ NotifyPhase (
> >>    PCI_RESOURCE_TYPE                     Index2;
> >>    BOOLEAN                               ResNodeHandled[TypeMax];
> >>    UINT64                                MaxAlignment;
> >>+  UINT64                                Translation;
> >>    HostBridge = PCI_HOST_BRIDGE_FROM_THIS (This);
> >>@@ -822,14 +875,43 @@ NotifyPhase (
> >>            BitsOfAlignment = LowBitSet64 (Alignment + 1);
> >>            BaseAddress = MAX_UINT64;
> >>+          //
> >>+          // RESTRICTION: To simplify the situation, we require the 
> >>alignment of
> >>+          // Translation must be larger than any BAR alignment in the same 
> >>root
> >>+          // bridge, so that resource allocation alignment can be applied 
> >>to
> >>+          // both device address and host address.
> >>+          //
> >>+          Translation = GetTranslationByResourceType (RootBridge, Index);
> >>+          if ((Translation & Alignment) != 0) {
> >>+            DEBUG ((DEBUG_ERROR, "[%a:%d] Translation %lx is not aligned 
> >>to %lx!\n",
> >>+              __FUNCTION__, __LINE__, Translation, Alignment
> >>+              ));
> >>+            ASSERT (FALSE);
> >>+            //
> >>+            // This may be caused by too large alignment or too small
> >>+            // Translation; pick the 1st possibility and return out of 
> >>resource,
> >>+            // which can also go thru the same process for out of resource
> >>+            // outside the loop.
> >>+            //
> >>+            ReturnStatus = EFI_OUT_OF_RESOURCES;
> >>+            continue;
> >>+          }
> >>+
> >>            switch (Index) {
> >>            case TypeIo:
> >>+            //
> >>+            // Base and Limit in PCI_ROOT_BRIDGE_APERTURE are device 
> >>address.
> >>+            // For AllocateResource is manipulating GCD resource, we need 
> >>to use
> >>+            // host address here.
> >>+            //
> >>              BaseAddress = AllocateResource (
> >>                              FALSE,
> >>                              RootBridge->ResAllocNode[Index].Length,
> >>                              MIN (15, BitsOfAlignment),
> >>-                            ALIGN_VALUE (RootBridge->Io.Base, Alignment + 
> >>1),
> >>-                            RootBridge->Io.Limit
> >>+                            TO_HOST_ADDRESS (ALIGN_VALUE 
> >>(RootBridge->Io.Base, Alignment + 1),
> >>+                              RootBridge->Io.Translation),
> >>+                            TO_HOST_ADDRESS (RootBridge->Io.Limit,
> >>+                              RootBridge->Io.Translation)
> >>                              );
> >>              break;
> >>@@ -838,8 +920,10 @@ NotifyPhase (
> >>                              TRUE,
> >>                              RootBridge->ResAllocNode[Index].Length,
> >>                              MIN (63, BitsOfAlignment),
> >>-                            ALIGN_VALUE (RootBridge->MemAbove4G.Base, 
> >>Alignment + 1),
> >>-                            RootBridge->MemAbove4G.Limit
> >>+                            TO_HOST_ADDRESS (ALIGN_VALUE 
> >>(RootBridge->MemAbove4G.Base, Alignment + 1),
> >>+                              RootBridge->MemAbove4G.Translation),
> >>+                            TO_HOST_ADDRESS (RootBridge->MemAbove4G.Limit,
> >>+                              RootBridge->MemAbove4G.Translation)
> >>                              );
> >>              if (BaseAddress != MAX_UINT64) {
> >>                break;
> >>@@ -853,8 +937,10 @@ NotifyPhase (
> >>                              TRUE,
> >>                              RootBridge->ResAllocNode[Index].Length,
> >>                              MIN (31, BitsOfAlignment),
> >>-                            ALIGN_VALUE (RootBridge->Mem.Base, Alignment + 
> >>1),
> >>-                            RootBridge->Mem.Limit
> >>+                            TO_HOST_ADDRESS (ALIGN_VALUE 
> >>(RootBridge->Mem.Base, Alignment + 1),
> >>+                              RootBridge->Mem.Translation),
> >>+                            TO_HOST_ADDRESS (RootBridge->Mem.Limit,
> >>+                              RootBridge->Mem.Translation)
> >>                              );
> >>              break;
> >>@@ -863,8 +949,10 @@ NotifyPhase (
> >>                              TRUE,
> >>                              RootBridge->ResAllocNode[Index].Length,
> >>                              MIN (63, BitsOfAlignment),
> >>-                            ALIGN_VALUE (RootBridge->PMemAbove4G.Base, 
> >>Alignment + 1),
> >>-                            RootBridge->PMemAbove4G.Limit
> >>+                            TO_HOST_ADDRESS (ALIGN_VALUE 
> >>(RootBridge->PMemAbove4G.Base, Alignment + 1),
> >>+                              RootBridge->PMemAbove4G.Translation),
> >>+                            TO_HOST_ADDRESS (RootBridge->PMemAbove4G.Limit,
> >>+                              RootBridge->PMemAbove4G.Translation)
> >>                              );
> >>              if (BaseAddress != MAX_UINT64) {
> >>                break;
> >>@@ -877,8 +965,10 @@ NotifyPhase (
> >>                              TRUE,
> >>                              RootBridge->ResAllocNode[Index].Length,
> >>                              MIN (31, BitsOfAlignment),
> >>-                            ALIGN_VALUE (RootBridge->PMem.Base, Alignment 
> >>+ 1),
> >>-                            RootBridge->PMem.Limit
> >>+                            TO_HOST_ADDRESS (ALIGN_VALUE 
> >>(RootBridge->PMem.Base, Alignment + 1),
> >>+                              RootBridge->PMem.Translation),
> >>+                            TO_HOST_ADDRESS (RootBridge->PMem.Limit,
> >>+                              RootBridge->PMem.Translation)
> >>                              );
> >>              break;
> >>@@ -1421,7 +1511,14 @@ GetProposedResources (
> >>            Descriptor->Desc                  = 
> >> ACPI_ADDRESS_SPACE_DESCRIPTOR;
> >>            Descriptor->Len                   = sizeof 
> >> (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) - 3;;
> >>            Descriptor->GenFlag               = 0;
> >>-          Descriptor->AddrRangeMin          = 
> >>RootBridge->ResAllocNode[Index].Base;
> >>+          //
> >>+          // AddrRangeMin in Resource Descriptor here should be device 
> >>address
> >>+          // instead of host address, or else PCI bus driver cannot set 
> >>correct
> >>+          // address into PCI BAR registers.
> >>+          // Base in ResAllocNode is a host address, so conversion is 
> >>needed.
> >>+          //
> >>+          Descriptor->AddrRangeMin          = TO_DEVICE_ADDRESS 
> >>(RootBridge->ResAllocNode[Index].Base,
> >>+            GetTranslationByResourceType (RootBridge, Index));
> >>            Descriptor->AddrRangeMax          = 0;
> >>            Descriptor->AddrTranslationOffset = (ResStatus == ResAllocated) 
> >> ? EFI_RESOURCE_SATISFIED : PCI_RESOURCE_LESS;
> >>            Descriptor->AddrLen               = 
> >> RootBridge->ResAllocNode[Index].Length;
> >>diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c 
> >>b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> >>index dc06c16dc038..5dd9d257d46d 100644
> >>--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> >>+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> >>@@ -86,12 +86,35 @@ CreateRootBridge (
> >>            (Bridge->AllocationAttributes & 
> >> EFI_PCI_HOST_BRIDGE_COMBINE_MEM_PMEM) != 0 ? L"CombineMemPMem " : L"",
> >>            (Bridge->AllocationAttributes & 
> >> EFI_PCI_HOST_BRIDGE_MEM64_DECODE) != 0 ? L"Mem64Decode" : L""
> >>            ));
> >>+  //
> >>+  // Translation for bus is not supported.
> >>+  //
> >>    DEBUG ((EFI_D_INFO, "           Bus: %lx - %lx\n", Bridge->Bus.Base, 
> >> Bridge->Bus.Limit));
> >>-  DEBUG ((EFI_D_INFO, "            Io: %lx - %lx\n", Bridge->Io.Base, 
> >>Bridge->Io.Limit));
> >>-  DEBUG ((EFI_D_INFO, "           Mem: %lx - %lx\n", Bridge->Mem.Base, 
> >>Bridge->Mem.Limit));
> >>-  DEBUG ((EFI_D_INFO, "    MemAbove4G: %lx - %lx\n", 
> >>Bridge->MemAbove4G.Base, Bridge->MemAbove4G.Limit));
> >>-  DEBUG ((EFI_D_INFO, "          PMem: %lx - %lx\n", Bridge->PMem.Base, 
> >>Bridge->PMem.Limit));
> >>-  DEBUG ((EFI_D_INFO, "   PMemAbove4G: %lx - %lx\n", 
> >>Bridge->PMemAbove4G.Base, Bridge->PMemAbove4G.Limit));
> >>+  ASSERT (Bridge->Bus.Translation == 0);
> >>+  if (Bridge->Bus.Translation != 0) {
> >>+    return NULL;
> >>+  }
> >>+
> >>+  DEBUG ((
> >>+    DEBUG_INFO, "            Io: %lx - %lx Translation=%lx\n",
> >>+    Bridge->Io.Base, Bridge->Io.Limit, Bridge->Io.Translation
> >>+    ));
> >>+  DEBUG ((
> >>+    DEBUG_INFO, "           Mem: %lx - %lx Translation=%lx\n",
> >>+    Bridge->Mem.Base, Bridge->Mem.Limit, Bridge->Mem.Translation
> >>+    ));
> >>+  DEBUG ((
> >>+    DEBUG_INFO, "    MemAbove4G: %lx - %lx Translation=%lx\n",
> >>+    Bridge->MemAbove4G.Base, Bridge->MemAbove4G.Limit, 
> >>Bridge->MemAbove4G.Translation
> >>+    ));
> >>+  DEBUG ((
> >>+    DEBUG_INFO, "          PMem: %lx - %lx Translation=%lx\n",
> >>+    Bridge->PMem.Base, Bridge->PMem.Limit, Bridge->PMem.Translation
> >>+    ));
> >>+  DEBUG ((
> >>+    DEBUG_INFO, "   PMemAbove4G: %lx - %lx Translation=%lx\n",
> >>+    Bridge->PMemAbove4G.Base, Bridge->PMemAbove4G.Limit, 
> >>Bridge->PMemAbove4G.Translation
> >>+    ));
> >>    //
> >>    // Make sure Mem and MemAbove4G apertures are valid
> >>@@ -206,7 +229,12 @@ CreateRootBridge (
> >>      }
> >>      RootBridge->ResAllocNode[Index].Type     = Index;
> >>      if (Bridge->ResourceAssigned && (Aperture->Limit >= Aperture->Base)) {
> >>-      RootBridge->ResAllocNode[Index].Base   = Aperture->Base;
> >>+      //
> >>+      // Base in ResAllocNode is a host address, while Base in Aperture is 
> >>a
> >>+      // device address.
> >>+      //
> >>+      RootBridge->ResAllocNode[Index].Base   = TO_HOST_ADDRESS 
> >>(Aperture->Base,
> >>+        Aperture->Translation);
> >>        RootBridge->ResAllocNode[Index].Length = Aperture->Limit - 
> >> Aperture->Base + 1;
> >>        RootBridge->ResAllocNode[Index].Status = ResAllocated;
> >>      } else {
> >>@@ -403,6 +431,28 @@ RootBridgeIoCheckParameter (
> >>    return EFI_SUCCESS;
> >>  }
> >>+EFI_STATUS
> >>+RootBridgeIoGetMemTranslationByAddress (
> >>+  IN PCI_ROOT_BRIDGE_INSTANCE               *RootBridge,
> >>+  IN UINT64                                 Address,
> >>+  IN OUT UINT64                             *Translation
> >>+  )
> >>+{
> >>+  if (Address >= RootBridge->Mem.Base && Address <= RootBridge->Mem.Limit) 
> >>{
> >>+    *Translation = RootBridge->Mem.Translation;
> >>+  } else if (Address >= RootBridge->PMem.Base && Address <= 
> >>RootBridge->PMem.Limit) {
> >>+    *Translation = RootBridge->PMem.Translation;
> >>+  } else if (Address >= RootBridge->MemAbove4G.Base && Address <= 
> >>RootBridge->MemAbove4G.Limit) {
> >>+    *Translation = RootBridge->MemAbove4G.Translation;
> >>+  } else if (Address >= RootBridge->PMemAbove4G.Base && Address <= 
> >>RootBridge->PMemAbove4G.Limit) {
> >>+    *Translation = RootBridge->PMemAbove4G.Translation;
> >>+  } else {
> >>+    return EFI_INVALID_PARAMETER;
> >>+  }
> >>+
> >>+  return EFI_SUCCESS;
> >>+}
> >>+
> >>  /**
> >>    Polls an address in memory mapped I/O space until an exit condition is 
> >> met,
> >>    or a timeout occurs.
> >>@@ -658,13 +708,25 @@ RootBridgeIoMemRead (
> >>    )
> >>  {
> >>    EFI_STATUS                             Status;
> >>+  PCI_ROOT_BRIDGE_INSTANCE               *RootBridge;
> >>+  UINT64                                 Translation;
> >>    Status = RootBridgeIoCheckParameter (This, MemOperation, Width, Address,
> >>                                         Count, Buffer);
> >>    if (EFI_ERROR (Status)) {
> >>      return Status;
> >>    }
> >>-  return mCpuIo->Mem.Read (mCpuIo, (EFI_CPU_IO_PROTOCOL_WIDTH) Width, 
> >>Address, Count, Buffer);
> >>+
> >>+  RootBridge = ROOT_BRIDGE_FROM_THIS (This);
> >>+  Status = RootBridgeIoGetMemTranslationByAddress (RootBridge, Address, 
> >>&Translation);
> >>+  if (EFI_ERROR (Status)) {
> >>+    return Status;
> >>+  }
> >>+
> >>+  // Address passed to CpuIo->Mem.Read needs to be a host address instead 
> >>of
> >>+  // device address.
> >>+  return mCpuIo->Mem.Read (mCpuIo, (EFI_CPU_IO_PROTOCOL_WIDTH) Width,
> >>+      TO_HOST_ADDRESS (Address, Translation), Count, Buffer);
> >>  }
> >>  /**
> >>@@ -705,13 +767,25 @@ RootBridgeIoMemWrite (
> >>    )
> >>  {
> >>    EFI_STATUS                             Status;
> >>+  PCI_ROOT_BRIDGE_INSTANCE               *RootBridge;
> >>+  UINT64                                 Translation;
> >>    Status = RootBridgeIoCheckParameter (This, MemOperation, Width, Address,
> >>                                         Count, Buffer);
> >>    if (EFI_ERROR (Status)) {
> >>      return Status;
> >>    }
> >>-  return mCpuIo->Mem.Write (mCpuIo, (EFI_CPU_IO_PROTOCOL_WIDTH) Width, 
> >>Address, Count, Buffer);
> >>+
> >>+  RootBridge = ROOT_BRIDGE_FROM_THIS (This);
> >>+  Status = RootBridgeIoGetMemTranslationByAddress (RootBridge, Address, 
> >>&Translation);
> >>+  if (EFI_ERROR (Status)) {
> >>+    return Status;
> >>+  }
> >>+
> >>+  // Address passed to CpuIo->Mem.Write needs to be a host address instead 
> >>of
> >>+  // device address.
> >>+  return mCpuIo->Mem.Write (mCpuIo, (EFI_CPU_IO_PROTOCOL_WIDTH) Width,
> >>+      TO_HOST_ADDRESS (Address, Translation), Count, Buffer);
> >>  }
> >>  /**
> >>@@ -746,6 +820,8 @@ RootBridgeIoIoRead (
> >>    )
> >>  {
> >>    EFI_STATUS                                    Status;
> >>+  PCI_ROOT_BRIDGE_INSTANCE                      *RootBridge;
> >>+
> >>    Status = RootBridgeIoCheckParameter (
> >>               This, IoOperation, Width,
> >>               Address, Count, Buffer
> >>@@ -753,7 +829,13 @@ RootBridgeIoIoRead (
> >>    if (EFI_ERROR (Status)) {
> >>      return Status;
> >>    }
> >>-  return mCpuIo->Io.Read (mCpuIo, (EFI_CPU_IO_PROTOCOL_WIDTH) Width, 
> >>Address, Count, Buffer);
> >>+
> >>+  RootBridge = ROOT_BRIDGE_FROM_THIS (This);
> >>+
> >>+  // Address passed to CpuIo->Io.Read needs to be a host address instead of
> >>+  // device address.
> >>+  return mCpuIo->Io.Read (mCpuIo, (EFI_CPU_IO_PROTOCOL_WIDTH) Width,
> >>+      TO_HOST_ADDRESS (Address, RootBridge->Io.Translation), Count, 
> >>Buffer);
> >>  }
> >>  /**
> >>@@ -788,6 +870,8 @@ RootBridgeIoIoWrite (
> >>    )
> >>  {
> >>    EFI_STATUS                                    Status;
> >>+  PCI_ROOT_BRIDGE_INSTANCE                      *RootBridge;
> >>+
> >>    Status = RootBridgeIoCheckParameter (
> >>               This, IoOperation, Width,
> >>               Address, Count, Buffer
> >>@@ -795,7 +879,13 @@ RootBridgeIoIoWrite (
> >>    if (EFI_ERROR (Status)) {
> >>      return Status;
> >>    }
> >>-  return mCpuIo->Io.Write (mCpuIo, (EFI_CPU_IO_PROTOCOL_WIDTH) Width, 
> >>Address, Count, Buffer);
> >>+
> >>+  RootBridge = ROOT_BRIDGE_FROM_THIS (This);
> >>+
> >>+  // Address passed to CpuIo->Io.Write needs to be a host address instead 
> >>of
> >>+  // device address.
> >>+  return mCpuIo->Io.Write (mCpuIo, (EFI_CPU_IO_PROTOCOL_WIDTH) Width,
> >>+      TO_HOST_ADDRESS (Address, RootBridge->Io.Translation), Count, 
> >>Buffer);
> >>  }
> >>  /**
> >>@@ -1615,9 +1705,17 @@ RootBridgeIoConfiguration (
> >>      Descriptor->Desc = ACPI_ADDRESS_SPACE_DESCRIPTOR;
> >>      Descriptor->Len  = sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) - 3;
> >>+    // According to UEFI 2.7, RootBridgeIo->Configuration should return 
> >>address
> >>+    // range in CPU view (host address), and ResAllocNode->Base is already 
> >>a CPU
> >>+    // view address (host address).
> >>      Descriptor->AddrRangeMin  = ResAllocNode->Base;
> >>      Descriptor->AddrRangeMax  = ResAllocNode->Base + ResAllocNode->Length 
> >> - 1;
> >>      Descriptor->AddrLen       = ResAllocNode->Length;
> >>+    Descriptor->AddrTranslationOffset = GetTranslationByResourceType (
> >>+      RootBridge,
> >>+      ResAllocNode->Type
> >>+      );
> >>+
> >>      switch (ResAllocNode->Type) {
> >>      case TypeIo:
> >>-- 
> >>2.7.4
> >>
> 
> 
> -- 
> Thanks,
> Ray
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to