On 21 December 2017 at 08:27, Guo Heyi <[email protected]> wrote: > On Wed, Dec 20, 2017 at 03:26:45PM +0000, Ard Biesheuvel wrote: >> On 20 December 2017 at 15:17, gary guo <[email protected]> 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 <[email protected]> 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. >
The MMIO address will not be used for I/O port addressing by the CPU. The MMIO to IO translation is an implementation detail of your CpuIo2 protocol implementation. So there will be two stacked translations, one for PCI I/O to CPU I/O, and one for CPU I/O to CPU MMIO. The latter is transparent to the PCI host bridge driver. > 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 <[email protected]> >> >> > Cc: Eric Dong <[email protected]> >> >> > Cc: Ruiyu Ni <[email protected]> >> >> > Cc: Ard Biesheuvel <[email protected]> >> >> > Cc: Jason Zhang <[email protected]> >> >> > >> >> > --- >> >> > 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 >> >> > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
