visibility pf PcdSet

visibility pf PcdSet

[Michael Zimmermann]

Do I understand correctly, that a PcdSet on a 'PcdsPatchableInModule'
is only visible to the current module?(SEC, driver, application, ...)
Because I've tested this and a PcdSet on
gArmTokenSpaceGuid.PcdSystemMemoryBase inisde PrePi is not visible
inside a DXE_DRIVER - which means that for everyone else the value is
still 0x0.

If this is the case and I don't have some platform bug, then there is
probably a bug in ArmVirtPkg's HighMemDxe where this Pcd is used in a
DXE_DRIVER:
https://github.com/tianocore/edk2/blob/76874be3d411bf8daac051718e20932e0bf97d70/ArmVirtPkg/HighMemDxe/HighMemDxe.c#L70
It doesn't cause anything bad but it would show 'Failed to add System
RAM @ START - END (Access Denied)' after calling AddMemorySpace for
memory which has already been registered.

Thanks
Michael

Re: visibility pf PcdSet

[Laszlo Ersek]

On 03/20/17 17:06, Michael Zimmermann wrote:
> Do I understand correctly, that a PcdSet on a 'PcdsPatchableInModule'
> is only visible to the current module?(SEC, driver, application, ...)

I've no experience with patchable-in-module PCDs.

> Because I've tested this and a PcdSet on
> gArmTokenSpaceGuid.PcdSystemMemoryBase inisde PrePi is not visible
> inside a DXE_DRIVER - which means that for everyone else the value is
> still 0x0.

No experience with PrePi...

> 
> If this is the case and I don't have some platform bug, then there is
> probably a bug in ArmVirtPkg's HighMemDxe where this Pcd is used in a
> DXE_DRIVER:
> https://github.com/tianocore/edk2/blob/76874be3d411bf8daac051718e20932e0bf97d70/ArmVirtPkg/HighMemDxe/HighMemDxe.c#L70
> It doesn't cause anything bad but it would show 'Failed to add System
> RAM @ START - END (Access Denied)' after calling AddMemorySpace for
> memory which has already been registered.

The DEC file that declares a given PCD provides the C language type for
it, the default value, the token space GUID and the token number, and
the set of "PCD flavors" that a specific platform DSC may choose for the
PCD.

In ArmVirtQemu.dsc, PcdSystemMemoryBase is defined under
PcdsFixedAtBuild, which works well, so I wouldn't say the error is in
HighMemDxe. Also, if some other DSC defined PcdSystemMemoryBase as a
dynamic one, it would again work well (assuming the PCD were set early
enough).

My point is, a platform DSC and a (DXE) driver have to work together.

Thanks
Laszlo

Re: visibility pf PcdSet

[Ard Biesheuvel]

On 20 March 2017 at 16:06, Michael Zimmermann <sigmaepsilon92@gmail.com> wrote:
> Do I understand correctly, that a PcdSet on a 'PcdsPatchableInModule'
> is only visible to the current module?(SEC, driver, application, ...)

Yes.

> Because I've tested this and a PcdSet on
> gArmTokenSpaceGuid.PcdSystemMemoryBase inisde PrePi is not visible
> inside a DXE_DRIVER - which means that for everyone else the value is
> still 0x0.
>

Indeed. The relocatable PrePi needs a patchable PCD because it assigns
the value really early, in assembly code. But PrePi is a bit peculiar
as well, and I am pretty use SetPcd() on dynamic PCDs would not work
there either.

> If this is the case and I don't have some platform bug, then there is
> probably a bug in ArmVirtPkg's HighMemDxe where this Pcd is used in a
> DXE_DRIVER:
> https://github.com/tianocore/edk2/blob/76874be3d411bf8daac051718e20932e0bf97d70/ArmVirtPkg/HighMemDxe/HighMemDxe.c#L70
> It doesn't cause anything bad but it would show 'Failed to add System
> RAM @ START - END (Access Denied)' after calling AddMemorySpace for
> memory which has already been registered.
>

Ah yes, well spotted. To be honest, PrePi is a bit of a hack, and I
actually recommend against it for new ports. However, for a self
relocating image (which I think you need for your application), PrePi
is really the only way to go.

