public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Ni, Ruiyu" <ruiyu.ni@Intel.com>
To: Guo Heyi <heyi.guo@linaro.org>,
	Michael D <michael.d.kinney@intel.com>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Eric Dong <eric.dong@intel.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
	linaro-uefi <linaro-uefi@lists.linaro.org>,
	Star Zeng <star.zeng@intel.com>
Subject: Re: [RFC] MdeModulePkg/PciHostBridgeDxe: Add support for address translation
Date: Mon, 22 Jan 2018 11:36:14 +0800	[thread overview]
Message-ID: <ba02262c-949b-6a4c-60e8-8fa10f5937fc@Intel.com> (raw)
In-Reply-To: <20180118012639.GA113567@SZX1000114654>

On 1/18/2018 9:26 AM, Guo Heyi wrote:
> On Wed, Jan 17, 2018 at 02:08:06PM +0000, Ard Biesheuvel wrote:
>> On 15 January 2018 at 14:46, Heyi Guo <heyi.guo@linaro.org> wrote:
>>> This is the draft patch for the discussion posted in edk2-devel
>>> mailing list:
>>> https://lists.01.org/pipermail/edk2-devel/2017-December/019289.html
>>>
>>> As discussed in the mailing list, we'd like to add support for PCI
>>> address translation which is necessary for some non-x86 platforms. I
>>> also want to minimize the changes to the generic host bridge driver
>>> and platform PciHostBridgeLib implemetations, so additional two
>>> interfaces are added to expose translation information of the
>>> platform. To be generic, I add translation for each type of IO or
>>> memory resources.
>>>
>>> The patch is still a RFC, so I only passed the build for qemu64 and
>>> the function has not been tested yet.
>>>
>>> Please let me know your comments about it.
>>>
>>> Thanks.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Heyi Guo <heyi.guo@linaro.org>
>>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> Cc: Star Zeng <star.zeng@intel.com>
>>> Cc: Eric Dong <eric.dong@intel.com>
>>> ---
>>>   .../FdtPciHostBridgeLib/FdtPciHostBridgeLib.c      |  19 ++++
>>>   .../Bus/Pci/PciHostBridgeDxe/PciHostBridge.c       |  53 ++++++++---
>>>   .../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h       |   8 +-
>>>   .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c     | 101 ++++++++++++++++++---
>>>   MdeModulePkg/Include/Library/PciHostBridgeLib.h    |  36 ++++++++
>>>   5 files changed, 192 insertions(+), 25 deletions(-)
>>>
>>> diff --git a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
>>> index 5b9c887..0c8371a 100644
>>> --- a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
>>> +++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
>>> @@ -360,6 +360,16 @@ PciHostBridgeGetRootBridges (
>>>     return &mRootBridge;
>>>   }
>>>
>>> +PCI_ROOT_BRIDGE_TRANSLATION *
>>> +EFIAPI
>>> +PciHostBridgeGetTranslations (
>>> +  UINTN *Count
>>> +  )
>>> +{
>>> +  *Count = 0;
>>> +  return NULL;
>>> +}
>>> +
>>>   /**
>>>     Free the root bridge instances array returned from
>>>     PciHostBridgeGetRootBridges().
>>> @@ -377,6 +387,15 @@ PciHostBridgeFreeRootBridges (
>>>     ASSERT (Count == 1);
>>>   }
>>>
>>> +VOID
>>> +EFIAPI
>>> +PciHostBridgeFreeTranslations (
>>> +  PCI_ROOT_BRIDGE_TRANSLATION *Translations,
>>> +  UINTN                       Count
>>> +  )
>>> +{
>>> +}
>>> +
>>>   /**
>>>     Inform the platform that the resource conflict happens.
>>>
>>> diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
>>> index 1494848..835e411 100644
>>> --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
>>> +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
>>> @@ -360,18 +360,38 @@ InitializePciHostBridge (
>>>     PCI_HOST_BRIDGE_INSTANCE    *HostBridge;
>>>     PCI_ROOT_BRIDGE_INSTANCE    *RootBridge;
>>>     PCI_ROOT_BRIDGE             *RootBridges;
>>> +  PCI_ROOT_BRIDGE_TRANSLATION *Translations;
>>>     UINTN                       RootBridgeCount;
>>> +  UINTN                       TranslationCount;
>>>     UINTN                       Index;
>>>     PCI_ROOT_BRIDGE_APERTURE    *MemApertures[4];
>>
>> Wouldn't it be much better to add a 'translation' member to
>> PCI_ROOT_BRIDGE_APERTURE? That way, existing code just default to a
>> translation of 0, and all the handling of the separate array can be
>> dropped.
>>
> Actually my first idea was the same, but when I looked at the implementation of
> PciHostBridgeLib of Ovmf, I found it a little tedious to change the
> existing code in this way. We'll need to check everywhere
> PCI_ROOT_BRIDGE_APERTURE or PCI_ROOT_BRIDGE is used, to make sure the
> translation field is initialized to be zero, e.g. line 235~245.
> 
> What I did in this RFC is not so straightforward indeed. So I can change the
> code if we all agree to add Translation field into PCI_ROOT_BRIDGE_APERTURE
> directly.
> 
> Thanks,
> 
> Gary (Heyi Guo)

I also agree to put the translation fields into the
PCI_ROOT_BRIDGE_APERTURE.


But I think we may have different understandings to the address
translation.
My understanding to the whole translation:
1. PciHostBridge.GetProposedResources () returns resource information
    for a single root bridge. It only carries the address range in PCI
    view.
    The address range/resource is reported/submitted by PciHostBridgeLib.
    Before the change, CPU view equals to PCI view. So PciHostBridgeDxe
    driver directly adds the resource to GCD.
    In your change, PciHostBridgeDxe assumes the source is in PCI view
    and it adds offset to convert to CPU view.
    CPU-address = PCI-address + offset. <-- Equation #A


2. PciRootBridgeIo.Configuration() returns the resource information
    for a single root bridge. It carries the address range in CPU view,
    and the translation offset.
    However, per UEFI spec, "Address Translation Offset. Offset to apply
    to the Starting address of a BAR to convert it to a PCI address. "
    PCI-address = CPU-address + offset. <-- Equation #B

    You can see that equation #A and #B are conflict.


3. PciIo.GetBarAttributes() returns the resource information for a
    single PCI BAR.
    It carries the address range in CPU view, and the translation offset.
    UEFI Spec content here is consistent to #2.


4. I didn't find spec requires the offset only applies to MEM address.
    So I am ok to extend the offset to IO address.

So I suggest:
1. PciHostBridgeLib still reports the resource in CPU view because
    #2 and #3 both report resource in CPU view.
    It also reports the resource offset (= PCI-address - CPU-address).

2. PciHostBridge.GetProposedResources() returns resource for a single
    root bridge in PCI view. Because per PI spec, the Offset field in
    the ACPI descriptor carries the allocation status. If the resource
    is in CPU view, PciBus driver doesn't know how to calculate the
    PCI address and update the BAR.

3. PciRootBridgeIo.Configuration() returns offset value reported by
    PciHostBridgeLib.

4. PciRootBridgeIo/PciIo.Mem.Read/Write(): I agree that the address is
    a PCI-address. So a conversion is needed when use CpuIo to access.
    But I think a spec clarification is needed to tell developers
    that the conversion happens in PciRootBridgeIo layer, not in PciIo
    layer.

> 
> 
>>> +  UINT64                      MemTranslation[4];
>>>     UINTN                       MemApertureIndex;
>>>     BOOLEAN                     ResourceAssigned;
>>>     LIST_ENTRY                  *Link;
>>> +  UINT64                      Trans;
>>>
>>>     RootBridges = PciHostBridgeGetRootBridges (&RootBridgeCount);
>>>     if ((RootBridges == NULL) || (RootBridgeCount == 0)) {
>>>       return EFI_UNSUPPORTED;
>>>     }
>>>
>>> +  Translations = PciHostBridgeGetTranslations (&TranslationCount);
>>> +  if (Translations == NULL || TranslationCount == 0) {
>>> +    TranslationCount = 0;
>>> +    Translations = AllocateZeroPool (RootBridgeCount * sizeof (*Translations));
>>> +    if (Translations == NULL) {
>>> +      PciHostBridgeFreeRootBridges (RootBridges, RootBridgeCount);
>>> +      return EFI_OUT_OF_RESOURCES;
>>> +    }
>>> +  }
>>> +
>>> +  if (TranslationCount != 0 && TranslationCount != RootBridgeCount) {
>>> +    DEBUG ((DEBUG_ERROR, "Error: count of root bridges (%d) and translation (%d) are different!\n",
>>> +      RootBridgeCount, TranslationCount));
>>> +    return EFI_INVALID_PARAMETER;
>>> +  }
>>> +
>>>     Status = gBS->LocateProtocol (&gEfiMetronomeArchProtocolGuid, NULL, (VOID **) &mMetronome);
>>>     ASSERT_EFI_ERROR (Status);
>>>     Status = gBS->LocateProtocol (&gEfiCpuIo2ProtocolGuid, NULL, (VOID **) &mCpuIo);
>>> @@ -395,7 +415,7 @@ InitializePciHostBridge (
>>>       //
>>>       // Create Root Bridge Handle Instance
>>>       //
>>> -    RootBridge = CreateRootBridge (&RootBridges[Index]);
>>> +    RootBridge = CreateRootBridge (&RootBridges[Index], &Translations[Index]);
>>>       ASSERT (RootBridge != NULL);
>>>       if (RootBridge == NULL) {
>>>         continue;
>>> @@ -411,8 +431,9 @@ InitializePciHostBridge (
>>>       }
>>>
>>>       if (RootBridges[Index].Io.Base <= RootBridges[Index].Io.Limit) {
>>> +      Trans = Translations[Index].IoTranslation;
>>>         Status = AddIoSpace (
>>> -                 RootBridges[Index].Io.Base,
>>> +                 RootBridges[Index].Io.Base + Trans,
>>>                    RootBridges[Index].Io.Limit - RootBridges[Index].Io.Base + 1
>>>                    );
>>>         ASSERT_EFI_ERROR (Status);
>>> @@ -422,7 +443,7 @@ InitializePciHostBridge (
>>>                           EfiGcdIoTypeIo,
>>>                           0,
>>>                           RootBridges[Index].Io.Limit - RootBridges[Index].Io.Base + 1,
>>> -                        &RootBridges[Index].Io.Base,
>>> +                        &RootBridges[Index].Io.Base + Trans,
>>>                           gImageHandle,
>>>                           NULL
>>>                           );
>>> @@ -437,20 +458,24 @@ InitializePciHostBridge (
>>>       // the MEM aperture in Mem
>>>       //
>>>       MemApertures[0] = &RootBridges[Index].Mem;
>>> +    MemTranslation[0] = Translations[Index].MemTranslation;
>>>       MemApertures[1] = &RootBridges[Index].MemAbove4G;
>>> +    MemTranslation[1] = Translations[Index].MemAbove4GTranslation;
>>>       MemApertures[2] = &RootBridges[Index].PMem;
>>> +    MemTranslation[2] = Translations[Index].PMemTranslation;
>>>       MemApertures[3] = &RootBridges[Index].PMemAbove4G;
>>> +    MemTranslation[3] = Translations[Index].PMemAbove4GTranslation;
>>>
>>>       for (MemApertureIndex = 0; MemApertureIndex < ARRAY_SIZE (MemApertures); MemApertureIndex++) {
>>>         if (MemApertures[MemApertureIndex]->Base <= MemApertures[MemApertureIndex]->Limit) {
>>>           Status = AddMemoryMappedIoSpace (
>>> -                   MemApertures[MemApertureIndex]->Base,
>>> +                   MemApertures[MemApertureIndex]->Base + MemTranslation[MemApertureIndex],
>>>                      MemApertures[MemApertureIndex]->Limit - MemApertures[MemApertureIndex]->Base + 1,
>>>                      EFI_MEMORY_UC
>>>                      );
>>>           ASSERT_EFI_ERROR (Status);
>>>           Status = gDS->SetMemorySpaceAttributes (
>>> -                        MemApertures[MemApertureIndex]->Base,
>>> +                        MemApertures[MemApertureIndex]->Base + MemTranslation[MemApertureIndex],
>>>                           MemApertures[MemApertureIndex]->Limit - MemApertures[MemApertureIndex]->Base + 1,
>>>                           EFI_MEMORY_UC
>>>                           );
>>> @@ -463,7 +488,7 @@ InitializePciHostBridge (
>>>                             EfiGcdMemoryTypeMemoryMappedIo,
>>>                             0,
>>>                             MemApertures[MemApertureIndex]->Limit - MemApertures[MemApertureIndex]->Base + 1,
>>> -                          &MemApertures[MemApertureIndex]->Base,
>>> +                          &MemApertures[MemApertureIndex]->Base + MemTranslation[MemApertureIndex],
>>>                             gImageHandle,
>>>                             NULL
>>>                             );
>>> @@ -514,7 +539,13 @@ InitializePciHostBridge (
>>>                       );
>>>       ASSERT_EFI_ERROR (Status);
>>>     }
>>> +
>>>     PciHostBridgeFreeRootBridges (RootBridges, RootBridgeCount);
>>> +  if (TranslationCount == 0) {
>>> +    FreePool (Translations);
>>> +  } else {
>>> +    PciHostBridgeFreeTranslations (Translations, TranslationCount);
>>> +  }
>>>
>>>     if (!EFI_ERROR (Status)) {
>>>       mIoMmuEvent = EfiCreateProtocolNotifyEvent (
>>> @@ -828,7 +859,7 @@ NotifyPhase (
>>>                               FALSE,
>>>                               RootBridge->ResAllocNode[Index].Length,
>>>                               MIN (15, BitsOfAlignment),
>>> -                            ALIGN_VALUE (RootBridge->Io.Base, Alignment + 1),
>>> +                            ALIGN_VALUE (RootBridge->Io.Base, Alignment + 1) + RootBridge->IoTranslation,
>>>                               RootBridge->Io.Limit
>>>                               );
>>>               break;
>>> @@ -838,7 +869,7 @@ NotifyPhase (
>>>                               TRUE,
>>>                               RootBridge->ResAllocNode[Index].Length,
>>>                               MIN (63, BitsOfAlignment),
>>> -                            ALIGN_VALUE (RootBridge->MemAbove4G.Base, Alignment + 1),
>>> +                            ALIGN_VALUE (RootBridge->MemAbove4G.Base, Alignment + 1) + RootBridge->MemAbove4GTranslation,
>>>                               RootBridge->MemAbove4G.Limit
>>>                               );
>>>               if (BaseAddress != MAX_UINT64) {
>>> @@ -853,7 +884,7 @@ NotifyPhase (
>>>                               TRUE,
>>>                               RootBridge->ResAllocNode[Index].Length,
>>>                               MIN (31, BitsOfAlignment),
>>> -                            ALIGN_VALUE (RootBridge->Mem.Base, Alignment + 1),
>>> +                            ALIGN_VALUE (RootBridge->Mem.Base, Alignment + 1) + RootBridge->MemTranslation,
>>>                               RootBridge->Mem.Limit
>>>                               );
>>>               break;
>>> @@ -863,7 +894,7 @@ NotifyPhase (
>>>                               TRUE,
>>>                               RootBridge->ResAllocNode[Index].Length,
>>>                               MIN (63, BitsOfAlignment),
>>> -                            ALIGN_VALUE (RootBridge->PMemAbove4G.Base, Alignment + 1),
>>> +                            ALIGN_VALUE (RootBridge->PMemAbove4G.Base, Alignment + 1) + RootBridge->PMemAbove4GTranslation,
>>>                               RootBridge->PMemAbove4G.Limit
>>>                               );
>>>               if (BaseAddress != MAX_UINT64) {
>>> @@ -877,7 +908,7 @@ NotifyPhase (
>>>                               TRUE,
>>>                               RootBridge->ResAllocNode[Index].Length,
>>>                               MIN (31, BitsOfAlignment),
>>> -                            ALIGN_VALUE (RootBridge->PMem.Base, Alignment + 1),
>>> +                            ALIGN_VALUE (RootBridge->PMem.Base, Alignment + 1) + RootBridge->PMemTranslation,
>>>                               RootBridge->PMem.Limit
>>>                               );
>>>               break;
>>> diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
>>> index d3dfb57..449c4b3 100644
>>> --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
>>> +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
>>> @@ -73,6 +73,11 @@ typedef struct {
>>>     PCI_ROOT_BRIDGE_APERTURE          PMem;
>>>     PCI_ROOT_BRIDGE_APERTURE          MemAbove4G;
>>>     PCI_ROOT_BRIDGE_APERTURE          PMemAbove4G;
>>> +  UINT64                            IoTranslation;
>>> +  UINT64                            MemTranslation;
>>> +  UINT64                            MemAbove4GTranslation;
>>> +  UINT64                            PMemTranslation;
>>> +  UINT64                            PMemAbove4GTranslation;
>>>     BOOLEAN                           DmaAbove4G;
>>>     BOOLEAN                           NoExtendedConfigSpace;
>>>     VOID                              *ConfigBuffer;
>>> @@ -98,7 +103,8 @@ typedef struct {
>>>   **/
>>>   PCI_ROOT_BRIDGE_INSTANCE *
>>>   CreateRootBridge (
>>> -  IN PCI_ROOT_BRIDGE       *Bridge
>>> +  IN PCI_ROOT_BRIDGE                *Bridge,
>>> +  IN PCI_ROOT_BRIDGE_TRANSLATION    *Translation
>>>     );
>>>
>>>   //
>>> diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
>>> index dc06c16..84b2d5a 100644
>>> --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
>>> +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
>>> @@ -67,7 +67,8 @@ UINT8 mOutStride[] = {
>>>   **/
>>>   PCI_ROOT_BRIDGE_INSTANCE *
>>>   CreateRootBridge (
>>> -  IN PCI_ROOT_BRIDGE       *Bridge
>>> +  IN PCI_ROOT_BRIDGE                *Bridge,
>>> +  IN PCI_ROOT_BRIDGE_TRANSLATION    *Translation
>>>     )
>>>   {
>>>     PCI_ROOT_BRIDGE_INSTANCE *RootBridge;
>>> @@ -87,11 +88,21 @@ CreateRootBridge (
>>>             (Bridge->AllocationAttributes & EFI_PCI_HOST_BRIDGE_MEM64_DECODE) != 0 ? L"Mem64Decode" : L""
>>>             ));
>>>     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));
>>> +  DEBUG ((DEBUG_INFO, "            Io: %lx - %lx translation=%lx\n",
>>> +          Bridge->Io.Base, Bridge->Io.Limit, Translation->IoTranslation
>>> +          ));
>>> +  DEBUG ((DEBUG_INFO, "           Mem: %lx - %lx translation=%lx\n",
>>> +          Bridge->Mem.Base, Bridge->Mem.Limit, Translation->MemTranslation
>>> +          ));
>>> +  DEBUG ((DEBUG_INFO, "    MemAbove4G: %lx - %lx translation=%lx\n",
>>> +          Bridge->MemAbove4G.Base, Bridge->MemAbove4G.Limit, Translation->MemAbove4GTranslation
>>> +          ));
>>> +  DEBUG ((DEBUG_INFO, "          PMem: %lx - %lx translation=%lx\n",
>>> +          Bridge->PMem.Base, Bridge->PMem.Limit, Translation->PMemTranslation
>>> +          ));
>>> +  DEBUG ((DEBUG_INFO, "   PMemAbove4G: %lx - %lx translation=%lx\n",
>>> +          Bridge->PMemAbove4G.Base, Bridge->PMemAbove4G.Limit, Translation->PMemAbove4GTranslation
>>> +          ));
>>>
>>>     //
>>>     // Make sure Mem and MemAbove4G apertures are valid
>>> @@ -174,10 +185,15 @@ CreateRootBridge (
>>>
>>>     CopyMem (&RootBridge->Bus, &Bridge->Bus, sizeof (PCI_ROOT_BRIDGE_APERTURE));
>>>     CopyMem (&RootBridge->Io, &Bridge->Io, sizeof (PCI_ROOT_BRIDGE_APERTURE));
>>> +  RootBridge->IoTranslation = Translation->IoTranslation;
>>>     CopyMem (&RootBridge->Mem, &Bridge->Mem, sizeof (PCI_ROOT_BRIDGE_APERTURE));
>>> +  RootBridge->MemTranslation = Translation->MemTranslation;
>>>     CopyMem (&RootBridge->MemAbove4G, &Bridge->MemAbove4G, sizeof (PCI_ROOT_BRIDGE_APERTURE));
>>> +  RootBridge->MemAbove4GTranslation = Translation->MemAbove4GTranslation;
>>>     CopyMem (&RootBridge->PMem, &Bridge->PMem, sizeof (PCI_ROOT_BRIDGE_APERTURE));
>>> +  RootBridge->PMemTranslation = Translation->PMemTranslation;
>>>     CopyMem (&RootBridge->PMemAbove4G, &Bridge->PMemAbove4G, sizeof (PCI_ROOT_BRIDGE_APERTURE));
>>> +  RootBridge->PMemAbove4GTranslation = Translation->PMemAbove4GTranslation;
>>>
>>>     for (Index = TypeIo; Index < TypeMax; Index++) {
>>>       switch (Index) {
>>> @@ -403,6 +419,28 @@ RootBridgeIoCheckParameter (
>>>     return EFI_SUCCESS;
>>>   }
>>>
>>> +EFI_STATUS
>>> +RootBridgeIoGetMemTranslation (
>>> +  IN PCI_ROOT_BRIDGE_INSTANCE               *RootBridge,
>>> +  IN UINT64                                 Address,
>>> +  IN OUT UINT64                             *Translation
>>> +  )
>>> +{
>>> +  if (Address >= RootBridge->Mem.Base && Address <= RootBridge->Mem.Limit) {
>>> +    *Translation = RootBridge->MemTranslation;
>>> +  } else if (Address >= RootBridge->PMem.Base && Address <= RootBridge->PMem.Limit) {
>>> +    *Translation = RootBridge->PMemTranslation;
>>> +  } else if (Address >= RootBridge->MemAbove4G.Base && Address <= RootBridge->MemAbove4G.Limit) {
>>> +    *Translation = RootBridge->MemAbove4GTranslation;
>>> +  } else if (Address >= RootBridge->PMemAbove4G.Base && Address <= RootBridge->PMemAbove4G.Limit) {
>>> +    *Translation = RootBridge->PMemAbove4GTranslation;
>>> +  } 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 +696,22 @@ 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 = RootBridgeIoGetMemTranslation (RootBridge, Address, &Translation);
>>> +  if (EFI_ERROR (Status)) {
>>> +    return Status;
>>> +  }
>>> +
>>> +  return mCpuIo->Mem.Read (mCpuIo, (EFI_CPU_IO_PROTOCOL_WIDTH) Width, Address + Translation, Count, Buffer);
>>>   }
>>>
>>>   /**
>>> @@ -705,13 +752,22 @@ 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 = RootBridgeIoGetMemTranslation (RootBridge, Address, &Translation);
>>> +  if (EFI_ERROR (Status)) {
>>> +    return Status;
>>> +  }
>>> +
>>> +  return mCpuIo->Mem.Write (mCpuIo, (EFI_CPU_IO_PROTOCOL_WIDTH) Width, Address + Translation, Count, Buffer);
>>>   }
>>>
>>>   /**
>>> @@ -746,6 +802,8 @@ RootBridgeIoIoRead (
>>>     )
>>>   {
>>>     EFI_STATUS                                    Status;
>>> +  PCI_ROOT_BRIDGE_INSTANCE                      *RootBridge;
>>> +
>>>     Status = RootBridgeIoCheckParameter (
>>>                This, IoOperation, Width,
>>>                Address, Count, Buffer
>>> @@ -753,7 +811,10 @@ 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);
>>> +
>>> +  return mCpuIo->Io.Read (mCpuIo, (EFI_CPU_IO_PROTOCOL_WIDTH) Width, Address + RootBridge->IoTranslation, Count, Buffer);
>>>   }
>>>
>>>   /**
>>> @@ -788,6 +849,8 @@ RootBridgeIoIoWrite (
>>>     )
>>>   {
>>>     EFI_STATUS                                    Status;
>>> +  PCI_ROOT_BRIDGE_INSTANCE                      *RootBridge;
>>> +
>>>     Status = RootBridgeIoCheckParameter (
>>>                This, IoOperation, Width,
>>>                Address, Count, Buffer
>>> @@ -795,7 +858,10 @@ 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);
>>> +
>>> +  return mCpuIo->Io.Write (mCpuIo, (EFI_CPU_IO_PROTOCOL_WIDTH) Width, Address + RootBridge->IoTranslation, Count, Buffer);
>>>   }
>>>
>>>   /**
>>> @@ -1621,19 +1687,28 @@ RootBridgeIoConfiguration (
>>>       switch (ResAllocNode->Type) {
>>>
>>>       case TypeIo:
>>> -      Descriptor->ResType       = ACPI_ADDRESS_SPACE_TYPE_IO;
>>> +      Descriptor->ResType               = ACPI_ADDRESS_SPACE_TYPE_IO;
>>> +      Descriptor->AddrTranslationOffset = RootBridge->IoTranslation;
>>>         break;
>>>
>>>       case TypePMem32:
>>> -      Descriptor->SpecificFlag = EFI_ACPI_MEMORY_RESOURCE_SPECIFIC_FLAG_CACHEABLE_PREFETCHABLE;
>>> +      Descriptor->SpecificFlag          = EFI_ACPI_MEMORY_RESOURCE_SPECIFIC_FLAG_CACHEABLE_PREFETCHABLE;
>>> +      Descriptor->AddrTranslationOffset = RootBridge->PMemTranslation;
>>> +      Descriptor->ResType               = ACPI_ADDRESS_SPACE_TYPE_MEM;
>>> +      Descriptor->AddrSpaceGranularity  = 32;
>>>       case TypeMem32:
>>> +      Descriptor->AddrTranslationOffset = RootBridge->MemTranslation;
>>>         Descriptor->ResType               = ACPI_ADDRESS_SPACE_TYPE_MEM;
>>>         Descriptor->AddrSpaceGranularity  = 32;
>>>         break;
>>>
>>>       case TypePMem64:
>>> -      Descriptor->SpecificFlag = EFI_ACPI_MEMORY_RESOURCE_SPECIFIC_FLAG_CACHEABLE_PREFETCHABLE;
>>> +      Descriptor->SpecificFlag          = EFI_ACPI_MEMORY_RESOURCE_SPECIFIC_FLAG_CACHEABLE_PREFETCHABLE;
>>> +      Descriptor->AddrTranslationOffset = RootBridge->PMemAbove4GTranslation;
>>> +      Descriptor->ResType               = ACPI_ADDRESS_SPACE_TYPE_MEM;
>>> +      Descriptor->AddrSpaceGranularity  = 64;
>>>       case TypeMem64:
>>> +      Descriptor->AddrTranslationOffset = RootBridge->MemAbove4GTranslation;
>>>         Descriptor->ResType               = ACPI_ADDRESS_SPACE_TYPE_MEM;
>>>         Descriptor->AddrSpaceGranularity  = 64;
>>>         break;
>>> diff --git a/MdeModulePkg/Include/Library/PciHostBridgeLib.h b/MdeModulePkg/Include/Library/PciHostBridgeLib.h
>>> index d42e9ec..4c297fd 100644
>>> --- a/MdeModulePkg/Include/Library/PciHostBridgeLib.h
>>> +++ b/MdeModulePkg/Include/Library/PciHostBridgeLib.h
>>> @@ -53,6 +53,14 @@ typedef struct {
>>>     EFI_DEVICE_PATH_PROTOCOL *DevicePath;           ///< Device path.
>>>   } PCI_ROOT_BRIDGE;
>>>
>>> +typedef struct {
>>> +  UINT64                   IoTranslation;
>>> +  UINT64                   MemTranslation;
>>> +  UINT64                   MemAbove4GTranslation;
>>> +  UINT64                   PMemTranslation;
>>> +  UINT64                   PMemAbove4GTranslation;
>>> +} PCI_ROOT_BRIDGE_TRANSLATION;
>>> +
>>>   /**
>>>     Return all the root bridge instances in an array.
>>>
>>> @@ -69,6 +77,21 @@ PciHostBridgeGetRootBridges (
>>>     );
>>>
>>>   /**
>>> +  Return all the root bridge instances in an array.
>>> +
>>> +  @param Count  Return the count of root bridge instances.
>>> +
>>> +  @return All the root bridge instances in an array.
>>> +          The array should be passed into PciHostBridgeFreeRootBridges()
>>> +          when it's not used.
>>> +**/
>>> +PCI_ROOT_BRIDGE_TRANSLATION *
>>> +EFIAPI
>>> +PciHostBridgeGetTranslations (
>>> +  UINTN *Count
>>> +  );
>>> +
>>> +/**
>>>     Free the root bridge instances array returned from PciHostBridgeGetRootBridges().
>>>
>>>     @param Bridges The root bridge instances array.
>>> @@ -82,6 +105,19 @@ PciHostBridgeFreeRootBridges (
>>>     );
>>>
>>>   /**
>>> +  Free the root bridge instances array returned from PciHostBridgeGetRootBridges().
>>> +
>>> +  @param Bridges The root bridge instances array.
>>> +  @param Count   The count of the array.
>>> +**/
>>> +VOID
>>> +EFIAPI
>>> +PciHostBridgeFreeTranslations (
>>> +  PCI_ROOT_BRIDGE_TRANSLATION  *Bridges,
>>> +  UINTN                        Count
>>> +  );
>>> +
>>> +/**
>>>     Inform the platform that the resource conflict happens.
>>>
>>>     @param HostBridgeHandle Handle of the Host Bridge.
>>> --
>>> 2.7.4
>>>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 


-- 
Thanks,
Ray


  reply	other threads:[~2018-01-22  3:31 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-15 14:46 [RFC] MdeModulePkg/PciHostBridgeDxe: Add support for address translation Heyi Guo
2018-01-17 14:08 ` Ard Biesheuvel
2018-01-18  1:26   ` Guo Heyi
2018-01-22  3:36     ` Ni, Ruiyu [this message]
2018-01-29  8:50       ` Guo Heyi
     [not found]         ` <685b27a9-e620-e7e7-e0fb-8cebe16e7b1a@Intel.com>
2018-02-01 17:22           ` Ard Biesheuvel
2018-02-02  0:34             ` Ni, Ruiyu
2018-02-02  8:22               ` Ard Biesheuvel
2018-02-03  6:00                 ` Ni, Ruiyu
2018-02-05  8:06                   ` Ni, Ruiyu
2018-02-05  9:11                     ` Ard Biesheuvel
2018-02-06  1:12                       ` Guo Heyi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ba02262c-949b-6a4c-60e8-8fa10f5937fc@Intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox