public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH V1 1/1] OvmfPkg/AcpiPlatformDxe: Add back log and fix code cohesion
@ 2023-01-04 14:13 Min Xu
  2023-01-05  9:09 ` [edk2-devel] " Laszlo Ersek
  2023-01-05 13:15 ` Gerd Hoffmann
  0 siblings, 2 replies; 6+ messages in thread
From: Min Xu @ 2023-01-04 14:13 UTC (permalink / raw)
  To: devel
  Cc: Min M Xu, Laszlo Ersek, Erdem Aktas, James Bottomley, Jiewen Yao,
	Gerd Hoffmann, Tom Lendacky, Sebastien Boeuf

From: Min M Xu <min.m.xu@intel.com>

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4237

Commit 9fdc70af6ba8 breaks log analysis and code cohesion in
AcpiPlatformDxe. See the detailed information in BZ#4237.

There are below changes in this patch:
1) Add back debug log
2) Add error checking and handling if InstallProtocolInterface failed.
3) Delete the QemuAcpiTableNotify.h because it is superfluous.
4) Delete the global variable "mQemuAcpiHandle" instead of a local
   variable.
5) Install the QEMU_ACPI_TABLE_NOTIFY_PROTOCOL with a NULL associated
   interface.

Above changes 2/4/5 are applied in both CloudHvAcpi.c and QemuFwCfgAcpi.c.

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Sebastien Boeuf <sebastien.boeuf@intel.com>
Reported-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c         | 25 +++++------
 OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c       | 42 +++++++++++++------
 .../Include/Protocol/QemuAcpiTableNotify.h    | 27 ------------
 3 files changed, 42 insertions(+), 52 deletions(-)
 delete mode 100644 OvmfPkg/Include/Protocol/QemuAcpiTableNotify.h

diff --git a/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c b/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
index cbe8bb9b0c75..81f387438a64 100644
--- a/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
+++ b/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
@@ -18,13 +18,9 @@
 
 #include <Protocol/AcpiSystemDescriptionTable.h>
 #include <Protocol/AcpiTable.h>
-#include <Protocol/QemuAcpiTableNotify.h>                 // QEMU_ACPI_TABLE_NOTIFY_PROTOCOL
 
 #include "AcpiPlatform.h"
 
