Re: [PATCH V3 1/6] OvmfPkg/AcpiPlatformDxe: Remove QEMU_ACPI_TABLE_NOTIFY_PROTOCOL

["Boeuf, Sebastien" <sebastien.boeuf@intel.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.