* [PATCH V3 1/6] OvmfPkg/AcpiPlatformDxe: Remove QEMU_ACPI_TABLE_NOTIFY_PROTOCOL
2023-01-11 1:22 [PATCH V3 0/6] Refactor installation of gQemuAcpiTableNotifyProtocol Min Xu
@ 2023-01-11 1:22 ` Min Xu
2023-01-12 10:03 ` Boeuf, Sebastien
2023-01-11 1:22 ` [PATCH V3 2/6] OvmfPkg/AcpiPlatformDxe: Use local variable in CloudHvAcpi.c Min Xu
` (5 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Min Xu @ 2023-01-11 1:22 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
The QEMU_ACPI_TABLE_NOTIFY_PROTOCOL structure is superfluous because NULL
protocol interfaces have been used in edk2 repeatedly. A protocol instance
can exist in the protocol database with a NULL associated interface.
Therefore the QEMU_ACPI_TABLE_NOTIFY_PROTOCOL type, the
"QemuAcpiTableNotify.h" header, and the "mAcpiNotifyProtocol" global
variable can be removed.
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>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c | 7 ++---
OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c | 6 ++---
.../Include/Protocol/QemuAcpiTableNotify.h | 27 -------------------
3 files changed, 4 insertions(+), 36 deletions(-)
delete mode 100644 OvmfPkg/Include/Protocol/QemuAcpiTableNotify.h
diff --git a/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c b/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
index cbe8bb9b0c75..ad39e4253478 100644
--- a/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
+++ b/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
@@ -15,15 +15,12 @@
#include <Library/PcdLib.h> // PcdGet32()
#include <Library/HobLib.h> // GetFirstGuidHob(), GetNextGuidHob()
#include <Library/UefiBootServicesTableLib.h> // gBS
-
#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_HANDLE mChAcpiHandle = NULL;
EFI_STATUS
EFIAPI
@@ -96,7 +93,7 @@ InstallCloudHvTablesTdx (
&mChAcpiHandle,
&gQemuAcpiTableNotifyProtocolGuid,
EFI_NATIVE_INTERFACE,
- &mChAcpiNotifyProtocol
+ NULL
);
return EFI_SUCCESS;
diff --git a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
index c8dee17c13e6..1a3852904df9 100644
--- a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
+++ b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
@@ -19,10 +19,8 @@
#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;
+EFI_HANDLE mQemuAcpiHandle = NULL;
//
// The user structure for the ordered collection that will track the fw_cfg
@@ -1284,7 +1282,7 @@ UninstallAcpiTables:
&mQemuAcpiHandle,
&gQemuAcpiTableNotifyProtocolGuid,
EFI_NATIVE_INTERFACE,
- &mAcpiNotifyProtocol
+ NULL
);
}
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] 13+ messages in thread* Re: [PATCH V3 1/6] OvmfPkg/AcpiPlatformDxe: Remove QEMU_ACPI_TABLE_NOTIFY_PROTOCOL
2023-01-11 1:22 ` [PATCH V3 1/6] OvmfPkg/AcpiPlatformDxe: Remove QEMU_ACPI_TABLE_NOTIFY_PROTOCOL Min Xu
@ 2023-01-12 10:03 ` Boeuf, Sebastien
0 siblings, 0 replies; 13+ messages in thread
From: Boeuf, Sebastien @ 2023-01-12 10:03 UTC (permalink / raw)
To: devel@edk2.groups.io, Xu, Min M
Cc: kraxel@redhat.com, jejb@linux.ibm.com, Yao, Jiewen,
thomas.lendacky@amd.com, Aktas, Erdem, lersek@redhat.com
On Wed, 2023-01-11 at 09:22 +0800, Min Xu wrote:
> From: Min M Xu <min.m.xu@intel.com>
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4237
>
> The QEMU_ACPI_TABLE_NOTIFY_PROTOCOL structure is superfluous because
> NULL
> protocol interfaces have been used in edk2 repeatedly. A protocol
> instance
> can exist in the protocol database with a NULL associated interface.
> Therefore the QEMU_ACPI_TABLE_NOTIFY_PROTOCOL type, the
> "QemuAcpiTableNotify.h" header, and the "mAcpiNotifyProtocol" global
> variable can be removed.
>
> 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>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>
> ---
> OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c | 7 ++---
> OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c | 6 ++---
> .../Include/Protocol/QemuAcpiTableNotify.h | 27 -----------------
> --
> 3 files changed, 4 insertions(+), 36 deletions(-)
> delete mode 100644 OvmfPkg/Include/Protocol/QemuAcpiTableNotify.h
>
> diff --git a/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
> b/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
> index cbe8bb9b0c75..ad39e4253478 100644
> --- a/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
> +++ b/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
> @@ -15,15 +15,12 @@
> #include <Library/PcdLib.h> //
> PcdGet32()
> #include <Library/HobLib.h> //
> GetFirstGuidHob(), GetNextGuidHob()
> #include <Library/UefiBootServicesTableLib.h> // gBS
> -
> #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_HANDLE mChAcpiHandle = NULL;
>
> EFI_STATUS
> EFIAPI
> @@ -96,7 +93,7 @@ InstallCloudHvTablesTdx (
> &mChAcpiHandle,
> &gQemuAcpiTableNotifyProtocolGuid,
> EFI_NATIVE_INTERFACE,
> - &mChAcpiNotifyProtocol
> + NULL
> );
>
> return EFI_SUCCESS;
> diff --git a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
> b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
> index c8dee17c13e6..1a3852904df9 100644
> --- a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
> +++ b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
> @@ -19,10 +19,8 @@
> #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;
> +EFI_HANDLE mQemuAcpiHandle = NULL;
>
> //
> // The user structure for the ordered collection that will track the
> fw_cfg
> @@ -1284,7 +1282,7 @@ UninstallAcpiTables:
> &mQemuAcpiHandle,
> &gQemuAcpiTableNotifyProtocolGuid,
> EFI_NATIVE_INTERFACE,
> - &mAcpiNotifyProtocol
> + NULL
> );
> }
>
> 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
Looks good and I've tested Cloud Hypervisor with both CloudHvX64 and
IntelTdxX64 targets.
Reviewed-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris,
92196 Meudon Cedex, France
Registration Number: 302 456 199 R.C.S. NANTERRE
Capital: 5 208 026.16 Euros
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH V3 2/6] OvmfPkg/AcpiPlatformDxe: Use local variable in CloudHvAcpi.c
2023-01-11 1:22 [PATCH V3 0/6] Refactor installation of gQemuAcpiTableNotifyProtocol Min Xu
2023-01-11 1:22 ` [PATCH V3 1/6] OvmfPkg/AcpiPlatformDxe: Remove QEMU_ACPI_TABLE_NOTIFY_PROTOCOL Min Xu
@ 2023-01-11 1:22 ` Min Xu
2023-01-12 10:04 ` Boeuf, Sebastien
2023-01-11 1:22 ` [PATCH V3 3/6] OvmfPkg/AcpiPlatformDxe: Use local variable in QemuFwCfgAcpi.c Min Xu
` (4 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Min Xu @ 2023-01-11 1:22 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
The handle of mChAcpiHandle is not needed for anything, beyond the
scope of the InstallCloudHvTablesTdx (). A local variable (ChAcpiHandle)
suffices for storing the handle.
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 | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c b/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
index ad39e4253478..8f90ea23996d 100644
--- a/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
+++ b/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
@@ -20,8 +20,6 @@
#include "AcpiPlatform.h"
-EFI_HANDLE mChAcpiHandle = NULL;
-
EFI_STATUS
EFIAPI
InstallCloudHvTablesTdx (
@@ -30,6 +28,7 @@ InstallCloudHvTablesTdx (
{
EFI_STATUS Status;
UINTN TableHandle;
+ EFI_HANDLE ChAcpiHandle;
EFI_PEI_HOB_POINTERS Hob;
EFI_ACPI_DESCRIPTION_HEADER *CurrentTable;
@@ -89,8 +88,9 @@ InstallCloudHvTablesTdx (
// Install a protocol to notify that the ACPI table provided by CH is
// ready.
//
+ ChAcpiHandle = NULL;
gBS->InstallProtocolInterface (
- &mChAcpiHandle,
+ &ChAcpiHandle,
&gQemuAcpiTableNotifyProtocolGuid,
EFI_NATIVE_INTERFACE,
NULL
--
2.29.2.windows.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH V3 2/6] OvmfPkg/AcpiPlatformDxe: Use local variable in CloudHvAcpi.c
2023-01-11 1:22 ` [PATCH V3 2/6] OvmfPkg/AcpiPlatformDxe: Use local variable in CloudHvAcpi.c Min Xu
@ 2023-01-12 10:04 ` Boeuf, Sebastien
0 siblings, 0 replies; 13+ messages in thread
From: Boeuf, Sebastien @ 2023-01-12 10:04 UTC (permalink / raw)
To: devel@edk2.groups.io, Xu, Min M
Cc: kraxel@redhat.com, jejb@linux.ibm.com, Yao, Jiewen,
thomas.lendacky@amd.com, Aktas, Erdem, lersek@redhat.com
On Wed, 2023-01-11 at 09:22 +0800, Min Xu wrote:
> From: Min M Xu <min.m.xu@intel.com>
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4237
>
> The handle of mChAcpiHandle is not needed for anything, beyond the
> scope of the InstallCloudHvTablesTdx (). A local variable
> (ChAcpiHandle)
> suffices for storing the handle.
>
> 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 | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
> b/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
> index ad39e4253478..8f90ea23996d 100644
> --- a/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
> +++ b/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
> @@ -20,8 +20,6 @@
>
> #include "AcpiPlatform.h"
>
> -EFI_HANDLE mChAcpiHandle = NULL;
> -
> EFI_STATUS
> EFIAPI
> InstallCloudHvTablesTdx (
> @@ -30,6 +28,7 @@ InstallCloudHvTablesTdx (
> {
> EFI_STATUS Status;
> UINTN TableHandle;
> + EFI_HANDLE ChAcpiHandle;
>
> EFI_PEI_HOB_POINTERS Hob;
> EFI_ACPI_DESCRIPTION_HEADER *CurrentTable;
> @@ -89,8 +88,9 @@ InstallCloudHvTablesTdx (
> // Install a protocol to notify that the ACPI table provided by CH
> is
> // ready.
> //
> + ChAcpiHandle = NULL;
> gBS->InstallProtocolInterface (
> - &mChAcpiHandle,
> + &ChAcpiHandle,
> &gQemuAcpiTableNotifyProtocolGuid,
> EFI_NATIVE_INTERFACE,
> NULL
Looks good and I've tested Cloud Hypervisor with both CloudHvX64 and
IntelTdxX64 targets.
Reviewed-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris,
92196 Meudon Cedex, France
Registration Number: 302 456 199 R.C.S. NANTERRE
Capital: 5 208 026.16 Euros
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH V3 3/6] OvmfPkg/AcpiPlatformDxe: Use local variable in QemuFwCfgAcpi.c
2023-01-11 1:22 [PATCH V3 0/6] Refactor installation of gQemuAcpiTableNotifyProtocol Min Xu
2023-01-11 1:22 ` [PATCH V3 1/6] OvmfPkg/AcpiPlatformDxe: Remove QEMU_ACPI_TABLE_NOTIFY_PROTOCOL Min Xu
2023-01-11 1:22 ` [PATCH V3 2/6] OvmfPkg/AcpiPlatformDxe: Use local variable in CloudHvAcpi.c Min Xu
@ 2023-01-11 1:22 ` Min Xu
2023-01-11 16:49 ` [edk2-devel] " Laszlo Ersek
2023-01-11 1:22 ` [PATCH V3 4/6] OvmfPkg/AcpiPlatformDxe: Add log to show the installed tables Min Xu
` (3 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Min Xu @ 2023-01-11 1:22 UTC (permalink / raw)
To: devel
Cc: Min M Xu, Laszlo Ersek, Erdem Aktas, James Bottomley, Jiewen Yao,
Gerd Hoffmann, Tom Lendacky
From: Min M Xu <min.m.xu@intel.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4237
The handle of mQemuAcpiHandle is not needed for anything, beyond the
scope of the InstallQemuFwCfgTables(). So a local variable will
suffice for storing the handle.
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>
Reported-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
index 1a3852904df9..9711335c6cac 100644
--- a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
+++ b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
@@ -20,7 +20,6 @@
#include <Library/UefiBootServicesTableLib.h> // gBS
#include "AcpiPlatform.h"
-EFI_HANDLE mQemuAcpiHandle = NULL;
//
// The user structure for the ordered collection that will track the fw_cfg
@@ -1101,6 +1100,7 @@ InstallQemuFwCfgTables (
ORDERED_COLLECTION_ENTRY *TrackerEntry, *TrackerEntry2;
ORDERED_COLLECTION *SeenPointers;
ORDERED_COLLECTION_ENTRY *SeenPointerEntry, *SeenPointerEntry2;
+ EFI_HANDLE QemuAcpiHandle;
Status = QemuFwCfgFindFile ("etc/table-loader", &FwCfgItem, &FwCfgSize);
if (EFI_ERROR (Status)) {
@@ -1278,8 +1278,9 @@ UninstallAcpiTables:
// Install a protocol to notify that the ACPI table provided by Qemu is
// ready.
//
+ QemuAcpiHandle = NULL;
gBS->InstallProtocolInterface (
- &mQemuAcpiHandle,
+ &QemuAcpiHandle,
&gQemuAcpiTableNotifyProtocolGuid,
EFI_NATIVE_INTERFACE,
NULL
--
2.29.2.windows.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [edk2-devel] [PATCH V3 3/6] OvmfPkg/AcpiPlatformDxe: Use local variable in QemuFwCfgAcpi.c
2023-01-11 1:22 ` [PATCH V3 3/6] OvmfPkg/AcpiPlatformDxe: Use local variable in QemuFwCfgAcpi.c Min Xu
@ 2023-01-11 16:49 ` Laszlo Ersek
0 siblings, 0 replies; 13+ messages in thread
From: Laszlo Ersek @ 2023-01-11 16:49 UTC (permalink / raw)
To: devel, min.m.xu
Cc: Erdem Aktas, James Bottomley, Jiewen Yao, Gerd Hoffmann,
Tom Lendacky
On 1/11/23 02:22, Min Xu wrote:
> From: Min M Xu <min.m.xu@intel.com>
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4237
>
> The handle of mQemuAcpiHandle is not needed for anything, beyond the
> scope of the InstallQemuFwCfgTables(). So a local variable will
> suffice for storing the handle.
>
> 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>
> Reported-by: Laszlo Ersek <lersek@redhat.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>
> ---
> OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
> index 1a3852904df9..9711335c6cac 100644
> --- a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
> +++ b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
> @@ -20,7 +20,6 @@
> #include <Library/UefiBootServicesTableLib.h> // gBS
>
> #include "AcpiPlatform.h"
> -EFI_HANDLE mQemuAcpiHandle = NULL;
>
> //
> // The user structure for the ordered collection that will track the fw_cfg
> @@ -1101,6 +1100,7 @@ InstallQemuFwCfgTables (
> ORDERED_COLLECTION_ENTRY *TrackerEntry, *TrackerEntry2;
> ORDERED_COLLECTION *SeenPointers;
> ORDERED_COLLECTION_ENTRY *SeenPointerEntry, *SeenPointerEntry2;
> + EFI_HANDLE QemuAcpiHandle;
>
> Status = QemuFwCfgFindFile ("etc/table-loader", &FwCfgItem, &FwCfgSize);
> if (EFI_ERROR (Status)) {
> @@ -1278,8 +1278,9 @@ UninstallAcpiTables:
> // Install a protocol to notify that the ACPI table provided by Qemu is
> // ready.
> //
> + QemuAcpiHandle = NULL;
> gBS->InstallProtocolInterface (
> - &mQemuAcpiHandle,
> + &QemuAcpiHandle,
> &gQemuAcpiTableNotifyProtocolGuid,
> EFI_NATIVE_INTERFACE,
> NULL
The update looks good, my R-b stands. Thanks!
Laszlo
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH V3 4/6] OvmfPkg/AcpiPlatformDxe: Add log to show the installed tables
2023-01-11 1:22 [PATCH V3 0/6] Refactor installation of gQemuAcpiTableNotifyProtocol Min Xu
` (2 preceding siblings ...)
2023-01-11 1:22 ` [PATCH V3 3/6] OvmfPkg/AcpiPlatformDxe: Use local variable in QemuFwCfgAcpi.c Min Xu
@ 2023-01-11 1:22 ` Min Xu
2023-01-11 1:22 ` [PATCH V3 5/6] OvmfPkg/AcpiPlatformDxe: Refactor QemuAcpiTableNotifyProtocol Min Xu
` (2 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Min Xu @ 2023-01-11 1:22 UTC (permalink / raw)
To: devel
Cc: Min M Xu, Laszlo Ersek, Erdem Aktas, James Bottomley, Jiewen Yao,
Gerd Hoffmann, Tom Lendacky
From: Min M Xu <min.m.xu@intel.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4237
Commit 9fdc70af6ba8 wrongly removed the log from InstallQemuFwCfgTables
after ACPI tables are successfully installed. This patch add the log
back after all operations succeed.
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>
Reported-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
index 9711335c6cac..462921466604 100644
--- a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
+++ b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
@@ -1264,6 +1264,8 @@ InstallQemuFwCfgTables (
S3Context = NULL;
}
+ DEBUG ((DEBUG_INFO, "%a: installed %d tables\n", __FUNCTION__, Installed));
+
UninstallAcpiTables:
if (EFI_ERROR (Status)) {
//
--
2.29.2.windows.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH V3 5/6] OvmfPkg/AcpiPlatformDxe: Refactor QemuAcpiTableNotifyProtocol
2023-01-11 1:22 [PATCH V3 0/6] Refactor installation of gQemuAcpiTableNotifyProtocol Min Xu
` (3 preceding siblings ...)
2023-01-11 1:22 ` [PATCH V3 4/6] OvmfPkg/AcpiPlatformDxe: Add log to show the installed tables Min Xu
@ 2023-01-11 1:22 ` Min Xu
2023-01-11 16:53 ` [edk2-devel] " Laszlo Ersek
2023-01-11 1:22 ` [PATCH V3 6/6] OvmfPkg/AcpiPlatformDxe: Return error if installing NotifyProtocol failed Min Xu
2023-01-15 11:50 ` [edk2-devel] [PATCH V3 0/6] Refactor installation of gQemuAcpiTableNotifyProtocol Laszlo Ersek
6 siblings, 1 reply; 13+ messages in thread
From: Min Xu @ 2023-01-11 1:22 UTC (permalink / raw)
To: devel
Cc: Min M Xu, Laszlo Ersek, Erdem Aktas, James Bottomley, Jiewen Yao,
Gerd Hoffmann, Tom Lendacky
From: Min M Xu <min.m.xu@intel.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4237
Commit 9fdc70af6ba8 install the QemuAcpiTableNotifyProtocol at a
wrong positioin. It should be called before TransferS3ContextToBootScript
because TransferS3ContextToBootScript is the last operation in
InstallQemuFwCfgTables(). Another error is that we should check the
returned value after installing the QemuAcpiTableNotifyProtocol.
This patch refactors the installation and error handling of
QemuAcpiTableNotifyProtocol in InstallQemuFwCfgTables ().
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>
Reported-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c | 38 ++++++++++++++++---------
1 file changed, 25 insertions(+), 13 deletions(-)
diff --git a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
index 462921466604..f0d81d6fd73d 100644
--- a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
+++ b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
@@ -1247,6 +1247,21 @@ InstallQemuFwCfgTables (
}
}
+ //
+ // Install a protocol to notify that the ACPI table provided by Qemu is
+ // ready.
+ //
+ QemuAcpiHandle = NULL;
+ 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
@@ -1255,7 +1270,7 @@ InstallQemuFwCfgTables (
if (S3Context != NULL) {
Status = TransferS3ContextToBootScript (S3Context);
if (EFI_ERROR (Status)) {
- goto UninstallAcpiTables;
+ goto UninstallQemuAcpiTableNotifyProtocol;
}
//
@@ -1266,6 +1281,15 @@ InstallQemuFwCfgTables (
DEBUG ((DEBUG_INFO, "%a: installed %d tables\n", __FUNCTION__, Installed));
+UninstallQemuAcpiTableNotifyProtocol:
+ if (EFI_ERROR (Status)) {
+ gBS->UninstallProtocolInterface (
+ QemuAcpiHandle,
+ &gQemuAcpiTableNotifyProtocolGuid,
+ NULL
+ );
+ }
+
UninstallAcpiTables:
if (EFI_ERROR (Status)) {
//
@@ -1275,18 +1299,6 @@ UninstallAcpiTables:
--Installed;
AcpiProtocol->UninstallAcpiTable (AcpiProtocol, InstalledKey[Installed]);
}
- } else {
- //
- // Install a protocol to notify that the ACPI table provided by Qemu is
- // ready.
- //
- QemuAcpiHandle = NULL;
- gBS->InstallProtocolInterface (
- &QemuAcpiHandle,
- &gQemuAcpiTableNotifyProtocolGuid,
- EFI_NATIVE_INTERFACE,
- NULL
- );
}
for (SeenPointerEntry = OrderedCollectionMin (SeenPointers);
--
2.29.2.windows.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [edk2-devel] [PATCH V3 5/6] OvmfPkg/AcpiPlatformDxe: Refactor QemuAcpiTableNotifyProtocol
2023-01-11 1:22 ` [PATCH V3 5/6] OvmfPkg/AcpiPlatformDxe: Refactor QemuAcpiTableNotifyProtocol Min Xu
@ 2023-01-11 16:53 ` Laszlo Ersek
0 siblings, 0 replies; 13+ messages in thread
From: Laszlo Ersek @ 2023-01-11 16:53 UTC (permalink / raw)
To: devel, min.m.xu
Cc: Erdem Aktas, James Bottomley, Jiewen Yao, Gerd Hoffmann,
Tom Lendacky
On 1/11/23 02:22, Min Xu wrote:
> From: Min M Xu <min.m.xu@intel.com>
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4237
>
> Commit 9fdc70af6ba8 install the QemuAcpiTableNotifyProtocol at a
> wrong positioin. It should be called before TransferS3ContextToBootScript
> because TransferS3ContextToBootScript is the last operation in
> InstallQemuFwCfgTables(). Another error is that we should check the
> returned value after installing the QemuAcpiTableNotifyProtocol.
>
> This patch refactors the installation and error handling of
> QemuAcpiTableNotifyProtocol in InstallQemuFwCfgTables ().
>
> 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>
> Reported-by: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>
> ---
> OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c | 38 ++++++++++++++++---------
> 1 file changed, 25 insertions(+), 13 deletions(-)
>
> diff --git a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
> index 462921466604..f0d81d6fd73d 100644
> --- a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
> +++ b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
> @@ -1247,6 +1247,21 @@ InstallQemuFwCfgTables (
> }
> }
>
> + //
> + // Install a protocol to notify that the ACPI table provided by Qemu is
> + // ready.
> + //
> + QemuAcpiHandle = NULL;
> + 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
> @@ -1255,7 +1270,7 @@ InstallQemuFwCfgTables (
> if (S3Context != NULL) {
> Status = TransferS3ContextToBootScript (S3Context);
> if (EFI_ERROR (Status)) {
> - goto UninstallAcpiTables;
> + goto UninstallQemuAcpiTableNotifyProtocol;
> }
>
> //
> @@ -1266,6 +1281,15 @@ InstallQemuFwCfgTables (
>
> DEBUG ((DEBUG_INFO, "%a: installed %d tables\n", __FUNCTION__, Installed));
>
> +UninstallQemuAcpiTableNotifyProtocol:
> + if (EFI_ERROR (Status)) {
> + gBS->UninstallProtocolInterface (
> + QemuAcpiHandle,
> + &gQemuAcpiTableNotifyProtocolGuid,
> + NULL
> + );
> + }
> +
> UninstallAcpiTables:
> if (EFI_ERROR (Status)) {
> //
> @@ -1275,18 +1299,6 @@ UninstallAcpiTables:
> --Installed;
> AcpiProtocol->UninstallAcpiTable (AcpiProtocol, InstalledKey[Installed]);
> }
> - } else {
> - //
> - // Install a protocol to notify that the ACPI table provided by Qemu is
> - // ready.
> - //
> - QemuAcpiHandle = NULL;
> - gBS->InstallProtocolInterface (
> - &QemuAcpiHandle,
> - &gQemuAcpiTableNotifyProtocolGuid,
> - EFI_NATIVE_INTERFACE,
> - NULL
> - );
> }
>
> for (SeenPointerEntry = OrderedCollectionMin (SeenPointers);
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH V3 6/6] OvmfPkg/AcpiPlatformDxe: Return error if installing NotifyProtocol failed
2023-01-11 1:22 [PATCH V3 0/6] Refactor installation of gQemuAcpiTableNotifyProtocol Min Xu
` (4 preceding siblings ...)
2023-01-11 1:22 ` [PATCH V3 5/6] OvmfPkg/AcpiPlatformDxe: Refactor QemuAcpiTableNotifyProtocol Min Xu
@ 2023-01-11 1:22 ` Min Xu
2023-01-12 10:04 ` Boeuf, Sebastien
2023-01-15 11:50 ` [edk2-devel] [PATCH V3 0/6] Refactor installation of gQemuAcpiTableNotifyProtocol Laszlo Ersek
6 siblings, 1 reply; 13+ messages in thread
From: Min Xu @ 2023-01-11 1:22 UTC (permalink / raw)
To: devel
Cc: Min M Xu, 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
Installation of gQemuAcpiTableNotifyProtocol may fail. The error code
should be returned so that the caller can handle it.
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>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c b/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
index 8f90ea23996d..d56eb074a987 100644
--- a/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
+++ b/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
@@ -89,12 +89,16 @@ InstallCloudHvTablesTdx (
// ready.
//
ChAcpiHandle = NULL;
- gBS->InstallProtocolInterface (
- &ChAcpiHandle,
- &gQemuAcpiTableNotifyProtocolGuid,
- EFI_NATIVE_INTERFACE,
- NULL
- );
+ Status = gBS->InstallProtocolInterface (
+ &ChAcpiHandle,
+ &gQemuAcpiTableNotifyProtocolGuid,
+ EFI_NATIVE_INTERFACE,
+ NULL
+ );
+ if (EFI_ERROR (Status)) {
+ ASSERT_EFI_ERROR (Status);
+ return Status;
+ }
return EFI_SUCCESS;
}
--
2.29.2.windows.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH V3 6/6] OvmfPkg/AcpiPlatformDxe: Return error if installing NotifyProtocol failed
2023-01-11 1:22 ` [PATCH V3 6/6] OvmfPkg/AcpiPlatformDxe: Return error if installing NotifyProtocol failed Min Xu
@ 2023-01-12 10:04 ` Boeuf, Sebastien
0 siblings, 0 replies; 13+ messages in thread
From: Boeuf, Sebastien @ 2023-01-12 10:04 UTC (permalink / raw)
To: devel@edk2.groups.io, Xu, Min M
Cc: kraxel@redhat.com, jejb@linux.ibm.com, Yao, Jiewen,
thomas.lendacky@amd.com, Aktas, Erdem
On Wed, 2023-01-11 at 09:22 +0800, Min Xu wrote:
> From: Min M Xu <min.m.xu@intel.com>
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4237
>
> Installation of gQemuAcpiTableNotifyProtocol may fail. The error code
> should be returned so that the caller can handle it.
>
> 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>
> Signed-off-by: Min Xu <min.m.xu@intel.com>
> ---
> OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
> b/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
> index 8f90ea23996d..d56eb074a987 100644
> --- a/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
> +++ b/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
> @@ -89,12 +89,16 @@ InstallCloudHvTablesTdx (
> // ready.
> //
> ChAcpiHandle = NULL;
> - gBS->InstallProtocolInterface (
> - &ChAcpiHandle,
> - &gQemuAcpiTableNotifyProtocolGuid,
> - EFI_NATIVE_INTERFACE,
> - NULL
> - );
> + Status = gBS->InstallProtocolInterface (
> + &ChAcpiHandle,
> + &gQemuAcpiTableNotifyProtocolGuid,
> + EFI_NATIVE_INTERFACE,
> + NULL
> + );
> + if (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + return Status;
> + }
>
> return EFI_SUCCESS;
> }
Looks good and I've tested Cloud Hypervisor with both CloudHvX64 and
IntelTdxX64 targets.
Reviewed-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris,
92196 Meudon Cedex, France
Registration Number: 302 456 199 R.C.S. NANTERRE
Capital: 5 208 026.16 Euros
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-devel] [PATCH V3 0/6] Refactor installation of gQemuAcpiTableNotifyProtocol
2023-01-11 1:22 [PATCH V3 0/6] Refactor installation of gQemuAcpiTableNotifyProtocol Min Xu
` (5 preceding siblings ...)
2023-01-11 1:22 ` [PATCH V3 6/6] OvmfPkg/AcpiPlatformDxe: Return error if installing NotifyProtocol failed Min Xu
@ 2023-01-15 11:50 ` Laszlo Ersek
6 siblings, 0 replies; 13+ messages in thread
From: Laszlo Ersek @ 2023-01-15 11:50 UTC (permalink / raw)
To: devel, min.m.xu
Cc: Erdem Aktas, James Bottomley, Jiewen Yao, Gerd Hoffmann,
Tom Lendacky, Sebastien Boeuf
On 1/11/23 02:22, Min Xu wrote:
> 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.
>
> This patch-set contains below patches to fix above issues.
> #1: Remove QEMU_ACPI_TABLE_NOTIFY_PROTOCOL and delete
> QemuAcpiTableNotify.h.
> #2: Use local variable of ChAcpiHandle to store the handle.
> #3: Use local variable of QemuAcpiHandle to store the handle.
> #4: Add log to show how many ACPI tables installed.
> #5: Refactor the installation of QemuAcpiTableNotifyProtocol and
> its error handling.
> #6: Return error code if QemuAcpiTableNotifyProtocol is failed to be
> installed so that the caller can handle the error.
>
> Code: https://github.com/mxu9/edk2/tree/AcpiPlatformDxe.v3
>
> v3 changes:
> - Set QemuAcpiHandle to NULL right before installing the
> gQemuAcpiTableNotifyProtocol.
>
> v2 changes:
> - Split v1 patch into 6 separate patches.
>
> 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>
>
> Min M Xu (6):
> OvmfPkg/AcpiPlatformDxe: Remove QEMU_ACPI_TABLE_NOTIFY_PROTOCOL
> OvmfPkg/AcpiPlatformDxe: Use local variable in CloudHvAcpi.c
> OvmfPkg/AcpiPlatformDxe: Use local variable in QemuFwCfgAcpi.c
> OvmfPkg/AcpiPlatformDxe: Add log to show the installed tables
> OvmfPkg/AcpiPlatformDxe: Refactor QemuAcpiTableNotifyProtocol
> OvmfPkg/AcpiPlatformDxe: Return error if installing NotifyProtocol
> failed
>
> OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c | 23 +++++-----
> OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c | 43 ++++++++++++-------
> .../Include/Protocol/QemuAcpiTableNotify.h | 27 ------------
> 3 files changed, 40 insertions(+), 53 deletions(-)
> delete mode 100644 OvmfPkg/Include/Protocol/QemuAcpiTableNotify.h
>
https://github.com/tianocore/edk2/pull/3897
Commit range ba08910df107..7cd55f300915.
Thanks.
^ permalink raw reply [flat|nested] 13+ messages in thread