On 06/08/15 21:07, Jordan Justen wrote: > On 2015-06-08 02:06:40, Laszlo Ersek wrote: >> On 06/06/15 21:10, Paulo Alcantara wrote: >>> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec >>> index 4cb70dc..a6586f3 100644 >>> --- a/OvmfPkg/OvmfPkg.dec >>> +++ b/OvmfPkg/OvmfPkg.dec >>> @@ -78,6 +78,10 @@ >>> # to PIIX4 function 3 offset 0x40-0x43 bits [15:6]. >>> gUefiOvmfPkgTokenSpaceGuid.PcdAcpiPmBaseAddress|0xB000|UINT16|5 >>> >>> + ## This flag determines the Root Complex Register Block BAR, written to >>> Q35 >>> + # function 31 offset 0xf0-0xf3 bits [31:14] >>> + >>> gUefiOvmfPkgTokenSpaceGuid.PcdRootComplexBaseAddress|0xfed1c000|UINT32|0x1e >>> + >> >> I understand Jordan doesn't like the new PCD here, and proposes a >> fixed macro for the same purpose, but I don't understand why we >> should follow a different avenue for this base address when we opted >> for a PCD with the PMBA. > > I'm not sure there is a good reason for the PMBA PCD at this point. > > Do you remember why we decided to add a PCD? It doesn't actually > change values. I wonder if we were only half committed to the 0x400 => > 0xb000 value change at that point? :)
It's a fixed PCD. I think the argument was simply "hey it's a build time constant". However, at this point PcdAcpiPmBaseAddress is used in quite a few places, so I think it should remain a PCD. > I could also see adding a PCD if it looks better for some 'common' > code to key off of the PCD, rather than including a chipset specific > include file. Options: (1) both PMBA and RCBA are PCDs, (2) PMBA is a PCD, RCBA is a macro, (3) both PMBA and RCBA are macros. I think I prefer (1), but I don't really insist on it: I'd be okay with (2). Whereas, (3) would be really bad. Thanks Laszlo