Hi
On Wed, Feb 26, 2020 at 11:46 AM Laszlo Ersek <ler...@redhat.com> wrote:
>
> Hi,
>
> On 02/26/20 10:34, marcandre.lur...@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lur...@redhat.com>
> >
> > Matches TPM 2.0 commit 3103389043bd ("OvmfPkg: Add TCG2 Configuration
> > menu to the Device Manager menu", 2019-02-11).
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> > ---
> > OvmfPkg/OvmfPkgIa32.dsc | 6 ++++++
> > OvmfPkg/OvmfPkgIa32X64.dsc | 6 ++++++
> > OvmfPkg/OvmfPkgX64.dsc | 6 ++++++
> > 3 files changed, 18 insertions(+)
> >
> > diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> > index 2fc10d2393e3..02300886563e 100644
> > --- a/OvmfPkg/OvmfPkgIa32.dsc
> > +++ b/OvmfPkg/OvmfPkgIa32.dsc
> > @@ -936,4 +936,10 @@
> > <LibraryClasses>
> >
> > Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibDTpm/Tpm12DeviceLibDTpm.inf
> > }
> > +!if $(TPM_CONFIG_ENABLE) == TRUE
> > + SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDxe.inf {
> > + <LibraryClasses>
> > + PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> > + }
> > +!endif
> > !endif
> > diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> > index cd9d2ac724ca..3adc75223d05 100644
> > --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> > +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> > @@ -950,4 +950,10 @@
> > <LibraryClasses>
> >
> > Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibDTpm/Tpm12DeviceLibDTpm.inf
> > }
> > +!if $(TPM_CONFIG_ENABLE) == TRUE
> > + SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDxe.inf {
> > + <LibraryClasses>
> > + PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> > + }
> > +!endif
> > !endif
> > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> > index 317a23b994b8..5f3740ae890a 100644
> > --- a/OvmfPkg/OvmfPkgX64.dsc
> > +++ b/OvmfPkg/OvmfPkgX64.dsc
> > @@ -948,4 +948,10 @@
> > <LibraryClasses>
> >
> > Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibDTpm/Tpm12DeviceLibDTpm.inf
> > }
> > +!if $(TPM_CONFIG_ENABLE) == TRUE
> > + SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDxe.inf {
> > + <LibraryClasses>
> > + PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> > + }
> > +!endif
> > !endif
> >
>
> I've got two comments on this:
>
> (1) I'm unsure why we need to explicitly specify the PcdLib class
> resolution here. The module in question is of type DXE_DRIVER, and we
> already have the following in the DSC files:
>
> [LibraryClasses.common.DXE_DRIVER]
> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
>
> So I'd think the bracketed { <LibraryClasses> ... } parts should be
> possible to omit.
>
> (2) The driver is not being added to the FDF files, so they will be
> complied, but not included in the firmware executable.
>
> TBH I'd suggest simply dropping this patch.
I agree, I'll drop it for now. It can be added later.
Thanks
>
> If Stefan insists on including this patch in the series, then please fix
> (1) and (2) above. Like that:
>
> Reviewed-by: Laszlo Ersek <ler...@redhat.com>
>
> Thanks!
> Laszlo
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#54887): https://edk2.groups.io/g/devel/message/54887
Mute This Topic: https://groups.io/mt/71562986/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
