Re: [Qemu-devel] [edk2] [PATCH] OvmfPkg/PlatformPei: Initialise RCBA (B0:D31:F0 0xf0) register

[Laszlo Ersek <lersek@redhat.com>]

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