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:c06::235; helo=mail-io0-x235.google.com; envelope-from=heyi.guo@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-io0-x235.google.com (mail-io0-x235.google.com [IPv6:2607:f8b0:4001:c06::235]) (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 56269208F7A23 for ; Thu, 21 Dec 2017 00:22:42 -0800 (PST) Received: by mail-io0-x235.google.com with SMTP id i143so83579ioa.3 for ; Thu, 21 Dec 2017 00:27:31 -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=kI5fH5fb5yaHRV0UWoZuR/55wVxNQORj8mCQvuDokRo=; b=dS2DQauYYC6XBUD4h+Hn06YPucXExmEtpmVoTsjOEGoTXBEWYR2tGFKELrCKTK3095 5HhB5FlzD+BxPyVrcyxmBDz8RQgq60zW+2X9zrYM8lllcpeIWu4BPQ7ZEJvRe4jRWLF7 mmfviTbbCw+tt9peIY9nWsDjgKqQyVqr2MKGY= 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=kI5fH5fb5yaHRV0UWoZuR/55wVxNQORj8mCQvuDokRo=; b=nGjP7pRC3T0mYEtPoYs9kklXwXOwhs4FMYfLQt073L7HC/r4Dfjo0v1n87YjuHJJGR 88RKwe+mt5WA1NaNW05qQl9tLFS7+zdoZ/pvyFxqKzHyn5DCMOY6BizT5Y7/ZMovvVNi do+mvMml+DlifWQhjAPzsn4R3Ee3yFcHRSvXl5OUwkGFm1W/eoHVmqnVlMrxKsAy5V7w 4gHl+kaRRA31gdt23k9XeHoG2cXyaKNXGXN++kuJcvMRMSTwe20APKAICadmHQz4qZdf DM/g7sYH2RSizFzkrHnqmQQ4N4zHWcfZuSl9TkipKuUUukLSxOW9JAPHlUXW+reWko22 8Ugg== X-Gm-Message-State: AKGB3mJynuMeWGXb9aCFY/YmWJeqaazi4XlEce/HAujbMs49GE+iST/o QRuCc3Ju4nthrNZ6d6oUi2GDtQ== X-Google-Smtp-Source: ACJfBotMhZAdmKoMesk2xopo7f8Ag/pgqLPGf9nXfQeSWcZyBnjfhNlXdf3h5VLnB2MYydXz4A1KUg== X-Received: by 10.107.44.71 with SMTP id s68mr1643018ios.110.1513844850927; Thu, 21 Dec 2017 00:27:30 -0800 (PST) Received: from SZX1000114654 ([45.56.152.249]) by smtp.gmail.com with ESMTPSA id x97sm3888422ita.9.2017.12.21.00.27.28 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 21 Dec 2017 00:27:30 -0800 (PST) From: Guo Heyi X-Google-Original-From: Guo Heyi Date: Thu, 21 Dec 2017 16:27:26 +0800 To: Ard Biesheuvel Cc: gary guo , linaro-uefi , "edk2-devel@lists.01.org" , Star Zeng , Eric Dong , Ruiyu Ni , Jason Zhang Message-ID: <20171221082726.GA100076@SZX1000114654> References: <1513758078-99534-1-git-send-email-heyi.guo@linaro.org> <20171220151704.GA2482@iwish> MIME-Version: 1.0 In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) 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 08:22:43 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Wed, Dec 20, 2017 at 03:26:45PM +0000, 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. > > > 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? IO translation on one of our platforms is like below: PCI IO space CPU memory space 0x0000 .. 0xffff -> 0xafff0000 .. 0xafffffff (The sizes are always 0x10000 so I will omit the limit for others) 0x0000 .. 0xffff -> 0x8abff0000 0x0000 .. 0xffff -> 0x8b7ff0000 ...... The translated addresses may be beyond 32bit address, will this violate IO space limitation? From EDK2 code, I didn't see such limitation for IO space. In my opinion, if we still treat the translated address as IO space in PCI host bridge driver while IO space is really translated to memory space, then we couldn't utilize the existing GCD manipulation code in PCI host bridge, including allocating memory space to avoid space colliding (we can't avoid colliding with other drivers which may also use the same memory address), setting MMU page tables. Anyway, this is not a blocking issue. Thanks and regards, Gary (Heyi Guo) > > 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 > >> >