So the best way to pass information from PrePi to DXE is using HOBs.
Actually, it might be best for you to clone HighmemDxe if you need it,
and use a HOB instead.

Re: visibility pf PcdSet

[Michael Zimmermann]

> So the best way to pass information from PrePi to DXE is using HOBs.
Actually, it might be best for you to clone HighmemDxe if you need it,
and use a HOB instead.
I actually started a new port fully based on ArmVirtQemuKernel.
So for me it's not a problem to adjust the code, I just thought you
might want to know about the problem.

> However, for a self
relocating image (which I think you need for your application), PrePi
is really the only way to go.
Speaking about PrePi vs Pei - as far as I understand the only
difference is that Pei is supposed to be booted via XIP from flash
right? How would that work when running from DRAM?(even if it wasn't
relocatable)
If that would work, why can't we make a relocatable version of Pei?

Thanks
Michael

On Mon, Mar 20, 2017 at 7:19 PM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> On 20 March 2017 at 16:06, Michael Zimmermann <sigmaepsilon92@gmail.com> wrote:
>> Do I understand correctly, that a PcdSet on a 'PcdsPatchableInModule'
>> is only visible to the current module?(SEC, driver, application, ...)
>
> Yes.
>
>> Because I've tested this and a PcdSet on
>> gArmTokenSpaceGuid.PcdSystemMemoryBase inisde PrePi is not visible
>> inside a DXE_DRIVER - which means that for everyone else the value is
>> still 0x0.
>>
>
> Indeed. The relocatable PrePi needs a patchable PCD because it assigns
> the value really early, in assembly code. But PrePi is a bit peculiar
> as well, and I am pretty use SetPcd() on dynamic PCDs would not work
> there either.
>
>> If this is the case and I don't have some platform bug, then there is
>> probably a bug in ArmVirtPkg's HighMemDxe where this Pcd is used in a
>> DXE_DRIVER:
>> https://github.com/tianocore/edk2/blob/76874be3d411bf8daac051718e20932e0bf97d70/ArmVirtPkg/HighMemDxe/HighMemDxe.c#L70
>> It doesn't cause anything bad but it would show 'Failed to add System
>> RAM @ START - END (Access Denied)' after calling AddMemorySpace for
>> memory which has already been registered.
>>
>
> Ah yes, well spotted. To be honest, PrePi is a bit of a hack, and I
> actually recommend against it for new ports. However, for a self
> relocating image (which I think you need for your application), PrePi
> is really the only way to go.
>
> So the best way to pass information from PrePi to DXE is using HOBs.
> Actually, it might be best for you to clone HighmemDxe if you need it,
> and use a HOB instead.

Re: visibility pf PcdSet

[Gao, Liming]

Yes. Feature, FixedAtBuild and PatchableInModule PCD are module level; Dyanmic and DynamicEx are platform level. 
If you have case to share the data between the different modules, you can configure PCD as Dynamic PCD. 

Ard: do you meet with the issue on SetPcd() with dynamic PCD?  

Thanks
>-----Original Message-----
>From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard
>Biesheuvel
>Sent: Tuesday, March 21, 2017 2:20 AM
>To: Michael Zimmermann <sigmaepsilon92@gmail.com>
>Cc: edk2-devel@lists.01.org <edk2-devel@ml01.01.org>; Gao, Liming
><liming.gao@intel.com>; Laszlo Ersek <lersek@redhat.com>; Leif Lindholm
><leif.lindholm@linaro.org>
>Subject: Re: [edk2] visibility pf PcdSet
>
>On 20 March 2017 at 16:06, Michael Zimmermann
><sigmaepsilon92@gmail.com> wrote:
>> Do I understand correctly, that a PcdSet on a 'PcdsPatchableInModule'
>> is only visible to the current module?(SEC, driver, application, ...)
>
>Yes.
>
>> Because I've tested this and a PcdSet on
>> gArmTokenSpaceGuid.PcdSystemMemoryBase inisde PrePi is not visible
>> inside a DXE_DRIVER - which means that for everyone else the value is
>> still 0x0.
>>
>
>Indeed. The relocatable PrePi needs a patchable PCD because it assigns
>the value really early, in assembly code. But PrePi is a bit peculiar
>as well, and I am pretty use SetPcd() on dynamic PCDs would not work
>there either.
>
>> If this is the case and I don't have some platform bug, then there is
>> probably a bug in ArmVirtPkg's HighMemDxe where this Pcd is used in a
>> DXE_DRIVER:
>>
>https://github.com/tianocore/edk2/blob/76874be3d411bf8daac051718e20932
>e0bf97d70/ArmVirtPkg/HighMemDxe/HighMemDxe.c#L70
>> It doesn't cause anything bad but it would show 'Failed to add System
>> RAM @ START - END (Access Denied)' after calling AddMemorySpace for
>> memory which has already been registered.
>>
>
>Ah yes, well spotted. To be honest, PrePi is a bit of a hack, and I
>actually recommend against it for new ports. However, for a self
>relocating image (which I think you need for your application), PrePi
>is really the only way to go.
>
>So the best way to pass information from PrePi to DXE is using HOBs.
>Actually, it might be best for you to clone HighmemDxe if you need it,
>and use a HOB instead.
>_______________________________________________
>edk2-devel mailing list
>edk2-devel@lists.01.org
>https://lists.01.org/mailman/listinfo/edk2-devel

