From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.100; helo=mga07.intel.com; envelope-from=ruiyu.ni@intel.com; receiver=edk2-devel@lists.01.org Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 6A70B2034D8C1 for ; Thu, 22 Feb 2018 00:50:43 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 22 Feb 2018 00:56:43 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.47,377,1515484800"; d="scan'208";a="32683688" Received: from ray-dev.ccr.corp.intel.com (HELO [10.239.9.13]) ([10.239.9.13]) by fmsmga001.fm.intel.com with ESMTP; 22 Feb 2018 00:56:41 -0800 To: Heyi Guo , edk2-devel@lists.01.org Cc: Ard Biesheuvel , Star Zeng , Eric Dong , Laszlo Ersek , Michael D Kinney References: <1519282474-94811-1-git-send-email-heyi.guo@linaro.org> <1519282474-94811-2-git-send-email-heyi.guo@linaro.org> From: "Ni, Ruiyu" Message-ID: <029fb61a-01f6-b6a1-ba62-769ccaefe30f@Intel.com> Date: Thu, 22 Feb 2018 16:56:41 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <1519282474-94811-2-git-send-email-heyi.guo@linaro.org> Subject: Re: [RFC v2 1/2] MdeModulePkg/PciHostBridgeDxe: Add support for address translation X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 22 Feb 2018 08:50:43 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit On 2/22/2018 2:54 PM, Heyi Guo 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 > Cc: Ruiyu Ni > Cc: Ard Biesheuvel > Cc: Star Zeng > Cc: Eric Dong > Cc: Laszlo Ersek > Cc: Michael D Kinney > --- > .../Bus/Pci/PciHostBridgeDxe/PciHostBridge.c | 57 ++++++++---- > .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 101 ++++++++++++++++++--- > MdeModulePkg/Include/Library/PciHostBridgeLib.h | 1 + > 3 files changed, 131 insertions(+), 28 deletions(-) > > diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c > index 1494848..fa22d8d 100644 > --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c > +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c > @@ -32,6 +32,29 @@ EDKII_IOMMU_PROTOCOL *mIoMmuProtocol; > EFI_EVENT mIoMmuEvent; > VOID *mIoMmuRegistration; > > +STATIC > +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; > + default: > + return 0; > + } > +} > + > /** > Ensure the compatibility of an IO space descriptor with the IO aperture. > > @@ -412,7 +435,7 @@ InitializePciHostBridge ( > > if (RootBridges[Index].Io.Base <= RootBridges[Index].Io.Limit) { > Status = AddIoSpace ( > - RootBridges[Index].Io.Base, > + RootBridges[Index].Io.Base + RootBridges[Index].Io.Translation, > RootBridges[Index].Io.Limit - RootBridges[Index].Io.Base + 1 > ); > ASSERT_EFI_ERROR (Status); > @@ -422,7 +445,7 @@ InitializePciHostBridge ( > EfiGcdIoTypeIo, > 0, > RootBridges[Index].Io.Limit - RootBridges[Index].Io.Base + 1, > - &RootBridges[Index].Io.Base, > + &RootBridges[Index].Io.Base + RootBridges[Index].Io.Translation, > gImageHandle, > NULL > ); > @@ -444,13 +467,13 @@ InitializePciHostBridge ( > for (MemApertureIndex = 0; MemApertureIndex < ARRAY_SIZE (MemApertures); MemApertureIndex++) { > if (MemApertures[MemApertureIndex]->Base <= MemApertures[MemApertureIndex]->Limit) { > Status = AddMemoryMappedIoSpace ( > - MemApertures[MemApertureIndex]->Base, > + MemApertures[MemApertureIndex]->Base + MemApertures[MemApertureIndex]->Translation, > MemApertures[MemApertureIndex]->Limit - MemApertures[MemApertureIndex]->Base + 1, > EFI_MEMORY_UC > ); > ASSERT_EFI_ERROR (Status); > Status = gDS->SetMemorySpaceAttributes ( > - MemApertures[MemApertureIndex]->Base, > + MemApertures[MemApertureIndex]->Base + MemApertures[MemApertureIndex]->Translation, > MemApertures[MemApertureIndex]->Limit - MemApertures[MemApertureIndex]->Base + 1, > EFI_MEMORY_UC > ); > @@ -463,7 +486,7 @@ InitializePciHostBridge ( > EfiGcdMemoryTypeMemoryMappedIo, > 0, > MemApertures[MemApertureIndex]->Limit - MemApertures[MemApertureIndex]->Base + 1, > - &MemApertures[MemApertureIndex]->Base, > + &MemApertures[MemApertureIndex]->Base + MemApertures[MemApertureIndex]->Translation, > gImageHandle, > NULL > ); > @@ -828,8 +851,8 @@ NotifyPhase ( > FALSE, > RootBridge->ResAllocNode[Index].Length, > MIN (15, BitsOfAlignment), > - ALIGN_VALUE (RootBridge->Io.Base, Alignment + 1), > - RootBridge->Io.Limit > + ALIGN_VALUE (RootBridge->Io.Base, Alignment + 1) + RootBridge->Io.Translation, > + RootBridge->Io.Limit + RootBridge->Io.Translation > ); > break; > > @@ -838,8 +861,8 @@ NotifyPhase ( > TRUE, > RootBridge->ResAllocNode[Index].Length, > MIN (63, BitsOfAlignment), > - ALIGN_VALUE (RootBridge->MemAbove4G.Base, Alignment + 1), > - RootBridge->MemAbove4G.Limit > + ALIGN_VALUE (RootBridge->MemAbove4G.Base, Alignment + 1) + RootBridge->MemAbove4G.Translation, > + RootBridge->MemAbove4G.Limit + RootBridge->MemAbove4G.Translation > ); > if (BaseAddress != MAX_UINT64) { > break; > @@ -853,8 +876,8 @@ NotifyPhase ( > TRUE, > RootBridge->ResAllocNode[Index].Length, > MIN (31, BitsOfAlignment), > - ALIGN_VALUE (RootBridge->Mem.Base, Alignment + 1), > - RootBridge->Mem.Limit > + ALIGN_VALUE (RootBridge->Mem.Base, Alignment + 1) + RootBridge->Mem.Translation, > + RootBridge->Mem.Limit + RootBridge->Mem.Translation > ); > break; > > @@ -863,8 +886,8 @@ NotifyPhase ( > TRUE, > RootBridge->ResAllocNode[Index].Length, > MIN (63, BitsOfAlignment), > - ALIGN_VALUE (RootBridge->PMemAbove4G.Base, Alignment + 1), > - RootBridge->PMemAbove4G.Limit > + ALIGN_VALUE (RootBridge->PMemAbove4G.Base, Alignment + 1) + RootBridge->PMemAbove4G.Translation, > + RootBridge->PMemAbove4G.Limit + RootBridge->PMemAbove4G.Translation > ); > if (BaseAddress != MAX_UINT64) { > break; > @@ -877,8 +900,8 @@ NotifyPhase ( > TRUE, > RootBridge->ResAllocNode[Index].Length, > MIN (31, BitsOfAlignment), > - ALIGN_VALUE (RootBridge->PMem.Base, Alignment + 1), > - RootBridge->PMem.Limit > + ALIGN_VALUE (RootBridge->PMem.Base, Alignment + 1) + RootBridge->PMem.Translation, > + RootBridge->PMem.Limit + RootBridge->PMem.Translation > ); > break; > > @@ -1152,6 +1175,7 @@ StartBusEnumeration ( > Descriptor->AddrSpaceGranularity = 0; > Descriptor->AddrRangeMin = RootBridge->Bus.Base; > Descriptor->AddrRangeMax = 0; > + // Ignore translation offset for bus > Descriptor->AddrTranslationOffset = 0; > Descriptor->AddrLen = RootBridge->Bus.Limit - RootBridge->Bus.Base + 1; > > @@ -1421,7 +1445,8 @@ 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; > + Descriptor->AddrRangeMin = 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 dc06c16..bd3394a 100644 > --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c > +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c > @@ -86,12 +86,23 @@ 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"" > )); > + // We don't see any scenario for bus translation, so translation for bus is just ignored. > 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, 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 > @@ -403,6 +414,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 +691,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 = RootBridgeIoGetMemTranslationByAddress (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 +747,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 = RootBridgeIoGetMemTranslationByAddress (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 +797,8 @@ RootBridgeIoIoRead ( > ) > { > EFI_STATUS Status; > + PCI_ROOT_BRIDGE_INSTANCE *RootBridge; > + > Status = RootBridgeIoCheckParameter ( > This, IoOperation, Width, > Address, Count, Buffer > @@ -753,7 +806,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->Io.Translation, Count, Buffer); > } > > /** > @@ -788,6 +844,8 @@ RootBridgeIoIoWrite ( > ) > { > EFI_STATUS Status; > + PCI_ROOT_BRIDGE_INSTANCE *RootBridge; > + > Status = RootBridgeIoCheckParameter ( > This, IoOperation, Width, > Address, Count, Buffer > @@ -795,7 +853,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->Io.Translation, Count, Buffer); > } > > /** > @@ -1615,25 +1676,41 @@ 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, and TranslationOffset = PCI view - CPU view. > Descriptor->AddrRangeMin = ResAllocNode->Base; > Descriptor->AddrRangeMax = ResAllocNode->Base + ResAllocNode->Length - 1; > Descriptor->AddrLen = ResAllocNode->Length; > switch (ResAllocNode->Type) { > > case TypeIo: > - Descriptor->ResType = ACPI_ADDRESS_SPACE_TYPE_IO; > + Descriptor->ResType = ACPI_ADDRESS_SPACE_TYPE_IO; > + // According to UEFI 2.7, translation = PCI address - CPU address, > + // so we change the sign here to make it consistent with UEFI spec. > + // The other translations will be treated as the same. > + Descriptor->AddrTranslationOffset = -RootBridge->Io.Translation; > 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->PMem.Translation; > + Descriptor->ResType = ACPI_ADDRESS_SPACE_TYPE_MEM; > + Descriptor->AddrSpaceGranularity = 32; > + break; > + > case TypeMem32: > + Descriptor->AddrTranslationOffset = -RootBridge->Mem.Translation; > 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->PMemAbove4G.Translation; > + Descriptor->ResType = ACPI_ADDRESS_SPACE_TYPE_MEM; > + Descriptor->AddrSpaceGranularity = 64; > case TypeMem64: > + Descriptor->AddrTranslationOffset = -RootBridge->MemAbove4G.Translation; > 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..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 { > And I think you'd better to add comments in code to clarify whether it's a PCI-view address or CPU-view address. -- Thanks, Ray