From: "Michael D Kinney" <michael.d.kinney@intel.com>
To: "Wang, Jian J" <jian.j.wang@intel.com>,
Laszlo Ersek <lersek@redhat.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>,
"Kinney, Michael D" <michael.d.kinney@intel.com>
Cc: "Sukerkar, Amol N" <amol.n.sukerkar@intel.com>,
"Yao, Jiewen" <jiewen.yao@intel.com>
Subject: Re: [edk2-devel] [Patch v10 2/2] CryptoPkg/BaseHashApiLib: Implement Unified Hash Calculation API
Date: Wed, 5 Feb 2020 16:18:54 +0000 [thread overview]
Message-ID: <E92EE9817A31E24EB0585FDF735412F5B9E86F1A@ORSMSX113.amr.corp.intel.com> (raw)
In-Reply-To: <D827630B58408649ACB04F44C510003625A0815E@SHSMSX107.ccr.corp.intel.com>
Jian,
I agree. If the PCD type is anything but FixedAtBuild,
the compiler can not optimize away the unused BaseCryptLib
functions.
I think the best solution is to limit this PCD to only
FixedAtBuild.
Thank you for noticing this issue Laszlo!
Mike
> -----Original Message-----
> From: Wang, Jian J <jian.j.wang@intel.com>
> Sent: Wednesday, February 5, 2020 5:54 AM
> To: Laszlo Ersek <lersek@redhat.com>;
> devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: Sukerkar, Amol N <amol.n.sukerkar@intel.com>; Yao,
> Jiewen <jiewen.yao@intel.com>
> Subject: RE: [edk2-devel] [Patch v10 2/2]
> CryptoPkg/BaseHashApiLib: Implement Unified Hash
> Calculation API
>
> Laszlo,
>
> According to RFC discussion, using PCD here is mainly
> for optimization purpose. So I
> think we should limit the PCD type to just
> FixedAtBuild. Then there's no problem
> for modules linking this library.
>
> Regards,
> Jian
>
> > -----Original Message-----
> > From: Laszlo Ersek <lersek@redhat.com>
> > Sent: Wednesday, February 05, 2020 7:00 PM
> > To: devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>
> > Cc: Sukerkar, Amol N <amol.n.sukerkar@intel.com>;
> Yao, Jiewen
> > <jiewen.yao@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>
> > Subject: Re: [edk2-devel] [Patch v10 2/2]
> CryptoPkg/BaseHashApiLib: Implement
> > Unified Hash Calculation API
> >
> > Hi,
> >
> > sorry I'm late to this discussion. I'd only like to
> mention a potential
> > future improvement:
> >
> > On 02/04/20 00:35, Michael D Kinney wrote:
> >
> > > +[PcdsFixedAtBuild, PcdsPatchableInModule,
> PcdsDynamic, PcdsDynamicEx]
> > > + ## This PCD indicates the HASH algorithm to
> calculate hash of data
> > > + # Based on the value set, the required
> algorithm is chosen to calculate
> > > + # the hash of data.<BR>
> > > + # The default hashing algorithm for
> BaseHashApiLib is set to SHA256.<BR>
> > > + # 0x00000001 - MD4.<BR>
> > > + # 0x00000002 - MD5.<BR>
> > > + # 0x00000003 - SHA1.<BR>
> > > + # 0x00000004 - SHA256.<BR>
> > > + # 0x00000005 - SHA384.<BR>
> > > + # 0x00000006 - SHA512.<BR>
> > > + # 0x00000007 - SM3_256.<BR>
> > > + # @Prompt Set policy for hashing unsigned image
> for Secure Boot.
> > > + # @ValidRange 0x80000001 | 0x00000001 -
> 0x00000007
> > > +
> >
> gEfiCryptoPkgTokenSpaceGuid.PcdHashApiLibPolicy|0x04|UI
> NT8|0x00000001
> > > +
> >
> > The platform may choose to make this PCD dynamic or
> dynamicEx. That's
> > good. But:
> >
> > > +UINTN
> > > +EFIAPI
> > > +HashApiGetContextSize (
> > > + VOID
> > > + )
> > > +{
> > > + switch (PcdGet8 (PcdHashApiLibPolicy)) {
> > > + case HASH_API_ALGO_MD4:
> > > + return Md4GetContextSize ();
> > > + break;
> >
> > we have direct PcdGet8() calls in the lib API
> implementations. And:
> >
> > > +[Defines]
> > > + INF_VERSION = 0x00010005
> > > + BASE_NAME = BaseHashApiLib
> > > + MODULE_UNI_FILE =
> BaseHashApiLib.uni
> > > + FILE_GUID = B1E566DD-DE7C-
> 4F04-BDA0-B1295D3BE927
> > > + MODULE_TYPE = BASE
> > > + VERSION_STRING = 1.0
> > > + LIBRARY_CLASS = BaseHashApiLib
> >
> > [...]
> >
> > > +[Pcd]
> > > + gEfiCryptoPkgTokenSpaceGuid.PcdHashApiLibPolicy
> ## CONSUMES
> >
> > The lib class is not restricted to any particular
> firmware phase, or
> > module type.
> >
> > This suggests that the lib instance is usable in DXE
> runtime drivers or
> > SMM drivers. If the serives are called outside of the
> entry point
> > functions, the dynamic PCD fetches would be a
> problem, I think.
> >
> > So the idea here would be to create a minimal
> separate INF file + C file
> > for runtime applications (runtime DXE and SMM
> drivers), and there a
> > constructor function could run PcdGet8(), and stash
> the value in a
> > global variable.
> >
> > Alternatively, if this is overkill, we could improve
> safety by restricting
> >
> > LIBRARY_CLASS = BaseHashApiLib|<module_type>
> <module_type> ...
> >
> > to every module type except runtime DXE drivers and
> SMM drivers.
> >
> > Thanks
> > Laszlo
next prev parent reply other threads:[~2020-02-05 16:19 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-03 23:35 [Patch v10 0/2] CryptoPkg/BaseHashApiLib: Implement Unified Hash Calculation API Michael D Kinney
2020-02-03 23:35 ` [Patch v10 1/2] CryptoPkg: Add CryptoPkg Token Space GUID Michael D Kinney
2020-02-03 23:35 ` [Patch v10 2/2] CryptoPkg/BaseHashApiLib: Implement Unified Hash Calculation API Michael D Kinney
2020-02-04 2:53 ` Yao, Jiewen
2020-02-04 5:24 ` [edk2-devel] " Zhang, Chao B
2020-02-04 16:29 ` Michael D Kinney
2020-02-04 16:26 ` Michael D Kinney
2020-02-04 17:10 ` Sukerkar, Amol N
2020-02-04 18:30 ` Sukerkar, Amol N
2020-02-04 23:06 ` Yao, Jiewen
2020-02-04 23:15 ` Sukerkar, Amol N
2020-02-04 23:20 ` Yao, Jiewen
2020-02-04 23:22 ` Sukerkar, Amol N
2020-02-04 22:54 ` Yao, Jiewen
2020-02-05 1:04 ` Michael D Kinney
2020-02-05 1:32 ` Yao, Jiewen
2020-02-05 11:00 ` [edk2-devel] " Laszlo Ersek
2020-02-05 13:53 ` Wang, Jian J
2020-02-05 16:18 ` Michael D Kinney [this message]
2020-02-05 21:23 ` Laszlo Ersek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=E92EE9817A31E24EB0585FDF735412F5B9E86F1A@ORSMSX113.amr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox