public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Yao, Jiewen" <jiewen.yao@intel.com>
To: "Poosapalli, Karunakar" <KarunakarPoosapalli@Dell.com>,
	"Gao, Liming" <gaoliming@byosoft.com.cn>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Yao, Jiewen" <jiewen.yao@intel.com>
Subject: Re: [edk2-devel] [PATCH] SecurityPkg: DxeTcg2PhysicalPresenceLib for Clear Graphics Screen To unblock and Display TPM messages
Date: Tue, 8 Aug 2023 23:13:31 +0000	[thread overview]
Message-ID: <MW4PR11MB5872A550D98A4DF5E7CB42318C0DA@MW4PR11MB5872.namprd11.prod.outlook.com> (raw)
In-Reply-To: <SA1PR19MB55727006B83F295203AC3E858B0DA@SA1PR19MB5572.namprd19.prod.outlook.com>

[-- Attachment #1: Type: text/plain, Size: 10232 bytes --]

My concern is that you unconditionally clear the screen. What if someone did want to show something on the screen?

This seems an incompatible change. That is why I think it should be a platform policy.

Another way is that you may consider to duplicate the library for your platform and clear it for your platform.

Thank you
Yao, Jiewen


From: Poosapalli, Karunakar <KarunakarPoosapalli@Dell.com>
Sent: Wednesday, August 9, 2023 2:13 AM
To: Yao, Jiewen <jiewen.yao@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io; Poosapalli, Karunakar <KarunakarPoosapalli@Dell.com>
Subject: RE: [edk2-devel] [PATCH] SecurityPkg: DxeTcg2PhysicalPresenceLib for Clear Graphics Screen To unblock and Display TPM messages

Hi @Yao, Jiewen<mailto:jiewen.yao@intel.com>,

Thanks for your review and feedback.
When there is a pending Tcg request, the control will be in Tcg2 library and it will not reach to BDS until user press the input key.
As there is no information the screen because of screen is blocked by some other messages/Logo.
So user won't able to press the key without any user information and control will not reach to BDS phase.

As TCG user confirmation is the highest priority and it blocks the POST until the user presses the input key. Before TCG messages Print on the console, there should be logic added to clear the graphics screen

Please let me know your thoughts.

Thanks & Regards
Karunakar Poosapalli
Firmware Principal Engineer, Client BIOS
Customer BIOS | Dell Core BIOS
CPG Software Engineering | Dell Technologies
Mobile +91 9951902957
Karunakar_poosapalli@Dell.Com<mailto:Karunakar_poosapalli@Dell.Com>



Internal Use - Confidential
From: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
Sent: Tuesday, July 25, 2023 5:48 PM
To: Poosapalli, Karunakar; Gao, Liming; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Subject: RE: [edk2-devel] [PATCH] SecurityPkg: DxeTcg2PhysicalPresenceLib for Clear Graphics Screen To unblock and Display TPM messages


[EXTERNAL EMAIL]
Hello
I agree with you on the problem statement.

But I don't think this is a desired solution.
We expect Platform BDS to call the PhysicalPresenceLib. As such, why not clear the  screen in the platform BDS?

Thank you
Yao, Jiewen

From: Poosapalli, Karunakar <KarunakarPoosapalli@Dell.com<mailto:KarunakarPoosapalli@Dell.com>>
Sent: Monday, July 24, 2023 11:26 PM
To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Gao, Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Subject: RE: [edk2-devel] [PATCH] SecurityPkg: DxeTcg2PhysicalPresenceLib for Clear Graphics Screen To unblock and Display TPM messages

Can you please review and share your feedback?


Thanks & Regards
Karunakar Poosapalli
Firmware Principal Engineer, Client BIOS
Customer BIOS | Dell Core BIOS
CPG Software Engineering | Dell Technologies
Mobile +91 9951902957
Karunakar_poosapalli@Dell.Com<mailto:Karunakar_poosapalli@Dell.Com>



Internal Use - Confidential
From: Poosapalli, Karunakar
Sent: Saturday, July 22, 2023 1:21 AM
To: jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>; gaoliming; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Subject: [edk2-devel] [PATCH] SecurityPkg: DxeTcg2PhysicalPresenceLib for Clear Graphics Screen To unblock and Display TPM messages

Patch review for Bugzilla -  https://bugzilla.tianocore.org/show_bug.cgi?id=4462 [bugzilla.tianocore.org]<https://urldefense.com/v3/__https:/bugzilla.tianocore.org/show_bug.cgi?id=4462__;!!LpKI!nIImq_Eo23HxKNbpCloDgiuP_82UEmFPLYEbrs-JRjj03rfEIF5h3MqG0lMX3qzdhStA76K3O2XvHbbFilQlXDMmwg$>

From c537f9c5c9e02c54e27466b96fe33555afccd358 Mon Sep 17 00:00:00 2001
From: Karunakar Poosapalli karunakar_poosapalli@dell.com<mailto:karunakar_poosapalli@dell.com>
Date: Sat, 22 Jul 2023 01:13:44 +0530
Subject: [PATCH] Patch - Enhance Tcg2 to clear graphics before printing the
messages on screen

[Background]
Tcg2UserConfirm() Function is used to display any user conformation messages on the console
Function Definition Full path - SecurityPkg\Library\DxeTcg2PhysicalPresenceLib\DxeTcg2PhysicalPresenceLib.c

[Issue]
In the current Tcg2UserConfirm() implementation, This function forms a destination string to be displayed on the console and directly Print the message on Console.

But there is no logic added to clear the graphics before printing the messages on the screen.

There are some scenarios where Tcg messages might have been blocked by some other GUI or messages on Console.
  1. When there are some messages or logos already displayed in the content on the console,
     TCG message will NOT be displayed or corrupted to the End user.
  2. There could be a Custom logo displaying on the screen which actually blocks the screen.

[Solution]
  1. As TCG user confirmation is the highest priority and it blocks the POST until the user
     presses the input key. Before TCG messages Print on the console, there should be logic
     added to clear the graphics screen

Cc: gaoliming gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>
Cc: Jiewen  Jiewen.yao@intel.com<mailto:Jiewen.yao@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4462 [bugzilla.tianocore.org]<https://urldefense.com/v3/__https:/bugzilla.tianocore.org/show_bug.cgi?id=4462__;!!LpKI!nIImq_Eo23HxKNbpCloDgiuP_82UEmFPLYEbrs-JRjj03rfEIF5h3MqG0lMX3qzdhStA76K3O2XvHbbFilQlXDMmwg$>
Signed-off-by: Karunakar Poosapalli karunakar_poosapalli@dell.com<mailto:karunakar_poosapalli@dell.com>
---
.../DxeTcg2PhysicalPresenceLib.c              | 74 +++++++++++++++++++
.../DxeTcg2PhysicalPresenceLib.inf            |  1 +
2 files changed, 75 insertions(+)

diff --git a/SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.c b/SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.c
index de4f5e583d..bd486f3b5b 100644
--- a/SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.c
+++ b/SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.c
@@ -251,6 +251,74 @@ Tcg2ExecutePhysicalPresence (
   }
}

