[PATCH] OvmfPkg: Call PlatformInitializeConsole for GPU passthrough case

[PATCH] OvmfPkg: Call PlatformInitializeConsole for GPU passthrough case

[Stefan Berger]

For GPU passthrough support we have to initialize the console after
EfiBootManagerDispatchDeferredImages() has loaded ROMs. This was the
calling order before the TCG physical presence support moved it to
before End-of-DXE since End-of-DXE disables the TPM 2 platform
hierarchy and some physical presence commands would not work otherwise.
To enable user interaction, the console initialization was also moved
to before End-of-DXE. With this fix, the console is now initialized twice
where the second initialization fixes the GPU passthrough support but it
doesn't allow user interaction for TCG physical presence when GPU
passthrough is used.

Fixes: b8675deaa819631db2667df63f89799fe65fc906
Fixes: https://bugzilla.tianocore.org/show_bug.cgi?id=3771
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c      | 7 +++++++
 OvmfPkg/Library/PlatformBootManagerLibBhyve/BdsPlatform.c | 6 ++++++
 OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c  | 6 ++++++
 3 files changed, 19 insertions(+)

diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
index 186401296a..faf5d2b9ae 100644
--- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
+++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
@@ -414,6 +414,13 @@ PlatformBootManagerBeforeConsole (
   //
   EfiBootManagerDispatchDeferredImages ();
 
+  //
+  // GPU passthrough only allows console enablement after ROM image load
+  //
+  PlatformInitializeConsole (
+    XenDetected() ? gXenPlatformConsole : gPlatformConsole);
+
+
   FrontPageTimeout = GetFrontPageTimeoutFromQemu ();
   PcdStatus = PcdSet16S (PcdPlatformBootTimeOut, FrontPageTimeout);
   ASSERT_RETURN_ERROR (PcdStatus);
diff --git a/OvmfPkg/Library/PlatformBootManagerLibBhyve/BdsPlatform.c b/OvmfPkg/Library/PlatformBootManagerLibBhyve/BdsPlatform.c
index e767c3b172..0dab0ecbb7 100644
--- a/OvmfPkg/Library/PlatformBootManagerLibBhyve/BdsPlatform.c
+++ b/OvmfPkg/Library/PlatformBootManagerLibBhyve/BdsPlatform.c
@@ -400,6 +400,12 @@ PlatformBootManagerBeforeConsole (
   //
   EfiBootManagerDispatchDeferredImages ();
 
+  //
+  // GPU passthrough only allows console enablement after ROM image load
+  //
+  PlatformInitializeConsole (
+    XenDetected() ? gXenPlatformConsole : gPlatformConsole);
+
   PlatformRegisterOptionsAndKeys ();
 
   //
diff --git a/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c b/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c
index fd80577355..b82931d726 100644
--- a/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c
+++ b/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c
@@ -363,6 +363,12 @@ PlatformBootManagerBeforeConsole (
   //
   EfiBootManagerDispatchDeferredImages ();
 
+  //
+  // GPU passthrough only allows console enablement after ROM image load
+  //
+  PlatformInitializeConsole (
+    XenDetected() ? gXenPlatformConsole : gPlatformConsole);
+
   Status = gRT->SetVariable (
                   EFI_TIME_OUT_VARIABLE_NAME,
                   &gEfiGlobalVariableGuid,
-- 
2.31.1

Re: [PATCH] OvmfPkg: Call PlatformInitializeConsole for GPU passthrough case

[Shivanshu Goyal]

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

Thank you for making this change Stefan. I tested your patch locally on my
GPU passthrough Windows 10 VM and it successfully fixes it.
I added a "Tested-by:" note inline in your patch.

Thank you,
Shivanshu Goyal

On Wed, Dec 15, 2021 at 7:44 AM Stefan Berger <stefanb@linux.ibm.com> wrote:

> For GPU passthrough support we have to initialize the console after
> EfiBootManagerDispatchDeferredImages() has loaded ROMs. This was the
> calling order before the TCG physical presence support moved it to
> before End-of-DXE since End-of-DXE disables the TPM 2 platform
> hierarchy and some physical presence commands would not work otherwise.
> To enable user interaction, the console initialization was also moved
> to before End-of-DXE. With this fix, the console is now initialized twice
> where the second initialization fixes the GPU passthrough support but it
> doesn't allow user interaction for TCG physical presence when GPU
> passthrough is used.
>
> Fixes: b8675deaa819631db2667df63f89799fe65fc906
> Fixes: https://bugzilla.tianocore.org/show_bug.cgi?id=3771
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
>
Tested-by: Shivanshu Goyal <shivanshu3@gmail.com>

> ---
>  OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c      | 7 +++++++
>  OvmfPkg/Library/PlatformBootManagerLibBhyve/BdsPlatform.c | 6 ++++++
>  OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c  | 6 ++++++
>  3 files changed, 19 insertions(+)
>
> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> index 186401296a..faf5d2b9ae 100644
> --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> @@ -414,6 +414,13 @@ PlatformBootManagerBeforeConsole (
>    //
>    EfiBootManagerDispatchDeferredImages ();
>
> +  //
> +  // GPU passthrough only allows console enablement after ROM image load
> +  //
> +  PlatformInitializeConsole (
> +    XenDetected() ? gXenPlatformConsole : gPlatformConsole);
> +
> +
>    FrontPageTimeout = GetFrontPageTimeoutFromQemu ();
>    PcdStatus = PcdSet16S (PcdPlatformBootTimeOut, FrontPageTimeout);
>    ASSERT_RETURN_ERROR (PcdStatus);
> diff --git a/OvmfPkg/Library/PlatformBootManagerLibBhyve/BdsPlatform.c
> b/OvmfPkg/Library/PlatformBootManagerLibBhyve/BdsPlatform.c
> index e767c3b172..0dab0ecbb7 100644
> --- a/OvmfPkg/Library/PlatformBootManagerLibBhyve/BdsPlatform.c
> +++ b/OvmfPkg/Library/PlatformBootManagerLibBhyve/BdsPlatform.c
> @@ -400,6 +400,12 @@ PlatformBootManagerBeforeConsole (
>    //
>    EfiBootManagerDispatchDeferredImages ();
>
> +  //
> +  // GPU passthrough only allows console enablement after ROM image load
> +  //
> +  PlatformInitializeConsole (
> +    XenDetected() ? gXenPlatformConsole : gPlatformConsole);
> +
>    PlatformRegisterOptionsAndKeys ();
>
>    //
> diff --git a/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c
> b/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c
> index fd80577355..b82931d726 100644
> --- a/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c
> +++ b/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c
> @@ -363,6 +363,12 @@ PlatformBootManagerBeforeConsole (
>    //
>    EfiBootManagerDispatchDeferredImages ();
>
> +  //
> +  // GPU passthrough only allows console enablement after ROM image load
> +  //
> +  PlatformInitializeConsole (
> +    XenDetected() ? gXenPlatformConsole : gPlatformConsole);
> +
>    Status = gRT->SetVariable (
>                    EFI_TIME_OUT_VARIABLE_NAME,
>                    &gEfiGlobalVariableGuid,
> --
> 2.31.1
>
>

-- 
Shivanshu Goyal
shivanshu.ca

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

Re: [PATCH] OvmfPkg: Call PlatformInitializeConsole for GPU passthrough case

[Gerd Hoffmann]

  Hi,

> +  //
> +  // GPU passthrough only allows console enablement after ROM image load
> +  //
> +  PlatformInitializeConsole (
> +    XenDetected() ? gXenPlatformConsole : gPlatformConsole);
> +

AmdSev build breaks:

/home/kraxel/projects/edk2/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c: In function ‘PlatformBootManagerBeforeConsole’:
/home/kraxel/projects/edk2/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c:399:5: error: implicit declaration of function ‘XenDetected’ [-Werror=implicit-function-declaration]
  399 |     XenDetected() ? gXenPlatformConsole : gPlatformConsole);
      |     ^~~~~~~~~~~
cc1: all warnings being treated as errors

Otherwise it seems to work fine, no bad side effects from calling
PlatformInitializeConsole() twice.

take care,
  Gerd