From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.81]) by mx.groups.io with SMTP id smtpd.web10.11107.1579860870575215124 for ; Fri, 24 Jan 2020 02:14:31 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=D9pmWzEz; spf=pass (domain: redhat.com, ip: 207.211.31.81, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1579860869; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=qYGAH7wmxZHVEAHKX0Jg7x1e0Jh61LTFppCM1qYOEWM=; b=D9pmWzEzgQkw7d1K53rZggR7sMMZJUeqk35FHAj7R3zlfoN75ggAtyO1vEeAa5n/hmkGBN PX7FEShDUAonlQbhPE2Gt7BMLQinBRAW+jcAnJkNTY0MYSV3PImOkePY7q3W2x2mn//GgJ RKwZH9EfoNqxiK2fnl8lJUe9dhgjWfU= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-253-N5yHetvhNx2F4wc_h9OYWQ-1; Fri, 24 Jan 2020 05:14:27 -0500 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 8F3E685EE74; Fri, 24 Jan 2020 10:14:26 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-117-77.ams2.redhat.com [10.36.117.77]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6E17C60BE1; Fri, 24 Jan 2020 10:14:25 +0000 (UTC) Subject: Re: [edk2-devel] PCD EX interface. To: Andrew Fish , devel@edk2.groups.io Cc: Ken_Taylor@phoenix.com References: <2fb8e97b19214251bf25f44c7b4149a3@SCL-EXCHMB-13.phoenix.com> <5D2A47A2-6DCE-448B-A5BA-14BE7BBA464B@apple.com> From: "Laszlo Ersek" Message-ID: <54362b07-c55e-c9f9-1f5d-86848f769384@redhat.com> Date: Fri, 24 Jan 2020 11:14:24 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <5D2A47A2-6DCE-448B-A5BA-14BE7BBA464B@apple.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-MC-Unique: N5yHetvhNx2F4wc_h9OYWQ-1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 01/23/20 22:08, Andrew Fish wrote: > >> On Jan 23, 2020, at 3:21 AM, Laszlo Ersek 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