On Fri, Apr 29, 2016 at 05:41:19PM +0200, Ard Biesheuvel wrote: > The PCI related PCDs are not platform specific, and architectural > protocols such as CpuIo2 are based on PCI provided MMIO to IO > translation, so these PCDs belong in ArmPkg not ArmPlatformPkg.
Very happy with this - but, could you before pushing add a notice to message saying something like: "This *WILL* break some out-of-tree platforms, the fix is changing all consumers of gArmPlatformTokenSpaceGuid.PcdPci* to gArmTokenSpaceGuid.PcdPci."? > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel <[email protected]> Reviewed-by: Leif Lindholm <[email protected]> > --- > ArmPkg/ArmPkg.dec | 62 > ++++++++++++++++++++ > ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf | 4 > +- > ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/PciHostBridgeDxe.inf | 16 > ++--- > ArmPlatformPkg/ArmJunoPkg/Library/ArmJunoLib/ArmJunoLib.inf | 8 > +-- > ArmPlatformPkg/ArmPlatformPkg.dec | 62 > -------------------- > 5 files changed, 76 insertions(+), 76 deletions(-) > > diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec > index e90306653864..75c238aa1e3d 100644 > --- a/ArmPkg/ArmPkg.dec > +++ b/ArmPkg/ArmPkg.dec > @@ -255,3 +255,65 @@ [PcdsFixedAtBuild.common, PcdsDynamic.common] > gArmTokenSpaceGuid.PcdGicRedistributorsBase|0|UINT32|0x0000000E > gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase|0|UINT32|0x0000000D > gArmTokenSpaceGuid.PcdGicSgiIntId|0|UINT32|0x00000025 > + > + # > + # Bases, sizes and translation offsets of IO and MMIO spaces, respectively. > + # Note that "IO" is just another MMIO range that simulates IO space; there > + # are no special instructions to access it. > + # > + # The base addresses PcdPciIoBase, PcdPciMmio32Base and PcdPciMmio64Base > are > + # specific to their containing address spaces. In order to get the physical > + # address for the CPU, for a given access, the respective translation value > + # has to be added. > + # > + # The translations always have to be initialized like this, using UINT64: > + # > + # UINT64 IoCpuBase; // mapping target in 64-bit cpu-physical space > + # UINT64 Mmio32CpuBase; // mapping target in 64-bit cpu-physical space > + # UINT64 Mmio64CpuBase; // mapping target in 64-bit cpu-physical space > + # > + # PcdPciIoTranslation = IoCpuBase - PcdPciIoBase; > + # PcdPciMmio32Translation = Mmio32CpuBase - (UINT64)PcdPciMmio32Base; > + # PcdPciMmio64Translation = Mmio64CpuBase - PcdPciMmio64Base; > + # > + # because (a) the target address space (ie. the cpu-physical space) is > + # 64-bit, and (b) the translation values are meant as offsets for *modular* > + # arithmetic. > + # > + # Accordingly, the translation itself needs to be implemented as: > + # > + # UINT64 UntranslatedIoAddress; // input parameter > + # UINT32 UntranslatedMmio32Address; // input parameter > + # UINT64 UntranslatedMmio64Address; // input parameter > + # > + # UINT64 TranslatedIoAddress; // output parameter > + # UINT64 TranslatedMmio32Address; // output parameter > + # UINT64 TranslatedMmio64Address; // output parameter > + # > + # TranslatedIoAddress = UntranslatedIoAddress + > + # PcdPciIoTranslation; > + # TranslatedMmio32Address = (UINT64)UntranslatedMmio32Address + > + # PcdPciMmio32Translation; > + # TranslatedMmio64Address = UntranslatedMmio64Address + > + # PcdPciMmio64Translation; > + # > + # The modular arithmetic performed in UINT64 ensures that the translation > + # works correctly regardless of the relation between IoCpuBase and > + # PcdPciIoBase, Mmio32CpuBase and PcdPciMmio32Base, and Mmio64CpuBase and > + # PcdPciMmio64Base. > + # > + gArmTokenSpaceGuid.PcdPciIoBase|0x0|UINT64|0x00000050 > + gArmTokenSpaceGuid.PcdPciIoSize|0x0|UINT64|0x00000051 > + gArmTokenSpaceGuid.PcdPciIoTranslation|0x0|UINT64|0x00000052 > + gArmTokenSpaceGuid.PcdPciMmio32Base|0x0|UINT32|0x00000053 > + gArmTokenSpaceGuid.PcdPciMmio32Size|0x0|UINT32|0x00000054 > + gArmTokenSpaceGuid.PcdPciMmio32Translation|0x0|UINT64|0x00000055 > + gArmTokenSpaceGuid.PcdPciMmio64Base|0x0|UINT64|0x00000056 > + gArmTokenSpaceGuid.PcdPciMmio64Size|0x0|UINT64|0x00000057 > + gArmTokenSpaceGuid.PcdPciMmio64Translation|0x0|UINT64|0x00000058 > + > + # > + # Inclusive range of allowed PCI buses. > + # > + gArmTokenSpaceGuid.PcdPciBusMin|0x0|UINT32|0x00000059 > + gArmTokenSpaceGuid.PcdPciBusMax|0x0|UINT32|0x0000005A > diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf > b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf > index 6ab81e8dd60a..a2617982b259 100644 > --- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf > +++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf > @@ -80,8 +80,8 @@ [FixedPcd] > > # PCI Root complex specific PCDs > gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceBaseAddress > - gArmPlatformTokenSpaceGuid.PcdPciBusMin > - gArmPlatformTokenSpaceGuid.PcdPciBusMax > + gArmTokenSpaceGuid.PcdPciBusMin > + gArmTokenSpaceGuid.PcdPciBusMax > > [Pcd] > gEmbeddedTokenSpaceGuid.PcdFdtDevicePaths > diff --git > a/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/PciHostBridgeDxe.inf > b/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/PciHostBridgeDxe.inf > index 9f526910c48a..de28c805ae6e 100644 > --- a/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/PciHostBridgeDxe.inf > +++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/PciHostBridgeDxe.inf > @@ -58,14 +58,14 @@ [Pcd.common] > gArmTokenSpaceGuid.PcdSystemMemoryBase > gArmTokenSpaceGuid.PcdSystemMemorySize > > - gArmPlatformTokenSpaceGuid.PcdPciBusMin > - gArmPlatformTokenSpaceGuid.PcdPciBusMax > - gArmPlatformTokenSpaceGuid.PcdPciIoBase > - gArmPlatformTokenSpaceGuid.PcdPciIoSize > - gArmPlatformTokenSpaceGuid.PcdPciMmio32Base > - gArmPlatformTokenSpaceGuid.PcdPciMmio32Size > - gArmPlatformTokenSpaceGuid.PcdPciMmio64Base > - gArmPlatformTokenSpaceGuid.PcdPciMmio64Size > + gArmTokenSpaceGuid.PcdPciBusMin > + gArmTokenSpaceGuid.PcdPciBusMax > + gArmTokenSpaceGuid.PcdPciIoBase > + gArmTokenSpaceGuid.PcdPciIoSize > + gArmTokenSpaceGuid.PcdPciMmio32Base > + gArmTokenSpaceGuid.PcdPciMmio32Size > + gArmTokenSpaceGuid.PcdPciMmio64Base > + gArmTokenSpaceGuid.PcdPciMmio64Size > > gArmJunoTokenSpaceGuid.PcdPcieControlBaseAddress > gArmJunoTokenSpaceGuid.PcdPcieRootPortBaseAddress > diff --git a/ArmPlatformPkg/ArmJunoPkg/Library/ArmJunoLib/ArmJunoLib.inf > b/ArmPlatformPkg/ArmJunoPkg/Library/ArmJunoLib/ArmJunoLib.inf > index 530bb1cb9dc0..8c8b141c35a2 100644 > --- a/ArmPlatformPkg/ArmJunoPkg/Library/ArmJunoLib/ArmJunoLib.inf > +++ b/ArmPlatformPkg/ArmJunoPkg/Library/ArmJunoLib/ArmJunoLib.inf > @@ -58,10 +58,10 @@ [FixedPcd] > gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceSize > > [Pcd] > - gArmPlatformTokenSpaceGuid.PcdPciMmio32Base > - gArmPlatformTokenSpaceGuid.PcdPciMmio32Size > - gArmPlatformTokenSpaceGuid.PcdPciMmio64Base > - gArmPlatformTokenSpaceGuid.PcdPciMmio64Size > + gArmTokenSpaceGuid.PcdPciMmio32Base > + gArmTokenSpaceGuid.PcdPciMmio32Size > + gArmTokenSpaceGuid.PcdPciMmio64Base > + gArmTokenSpaceGuid.PcdPciMmio64Size > > [Ppis] > gArmMpCoreInfoPpiGuid > diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec > b/ArmPlatformPkg/ArmPlatformPkg.dec > index 0a7e78443eff..1c05132cd625 100644 > --- a/ArmPlatformPkg/ArmPlatformPkg.dec > +++ b/ArmPlatformPkg/ArmPlatformPkg.dec > @@ -125,68 +125,6 @@ [PcdsFixedAtBuild.common,PcdsDynamic.common] > gArmPlatformTokenSpaceGuid.PcdPL031RtcBase|0x0|UINT32|0x00000024 > > gArmPlatformTokenSpaceGuid.PcdPL031RtcPpmAccuracy|300000000|UINT32|0x00000022 > > - # > - # Inclusive range of allowed PCI buses. > - # > - gArmPlatformTokenSpaceGuid.PcdPciBusMin|0x0|UINT32|0x0000003E > - gArmPlatformTokenSpaceGuid.PcdPciBusMax|0x0|UINT32|0x0000003F > - > - # > - # Bases, sizes and translation offsets of IO and MMIO spaces, respectively. > - # Note that "IO" is just another MMIO range that simulates IO space; there > - # are no special instructions to access it. > - # > - # The base addresses PcdPciIoBase, PcdPciMmio32Base and PcdPciMmio64Base > are > - # specific to their containing address spaces. In order to get the physical > - # address for the CPU, for a given access, the respective translation value > - # has to be added. > - # > - # The translations always have to be initialized like this, using UINT64: > - # > - # UINT64 IoCpuBase; // mapping target in 64-bit cpu-physical space > - # UINT64 Mmio32CpuBase; // mapping target in 64-bit cpu-physical space > - # UINT64 Mmio64CpuBase; // mapping target in 64-bit cpu-physical space > - # > - # PcdPciIoTranslation = IoCpuBase - PcdPciIoBase; > - # PcdPciMmio32Translation = Mmio32CpuBase - (UINT64)PcdPciMmio32Base; > - # PcdPciMmio64Translation = Mmio64CpuBase - PcdPciMmio64Base; > - # > - # because (a) the target address space (ie. the cpu-physical space) is > - # 64-bit, and (b) the translation values are meant as offsets for *modular* > - # arithmetic. > - # > - # Accordingly, the translation itself needs to be implemented as: > - # > - # UINT64 UntranslatedIoAddress; // input parameter > - # UINT32 UntranslatedMmio32Address; // input parameter > - # UINT64 UntranslatedMmio64Address; // input parameter > - # > - # UINT64 TranslatedIoAddress; // output parameter > - # UINT64 TranslatedMmio32Address; // output parameter > - # UINT64 TranslatedMmio64Address; // output parameter > - # > - # TranslatedIoAddress = UntranslatedIoAddress + > - # PcdPciIoTranslation; > - # TranslatedMmio32Address = (UINT64)UntranslatedMmio32Address + > - # PcdPciMmio32Translation; > - # TranslatedMmio64Address = UntranslatedMmio64Address + > - # PcdPciMmio64Translation; > - # > - # The modular arithmetic performed in UINT64 ensures that the translation > - # works correctly regardless of the relation between IoCpuBase and > - # PcdPciIoBase, Mmio32CpuBase and PcdPciMmio32Base, and Mmio64CpuBase and > - # PcdPciMmio64Base. > - # > - gArmPlatformTokenSpaceGuid.PcdPciIoBase|0x0|UINT64|0x00000040 > - gArmPlatformTokenSpaceGuid.PcdPciIoSize|0x0|UINT64|0x00000041 > - gArmPlatformTokenSpaceGuid.PcdPciIoTranslation|0x0|UINT64|0x00000042 > - gArmPlatformTokenSpaceGuid.PcdPciMmio32Base|0x0|UINT32|0x00000043 > - gArmPlatformTokenSpaceGuid.PcdPciMmio32Size|0x0|UINT32|0x00000044 > - gArmPlatformTokenSpaceGuid.PcdPciMmio32Translation|0x0|UINT64|0x00000045 > - gArmPlatformTokenSpaceGuid.PcdPciMmio64Base|0x0|UINT64|0x00000046 > - gArmPlatformTokenSpaceGuid.PcdPciMmio64Size|0x0|UINT64|0x00000047 > - gArmPlatformTokenSpaceGuid.PcdPciMmio64Translation|0x0|UINT64|0x00000048 > - > [PcdsFixedAtBuild.ARM] > # Stack for CPU Cores in Secure Monitor Mode > gArmPlatformTokenSpaceGuid.PcdCPUCoresSecMonStackBase|0|UINT64|0x00000007 > -- > 2.7.4 > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel

