* [PATCH 1/3] OvmfPkg/Tcg2ConfigPei: clean up some lib class dependencies
2020-05-20 22:58 [PATCH 0/3] OvmfPkg/Tcg2ConfigPei: fix ARM/AARCH64 build failure Laszlo Ersek
@ 2020-05-20 22:58 ` Laszlo Ersek
2020-05-21 10:29 ` Philippe Mathieu-Daudé
2020-05-20 22:58 ` [PATCH 2/3] OvmfPkg/Tcg2ConfigPei: factor out InternalTpm12Detect() Laszlo Ersek
` (4 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Laszlo Ersek @ 2020-05-20 22:58 UTC (permalink / raw)
To: edk2-devel-groups-io
Cc: Ard Biesheuvel, Eric Auger, Jordan Justen, Marc-André Lureau,
Philippe Mathieu-Daudé, Simon Hardy, Stefan Berger
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 = {
--
2.19.1.3.g30247aa5d201
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 1/3] OvmfPkg/Tcg2ConfigPei: clean up some lib class dependencies
2020-05-20 22:58 ` [PATCH 1/3] OvmfPkg/Tcg2ConfigPei: clean up some lib class dependencies Laszlo Ersek
@ 2020-05-21 10:29 ` Philippe Mathieu-Daudé
2020-05-21 18:04 ` Laszlo Ersek
0 siblings, 1 reply; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-05-21 10:29 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-groups-io
Cc: Ard Biesheuvel, Eric Auger, Jordan Justen, Marc-André Lureau,
Simon Hardy, Stefan Berger
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>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 1/3] OvmfPkg/Tcg2ConfigPei: clean up some lib class dependencies
2020-05-21 10:29 ` Philippe Mathieu-Daudé
@ 2020-05-21 18:04 ` Laszlo Ersek
2020-05-21 18:52 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 15+ messages in thread
From: Laszlo Ersek @ 2020-05-21 18:04 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, edk2-devel-groups-io
Cc: Ard Biesheuvel, Eric Auger, Jordan Justen, Marc-André Lureau,
Simon Hardy, Stefan Berger
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
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 1/3] OvmfPkg/Tcg2ConfigPei: clean up some lib class dependencies
2020-05-21 18:04 ` Laszlo Ersek
@ 2020-05-21 18:52 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-05-21 18:52 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-groups-io
Cc: Ard Biesheuvel, Eric Auger, Jordan Justen, Marc-André Lureau,
Simon Hardy, Stefan Berger
On 5/21/20 8:04 PM, Laszlo Ersek wrote:
> 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.
Don't worry, no problem!
This was an easy refactor to review. As it is a buildfix I wanted to
review it ASAP so it get merged for the freeze. I'm glad it is merged.
>
> Laszlo
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/3] OvmfPkg/Tcg2ConfigPei: factor out InternalTpm12Detect()
2020-05-20 22:58 [PATCH 0/3] OvmfPkg/Tcg2ConfigPei: fix ARM/AARCH64 build failure Laszlo Ersek
2020-05-20 22:58 ` [PATCH 1/3] OvmfPkg/Tcg2ConfigPei: clean up some lib class dependencies Laszlo Ersek
@ 2020-05-20 22:58 ` Laszlo Ersek
2020-05-21 10:33 ` Philippe Mathieu-Daudé
2020-05-20 22:58 ` [PATCH 3/3] OvmfPkg/Tcg2ConfigPei: skip TPM-1.2 detection when building for ARM/AARCH64 Laszlo Ersek
` (3 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Laszlo Ersek @ 2020-05-20 22:58 UTC (permalink / raw)
To: edk2-devel-groups-io
Cc: Ard Biesheuvel, Eric Auger, Jordan Justen, Marc-André Lureau,
Philippe Mathieu-Daudé, Simon Hardy, Stefan Berger
Move the calls to the Tpm12RequestUseTpm() and Tpm12SubmitCommand()
Tpm12DeviceLib functions to a separate C file, so that we can override
these actions in a subsequent patch.
This code movement requires moving the TPM_RSP_GET_TICKS / TestTpm12()
helper structure / function too.
While at it, give the TestTpm12() function @retval / @return
documentation, plus wrap an overlong line in it.
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/Tpm12Support.h | 30 ++++++++
OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c | 46 +-----------
OvmfPkg/Tcg/Tcg2Config/Tpm12Support.c | 79 ++++++++++++++++++++
4 files changed, 115 insertions(+), 42 deletions(-)
diff --git a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
index b79d0a3fb912..aa996b7da778 100644
--- a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
+++ b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
@@ -21,6 +21,8 @@ [Defines]
[Sources]
Tcg2ConfigPeim.c
+ Tpm12Support.c
+ Tpm12Support.h
[Packages]
MdePkg/MdePkg.dec
diff --git a/OvmfPkg/Tcg/Tcg2Config/Tpm12Support.h b/OvmfPkg/Tcg/Tcg2Config/Tpm12Support.h
new file mode 100644
index 000000000000..c739775d2353
--- /dev/null
+++ b/OvmfPkg/Tcg/Tcg2Config/Tpm12Support.h
@@ -0,0 +1,30 @@
+/** @file
+ Declare the InternalTpm12Detect() function, hiding the TPM-1.2 detection
+ internals.
+
+ Copyright (C) 2020, Red Hat, Inc.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef TPM12_SUPPORT_H_
+#define TPM12_SUPPORT_H_
+
+#include <Uefi/UefiBaseType.h>
+
+/**
+ Detect the presence of a TPM with interface version 1.2.
+
+ @retval EFI_SUCCESS TPM-1.2 available. The Tpm12RequestUseTpm() and
+ Tpm12SubmitCommand(TPM_ORD_GetTicks) operations
+ (from the Tpm12DeviceLib class) have succeeded.
+
+ @return Error codes propagated from Tpm12RequestUseTpm() and
+ Tpm12SubmitCommand().
+**/
+EFI_STATUS
+InternalTpm12Detect (
+ VOID
+ );
+
+#endif // TPM12_SUPPORT_H_
diff --git a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c
index 44abd6c541f9..cc54d95cad19 100644
--- a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c
+++ b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c
@@ -15,13 +15,13 @@
#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 <Ppi/TpmInitialized.h>
+#include "Tpm12Support.h"
+
STATIC CONST EFI_PEI_PPI_DESCRIPTOR mTpmSelectedPpi = {
(EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
&gEfiTpmDeviceSelectedGuid,
@@ -34,44 +34,6 @@ STATIC CONST EFI_PEI_PPI_DESCRIPTOR mTpmInitializationDonePpiList = {
NULL
};
-#pragma pack (1)
-
-typedef struct {
- TPM_RSP_COMMAND_HDR Hdr;
- TPM_CURRENT_TICKS CurrentTicks;
-} TPM_RSP_GET_TICKS;
-
-#pragma pack ()
-
-/**
- Probe for the TPM for 1.2 version, by sending TPM1.2 GetTicks
-
- Sending a TPM1.2 command to a TPM2 should return a TPM1.2
- header (tag = 0xc4) and error code (TPM_BADTAG = 0x1e)
-**/
-static
-EFI_STATUS
-TestTpm12 (
- )
-{
- EFI_STATUS Status;
- TPM_RQU_COMMAND_HDR Command;
- TPM_RSP_GET_TICKS Response;
- UINT32 Length;
-
- Command.tag = SwapBytes16 (TPM_TAG_RQU_COMMAND);
- Command.paramSize = SwapBytes32 (sizeof (Command));
- Command.ordinal = SwapBytes32 (TPM_ORD_GetTicks);
-
- Length = sizeof (Response);
- Status = Tpm12SubmitCommand (sizeof (Command), (UINT8 *)&Command, &Length, (UINT8 *)&Response);
- if (EFI_ERROR (Status)) {
- return Status;
- }
-
- return EFI_SUCCESS;
-}
-
/**
The entry point for Tcg2 configuration driver.
@@ -90,8 +52,8 @@ Tcg2ConfigPeimEntryPoint (
DEBUG ((DEBUG_INFO, "%a\n", __FUNCTION__));
- Status = Tpm12RequestUseTpm ();
- if (!EFI_ERROR (Status) && !EFI_ERROR (TestTpm12 ())) {
+ Status = InternalTpm12Detect ();
+ if (!EFI_ERROR (Status)) {
DEBUG ((DEBUG_INFO, "%a: TPM1.2 detected\n", __FUNCTION__));
Size = sizeof (gEfiTpmDeviceInstanceTpm12Guid);
Status = PcdSetPtrS (
diff --git a/OvmfPkg/Tcg/Tcg2Config/Tpm12Support.c b/OvmfPkg/Tcg/Tcg2Config/Tpm12Support.c
new file mode 100644
index 000000000000..4f5a775c7a03
--- /dev/null
+++ b/OvmfPkg/Tcg/Tcg2Config/Tpm12Support.c
@@ -0,0 +1,79 @@
+/** @file
+ Implement the InternalTpm12Detect() function on top of the Tpm12DeviceLib
+ class.
+
+ Copyright (C) 2020, Red Hat, Inc.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <Library/BaseLib.h>
+#include <Library/Tpm12DeviceLib.h>
+
+#include "Tpm12Support.h"
+
+#pragma pack (1)
+typedef struct {
+ TPM_RSP_COMMAND_HDR Hdr;
+ TPM_CURRENT_TICKS CurrentTicks;
+} TPM_RSP_GET_TICKS;
+#pragma pack ()
+
+/**
+ Probe for the TPM for 1.2 version, by sending TPM1.2 GetTicks
+
+ Sending a TPM1.2 command to a TPM2 should return a TPM1.2
+ header (tag = 0xc4) and error code (TPM_BADTAG = 0x1e)
+
+ @retval EFI_SUCCESS TPM version 1.2 probing successful.
+
+ @return Error codes propagated from Tpm12SubmitCommand().
+**/
+STATIC
+EFI_STATUS
+TestTpm12 (
+ )
+{
+ EFI_STATUS Status;
+ TPM_RQU_COMMAND_HDR Command;
+ TPM_RSP_GET_TICKS Response;
+ UINT32 Length;
+
+ Command.tag = SwapBytes16 (TPM_TAG_RQU_COMMAND);
+ Command.paramSize = SwapBytes32 (sizeof (Command));
+ Command.ordinal = SwapBytes32 (TPM_ORD_GetTicks);
+
+ Length = sizeof (Response);
+ Status = Tpm12SubmitCommand (sizeof (Command), (UINT8 *)&Command, &Length,
+ (UINT8 *)&Response);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ return EFI_SUCCESS;
+}
+
+/**
+ Detect the presence of a TPM with interface version 1.2.
+
+ @retval EFI_SUCCESS TPM-1.2 available. The Tpm12RequestUseTpm() and
+ Tpm12SubmitCommand(TPM_ORD_GetTicks) operations
+ (from the Tpm12DeviceLib class) have succeeded.
+
+ @return Error codes propagated from Tpm12RequestUseTpm() and
+ Tpm12SubmitCommand().
+**/
+EFI_STATUS
+InternalTpm12Detect (
+ VOID
+ )
+{
+ EFI_STATUS Status;
+
+ Status = Tpm12RequestUseTpm ();
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ return TestTpm12 ();
+}
--
2.19.1.3.g30247aa5d201
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 2/3] OvmfPkg/Tcg2ConfigPei: factor out InternalTpm12Detect()
2020-05-20 22:58 ` [PATCH 2/3] OvmfPkg/Tcg2ConfigPei: factor out InternalTpm12Detect() Laszlo Ersek
@ 2020-05-21 10:33 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-05-21 10:33 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-groups-io
Cc: Ard Biesheuvel, Eric Auger, Jordan Justen, Marc-André Lureau,
Simon Hardy, Stefan Berger
On 5/21/20 12:58 AM, Laszlo Ersek wrote:
> Move the calls to the Tpm12RequestUseTpm() and Tpm12SubmitCommand()
> Tpm12DeviceLib functions to a separate C file, so that we can override
> these actions in a subsequent patch.
>
> This code movement requires moving the TPM_RSP_GET_TICKS / TestTpm12()
> helper structure / function too.
>
> While at it, give the TestTpm12() function @retval / @return
> documentation, plus wrap an overlong line in it.
>
> 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/Tpm12Support.h | 30 ++++++++
> OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c | 46 +-----------
> OvmfPkg/Tcg/Tcg2Config/Tpm12Support.c | 79 ++++++++++++++++++++
> 4 files changed, 115 insertions(+), 42 deletions(-)
>
> diff --git a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> index b79d0a3fb912..aa996b7da778 100644
> --- a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> +++ b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> @@ -21,6 +21,8 @@ [Defines]
>
> [Sources]
> Tcg2ConfigPeim.c
> + Tpm12Support.c
> + Tpm12Support.h
>
> [Packages]
> MdePkg/MdePkg.dec
> diff --git a/OvmfPkg/Tcg/Tcg2Config/Tpm12Support.h b/OvmfPkg/Tcg/Tcg2Config/Tpm12Support.h
> new file mode 100644
> index 000000000000..c739775d2353
> --- /dev/null
> +++ b/OvmfPkg/Tcg/Tcg2Config/Tpm12Support.h
> @@ -0,0 +1,30 @@
> +/** @file
> + Declare the InternalTpm12Detect() function, hiding the TPM-1.2 detection
> + internals.
> +
> + Copyright (C) 2020, Red Hat, Inc.
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#ifndef TPM12_SUPPORT_H_
> +#define TPM12_SUPPORT_H_
> +
> +#include <Uefi/UefiBaseType.h>
> +
> +/**
> + Detect the presence of a TPM with interface version 1.2.
> +
> + @retval EFI_SUCCESS TPM-1.2 available. The Tpm12RequestUseTpm() and
> + Tpm12SubmitCommand(TPM_ORD_GetTicks) operations
> + (from the Tpm12DeviceLib class) have succeeded.
> +
> + @return Error codes propagated from Tpm12RequestUseTpm() and
> + Tpm12SubmitCommand().
> +**/
> +EFI_STATUS
> +InternalTpm12Detect (
> + VOID
> + );
> +
> +#endif // TPM12_SUPPORT_H_
> diff --git a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c
> index 44abd6c541f9..cc54d95cad19 100644
> --- a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c
> +++ b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c
> @@ -15,13 +15,13 @@
> #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 <Ppi/TpmInitialized.h>
>
> +#include "Tpm12Support.h"
> +
> STATIC CONST EFI_PEI_PPI_DESCRIPTOR mTpmSelectedPpi = {
> (EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
> &gEfiTpmDeviceSelectedGuid,
> @@ -34,44 +34,6 @@ STATIC CONST EFI_PEI_PPI_DESCRIPTOR mTpmInitializationDonePpiList = {
> NULL
> };
>
> -#pragma pack (1)
> -
> -typedef struct {
> - TPM_RSP_COMMAND_HDR Hdr;
> - TPM_CURRENT_TICKS CurrentTicks;
> -} TPM_RSP_GET_TICKS;
> -
> -#pragma pack ()
> -
> -/**
> - Probe for the TPM for 1.2 version, by sending TPM1.2 GetTicks
> -
> - Sending a TPM1.2 command to a TPM2 should return a TPM1.2
> - header (tag = 0xc4) and error code (TPM_BADTAG = 0x1e)
> -**/
> -static
> -EFI_STATUS
> -TestTpm12 (
> - )
> -{
> - EFI_STATUS Status;
> - TPM_RQU_COMMAND_HDR Command;
> - TPM_RSP_GET_TICKS Response;
> - UINT32 Length;
> -
> - Command.tag = SwapBytes16 (TPM_TAG_RQU_COMMAND);
> - Command.paramSize = SwapBytes32 (sizeof (Command));
> - Command.ordinal = SwapBytes32 (TPM_ORD_GetTicks);
> -
> - Length = sizeof (Response);
> - Status = Tpm12SubmitCommand (sizeof (Command), (UINT8 *)&Command, &Length, (UINT8 *)&Response);
> - if (EFI_ERROR (Status)) {
> - return Status;
> - }
> -
> - return EFI_SUCCESS;
> -}
> -
> /**
> The entry point for Tcg2 configuration driver.
>
> @@ -90,8 +52,8 @@ Tcg2ConfigPeimEntryPoint (
>
> DEBUG ((DEBUG_INFO, "%a\n", __FUNCTION__));
>
> - Status = Tpm12RequestUseTpm ();
> - if (!EFI_ERROR (Status) && !EFI_ERROR (TestTpm12 ())) {
> + Status = InternalTpm12Detect ();
> + if (!EFI_ERROR (Status)) {
> DEBUG ((DEBUG_INFO, "%a: TPM1.2 detected\n", __FUNCTION__));
> Size = sizeof (gEfiTpmDeviceInstanceTpm12Guid);
> Status = PcdSetPtrS (
> diff --git a/OvmfPkg/Tcg/Tcg2Config/Tpm12Support.c b/OvmfPkg/Tcg/Tcg2Config/Tpm12Support.c
> new file mode 100644
> index 000000000000..4f5a775c7a03
> --- /dev/null
> +++ b/OvmfPkg/Tcg/Tcg2Config/Tpm12Support.c
> @@ -0,0 +1,79 @@
> +/** @file
> + Implement the InternalTpm12Detect() function on top of the Tpm12DeviceLib
> + class.
> +
> + Copyright (C) 2020, Red Hat, Inc.
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#include <Library/BaseLib.h>
> +#include <Library/Tpm12DeviceLib.h>
> +
> +#include "Tpm12Support.h"
> +
> +#pragma pack (1)
> +typedef struct {
> + TPM_RSP_COMMAND_HDR Hdr;
> + TPM_CURRENT_TICKS CurrentTicks;
> +} TPM_RSP_GET_TICKS;
> +#pragma pack ()
> +
> +/**
> + Probe for the TPM for 1.2 version, by sending TPM1.2 GetTicks
> +
> + Sending a TPM1.2 command to a TPM2 should return a TPM1.2
> + header (tag = 0xc4) and error code (TPM_BADTAG = 0x1e)
> +
> + @retval EFI_SUCCESS TPM version 1.2 probing successful.
> +
> + @return Error codes propagated from Tpm12SubmitCommand().
> +**/
> +STATIC
> +EFI_STATUS
> +TestTpm12 (
> + )
> +{
> + EFI_STATUS Status;
> + TPM_RQU_COMMAND_HDR Command;
> + TPM_RSP_GET_TICKS Response;
> + UINT32 Length;
> +
> + Command.tag = SwapBytes16 (TPM_TAG_RQU_COMMAND);
> + Command.paramSize = SwapBytes32 (sizeof (Command));
> + Command.ordinal = SwapBytes32 (TPM_ORD_GetTicks);
> +
> + Length = sizeof (Response);
> + Status = Tpm12SubmitCommand (sizeof (Command), (UINT8 *)&Command, &Length,
> + (UINT8 *)&Response);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + return EFI_SUCCESS;
> +}
> +
> +/**
> + Detect the presence of a TPM with interface version 1.2.
> +
> + @retval EFI_SUCCESS TPM-1.2 available. The Tpm12RequestUseTpm() and
> + Tpm12SubmitCommand(TPM_ORD_GetTicks) operations
> + (from the Tpm12DeviceLib class) have succeeded.
> +
> + @return Error codes propagated from Tpm12RequestUseTpm() and
> + Tpm12SubmitCommand().
> +**/
> +EFI_STATUS
> +InternalTpm12Detect (
> + VOID
> + )
> +{
> + EFI_STATUS Status;
> +
> + Status = Tpm12RequestUseTpm ();
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + return TestTpm12 ();
> +}
>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/3] OvmfPkg/Tcg2ConfigPei: skip TPM-1.2 detection when building for ARM/AARCH64
2020-05-20 22:58 [PATCH 0/3] OvmfPkg/Tcg2ConfigPei: fix ARM/AARCH64 build failure Laszlo Ersek
2020-05-20 22:58 ` [PATCH 1/3] OvmfPkg/Tcg2ConfigPei: clean up some lib class dependencies Laszlo Ersek
2020-05-20 22:58 ` [PATCH 2/3] OvmfPkg/Tcg2ConfigPei: factor out InternalTpm12Detect() Laszlo Ersek
@ 2020-05-20 22:58 ` Laszlo Ersek
2020-05-21 10:34 ` Philippe Mathieu-Daudé
2020-05-21 8:26 ` [edk2-devel] [PATCH 0/3] OvmfPkg/Tcg2ConfigPei: fix ARM/AARCH64 build failure Laszlo Ersek
` (2 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Laszlo Ersek @ 2020-05-20 22:58 UTC (permalink / raw)
To: edk2-devel-groups-io
Cc: Ard Biesheuvel, Eric Auger, Jordan Justen, Marc-André Lureau,
Philippe Mathieu-Daudé, Simon Hardy, Stefan Berger
Dating back to commits f5cb3767038e and ddd34a818315d, the
"ArmVirtPkg/ArmVirtQemu.dsc" platform includes the
"OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf" module when the TPM2_ENABLE
build flag is defined.
This was regressed in commit 89236992913f, which added a Tpm12DeviceLib
dependency to Tcg2ConfigPei. "ArmVirtQemu.dsc" does not resolve that class
to any instance, so now we get a build failure:
> build.py...
> ArmVirtPkg/ArmVirtQemu.dsc(...): error 4000: Instance of library class
> [Tpm12DeviceLib] is not found
> in [OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf] [AARCH64]
> consumed by module [OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf]
The TPM-1.2 code in OvmfPkg/Tcg2ConfigPei is limited to a special use case
(a kind of physical TPM-1.2 assignment), and that has never applied to
"ArmVirtQemu.dsc".
Short-circuit the TPM-1.2 detection in the ARM/AARCH64 builds of
OvmfPkg/Tcg2ConfigPei, removing the Tpm12DeviceLib dependency.
Functionally, this patch is a no-op on IA32 / X64.
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 | 11 +++++++--
OvmfPkg/Tcg/Tcg2Config/Tpm12Support.h | 4 ++++
OvmfPkg/Tcg/Tcg2Config/Tpm12SupportNull.c | 25 ++++++++++++++++++++
3 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
index aa996b7da778..194ebfba6409 100644
--- a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
+++ b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
@@ -21,9 +21,14 @@ [Defines]
[Sources]
Tcg2ConfigPeim.c
- Tpm12Support.c
Tpm12Support.h
+[Sources.IA32, Sources.X64]
+ Tpm12Support.c
+
+[Sources.ARM, Sources.AARCH64]
+ Tpm12SupportNull.c
+
[Packages]
MdePkg/MdePkg.dec
MdeModulePkg/MdeModulePkg.dec
@@ -35,9 +40,11 @@ [LibraryClasses]
BaseLib
DebugLib
PeiServicesLib
- Tpm12DeviceLib
Tpm2DeviceLib
+[LibraryClasses.IA32, LibraryClasses.X64]
+ Tpm12DeviceLib
+
[Guids]
gEfiTpmDeviceSelectedGuid ## PRODUCES ## GUID # Used as a PPI GUID
gEfiTpmDeviceInstanceTpm20DtpmGuid ## SOMETIMES_CONSUMES
diff --git a/OvmfPkg/Tcg/Tcg2Config/Tpm12Support.h b/OvmfPkg/Tcg/Tcg2Config/Tpm12Support.h
index c739775d2353..d92c43253081 100644
--- a/OvmfPkg/Tcg/Tcg2Config/Tpm12Support.h
+++ b/OvmfPkg/Tcg/Tcg2Config/Tpm12Support.h
@@ -15,6 +15,10 @@
/**
Detect the presence of a TPM with interface version 1.2.
+ @retval EFI_UNSUPPORTED The platform that includes this particular
+ implementation of the function does not support
+ TPM-1.2.
+
@retval EFI_SUCCESS TPM-1.2 available. The Tpm12RequestUseTpm() and
Tpm12SubmitCommand(TPM_ORD_GetTicks) operations
(from the Tpm12DeviceLib class) have succeeded.
diff --git a/OvmfPkg/Tcg/Tcg2Config/Tpm12SupportNull.c b/OvmfPkg/Tcg/Tcg2Config/Tpm12SupportNull.c
new file mode 100644
index 000000000000..7bb377b9b9b0
--- /dev/null
+++ b/OvmfPkg/Tcg/Tcg2Config/Tpm12SupportNull.c
@@ -0,0 +1,25 @@
+/** @file
+ Null implementation of InternalTpm12Detect(), always returning
+ EFI_UNSUPPORTED.
+
+ Copyright (C) 2020, Red Hat, Inc.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include "Tpm12Support.h"
+
+/**
+ Detect the presence of a TPM with interface version 1.2.
+
+ @retval EFI_UNSUPPORTED The platform that includes this particular
+ implementation of the function does not support
+ TPM-1.2.
+**/
+EFI_STATUS
+InternalTpm12Detect (
+ VOID
+ )
+{
+ return EFI_UNSUPPORTED;
+}
--
2.19.1.3.g30247aa5d201
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 3/3] OvmfPkg/Tcg2ConfigPei: skip TPM-1.2 detection when building for ARM/AARCH64
2020-05-20 22:58 ` [PATCH 3/3] OvmfPkg/Tcg2ConfigPei: skip TPM-1.2 detection when building for ARM/AARCH64 Laszlo Ersek
@ 2020-05-21 10:34 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-05-21 10:34 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-groups-io
Cc: Ard Biesheuvel, Eric Auger, Jordan Justen, Marc-André Lureau,
Simon Hardy, Stefan Berger
On 5/21/20 12:58 AM, Laszlo Ersek wrote:
> Dating back to commits f5cb3767038e and ddd34a818315d, the
> "ArmVirtPkg/ArmVirtQemu.dsc" platform includes the
> "OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf" module when the TPM2_ENABLE
> build flag is defined.
>
> This was regressed in commit 89236992913f, which added a Tpm12DeviceLib
> dependency to Tcg2ConfigPei. "ArmVirtQemu.dsc" does not resolve that class
> to any instance, so now we get a build failure:
>
>> build.py...
>> ArmVirtPkg/ArmVirtQemu.dsc(...): error 4000: Instance of library class
>> [Tpm12DeviceLib] is not found
>> in [OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf] [AARCH64]
>> consumed by module [OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf]
>
> The TPM-1.2 code in OvmfPkg/Tcg2ConfigPei is limited to a special use case
> (a kind of physical TPM-1.2 assignment), and that has never applied to
> "ArmVirtQemu.dsc".
>
> Short-circuit the TPM-1.2 detection in the ARM/AARCH64 builds of
> OvmfPkg/Tcg2ConfigPei, removing the Tpm12DeviceLib dependency.
>
> Functionally, this patch is a no-op on IA32 / X64.
>
> 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 | 11 +++++++--
> OvmfPkg/Tcg/Tcg2Config/Tpm12Support.h | 4 ++++
> OvmfPkg/Tcg/Tcg2Config/Tpm12SupportNull.c | 25 ++++++++++++++++++++
> 3 files changed, 38 insertions(+), 2 deletions(-)
>
> diff --git a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> index aa996b7da778..194ebfba6409 100644
> --- a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> +++ b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> @@ -21,9 +21,14 @@ [Defines]
>
> [Sources]
> Tcg2ConfigPeim.c
> - Tpm12Support.c
> Tpm12Support.h
>
> +[Sources.IA32, Sources.X64]
> + Tpm12Support.c
> +
> +[Sources.ARM, Sources.AARCH64]
> + Tpm12SupportNull.c
> +
> [Packages]
> MdePkg/MdePkg.dec
> MdeModulePkg/MdeModulePkg.dec
> @@ -35,9 +40,11 @@ [LibraryClasses]
> BaseLib
> DebugLib
> PeiServicesLib
> - Tpm12DeviceLib
> Tpm2DeviceLib
>
> +[LibraryClasses.IA32, LibraryClasses.X64]
> + Tpm12DeviceLib
> +
> [Guids]
> gEfiTpmDeviceSelectedGuid ## PRODUCES ## GUID # Used as a PPI GUID
> gEfiTpmDeviceInstanceTpm20DtpmGuid ## SOMETIMES_CONSUMES
> diff --git a/OvmfPkg/Tcg/Tcg2Config/Tpm12Support.h b/OvmfPkg/Tcg/Tcg2Config/Tpm12Support.h
> index c739775d2353..d92c43253081 100644
> --- a/OvmfPkg/Tcg/Tcg2Config/Tpm12Support.h
> +++ b/OvmfPkg/Tcg/Tcg2Config/Tpm12Support.h
> @@ -15,6 +15,10 @@
> /**
> Detect the presence of a TPM with interface version 1.2.
>
> + @retval EFI_UNSUPPORTED The platform that includes this particular
> + implementation of the function does not support
> + TPM-1.2.
> +
> @retval EFI_SUCCESS TPM-1.2 available. The Tpm12RequestUseTpm() and
> Tpm12SubmitCommand(TPM_ORD_GetTicks) operations
> (from the Tpm12DeviceLib class) have succeeded.
> diff --git a/OvmfPkg/Tcg/Tcg2Config/Tpm12SupportNull.c b/OvmfPkg/Tcg/Tcg2Config/Tpm12SupportNull.c
> new file mode 100644
> index 000000000000..7bb377b9b9b0
> --- /dev/null
> +++ b/OvmfPkg/Tcg/Tcg2Config/Tpm12SupportNull.c
> @@ -0,0 +1,25 @@
> +/** @file
> + Null implementation of InternalTpm12Detect(), always returning
> + EFI_UNSUPPORTED.
> +
> + Copyright (C) 2020, Red Hat, Inc.
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#include "Tpm12Support.h"
> +
> +/**
> + Detect the presence of a TPM with interface version 1.2.
> +
> + @retval EFI_UNSUPPORTED The platform that includes this particular
> + implementation of the function does not support
> + TPM-1.2.
> +**/
> +EFI_STATUS
> +InternalTpm12Detect (
> + VOID
> + )
> +{
> + return EFI_UNSUPPORTED;
> +}
>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [edk2-devel] [PATCH 0/3] OvmfPkg/Tcg2ConfigPei: fix ARM/AARCH64 build failure
2020-05-20 22:58 [PATCH 0/3] OvmfPkg/Tcg2ConfigPei: fix ARM/AARCH64 build failure Laszlo Ersek
` (2 preceding siblings ...)
2020-05-20 22:58 ` [PATCH 3/3] OvmfPkg/Tcg2ConfigPei: skip TPM-1.2 detection when building for ARM/AARCH64 Laszlo Ersek
@ 2020-05-21 8:26 ` Laszlo Ersek
2020-05-21 9:12 ` Laszlo Ersek
2020-05-21 8:37 ` Ard Biesheuvel
2020-05-21 12:26 ` [edk2-devel] " Laszlo Ersek
5 siblings, 1 reply; 15+ messages in thread
From: Laszlo Ersek @ 2020-05-21 8:26 UTC (permalink / raw)
To: edk2-devel-groups-io
Cc: Ard Biesheuvel, Eric Auger, Jordan Justen, Marc-André Lureau,
Philippe Mathieu-Daudé, Simon Hardy, Stefan Berger
On 05/21/20 00:58, Laszlo Ersek wrote:
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2728
> Repo: https://pagure.io/lersek/edk2.git
> Branch: restrict_tpm12_to_x86_bz_2728
>
> Another regression fix for edk2-stable202005.
>
> End of February 2020, Ard and Marc-André worked on two TPM-related
> features in parallel. Respectively:
>
> - [edk2-devel] [PATCH v4 00/11]
> ArmVirtPkg: implement measured boot for ArmVirtQemu
>
> http://mid.mail-archive.com/20200227144056.56988-1-ard.biesheuvel@linaro.org
> https://edk2.groups.io/g/devel/message/55004
>
> - [edk2-devel] [PATCH v4 0/5]
> Ovmf: enable TPM 1.2
>
> http://mid.mail-archive.com/20200226152433.1295789-1-marcandre.lureau@redhat.com
> https://edk2.groups.io/g/devel/message/54894
>
> Both series were merged tightly one after the other. There was no merge
> conflict, and standing alone (without rebasing one on the other), each
> series was self-contained and correct. Their combination however led to
> an ArmVirtQemu build regression. There never was an intent to support
> TPM-1.2 in ArmVirtQemu, but the TPM-1.2 series for OVMF kind of made
> that "mandatory".
>
> Worse, the build regression has remained hidden for 2+ months because
> (a) I didn't expect Marc-André's series to affect any ArmVirtPkg
> platform, (b) my ArmVirtQemu build script did not set TPM2_ENABLE.
>
> This series fixes the build regression, and intends no functional
> changes at all.
>
> Functional regression-testing would be appreciated:
>
> - from Simon regarding their TPM-1.2 passthrough use case,
>
> - from Marc-André regarding vTPM-2.0 on X64,
>
> - from Eric regarding vTPM-2.0 on AARCH64.
>
> This is a regression fix, therefore it is eligible for merging during
> the edk2-stable202005 Hard Feature Freeze too
> <https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning>.
>
> If you plan to regression-test this series, then please say so soon,
> otherwise I wouldn't like to wait for long -- assuming an R-b from Ard
> or Jordan -- even without Tested-by's.
>
> In the future we should likely set some "-D" flags somewhere under
> "ArmVirtPkg/PlatformCI/" (so that our CI coverage grow). The best I can
> personally do about that is maybe file a BZ?...
>
> 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>
>
> Thanks,
> Laszlo
>
> Laszlo Ersek (3):
> OvmfPkg/Tcg2ConfigPei: clean up some lib class dependencies
> OvmfPkg/Tcg2ConfigPei: factor out InternalTpm12Detect()
> OvmfPkg/Tcg2ConfigPei: skip TPM-1.2 detection when building for
> ARM/AARCH64
>
> OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf | 13 +++-
> OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c | 46 +-----------
> OvmfPkg/Tcg/Tcg2Config/Tpm12Support.c | 79 ++++++++++++++++++++
> OvmfPkg/Tcg/Tcg2Config/Tpm12Support.h | 34 +++++++++
> OvmfPkg/Tcg/Tcg2Config/Tpm12SupportNull.c | 25 +++++++
> 5 files changed, 153 insertions(+), 44 deletions(-)
> create mode 100644 OvmfPkg/Tcg/Tcg2Config/Tpm12Support.c
> create mode 100644 OvmfPkg/Tcg/Tcg2Config/Tpm12Support.h
> create mode 100644 OvmfPkg/Tcg/Tcg2Config/Tpm12SupportNull.c
>
It seems like noone has started reviewing / testing yet. That's good:
Nacked-by: Laszlo Ersek <lersek@redhat.com>
I'll send a v2 with the following changes:
- the first patch will also clean up some comments that are now stale
(after the TPM-1.2 addition)
- the last patch will restrict the BaseLib dependency as well to IA32/X64
Thanks
Laszlo
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [edk2-devel] [PATCH 0/3] OvmfPkg/Tcg2ConfigPei: fix ARM/AARCH64 build failure
2020-05-21 8:26 ` [edk2-devel] [PATCH 0/3] OvmfPkg/Tcg2ConfigPei: fix ARM/AARCH64 build failure Laszlo Ersek
@ 2020-05-21 9:12 ` Laszlo Ersek
2020-05-21 11:07 ` Laszlo Ersek
0 siblings, 1 reply; 15+ messages in thread
From: Laszlo Ersek @ 2020-05-21 9:12 UTC (permalink / raw)
To: edk2-devel-groups-io
Cc: Ard Biesheuvel, Eric Auger, Jordan Justen, Marc-André Lureau,
Philippe Mathieu-Daudé, Simon Hardy, Stefan Berger
On 05/21/20 10:26, Laszlo Ersek wrote:
> On 05/21/20 00:58, Laszlo Ersek wrote:
>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2728
>> Repo: https://pagure.io/lersek/edk2.git
>> Branch: restrict_tpm12_to_x86_bz_2728
>>
>> Another regression fix for edk2-stable202005.
>>
>> End of February 2020, Ard and Marc-André worked on two TPM-related
>> features in parallel. Respectively:
>>
>> - [edk2-devel] [PATCH v4 00/11]
>> ArmVirtPkg: implement measured boot for ArmVirtQemu
>>
>> http://mid.mail-archive.com/20200227144056.56988-1-ard.biesheuvel@linaro.org
>> https://edk2.groups.io/g/devel/message/55004
>>
>> - [edk2-devel] [PATCH v4 0/5]
>> Ovmf: enable TPM 1.2
>>
>> http://mid.mail-archive.com/20200226152433.1295789-1-marcandre.lureau@redhat.com
>> https://edk2.groups.io/g/devel/message/54894
>>
>> Both series were merged tightly one after the other. There was no merge
>> conflict, and standing alone (without rebasing one on the other), each
>> series was self-contained and correct. Their combination however led to
>> an ArmVirtQemu build regression. There never was an intent to support
>> TPM-1.2 in ArmVirtQemu, but the TPM-1.2 series for OVMF kind of made
>> that "mandatory".
>>
>> Worse, the build regression has remained hidden for 2+ months because
>> (a) I didn't expect Marc-André's series to affect any ArmVirtPkg
>> platform, (b) my ArmVirtQemu build script did not set TPM2_ENABLE.
>>
>> This series fixes the build regression, and intends no functional
>> changes at all.
>>
>> Functional regression-testing would be appreciated:
>>
>> - from Simon regarding their TPM-1.2 passthrough use case,
>>
>> - from Marc-André regarding vTPM-2.0 on X64,
>>
>> - from Eric regarding vTPM-2.0 on AARCH64.
>>
>> This is a regression fix, therefore it is eligible for merging during
>> the edk2-stable202005 Hard Feature Freeze too
>> <https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning>.
>>
>> If you plan to regression-test this series, then please say so soon,
>> otherwise I wouldn't like to wait for long -- assuming an R-b from Ard
>> or Jordan -- even without Tested-by's.
>>
>> In the future we should likely set some "-D" flags somewhere under
>> "ArmVirtPkg/PlatformCI/" (so that our CI coverage grow). The best I can
>> personally do about that is maybe file a BZ?...
>>
>> 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>
>>
>> Thanks,
>> Laszlo
>>
>> Laszlo Ersek (3):
>> OvmfPkg/Tcg2ConfigPei: clean up some lib class dependencies
>> OvmfPkg/Tcg2ConfigPei: factor out InternalTpm12Detect()
>> OvmfPkg/Tcg2ConfigPei: skip TPM-1.2 detection when building for
>> ARM/AARCH64
>>
>> OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf | 13 +++-
>> OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c | 46 +-----------
>> OvmfPkg/Tcg/Tcg2Config/Tpm12Support.c | 79 ++++++++++++++++++++
>> OvmfPkg/Tcg/Tcg2Config/Tpm12Support.h | 34 +++++++++
>> OvmfPkg/Tcg/Tcg2Config/Tpm12SupportNull.c | 25 +++++++
>> 5 files changed, 153 insertions(+), 44 deletions(-)
>> create mode 100644 OvmfPkg/Tcg/Tcg2Config/Tpm12Support.c
>> create mode 100644 OvmfPkg/Tcg/Tcg2Config/Tpm12Support.h
>> create mode 100644 OvmfPkg/Tcg/Tcg2Config/Tpm12SupportNull.c
>>
>
> It seems like noone has started reviewing / testing yet. That's good:
>
> Nacked-by: Laszlo Ersek <lersek@redhat.com>
>
> I'll send a v2 with the following changes:
>
> - the first patch will also clean up some comments that are now stale
> (after the TPM-1.2 addition)
>
> - the last patch will restrict the BaseLib dependency as well to IA32/X64
I'm rescinding my self-NAK in light of Ard's Tested-by; the latter is
something I won't waste.
The above-listed v2 updates can perfectly well wait until after the
stable tag.
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [edk2-devel] [PATCH 0/3] OvmfPkg/Tcg2ConfigPei: fix ARM/AARCH64 build failure
2020-05-21 9:12 ` Laszlo Ersek
@ 2020-05-21 11:07 ` Laszlo Ersek
0 siblings, 0 replies; 15+ messages in thread
From: Laszlo Ersek @ 2020-05-21 11:07 UTC (permalink / raw)
To: edk2-devel-groups-io
Cc: Ard Biesheuvel, Eric Auger, Jordan Justen, Marc-André Lureau,
Philippe Mathieu-Daudé, Simon Hardy, Stefan Berger
On 05/21/20 11:12, Laszlo Ersek wrote:
> On 05/21/20 10:26, Laszlo Ersek wrote:
>> On 05/21/20 00:58, Laszlo Ersek wrote:
>>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2728
>>> Repo: https://pagure.io/lersek/edk2.git
>>> Branch: restrict_tpm12_to_x86_bz_2728
>>>
>>> Another regression fix for edk2-stable202005.
>>>
>>> End of February 2020, Ard and Marc-André worked on two TPM-related
>>> features in parallel. Respectively:
>>>
>>> - [edk2-devel] [PATCH v4 00/11]
>>> ArmVirtPkg: implement measured boot for ArmVirtQemu
>>>
>>> http://mid.mail-archive.com/20200227144056.56988-1-ard.biesheuvel@linaro.org
>>> https://edk2.groups.io/g/devel/message/55004
>>>
>>> - [edk2-devel] [PATCH v4 0/5]
>>> Ovmf: enable TPM 1.2
>>>
>>> http://mid.mail-archive.com/20200226152433.1295789-1-marcandre.lureau@redhat.com
>>> https://edk2.groups.io/g/devel/message/54894
>>>
>>> Both series were merged tightly one after the other. There was no merge
>>> conflict, and standing alone (without rebasing one on the other), each
>>> series was self-contained and correct. Their combination however led to
>>> an ArmVirtQemu build regression. There never was an intent to support
>>> TPM-1.2 in ArmVirtQemu, but the TPM-1.2 series for OVMF kind of made
>>> that "mandatory".
>>>
>>> Worse, the build regression has remained hidden for 2+ months because
>>> (a) I didn't expect Marc-André's series to affect any ArmVirtPkg
>>> platform, (b) my ArmVirtQemu build script did not set TPM2_ENABLE.
>>>
>>> This series fixes the build regression, and intends no functional
>>> changes at all.
>>>
>>> Functional regression-testing would be appreciated:
>>>
>>> - from Simon regarding their TPM-1.2 passthrough use case,
>>>
>>> - from Marc-André regarding vTPM-2.0 on X64,
>>>
>>> - from Eric regarding vTPM-2.0 on AARCH64.
>>>
>>> This is a regression fix, therefore it is eligible for merging during
>>> the edk2-stable202005 Hard Feature Freeze too
>>> <https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning>.
>>>
>>> If you plan to regression-test this series, then please say so soon,
>>> otherwise I wouldn't like to wait for long -- assuming an R-b from Ard
>>> or Jordan -- even without Tested-by's.
>>>
>>> In the future we should likely set some "-D" flags somewhere under
>>> "ArmVirtPkg/PlatformCI/" (so that our CI coverage grow). The best I can
>>> personally do about that is maybe file a BZ?...
>>>
>>> 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>
>>>
>>> Thanks,
>>> Laszlo
>>>
>>> Laszlo Ersek (3):
>>> OvmfPkg/Tcg2ConfigPei: clean up some lib class dependencies
>>> OvmfPkg/Tcg2ConfigPei: factor out InternalTpm12Detect()
>>> OvmfPkg/Tcg2ConfigPei: skip TPM-1.2 detection when building for
>>> ARM/AARCH64
>>>
>>> OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf | 13 +++-
>>> OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c | 46 +-----------
>>> OvmfPkg/Tcg/Tcg2Config/Tpm12Support.c | 79 ++++++++++++++++++++
>>> OvmfPkg/Tcg/Tcg2Config/Tpm12Support.h | 34 +++++++++
>>> OvmfPkg/Tcg/Tcg2Config/Tpm12SupportNull.c | 25 +++++++
>>> 5 files changed, 153 insertions(+), 44 deletions(-)
>>> create mode 100644 OvmfPkg/Tcg/Tcg2Config/Tpm12Support.c
>>> create mode 100644 OvmfPkg/Tcg/Tcg2Config/Tpm12Support.h
>>> create mode 100644 OvmfPkg/Tcg/Tcg2Config/Tpm12SupportNull.c
>>>
>>
>> It seems like noone has started reviewing / testing yet. That's good:
>>
>> Nacked-by: Laszlo Ersek <lersek@redhat.com>
>>
>> I'll send a v2 with the following changes:
>>
>> - the first patch will also clean up some comments that are now stale
>> (after the TPM-1.2 addition)
>>
>> - the last patch will restrict the BaseLib dependency as well to IA32/X64
>
> I'm rescinding my self-NAK in light of Ard's Tested-by; the latter is
> something I won't waste.
>
> The above-listed v2 updates can perfectly well wait until after the
> stable tag.
Right; I'm going to merge this series soon, with the R-b and T-b from
Ard picked up. I've learned that:
- Simon and Marc-André are both out of office at this time (auto-reply
from Simon, independent message regarding Marc-André),
- Ard covered the testing on AARCH64 (thanks again!), so no need to
additionally burden Eric with that.
I've now filed the followup BZ (for after the edk2-stable202005 tag, and
dependent on TianoCore#2728 / this series):
https://bugzilla.tianocore.org/show_bug.cgi?id=2752
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/3] OvmfPkg/Tcg2ConfigPei: fix ARM/AARCH64 build failure
2020-05-20 22:58 [PATCH 0/3] OvmfPkg/Tcg2ConfigPei: fix ARM/AARCH64 build failure Laszlo Ersek
` (3 preceding siblings ...)
2020-05-21 8:26 ` [edk2-devel] [PATCH 0/3] OvmfPkg/Tcg2ConfigPei: fix ARM/AARCH64 build failure Laszlo Ersek
@ 2020-05-21 8:37 ` Ard Biesheuvel
2020-05-21 9:08 ` Laszlo Ersek
2020-05-21 12:26 ` [edk2-devel] " Laszlo Ersek
5 siblings, 1 reply; 15+ messages in thread
From: Ard Biesheuvel @ 2020-05-21 8:37 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-groups-io
Cc: Eric Auger, Jordan Justen, Marc-André Lureau,
Philippe Mathieu-Daudé, Simon Hardy, Stefan Berger
On 5/21/20 12:58 AM, Laszlo Ersek wrote:
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2728
> Repo: https://pagure.io/lersek/edk2.git
> Branch: restrict_tpm12_to_x86_bz_2728
>
> Another regression fix for edk2-stable202005.
>
> End of February 2020, Ard and Marc-André worked on two TPM-related
> features in parallel. Respectively:
>
> - [edk2-devel] [PATCH v4 00/11]
> ArmVirtPkg: implement measured boot for ArmVirtQemu
>
> http://mid.mail-archive.com/20200227144056.56988-1-ard.biesheuvel@linaro.org
> https://edk2.groups.io/g/devel/message/55004
>
> - [edk2-devel] [PATCH v4 0/5]
> Ovmf: enable TPM 1.2
>
> http://mid.mail-archive.com/20200226152433.1295789-1-marcandre.lureau@redhat.com
> https://edk2.groups.io/g/devel/message/54894
>
> Both series were merged tightly one after the other. There was no merge
> conflict, and standing alone (without rebasing one on the other), each
> series was self-contained and correct. Their combination however led to
> an ArmVirtQemu build regression. There never was an intent to support
> TPM-1.2 in ArmVirtQemu, but the TPM-1.2 series for OVMF kind of made
> that "mandatory".
>
> Worse, the build regression has remained hidden for 2+ months because
> (a) I didn't expect Marc-André's series to affect any ArmVirtPkg
> platform, (b) my ArmVirtQemu build script did not set TPM2_ENABLE.
>
> This series fixes the build regression, and intends no functional
> changes at all.
>
> Functional regression-testing would be appreciated:
>
> - from Simon regarding their TPM-1.2 passthrough use case,
>
> - from Marc-André regarding vTPM-2.0 on X64,
>
> - from Eric regarding vTPM-2.0 on AARCH64.
>
> This is a regression fix, therefore it is eligible for merging during
> the edk2-stable202005 Hard Feature Freeze too
> <https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning>.
>
> If you plan to regression-test this series, then please say so soon,
> otherwise I wouldn't like to wait for long -- assuming an R-b from Ard
> or Jordan -- even without Tested-by's.
>
> In the future we should likely set some "-D" flags somewhere under
> "ArmVirtPkg/PlatformCI/" (so that our CI coverage grow). The best I can
> personally do about that is maybe file a BZ?...
>
Thanks a lot for fixing this. I know TPM support is not a feature you
care deeply about.
For the series,
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
Tested-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
(on AArch64 only)
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 0/3] OvmfPkg/Tcg2ConfigPei: fix ARM/AARCH64 build failure
2020-05-21 8:37 ` Ard Biesheuvel
@ 2020-05-21 9:08 ` Laszlo Ersek
0 siblings, 0 replies; 15+ messages in thread
From: Laszlo Ersek @ 2020-05-21 9:08 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel-groups-io
Cc: Eric Auger, Jordan Justen, Marc-André Lureau,
Philippe Mathieu-Daudé, Simon Hardy, Stefan Berger
On 05/21/20 10:37, Ard Biesheuvel wrote:
> On 5/21/20 12:58 AM, Laszlo Ersek wrote:
>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2728
>> Repo: https://pagure.io/lersek/edk2.git
>> Branch: restrict_tpm12_to_x86_bz_2728
>>
>> Another regression fix for edk2-stable202005.
>>
>> End of February 2020, Ard and Marc-André worked on two TPM-related
>> features in parallel. Respectively:
>>
>> - [edk2-devel] [PATCH v4 00/11]
>> ArmVirtPkg: implement measured boot for ArmVirtQemu
>>
>>
>> http://mid.mail-archive.com/20200227144056.56988-1-ard.biesheuvel@linaro.org
>>
>> https://edk2.groups.io/g/devel/message/55004
>>
>> - [edk2-devel] [PATCH v4 0/5]
>> Ovmf: enable TPM 1.2
>>
>>
>> http://mid.mail-archive.com/20200226152433.1295789-1-marcandre.lureau@redhat.com
>>
>> https://edk2.groups.io/g/devel/message/54894
>>
>> Both series were merged tightly one after the other. There was no merge
>> conflict, and standing alone (without rebasing one on the other), each
>> series was self-contained and correct. Their combination however led to
>> an ArmVirtQemu build regression. There never was an intent to support
>> TPM-1.2 in ArmVirtQemu, but the TPM-1.2 series for OVMF kind of made
>> that "mandatory".
>>
>> Worse, the build regression has remained hidden for 2+ months because
>> (a) I didn't expect Marc-André's series to affect any ArmVirtPkg
>> platform, (b) my ArmVirtQemu build script did not set TPM2_ENABLE.
>>
>> This series fixes the build regression, and intends no functional
>> changes at all.
>>
>> Functional regression-testing would be appreciated:
>>
>> - from Simon regarding their TPM-1.2 passthrough use case,
>>
>> - from Marc-André regarding vTPM-2.0 on X64,
>>
>> - from Eric regarding vTPM-2.0 on AARCH64.
>>
>> This is a regression fix, therefore it is eligible for merging during
>> the edk2-stable202005 Hard Feature Freeze too
>> <https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning>.
>>
>>
>> If you plan to regression-test this series, then please say so soon,
>> otherwise I wouldn't like to wait for long -- assuming an R-b from Ard
>> or Jordan -- even without Tested-by's.
>>
>> In the future we should likely set some "-D" flags somewhere under
>> "ArmVirtPkg/PlatformCI/" (so that our CI coverage grow). The best I can
>> personally do about that is maybe file a BZ?...
>>
>
> Thanks a lot for fixing this. I know TPM support is not a feature you
> care deeply about.
>
> For the series,
>
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Tested-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
>
> (on AArch64 only)
>
Thanks!
In that case, I will *not* send a v2. The updates I'd implement in v2
are not important enough to invalidate your testing. I'll send those
small fixups after the stable tag is made.
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [edk2-devel] [PATCH 0/3] OvmfPkg/Tcg2ConfigPei: fix ARM/AARCH64 build failure
2020-05-20 22:58 [PATCH 0/3] OvmfPkg/Tcg2ConfigPei: fix ARM/AARCH64 build failure Laszlo Ersek
` (4 preceding siblings ...)
2020-05-21 8:37 ` Ard Biesheuvel
@ 2020-05-21 12:26 ` Laszlo Ersek
5 siblings, 0 replies; 15+ messages in thread
From: Laszlo Ersek @ 2020-05-21 12:26 UTC (permalink / raw)
To: edk2-devel-groups-io
Cc: Ard Biesheuvel, Eric Auger, Jordan Justen, Marc-André Lureau,
Philippe Mathieu-Daudé, Simon Hardy, Stefan Berger
On 05/21/20 00:58, Laszlo Ersek wrote:
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2728
> Repo: https://pagure.io/lersek/edk2.git
> Branch: restrict_tpm12_to_x86_bz_2728
>
> Another regression fix for edk2-stable202005.
>
> End of February 2020, Ard and Marc-André worked on two TPM-related
> features in parallel. Respectively:
>
> - [edk2-devel] [PATCH v4 00/11]
> ArmVirtPkg: implement measured boot for ArmVirtQemu
>
> http://mid.mail-archive.com/20200227144056.56988-1-ard.biesheuvel@linaro.org
> https://edk2.groups.io/g/devel/message/55004
>
> - [edk2-devel] [PATCH v4 0/5]
> Ovmf: enable TPM 1.2
>
> http://mid.mail-archive.com/20200226152433.1295789-1-marcandre.lureau@redhat.com
> https://edk2.groups.io/g/devel/message/54894
>
> Both series were merged tightly one after the other. There was no merge
> conflict, and standing alone (without rebasing one on the other), each
> series was self-contained and correct. Their combination however led to
> an ArmVirtQemu build regression. There never was an intent to support
> TPM-1.2 in ArmVirtQemu, but the TPM-1.2 series for OVMF kind of made
> that "mandatory".
>
> Worse, the build regression has remained hidden for 2+ months because
> (a) I didn't expect Marc-André's series to affect any ArmVirtPkg
> platform, (b) my ArmVirtQemu build script did not set TPM2_ENABLE.
>
> This series fixes the build regression, and intends no functional
> changes at all.
>
> Functional regression-testing would be appreciated:
>
> - from Simon regarding their TPM-1.2 passthrough use case,
>
> - from Marc-André regarding vTPM-2.0 on X64,
>
> - from Eric regarding vTPM-2.0 on AARCH64.
>
> This is a regression fix, therefore it is eligible for merging during
> the edk2-stable202005 Hard Feature Freeze too
> <https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning>.
>
> If you plan to regression-test this series, then please say so soon,
> otherwise I wouldn't like to wait for long -- assuming an R-b from Ard
> or Jordan -- even without Tested-by's.
>
> In the future we should likely set some "-D" flags somewhere under
> "ArmVirtPkg/PlatformCI/" (so that our CI coverage grow). The best I can
> personally do about that is maybe file a BZ?...
>
> 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>
>
> Thanks,
> Laszlo
>
> Laszlo Ersek (3):
> OvmfPkg/Tcg2ConfigPei: clean up some lib class dependencies
> OvmfPkg/Tcg2ConfigPei: factor out InternalTpm12Detect()
> OvmfPkg/Tcg2ConfigPei: skip TPM-1.2 detection when building for
> ARM/AARCH64
>
> OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf | 13 +++-
> OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c | 46 +-----------
> OvmfPkg/Tcg/Tcg2Config/Tpm12Support.c | 79 ++++++++++++++++++++
> OvmfPkg/Tcg/Tcg2Config/Tpm12Support.h | 34 +++++++++
> OvmfPkg/Tcg/Tcg2Config/Tpm12SupportNull.c | 25 +++++++
> 5 files changed, 153 insertions(+), 44 deletions(-)
> create mode 100644 OvmfPkg/Tcg/Tcg2Config/Tpm12Support.c
> create mode 100644 OvmfPkg/Tcg/Tcg2Config/Tpm12Support.h
> create mode 100644 OvmfPkg/Tcg/Tcg2Config/Tpm12SupportNull.c
>
Merged in commit range 3f89db869028..74f90d38c446, via
<https://github.com/tianocore/edk2/pull/639>.
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 15+ messages in thread