Re: [PATCH V3 2/6] IntelSiliconPkg IntelVTdDxe: Use new EfiLocateFirstAcpiTable()

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

Reviewed-by: Jiewen.yao@intel.com

> -----Original Message-----
> From: Zeng, Star
> Sent: Monday, September 17, 2018 5:08 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>; Younas khan
> <pmdyounaskhan786@gmail.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Yao,
> Jiewen <jiewen.yao@intel.com>
> Subject: [PATCH V3 2/6] IntelSiliconPkg IntelVTdDxe: Use new
> EfiLocateFirstAcpiTable()
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=967
> Request to add a library function for GetAcpiTable() in order
> to get ACPI table using signature as input.
> 
> After evaluation, we found there are many duplicated code to
> find ACPI table by signature in different modules.
> 
> This patch updates IntelVTdDxe to use new
> EfiLocateFirstAcpiTable() and remove the duplicated code.
> 
> Cc: Younas khan <pmdyounaskhan786@gmail.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> 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/DmarAcpiTable.c        | 136
> +--------------------
>  1 file changed, 3 insertions(+), 133 deletions(-)
> 
> diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c
> b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c
> index 24723fe4972a..92b09f985533 100644
> --- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c
> +++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c
> @@ -868,116 +868,6 @@ ParseDmarAcpiTableRmrr (
>  }
> 
>  /**
> -  This function scan ACPI table in RSDT.
> -
> -  @param[in]  Rsdt      ACPI RSDT
> -  @param[in]  Signature ACPI table signature
> -
> -  @return ACPI table
> -**/
> -VOID *
> -ScanTableInRSDT (
> -  IN RSDT_TABLE                   *Rsdt,
> -  IN UINT32                       Signature
> -  )
> -{
> -  UINTN                         Index;
> -  UINT32                        EntryCount;
> -  UINT32                        *EntryPtr;
> -  EFI_ACPI_DESCRIPTION_HEADER   *Table;
> -
> -  EntryCount = (Rsdt->Header.Length - sizeof
> (EFI_ACPI_DESCRIPTION_HEADER)) / sizeof(UINT32);
> -
> -  EntryPtr = &Rsdt->Entry;
> -  for (Index = 0; Index < EntryCount; Index ++, EntryPtr ++) {
> -    Table = (EFI_ACPI_DESCRIPTION_HEADER*)((UINTN)(*EntryPtr));
> -    if ((Table != NULL) && (Table->Signature == Signature)) {
> -      return Table;
> -    }
> -  }
> -
> -  return NULL;
> -}
> -
> -/**
> -  This function scan ACPI table in XSDT.
> -
> -  @param[in]  Xsdt      ACPI XSDT
> -  @param[in]  Signature ACPI table signature
> -
> -  @return ACPI table
> -**/
> -VOID *
> -ScanTableInXSDT (
> -  IN XSDT_TABLE                   *Xsdt,
> -  IN UINT32                       Signature
> -  )
> -{
> -  UINTN                        Index;
> -  UINT32                       EntryCount;
> -  UINT64                       EntryPtr;
> -  UINTN                        BasePtr;
> -  EFI_ACPI_DESCRIPTION_HEADER  *Table;
> -
> -  EntryCount = (Xsdt->Header.Length - sizeof
> (EFI_ACPI_DESCRIPTION_HEADER)) / sizeof(UINT64);
> -
> -  BasePtr = (UINTN)(&(Xsdt->Entry));
> -  for (Index = 0; Index < EntryCount; Index ++) {
> -    CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * sizeof(UINT64)),
> sizeof(UINT64));
> -    Table = (EFI_ACPI_DESCRIPTION_HEADER*)((UINTN)(EntryPtr));
> -    if ((Table != NULL) && (Table->Signature == Signature)) {
> -      return Table;
> -    }
> -  }
> -
> -  return NULL;
> -}
> -
> -/**
> -  This function scan ACPI table in RSDP.
> -
> -  @param[in]  Rsdp      ACPI RSDP
> -  @param[in]  Signature ACPI table signature
> -
> -  @return ACPI table
> -**/
> -VOID *
> -FindAcpiPtr (
> -  IN EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER *Rsdp,
> -  IN UINT32                                       Signature
> -  )
> -{
> -  EFI_ACPI_DESCRIPTION_HEADER                    *AcpiTable;
> -  RSDT_TABLE                                     *Rsdt;
> -  XSDT_TABLE                                     *Xsdt;
> -
> -  AcpiTable = NULL;
> -
> -  //
> -  // Check ACPI2.0 table
> -  //
> -  Rsdt = (RSDT_TABLE *)(UINTN)Rsdp->RsdtAddress;
> -  Xsdt = NULL;
> -  if ((Rsdp->Revision >= 2) && (Rsdp->XsdtAddress < (UINT64)(UINTN)-1)) {
> -    Xsdt = (XSDT_TABLE *)(UINTN)Rsdp->XsdtAddress;
> -  }
> -  //
> -  // Check Xsdt
> -  //
> -  if (Xsdt != NULL) {
> -    AcpiTable = ScanTableInXSDT (Xsdt, Signature);
> -  }
> -  //
> -  // Check Rsdt
> -  //
> -  if ((AcpiTable == NULL) && (Rsdt != NULL)) {
> -    AcpiTable = ScanTableInRSDT (Rsdt, Signature);
> -  }
> -
> -  return AcpiTable;
> -}
> -
> -/**
>    Get the DMAR ACPI table.
> 
>    @retval EFI_SUCCESS           The DMAR ACPI table is got.
> @@ -989,33 +879,13 @@ GetDmarAcpiTable (
>    VOID
>    )
>  {
> -  VOID                              *AcpiTable;
> -  EFI_STATUS                        Status;
> -
>    if (mAcpiDmarTable != NULL) {
>      return EFI_ALREADY_STARTED;
>    }
> 
> -  AcpiTable = NULL;
> -  Status = EfiGetSystemConfigurationTable (
> -             &gEfiAcpi20TableGuid,
> -             &AcpiTable
> -             );
> -  if (EFI_ERROR (Status)) {
> -    Status = EfiGetSystemConfigurationTable (
> -               &gEfiAcpi10TableGuid,
> -               &AcpiTable
> -               );
> -  }
> -  if (EFI_ERROR (Status)) {
> -    return EFI_NOT_FOUND;
> -  }
> -  ASSERT (AcpiTable != NULL);
> -
> -  mAcpiDmarTable = FindAcpiPtr (
> -
> (EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER *)AcpiTable,
> -
> EFI_ACPI_4_0_DMA_REMAPPING_TABLE_SIGNATURE
> -                      );
> +  mAcpiDmarTable = (EFI_ACPI_DMAR_HEADER *) EfiLocateFirstAcpiTable
> (
> +
> EFI_ACPI_4_0_DMA_REMAPPING_TABLE_SIGNATURE
> +                                              );
>    if (mAcpiDmarTable == NULL) {
>      return EFI_NOT_FOUND;
>    }
> --
> 2.7.0.windows.1