Re: visibility pf PcdSet

[Ard Biesheuvel]

On 21 March 2017 at 02:23, Gao, Liming <liming.gao@intel.com> wrote:
> Yes. Feature, FixedAtBuild and PatchableInModule PCD are module level; Dyanmic and DynamicEx are platform level.
> If you have case to share the data between the different modules, you can configure PCD as Dynamic PCD.
>
> Ard: do you meet with the issue on SetPcd() with dynamic PCD?
>

No. MIchael reported that DXE level drivers don't see the value
written to a patchable PCD by a SEC module, but as you confirmed, this
is expected behavior.

Thanks,
Ard.

>>-----Original Message-----
>>From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard
>>Biesheuvel
>>Sent: Tuesday, March 21, 2017 2:20 AM
>>To: Michael Zimmermann <sigmaepsilon92@gmail.com>
>>Cc: edk2-devel@lists.01.org <edk2-devel@ml01.01.org>; Gao, Liming
>><liming.gao@intel.com>; Laszlo Ersek <lersek@redhat.com>; Leif Lindholm
>><leif.lindholm@linaro.org>
>>Subject: Re: [edk2] visibility pf PcdSet
>>
>>On 20 March 2017 at 16:06, Michael Zimmermann
>><sigmaepsilon92@gmail.com> wrote:
>>> Do I understand correctly, that a PcdSet on a 'PcdsPatchableInModule'
>>> is only visible to the current module?(SEC, driver, application, ...)
>>
>>Yes.
>>
>>> Because I've tested this and a PcdSet on
>>> gArmTokenSpaceGuid.PcdSystemMemoryBase inisde PrePi is not visible
>>> inside a DXE_DRIVER - which means that for everyone else the value is
>>> still 0x0.
>>>
>>
>>Indeed. The relocatable PrePi needs a patchable PCD because it assigns
>>the value really early, in assembly code. But PrePi is a bit peculiar
>>as well, and I am pretty use SetPcd() on dynamic PCDs would not work
>>there either.
>>
>>> If this is the case and I don't have some platform bug, then there is
>>> probably a bug in ArmVirtPkg's HighMemDxe where this Pcd is used in a
>>> DXE_DRIVER:
>>>
>>https://github.com/tianocore/edk2/blob/76874be3d411bf8daac051718e20932
>>e0bf97d70/ArmVirtPkg/HighMemDxe/HighMemDxe.c#L70
>>> It doesn't cause anything bad but it would show 'Failed to add System
>>> RAM @ START - END (Access Denied)' after calling AddMemorySpace for
>>> memory which has already been registered.
>>>
>>
>>Ah yes, well spotted. To be honest, PrePi is a bit of a hack, and I
>>actually recommend against it for new ports. However, for a self
>>relocating image (which I think you need for your application), PrePi
>>is really the only way to go.
>>
>>So the best way to pass information from PrePi to DXE is using HOBs.
>>Actually, it might be best for you to clone HighmemDxe if you need it,
>>and use a HOB instead.
>>_______________________________________________
>>edk2-devel mailing list
>>edk2-devel@lists.01.org
>>https://lists.01.org/mailman/listinfo/edk2-devel