Re: [PATCH 1/3] OvmfPkg/Tcg2ConfigPei: clean up some lib class dependencies

["Laszlo Ersek" <lersek@redhat.com>]

Hi Phil,

On 05/21/20 12:29, Philippe Mathieu-Daudé wrote:
> On 5/21/20 12:58 AM, Laszlo Ersek wrote:
>> Commit 89236992913f introduced an explicit Tpm12CommandLib dependency to
>> Tcg2ConfigPei.
>>
>> In reality this lib class is not consumed by Tcg2ConfigPei at all (such a
>> dependency is not even inherited from other lib instances). Simplify the
>> module by dropping the superfluous dependency.
>>
>> (The Tpm12CommandLib class resolution that was also added in commit
>> 89236992913f is not useless, at the platform build level: it is consumed
>> by TcgPei and TcgDxe. Meaning that said Tpm12CommandLib resolution should
>> have likely been a part of the subsequent patch in the original series,
>> namely commit 6be54f15a0c9.)
>>
>> Commit 89236992913f also introduced SwapBytesXx() calls. Those functions
>> are provided by BaseLib. Spell out the BaseLib dependency.
>>
>> Functionally, this patch is a no-op.
>>
>> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>> Cc: Eric Auger <eric.auger@redhat.com>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
>> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
>> Cc: Simon Hardy <simon.hardy@itdev.co.uk>
>> Cc: Stefan Berger <stefanb@linux.ibm.com>
>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2728
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>   OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf | 2 +-
>>   OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c  | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
>> b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
>> index 97c529c91d0b..b79d0a3fb912 100644
>> --- a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
>> +++ b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
>> @@ -30,9 +30,9 @@ [Packages]
>>     [LibraryClasses]
>>     PeimEntryPoint
>> +  BaseLib
>>     DebugLib
>>     PeiServicesLib
>> -  Tpm12CommandLib
>>     Tpm12DeviceLib
>>     Tpm2DeviceLib
>>   diff --git a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c
>> b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c
>> index 5b5075bded92..44abd6c541f9 100644
>> --- a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c
>> +++ b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c
>> @@ -15,11 +15,11 @@
>>   #include <PiPei.h>
>>     #include <Guid/TpmInstance.h>
>> +#include <Library/BaseLib.h>
>>   #include <Library/DebugLib.h>
>>   #include <Library/PeiServicesLib.h>
>>   #include <Library/Tpm2DeviceLib.h>
>>   #include <Library/Tpm12DeviceLib.h>
>> -#include <Library/Tpm12CommandLib.h>
>>   #include <Ppi/TpmInitialized.h>
>>     STATIC CONST EFI_PEI_PPI_DESCRIPTOR mTpmSelectedPpi = {
>>
> 
> Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
> 

I'm extremely sorry for missing your R-b's. When I read your email with
msgid <60493dcc-0832-1fb3-f380-867410691672@redhat.com>, and
consequently decided that I'd go ahead with the feedback provided thus
far, I didn't realize you were going to review the series as well! I
switched off my email refresh very soon after.

Normally I let a series sit for at least a day (24 hours) on the list,
so that everyone (in different timezones too) can at least state an
intent (even in private) to review or test the series. I encourage other
maintainers to do the same, and I've complained in the past when I would
have liked to review or test a series but wasn't given enough time even
to signal my interest.

So this is totally my fault. My only excuse is that the feature
freeze(s) are upon us, and fixing stuff feels "urgent". I wish we could
append release candidates (RCs) "on-demand", like QEMU does.

I'm sorry! And thank you for your review.

Laszlo