On 01/23/20 22:08, Andrew Fish wrote:
>
>> On Jan 23, 2020, at 3:21 AM, Laszlo Ersek <ler...@redhat.com> wrote:
>>
>> Hi Ken,
>>
>> On 01/23/20 02:37, Ken Taylor wrote:
>>
>>> If I try to get the size of a DynamicEx PCD in the context of a BIOS
>>> build for which that PCD is undefined, the call locks up. I
>>> expected to just get a size of 0, since the PCD is not defined in
>>> the build context of the PCD DXE service. Is this a problem that's
>>> been fixed since my BIOS source code was cut? What can I do for
>>> older builds that haven't been fixed (and probably never will)? Do
>>> I have to just accept that I'm going to get garbage or lockup if I
>>> run my shell utility on some builds? Do I have to write a DXE
>>> driver and expose a protocol, just so I can know if that PCD exists
>>> and is properly defined?
>>
>> I think the ASSERT() that you run into is the one in
>> GetExPcdTokenNumber(), file
>> "MdeModulePkg/Universal/PCD/Dxe/Service.c":
>>
>>> MatchGuid = ScanGuid (GuidTable, mDxeGuidTableSize, Guid);
>>> //
>>> // We need to ASSERT here. If GUID can't be found in GuidTable, this is a
>>> // error in the BUILD system.
>>> //
>>> ASSERT (MatchGuid != NULL);
>>
>> Can you try the following:
>>
>> - Locate EFI_PCD_PROTOCOL.
>>
>> - Call EFI_PCD_PROTOCOL.GetNextTokenSpace() in a loop, until you find
>> the GUID of your own token space GUID, or the function returns an
>> error.
>>
>> - If your token space GUID has been found, call PcdGetEx8().
>>
>
> Laszlo,
>
> I think it may be better to call PCD_PROTOCOL.GetNextToken() as the
> GUID and Token spaces are both sparse name spaces. The Get*Ex()
> functions don't return errors so I guess this is your only choice.
I agree.
I've been thinking about a way to identify the token value that we need
to stop at, when enumerating the existing tokens (in the already-found
vendor GUID token space). It looks like we need to do three things:
- first check whether the vendor token namespace GUID is known by the
platform (using the above-quoted EFI_PCD_PROTOCOL.GetNextTokenSpace()
loop),
- if so, call PcdTokenEx (Guid,TokenName); save the return value
- call EFI_PCD_PROTOCOL.GetNextToken (Guid, &TokenNumber) in a loop,
until it finishes with EFI_NOT_FOUND, or the desired token number is
found
- then call PcdGetEx8().
> You could probably make a MyLibGet*Ex() function that returns
> EFI_STATUS, and returns the data via an arg. MyLibGet*Ex() could
> abstract the PCD_PROTOCOL.GetNextToken() check, and it could also
> probably abstract grabbing the protocol.
>
> FYI it looks like the function header is much more description for
> GetNextToken() in PCD_PROTOCOL [1] vs EFI_PCD_PROTOCOL [2]. Also
> EFI_PCD_PROTOCOL does not have the *Ex functions, so you need
> PCD_PROTOCOL anyway to have access to the *Ex versions of the API.
EFI_PCD_PROTOCOL is the portable (PI standard) protocol, and I do think
it provides access to the DynamicEx (i.e., GUID-ed) PCDs. What
EFI_PCD_PROTOCOL does not provide access to is the *non-Ex* (i.e.,
GUID-less) Dynamic PCDs.
When I browsed the PCD DXE driver yesterday, this confused me hugely.
The names of the EFI_PCD_PROTOCOL member functions do not *say* "Ex",
but they do *mean* "Ex" -- they take GUID parameters.
We can compare the MdePkg/Library/DxePcdLib implementations of
LibPcdGet8() and LibPcdGetEx8():
> UINT8
> EFIAPI
> LibPcdGet8 (
> IN UINTN TokenNumber
> )
> {
> return GetPcdProtocol()->Get8 (TokenNumber);
> }
vs.
> UINT8
> EFIAPI
> LibPcdGetEx8 (
> IN CONST GUID *Guid,
> IN UINTN TokenNumber
> )
> {
> ASSERT (Guid != NULL);
>
> return GetPiPcdProtocol()->Get8 (Guid, TokenNumber);
> }
The non-Ex version calls GetPcdProtocol(), which returns PCD_PROTOCOL.
Then we call PCD_PROTOCOL.Get8(). To that, we don't pass a GUID.
The Ex version calls GetPiPcdProtocol), which returns EFI_PCD_PROTOCOL.
Then we call EFI_PCD_PROTOCOL.Get8(). To that, we do pass a GUID -- in
spite of the member name not containing "Ex".
And then, we can check both protocol member implementations in
"MdeModulePkg/Universal/PCD/Dxe/Pcd.c":
- "mPcdInstance" is of type PCD_PROTOCOL, and "mPcdInstance.Get8" points
to DxePcdGet8(),
- "mEfiPcdInstance" is of type EFI_PCD_PROTOCOL, and
"mEfiPcdInstance.Get8" points to DxePcdGet8Ex().
DxePcdGet8() only takes a TokenNumber, DxePcdGet8Ex() takes both Guid
and ExTokenNumber.
So I guess, when PCD_PROTOCOL got standardized for the PI spec, as
EFI_PCD_PROTOCOL, people must have said, 'we don't need no stinky
GUID-less PCD access -- and then, why state "Ex" in the member names, if
"Ex" is the *only* way anyway?'
I do find it strange though that, as you say, there seem to be no
"PcdLib.h" interfaces that look up both the GUID and the TokenNumber
automatically, return RETURN_NOT_FOUND if either is absent, and store
the value to an output parameter otherwise.
(While we do have LibPcdSetXxxS() functions -- note the trailing "S" --
that return status values, those don't seem to cope with missing GUID or
TokenNumber either.)
Thanks!
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#53470): https://edk2.groups.io/g/devel/message/53470
Mute This Topic: https://groups.io/mt/70044584/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
