Re: [PATCH] IntelSiliconPkg IntelVTdDxe: Use ACPI table event to get DMAR table

["Yao, Jiewen" <jiewen.yao@intel.com>]

Looks good to remove the dependency.

Reviewed-by: Jiewen.yao@intel.com

> -----Original Message-----
> From: Zeng, Star
> Sent: Friday, November 3, 2017 1:40 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: [PATCH] IntelSiliconPkg IntelVTdDxe: Use ACPI table event to get DMAR
> table
> 
> Use ACPI table event to get DMAR table instead of using ACPI SDT
> notification as ACPI SDT is optional and the default value of
> PcdInstallAcpiSdtProtocol is FALSE in MdeModulePkg.dec.
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
>  .../Feature/VTd/IntelVTdDxe/DmaProtection.c        | 55
> +++++++++++++---------
>  .../Feature/VTd/IntelVTdDxe/DmaProtection.h        |  1 -
>  .../Feature/VTd/IntelVTdDxe/DmarAcpiTable.c        |  8 +++-
>  .../Feature/VTd/IntelVTdDxe/IntelVTdDxe.inf        | 12 +++--
>  4 files changed, 46 insertions(+), 30 deletions(-)
> 
> diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.c
> b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.c
> index 4a4d82e7f106..6052a0aebe45 100644
> --- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.c
> +++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.c
> @@ -13,7 +13,6 @@
> 
>  #include "DmaProtection.h"
> 
> -EFI_ACPI_SDT_PROTOCOL                   *mAcpiSdt;
>  UINT64                                  mBelow4GMemoryLimit;
>  UINT64                                  mAbove4GMemoryLimit;
> 
> @@ -350,11 +349,6 @@ SetupVtd (
>    //
>    // 1. setup
>    //
> -  DEBUG ((DEBUG_INFO, "GetDmarAcpiTable\n"));
> -  Status = GetDmarAcpiTable ();
> -  if (EFI_ERROR (Status)) {
> -    return;
> -  }
>    DEBUG ((DEBUG_INFO, "ParseDmarAcpiTable\n"));
>    Status = ParseDmarAcpiTableDrhd ();
>    if (EFI_ERROR (Status)) {
> @@ -399,27 +393,29 @@ SetupVtd (
>  }
> 
>  /**
> -  ACPI notification function.
> +  Notification function of ACPI Table change.
> +
> +  This is a notification function registered on ACPI Table change event.
> 
> -  @param[in] Table    A pointer to the ACPI table header.
> -  @param[in] Version  The ACPI table's version.
> -  @param[in] TableKey The table key for this ACPI table.
> +  @param  Event        Event whose notification function is being invoked.
> +  @param  Context      Pointer to the notification function's context.
> 
> -  @retval EFI_SUCCESS The notification function is executed.
>  **/
> -EFI_STATUS
> +VOID
>  EFIAPI
>  AcpiNotificationFunc (
> -  IN EFI_ACPI_SDT_HEADER    *Table,
> -  IN EFI_ACPI_TABLE_VERSION Version,
> -  IN UINTN                  TableKey
> +  IN EFI_EVENT        Event,
> +  IN VOID             *Context
>    )
>  {
> -  if (Table->Signature == EFI_ACPI_4_0_DMA_REMAPPING_TABLE_SIGNATURE)
> {
> -    DEBUG((DEBUG_INFO, "Vtd AcpiNotificationFunc\n"));
> -    SetupVtd ();
> +  EFI_STATUS          Status;
> +
> +  Status = GetDmarAcpiTable ();
> +  if (EFI_ERROR (Status)) {
> +    return;
>    }
> -  return EFI_SUCCESS;
> +  SetupVtd ();
> +  gBS->CloseEvent (Event);
>  }
> 
>  /**
> @@ -474,11 +470,26 @@ InitializeDmaProtection (
>    EFI_STATUS  Status;
>    EFI_EVENT   ExitBootServicesEvent;
>    EFI_EVENT   LegacyBootEvent;
> -
> -  Status = gBS->LocateProtocol (&gEfiAcpiSdtProtocolGuid, NULL, (VOID **)
> &mAcpiSdt);
> +  EFI_EVENT   Event;
> +
> +  Status = gBS->CreateEventEx (
> +                  EVT_NOTIFY_SIGNAL,
> +                  TPL_CALLBACK,
> +                  AcpiNotificationFunc,
> +                  NULL,
> +                  &gEfiAcpi10TableGuid,
> +                  &Event
> +                  );
>    ASSERT_EFI_ERROR (Status);
> 
> -  Status = mAcpiSdt->RegisterNotify (TRUE, AcpiNotificationFunc);
> +  Status = gBS->CreateEventEx (
> +                  EVT_NOTIFY_SIGNAL,
> +                  TPL_CALLBACK,
> +                  AcpiNotificationFunc,
> +                  NULL,
> +                  &gEfiAcpi20TableGuid,
> +                  &Event
> +                  );
>    ASSERT_EFI_ERROR (Status);
> 
>    Status = gBS->CreateEventEx (
> diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.h
> b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.h
> index f7b5292f23e8..0886647ea673 100644
> --- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.h
> +++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.h
> @@ -36,7 +36,6 @@
>  #include <Protocol/PciRootBridgeIo.h>
>  #include <Protocol/PciIo.h>
>  #include <Protocol/PciEnumerationComplete.h>
> -#include <Protocol/AcpiSystemDescriptionTable.h>
>  #include <Protocol/PlatformVtdPolicy.h>
>  #include <Protocol/IoMmu.h>
> 
> diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c
> b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c
> index 39b70a134e1e..81dec109675b 100644
> --- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c
> +++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c
> @@ -989,6 +989,10 @@ GetDmarAcpiTable (
>    VOID                              *AcpiTable;
>    EFI_STATUS                        Status;
> 
> +  if (mAcpiDmarTable != NULL) {
> +    return EFI_SUCCESS;
> +  }
> +
>    AcpiTable = NULL;
>    Status = EfiGetSystemConfigurationTable (
>               &gEfiAcpi20TableGuid,
> @@ -1006,10 +1010,10 @@ GetDmarAcpiTable (
> 
> (EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER *)AcpiTable,
>                        EFI_ACPI_4_0_DMA_REMAPPING_TABLE_SIGNATURE
>                        );
> -  DEBUG ((DEBUG_INFO,"DMAR Table - 0x%08x\n", mAcpiDmarTable));
>    if (mAcpiDmarTable == NULL) {
> -    return EFI_UNSUPPORTED;
> +    return EFI_NOT_FOUND;
>    }
> +  DEBUG ((DEBUG_INFO,"DMAR Table - 0x%08x\n", mAcpiDmarTable));
>    VtdDumpDmarTable();
> 
>    return EFI_SUCCESS;
> diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.inf
> b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.inf
> index fde33bb224ca..bfb677704069 100644
> --- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.inf
> +++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.inf
> @@ -63,12 +63,15 @@ [LibraryClasses]
> 
>  [Guids]
>    gEfiEventExitBootServicesGuid   ## CONSUMES ## Event
> -  gEfiAcpi20TableGuid             ## CONSUMES ## SystemTable
> -  gEfiAcpi10TableGuid             ## CONSUMES ## SystemTable
> +  ## CONSUMES ## SystemTable
> +  ## CONSUMES ## Event
> +  gEfiAcpi20TableGuid
> +  ## CONSUMES ## SystemTable
> +  ## CONSUMES ## Event
> +  gEfiAcpi10TableGuid
> 
>  [Protocols]
>    gEdkiiIoMmuProtocolGuid                     ## PRODUCES
> -  gEfiAcpiSdtProtocolGuid                     ## CONSUMES
>    gEfiPciIoProtocolGuid                       ## CONSUMES
>    gEfiPciEnumerationCompleteProtocolGuid      ## CONSUMES
>    gEdkiiPlatformVTdPolicyProtocolGuid         ## SOMETIMES_CONSUMES
> @@ -77,8 +80,7 @@ [Pcd]
>    gIntelSiliconPkgTokenSpaceGuid.PcdVTdPolicyPropertyMask   ##
> CONSUMES
> 
>  [Depex]
> -  gEfiPciRootBridgeIoProtocolGuid AND
> -  gEfiAcpiSdtProtocolGuid
> +  gEfiPciRootBridgeIoProtocolGuid
> 
>  [UserExtensions.TianoCore."ExtraFiles"]
>    IntelVTdDxeExtra.uni
> --
> 2.7.0.windows.1