+/**
+  Clear Graphics Screen To unblock and Display TPM messages
+
+  @param[in]  VOID
+  @retval     EFI_STATUS
+**/
+EFI_STATUS
+ClearGraphicsScreenToDisplayTpmMessages()
+{
+  EFI_GRAPHICS_OUTPUT_PROTOCOL *Gop;
+  EFI_STATUS Status;
+  EFI_HANDLE                              *HandleBuffer;
+  UINTN                                   NumberOfHandles;
+  UINTN                                   Index;
+  EFI_DEVICE_PATH_PROTOCOL                *GopDevicePath;
+
+  DEBUG((DEBUG_INFO, "ClearGraphicsScreenToDisplayTpmMessages Entry...\n"));
+
+  Status = gBS->LocateHandleBuffer (
+            ByProtocol,
+            &gEfiGraphicsOutputProtocolGuid,
+            NULL,
+            &NumberOfHandles,
+            &HandleBuffer
+            );
+  DEBUG((DEBUG_INFO, "LocateHandleBuffer Status = %r, NumberOfHandles = %x\n", Status, NumberOfHandles));
+
+  if (EFI_ERROR (Status))
+  {
+    return Status;
+  }
+
+  for (Index = 0; Index < NumberOfHandles; Index++)
+    {
+      Status = gBS->HandleProtocol (
+                HandleBuffer[Index],
+                &gEfiDevicePathProtocolGuid,
+                (VOID *)&GopDevicePath
+                );
+      DEBUG((DEBUG_INFO, "HandleProtocol GopDevicePath Status = %r, Index = %x\n", Status, Index));
+      if (EFI_ERROR (Status))
+        {
+          continue;
+        }
+
+      Status = gBS->HandleProtocol (
+                HandleBuffer[Index],
+                &gEfiGraphicsOutputProtocolGuid,
+                (VOID **) &Gop
+                );
+      DEBUG((DEBUG_INFO, "HandleProtocol Gop Status = %r, Index = %x\n", Status, Index));
+      if (EFI_ERROR(Status))
+        {
+            continue;
+        }
+
+      // Clear the graphics screen to black
+      Status = Gop->Blt(Gop, NULL, EfiBltVideoFill, 0, 0, 0, 0, Gop->Mode->Info->HorizontalResolution, Gop->Mode->Info->VerticalResolution, 0);
+      DEBUG((DEBUG_INFO, "Gop->Blt Status = %r, Index = %x\n", Status, Index));
+      if (EFI_ERROR(Status)) {
+        continue;
+      }
+    }
+
+  gBS->FreePool (HandleBuffer);
+  DEBUG((DEBUG_INFO, "ClearGraphicsScreenToDisplayTpmMessages Exit...\n"));
+  return Status;
+}
/**
   Read the specified key for user confirmation.

@@ -576,6 +644,12 @@ Tcg2UserConfirm (
   BufSize -= StrSize (ConfirmText);
   UnicodeSPrint (ConfirmText + StrLen (ConfirmText), BufSize, TmpStr1, TmpStr2);

+  //
+  //Clear Graphics Screen To unblock and Display TPM messages
+  //
+  Status = ClearGraphicsScreenToDisplayTpmMessages();
+  DEBUG((DEBUG_INFO, "ClearGraphicsScreenToDisplayTpmMessages Status = %r\n", Status));
+
   DstStr[80] = L'\0';
   for (Index = 0; Index < StrLen (ConfirmText); Index += 80) {
     StrnCpyS (DstStr, sizeof (DstStr) / sizeof (CHAR16), ConfirmText + Index, sizeof (DstStr) / sizeof (CHAR16) - 1);
diff --git a/SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.inf b/SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.inf
index e1c7c20d52..de423cfd13 100644
--- a/SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.inf
+++ b/SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.inf
@@ -54,6 +54,7 @@
[Protocols]
   gEfiTcg2ProtocolGuid                 ## SOMETIMES_CONSUMES
   gEdkiiVariableLockProtocolGuid       ## SOMETIMES_CONSUMES
+  gEfiGraphicsOutputProtocolGuid       ## CONSUMES

 [Pcd]
   gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags       ## SOMETIMES_CONSUMES
--
2.17.0.windows.1



Internal Use - Confidential


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107648): https://edk2.groups.io/g/devel/message/107648
Mute This Topic: https://groups.io/mt/100333271/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 26770 bytes --]

  reply	other threads:[~2023-08-08 23:13 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-21 19:51 [edk2-devel] [PATCH] SecurityPkg: DxeTcg2PhysicalPresenceLib for Clear Graphics Screen To unblock and Display TPM messages Poosapalli, Karunakar via groups.io
2023-07-24 15:26 ` Poosapalli, Karunakar via groups.io
2023-07-25 12:17   ` Yao, Jiewen
2023-08-08 18:12     ` Poosapalli, Karunakar via groups.io
2023-08-08 23:13       ` Yao, Jiewen [this message]
2023-08-09  4:29         ` Poosapalli, Karunakar via groups.io
2023-08-16 17:28           ` Poosapalli, Karunakar via groups.io
2023-08-17  3:00             ` Yao, Jiewen
2023-08-29  9:49               ` Poosapalli, Karunakar via groups.io

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=MW4PR11MB5872A550D98A4DF5E7CB42318C0DA@MW4PR11MB5872.namprd11.prod.outlook.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