-EFI_HANDLE                       mChAcpiHandle = NULL;
-QEMU_ACPI_TABLE_NOTIFY_PROTOCOL  mChAcpiNotifyProtocol;
-
 EFI_STATUS
 EFIAPI
 InstallCloudHvTablesTdx (
@@ -33,13 +29,15 @@ InstallCloudHvTablesTdx (
 {
   EFI_STATUS  Status;
   UINTN       TableHandle;
+  EFI_HANDLE  ChAcpiHandle;
 
   EFI_PEI_HOB_POINTERS         Hob;
   EFI_ACPI_DESCRIPTION_HEADER  *CurrentTable;
   EFI_ACPI_DESCRIPTION_HEADER  *DsdtTable;
 
-  DsdtTable   = NULL;
-  TableHandle = 0;
+  DsdtTable    = NULL;
+  TableHandle  = 0;
+  ChAcpiHandle = NULL;
 
   Hob.Guid = (EFI_HOB_GUID_TYPE *)GetFirstGuidHob (&gUefiOvmfPkgTdxAcpiHobGuid);
 
@@ -92,12 +90,15 @@ InstallCloudHvTablesTdx (
   // Install a protocol to notify that the ACPI table provided by CH is
   // ready.
   //
-  gBS->InstallProtocolInterface (
-         &mChAcpiHandle,
-         &gQemuAcpiTableNotifyProtocolGuid,
-         EFI_NATIVE_INTERFACE,
-         &mChAcpiNotifyProtocol
-         );
+  Status = gBS->InstallProtocolInterface (
+                  &ChAcpiHandle,
+                  &gQemuAcpiTableNotifyProtocolGuid,
+                  EFI_NATIVE_INTERFACE,
+                  NULL
+                  );
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
 
   return EFI_SUCCESS;
 }
diff --git a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
index c8dee17c13e6..22340b13e3ee 100644
--- a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
+++ b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
@@ -19,10 +19,7 @@
 #include <Library/QemuFwCfgS3Lib.h>           // QemuFwCfgS3Enabled()
 #include <Library/UefiBootServicesTableLib.h> // gBS
 
-#include <Protocol/QemuAcpiTableNotify.h>
 #include "AcpiPlatform.h"
-EFI_HANDLE                       mQemuAcpiHandle = NULL;
-QEMU_ACPI_TABLE_NOTIFY_PROTOCOL  mAcpiNotifyProtocol;
 
 //
 // The user structure for the ordered collection that will track the fw_cfg
@@ -1103,6 +1100,9 @@ InstallQemuFwCfgTables (
   ORDERED_COLLECTION_ENTRY  *TrackerEntry, *TrackerEntry2;
   ORDERED_COLLECTION        *SeenPointers;
   ORDERED_COLLECTION_ENTRY  *SeenPointerEntry, *SeenPointerEntry2;
+  EFI_HANDLE                QemuAcpiHandle;
+
+  QemuAcpiHandle = NULL;
 
   Status = QemuFwCfgFindFile ("etc/table-loader", &FwCfgItem, &FwCfgSize);
   if (EFI_ERROR (Status)) {
@@ -1249,6 +1249,20 @@ InstallQemuFwCfgTables (
     }
   }
 
+  //
+  // Install a protocol to notify that the ACPI table provided by Qemu is
+  // ready.
+  //
+  Status = gBS->InstallProtocolInterface (
+                  &QemuAcpiHandle,
+                  &gQemuAcpiTableNotifyProtocolGuid,
+                  EFI_NATIVE_INTERFACE,
+                  NULL
+                  );
+  if (EFI_ERROR (Status)) {
+    goto UninstallAcpiTables;
+  }
+
   //
   // Translating the condensed QEMU_LOADER_WRITE_POINTER commands to ACPI S3
   // Boot Script opcodes has to be the last operation in this function, because
@@ -1275,17 +1289,19 @@ UninstallAcpiTables:
       --Installed;
       AcpiProtocol->UninstallAcpiTable (AcpiProtocol, InstalledKey[Installed]);
     }
+
+    //
+    // Uninstall the notification if it has been installed.
+    //
+    if (QemuAcpiHandle != NULL) {
+      gBS->UninstallProtocolInterface (
+             QemuAcpiHandle,
+             &gQemuAcpiTableNotifyProtocolGuid,
+             NULL
+             );
+    }
   } else {
-    //
-    // Install a protocol to notify that the ACPI table provided by Qemu is
-    // ready.
-    //
-    gBS->InstallProtocolInterface (
-           &mQemuAcpiHandle,
-           &gQemuAcpiTableNotifyProtocolGuid,
-           EFI_NATIVE_INTERFACE,
-           &mAcpiNotifyProtocol
-           );
+    DEBUG ((DEBUG_INFO, "%a: installed %d tables\n", __FUNCTION__, Installed));
   }
 
   for (SeenPointerEntry = OrderedCollectionMin (SeenPointers);
diff --git a/OvmfPkg/Include/Protocol/QemuAcpiTableNotify.h b/OvmfPkg/Include/Protocol/QemuAcpiTableNotify.h
deleted file mode 100644
index a3dd2fc1dc91..000000000000
--- a/OvmfPkg/Include/Protocol/QemuAcpiTableNotify.h
+++ /dev/null
@@ -1,27 +0,0 @@
-/** @file
-
-  SPDX-License-Identifier: BSD-2-Clause-Patent
-
-**/
-
-#ifndef QEMU_ACPI_TABLE_NOTIFY_H_
-#define QEMU_ACPI_TABLE_NOTIFY_H_
-
-#define QEMU_ACPI_TABLE_NOTIFY_GUID \
-  { 0x928939b2, 0x4235, 0x462f, { 0x95, 0x80, 0xf6, 0xa2, 0xb2, 0xc2, 0x1a, 0x4f } };
-
-///
-/// Forward declaration
-///
-typedef struct _QEMU_ACPI_TABLE_NOTIFY_PROTOCOL QEMU_ACPI_TABLE_NOTIFY_PROTOCOL;
-
-///
-/// Protocol structure
-///
-struct _QEMU_ACPI_TABLE_NOTIFY_PROTOCOL {
-  UINT8    Notify;
-};
-
-extern EFI_GUID  gQemuAcpiTableNotifyProtocolGuid;
-
-#endif
-- 
2.29.2.windows.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-01-09  8:35 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-04 14:13 [PATCH V1 1/1] OvmfPkg/AcpiPlatformDxe: Add back log and fix code cohesion Min Xu
2023-01-05  9:09 ` [edk2-devel] " Laszlo Ersek
2023-01-09  8:34   ` Min Xu
2023-01-05 13:15 ` Gerd Hoffmann
2023-01-05 15:10   ` Laszlo Ersek
2023-01-09  0:49     ` Min Xu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox