From: Guo Heyi <heyi.guo@linaro.org>
To: "Ni, Ruiyu" <ruiyu.ni@Intel.com>
Cc: Guo Heyi <heyi.guo@linaro.org>,
Michael D <michael.d.kinney@intel.com>,
Ard Biesheuvel <ard.biesheuvel@linaro.org>,
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, 29 Jan 2018 16:50:20 +0800 [thread overview]
Message-ID: <20180129085020.GA127987@SZX1000114654> (raw)
In-Reply-To: <ba02262c-949b-6a4c-60e8-8fa10f5937fc@Intel.com>
Sorry for the late; I caught cold and didn't work for several days last week :(
Please see my comments below:
On Mon, Jan 22, 2018 at 11:36:14AM +0800, Ni, Ruiyu wrote:
> 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
If we get Equation #B from the statement in UEFI spec, does it also imply
Starting address is "Address Range Minimum" and so it is CPU view address?
If we use Equation #B, can offset be a negative value? If it is not, then it
will make translation useless, since we cannot change CPU address above 4G into
PCI address under 4G for legacy compatibility.
Equation #B is also not consistent with how OS treats address translation;
please see the ACPI ASL code which works well for OS:
https://github.com/tianocore/edk2-platforms/blob/master/Silicon/Hisilicon/Hi1616/D05AcpiTables/Dsdt/D05Pci.asl#L201
Thanks,
Gary (Heyi Guo)
>
> 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
next prev parent reply other threads:[~2018-01-29 8:44 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
2018-01-29 8:50 ` Guo Heyi [this message]
[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=20180129085020.GA127987@SZX1000114654 \
--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