From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.115; helo=mga14.intel.com; envelope-from=ruiyu.ni@intel.com; receiver=edk2-devel@lists.01.org Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) (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 E0FEC22198F44 for ; Thu, 21 Dec 2017 01:38:30 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 21 Dec 2017 01:43:19 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.45,435,1508828400"; d="scan'208";a="17848294" Received: from ray-dev.ccr.corp.intel.com (HELO [10.239.9.14]) ([10.239.9.14]) by orsmga001.jf.intel.com with ESMTP; 21 Dec 2017 01:43:17 -0800 To: Ard Biesheuvel , gary guo Cc: linaro-uefi , "edk2-devel@lists.01.org" , "Zeng, Star" , "Dong, Eric" , Jason Zhang References: <1513758078-99534-1-git-send-email-heyi.guo@linaro.org> <20171220151704.GA2482@iwish> From: "Ni, Ruiyu" Message-ID: Date: Thu, 21 Dec 2017 17:43:17 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: Subject: Re: [RFC] MdeModulePkg/PciHostBridge: Add address translation support 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, 21 Dec 2017 09:38:31 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit On 12/20/2017 11:26 PM, Ard Biesheuvel wrote: > On 20 December 2017 at 15:17, gary guo wrote: >> On Wed, Dec 20, 2017 at 09:13:58AM +0000, Ard Biesheuvel wrote: >>> Hi Heyi, >>> >>> On 20 December 2017 at 08:21, Heyi Guo wrote: >>>> PCIe on some ARM platforms requires address translation, not only for >>>> legacy IO access, but also for 32bit memory BAR access as well. There >>>> will be "Address Translation Unit" or something similar in PCI host >>>> bridges to translation CPU address to PCI address and vice versa. So >>>> we think it may be useful to add address translation support to the >>>> generic PCI host bridge driver. >>>> >>> >>> I agree. While unusual on a PC, it is quite common on other >>> architectures to have more complex non 1:1 topologies, which currently >>> require a forked PciHostBridgeDxe driver with local changes applied. >>> >>>> This RFC only contains one minor change to the definition of >>>> PciHostBridgeLib, and there certainly will be a lot of other changes >>>> to make it work, including: >>>> >>>> 1. Use CPU address for GCD space add and allocate operations, instead >>>> of PCI address; also IO space will be changed to memory space if >>>> translation exists. >>>> >>> >>> For I/O space, the translation should simply be applied to the I/O >>> range. I don't think it makes sense to use memory space here, given >>> that it is specific to architectures that lack native port I/O. >>> >> >> I made an assumption here that platforms supporting real port IO space >> do not need address translation, like IA32 and X64, and port IO >> translation implies the platform does not support real port IO space. >> > > This may be a reasonable assumption. But I still think it is better > not to encode any assumptions in the first place. > >> Indeed the assumption is not so "generic", so I'll agree if you >> recommend to support IO to IO translation as well. But I still hope to >> have IO to memory translation support in PCI host bridge driver, >> rather than in CPU IO protocol, since the faked IO space might only be >> used for PCI host bridge and we may have overlapping IO ranges for >> each host bridge, which is compatible with PCIe specification and PCIe >> ACPI description. >> > > That is fine. Under UEFI, these will translate to non-overlapping I/O > spaces in the CPU's view. Under the OS, this could be totally > different. > I fully agree with the PciRootBridgeLib interface change to support MMIO offset. I guess that you will propose to add a AddrTranslationOffset to PCI_ROOT_BRIDGE structure. But I don't quite understand the needs of IO offset support. And further more, there was a discussion in the mailing list regarding how to utilize the Offset. There was 2 points about the meaning of Offset and we cannot agree with each other. Spec is not quite clear. 1. Offset = Host - Device 2. Offset = Device - Host > > For example, > > RC0 IO 0x0000 .. 0xffff -> CPU 0x00000 .. 0x0ffff > RC1 IO 0x0000 .. 0xffff -> CPU 0x10000 .. 0x1ffff > > This is very similar to how MMIO translation works, and makes I/O > devices behind the host bridges uniquely addressable for drivers. > > For our understanding, could you share the host bridge configuration > that you are targetting? > > Thanks, > Ard. > > >> How about adding a flag to indicate whether port IO is translated to >> real port IO space or system memory space? >> >> Thanks and regards, >> >> Heyi >> >>>> 2. RootBridgeIoMemRead/Write, RootBridgeIoRead/Write need to get >>>> translation of the corresponding aperture, add the translation to the >>>> input address, and then call CpuIo2 protocol; IO access will also be >>>> converted to memory access if IO translation exists. >>>> >>> >>> Again, why is this necessary? A host bridge that implements a non 1:1 >>> translation for port I/O ranges may be part of a system that has >>> native port I/O, and so the translation should be based on that. >>> >>>> 3. RootBridgeIoConfiguration needs to fill AddrTranslationOffset in >>>> the discriptor. >>>> >>> >>> Indeed. Note that this has been a source of much confusion lately, >>> should we should discuss this carefully before spending time on an >>> implementation. >>> >>>> If it makes sense, then I'll continue to prepare the formal patch. >>>> >>>> Any comments? >>>> >>>> Thanks, >>>> >>>> Gary (Heyi Guo) >>>> >>>> Cc: Star Zeng >>>> Cc: Eric Dong >>>> Cc: Ruiyu Ni >>>> Cc: Ard Biesheuvel >>>> Cc: Jason Zhang >>>> >>>> --- >>>> MdeModulePkg/Include/Library/PciHostBridgeLib.h | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> 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 { >>>> -- >>>> 2.7.4 >>>> -- Thanks, Ray