Re: [RFC] MdeModulePkg/PciHostBridge: Add address translation support

[Guo Heyi <heyi.guo@linaro.org>]

Hi Ard, Ray,

Have we come to the final conclusion? Or are we still waiting for more comments on this?

Thanks,

Gary

On Thu, Dec 21, 2017 at 10:07:51AM +0000, Ard Biesheuvel wrote:
> On 21 December 2017 at 09:59, Ni, Ruiyu <ruiyu.ni@intel.com> wrote:
> > On 12/21/2017 5:52 PM, Ard Biesheuvel wrote:
> >>
> >> On 21 December 2017 at 09:48, Ni, Ruiyu <ruiyu.ni@intel.com> wrote:
> >>>
> >>> On 12/21/2017 5:14 PM, Guo Heyi wrote:
> >>>>
> >>>>
> >>>> On Thu, Dec 21, 2017 at 08:32:37AM +0000, Ard Biesheuvel wrote:
> >>>>>
> >>>>>
> >>>>> On 21 December 2017 at 08:27, Guo Heyi <heyi.guo@linaro.org> wrote:
> >>>>>>
> >>>>>>
> >>>>>> On Wed, Dec 20, 2017 at 03:26:45PM +0000, Ard Biesheuvel wrote:
> >>>>>>>
> >>>>>>>
> >>>>>>> On 20 December 2017 at 15:17, gary guo <heyi.guo@linaro.org> 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 <heyi.guo@linaro.org> 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.
> >>>>
> >>>>
> >>>>
> >>>> Yes this should work.
> >>>>
> >>>> Hi Star, Eric and Ruiyu,
> >>>>
> >>>> Any comments on this RFC?
> >>>
> >>>
> >>>
> >>> Let me confirm my understanding:
> >>> The PciHostBridge core driver/library interface changes only
> >>> take care of the MMIO translation.
> >>>
> >>> Heyi you will implement a special CpuIo driver in your
> >>> platform code to take care of the IO to MMIO translation.
> >>>
> >>> But let me confirm, will you need to additional translate
> >>> the MMIO (translated from IO) to another MMIO using an offset?
> >>> If yes, will you handle that translation in your CpuIo driver?
> >>>
> >>
> >> Hi Ray,
> >>
> >> The issue is that several PCIe root complexes have colliding I/O ranges:
> >
> >
> > Ard,
> > The IO-MMIO mapping needs CPU support. I am not sure whether IA32 or
> > x64 supports.
> > But I guess ARM supports. right?
> >
> 
> Yes. Exposing PCI I/O ranges via MMIO translation is specific to the
> CpuIo2 implementations we have for ARM.
> 
> > Will all the IO part be implemented in ARM CpuIo2 protocol?
> >
> 
> No. The CPU to PCI I/O translation needs to be in PciHostBridgeDxe,
> because it is in charge of allocating the GCD space, and without
> translation, those allocations will collide. Also, while perhaps
> non-existent in reality, it is imaginable that a host bridge could
> translate port I/O addresses between the two sides of the bridge,
> similar to how non 1:1 mapped PCI MMIO is handled.
> 
> So just like MMIO, the port I/O address used by the CPU, and the port
> I/O address programmed into the device BAR could be subject to
> translation.
> 
> This is not solvable in the CpuIo2 protocol, because without
> translation at the host bridge driver level, a CPU port I/O address is
> ambiguous: e.g., address 0x1000 may apply to each of the RCs.
> 
> 
> >>
> >>>>>> 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
> >>
> >>
> >> So the CPU view is different from the PCI view, and to create a single
> >> CPU I/O space where all I/O port ranges behind all host bridges are
> >> accessible, we need I/O translation for the CPU. This will result in
> >> an intermediate representation
> >>
> >>>>>> PCI IO space        CPU IO space
> >>>>>> 0x0000 .. 0xffff -> 0x00000 .. 0x0ffff
> >>>>>> 0x0000 .. 0xffff -> 0x10000 .. 0x1ffff
> >>>>>> 0x0000 .. 0xffff -> 0x20000 .. 0x2ffff
> >>
> >>
> >> On top of that, given that ARM has no native port I/O instructions, we
> >> will need to implement MMIO to IO translation, but this can be
> >> implemented in the CpuIo2 protocol.
> >> _______________________________________________
> >> edk2-devel mailing list
> >> edk2-devel@lists.01.org
> >> https://lists.01.org/mailman/listinfo/edk2-devel
> >>
> >
> >
> > --
> > Thanks,
> > Ray