From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c0b::244; helo=mail-it0-x244.google.com; envelope-from=heyi.guo@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it0-x244.google.com (mail-it0-x244.google.com [IPv6:2607:f8b0:4001:c0b::244]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 1932820954CC1 for ; Tue, 6 Mar 2018 21:55:34 -0800 (PST) Received: by mail-it0-x244.google.com with SMTP id u5so1942694itc.1 for ; Tue, 06 Mar 2018 22:01:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:date:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=n8HdRcTgJjYZXCvzungJuEoC3OwaALga236G6Qu5WkI=; b=emqmJMHC8UcWP33Zvqn9zBlkKsjLFseKmk3VHcJlBYjFEoeFMnhWmaLxxB8PYUJZTb EbP2X/B7zoAcRZo8Z2nKlQdzJOvN0R0mvNPK8OVsCxU7i+Rbcnt5c8QTveMT3lF6BUlN wfoT+ote8CJv8C/oh8atuNFtIKLm+GRPN8VyU= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:date:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=n8HdRcTgJjYZXCvzungJuEoC3OwaALga236G6Qu5WkI=; b=Gsxg/V6qRaQhZtlsMcm7REAV8xVUMDSaGjUclDcSfq6BRnpww6F23mfjS5Sak0B2If c+mmz/f7sxgCGCNOh1UAviv35vGA1tcXJlPMQVR3T5r6TPaGXTYRhG4T6S0TS25nfgyI NDm6shsjQmiDkheR6JQsfMonO/0pJLrISsCmVu0UzQ5WH3xUJ7UCMvDqJop5Cz5bvsmE XQS4mJoJzTkG9a5m/iddAYT87JomJoGEbVkkTbOg9CznaaLu3BZ+OWjia9b2+QFL3aIS l4RCfelH627ENSZA6kDP/0mpTAxcTl23VnMuwM9B8oRBn+iJ16YlZBuMs9139S2MPyH/ H38w== X-Gm-Message-State: AElRT7EWJHb4+cgPCUp9XZdbTFC6423BoiLM5R8Pe9tDPy3u64DLOrwE Htj3aBb/Ig7HhbWn6glPKlY61w== X-Google-Smtp-Source: AG47ELv0BRx1JLFEJWK18R46upf1VXJoLkwhEDK9cu/HHe0STethRotYvSYLIVlaChpzqrluOzrYVw== X-Received: by 10.36.224.9 with SMTP id c9mr20991730ith.107.1520402508496; Tue, 06 Mar 2018 22:01:48 -0800 (PST) Received: from SZX1000114654 ([45.56.152.76]) by smtp.gmail.com with ESMTPSA id k65sm2419229ita.37.2018.03.06.22.01.45 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 06 Mar 2018 22:01:47 -0800 (PST) From: Guo Heyi X-Google-Original-From: Guo Heyi Date: Wed, 7 Mar 2018 14:01:43 +0800 To: "Ni, Ruiyu" Cc: Guo Heyi , edk2-devel@lists.01.org, Ard Biesheuvel , Star Zeng , Eric Dong , Laszlo Ersek , Michael D Kinney Message-ID: <20180307060143.GB91936@SZX1000114654> References: <1519887444-75510-1-git-send-email-heyi.guo@linaro.org> <1519887444-75510-5-git-send-email-heyi.guo@linaro.org> <20180306024440.GA90780@SZX1000114654> <71a21699-a1b3-155a-e743-ee5e6288ce20@Intel.com> MIME-Version: 1.0 In-Reply-To: <71a21699-a1b3-155a-e743-ee5e6288ce20@Intel.com> User-Agent: Mutt/1.5.24 (2015-08-30) Subject: Re: [PATCH v5 4/6] 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: Wed, 07 Mar 2018 05:55:35 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 > >>Cc: Ruiyu Ni > >>Cc: Ard Biesheuvel > >>Cc: Star Zeng > >>Cc: Eric Dong > >>Cc: Laszlo Ersek > >>Cc: Michael D Kinney > >>--- > >> > >>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